From ce0cc1699f692625ad7ea7be6730ad440ce24745 Mon Sep 17 00:00:00 2001 From: prabathabey Date: Sun, 23 Aug 2015 20:19:20 +0530 Subject: [PATCH] Fixing more transaction handling related issues --- .../core/dao/DeviceManagementDAOFactory.java | 4 +- .../operation/mgt/OperationManagerImpl.java | 138 +++++++----------- .../dao/OperationManagementDAOFactory.java | 41 +++--- .../mgt/dao/impl/PolicyOperationDAOImpl.java | 3 +- .../mgt/dao/impl/ProfileOperationDAOImpl.java | 4 +- 5 files changed, 76 insertions(+), 114 deletions(-) diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/dao/DeviceManagementDAOFactory.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/dao/DeviceManagementDAOFactory.java index be15bd9551b..7c86387ee46 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/dao/DeviceManagementDAOFactory.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/dao/DeviceManagementDAOFactory.java @@ -87,7 +87,7 @@ public class DeviceManagementDAOFactory { return currentConnection.get(); } - public static void commitTransaction() throws TransactionManagementException { + public static void commitTransaction() { try { Connection conn = currentConnection.get(); if (conn != null) { @@ -99,7 +99,7 @@ public class DeviceManagementDAOFactory { } } } catch (SQLException e) { - throw new TransactionManagementException("Error occurred while committing the transaction", e); + log.error("Error occurred while committing the transaction", e); } } diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/OperationManagerImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/OperationManagerImpl.java index a5d1285a92e..3039d0d7651 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/OperationManagerImpl.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/OperationManagerImpl.java @@ -23,6 +23,7 @@ import org.apache.commons.logging.LogFactory; import org.wso2.carbon.context.CarbonContext; import org.wso2.carbon.device.mgt.common.DeviceIdentifier; import org.wso2.carbon.device.mgt.common.EnrolmentInfo; +import org.wso2.carbon.device.mgt.common.TransactionManagementException; import org.wso2.carbon.device.mgt.common.operation.mgt.Operation; import org.wso2.carbon.device.mgt.common.operation.mgt.OperationManagementException; import org.wso2.carbon.device.mgt.common.operation.mgt.OperationManager; @@ -36,6 +37,7 @@ import org.wso2.carbon.device.mgt.core.operation.mgt.dao.OperationMappingDAO; import org.wso2.carbon.device.mgt.core.operation.mgt.dao.util.OperationDAOUtil; import org.wso2.carbon.device.mgt.core.operation.mgt.util.OperationCreateTimeComparator; +import java.sql.SQLException; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -101,19 +103,15 @@ public class OperationManagerImpl implements OperationManager { OperationManagementDAOFactory.commitTransaction(); return operationId; } catch (OperationManagementDAOException e) { - try { - OperationManagementDAOFactory.rollbackTransaction(); - } catch (OperationManagementDAOException e1) { - log.warn("Error occurred while roll-backing the transaction", e1); - } + OperationManagementDAOFactory.rollbackTransaction(); throw new OperationManagementException("Error occurred while adding operation", e); } catch (DeviceManagementDAOException e) { - try { - OperationManagementDAOFactory.rollbackTransaction(); - } catch (OperationManagementDAOException e1) { - log.warn("Error occurred while roll-backing the transaction", e1); - } + OperationManagementDAOFactory.rollbackTransaction(); throw new OperationManagementException("Error occurred while retrieving device metadata", e); + } catch (TransactionManagementException e) { + throw new OperationManagementException("Error occurred while initiating the transaction", e); + } finally { + OperationManagementDAOFactory.closeConnection(); } } @@ -122,7 +120,7 @@ public class OperationManagerImpl implements OperationManager { int enrolmentId; List operations = new ArrayList<>(); try { - OperationManagementDAOFactory.getConnection(); + OperationManagementDAOFactory.openConnection(); int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); enrolmentId = deviceDAO.getEnrolmentByStatus(deviceId, EnrolmentInfo.Status.ACTIVE, tenantId); @@ -145,12 +143,10 @@ public class OperationManagerImpl implements OperationManager { } catch (DeviceManagementDAOException e) { throw new OperationManagementException("Error occurred while retrieving metadata of '" + deviceId.getType() + "' device carrying the identifier '" + deviceId.getId() + "'"); + } catch (SQLException e) { + throw new OperationManagementException("Error occurred while opening a connection to the data source", e); } finally { - try { - OperationManagementDAOFactory.closeConnection(); - } catch (OperationManagementDAOException e) { - log.warn("Error occurred while closing data source connection", e); - } + OperationManagementDAOFactory.closeConnection(); } } @@ -165,7 +161,7 @@ public class OperationManagerImpl implements OperationManager { List operations = new ArrayList<>(); List dtoOperationList = new ArrayList<>(); try { - OperationManagementDAOFactory.getConnection(); + OperationManagementDAOFactory.openConnection(); int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); enrolmentId = deviceDAO.getEnrolmentByStatus(deviceId, EnrolmentInfo.Status.ACTIVE, tenantId); @@ -202,12 +198,10 @@ public class OperationManagerImpl implements OperationManager { throw new OperationManagementException("Error occurred while retrieving the device " + "for device Identifier type -'" + deviceId.getType() + "' and device Id '" + deviceId.getId() + "'", e); + } catch (SQLException e) { + throw new OperationManagementException("Error occurred while opening a connection to the data source", e); } finally { - try { - OperationManagementDAOFactory.closeConnection(); - } catch (OperationManagementDAOException e) { - log.warn("Error occurred while closing data source connection", e); - } + OperationManagementDAOFactory.closeConnection(); } } @@ -220,7 +214,7 @@ public class OperationManagerImpl implements OperationManager { Operation operation = null; int enrolmentId; try { - OperationManagementDAOFactory.getConnection(); + OperationManagementDAOFactory.openConnection(); int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); enrolmentId = deviceDAO.getEnrolmentByStatus(deviceId, EnrolmentInfo.Status.ACTIVE, tenantId); @@ -258,23 +252,19 @@ public class OperationManagerImpl implements OperationManager { } catch (DeviceManagementDAOException e) { throw new OperationManagementException("Error occurred while retrieving the device " + "for device Identifier type -'" + deviceId.getType() + "' and device Id '" + deviceId.getId(), e); + } catch (SQLException e) { + throw new OperationManagementException("Error occurred while opening a connection to the data source", e); } finally { - try { - OperationManagementDAOFactory.closeConnection(); - } catch (OperationManagementDAOException e) { - log.warn("Error occurred while closing data source connection", e); - } + OperationManagementDAOFactory.closeConnection(); } } @Override public void updateOperation(DeviceIdentifier deviceId, Operation operation) throws OperationManagementException { int operationId = operation.getId(); - if (log.isDebugEnabled()) { log.debug("operation Id:" + operationId + " status:" + operation.getStatus()); } - try { OperationManagementDAOFactory.beginTransaction(); @@ -282,65 +272,49 @@ public class OperationManagerImpl implements OperationManager { int enrolmentId = deviceDAO.getEnrolmentByStatus(deviceId, EnrolmentInfo.Status.ACTIVE, tenantId); if (operation.getStatus() != null) { - OperationManagementDAOFactory.beginTransaction(); operationDAO.updateOperationStatus(enrolmentId, operationId, org.wso2.carbon.device.mgt.core.dto.operation.mgt.Operation.Status .valueOf(operation.getStatus().toString())); } - - if (operation.getOperationResponse() != null) { - OperationManagementDAOFactory.beginTransaction(); operationDAO.addOperationResponse(enrolmentId, operationId, operation.getOperationResponse()); - OperationManagementDAOFactory.commitTransaction(); } + OperationManagementDAOFactory.commitTransaction(); } catch (OperationManagementDAOException e) { - try { - OperationManagementDAOFactory.rollbackTransaction(); - } catch (OperationManagementDAOException e1) { - log.warn("Error occurred while roll-backing the update operation transaction", e1); - } + OperationManagementDAOFactory.rollbackTransaction(); throw new OperationManagementException("Error occurred while updating the operation: " + operationId + " status:" + operation.getStatus(), e); } catch (DeviceManagementDAOException e) { - try { - OperationManagementDAOFactory.rollbackTransaction(); - } catch (OperationManagementDAOException e1) { - log.warn("Error occurred while roll-backing the update operation transaction", e1); - } + OperationManagementDAOFactory.rollbackTransaction(); throw new OperationManagementException("Error occurred while fetching the device for device identifier: " + deviceId.getId() + "type:" + deviceId.getType(), e); + } catch (TransactionManagementException e) { + throw new OperationManagementException("Error occurred while initiating a transaction", e); } finally { - try { - OperationManagementDAOFactory.closeConnection(); - } catch (OperationManagementDAOException e) { - log.warn("Error occurred while closing data source connection", e); - } + OperationManagementDAOFactory.closeConnection(); } } @Override public void deleteOperation(int operationId) throws OperationManagementException { - try { OperationManagementDAOFactory.beginTransaction(); - org.wso2.carbon.device.mgt.core.dto.operation.mgt.Operation operation = operationDAO.getOperation - (operationId); + org.wso2.carbon.device.mgt.core.dto.operation.mgt.Operation operation = + operationDAO.getOperation(operationId); if (operation == null) { throw new OperationManagementException("Operation not found for operation id:" + operationId); } - lookupOperationDAO(operation).deleteOperation(operationId); - OperationManagementDAOFactory.commitTransaction(); + OperationManagementDAOFactory.commitTransaction(); } catch (OperationManagementDAOException e) { - try { - OperationManagementDAOFactory.rollbackTransaction(); - } catch (OperationManagementDAOException e1) { - log.warn("Error occurred while roll-backing the delete operation transaction", e1); - } + OperationManagementDAOFactory.rollbackTransaction(); throw new OperationManagementException("Error occurred while deleting the operation: " + operationId, e); + } catch (TransactionManagementException e) { + throw new OperationManagementException("Error occurred while initiating a transaction", e); + } finally { + OperationManagementDAOFactory.closeConnection(); } } @@ -349,13 +323,12 @@ public class OperationManagerImpl implements OperationManager { throws OperationManagementException { int enrolmentId; Operation operation; - if (log.isDebugEnabled()) { log.debug("Operation Id:" + operationId + " Device Type:" + deviceId.getType() + " Device Identifier:" + deviceId.getId()); } try { - OperationManagementDAOFactory.getConnection(); + OperationManagementDAOFactory.openConnection(); int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); enrolmentId = deviceDAO.getEnrolmentByStatus(deviceId, EnrolmentInfo.Status.ACTIVE, tenantId); @@ -376,12 +349,11 @@ public class OperationManagerImpl implements OperationManager { } else if (dtoOperation.getType() .equals(org.wso2.carbon.device.mgt.core.dto.operation.mgt.Operation.Type.CONFIG)) { dtoOperation = configOperationDAO.getOperation(dtoOperation.getId()); - } else if (dtoOperation.getType().equals(org.wso2.carbon.device.mgt.core.dto.operation.mgt.Operation.Type - .PROFILE)) { + } else if (dtoOperation.getType().equals( + org.wso2.carbon.device.mgt.core.dto.operation.mgt.Operation.Type.PROFILE)) { dtoOperation = profileOperationDAO.getOperation(dtoOperation.getId()); - } else if (dtoOperation.getType().equals(org.wso2.carbon.device.mgt.core.dto.operation.mgt.Operation.Type - .POLICY)) { - + } else if (dtoOperation.getType().equals( + org.wso2.carbon.device.mgt.core.dto.operation.mgt.Operation.Type.POLICY)) { dtoOperation = policyOperationDAO.getOperation(dtoOperation.getId()); } @@ -398,12 +370,10 @@ public class OperationManagerImpl implements OperationManager { throw new OperationManagementException("Error occurred while retrieving the device " + "for device Identifier type -'" + deviceId.getType() + "' and device Id '" + deviceId.getId() + "'", e); + } catch (SQLException e) { + throw new OperationManagementException("Error occurred while opening connection to the data source", e); } finally { - try { - OperationManagementDAOFactory.closeConnection(); - } catch (OperationManagementDAOException e) { - log.warn("Error occurred while closing data source connection", e); - } + OperationManagementDAOFactory.closeConnection(); } return operation; } @@ -415,7 +385,7 @@ public class OperationManagerImpl implements OperationManager { List dtoOperationList = new ArrayList<>(); try { - OperationManagementDAOFactory.getConnection(); + OperationManagementDAOFactory.openConnection(); int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); int enrolmentId = deviceDAO.getEnrolmentByStatus(deviceId, EnrolmentInfo.Status.ACTIVE, tenantId); @@ -455,18 +425,15 @@ public class OperationManagerImpl implements OperationManager { } catch (DeviceManagementDAOException e) { throw new OperationManagementException("Error occurred while retrieving the device " + "for device Identifier type -'" + deviceId.getType() + "' and device Id '" + deviceId.getId(), e); + } catch (SQLException e) { + throw new OperationManagementException("Error occurred while opening a connection to the data source", e); } finally { - try { - OperationManagementDAOFactory.closeConnection(); - } catch (OperationManagementDAOException e) { - log.warn("Error occurred while closing data source connection", e); - } + OperationManagementDAOFactory.closeConnection(); } } @Override public Operation getOperation(int operationId) throws OperationManagementException { - Operation operation; try { OperationManagementDAOFactory.getConnection(); @@ -480,8 +447,9 @@ public class OperationManagerImpl implements OperationManager { if (dtoOperation.getType() .equals(org.wso2.carbon.device.mgt.core.dto.operation.mgt.Operation.Type.COMMAND)) { org.wso2.carbon.device.mgt.core.dto.operation.mgt.CommandOperation commandOperation; - commandOperation = (org.wso2.carbon.device.mgt.core.dto.operation.mgt.CommandOperation) commandOperationDAO - .getOperation(dtoOperation.getId()); + commandOperation = + (org.wso2.carbon.device.mgt.core.dto.operation.mgt.CommandOperation) commandOperationDAO. + getOperation(dtoOperation.getId()); dtoOperation.setEnabled(commandOperation.isEnabled()); } else if (dtoOperation.getType() .equals(org.wso2.carbon.device.mgt.core.dto.operation.mgt.Operation.Type.CONFIG)) { @@ -499,12 +467,10 @@ public class OperationManagerImpl implements OperationManager { String errorMsg = "Error occurred while retrieving the operation with operation Id '" + operationId; log.error(errorMsg, e); throw new OperationManagementException(errorMsg, e); + } catch (SQLException e) { + throw new OperationManagementException("Error occurred while opening a connection to the data source", e); } finally { - try { - OperationManagementDAOFactory.closeConnection(); - } catch (OperationManagementDAOException e) { - log.warn("Error occurred while closing data source connection", e); - } + OperationManagementDAOFactory.closeConnection(); } return operation; } diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/dao/OperationManagementDAOFactory.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/dao/OperationManagementDAOFactory.java index 68aa4b0608d..6ed2e443ad5 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/dao/OperationManagementDAOFactory.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/dao/OperationManagementDAOFactory.java @@ -20,6 +20,7 @@ package org.wso2.carbon.device.mgt.core.operation.mgt.dao; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.wso2.carbon.device.mgt.common.TransactionManagementException; import org.wso2.carbon.device.mgt.core.config.datasource.DataSourceConfig; import org.wso2.carbon.device.mgt.core.config.datasource.JNDILookupDefinition; import org.wso2.carbon.device.mgt.core.dao.util.DeviceManagementDAOUtil; @@ -69,27 +70,29 @@ public class OperationManagementDAOFactory { dataSource = resolveDataSource(config); } - public static void beginTransaction() throws OperationManagementDAOException { + public static void beginTransaction() throws TransactionManagementException { try { - currentConnection.set(dataSource.getConnection()); + Connection conn = dataSource.getConnection(); + conn.setAutoCommit(false); + currentConnection.set(conn); } catch (SQLException e) { - throw new OperationManagementDAOException( - "Error occurred while retrieving config.datasource connection", e); + throw new TransactionManagementException( + "Error occurred while retrieving config.datasource connection", e); } } - public static Connection getConnection() throws OperationManagementDAOException { + public static void openConnection() throws SQLException { + currentConnection.set(dataSource.getConnection()); + } + + public static Connection getConnection() throws SQLException { if (currentConnection.get() == null) { - try { - currentConnection.set(dataSource.getConnection()); - } catch (SQLException e) { - throw new OperationManagementDAOException("Error occurred while retrieving data source connection", e); - } + currentConnection.set(dataSource.getConnection()); } return currentConnection.get(); } - public static void closeConnection() throws OperationManagementDAOException { + public static void closeConnection() { Connection con = currentConnection.get(); if (con != null) { try { @@ -101,7 +104,7 @@ public class OperationManagementDAOFactory { } } - public static void commitTransaction() throws OperationManagementDAOException { + public static void commitTransaction() { try { Connection conn = currentConnection.get(); if (conn != null) { @@ -113,13 +116,11 @@ public class OperationManagementDAOFactory { } } } catch (SQLException e) { - throw new OperationManagementDAOException("Error occurred while committing the transaction", e); - } finally { - closeConnection(); + log.error("Error occurred while committing the transaction", e); } } - public static void rollbackTransaction() throws OperationManagementDAOException { + public static void rollbackTransaction() { try { Connection conn = currentConnection.get(); if (conn != null) { @@ -127,13 +128,11 @@ public class OperationManagementDAOFactory { } else { if (log.isDebugEnabled()) { log.debug("Datasource connection associated with the current thread is null, hence rollback " + - "has not been attempted"); + "has not been attempted"); } } } catch (SQLException e) { - throw new OperationManagementDAOException("Error occurred while roll-backing the transaction", e); - } finally { - closeConnection(); + log.error("Error occurred while roll-backing the transaction", e); } } @@ -147,7 +146,7 @@ public class OperationManagementDAOFactory { DataSource dataSource = null; if (config == null) { throw new RuntimeException("Device Management Repository data source configuration is null and " + - "thus, is not initialized"); + "thus, is not initialized"); } JNDILookupDefinition jndiConfig = config.getJndiLookupDefinition(); if (jndiConfig != null) { diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/dao/impl/PolicyOperationDAOImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/dao/impl/PolicyOperationDAOImpl.java index cd4822ced0c..712a2a777f3 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/dao/impl/PolicyOperationDAOImpl.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/dao/impl/PolicyOperationDAOImpl.java @@ -42,10 +42,9 @@ public class PolicyOperationDAOImpl extends OperationDAOImpl { operation.setId(operationId); operation.setEnabled(true); PolicyOperation policyOperation = (PolicyOperation) operation; - Connection conn = OperationManagementDAOFactory.getConnection(); - PreparedStatement stmt = null; try { + Connection conn = OperationManagementDAOFactory.getConnection(); stmt = conn.prepareStatement("INSERT INTO DM_POLICY_OPERATION(OPERATION_ID, OPERATION_DETAILS) " + "VALUES(?, ?)"); stmt.setInt(1, operationId); diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/dao/impl/ProfileOperationDAOImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/dao/impl/ProfileOperationDAOImpl.java index d8cab721c79..53778be7390 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/dao/impl/ProfileOperationDAOImpl.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/dao/impl/ProfileOperationDAOImpl.java @@ -42,11 +42,9 @@ public class ProfileOperationDAOImpl extends OperationDAOImpl { operation.setId(operationId); operation.setEnabled(true); ProfileOperation profileOp = (ProfileOperation) operation; - Connection conn = OperationManagementDAOFactory.getConnection(); - PreparedStatement stmt = null; - try { + Connection conn = OperationManagementDAOFactory.getConnection(); stmt = conn.prepareStatement("INSERT INTO DM_PROFILE_OPERATION(OPERATION_ID, OPERATION_DETAILS) " + "VALUES(?, ?)"); stmt.setInt(1, operationId);