Improve app subscribing flow

2025-10-06 02:01:45 +00:00 · 2019-10-19 10:58:15 +05:30 · 2019-10-19 10:58:15 +05:30 · 60947eb1e8
commit 60947eb1e8
parent d1e6aab5cd
5 changed files with 59 additions and 97 deletions
--- 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
@ -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;
+    }
 }
--- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/dao/SubscriptionDAO.java
+++ b/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,
-            String actionTriggeredFrom, String installStatus, int releaseId, int tenantId)
-            throws ApplicationManagementDAOException;
+    void updateDeviceSubscription(String updateBy, List<Integer> deviceIds, String action, String actionTriggeredFrom,
+            String installStatus, int releaseId, int tenantId) 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;
--- a/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
+++ b/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,
-            boolean isUnsubscribed, String actionTriggeredFrom, String installStatus, int releaseId, int tenantId)
+    public void updateDeviceSubscription(String updateBy, List<Integer> deviceIds,
+            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;
--- a/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
+++ b/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)
--- 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
@ -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,
-                            subscribingDeviceIdHolder.getAppReInstallableDevices());
-                    List<Integer> deviceIdsOfNewSubscriptions = getOperationAddedDeviceIds(activity,
-                            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);
+                    subUpdatingDeviceIds.addAll(getOperationAddedDeviceIds(activity,
+                            subscribingDeviceIdHolder.getAppReInstallableDevices()));
+                    subInsertingDeviceIds.addAll(getOperationAddedDeviceIds(activity,
+                            subscribingDeviceIdHolder.getAppInstallableDevices()));
+
                } else if (SubAction.UNINSTALL.toString().equalsIgnoreCase(action)) {
-                    List<Integer> alreadySubscribedDevices = getOperationAddedDeviceIds(activity,
-                            subscribingDeviceIdHolder.getAppInstalledDevices());
-                    List<Integer> deviceResubscribingIds = subscriptionDAO
-                            .updateDeviceSubscription(username, alreadySubscribedDevices, true, subType,
-                                    Operation.Status.PENDING.toString(), applicationReleaseId, tenantId);
-                    deviceSubIds.addAll(deviceResubscribingIds);
+                    subUpdatingDeviceIds.addAll(getOperationAddedDeviceIds(activity,
+                            subscribingDeviceIdHolder.getAppInstalledDevices()));
                }
+
+                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()