Fix Update bug of softwareModuleType, refactored based on git comments

Signed-off-by: Melanie Retter <melanie.retter@bosch-si.com>

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaSoftwareManagement.java

@@ -132,7 +132,7 @@ public class JpaSoftwareManagement implements SoftwareManagement {
         final JpaSoftwareModuleType type = softwareModuleTypeRepository.findOne(sm.getId());
 
         boolean updated = false;
-        if (sm.getDescription() != null && !sm.getDescription().equals(type.getDescription())) {
+        if (sm.getDescription() == null || !sm.getDescription().equals(type.getDescription())) {
             type.setDescription(sm.getDescription());
             updated = true;
         }

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/smtype/CreateUpdateSoftwareTypeLayout.java

@@ -220,21 +220,14 @@ public class CreateUpdateSoftwareTypeLayout extends CreateUpdateTypeLayout {
             SoftwareModuleType newSWType = entityFactory.generateSoftwareModuleType(typeKeyValue, typeNameValue,
                     typeDescValue, assignNumber);
             newSWType.setColour(colorPicked);
-
+            newSWType.setDescription(typeDescValue);
-            if (null != typeDescValue) {
-                newSWType.setDescription(typeDescValue);
-            }
-
             newSWType.setColour(colorPicked);
-
             newSWType = swTypeManagementService.createSoftwareModuleType(newSWType);
             uiNotification.displaySuccess(i18n.get("message.save.success", new Object[] { newSWType.getName() }));
             eventBus.publish(this,
                     new SoftwareModuleTypeEvent(SoftwareModuleTypeEnum.ADD_SOFTWARE_MODULE_TYPE, newSWType));
-
         } else {
             uiNotification.displayValidationError(i18n.get("message.error.missing.typenameorkey"));
-
         }
     }
 
@@ -244,19 +237,15 @@ public class CreateUpdateSoftwareTypeLayout extends CreateUpdateTypeLayout {
         final String typeDescValue = HawkbitCommonUtil.trimAndNullIfEmpty(tagDesc.getValue());
         if (null != typeNameValue) {
             existingType.setName(typeNameValue);
-
+            existingType.setDescription(typeDescValue);
-            existingType.setDescription(null != typeDescValue ? typeDescValue : null);
-
             existingType.setColour(ColorPickerHelper.getColorPickedString(getColorPickerLayout().getSelPreview()));
             swTypeManagementService.updateSoftwareModuleType(existingType);
             uiNotification.displaySuccess(i18n.get("message.update.success", new Object[] { existingType.getName() }));
             eventBus.publish(this,
                     new SoftwareModuleTypeEvent(SoftwareModuleTypeEnum.UPDATE_SOFTWARE_MODULE_TYPE, existingType));
-
         } else {
             uiNotification.displayValidationError(i18n.get("message.tag.update.mandatory"));
         }
-
     }
 
     @Override

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/CommonDialogWindow.java

@@ -52,6 +52,7 @@ import com.vaadin.ui.AbstractOrderedLayout;
 import com.vaadin.ui.Alignment;
 import com.vaadin.ui.Button;
 import com.vaadin.ui.Button.ClickListener;
+import com.vaadin.ui.CheckBox;
 import com.vaadin.ui.Component;
 import com.vaadin.ui.Field;
 import com.vaadin.ui.GridLayout;
@@ -207,6 +208,10 @@ public class CommonDialogWindow extends Window implements Serializable {
         addListeners();
     }
 
+    /**
+     * saves the original values in a Map so we can use them for detecting
+     * changes
+     */
     public final void setOrginaleValues() {
         for (final AbstractField<?> field : allComponents) {
             Object value = field.getValue();
@@ -244,10 +249,13 @@ public class CommonDialogWindow extends Window implements Serializable {
 
     private boolean isValuesChanged(final Component currentChangedComponent, final Object newValue) {
         for (final AbstractField<?> field : allComponents) {
-            final Object orginalValue = orginalValues.get(field);
+            Object originalValue = orginalValues.get(field);
+            if (field instanceof CheckBox && originalValue == null) {
+                originalValue = Boolean.FALSE;
+            }
             final Object currentValue = getCurrentVaue(currentChangedComponent, newValue, field);
 
-            if (!isValueEquals(field, orginalValue, currentValue)) {
+            if (!isValueEquals(field, originalValue, currentValue)) {
                 return true;
             }
         }
@@ -300,7 +308,15 @@ public class CommonDialogWindow extends Window implements Serializable {
         requiredComponents.addAll(allComponents.stream().filter(this::hasNullValidator).collect(Collectors.toList()));
 
         for (final AbstractField field : requiredComponents) {
-            final Object value = getCurrentVaue(currentChangedComponent, newValue, field);
+            Object value = getCurrentVaue(currentChangedComponent, newValue, field);
+
+            if (String.class.equals(field.getType())) {
+                value = Strings.emptyToNull((String) value);
+            }
+
+            if (Set.class.equals(field.getType())) {
+                value = emptyToNull((Collection<?>) value);
+            }
 
             if (value == null) {
                 return false;
@@ -319,6 +335,10 @@ public class CommonDialogWindow extends Window implements Serializable {
         return valid;
     }
 
+    private static Object emptyToNull(final Collection<?> c) {
+        return (c == null || c.isEmpty()) ? null : c;
+    }
+
     private boolean hasNullValidator(final Component component) {
 
         if (component instanceof AbstractField<?>) {
@@ -459,4 +479,18 @@ public class CommonDialogWindow extends Window implements Serializable {
         }
     }
 
+    /**
+     * Adds the component manually to the allComponents-List and adds a
+     * ValueChangeListener to it. Necessary in Update Distribution Type as the
+     * CheckBox concerned is an ItemProperty...
+     * 
+     * @param component
+     *            AbstractField
+     */
+    public void updateAllComponents(final AbstractField component) {
+
+        allComponents.add(component);
+        component.addValueChangeListener(new ChangeListener(component));
+    }
+
 }

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/disttype/CreateUpdateDistSetTypeLayout.java

@@ -9,6 +9,7 @@
 package org.eclipse.hawkbit.ui.distributions.disttype;
 
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import org.eclipse.hawkbit.repository.DistributionSetManagement;
@@ -83,6 +84,8 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout {
 
     private IndexedContainer originalSelectedTableContainer;
 
+    private Map<CheckBox, Boolean> mandatoryCheckboxMap;
+
     @Override
     protected void createRequiredComponents() {
 
@@ -315,7 +318,9 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout {
             saveTblitem = selectedTableContainer.addItem(id);
             saveTblitem.getItemProperty(DIST_TYPE_NAME).setValue(
                     sourceTable.getContainerDataSource().getItem(id).getItemProperty(DIST_TYPE_NAME).getValue());
-            saveTblitem.getItemProperty(DIST_TYPE_MANDATORY).setValue(new CheckBox());
+            final CheckBox mandatoryCheckBox = new CheckBox();
+            window.updateAllComponents(mandatoryCheckBox);
+            saveTblitem.getItemProperty(DIST_TYPE_MANDATORY).setValue(mandatoryCheckBox);
             saveTblitem.getItemProperty(DIST_TYPE_DESCRIPTION).setValue(
                     sourceTable.getContainerDataSource().getItem(id).getItemProperty(DIST_TYPE_DESCRIPTION).getValue());
         }
@@ -327,7 +332,6 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout {
         if (sourceTableContainer != null) {
             Item saveTblitem;
             saveTblitem = sourceTableContainer.addItem(selectedId);
-            selectedTable.getContainerDataSource().getItem(selectedId).getItemProperty(DIST_TYPE_NAME);
             saveTblitem.getItemProperty(DIST_TYPE_NAME).setValue(selectedTable.getContainerDataSource()
                     .getItem(selectedId).getItemProperty(DIST_TYPE_NAME).getValue());
             saveTblitem.getItemProperty(DIST_TYPE_DESCRIPTION).setValue(selectedTable.getContainerDataSource()
@@ -357,10 +361,7 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout {
                 final SoftwareModuleType swModuleType = softwareManagement.findSoftwareModuleTypeByName(distTypeName);
                 checkMandatoryAndAddMandatoryModuleType(newDistType, isMandatory, swModuleType);
             }
-            if (null != typeDescValue) {
+            newDistType.setDescription(typeDescValue);
-                newDistType.setDescription(typeDescValue);
-            }
-
             newDistType.setColour(colorPicked);
             newDistType = distributionSetManagement.createDistributionSetType(newDistType);
             uiNotification.displaySuccess(i18n.get("message.save.success", new Object[] { newDistType.getName() }));
@@ -387,7 +388,7 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout {
         if (null != typeNameValue) {
             updateDistSetType.setName(typeNameValue);
             updateDistSetType.setKey(typeKeyValue);
-            updateDistSetType.setDescription(null != typeDescValue ? typeDescValue : null);
+            updateDistSetType.setDescription(typeDescValue);
 
             if (distributionSetManagement.countDistributionSetsByType(existingType) <= 0 && null != itemIds
                     && !itemIds.isEmpty()) {
@@ -407,7 +408,6 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout {
                     .displaySuccess(i18n.get("message.update.success", new Object[] { updateDistSetType.getName() }));
             eventBus.publish(this,
                     new DistributionSetTypeEvent(DistributionSetTypeEnum.UPDATE_DIST_SET_TYPE, updateDistSetType));
-
         } else {
             uiNotification.displayValidationError(i18n.get("message.tag.update.mandatory"));
         }
@@ -417,7 +417,6 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout {
             final Boolean isMandatory, final SoftwareModuleType swModuleType) {
         if (isMandatory) {
             updateDistSetType.addMandatoryModuleType(swModuleType);
-
         } else {
             updateDistSetType.addOptionalModuleType(swModuleType);
         }
@@ -561,15 +560,15 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout {
         final Item saveTblitem = selectedTableContainer.addItem(swModuleType.getId());
         sourceTable.removeItem(swModuleType.getId());
         saveTblitem.getItemProperty(DIST_TYPE_NAME).setValue(swModuleType.getName());
-        saveTblitem.getItemProperty(DIST_TYPE_MANDATORY).setValue(new CheckBox("", mandatory));
+        final CheckBox mandatoryCheckbox = new CheckBox("", mandatory);
-
+        mandatoryCheckbox.setId(swModuleType.getName());
-        // TODO CHECKBOX
+        saveTblitem.getItemProperty(DIST_TYPE_MANDATORY).setValue(mandatoryCheckbox);
-        final CheckBox mandatoryCheckbox = (CheckBox) selectedTableContainer
-                .getContainerProperty(swModuleType.getId(), DIST_TYPE_MANDATORY).getValue();
 
         final Item originalItem = originalSelectedTableContainer.addItem(swModuleType.getId());
         originalItem.getItemProperty(DIST_TYPE_NAME).setValue(swModuleType.getName());
-        originalItem.getItemProperty(DIST_TYPE_MANDATORY).setValue(new CheckBox("", mandatory));
+        originalItem.getItemProperty(DIST_TYPE_MANDATORY).setValue(mandatoryCheckbox);
+
+        window.updateAllComponents(mandatoryCheckbox);
     }
 
     @Override

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/layouts/AbstractCreateUpdateTagLayout.java

@@ -602,14 +602,10 @@ public abstract class AbstractCreateUpdateTagLayout extends CustomComponent
 
     @Override
     public void addColorChangeListener(final ColorChangeListener listener) {
-        // TODO Auto-generated method stub
-
     }
 
     @Override
     public void removeColorChangeListener(final ColorChangeListener listener) {
-        // TODO Auto-generated method stub
-
     }
 
 }

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/targettable/TargetAddUpdateWindowLayout.java

@@ -171,6 +171,13 @@ public class TargetAddUpdateWindowLayout extends CustomComponent {
         return window;
     }
 
+    public Window getWindow(final String entityId) {
+        populateValuesOfTarget(entityId);
+        getWindow();
+        window.addStyleName("target-update-window");
+        return window;
+    }
+
     /**
      * clear all fields of Target Edit Window.
      */
@@ -203,7 +210,7 @@ public class TargetAddUpdateWindowLayout extends CustomComponent {
     /**
      * @param controllerId
      */
-    public void populateValuesOfTarget(final String controllerId) {
+    private void populateValuesOfTarget(final String controllerId) {
         resetComponents();
         this.controllerId = controllerId;
         editTarget = Boolean.TRUE;
@@ -214,7 +221,6 @@ public class TargetAddUpdateWindowLayout extends CustomComponent {
         if (target.getDescription() != null) {
             descTextArea.setValue(target.getDescription());
         }
-        window.addStyleName("target-update-window");
     }
 
     public FormLayout getFormLayout() {

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/targettable/TargetDetails.java

@@ -106,7 +106,8 @@ public class TargetDetails extends AbstractTableDetailsLayout<Target> {
         if (getSelectedBaseEntity() == null) {
             return;
         }
-        targetAddUpdateWindowLayout.populateValuesOfTarget(getSelectedBaseEntity().getControllerId());
+        targetAddUpdateWindowLayout.getWindow(getSelectedBaseEntity().getControllerId());
+        // targetAddUpdateWindowLayout.populateValuesOfTarget(getSelectedBaseEntity().getControllerId());
         openWindow();
     }
 

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/AddUpdateRolloutWindowLayout.java

@@ -155,14 +155,18 @@ public class AddUpdateRolloutWindowLayout extends GridLayout {
     }
 
     public CommonDialogWindow getWindow(final Long rolloutId) {
-        resetComponents();
+        window = getWindow();
-        window = SPUIWindowDecorator.getWindow(i18n.get("caption.configure.rollout"), null,
-                SPUIDefinitions.CREATE_UPDATE_WINDOW, this, event -> onRolloutSave(), null,
-                uiProperties.getLinks().getDocumentation().getRolloutView(), this, i18n);
         populateData(rolloutId);
         return window;
     }
 
+    public CommonDialogWindow getWindow() {
+        resetComponents();
+        return SPUIWindowDecorator.getWindow(i18n.get("caption.configure.rollout"), null,
+                SPUIDefinitions.CREATE_UPDATE_WINDOW, this, event -> onRolloutSave(), null,
+                uiProperties.getLinks().getDocumentation().getRolloutView(), this, i18n);
+    }
+
     /**
      * Reset the field values.
      */
@@ -610,9 +614,7 @@ public class AddUpdateRolloutWindowLayout extends GridLayout {
         @Override
         public void validate(final Object value) {
             try {
-                if (HawkbitCommonUtil.trimAndNullIfEmpty(noOfGroups.getValue()) == null
+                if (isNoOfGroupsOrTargetFilterEmpty()) {
-                        || (HawkbitCommonUtil.trimAndNullIfEmpty((String) targetFilterQueryCombo.getValue()) == null
-                                && targetFilterQuery.getValue() == null)) {
                     uiNotification
                             .displayValidationError(i18n.get("message.rollout.noofgroups.or.targetfilter.missing"));
                 } else {
@@ -626,14 +628,18 @@ public class AddUpdateRolloutWindowLayout extends GridLayout {
             // suppress the need of preserve original exception, will blow
             // up the
             // log and not necessary here
-            catch (
+            catch (final InvalidValueException ex) {
-
-            @SuppressWarnings("squid:S1166") final InvalidValueException ex) {
                 // we have to throw the exception here, otherwise the UI won't
                 // show the vaadin validation error!
                 throw ex;
             }
         }
+
+        private boolean isNoOfGroupsOrTargetFilterEmpty() {
+            return HawkbitCommonUtil.trimAndNullIfEmpty(noOfGroups.getValue()) == null
+                    || (HawkbitCommonUtil.trimAndNullIfEmpty((String) targetFilterQueryCombo.getValue()) == null
+                            && targetFilterQuery.getValue() == null);
+        }
     }
 
     private int getGroupSize() {
@@ -654,7 +660,7 @@ public class AddUpdateRolloutWindowLayout extends GridLayout {
             // suppress the need of preserve original exception, will blow
             // up the
             // log and not necessary here
-            catch (@SuppressWarnings("squid:S1166") final InvalidValueException ex) {
+            catch (final InvalidValueException ex) {
                 // we have to throw the exception here, otherwise the UI won't
                 // show the vaadin validation error!
                 throw ex;
@@ -676,7 +682,7 @@ public class AddUpdateRolloutWindowLayout extends GridLayout {
             // suppress the need of preserve original exception, will blow
             // up the
             // log and not necessary here
-            catch (@SuppressWarnings("squid:S1166") final InvalidValueException ex) {
+            catch (final InvalidValueException ex) {
                 // we have to throw the exception here, otherwise the UI won't
                 // show the vaadin validation error!
                 throw ex;

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/RolloutListHeader.java

@@ -91,7 +91,7 @@ public class RolloutListHeader extends AbstractGridHeader {
 
     @Override
     protected void addNewItem(final ClickEvent event) {
-        final Window addTargetWindow = addUpdateRolloutWindow.getWindow(null);
+        final Window addTargetWindow = addUpdateRolloutWindow.getWindow();
         UI.getCurrent().addWindow(addTargetWindow);
         addTargetWindow.setVisible(Boolean.TRUE);