Refactoring of query and tests

Signed-off-by: Jonathan Philip Knoblauch <JonathanPhilip.Knoblauch@bosch-si.com>

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
+     * @param rolloutGroupId
-     *            the rollout group for the count
+     *            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);
+    Long countTargetsOfRolloutsGroup(@NotNull Long rolloutGroupId);
 }

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<Long> countQuery = cb.createQuery(Long.class);
         final Root<RolloutTargetGroup> countQueryFrom = countQuery.from(RolloutTargetGroup.class);
-        countQuery.select(cb.count(countQueryFrom))
+        countQuery.select(cb.count(countQueryFrom)).where(cb
-                .where(cb.equal(countQueryFrom.get(RolloutTargetGroup_.rolloutGroup), rolloutGroup));
+                .equal(countQueryFrom.get(RolloutTargetGroup_.rolloutGroup).get(JpaRolloutGroup_.id), rolloutGroupId));
         return entityManager.createQuery(countQuery).getSingleResult();
-
     }
 
 }

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<JpaRolloutGroup> 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,25 +486,29 @@ public class JpaRolloutManagement implements RolloutManagement {
         }
     }
 
-    private void checkIfTargetsOfRolloutGroupDeleted(final JpaRolloutGroup rolloutGroup) {
+    private void checkIfTargetsWhereDeleted(final JpaRolloutGroup rolloutGroup) {
 
-        final long countTargetsOfRolloutGroup = rolloutGroupManagement.countTargetsOfRolloutsGroup(rolloutGroup);
+        final long countTargetsOfRolloutGroup = rolloutGroupManagement
-        if (rolloutGroup.getTotalTargets() != countTargetsOfRolloutGroup) {
+                .countTargetsOfRolloutsGroup(rolloutGroup.getId());
-            // targets have been deleted and we have to update the
+        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) {
         final List<JpaRolloutGroup> latestRolloutGroup = rolloutGroupRepository
                 .findByRolloutAndStatusNotOrderByIdDesc(rollout, RolloutGroupStatus.SCHEDULED);

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,
+        final Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups,
-                amountOtherTargets, amountGroups, successCondition, errorCondition);
+                successCondition, errorCondition);
-
-        // start the rollout
-        rolloutManagement.startRollout(createdRollout);
 
         // 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,
+        final Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups,
-                amountOtherTargets, amountGroups, successCondition, errorCondition);
+                successCondition, errorCondition);
 
-        rolloutManagement.startRollout(createdRollout);
         final List<Action> 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<Action> 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")
+    @Step("Check the status of the rollout groups, second group should be in running status")
-    public void checkStatusOfRolloutGroupsAfterDeletingTargets(final Rollout createdRollout) {
+    private void checkSecondGroupStatusIsRunning(final Rollout createdRollout) {
         rolloutManagement.checkRunningRollouts(0);
         // validate that the second group is in running state
         final List<RolloutGroup> 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<Action> 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<Action> 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<RolloutGroup> 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,
+        final Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups,
-                amountOtherTargets, amountGroups, successCondition, errorCondition);
+                successCondition, errorCondition);
 
-        rolloutManagement.startRollout(createdRollout);
         // set both actions in error state so error condition is hit and error
         // action is executed
         final List<Action> 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,
+        final Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups,
-                amountOtherTargets, amountGroups, successCondition, errorCondition);
+                successCondition, errorCondition);
-        rolloutManagement.startRollout(createdRollout);
+
         // set both actions in error state so error condition is hit and error
         // action is executed
         final List<Action> 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,
+        final Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups,
-                amountOtherTargets, amountGroups, successCondition, errorCondition);
+                successCondition, errorCondition);
-        rolloutManagement.startRollout(createdRollout);
+
         // 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,
+        Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups,
-                amountOtherTargets, amountGroups, successCondition, errorCondition);
+                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,
+        Rollout createdRollout = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups,
-                amountOtherTargets, amountGroups, successCondition, errorCondition);
+                successCondition, errorCondition);
         final DistributionSet ds = createdRollout.getDistributionSet();
 
-        rolloutManagement.startRollout(createdRollout);
         createdRollout = rolloutManagement.findRolloutById(createdRollout.getId());
         // 5 targets are running
         final List<Action> 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,
+        Rollout rolloutOne = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups,
-                amountOtherTargets, amountGroups, successCondition, errorCondition);
+                successCondition, errorCondition);
-        rolloutManagement.startRollout(rolloutOne);
+
         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,
+        Rollout rolloutOne = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups,
-                amountOtherTargets, amountGroups, successCondition, errorCondition);
+                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,
+        Rollout rolloutOne = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups,
-                amountOtherTargets, amountGroups, successCondition, errorCondition);
+                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,
+        Rollout rolloutOne = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups,
-                amountOtherTargets, amountGroups, successCondition, errorCondition);
+                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,
+        Rollout rolloutOne = createAndStartRollout(amountTargetsForRollout, amountOtherTargets, amountGroups,
-                amountOtherTargets, amountGroups, successCondition, errorCondition);
+                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";