fixed compatibility check bug (#1198)

* fixed compatibility check bug & improved testing

Signed-off-by: Robert Sing <robert.sing@bosch-si.com>

* fixed review finding

Signed-off-by: Robert Sing <robert.sing@bosch-si.com>

* reduced amount of targets in test

Signed-off-by: Robert Sing <robert.sing@bosch-si.com>
This commit is contained in:
Robert Sing
2021-10-29 14:15:16 +02:00
committed by GitHub
parent ae4fb5fe00
commit f6c39a3589
2 changed files with 47 additions and 16 deletions

View File

@@ -22,6 +22,7 @@ import javax.persistence.criteria.Path;
import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Predicate;
import javax.persistence.criteria.Root; import javax.persistence.criteria.Root;
import javax.persistence.criteria.SetJoin; import javax.persistence.criteria.SetJoin;
import javax.persistence.criteria.Subquery;
import javax.validation.constraints.NotNull; import javax.validation.constraints.NotNull;
import org.eclipse.hawkbit.repository.jpa.model.JpaAction; import org.eclipse.hawkbit.repository.jpa.model.JpaAction;
@@ -418,11 +419,14 @@ public final class TargetSpecifications {
// isNotNull predicate first // isNotNull predicate first
final Predicate targetTypeNotNull = targetRoot.get(JpaTarget_.targetType).isNotNull(); final Predicate targetTypeNotNull = targetRoot.get(JpaTarget_.targetType).isNotNull();
// We need to check for isNull(...) and notEqual(...) since we allow final Subquery<Long> compatibilitySubQuery = query.subquery(Long.class);
// target types that don't have any compatible distribution set type final Root<JpaTarget> subQueryTargetRoot = compatibilitySubQuery.from(JpaTarget.class);
return cb.and(targetTypeNotNull,
cb.or(cb.isNull(getDsTypeIdPath(targetRoot)), compatibilitySubQuery.select(subQueryTargetRoot.get(JpaTarget_.id))
cb.notEqual(getDsTypeIdPath(targetRoot), distributionSetTypeId))); .where(cb.and(cb.equal(targetRoot.get(JpaTarget_.id), subQueryTargetRoot.get(JpaTarget_.id)),
cb.equal(getDsTypeIdPath(subQueryTargetRoot), distributionSetTypeId)));
return cb.and(targetTypeNotNull, cb.not(cb.exists(compatibilitySubQuery)));
}; };
} }
@@ -586,7 +590,7 @@ public final class TargetSpecifications {
* distribution set to consider * distribution set to consider
* @return specification that applies order by ds, may be overwritten * @return specification that applies order by ds, may be overwritten
*/ */
public static Specification<JpaTarget> orderedByLinkedDistributionSet(long distributionSetIdForOrder) { public static Specification<JpaTarget> orderedByLinkedDistributionSet(final long distributionSetIdForOrder) {
return (targetRoot, query, cb) -> { return (targetRoot, query, cb) -> {
// Enhance query with custom select based sort // Enhance query with custom select based sort
final Expression<Object> selectCase = cb.selectCase() final Expression<Object> selectCase = cb.selectCase()

View File

@@ -11,10 +11,13 @@ package org.eclipse.hawkbit.repository.jpa.autoassign;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.eclipse.hawkbit.repository.exception.IncompleteDistributionSetException; import org.eclipse.hawkbit.repository.exception.IncompleteDistributionSetException;
import org.eclipse.hawkbit.repository.jpa.AbstractJpaIntegrationTest; import org.eclipse.hawkbit.repository.jpa.AbstractJpaIntegrationTest;
@@ -24,6 +27,7 @@ import org.eclipse.hawkbit.repository.model.Action.ActionType;
import org.eclipse.hawkbit.repository.model.Action.Status; import org.eclipse.hawkbit.repository.model.Action.Status;
import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.DistributionSet;
import org.eclipse.hawkbit.repository.model.DistributionSetAssignmentResult; import org.eclipse.hawkbit.repository.model.DistributionSetAssignmentResult;
import org.eclipse.hawkbit.repository.model.DistributionSetType;
import org.eclipse.hawkbit.repository.model.Target; import org.eclipse.hawkbit.repository.model.Target;
import org.eclipse.hawkbit.repository.model.TargetFilterQuery; import org.eclipse.hawkbit.repository.model.TargetFilterQuery;
import org.eclipse.hawkbit.repository.model.TargetType; import org.eclipse.hawkbit.repository.model.TargetType;
@@ -319,28 +323,51 @@ class AutoAssignCheckerTest extends AbstractJpaIntegrationTest {
@Test @Test
@Description("Verifies an auto assignment only creates actions for compatible targets") @Description("Verifies an auto assignment only creates actions for compatible targets")
void checkAutoAssignmentWithIncompatibleTargets() throws Exception { void checkAutoAssignmentWithIncompatibleTargets() {
final int TARGET_COUNT = 5;
final DistributionSet testDs = testdataFactory.createDistributionSet(); final DistributionSet testDs = testdataFactory.createDistributionSet();
final DistributionSetType incompatibleDsType1 = testdataFactory
.findOrCreateDistributionSetType("incompatibleDsType1", "incompDsType1");
final DistributionSetType incompatibleDsType2 = testdataFactory
.findOrCreateDistributionSetType("incompatibleDsType2", "incompDsType2");
final TargetFilterQuery testFilter = targetFilterQueryManagement.create(entityFactory.targetFilterQuery() final TargetFilterQuery testFilter = targetFilterQueryManagement.create(entityFactory.targetFilterQuery()
.create().name("test-filter").query("name==*").autoAssignDistributionSet(testDs)); .create().name("test-filter").query("name==*").autoAssignDistributionSet(testDs));
final TargetType incompatibleType = testdataFactory.createTargetType("incompatibleType", final TargetType incompatibleEmptyType = testdataFactory.createTargetType("incompatibleEmptyType",
Collections.emptyList()); Collections.emptyList());
final TargetType compatibleType = testdataFactory.createTargetType("compatibleType", final TargetType incompatibleSingleType = testdataFactory.createTargetType("incompatibleSingleType",
Collections.singletonList(incompatibleDsType1));
final TargetType incompatibleMultiType = testdataFactory.createTargetType("incompatibleMultiType",
Arrays.asList(incompatibleDsType1, incompatibleDsType2));
final TargetType compatibleSingleType = testdataFactory.createTargetType("compatibleSingleType",
Collections.singletonList(testDs.getType())); Collections.singletonList(testDs.getType()));
testdataFactory.createTargetsWithType(10, "incompatible", incompatibleType); final TargetType compatibleMultiType = testdataFactory.createTargetType("compatibleMultiType",
final Target compatibleTarget = testdataFactory.createTargetsWithType(1, "compatible", compatibleType).get(0); Arrays.asList(testDs.getType(), incompatibleDsType1));
final Target targetWithoutType = testdataFactory.createTarget();
testdataFactory.createTargetsWithType(TARGET_COUNT, "incompatibleEmpty", incompatibleEmptyType);
testdataFactory.createTargetsWithType(TARGET_COUNT, "incompatibleSingle", incompatibleSingleType);
testdataFactory.createTargetsWithType(TARGET_COUNT, "incompatibleMulti", incompatibleMultiType);
final List<Target> compatibleTargetsSingleType = testdataFactory.createTargetsWithType(TARGET_COUNT,
"compatibleSingle", compatibleSingleType);
final List<Target> compatibleTargetsMultiType = testdataFactory.createTargetsWithType(TARGET_COUNT,
"compatibleMulti", compatibleMultiType);
final List<Target> compatibleTargetsWithoutType = testdataFactory.createTargets(TARGET_COUNT,
"compatibleSingleWithoutType");
final List<Long> compatibleTargets = Stream
.of(compatibleTargetsSingleType, compatibleTargetsMultiType, compatibleTargetsWithoutType)
.flatMap(Collection::stream).map(Target::getId).collect(Collectors.toList());
final long compatibleCount = targetManagement.countByRsqlAndNonDSAndCompatible(testDs.getId(), final long compatibleCount = targetManagement.countByRsqlAndNonDSAndCompatible(testDs.getId(),
testFilter.getQuery()); testFilter.getQuery());
assertThat(compatibleCount).isEqualTo(2); assertThat(compatibleCount).isEqualTo(compatibleTargets.size());
autoAssignChecker.check(); autoAssignChecker.check();
final List<Action> actions = deploymentManagement.findActionsAll(PAGE).getContent(); final List<Action> actions = deploymentManagement.findActionsAll(Pageable.unpaged()).getContent();
assertThat(actions).hasSize(2); assertThat(actions).hasSize(compatibleTargets.size());
final List<Long> actionTargets = actions.stream().map(a -> a.getTarget().getId()).collect(Collectors.toList()); final List<Long> actionTargets = actions.stream().map(a -> a.getTarget().getId()).collect(Collectors.toList());
assertThat(actionTargets).containsExactlyInAnyOrder(compatibleTarget.getId(), targetWithoutType.getId()); assertThat(actionTargets).containsExactlyInAnyOrderElementsOf(compatibleTargets);
} }
} }