From d8c8e80125056b9fdbf1cb8ff6398f142624b181 Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Wed, 11 Dec 2024 13:44:50 +0200 Subject: [PATCH] Remove unused DistributionSetManagement#findByDistributionSetFilterOrderByLinkedTarget (#2141) Signed-off-by: Avgustin Marinov --- .../repository/DistributionSetManagement.java | 22 --- .../JpaDistributionSetManagement.java | 26 +--- .../repository/DistributionSetRepository.java | 15 +- .../DistributionSetSpecification.java | 133 +++++------------- .../AbstractAccessControllerTest.java | 35 +++-- .../DistributionSetAccessControllerTest.java | 72 +++++----- .../TargetAccessControllerTest.java | 64 +++++---- .../controller/TestAccessControlManger.java | 10 +- .../DistributionSetManagementTest.java | 86 ----------- 9 files changed, 138 insertions(+), 325 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DistributionSetManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DistributionSetManagement.java index 7dc7b56cd..18bf411d2 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DistributionSetManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DistributionSetManagement.java @@ -279,28 +279,6 @@ public interface DistributionSetManagement Slice findByDistributionSetFilter(@NotNull Pageable pageable, @NotNull DistributionSetFilter distributionSetFilter); - /** - * Method retrieves all {@link DistributionSet}s from the repository in the - * following order: - *

- * 1) {@link DistributionSet}s which have the given {@link Target} as - * {@link TargetInfo#getInstalledDistributionSet()} - *

- * 2) {@link DistributionSet}s which have the given {@link Target} as - * {@link Target#getAssignedDistributionSet()} - *

- * 3) {@link DistributionSet}s which have no connection to the given - * {@link Target} ordered by ID of the DistributionSet. - * - * @param pageable the page request to page the result set * - * @param distributionSetFilter has details of filters to be applied. - * @param assignedOrInstalled the id of the Target to be ordered by - * @return {@link DistributionSet}s - */ - @PreAuthorize(SpringEvalExpressions.HAS_AUTH_READ_REPOSITORY) - Slice findByDistributionSetFilterOrderByLinkedTarget(@NotNull Pageable pageable, - @NotNull DistributionSetFilter distributionSetFilter, @NotEmpty String assignedOrInstalled); - /** * Counts all {@link DistributionSet}s in repository based on given filter. * diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetManagement.java index a2e6dec7d..66e1122d9 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetManagement.java @@ -32,7 +32,6 @@ import org.eclipse.hawkbit.repository.DistributionSetManagement; import org.eclipse.hawkbit.repository.DistributionSetMetadataFields; import org.eclipse.hawkbit.repository.DistributionSetTagManagement; import org.eclipse.hawkbit.repository.DistributionSetTypeManagement; -import org.eclipse.hawkbit.repository.OffsetBasedPageRequest; import org.eclipse.hawkbit.repository.QuotaManagement; import org.eclipse.hawkbit.repository.RepositoryProperties; import org.eclipse.hawkbit.repository.SystemManagement; @@ -83,7 +82,6 @@ import org.springframework.dao.ConcurrencyFailureException; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; -import org.springframework.data.domain.Sort; import org.springframework.data.jpa.domain.Specification; import org.springframework.orm.jpa.vendor.Database; import org.springframework.retry.annotation.Backoff; @@ -409,7 +407,7 @@ public class JpaDistributionSetManagement implements DistributionSetManagement { throw new InsufficientPermissionException("Target not accessible (or not found)!"); } return distributionSetRepository - .findOne(DistributionSetSpecification.byId(action.getDistributionSet().getId())) + .findOne(DistributionSetSpecification.byIdFetch(action.getDistributionSet().getId())) .orElseThrow(() -> new InsufficientPermissionException("DistributionSet not accessible (or not found)!")); }) @@ -518,25 +516,9 @@ public class JpaDistributionSetManagement implements DistributionSetManagement { return JpaManagementHelper.findAllWithoutCountBySpec(distributionSetRepository, pageable, specList); } - @Override - public Slice findByDistributionSetFilterOrderByLinkedTarget(final Pageable pageable, - final DistributionSetFilter distributionSetFilter, final String assignedOrInstalled) { - // remove default sort from pageable to not overwrite sorted spec - final OffsetBasedPageRequest unsortedPage = new OffsetBasedPageRequest(pageable.getOffset(), - pageable.getPageSize(), Sort.unsorted()); - - final List> specList = buildDistributionSetSpecifications( - distributionSetFilter); - specList.add(DistributionSetSpecification.orderedByLinkedTarget(assignedOrInstalled, pageable.getSort())); - - return JpaManagementHelper.findAllWithoutCountBySpec(distributionSetRepository, unsortedPage, specList); - } - @Override public long countByDistributionSetFilter(@NotNull final DistributionSetFilter distributionSetFilter) { - final List> specList = buildDistributionSetSpecifications( - distributionSetFilter); - + final List> specList = buildDistributionSetSpecifications(distributionSetFilter); return JpaManagementHelper.countBySpec(distributionSetRepository, specList); } @@ -791,7 +773,7 @@ public class JpaDistributionSetManagement implements DistributionSetManagement { distributionSetRepository.findById(dsIds.iterator().next()) .map(List::of) .orElseGet(Collections::emptyList) : - distributionSetRepository.findAll(DistributionSetSpecification.byIds(dsIds)); + distributionSetRepository.findAll(DistributionSetSpecification.byIdsFetch(dsIds)); if (allDs.size() < dsIds.size()) { throw new EntityNotFoundException(DistributionSet.class, notFound(dsIds, allDs)); } @@ -898,7 +880,7 @@ public class JpaDistributionSetManagement implements DistributionSetManagement { final BiFunction, DistributionSetTag, T> updater) { final List allDs = dsIds.size() == 1 ? distributionSetRepository.findById(dsIds.iterator().next()).map(List::of).orElseGet(Collections::emptyList) : - distributionSetRepository.findAll(DistributionSetSpecification.byIds(dsIds)); + distributionSetRepository.findAll(DistributionSetSpecification.byIdsFetch(dsIds)); if (allDs.size() < dsIds.size()) { throw new EntityNotFoundException(DistributionSet.class, dsIds, allDs.stream().map(DistributionSet::getId).toList()); } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/DistributionSetRepository.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/DistributionSetRepository.java index 1a1bfd479..8459fca75 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/DistributionSetRepository.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/repository/DistributionSetRepository.java @@ -66,8 +66,7 @@ public interface DistributionSetRepository extends BaseEntityRepository * No access control applied. * @@ -77,8 +76,7 @@ public interface DistributionSetRepository extends BaseEntityRepository * No access control applied. * @@ -89,8 +87,7 @@ public interface DistributionSetRepository extends BaseEntityRepository findAssignedToTargetDistributionSetsById(@Param("ids") Collection ids); /** - * Finds {@link DistributionSet}s based on given ID that are assigned yet to - * an {@link Rollout}, i.e. in use. + * Finds {@link DistributionSet}s based on given ID that are assigned yet to an {@link Rollout}, i.e. in use. *

* No access control applied. * @@ -111,10 +108,8 @@ public interface DistributionSetRepository extends BaseEntityRepositoryfalse - i.e. is not deleted. + * {@link Specification} for retrieving {@link DistributionSet}s with DELETED attribute false - i.e. is not deleted. * * @return the {@link DistributionSet} {@link Specification} */ @@ -55,51 +47,44 @@ public final class DistributionSetSpecification { } /** - * {@link Specification} for retrieving {@link DistributionSet}s by its - * DELETED attribute. + * {@link Specification} for retrieving {@link DistributionSet}s by its DELETED attribute. * - * @param isDeleted TRUE/FALSE are compared to the attribute DELETED. If NULL the - * attribute is ignored + * @param isDeleted TRUE/FALSE are compared to the attribute DELETED. If NULL the attribute is ignored * @return the {@link DistributionSet} {@link Specification} */ public static Specification isDeleted(final Boolean isDeleted) { - return (dsRoot, query, cb) -> cb.equal(dsRoot. get(JpaDistributionSet_.deleted), isDeleted); + return (dsRoot, query, cb) -> cb.equal(dsRoot.get(JpaDistributionSet_.deleted), isDeleted); } /** - * {@link Specification} for retrieving {@link DistributionSet}s by its - * COMPLETED attribute. + * {@link Specification} for retrieving {@link DistributionSet}s by its COMPLETED attribute. * - * @param isCompleted TRUE/FALSE are compared to the attribute COMPLETED. If NULL the - * attribute is ignored + * @param isCompleted TRUE/FALSE are compared to the attribute COMPLETED. If NULL the attribute is ignored * @return the {@link DistributionSet} {@link Specification} */ public static Specification isCompleted(final Boolean isCompleted) { - return (dsRoot, query, cb) -> cb.equal(dsRoot. get(JpaDistributionSet_.complete), isCompleted); + return (dsRoot, query, cb) -> cb.equal(dsRoot.get(JpaDistributionSet_.complete), isCompleted); } /** - * {@link Specification} for retrieving {@link DistributionSet}s by its VALID - * attribute. + * {@link Specification} for retrieving {@link DistributionSet}s by its VALID attribute. * - * @param isValid TRUE/FALSE are compared to the attribute VALID. If NULL the - * attribute is ignored + * @param isValid TRUE/FALSE are compared to the attribute VALID. If NULL the attribute is ignored * @return the {@link DistributionSet} {@link Specification} */ public static Specification isValid(final Boolean isValid) { - return (dsRoot, query, cb) -> cb.equal(dsRoot. get(JpaDistributionSet_.valid), isValid); + return (dsRoot, query, cb) -> cb.equal(dsRoot.get(JpaDistributionSet_.valid), isValid); } /** - * {@link Specification} for retrieving {@link DistributionSet} with given - * {@link DistributionSet#getId()}. + * {@link Specification} for retrieving {@link DistributionSet} with given {@link DistributionSet#getId()}. * * @param distid to search * @return the {@link DistributionSet} {@link Specification} */ - public static Specification byId(final Long distid) { + public static Specification byIdFetch(final Long distid) { return (dsRoot, query, cb) -> { - final Predicate predicate = cb.equal(dsRoot. get(JpaDistributionSet_.id), distid); + final Predicate predicate = cb.equal(dsRoot.get(JpaDistributionSet_.id), distid); dsRoot.fetch(JpaDistributionSet_.modules, JoinType.LEFT); dsRoot.fetch(JpaDistributionSet_.type, JoinType.LEFT); query.distinct(true); @@ -109,15 +94,14 @@ public final class DistributionSetSpecification { } /** - * {@link Specification} for retrieving {@link DistributionSet} with given - * {@link DistributionSet#getId()}s. + * {@link Specification} for retrieving {@link DistributionSet} with given {@link DistributionSet#getId()}s. * * @param distids to search * @return the {@link DistributionSet} {@link Specification} */ - public static Specification byIds(final Collection distids) { + public static Specification byIdsFetch(final Collection distids) { return (dsRoot, query, cb) -> { - final Predicate predicate = dsRoot. get(JpaDistributionSet_.id).in(distids); + final Predicate predicate = dsRoot.get(JpaDistributionSet_.id).in(distids); dsRoot.fetch(JpaDistributionSet_.modules, JoinType.LEFT); dsRoot.fetch(JpaDistributionSet_.tags, JoinType.LEFT); dsRoot.fetch(JpaDistributionSet_.type, JoinType.LEFT); @@ -127,8 +111,7 @@ public final class DistributionSetSpecification { } /** - * {@link Specification} for retrieving {@link DistributionSet}s by "like name - * and like version". + * {@link Specification} for retrieving {@link DistributionSet}s by "like name and like version". * * @param name to be filtered on * @param version to be filtered on @@ -136,20 +119,18 @@ public final class DistributionSetSpecification { */ public static Specification likeNameAndVersion(final String name, final String version) { return (dsRoot, query, cb) -> cb.and( - cb.like(cb.lower(dsRoot. get(JpaDistributionSet_.name)), name.toLowerCase()), - cb.like(cb.lower(dsRoot. get(JpaDistributionSet_.version)), version.toLowerCase())); + cb.like(cb.lower(dsRoot.get(JpaDistributionSet_.name)), name.toLowerCase()), + cb.like(cb.lower(dsRoot.get(JpaDistributionSet_.version)), version.toLowerCase())); } /** - * {@link Specification} for retrieving {@link DistributionSet}s by "has at - * least one of the given tag names". + * {@link Specification} for retrieving {@link DistributionSet}s by "has at least one of the given tag names". * * @param tagNames to be filtered on * @param selectDSWithNoTag flag to select distribution sets with no tag * @return the {@link DistributionSet} {@link Specification} */ - public static Specification hasTags(final Collection tagNames, - final Boolean selectDSWithNoTag) { + public static Specification hasTags(final Collection tagNames, final Boolean selectDSWithNoTag) { return (dsRoot, query, cb) -> { final Predicate predicate = getHasTagsPredicate(dsRoot, cb, selectDSWithNoTag, tagNames); query.distinct(true); @@ -158,36 +139,30 @@ public final class DistributionSetSpecification { } /** - * returns query criteria {@link Specification} comparing case insensitive "NAME - * == AND VERSION ==". + * returns query criteria {@link Specification} comparing case insensitive "NAME == AND VERSION ==". * * @param name to be filtered on * @param version to be filtered on * @return the {@link Specification} */ - public static Specification equalsNameAndVersionIgnoreCase(final String name, - final String version) { + public static Specification equalsNameAndVersionIgnoreCase(final String name, final String version) { return (dsRoot, query, cb) -> cb.and( - cb.equal(cb.lower(dsRoot. get(JpaDistributionSet_.name)), name.toLowerCase()), - cb.equal(cb.lower(dsRoot. get(JpaDistributionSet_.version)), version.toLowerCase())); - + cb.equal(cb.lower(dsRoot.get(JpaDistributionSet_.name)), name.toLowerCase()), + cb.equal(cb.lower(dsRoot.get(JpaDistributionSet_.version)), version.toLowerCase())); } /** - * {@link Specification} for retrieving {@link DistributionSet} with given - * {@link DistributionSet#getType()}. + * {@link Specification} for retrieving {@link DistributionSet} with given {@link DistributionSet#getType()}. * * @param typeId id of distribution set type to search * @return the {@link DistributionSet} {@link Specification} */ public static Specification byType(final Long typeId) { - return (dsRoot, query, cb) -> cb.equal(dsRoot.get(JpaDistributionSet_.type).get(JpaDistributionSetType_.id), - typeId); + return (dsRoot, query, cb) -> cb.equal(dsRoot.get(JpaDistributionSet_.type).get(JpaDistributionSetType_.id), typeId); } /** - * {@link Specification} for retrieving {@link DistributionSet} for given id - * collection of {@link DistributionSet#getType()}. + * {@link Specification} for retrieving {@link DistributionSet} for given id collection of {@link DistributionSet#getType()}. * * @param typeIds id collection of distribution set type to search * @return the {@link DistributionSet} {@link Specification} @@ -203,54 +178,16 @@ public final class DistributionSetSpecification { * @return the {@link DistributionSet} {@link Specification} */ public static Specification hasTag(final Long tagId) { - return (dsRoot, query, cb) -> { - final SetJoin tags = dsRoot.join(JpaDistributionSet_.tags, - JoinType.LEFT); + final SetJoin tags = dsRoot.join(JpaDistributionSet_.tags, JoinType.LEFT); return cb.equal(tags.get(JpaDistributionSetTag_.id), tagId); }; } - /** - * Can be added to specification chain to order result by provided target - * - * Order: 1. Distribution set installed on target, 2. Distribution set(s) - * assigned to target, 3. Based on requested sorting or id if null. - * - * NOTE: Other specs, pagables and sort objects may alter the queries orderBy - * entry too, possibly invalidating the applied order, keep in mind when using - * this - * - * @param linkedControllerId controller id to get installed/assigned DS for - * @param sort - * @return specification that applies order by target, may be overwritten - */ - public static Specification orderedByLinkedTarget(final String linkedControllerId, - final Sort sort) { - return (dsRoot, query, cb) -> { - final Root targetRoot = query.from(JpaTarget.class); - - final Expression assignedInstalledCase = cb.selectCase() - .when(cb.equal(targetRoot.get(JpaTarget_.installedDistributionSet), dsRoot), 1) - .when(cb.equal(targetRoot.get(JpaTarget_.assignedDistributionSet), dsRoot), 2).otherwise(3); - - final List orders = new ArrayList<>(); - orders.add(cb.asc(assignedInstalledCase)); - if (sort == null || sort.isEmpty()) { - orders.add(cb.asc(dsRoot.get(JpaDistributionSet_.id))); - } else { - orders.addAll(QueryUtils.toOrders(sort, dsRoot, cb)); - } - query.orderBy(orders); - - return cb.equal(targetRoot.get(JpaTarget_.controllerId), linkedControllerId); - }; - } - - private static Predicate getHasTagsPredicate(final Root dsRoot, final CriteriaBuilder cb, + private static Predicate getHasTagsPredicate( + final Root dsRoot, final CriteriaBuilder cb, final Boolean selectDSWithNoTag, final Collection tagNames) { - final SetJoin tags = dsRoot.join(JpaDistributionSet_.tags, - JoinType.LEFT); + final SetJoin tags = dsRoot.join(JpaDistributionSet_.tags, JoinType.LEFT); final Path exp = tags.get(JpaDistributionSetTag_.name); final List hasTagsPredicates = new ArrayList<>(); @@ -261,8 +198,8 @@ public final class DistributionSetSpecification { hasTagsPredicates.add(exp.in(tagNames)); } - return hasTagsPredicates.stream().reduce(cb::or).orElseThrow( - () -> new RuntimeException("Neither NO_TAG, nor TAG distribution set tag filter was provided!")); + return hasTagsPredicates.stream().reduce(cb::or) + .orElseThrow(() -> new RuntimeException("Neither NO_TAG, nor TAG distribution set tag filter was provided!")); } private static boolean isNoTagActive(final Boolean selectDSWithNoTag) { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/AbstractAccessControllerTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/AbstractAccessControllerTest.java index ce91fb24a..d6e49829b 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/AbstractAccessControllerTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/AbstractAccessControllerTest.java @@ -41,12 +41,9 @@ public abstract class AbstractAccessControllerTest extends AbstractJpaIntegratio } protected void permitAllOperations(final AccessController.Operation operation) { - testAccessControlManger.defineAccessRule( - JpaTarget.class, operation, Specification.where(null), type -> true); - testAccessControlManger.defineAccessRule( - JpaTargetType.class, operation, Specification.where(null), type -> true); - testAccessControlManger.defineAccessRule( - JpaDistributionSet.class, operation, Specification.where(null), type -> true); + testAccessControlManger.defineAccessRule(JpaTarget.class, operation, Specification.where(null), type -> true); + testAccessControlManger.defineAccessRule(JpaTargetType.class, operation, Specification.where(null), type -> true); + testAccessControlManger.defineAccessRule(JpaDistributionSet.class, operation, Specification.where(null), type -> true); } @BeforeEach @@ -79,17 +76,17 @@ public abstract class AbstractAccessControllerTest extends AbstractJpaIntegratio @Override public Optional> getAccessRules(final Operation operation) { - if (contextAware.getCurrentTenant() != null && SecurityContextTenantAware.SYSTEM_USER.equals( - contextAware.getCurrentUsername())) { + if (contextAware.getCurrentTenant() != null + && SecurityContextTenantAware.SYSTEM_USER.equals(contextAware.getCurrentUsername())) { // as tenant, no restrictions return Optional.empty(); } + return Optional.ofNullable(testAccessControlManger.getAccessRule(JpaTarget.class, operation)); } @Override - public void assertOperationAllowed(final Operation operation, final JpaTarget entity) - throws InsufficientPermissionException { + public void assertOperationAllowed(final Operation operation, final JpaTarget entity) throws InsufficientPermissionException { testAccessControlManger.assertOperation(JpaTarget.class, operation, List.of(entity)); } @@ -101,17 +98,17 @@ public abstract class AbstractAccessControllerTest extends AbstractJpaIntegratio } @Bean - public AccessController targetTypeAccessController( - final TestAccessControlManger testAccessControlManger) { + public AccessController targetTypeAccessController(final TestAccessControlManger testAccessControlManger) { return new AccessController<>() { @Override public Optional> getAccessRules(final Operation operation) { - if (contextAware.getCurrentTenant() != null && SecurityContextTenantAware.SYSTEM_USER.equals( - contextAware.getCurrentUsername())) { + if (contextAware.getCurrentTenant() != null + && SecurityContextTenantAware.SYSTEM_USER.equals(contextAware.getCurrentUsername())) { // as tenant, no restrictions return Optional.empty(); } + return Optional.ofNullable(testAccessControlManger.getAccessRule(JpaTargetType.class, operation)); } @@ -129,17 +126,17 @@ public abstract class AbstractAccessControllerTest extends AbstractJpaIntegratio } @Bean - public AccessController distributionSetAccessController( - final TestAccessControlManger testAccessControlManger) { + public AccessController distributionSetAccessController(final TestAccessControlManger testAccessControlManger) { return new AccessController<>() { @Override public Optional> getAccessRules(final Operation operation) { - if (contextAware.getCurrentTenant() != null && SecurityContextTenantAware.SYSTEM_USER.equals( - contextAware.getCurrentUsername())) { + if (contextAware.getCurrentTenant() != null + && SecurityContextTenantAware.SYSTEM_USER.equals(contextAware.getCurrentUsername())) { // as tenant, no restrictions return Optional.empty(); } + return Optional.ofNullable(testAccessControlManger.getAccessRule(JpaDistributionSet.class, operation)); } @@ -156,4 +153,4 @@ public abstract class AbstractAccessControllerTest extends AbstractJpaIntegratio }; } } -} +} \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/DistributionSetAccessControllerTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/DistributionSetAccessControllerTest.java index 002b38b92..24e5aad46 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/DistributionSetAccessControllerTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/DistributionSetAccessControllerTest.java @@ -13,9 +13,12 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.List; +import jakarta.persistence.criteria.Predicate; + import io.qameta.allure.Description; import io.qameta.allure.Feature; import io.qameta.allure.Story; @@ -26,8 +29,8 @@ import org.eclipse.hawkbit.repository.exception.InsufficientPermissionException; import org.eclipse.hawkbit.repository.jpa.acm.AccessController; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSetMetadata; +import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet_; import org.eclipse.hawkbit.repository.jpa.model.JpaTarget; -import org.eclipse.hawkbit.repository.jpa.specifications.DistributionSetSpecification; import org.eclipse.hawkbit.repository.jpa.specifications.TargetSpecifications; import org.eclipse.hawkbit.repository.model.Action; import org.eclipse.hawkbit.repository.model.DistributionSet; @@ -37,6 +40,7 @@ import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.model.TargetFilterQuery; import org.junit.jupiter.api.Test; import org.springframework.data.domain.Pageable; +import org.springframework.data.jpa.domain.Specification; @Feature("Component Tests - Access Control") @Story("Test Distribution Set Access Controller") @@ -136,47 +140,43 @@ class DistributionSetAccessControllerTest extends AbstractAccessControllerTest { defineAccess(AccessController.Operation.UPDATE, permitted); // verify distributionSetManagement#assignSoftwareModules - assertThat(distributionSetManagement.assignSoftwareModules(permitted.getId(), - Collections.singletonList(swModule.getId()))).satisfies(ds -> { - assertThat(ds.getModules().stream().map(Identifiable::getId).toList()).contains(swModule.getId()); - }); - assertThatThrownBy(() -> { - distributionSetManagement.assignSoftwareModules(readOnly.getId(), - Collections.singletonList(swModule.getId())); - }).as("Distribution set not allowed to me modified.").isInstanceOf(EntityNotFoundException.class); - assertThatThrownBy(() -> { - distributionSetManagement.assignSoftwareModules(hidden.getId(), - Collections.singletonList(swModule.getId())); - }).as("Distribution set should not be visible.").isInstanceOf(EntityNotFoundException.class); + assertThat(distributionSetManagement.assignSoftwareModules(permitted.getId(), Collections.singletonList(swModule.getId()))) + .satisfies(ds -> assertThat(ds.getModules().stream().map(Identifiable::getId).toList()).contains(swModule.getId())); + assertThatThrownBy(() -> distributionSetManagement.assignSoftwareModules(readOnly.getId(), Collections.singletonList(swModule.getId()))) + .as("Distribution set not allowed to me modified.") + .isInstanceOf(EntityNotFoundException.class); + assertThatThrownBy(() -> distributionSetManagement.assignSoftwareModules(hidden.getId(), Collections.singletonList(swModule.getId()))) + .as("Distribution set should not be visible.") + .isInstanceOf(EntityNotFoundException.class); final JpaDistributionSetMetadata metadata = new JpaDistributionSetMetadata("test", "test"); // verify distributionSetManagement#createMetaData distributionSetManagement.createMetaData(permitted.getId(), Collections.singletonList(metadata)); - assertThatThrownBy(() -> { - distributionSetManagement.createMetaData(readOnly.getId(), Collections.singletonList(metadata)); - }).as("Distribution set not allowed to me modified.").isInstanceOf(EntityNotFoundException.class); - assertThatThrownBy(() -> { - distributionSetManagement.createMetaData(hidden.getId(), Collections.singletonList(metadata)); - }).as("Distribution set should not be visible.").isInstanceOf(EntityNotFoundException.class); + assertThatThrownBy(() -> distributionSetManagement.createMetaData(readOnly.getId(), Collections.singletonList(metadata))) + .as("Distribution set not allowed to me modified.") + .isInstanceOf(EntityNotFoundException.class); + assertThatThrownBy(() -> distributionSetManagement.createMetaData(hidden.getId(), Collections.singletonList(metadata))) + .as("Distribution set should not be visible.") + .isInstanceOf(EntityNotFoundException.class); // verify distributionSetManagement#updateMetaData distributionSetManagement.updateMetaData(permitted.getId(), metadata); - assertThatThrownBy(() -> { - distributionSetManagement.updateMetaData(readOnly.getId(), metadata); - }).as("Distribution set not allowed to me modified.").isInstanceOf(EntityNotFoundException.class); - assertThatThrownBy(() -> { - distributionSetManagement.updateMetaData(hidden.getId(), metadata); - }).as("Distribution set should not be visible.").isInstanceOf(EntityNotFoundException.class); + assertThatThrownBy(() -> distributionSetManagement.updateMetaData(readOnly.getId(), metadata)) + .as("Distribution set not allowed to me modified.") + .isInstanceOf(EntityNotFoundException.class); + assertThatThrownBy(() -> distributionSetManagement.updateMetaData(hidden.getId(), metadata)) + .as("Distribution set should not be visible.") + .isInstanceOf(EntityNotFoundException.class); // verify distributionSetManagement#deleteMetaData distributionSetManagement.deleteMetaData(permitted.getId(), metadata.getKey()); - assertThatThrownBy(() -> { - distributionSetManagement.deleteMetaData(readOnly.getId(), metadata.getKey()); - }).as("Distribution set not allowed to me modified.").isInstanceOf(EntityNotFoundException.class); - assertThatThrownBy(() -> { - distributionSetManagement.deleteMetaData(hidden.getId(), metadata.getKey()); - }).as("Distribution set should not be visible.").isInstanceOf(EntityNotFoundException.class); + assertThatThrownBy(() -> distributionSetManagement.deleteMetaData(readOnly.getId(), metadata.getKey())) + .as("Distribution set not allowed to me modified.") + .isInstanceOf(EntityNotFoundException.class); + assertThatThrownBy(() -> distributionSetManagement.deleteMetaData(hidden.getId(), metadata.getKey())) + .as("Distribution set should not be visible.") + .isInstanceOf(EntityNotFoundException.class); } @Test @@ -303,7 +303,15 @@ class DistributionSetAccessControllerTest extends AbstractAccessControllerTest { final List ids = targets.stream().map(DistributionSet::getId).toList(); testAccessControlManger.defineAccessRule( JpaDistributionSet.class, operation, - DistributionSetSpecification.byIds(ids), + dsByIds(ids), distributionSet -> ids.contains(distributionSet.getId())); } + + private static Specification dsByIds(final Collection distids) { + return (dsRoot, query, cb) -> { + final Predicate predicate = dsRoot.get(JpaDistributionSet_.id).in(distids); + query.distinct(true); + return predicate; + }; + } } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/TargetAccessControllerTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/TargetAccessControllerTest.java index ffc9a8b4a..1b2e6ca2f 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/TargetAccessControllerTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/TargetAccessControllerTest.java @@ -16,6 +16,8 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import jakarta.persistence.criteria.Predicate; + import io.qameta.allure.Description; import io.qameta.allure.Feature; import io.qameta.allure.Story; @@ -25,8 +27,8 @@ import org.eclipse.hawkbit.repository.exception.InsufficientPermissionException; import org.eclipse.hawkbit.repository.jpa.acm.AccessController; import org.eclipse.hawkbit.repository.jpa.autoassign.AutoAssignChecker; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet; +import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet_; import org.eclipse.hawkbit.repository.jpa.model.JpaTarget; -import org.eclipse.hawkbit.repository.jpa.specifications.DistributionSetSpecification; import org.eclipse.hawkbit.repository.jpa.specifications.TargetSpecifications; import org.eclipse.hawkbit.repository.model.Action; import org.eclipse.hawkbit.repository.model.DistributionSet; @@ -39,6 +41,7 @@ import org.eclipse.hawkbit.repository.model.TargetUpdateStatus; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Pageable; +import org.springframework.data.jpa.domain.Specification; @Feature("Component Tests - Access Control") @Story("Test Target Access Controller") @@ -75,13 +78,14 @@ class TargetAccessControllerTest extends AbstractAccessControllerTest { // verify targetManagement#getByControllerID assertThat(targetManagement.getByControllerID(permittedTarget.getControllerId())).isPresent(); - assertThatThrownBy(() -> targetManagement.getByControllerID(hiddenTarget.getControllerId())) + final String hiddenTargetControllerId = hiddenTarget.getControllerId(); + assertThatThrownBy(() -> targetManagement.getByControllerID(hiddenTargetControllerId)) .as("Missing read permissions for hidden target.") .isInstanceOf(InsufficientPermissionException.class); // verify targetManagement#getByControllerID assertThat(targetManagement - .getByControllerID(Arrays.asList(permittedTarget.getControllerId(), hiddenTarget.getControllerId())) + .getByControllerID(Arrays.asList(permittedTarget.getControllerId(), hiddenTargetControllerId)) .stream().map(Identifiable::getId).toList()).containsOnly(permittedTarget.getId()); // verify targetManagement#get @@ -94,9 +98,9 @@ class TargetAccessControllerTest extends AbstractAccessControllerTest { // verify targetManagement#getControllerAttributes assertThat(targetManagement.getControllerAttributes(permittedTarget.getControllerId())).isEmpty(); - assertThatThrownBy(() -> { - assertThat(targetManagement.getControllerAttributes(hiddenTarget.getControllerId())).isEmpty(); - }).as("Target should not be found.").isInstanceOf(InsufficientPermissionException.class); + assertThatThrownBy(() -> targetManagement.getControllerAttributes(hiddenTargetControllerId)) + .as("Target should not be found.") + .isInstanceOf(InsufficientPermissionException.class); final TargetFilterQuery targetFilterQuery = targetFilterQueryManagement .create(entityFactory.targetFilterQuery().create().name("test").query("id==*")); @@ -149,16 +153,13 @@ class TargetAccessControllerTest extends AbstractAccessControllerTest { .map(Identifiable::getId).toList()).containsOnly(permittedTarget.getId(), readOnlyTarget.getId()); // verify targetManagement#assignTag on permitted target - assertThat(targetManagement - .assignTag(Collections.singletonList(permittedTarget.getControllerId()), myTag.getId()) - .size()).isEqualTo(1); + assertThat(targetManagement.assignTag(Collections.singletonList(permittedTarget.getControllerId()), myTag.getId())) + .hasSize(1); // verify targetManagement#unassignTag on permitted target - assertThat(targetManagement - .unassignTag(Collections.singletonList(permittedTarget.getControllerId()), myTag.getId()) - .size()).isEqualTo(1); + assertThat(targetManagement.unassignTag(Collections.singletonList(permittedTarget.getControllerId()), myTag.getId())) + .hasSize(1); // verify targetManagement#assignTag on permitted target - assertThat( - targetManagement.assignTag(Collections.singletonList(permittedTarget.getControllerId()), myTag.getId())) + assertThat(targetManagement.assignTag(Collections.singletonList(permittedTarget.getControllerId()), myTag.getId())) .hasSize(1); // verify targetManagement#unAssignTag on permitted target assertThat(targetManagement.unassignTag(List.of(permittedTarget.getControllerId()), myTag.getId()).get(0).getControllerId()) @@ -174,29 +175,23 @@ class TargetAccessControllerTest extends AbstractAccessControllerTest { // .isInstanceOf(InsufficientPermissionException.class); // assignment is denied for readOnlyTarget (read, but no update permissions) - assertThatThrownBy(() -> { - targetManagement.assignTag(Collections.singletonList(readOnlyTarget.getControllerId()), myTag.getId()); - }).as("Missing update permissions for target to toggle tag assignment.") + assertThatThrownBy(() -> targetManagement.assignTag(Collections.singletonList(readOnlyTarget.getControllerId()), myTag.getId())) + .as("Missing update permissions for target to toggle tag assignment.") .isInstanceOfAny(InsufficientPermissionException.class); // assignment is denied for readOnlyTarget (read, but no update permissions) - assertThatThrownBy(() -> { - targetManagement.unassignTag(List.of(readOnlyTarget.getControllerId()), myTag.getId()); - }).as("Missing update permissions for target to toggle tag assignment.") + assertThatThrownBy(() -> targetManagement.unassignTag(List.of(readOnlyTarget.getControllerId()), myTag.getId())) + .as("Missing update permissions for target to toggle tag assignment.") .isInstanceOf(InsufficientPermissionException.class); // assignment is denied for hiddenTarget since it's hidden - assertThatThrownBy(() -> { - targetManagement - .assignTag(Collections.singletonList(hiddenTarget.getControllerId()), myTag.getId()) - .size(); - }).as("Missing update permissions for target to toggle tag assignment.") + assertThatThrownBy(() -> targetManagement.assignTag(Collections.singletonList(hiddenTarget.getControllerId()), myTag.getId())) + .as("Missing update permissions for target to toggle tag assignment.") .isInstanceOf(InsufficientPermissionException.class); // assignment is denied for hiddenTarget since it's hidden - assertThatThrownBy(() -> { - targetManagement.assignTag(Collections.singletonList(hiddenTarget.getControllerId()), myTag.getId()); - }).as("Missing update permissions for target to toggle tag assignment.") + assertThatThrownBy(() -> targetManagement.assignTag(Collections.singletonList(hiddenTarget.getControllerId()), myTag.getId())) + .as("Missing update permissions for target to toggle tag assignment.") .isInstanceOf(InsufficientPermissionException.class); // assignment is denied for hiddenTarget since it's hidden @@ -358,14 +353,13 @@ class TargetAccessControllerTest extends AbstractAccessControllerTest { defineAccess(AccessController.Operation.UPDATE, updateTargets); defineAccess(AccessController.Operation.READ, merge(updateTargets, readTargets)); - ; final TargetFilterQuery targetFilterQuery = targetFilterQueryManagement .create(entityFactory.targetFilterQuery().create().name("testName").query("id==*")); testAccessControlManger.defineAccessRule( JpaDistributionSet.class, AccessController.Operation.READ, - DistributionSetSpecification.byId(distributionSet.getId()), + dsById(distributionSet.getId()), ds -> ds.getId().equals(distributionSet.getId())); targetFilterQueryManagement.updateAutoAssignDS(entityFactory.targetFilterQuery() @@ -395,4 +389,12 @@ class TargetAccessControllerTest extends AbstractAccessControllerTest { TargetSpecifications.hasIdIn(ids), target -> ids.contains(target.getId())); } -} + + private static Specification dsById(final Long distid) { + return (dsRoot, query, cb) -> { + final Predicate predicate = cb.equal(dsRoot.get(JpaDistributionSet_.id), distid); + query.distinct(true); + return predicate; + }; + } +} \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/TestAccessControlManger.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/TestAccessControlManger.java index bd1e69dbf..5728454fe 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/TestAccessControlManger.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/TestAccessControlManger.java @@ -31,13 +31,13 @@ public class TestAccessControlManger { public void defineAccessRule( final Class ruleClass, final AccessController.Operation operation, final Specification specification, final Predicate check) { - accessRules.put(new AccessRuleId(ruleClass, operation), new AccessRule(specification, check)); + accessRules.put(new AccessRuleId<>(ruleClass, operation), new AccessRule<>(specification, check)); } public Specification getAccessRule(final Class ruleClass, final AccessController.Operation operation) { - @SuppressWarnings("unchecked") final AccessRule accessRule = (AccessRule) accessRules.getOrDefault( - new AccessRuleId(ruleClass, operation), null); + @SuppressWarnings("unchecked") + final AccessRule accessRule = (AccessRule) accessRules.getOrDefault(new AccessRuleId<>(ruleClass, operation), null); if (accessRule == null) { return nop(); } else { @@ -46,8 +46,8 @@ public class TestAccessControlManger { } public void assertOperation(final Class ruleClass, final AccessController.Operation operation, final List entities) { - @SuppressWarnings("unchecked") final AccessRule accessRule = (AccessRule) accessRules.getOrDefault( - new AccessRuleId(ruleClass, operation), null); + @SuppressWarnings("unchecked") + final AccessRule accessRule = (AccessRule) accessRules.getOrDefault(new AccessRuleId<>(ruleClass, operation), null); if (accessRule == null) { throw new InsufficientPermissionException("No access define - reject all"); } else { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DistributionSetManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DistributionSetManagementTest.java index 27f3ab6ef..cadaf9be6 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DistributionSetManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DistributionSetManagementTest.java @@ -18,7 +18,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.Iterator; import java.util.List; import java.util.NoSuchElementException; import java.util.Optional; @@ -75,8 +74,6 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; -import org.springframework.data.domain.Sort; -import org.springframework.data.domain.Sort.Direction; /** * {@link DistributionSetManagement} tests. @@ -532,89 +529,6 @@ class DistributionSetManagementTest extends AbstractJpaIntegrationTest { assertThat(updated.getDistributionSet().getId()).isEqualTo(ds.getId()); } - @Test - @Description("Tests that a DS queue is possible where the result is ordered by the target assignment, i.e. assigned first in the list.") - void findDistributionSetsAllOrderedByLinkTarget() { - final List buildDistributionSets = testdataFactory.createDistributionSets("dsOrder", 10); - - final List buildTargetFixtures = testdataFactory.createTargets(5, "tOrder", "someDesc"); - - final Iterator dsIterator = buildDistributionSets.iterator(); - final Iterator tIterator = buildTargetFixtures.iterator(); - final DistributionSet dsFirst = dsIterator.next(); - final DistributionSet dsSecond = dsIterator.next(); - final DistributionSet dsThree = dsIterator.next(); - final DistributionSet dsFour = dsIterator.next(); - final Target tFirst = tIterator.next(); - final Target tSecond = tIterator.next(); - - // set assigned - assignDistributionSet(dsSecond.getId(), tSecond.getControllerId()); - implicitLock(dsSecond); - assignDistributionSet(dsThree.getId(), tFirst.getControllerId()); - implicitLock(dsThree); - // set installed - testdataFactory.sendUpdateActionStatusToTargets(Collections.singleton(tSecond), Status.FINISHED, - singletonList("some message")); - - assignDistributionSet(dsFour.getId(), tSecond.getControllerId()); - implicitLock(dsFour); - - final DistributionSetFilter distributionSetFilter = - DistributionSetFilter.builder() - .isDeleted(false) - .isComplete(true) - .selectDSWithNoTag(Boolean.FALSE).build(); - - // target first only has an assigned DS-three so check order correct - final List tFirstPin = distributionSetManagement - .findByDistributionSetFilterOrderByLinkedTarget(PAGE, distributionSetFilter, tFirst.getControllerId()) - .getContent(); - assertThat(tFirstPin).hasSize(10); - // assigned - assertThat(tFirstPin.get(0)).isEqualTo(dsThree); - // remaining id:ASC - assertThat(tFirstPin.get(1)).isEqualTo(dsFirst); - assertThat(tFirstPin.get(2)).isEqualTo(dsSecond); - assertThat(tFirstPin.get(3)).isEqualTo(dsFour); - - // target second has installed DS-2 and assigned DS-4 so check order - // correct - final List tSecondPin = distributionSetManagement - .findByDistributionSetFilterOrderByLinkedTarget(PAGE, distributionSetFilter, tSecond.getControllerId()) - .getContent(); - assertThat(tSecondPin).hasSize(10); - // installed - assertThat(tSecondPin.get(0)).isEqualTo(dsSecond); - // assigned - assertThat(tSecondPin.get(1)).isEqualTo(dsFour); - // remaining id:ASC - assertThat(tSecondPin.get(2)).isEqualTo(dsFirst); - assertThat(tSecondPin.get(3)).isEqualTo(dsThree); - - // target second has installed DS-2 and assigned DS-4 so check order - // correct - final List tSecondPinOrderedByName = distributionSetManagement - .findByDistributionSetFilterOrderByLinkedTarget( - PageRequest.of(0, 500, Sort.by(Direction.DESC, "version")), distributionSetFilter, - tSecond.getControllerId()) - .getContent(); - assertThat(tSecondPinOrderedByName).hasSize(10); - // installed - assertThat(tSecondPinOrderedByName.get(0)).isEqualTo(buildDistributionSets.get(1)); - // assigned - assertThat(tSecondPinOrderedByName.get(1)).isEqualTo(buildDistributionSets.get(3)); - // remaining version:DESC - assertThat(tSecondPinOrderedByName.get(2)).isEqualTo(buildDistributionSets.get(9)); - assertThat(tSecondPinOrderedByName.get(3)).isEqualTo(buildDistributionSets.get(8)); - assertThat(tSecondPinOrderedByName.get(4)).isEqualTo(buildDistributionSets.get(7)); - assertThat(tSecondPinOrderedByName.get(5)).isEqualTo(buildDistributionSets.get(6)); - assertThat(tSecondPinOrderedByName.get(6)).isEqualTo(buildDistributionSets.get(5)); - assertThat(tSecondPinOrderedByName.get(7)).isEqualTo(buildDistributionSets.get(4)); - assertThat(tSecondPinOrderedByName.get(8)).isEqualTo(buildDistributionSets.get(2)); - assertThat(tSecondPinOrderedByName.get(9)).isEqualTo(buildDistributionSets.get(0)); - } - @Test @Description("searches for distribution sets based on the various filter options, e.g. name, version, desc., tags.") void searchDistributionSetsOnFilters() {