From 347fac7f009e0cefbdf0b238d1b0878b5a176e0f Mon Sep 17 00:00:00 2001 From: Stefan Klotz <35995139+StefanKlt@users.noreply.github.com> Date: Fri, 30 Nov 2018 09:05:18 +0100 Subject: [PATCH] Fix empty collection saved in ds table (#768) * check if collection is empty, clean up and refactor code * fix map and set types * fix array conversion * fix review findings Signed-off-by: Stefan Klotz --- .../common/entity/SoftwareModuleIdName.java | 24 ++- .../dstable/DistributionSetTable.java | 161 +++++++++--------- .../state/ManageDistUIState.java | 8 +- .../hawkbit/ui/utils/HawkbitCommonUtil.java | 9 + .../src/main/resources/messages.properties | 2 +- 5 files changed, 119 insertions(+), 85 deletions(-) diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/entity/SoftwareModuleIdName.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/entity/SoftwareModuleIdName.java index 17a6b58e8..2c8ee42d9 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/entity/SoftwareModuleIdName.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/entity/SoftwareModuleIdName.java @@ -22,16 +22,30 @@ public class SoftwareModuleIdName implements Serializable { private final Long id; private final String name; + private final String version; /** * @param id - * if the {@link SoftwareModule} + * of the {@link SoftwareModule#getId()} * @param name - * of the {@link SoftwareModule} + * of the {@link SoftwareModule#getName()} + * @param version + * the {@link SoftwareModule#getVersion()} */ - public SoftwareModuleIdName(final Long id, final String name) { + public SoftwareModuleIdName(final Long id, final String name, final String version) { this.id = id; this.name = name; + this.version = version; + } + + /** + * Constructor. + * + * @param softwareModule + * the softwareModule + */ + public SoftwareModuleIdName(final SoftwareModule softwareModule) { + this(softwareModule.getId(), softwareModule.getName(), softwareModule.getVersion()); } public Long getId() { @@ -42,6 +56,10 @@ public class SoftwareModuleIdName implements Serializable { return name; } + public String getVersion() { + return version; + } + @Override public int hashCode() {// NOSONAR - as this is generated final int prime = 31; diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/dstable/DistributionSetTable.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/dstable/DistributionSetTable.java index bd9174fbe..349628b04 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/dstable/DistributionSetTable.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/dstable/DistributionSetTable.java @@ -210,7 +210,7 @@ public class DistributionSetTable extends AbstractNamedVersionTable softwareModulesIdList, final long distId) { + private void handleSmToDsAssignment(final Set softwareModulesIdList, final long distId) { final Optional distributionSet = distributionSetManagement.get(distId); - distributionSet.ifPresent(set -> { - final DistributionSetIdName distributionSetIdName = new DistributionSetIdName(set); - selectDroppedEntities(distributionSetIdName.getId()); - final HashMap> map = createAssignmentMap(distributionSetIdName); - handleSoftwareModulesForAssignment(source, softwareModulesIdList, distId, map); - final HashSet softwareModules = new HashSet<>(); - map.keySet().forEach(typeId -> softwareModules.addAll(map.get(typeId))); - manageDistUIState.getAssignedList().put(distributionSetIdName, softwareModules); - openConfirmationWindowForAssignment(distributionSetIdName.getName(), - softwareModules.toArray(new SoftwareModuleIdName[softwareModules.size()])); - }); if (!distributionSet.isPresent()) { getNotification().displayWarning(getI18n().getMessage("distributionset.not.exists")); + return; } + distributionSet.ifPresent(ds -> { + selectDroppedEntities(ds.getId()); + final Set softwareModules = getAssignableSoftwareModules(softwareModulesIdList, ds); + if (softwareModules.isEmpty()) { + return; + } + assignSoftwareModulesToDs(softwareModules, ds); + + openConfirmationWindowForAssignment(ds.getName(), + softwareModules.stream().map(SoftwareModuleIdName::new) + .toArray(size -> new SoftwareModuleIdName[size])); + }); } - private HashMap> createAssignmentMap( - final DistributionSetIdName distributionSetIdName) { - final HashMap> map; + private Set getAssignableSoftwareModules(final Set softwareModulesIds, + final DistributionSet distributionSet) { + final Set assignableModules = new HashSet<>(); + for (final Long softwareModuleId : softwareModulesIds) { + final Optional softwareModule = softwareModuleManagement.get(softwareModuleId); + softwareModule.ifPresent(sm -> { + if (validateAssignment(sm, distributionSet)){ + assignableModules.add(sm); + } + }); + } + return assignableModules; + } + + private void assignSoftwareModulesToDs(final Set softwareModules, + final DistributionSet distributionSet) { + addSoftwareModulesToConsolidatedDsAssignmentMap(distributionSet, softwareModules); + + final Set softwareModulesThatNeedToBeAssigned = new HashSet<>(); + getConsolidatedAssignmentMap(distributionSet).values() + .forEach(softwareModulesThatNeedToBeAssigned::addAll); + + manageDistUIState.getAssignedList().put(new DistributionSetIdName(distributionSet), + softwareModulesThatNeedToBeAssigned); + } + + private Map> getConsolidatedAssignmentMap( + final DistributionSet distributionSet) { + final DistributionSetIdName distributionSetIdName = new DistributionSetIdName(distributionSet); + final Map> map; if (manageDistUIState.getConsolidatedDistSoftwareList().containsKey(distributionSetIdName)) { map = manageDistUIState.getConsolidatedDistSoftwareList().get(distributionSetIdName); } else { @@ -260,22 +289,24 @@ public class DistributionSetTable extends AbstractNamedVersionTable softwareModulesIdList, - final long distId, final HashMap> map) { - for (final Long softwareModuleId : softwareModulesIdList) { - final Item softwareItem = source.getContainerDataSource().getItem(softwareModuleId); - final String name = (String) softwareItem.getItemProperty(SPUILabelDefinitions.VAR_NAME).getValue(); - final String swVersion = (String) softwareItem.getItemProperty(SPUILabelDefinitions.VAR_VERSION).getValue(); + private void addSoftwareModulesToConsolidatedDsAssignmentMap(final DistributionSet distributionSet, + final Set softwareModules) { + final Map> assignmentMap = getConsolidatedAssignmentMap(distributionSet); + for (final SoftwareModule softwareModule : softwareModules) { + final SoftwareModuleIdName softwareModuleIdName = new SoftwareModuleIdName(softwareModule); + final Long smTypeID = softwareModule.getType().getId(); - final Optional softwareModule = softwareModuleManagement.get(softwareModuleId); + assignmentMap.computeIfAbsent(smTypeID, key -> new HashSet()); - if (softwareModule.isPresent() && validSoftwareModule(distId, softwareModule.get())) { - final SoftwareModuleIdName softwareModuleIdName = new SoftwareModuleIdName(softwareModuleId, - name.concat(":" + swVersion)); - publishAssignEvent(distId, softwareModule.get()); - handleSoftwareCase(map, softwareModule.get(), softwareModuleIdName); - handleFirmwareCase(map, softwareModule.get(), softwareModuleIdName); + if (softwareModule.getType().getMaxAssignments() > 1) { + // add application + assignmentMap.get(smTypeID).add(softwareModuleIdName); + } else if (softwareModule.getType().getMaxAssignments() == 1) { + // replace firmware + assignmentMap.get(smTypeID).clear(); + assignmentMap.get(smTypeID).add(softwareModuleIdName); } + publishAssignEvent(distributionSet.getId(), softwareModule); } } @@ -286,27 +317,6 @@ public class DistributionSetTable extends AbstractNamedVersionTable> map, - final SoftwareModule softwareModule, final SoftwareModuleIdName softwareModuleIdName) { - if (softwareModule.getType().getMaxAssignments() == 1) { - if (!map.containsKey(softwareModule.getType().getId())) { - map.put(softwareModule.getType().getId(), new HashSet()); - } - map.get(softwareModule.getType().getId()).clear(); - map.get(softwareModule.getType().getId()).add(softwareModuleIdName); - } - } - - private static void handleSoftwareCase(final Map> map, - final SoftwareModule softwareModule, final SoftwareModuleIdName softwareModuleIdName) { - if (softwareModule.getType().getMaxAssignments() > 1) { - if (!map.containsKey(softwareModule.getType().getId())) { - map.put(softwareModule.getType().getId(), new HashSet()); - } - map.get(softwareModule.getType().getId()).add(softwareModuleIdName); - } - } - private void openConfirmationWindowForAssignment(final String distributionNameToAssign, final SoftwareModuleIdName[] softwareModules) { final String confirmQuestion = createConfirmationMessageForAssignment(distributionNameToAssign, @@ -332,7 +342,8 @@ public class DistributionSetTable extends AbstractNamedVersionTable> entry : manageDistUIState + for (final Entry> entry : manageDistUIState .getAssignedList().entrySet()) { count += entry.getValue().size(); } @@ -367,58 +378,54 @@ public class DistributionSetTable extends AbstractNamedVersionTable ds = distributionSetManagement.getWithDetails(distId); - if (!ds.isPresent() || !validateSoftwareModule(sm, ds.get())) { - return false; - } - if (distributionSetManagement.isInUse(ds.get().getId())) { - getNotification().displayValidationError(getI18n().getMessage( - "message.error.notification.ds.target.assigned", ds.get().getName(), ds.get().getVersion())); + private boolean validateAssignment(final SoftwareModule sm, final DistributionSet ds) { + final String dsNameAndVersion = HawkbitCommonUtil.concatStrings(":", ds.getName(), ds.getVersion()); + final String smNameAndVersion = HawkbitCommonUtil.concatStrings(":", sm.getName(), sm.getVersion()); + if (!isSoftwareModuleDragged(ds.getId(), sm)) { return false; } - return true; - } - - private boolean validateSoftwareModule(final SoftwareModule sm, final DistributionSet ds) { - if (targetManagement.countByFilters(null, null, null, ds.getId(), Boolean.FALSE, new String[] {}) > 0) { + if (sm.getType().getMaxAssignments() < 1) { + return false; + } + if (targetManagement.countByFilters(null, null, null, ds.getId(), Boolean.FALSE, + new String[] {}) > 0) { /* Distribution is already assigned */ getNotification().displayValidationError(getI18n().getMessage("message.dist.inuse", - HawkbitCommonUtil.concatStrings(":", ds.getName(), ds.getVersion()))); + dsNameAndVersion)); return false; } - if (ds.getModules().contains(sm)) { /* Already has software module */ getNotification().displayValidationError(getI18n().getMessage("message.software.dist.already.assigned", - HawkbitCommonUtil.concatStrings(":", sm.getName(), sm.getVersion()), - HawkbitCommonUtil.concatStrings(":", ds.getName(), ds.getVersion()))); + smNameAndVersion, dsNameAndVersion)); return false; } - if (!ds.getType().containsModuleType(sm.getType())) { /* Invalid type of the software module */ getNotification().displayValidationError(getI18n().getMessage("message.software.dist.type.notallowed", - HawkbitCommonUtil.concatStrings(":", sm.getName(), sm.getVersion()), - HawkbitCommonUtil.concatStrings(":", ds.getName(), ds.getVersion()), sm.getType().getName())); + smNameAndVersion, dsNameAndVersion, sm.getType().getName())); + return false; + } + if (distributionSetManagement.isInUse(ds.getId())) { + getNotification() + .displayValidationError(getI18n().getMessage("message.error.notification.ds.target.assigned", + ds.getName(), ds.getVersion())); return false; } return true; } private boolean isSoftwareModuleDragged(final Long distId, final SoftwareModule sm) { - for (final Entry> entry : manageDistUIState + for (final Entry> entry : manageDistUIState .getAssignedList().entrySet()) { if (!distId.equals(entry.getKey().getId())) { continue; } final Set swModuleIdNames = entry.getValue(); for (final SoftwareModuleIdName swModuleIdName : swModuleIdNames) { - if ((sm.getName().concat(":" + sm.getVersion())).equals(swModuleIdName.getName())) { + if (sm.getName().equals(swModuleIdName.getName()) + && sm.getVersion().equals(swModuleIdName.getVersion())) { getNotification().displayValidationError(getI18n().getMessage("message.software.already.dragged", HawkbitCommonUtil.concatStrings(":", sm.getName(), sm.getVersion()))); return false; diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/state/ManageDistUIState.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/state/ManageDistUIState.java index 09a7f5008..a7eea12cd 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/state/ManageDistUIState.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/state/ManageDistUIState.java @@ -37,7 +37,7 @@ public class ManageDistUIState implements ManagementEntityState, Serializable { private final ManageSoftwareModuleFilters softwareModuleFilters; - private final Map> assignedList = new HashMap<>(); + private final Map> assignedList = new HashMap<>(); private final Set deletedDistributionList = new HashSet<>(); @@ -65,7 +65,7 @@ public class ManageDistUIState implements ManagementEntityState, Serializable { private final Map assignedSoftwareModuleDetails = new HashMap<>(); - private final Map>> consolidatedDistSoftwarewList = new HashMap<>(); + private final Map>> consolidatedDistSoftwarewList = new HashMap<>(); private boolean noDataAvilableSwModule; @@ -91,7 +91,7 @@ public class ManageDistUIState implements ManagementEntityState, Serializable { * * @return the assignedList */ - public Map> getAssignedList() { + public Map> getAssignedList() { return assignedList; } @@ -201,7 +201,7 @@ public class ManageDistUIState implements ManagementEntityState, Serializable { this.noDataAvailableDist = noDataAvailableDist; } - public Map>> getConsolidatedDistSoftwareList() { + public Map>> getConsolidatedDistSoftwareList() { return consolidatedDistSoftwarewList; } diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtil.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtil.java index a3fd5789e..40dc07fcb 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtil.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/utils/HawkbitCommonUtil.java @@ -269,6 +269,15 @@ public final class HawkbitCommonUtil { return new StringBuilder(distName).append(':').append(distVersion).toString(); } + /** + * @param smName + * @param smVersion + * @return SoftwareModuleNameAndVersion + */ + public static String getSoftwareModuleNameAndVersion(final String smName, final String smVersion) { + return new StringBuilder(smName).append(':').append(smVersion).toString(); + } + /** * Display Target Tag action message. * diff --git a/hawkbit-ui/src/main/resources/messages.properties b/hawkbit-ui/src/main/resources/messages.properties index 704831eaf..fba3de462 100644 --- a/hawkbit-ui/src/main/resources/messages.properties +++ b/hawkbit-ui/src/main/resources/messages.properties @@ -457,7 +457,7 @@ custom.created.date = Created Date label.drop.dist.delete.area = Drop here
to delete label.no.tag.assigned = NO TAG caption.assign.software.dist.accordion.tab = Assign Software Modules -message.software.assignment = {0} Software Module(s) Assignment(s) done +message.software.assignment = {0} Software Module Assignment(s) done message.dist.inuse = Distribution {0} is already assigned to target message.software.dist.already.assigned = Software Module {0} is already assigned to Distribution {1} message.software.dist.type.notallowed = Software Module {0} cannot be assigned, because Distribution {1} does not support the Software Module Type {2}