Fix target group assignment with RSQL negation filters (e.g. tag!=tag… (#3116)
* Fix target group assignment with RSQL negation filters (e.g. tag!=tag1) failing on EclipseLink/MySQL Signed-off-by: strailov <Stanislav.Trailov@bosch.io> * address comments Signed-off-by: strailov <Stanislav.Trailov@bosch.io> * Fix test for chunked calls to use also negate operator Signed-off-by: strailov <Stanislav.Trailov@bosch.io> * address comments Signed-off-by: strailov <Stanislav.Trailov@bosch.io> --------- Signed-off-by: strailov <Stanislav.Trailov@bosch.io>
This commit is contained in:
committed by
GitHub
parent
6a85a43dde
commit
95680962cc
@@ -27,6 +27,7 @@ import org.eclipse.hawkbit.repository.model.TargetTag;
|
|||||||
import org.hamcrest.Matchers;
|
import org.hamcrest.Matchers;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.springframework.http.MediaType;
|
import org.springframework.http.MediaType;
|
||||||
|
import org.springframework.test.util.ReflectionTestUtils;
|
||||||
|
|
||||||
public class MgmtTargetGroupResourceTest extends AbstractManagementApiIntegrationTest {
|
public class MgmtTargetGroupResourceTest extends AbstractManagementApiIntegrationTest {
|
||||||
|
|
||||||
@@ -257,4 +258,40 @@ public class MgmtTargetGroupResourceTest extends AbstractManagementApiIntegratio
|
|||||||
.andExpect(jsonPath("content", Matchers.hasSize(1)))
|
.andExpect(jsonPath("content", Matchers.hasSize(1)))
|
||||||
.andExpect(jsonPath("content.[0].controllerId", Matchers.equalTo("target3")));
|
.andExpect(jsonPath("content.[0].controllerId", Matchers.equalTo("target3")));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void shouldAssignGroupInChunksWhenTargetCountExceedsChunkSize() throws Exception {
|
||||||
|
// create 5 targets with tag "exclude", 2 without — chunk size 2 forces multiple batches
|
||||||
|
final TargetTag excludeTag = targetTagManagement.create(TargetTagManagement.Create.builder().name("exclude").build());
|
||||||
|
for (int i = 1; i <= 5; i++) {
|
||||||
|
targetManagement.create(builder().controllerId("chunked-" + i).build());
|
||||||
|
}
|
||||||
|
targetManagement.create(builder().controllerId("excluded-1").build());
|
||||||
|
targetManagement.create(builder().controllerId("excluded-2").build());
|
||||||
|
targetManagement.assignTag(List.of("excluded-1", "excluded-2"), excludeTag.getId());
|
||||||
|
|
||||||
|
// override chunk size to 2 for this test
|
||||||
|
ReflectionTestUtils.setField(targetManagement, "assignTargetGroupChunkSize", 2);
|
||||||
|
try {
|
||||||
|
// tag!=exclude triggers chunked path and should assign only non-tagged targets
|
||||||
|
mvc.perform(put(MgmtTargetGroupRestApi.TARGETGROUPS_V1 + "/ChunkedGroup")
|
||||||
|
.contentType(MediaType.APPLICATION_JSON)
|
||||||
|
.param("q", "tag!=exclude"))
|
||||||
|
.andExpect(status().isNoContent());
|
||||||
|
|
||||||
|
mvc.perform(get(MgmtTargetGroupRestApi.TARGETGROUPS_V1 + "/ChunkedGroup/assigned")
|
||||||
|
.param(MgmtRestConstants.REQUEST_PARAMETER_SORTING, "ID:ASC")
|
||||||
|
.contentType(MediaType.APPLICATION_JSON))
|
||||||
|
.andExpect(status().isOk())
|
||||||
|
.andExpect(jsonPath("content", Matchers.hasSize(5)))
|
||||||
|
.andExpect(jsonPath("content.[0].controllerId", Matchers.equalTo("chunked-1")))
|
||||||
|
.andExpect(jsonPath("content.[1].controllerId", Matchers.equalTo("chunked-2")))
|
||||||
|
.andExpect(jsonPath("content.[2].controllerId", Matchers.equalTo("chunked-3")))
|
||||||
|
.andExpect(jsonPath("content.[3].controllerId", Matchers.equalTo("chunked-4")))
|
||||||
|
.andExpect(jsonPath("content.[4].controllerId", Matchers.equalTo("chunked-5")));
|
||||||
|
} finally {
|
||||||
|
// restore default
|
||||||
|
ReflectionTestUtils.setField(targetManagement, "assignTargetGroupChunkSize", 1000);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -29,12 +29,11 @@ import jakarta.persistence.criteria.CriteriaBuilder;
|
|||||||
import jakarta.persistence.criteria.CriteriaQuery;
|
import jakarta.persistence.criteria.CriteriaQuery;
|
||||||
import jakarta.persistence.criteria.CriteriaUpdate;
|
import jakarta.persistence.criteria.CriteriaUpdate;
|
||||||
import jakarta.persistence.criteria.MapJoin;
|
import jakarta.persistence.criteria.MapJoin;
|
||||||
import jakarta.persistence.criteria.Predicate;
|
|
||||||
import jakarta.persistence.criteria.Root;
|
import jakarta.persistence.criteria.Root;
|
||||||
import jakarta.persistence.criteria.Subquery;
|
|
||||||
import jakarta.persistence.metamodel.MapAttribute;
|
import jakarta.persistence.metamodel.MapAttribute;
|
||||||
import jakarta.validation.constraints.NotEmpty;
|
import jakarta.validation.constraints.NotEmpty;
|
||||||
|
|
||||||
|
import lombok.extern.slf4j.Slf4j;
|
||||||
import org.eclipse.hawkbit.ql.jpa.QLSupport;
|
import org.eclipse.hawkbit.ql.jpa.QLSupport;
|
||||||
import org.eclipse.hawkbit.repository.QuotaManagement;
|
import org.eclipse.hawkbit.repository.QuotaManagement;
|
||||||
import org.eclipse.hawkbit.repository.TargetManagement;
|
import org.eclipse.hawkbit.repository.TargetManagement;
|
||||||
@@ -58,6 +57,7 @@ import org.eclipse.hawkbit.repository.model.DistributionSet;
|
|||||||
import org.eclipse.hawkbit.repository.model.Target;
|
import org.eclipse.hawkbit.repository.model.Target;
|
||||||
import org.eclipse.hawkbit.repository.model.TargetTag;
|
import org.eclipse.hawkbit.repository.model.TargetTag;
|
||||||
import org.eclipse.hawkbit.repository.qfields.TargetFields;
|
import org.eclipse.hawkbit.repository.qfields.TargetFields;
|
||||||
|
import org.springframework.beans.factory.annotation.Value;
|
||||||
import org.springframework.boot.autoconfigure.condition.ConditionalOnBooleanProperty;
|
import org.springframework.boot.autoconfigure.condition.ConditionalOnBooleanProperty;
|
||||||
import org.springframework.dao.ConcurrencyFailureException;
|
import org.springframework.dao.ConcurrencyFailureException;
|
||||||
import org.springframework.data.domain.Page;
|
import org.springframework.data.domain.Page;
|
||||||
@@ -72,6 +72,7 @@ import org.springframework.validation.annotation.Validated;
|
|||||||
/**
|
/**
|
||||||
* JPA implementation of {@link TargetManagement}.
|
* JPA implementation of {@link TargetManagement}.
|
||||||
*/
|
*/
|
||||||
|
@Slf4j
|
||||||
@Transactional(readOnly = true)
|
@Transactional(readOnly = true)
|
||||||
@Validated
|
@Validated
|
||||||
@Service
|
@Service
|
||||||
@@ -85,6 +86,9 @@ public class JpaTargetManagement
|
|||||||
private final TargetTypeRepository targetTypeRepository;
|
private final TargetTypeRepository targetTypeRepository;
|
||||||
private final TargetTagRepository targetTagRepository;
|
private final TargetTagRepository targetTagRepository;
|
||||||
|
|
||||||
|
@Value("${hawkbit.target-group.assign.chunk-size:1000}")
|
||||||
|
private int assignTargetGroupChunkSize;
|
||||||
|
|
||||||
@SuppressWarnings("java:S107")
|
@SuppressWarnings("java:S107")
|
||||||
protected JpaTargetManagement(
|
protected JpaTargetManagement(
|
||||||
final TargetRepository jpaRepository, final EntityManager entityManager,
|
final TargetRepository jpaRepository, final EntityManager entityManager,
|
||||||
@@ -335,30 +339,62 @@ public class JpaTargetManagement
|
|||||||
@Transactional
|
@Transactional
|
||||||
@Retryable(includes = ConcurrencyFailureException.class, maxRetriesString = Constants.RETRY_MAX, delayString = Constants.RETRY_DELAY)
|
@Retryable(includes = ConcurrencyFailureException.class, maxRetriesString = Constants.RETRY_MAX, delayString = Constants.RETRY_DELAY)
|
||||||
public void assignTargetGroupWithRsql(String group, String rsql) {
|
public void assignTargetGroupWithRsql(String group, String rsql) {
|
||||||
final Specification<JpaTarget> rsqlSpecification = QLSupport.getInstance().buildSpec(rsql, TargetFields.class);
|
|
||||||
|
|
||||||
final CriteriaBuilder cb = entityManager.getCriteriaBuilder();
|
// Switch back to UpdateAllQuery if switching back to hibernate. (EclipseLink does not work well with UpdateAllQuery)
|
||||||
final CriteriaUpdate<JpaTarget> criteriaUpdateQuery = cb.createCriteriaUpdate(JpaTarget.class);
|
// EclipseLink: using subquery approach — applying predicate directly to the UPDATE root
|
||||||
final Root<JpaTarget> updateRoot = criteriaUpdateQuery.getRoot();
|
// fails for NOT EXISTS due to UpdateAllQuery's @Id resolution bug
|
||||||
criteriaUpdateQuery.set("group", group);
|
// BUG Reported: https://github.com/eclipse-ee4j/eclipselink/issues/2757
|
||||||
|
// Hibernate: applies predicate directly to the UPDATE root — Hibernate handles
|
||||||
if (Jpa.JPA_VENDOR == Jpa.JpaVendor.ECLIPSELINK) {
|
// NOT EXISTS subqueries correctly in CriteriaUpdate context. So this problem does not exist there.
|
||||||
// EclipseLink: use subquery approach — applying predicate directly to the UPDATE root
|
if (Jpa.JPA_VENDOR == Jpa.JpaVendor.ECLIPSELINK && containsNegation(rsql)) {
|
||||||
// fails for NOT EXISTS due to UpdateAllQuery's @Id resolution bug
|
log.debug("Assigning group {} with rsql {} on chunks.", group, rsql);
|
||||||
// BUG Reported: https://github.com/eclipse-ee4j/eclipselink/issues/2757
|
assignTargetGroupOnChunks(group, rsql);
|
||||||
final Subquery<Long> subquery = criteriaUpdateQuery.subquery(Long.class);
|
|
||||||
final Root<JpaTarget> subRoot = subquery.from(JpaTarget.class);
|
|
||||||
subquery.select(subRoot.get(AbstractJpaBaseEntity_.ID));
|
|
||||||
subquery.where(rsqlSpecification.toPredicate(subRoot, cb.createQuery(JpaTarget.class), cb));
|
|
||||||
criteriaUpdateQuery.where(updateRoot.get(AbstractJpaBaseEntity_.ID).in(subquery));
|
|
||||||
} else {
|
} else {
|
||||||
// Hibernate: apply predicate directly to the UPDATE root — Hibernate handles
|
log.debug("Assigning group {} with rsql {} with batch update", group, rsql);
|
||||||
// NOT EXISTS subqueries correctly in CriteriaUpdate context
|
assignTargetGroupDirect(group, rsql);
|
||||||
final Predicate predicate = rsqlSpecification.toPredicate(updateRoot, cb.createQuery(JpaTarget.class), cb);
|
|
||||||
criteriaUpdateQuery.where(predicate);
|
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
entityManager.createQuery(criteriaUpdateQuery).executeUpdate();
|
private static boolean containsNegation(final String rsql) {
|
||||||
|
return rsql.contains("!=") || rsql.contains("=out=") || rsql.contains("=notlike=");
|
||||||
|
}
|
||||||
|
|
||||||
|
private void assignTargetGroupDirect(final String group, final String rsql) {
|
||||||
|
final Specification<JpaTarget> rsqlSpecification = QLSupport.getInstance().buildSpec(rsql, TargetFields.class);
|
||||||
|
final CriteriaBuilder cb = entityManager.getCriteriaBuilder();
|
||||||
|
final CriteriaUpdate<JpaTarget> update = cb.createCriteriaUpdate(JpaTarget.class);
|
||||||
|
final Root<JpaTarget> root = update.getRoot();
|
||||||
|
update.set("group", group);
|
||||||
|
update.where(rsqlSpecification.toPredicate(root, cb.createQuery(JpaTarget.class), cb));
|
||||||
|
entityManager.createQuery(update).executeUpdate();
|
||||||
|
}
|
||||||
|
|
||||||
|
private void assignTargetGroupOnChunks(final String group, final String rsql) {
|
||||||
|
final Specification<JpaTarget> spec = QLSupport.getInstance().buildSpec(rsql, TargetFields.class);
|
||||||
|
final CriteriaBuilder cb = entityManager.getCriteriaBuilder();
|
||||||
|
// SELECT Target IDs
|
||||||
|
final CriteriaQuery<Long> select = cb.createQuery(Long.class);
|
||||||
|
final Root<JpaTarget> root = select.from(JpaTarget.class);
|
||||||
|
select.select(root.get(AbstractJpaBaseEntity_.ID));
|
||||||
|
select.where(spec.toPredicate(root, select, cb));
|
||||||
|
|
||||||
|
List<Long> chunk;
|
||||||
|
int offset = 0;
|
||||||
|
do {
|
||||||
|
chunk = entityManager.createQuery(select)
|
||||||
|
.setFirstResult(offset)
|
||||||
|
.setMaxResults(assignTargetGroupChunkSize)
|
||||||
|
.getResultList();
|
||||||
|
|
||||||
|
if (chunk.isEmpty()) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
final CriteriaUpdate<JpaTarget> update = cb.createCriteriaUpdate(JpaTarget.class);
|
||||||
|
update.set("group", group);
|
||||||
|
update.where(update.getRoot().get(AbstractJpaBaseEntity_.ID).in(chunk));
|
||||||
|
entityManager.createQuery(update).executeUpdate();
|
||||||
|
offset += chunk.size();
|
||||||
|
} while (true);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
|||||||
Reference in New Issue
Block a user