From 60947eb1e8b42333086977115e08114f2ae27639 Mon Sep 17 00:00:00 2001 From: lasanthaDLPDS Date: Sat, 19 Oct 2019 10:58:15 +0530 Subject: [PATCH] Improve app subscribing flow --- .../mgt/common/SubscribingDeviceIdHolder.java | 7 +++ .../mgt/core/dao/SubscriptionDAO.java | 7 +-- .../GenericSubscriptionDAOImpl.java | 29 +++++----- .../SQLServerSubscriptionDAOImpl.java | 55 ------------------ .../core/impl/SubscriptionManagerImpl.java | 58 ++++++++++++------- 5 files changed, 59 insertions(+), 97 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 64db67a3c2d..eb92f07bc06 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 @@ -26,6 +26,7 @@ public class SubscribingDeviceIdHolder { private Map appInstalledDevices = new HashMap<>(); private Map appInstallableDevices = new HashMap<>(); private Map appReInstallableDevices = new HashMap<>(); + private Map skippedDevices = new HashMap<>(); public Map getAppInstalledDevices() { return appInstalledDevices; @@ -50,4 +51,10 @@ public class SubscribingDeviceIdHolder { public void setAppReInstallableDevices(Map appReInstallableDevices) { this.appReInstallableDevices = appReInstallableDevices; } + + public Map getSkippedDevices() { return skippedDevices; } + + public void setSkippedDevices(Map skippedDevices) { + this.skippedDevices = skippedDevices; + } } diff --git 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 index a712907ac87..33fafe57dbe 100644 --- 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 addDeviceSubscription(String subscribedBy, List deviceIds, String subscribedFrom, String installStatus, int releaseId, int tenantId ) throws ApplicationManagementDAOException; - List updateDeviceSubscription(String updateBy, List deviceIds, boolean isUnsubscribed, - String actionTriggeredFrom, String installStatus, int releaseId, int tenantId) - throws ApplicationManagementDAOException; + void updateDeviceSubscription(String updateBy, List deviceIds, String action, String actionTriggeredFrom, + String installStatus, int releaseId, int tenantId) throws ApplicationManagementDAOException; void addOperationMapping (int operationId, List deviceSubscriptionId, int tenantId) throws ApplicationManagementDAOException; @@ -79,7 +78,7 @@ public interface SubscriptionDAO { void updateSubscriptions(int tenantId, String updateBy, List paramList, int releaseId, String subType, String action) throws ApplicationManagementDAOException; - List getSubscribedDeviceIds(List deviceIds, int applicationReleaseId, int tenantId) + List getDeviceSubIds(List deviceIds, int applicationReleaseId, int tenantId) throws ApplicationManagementDAOException; List getDeviceSubIdsForOperation (int operationId, int tenantId) throws ApplicationManagementDAOException; diff --git 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 index aaafcec75c2..ace7d41f75e 100644 --- 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 updateDeviceSubscription(String updateBy, List deviceIds, - boolean isUnsubscribed, String actionTriggeredFrom, String installStatus, int releaseId, int tenantId) + public void updateDeviceSubscription(String updateBy, List 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 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 getSubscribedDeviceIds(List deviceIds, int applicationReleaseId, + public List getDeviceSubIds(List 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 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 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; diff --git 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 index 7900c36acbb..dde186109cc 100644 --- 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 updateDeviceSubscription(String updateBy, List 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 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 addDeviceSubscription(String subscribedBy, List deviceIds, String subscribedFrom, String installStatus, int releaseId, int tenantId) 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 c59407b1baf..ca294e7495e 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 @@ -438,6 +438,7 @@ public class SubscriptionManagerImpl implements SubscriptionManager { Map appInstalledDevices = new HashMap<>(); Map appInstallableDevices = new HashMap<>(); Map appReInstallableDevices = new HashMap<>(); + Map skippedDevices = new HashMap<>(); List 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 deviceSubIds = new ArrayList<>(); - if (SubscriptionType.USER.toString().equalsIgnoreCase(subType)) { List 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 subUpdatingDeviceIds = new ArrayList<>(); + List subInsertingDeviceIds = new ArrayList<>(); + List deviceSubIds = new ArrayList<>(); + if (SubAction.INSTALL.toString().equalsIgnoreCase(action)) { - List alreadyUnsubscribedDevices = getOperationAddedDeviceIds(activity, - subscribingDeviceIdHolder.getAppReInstallableDevices()); - List deviceIdsOfNewSubscriptions = getOperationAddedDeviceIds(activity, - subscribingDeviceIdHolder.getAppInstallableDevices()); - if (!alreadyUnsubscribedDevices.isEmpty()) { - List deviceResubscribingIds = subscriptionDAO - .updateDeviceSubscription(username, alreadyUnsubscribedDevices, false, subType, - Operation.Status.PENDING.toString(), applicationReleaseId, tenantId); - deviceSubIds.addAll(deviceResubscribingIds); - } - List 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 alreadySubscribedDevices = getOperationAddedDeviceIds(activity, - subscribingDeviceIdHolder.getAppInstalledDevices()); - List deviceResubscribingIds = subscriptionDAO - .updateDeviceSubscription(username, alreadySubscribedDevices, true, subType, - Operation.Status.PENDING.toString(), applicationReleaseId, tenantId); - deviceSubIds.addAll(deviceResubscribingIds); + subUpdatingDeviceIds.addAll(getOperationAddedDeviceIds(activity, + subscribingDeviceIdHolder.getAppInstalledDevices())); } + + List 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 getOperationAddedDeviceIds(Activity activity, Map deviceMap) { List activityStatuses = activity.getActivityStatus(); return activityStatuses.stream()