Fixing more transaction handling related issues

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);
         }
     }
 

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();
-                OperationManagementDAOFactory.rollbackTransaction();
-            } catch (OperationManagementDAOException e1) {
-                log.warn("Error occurred while roll-backing the transaction", e1);
-            }
             throw new OperationManagementException("Error occurred while adding operation", e);
         } catch (DeviceManagementDAOException e) {
-            try {
+            OperationManagementDAOFactory.rollbackTransaction();
-                OperationManagementDAOFactory.rollbackTransaction();
-            } catch (OperationManagementDAOException e1) {
-                log.warn("Error occurred while roll-backing the transaction", e1);
-            }
             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<Operation> 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();
-                OperationManagementDAOFactory.closeConnection();
-            } catch (OperationManagementDAOException e) {
-                log.warn("Error occurred while closing data source connection", e);
-            }
         }
 
     }
@@ -165,7 +161,7 @@ public class OperationManagerImpl implements OperationManager {
         List<Operation> operations = new ArrayList<>();
         List<org.wso2.carbon.device.mgt.core.dto.operation.mgt.Operation> 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();
-                OperationManagementDAOFactory.closeConnection();
-            } catch (OperationManagementDAOException e) {
-                log.warn("Error occurred while closing data source connection", e);
-            }
         }
     }
 
@@ -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();
-                OperationManagementDAOFactory.closeConnection();
-            } catch (OperationManagementDAOException e) {
-                log.warn("Error occurred while closing data source connection", e);
-            }
         }
     }
 
     @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();
-                OperationManagementDAOFactory.rollbackTransaction();
-            } catch (OperationManagementDAOException e1) {
-                log.warn("Error occurred while roll-backing the update operation transaction", e1);
-            }
             throw new OperationManagementException("Error occurred while updating the operation: " + operationId +
                     " status:" + operation.getStatus(), e);
         } catch (DeviceManagementDAOException e) {
-            try {
+            OperationManagementDAOFactory.rollbackTransaction();
-                OperationManagementDAOFactory.rollbackTransaction();
-            } catch (OperationManagementDAOException e1) {
-                log.warn("Error occurred while roll-backing the update operation transaction", e1);
-            }
             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();
-                OperationManagementDAOFactory.closeConnection();
-            } catch (OperationManagementDAOException e) {
-                log.warn("Error occurred while closing data source connection", e);
-            }
         }
     }
 
     @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();
-                OperationManagementDAOFactory.rollbackTransaction();
-            } catch (OperationManagementDAOException e1) {
-                log.warn("Error occurred while roll-backing the delete operation transaction", e1);
-            }
             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
+            } else if (dtoOperation.getType().equals(
-                    .PROFILE)) {
+                    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
+            } else if (dtoOperation.getType().equals(
-                    .POLICY)) {
+                    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();
-                OperationManagementDAOFactory.closeConnection();
-            } catch (OperationManagementDAOException e) {
-                log.warn("Error occurred while closing data source connection", e);
-            }
         }
         return operation;
     }
@@ -415,7 +385,7 @@ public class OperationManagerImpl implements OperationManager {
         List<org.wso2.carbon.device.mgt.core.dto.operation.mgt.Operation> 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();
-                OperationManagementDAOFactory.closeConnection();
-            } catch (OperationManagementDAOException e) {
-                log.warn("Error occurred while closing data source connection", e);
-            }
         }
     }
 
     @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
+                commandOperation =
-                        .getOperation(dtoOperation.getId());
+                        (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();
-                OperationManagementDAOFactory.closeConnection();
-            } catch (OperationManagementDAOException e) {
-                log.warn("Error occurred while closing data source connection", e);
-            }
         }
         return operation;
     }

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(
+            throw new TransactionManagementException(
-                "Error occurred while retrieving config.datasource connection", e);
+                    "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());
-                currentConnection.set(dataSource.getConnection());
-            } catch (SQLException e) {
-                throw new OperationManagementDAOException("Error occurred while retrieving data source connection", e);
-            }
         }
         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);
+            log.error("Error occurred while committing the transaction", e);
-        } finally {
-            closeConnection();
         }
     }
 
-    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);
+            log.error("Error occurred while roll-backing the transaction", e);
-        } finally {
-            closeConnection();
         }
     }
 
@@ -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) {

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);

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);