Slight improvements in RSQL to SQL logic (#1833)

Signed-off-by: Marinov Avgustin <Avgustin.Marinov@bosch.com>

hawkbit-core/pom.xml

@@ -39,7 +39,12 @@
          <version>${commons-io.version}</version>
       </dependency>
     
-      <!-- Test -->
+      <!-- TEST -->
+      <dependency>
+         <groupId>io.github.classgraph</groupId>
+         <artifactId>classgraph</artifactId>
+         <scope>test</scope>
+      </dependency>
       <dependency>
          <groupId>org.springframework.boot</groupId>
          <artifactId>spring-boot-starter-test</artifactId>

hawkbit-core/src/main/java/org/eclipse/hawkbit/repository/FieldNameProvider.java

@@ -46,14 +46,12 @@ public interface FieldNameProvider {
     default String[] getSubAttributes(final String propertyFieldName) {
         if (isMap()) {
             final String[] subAttributes = propertyFieldName.split(SUB_ATTRIBUTE_SPLIT_REGEX, 2);
-            // [0] field name | [1] key name
+            // [0] field name | [1] key name (could miss, e.g. for target attributes)
             final String mapKeyName = subAttributes.length == 2 ? subAttributes[1] : null;
-            if (ObjectUtils.isEmpty(mapKeyName)) {
+            return ObjectUtils.isEmpty(mapKeyName) ? new String[] { getFieldName() } : new String[] { getFieldName(), mapKeyName };
-                return new String[] { getFieldName() };
+        } else {
-            }
+            return propertyFieldName.split(SUB_ATTRIBUTE_SPLIT_REGEX);
-            return new String[] { getFieldName(), mapKeyName };
         }
-        return propertyFieldName.split(SUB_ATTRIBUTE_SPLIT_REGEX);
     }
 
     /**

hawkbit-core/src/test/java/org/eclipse/hawkbit/repository/FileNameFieldsTest.java

@@ -0,0 +1,48 @@
+/**
+ * Copyright (c) 2024 Contributors to the Eclipse Foundation
+ *
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.eclipse.hawkbit.repository;
+
+import io.github.classgraph.ClassGraph;
+import io.github.classgraph.ClassInfo;
+import io.github.classgraph.ScanResult;
+import io.qameta.allure.Description;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class FileNameFieldsTest {
+
+    @Test
+    @Description("Verifies that fields classes are correctly implemented")
+    @SuppressWarnings("unchecked")
+    public void repositoryManagementMethodsArePreAuthorizedAnnotated() {
+        final String packageName = getClass().getPackage().getName();
+        try (final ScanResult scanResult = new ClassGraph().acceptPackages(packageName).scan()) {
+            final List<? extends Class<? extends FieldNameProvider>> matchingClasses = scanResult.getAllClasses()
+                    .stream()
+                    .filter(classInPackage -> classInPackage.implementsInterface(FieldNameProvider.class))
+                    .map(ClassInfo::loadClass)
+                    .map(clazz -> (Class<? extends FieldNameProvider>) clazz)
+                    .toList();
+            assertThat(matchingClasses).isNotEmpty();
+            matchingClasses.forEach(providerClass -> {
+                assertThat(providerClass.getEnumConstants()).isNotEmpty();
+                for (final FieldNameProvider provider : providerClass.getEnumConstants()) {
+                    if (provider.isMap() && !provider.getSubEntityAttributes().isEmpty()) {
+                        throw new UnsupportedOperationException(
+                                "Currently sub-entity attributes for maps are not supported, alternatively you could use the key/value tuple, defined by SimpleImmutableEntry class");
+                    }
+                }
+            });
+        }
+    }
+}

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/AbstractFieldNameRSQLVisitor.java

@@ -43,48 +43,36 @@ public abstract class AbstractFieldNameRSQLVisitor<A extends Enum<A> & FieldName
 
     protected String getAndValidatePropertyFieldName(final A propertyEnum, final ComparisonNode node) {
         final String[] subAttributes = propertyEnum.getSubAttributes(node.getSelector());
-        validateMapParameter(propertyEnum, node, subAttributes);
 
-        // sub entity need minimum 1 dot
+        if (propertyEnum.isMap()) {
-        if (!propertyEnum.getSubEntityAttributes().isEmpty() && subAttributes.length < 2) {
+            // enum.key
-            throw createRSQLParameterUnsupportedException(node, null);
+            if (subAttributes.length != 2) {
-        }
+                throw new RSQLParameterUnsupportedFieldException(
-
+                        "The syntax of the given map search parameter field {" + node.getSelector() + "} is wrong. Syntax is: <enum name>.<key name>");
-        final StringBuilder fieldNameBuilder = new StringBuilder(propertyEnum.getFieldName());
-        for (int i = 1; i < subAttributes.length; i++) {
-            final String propertyField = getFormattedSubEntityAttribute(propertyEnum ,subAttributes[i]);
-            fieldNameBuilder.append(FieldNameProvider.SUB_ATTRIBUTE_SEPARATOR).append(propertyField);
-
-            // the key of map is not in the graph
-            if (propertyEnum.isMap() && subAttributes.length == (i + 1)) {
-                continue;
             }
-
+        } else {
-            if (!propertyEnum.containsSubEntityAttribute(propertyField)) {
+            // sub entity need minimum 1 dot
+            if (!propertyEnum.getSubEntityAttributes().isEmpty() && subAttributes.length < 2) {
                 throw createRSQLParameterUnsupportedException(node, null);
             }
         }
 
+        final StringBuilder fieldNameBuilder = new StringBuilder(propertyEnum.getFieldName());
+        for (int i = 1; i < subAttributes.length; i++) {
+            final String propertyField = getFormattedSubEntityAttribute(propertyEnum, subAttributes[i]);
+
+            if (!propertyEnum.containsSubEntityAttribute(propertyField)) {
+                if (i != subAttributes.length - 1 || !propertyEnum.isMap()) {
+                    throw createRSQLParameterUnsupportedException(node, null);
+                } // otherwise - the key of map is not in the sub entity attributes
+            }
+
+            fieldNameBuilder.append(FieldNameProvider.SUB_ATTRIBUTE_SEPARATOR).append(propertyField);
+        }
+
         return fieldNameBuilder.toString();
     }
 
-    private void validateMapParameter(final A propertyEnum, final ComparisonNode node, final String[] subAttributes) {
-        if (!propertyEnum.isMap()) {
-            return;
-        }
-
-        if (!propertyEnum.getSubEntityAttributes().isEmpty()) {
-            throw new UnsupportedOperationException(
-                    "Currently sub-entity attributes for maps are not supported, alternatively you could use the key/value tuple, defined by SimpleImmutableEntry class");
-        }
-
-        // enum.key
-        if (subAttributes.length != 2) {
-            throw new RSQLParameterUnsupportedFieldException("The syntax of the given map search parameter field {" +
-                    node.getSelector() + "} is wrong. Syntax is: <enum name>.<key name>");
-        }
-    }
-
     /**
      * @param node current processing node
      * @param rootException in case there is a cause otherwise {@code null}

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/JpaQueryRsqlVisitorG2.java

@@ -51,8 +51,7 @@ import org.springframework.util.CollectionUtils;
 import org.springframework.util.ObjectUtils;
 
 /**
- * An implementation of the {@link RSQLVisitor} to visit the parsed tokens and
+ * An implementation of the {@link RSQLVisitor} to visit the parsed tokens and build JPA where clauses.
- * build JPA where clauses.
  *
  * @param <A> the enum for providing the field name of the entity field to filter on.
  * @param <T> the entity type referenced by the root
@@ -63,8 +62,8 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
 
     public static final Character LIKE_WILDCARD = '*';
     private static final char ESCAPE_CHAR = '\\';
-    private static final List<String> NO_JOINS_OPERATOR = List.of("!=", "=out=");
     private static final String ESCAPE_CHAR_WITH_ASTERISK = ESCAPE_CHAR +"*";
+    private static final List<String> NO_JOINS_OPERATOR = List.of("!=", "=out=");
 
     private final Root<T> root;
     private final CriteriaQuery<?> query;
@@ -94,11 +93,7 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
     @Override
     public List<Predicate> visit(final AndNode node, final String param) {
         final List<Predicate> children = acceptChildren(node);
-        if (children.isEmpty()) {
+        return Collections.singletonList(children.isEmpty() ? cb.conjunction() : cb.and(children.toArray(new Predicate[0])));
-            return toSingleList(cb.conjunction());
-        } else {
-            return toSingleList(cb.and(children.toArray(new Predicate[0])));
-        }
     }
 
     @Override
@@ -106,11 +101,7 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
         inOr = true;
         try {
             final List<Predicate> children = acceptChildren(node);
-            if (children.isEmpty()) {
+            return Collections.singletonList(children.isEmpty() ? cb.conjunction() : cb.or(children.toArray(new Predicate[0])));
-                return toSingleList(cb.conjunction());
-            } else {
-                return toSingleList(cb.or(children.toArray(new Predicate[0])));
-            }
         } finally {
             inOr = false;
             javaTypeToPath.clear();
@@ -143,11 +134,17 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
 
         final Predicate mapPredicate = mapToMapPredicate(node, enumField, fieldPath);
         final Predicate valuePredicate = addOperatorPredicate(node, enumField, finalProperty,
-                getMapValueFieldPath(enumField, fieldPath), transformedValues, value);
+                getValueFieldPath(enumField, fieldPath), transformedValues, value);
 
-        return toSingleList(mapPredicate != null ? cb.and(mapPredicate, valuePredicate) : valuePredicate);
+        return Collections.singletonList(mapPredicate != null ? cb.and(mapPredicate, valuePredicate) : valuePredicate);
+    }
+    private Path<Object> getValueFieldPath(final A enumField, final Path<Object> fieldPath) {
+        if (enumField.isMap()) {
+            return enumField.getSubEntityMapTuple().map(Entry::getValue).map(fieldPath::get).orElse(fieldPath);
+        } else {
+            return fieldPath;
+        }
     }
-
     @SuppressWarnings("unchecked")
     private Predicate mapToMapPredicate(final ComparisonNode node, final A enumField, final Path<Object> fieldPath) {
         if (!enumField.isMap()) {
@@ -168,7 +165,6 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
                                 "SimpleImmutableEntry are allowed. Neither of those could be found!"));
         return equal(fieldPath.get(keyFieldName), keyValue);
     }
-
     private Predicate addOperatorPredicate(final ComparisonNode node, final A enumField, final String finalProperty,
             final Path<Object> fieldPath, final List<Object> transformedValues, final String value) {
         // only 'equal' and 'notEqual' can handle transformed value like enums.
@@ -188,15 +184,14 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
                     "Operator symbol {" + operator + "} is either not supported or not implemented");
         };
     }
-
     private Predicate getEqualToPredicate(final Path<Object> fieldPath, final Object transformedValue) {
         if (transformedValue == null) {
-            return cb.isNull(pathOfString(fieldPath));
+            return cb.isNull(fieldPath);
         }
 
         if ((transformedValue instanceof String transformedValueStr) && !NumberUtils.isCreatable(transformedValueStr)) {
             if (ObjectUtils.isEmpty(transformedValue)) {
-                return cb.or(cb.isNull(pathOfString(fieldPath)), cb.equal(pathOfString(fieldPath), ""));
+                return cb.or(cb.isNull(fieldPath), cb.equal(pathOfString(fieldPath), ""));
             }
 
             if (isPattern(transformedValueStr)) { // a pattern, use like
@@ -212,26 +207,25 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
     private Predicate getNotEqualToPredicate(final A enumField, final String finalProperty,
             final Path<Object> fieldPath, final Object transformedValue) {
         if (transformedValue == null) {
-            return cb.isNotNull(pathOfString(fieldPath));
+            return cb.isNotNull(fieldPath);
         }
 
         if ((transformedValue instanceof String transformedValueStr) && !NumberUtils.isCreatable(transformedValueStr)) {
             if (ObjectUtils.isEmpty(transformedValue)) {
-                return cb.and(cb.isNotNull(pathOfString(fieldPath)), cb.notEqual(pathOfString(fieldPath), ""));
+                return cb.and(cb.isNotNull(fieldPath), cb.notEqual(pathOfString(fieldPath), ""));
             }
 
             final String[] fieldNames = enumField.getSubAttributes(finalProperty);
 
             if (isSimpleField(fieldNames, enumField.isMap())) {
                 if (isPattern(transformedValueStr)) { // a pattern, use like
-                    return cb.or(cb.isNull(pathOfString(fieldPath)), notLike(pathOfString(fieldPath), toSQL(transformedValueStr)));
+                    return cb.or(cb.isNull(fieldPath), notLike(pathOfString(fieldPath), toSQL(transformedValueStr)));
                 } else {
                     return toNullOrNotEqualPredicate(fieldPath, transformedValueStr);
                 }
             }
 
             clearJoinsIfNotNeeded();
-
             return toNotExistsSubQueryPredicate(enumField, fieldNames, expressionToCompare ->
                             isPattern(transformedValueStr) ? // a pattern, use like
                                     like(expressionToCompare, toSQL(transformedValueStr)) :
@@ -246,12 +240,10 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
         final String[] fieldNames = enumField.getSubAttributes(finalProperty);
 
         if (isSimpleField(fieldNames, enumField.isMap())) {
-            final Path<String> pathOfString = pathOfString(fieldPath);
+            return cb.or(cb.isNull(fieldPath), cb.not(in(pathOfString(fieldPath), transformedValues)));
-            return cb.or(cb.isNull(pathOfString), cb.not(in(pathOfString, transformedValues)));
         }
 
         clearJoinsIfNotNeeded();
-
         return toNotExistsSubQueryPredicate(enumField, fieldNames,
                 expressionToCompare -> in(expressionToCompare, transformedValues));
     }
@@ -261,8 +253,7 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
         Path<Object> fieldPath = null;
         for (int i = 0, end = isMapKeyField ? split.length - 1 : split.length; i < end; i++) {
             final String fieldNameSplit = split[i];
-            fieldPath = fieldPath == null ?
+            fieldPath = fieldPath == null ? getPath(root, fieldNameSplit) : fieldPath.get(fieldNameSplit);
-                    getPath(root, fieldNameSplit) : fieldPath.get(fieldNameSplit);
         }
         if (fieldPath == null) {
             throw new RSQLParameterUnsupportedFieldException("RSQL field path cannot be empty", null);
@@ -336,8 +327,8 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
             return simpleTypeConverter.convertIfNecessary(value, javaType);
         } catch (final TypeMismatchException e) {
             throw new RSQLParameterSyntaxException(
-                    "The value of the given search parameter field {" + node.getSelector()
+                    "The value of the given search parameter field {" + node.getSelector() +
-                            + "} is not well formed. Only a boolean (true or false) value will be expected {",
+                            "} is not well formed. Only a boolean (true or false) value will be expected {",
                     e);
         }
     }
@@ -346,23 +337,14 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
         final Object convertedValue = ((FieldValueConverter) fieldName).convertValue(fieldName, value);
         if (convertedValue == null) {
             throw new RSQLParameterUnsupportedFieldException(
-                    "field {" + node.getSelector() + "} must be one of the following values {"
+                    "field {" + node.getSelector() + "} must be one of the following values {" +
-                            + Arrays.toString(((FieldValueConverter) fieldName).possibleValues(fieldName)) + "}",
+                            Arrays.toString(((FieldValueConverter) fieldName).possibleValues(fieldName)) + "}",
                     null);
         } else {
             return convertedValue;
         }
     }
 
-    private Path<Object> getMapValueFieldPath(final A enumField, final Path<Object> fieldPath) {
-        final String valueFieldNameFromSubEntity = enumField.getSubEntityMapTuple().map(Entry::getValue).orElse(null);
-
-        if (!enumField.isMap() || valueFieldNameFromSubEntity == null) {
-            return fieldPath;
-        }
-        return fieldPath.get(valueFieldNameFromSubEntity);
-    }
-
     private void clearJoinsIfNotNeeded() {
         if (!joinsNeeded) {
             root.getJoins().clear();
@@ -371,7 +353,7 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
 
     private Predicate toNullOrNotEqualPredicate(final Path<Object> fieldPath, final Object transformedValue) {
         return cb.or(
-                cb.isNull(pathOfString(fieldPath)),
+                cb.isNull(fieldPath),
                 transformedValue instanceof String transformedValueStr
                         ? notEqual(pathOfString(fieldPath), transformedValueStr)
                         : cb.notEqual(fieldPath, transformedValue));
@@ -467,10 +449,6 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
         return split.length == 1 || (split.length == 2 && isMapKeyField);
     }
 
-    private static List<Predicate> toSingleList(final Predicate predicate) {
-        return Collections.singletonList(predicate);
-    }
-
     @SuppressWarnings("unchecked")
     private static <Y> Path<Y> pathOfString(final Path<?> path) {
         return (Path<Y>) path;