From 979c2a21d6ca5dc02667ff0dedb9b7ddee8da237 Mon Sep 17 00:00:00 2001 From: Jonathan Philip Knoblauch Date: Fri, 28 Oct 2016 11:17:23 +0200 Subject: [PATCH] Refactoring of query and tests Signed-off-by: Jonathan Philip Knoblauch --- .../repository/RolloutGroupManagement.java | 8 +- .../jpa/JpaRolloutGroupManagement.java | 7 +- .../repository/jpa/JpaRolloutManagement.java | 34 ++++---- .../repository/jpa/RolloutManagementTest.java | 85 ++++++++----------- 4 files changed, 63 insertions(+), 71 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RolloutGroupManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RolloutGroupManagement.java index 3c87a6fbd..292777e62 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RolloutGroupManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RolloutGroupManagement.java @@ -154,10 +154,10 @@ public interface RolloutGroupManagement { /** * Count targets of rollout group. * - * @param rolloutGroup - * the rollout group for the count + * @param rolloutGroupId + * the rollout group id for the count * @return the target rollout group count */ @PreAuthorize(SpringEvalExpressions.HAS_AUTH_ROLLOUT_MANAGEMENT_READ) - Long countTargetsOfRolloutsGroup(@NotNull RolloutGroup rolloutGroup); -} \ No newline at end of file + Long countTargetsOfRolloutsGroup(@NotNull Long rolloutGroupId); +} diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutGroupManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutGroupManagement.java index 6a447c5fe..f5873b02c 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutGroupManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutGroupManagement.java @@ -206,14 +206,13 @@ public class JpaRolloutGroupManagement implements RolloutGroupManagement { } @Override - public Long countTargetsOfRolloutsGroup(@NotNull final RolloutGroup rolloutGroup) { + public Long countTargetsOfRolloutsGroup(@NotNull final Long rolloutGroupId) { final CriteriaBuilder cb = entityManager.getCriteriaBuilder(); final CriteriaQuery countQuery = cb.createQuery(Long.class); final Root countQueryFrom = countQuery.from(RolloutTargetGroup.class); - countQuery.select(cb.count(countQueryFrom)) - .where(cb.equal(countQueryFrom.get(RolloutTargetGroup_.rolloutGroup), rolloutGroup)); + countQuery.select(cb.count(countQueryFrom)).where(cb + .equal(countQueryFrom.get(RolloutTargetGroup_.rolloutGroup).get(JpaRolloutGroup_.id), rolloutGroupId)); return entityManager.createQuery(countQuery).getSingleResult(); - } } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java index 8d53f4679..bfe1097b7 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java @@ -464,7 +464,7 @@ public class JpaRolloutManagement implements RolloutManagement { private void executeRolloutGroups(final JpaRollout rollout, final List rolloutGroups) { for (final JpaRolloutGroup rolloutGroup : rolloutGroups) { - checkIfTargetsOfRolloutGroupDeleted(rolloutGroup); + checkIfTargetsWhereDeleted(rolloutGroup); // error state check, do we need to stop the whole // rollout because of error? @@ -486,23 +486,27 @@ public class JpaRolloutManagement implements RolloutManagement { } } - private void checkIfTargetsOfRolloutGroupDeleted(final JpaRolloutGroup rolloutGroup) { + private void checkIfTargetsWhereDeleted(final JpaRolloutGroup rolloutGroup) { - final long countTargetsOfRolloutGroup = rolloutGroupManagement.countTargetsOfRolloutsGroup(rolloutGroup); - if (rolloutGroup.getTotalTargets() != countTargetsOfRolloutGroup) { - // targets have been deleted and we have to update the - // total target count in the rollout and the rollout group - final JpaRollout jpaRollout = (JpaRollout) rolloutGroup.getRollout(); - final long updatedTargetCount = jpaRollout.getTotalTargets() - - (rolloutGroup.getTotalTargets() - countTargetsOfRolloutGroup); - jpaRollout.setTotalTargets(updatedTargetCount); - final JpaRolloutGroup jpaRolloutGroup = rolloutGroup; - jpaRolloutGroup.setTotalTargets((int) countTargetsOfRolloutGroup); - - rolloutRepository.save(jpaRollout); - rolloutGroupRepository.save(jpaRolloutGroup); + final long countTargetsOfRolloutGroup = rolloutGroupManagement + .countTargetsOfRolloutsGroup(rolloutGroup.getId()); + if (rolloutGroup.getTotalTargets() == countTargetsOfRolloutGroup) { + return; } + // else targets have been deleted and we have to update the + // total target count in the rollout and the rollout group + updateTargetCount(rolloutGroup, countTargetsOfRolloutGroup); + } + private void updateTargetCount(final JpaRolloutGroup rolloutGroup, final long countTargetsOfRolloutGroup) { + final JpaRollout jpaRollout = (JpaRollout) rolloutGroup.getRollout(); + final long updatedTargetCount = jpaRollout.getTotalTargets() + - (rolloutGroup.getTotalTargets() - countTargetsOfRolloutGroup); + jpaRollout.setTotalTargets(updatedTargetCount); + final JpaRolloutGroup jpaRolloutGroup = rolloutGroup; + jpaRolloutGroup.setTotalTargets((int) countTargetsOfRolloutGroup); + rolloutRepository.save(jpaRollout); + rolloutGroupRepository.save(jpaRolloutGroup); } private void executeLatestRolloutGroup(final JpaRollout rollout) { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/RolloutManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/RolloutManagementTest.java index eb7a2e37f..8969f14a5 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/RolloutManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/RolloutManagementTest.java @@ -96,11 +96,8 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { final int amountGroups = 5; final String successCondition = "50"; final String errorCondition = "80"; - final Rollout createdRollout = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout, - amountOtherTargets, amountGroups, successCondition, errorCondition); - - // start the rollout - rolloutManagement.startRollout(createdRollout); + final Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups, + successCondition, errorCondition); // verify first group is running final RolloutGroup firstGroup = rolloutGroupManagement.findRolloutGroupsByRolloutId(createdRollout.getId(), @@ -130,10 +127,9 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { final int amountGroups = 5; final String successCondition = "50"; final String errorCondition = "80"; - final Rollout createdRollout = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout, - amountOtherTargets, amountGroups, successCondition, errorCondition); + final Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups, + successCondition, errorCondition); - rolloutManagement.startRollout(createdRollout); final List runningActions = deploymentManagement.findActionsByRolloutAndStatus(createdRollout, Status.RUNNING); // finish one action should be sufficient due the finish condition is at @@ -177,18 +173,18 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { finishActionAndDeleteTargetsOfFirstRunningGroup(createdRollout); - checkStatusOfRolloutGroupsAfterDeletingTargets(createdRollout); + checkSecondGroupStatusIsRunning(createdRollout); finishActionAndDeleteTargetsOfSecondRunningGroup(createdRollout); - deleteThrirdGroup(createdRollout); + deleteAllTargetsFromThirdGroup(createdRollout); checkStatusOfRolloutAndRolloutGroupsAfterDeletingTargets(createdRollout); } @Step("Create a rollout by the given parameter and start it.") - public Rollout createAndStartRollout(final int amountTargetsForRollout, final int amountOtherTargets, + private Rollout createAndStartRollout(final int amountTargetsForRollout, final int amountOtherTargets, final int amountGroups, final String successCondition, final String errorCondition) { final Rollout createdRollout = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout, @@ -198,7 +194,7 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { } @Step("Finish three actions of the rollout group and delete two targets") - public void finishActionAndDeleteTargetsOfFirstRunningGroup(final Rollout createdRollout) { + private void finishActionAndDeleteTargetsOfFirstRunningGroup(final Rollout createdRollout) { // finish group one by finishing targets and deleting targets final List runningActions = deploymentManagement.findActionsByRolloutAndStatus(createdRollout, Status.RUNNING); @@ -209,8 +205,8 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { runningActions.get(4).getTarget().getId()); } - @Step("Check the status of the rollout groups") - public void checkStatusOfRolloutGroupsAfterDeletingTargets(final Rollout createdRollout) { + @Step("Check the status of the rollout groups, second group should be in running status") + private void checkSecondGroupStatusIsRunning(final Rollout createdRollout) { rolloutManagement.checkRunningRollouts(0); // validate that the second group is in running state final List runningRolloutGroups = rolloutGroupManagement.findRolloutGroupsByRolloutId( @@ -221,7 +217,7 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { } @Step("Finish one action of the rollout group and delete four targets") - public void finishActionAndDeleteTargetsOfSecondRunningGroup(final Rollout createdRollout) { + private void finishActionAndDeleteTargetsOfSecondRunningGroup(final Rollout createdRollout) { final List runningActions = deploymentManagement.findActionsByRolloutAndStatus(createdRollout, Status.RUNNING); finishAction(runningActions.get(0)); @@ -232,7 +228,7 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { } @Step("Delete all targets of the rollout group") - public void deleteThrirdGroup(final Rollout createdRollout) { + private void deleteAllTargetsFromThirdGroup(final Rollout createdRollout) { // delete all targets of the third group final List runningActions = deploymentManagement.findActionsByRolloutAndStatus(createdRollout, Status.SCHEDULED); @@ -242,7 +238,7 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { } @Step("Check the status of the rollout groups and the rollout") - public void checkStatusOfRolloutAndRolloutGroupsAfterDeletingTargets(final Rollout createdRollout) { + private void checkStatusOfRolloutAndRolloutGroupsAfterDeletingTargets(final Rollout createdRollout) { rolloutManagement.checkRunningRollouts(0); final List runningRolloutGroups = rolloutGroupManagement.findRolloutGroupsByRolloutId( createdRollout.getId(), new OffsetBasedPageRequest(0, 10, new Sort(Direction.ASC, "id"))).getContent(); @@ -269,10 +265,9 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { final int amountGroups = 5; final String successCondition = "50"; final String errorCondition = "80"; - final Rollout createdRollout = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout, - amountOtherTargets, amountGroups, successCondition, errorCondition); + final Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups, + successCondition, errorCondition); - rolloutManagement.startRollout(createdRollout); // set both actions in error state so error condition is hit and error // action is executed final List runningActions = deploymentManagement.findActionsByRolloutAndStatus(createdRollout, @@ -313,9 +308,9 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { final int amountGroups = 5; final String successCondition = "50"; final String errorCondition = "80"; - final Rollout createdRollout = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout, - amountOtherTargets, amountGroups, successCondition, errorCondition); - rolloutManagement.startRollout(createdRollout); + final Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups, + successCondition, errorCondition); + // set both actions in error state so error condition is hit and error // action is executed final List runningActions = deploymentManagement.findActionsByRolloutAndStatus(createdRollout, @@ -365,9 +360,9 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { final int amountGroups = 5; final String successCondition = "50"; final String errorCondition = "80"; - final Rollout createdRollout = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout, - amountOtherTargets, amountGroups, successCondition, errorCondition); - rolloutManagement.startRollout(createdRollout); + final Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups, + successCondition, errorCondition); + // finish running actions, 2 actions should be finished assertThat(changeStatusForAllRunningActions(createdRollout, Status.FINISHED)).isEqualTo(2); @@ -497,10 +492,9 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { final int amountGroups = 4; final String successCondition = "50"; final String errorCondition = "80"; - Rollout createdRollout = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout, - amountOtherTargets, amountGroups, successCondition, errorCondition); + Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups, + successCondition, errorCondition); - rolloutManagement.startRollout(createdRollout); changeStatusForAllRunningActions(createdRollout, Status.FINISHED); rolloutManagement.checkRunningRollouts(0); @@ -529,11 +523,10 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { final int amountGroups = 2; final String successCondition = "50"; final String errorCondition = "80"; - Rollout createdRollout = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout, - amountOtherTargets, amountGroups, successCondition, errorCondition); + Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups, + successCondition, errorCondition); final DistributionSet ds = createdRollout.getDistributionSet(); - rolloutManagement.startRollout(createdRollout); createdRollout = rolloutManagement.findRolloutById(createdRollout.getId()); // 5 targets are running final List runningActions = deploymentManagement.findActionsByRolloutAndStatus(createdRollout, @@ -574,9 +567,9 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { final int amountGroups = 3; final String successCondition = "50"; final String errorCondition = "80"; - Rollout rolloutOne = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout, - amountOtherTargets, amountGroups, successCondition, errorCondition); - rolloutManagement.startRollout(rolloutOne); + Rollout rolloutOne = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups, + successCondition, errorCondition); + rolloutOne = rolloutManagement.findRolloutById(rolloutOne.getId()); final DistributionSet dsForRolloutTwo = testdataFactory.createDistributionSet("dsForRolloutTwo"); @@ -611,10 +604,10 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { final int amountGroups = 3; final String successCondition = "50"; final String errorCondition = "80"; - Rollout rolloutOne = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout, - amountOtherTargets, amountGroups, successCondition, errorCondition); + Rollout rolloutOne = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups, + successCondition, errorCondition); final DistributionSet distributionSet = rolloutOne.getDistributionSet(); - rolloutManagement.startRollout(rolloutOne); + rolloutOne = rolloutManagement.findRolloutById(rolloutOne.getId()); changeStatusForRunningActions(rolloutOne, Status.ERROR, 2); changeStatusForRunningActions(rolloutOne, Status.FINISHED, 3); @@ -666,10 +659,9 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { final int amountGroups = 2; final String successCondition = "50"; final String errorCondition = "80"; - Rollout rolloutOne = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout, - amountOtherTargets, amountGroups, successCondition, errorCondition); + Rollout rolloutOne = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups, + successCondition, errorCondition); - rolloutManagement.startRollout(rolloutOne); rolloutOne = rolloutManagement.findRolloutById(rolloutOne.getId()); changeStatusForRunningActions(rolloutOne, Status.ERROR, 2); changeStatusForRunningActions(rolloutOne, Status.FINISHED, 3); @@ -691,10 +683,9 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { final int amountGroups = 2; final String successCondition = "80"; final String errorCondition = "90"; - Rollout rolloutOne = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout, - amountOtherTargets, amountGroups, successCondition, errorCondition); + Rollout rolloutOne = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups, + successCondition, errorCondition); - rolloutManagement.startRollout(rolloutOne); rolloutOne = rolloutManagement.findRolloutById(rolloutOne.getId()); changeStatusForRunningActions(rolloutOne, Status.ERROR, 2); changeStatusForRunningActions(rolloutOne, Status.FINISHED, 3); @@ -717,10 +708,9 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { final int amountGroups = 2; final String successCondition = "50"; final String errorCondition = "20"; - Rollout rolloutOne = createSimpleTestRolloutWithTargetsAndDistributionSet(amountTargetsForRollout, - amountOtherTargets, amountGroups, successCondition, errorCondition); + Rollout rolloutOne = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups, + successCondition, errorCondition); - rolloutManagement.startRollout(rolloutOne); rolloutOne = rolloutManagement.findRolloutById(rolloutOne.getId()); changeStatusForRunningActions(rolloutOne, Status.ERROR, 2); changeStatusForRunningActions(rolloutOne, Status.FINISHED, 3); @@ -804,7 +794,6 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest { public void rightCountForAllRollouts() { final int amountTargetsForRollout = 6; - ; final int amountGroups = 2; final String successCondition = "50"; final String errorCondition = "80";