From a876b65974ebef2cae25b3c2849a13c080381222 Mon Sep 17 00:00:00 2001 From: charitha Date: Mon, 12 Feb 2018 20:16:06 +0530 Subject: [PATCH 1/2] Fixed regression issues related to https://github.com/wso2/product-iots/issues/1671 --- .../operation/mgt/OperationManagerImpl.java | 34 +++++++++++++------ .../template/DeviceTypeManagerService.java | 15 +++++--- 2 files changed, 34 insertions(+), 15 deletions(-) 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 8c6c23352c1..d45fb2deadb 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 @@ -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 notificationStrategies; + private Map 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: '" + - pushNoteConfig.getType() + "'."); + log.error("No registered push notification provider found for the type '" + + pushNoteConfig.getType() + "' under tenant ID '" + tenantId + "'."); return null; } - notificationStrategy = provider.getNotificationStrategy(pushNoteConfig); - } else { - notificationStrategy = null; + notificationStrategies.put(tenantId, provider.getNotificationStrategy(pushNoteConfig)); + } else if (notificationStrategies.containsKey(tenantId)){ + 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 diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.extensions/src/main/java/org/wso2/carbon/device/mgt/extensions/device/type/template/DeviceTypeManagerService.java b/components/device-mgt/org.wso2.carbon.device.mgt.extensions/src/main/java/org/wso2/carbon/device/mgt/extensions/device/type/template/DeviceTypeManagerService.java index 4c481fc9f35..ca245a1d001 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.extensions/src/main/java/org/wso2/carbon/device/mgt/extensions/device/type/template/DeviceTypeManagerService.java +++ b/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 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 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(); } From ab4f2b483377e2e55c6209ad741abc5bc14e4c25 Mon Sep 17 00:00:00 2001 From: charitha Date: Sat, 3 Mar 2018 22:47:32 +0530 Subject: [PATCH 2/2] Add response status to device property update API and improve db connection handling in device property update --- .../service/impl/DeviceAgentServiceImpl.java | 9 ++-- .../type/template/DeviceTypeManager.java | 48 +++++++++++++++++-- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/DeviceAgentServiceImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/DeviceAgentServiceImpl.java index 08328c424d7..5bda5a70a76 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/DeviceAgentServiceImpl.java +++ b/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 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 diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.extensions/src/main/java/org/wso2/carbon/device/mgt/extensions/device/type/template/DeviceTypeManager.java b/components/device-mgt/org.wso2.carbon.device.mgt.extensions/src/main/java/org/wso2/carbon/device/mgt/extensions/device/type/template/DeviceTypeManager.java index e848a7fc826..b84d1658148 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.extensions/src/main/java/org/wso2/carbon/device/mgt/extensions/device/type/template/DeviceTypeManager.java +++ b/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.exception.DeviceTypeDeployerPayloadException; -import org.wso2.carbon.device.mgt.extensions.device.type.template.exception.DeviceTypeMgtPluginException; +import org.wso2.carbon.device.mgt.extensions.device.type.template.config.DataSource; +import org.wso2.carbon.device.mgt.extensions.device.type.template.config.DeviceDetails; +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; }