Added configurable late feedback functionality, i.e. action feedback

still allowed even for closed action.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

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 <code>true</code> 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;
+    }
+
+}

hawkbit-repository/hawkbit-repository-jpa/pom.xml

@@ -136,6 +136,14 @@
          <artifactId>fest-assert</artifactId>
          <scope>test</scope>
       </dependency>
+      <dependency>
+         <groupId>org.springframework.boot</groupId>
+         <artifactId>spring-boot-starter-hornetq</artifactId>
+      </dependency>
+      <dependency>
+         <groupId>org.springframework.boot</groupId>
+         <artifactId>spring-boot-starter-hornetq</artifactId>
+      </dependency>
    </dependencies>
 
    <build>

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

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,
+            handleIntermediateFeedback(mergedAction, mergedTarget);
-                    entityManager);
             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);

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<Target> 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;
+    }
 }