From e7765bf4d25b501c7f25f13ad2d43debb2f13b46 Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Tue, 23 Sep 2025 17:15:48 +0300 Subject: [PATCH] Add suppor for deeper RSQL search (#2682) Signed-off-by: Avgustin Marinov --- .../repository/jpa/ql/EntityMatcher.java | 89 +++++++++++++------ .../ql/SpecificationBuilderLegacyTest.java | 6 ++ .../jpa/ql/SpecificationBuilderTest.java | 84 +++++++++++++++++ .../hawkbit/repository/jpa/ql/SubSub.java | 4 + .../DefaultAccessControllerConfiguration.java | 48 +--------- 5 files changed, 159 insertions(+), 72 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-jpa-ql/src/main/java/org/eclipse/hawkbit/repository/jpa/ql/EntityMatcher.java b/hawkbit-repository/hawkbit-repository-jpa-ql/src/main/java/org/eclipse/hawkbit/repository/jpa/ql/EntityMatcher.java index 758774ef1..41f87d877 100644 --- a/hawkbit-repository/hawkbit-repository-jpa-ql/src/main/java/org/eclipse/hawkbit/repository/jpa/ql/EntityMatcher.java +++ b/hawkbit-repository/hawkbit-repository-jpa-ql/src/main/java/org/eclipse/hawkbit/repository/jpa/ql/EntityMatcher.java @@ -23,11 +23,12 @@ import static org.eclipse.hawkbit.repository.jpa.ql.Node.Comparison.Operator.NOT import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; import java.util.Arrays; import java.util.Collection; import java.util.Map; import java.util.Objects; -import java.util.function.BiFunction; +import java.util.function.BiPredicate; import org.eclipse.hawkbit.repository.jpa.ql.Node.Comparison.Operator; import org.eclipse.hawkbit.repository.jpa.rsql.RsqlParser; @@ -55,13 +56,13 @@ public class EntityMatcher { return match(t, root); } + @SuppressWarnings({"java:S3776", "java:S3358", "java:S1125", "java:S6541"}) // better readable this way private static boolean match(final T t, final Node node) { if (node instanceof Node.Comparison comparison) { final String[] split = comparison.getKey().split("\\.", 2); try { - final Method fieldGetter = getGetter(t.getClass(), split[0]); - fieldGetter.setAccessible(true); - final Object fieldValue = fieldGetter.invoke(t); + final Getter fieldGetter = getGetter(t.getClass(), split[0]); + final Object fieldValue = fieldGetter.get(t); final Operator op = comparison.getOp(); if (Map.class.isAssignableFrom(getReturnType(fieldGetter))) { if ((op == NE || op == NOT_IN || op == NOT_LIKE) @@ -74,20 +75,20 @@ public class EntityMatcher { op, map( comparison.getValue(), - (Class) ((ParameterizedType) fieldGetter.getGenericReturnType()).getActualTypeArguments()[1])); + (Class) ((ParameterizedType) fieldGetter.type()).getActualTypeArguments()[1])); } else if (Collection.class.isAssignableFrom(getReturnType(fieldGetter))) { // Set / List final Object value; - final BiFunction compare; + final BiPredicate compare; if (split.length == 1) { value = map(comparison.getValue(), getReturnType(fieldGetter)); compare = (e, operator) -> compare(e, operator, value); } else { - final Method valueGetter = getGetter( - (Class) ((ParameterizedType) fieldGetter.getGenericReturnType()).getActualTypeArguments()[0], split[1]); + final Getter valueGetter = getGetter( + (Class) ((ParameterizedType) fieldGetter.type()).getActualTypeArguments()[0], split[1]); value = map(comparison.getValue(), getReturnType(valueGetter)); compare = (e, operator) -> { try { - return compare(map(e == null ? null : valueGetter.invoke(e), getReturnType(valueGetter)), operator, value); + return compare(map(e == null ? null : valueGetter.get(e), getReturnType(valueGetter)), operator, value); } catch (final IllegalAccessException | InvocationTargetException ex) { throw new IllegalArgumentException(ex); } @@ -97,10 +98,10 @@ public class EntityMatcher { return switch (op) { case EQ, GT, GTE, LT, LTE, IN, LIKE -> set == null ? false - : set.stream().anyMatch(e -> compare.apply(e, op)); + : set.stream().anyMatch(e -> compare.test(e, op)); case NE, NOT_IN, NOT_LIKE -> set == null ? true - : set.stream().noneMatch(e -> compare.apply(e, op == NE ? EQ : op == NOT_IN ? IN : LIKE)); + : set.stream().noneMatch(e -> compare.test(e, op == NE ? EQ : op == NOT_IN ? IN : LIKE)); }; } else { if (split.length == 1) { @@ -109,17 +110,18 @@ public class EntityMatcher { if (split[1].contains(".")) { // nested field access final String[] nestedSplit = split[1].split("\\.", 2); - final Method nestedFieldGetter = getGetter(getReturnType(fieldGetter), nestedSplit[0]); - nestedFieldGetter.setAccessible(true); - final Method valueGetter = getGetter(getReturnType(nestedFieldGetter), nestedSplit[1]); - final Object nestedFieldValue = fieldValue == null ? null : nestedFieldGetter.invoke(fieldValue); + final Getter nestedFieldGetter = getGetter(getReturnType(fieldGetter), nestedSplit[0]); + final Getter valueGetter = getGetter(getReturnType(nestedFieldGetter), nestedSplit[1]); + final Object nestedFieldValue = fieldValue == null ? null : nestedFieldGetter.get(fieldValue); return compare( - nestedFieldValue == null ? null : valueGetter.invoke(nestedFieldValue), + nestedFieldValue == null ? null : valueGetter.get(nestedFieldValue), op, map(comparison.getValue(), getReturnType(valueGetter))); } else { - final Method valueGetter = getGetter(getReturnType(fieldGetter), split[1]); - return compare(fieldValue == null ? null : valueGetter.invoke(fieldValue), op, + final Getter valueGetter = getGetter(getReturnType(fieldGetter), split[1]); + return compare( + fieldValue == null ? null : valueGetter.get(fieldValue), + op, map(comparison.getValue(), getReturnType(valueGetter))); } } @@ -137,7 +139,26 @@ public class EntityMatcher { } } - private static Method getGetter(final Class t, final String fieldName) throws NoSuchMethodException { + @SuppressWarnings("java:S3011") // java:S3011 uses reflection to private members antway + private static Getter getGetter(final Class t, final String fieldName) throws NoSuchMethodException { + final String[] parts = fieldName.split("\\."); + if (parts.length > 1) { + final Getter firstGetter = getGetter(t, parts[0]); + final Getter nextGetter = getGetter(t, fieldName.substring(parts[0].length() + 1)); + return new Getter() { + + @Override + public Object get(final Object obj) throws IllegalAccessException, InvocationTargetException { + return nextGetter.get(firstGetter.get(obj)); + } + + @Override + public Type type() { + return nextGetter.type(); + } + }; + } + final String getterLowercase = "get" + fieldName.toLowerCase(); return Arrays.stream(t.getMethods()) .filter(method -> getterLowercase.equals(method.getName().toLowerCase())) @@ -145,22 +166,33 @@ public class EntityMatcher { .map(Method::getName) .map(getterName -> { try { - // gets method via Class.getMethod(String, Class...) because in listing it might has no the - // correct return type, but the type got from a declaring generic type + // gets method via Class.getMethod(String, Class...) because in listing it might have not + // the correct return type, but the type got from a declaring generic type final Method getter = t.getMethod(getterName); getter.setAccessible(true); - return getter; + return new Getter() { + + @Override + public Object get(final Object obj) throws IllegalAccessException, InvocationTargetException { + return getter.invoke(obj); + } + + @Override + public Type type() { + return getter.getGenericReturnType(); + } + }; } catch (final NoSuchMethodException e) { throw new IllegalStateException("Unexpected: No getter found for field: " + fieldName + " in class: " + t.getName(), e); } }).orElseThrow(() -> new NoSuchMethodException("No getter found for field: " + fieldName + " in class: " + t.getName())); } - private static Class getReturnType(final Method valueGetter) { - return valueGetter.getReturnType(); + private static Class getReturnType(final Getter getter) { + return getter.type() instanceof Class clazz ? clazz : (Class) ((ParameterizedType) getter.type()).getRawType(); } - @SuppressWarnings({ "unchecked", "rawtypes" }) + @SuppressWarnings({ "unchecked", "rawtypes", "java:S3776" }) // java:S3776 - better readable this way private static Object map(final Object value, final Class type) { if (value instanceof Collection collection) { // in / out return collection.stream().map(e -> map(e, type)).toList(); @@ -243,4 +275,11 @@ public class EntityMatcher { throw new IllegalArgumentException("LIKE pattern must be String. Found: " + (pattern == null ? null : pattern.getClass())); } } + + private interface Getter { + + Object get(Object obj) throws IllegalAccessException, InvocationTargetException; + + Type type(); + } } \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa-ql/src/test/java/org/eclipse/hawkbit/repository/jpa/ql/SpecificationBuilderLegacyTest.java b/hawkbit-repository/hawkbit-repository-jpa-ql/src/test/java/org/eclipse/hawkbit/repository/jpa/ql/SpecificationBuilderLegacyTest.java index 2dbf27cb6..71a9daeac 100644 --- a/hawkbit-repository/hawkbit-repository-jpa-ql/src/test/java/org/eclipse/hawkbit/repository/jpa/ql/SpecificationBuilderLegacyTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa-ql/src/test/java/org/eclipse/hawkbit/repository/jpa/ql/SpecificationBuilderLegacyTest.java @@ -98,6 +98,12 @@ class SpecificationBuilderLegacyTest extends SpecificationBuilderTest { runWithRsqlToSpecBuilder(super::singularEntitySubSubAttribute, LEGACY_G2); } + @Override + @Test + void deapSearchSubSubSubSubAttribute() { + // legacy builders doesn't support deep search + } + @Override protected Specification getSpecification(final String rsql) { return builder.specification(rsql); diff --git a/hawkbit-repository/hawkbit-repository-jpa-ql/src/test/java/org/eclipse/hawkbit/repository/jpa/ql/SpecificationBuilderTest.java b/hawkbit-repository/hawkbit-repository-jpa-ql/src/test/java/org/eclipse/hawkbit/repository/jpa/ql/SpecificationBuilderTest.java index d416e6778..be2373c02 100644 --- a/hawkbit-repository/hawkbit-repository-jpa-ql/src/test/java/org/eclipse/hawkbit/repository/jpa/ql/SpecificationBuilderTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa-ql/src/test/java/org/eclipse/hawkbit/repository/jpa/ql/SpecificationBuilderTest.java @@ -467,6 +467,90 @@ class SpecificationBuilderTest { } } + @Test + void deapSearchSubSubSubSubAttribute() { + final SubSub subSubSubSub1 = subSubRepository.save(new SubSub().setStrValue("subx").setIntValue(0)); + final SubSub subSubSubSub2 = subSubRepository.save(new SubSub().setStrValue("suby").setIntValue(1)); + final SubSub subSub1 = subSubRepository.save(new SubSub().setStrValue("subx").setIntValue(0).setSubSub(subSubSubSub1)); + final SubSub subSub2 = subSubRepository.save(new SubSub().setStrValue("suby").setIntValue(1).setSubSub(subSubSubSub2)); + final Sub sub1 = subRepository.save(new Sub().setSubSub(subSub1)); + final Sub sub2 = subRepository.save(new Sub().setSubSub(subSub2)); + final Root root1 = rootRepository.save(new Root().setSubEntity(sub1)); + final Root root2 = rootRepository.save(new Root().setSubEntity(sub1)); + final Root root3 = rootRepository.save(new Root().setSubEntity(sub2)); + final Root root4 = rootRepository.save(new Root().setSubEntity(sub2)); + final Root root5 = rootRepository.save(new Root()); // no sub set + + // by sub entity string + assertThat(filter("subEntity.subSub.subSub.strValue==subx")).hasSize(2).containsExactlyInAnyOrder(root1, root2); + assertThat(filter("subEntity.subSub.subSub.strValue==nostr")).isEmpty(); + // TODO / recheck - when missing entity shall it be included or not in != or =out=? - now it is + assertThat(filter("subEntity.subSub.subSub.strValue!=subx")).hasSize(3).containsExactlyInAnyOrder(root3, root4, root5); + assertThat(filter("subEntity.subSub.subSub.strValue!=nostr")).hasSize(5); + assertThat(filter("subEntity.subSub.subSub.strValuesubx")).hasSize(2).containsExactlyInAnyOrder(root3, root4); + assertThat(filter("subEntity.subSub.subSub.strValue>=subx")).hasSize(4).containsExactlyInAnyOrder(root1, root2, root3, root4); + assertThat(filter("subEntity.subSub.subSub.strValue=in=subx")).hasSize(2).containsExactlyInAnyOrder(root1, root2); + assertThat(filter("subEntity.subSub.subSub.strValue=in=(subx, suby)")).hasSize(4).containsExactlyInAnyOrder(root1, root2, root3, root4); + assertThat(filter("subEntity.subSub.subSub.strValue=in=(subZ, subT)")).isEmpty(); + assertThat(filter("subEntity.subSub.subSub.strValue=out=subx")).hasSize(3).containsExactlyInAnyOrder(root3, root4, root5); + assertThat(filter("subEntity.subSub.subSub.strValue=out=(subx, suby)")).hasSize(1).containsExactlyInAnyOrder(root5); + // wildcard, like + assertThat(filter("subEntity.subSub.subSub.strValue==sub*")).hasSize(4).containsExactlyInAnyOrder(root1, root2, root3, root4); + assertThat(filter("subEntity.subSub.subSub.strValue==*bx")).hasSize(2).containsExactlyInAnyOrder(root1, root2); + assertThat(filter("subEntity.subSub.subSub.strValue!=sub*")).hasSize(1).containsExactlyInAnyOrder(root5); + assertThat(filter("subEntity.subSub.subSub.strValue!=*bx")).hasSize(3).containsExactlyInAnyOrder(root3, root4, root5); + assertThat(filter("subEntity.subSub.subSub.strValue==*")).hasSize(4).containsExactlyInAnyOrder(root1, root2, root3, root4); + assertThat(filter("subEntity.subSub.subSub.strValue!=*")).hasSize(1).containsExactlyInAnyOrder(root5); + + assertThat(filter("subEntity.subSub.subSub.strValue=is=null")).hasSize(1).containsExactlyInAnyOrder(root5); + + assertThat(filter("subEntity.subSub.subSub.strValue==*bx and subEntity.subSub.strValue==suby")).isEmpty(); + assertThat(filter("subEntity.subSub.subSub.strValue==*bx and subEntity.subSub.strValue!=subx")).isEmpty(); + assertThat(filter("subEntity.subSub.subSub.strValue==*bx and subEntity.subSub.strValue==subx")) + .hasSize(2).containsExactlyInAnyOrder(root1, root2); + assertThat(filter("subEntity.subSub.subSub.strValue==*bx or subEntity.subSub.strValue==suby")) + .hasSize(4).containsExactlyInAnyOrder(root1, root2, root3, root4); + assertThat(filter("subEntity.subSub.subSub.strValue==*bx or subEntity.subSub.strValue!=subx")) + .hasSize(5).containsExactlyInAnyOrder(root1, root2, root3, root4, root5); + assertThat(filter("subEntity.subSub.subSub.strValue==*bx or subEntity.subSub.strValue=is=null")) + .hasSize(3).containsExactlyInAnyOrder(root1, root2, root5); + + // by sub entity int + assertThat(filter("subEntity.subSub.subSub.intValue==0")).hasSize(2).containsExactlyInAnyOrder(root1, root2); + assertThat(filter("subEntity.subSub.subSub.intValue==2")).isEmpty(); + assertThat(filter("subEntity.subSub.subSub.intValue!=0")).hasSize(3).containsExactlyInAnyOrder(root3, root4, root5); + assertThat(filter("subEntity.subSub.subSub.intValue!=2")).hasSize(5); + assertThat(filter("subEntity.subSub.subSub.intValue<1")).hasSize(2).containsExactlyInAnyOrder(root1, root2); + assertThat(filter("subEntity.subSub.subSub.intValue<=1")).hasSize(4).containsExactlyInAnyOrder(root1, root2, root3, root4); + assertThat(filter("subEntity.subSub.subSub.intValue>0")).hasSize(2).containsExactlyInAnyOrder(root3, root4); + assertThat(filter("subEntity.subSub.subSub.intValue>=0")).hasSize(4); + assertThat(filter("subEntity.subSub.subSub.intValue=in=0")).hasSize(2).containsExactlyInAnyOrder(root1, root2); + assertThat(filter("subEntity.subSub.subSub.intValue=in=(0, 1)")).hasSize(4).containsExactlyInAnyOrder(root1, root2, root3, root4); + assertThat(filter("subEntity.subSub.subSub.intValue=in=(2, 3)")).isEmpty(); + assertThat(filter("subEntity.subSub.subSub.intValue=out=0")).hasSize(3).containsExactlyInAnyOrder(root3, root4, root5); + assertThat(filter("subEntity.subSub.subSub.intValue=out=(0, 1)")).hasSize(1).containsExactlyInAnyOrder(root5); + + assertThat(filter("subEntity.subSub.subSub.intValue==0 and subEntity.subSub.intValue==1")).isEmpty(); + assertThat(filter("subEntity.subSub.subSub.intValue==0 and subEntity.subSub.intValue!=1")).hasSize(2).containsExactlyInAnyOrder(root1, root2); + assertThat(filter("subEntity.subSub.subSub.intValue==0 and subEntity.subSub.intValue==0")).hasSize(2).containsExactlyInAnyOrder(root1, root2); + assertThat(filter("subEntity.subSub.subSub.intValue==0 or subEntity.subSub.intValue==1")).hasSize(4) + .containsExactlyInAnyOrder(root1, root2, root3, root4); + assertThat(filter("subEntity.subSub.subSub.intValue==0 or subEntity.subSub.subSub.intValue!=1")) + .hasSize(3).containsExactlyInAnyOrder(root1, root2, root5); + + assertThat(filter("subEntity.subSub.subSub.strValue==subx and subEntity.subSub.intValue==1")).isEmpty(); + assertThat(filter("subEntity.subSub.subSub.strValue==subx and subEntity.subSub.intValue!=1")).hasSize(2) + .containsExactlyInAnyOrder(root1, root2); + assertThat(filter("subEntity.subSub.subSub.strValue==subx and subEntity.subSub.intValue==0")) + .hasSize(2).containsExactlyInAnyOrder(root1, root2); + assertThat(filter("subEntity.subSub.subSub.strValue==subx or subEntity.subSub.intValue==1")) + .hasSize(4).containsExactlyInAnyOrder(root1, root2, root3, root4); + assertThat(filter("subEntity.subSub.subSub.strValue==subx or subEntity.subSub.intValue!=1")) + .hasSize(3).containsExactlyInAnyOrder(root1, root2, root5); + } + private List filter(final String rsql) { // reference / auto filter (using elements and reflection) final EntityMatcher matcher = EntityMatcher.forRsql(rsql); diff --git a/hawkbit-repository/hawkbit-repository-jpa-ql/src/test/java/org/eclipse/hawkbit/repository/jpa/ql/SubSub.java b/hawkbit-repository/hawkbit-repository-jpa-ql/src/test/java/org/eclipse/hawkbit/repository/jpa/ql/SubSub.java index f9ecd4a72..9eb9c9ef6 100644 --- a/hawkbit-repository/hawkbit-repository-jpa-ql/src/test/java/org/eclipse/hawkbit/repository/jpa/ql/SubSub.java +++ b/hawkbit-repository/hawkbit-repository-jpa-ql/src/test/java/org/eclipse/hawkbit/repository/jpa/ql/SubSub.java @@ -13,6 +13,7 @@ import jakarta.persistence.Entity; import jakarta.persistence.GeneratedValue; import jakarta.persistence.GenerationType; import jakarta.persistence.Id; +import jakarta.persistence.ManyToOne; import lombok.Data; import lombok.experimental.Accessors; @@ -30,4 +31,7 @@ class SubSub { // basic private String strValue; private int intValue; + + @ManyToOne + private SubSub subSub; } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/DefaultAccessControllerConfiguration.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/DefaultAccessControllerConfiguration.java index 06a5f5f5e..7b9784d3a 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/DefaultAccessControllerConfiguration.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/DefaultAccessControllerConfiguration.java @@ -10,7 +10,6 @@ package org.eclipse.hawkbit.repository.jpa.acm; import java.lang.reflect.Proxy; -import java.util.List; import java.util.Optional; import jakarta.persistence.criteria.From; @@ -18,14 +17,11 @@ import jakarta.persistence.criteria.Join; import jakarta.persistence.criteria.Root; import jakarta.persistence.metamodel.EntityType; -import lombok.Getter; import org.eclipse.hawkbit.im.authentication.SpPermission; import org.eclipse.hawkbit.repository.DistributionSetFields; import org.eclipse.hawkbit.repository.DistributionSetTypeFields; -import org.eclipse.hawkbit.repository.RsqlQueryField; import org.eclipse.hawkbit.repository.SoftwareModuleFields; import org.eclipse.hawkbit.repository.SoftwareModuleTypeFields; -import org.eclipse.hawkbit.repository.TagFields; import org.eclipse.hawkbit.repository.TargetFields; import org.eclipse.hawkbit.repository.TargetTypeFields; import org.eclipse.hawkbit.repository.exception.InsufficientPermissionException; @@ -115,46 +111,4 @@ public class DefaultAccessControllerConfiguration { AccessController distributionSetTypeAccessController() { return new DefaultAccessController<>(DistributionSetTypeFields.class, SpPermission.DISTRIBUTION_SET_TYPE); } - - // contains the same fields as TargetFields, but with "target." prefix for JPA queries in order to be applied to JpaAction repository - @Getter - public enum ActionFieldsInternal implements RsqlQueryField { - - ID("controllerId"), - NAME("name"), - DESCRIPTION("description"), - CREATEDAT("createdAt"), - CREATEDBY("createdBy"), - LASTMODIFIEDAT("lastModifiedAt"), - LASTMODIFIEDBY("lastModifiedBy"), - CONTROLLERID("controllerId"), - UPDATESTATUS("updateStatus"), - IPADDRESS("address"), - ATTRIBUTE("controllerAttributes"), - GROUP("group"), - ASSIGNEDDS("assignedDistributionSet", - DistributionSetFields.NAME.getJpaEntityFieldName(), DistributionSetFields.VERSION.getJpaEntityFieldName()), - INSTALLEDDS("installedDistributionSet", - DistributionSetFields.NAME.getJpaEntityFieldName(), DistributionSetFields.VERSION.getJpaEntityFieldName()), - TAG("tags", TagFields.NAME.getJpaEntityFieldName()), - LASTCONTROLLERREQUESTAT("lastTargetQuery"), - METADATA("metadata"), - TARGETTYPE("targetType", - TargetTypeFields.ID.getJpaEntityFieldName(), - TargetTypeFields.KEY.getJpaEntityFieldName(), - TargetTypeFields.NAME.getJpaEntityFieldName()); - - private final String jpaEntityFieldName; - private final List subEntityAttributes; - - ActionFieldsInternal(final String jpaEntityFieldName, final String... subEntityAttributes) { - this.jpaEntityFieldName = "target." + jpaEntityFieldName; - this.subEntityAttributes = List.of(subEntityAttributes); - } - - @Override - public boolean isMap() { - return this == ATTRIBUTE || this == METADATA; - } - } -} +} \ No newline at end of file