From 681df6c1f112316d4f6f5b51062f5160e4250087 Mon Sep 17 00:00:00 2001 From: Natalia Kislicyn <47418786+Nkyn@users.noreply.github.com> Date: Tue, 22 Feb 2022 15:09:51 +0100 Subject: [PATCH] Fix installedBase link in DDI base resource (#1232) * add test for installedBase bug Signed-off-by: Natalia Kislicyn * fix for test scenario I Signed-off-by: Natalia Kislicyn * add another test scenario for installedbase bug Signed-off-by: Natalia Kislicyn * add fix for scenario 2a+b Signed-off-by: Natalia Kislicyn --- .../repository/jpa/ActionRepository.java | 11 +- .../jpa/JpaControllerManagement.java | 4 +- .../AbstractDDiApiIntegrationTest.java | 14 ++ .../rest/resource/DdiInstalledBaseTest.java | 150 ++++++++++++++++++ 4 files changed, 173 insertions(+), 6 deletions(-) diff --git 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 index ae1e86ba6..7d6d0cd1e 100644 --- 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 @@ -190,16 +190,19 @@ public interface ActionRepository extends BaseEntityRepository, List 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 findFirstByTargetIdAndDistributionSetId(@Param("target") long targetId, @Param("ds") Long dsId); + Optional findFirstByTargetIdAndDistributionSetIdAndStatusOrderByIdDesc(@Param("target") long targetId, + @Param("ds") Long dsId, @Param("status") Action.Status status); /** * Retrieves all {@link Action}s which are referring the given 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 14f9824d5..4df948243 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 @@ -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(); } diff --git a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/AbstractDDiApiIntegrationTest.java b/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/AbstractDDiApiIntegrationTest.java index efbc251d7..14ba48bf5 100644 --- a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/AbstractDDiApiIntegrationTest.java +++ b/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)) diff --git a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiInstalledBaseTest.java b/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiInstalledBaseTest.java index 81c0c7355..545152703 100644 --- a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiInstalledBaseTest.java +++ b/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 {