From fb59dca1687b4d54b1313ddebff63bb4d8de35f9 Mon Sep 17 00:00:00 2001 From: kaizimmerm Date: Mon, 20 Jun 2016 08:46:50 +0200 Subject: [PATCH] Added configurable late feedback functionality, i.e. action feedback still allowed even for closed action. Signed-off-by: kaizimmerm --- .../repository/RepositoryProperties.java | 29 ++++++ .../hawkbit-repository-jpa/pom.xml | 8 ++ .../RepositoryApplicationConfiguration.java | 5 +- .../jpa/JpaControllerManagement.java | 31 +++++- .../jpa/ControllerManagementTest.java | 95 ++++++++++++++++++- 5 files changed, 161 insertions(+), 7 deletions(-) create mode 100644 hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RepositoryProperties.java diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RepositoryProperties.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RepositoryProperties.java new file mode 100644 index 000000000..982003812 --- /dev/null +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RepositoryProperties.java @@ -0,0 +1,29 @@ +package org.eclipse.hawkbit.repository; + +import org.eclipse.hawkbit.repository.model.ActionStatus; +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * Configuration properties for the repository. + * + */ +@ConfigurationProperties("hawkbit.server.repository") +public class RepositoryProperties { + + /** + * Set to true if the repository has to reject + * {@link ActionStatus} entries for actions that are closed. Note: if this + * is enforced you have to make sure that the feedback channel from the + * devices i in order. + */ + private boolean rejectActionStatusForClosedAction = false; + + public boolean isRejectActionStatusForClosedAction() { + return rejectActionStatusForClosedAction; + } + + public void setRejectActionStatusForClosedAction(final boolean rejectActionStatusForClosedAction) { + this.rejectActionStatusForClosedAction = rejectActionStatusForClosedAction; + } + +} diff --git a/hawkbit-repository/hawkbit-repository-jpa/pom.xml b/hawkbit-repository/hawkbit-repository-jpa/pom.xml index bd56e9ca5..b1d60ec08 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/pom.xml +++ b/hawkbit-repository/hawkbit-repository-jpa/pom.xml @@ -136,6 +136,14 @@ fest-assert test + + org.springframework.boot + spring-boot-starter-hornetq + + + org.springframework.boot + spring-boot-starter-hornetq + diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/RepositoryApplicationConfiguration.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/RepositoryApplicationConfiguration.java index 50207037a..0f1d98166 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/RepositoryApplicationConfiguration.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/RepositoryApplicationConfiguration.java @@ -11,6 +11,7 @@ package org.eclipse.hawkbit; import java.util.HashMap; import java.util.Map; +import org.eclipse.hawkbit.repository.RepositoryProperties; import org.eclipse.hawkbit.repository.SystemManagement; import org.eclipse.hawkbit.repository.TenantConfigurationManagement; import org.eclipse.hawkbit.repository.jpa.aspects.ExceptionMappingAspectHandler; @@ -27,6 +28,7 @@ import org.eclipse.hawkbit.security.SystemSecurityContext; import org.eclipse.hawkbit.tenancy.TenantAware; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.orm.jpa.JpaBaseConfiguration; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.Configuration; @@ -40,7 +42,7 @@ import org.springframework.transaction.annotation.EnableTransactionManagement; import org.springframework.validation.beanvalidation.MethodValidationPostProcessor; /** - * General configuration for the SP Repository. + * General configuration for hawlBit's Repository. * */ @EnableJpaRepositories(basePackages = { "org.eclipse.hawkbit.repository.jpa" }) @@ -50,6 +52,7 @@ import org.springframework.validation.beanvalidation.MethodValidationPostProcess @Configuration @ComponentScan @EnableAutoConfiguration +@EnableConfigurationProperties(RepositoryProperties.class) public class RepositoryApplicationConfiguration extends JpaBaseConfiguration { /** * @return the {@link SystemSecurityContext} singleton bean which make it diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaControllerManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaControllerManagement.java index 77b71e661..b8922c151 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaControllerManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaControllerManagement.java @@ -21,6 +21,7 @@ import javax.validation.constraints.NotNull; import org.eclipse.hawkbit.repository.ControllerManagement; import org.eclipse.hawkbit.repository.RepositoryConstants; +import org.eclipse.hawkbit.repository.RepositoryProperties; import org.eclipse.hawkbit.repository.TargetManagement; import org.eclipse.hawkbit.repository.TenantConfigurationManagement; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; @@ -94,6 +95,9 @@ public class JpaControllerManagement implements ControllerManagement { @Autowired private HawkbitSecurityProperties securityProperties; + @Autowired + private RepositoryProperties repositoryProperties; + @Autowired private TenantConfigurationRepository tenantConfigurationRepository; @@ -251,7 +255,13 @@ public class JpaControllerManagement implements ControllerManagement { public Action addUpdateActionStatus(@NotNull final ActionStatus actionStatus) { final JpaAction action = (JpaAction) actionStatus.getAction(); - if (!action.isActive()) { + // TODO: test + // if action is already closed we accept further status updates on if + // permitted so by configuration. This is especially use full if the + // action status feedback channel order from the device cannot be + // guaranteed. However, if an action is closed we do not accept further + // close messages. + if (actionIsNotActiveButIntermediateFeedbackStillAllowed(actionStatus, action)) { LOG.debug("Update of actionStatus {} for action {} not possible since action not active anymore.", actionStatus.getId(), action.getId()); return action; @@ -259,6 +269,12 @@ public class JpaControllerManagement implements ControllerManagement { return handleAddUpdateActionStatus((JpaActionStatus) actionStatus, action); } + private boolean actionIsNotActiveButIntermediateFeedbackStillAllowed(final ActionStatus actionStatus, + final JpaAction action) { + return !action.isActive() && (repositoryProperties.isRejectActionStatusForClosedAction() + || (Status.ERROR.equals(actionStatus.getStatus()) || Status.FINISHED.equals(actionStatus.getStatus()))); + } + /** * Sets {@link TargetUpdateStatus} based on given {@link ActionStatus}. * @@ -286,8 +302,7 @@ public class JpaControllerManagement implements ControllerManagement { case CANCELED: case WARNING: case RUNNING: - DeploymentHelper.updateTargetInfo(mergedTarget, TargetUpdateStatus.PENDING, false, targetInfoRepository, - entityManager); + handleIntermediateFeedback(mergedAction, mergedTarget); break; default: break; @@ -300,6 +315,16 @@ public class JpaControllerManagement implements ControllerManagement { return actionRepository.save(mergedAction); } + private void handleIntermediateFeedback(final JpaAction mergedAction, final JpaTarget mergedTarget) { + // we change the target state only if the action is still running + // otherwise this is considered as late feedback that does not have + // an impact on the state anymore. + if (mergedAction.isActive()) { + DeploymentHelper.updateTargetInfo(mergedTarget, TargetUpdateStatus.PENDING, false, targetInfoRepository, + entityManager); + } + } + private void handleErrorOnAction(final JpaAction mergedAction, final JpaTarget mergedTarget) { mergedAction.setActive(false); mergedAction.setStatus(Status.ERROR); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ControllerManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ControllerManagementTest.java index f33c5c170..77adfa45b 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ControllerManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ControllerManagementTest.java @@ -17,6 +17,7 @@ import java.util.List; import javax.validation.ConstraintViolationException; import org.apache.commons.lang3.RandomStringUtils; +import org.eclipse.hawkbit.repository.RepositoryProperties; import org.eclipse.hawkbit.repository.jpa.model.JpaActionStatus; import org.eclipse.hawkbit.repository.jpa.model.JpaTarget; import org.eclipse.hawkbit.repository.model.Action; @@ -26,6 +27,7 @@ import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.Target; import org.eclipse.hawkbit.repository.model.TargetUpdateStatus; import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; import ru.yandex.qatools.allure.annotations.Description; import ru.yandex.qatools.allure.annotations.Features; @@ -34,6 +36,8 @@ import ru.yandex.qatools.allure.annotations.Stories; @Features("Component Tests - Repository") @Stories("Controller Management") public class ControllerManagementTest extends AbstractJpaIntegrationTest { + @Autowired + private RepositoryProperties repositoryProperties; @Test @Description("Controller adds a new action status.") @@ -94,7 +98,7 @@ public class ControllerManagementTest extends AbstractJpaIntegrationTest { @Test @Description("Controller trys to finish an update process after it has been finished by an error action status.") - public void tryToFinishUpdateProcessMoreThenOnce() { + public void tryToFinishUpdateProcessMoreThanOnce() { // mock final Target target = new JpaTarget("Rabbit"); @@ -120,16 +124,101 @@ public class ControllerManagementTest extends AbstractJpaIntegrationTest { assertThat(targetManagement.findTargetByControllerID("Rabbit").getTargetInfo().getUpdateStatus()) .isEqualTo(TargetUpdateStatus.ERROR); + // try with disabled late feedback + repositoryProperties.setRejectActionStatusForClosedAction(true); final ActionStatus actionStatusMessage3 = new JpaActionStatus(savedAction, Action.Status.FINISHED, System.currentTimeMillis()); actionStatusMessage3.addMessage("finish"); - controllerManagament.addUpdateActionStatus(actionStatusMessage3); + savedAction = controllerManagament.addUpdateActionStatus(actionStatusMessage3); - targetManagement.findTargetByControllerID("Rabbit").getTargetInfo().getUpdateStatus(); + // test + assertThat(targetManagement.findTargetByControllerID("Rabbit").getTargetInfo().getUpdateStatus()) + .isEqualTo(TargetUpdateStatus.ERROR); + + // try with enabled late feedback + repositoryProperties.setRejectActionStatusForClosedAction(false); + final ActionStatus actionStatusMessage4 = new JpaActionStatus(savedAction, Action.Status.FINISHED, + System.currentTimeMillis()); + actionStatusMessage4.addMessage("finish"); + controllerManagament.addUpdateActionStatus(actionStatusMessage3); // test assertThat(targetManagement.findTargetByControllerID("Rabbit").getTargetInfo().getUpdateStatus()) .isEqualTo(TargetUpdateStatus.ERROR); } + + @Test + @Description("Controller trys to send an update feedback after it has been finished which is reject as the repository is " + + "configured to reject that.") + public void sendUpdatesForFinishUpdateProcessDropedIfDisabled() { + repositoryProperties.setRejectActionStatusForClosedAction(true); + + final Action action = prepareFinishedUpdate("Rabbit"); + + final ActionStatus actionStatusMessage1 = new JpaActionStatus(action, Action.Status.RUNNING, + System.currentTimeMillis()); + actionStatusMessage1.addMessage("got some additional feedback"); + controllerManagament.addUpdateActionStatus(actionStatusMessage1); + + // nothing changed as "feedback after close" is disabled + assertThat(targetManagement.findTargetByControllerID("Rabbit").getTargetInfo().getUpdateStatus()) + .isEqualTo(TargetUpdateStatus.IN_SYNC); + assertThat(actionStatusRepository.findAll(pageReq).getNumberOfElements()).isEqualTo(3); + assertThat(deploymentManagement.findActionStatusByAction(pageReq, action).getNumberOfElements()).isEqualTo(3); + } + + @Test + @Description("Controller trys to send an update feedback after it has been finished which is actepted as the repository is " + + "configured to accept them.") + public void sendUpdatesForFinishUpdateProcessAcceptedIfEnabled() { + repositoryProperties.setRejectActionStatusForClosedAction(false); + + Action action = prepareFinishedUpdate("Rabbit"); + + final ActionStatus actionStatusMessage1 = new JpaActionStatus(action, Action.Status.RUNNING, + System.currentTimeMillis()); + actionStatusMessage1.addMessage("got some additional feedback"); + action = controllerManagament.addUpdateActionStatus(actionStatusMessage1); + + // nothing changed as "feedback after close" is disabled + assertThat(targetManagement.findTargetByControllerID("Rabbit").getTargetInfo().getUpdateStatus()) + .isEqualTo(TargetUpdateStatus.IN_SYNC); + assertThat(actionStatusRepository.findAll(pageReq).getNumberOfElements()).isEqualTo(4); + assertThat(deploymentManagement.findActionStatusByAction(pageReq, action).getNumberOfElements()).isEqualTo(4); + } + + private Action prepareFinishedUpdate(final String controllerId) { + // mock + final Target target = new JpaTarget(controllerId); + final DistributionSet ds = testdataFactory.createDistributionSet(""); + Target savedTarget = targetManagement.createTarget(target); + final List toAssign = new ArrayList<>(); + toAssign.add(savedTarget); + savedTarget = deploymentManagement.assignDistributionSet(ds, toAssign).getAssignedEntity().iterator().next(); + Action savedAction = deploymentManagement.findActiveActionsByTarget(savedTarget).get(0); + + // test and verify + final ActionStatus actionStatusMessage = new JpaActionStatus(savedAction, Action.Status.RUNNING, + System.currentTimeMillis()); + actionStatusMessage.addMessage("running"); + savedAction = controllerManagament.addUpdateActionStatus(actionStatusMessage); + assertThat(targetManagement.findTargetByControllerID(controllerId).getTargetInfo().getUpdateStatus()) + .isEqualTo(TargetUpdateStatus.PENDING); + + final ActionStatus actionStatusMessage2 = new JpaActionStatus(savedAction, Action.Status.FINISHED, + System.currentTimeMillis()); + actionStatusMessage2.addMessage("finish"); + savedAction = controllerManagament.addUpdateActionStatus(actionStatusMessage2); + + // test + assertThat(targetManagement.findTargetByControllerID(controllerId).getTargetInfo().getUpdateStatus()) + .isEqualTo(TargetUpdateStatus.IN_SYNC); + + assertThat(actionStatusRepository.findAll(pageReq).getNumberOfElements()).isEqualTo(3); + assertThat(deploymentManagement.findActionStatusByAction(pageReq, savedAction).getNumberOfElements()) + .isEqualTo(3); + + return savedAction; + } }