From d9d4469a95ee8d206d26b2c7446289905419ad8c Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Tue, 27 Aug 2024 10:48:55 +0300 Subject: [PATCH] Fix RSQL filter for no target tag and OR (#1824) * Fix RSQL filter for no target tag and OR * add test for such filter * Clean up the code keeps the legacy Rsql Visitor which could be used with hawkbit.rsql.legacyRsqlVisitor=true --------- Signed-off-by: Marinov Avgustin --- .../repository/rsql/RsqlConfigHolder.java | 4 + .../jpa/rsql/JpaQueryRsqlVisitor.java | 4 + .../jpa/rsql/JpaQueryRsqlVisitorG2.java | 489 ++++++++++++++++++ .../repository/jpa/rsql/RSQLUtility.java | 19 +- .../jpa/management/TargetManagementTest.java | 28 + .../management/TargetTagManagementTest.java | 4 +- .../jpa/rsql/RSQLTargetFieldTest.java | 4 +- .../repository/jpa/rsql/RSQLUtilityTest.java | 56 +- .../src/test/resources/jpa-test.properties | 5 + 9 files changed, 592 insertions(+), 21 deletions(-) create mode 100644 hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/JpaQueryRsqlVisitorG2.java diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/rsql/RsqlConfigHolder.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/rsql/RsqlConfigHolder.java index 5795f9743..eba63a717 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/rsql/RsqlConfigHolder.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/rsql/RsqlConfigHolder.java @@ -42,6 +42,10 @@ public final class RsqlConfigHolder { @Autowired private RsqlVisitorFactory rsqlVisitorFactory; + @Deprecated + @Value("${hawkbit.rsql.legacyRsqlVisitor:false}") + private boolean legacyRsqlVisitor; + /** * @return The holder singleton instance. */ diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/JpaQueryRsqlVisitor.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/JpaQueryRsqlVisitor.java index 9efa097d1..9d5b037c7 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/JpaQueryRsqlVisitor.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/JpaQueryRsqlVisitor.java @@ -58,9 +58,13 @@ import org.springframework.util.ObjectUtils; * An implementation of the {@link RSQLVisitor} to visit the parsed tokens and * build JPA where clauses. * + * @deprecated Old implementation of RSQL Visitor. Deprecated in favour of next gen implementation - {@link JpaQueryRsqlVisitorG2}. + * It will be kept for some time in order to keep backward compatibility and to allow for a smooth transition. Also, in case of + * problems with the new implementation, this one can be used as a fallback. * @param the enum for providing the field name of the entity field to filter on. * @param the entity type referenced by the root */ +@Deprecated(forRemoval = true) @Slf4j public class JpaQueryRsqlVisitor & FieldNameProvider, T> extends AbstractFieldNameRSQLVisitor implements RSQLVisitor, String> { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/JpaQueryRsqlVisitorG2.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/JpaQueryRsqlVisitorG2.java new file mode 100644 index 000000000..b063feac8 --- /dev/null +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/JpaQueryRsqlVisitorG2.java @@ -0,0 +1,489 @@ +/** + * Copyright (c) 2021 Bosch.IO GmbH and others + * + * 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.jpa.rsql; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map.Entry; +import java.util.function.Function; +import java.util.stream.Collectors; +import jakarta.persistence.criteria.CriteriaBuilder; +import jakarta.persistence.criteria.CriteriaQuery; +import jakarta.persistence.criteria.Expression; +import jakarta.persistence.criteria.JoinType; +import jakarta.persistence.criteria.MapJoin; +import jakarta.persistence.criteria.Path; +import jakarta.persistence.criteria.Predicate; +import jakarta.persistence.criteria.Root; +import jakarta.persistence.criteria.Subquery; + +import cz.jirutka.rsql.parser.ast.AndNode; +import cz.jirutka.rsql.parser.ast.ComparisonNode; +import cz.jirutka.rsql.parser.ast.LogicalNode; +import cz.jirutka.rsql.parser.ast.Node; +import cz.jirutka.rsql.parser.ast.OrNode; +import cz.jirutka.rsql.parser.ast.RSQLVisitor; +import jakarta.persistence.metamodel.Attribute; +import jakarta.persistence.metamodel.SingularAttribute; +import jakarta.persistence.metamodel.Type; +import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.math.NumberUtils; +import org.eclipse.hawkbit.repository.FieldNameProvider; +import org.eclipse.hawkbit.repository.FieldValueConverter; +import org.eclipse.hawkbit.repository.exception.RSQLParameterSyntaxException; +import org.eclipse.hawkbit.repository.exception.RSQLParameterUnsupportedFieldException; +import org.eclipse.hawkbit.repository.rsql.VirtualPropertyReplacer; +import org.springframework.beans.SimpleTypeConverter; +import org.springframework.beans.TypeMismatchException; +import org.springframework.orm.jpa.vendor.Database; +import org.springframework.util.CollectionUtils; +import org.springframework.util.ObjectUtils; + +/** + * An implementation of the {@link RSQLVisitor} to visit the parsed tokens and + * build JPA where clauses. + * + * @param the enum for providing the field name of the entity field to filter on. + * @param the entity type referenced by the root + */ +@Slf4j +public class JpaQueryRsqlVisitorG2 & FieldNameProvider, T> extends AbstractFieldNameRSQLVisitor + implements RSQLVisitor, String> { + + public static final Character LIKE_WILDCARD = '*'; + private static final char ESCAPE_CHAR = '\\'; + private static final List NO_JOINS_OPERATOR = List.of("!=", "=out="); + private static final String ESCAPE_CHAR_WITH_ASTERISK = ESCAPE_CHAR +"*"; + + private final Root root; + private final CriteriaQuery query; + private final CriteriaBuilder cb; + private final Database database; + private final VirtualPropertyReplacer virtualPropertyReplacer; + private final boolean ensureIgnoreCase; + + private final SimpleTypeConverter simpleTypeConverter = new SimpleTypeConverter(); + + private boolean joinsNeeded; + + public JpaQueryRsqlVisitorG2(final Class enumType, + final Root root, final CriteriaQuery query, final CriteriaBuilder cb, + final Database database, final VirtualPropertyReplacer virtualPropertyReplacer, final boolean ensureIgnoreCase) { + super(enumType); + this.root = root; + this.cb = cb; + this.query = query; + this.virtualPropertyReplacer = virtualPropertyReplacer; + this.database = database; + this.ensureIgnoreCase = ensureIgnoreCase; + } + + @Override + public List visit(final AndNode node, final String param) { + final List children = acceptChildren(node); + if (children.isEmpty()) { + return toSingleList(cb.conjunction()); + } else { + return toSingleList(cb.and(children.toArray(new Predicate[0]))); + } + } + + @Override + public List visit(final OrNode node, final String param) { + final List children = acceptChildren(node); + if (children.isEmpty()) { + return toSingleList(cb.conjunction()); + } else { + return toSingleList(cb.or(children.toArray(new Predicate[0]))); + } + } + + private static List toSingleList(final Predicate predicate) { + return Collections.singletonList(predicate); + } + + private static Path getFieldPath( + final Root root, final String[] split, final boolean isMapKeyField) { + Path fieldPath = null; + for (int i = 0, end = isMapKeyField ? split.length - 1 : split.length; i < end; i++) { + final String fieldNameSplit = split[i]; + fieldPath = fieldPath == null ? + // if root.get creates a join we call join directly in order to specify LEFT JOIN type, + // to include rows for missing in particular table / criteria (root.get creates INNER JOIN) + // (see org.eclipse.persistence.internal.jpa.querydef.FromImpl implementation for more details) + // otherwise delegate to root.get + (isJoin(root, fieldNameSplit) ? root.join(fieldNameSplit, JoinType.LEFT) : root.get(fieldNameSplit)) : + fieldPath.get(fieldNameSplit); + } + if (fieldPath == null) { + throw new RSQLParameterUnsupportedFieldException("RSQL field path cannot be empty", null); + } + return fieldPath; + } + private static boolean isJoin(final Root root, final String fieldNameSplit) { + // see org.eclipse.persistence.internal.jpa.querydef.FromImpl implementation for more details + // when root.get creates a join + final Attribute attribute = root.getModel().getAttribute(fieldNameSplit); + if (!attribute.isCollection()) { + // it is a SingularAttribute and not join if it is of basic persistent type + return !((SingularAttribute) attribute).getType().getPersistenceType().equals(Type.PersistenceType.BASIC); + } // if a collection - it is join + return true; + } + + @Override + // Exception squid:S2095 - see + // https://jira.sonarsource.com/browse/SONARJAVA-1478 + @SuppressWarnings({ "squid:S2095" }) + public List visit(final ComparisonNode node, final String param) { + final A fieldName = getFieldEnumByName(node); + final String finalProperty = getAndValidatePropertyFieldName(fieldName, node); + + final List values = node.getArguments(); + final List transformedValues = new ArrayList<>(); + final Path fieldPath = getFieldPath(root, fieldName.getSubAttributes(finalProperty), fieldName.isMap()); + + for (final String value : values) { + transformedValues.add(convertValueIfNecessary(node, fieldName, value, fieldPath)); + } + + this.joinsNeeded = this.joinsNeeded || areJoinsNeeded(node); + + return mapToPredicate(node, fieldPath, node.getArguments(), transformedValues, fieldName, finalProperty); + } + + private static boolean areJoinsNeeded(final ComparisonNode node) { + return !NO_JOINS_OPERATOR.contains(node.getOperator().getSymbol()); + } + + private Object convertValueIfNecessary(final ComparisonNode node, final A fieldName, final String value, + final Path fieldPath) { + // in case the value of an RSQL query e.g. type==application is an + // enum we need to handle it separately because JPA needs the + // correct java-type to build an expression. So String and numeric + // values JPA can do it by it's own but not for classes like enums. + // So we need to transform the given value string into the enum + // class. + final Class javaType = fieldPath.getJavaType(); + if (javaType != null && javaType.isEnum()) { + return transformEnumValue(node, value, javaType); + } + if (fieldName instanceof FieldValueConverter) { + return convertFieldConverterValue(node, fieldName, value); + } + + if (Boolean.TYPE.equals(javaType)) { + return convertBooleanValue(node, value, javaType); + } + + return value; + } + + private Object convertBooleanValue(final ComparisonNode node, final String value, final Class javaType) { + try { + return simpleTypeConverter.convertIfNecessary(value, javaType); + } catch (final TypeMismatchException e) { + throw new RSQLParameterSyntaxException( + "The value of the given search parameter field {" + node.getSelector() + + "} is not well formed. Only a boolean (true or false) value will be expected {", + e); + } + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + private Object convertFieldConverterValue(final ComparisonNode node, final A fieldName, final String value) { + final Object convertedValue = ((FieldValueConverter) fieldName).convertValue(fieldName, value); + if (convertedValue == null) { + throw new RSQLParameterUnsupportedFieldException( + "field {" + node.getSelector() + "} must be one of the following values {" + + Arrays.toString(((FieldValueConverter) fieldName).possibleValues(fieldName)) + "}", + null); + } else { + return convertedValue; + } + } + + // Exception squid:S2095 - see + // https://jira.sonarsource.com/browse/SONARJAVA-1478 + @SuppressWarnings({ "rawtypes", "unchecked", "squid:S2095" }) + private static Object transformEnumValue(final ComparisonNode node, final String value, final Class javaType) { + final Class tmpEnumType = (Class) javaType; + try { + return Enum.valueOf(tmpEnumType, value.toUpperCase()); + } catch (final IllegalArgumentException e) { + // we could not transform the given string value into the enum + // type, so ignore it and return null and do not filter + log.info("given value {} cannot be transformed into the correct enum type {}", value.toUpperCase(), + javaType); + log.debug("value cannot be transformed to an enum", e); + + throw new RSQLParameterUnsupportedFieldException("field {" + node.getSelector() + + "} must be one of the following values {" + Arrays.stream(tmpEnumType.getEnumConstants()) + .map(v -> v.name().toLowerCase()).toList() + + "}", e); + } + } + + private List mapToPredicate(final ComparisonNode node, final Path fieldPath, + final List values, final List transformedValues, final A enumField, + final String finalProperty) { + // if lookup is available, replace macros ... + final String value = virtualPropertyReplacer == null ? values.get(0) : virtualPropertyReplacer.replace(values.get(0)); + + final Predicate mapPredicate = mapToMapPredicate(node, fieldPath, enumField); + + final Predicate valuePredicate = addOperatorPredicate(node, getMapValueFieldPath(enumField, fieldPath), + transformedValues, value, finalProperty, enumField); + + return toSingleList(mapPredicate != null ? cb.and(mapPredicate, valuePredicate) : valuePredicate); + } + + private Predicate addOperatorPredicate(final ComparisonNode node, final Path fieldPath, + final List transformedValues, final String value, final String finalProperty, final A enumField) { + // only 'equal' and 'notEqual' can handle transformed value like enums. + // The JPA API cannot handle object types for greaterThan etc methods. + final Object transformedValue = transformedValues.get(0); + final String operator = node.getOperator().getSymbol(); + return switch (operator) { + case "==" -> getEqualToPredicate(transformedValue, fieldPath); + case "!=" -> getNotEqualToPredicate(transformedValue, fieldPath, finalProperty, enumField); + case "=gt=" -> cb.greaterThan(pathOfString(fieldPath), value); + case "=ge=" -> cb.greaterThanOrEqualTo(pathOfString(fieldPath), value); + case "=lt=" -> cb.lessThan(pathOfString(fieldPath), value); + case "=le=" -> cb.lessThanOrEqualTo(pathOfString(fieldPath), value); + case "=in=" -> in(pathOfString(fieldPath), transformedValues); + case "=out=" -> getOutPredicate(transformedValues, finalProperty, enumField, fieldPath); + default -> throw new RSQLParameterSyntaxException( + "Operator symbol {" + operator + "} is either not supported or not implemented"); + }; + } + + private Predicate getOutPredicate( + final List transformedValues, final String finalProperty, + final A enumField, final Path fieldPath) { + final String[] fieldNames = enumField.getSubAttributes(finalProperty); + + if (isSimpleField(fieldNames, enumField.isMap())) { + final Path pathOfString = pathOfString(fieldPath); + return cb.or(cb.isNull(pathOfString), cb.not(in(pathOfString, transformedValues))); + } + + clearOuterJoinsIfNotNeeded(); + + return toNotExistsSubQueryPredicate(fieldNames, enumField, expressionToCompare -> in(expressionToCompare, transformedValues)); + } + + private Path getMapValueFieldPath(final A enumField, final Path fieldPath) { + final String valueFieldNameFromSubEntity = enumField.getSubEntityMapTuple().map(Entry::getValue).orElse(null); + + if (!enumField.isMap() || valueFieldNameFromSubEntity == null) { + return fieldPath; + } + return fieldPath.get(valueFieldNameFromSubEntity); + } + + @SuppressWarnings("unchecked") + private Predicate mapToMapPredicate(final ComparisonNode node, final Path fieldPath, final A enumField) { + if (!enumField.isMap()) { + return null; + } + + final String[] graph = enumField.getSubAttributes(node.getSelector()); + + final String keyValue = graph[graph.length - 1]; + if (fieldPath instanceof MapJoin) { + // Currently we support only string key. So below cast is safe. + return equal((Expression) (((MapJoin) fieldPath).key()), keyValue); + } + + final String keyFieldName = enumField.getSubEntityMapTuple().map(Entry::getKey) + .orElseThrow(() -> new UnsupportedOperationException( + "For the fields, defined as Map, only Map java type or tuple in the form of " + + "SimpleImmutableEntry are allowed. Neither of those could be found!")); + return equal(fieldPath.get(keyFieldName), keyValue); + } + + private Predicate getEqualToPredicate(final Object transformedValue, final Path fieldPath) { + if (transformedValue == null) { + return cb.isNull(pathOfString(fieldPath)); + } + + if ((transformedValue instanceof String transformedValueStr) && !NumberUtils.isCreatable(transformedValueStr)) { + if (ObjectUtils.isEmpty(transformedValue)) { + return cb.or(cb.isNull(pathOfString(fieldPath)), cb.equal(pathOfString(fieldPath), "")); + } + + if (isPattern(transformedValueStr)) { // a pattern, use like + return like(pathOfString(fieldPath), toSQL(transformedValueStr)); + } else { + return equal(pathOfString(fieldPath), transformedValueStr); + } + } + + return cb.equal(fieldPath, transformedValue); + } + + private Predicate getNotEqualToPredicate(final Object transformedValue, final Path fieldPath, + final String finalProperty, final A enumField) { + + if (transformedValue == null) { + return cb.isNotNull(pathOfString(fieldPath)); + } + + if ((transformedValue instanceof String transformedValueStr) && !NumberUtils.isCreatable(transformedValueStr)) { + if (ObjectUtils.isEmpty(transformedValue)) { + return cb.and(cb.isNotNull(pathOfString(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))); + } else { + return toNullOrNotEqualPredicate(fieldPath, transformedValueStr); + } + } + + clearOuterJoinsIfNotNeeded(); + + return toNotExistsSubQueryPredicate( + fieldNames, enumField, expressionToCompare -> + isPattern(transformedValueStr) ? // a pattern, use like + like(expressionToCompare, toSQL(transformedValueStr)) : + equal(expressionToCompare, transformedValueStr)); + } + + return toNullOrNotEqualPredicate(fieldPath, transformedValue); + } + + private void clearOuterJoinsIfNotNeeded() { + if (!joinsNeeded) { + root.getJoins().clear(); + } + } + + private Predicate toNullOrNotEqualPredicate(final Path fieldPath, final Object transformedValue) { + return cb.or( + cb.isNull(pathOfString(fieldPath)), + transformedValue instanceof String transformedValueStr + ? notEqual(pathOfString(fieldPath), transformedValueStr) + : cb.notEqual(fieldPath, transformedValue)); + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private Predicate toNotExistsSubQueryPredicate(final String[] fieldNames, final A enumField, + final Function, Predicate> subQueryPredicateProvider) { + final Class javaType = root.getJavaType(); + final Subquery subquery = query.subquery(javaType); + final Root subqueryRoot = subquery.from(javaType); + final Predicate equalPredicate = cb.equal(root.get(enumField.identifierFieldName()), + subqueryRoot.get(enumField.identifierFieldName())); + final Expression expressionToCompare = getExpressionToCompare( + getFieldPath(subqueryRoot, fieldNames, enumField.isMap()), enumField); + final Predicate subQueryPredicate = subQueryPredicateProvider.apply(expressionToCompare); + subquery.select(subqueryRoot).where(cb.and(equalPredicate, subQueryPredicate)); + return cb.not(cb.exists(subquery)); + } + + private static boolean isSimpleField(final String[] split, final boolean isMapKeyField) { + return split.length == 1 || (split.length == 2 && isMapKeyField); + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + private Expression getExpressionToCompare(final Path fieldPath, final A enumField) { + if (!enumField.isMap()) { + return pathOfString(fieldPath); + } + if (fieldPath instanceof MapJoin) { + // Currently we support only string key. So below cast is safe. + return (Expression) (((MapJoin) pathOfString(fieldPath)).value()); + } + final String valueFieldName = enumField.getSubEntityMapTuple().map(Entry::getValue) + .orElseThrow(() -> new UnsupportedOperationException( + "For the fields, defined as Map, only Map java type or tuple in the form of SimpleImmutableEntry are allowed. Neither of those could be found!")); + return pathOfString(fieldPath).get(valueFieldName); + } + + private static boolean isPattern(final String transformedValue) { + if (transformedValue.contains(ESCAPE_CHAR_WITH_ASTERISK)) { + return transformedValue.replace(ESCAPE_CHAR_WITH_ASTERISK, "$").indexOf(LIKE_WILDCARD) != -1; + } else { + return transformedValue.indexOf(LIKE_WILDCARD) != -1; + } + } + + private String toSQL(final String transformedValue) { + final String escaped; + + if (database == Database.SQL_SERVER) { + escaped = transformedValue.replace("%", "[%]").replace("_", "[_]"); + } else { + escaped = transformedValue.replace("%", ESCAPE_CHAR + "%").replace("_", ESCAPE_CHAR + "_"); + } + return replaceIfRequired(escaped); + } + + private String replaceIfRequired(final String escapedValue) { + final String finalizedValue; + if (escapedValue.contains(ESCAPE_CHAR_WITH_ASTERISK)) { + finalizedValue = escapedValue.replace(ESCAPE_CHAR_WITH_ASTERISK, "$").replace(LIKE_WILDCARD, '%') + .replace("$", ESCAPE_CHAR_WITH_ASTERISK); + } else { + finalizedValue = escapedValue.replace(LIKE_WILDCARD, '%'); + } + return finalizedValue; + } + + @SuppressWarnings("unchecked") + private static Path pathOfString(final Path path) { + return (Path) path; + } + + private List acceptChildren(final LogicalNode node) { + final List children = new ArrayList<>(); + for (final Node child : node.getChildren()) { + final List accept = child.accept(this); + if (!CollectionUtils.isEmpty(accept)) { + children.addAll(accept); + } else { + log.debug("visit logical node children but could not parse it, ignoring {}", child); + } + } + return children; + } + + private Predicate equal(final Expression expressionToCompare, final String sqlValue) { + return cb.equal(caseWise(cb, expressionToCompare), caseWise(sqlValue)); + } + private Predicate notEqual(final Expression expressionToCompare, String transformedValueStr) { + return cb.notEqual(caseWise(cb, expressionToCompare), caseWise(transformedValueStr)); + } + private Predicate like(final Expression expressionToCompare, final String sqlValue) { + return cb.like(caseWise(cb, expressionToCompare), caseWise(sqlValue), ESCAPE_CHAR); + } + private Predicate notLike(final Expression expressionToCompare, final String sqlValue) { + return cb.notLike(caseWise(cb, expressionToCompare), caseWise(sqlValue), ESCAPE_CHAR); + } + private Predicate in(final Expression expressionToCompare, final List transformedValues) { + final List inParams = transformedValues.stream().filter(String.class::isInstance) + .map(String.class::cast).map(this::caseWise).collect(Collectors.toList()); + return inParams.isEmpty() ? expressionToCompare.in(transformedValues) : caseWise(cb, expressionToCompare).in(inParams); + } + + private Expression caseWise(final CriteriaBuilder cb, final Expression expression) { + return ensureIgnoreCase ? cb.upper(expression) : expression; + } + private String caseWise(final String str) { + return ensureIgnoreCase ? str.toUpperCase() : str; + } +} \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtility.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtility.java index f0effa00c..2d8bb6e3b 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtility.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtility.java @@ -150,15 +150,24 @@ public final class RSQLUtility { final Node rootNode = parseRsql(rsql); query.distinct(true); - final JpaQueryRsqlVisitor jpqQueryRSQLVisitor = new JpaQueryRsqlVisitor<>(root, cb, enumType, - virtualPropertyReplacer, database, query, - !RsqlConfigHolder.getInstance().isCaseInsensitiveDB() && RsqlConfigHolder.getInstance().isIgnoreCase()); + final RSQLVisitor, String> jpqQueryRSQLVisitor = + RsqlConfigHolder.getInstance().isLegacyRsqlVisitor() ? + new JpaQueryRsqlVisitor<>( + root, cb, enumType, + virtualPropertyReplacer, database, query, + !RsqlConfigHolder.getInstance().isCaseInsensitiveDB() && RsqlConfigHolder.getInstance().isIgnoreCase()) + : + new JpaQueryRsqlVisitorG2<>( + enumType, root, query, cb, + database, virtualPropertyReplacer, + !RsqlConfigHolder.getInstance().isCaseInsensitiveDB() && RsqlConfigHolder.getInstance().isIgnoreCase()); final List accept = rootNode.accept(jpqQueryRSQLVisitor); - if (!CollectionUtils.isEmpty(accept)) { + if (CollectionUtils.isEmpty(accept)) { + return cb.conjunction(); + } else { return cb.and(accept.toArray(new Predicate[0])); } - return cb.conjunction(); } } } \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java index 23d4c60b1..45a4ad542 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java @@ -1203,6 +1203,16 @@ class TargetManagementTest extends AbstractJpaIntegrationTest { return target; } + private Target createTargetWithTargetTypeAndMetadata(final String controllerId, final long targetTypeId, final int count) { + final Target target = testdataFactory.createTarget(controllerId, controllerId, targetTypeId); + + for (int index = 1; index <= count; index++) { + insertTargetMetadata("key" + index, controllerId + "-value" + index, target); + } + + return target; + } + @Test @WithUser(allSpPermissions = true) @Description("Checks that target type is not assigned to target if invalid.") @@ -1278,6 +1288,24 @@ class TargetManagementTest extends AbstractJpaIntegrationTest { validateFoundTargetsByRsql(rsqlOrControllerIdNotEqualFilter, controllerId1, controllerId2); } + @Test + @Description("Test that RSQL filter finds targets with tag and metadata.") + void findTargetsByRsqlWithTypeAndMetadata() { + final String controllerId1 = "target1"; + final String controllerId2 = "target2"; + createTargetWithMetadata(controllerId1, 2); + final TargetType type = testdataFactory.createTargetType("type1", Collections.emptyList()); + createTargetWithTargetTypeAndMetadata(controllerId2, type.getId(), 2); + + assertThat(targetManagement.count()).as("Total targets").isEqualTo(2); + + final String rsqlAndByBoth = "targettype.key==type1 or metadata.key1==target1-value1"; + validateFoundTargetsByRsql(rsqlAndByBoth, controllerId1, controllerId2); + + final String rsqlAndControllerIdFilter = "targettype.key==type1 and metadata.key1==target1-value1"; + validateFoundTargetsByRsql(rsqlAndControllerIdFilter); + } + @Test @Description("Target matches filter.") void matchesFilter() { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetTagManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetTagManagementTest.java index cf80c8bda..788c71b89 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetTagManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetTagManagementTest.java @@ -273,7 +273,7 @@ class TargetTagManagementTest extends AbstractJpaIntegrationTest { } @Test - @Description("Ensures that a tag cannot be created if one exists already with that name (ecpects EntityAlreadyExistsException).") + @Description("Ensures that a tag cannot be created if one exists already with that name (expects EntityAlreadyExistsException).") void failedDuplicateTargetTagNameException() { targetTagManagement.create(entityFactory.tag().create().name("A")); assertThatExceptionOfType(EntityAlreadyExistsException.class) @@ -281,7 +281,7 @@ class TargetTagManagementTest extends AbstractJpaIntegrationTest { } @Test - @Description("Ensures that a tag cannot be updated to a name that already exists on another tag (ecpects EntityAlreadyExistsException).") + @Description("Ensures that a tag cannot be updated to a name that already exists on another tag (expects EntityAlreadyExistsException).") void failedDuplicateTargetTagNameExceptionAfterUpdate() { targetTagManagement.create(entityFactory.tag().create().name("A")); final TargetTag tag = targetTagManagement.create(entityFactory.tag().create().name("B")); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldTest.java index 31d58d24b..914066498 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldTest.java @@ -338,9 +338,9 @@ class RSQLTargetFieldTest extends AbstractJpaIntegrationTest { private void assertRSQLQuery(final String rsqlParam, final long expectedTargets) { final Slice findTargetPage = targetManagement.findByRsql(PAGE, rsqlParam); - final long countTargetsAll = targetManagement.countByRsql(rsqlParam); assertThat(findTargetPage).isNotNull(); - assertThat(findTargetPage.getNumberOfElements()).isEqualTo(countTargetsAll).isEqualTo(expectedTargets); + assertThat(findTargetPage.getNumberOfElements()).isEqualTo(expectedTargets); + assertThat(targetManagement.countByRsql(rsqlParam)).isEqualTo(expectedTargets); } private void assertRSQLQueryThrowsException(final String rsqlParam) { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtilityTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtilityTest.java index 28c0784bb..c004d89c6 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtilityTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtilityTest.java @@ -34,6 +34,9 @@ import jakarta.persistence.criteria.Root; import jakarta.persistence.criteria.Subquery; import jakarta.persistence.metamodel.Attribute; +import jakarta.persistence.metamodel.EntityType; +import jakarta.persistence.metamodel.SingularAttribute; +import jakarta.persistence.metamodel.Type; import org.eclipse.hawkbit.repository.DistributionSetFields; import org.eclipse.hawkbit.repository.FieldNameProvider; import org.eclipse.hawkbit.repository.SoftwareModuleFields; @@ -51,6 +54,7 @@ import org.eclipse.hawkbit.repository.rsql.VirtualPropertyReplacer; import org.eclipse.hawkbit.repository.rsql.VirtualPropertyResolver; import org.eclipse.hawkbit.security.SystemSecurityContext; import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -126,6 +130,12 @@ public class RSQLUtilityTest { private static final TenantConfigurationValue TEST_POLLING_OVERDUE_TIME_INTERVAL = TenantConfigurationValue . builder().value("00:07:37").build(); + @BeforeEach + public void beforeEach() { + setupRoot(baseSoftwareModuleRootMock); + setupRoot(subqueryRootMock); + } + @Test @Description("Testing throwing exception in case of not allowed RSQL key") public void rsqlUnsupportedFieldExceptionTest() { @@ -264,7 +274,7 @@ public class RSQLUtilityTest { @Test public void correctRsqlBuildsPredicate() { - reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); + reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); final String correctRsql = "name==abc;version==1.2"; when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock); when(baseSoftwareModuleRootMock.get("version")).thenReturn(baseSoftwareModuleRootMock); @@ -285,7 +295,7 @@ public class RSQLUtilityTest { @Test public void correctRsqlBuildsSimpleNotEqualPredicate() { - reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); + reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); final String correctRsql = "name!=abc"; when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock); when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class); @@ -309,7 +319,7 @@ public class RSQLUtilityTest { @Test public void correctRsqlBuildsSimpleNotLikePredicate() { - reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); + reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); final String correctRsql = "name!=abc*"; when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock); when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class); @@ -333,7 +343,7 @@ public class RSQLUtilityTest { @Test public void correctRsqlBuildsNotSimpleNotLikePredicate() { - reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); + reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); // with this query a subquery has to be made, so it is no simple query final String correctRsql = "type!=abc"; when(baseSoftwareModuleRootMock.get(anyString())).thenReturn(baseSoftwareModuleRootMock); @@ -360,7 +370,7 @@ public class RSQLUtilityTest { @Test public void correctRsqlBuildsEqualPredicateWithPercentage() { - reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); + reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); final String correctRsql = "name==a%"; when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock); when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class); @@ -381,7 +391,7 @@ public class RSQLUtilityTest { @Test public void correctRsqlBuildsLikePredicateWithPercentage() { - reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); + reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); final String correctRsql = "name==a%*"; when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock); when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class); @@ -402,7 +412,7 @@ public class RSQLUtilityTest { @Test public void correctRsqlBuildsLikePredicateWithPercentageSQLServer() { - reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); + reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); final String correctRsql = "name==a%*"; when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock); when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class); @@ -424,7 +434,7 @@ public class RSQLUtilityTest { @Test public void correctRsqlBuildsLessThanPredicate() { - reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); + reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); final String correctRsql = "name=lt=abc"; when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock); when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class); @@ -442,7 +452,7 @@ public class RSQLUtilityTest { @Test public void correctRsqlWithEnumValue() { - reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); + reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); final String correctRsql = "testfield==bumlux"; when(baseSoftwareModuleRootMock.get("testfield")).thenReturn(baseSoftwareModuleRootMock); when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) TestValueEnum.class); @@ -459,7 +469,7 @@ public class RSQLUtilityTest { @Test public void wrongRsqlWithWrongEnumValue() { - reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); + reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); final String correctRsql = "testfield==unknownValue"; when(baseSoftwareModuleRootMock.get("testfield")).thenReturn(baseSoftwareModuleRootMock); when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) TestValueEnum.class); @@ -478,7 +488,7 @@ public class RSQLUtilityTest { @Test @Description("Tests the resolution of overdue_ts placeholder in context of a RSQL expression.") public void correctRsqlWithOverdueMacro() { - reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); + reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); final String overdueProp = "overdue_ts"; final String overduePropPlaceholder = "${" + overdueProp + "}"; final String correctRsql = "testfield=le=" + overduePropPlaceholder; @@ -506,7 +516,7 @@ public class RSQLUtilityTest { @Test @Description("Tests RSQL expression with an unknown placeholder.") public void correctRsqlWithUnknownMacro() { - reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); + reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); final String overdueProp = "unknown"; final String overduePropPlaceholder = "${" + overdueProp + "}"; final String correctRsql = "testfield=le=" + overduePropPlaceholder; @@ -575,6 +585,28 @@ public class RSQLUtilityTest { RSQLUtility.validateRsqlFor(rsql, TestFieldEnum.class); } + private void reset0(final Object... mocks) { + reset(mocks); + if (Arrays.asList(mocks).contains(baseSoftwareModuleRootMock)) { + setupRoot(baseSoftwareModuleRootMock); + } + if (Arrays.asList(mocks).contains(subqueryRootMock)) { + setupRoot(subqueryRootMock); + } + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + private void setupRoot(final Root root) { + final Type type = Mockito.mock(Type.class); + when(type.getPersistenceType()).thenReturn(Type.PersistenceType.BASIC); + final SingularAttribute singularAttribute = Mockito.mock(SingularAttribute.class); + when(singularAttribute.getType()).thenReturn(type); + final EntityType entityType = Mockito.mock(EntityType.class); + when(entityType.getAttribute(any())).thenReturn(singularAttribute); + when(entityType.getPersistenceType()).thenReturn(Type.PersistenceType.BASIC); + when(root.getModel()).thenReturn(entityType); + } + private enum TestValueEnum { BUMLUX; } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/resources/jpa-test.properties b/hawkbit-repository/hawkbit-repository-jpa/src/test/resources/jpa-test.properties index 52893a920..8c0d0f609 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/resources/jpa-test.properties +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/resources/jpa-test.properties @@ -10,7 +10,12 @@ # Debug utility functions - START logging.level.org.eclipse.persistence=ERROR +#incomment to see the debug of persistence, e.g. to see the generated SQLs +#logging.level.org.eclipse.persistence=DEBUG spring.jpa.properties.eclipselink.logging.level=FINE spring.jpa.properties.eclipselink.logging.level.sql=FINE spring.jpa.properties.eclipselink.logging.parameters=true # Debug utility functions - END + +# enable / disable case sensitiveness of the DB when playing around +#hawkbit.rsql.caseInsensitiveDB=true