Improve app subscribing flow

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

@@ -26,6 +26,7 @@ public class SubscribingDeviceIdHolder {
     private Map<DeviceIdentifier, Integer> appInstalledDevices = new HashMap<>();
     private Map<DeviceIdentifier, Integer> appInstallableDevices = new HashMap<>();
     private Map<DeviceIdentifier, Integer> appReInstallableDevices = new HashMap<>();
+    private Map<DeviceIdentifier, Integer> skippedDevices = new HashMap<>();
 
     public Map<DeviceIdentifier, Integer> getAppInstalledDevices() {
         return appInstalledDevices;
@@ -50,4 +51,10 @@ public class SubscribingDeviceIdHolder {
     public void setAppReInstallableDevices(Map<DeviceIdentifier, Integer> appReInstallableDevices) {
         this.appReInstallableDevices = appReInstallableDevices;
     }
+
+    public Map<DeviceIdentifier, Integer> getSkippedDevices() { return skippedDevices; }
+
+    public void setSkippedDevices(Map<DeviceIdentifier, Integer> skippedDevices) {
+        this.skippedDevices = skippedDevices;
+    }
 }

components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/SubscriptionDAO.java

@@ -36,9 +36,8 @@ public interface SubscriptionDAO {
     List<Integer> addDeviceSubscription(String subscribedBy, List<Integer> deviceIds, String subscribedFrom,
             String installStatus, int releaseId, int tenantId ) throws ApplicationManagementDAOException;
 
-    List<Integer> updateDeviceSubscription(String updateBy, List<Integer> deviceIds, boolean isUnsubscribed,
+    void updateDeviceSubscription(String updateBy, List<Integer> deviceIds, String action, String actionTriggeredFrom,
-            String actionTriggeredFrom, String installStatus, int releaseId, int tenantId)
+            String installStatus, int releaseId, int tenantId) throws ApplicationManagementDAOException;
-            throws ApplicationManagementDAOException;
 
     void addOperationMapping (int operationId, List<Integer> deviceSubscriptionId, int tenantId) throws ApplicationManagementDAOException;
 
@@ -79,7 +78,7 @@ public interface SubscriptionDAO {
     void updateSubscriptions(int tenantId, String updateBy, List<String> paramList,
             int releaseId, String subType, String action) throws ApplicationManagementDAOException;
 
-    List<Integer> getSubscribedDeviceIds(List<Integer> deviceIds, int applicationReleaseId, int tenantId)
+    List<Integer> getDeviceSubIds(List<Integer> deviceIds, int applicationReleaseId, int tenantId)
             throws ApplicationManagementDAOException;
 
     List<Integer> getDeviceSubIdsForOperation (int operationId, int tenantId) throws ApplicationManagementDAOException;

components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/subscription/GenericSubscriptionDAOImpl.java

@@ -104,16 +104,20 @@ public class GenericSubscriptionDAOImpl extends AbstractDAOImpl implements Subsc
     }
 
     @Override
-    public List<Integer> updateDeviceSubscription(String updateBy, List<Integer> deviceIds,
+    public void updateDeviceSubscription(String updateBy, List<Integer> deviceIds,
-            boolean isUnsubscribed, String actionTriggeredFrom, String installStatus, int releaseId, int tenantId)
+            String action, String actionTriggeredFrom, String installStatus, int releaseId, int tenantId)
             throws ApplicationManagementDAOException {
         try {
             String sql = "UPDATE AP_DEVICE_SUBSCRIPTION SET ";
 
-            if (isUnsubscribed) {
+            if (SubAction.UNINSTALL.toString().equalsIgnoreCase(action)) {
                 sql += "UNSUBSCRIBED = true, UNSUBSCRIBED_BY = ?, UNSUBSCRIBED_TIMESTAMP = ?, ";
+            } else if (SubAction.INSTALL.toString().equalsIgnoreCase(action)) {
+                sql += "UNSUBSCRIBED = false, SUBSCRIBED_BY = ?, SUBSCRIBED_TIMESTAMP = ?, ";
             } else {
-                sql += "SUBSCRIBED_BY = ?, SUBSCRIBED_TIMESTAMP = ?, ";
+                String msg = "Found invalid action " + action + ". Hence can't construct the query.";
+                log.error(msg);
+                throw new ApplicationManagementDAOException(msg);
             }
             sql += "ACTION_TRIGGERED_FROM = ?, " +
                     "STATUS = ? " +
@@ -137,13 +141,6 @@ public class GenericSubscriptionDAOImpl extends AbstractDAOImpl implements Subsc
                     stmt.addBatch();
                 }
                 stmt.executeBatch();
-                try (ResultSet rs = stmt.getGeneratedKeys()){
-                    List<Integer> updatedDeviceSubIds = new ArrayList<>();
-                    while (rs.next()) {
-                        updatedDeviceSubIds.add(rs.getInt(1));
-                    }
-                    return updatedDeviceSubIds;
-                }
             }
         } catch (DBConnectionException e) {
             String msg = "Error occurred while obtaining the DB connection to update device subscriptions of "
@@ -546,7 +543,7 @@ public class GenericSubscriptionDAOImpl extends AbstractDAOImpl implements Subsc
     }
 
     @Override
-    public List<Integer> getSubscribedDeviceIds(List<Integer> deviceIds, int applicationReleaseId,
+    public List<Integer> getDeviceSubIds(List<Integer> deviceIds, int applicationReleaseId,
             int tenantId)
             throws ApplicationManagementDAOException {
         if (log.isDebugEnabled()) {
@@ -557,7 +554,7 @@ public class GenericSubscriptionDAOImpl extends AbstractDAOImpl implements Subsc
             int index = 1;
             List<Integer> subscribedDevices = new ArrayList<>();
             StringJoiner joiner = new StringJoiner(",",
-                    "SELECT DS.DM_DEVICE_ID "
+                    "SELECT DS.ID "
                             + "FROM AP_DEVICE_SUBSCRIPTION DS "
                             + "WHERE DS.DM_DEVICE_ID IN (", ") AND AP_APP_RELEASE_ID = ? AND TENANT_ID = ?");
             deviceIds.stream().map(ignored -> "?").forEach(joiner::add);
@@ -604,7 +601,7 @@ public class GenericSubscriptionDAOImpl extends AbstractDAOImpl implements Subsc
             if (SubAction.UNINSTALL.toString().equalsIgnoreCase(action)) {
                 sql += "UNSUBSCRIBED = true, UNSUBSCRIBED_BY = ?, UNSUBSCRIBED_TIMESTAMP = ? ";
             } else {
-                sql += "SUBSCRIBED_BY = ?, SUBSCRIBED_TIMESTAMP = ? ";
+                sql += "UNSUBSCRIBED = false, SUBSCRIBED_BY = ?, SUBSCRIBED_TIMESTAMP = ? ";
             }
 
             if (SubscriptionType.USER.toString().equalsIgnoreCase(subType)) {
@@ -650,7 +647,7 @@ public class GenericSubscriptionDAOImpl extends AbstractDAOImpl implements Subsc
             Connection conn = this.getDBConnection();
             List<Integer> deviceSubIds = new ArrayList<>();
             String sql = "SELECT "
-                    + "ID "
+                    + "AP_DEVICE_SUBSCRIPTION_ID "
                     + "FROM AP_APP_SUB_OP_MAPPING "
                     + "WHERE "
                     + "OPERATION_ID = ? AND "
@@ -660,7 +657,7 @@ public class GenericSubscriptionDAOImpl extends AbstractDAOImpl implements Subsc
                 stmt.setInt(2, tenantId);
                 try (ResultSet rs = stmt.executeQuery()) {
                     while (rs.next()) {
-                        deviceSubIds.add(rs.getInt("ID"));
+                        deviceSubIds.add(rs.getInt("AP_DEVICE_SUBSCRIPTION_ID"));
                     }
                 }
                 return deviceSubIds;

components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/impl/subscription/SQLServerSubscriptionDAOImpl.java

@@ -159,61 +159,6 @@ public class SQLServerSubscriptionDAOImpl extends GenericSubscriptionDAOImpl {
         }
     }
 
-    @Override
-    public List<Integer> updateDeviceSubscription(String updateBy, List<Integer> deviceIds,
-            boolean isUnsubscribed, String actionTriggeredFrom, String installStatus, int releaseId, int tenantId)
-            throws ApplicationManagementDAOException {
-        try {
-            String sql = "UPDATE AP_DEVICE_SUBSCRIPTION SET ";
-
-            if (isUnsubscribed) {
-                sql += "UNSUBSCRIBED = true, UNSUBSCRIBED_BY = ?, UNSUBSCRIBED_TIMESTAMP = ?, ";
-            } else {
-                sql += "SUBSCRIBED_BY = ?, SUBSCRIBED_TIMESTAMP = ?, ";
-            }
-            sql += "ACTION_TRIGGERED_FROM = ?, " +
-                    "STATUS = ? " +
-                    "WHERE " +
-                    "DM_DEVICE_ID = ? AND " +
-                    "AP_APP_RELEASE_ID = ? AND " +
-                    "TENANT_ID = ?";
-
-            Connection conn = this.getDBConnection();
-            try (PreparedStatement stmt = conn.prepareStatement(sql)) {
-                Calendar calendar = Calendar.getInstance();
-                Timestamp timestamp = new Timestamp(calendar.getTime().getTime());
-                List<Integer> updatedDeviceSubIds = new ArrayList<>();
-                for (Integer deviceId : deviceIds) {
-                    stmt.setString(1, updateBy);
-                    stmt.setTimestamp(2, timestamp);
-                    stmt.setString(3, actionTriggeredFrom);
-                    stmt.setString(4, installStatus);
-                    stmt.setInt(5, deviceId);
-                    stmt.setInt(6, releaseId);
-                    stmt.setInt(7, tenantId);
-                    stmt.executeUpdate();
-                    try (ResultSet rs = stmt.getGeneratedKeys()) {
-                        if (rs.next()) {
-                            updatedDeviceSubIds.add(rs.getInt(1));
-                        }
-                    }
-                }
-                return updatedDeviceSubIds;
-            }
-        } catch (DBConnectionException e) {
-            String msg = "Error occurred while obtaining the DB connection to update device subscriptions of "
-                    + "application. Updated by: " + updateBy + " and updating action triggered from "
-                    + actionTriggeredFrom;
-            log.error(msg, e);
-            throw new ApplicationManagementDAOException(msg, e);
-        } catch (SQLException e) {
-            String msg = "Error occurred while executing SQL to update the device subscriptions of application. "
-                    + "Updated by: " + updateBy + " and updating action triggered from " + actionTriggeredFrom;
-            log.error(msg, e);
-            throw new ApplicationManagementDAOException(msg, e);
-        }
-    }
-
     @Override
     public List<Integer> addDeviceSubscription(String subscribedBy, List<Integer> deviceIds,
             String subscribedFrom, String installStatus, int releaseId, int tenantId)

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

@@ -438,6 +438,7 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
         Map<DeviceIdentifier, Integer> appInstalledDevices = new HashMap<>();
         Map<DeviceIdentifier, Integer> appInstallableDevices = new HashMap<>();
         Map<DeviceIdentifier, Integer> appReInstallableDevices = new HashMap<>();
+        Map<DeviceIdentifier, Integer> skippedDevices = new HashMap<>();
 
         List<Integer> deviceIds = devices.stream().map(Device::getId).collect(Collectors.toList());
         //get device subscriptions for given device id list.
@@ -449,6 +450,9 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
                 if (!deviceSubscriptionDTO.isUnsubscribed() && Operation.Status.COMPLETED.toString()
                         .equals(deviceSubscriptionDTO.getStatus())) {
                     appInstalledDevices.put(deviceIdentifier, device.getId());
+                } else if (Operation.Status.PENDING.toString().equals(deviceSubscriptionDTO.getStatus())
+                        || Operation.Status.IN_PROGRESS.toString().equals(deviceSubscriptionDTO.getStatus())) {
+                    skippedDevices.put(deviceIdentifier, device.getId());
                 } else {
                     appReInstallableDevices.put(deviceIdentifier, device.getId());
                 }
@@ -461,6 +465,7 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
         subscribingDeviceIdHolder.setAppInstallableDevices(appInstallableDevices);
         subscribingDeviceIdHolder.setAppInstalledDevices(appInstalledDevices);
         subscribingDeviceIdHolder.setAppReInstallableDevices(appReInstallableDevices);
+        subscribingDeviceIdHolder.setSkippedDevices(skippedDevices);
         return subscribingDeviceIdHolder;
     }
 
@@ -530,8 +535,6 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
         String username = PrivilegedCarbonContext.getThreadLocalCarbonContext().getUsername();
         try {
             ConnectionManagerUtil.beginDBTransaction();
-            List<Integer> deviceSubIds = new ArrayList<>();
-
             if (SubscriptionType.USER.toString().equalsIgnoreCase(subType)) {
                 List<String> subscribedEntities = subscriptionDAO.getSubscribedUserNames(params, tenantId);
                 if (SubAction.INSTALL.toString().equalsIgnoreCase(action)) {
@@ -560,29 +563,33 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
 
             for (Activity activity : activities) {
                 int operationId = Integer.parseInt(activity.getActivityId().split("ACTIVITY_")[1]);
+                List<Integer> subUpdatingDeviceIds = new ArrayList<>();
+                List<Integer> subInsertingDeviceIds = new ArrayList<>();
+                List<Integer> deviceSubIds = new ArrayList<>();
+
                 if (SubAction.INSTALL.toString().equalsIgnoreCase(action)) {
-                    List<Integer> alreadyUnsubscribedDevices = getOperationAddedDeviceIds(activity,
+                    subUpdatingDeviceIds.addAll(getOperationAddedDeviceIds(activity,
-                            subscribingDeviceIdHolder.getAppReInstallableDevices());
+                            subscribingDeviceIdHolder.getAppReInstallableDevices()));
-                    List<Integer> deviceIdsOfNewSubscriptions = getOperationAddedDeviceIds(activity,
+                    subInsertingDeviceIds.addAll(getOperationAddedDeviceIds(activity,
-                            subscribingDeviceIdHolder.getAppInstallableDevices());
+                            subscribingDeviceIdHolder.getAppInstallableDevices()));
-                    if (!alreadyUnsubscribedDevices.isEmpty()) {
+
-                        List<Integer> deviceResubscribingIds = subscriptionDAO
-                                .updateDeviceSubscription(username, alreadyUnsubscribedDevices, false, subType,
-                                        Operation.Status.PENDING.toString(), applicationReleaseId, tenantId);
-                        deviceSubIds.addAll(deviceResubscribingIds);
-                    }
-                    List<Integer> subscribingDevices = subscriptionDAO
-                            .addDeviceSubscription(username, deviceIdsOfNewSubscriptions, subType,
-                                    Operation.Status.PENDING.toString(), applicationReleaseId, tenantId);
-                    deviceSubIds.addAll(subscribingDevices);
                 } else if (SubAction.UNINSTALL.toString().equalsIgnoreCase(action)) {
-                    List<Integer> alreadySubscribedDevices = getOperationAddedDeviceIds(activity,
+                    subUpdatingDeviceIds.addAll(getOperationAddedDeviceIds(activity,
-                            subscribingDeviceIdHolder.getAppInstalledDevices());
+                            subscribingDeviceIdHolder.getAppInstalledDevices()));
-                    List<Integer> deviceResubscribingIds = subscriptionDAO
-                            .updateDeviceSubscription(username, alreadySubscribedDevices, true, subType,
-                                    Operation.Status.PENDING.toString(), applicationReleaseId, tenantId);
-                    deviceSubIds.addAll(deviceResubscribingIds);
                 }
+
+                List<Integer> subscribingDevices = subscriptionDAO
+                        .addDeviceSubscription(username, subInsertingDeviceIds, subType,
+                                Operation.Status.PENDING.toString(), applicationReleaseId, tenantId);
+                subscriptionDAO.updateDeviceSubscription(username, subUpdatingDeviceIds, action, subType,
+                        Operation.Status.PENDING.toString(), applicationReleaseId, tenantId);
+
+                if (!subUpdatingDeviceIds.isEmpty()) {
+                    deviceSubIds.addAll(subscriptionDAO
+                            .getDeviceSubIds(subUpdatingDeviceIds, applicationReleaseId, tenantId));
+                }
+                deviceSubIds.addAll(subscribingDevices);
+
                 subscriptionDAO.addOperationMapping(operationId, deviceSubIds, tenantId);
             }
             ConnectionManagerUtil.commitDBTransaction();
@@ -606,6 +613,13 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
         }
     }
 
+    /**
+     * This method is responsible to get device IDs thta operation has added.
+     *
+     * @param activity Activity
+     * @param deviceMap Device map, key is device identifier and value is primary key of device.
+     * @return List of device primary keys
+     */
     private List<Integer> getOperationAddedDeviceIds(Activity activity, Map<DeviceIdentifier, Integer> deviceMap) {
         List<ActivityStatus> activityStatuses = activity.getActivityStatus();
         return activityStatuses.stream()