Merge pull request #1208 from charithag/master

Improve device type API and db connection handling

components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/DeviceAgentServiceImpl.java

@@ -521,7 +521,7 @@ public class DeviceAgentServiceImpl implements DeviceAgentService {
 
     @Override
     @PUT
-    @Path("/operations/{type}/{id}")
+    @Path("/properties/{type}/{id}")
     public Response updateDeviceProperties(@PathParam("type") String type, @PathParam("id") String deviceId,
             @Valid List<Device.Property> properties) {
         try {
@@ -542,13 +542,16 @@ public class DeviceAgentServiceImpl implements DeviceAgentService {
                 return Response.status(Response.Status.BAD_REQUEST).entity(msg).build();
             }
 
-            DeviceMgtAPIUtils.getDeviceManagementService().updateProperties(deviceIdentifier, properties);
+            if (DeviceMgtAPIUtils.getDeviceManagementService().updateProperties(deviceIdentifier, properties)){
+                return Response.status(Response.Status.ACCEPTED).entity("Device properties updated.").build();
+            } else {
+                return Response.status(Response.Status.NOT_ACCEPTABLE).entity("Device properties not updated.").build();
+            }
         } catch (DeviceManagementException e) {
             String errorMessage = "Issue in retrieving device management service instance";
             log.error(errorMessage, e);
             return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(errorMessage).build();
         }
-        return null;
     }
 
     @GET

components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/operation/mgt/OperationManagerImpl.java

@@ -21,6 +21,7 @@ package org.wso2.carbon.device.mgt.core.operation.mgt;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.wso2.carbon.context.CarbonContext;
+import org.wso2.carbon.context.PrivilegedCarbonContext;
 import org.wso2.carbon.device.mgt.common.Device;
 import org.wso2.carbon.device.mgt.common.DeviceIdentifier;
 import org.wso2.carbon.device.mgt.common.DeviceManagementException;
@@ -67,7 +68,9 @@ import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.Collections;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 /**
  * This class implements all the functionality exposed as part of the OperationManager. Any transaction initiated
@@ -88,10 +91,10 @@ public class OperationManagerImpl implements OperationManager {
     private OperationDAO operationDAO;
     private DeviceDAO deviceDAO;
     private EnrollmentDAO enrollmentDAO;
-    private NotificationStrategy notificationStrategy;
     private String deviceType;
     private DeviceManagementService deviceManagementService;
-    private long lastUpdatedTimeStamp = 0;
+    private Map<Integer, NotificationStrategy> notificationStrategies;
+    private Map<Integer, Long> lastUpdatedTimeStamps;
 
     public OperationManagerImpl() {
         commandOperationDAO = OperationManagementDAOFactory.getCommandOperationDAO();
@@ -102,6 +105,8 @@ public class OperationManagerImpl implements OperationManager {
         operationDAO = OperationManagementDAOFactory.getOperationDAO();
         deviceDAO = DeviceManagementDAOFactory.getDeviceDAO();
         enrollmentDAO = DeviceManagementDAOFactory.getEnrollmentDAO();
+        notificationStrategies = new HashMap<>();
+        lastUpdatedTimeStamps = new HashMap<>();
     }
 
     public OperationManagerImpl(String deviceType, DeviceManagementService deviceManagementService) {
@@ -111,23 +116,32 @@ public class OperationManagerImpl implements OperationManager {
     }
 
     public NotificationStrategy getNotificationStrategy() {
+        // Notification strategy can be set by the platform configurations. Therefore it is needed to
+        // get tenant specific notification strategy dynamically in the runtime. However since this is
+        // a resource intensive retrieval, we are maintaining tenant aware local cache here to keep device
+        // type specific notification strategy.
+        int tenantId = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantId(false);
+        long lastUpdatedTimeStamp = 0;
+        if (lastUpdatedTimeStamps.containsKey(tenantId)){
+            lastUpdatedTimeStamp = lastUpdatedTimeStamps.get(tenantId);
+        }
         if (Calendar.getInstance().getTimeInMillis() - lastUpdatedTimeStamp > CACHE_VALIDITY_PERIOD) {
             PushNotificationConfig pushNoteConfig = deviceManagementService.getPushNotificationConfig();
             if (pushNoteConfig != null && !NOTIFIER_TYPE_LOCAL.equals(pushNoteConfig.getType())) {
                 PushNotificationProvider provider = DeviceManagementDataHolder.getInstance()
                         .getPushNotificationProviderRepository().getProvider(pushNoteConfig.getType());
                 if (provider == null) {
-                    log.error("No registered push notification provider found for the type: '" +
+                    log.error("No registered push notification provider found for the type '" +
-                              pushNoteConfig.getType() + "'.");
+                              pushNoteConfig.getType() + "' under tenant ID '" + tenantId + "'.");
                     return null;
                 }
-                notificationStrategy = provider.getNotificationStrategy(pushNoteConfig);
+                notificationStrategies.put(tenantId, provider.getNotificationStrategy(pushNoteConfig));
-            } else {
+            } else if (notificationStrategies.containsKey(tenantId)){
-                notificationStrategy = null;
+                notificationStrategies.remove(tenantId);
             }
-            lastUpdatedTimeStamp = Calendar.getInstance().getTimeInMillis();
+            lastUpdatedTimeStamps.put(tenantId, Calendar.getInstance().getTimeInMillis());
         }
-        return notificationStrategy;
+        return notificationStrategies.get(tenantId);
     }
 
     @Override
@@ -164,7 +178,7 @@ public class OperationManagerImpl implements OperationManager {
                 boolean isScheduledOperation = this.isTaskScheduledOperation(operation);
                 boolean isNotRepeated = false;
                 boolean isScheduled = false;
-                notificationStrategy = getNotificationStrategy();
+                NotificationStrategy notificationStrategy = getNotificationStrategy();
 
                 // check whether device list is greater than batch size notification strategy has enable to send push
                 // notification using scheduler task

components/device-mgt/org.wso2.carbon.device.mgt.extensions/src/main/java/org/wso2/carbon/device/mgt/extensions/device/type/template/DeviceTypeManager.java

@@ -32,11 +32,16 @@ import org.wso2.carbon.device.mgt.common.configuration.mgt.PlatformConfiguration
 import org.wso2.carbon.device.mgt.common.license.mgt.License;
 import org.wso2.carbon.device.mgt.common.license.mgt.LicenseManagementException;
 import org.wso2.carbon.device.mgt.common.license.mgt.LicenseManager;
-import org.wso2.carbon.device.mgt.extensions.device.type.template.config.*;
+import org.wso2.carbon.device.mgt.extensions.device.type.template.config.DataSource;
-import org.wso2.carbon.device.mgt.extensions.device.type.template.exception.DeviceTypeDeployerPayloadException;
+import org.wso2.carbon.device.mgt.extensions.device.type.template.config.DeviceDetails;
-import org.wso2.carbon.device.mgt.extensions.device.type.template.exception.DeviceTypeMgtPluginException;
+import org.wso2.carbon.device.mgt.extensions.device.type.template.config.DeviceTypeConfiguration;
+import org.wso2.carbon.device.mgt.extensions.device.type.template.config.Feature;
+import org.wso2.carbon.device.mgt.extensions.device.type.template.config.Table;
+import org.wso2.carbon.device.mgt.extensions.device.type.template.config.TableConfig;
 import org.wso2.carbon.device.mgt.extensions.device.type.template.dao.DeviceDAODefinition;
 import org.wso2.carbon.device.mgt.extensions.device.type.template.dao.DeviceTypePluginDAOManager;
+import org.wso2.carbon.device.mgt.extensions.device.type.template.exception.DeviceTypeDeployerPayloadException;
+import org.wso2.carbon.device.mgt.extensions.device.type.template.exception.DeviceTypeMgtPluginException;
 import org.wso2.carbon.device.mgt.extensions.device.type.template.feature.ConfigurationBasedFeatureManager;
 import org.wso2.carbon.device.mgt.extensions.device.type.template.util.DeviceTypePluginConstants;
 import org.wso2.carbon.device.mgt.extensions.device.type.template.util.DeviceTypeUtils;
@@ -108,7 +113,7 @@ public class DeviceTypeManager implements DeviceManager {
                 }
             }
         } catch (LicenseManagementException e) {
-            String msg = "Error occurred while adding default license for " + deviceType + " devices";
+            String msg = "Error occurred while adding default license for " + deviceType + " devices.";
             throw new DeviceTypeDeployerPayloadException(msg, e);
         }
         claimable = false;
@@ -346,6 +351,7 @@ public class DeviceTypeManager implements DeviceManager {
                 if (log.isDebugEnabled()) {
                     log.debug("Checking the enrollment of Android device : " + deviceId.getId());
                 }
+                deviceTypePluginDAOManager.getDeviceTypeDAOHandler().beginTransaction();
                 Device device =
                         deviceTypePluginDAOManager.getDeviceDAO().getDevice(deviceId.getId());
                 if (device != null) {
@@ -353,8 +359,16 @@ public class DeviceTypeManager implements DeviceManager {
                 }
             } catch (DeviceTypeMgtPluginException e) {
                 String msg = "Error while checking the enrollment status of " + deviceType + " device : " +
-                        deviceId.getId();
+                             deviceId.getId();
                 throw new DeviceManagementException(msg, e);
+            } finally {
+                try {
+                    deviceTypePluginDAOManager.getDeviceTypeDAOHandler().closeConnection();
+                } catch (DeviceTypeMgtPluginException e) {
+                    String msg = "Error occurred while closing the transaction to check device " +
+                                 deviceId.getId() + " is enrolled.";
+                    log.warn(msg, e);
+                }
             }
             return isEnrolled;
         }
@@ -383,10 +397,18 @@ public class DeviceTypeManager implements DeviceManager {
                 if (log.isDebugEnabled()) {
                     log.debug("Getting the details of " + deviceType + " device : '" + deviceId.getId() + "'");
                 }
+                deviceTypePluginDAOManager.getDeviceTypeDAOHandler().beginTransaction();
                 device = deviceTypePluginDAOManager.getDeviceDAO().getDevice(deviceId.getId());
             } catch (DeviceTypeMgtPluginException e) {
                 throw new DeviceManagementException(
                         "Error occurred while fetching the " + deviceType + " device: '" + deviceId.getId() + "'", e);
+            } finally {
+                try {
+                    deviceTypePluginDAOManager.getDeviceTypeDAOHandler().closeConnection();
+                } catch (DeviceTypeMgtPluginException e) {
+                    String msg = "Error occurred while closing the transaction to get device " + deviceId.getId();
+                    log.warn(msg, e);
+                }
             }
             return device;
         }
@@ -405,8 +427,16 @@ public class DeviceTypeManager implements DeviceManager {
                 Device updatedDevice = new Device();
                 updatedDevice.setDeviceIdentifier(deviceId.getId());
                 updatedDevice.setProperties(propertyList);
+                deviceTypePluginDAOManager.getDeviceTypeDAOHandler().beginTransaction();
                 status = deviceTypePluginDAOManager.getDeviceDAO().updateDevice(updatedDevice);
+                deviceTypePluginDAOManager.getDeviceTypeDAOHandler().commitTransaction();
             } catch (DeviceTypeMgtPluginException e) {
+                try {
+                    deviceTypePluginDAOManager.getDeviceTypeDAOHandler().rollbackTransaction();
+                } catch (DeviceTypeMgtPluginException transactionException) {
+                    String msg = "Error occurred while rolling back transaction for device: " + deviceId.getId();
+                    log.warn(msg, transactionException);
+                }
                 throw new DeviceManagementException(
                         "Error occurred while fetching the " + deviceType + " device: '" + deviceId.getId() + "'", e);
             }
@@ -521,9 +551,17 @@ public class DeviceTypeManager implements DeviceManager {
                 if (log.isDebugEnabled()) {
                     log.debug("Fetching the details of all " + deviceType + " devices");
                 }
+                deviceTypePluginDAOManager.getDeviceTypeDAOHandler().beginTransaction();
                 devices = deviceTypePluginDAOManager.getDeviceDAO().getAllDevices();
             } catch (DeviceTypeMgtPluginException e) {
                 throw new DeviceManagementException("Error occurred while fetching all " + deviceType + " devices", e);
+            } finally {
+                try {
+                    deviceTypePluginDAOManager.getDeviceTypeDAOHandler().closeConnection();
+                } catch (DeviceTypeMgtPluginException e) {
+                    String msg = "Error occurred while closing the transaction to get all devices.";
+                    log.warn(msg, e);
+                }
             }
             return devices;
         }

components/device-mgt/org.wso2.carbon.device.mgt.extensions/src/main/java/org/wso2/carbon/device/mgt/extensions/device/type/template/DeviceTypeManagerService.java

@@ -22,10 +22,10 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.wso2.carbon.device.mgt.common.DeviceManagementException;
 import org.wso2.carbon.device.mgt.common.DeviceManager;
+import org.wso2.carbon.device.mgt.common.DeviceStatusTaskPluginConfig;
 import org.wso2.carbon.device.mgt.common.InitialOperationConfig;
 import org.wso2.carbon.device.mgt.common.MonitoringOperation;
 import org.wso2.carbon.device.mgt.common.OperationMonitoringTaskConfig;
-import org.wso2.carbon.device.mgt.common.DeviceStatusTaskPluginConfig;
 import org.wso2.carbon.device.mgt.common.ProvisioningConfig;
 import org.wso2.carbon.device.mgt.common.app.mgt.ApplicationManager;
 import org.wso2.carbon.device.mgt.common.configuration.mgt.ConfigurationEntry;
@@ -136,7 +136,7 @@ public class DeviceTypeManagerService implements DeviceManagementService {
                 ConfigProperties configProperties = pushNotificationProvider.getConfigProperties();
                 if (configProperties != null) {
                     List<Property> properties = configProperties.getProperty();
-                    if (properties != null && properties.size() > 0) {
+                    if (properties != null && !properties.isEmpty()) {
                         for (Property property : properties) {
                             staticProps.put(property.getName(), property.getValue());
                         }
@@ -154,6 +154,7 @@ public class DeviceTypeManagerService implements DeviceManagementService {
     }
 
     private void refreshPlatformConfigurations() {
+        //Build up push notification configs to use with push notification provider.
         try {
             PlatformConfiguration deviceTypeConfig = deviceManager.getConfiguration();
             if (deviceTypeConfig != null) {
@@ -161,14 +162,17 @@ public class DeviceTypeManagerService implements DeviceManagementService {
                 if (!configuration.isEmpty()) {
                     Map<String, String> properties = this.getConfigProperty(configuration);
                     String notifierValue = properties.get(NOTIFIER_PROPERTY);
+                    String enabledNotifierType = notifierType;
+                    //In registry we are keeping local notifier as value "1". Other notifiers will have
+                    // a number grater than 1.
                     if (notifierValue != null && notifierValue.equals("1")) {
-                        notifierType = NOTIFIER_TYPE_LOCAL;
+                        enabledNotifierType = NOTIFIER_TYPE_LOCAL;
                     }
-                    pushNotificationConfig = new PushNotificationConfig(notifierType, isScheduled, properties);
+                    pushNotificationConfig = new PushNotificationConfig(enabledNotifierType, isScheduled, properties);
                 }
             }
         } catch (DeviceManagementException e) {
-            log.error("Unable to get the " + type + " platform configuration from registry.");
+            log.error("Unable to get the " + type + " platform configuration from registry.", e);
         }
     }
 
@@ -189,6 +193,7 @@ public class DeviceTypeManagerService implements DeviceManagementService {
 
     @Override
     public PushNotificationConfig getPushNotificationConfig() {
+        //We only need to update push notification configs if this device type uses registry based configs.
         if (isRegistryBasedConfigs) {
             refreshPlatformConfigurations();
         }