Fix installedBase link in DDI base resource (#1232)

* add test for installedBase bug

Signed-off-by: Natalia Kislicyn <natalia.kislicyn@bosch.io>

* fix for test scenario I

Signed-off-by: Natalia Kislicyn <natalia.kislicyn@bosch.io>

* add another test scenario for installedbase bug

Signed-off-by: Natalia Kislicyn <natalia.kislicyn@bosch.io>

* add fix for scenario 2a+b

Signed-off-by: Natalia Kislicyn <natalia.kislicyn@bosch.io>

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

@@ -190,16 +190,19 @@ public interface ActionRepository extends BaseEntityRepository<JpaAction, Long>,
     List<Action> findActionByTargetAndSoftwareModule(@Param("target") String targetId, @Param("module") Long moduleId);
 
     /**
-     * Retrieves latest {@link Action} for given target and {@link DistributionSet}.
+     * Retrieves the latest finished {@link Action} for given target and {@link DistributionSet}.
      *
      * @param targetId
-     *            to search for
+     *            the action belongs to
      * @param dsId
-     *            to search for
+     *            of the ds that is assigned to the target
+     * @param status
+     *            of the action
      * @return action if there is one with assigned target and assigned
      *         {@link DistributionSet}.
      */
-    Optional<Action> findFirstByTargetIdAndDistributionSetId(@Param("target") long targetId, @Param("ds") Long dsId);
+    Optional<Action> findFirstByTargetIdAndDistributionSetIdAndStatusOrderByIdDesc(@Param("target") long targetId,
+            @Param("ds") Long dsId, @Param("status") Action.Status status);
 
     /**
      * Retrieves all {@link Action}s which are referring the given

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

@@ -1067,8 +1067,8 @@ public class JpaControllerManagement extends JpaActionManagement implements Cont
 
         final JpaDistributionSet installedDistributionSet = jpaTarget.getInstalledDistributionSet();
         if (null != installedDistributionSet) {
-            return actionRepository.findFirstByTargetIdAndDistributionSetId(jpaTarget.getId(),
-                    installedDistributionSet.getId());
+            return actionRepository.findFirstByTargetIdAndDistributionSetIdAndStatusOrderByIdDesc(jpaTarget.getId(),
+                    installedDistributionSet.getId(), FINISHED);
         } else {
             return Optional.empty();
         }

hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/AbstractDDiApiIntegrationTest.java

@@ -118,6 +118,20 @@ public abstract class AbstractDDiApiIntegrationTest extends AbstractRestIntegrat
                 .andDo(MockMvcResultPrinter.print()).andExpect(statusMatcher);
     }
 
+    protected ResultActions postCancelFeedback(final String controllerId, final Long actionId, final String content,
+            final ResultMatcher statusMatcher) throws Exception {
+        return postCancelFeedback(MediaType.APPLICATION_JSON, controllerId, actionId, content.getBytes(),
+                statusMatcher);
+    }
+
+    protected ResultActions postCancelFeedback(final MediaType mediaType, final String controllerId,
+            final Long actionId, final byte[] content, final ResultMatcher statusMatcher) throws Exception {
+        return mvc
+                .perform(post(CANCEL_FEEDBACK, tenantAware.getCurrentTenant(), controllerId, actionId).content(content)
+                        .contentType(mediaType).accept(mediaType))
+                .andDo(MockMvcResultPrinter.print()).andExpect(statusMatcher);
+    }
+
     protected ResultActions performGet(final String url, final MediaType mediaType, final ResultMatcher statusMatcher,
             final String... values) throws Exception {
         return mvc.perform(MockMvcRequestBuilders.get(url, values).accept(mediaType))

hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiInstalledBaseTest.java

@@ -160,6 +160,156 @@ public class DdiInstalledBaseTest extends AbstractDDiApiIntegrationTest {
                 actionId1, ds1.findFirstModuleByType(osType).get().getId(), Action.ActionType.SOFT);
     }
 
+    @Test
+    @Description("Test several deployments of same ds to a controller. Checks that cancelled action in history is not linked as installedBase.")
+    public void deploymentActionsOfSameDsWithCancelledActionInHistory() throws Exception {
+        // Prepare test data
+        final Target target = createTargetAndAssertNoActiveActions();
+
+        final DistributionSet ds1 = testdataFactory.createDistributionSet("1", true);
+        final Artifact artifact1 = testdataFactory.createArtifact(RandomUtils.nextBytes(ARTIFACT_SIZE),
+                getOsModule(ds1), "test1", ARTIFACT_SIZE);
+        final Artifact artifactSignature1 = testdataFactory.createArtifact(RandomUtils.nextBytes(ARTIFACT_SIZE),
+                getOsModule(ds1), "test1.signature", ARTIFACT_SIZE);
+
+        // assign ds1, action1 - and provide cancel feedback
+        final Long actionId1 = getFirstAssignedActionId(
+                assignDistributionSet(ds1.getId(), target.getControllerId(), Action.ActionType.SOFT));
+        deploymentManagement.cancelAction(actionId1);
+        postCancelFeedback(target.getControllerId(), actionId1,
+                JsonBuilder.cancelActionFeedback(actionId1.toString(), "closed", "Canceled"), status().isOk());
+
+        // assign ds1, action2 - and provide cancel feedback
+        final Long actionId2 = getFirstAssignedActionId(
+                assignDistributionSet(ds1.getId(), target.getControllerId(), Action.ActionType.FORCED));
+        deploymentManagement.cancelAction(actionId2);
+        postCancelFeedback(target.getControllerId(), actionId2,
+                JsonBuilder.cancelActionFeedback(actionId2.toString(), "closed", "Canceled"), status().isOk());
+
+        // assign ds1, action 3 - and provide success feedback
+        final Long actionId3 = getFirstAssignedActionId(
+                assignDistributionSet(ds1.getId(), target.getControllerId(), Action.ActionType.SOFT));
+        postDeploymentFeedback(target.getControllerId(), actionId3,
+                JsonBuilder.deploymentActionFeedback(actionId3.toString(), "closed", "success", "Closed"),
+                status().isOk());
+
+        // Test: latest succeeded action is returned in installedBase
+        performGet(CONTROLLER_BASE, MediaTypes.HAL_JSON, status().isOk(), tenantAware.getCurrentTenant(), CONTROLLER_ID)
+                .andExpect(jsonPath("$.config.polling.sleep", equalTo("00:01:00")))
+                .andExpect(jsonPath("$._links.installedBase.href",
+                        startsWith(installedBaseLink(CONTROLLER_ID, actionId3.toString()))))
+                .andExpect(jsonPath("$._links.deploymentBase.href").doesNotExist());
+
+        getAndVerifyInstalledBasePayload(CONTROLLER_ID, MediaType.APPLICATION_JSON, ds1, artifact1, artifactSignature1,
+                actionId3, ds1.findFirstModuleByType(osType).get().getId(), Action.ActionType.SOFT);
+
+        // cancelled action are not accessible
+        mvc.perform(MockMvcRequestBuilders.get(INSTALLED_BASE, tenantAware.getCurrentTenant(), target.getControllerId(),
+                actionId1.toString())).andDo(MockMvcResultPrinter.print()).andExpect(status().isNotFound());
+        mvc.perform(MockMvcRequestBuilders.get(INSTALLED_BASE, tenantAware.getCurrentTenant(), target.getControllerId(),
+                actionId2.toString())).andDo(MockMvcResultPrinter.print()).andExpect(status().isNotFound());
+    }
+
+    @Test
+    @Description("Test several deployments of same ds to a controller. Checks that latest cancelled action does not override actual installed ds.")
+    public void deploymentActionsOfSameDsWithCancelledAction() throws Exception {
+        // Prepare test data
+        final Target target = createTargetAndAssertNoActiveActions();
+
+        final DistributionSet ds1 = testdataFactory.createDistributionSet("1", true);
+        final Artifact artifact1 = testdataFactory.createArtifact(RandomUtils.nextBytes(ARTIFACT_SIZE),
+                getOsModule(ds1), "test1", ARTIFACT_SIZE);
+        final Artifact artifactSignature1 = testdataFactory.createArtifact(RandomUtils.nextBytes(ARTIFACT_SIZE),
+                getOsModule(ds1), "test1.signature", ARTIFACT_SIZE);
+
+        final DistributionSet ds2 = testdataFactory.createDistributionSet("2", true);
+
+        // assign ds1, action1 - and provide success feedback
+        final Long actionId1 = getFirstAssignedActionId(
+                assignDistributionSet(ds1.getId(), target.getControllerId(), Action.ActionType.SOFT));
+        postDeploymentFeedback(target.getControllerId(), actionId1,
+                JsonBuilder.deploymentActionFeedback(actionId1.toString(), "closed", "success", "Success"),
+                status().isOk());
+
+        // assign ds2, action2 - assign ds1, action 3 - and cancel both
+        final Long actionId2 = getFirstAssignedActionId(
+                assignDistributionSet(ds2.getId(), target.getControllerId(), Action.ActionType.FORCED));
+        final Long actionId3 = getFirstAssignedActionId(
+                assignDistributionSet(ds1.getId(), target.getControllerId(), Action.ActionType.SOFT));
+        deploymentManagement.cancelAction(actionId2);
+        postCancelFeedback(target.getControllerId(), actionId2,
+                JsonBuilder.cancelActionFeedback(actionId2.toString(), "closed", "Canceled"), status().isOk());
+        deploymentManagement.cancelAction(actionId3);
+        postCancelFeedback(target.getControllerId(), actionId3,
+                JsonBuilder.cancelActionFeedback(actionId3.toString(), "closed", "Canceled"), status().isOk());
+
+        // Test: the succeeded action is returned in installedBase instead of the latest
+        // cancelled action
+        performGet(CONTROLLER_BASE, MediaTypes.HAL_JSON, status().isOk(), tenantAware.getCurrentTenant(), CONTROLLER_ID)
+                .andExpect(jsonPath("$.config.polling.sleep", equalTo("00:01:00")))
+                .andExpect(jsonPath("$._links.installedBase.href",
+                        startsWith(installedBaseLink(CONTROLLER_ID, actionId1.toString()))))
+                .andExpect(jsonPath("$._links.deploymentBase.href").doesNotExist());
+
+        getAndVerifyInstalledBasePayload(CONTROLLER_ID, MediaType.APPLICATION_JSON, ds1, artifact1, artifactSignature1,
+                actionId1, ds1.findFirstModuleByType(osType).get().getId(), Action.ActionType.SOFT);
+
+        // cancelled action are not accessible
+        mvc.perform(MockMvcRequestBuilders.get(INSTALLED_BASE, tenantAware.getCurrentTenant(), target.getControllerId(),
+                actionId2.toString())).andDo(MockMvcResultPrinter.print()).andExpect(status().isNotFound());
+        mvc.perform(MockMvcRequestBuilders.get(INSTALLED_BASE, tenantAware.getCurrentTenant(), target.getControllerId(),
+                actionId3.toString())).andDo(MockMvcResultPrinter.print()).andExpect(status().isNotFound());
+    }
+
+    @Test
+    @Description("Test several deployments of same ds to a controller. Checks that latest running action does not override actual installed ds.")
+    public void deploymentActionsOfSameDsWithRunningAction() throws Exception {
+        // Prepare test data
+        final Target target = createTargetAndAssertNoActiveActions();
+
+        final DistributionSet ds1 = testdataFactory.createDistributionSet("1", true);
+        final Artifact artifact1 = testdataFactory.createArtifact(RandomUtils.nextBytes(ARTIFACT_SIZE),
+                getOsModule(ds1), "test1", ARTIFACT_SIZE);
+        final Artifact artifactSignature1 = testdataFactory.createArtifact(RandomUtils.nextBytes(ARTIFACT_SIZE),
+                getOsModule(ds1), "test1.signature", ARTIFACT_SIZE);
+
+        final DistributionSet ds2 = testdataFactory.createDistributionSet("2", true);
+
+        // assign ds1, action1 - and provide success feedback
+        final Long actionId1 = getFirstAssignedActionId(
+                assignDistributionSet(ds1.getId(), target.getControllerId(), Action.ActionType.SOFT));
+        postDeploymentFeedback(target.getControllerId(), actionId1,
+                JsonBuilder.deploymentActionFeedback(actionId1.toString(), "closed", "success", "Success"),
+                status().isOk());
+
+        // assign ds2, action2 - assign ds1, action 3 - and cancel action 2
+        final Long actionId2 = getFirstAssignedActionId(
+                assignDistributionSet(ds2.getId(), target.getControllerId(), Action.ActionType.FORCED));
+        final Long actionId3 = getFirstAssignedActionId(
+                assignDistributionSet(ds1.getId(), target.getControllerId(), Action.ActionType.SOFT));
+        deploymentManagement.cancelAction(actionId2);
+        postCancelFeedback(target.getControllerId(), actionId2,
+                JsonBuilder.cancelActionFeedback(actionId2.toString(), "closed", "Canceled"), status().isOk());
+
+        // Test: the succeeded action is returned in installedBase instead of the latest
+        // cancelled action
+        performGet(CONTROLLER_BASE, MediaTypes.HAL_JSON, status().isOk(), tenantAware.getCurrentTenant(), CONTROLLER_ID)
+                .andExpect(jsonPath("$.config.polling.sleep", equalTo("00:01:00")))
+                .andExpect(jsonPath("$._links.installedBase.href",
+                        startsWith(installedBaseLink(CONTROLLER_ID, actionId1.toString()))))
+                .andExpect(jsonPath("$._links.deploymentBase.href",
+                        startsWith(deploymentBaseLink(CONTROLLER_ID, actionId3.toString()))));
+
+        getAndVerifyInstalledBasePayload(CONTROLLER_ID, MediaType.APPLICATION_JSON, ds1, artifact1, artifactSignature1,
+                actionId1, ds1.findFirstModuleByType(osType).get().getId(), Action.ActionType.SOFT);
+
+        // cancelled action are not accessible
+        mvc.perform(MockMvcRequestBuilders.get(INSTALLED_BASE, tenantAware.getCurrentTenant(), target.getControllerId(),
+                actionId2.toString())).andDo(MockMvcResultPrinter.print()).andExpect(status().isNotFound());
+        mvc.perform(MockMvcRequestBuilders.get(INSTALLED_BASE, tenantAware.getCurrentTenant(), target.getControllerId(),
+                actionId3.toString())).andDo(MockMvcResultPrinter.print()).andExpect(status().isNotFound());
+    }
+
     @Test
     @Description("Test open deployment to a controller. Checks that installedBase returns 404 for a pending action.")
     public void installedBaseReturns404ForPendingAction() throws Exception {