Optional support for autoclose of deprecated actions (#585)

* Introduce action autoclose into repository. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * Extended test. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * Fix typo. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * Added checkbox to management ui. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * Fix Ui allignment. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * Fix item ID. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * Fix sonar issues. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com> * readibility. Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>
2017-10-10 17:26:43 +02:00
parent c3908c9c34
commit 8e890424c0
26 changed files with 392 additions and 51 deletions
--- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/tenancy/configuration/TenantConfigurationProperties.java
+++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/tenancy/configuration/TenantConfigurationProperties.java
@@ -102,6 +102,12 @@ public class TenantConfigurationProperties {
         */
        public static final String ANONYMOUS_DOWNLOAD_MODE_ENABLED = "anonymous.download.enabled";

+        /**
+         * Repository on autoclose mode instead of canceling in case of new DS
+         * assignment over active actions.
+         */
+        public static final String REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED = "repository.actions.autoclose.enabled";
+
        private String keyName;
        private String defaultValue = "";
        private Class<?> dataType = String.class;
--- a/hawkbit-repository/hawkbit-repository-core/src/main/resources/hawkbit-repository-defaults.properties
+++ b/hawkbit-repository/hawkbit-repository-core/src/main/resources/hawkbit-repository-defaults.properties
@@ -20,6 +20,11 @@ hawkbit.server.tenant.configuration.authentication-header-enabled.defaultValue=$
 hawkbit.server.tenant.configuration.authentication-header-enabled.dataType=java.lang.Boolean
 hawkbit.server.tenant.configuration.authentication-header-enabled.validator=org.eclipse.hawkbit.tenancy.configuration.validator.TenantConfigurationBooleanValidator

+hawkbit.server.tenant.configuration.repository-actions-autoclose-enabled.keyName=repository.actions.autoclose.enabled
+hawkbit.server.tenant.configuration.repository-actions-autoclose-enabled.defaultValue=false
+hawkbit.server.tenant.configuration.repository-actions-autoclose-enabled.dataType=java.lang.Boolean
+hawkbit.server.tenant.configuration.repository-actions-autoclose-enabled.validator=org.eclipse.hawkbit.tenancy.configuration.validator.TenantConfigurationBooleanValidator
+
 hawkbit.server.tenant.configuration.authentication-header-authority.keyName=authentication.header.authority
 hawkbit.server.tenant.configuration.authentication-header-authority.defaultValue=${hawkbit.server.ddi.security.authentication.header.authority}

--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/AbstractDsAssignmentStrategy.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/AbstractDsAssignmentStrategy.java
@@ -92,7 +92,19 @@ public abstract class AbstractDsAssignmentStrategy {
     *            to cancel actions for
     * @return {@link Set} of {@link Target#getId()}s
     */
-    abstract Set<Long> findTargetIdsToCancel(List<List<Long>> targetIds);
+    abstract Set<Long> cancelActiveActions(List<List<Long>> targetIds);
+
+    /**
+     * Cancels actions that can be canceled (i.e.
+     * {@link DistributionSet#isRequiredMigrationStep() is <code>false</code>})
+     * as a result of the new assignment and returns all {@link Target}s where
+     * such actions existed.
+     * 
+     * @param targetIds
+     *            to cancel actions for
+     * @return {@link Set} of {@link Target#getId()}s
+     */
+    abstract void closeActiveActions(List<List<Long>> targetIds);

    /**
     * Handles event sending related to the assignment.
@@ -119,7 +131,7 @@ public abstract class AbstractDsAssignmentStrategy {
    }

    /**
-     * Removes {@link Action}s that are no longer necessary and sends
+     * Cancels {@link Action}s that are no longer necessary and sends
     * cancellations to the controller.
     *
     * @param targetsIds
@@ -130,8 +142,8 @@ public abstract class AbstractDsAssignmentStrategy {
        // Figure out if there are potential target/action combinations that
        // need to be considered for cancellation
        final List<JpaAction> activeActions = actionRepository
-                .findByActiveAndTargetIdInAndActionStatusNotEqualToAndDistributionSetRequiredMigrationStep(targetsIds,
-                        Action.Status.CANCELING);
+                .findByActiveAndTargetIdInAndActionStatusNotEqualToAndDistributionSetNotRequiredMigrationStep(
+                        targetsIds, Action.Status.CANCELING);

        return activeActions.stream().map(action -> {
            action.setStatus(Status.CANCELING);
@@ -148,6 +160,34 @@ public abstract class AbstractDsAssignmentStrategy {

    }

+    /**
+     * Closes {@link Action}s that are no longer necessary without sending a
+     * hint to the controller.
+     *
+     * @param targetsIds
+     *            to override {@link Action}s
+     */
+    protected List<Long> closeObsoleteUpdateActions(final Collection<Long> targetsIds) {
+
+        // Figure out if there are potential target/action combinations that
+        // need to be considered for cancellation
+        final List<JpaAction> activeActions = actionRepository
+                .findByActiveAndTargetIdInAndDistributionSetNotRequiredMigrationStep(targetsIds);
+
+        return activeActions.stream().map(action -> {
+            action.setStatus(Status.CANCELED);
+            action.setActive(false);
+
+            // document that the status has been retrieved
+            actionStatusRepository.save(new JpaActionStatus(action, Status.CANCELED, System.currentTimeMillis(),
+                    RepositoryConstants.SERVER_MESSAGE_PREFIX + "close obsolete action due to new update"));
+            actionRepository.save(action);
+
+            return action.getTarget().getId();
+        }).collect(Collectors.toList());
+
+    }
+
    /**
     * Sends the {@link CancelTargetAssignmentEvent} for a specific target to
     * the eventPublisher.
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/ActionRepository.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/ActionRepository.java
@@ -219,9 +219,22 @@ public interface ActionRepository extends BaseEntityRepository<JpaAction, Long>,
     */
    @EntityGraph(attributePaths = { "target" }, type = EntityGraphType.LOAD)
    @Query("SELECT a FROM JpaAction a WHERE a.active = true AND a.distributionSet.requiredMigrationStep = false AND a.target IN ?1 AND a.status != ?2")
-    List<JpaAction> findByActiveAndTargetIdInAndActionStatusNotEqualToAndDistributionSetRequiredMigrationStep(
+    List<JpaAction> findByActiveAndTargetIdInAndActionStatusNotEqualToAndDistributionSetNotRequiredMigrationStep(
            Collection<Long> targetIds, Action.Status notStatus);

+    /**
+     *
+     * Retrieves all {@link Action}s which are active and referring to the given
+     * target Ids and distribution set required migration step.
+     *
+     * @param targetIds
+     *            the IDs of targets for the actions
+     * @return the found list of {@link Action}s
+     */
+    @EntityGraph(attributePaths = { "target" }, type = EntityGraphType.LOAD)
+    @Query("SELECT a FROM JpaAction a WHERE a.active = true AND a.distributionSet.requiredMigrationStep = false AND a.target IN ?1")
+    List<JpaAction> findByActiveAndTargetIdInAndDistributionSetNotRequiredMigrationStep(Collection<Long> targetIds);
+
    /**
     * Counts all {@link Action}s referring to the given target.
     *
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java
@@ -30,6 +30,7 @@ import org.eclipse.hawkbit.repository.ActionFields;
 import org.eclipse.hawkbit.repository.DeploymentManagement;
 import org.eclipse.hawkbit.repository.RepositoryConstants;
 import org.eclipse.hawkbit.repository.TargetManagement;
+import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
 import org.eclipse.hawkbit.repository.event.remote.TargetAssignDistributionSetEvent;
 import org.eclipse.hawkbit.repository.exception.CancelActionNotAllowedException;
 import org.eclipse.hawkbit.repository.exception.EntityNotFoundException;
@@ -57,6 +58,8 @@ import org.eclipse.hawkbit.repository.model.Target;
 import org.eclipse.hawkbit.repository.model.TargetUpdateStatus;
 import org.eclipse.hawkbit.repository.model.TargetWithActionType;
 import org.eclipse.hawkbit.repository.rsql.VirtualPropertyReplacer;
+import org.eclipse.hawkbit.security.SystemSecurityContext;
+import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.context.ApplicationContext;
@@ -109,13 +112,17 @@ public class JpaDeploymentManagement implements DeploymentManagement {
    private final PlatformTransactionManager txManager;
    private final OnlineDsAssignmentStrategy onlineDsAssignmentStrategy;
    private final OfflineDsAssignmentStrategy offlineDsAssignmentStrategy;
+    private final TenantConfigurationManagement tenantConfigurationManagement;
+    private final SystemSecurityContext systemSecurityContext;

    JpaDeploymentManagement(final EntityManager entityManager, final ActionRepository actionRepository,
            final DistributionSetRepository distributionSetRepository, final TargetRepository targetRepository,
            final ActionStatusRepository actionStatusRepository, final TargetManagement targetManagement,
            final AuditorAware<String> auditorProvider, final ApplicationEventPublisher eventPublisher,
            final ApplicationContext applicationContext, final AfterTransactionCommitExecutor afterCommit,
-            final VirtualPropertyReplacer virtualPropertyReplacer, final PlatformTransactionManager txManager) {
+            final VirtualPropertyReplacer virtualPropertyReplacer, final PlatformTransactionManager txManager,
+            final TenantConfigurationManagement tenantConfigurationManagement,
+            final SystemSecurityContext systemSecurityContext) {
        this.entityManager = entityManager;
        this.actionRepository = actionRepository;
        this.distributionSetRepository = distributionSetRepository;
@@ -132,6 +139,8 @@ public class JpaDeploymentManagement implements DeploymentManagement {
                applicationContext, actionRepository, actionStatusRepository);
        offlineDsAssignmentStrategy = new OfflineDsAssignmentStrategy(targetRepository, afterCommit, eventPublisher,
                applicationContext, actionRepository, actionStatusRepository);
+        this.tenantConfigurationManagement = tenantConfigurationManagement;
+        this.systemSecurityContext = systemSecurityContext;
    }

    @Override
@@ -254,7 +263,16 @@ public class JpaDeploymentManagement implements DeploymentManagement {
        // need to remember which one we have been switched to canceling state
        // because for targets which we have changed to canceling we don't want
        // to publish the new action update event.
-        final Set<Long> targetIdsCancellList = assignmentStrategy.findTargetIdsToCancel(targetIds);
+        final Set<Long> targetIdsCancellList;
+
+        if (systemSecurityContext.runAsSystem(() -> tenantConfigurationManagement
+                .getConfigurationValue(TenantConfigurationKey.REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED, Boolean.class)
+                .getValue())) {
+            targetIdsCancellList = Collections.emptySet();
+            assignmentStrategy.closeActiveActions(targetIds);
+        } else {
+            targetIdsCancellList = assignmentStrategy.cancelActiveActions(targetIds);
+        }

        // cancel all scheduled actions which are in-active, these actions were
        // not active before and the manual assignment which has been done
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetFilterQueryManagement.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetFilterQueryManagement.java
@@ -85,8 +85,9 @@ public class JpaTargetFilterQueryManagement implements TargetFilterQueryManageme
    @Retryable(include = {
            ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
    public void delete(final Long targetFilterQueryId) {
-        get(targetFilterQueryId)
-                .orElseThrow(() -> new EntityNotFoundException(TargetFilterQuery.class, targetFilterQueryId));
+        if (!targetFilterQueryRepository.exists(targetFilterQueryId)) {
+            throw new EntityNotFoundException(TargetFilterQuery.class, targetFilterQueryId);
+        }

        targetFilterQueryRepository.delete(targetFilterQueryId);
    }
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantStatsManagement.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTenantStatsManagement.java
@@ -44,7 +44,7 @@ public class JpaTenantStatsManagement implements TenantStatsManagement {

        result.setTargets(targetRepository.count());
        result.setArtifacts(artifactRepository.countBySoftwareModuleDeleted(false));
-        artifactRepository.getSumOfUndeletedArtifactSize().map(result::setOverallArtifactVolumeInBytes);
+        artifactRepository.getSumOfUndeletedArtifactSize().ifPresent(result::setOverallArtifactVolumeInBytes);
        result.setActions(actionRepository.count());

        return result;
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/OfflineDsAssignmentStrategy.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/OfflineDsAssignmentStrategy.java
@@ -67,10 +67,15 @@ public class OfflineDsAssignmentStrategy extends AbstractDsAssignmentStrategy {
    }

    @Override
-    public Set<Long> findTargetIdsToCancel(final List<List<Long>> targetIds) {
+    public Set<Long> cancelActiveActions(final List<List<Long>> targetIds) {
        return Collections.emptySet();
    }

+    @Override
+    void closeActiveActions(final List<List<Long>> targetIds) {
+        // Not supported by offline case
+    }
+
    @Override
    void updateTargetStatus(final JpaDistributionSet set, final List<List<Long>> targetIds, final String currentUser) {
        targetIds.forEach(tIds -> targetRepository.setAssignedAndInstalledDistributionSetAndUpdateStatus(
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/OnlineDsAssignmentStrategy.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/OnlineDsAssignmentStrategy.java
@@ -67,11 +67,16 @@ public class OnlineDsAssignmentStrategy extends AbstractDsAssignmentStrategy {
    }

    @Override
-    Set<Long> findTargetIdsToCancel(final List<List<Long>> targetIds) {
+    Set<Long> cancelActiveActions(final List<List<Long>> targetIds) {
        return targetIds.stream().map(this::overrideObsoleteUpdateActions).flatMap(Collection::stream)
                .collect(Collectors.toSet());
    }

+    @Override
+    void closeActiveActions(final List<List<Long>> targetIds) {
+        targetIds.forEach(this::closeObsoleteUpdateActions);
+    }
+
    @Override
    void updateTargetStatus(final JpaDistributionSet set, final List<List<Long>> targetIds, final String currentUser) {
        targetIds.forEach(tIds -> targetRepository.setAssignedDistributionSetAndUpdateStatus(TargetUpdateStatus.PENDING,
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java
@@ -531,10 +531,12 @@ public class RepositoryApplicationConfiguration extends JpaBaseConfiguration {
            final TargetManagement targetManagement, final AuditorAware<String> auditorProvider,
            final ApplicationEventPublisher eventPublisher, final ApplicationContext applicationContext,
            final AfterTransactionCommitExecutor afterCommit, final VirtualPropertyReplacer virtualPropertyReplacer,
-            final PlatformTransactionManager txManager) {
+            final PlatformTransactionManager txManager,
+            final TenantConfigurationManagement tenantConfigurationManagement,
+            final SystemSecurityContext systemSecurityContext) {
        return new JpaDeploymentManagement(entityManager, actionRepository, distributionSetRepository, targetRepository,
                actionStatusRepository, targetManagement, auditorProvider, eventPublisher, applicationContext,
-                afterCommit, virtualPropertyReplacer, txManager);
+                afterCommit, virtualPropertyReplacer, txManager, tenantConfigurationManagement, systemSecurityContext);
    }

    /**
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaDistributionSet.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaDistributionSet.java
@@ -272,7 +272,7 @@ public class JpaDistributionSet extends AbstractJpaNamedVersionedEntity implemen

        if (allready >= softwareModule.getType().getMaxAssignments()) {
            modules.stream().filter(module -> module.getType().getKey().equals(softwareModule.getType().getKey()))
-                    .findAny().map(modules::remove);
+                    .findAny().ifPresent(modules::remove);
        }

        if (modules.add(softwareModule)) {
--- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/DeploymentManagementTest.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/DeploymentManagementTest.java
@@ -55,6 +55,7 @@ import org.eclipse.hawkbit.repository.model.TargetUpdateStatus;
 import org.eclipse.hawkbit.repository.model.TargetWithActionType;
 import org.eclipse.hawkbit.repository.test.matcher.Expect;
 import org.eclipse.hawkbit.repository.test.matcher.ExpectEvents;
+import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey;
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -460,6 +461,52 @@ public class DeploymentManagementTest extends AbstractJpaIntegrationTest {
                .allMatch(target -> target.getLastModifiedAt().equals(target.getInstallationDate()));
    }

+    @Test
+    @Description("Verifies that if an account is set to action autoclose running actions in case of a new assigned set get closed and set to CANCELED.")
+    @ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 10),
+            @Expect(type = TargetUpdatedEvent.class, count = 20), @Expect(type = ActionCreatedEvent.class, count = 20),
+            @Expect(type = ActionUpdatedEvent.class, count = 10),
+            @Expect(type = DistributionSetCreatedEvent.class, count = 2),
+            @Expect(type = SoftwareModuleCreatedEvent.class, count = 6),
+            @Expect(type = TargetAssignDistributionSetEvent.class, count = 20) })
+    public void assigneDistributionSetAndAutoCloseActiveActions() {
+        tenantConfigurationManagement
+                .addOrUpdateConfiguration(TenantConfigurationKey.REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED, true);
+
+        try {
+            final List<Target> targets = testdataFactory.createTargets(10);
+
+            // First assignment
+            final DistributionSet ds1 = testdataFactory.createDistributionSet("1");
+            assignDistributionSet(ds1, targets);
+
+            List<Action> assignmentOne = actionRepository.findByDistributionSetId(PAGE, ds1.getId()).getContent();
+            assertThat(assignmentOne).hasSize(10).as("Is active").allMatch(Action::isActive).as("Is assigned DS")
+                    .allMatch(action -> action.getDistributionSet().getId() == ds1.getId()).as("Is running")
+                    .allMatch(action -> action.getStatus() == Status.RUNNING);
+
+            // Second assignment
+            final DistributionSet ds2 = testdataFactory.createDistributionSet("2");
+            assignDistributionSet(ds2, targets);
+
+            final List<Action> assignmentTwo = actionRepository.findByDistributionSetId(PAGE, ds2.getId()).getContent();
+            assignmentOne = actionRepository.findByDistributionSetId(PAGE, ds1.getId()).getContent();
+            assertThat(assignmentTwo).hasSize(10).as("Is active").allMatch(Action::isActive).as("Is assigned DS")
+                    .allMatch(action -> action.getDistributionSet().getId() == ds2.getId()).as("Is running")
+                    .allMatch(action -> action.getStatus() == Status.RUNNING);
+            assertThat(assignmentOne).hasSize(10).as("Is active").allMatch(action -> !action.isActive())
+                    .as("Is assigned to DS").allMatch(action -> action.getDistributionSet().getId() == ds1.getId())
+                    .as("Is cancelled").allMatch(action -> action.getStatus() == Status.CANCELED);
+
+            assertThat(targetManagement.findByAssignedDistributionSet(PAGE, ds2.getId()).getContent()).hasSize(10)
+                    .as("InstallationDate not set").allMatch(target -> (target.getInstallationDate() == null));
+
+        } finally {
+            tenantConfigurationManagement
+                    .addOrUpdateConfiguration(TenantConfigurationKey.REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED, false);
+        }
+    }
+
    /**
     * test a simple deployment by calling the
     * {@link TargetRepository#assignDistributionSet(DistributionSet, Iterable)}