Second Refactoring after reivew

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

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

@@ -464,12 +464,14 @@ public class JpaRolloutManagement implements RolloutManagement {
     private void executeRolloutGroups(final JpaRollout rollout, final List<JpaRolloutGroup> rolloutGroups) {
         for (final JpaRolloutGroup rolloutGroup : rolloutGroups) {
 
-            checkIfTargetsWhereDeleted(rolloutGroup);
+            final long targetCount = countTargetsFrom(rolloutGroup);
+            if (rolloutGroup.getTotalTargets() != targetCount) {
+                updateTotalTargetCount(rolloutGroup, targetCount);
+            }
 
             // error state check, do we need to stop the whole
             // rollout because of error?
-            final RolloutGroupErrorCondition errorCondition = rolloutGroup.getErrorCondition();
+            final boolean isError = checkErrorState(rollout, rolloutGroup);
-            final boolean isError = checkErrorState(rollout, rolloutGroup, errorCondition);
             if (isError) {
                 LOGGER.info("Rollout {} {} has error, calling error action", rollout.getName(), rollout.getId());
                 callErrorAction(rollout, rolloutGroup);
@@ -486,19 +488,7 @@ public class JpaRolloutManagement implements RolloutManagement {
         }
     }
 
-    private void checkIfTargetsWhereDeleted(final JpaRolloutGroup rolloutGroup) {
+    private void updateTotalTargetCount(final JpaRolloutGroup rolloutGroup, final long countTargetsOfRolloutGroup) {
-
-        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);
@@ -509,6 +499,10 @@ public class JpaRolloutManagement implements RolloutManagement {
         rolloutGroupRepository.save(jpaRolloutGroup);
     }
 
+    private long countTargetsFrom(final JpaRolloutGroup rolloutGroup) {
+        return rolloutGroupManagement.countTargetsOfRolloutsGroup(rolloutGroup.getId());
+    }
+
     private void executeLatestRolloutGroup(final JpaRollout rollout) {
         final List<JpaRolloutGroup> latestRolloutGroup = rolloutGroupRepository
                 .findByRolloutAndStatusNotOrderByIdDesc(rollout, RolloutGroupStatus.SCHEDULED);
@@ -541,8 +535,10 @@ public class JpaRolloutManagement implements RolloutManagement {
         return actionsLeftForRollout == 0;
     }
 
-    private boolean checkErrorState(final Rollout rollout, final RolloutGroup rolloutGroup,
+    private boolean checkErrorState(final Rollout rollout, final RolloutGroup rolloutGroup) {
-            final RolloutGroupErrorCondition errorCondition) {
+
+        final RolloutGroupErrorCondition errorCondition = rolloutGroup.getErrorCondition();
+
         if (errorCondition == null) {
             // there is no error condition, so return false, don't have error.
             return false;

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/RolloutManagementTest.java

@@ -179,7 +179,7 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest {
 
         deleteAllTargetsFromThirdGroup(createdRollout);
 
-        checkStatusOfRolloutAndRolloutGroupsAfterDeletingTargets(createdRollout);
+        verifyRolloutAndAllGroupsAreFinished(createdRollout);
 
     }
 
@@ -208,7 +208,6 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest {
     @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<RolloutGroup> runningRolloutGroups = rolloutGroupManagement.findRolloutGroupsByRolloutId(
                 createdRollout.getId(), new OffsetBasedPageRequest(0, 10, new Sort(Direction.ASC, "id"))).getContent();
         assertThat(runningRolloutGroups.get(0).getStatus()).isEqualTo(RolloutGroupStatus.FINISHED);
@@ -229,7 +228,6 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest {
 
     @Step("Delete all targets of the rollout group")
     private void deleteAllTargetsFromThirdGroup(final Rollout createdRollout) {
-        // delete all targets of the third group
         final List<Action> runningActions = deploymentManagement.findActionsByRolloutAndStatus(createdRollout,
                 Status.SCHEDULED);
         targetManagement.deleteTargets(runningActions.get(0).getTarget().getId(),
@@ -238,7 +236,7 @@ public class RolloutManagementTest extends AbstractJpaIntegrationTest {
     }
 
     @Step("Check the status of the rollout groups and the rollout")
-    private void checkStatusOfRolloutAndRolloutGroupsAfterDeletingTargets(final Rollout createdRollout) {
+    private void verifyRolloutAndAllGroupsAreFinished(final Rollout createdRollout) {
         rolloutManagement.checkRunningRollouts(0);
         final List<RolloutGroup> runningRolloutGroups = rolloutGroupManagement.findRolloutGroupsByRolloutId(
                 createdRollout.getId(), new OffsetBasedPageRequest(0, 10, new Sort(Direction.ASC, "id"))).getContent();