Fix DistributionSet type should not be updatable (#659)

* DistributionSet type is no longer changable for an existing distribution
set in the ui.

Signed-off-by: Markus Block <markus.block@bosch-si.com>

* Removed field "type" from a distribution update.

Signed-off-by: Markus Block <markus.block@bosch-si.com>

* Incorporated review comments.

Signed-off-by: Markus Block <markus.block@bosch-si.com>

hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/builder/DistributionSetUpdate.java

@@ -8,13 +8,10 @@
  */
 package org.eclipse.hawkbit.repository.builder;
 
-import java.util.Optional;
-
 import javax.validation.constraints.NotNull;
 import javax.validation.constraints.Size;
 
 import org.eclipse.hawkbit.repository.model.DistributionSet;
-import org.eclipse.hawkbit.repository.model.DistributionSetType;
 import org.eclipse.hawkbit.repository.model.NamedEntity;
 import org.eclipse.hawkbit.repository.model.NamedVersionedEntity;
 
@@ -45,22 +42,6 @@ public interface DistributionSetUpdate {
      */
     DistributionSetUpdate description(@Size(max = NamedEntity.DESCRIPTION_MAX_SIZE) String description);
 
-    /**
-     * @param typeKey
-     *            for {@link DistributionSet#getType()}
-     * @return updated builder instance
-     */
-    DistributionSetUpdate type(@Size(min = 1, max = DistributionSetType.KEY_MAX_SIZE) @NotNull String typeKey);
-
-    /**
-     * @param type
-     *            for {@link DistributionSet#getType()}
-     * @return updated builder instance
-     */
-    default DistributionSetUpdate type(final DistributionSetType type) {
-        return type(Optional.ofNullable(type).map(DistributionSetType::getKey).orElse(null));
-    }
-
     /**
      * @param requiredMigrationStep
      *            for {@link DistributionSet#isRequiredMigrationStep()}

hawkbit-repository/hawkbit-repository-core/src/main/java/org/eclipse/hawkbit/repository/builder/AbstractDistributionSetUpdateCreate.java

@@ -25,8 +25,7 @@ public abstract class AbstractDistributionSetUpdateCreate<T> extends AbstractNam
     protected String version;
     protected Boolean requiredMigrationStep;
 
-    @ValidString
+
-    protected String type;
     protected Collection<Long> modules;
 
     public T modules(final Collection<Long> modules) {
@@ -38,15 +37,6 @@ public abstract class AbstractDistributionSetUpdateCreate<T> extends AbstractNam
         return modules;
     }
 
-    public T type(final String type) {
-        this.type = StringUtils.trimWhitespace(type);
-        return (T) this;
-    }
-
-    public String getType() {
-        return type;
-    }
-
     public T requiredMigrationStep(final Boolean requiredMigrationStep) {
         this.requiredMigrationStep = requiredMigrationStep;
         return (T) this;

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

@@ -38,7 +38,6 @@ import org.eclipse.hawkbit.repository.jpa.model.DsMetadataCompositeKey;
 import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet;
 import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSetMetadata;
 import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSetMetadata_;
-import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSetType;
 import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet_;
 import org.eclipse.hawkbit.repository.jpa.model.JpaSoftwareModule;
 import org.eclipse.hawkbit.repository.jpa.rsql.RSQLUtility;
@@ -225,28 +224,13 @@ public class JpaDistributionSetManagement implements DistributionSetManagement {
 
         if (update.isRequiredMigrationStep() != null
                 && !update.isRequiredMigrationStep().equals(set.isRequiredMigrationStep())) {
-            checkDistributionSetIsAssignedToTargets(update.getId());
+            assertDistributionSetIsNotAssignedToTargets(update.getId());
             set.setRequiredMigrationStep(update.isRequiredMigrationStep());
         }
 
-        if (update.getType() != null) {
-            final DistributionSetType type = findDistributionSetTypeAndThrowExceptionIfNotFound(update.getType());
-            if (!type.getId().equals(set.getType().getId())) {
-                checkDistributionSetIsAssignedToTargets(update.getId());
-
-                set.setType(type);
-            }
-        }
-
         return distributionSetRepository.save(set);
     }
 
-    private JpaDistributionSetType findDistributionSetTypeAndThrowExceptionIfNotFound(final String key) {
-        return (JpaDistributionSetType) distributionSetTypeManagement.getByKey(key)
-                .orElseThrow(() -> new EntityNotFoundException(DistributionSetType.class, key));
-
-    }
-
     private JpaDistributionSet findDistributionSetAndThrowExceptionIfNotFound(final Long setId) {
         return (JpaDistributionSet) get(setId)
                 .orElseThrow(() -> new EntityNotFoundException(DistributionSet.class, setId));
@@ -330,7 +314,7 @@ public class JpaDistributionSetManagement implements DistributionSetManagement {
                     modules.stream().map(SoftwareModule::getId).collect(Collectors.toList()));
         }
 
-        checkDistributionSetIsAssignedToTargets(setId);
+        assertDistributionSetIsNotAssignedToTargets(setId);
 
         final JpaDistributionSet set = findDistributionSetAndThrowExceptionIfNotFound(setId);
         modules.forEach(set::addModule);
@@ -346,7 +330,7 @@ public class JpaDistributionSetManagement implements DistributionSetManagement {
         final JpaDistributionSet set = findDistributionSetAndThrowExceptionIfNotFound(setId);
         final JpaSoftwareModule module = findSoftwareModuleAndThrowExceptionIfNotFound(moduleId);
 
-        checkDistributionSetIsAssignedToTargets(setId);
+        assertDistributionSetIsNotAssignedToTargets(setId);
 
         set.removeModule(module);
 
@@ -636,10 +620,10 @@ public class JpaDistributionSetManagement implements DistributionSetManagement {
         return specList;
     }
 
-    private void checkDistributionSetIsAssignedToTargets(final Long distributionSet) {
+    private void assertDistributionSetIsNotAssignedToTargets(final Long distributionSet) {
         if (actionRepository.countByDistributionSetId(distributionSet) > 0) {
             throw new EntityReadOnlyException(String.format(
-                    "distribution set %s is already assigned to targets and cannot be changed", distributionSet));
+                    "Distribution set %s is already assigned to targets and cannot be changed", distributionSet));
         }
     }
 

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/builder/JpaDistributionSetCreate.java

@@ -14,6 +14,7 @@ import java.util.Optional;
 
 import org.eclipse.hawkbit.repository.DistributionSetTypeManagement;
 import org.eclipse.hawkbit.repository.SoftwareModuleManagement;
+import org.eclipse.hawkbit.repository.ValidString;
 import org.eclipse.hawkbit.repository.builder.AbstractDistributionSetUpdateCreate;
 import org.eclipse.hawkbit.repository.builder.DistributionSetCreate;
 import org.eclipse.hawkbit.repository.exception.EntityNotFoundException;
@@ -21,6 +22,7 @@ import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet;
 import org.eclipse.hawkbit.repository.model.DistributionSetType;
 import org.eclipse.hawkbit.repository.model.SoftwareModule;
 import org.springframework.util.CollectionUtils;
+import org.springframework.util.StringUtils;
 
 /**
  * Create/build implementation.
@@ -29,6 +31,9 @@ import org.springframework.util.CollectionUtils;
 public class JpaDistributionSetCreate extends AbstractDistributionSetUpdateCreate<DistributionSetCreate>
         implements DistributionSetCreate {
 
+    @ValidString
+    private String type;
+
     private final DistributionSetTypeManagement distributionSetTypeManagement;
     private final SoftwareModuleManagement softwareModuleManagement;
 
@@ -46,6 +51,16 @@ public class JpaDistributionSetCreate extends AbstractDistributionSetUpdateCreat
                 Optional.ofNullable(requiredMigrationStep).orElse(Boolean.FALSE));
     }
 
+    @Override
+    public DistributionSetCreate type(final String type) {
+        this.type = StringUtils.trimWhitespace(type);
+        return this;
+    }
+
+    public String getType() {
+        return type;
+    }
+
     private DistributionSetType findDistributionSetTypeWithExceptionIfNotFound(final String distributionSetTypekey) {
         return distributionSetTypeManagement.getByKey(distributionSetTypekey)
                 .orElseThrow(() -> new EntityNotFoundException(DistributionSetType.class, distributionSetTypekey));

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/detailslayout/AbstractDistributionSetDetails.java

@@ -82,8 +82,7 @@ public abstract class AbstractDistributionSetDetails
 
     @Override
     protected void onEdit(final ClickEvent event) {
-        final Window newDistWindow = distributionAddUpdateWindowLayout.getWindow(getSelectedBaseEntityId());
+        final Window newDistWindow = distributionAddUpdateWindowLayout.getWindowForUpdateDistributionSet(getSelectedBaseEntityId());
-        newDistWindow.setCaption(getI18n().getMessage(UIComponentIdProvider.DIST_UPDATE_CAPTION));
         UI.getCurrent().addWindow(newDistWindow);
         newDistWindow.setVisible(Boolean.TRUE);
     }

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

@@ -16,7 +16,6 @@ import org.eclipse.hawkbit.ui.distributions.state.ManageDistUIState;
 import org.eclipse.hawkbit.ui.management.dstable.DistributionAddUpdateWindowLayout;
 import org.eclipse.hawkbit.ui.management.event.DistributionTableEvent;
 import org.eclipse.hawkbit.ui.management.event.RefreshDistributionTableByFilterEvent;
-import org.eclipse.hawkbit.ui.utils.UIComponentIdProvider;
 import org.eclipse.hawkbit.ui.utils.VaadinMessageSource;
 import org.vaadin.spring.events.EventBus.UIEventBus;
 import org.vaadin.spring.events.EventScope;
@@ -98,8 +97,7 @@ public class DistributionSetTableHeader extends AbstractDistributionSetTableHead
 
     @Override
     protected void addNewItem(final ClickEvent event) {
-        final Window newDistWindow = addUpdateWindowLayout.getWindow(null);
+        final Window newDistWindow = addUpdateWindowLayout.getWindowForCreateDistributionSet();
-        newDistWindow.setCaption(i18n.getMessage(UIComponentIdProvider.DIST_ADD_CAPTION));
         UI.getCurrent().addWindow(newDistWindow);
         newDistWindow.setVisible(Boolean.TRUE);
     }

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/dstable/DistributionAddUpdateWindowLayout.java

@@ -73,8 +73,6 @@ public class DistributionAddUpdateWindowLayout extends CustomComponent {
     private TextArea descTextArea;
     private CheckBox reqMigStepCheckbox;
     private ComboBox distsetTypeNameComboBox;
-    private boolean editDistribution;
-    private Long editDistId;
 
     private FormLayout formLayout;
 
@@ -115,27 +113,19 @@ public class DistributionAddUpdateWindowLayout extends CustomComponent {
     }
 
     /**
-     * Save or update distribution set.
+     * Updates the distribution set on close.
-     *
      */
-    private final class SaveOnCloseDialogListener implements SaveDialogCloseListener {
+    private final class UpdateOnCloseDialogListener implements SaveDialogCloseListener {
+
+        private final Long editDistId;
+
+        public UpdateOnCloseDialogListener(final Long editDistId) {
+            this.editDistId = editDistId;
+        }
+
         @Override
         public void saveOrUpdate() {
-            if (editDistribution) {
+            if (isDuplicate(editDistId)) {
-                updateDistribution();
-                return;
-            }
-            addNewDistribution();
-        }
-
-        @Override
-        public boolean canWindowSaveOrUpdate() {
-            return !isDuplicate();
-        }
-
-        private void updateDistribution() {
-
-            if (isDuplicate()) {
                 return;
             }
             final boolean isMigStepReq = reqMigStepCheckbox.getValue();
@@ -144,7 +134,7 @@ public class DistributionAddUpdateWindowLayout extends CustomComponent {
             distributionSetTypeManagement.get(distSetTypeId).ifPresent(type -> {
                 final DistributionSet currentDS = distributionSetManagement.update(entityFactory.distributionSet()
                         .update(editDistId).name(distNameTextField.getValue()).description(descTextArea.getValue())
-                        .version(distVersionTextField.getValue()).requiredMigrationStep(isMigStepReq).type(type));
+                        .version(distVersionTextField.getValue()).requiredMigrationStep(isMigStepReq));
                 notificationMessage.displaySuccess(i18n.getMessage("message.new.dist.save.success",
                         new Object[] { currentDS.getName(), currentDS.getVersion() }));
                 // update table row+details layout
@@ -152,12 +142,20 @@ public class DistributionAddUpdateWindowLayout extends CustomComponent {
             });
         }
 
-        /**
+        @Override
-         * Add new Distribution set.
+        public boolean canWindowSaveOrUpdate() {
-         */
+            return !isDuplicate(editDistId);
-        private void addNewDistribution() {
+        }
-            editDistribution = Boolean.FALSE;
+    }
 
+    /**
+     * Creates the distribution set on close.
+     *
+     */
+    private final class CreateOnCloseDialogListener implements SaveDialogCloseListener {
+
+        @Override
+        public void saveOrUpdate() {
             final String name = distNameTextField.getValue();
             final String version = distVersionTextField.getValue();
             final Long distSetTypeId = (Long) distsetTypeNameComboBox.getValue();
@@ -176,17 +174,22 @@ public class DistributionAddUpdateWindowLayout extends CustomComponent {
             distributionSetTable.setValue(Sets.newHashSet(newDist.getId()));
         }
 
-        private boolean isDuplicate() {
+        @Override
+        public boolean canWindowSaveOrUpdate() {
+            return !isDuplicate(null);
+        }
+    }
+
+    private boolean isDuplicate(final Long editDistId) {
         final String name = distNameTextField.getValue();
         final String version = distVersionTextField.getValue();
 
         final Optional<DistributionSet> existingDs = distributionSetManagement.getByNameAndVersion(name, version);
         /*
-             * Distribution should not exists with the same name & version.
+         * Distribution should not exists with the same name & version. Display
-             * Display error message, when the "existingDs" is not null and it
+         * error message, when the "existingDs" is not null and it is add window
-             * is add window (or) when the "existingDs" is not null and it is
+         * (or) when the "existingDs" is not null and it is edit window and the
-             * edit window and the distribution Id of the edit window is
+         * distribution Id of the edit window is different then the "existingDs"
-             * different then the "existingDs"
          */
         if (existingDs.isPresent() && !existingDs.get().getId().equals(editDistId)) {
             distNameTextField.addStyleName(SPUIStyleDefinitions.SP_TEXTFIELD_LAYOUT_ERROR_HIGHTLIGHT);
@@ -198,9 +201,6 @@ public class DistributionAddUpdateWindowLayout extends CustomComponent {
         }
 
         return false;
-
-        }
-
     }
 
     private void buildLayout() {
@@ -277,53 +277,86 @@ public class DistributionAddUpdateWindowLayout extends CustomComponent {
      * clear all the fields.
      */
     public void resetComponents() {
-        editDistribution = Boolean.FALSE;
         distNameTextField.clear();
         distNameTextField.removeStyleName("v-textfield-error");
         distVersionTextField.clear();
         distVersionTextField.removeStyleName(SPUIStyleDefinitions.SP_TEXTFIELD_LAYOUT_ERROR_HIGHTLIGHT);
         distsetTypeNameComboBox.removeStyleName(SPUIStyleDefinitions.SP_TEXTFIELD_LAYOUT_ERROR_HIGHTLIGHT);
+        distsetTypeNameComboBox.clear();
+        distsetTypeNameComboBox.setEnabled(true);
         descTextArea.clear();
         reqMigStepCheckbox.clear();
     }
 
     private void populateValuesOfDistribution(final Long editDistId) {
-        this.editDistId = editDistId;
-
-        if (editDistId == null) {
-            return;
-        }
 
         final Optional<DistributionSet> distSet = distributionSetManagement.getWithDetails(editDistId);
         if (!distSet.isPresent()) {
             return;
         }
 
-        editDistribution = Boolean.TRUE;
         distNameTextField.setValue(distSet.get().getName());
         distVersionTextField.setValue(distSet.get().getVersion());
         if (distSet.get().getType().isDeleted()) {
             distsetTypeNameComboBox.addItem(distSet.get().getType().getId());
         }
         distsetTypeNameComboBox.setValue(distSet.get().getType().getId());
+        distsetTypeNameComboBox.setEnabled(false);
 
         reqMigStepCheckbox.setValue(distSet.get().isRequiredMigrationStep());
         descTextArea.setValue(distSet.get().getDescription());
     }
 
     /**
-     * Returns the dialog window for the distributions.
+     * Returns the dialog window for creating a distribution.
      * 
-     * @param editDistId
      * @return window
      */
-    public CommonDialogWindow getWindow(final Long editDistId) {
+    public CommonDialogWindow getWindowForCreateDistributionSet() {
+        return getWindow(null);
+    }
+
+    /**
+     * Returns the dialog window for updating a distribution.
+     * 
+     * @param editDistId
+     *            the id of the distribution that should be updated
+     * @return window
+     */
+    public CommonDialogWindow getWindowForUpdateDistributionSet(final Long editDistId) {
+        return getWindow(editDistId);
+    }
+
+    /**
+     * Internal method to create a window to create or update a DistributionSet.
+     * 
+     * @param editDistId
+     *            if <code>null</code> is provided the window is configured to
+     *            create a DistributionSet otherwise it is configured for
+     *            update.
+     * @return
+     */
+    private CommonDialogWindow getWindow(final Long editDistId) {
+
+        final SaveDialogCloseListener saveDialogCloseListener;
+        String captionId;
+
         resetComponents();
         populateDistSetTypeNameCombo();
+
+        if (editDistId == null) {
+            saveDialogCloseListener = new CreateOnCloseDialogListener();
+            captionId = UIComponentIdProvider.DIST_ADD_CAPTION;
+        } else {
+            saveDialogCloseListener = new UpdateOnCloseDialogListener(editDistId);
+            captionId = UIComponentIdProvider.DIST_UPDATE_CAPTION;
+
             populateValuesOfDistribution(editDistId);
-        return new WindowBuilder(SPUIDefinitions.CREATE_UPDATE_WINDOW)
+        }
-                .caption(i18n.getMessage(UIComponentIdProvider.DIST_ADD_CAPTION)).content(this).layout(formLayout)
+
-                .i18n(i18n).saveDialogCloseListener(new SaveOnCloseDialogListener()).buildCommonDialogWindow();
+        return new WindowBuilder(SPUIDefinitions.CREATE_UPDATE_WINDOW).caption(i18n.getMessage(captionId)).content(this)
+                .layout(formLayout).i18n(i18n).saveDialogCloseListener(saveDialogCloseListener)
+                .buildCommonDialogWindow();
     }
 
     /**