Remove unused TargetManagement#findByFilterOrderByLinkedDistributionSet (#2138)

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>
This commit is contained in:
Avgustin Marinov
2024-12-11 10:04:56 +02:00
committed by GitHub
parent ede05fe7b1
commit 68c0b616b7
5 changed files with 21 additions and 243 deletions

View File

@@ -453,30 +453,6 @@ public interface TargetManagement {
@PreAuthorize(SpringEvalExpressions.HAS_AUTH_READ_TARGET)
Slice<Target> findByTargetFilterQuery(@NotNull Pageable pageable, long targetFilterQueryId);
/**
* method retrieves all {@link Target}s from the repo in the following order:
* <p>
* <ol>
* <li>{@link Target}s which have the given {@link DistributionSet} as installed
* distribution set</li>
* <li>{@link Target}s which have the given {@link DistributionSet} as assigned
* distribution set</li>
* <li>{@link Target}s which have no connection to the given
* {@link DistributionSet}</li>
* </ol>
*
* @param pageable the page request to page the result set
* @param orderByDistributionSetId {@link DistributionSet#getId()} to be ordered by
* @param filterParams the filters to apply; only filters are enabled that have non-null
* value; filters are AND-gated
* @return a paged result {@link Page} of the {@link Target}s in a defined
* order.
* @throws EntityNotFoundException if distribution set with given ID does not exist
*/
@PreAuthorize(SpringEvalExpressions.HAS_AUTH_READ_TARGET)
Slice<Target> findByFilterOrderByLinkedDistributionSet(@NotNull Pageable pageable, long orderByDistributionSetId,
@NotNull FilterParams filterParams);
/**
* Find targets by tag name.
*

View File

@@ -464,19 +464,6 @@ public class JpaTargetManagement implements TargetManagement {
virtualPropertyReplacer, database)));
}
@Override
public Slice<Target> findByFilterOrderByLinkedDistributionSet(final Pageable pageable,
final long orderByDistributionSetId, final FilterParams filterParams) {
// remove default sort from pageable to not overwrite sorted spec
final OffsetBasedPageRequest unsortedPage = new OffsetBasedPageRequest(pageable.getOffset(),
pageable.getPageSize(), Sort.unsorted());
final List<Specification<JpaTarget>> specList = buildSpecificationList(filterParams);
specList.add(TargetSpecifications.orderedByLinkedDistributionSet(orderByDistributionSetId, pageable.getSort()));
return JpaManagementHelper.findAllWithoutCountBySpec(targetRepository, unsortedPage, specList);
}
@Override
public Page<Target> findByTag(final Pageable pageable, final long tagId) {
throwEntityNotFoundExceptionIfTagDoesNotExist(tagId);

View File

@@ -14,11 +14,9 @@ import java.util.Collection;
import java.util.List;
import jakarta.persistence.criteria.CriteriaBuilder;
import jakarta.persistence.criteria.Expression;
import jakarta.persistence.criteria.Join;
import jakarta.persistence.criteria.JoinType;
import jakarta.persistence.criteria.ListJoin;
import jakarta.persistence.criteria.Order;
import jakarta.persistence.criteria.Path;
import jakarta.persistence.criteria.Predicate;
import jakarta.persistence.criteria.Root;
@@ -31,7 +29,6 @@ import lombok.NoArgsConstructor;
import org.eclipse.hawkbit.repository.jpa.model.JpaAction;
import org.eclipse.hawkbit.repository.jpa.model.JpaAction_;
import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet;
import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSetType;
import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSetType_;
import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet_;
import org.eclipse.hawkbit.repository.jpa.model.JpaRolloutGroup_;
@@ -52,9 +49,7 @@ import org.eclipse.hawkbit.repository.model.Target;
import org.eclipse.hawkbit.repository.model.TargetTag;
import org.eclipse.hawkbit.repository.model.TargetType;
import org.eclipse.hawkbit.repository.model.TargetUpdateStatus;
import org.springframework.data.domain.Sort;
import org.springframework.data.jpa.domain.Specification;
import org.springframework.data.jpa.repository.query.QueryUtils;
/**
* Specifications class for {@link Target}s. The class provides Spring Data JPQL Specifications.
@@ -222,8 +217,7 @@ public final class TargetSpecifications {
}
/**
* Finds all targets by given {@link Target#getControllerId()}s and which
* are not yet assigned to given {@link DistributionSet}.
* Finds all targets by given {@link Target#getControllerId()}s and which are not yet assigned to given {@link DistributionSet}.
*
* @param tIDs to search for.
* @param distributionId set that is not yet assigned
@@ -232,8 +226,8 @@ public final class TargetSpecifications {
public static Specification<JpaTarget> hasControllerIdAndAssignedDistributionSetIdNot(final List<String> tIDs,
@NotNull final Long distributionId) {
return (targetRoot, query, cb) -> cb.and(targetRoot.get(JpaTarget_.controllerId).in(tIDs),
cb.or(cb.notEqual(targetRoot.<JpaDistributionSet> get(JpaTarget_.assignedDistributionSet)
.get(JpaDistributionSet_.id), distributionId),
cb.or(
cb.notEqual(targetRoot.get(JpaTarget_.assignedDistributionSet).get(JpaDistributionSet_.id), distributionId),
cb.isNull(targetRoot.<JpaDistributionSet> get(JpaTarget_.assignedDistributionSet))));
}
@@ -276,8 +270,7 @@ public final class TargetSpecifications {
*/
public static Specification<JpaTarget> hasAssignedDistributionSet(final Long distributionSetId) {
return (targetRoot, query, cb) -> cb.equal(
targetRoot.<JpaDistributionSet> get(JpaTarget_.assignedDistributionSet).get(JpaDistributionSet_.id),
distributionSetId);
targetRoot.get(JpaTarget_.assignedDistributionSet).get(JpaDistributionSet_.id), distributionSetId);
}
/**
@@ -481,45 +474,6 @@ public final class TargetSpecifications {
cb.notEqual(targetRoot.get(JpaTarget_.targetType).get(JpaTargetType_.id), typeId));
}
/**
* Can be added to specification chain to order result by provided
* distribution set
*
* Order: 1. Targets with DS installed, 2. Targets with DS assigned, 3.
* Based on requested sorting or id if <code>null</code>.
*
* 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 distributionSetIdForOrder distribution set to consider
* @param sort the sorting requested
* @return specification that applies order by ds, may be overwritten
*/
public static Specification<JpaTarget> orderedByLinkedDistributionSet(final long distributionSetIdForOrder,
final Sort sort) {
return (targetRoot, query, cb) -> {
// Enhance query with custom select based sort
final Expression<Object> selectCase = cb.selectCase()
.when(cb.equal(targetRoot.get(JpaTarget_.installedDistributionSet).get(JpaDistributionSet_.id),
distributionSetIdForOrder), 1)
.when(cb.equal(targetRoot.get(JpaTarget_.assignedDistributionSet).get(JpaDistributionSet_.id),
distributionSetIdForOrder), 2)
.otherwise(100);
final List<Order> orders = new ArrayList<>();
orders.add(cb.asc(selectCase));
if (sort == null || sort.isEmpty()) {
orders.add(cb.desc(targetRoot.get(JpaTarget_.id)));
} else {
orders.addAll(QueryUtils.toOrders(sort, targetRoot, cb));
}
query.orderBy(orders);
// Spec only provides order, so no further filtering
return query.getRestriction();
};
}
public static Specification<JpaTarget> failedActionsForRollout(final String rolloutId) {
return (targetRoot, query, cb) -> {
Join<JpaTarget, Action> targetActions =

View File

@@ -11,23 +11,18 @@ package org.eclipse.hawkbit.repository.jpa.management;
import static org.assertj.core.api.Assertions.assertThat;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.stream.Collectors;
import io.qameta.allure.Description;
import io.qameta.allure.Feature;
import io.qameta.allure.Step;
import io.qameta.allure.Story;
import org.eclipse.hawkbit.repository.FilterParams;
import org.eclipse.hawkbit.repository.Identifiable;
import org.eclipse.hawkbit.repository.builder.DistributionSetCreate;
import org.eclipse.hawkbit.repository.jpa.AbstractJpaIntegrationTest;
import org.eclipse.hawkbit.repository.model.Action;
import org.eclipse.hawkbit.repository.model.Action.Status;
import org.eclipse.hawkbit.repository.model.DistributionSet;
import org.eclipse.hawkbit.repository.model.DistributionSetType;
@@ -36,12 +31,7 @@ import org.eclipse.hawkbit.repository.model.TargetFilterQuery;
import org.eclipse.hawkbit.repository.model.TargetTag;
import org.eclipse.hawkbit.repository.model.TargetType;
import org.eclipse.hawkbit.repository.model.TargetUpdateStatus;
import org.eclipse.hawkbit.repository.model.TenantAwareBaseEntity;
import org.junit.jupiter.api.Test;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Slice;
import org.springframework.data.domain.Sort;
import org.springframework.data.domain.Sort.Direction;
@Feature("Component Tests - Repository")
@Story("Target Management Searches")
@@ -142,12 +132,9 @@ class TargetManagementSearchTest extends AbstractJpaIntegrationTest {
final List<TargetUpdateStatus> both = List.of(TargetUpdateStatus.UNKNOWN, TargetUpdateStatus.PENDING);
// get final updated version of targets
targAs = targetManagement
.getByControllerID(targAs.stream().map(Target::getControllerId).collect(Collectors.toList()));
targBs = targetManagement
.getByControllerID(targBs.stream().map(Target::getControllerId).collect(Collectors.toList()));
targCs = targetManagement
.getByControllerID(targCs.stream().map(Target::getControllerId).collect(Collectors.toList()));
targAs = targetManagement.getByControllerID(targAs.stream().map(Target::getControllerId).toList());
targBs = targetManagement.getByControllerID(targBs.stream().map(Target::getControllerId).toList());
targCs = targetManagement.getByControllerID(targCs.stream().map(Target::getControllerId).toList());
// try to find several targets with different filter settings
verifyThat1TargetHasNameAndId("targ-A-special", targSpecialName.getControllerId());
@@ -157,8 +144,7 @@ class TargetManagementSearchTest extends AbstractJpaIntegrationTest {
verifyThat1TargetHasTagHasDescOrNameAndDs(targTagW, setA, targetManagement.getByControllerID(assignedC).get());
verifyThat0TargetsWithTagAndDescOrNameHasDS(targTagW, setA);
verifyThat0TargetsWithNameOrdescAndDSHaveTag(targTagX, setA);
verifyThat3TargetsHaveDSAssigned(setA,
targetManagement.getByControllerID(Arrays.asList(assignedA, assignedB, assignedC)));
verifyThat3TargetsHaveDSAssigned(setA, targetManagement.getByControllerID(Arrays.asList(assignedA, assignedB, assignedC)));
verifyThat1TargetWithDescOrNameHasDS(setA, targetManagement.getByControllerID(assignedA).get());
List<Target> expected = concat(targAs, targBs, targCs, targDs);
expected.removeAll(targetManagement.getByControllerID(Arrays.asList(assignedA, assignedB, assignedC)));
@@ -185,7 +171,7 @@ class TargetManagementSearchTest extends AbstractJpaIntegrationTest {
targetManagement.getByControllerID(Arrays.asList(assignedB, assignedC)));
verifyThat2TargetsWithGivenTagAreInPending(targTagW, pending,
targetManagement.getByControllerID(Arrays.asList(assignedB, assignedC)));
verifyThat200targetsWithGivenTagAreInStatusPendingorUnknown(targTagW, both, concat(targBs, targCs));
verifyThat200targetsWithGivenTagAreInStatusPendingOrUnknown(targTagW, both, concat(targBs, targCs));
verifyThat1TargetAIsInStatusPendingAndHasDSInstalled(installedSet, pending,
targetManagement.getByControllerID(installedC).get());
verifyThat1TargetHasTypeAndDSAssigned(targetTypeX, setB, targetManagement.getByControllerID(assignedE).get());
@@ -201,131 +187,6 @@ class TargetManagementSearchTest extends AbstractJpaIntegrationTest {
verifyThat198TargetsAreInStatusUnknownAndOverdue(unknown, expected);
}
@Test
@Description("Tests the correct order of targets based on selected distribution set. The system expects to have an order based on installed, assigned DS.")
void targetSearchWithVariousFilterCombinationsAndOrderByDistributionSet() {
final List<Target> notAssigned = testdataFactory.createTargets(3, "not", "first description");
List<Target> targAssigned = testdataFactory.createTargets(3, "assigned", "first description");
List<Target> targInstalled = testdataFactory.createTargets(3, "installed", "first description");
final DistributionSet ds = testdataFactory.createDistributionSet("a");
targAssigned = assignDistributionSet(ds, targAssigned).getAssignedEntity().stream().map(Action::getTarget)
.collect(Collectors.toList());
targInstalled = assignDistributionSet(ds, targInstalled).getAssignedEntity().stream().map(Action::getTarget)
.collect(Collectors.toList());
targInstalled = testdataFactory
.sendUpdateActionStatusToTargets(targInstalled, Status.FINISHED, Collections.singletonList("installed"))
.stream().map(Action::getTarget).collect(Collectors.toList());
final Slice<Target> result = targetManagement.findByFilterOrderByLinkedDistributionSet(PAGE, ds.getId(),
new FilterParams(null, null, null, null, Boolean.FALSE));
final Comparator<TenantAwareBaseEntity> byId = Comparator.comparingLong(Identifiable::getId);
assertThat(result.getNumberOfElements()).isEqualTo(9);
final List<Target> expected = new ArrayList<>();
targInstalled.sort(byId);
targAssigned.sort(byId);
notAssigned.sort(byId);
expected.addAll(targInstalled);
expected.addAll(targAssigned);
expected.addAll(notAssigned);
assertThat(result.getContent()).usingElementComparator(controllerIdComparator())
.containsExactly(expected.toArray(new Target[0]));
}
@Test
@Description("Tests the correct order of targets based on selected distribution set and sort parameter. The system expects to have an order based on installed, assigned DS.")
void targetSearchWithOrderByDistributionSetAndSortParam() {
final List<Target> notAssigned = testdataFactory.createTargets(3, "not", "first description");
List<Target> targAssigned = testdataFactory.createTargets(3, "assigned", "first description");
List<Target> targInstalled = testdataFactory.createTargets(3, "installed", "first description");
final DistributionSet ds = testdataFactory.createDistributionSet("a");
targAssigned = assignDistributionSet(ds, targAssigned).getAssignedEntity().stream().map(Action::getTarget)
.collect(Collectors.toList());
targInstalled = assignDistributionSet(ds, targInstalled).getAssignedEntity().stream().map(Action::getTarget)
.collect(Collectors.toList());
targInstalled = testdataFactory
.sendUpdateActionStatusToTargets(targInstalled, Status.FINISHED, Collections.singletonList("installed"))
.stream().map(Action::getTarget).collect(Collectors.toList());
final List<Target> targetsOrderedByDistAndName = targetManagement
.findByFilterOrderByLinkedDistributionSet(PageRequest.of(0, 500, Sort.by(Direction.DESC, "name")),
ds.getId(), new FilterParams(null, null, null, null, Boolean.FALSE))
.getContent();
assertThat(targetsOrderedByDistAndName).hasSize(9);
assertThatTargetNameEquals(targetsOrderedByDistAndName, 0, targInstalled, 2);
assertThatTargetNameEquals(targetsOrderedByDistAndName, 1, targInstalled, 1);
assertThatTargetNameEquals(targetsOrderedByDistAndName, 2, targInstalled, 0);
assertThatTargetNameEquals(targetsOrderedByDistAndName, 3, targAssigned, 2);
assertThatTargetNameEquals(targetsOrderedByDistAndName, 4, targAssigned, 1);
assertThatTargetNameEquals(targetsOrderedByDistAndName, 5, targAssigned, 0);
assertThatTargetNameEquals(targetsOrderedByDistAndName, 6, notAssigned, 2);
assertThatTargetNameEquals(targetsOrderedByDistAndName, 7, notAssigned, 1);
assertThatTargetNameEquals(targetsOrderedByDistAndName, 8, notAssigned, 0);
}
@Test
@Description("Tests the correct order of targets with applied overdue filter based on selected distribution set. The system expects to have an order based on installed, assigned DS.")
void targetSearchWithOverdueFilterAndOrderByDistributionSet() {
final Long lastTargetQueryAlwaysOverdue = 0L;
final long lastTargetQueryNotOverdue = System.currentTimeMillis();
final Long[] overdueMix = { lastTargetQueryAlwaysOverdue, lastTargetQueryNotOverdue,
lastTargetQueryAlwaysOverdue, null, lastTargetQueryAlwaysOverdue };
final List<Target> notAssigned = new ArrayList<>(overdueMix.length);
List<Target> targAssigned = new ArrayList<>(overdueMix.length);
List<Target> targInstalled = new ArrayList<>(overdueMix.length);
for (int i = 0; i < overdueMix.length; i++) {
notAssigned.add(targetManagement
.create(entityFactory.target().create().controllerId("not" + i).lastTargetQuery(overdueMix[i])));
targAssigned.add(targetManagement.create(
entityFactory.target().create().controllerId("assigned" + i).lastTargetQuery(overdueMix[i])));
targInstalled.add(targetManagement.create(
entityFactory.target().create().controllerId("installed" + i).lastTargetQuery(overdueMix[i])));
}
final DistributionSet ds = testdataFactory.createDistributionSet("a");
targAssigned = assignDistributionSet(ds, targAssigned).getAssignedEntity().stream().map(Action::getTarget)
.collect(Collectors.toList());
targInstalled = assignDistributionSet(ds, targInstalled).getAssignedEntity().stream().map(Action::getTarget)
.collect(Collectors.toList());
targInstalled = testdataFactory
.sendUpdateActionStatusToTargets(targInstalled, Status.FINISHED, Collections.singletonList("installed"))
.stream().map(Action::getTarget).collect(Collectors.toList());
final Slice<Target> result = targetManagement.findByFilterOrderByLinkedDistributionSet(PAGE, ds.getId(),
new FilterParams(null, Boolean.TRUE, null, null, Boolean.FALSE));
final Comparator<TenantAwareBaseEntity> byId = Comparator.comparingLong(Identifiable::getId);
assertThat(result.getNumberOfElements()).isEqualTo(9);
final List<Target> expected = new ArrayList<>();
expected.addAll(targInstalled.stream().sorted(byId)
.filter(item -> lastTargetQueryAlwaysOverdue.equals(item.getLastTargetQuery()))
.toList());
expected.addAll(targAssigned.stream().sorted(byId)
.filter(item -> lastTargetQueryAlwaysOverdue.equals(item.getLastTargetQuery()))
.toList());
expected.addAll(notAssigned.stream().sorted(byId)
.filter(item -> lastTargetQueryAlwaysOverdue.equals(item.getLastTargetQuery()))
.toList());
assertThat(result.getContent()).usingElementComparator(controllerIdComparator())
.containsExactly(expected.toArray(new Target[0]));
}
@Test
@Description("Verifies that targets with given assigned DS are returned from repository.")
void findTargetByAssignedDistributionSet() {
@@ -336,8 +197,7 @@ class TargetManagementSearchTest extends AbstractJpaIntegrationTest {
assignDistributionSet(assignedSet, assignedtargets);
// get final updated version of targets
assignedtargets = targetManagement
.getByControllerID(assignedtargets.stream().map(Target::getControllerId).collect(Collectors.toList()));
assignedtargets = targetManagement.getByControllerID(assignedtargets.stream().map(Target::getControllerId).toList());
assertThat(targetManagement.findByAssignedDistributionSet(PAGE, assignedSet.getId()))
.as("Contains the assigned targets").containsAll(assignedtargets)
@@ -372,13 +232,12 @@ class TargetManagementSearchTest extends AbstractJpaIntegrationTest {
List<Target> installedtargets = testdataFactory.createTargets(10, "assigned", "assigned");
// set on installed and assign another one
assignDistributionSet(installedSet, installedtargets).getAssignedEntity().forEach(action -> controllerManagement
.addUpdateActionStatus(entityFactory.actionStatus().create(action.getId()).status(Status.FINISHED)));
assignDistributionSet(installedSet, installedtargets).getAssignedEntity().forEach(action ->
controllerManagement.addUpdateActionStatus(entityFactory.actionStatus().create(action.getId()).status(Status.FINISHED)));
assignDistributionSet(assignedSet, installedtargets);
// get final updated version of targets
installedtargets = targetManagement
.getByControllerID(installedtargets.stream().map(Target::getControllerId).collect(Collectors.toList()));
installedtargets = targetManagement.getByControllerID(installedtargets.stream().map(Target::getControllerId).toList());
assertThat(targetManagement.findByInstalledDistributionSet(PAGE, installedSet.getId()))
.as("Contains the assigned targets").containsAll(installedtargets)
@@ -450,7 +309,7 @@ class TargetManagementSearchTest extends AbstractJpaIntegrationTest {
}
@Step
private void verifyThat200targetsWithGivenTagAreInStatusPendingorUnknown(final TargetTag targTagW,
private void verifyThat200targetsWithGivenTagAreInStatusPendingOrUnknown(final TargetTag targTagW,
final List<TargetUpdateStatus> both, final List<Target> expected) {
final FilterParams filterParams = new FilterParams(both, null, null, null, Boolean.FALSE, targTagW.getName());
final String query = "(updatestatus==pending or updatestatus==unknown) and tag==" + targTagW.getName();
@@ -591,8 +450,10 @@ class TargetManagementSearchTest extends AbstractJpaIntegrationTest {
final String query = "updatestatus==unknown and (assignedds.name==" + setA.getName() + " or installedds.name=="
+ setA.getName() + ")";
assertThat(targetManagement.findByFilters(PAGE, filterParams).getContent()).as("has number of elements")
.hasSize(0).as("that number is also returned by count query")
assertThat(targetManagement.findByFilters(PAGE, filterParams).getContent())
.as("has number of elements")
.hasSize(0)
.as("that number is also returned by count query")
.hasSize((int) targetManagement.countByFilters(filterParams))
.as("and filter query returns the same result")
.hasSize(targetManagement.findByRsql(PAGE, query).getContent().size());
@@ -799,8 +660,8 @@ class TargetManagementSearchTest extends AbstractJpaIntegrationTest {
// Comparing the controller ids, as one of the targets was modified, so
// a 1:1
// comparison of the objects is not possible
assertThat(filteredTargets.stream().map(Target::getControllerId).collect(Collectors.toList()))
.containsAll(expected.stream().map(Target::getControllerId).collect(Collectors.toList()));
assertThat(filteredTargets.stream().map(Target::getControllerId).toList())
.containsAll(expected.stream().map(Target::getControllerId).toList());
}
@Step

View File

@@ -288,7 +288,7 @@ public abstract class AbstractIntegrationTest {
}
protected static Comparator<Target> controllerIdComparator() {
return (o1, o2) -> o1.getControllerId().equals(o2.getControllerId()) ? 0 : 1;
return Comparator.comparing(Target::getControllerId);
}
protected DistributionSetAssignmentResult assignDistributionSet(final long dsID, final String controllerId) {