From a5a505024b72a8e5139555af6f4aa65313379724 Mon Sep 17 00:00:00 2001 From: Kai Zimmermann Date: Wed, 5 Apr 2017 17:15:59 +0200 Subject: [PATCH] Deleted rollouts are immutable (#475) * Update throws read only exception if rollout is in deleting or deleted state. Signed-off-by: kaizimmerm * Fix typo. Signed-off-by: kaizimmerm --- .../hawkbit/repository/RolloutManagement.java | 4 +++ .../repository/jpa/JpaRolloutManagement.java | 9 ++++++ .../repository/jpa/RolloutManagementTest.java | 8 +++++ .../rollout/AddUpdateRolloutWindowLayout.java | 32 +++++++++++-------- 4 files changed, 40 insertions(+), 13 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RolloutManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RolloutManagement.java index a034479ff..9f135a9fa 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RolloutManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RolloutManagement.java @@ -19,6 +19,7 @@ import org.eclipse.hawkbit.repository.builder.RolloutGroupCreate; import org.eclipse.hawkbit.repository.builder.RolloutUpdate; import org.eclipse.hawkbit.repository.exception.ConstraintViolationException; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; +import org.eclipse.hawkbit.repository.exception.EntityReadOnlyException; import org.eclipse.hawkbit.repository.exception.RSQLParameterSyntaxException; import org.eclipse.hawkbit.repository.exception.RSQLParameterUnsupportedFieldException; import org.eclipse.hawkbit.repository.exception.RolloutIllegalStateException; @@ -371,6 +372,9 @@ public interface RolloutManagement { * * @throws EntityNotFoundException * if rollout or DS with given IDs do not exist + * @throws EntityReadOnlyException + * if rollout is in soft deleted state, i.e. only kept as + * reference * */ @PreAuthorize(SpringEvalExpressions.HAS_AUTH_ROLLOUT_MANAGEMENT_WRITE) diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java index e1bad1f49..8442ec3fb 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java @@ -36,6 +36,7 @@ import org.eclipse.hawkbit.repository.event.remote.entity.RolloutUpdatedEvent; import org.eclipse.hawkbit.repository.exception.ConstraintViolationException; import org.eclipse.hawkbit.repository.exception.EntityAlreadyExistsException; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; +import org.eclipse.hawkbit.repository.exception.EntityReadOnlyException; import org.eclipse.hawkbit.repository.exception.RolloutIllegalStateException; import org.eclipse.hawkbit.repository.jpa.executor.AfterTransactionCommitExecutor; import org.eclipse.hawkbit.repository.jpa.model.JpaAction; @@ -895,6 +896,8 @@ public class JpaRolloutManagement extends AbstractRolloutManagement { final GenericRolloutUpdate update = (GenericRolloutUpdate) u; final JpaRollout rollout = getRolloutAndThrowExceptionIfNotFound(update.getId()); + checkIfDeleted(update.getId(), rollout.getStatus()); + update.getName().ifPresent(rollout::setName); update.getDescription().ifPresent(rollout::setDescription); update.getActionType().ifPresent(rollout::setActionType); @@ -910,6 +913,12 @@ public class JpaRolloutManagement extends AbstractRolloutManagement { return rolloutRepository.save(rollout); } + private static void checkIfDeleted(final Long rolloutId, final RolloutStatus status) { + if (RolloutStatus.DELETING.equals(status) || RolloutStatus.DELETED.equals(status)) { + throw new EntityReadOnlyException("Rollout " + rolloutId + " is soft deleted and cannot be changed"); + } + } + private JpaRollout getRolloutAndThrowExceptionIfNotFound(final Long rolloutId) { return rolloutRepository.findById(rolloutId) .orElseThrow(() -> new EntityNotFoundException(Rollout.class, rolloutId)); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/RolloutManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/RolloutManagementTest.java index c4f12a7a3..5a27e732f 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/RolloutManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/RolloutManagementTest.java @@ -9,6 +9,7 @@ package org.eclipse.hawkbit.repository.jpa; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.fail; import java.util.ArrayList; @@ -36,6 +37,7 @@ import org.eclipse.hawkbit.repository.event.remote.entity.TargetUpdatedEvent; import org.eclipse.hawkbit.repository.exception.ConstraintViolationException; import org.eclipse.hawkbit.repository.exception.EntityAlreadyExistsException; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; +import org.eclipse.hawkbit.repository.exception.EntityReadOnlyException; import org.eclipse.hawkbit.repository.exception.RolloutIllegalStateException; import org.eclipse.hawkbit.repository.jpa.model.JpaAction; import org.eclipse.hawkbit.repository.jpa.model.JpaRollout; @@ -1519,7 +1521,13 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { // verify final JpaRollout deletedRollout = rolloutRepository.findOne(createdRollout.getId()); assertThat(deletedRollout).isNotNull(); + assertThat(deletedRollout.getStatus()).isEqualTo(RolloutStatus.DELETED); + assertThatExceptionOfType(EntityReadOnlyException.class) + .isThrownBy(() -> rolloutManagement + .updateRollout(entityFactory.rollout().update(createdRollout.getId()).description("test"))) + .withMessageContaining("" + createdRollout.getId()); + assertThat(rolloutManagement.findAll(pageReq, true).getContent()).hasSize(1); assertThat(rolloutManagement.findAll(pageReq, false).getContent()).hasSize(0); assertThat(rolloutGroupManagement.findAllRolloutGroupsWithDetailedStatus(createdRollout.getId(), pageReq) diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/AddUpdateRolloutWindowLayout.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/AddUpdateRolloutWindowLayout.java index 934e483b7..2bf808f03 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/AddUpdateRolloutWindowLayout.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/AddUpdateRolloutWindowLayout.java @@ -25,6 +25,7 @@ import org.eclipse.hawkbit.repository.builder.RolloutCreate; import org.eclipse.hawkbit.repository.builder.RolloutGroupCreate; import org.eclipse.hawkbit.repository.builder.RolloutUpdate; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; +import org.eclipse.hawkbit.repository.exception.EntityReadOnlyException; import org.eclipse.hawkbit.repository.model.Action.ActionType; import org.eclipse.hawkbit.repository.model.RepositoryModelConstants; import org.eclipse.hawkbit.repository.model.Rollout; @@ -51,13 +52,13 @@ import org.eclipse.hawkbit.ui.management.footer.ActionTypeOptionGroupLayout.Acti import org.eclipse.hawkbit.ui.rollout.event.RolloutEvent; import org.eclipse.hawkbit.ui.rollout.groupschart.GroupsPieChart; import org.eclipse.hawkbit.ui.utils.HawkbitCommonUtil; -import org.eclipse.hawkbit.ui.utils.VaadinMessageSource; import org.eclipse.hawkbit.ui.utils.SPDateTimeUtil; import org.eclipse.hawkbit.ui.utils.SPUIDefinitions; import org.eclipse.hawkbit.ui.utils.SPUILabelDefinitions; import org.eclipse.hawkbit.ui.utils.SPUIStyleDefinitions; import org.eclipse.hawkbit.ui.utils.UIComponentIdProvider; import org.eclipse.hawkbit.ui.utils.UINotification; +import org.eclipse.hawkbit.ui.utils.VaadinMessageSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.data.domain.Page; @@ -161,7 +162,8 @@ public class AddUpdateRolloutWindowLayout extends GridLayout { AddUpdateRolloutWindowLayout(final RolloutManagement rolloutManagement, final TargetManagement targetManagement, final UINotification uiNotification, final UiProperties uiProperties, final EntityFactory entityFactory, - final VaadinMessageSource i18n, final UIEventBus eventBus, final TargetFilterQueryManagement targetFilterQueryManagement) { + final VaadinMessageSource i18n, final UIEventBus eventBus, + final TargetFilterQueryManagement targetFilterQueryManagement) { actionTypeOptionGroupLayout = new ActionTypeOptionGroupLayout(i18n); autoStartOptionGroupLayout = new AutoStartOptionGroupLayout(i18n); this.rolloutManagement = rolloutManagement; @@ -215,7 +217,8 @@ public class AddUpdateRolloutWindowLayout extends GridLayout { private boolean duplicateCheck() { if (rolloutManagement.findRolloutByName(getRolloutName()).isPresent()) { - uiNotification.displayValidationError(i18n.getMessage("message.rollout.duplicate.check", getRolloutName())); + uiNotification + .displayValidationError(i18n.getMessage("message.rollout.duplicate.check", getRolloutName())); return false; } return true; @@ -242,7 +245,7 @@ public class AddUpdateRolloutWindowLayout extends GridLayout { Rollout updatedRollout; try { updatedRollout = rolloutManagement.updateRollout(rolloutUpdate); - } catch (final EntityNotFoundException e) { + } catch (final EntityNotFoundException | EntityReadOnlyException e) { LOGGER.warn("Rollout was deleted. Redirect to Rollouts overview.", e); uiNotification.displayWarning( "Rollout with name " + rolloutName.getValue() + " was deleted. Update is not poosible"); @@ -258,7 +261,8 @@ public class AddUpdateRolloutWindowLayout extends GridLayout { final String rolloutNameVal = getRolloutName(); if (!rollout.getName().equals(rolloutNameVal) && rolloutManagement.findRolloutByName(rolloutNameVal).isPresent()) { - uiNotification.displayValidationError(i18n.getMessage("message.rollout.duplicate.check", rolloutNameVal)); + uiNotification + .displayValidationError(i18n.getMessage("message.rollout.duplicate.check", rolloutNameVal)); return false; } return true; @@ -342,8 +346,8 @@ public class AddUpdateRolloutWindowLayout extends GridLayout { } private CommonDialogWindow createWindow() { - return new WindowBuilder(SPUIDefinitions.CREATE_UPDATE_WINDOW).caption(i18n.getMessage("caption.configure.rollout")) - .content(this).layout(this).i18n(i18n) + return new WindowBuilder(SPUIDefinitions.CREATE_UPDATE_WINDOW) + .caption(i18n.getMessage("caption.configure.rollout")).content(this).layout(this).i18n(i18n) .helpLink(uiProperties.getLinks().getDocumentation().getRolloutView()) .saveDialogCloseListener(new SaveOnDialogCloseListener()).buildCommonDialogWindow(); } @@ -533,7 +537,8 @@ public class AddUpdateRolloutWindowLayout extends GridLayout { i18n.getMessage("caption.rollout.tabs.simple")); simpleTab.setId(UIComponentIdProvider.ROLLOUT_SIMPLE_TAB); - final TabSheet.Tab advancedTab = tabSheet.addTab(defineGroupsLayout, i18n.getMessage("caption.rollout.tabs.advanced")); + final TabSheet.Tab advancedTab = tabSheet.addTab(defineGroupsLayout, + i18n.getMessage("caption.rollout.tabs.advanced")); advancedTab.setId(UIComponentIdProvider.ROLLOUT_ADVANCED_TAB); tabSheet.addSelectedTabChangeListener(event -> validateGroups()); @@ -693,8 +698,8 @@ public class AddUpdateRolloutWindowLayout extends GridLayout { private ComboBox createTargetFilterQueryCombo() { return new ComboBoxBuilder().setValueChangeListener(this::onTargetFilterChange) - .setPrompt(i18n.getMessage("prompt.target.filter")).setId(UIComponentIdProvider.ROLLOUT_TARGET_FILTER_COMBO_ID) - .buildCombBox(); + .setPrompt(i18n.getMessage("prompt.target.filter")) + .setId(UIComponentIdProvider.ROLLOUT_TARGET_FILTER_COMBO_ID).buildCombBox(); } private void onTargetFilterChange(final ValueChangeEvent event) { @@ -833,12 +838,13 @@ public class AddUpdateRolloutWindowLayout extends GridLayout { @Override public void validate(final Object value) { if (isNoOfGroupsOrTargetFilterEmpty()) { - uiNotification.displayValidationError(i18n.getMessage("message.rollout.noofgroups.or.targetfilter.missing")); + uiNotification + .displayValidationError(i18n.getMessage("message.rollout.noofgroups.or.targetfilter.missing")); } else { if (value != null) { final int groupSize = getGroupSize(); - new IntegerRangeValidator(i18n.getMessage(MESSAGE_ROLLOUT_FIELD_VALUE_RANGE, 0, groupSize), 0, groupSize) - .validate(Integer.valueOf(value.toString())); + new IntegerRangeValidator(i18n.getMessage(MESSAGE_ROLLOUT_FIELD_VALUE_RANGE, 0, groupSize), 0, + groupSize).validate(Integer.valueOf(value.toString())); } } }