Improve APPM subscribing method

components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/ApplicationInstallResponse.java

@@ -32,6 +32,14 @@ public class ApplicationInstallResponse {
     )
     private List<DeviceIdentifier> alreadyInstalledDevices;
 
+    @ApiModelProperty(
+            name = "errorDevices",
+            value = "List of devices that either device identity is not exist or device type doesn't compatible with the supported device type of the .",
+            dataType = "List[org.wso2.carbon.device.mgt.common.DeviceIdentifier]"
+    )
+    private List<DeviceIdentifier> errorDevices;
+
+
     @ApiModelProperty(
             name = "activity",
             value = "Activity corresponding to the operation"
@@ -53,4 +61,8 @@ public class ApplicationInstallResponse {
     public void setAlreadyInstalledDevices(List<DeviceIdentifier> alreadyInstalledDevices) {
         this.alreadyInstalledDevices = alreadyInstalledDevices;
     }
+
+    public List<DeviceIdentifier> getErrorDevices() { return errorDevices; }
+
+    public void setErrorDevices(List<DeviceIdentifier> errorDevices) { this.errorDevices = errorDevices; }
 }

components/application-mgt/org.wso2.carbon.device.application.mgt.common/src/main/java/org/wso2/carbon/device/application/mgt/common/SubscribingDeviceIdHolder.java

@@ -0,0 +1,56 @@
+/* Copyright (c) 2019, Entgra (Pvt) Ltd. (http://www.entgra.io) All Rights Reserved.
+ *
+ * Entgra (Pvt) Ltd. licenses this file to you under the Apache License,
+ * Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.wso2.carbon.device.application.mgt.common;
+
+import org.wso2.carbon.device.application.mgt.common.dto.DeviceSubscriptionDTO;
+import org.wso2.carbon.device.mgt.common.DeviceIdentifier;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+public class SubscribingDeviceIdHolder {
+    private List<DeviceIdentifier> installedDevices = new ArrayList<>();
+    private Map<DeviceIdentifier, Integer> compatibleDevices = new HashMap<>();
+    private Map<Integer, DeviceSubscriptionDTO> deviceSubscriptions = new HashMap<>();
+
+    public List<DeviceIdentifier> getInstalledDevices() {
+        return installedDevices;
+    }
+
+    public void setInstalledDevices(List<DeviceIdentifier> installedDevices) {
+        this.installedDevices = installedDevices;
+    }
+
+    public Map<DeviceIdentifier, Integer> getCompatibleDevices() {
+        return compatibleDevices;
+    }
+
+    public void setCompatibleDevices(Map<DeviceIdentifier, Integer> compatibleDevices) {
+        this.compatibleDevices = compatibleDevices;
+    }
+
+    public Map<Integer, DeviceSubscriptionDTO> getDeviceSubscriptions() {
+        return deviceSubscriptions;
+    }
+
+    public void setDeviceSubscriptions(Map<Integer, DeviceSubscriptionDTO> deviceSubscriptions) {
+        this.deviceSubscriptions = deviceSubscriptions;
+    }
+}

components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationManagerImpl.java

@@ -1,7 +1,6 @@
-/*
+/* Copyright (c) 2019, Entgra (Pvt) Ltd. (http://www.entgra.io) All Rights Reserved.
- *   Copyright (c) 2017, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
  *
- *   WSO2 Inc. licenses this file to you under the Apache License,
+ * Entgra (Pvt) Ltd. licenses this file to you under the Apache License,
  * Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License.
  * You may obtain a copy of the License at
@@ -14,8 +13,8 @@
  * KIND, either express or implied. See the License for the
  * specific language governing permissions and limitations
  * under the License.
- *
  */
+
 package org.wso2.carbon.device.application.mgt.core.impl;
 
 import org.apache.commons.io.IOUtils;

components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ApplicationStorageManagerImpl.java

@@ -1,7 +1,6 @@
-/*
+/* Copyright (c) 2019, Entgra (Pvt) Ltd. (http://www.entgra.io) All Rights Reserved.
- *   Copyright (c) 2017, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
  *
- *   WSO2 Inc. licenses this file to you under the Apache License,
+ * Entgra (Pvt) Ltd. licenses this file to you under the Apache License,
  * Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License.
  * You may obtain a copy of the License at
@@ -14,7 +13,6 @@
  * KIND, either express or implied. See the License for the
  * specific language governing permissions and limitations
  * under the License.
- *
  */
 
 package org.wso2.carbon.device.application.mgt.core.impl;

components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/AppmDataHandlerImpl.java

@@ -1,4 +1,4 @@
-/* Copyright (c) 2018, Entgra (Pvt) Ltd. (http://www.entgra.io) All Rights Reserved.
+/* Copyright (c) 2019, Entgra (Pvt) Ltd. (http://www.entgra.io) All Rights Reserved.
  *
  * Entgra (Pvt) Ltd. licenses this file to you under the Apache License,
  * Version 2.0 (the "License"); you may not use this file except

components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/ReviewManagerImpl.java

@@ -1,6 +1,6 @@
-/*
+/* Copyright (c) 2019, Entgra (Pvt) Ltd. (http://www.entgra.io) All Rights Reserved.
- *  Copyright (c) 2018, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
+ *
- *  WSO2 Inc. licenses this file to you under the Apache License,
+ * Entgra (Pvt) Ltd. licenses this file to you under the Apache License,
  * Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License.
  * You may obtain a copy of the License at
@@ -13,8 +13,8 @@
  * KIND, either express or implied. See the License for the
  * specific language governing permissions and limitations
  * under the License.
- *
  */
+
 package org.wso2.carbon.device.application.mgt.core.impl;
 
 import org.apache.commons.logging.Log;

components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/SubscriptionManagerImpl.java

@@ -1,6 +1,6 @@
-/*
+/* Copyright (c) 2019, Entgra (Pvt) Ltd. (http://www.entgra.io) All Rights Reserved.
-*  Copyright (c) 2017, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
+ *
-*  WSO2 Inc. licenses this file to you under the Apache License,
+ * Entgra (Pvt) Ltd. licenses this file to you under the Apache License,
  * Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License.
  * You may obtain a copy of the License at
@@ -13,10 +13,11 @@
  * KIND, either express or implied. See the License for the
  * specific language governing permissions and limitations
  * under the License.
-*
  */
+
 package org.wso2.carbon.device.application.mgt.core.impl;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.wso2.carbon.context.PrivilegedCarbonContext;
@@ -24,6 +25,7 @@ import org.wso2.carbon.device.application.mgt.common.AppOperation;
 import org.wso2.carbon.device.application.mgt.common.ApplicationInstallResponse;
 import org.wso2.carbon.device.application.mgt.common.ApplicationType;
 import org.wso2.carbon.device.application.mgt.common.SubsciptionType;
+import org.wso2.carbon.device.application.mgt.common.SubscribingDeviceIdHolder;
 import org.wso2.carbon.device.application.mgt.common.dto.ApplicationDTO;
 import org.wso2.carbon.device.application.mgt.common.ApplicationInstallResponseTmp;
 import org.wso2.carbon.device.application.mgt.common.dto.DeviceSubscriptionDTO;
@@ -55,9 +57,9 @@ import org.wso2.carbon.device.mgt.common.operation.mgt.Activity;
 import org.wso2.carbon.device.mgt.common.operation.mgt.ActivityStatus;
 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.core.DeviceManagementConstants;
 import org.wso2.carbon.device.mgt.core.dto.DeviceType;
 import org.wso2.carbon.device.mgt.core.operation.mgt.ProfileOperation;
+import org.wso2.carbon.device.mgt.core.operation.mgt.util.DeviceIDHolder;
 import org.wso2.carbon.device.mgt.core.service.DeviceManagementProviderService;
 import org.wso2.carbon.device.mgt.core.service.GroupManagementProviderService;
 import org.wso2.carbon.device.mgt.core.util.DeviceManagerUtil;
@@ -92,52 +94,136 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
             log.debug("Install application which has UUID: " + applicationUUID + " to " + deviceIdentifiers.size()
                     + "devices.");
         }
+        DeviceManagementProviderService deviceManagementProviderService = HelperUtil
+                .getDeviceManagementProviderService();
         ApplicationDTO applicationDTO = getApplicationDTO(applicationUUID);
-        List<Integer> operationTriggeredDeviceIds = new ArrayList<>();
+        DeviceIDHolder filteredDeviceHolder = filterDeviceIdentifiers(applicationDTO, deviceIdentifiers);
-        List <Device> filteredDevices = validateAppInstallingForDevicesRequest(applicationDTO, deviceIdentifiers);
+        List<DeviceIdentifier> errorDeviceIdentifierList = filteredDeviceHolder.getErrorDeviceIdList();
-        List<Integer> filteredDeviceIds = new ArrayList<>();
+        List<Device> filteredDeviceObjs = new ArrayList<>();
-        List<DeviceIdentifier> installedDeviceIdentifiers = new ArrayList<>();
+
-        Map<DeviceIdentifier , Integer> compatibleDevices = new HashMap<>();
+        for (DeviceIdentifier deviceIdentifier : filteredDeviceHolder.getValidDeviceIDList()) {
-        Map<Integer, DeviceSubscriptionDTO> deviceSubscriptions;
+            try {
+                Device device = deviceManagementProviderService.getDevice(deviceIdentifier, false);
+                if (device == null) {
+                    String msg = "Couldn't found an device for device identifier " + deviceIdentifier.getId()
+                            + " and device type: " + deviceIdentifier.getType();
+                    log.warn(msg);
+                    errorDeviceIdentifierList.add(deviceIdentifier);
+                } else {
+                    filteredDeviceObjs.add(device);
+                }
+            } catch (DeviceManagementException e) {
+                String msg = "Error occurred when getting device data for device identifier " + deviceIdentifier.getId()
+                        + " and device type " + deviceIdentifier.getType();
+                log.warn(msg);
+                errorDeviceIdentifierList.add(deviceIdentifier);
+                throw new ApplicationManagementException(msg, e);
+            }
+        }
 
-        for (Device device : filteredDevices){
+        SubscribingDeviceIdHolder subscribingDeviceIdHolder = getSubscribingDeviceIdHolder(filteredDeviceObjs);
-            filteredDeviceIds.add(device.getId());
+        if (subscribingDeviceIdHolder.getCompatibleDevices().isEmpty()){
+            ApplicationInstallResponse applicationInstallResponse = new ApplicationInstallResponse();
+            applicationInstallResponse.setAlreadyInstalledDevices(subscribingDeviceIdHolder.getInstalledDevices());
+            applicationInstallResponse.setErrorDevices(errorDeviceIdentifierList);
+            return applicationInstallResponse;
         }
+        Activity activity = installToDevices(applicationDTO,
+                new ArrayList<>(subscribingDeviceIdHolder.getCompatibleDevices().keySet()),
+                deviceIdentifiers.get(0).getType());
 
+        ApplicationInstallResponse applicationInstallResponse = new ApplicationInstallResponse();
+        applicationInstallResponse.setActivity(activity);
+        applicationInstallResponse.setAlreadyInstalledDevices(subscribingDeviceIdHolder.getInstalledDevices());
+        applicationInstallResponse.setErrorDevices(errorDeviceIdentifierList);
 
-        deviceSubscriptions = getDeviceSubscriptions(filteredDeviceIds);
+        addSubscriptions(applicationDTO.getApplicationReleaseDTOs().get(0).getId(), activity,
-        for (Device device : filteredDevices) {
+                subscribingDeviceIdHolder.getCompatibleDevices(),
-            DeviceIdentifier deviceIdentifier = new DeviceIdentifier(device.getDeviceIdentifier(),
+                new ArrayList<>(subscribingDeviceIdHolder.getDeviceSubscriptions().keySet()), null,
-                    device.getType());
+                SubsciptionType.DEVICE.toString());
-            DeviceSubscriptionDTO deviceSubscriptionDTO = deviceSubscriptions.get(device.getId());
+        return applicationInstallResponse;
-            if (deviceSubscriptionDTO != null && !deviceSubscriptionDTO.isUnsubscribed()
-                    && Operation.Status.COMPLETED.toString().equals(deviceSubscriptionDTO.getStatus())) {
-                installedDeviceIdentifiers.add(deviceIdentifier);
-            } else {
-                compatibleDevices.put(deviceIdentifier, device.getId());
     }
+
+    @Override
+    public ApplicationInstallResponse installApplicationForUsers(String applicationUUID,
+            List<String> userList) throws ApplicationManagementException {
+        if (log.isDebugEnabled()) {
+            log.debug("Install application release which has UUID " + applicationUUID + " to " + userList.size()
+                    + " users.");
         }
-        Activity activity = installToDevices(applicationDTO, deviceIdentifiers, deviceIdentifiers.get(0).getType());
 
-        List<ActivityStatus> activityStatuses = activity.getActivityStatus();
+        //todo check valid user list - throw BadRequest exception
-        for (ActivityStatus status : activityStatuses) {
+        ApplicationDTO applicationDTO = getApplicationDTO(applicationUUID);
-            if (status.getStatus().equals(ActivityStatus.Status.PENDING)){
+        DeviceType appDeviceType = APIUtil.getDeviceTypeData(applicationDTO.getDeviceTypeId());
-                operationTriggeredDeviceIds.add(compatibleDevices.get(status.getDeviceIdentifier()));
+        Map<DeviceIdentifier, Integer> compatibleDevices = new HashMap<>();
+        List<DeviceIdentifier> installedDeviceIdentifiers = new ArrayList<>();
+        Map<Integer, DeviceSubscriptionDTO> deviceSubscriptions = new HashMap<>();
+
+        for (String user : userList) {
+            try {
+                List<Device> userDevices = HelperUtil.getDeviceManagementProviderService().getDevicesOfUser(user);
+                List<Device> filteredDevices = new ArrayList<>();
+
+                //todo improve for web clips
+                for (Device device : userDevices) {
+                    if (appDeviceType.getName().equals(device.getType())) {
+                        filteredDevices.add(device);
                     }
                 }
+
+                SubscribingDeviceIdHolder subscribingDeviceIdHolder = getSubscribingDeviceIdHolder(filteredDevices);
+                compatibleDevices.putAll(subscribingDeviceIdHolder.getCompatibleDevices());
+                deviceSubscriptions.putAll(subscribingDeviceIdHolder.getDeviceSubscriptions());
+                installedDeviceIdentifiers.addAll(subscribingDeviceIdHolder.getInstalledDevices());
+            } catch (DeviceManagementException e) {
+                String msg = "Error occurred when extracting the device list of user[" + user + "].";
+                log.error(msg);
+                throw new ApplicationManagementException(msg, e);
+            }
+        }
+
+        if (compatibleDevices.keySet().isEmpty()){
+            ApplicationInstallResponse applicationInstallResponse = new ApplicationInstallResponse();
+            applicationInstallResponse.setAlreadyInstalledDevices(installedDeviceIdentifiers);
+            return applicationInstallResponse;
+        }
+        Activity activity = installToDevices(applicationDTO, new ArrayList<>(compatibleDevices.keySet()),
+                appDeviceType.getName());
+
         ApplicationInstallResponse applicationInstallResponse = new ApplicationInstallResponse();
         applicationInstallResponse.setActivity(activity);
         applicationInstallResponse.setAlreadyInstalledDevices(installedDeviceIdentifiers);
 
-//        int operationId = Integer
+        addSubscriptions(applicationDTO.getApplicationReleaseDTOs().get(0).getId(), activity, compatibleDevices,
-//                .parseInt(activity.getActivityId().split(DeviceManagementConstants.OperationAttributes.ACTIVITY)[1]);
+                new ArrayList<>(deviceSubscriptions.keySet()), userList, SubsciptionType.USER.toString());
-        int operationId = Integer.parseInt(activity.getActivityId().split("ACTIVITY_")[1]);
-        addDeviceSubscriptions(applicationDTO.getApplicationReleaseDTOs().get(0).getId(),
-                operationTriggeredDeviceIds, new ArrayList<>(deviceSubscriptions.keySet()), null, operationId,
-                SubsciptionType.DEVICE.toString());
         return applicationInstallResponse;
     }
 
+    private SubscribingDeviceIdHolder getSubscribingDeviceIdHolder(List<Device> devices)
+            throws ApplicationManagementException {
+        List<DeviceIdentifier> installedDevices = new ArrayList<>();
+        Map<DeviceIdentifier, Integer> compatibleDevices = new HashMap<>();
+
+        List<Integer> filteredDeviceIds = devices.stream().map(Device::getId).collect(Collectors.toList());
+        Map<Integer, DeviceSubscriptionDTO> deviceSubscriptions = getDeviceSubscriptions(filteredDeviceIds);
+        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())) {
+                installedDevices.add(deviceIdentifier);
+            } else {
+                compatibleDevices.put(deviceIdentifier, device.getId());
+            }
+        }
+
+        SubscribingDeviceIdHolder subscribingDeviceIdHolder = new SubscribingDeviceIdHolder();
+        subscribingDeviceIdHolder.setCompatibleDevices(compatibleDevices);
+        subscribingDeviceIdHolder.setInstalledDevices(installedDevices);
+        subscribingDeviceIdHolder.setDeviceSubscriptions(deviceSubscriptions);
+        return subscribingDeviceIdHolder;
+    }
+
     private ApplicationDTO getApplicationDTO(String uuid) throws ApplicationManagementException {
         ApplicationDTO applicationDTO;
         int tenantId = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantId(true);
@@ -173,117 +259,48 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
         }
     }
 
-    private List <Device> validateAppInstallingForDevicesRequest(ApplicationDTO applicationDTO,
+    private DeviceIDHolder filterDeviceIdentifiers(ApplicationDTO applicationDTO,
             List<DeviceIdentifier> deviceIdentifiers) throws ApplicationManagementException {
         DeviceType deviceType = null;
-        List <Device> existingDevices = new ArrayList<>();
+        List<DeviceIdentifier> errorDeviceIdList = new ArrayList<>();
-        DeviceManagementProviderService deviceManagementProviderService = HelperUtil
+        List<DeviceIdentifier> validDeviceIDList = new ArrayList<>();
-                .getDeviceManagementProviderService();
         if (!ApplicationType.WEB_CLIP.toString().equals(applicationDTO.getType())) {
             deviceType = APIUtil.getDeviceTypeData(applicationDTO.getDeviceTypeId());
         }
 
         for (DeviceIdentifier deviceIdentifier : deviceIdentifiers) {
+            if (StringUtils.isEmpty(deviceIdentifier.getId()) || StringUtils.isEmpty(deviceIdentifier.getType())) {
+                log.warn("Found a device identifier which has either empty identity of the device or empty device "
+                        + "type. Hence ignoring the device identifier. ");
+                continue;
+            }
             if (!ApplicationType.WEB_CLIP.toString().equals(applicationDTO.getType()) && deviceType != null
                     && !deviceType.getName().equals(deviceIdentifier.getType())) {
                 String msg =
                         "Found a device identifier which is not matched with the application device Type. Application "
-                                + "device type is " + deviceType.getName()
+                                + "device type is " + deviceType.getName() + " and the identifier of which has a "
-                                + " and identifier which has different device" + " type is " + deviceIdentifier.getId();
+                                + "different device type is " + deviceIdentifier.getId();
-                log.error(msg);
+                log.warn(msg);
-                throw new BadRequestException(msg);
+                errorDeviceIdList.add(deviceIdentifier);
-            }
-            try {
-                Device device = deviceManagementProviderService.getDevice(deviceIdentifier, false);
-                if (device == null) {
-                    String msg = "Couldn't found an device for device identifier " + deviceIdentifier.getId()
-                            + " and device type: " + deviceIdentifier.getType();
-                    log.error(msg);
             } else {
-                    existingDevices.add(device);
+                validDeviceIDList.add(deviceIdentifier);
-                }
-            } catch (DeviceManagementException e) {
-                String msg = "Error occuered when getting device data for divice identifier " + deviceIdentifier.getId()
-                        + " and device type " + deviceIdentifier.getType();
-                log.error(msg);
-                throw new ApplicationManagementException(msg, e);
             }
         }
-        return existingDevices;
-    }
-
-    @Override
-    public ApplicationInstallResponse installApplicationForUsers(String applicationUUID,
-            List<String> userList) throws ApplicationManagementException {
-        if (log.isDebugEnabled()) {
-            log.debug("Install application release which has UUID " + applicationUUID + " to " + userList.size()
-                    + " users.");
-        }
-
-        //todo check valid user list - throw BadRequest exception
-        ApplicationDTO applicationDTO = getApplicationDTO(applicationUUID);
-        DeviceType appDeviceType = APIUtil.getDeviceTypeData(applicationDTO.getDeviceTypeId());
-        Map<DeviceIdentifier , Integer> compatibleDevices = new HashMap<>();
-        List<Integer> operationTriggeredDeviceIds = new ArrayList<>();
-        List<DeviceIdentifier> installedDeviceIdentifiers = new ArrayList<>();
-        Map<Integer, DeviceSubscriptionDTO> deviceSubscriptions = new HashMap<>();
-
-        for (String user : userList) {
-            try {
-                List<Device> userDevices = HelperUtil.getDeviceManagementProviderService().getDevicesOfUser(user);
-                List<Integer> filteredDeviceIds = new ArrayList<>();
-                List<Device> filteredDevices = new ArrayList<>();
 
-                //todo improve for web clips
+        if (validDeviceIDList.isEmpty()) {
-                for (Device device : userDevices) {
+            String msg = "Couldn't found at least one valid device identifier to install the application.";
-                    if (appDeviceType.getName().equals(device.getType())) {
-                        filteredDevices.add(device);
-                        filteredDeviceIds.add(device.getId());
-                    }
-                }
-                deviceSubscriptions = getDeviceSubscriptions(filteredDeviceIds);
-                for (Device device : filteredDevices) {
-                    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())) {
-                        installedDeviceIdentifiers.add(deviceIdentifier);
-                    } else {
-                        compatibleDevices.put(deviceIdentifier, device.getId());
-                    }
-                }
-            } catch (DeviceManagementException e) {
-                String msg = "Error occurred when extracting the device list of user[" + user + "].";
             log.error(msg);
-                throw new ApplicationManagementException(msg, e);
+            throw new BadRequestException(msg);
-            }
-        }
-        Activity activity = installToDevices(applicationDTO, new ArrayList<>(compatibleDevices.keySet()),
-                appDeviceType.getName());
-
-        List<ActivityStatus> activityStatuses = activity.getActivityStatus();
-        for (ActivityStatus status : activityStatuses) {
-            if (status.getStatus().equals(ActivityStatus.Status.PENDING)){
-                operationTriggeredDeviceIds.add(compatibleDevices.get(status.getDeviceIdentifier()));
-            }
         }
-        ApplicationInstallResponse applicationInstallResponse = new ApplicationInstallResponse();
+        DeviceIDHolder deviceIDHolder = new DeviceIDHolder();
-        applicationInstallResponse.setActivity(activity);
+        deviceIDHolder.setValidDeviceIDList(validDeviceIDList);
-        applicationInstallResponse.setAlreadyInstalledDevices(installedDeviceIdentifiers);
+        deviceIDHolder.setErrorDeviceIdList(errorDeviceIdList);
-
+        return deviceIDHolder;
-//        int operationId = Integer
-//                .parseInt(activity.getActivityId().split(DeviceManagementConstants.OperationAttributes.ACTIVITY)[1]);
-        int operationId = Integer.parseInt(activity.getActivityId().split("ACTIVITY_")[1]);
-
-        addDeviceSubscriptions(applicationDTO.getApplicationReleaseDTOs().get(0).getId(),
-                operationTriggeredDeviceIds, new ArrayList<>(deviceSubscriptions.keySet()), userList, operationId, SubsciptionType.USER.toString());
-        return applicationInstallResponse;
     }
 
-    private void addDeviceSubscriptions(int applicationReleaseId, List<Integer> deviceIds,
+    private void addSubscriptions(int applicationReleaseId, Activity activity,
-            List<Integer> subDeviceIds, List<String> subscribers, int operationId, String subType)
+            Map<DeviceIdentifier, Integer> compatibleDevices, List<Integer> subDeviceIds, List<String> subscribers,
-            throws ApplicationManagementException {
+            String subType) throws ApplicationManagementException {
         int tenantId = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantId(true);
         String subscriber = PrivilegedCarbonContext.getThreadLocalCarbonContext().getUsername();
         try {
@@ -301,10 +318,18 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
                 subscriptionDAO.subscribeUserToApplication(tenantId, subscriber, subscribers, applicationReleaseId);
             }
 
+            List<Integer> deviceIds = new ArrayList<>();
+            List<ActivityStatus> activityStatuses = activity.getActivityStatus();
+            for (ActivityStatus status : activityStatuses) {
+                if (status.getStatus().equals(ActivityStatus.Status.PENDING)) {
+                    deviceIds.add(compatibleDevices.get(status.getDeviceIdentifier()));
+                }
+            }
+
+            int operationId = Integer.parseInt(activity.getActivityId().split("ACTIVITY_")[1]);
             if (!subDeviceIds.isEmpty()) {
-                deviceResubscribingIds = subscriptionDAO
+                deviceResubscribingIds = subscriptionDAO.updateDeviceSubscription(subscriber, subDeviceIds, subType,
-                        .updateDeviceSubscription(subscriber, subDeviceIds, subType, Operation.Status.PENDING.toString(),
+                        Operation.Status.PENDING.toString(), applicationReleaseId, tenantId);
-                                applicationReleaseId, tenantId);
                 deviceIds.removeAll(subDeviceIds);
             }
             deviceSubscriptingIds = subscriptionDAO
@@ -315,8 +340,8 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
             ConnectionManagerUtil.commitDBTransaction();
         } catch (ApplicationManagementDAOException e) {
             ConnectionManagerUtil.rollbackDBTransaction();
-            String msg =
+            String msg = "Error occurred when adding subscription data for application release UUID: "
-                    "Error occurred when adding subscription data for application release UUID: " + applicationReleaseId;
+                    + applicationReleaseId;
             log.error(msg);
             throw new ApplicationManagementException(msg, e);
         } catch (DBConnectionException e) {
@@ -340,8 +365,7 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
 
         try {
             ConnectionManagerUtil.openDBConnection();
-            return this.subscriptionDAO
+            return this.subscriptionDAO.getDeviceSubscriptions(filteredDeviceIds, tenantId);
-                    .getDeviceSubscriptions(filteredDeviceIds, tenantId);
         } catch (ApplicationManagementDAOException e) {
             String msg = "Error occured when getting device subscriptions for given device IDs";
             log.error(msg);
@@ -356,9 +380,10 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
 
     }
 
-    @Override
+    //todo not implemented below code properly
-    public ApplicationInstallResponseTmp installApplicationForRoles(String applicationUUID, List<String> roleList)
+
-            throws ApplicationManagementException {
+    @Override public ApplicationInstallResponseTmp installApplicationForRoles(String applicationUUID,
+            List<String> roleList) throws ApplicationManagementException {
         if (log.isDebugEnabled()) {
             log.debug("Install application: " + applicationUUID + " to " + roleList.size() + " roles.");
         }
@@ -400,9 +425,8 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
         return response;
     }
 
-    @Override
+    @Override public ApplicationInstallResponseTmp installApplicationForGroups(String applicationUUID,
-    public ApplicationInstallResponseTmp installApplicationForGroups(String applicationUUID, List<String> deviceGroupList)
+            List<String> deviceGroupList) throws ApplicationManagementException {
-            throws ApplicationManagementException {
         if (log.isDebugEnabled()) {
             log.debug("Install application: " + applicationUUID + " to " + deviceGroupList.size() + " groups.");
         }
@@ -452,25 +476,23 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
         return null;
     }
 
-    private Activity installToDevices(ApplicationDTO application,
+    private Activity installToDevices(ApplicationDTO application, List<DeviceIdentifier> deviceIdentifierList,
-            List<DeviceIdentifier> deviceIdentifierList, String deviceType) throws ApplicationManagementException {
+            String deviceType) throws ApplicationManagementException {
         DeviceManagementProviderService deviceManagementProviderService = HelperUtil
                 .getDeviceManagementProviderService();
         try {
             Operation operation = generateOperationPayloadByDeviceType(deviceType, application);
             //todo refactor add operation code to get successful operations
-            return deviceManagementProviderService
+            return deviceManagementProviderService.addOperation(deviceType, operation, deviceIdentifierList);
-                    .addOperation(deviceType, operation, deviceIdentifierList);
         } catch (OperationManagementException e) {
-            throw new ApplicationManagementException("Error occurred while adding the application install "
+            throw new ApplicationManagementException(
-                    + "operation to devices", e);
+                    "Error occurred while adding the application install " + "operation to devices", e);
         } catch (InvalidDeviceException e) {
             //This exception should not occur because the validation has already been done.
             throw new ApplicationManagementException("The list of device identifiers are invalid");
         }
     }
 
-
     private ApplicationInstallResponseTmp installToDevicesTmp(ApplicationDTO application,
             List<DeviceIdentifier> deviceIdentifierList) throws ApplicationManagementException {
         DeviceManagementProviderService deviceManagementProviderService = HelperUtil
@@ -493,8 +515,8 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
             } catch (OperationManagementException e) {
                 response.setSuccessfulDevices(null);
                 response.setFailedDevices(deviceIdentifierList);
-                throw new ApplicationManagementException("Error occurred while adding the application install "
+                throw new ApplicationManagementException(
-                        + "operation to devices", e);
+                        "Error occurred while adding the application install " + "operation to devices", e);
             } catch (InvalidDeviceException e) {
                 //This exception should not occur because the validation has already been done.
                 throw new ApplicationManagementException("The list of device identifiers are invalid");
@@ -541,7 +563,7 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
     /**
      * Validates the preconditions which is required to satisfy from the device which is required to install the
      * application.
-     *
+     * <p>
      * This method check two preconditions whether the application type is compatible to install in the device and
      * whether the device is enrolled in the system.
      *
@@ -572,5 +594,4 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
         return applicationInstallResponseTmp;
     }
 
-
 }

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

@@ -171,8 +171,6 @@ public class OperationManagerImpl implements OperationManager {
                         + PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain() + ", device count:"
                         + deviceIds.size() + " operation type:" + operation.getCode());
             }
-//            Map<Integer, List<DeviceIdentifier>> ignoredDeviceIdentifierMap = new HashMap<>();
-//            Map<Integer, List<DeviceIdentifier>> authorizedDeviceIdentifierMap = new HashMap<>();
             DeviceIDHolder deviceAuthorizationResult = this.authorizeDevices(operation, validDeviceIds);
             List<DeviceIdentifier> authorizedDeviceIds = deviceAuthorizationResult.getValidDeviceIDList();
             if (authorizedDeviceIds.isEmpty()) {
@@ -221,17 +219,6 @@ public class OperationManagerImpl implements OperationManager {
                         ignoredDevices.add(device);
                         operation.setId(existingOperationID);
                         this.sendNotification(operation, device);
-//                        List<DeviceIdentifier> deviceIdentifiers;
-//                        if (ignoredDeviceIdentifierMap.containsKey(existingOperationID)) {
-//                            deviceIdentifiers = ignoredDeviceIdentifierMap.get(existingOperationID);
-//                        } else {
-//                            deviceIdentifiers = new ArrayList<>();
-//                        }
-//                        DeviceIdentifier deviceIdentifier = new DeviceIdentifier();
-//                        deviceIdentifier.setType(device.getType());
-//                        deviceIdentifier.setId(device.getDeviceIdentifier());
-//                        deviceIdentifiers.add(deviceIdentifier);
-//                        ignoredDeviceIdentifierMap.put(existingOperationID, deviceIdentifiers);
                     }
                 }
             }
@@ -266,20 +253,12 @@ public class OperationManagerImpl implements OperationManager {
             }
 
             //TODO have to create a sql to load device details from deviceDAO using single query.
-            List<DeviceIdentifier> authorizedDeviceIdentifiers = new ArrayList<>();
             for (Device device : authorizedDevices) {
                 enrolmentId = device.getEnrolmentInfo().getId();
                 //Do not repeat the task operations
                 operationMappingDAO.addOperationMapping(operationId, enrolmentId, isScheduled);
-//                DeviceIdentifier deviceIdentifier = new DeviceIdentifier();
-//                deviceIdentifier.setId(device.getDeviceIdentifier());
-//                deviceIdentifier.setType(device.getType());
-//                authorizedDeviceIdentifiers.add(deviceIdentifier);
             }
-
-//            authorizedDeviceIdentifierMap.put(operationId, authorizedDeviceIdentifiers);
             OperationManagementDAOFactory.commitTransaction();
-
             if (!isScheduled) {
                 for (Device device : authorizedDevices) {
                     this.sendNotification(operation, device);

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

@@ -20,13 +20,6 @@ package org.wso2.carbon.device.mgt.core.operation.mgt;
 
 public class OperationMgtConstants {
 
-    public final class DeviceConstants {
-        private DeviceConstants() {
-        }
-
-        public static final String DEVICE_ID_NOT_FOUND = "Device not found for device id: %s";
-    }
-
     public final class OperationCodes {
         private OperationCodes() {
         }

components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/util/DeviceManagerUtil.java

@@ -463,25 +463,16 @@ public final class DeviceManagerUtil {
     }
 
     public static DeviceIDHolder validateDeviceIdentifiers(List<DeviceIdentifier> deviceIDs) {
-
         List<DeviceIdentifier> errorDeviceIdList = new ArrayList<>();
         List<DeviceIdentifier> validDeviceIDList = new ArrayList<>();
-
-        int deviceIDCounter = 0;
         for (DeviceIdentifier deviceIdentifier : deviceIDs) {
-
-            deviceIDCounter++;
             String deviceID = deviceIdentifier.getId();
-
             if (deviceID == null || deviceID.isEmpty()) {
-                log.warn(String.format(OperationMgtConstants.DeviceConstants.DEVICE_ID_NOT_FOUND,
+                log.warn("When adding operation for devices, found a device identifiers which doesn't have defined "
-                        deviceIDCounter));
+                        + "the identity of the device, with the request. Hence ignoring the device identifier.");
-                errorDeviceIdList.add(deviceIdentifier);
                 continue;
             }
-
             try {
-
                 if (isValidDeviceIdentifier(deviceIdentifier)) {
                     validDeviceIDList.add(deviceIdentifier);
                 } else {
@@ -491,7 +482,6 @@ public final class DeviceManagerUtil {
                 errorDeviceIdList.add(deviceIdentifier);
             }
         }
-
         DeviceIDHolder deviceIDHolder = new DeviceIDHolder();
         deviceIDHolder.setValidDeviceIDList(validDeviceIDList);
         deviceIDHolder.setErrorDeviceIdList(errorDeviceIdList);
@@ -505,10 +495,8 @@ public final class DeviceManagerUtil {
         if (device == null || device.getDeviceIdentifier() == null ||
                 device.getDeviceIdentifier().isEmpty() || device.getEnrolmentInfo() == null) {
             return false;
-        } else if (EnrolmentInfo.Status.REMOVED.equals(device.getEnrolmentInfo().getStatus())) {
+        } else
-            return false;
+            return !EnrolmentInfo.Status.REMOVED.equals(device.getEnrolmentInfo().getStatus());
-        }
-        return true;
     }
 
     public static boolean isDeviceExists(DeviceIdentifier deviceIdentifier) throws DeviceManagementException {