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 <stefan.klotz@bosch-si.com>

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;

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/dstable/DistributionSetTable.java

@@ -210,7 +210,7 @@ public class DistributionSetTable extends AbstractNamedVersionTable<Distribution
 
         final Object distItemId = dropData.getItemIdOver();
         if (distItemId != null) {
-            assignSwmToDs(source, softwareModulesIdList, (long) distItemId);
+            handleSmToDsAssignment(softwareModulesIdList, (long) distItemId);
         }
     }
 
@@ -230,27 +230,56 @@ public class DistributionSetTable extends AbstractNamedVersionTable<Distribution
         return super.isDropValid(dragEvent);
     }
 
-    private void assignSwmToDs(final Table source, final Set<Long> softwareModulesIdList, final long distId) {
+    private void handleSmToDsAssignment(final Set<Long> softwareModulesIdList, final long distId) {
         final Optional<DistributionSet> distributionSet = distributionSetManagement.get(distId);
-        distributionSet.ifPresent(set -> {
-            final DistributionSetIdName distributionSetIdName = new DistributionSetIdName(set);
-            selectDroppedEntities(distributionSetIdName.getId());
-            final HashMap<Long, HashSet<SoftwareModuleIdName>> map = createAssignmentMap(distributionSetIdName);
-            handleSoftwareModulesForAssignment(source, softwareModulesIdList, distId, map);
-            final HashSet<SoftwareModuleIdName> 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<SoftwareModule> 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<Long, HashSet<SoftwareModuleIdName>> createAssignmentMap(
+    private Set<SoftwareModule> getAssignableSoftwareModules(final Set<Long> softwareModulesIds,
-            final DistributionSetIdName distributionSetIdName) {
+            final DistributionSet distributionSet) {
-        final HashMap<Long, HashSet<SoftwareModuleIdName>> map;
+        final Set<SoftwareModule> assignableModules = new HashSet<>();
+        for (final Long softwareModuleId : softwareModulesIds) {
+            final Optional<SoftwareModule> softwareModule = softwareModuleManagement.get(softwareModuleId);
+            softwareModule.ifPresent(sm -> {
+                if (validateAssignment(sm, distributionSet)){
+                    assignableModules.add(sm);
+                }
+            });
+        }
+        return assignableModules;
+    }
+
+    private void assignSoftwareModulesToDs(final Set<SoftwareModule> softwareModules,
+            final DistributionSet distributionSet) {
+        addSoftwareModulesToConsolidatedDsAssignmentMap(distributionSet, softwareModules);
+
+        final Set<SoftwareModuleIdName> softwareModulesThatNeedToBeAssigned = new HashSet<>();
+        getConsolidatedAssignmentMap(distributionSet).values()
+                .forEach(softwareModulesThatNeedToBeAssigned::addAll);
+
+        manageDistUIState.getAssignedList().put(new DistributionSetIdName(distributionSet),
+                softwareModulesThatNeedToBeAssigned);
+    }
+
+    private Map<Long, Set<SoftwareModuleIdName>> getConsolidatedAssignmentMap(
+            final DistributionSet distributionSet) {
+        final DistributionSetIdName distributionSetIdName = new DistributionSetIdName(distributionSet);
+        final Map<Long, Set<SoftwareModuleIdName>> map;
         if (manageDistUIState.getConsolidatedDistSoftwareList().containsKey(distributionSetIdName)) {
             map = manageDistUIState.getConsolidatedDistSoftwareList().get(distributionSetIdName);
         } else {
@@ -260,22 +289,24 @@ public class DistributionSetTable extends AbstractNamedVersionTable<Distribution
         return map;
     }
 
-    private void handleSoftwareModulesForAssignment(final Table source, final Set<Long> softwareModulesIdList,
+    private void addSoftwareModulesToConsolidatedDsAssignmentMap(final DistributionSet distributionSet,
-            final long distId, final HashMap<Long, HashSet<SoftwareModuleIdName>> map) {
+            final Set<SoftwareModule> softwareModules) {
-        for (final Long softwareModuleId : softwareModulesIdList) {
+        final Map<Long, Set<SoftwareModuleIdName>> assignmentMap = getConsolidatedAssignmentMap(distributionSet);
-            final Item softwareItem = source.getContainerDataSource().getItem(softwareModuleId);
+        for (final SoftwareModule softwareModule : softwareModules) {
-            final String name = (String) softwareItem.getItemProperty(SPUILabelDefinitions.VAR_NAME).getValue();
+            final SoftwareModuleIdName softwareModuleIdName = new SoftwareModuleIdName(softwareModule);
-            final String swVersion = (String) softwareItem.getItemProperty(SPUILabelDefinitions.VAR_VERSION).getValue();
+            final Long smTypeID = softwareModule.getType().getId();
 
-            final Optional<SoftwareModule> softwareModule = softwareModuleManagement.get(softwareModuleId);
+            assignmentMap.computeIfAbsent(smTypeID, key -> new HashSet<SoftwareModuleIdName>());
 
-            if (softwareModule.isPresent() && validSoftwareModule(distId, softwareModule.get())) {
+            if (softwareModule.getType().getMaxAssignments() > 1) {
-                final SoftwareModuleIdName softwareModuleIdName = new SoftwareModuleIdName(softwareModuleId,
+                // add application
-                        name.concat(":" + swVersion));
+                assignmentMap.get(smTypeID).add(softwareModuleIdName);
-                publishAssignEvent(distId, softwareModule.get());
+            } else if (softwareModule.getType().getMaxAssignments() == 1) {
-                handleSoftwareCase(map, softwareModule.get(), softwareModuleIdName);
+                // replace firmware
-                handleFirmwareCase(map, softwareModule.get(), softwareModuleIdName);
+                assignmentMap.get(smTypeID).clear();
+                assignmentMap.get(smTypeID).add(softwareModuleIdName);
             }
+            publishAssignEvent(distributionSet.getId(), softwareModule);
         }
     }
 
@@ -286,27 +317,6 @@ public class DistributionSetTable extends AbstractNamedVersionTable<Distribution
         }
     }
 
-    private static void handleFirmwareCase(final Map<Long, HashSet<SoftwareModuleIdName>> 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<SoftwareModuleIdName>());
-            }
-            map.get(softwareModule.getType().getId()).clear();
-            map.get(softwareModule.getType().getId()).add(softwareModuleIdName);
-        }
-    }
-
-    private static void handleSoftwareCase(final Map<Long, HashSet<SoftwareModuleIdName>> 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<SoftwareModuleIdName>());
-            }
-            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<Distribution
             final SoftwareModuleIdName[] softwareModules) {
         if (softwareModules.length == 1) {
             return getI18n().getMessage(MESSAGE_CONFIRM_ASSIGN_ENTITY, distributionNameToAssign, "software module",
-                    softwareModules[0].getName());
+                    HawkbitCommonUtil.getSoftwareModuleNameAndVersion(softwareModules[0].getName(),
+                            softwareModules[0].getVersion()));
         } else {
             return getI18n().getMessage(MESSAGE_CONFIRM_ASSIGN_MULTIPLE_ENTITIES, softwareModules.length,
                     "software modules", distributionNameToAssign);
@@ -347,7 +358,7 @@ public class DistributionSetTable extends AbstractNamedVersionTable<Distribution
         });
 
         int count = 0;
-        for (final Entry<DistributionSetIdName, HashSet<SoftwareModuleIdName>> entry : manageDistUIState
+        for (final Entry<DistributionSetIdName, Set<SoftwareModuleIdName>> entry : manageDistUIState
                 .getAssignedList().entrySet()) {
             count += entry.getValue().size();
         }
@@ -367,58 +378,54 @@ public class DistributionSetTable extends AbstractNamedVersionTable<Distribution
         manageDistUIState.getConsolidatedDistSoftwareList().clear();
     }
 
-    private boolean validSoftwareModule(final Long distId, final SoftwareModule sm) {
-        if (!isSoftwareModuleDragged(distId, sm)) {
-            return false;
-        }
-        final Optional<DistributionSet> ds = distributionSetManagement.getWithDetails(distId);
-        if (!ds.isPresent() || !validateSoftwareModule(sm, ds.get())) {
-            return false;
-        }
 
-        if (distributionSetManagement.isInUse(ds.get().getId())) {
+    private boolean validateAssignment(final SoftwareModule sm, final DistributionSet ds) {
-            getNotification().displayValidationError(getI18n().getMessage(
+        final String dsNameAndVersion = HawkbitCommonUtil.concatStrings(":", ds.getName(), ds.getVersion());
-                    "message.error.notification.ds.target.assigned", ds.get().getName(), ds.get().getVersion()));
+        final String smNameAndVersion = HawkbitCommonUtil.concatStrings(":", sm.getName(), sm.getVersion());
+        if (!isSoftwareModuleDragged(ds.getId(), sm)) {
             return false;
         }
-        return true;
+        if (sm.getType().getMaxAssignments() < 1) {
-    }
+            return false;
-
+        }
-    private boolean validateSoftwareModule(final SoftwareModule sm, final DistributionSet ds) {
+        if (targetManagement.countByFilters(null, null, null, ds.getId(), Boolean.FALSE,
-        if (targetManagement.countByFilters(null, null, null, ds.getId(), Boolean.FALSE, new String[] {}) > 0) {
+                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()),
+                    smNameAndVersion, dsNameAndVersion));
-                    HawkbitCommonUtil.concatStrings(":", ds.getName(), ds.getVersion())));
             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()),
+                    smNameAndVersion, dsNameAndVersion, sm.getType().getName()));
-                    HawkbitCommonUtil.concatStrings(":", ds.getName(), ds.getVersion()), 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<DistributionSetIdName, HashSet<SoftwareModuleIdName>> entry : manageDistUIState
+        for (final Entry<DistributionSetIdName, Set<SoftwareModuleIdName>> entry : manageDistUIState
                 .getAssignedList().entrySet()) {
             if (!distId.equals(entry.getKey().getId())) {
                 continue;
             }
             final Set<SoftwareModuleIdName> 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;

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<DistributionSetIdName, HashSet<SoftwareModuleIdName>> assignedList = new HashMap<>();
+    private final Map<DistributionSetIdName, Set<SoftwareModuleIdName>> assignedList = new HashMap<>();
 
     private final Set<DistributionSetIdName> deletedDistributionList = new HashSet<>();
 
@@ -65,7 +65,7 @@ public class ManageDistUIState implements ManagementEntityState, Serializable {
 
     private final Map<String, SoftwareModuleIdName> assignedSoftwareModuleDetails = new HashMap<>();
 
-    private final Map<DistributionSetIdName, HashMap<Long, HashSet<SoftwareModuleIdName>>> consolidatedDistSoftwarewList = new HashMap<>();
+    private final Map<DistributionSetIdName, Map<Long, Set<SoftwareModuleIdName>>> consolidatedDistSoftwarewList = new HashMap<>();
 
     private boolean noDataAvilableSwModule;
 
@@ -91,7 +91,7 @@ public class ManageDistUIState implements ManagementEntityState, Serializable {
      *
      * @return the assignedList
      */
-    public Map<DistributionSetIdName, HashSet<SoftwareModuleIdName>> getAssignedList() {
+    public Map<DistributionSetIdName, Set<SoftwareModuleIdName>> getAssignedList() {
         return assignedList;
     }
 
@@ -201,7 +201,7 @@ public class ManageDistUIState implements ManagementEntityState, Serializable {
         this.noDataAvailableDist = noDataAvailableDist;
     }
 
-    public Map<DistributionSetIdName, HashMap<Long, HashSet<SoftwareModuleIdName>>> getConsolidatedDistSoftwareList() {
+    public Map<DistributionSetIdName, Map<Long, Set<SoftwareModuleIdName>>> getConsolidatedDistSoftwareList() {
         return consolidatedDistSoftwarewList;
     }
 

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.
      *

hawkbit-ui/src/main/resources/messages.properties

@@ -457,7 +457,7 @@ custom.created.date = Created Date
 label.drop.dist.delete.area = Drop here<br>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}