From 0e0b5ed6ffc3790cfb9fca4a1bf31d97f2c41400 Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Fri, 6 Jun 2025 16:13:21 +0300 Subject: [PATCH] Fix dynamic rollouts when there are finished actions from previous rollouts (#2434) Signed-off-by: Avgustin Marinov --- .../hawkbit/repository/TargetManagement.java | 4 +- .../repository/jpa/JpaRolloutExecutor.java | 4 +- .../jpa/management/JpaTargetManagement.java | 156 ++++++++---------- .../specifications/TargetSpecifications.java | 141 ++++++---------- .../TargetManagementSecurityTest.java | 4 +- .../jpa/management/TargetManagementTest.java | 51 +++--- 6 files changed, 151 insertions(+), 209 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TargetManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TargetManagement.java index b02f08bae..07998da27 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TargetManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TargetManagement.java @@ -253,12 +253,12 @@ public interface TargetManagement { * @return a page of the found {@link Target}s */ @PreAuthorize(SpringEvalExpressions.HAS_AUTH_ROLLOUT_MANAGEMENT_READ_AND_TARGET_READ) - Slice findByTargetFilterQueryAndNotInRolloutGroupsAndCompatibleAndUpdatable(@NotNull Pageable pageRequest, + Slice findByTargetFilterQueryAndNotInRolloutAndCompatibleAndUpdatable(@NotNull Pageable pageRequest, @NotEmpty Collection groups, @NotNull String targetFilterQuery, @NotNull DistributionSetType distributionSetType); @PreAuthorize(SpringEvalExpressions.HAS_AUTH_ROLLOUT_MANAGEMENT_READ_AND_TARGET_READ) - Slice findByNotInGEGroupAndNotInActiveActionGEWeightOrInRolloutAndTargetFilterQueryAndCompatibleAndUpdatable( + Slice findByTargetFilterQueryAndNoOverridingActionsAndNotInRolloutAndCompatibleAndUpdatable( @NotNull Pageable pageRequest, final long rolloutId, final int weight, final long firstGroupId, @NotNull String targetFilterQuery, @NotNull DistributionSetType distributionSetType); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutExecutor.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutExecutor.java index eec133d6b..7c97a4d82 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutExecutor.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutExecutor.java @@ -647,7 +647,7 @@ public class JpaRolloutExecutor implements RolloutExecutor { rollout.getRolloutGroups(), RolloutGroupStatus.READY, group); final Slice targets; if (!RolloutHelper.isRolloutRetried(rollout.getTargetFilterQuery())) { - targets = targetManagement.findByTargetFilterQueryAndNotInRolloutGroupsAndCompatibleAndUpdatable( + targets = targetManagement.findByTargetFilterQueryAndNotInRolloutAndCompatibleAndUpdatable( pageRequest, readyGroups, targetFilter, rollout.getDistributionSet().getType()); } else { targets = targetManagement.findByFailedRolloutAndNotInRolloutGroups( @@ -771,7 +771,7 @@ public class JpaRolloutExecutor implements RolloutExecutor { final String targetFilter, final long limit) { return DeploymentHelper.runInNewTransaction(txManager, "createActionsForRolloutDynamicGroup", status -> { final PageRequest pageRequest = PageRequest.of(0, Math.toIntExact(limit)); - final Slice targets = targetManagement.findByNotInGEGroupAndNotInActiveActionGEWeightOrInRolloutAndTargetFilterQueryAndCompatibleAndUpdatable( + final Slice targets = targetManagement.findByTargetFilterQueryAndNoOverridingActionsAndNotInRolloutAndCompatibleAndUpdatable( pageRequest, rollout.getId(), rollout.getWeight().orElse(1000), // Dynamic rollouts shall always have weight! rolloutGroupRepository.findByRolloutOrderByIdAsc(rollout).get(0).getId(), diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetManagement.java index feac7d3d0..f0cc7acca 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetManagement.java @@ -12,7 +12,6 @@ package org.eclipse.hawkbit.repository.jpa.management; import static org.eclipse.hawkbit.repository.jpa.JpaManagementHelper.combineWithAnd; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashMap; @@ -135,7 +134,6 @@ public class JpaTargetManagement implements TargetManagement { @Override public long countByAssignedDistributionSet(final long distributionSetId) { final DistributionSet validDistSet = distributionSetManagement.getOrElseThrowException((distributionSetId)); - return targetRepository.count(TargetSpecifications.hasAssignedDistributionSet(validDistSet.getId())); } @@ -148,29 +146,29 @@ public class JpaTargetManagement implements TargetManagement { @Override public long countByInstalledDistributionSet(final long distributionSetId) { final DistributionSet validDistSet = distributionSetManagement.getOrElseThrowException((distributionSetId)); - return targetRepository.count(TargetSpecifications.hasInstalledDistributionSet(validDistSet.getId())); } @Override public boolean existsByInstalledOrAssignedDistributionSet(final long distributionSetId) { final DistributionSet validDistSet = distributionSetManagement.getOrElseThrowException((distributionSetId)); - - return targetRepository - .exists(TargetSpecifications.hasInstalledOrAssignedDistributionSet(validDistSet.getId())); + return targetRepository.exists(TargetSpecifications.hasInstalledOrAssignedDistributionSet(validDistSet.getId())); } @Override public long countByRsql(final String targetFilterQuery) { - return JpaManagementHelper.countBySpec(targetRepository, List.of(RSQLUtility - .buildRsqlSpecification(targetFilterQuery, TargetFields.class, virtualPropertyReplacer, database))); + return JpaManagementHelper.countBySpec( + targetRepository, + List.of(RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, virtualPropertyReplacer, database))); } @Override public long countByRsqlAndUpdatable(String targetFilterQuery) { - final List> specList = List.of(RSQLUtility.buildRsqlSpecification(targetFilterQuery, - TargetFields.class, virtualPropertyReplacer, database)); - return targetRepository.count(AccessController.Operation.UPDATE, combineWithAnd(specList)); + final List> specList = List.of( + RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, virtualPropertyReplacer, database)); + return targetRepository.count( + AccessController.Operation.UPDATE, + combineWithAnd(specList)); } @Override @@ -197,7 +195,8 @@ public class JpaTargetManagement implements TargetManagement { @Override public long countByTargetFilterQuery(final long targetFilterQueryId) { - final TargetFilterQuery targetFilterQuery = targetFilterQueryRepository.findById(targetFilterQueryId) + final TargetFilterQuery targetFilterQuery = targetFilterQueryRepository + .findById(targetFilterQueryId) .orElseThrow(() -> new EntityNotFoundException(TargetFilterQuery.class, targetFilterQueryId)); return countByRsql(targetFilterQuery.getQuery()); } @@ -221,8 +220,7 @@ public class JpaTargetManagement implements TargetManagement { @Retryable(retryFor = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) public List create(final Collection targets) { - final List targetList = targets.stream().map(JpaTargetCreate.class::cast).map(JpaTargetCreate::build) - .toList(); + final List targetList = targets.stream().map(JpaTargetCreate.class::cast).map(JpaTargetCreate::build).toList(); return Collections.unmodifiableList(targetRepository.saveAll(AccessController.Operation.CREATE, targetList)); } @@ -236,7 +234,6 @@ public class JpaTargetManagement implements TargetManagement { throw new EntityNotFoundException(Target.class, ids, targets.stream().map(Target::getId).filter(id -> !ids.contains(id)).toList()); } - targetRepository.deleteAll(targets); } @@ -255,10 +252,10 @@ public class JpaTargetManagement implements TargetManagement { final Long distSetTypeId = jpaDistributionSet.getType().getId(); return targetRepository - .findAllWithoutCount(AccessController.Operation.UPDATE, + .findAllWithoutCount( + AccessController.Operation.UPDATE, combineWithAnd(List.of( - RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, - virtualPropertyReplacer, database), + RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, virtualPropertyReplacer, database), TargetSpecifications.hasNotDistributionSetInActions(distributionSetId), TargetSpecifications.isCompatibleWithDistributionSetType(distSetTypeId))), pageRequest) @@ -266,8 +263,7 @@ public class JpaTargetManagement implements TargetManagement { } @Override - public long countByRsqlAndNonDSAndCompatibleAndUpdatable(final long distributionSetId, - final String targetFilterQuery) { + public long countByRsqlAndNonDSAndCompatibleAndUpdatable(final long distributionSetId, final String targetFilterQuery) { final DistributionSet jpaDistributionSet = distributionSetManagement.getOrElseThrowException(distributionSetId); final Long distSetTypeId = jpaDistributionSet.getType().getId(); @@ -281,14 +277,12 @@ public class JpaTargetManagement implements TargetManagement { } @Override - public Slice findByTargetFilterQueryAndNotInRolloutGroupsAndCompatibleAndUpdatable( - final Pageable pageRequest, final Collection groups, final String targetFilterQuery, - final DistributionSetType dsType) { + public Slice findByTargetFilterQueryAndNotInRolloutAndCompatibleAndUpdatable( + final Pageable pageRequest, final Collection groups, final String targetFilterQuery, final DistributionSetType dsType) { return targetRepository .findAllWithoutCount(AccessController.Operation.UPDATE, combineWithAnd(List.of( - RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, - virtualPropertyReplacer, database), + RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, virtualPropertyReplacer, database), TargetSpecifications.isNotInRolloutGroups(groups), TargetSpecifications.isCompatibleWithDistributionSetType(dsType.getId()))), pageRequest) @@ -296,16 +290,14 @@ public class JpaTargetManagement implements TargetManagement { } @Override - public Slice findByNotInGEGroupAndNotInActiveActionGEWeightOrInRolloutAndTargetFilterQueryAndCompatibleAndUpdatable( + public Slice findByTargetFilterQueryAndNoOverridingActionsAndNotInRolloutAndCompatibleAndUpdatable( final Pageable pageRequest, final long rolloutId, final int weight, final long firstGroupId, final String targetFilterQuery, final DistributionSetType distributionSetType) { return targetRepository .findAllWithoutCount(AccessController.Operation.UPDATE, combineWithAnd(List.of( - RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, - virtualPropertyReplacer, database), - TargetSpecifications.isNotInGERolloutGroup(firstGroupId), - TargetSpecifications.hasNoActiveActionWithGEWeightOrInRollout(weight, rolloutId), + RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, virtualPropertyReplacer, database), + TargetSpecifications.hasNoOverridingActionsAndNotInRollout(weight, rolloutId), TargetSpecifications.isCompatibleWithDistributionSetType(distributionSetType.getId()))), pageRequest) .map(Target.class::cast); @@ -319,7 +311,7 @@ public class JpaTargetManagement implements TargetManagement { @Override public Slice findByFailedRolloutAndNotInRolloutGroups(Pageable pageRequest, Collection groups, String rolloutId) { - final List> specList = Arrays.asList( + final List> specList = List.of( TargetSpecifications.failedActionsForRollout(rolloutId), TargetSpecifications.isNotInRolloutGroups(groups)); @@ -327,22 +319,20 @@ public class JpaTargetManagement implements TargetManagement { } @Override - public long countByRsqlAndNotInRolloutGroupsAndCompatibleAndUpdatable(final String targetFilterQuery, final Collection groups, - final DistributionSetType dsType) { + public long countByRsqlAndNotInRolloutGroupsAndCompatibleAndUpdatable( + final String targetFilterQuery, final Collection groups, final DistributionSetType dsType) { return targetRepository.count(AccessController.Operation.UPDATE, combineWithAnd(List.of( - RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, - virtualPropertyReplacer, database), + RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, virtualPropertyReplacer, database), TargetSpecifications.isNotInRolloutGroups(groups), TargetSpecifications.isCompatibleWithDistributionSetType(dsType.getId())))); } @Override public long countByFailedRolloutAndNotInRolloutGroups(String rolloutId, Collection groups) { - final List> specList = Arrays.asList( + final List> specList = List.of( TargetSpecifications.failedActionsForRollout(rolloutId), TargetSpecifications.isNotInRolloutGroups(groups)); - return JpaManagementHelper.countBySpec(targetRepository, specList); } @@ -352,22 +342,21 @@ public class JpaTargetManagement implements TargetManagement { throw new EntityNotFoundException(RolloutGroup.class, group); } - return JpaManagementHelper.findAllWithoutCountBySpec(targetRepository, pageRequest, - List.of(TargetSpecifications.hasNoActionInRolloutGroup(group))); + return JpaManagementHelper.findAllWithoutCountBySpec( + targetRepository, pageRequest, List.of(TargetSpecifications.hasNoActionInRolloutGroup(group))); } @Override public Page findByAssignedDistributionSet(final Pageable pageReq, final long distributionSetId) { final DistributionSet validDistSet = distributionSetManagement.getOrElseThrowException(distributionSetId); - return JpaManagementHelper.findAllWithCountBySpec(targetRepository, - List.of(TargetSpecifications.hasAssignedDistributionSet(validDistSet.getId())), pageReq - ); + return JpaManagementHelper.findAllWithCountBySpec( + targetRepository, + List.of(TargetSpecifications.hasAssignedDistributionSet(validDistSet.getId())), pageReq); } @Override - public Page findByAssignedDistributionSetAndRsql(final Pageable pageReq, final long distributionSetId, - final String rsqlParam) { + public Page findByAssignedDistributionSetAndRsql(final Pageable pageReq, final long distributionSetId, final String rsqlParam) { final DistributionSet validDistSet = distributionSetManagement.getOrElseThrowException(distributionSetId); final List> specList = List.of( @@ -379,8 +368,7 @@ public class JpaTargetManagement implements TargetManagement { @Override public List getByControllerID(final Collection controllerIDs) { - return Collections.unmodifiableList( - targetRepository.findAll(TargetSpecifications.byControllerIdWithAssignedDsInJoin(controllerIDs))); + return Collections.unmodifiableList(targetRepository.findAll(TargetSpecifications.byControllerIdWithAssignedDsInJoin(controllerIDs))); } @Override @@ -403,14 +391,12 @@ public class JpaTargetManagement implements TargetManagement { public Page findByInstalledDistributionSet(final Pageable pageReq, final long distributionSetId) { final DistributionSet validDistSet = distributionSetManagement.getOrElseThrowException(distributionSetId); - return JpaManagementHelper.findAllWithCountBySpec(targetRepository, - List.of(TargetSpecifications.hasInstalledDistributionSet(validDistSet.getId())), pageReq - ); + return JpaManagementHelper.findAllWithCountBySpec( + targetRepository, List.of(TargetSpecifications.hasInstalledDistributionSet(validDistSet.getId())), pageReq); } @Override - public Page findByInstalledDistributionSetAndRsql(final Pageable pageable, final long distributionSetId, - final String rsqlParam) { + public Page findByInstalledDistributionSetAndRsql(final Pageable pageable, final long distributionSetId, final String rsqlParam) { final DistributionSet validDistSet = distributionSetManagement.getOrElseThrowException(distributionSetId); final List> specList = List.of( @@ -422,9 +408,8 @@ public class JpaTargetManagement implements TargetManagement { @Override public Page findByUpdateStatus(final Pageable pageable, final TargetUpdateStatus status) { - return JpaManagementHelper.findAllWithCountBySpec(targetRepository, List.of(TargetSpecifications.hasTargetUpdateStatus(status)), - pageable - ); + return JpaManagementHelper.findAllWithCountBySpec( + targetRepository, List.of(TargetSpecifications.hasTargetUpdateStatus(status)), pageable); } @Override @@ -434,8 +419,9 @@ public class JpaTargetManagement implements TargetManagement { @Override public Slice findByRsql(final Pageable pageable, final String targetFilterQuery) { - return JpaManagementHelper.findAllWithoutCountBySpec(targetRepository, pageable, List.of(RSQLUtility - .buildRsqlSpecification(targetFilterQuery, TargetFields.class, virtualPropertyReplacer, database))); + return JpaManagementHelper.findAllWithoutCountBySpec( + targetRepository, pageable, + List.of(RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, virtualPropertyReplacer, database))); } @Override @@ -443,24 +429,23 @@ public class JpaTargetManagement implements TargetManagement { final TargetFilterQuery targetFilterQuery = targetFilterQueryRepository.findById(targetFilterQueryId) .orElseThrow(() -> new EntityNotFoundException(TargetFilterQuery.class, targetFilterQueryId)); - return JpaManagementHelper.findAllWithoutCountBySpec(targetRepository, pageable, - List.of(RSQLUtility.buildRsqlSpecification(targetFilterQuery.getQuery(), TargetFields.class, - virtualPropertyReplacer, database))); + return JpaManagementHelper.findAllWithoutCountBySpec( + targetRepository, pageable, + List.of(RSQLUtility.buildRsqlSpecification( + targetFilterQuery.getQuery(), TargetFields.class, virtualPropertyReplacer, database))); } @Override public Page findByTag(final Pageable pageable, final long tagId) { throwEntityNotFoundExceptionIfTagDoesNotExist(tagId); - - return JpaManagementHelper.findAllWithCountBySpec(targetRepository, List.of(TargetSpecifications.hasTag(tagId)), pageable - ); + return JpaManagementHelper.findAllWithCountBySpec(targetRepository, List.of(TargetSpecifications.hasTag(tagId)), pageable); } @Override public Page findByRsqlAndTag(final Pageable pageable, final String rsqlParam, final long tagId) { throwEntityNotFoundExceptionIfTagDoesNotExist(tagId); - final List> specList = Arrays.asList( + final List> specList = List.of( RSQLUtility.buildRsqlSpecification(rsqlParam, TargetFields.class, virtualPropertyReplacer, database), TargetSpecifications.hasTag(tagId)); @@ -472,20 +457,19 @@ public class JpaTargetManagement implements TargetManagement { @Retryable(retryFor = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) public TargetTypeAssignmentResult assignType(final Collection controllerIds, final Long typeId) { - final JpaTargetType type = targetTypeRepository.findById(typeId) + final JpaTargetType type = targetTypeRepository + .findById(typeId) .orElseThrow(() -> new EntityNotFoundException(TargetType.class, typeId)); - final List targetsWithSameType = findTargetsByInSpecification(controllerIds, - TargetSpecifications.hasTargetType(typeId)); - - final List targetsWithoutSameType = findTargetsByInSpecification(controllerIds, - TargetSpecifications.hasTargetTypeNot(typeId)); + final List targetsWithSameType = findTargetsByInSpecification(controllerIds, TargetSpecifications.hasTargetType(typeId)); + final List targetsWithoutSameType = + findTargetsByInSpecification(controllerIds, TargetSpecifications.hasTargetTypeNot(typeId)); // set new target type to all targets without that type targetsWithoutSameType.forEach(target -> target.setTargetType(type)); - final TargetTypeAssignmentResult result = new TargetTypeAssignmentResult(targetsWithSameType.size(), - targetRepository.saveAll(targetsWithoutSameType), Collections.emptyList(), type); + final TargetTypeAssignmentResult result = new TargetTypeAssignmentResult( + targetsWithSameType.size(), targetRepository.saveAll(targetsWithoutSameType), Collections.emptyList(), type); // no reason to persist the type entityManager.detach(type); @@ -500,8 +484,7 @@ public class JpaTargetManagement implements TargetManagement { final List allTargets = findTargetsByInSpecification(controllerIds, null); if (allTargets.size() < controllerIds.size()) { - throw new EntityNotFoundException(Target.class, controllerIds, - allTargets.stream().map(Target::getControllerId).toList()); + throw new EntityNotFoundException(Target.class, controllerIds, allTargets.stream().map(Target::getControllerId).toList()); } // set new target type to null for all targets @@ -514,8 +497,8 @@ public class JpaTargetManagement implements TargetManagement { @Transactional @Retryable(retryFor = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) - public List assignTag(final Collection controllerIds, final long targetTagId, - final Consumer> notFoundHandler) { + public List assignTag( + final Collection controllerIds, final long targetTagId, final Consumer> notFoundHandler) { return assignTag0(controllerIds, targetTagId, notFoundHandler); } @@ -527,8 +510,8 @@ public class JpaTargetManagement implements TargetManagement { return assignTag0(controllerIds, targetTagId, null); } - private List assignTag0(final Collection controllerIds, final long targetTagId, - final Consumer> notFoundHandler) { + private List assignTag0( + final Collection controllerIds, final long targetTagId, final Consumer> notFoundHandler) { return updateTag(controllerIds, targetTagId, notFoundHandler, (tag, target) -> { if (target.getTags().contains(tag)) { return target; @@ -543,8 +526,8 @@ public class JpaTargetManagement implements TargetManagement { @Transactional @Retryable(retryFor = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) - public List unassignTag(final Collection controllerIds, final long targetTagId, - final Consumer> notFoundHandler) { + public List unassignTag( + final Collection controllerIds, final long targetTagId, final Consumer> notFoundHandler) { return unassignTag0(controllerIds, targetTagId, notFoundHandler); } @@ -556,8 +539,8 @@ public class JpaTargetManagement implements TargetManagement { return unassignTag0(controllerIds, targetTagId, null); } - private List unassignTag0(final Collection controllerIds, final long targetTagId, - final Consumer> notFoundHandler) { + private List unassignTag0( + final Collection controllerIds, final long targetTagId, final Consumer> notFoundHandler) { return updateTag(controllerIds, targetTagId, notFoundHandler, (tag, target) -> { if (target.getTags().contains(tag)) { target.removeTag(tag); @@ -630,13 +613,13 @@ public class JpaTargetManagement implements TargetManagement { } @Override - public boolean isTargetMatchingQueryAndDSNotAssignedAndCompatibleAndUpdatable(final String controllerId, - final long distributionSetId, final String targetFilterQuery) { + public boolean isTargetMatchingQueryAndDSNotAssignedAndCompatibleAndUpdatable( + final String controllerId, final long distributionSetId, final String targetFilterQuery) { RSQLUtility.validateRsqlFor(targetFilterQuery, TargetFields.class, JpaTarget.class, virtualPropertyReplacer, entityManager); final DistributionSet ds = distributionSetManagement.get(distributionSetId) .orElseThrow(() -> new EntityNotFoundException(DistributionSet.class, distributionSetId)); final Long distSetTypeId = ds.getType().getId(); - final List> specList = Arrays.asList( + final List> specList = List.of( RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, virtualPropertyReplacer, database), TargetSpecifications.hasNotDistributionSetInActions(distributionSetId), TargetSpecifications.isCompatibleWithDistributionSetType(distSetTypeId), @@ -830,8 +813,7 @@ public class JpaTargetManagement implements TargetManagement { return specList; } - private List findTargetsByInSpecification(final Collection controllerIds, - final Specification specification) { + private List findTargetsByInSpecification(final Collection controllerIds, final Specification specification) { return ListUtils.partition(new ArrayList<>(controllerIds), Constants.MAX_ENTRIES_IN_STATEMENT).stream() .map(ids -> targetRepository.findAll(TargetSpecifications.hasControllerIdIn(ids).and(specification))) .flatMap(List::stream).toList(); @@ -873,4 +855,4 @@ public class JpaTargetManagement implements TargetManagement { throw new EntityNotFoundException(TargetTag.class, tagId); } } -} +} \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/TargetSpecifications.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/TargetSpecifications.java index 00d9db2e5..7fc5bfdb7 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/TargetSpecifications.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/TargetSpecifications.java @@ -119,15 +119,13 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link JpaTarget}s including - * {@link JpaTarget#getAssignedDistributionSet()}. + * {@link Specification} for retrieving {@link JpaTarget}s including {@link JpaTarget#getAssignedDistributionSet()}. * * @param controllerIDs to search for * @return the {@link Target} {@link Specification} */ public static Specification byControllerIdWithAssignedDsInJoin(final Collection controllerIDs) { return (targetRoot, query, cb) -> { - final Predicate predicate = targetRoot.get(JpaTarget_.controllerId).in(controllerIDs); targetRoot.fetch(JpaTarget_.assignedDistributionSet, JoinType.LEFT); return predicate; @@ -135,8 +133,7 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s by "equal to any - * given {@link TargetUpdateStatus}". + * {@link Specification} for retrieving {@link Target}s by "equal to any given {@link TargetUpdateStatus}". * * @param updateStatus to be filtered on * @return the {@link Target} {@link Specification} @@ -146,8 +143,7 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s by "equal to given - * {@link TargetUpdateStatus}". + * {@link Specification} for retrieving {@link Target}s by "equal to given {@link TargetUpdateStatus}". * * @param updateStatus to be filtered on * @return the {@link Target} {@link Specification} @@ -157,8 +153,7 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s by "not equal to - * given {@link TargetUpdateStatus}". + * {@link Specification} for retrieving {@link Target}s by "not equal to given {@link TargetUpdateStatus}". * * @param updateStatus to be filtered on * @return the {@link Target} {@link Specification} @@ -168,17 +163,13 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s that are overdue. A - * target is overdue if it did not respond during the configured + * {@link Specification} for retrieving {@link Target}s that are overdue. A target is overdue if it did not respond during the configured * intervals:
* poll_itvl + overdue_itvl * - * @param overdueTimestamp the calculated timestamp to compare with the last respond of a - * target (lastTargetQuery).
- * The overdueTimestamp has to be calculated with - * the following expression:
- * overdueTimestamp = nowTimestamp - poll_itvl - - * overdue_itvl + * @param overdueTimestamp the calculated timestamp to compare with the last response of a target (lastTargetQuery).
+ * The overdueTimestamp has to be calculated with the following expression:
+ * overdueTimestamp = nowTimestamp - poll_itvl - overdue_itvl * @return the {@link Target} {@link Specification} */ public static Specification isOverdue(final long overdueTimestamp) { @@ -187,8 +178,7 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s by "like - * controllerId or like name". + * {@link Specification} for retrieving {@link Target}s by "like controllerId or like name". * * @param searchText to be filtered on * @return the {@link Target} {@link Specification} @@ -202,8 +192,7 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s by "like - * controllerId". + * {@link Specification} for retrieving {@link Target}s by "like controllerId". * * @param distributionId to be filtered on * @return the {@link Target} {@link Specification} @@ -228,8 +217,7 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s based on a - * {@link TargetTag} name. + * {@link Specification} for retrieving {@link Target}s based on a {@link TargetTag} name. * * @param tagName to search for * @return the {@link Target} {@link Specification} @@ -242,8 +230,7 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s by "has no tag - * names"or "has at least on of the given tag names". + * {@link Specification} for retrieving {@link Target}s by "has no tag names"or "has at least on of the given tag names". * * @param tagNames to be filtered on * @param selectTargetWithNoTag flag to get targets with no tag assigned @@ -258,8 +245,7 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s by assigned - * distribution set. + * {@link Specification} for retrieving {@link Target}s by assigned distribution set. * * @param distributionSetId the ID of the distribution set which must be assigned * @return the {@link Target} {@link Specification} @@ -270,8 +256,7 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s that don't have the - * given distribution set in their action history + * {@link Specification} for retrieving {@link Target}s that don't have the given distribution set in their action history * * @param distributionSetId the ID of the distribution set which must not be assigned * @return the {@link Target} {@link Specification} @@ -285,10 +270,9 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s that are compatible - * with given {@link DistributionSetType}. Compatibility is evaluated by - * checking the {@link TargetType} of a target. Targets that don't have a - * {@link TargetType} are compatible with all {@link DistributionSetType} + * {@link Specification} for retrieving {@link Target}s that are compatible with given {@link DistributionSetType}. Compatibility is + * evaluated by checking the {@link TargetType} of a target. Targets that don't have a {@link TargetType} are compatible with all + * {@link DistributionSetType} * * @param distributionSetTypeId the ID of the distribution set type which must be compatible * @return the {@link Target} {@link Specification} @@ -302,10 +286,8 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s that are NOT - * compatible with given {@link DistributionSetType}. Compatibility is - * evaluated by checking the {@link TargetType} of a target. Targets that - * don't have a {@link TargetType} are compatible with all + * {@link Specification} for retrieving {@link Target}s that are NOT compatible with given {@link DistributionSetType}. Compatibility is + * evaluated by checking the {@link TargetType} of a target. Targets that don't have a {@link TargetType} are compatible with all * {@link DistributionSetType} * * @param distributionSetTypeId the ID of the distribution set type which must be incompatible @@ -329,23 +311,20 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s that are in a given - * {@link RolloutGroup} + * {@link Specification} for retrieving {@link Target}s that are in a given {@link RolloutGroup} * * @param group the {@link RolloutGroup} * @return the {@link Target} {@link Specification} */ public static Specification isInRolloutGroup(final Long group) { return (targetRoot, query, cb) -> { - final ListJoin targetGroupJoin = targetRoot - .join(JpaTarget_.rolloutTargetGroup); + final ListJoin targetGroupJoin = targetRoot.join(JpaTarget_.rolloutTargetGroup); return cb.equal(targetGroupJoin.get(RolloutTargetGroup_.rolloutGroup).get(AbstractJpaBaseEntity_.id), group); }; } /** - * {@link Specification} for retrieving {@link Target}s that are in an - * action for a given {@link RolloutGroup} + * {@link Specification} for retrieving {@link Target}s that are in an action for a given {@link RolloutGroup} * * @param group the {@link RolloutGroup} * @return the {@link Target} {@link Specification} @@ -358,41 +337,21 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s that are not in the - * given {@link RolloutGroup}s + * {@link Specification} for retrieving {@link Target}s that are not in the given {@link RolloutGroup}s * * @param groups the {@link RolloutGroup}s * @return the {@link Target} {@link Specification} */ public static Specification isNotInRolloutGroups(final Collection groups) { return (targetRoot, query, cb) -> { - final ListJoin rolloutTargetJoin = targetRoot - .join(JpaTarget_.rolloutTargetGroup, JoinType.LEFT); - rolloutTargetJoin.on(rolloutTargetJoin.get(RolloutTargetGroup_.rolloutGroup) - .get(AbstractJpaBaseEntity_.id).in(groups)); + final ListJoin rolloutTargetJoin = targetRoot.join(JpaTarget_.rolloutTargetGroup, JoinType.LEFT); + rolloutTargetJoin.on(rolloutTargetJoin.get(RolloutTargetGroup_.rolloutGroup).get(AbstractJpaBaseEntity_.id).in(groups)); return cb.isNull(rolloutTargetJoin.get(RolloutTargetGroup_.target)); }; } /** - * {@link Specification} for retrieving {@link Target}s that are not in - * any {@link RolloutGroup}s - * - * @return the {@link Target} {@link Specification} - */ - public static Specification isNotInGERolloutGroup(final long groupId) { - return (targetRoot, query, cb) -> { - final ListJoin rolloutTargetJoin = targetRoot - .join(JpaTarget_.rolloutTargetGroup, JoinType.LEFT); - rolloutTargetJoin.on(cb.ge(rolloutTargetJoin.get(RolloutTargetGroup_.rolloutGroup) - .get(AbstractJpaBaseEntity_.id), groupId)); - return cb.isNull(rolloutTargetJoin.get(RolloutTargetGroup_.target)); - }; - } - - /** - * {@link Specification} for retrieving {@link Target}s that have no Action - * of the {@link RolloutGroup}. + * {@link Specification} for retrieving {@link Target}s that have no Action of the {@link RolloutGroup}. * * @param group the {@link RolloutGroup} * @return the {@link Target} {@link Specification} @@ -412,8 +371,7 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s by assigned - * distribution set. + * {@link Specification} for retrieving {@link Target}s by assigned distribution set. * * @param distributionSetId the ID of the distribution set which must be assigned * @return the {@link Target} {@link Specification} @@ -448,8 +406,7 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s by target type id is - * equal to null + * {@link Specification} for retrieving {@link Target}s by target type id is equal to null * * @return the {@link Target} {@link Specification} */ @@ -458,8 +415,7 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s that don't have - * target type assigned + * {@link Specification} for retrieving {@link Target}s that don't have target type assigned * * @param typeId the id of the target type * @return the {@link Target} {@link Specification} @@ -471,9 +427,7 @@ public final class TargetSpecifications { public static Specification failedActionsForRollout(final String rolloutId) { return (targetRoot, query, cb) -> { - Join targetActions = - targetRoot.join("actions"); - + final Join targetActions = targetRoot.join("actions"); return cb.and( cb.equal(targetActions.get("rollout").get("id"), rolloutId), cb.equal(targetActions.get("status"), Action.Status.ERROR)); @@ -481,34 +435,31 @@ public final class TargetSpecifications { } /** - * {@link Specification} for retrieving {@link Target}s that have no active (non-finished) action - * with great or equal weight (GEWeight. + * {@link Specification} for retrieving {@link Target}s that have: + *
    + *
  • no active (non-finished) actions with greater weight in the older rollouts
    • + *
  • or have actions in the current rollout
    • + *
  • or have actions with great or equal weight in the newer rollouts
    • + *
* * @param weight the referent weight * @return the {@link Target} {@link Specification} */ - public static Specification hasNoActiveActionWithGEWeightOrInRollout(final int weight, final long rolloutId) { + public static Specification hasNoOverridingActionsAndNotInRollout(final int weight, final long rolloutId) { return (targetRoot, query, cb) -> { final ListJoin actionsJoin = targetRoot.join(JpaTarget_.actions, JoinType.LEFT); actionsJoin.on( cb.or( - cb.gt(actionsJoin.get(JpaAction_.weight), weight), cb.and( - cb.equal(actionsJoin.get(JpaAction_.weight), weight), - cb.ge(actionsJoin.get(JpaAction_.ROLLOUT).get(AbstractJpaBaseEntity_.ID), rolloutId)))); - // another, but probably heavier variant -// actionsJoin.on( -// cb.or( -// // in rollout -// cb.equal(actionsJoin.get(JpaAction_.ROLLOUT).get(AbstractJpaBaseEntity_.ID), rolloutId), -// // or, in newer rollout with greater or equal weight -// cb.and( -// cb.gt(actionsJoin.get(JpaAction_.ROLLOUT).get(AbstractJpaBaseEntity_.ID), rolloutId), -// cb.ge(actionsJoin.get(JpaAction_.weight), weight)), -// // or, in older with greater status -// cb.and( -// cb.lt(actionsJoin.get(JpaAction_.ROLLOUT).get(AbstractJpaBaseEntity_.ID), rolloutId), -// cb.gt(actionsJoin.get(JpaAction_.weight), weight)))); + cb.and( + cb.lt(actionsJoin.get(JpaAction_.rollout).get(AbstractJpaBaseEntity_.id), rolloutId), + cb.gt(actionsJoin.get(JpaAction_.weight), weight)), + cb.or( + cb.equal(actionsJoin.get(JpaAction_.active), true), + cb.equal(actionsJoin.get(JpaAction_.status), Action.Status.SCHEDULED))), + cb.and( + cb.ge(actionsJoin.get(JpaAction_.rollout).get(AbstractJpaBaseEntity_.id), rolloutId), + cb.ge(actionsJoin.get(JpaAction_.weight), weight)))); return cb.isNull(actionsJoin.get(AbstractJpaBaseEntity_.id)); }; } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementSecurityTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementSecurityTest.java index 3f9c5ef89..88535be37 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementSecurityTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementSecurityTest.java @@ -147,9 +147,9 @@ class TargetManagementSecurityTest extends AbstractJpaIntegrationTest { @Test @Description("Tests ManagementAPI PreAuthorized method with correct and insufficient permissions.") - void findByTargetFilterQueryAndNotInRolloutGroupsAndCompatibleAndUpdatablePermissionsCheck() { + void findByTargetFilterQueryAndNotInRolloutAndCompatibleAndUpdatablePermissionsCheck() { assertPermissions( - () -> targetManagement.findByTargetFilterQueryAndNotInRolloutGroupsAndCompatibleAndUpdatable(PAGE, List.of(1L), + () -> targetManagement.findByTargetFilterQueryAndNotInRolloutAndCompatibleAndUpdatable(PAGE, List.of(1L), "controllerId==id", entityFactory.distributionSetType().create().build()), List.of(SpPermission.READ_TARGET, SpPermission.READ_ROLLOUT)); } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java index 27163dde0..dead17069 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java @@ -789,8 +789,6 @@ class TargetManagementTest extends AbstractJpaIntegrationTest { .isThrownBy(() -> targetManagement.createMetadata(target3ControllerId, metaData3)); } - - @Test @Description("Queries and loads the metadata related to a given target.") void getMetadata() { @@ -1114,8 +1112,8 @@ class TargetManagementTest extends AbstractJpaIntegrationTest { void matchesFilterTargetNotExists() { final DistributionSet ds = testdataFactory.createDistributionSet(); - assertThat(targetManagement.isTargetMatchingQueryAndDSNotAssignedAndCompatibleAndUpdatable("notExisting", ds.getId(), - "name==*")).isFalse(); + assertThat(targetManagement.isTargetMatchingQueryAndDSNotAssignedAndCompatibleAndUpdatable( + "notExisting", ds.getId(),"name==*")).isFalse(); } /** @@ -1123,43 +1121,53 @@ class TargetManagementTest extends AbstractJpaIntegrationTest { */ @Test @Description("Target matches filter no active action with ge weight.") - void findByNotInGEGroupAndNotInActiveActionGEWeightOrInRolloutAndTargetFilterQueryAndCompatibleAndUpdatable() { + void findByTargetFilterQueryAndNoOverridingActionsAndNotInRolloutAndCompatibleAndUpdatable() { final String targetPrefix = "dyn_action_filter_"; final DistributionSet distributionSet = testdataFactory.createDistributionSet(); - final List targets = testdataFactory.createTargets(targetPrefix, 9); + final List targets = testdataFactory.createTargets(targetPrefix, 10); final Rollout rolloutOlder = testdataFactory.createRollout(); final Rollout rollout = testdataFactory.createRollout(); final Rollout rolloutNewer = testdataFactory.createRollout(); + int target = 0; + final List expected = new ArrayList<>(); // old ro with less weight - match - createAction(targets.get(0), rolloutOlder, 0, Status.RUNNING, distributionSet); - // old ro with less weight - match - createAction(targets.get(1), rolloutOlder, 5, Status.SCHEDULED, distributionSet); + expected.add(target); + createAction(targets.get(target++), rolloutOlder, 0, Status.RUNNING, distributionSet); // old ro with equal weight - match - createAction(targets.get(2), rolloutOlder, 10, Status.RUNNING, distributionSet); - // old ro with BIGGER weight - doesn't match - createAction(targets.get(3), rolloutOlder, 20, Status.WAIT_FOR_CONFIRMATION, distributionSet); + expected.add(target); + createAction(targets.get(target++), rolloutOlder, 10, Status.RUNNING, distributionSet); + // old ro with bigger weight, scheduled - doesn't match + createAction(targets.get(target++), rolloutOlder, 11, Status.SCHEDULED, distributionSet); + // old ro with bigger weight, running - doesn't match + createAction(targets.get(target++), rolloutOlder, 11, Status.RUNNING, distributionSet); + // old ro with bigger weight, running match + expected.add(target); + createAction(targets.get(target++), rolloutOlder, 11, Status.FINISHED, distributionSet); // same ro - doesn't match - createAction(targets.get(4), rollout, 10, Status.RUNNING, distributionSet); + createAction(targets.get(target++), rollout, 10, Status.RUNNING, distributionSet); // new ro with less weight - match - createAction(targets.get(5), rolloutNewer, 0, Status.RUNNING, distributionSet); + expected.add(target); + createAction(targets.get(target++), rolloutNewer, 0, Status.RUNNING, distributionSet); // new ro with less weight - match - createAction(targets.get(6), rolloutNewer, 5, Status.WARNING, distributionSet); + expected.add(target); + createAction(targets.get(target++), rolloutNewer, 5, Status.WARNING, distributionSet); // NEW ro with EQUAL weight - doesn't match - createAction(targets.get(7), rolloutNewer, 10, Status.RUNNING, distributionSet); + createAction(targets.get(target++), rolloutNewer, 10, Status.RUNNING, distributionSet); // new ro with BIGGER weight - doesn't match - createAction(targets.get(8), rolloutNewer, 20, Status.DOWNLOADED, distributionSet); + createAction(targets.get(target), rolloutNewer, 20, Status.DOWNLOADED, distributionSet); - final Slice matching = targetManagement.findByNotInGEGroupAndNotInActiveActionGEWeightOrInRolloutAndTargetFilterQueryAndCompatibleAndUpdatable( - PAGE, rollout.getId(), 10, Long.MAX_VALUE, "controllerid==dyn_action_filter_*", distributionSet.getType()); + final Slice matching = + targetManagement.findByTargetFilterQueryAndNoOverridingActionsAndNotInRolloutAndCompatibleAndUpdatable( + PAGE, rollout.getId(), 10, Long.MAX_VALUE, "controllerid==dyn_action_filter_*", distributionSet.getType()); - assertThat(matching.getNumberOfElements()).isEqualTo(5); + assertThat(matching.getNumberOfElements()).isEqualTo(expected.size()); assertThat(matching.stream() .map(Target::getControllerId) .map(s -> s.substring(targetPrefix.length())) .map(Integer::parseInt) .sorted() - .toList()).isEqualTo(List.of(0, 1, 2, 5, 6)); + .toList()).isEqualTo(expected); } @Test @@ -1435,6 +1443,7 @@ class TargetManagementTest extends AbstractJpaIntegrationTest { action.setWeight(weight); } action.setStatus(status); + action.setActive(status != Status.FINISHED && status != Status.ERROR && status != Status.CANCELED); action.setDistributionSet(distributionSet); actionRepository.save(action); }