From 5e384c94033039384188aa426849f855eeab755e Mon Sep 17 00:00:00 2001 From: lasanthaDLPDS Date: Thu, 17 Oct 2019 20:43:08 +0530 Subject: [PATCH] Improve APPM subscription flow --- .../mgt/common/SubscribingDeviceIdHolder.java | 9 +++ .../mgt/core/impl/ApplicationManagerImpl.java | 1 + .../core/impl/SubscriptionManagerImpl.java | 69 +++++++++++-------- 3 files changed, 49 insertions(+), 30 deletions(-) diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/SubscribingDeviceIdHolder.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/SubscribingDeviceIdHolder.java index cba18e6733b..64db67a3c2d 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/SubscribingDeviceIdHolder.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/SubscribingDeviceIdHolder.java @@ -25,6 +25,7 @@ import java.util.Map; public class SubscribingDeviceIdHolder { private Map appInstalledDevices = new HashMap<>(); private Map appInstallableDevices = new HashMap<>(); + private Map appReInstallableDevices = new HashMap<>(); public Map getAppInstalledDevices() { return appInstalledDevices; @@ -41,4 +42,12 @@ public class SubscribingDeviceIdHolder { public void setAppInstallableDevices(Map appInstallableDevices) { this.appInstallableDevices = appInstallableDevices; } + + public Map getAppReInstallableDevices() { + return appReInstallableDevices; + } + + public void setAppReInstallableDevices(Map appReInstallableDevices) { + this.appReInstallableDevices = appReInstallableDevices; + } } diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationManagerImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationManagerImpl.java index e4b186bd3ab..a4db52b032b 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationManagerImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationManagerImpl.java @@ -3090,6 +3090,7 @@ public class ApplicationManagerImpl implements ApplicationManager { ConnectionManagerUtil.beginDBTransaction(); List deviceSubIds = subscriptionDAO.getDeviceSubIdsForOperation(operationId, tenantId); if (deviceSubIds.isEmpty()){ + ConnectionManagerUtil.rollbackDBTransaction(); String msg = "Couldn't find device subscription for operation id " + operationId; log.error(msg); throw new ApplicationManagementException(msg); diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/SubscriptionManagerImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/SubscriptionManagerImpl.java index eaea262fe01..639b4b5844b 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/SubscriptionManagerImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/SubscriptionManagerImpl.java @@ -123,7 +123,7 @@ public class SubscriptionManagerImpl implements SubscriptionManager { .getDeviceManagementProviderService(); GroupManagementProviderService groupManagementProviderService = HelperUtil .getGroupManagementProviderService(); - List filteredDevices = new ArrayList<>(); + String deviceTypeName = null; List devices = new ArrayList<>(); List subscribers = new ArrayList<>(); List errorDeviceIdentifiers = new ArrayList<>(); @@ -175,19 +175,14 @@ public class SubscriptionManagerImpl implements SubscriptionManager { if (!ApplicationType.WEB_CLIP.toString().equals(applicationDTO.getType())) { DeviceType deviceType = APIUtil.getDeviceTypeData(applicationDTO.getDeviceTypeId()); - String deviceTypeName = deviceType.getName(); + deviceTypeName = deviceType.getName(); //filter devices by device type - for (Device device : devices) { - if (deviceTypeName.equals(device.getType())) { - filteredDevices.add(device); - } - } - applicationInstallResponse = performActionOnDevices(deviceTypeName, filteredDevices, applicationDTO, - subType, subscribers, action); - } else { - applicationInstallResponse = performActionOnDevices(null, devices, applicationDTO, subType, - subscribers, action); + String tmpDeviceTypeName = deviceTypeName; + devices.removeIf(device -> !tmpDeviceTypeName.equals(device.getType())); } + + applicationInstallResponse = performActionOnDevices(deviceTypeName, devices, applicationDTO, + subType, subscribers, action); applicationInstallResponse.setErrorDeviceIdentifiers(errorDeviceIdentifiers); return applicationInstallResponse; } catch (DeviceManagementException e) { @@ -368,11 +363,14 @@ public class SubscriptionManagerImpl implements SubscriptionManager { Map> deviceIdentifierMap = new HashMap<>(); if (SubAction.INSTALL.toString().equalsIgnoreCase(action)) { - deviceIdentifiers = new ArrayList<>(subscribingDeviceIdHolder.getAppInstallableDevices().keySet()); - ignoredDeviceIdentifiers = new ArrayList<>(subscribingDeviceIdHolder.getAppInstalledDevices().keySet()); + deviceIdentifiers.addAll(new ArrayList<>(subscribingDeviceIdHolder.getAppInstallableDevices().keySet())); + deviceIdentifiers.addAll(new ArrayList<>(subscribingDeviceIdHolder.getAppReInstallableDevices().keySet())); + ignoredDeviceIdentifiers + .addAll(new ArrayList<>(subscribingDeviceIdHolder.getAppInstalledDevices().keySet())); } else if (SubAction.UNINSTALL.toString().equalsIgnoreCase(action)) { - deviceIdentifiers = new ArrayList<>(subscribingDeviceIdHolder.getAppInstalledDevices().keySet()); - ignoredDeviceIdentifiers = new ArrayList<>(subscribingDeviceIdHolder.getAppInstallableDevices().keySet()); + deviceIdentifiers.addAll(new ArrayList<>(subscribingDeviceIdHolder.getAppInstalledDevices().keySet())); + ignoredDeviceIdentifiers + .addAll(new ArrayList<>(subscribingDeviceIdHolder.getAppInstallableDevices().keySet())); } if (deviceIdentifiers.isEmpty()) { @@ -439,6 +437,7 @@ public class SubscriptionManagerImpl implements SubscriptionManager { throws ApplicationManagementException { Map appInstalledDevices = new HashMap<>(); Map appInstallableDevices = new HashMap<>(); + Map appReInstallableDevices = new HashMap<>(); List deviceIds = devices.stream().map(Device::getId).collect(Collectors.toList()); //get device subscriptions for given device id list. @@ -446,9 +445,14 @@ public class SubscriptionManagerImpl implements SubscriptionManager { for (Device device : devices) { DeviceIdentifier deviceIdentifier = new DeviceIdentifier(device.getDeviceIdentifier(), device.getType()); DeviceSubscriptionDTO deviceSubscriptionDTO = deviceSubscriptions.get(device.getId()); - if (deviceSubscriptionDTO != null && !deviceSubscriptionDTO.isUnsubscribed() && Operation.Status.COMPLETED - .toString().equals(deviceSubscriptionDTO.getStatus())) { - appInstalledDevices.put(deviceIdentifier, device.getId()); + if (deviceSubscriptionDTO != null) { + if (deviceSubscriptionDTO.isUnsubscribed()) { + appReInstallableDevices.put(deviceIdentifier, device.getId()); + } + if (!deviceSubscriptionDTO.isUnsubscribed() && Operation.Status.COMPLETED.toString() + .equals(deviceSubscriptionDTO.getStatus())) { + appInstalledDevices.put(deviceIdentifier, device.getId()); + } } else { appInstallableDevices.put(deviceIdentifier, device.getId()); } @@ -457,6 +461,7 @@ public class SubscriptionManagerImpl implements SubscriptionManager { SubscribingDeviceIdHolder subscribingDeviceIdHolder = new SubscribingDeviceIdHolder(); subscribingDeviceIdHolder.setAppInstallableDevices(appInstallableDevices); subscribingDeviceIdHolder.setAppInstalledDevices(appInstalledDevices); + subscribingDeviceIdHolder.setAppReInstallableDevices(appReInstallableDevices); return subscribingDeviceIdHolder; } @@ -556,23 +561,24 @@ public class SubscriptionManagerImpl implements SubscriptionManager { for (Activity activity : activities) { int operationId = Integer.parseInt(activity.getActivityId().split("ACTIVITY_")[1]); - List operationAddedDeviceIds = getOperationAddedDeviceIds(activity, - subscribingDeviceIdHolder.getAppInstallableDevices()); - List alreadySubscribedDevices = subscriptionDAO - .getSubscribedDeviceIds(operationAddedDeviceIds, applicationReleaseId, tenantId); if (SubAction.INSTALL.toString().equalsIgnoreCase(action)) { - if (!alreadySubscribedDevices.isEmpty()) { + List alreadyUnsubscribedDevices = getOperationAddedDeviceIds(activity, + subscribingDeviceIdHolder.getAppReInstallableDevices()); + List deviceIdsOfNewSubscriptions = getOperationAddedDeviceIds(activity, + subscribingDeviceIdHolder.getAppInstallableDevices()); + if (!alreadyUnsubscribedDevices.isEmpty()) { List deviceResubscribingIds = subscriptionDAO - .updateDeviceSubscription(username, alreadySubscribedDevices, false, subType, + .updateDeviceSubscription(username, alreadyUnsubscribedDevices, false, subType, Operation.Status.PENDING.toString(), applicationReleaseId, tenantId); - operationAddedDeviceIds.removeAll(alreadySubscribedDevices); deviceSubIds.addAll(deviceResubscribingIds); } List subscribingDevices = subscriptionDAO - .addDeviceSubscription(username, operationAddedDeviceIds, subType, + .addDeviceSubscription(username, deviceIdsOfNewSubscriptions, subType, Operation.Status.PENDING.toString(), applicationReleaseId, tenantId); deviceSubIds.addAll(subscribingDevices); - } else if (SubAction.UNINSTALL.toString().equalsIgnoreCase(action) && !alreadySubscribedDevices.isEmpty()) { + } else if (SubAction.UNINSTALL.toString().equalsIgnoreCase(action)) { + List alreadySubscribedDevices = getOperationAddedDeviceIds(activity, + subscribingDeviceIdHolder.getAppInstalledDevices()); List deviceResubscribingIds = subscriptionDAO .updateDeviceSubscription(username, alreadySubscribedDevices, true, subType, Operation.Status.PENDING.toString(), applicationReleaseId, tenantId); @@ -603,8 +609,9 @@ public class SubscriptionManagerImpl implements SubscriptionManager { private List getOperationAddedDeviceIds(Activity activity, Map deviceMap) { List activityStatuses = activity.getActivityStatus(); - return activityStatuses.stream().map(status -> deviceMap.get(status.getDeviceIdentifier())) - .collect(Collectors.toList()); + return activityStatuses.stream() + .filter(status -> deviceMap.get(status.getDeviceIdentifier()) != null) + .map(status -> deviceMap.get(status.getDeviceIdentifier())).collect(Collectors.toList()); } /** @@ -689,6 +696,8 @@ public class SubscriptionManagerImpl implements SubscriptionManager { if (SubAction.INSTALL.toString().equalsIgnoreCase(action)) { app.setType(mobileAppType); app.setLocation(application.getApplicationReleases().get(0).getInstallerPath()); + app.setIdentifier(application.getPackageName()); + app.setName(application.getName()); return MDMAndroidOperationUtil.createInstallAppOperation(app); } else if (SubAction.UNINSTALL.toString().equalsIgnoreCase(action)) { app.setType(mobileAppType);