diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetFilterQueryManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetFilterQueryManagement.java index a246a11d0..91a9a247c 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetFilterQueryManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetFilterQueryManagement.java @@ -27,6 +27,7 @@ import org.eclipse.hawkbit.repository.builder.TargetFilterQueryUpdate; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; import org.eclipse.hawkbit.repository.exception.InvalidAutoAssignActionTypeException; import org.eclipse.hawkbit.repository.exception.InvalidAutoAssignDistributionSetException; +import org.eclipse.hawkbit.repository.exception.RSQLParameterUnsupportedFieldException; import org.eclipse.hawkbit.repository.jpa.builder.JpaTargetFilterQueryCreate; import org.eclipse.hawkbit.repository.jpa.configuration.Constants; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet; @@ -43,6 +44,8 @@ import org.eclipse.hawkbit.repository.model.TargetFilterQuery; import org.eclipse.hawkbit.repository.rsql.VirtualPropertyReplacer; import org.eclipse.hawkbit.security.SystemSecurityContext; import org.eclipse.hawkbit.tenancy.TenantAware; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.dao.ConcurrencyFailureException; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; @@ -58,6 +61,8 @@ import org.springframework.validation.annotation.Validated; import com.google.common.collect.Lists; +import cz.jirutka.rsql.parser.RSQLParserException; + /** * JPA implementation of {@link TargetFilterQueryManagement}. * @@ -66,6 +71,8 @@ import com.google.common.collect.Lists; @Validated public class JpaTargetFilterQueryManagement implements TargetFilterQueryManagement { + private static final Logger LOGGER = LoggerFactory.getLogger(JpaTargetFilterQueryManagement.class); + private final TargetFilterQueryRepository targetFilterQueryRepository; private final TargetManagement targetManagement; @@ -102,12 +109,18 @@ public class JpaTargetFilterQueryManagement implements TargetFilterQueryManageme public TargetFilterQuery create(final TargetFilterQueryCreate c) { final JpaTargetFilterQueryCreate create = (JpaTargetFilterQueryCreate) c; - // enforce the 'max targets per auto assign' quota right here even if - // the result of the filter query can vary over time - if (create.getAutoAssignDistributionSetId().isPresent()) { - WeightValidationHelper.usingContext(systemSecurityContext, tenantConfigurationManagement).validate(create); - create.getQuery().ifPresent(this::assertMaxTargetsQuota); - } + create.getQuery().ifPresent(query -> { + // validate the RSQL query syntax + RSQLUtility.validateRsqlFor(query, TargetFields.class); + + // enforce the 'max targets per auto assign' quota right here even + // if the result of the filter query can vary over time + if (create.getAutoAssignDistributionSetId().isPresent()) { + WeightValidationHelper.usingContext(systemSecurityContext, tenantConfigurationManagement) + .validate(create); + assertMaxTargetsQuota(query); + } + }); return targetFilterQueryRepository.save(create.build()); } @@ -291,8 +304,13 @@ public class JpaTargetFilterQueryManagement implements TargetFilterQueryManageme @Override public boolean verifyTargetFilterQuerySyntax(final String query) { - RSQLUtility.parse(query, TargetFields.class, virtualPropertyReplacer, database); - return true; + try { + RSQLUtility.validateRsqlFor(query, TargetFields.class); + return true; + } catch (RSQLParserException | RSQLParameterUnsupportedFieldException e) { + LOGGER.debug("The RSQL query '" + query + "' is invalid.", e); + return false; + } } private void assertMaxTargetsQuota(final String query) { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/AbstractFieldNameRSQLVisitor.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/AbstractFieldNameRSQLVisitor.java new file mode 100644 index 000000000..ece88ff41 --- /dev/null +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/AbstractFieldNameRSQLVisitor.java @@ -0,0 +1,147 @@ +/** + * Copyright (c) 2020 devolo AG and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.repository.jpa.rsql; + +import cz.jirutka.rsql.parser.ast.ComparisonNode; +import org.eclipse.hawkbit.repository.FieldNameProvider; +import org.eclipse.hawkbit.repository.exception.RSQLParameterUnsupportedFieldException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +public abstract class AbstractFieldNameRSQLVisitor & FieldNameProvider> { + + private static final Logger LOGGER = LoggerFactory.getLogger(AbstractFieldNameRSQLVisitor.class); + + private final Class fieldNameProvider; + + public AbstractFieldNameRSQLVisitor(final Class fieldNameProvider) { + this.fieldNameProvider = fieldNameProvider; + } + + protected A getFieldEnumByName(final ComparisonNode node) { + String enumName = node.getSelector(); + final String[] graph = getSubAttributesFrom(enumName); + if (graph.length != 0) { + enumName = graph[0]; + } + LOGGER.debug("get field identifier by name {} of enum type {}", enumName, fieldNameProvider); + try { + return Enum.valueOf(fieldNameProvider, enumName.toUpperCase()); + } catch (final IllegalArgumentException e) { + throw createRSQLParameterUnsupportedException(node, e); + } + } + + protected static String[] getSubAttributesFrom(final String property) { + return property.split("\\" + FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR); + } + + protected String getAndValidatePropertyFieldName(final A propertyEnum, final ComparisonNode node) { + + final String[] graph = getSubAttributesFrom(node.getSelector()); + + validateMapParameter(propertyEnum, node, graph); + + // sub entity need minimum 1 dot + if (!propertyEnum.getSubEntityAttributes().isEmpty() && graph.length < 2) { + throw createRSQLParameterUnsupportedException(node); + } + + final StringBuilder fieldNameBuilder = new StringBuilder(propertyEnum.getFieldName()); + + for (int i = 1; i < graph.length; i++) { + + final String propertyField = graph[i]; + fieldNameBuilder.append(FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR).append(propertyField); + + // the key of map is not in the graph + if (propertyEnum.isMap() && graph.length == (i + 1)) { + continue; + } + + if (!propertyEnum.containsSubEntityAttribute(propertyField)) { + throw createRSQLParameterUnsupportedException(node); + } + } + + return fieldNameBuilder.toString(); + } + + protected void validateMapParameter(final A propertyEnum, final ComparisonNode node, final String[] graph) { + if (!propertyEnum.isMap()) { + return; + + } + + if (!propertyEnum.getSubEntityAttributes().isEmpty()) { + throw new UnsupportedOperationException( + "Currently subentity attributes for maps are not supported, alternatively you could use the key/value tuple, defined by SimpleImmutableEntry class"); + } + + // enum.key + final int minAttributeForMap = 2; + if (graph.length != minAttributeForMap) { + throw new RSQLParameterUnsupportedFieldException("The syntax of the given map search parameter field {" + + node.getSelector() + "} is wrong. Syntax is: fieldname.keyname", new Exception()); + } + } + + protected RSQLParameterUnsupportedFieldException createRSQLParameterUnsupportedException( + final ComparisonNode node) { + return createRSQLParameterUnsupportedException(node, new Exception()); + } + + protected RSQLParameterUnsupportedFieldException createRSQLParameterUnsupportedException(final ComparisonNode node, + final Exception rootException) { + return createRSQLParameterUnsupportedException(String.format( + "The given search parameter field {%s} does not exist, must be one of the following fields %s", + node.getSelector(), getExpectedFieldList()), rootException); + } + + protected RSQLParameterUnsupportedFieldException createRSQLParameterUnsupportedException(final String message) { + return createRSQLParameterUnsupportedException(message, null); + } + + protected RSQLParameterUnsupportedFieldException createRSQLParameterUnsupportedException(final String message, + final Exception rootException) { + return new RSQLParameterUnsupportedFieldException(message, rootException); + } + + // Exception squid:S2095 - see + // https://jira.sonarsource.com/browse/SONARJAVA-1478 + @SuppressWarnings({ "squid:S2095" }) + private List getExpectedFieldList() { + final List expectedFieldList = Arrays.stream(fieldNameProvider.getEnumConstants()) + .filter(enumField -> enumField.getSubEntityAttributes().isEmpty()).map(enumField -> { + final String enumFieldName = enumField.name().toLowerCase(); + + if (enumField.isMap()) { + return enumFieldName + FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR + "keyName"; + } + + return enumFieldName; + }).collect(Collectors.toList()); + + final List expectedSubFieldList = Arrays.stream(fieldNameProvider.getEnumConstants()) + .filter(enumField -> !enumField.getSubEntityAttributes().isEmpty()).flatMap(enumField -> { + final List subEntity = enumField + .getSubEntityAttributes().stream().map(fieldName -> enumField.name().toLowerCase() + + FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR + fieldName) + .collect(Collectors.toList()); + + return subEntity.stream(); + }).collect(Collectors.toList()); + expectedFieldList.addAll(expectedSubFieldList); + return expectedFieldList; + } +} 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 7e0e03dff..6b2fac015 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 @@ -140,19 +140,27 @@ public final class RSQLUtility { } /** - * Validate the given rsql string regarding existence and correct syntax. - * + * Validates the RSQL string + * * @param rsql - * the rsql string to get validated - * + * RSQL string to validate + * @param fieldNameProvider + * + * @throws RSQLParserException + * if RSQL syntax is invalid + * @throws RSQLParameterUnsupportedFieldException + * if RSQL key is not allowed */ - public static void isValid(final String rsql) { - parseRsql(rsql); + public static & FieldNameProvider> void validateRsqlFor(final String rsql, + final Class fieldNameProvider) { + final RSQLVisitor visitor = new ValidationRSQLVisitor<>(fieldNameProvider); + final Node rootNode = parseRsql(rsql); + rootNode.accept(visitor); } private static Node parseRsql(final String rsql) { try { - LOGGER.debug("parsing rsql string {}", rsql); + LOGGER.debug("Parsing rsql string {}", rsql); final Set operators = RSQLOperators.defaultOperators(); return new RSQLParser(operators).parse(rsql); } catch (final IllegalArgumentException e) { @@ -162,6 +170,36 @@ public final class RSQLUtility { } } + private static final class ValidationRSQLVisitor & FieldNameProvider> + extends AbstractFieldNameRSQLVisitor implements RSQLVisitor { + + public ValidationRSQLVisitor(final Class fieldNameProvider) { + super(fieldNameProvider); + } + + @Override + public Void visit(final AndNode node, final String param) { + return visitNode(node, param); + } + + @Override + public Void visit(final OrNode node, final String param) { + return visitNode(node, param); + } + + @Override + public Void visit(final ComparisonNode node, final String param) { + final A fieldName = getFieldEnumByName(node); + getAndValidatePropertyFieldName(fieldName, node); + return null; + } + + private Void visitNode(final LogicalNode node, final String param) { + node.getChildren().forEach(child -> child.accept(this, param)); + return null; + } + } + private static final class RSQLSpecification & FieldNameProvider, T> implements Specification { private static final long serialVersionUID = 1L; @@ -184,7 +222,7 @@ public final class RSQLUtility { final Node rootNode = parseRsql(rsql); query.distinct(true); - final JpqQueryRSQLVisitor jpqQueryRSQLVisitor = new JpqQueryRSQLVisitor<>(root, cb, enumType, + final JpaQueryRSQLVisitor jpqQueryRSQLVisitor = new JpaQueryRSQLVisitor<>(root, cb, enumType, virtualPropertyReplacer, database, query); final List accept = rootNode., String> accept(jpqQueryRSQLVisitor); @@ -198,9 +236,7 @@ public final class RSQLUtility { /** * An implementation of the {@link RSQLVisitor} to visit the parsed tokens - * and build jpa where clauses. - * - * + * and build JPA where clauses. * * @param * the enum for providing the field name of the entity field to @@ -208,34 +244,35 @@ public final class RSQLUtility { * @param * the entity type referenced by the root */ - private static final class JpqQueryRSQLVisitor & FieldNameProvider, T> - implements RSQLVisitor, String> { + private static final class JpaQueryRSQLVisitor & FieldNameProvider, T> + extends AbstractFieldNameRSQLVisitor implements RSQLVisitor, String> { + + private static final Logger LOGGER = LoggerFactory.getLogger(JpaQueryRSQLVisitor.class); + + public static final Character LIKE_WILDCARD = '*'; private static final char ESCAPE_CHAR = '\\'; private static final List NO_JOINS_OPERATOR = Lists.newArrayList("!=", "=out="); - public static final Character LIKE_WILDCARD = '*'; + private final Map>> joinsInLevel = new HashMap<>(3); - private final Root root; private final CriteriaBuilder cb; private final CriteriaQuery query; - private final Class enumType; + private final Database database; + private final Root root; + private final SimpleTypeConverter simpleTypeConverter; private final VirtualPropertyReplacer virtualPropertyReplacer; + private int level; private boolean isOrLevel; - private final Map>> joinsInLevel = new HashMap<>(3); private boolean joinsNeeded; - private final SimpleTypeConverter simpleTypeConverter; - - private final Database database; - - private JpqQueryRSQLVisitor(final Root root, final CriteriaBuilder cb, final Class enumType, + private JpaQueryRSQLVisitor(final Root root, final CriteriaBuilder cb, final Class enumType, final VirtualPropertyReplacer virtualPropertyReplacer, final Database database, final CriteriaQuery query) { + super(enumType); this.root = root; this.cb = cb; this.query = query; - this.enumType = enumType; this.virtualPropertyReplacer = virtualPropertyReplacer; this.simpleTypeConverter = new SimpleTypeConverter(); this.database = database; @@ -297,64 +334,6 @@ public final class RSQLUtility { return Collections.singletonList(predicate); } - private String getAndValidatePropertyFieldName(final A propertyEnum, final ComparisonNode node) { - - final String[] graph = getSubAttributesFrom(node.getSelector()); - - validateMapParameter(propertyEnum, node, graph); - - // sub entity need minium 1 dot - if (!propertyEnum.getSubEntityAttributes().isEmpty() && graph.length < 2) { - throw createRSQLParameterUnsupportedException(node); - } - - final StringBuilder fieldNameBuilder = new StringBuilder(propertyEnum.getFieldName()); - - for (int i = 1; i < graph.length; i++) { - - final String propertyField = graph[i]; - fieldNameBuilder.append(FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR).append(propertyField); - - // the key of map is not in the graph - if (propertyEnum.isMap() && graph.length == (i + 1)) { - continue; - } - - if (!propertyEnum.containsSubEntityAttribute(propertyField)) { - throw createRSQLParameterUnsupportedException(node); - } - } - - return fieldNameBuilder.toString(); - } - - private void validateMapParameter(final A propertyEnum, final ComparisonNode node, final String[] graph) { - if (!propertyEnum.isMap()) { - return; - - } - - if (!propertyEnum.getSubEntityAttributes().isEmpty()) { - throw new UnsupportedOperationException( - "Currently subentity attributes for maps are not supported, alternatively you could use the key/value tuple, defined by SimpleImmutableEntry class"); - } - - // enum.key - final int minAttributeForMap = 2; - if (graph.length != minAttributeForMap) { - throw new RSQLParameterUnsupportedFieldException("The syntax of the given map search parameter field {" - + node.getSelector() + "} is wrong. Syntax is: fieldname.keyname", new Exception()); - } - } - - private RSQLParameterUnsupportedFieldException createRSQLParameterUnsupportedException( - final ComparisonNode node) { - return new RSQLParameterUnsupportedFieldException( - "The given search parameter field {" + node.getSelector() - + "} does not exist, must be one of the following fields {" + getExpectedFieldList() + "}", - new Exception()); - } - /** * Resolves the Path for a field in the persistence layer and joins the * required models. This operation is part of a tree traversal through @@ -374,12 +353,14 @@ public final class RSQLUtility { * dot notated field path * @return the Path for a field */ + @SuppressWarnings("unchecked") private Path getFieldPath(final A enumField, final String finalProperty) { return (Path) getFieldPath(root, getSubAttributesFrom(finalProperty), enumField.isMap(), this::getJoinFieldPath).orElseThrow( - () -> new RSQLParameterUnsupportedFieldException("RSQL field path cannot be empty", null)); + () -> createRSQLParameterUnsupportedException("RSQL field path cannot be empty", null)); } + @SuppressWarnings("unchecked") private Path getJoinFieldPath(final Path fieldPath, final String fieldNameSplit) { if (fieldPath instanceof PluralJoin) { final Join join = (Join) fieldPath; @@ -416,17 +397,7 @@ public final class RSQLUtility { // https://jira.sonarsource.com/browse/SONARJAVA-1478 @SuppressWarnings({ "squid:S2095" }) public List visit(final ComparisonNode node, final String param) { - A fieldName = null; - try { - fieldName = getFieldEnumByName(node); - } catch (final IllegalArgumentException e) { - throw new RSQLParameterUnsupportedFieldException("The given search parameter field {" - + node.getSelector() + "} does not exist, must be one of the following fields {" - + Arrays.stream(enumType.getEnumConstants()).map(v -> v.name().toLowerCase()) - .collect(Collectors.toList()) - + "}", e); - - } + final A fieldName = getFieldEnumByName(node); final String finalProperty = getAndValidatePropertyFieldName(fieldName, node); final List values = node.getArguments(); @@ -446,44 +417,6 @@ public final class RSQLUtility { return !NO_JOINS_OPERATOR.contains(node.getOperator().getSymbol()); } - // Exception squid:S2095 - see - // https://jira.sonarsource.com/browse/SONARJAVA-1478 - @SuppressWarnings({ "squid:S2095" }) - private List getExpectedFieldList() { - final List expectedFieldList = Arrays.stream(enumType.getEnumConstants()) - .filter(enumField -> enumField.getSubEntityAttributes().isEmpty()).map(enumField -> { - final String enumFieldName = enumField.name().toLowerCase(); - - if (enumField.isMap()) { - return enumFieldName + FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR + "keyName"; - } - - return enumFieldName; - }).collect(Collectors.toList()); - - final List expectedSubFieldList = Arrays.stream(enumType.getEnumConstants()) - .filter(enumField -> !enumField.getSubEntityAttributes().isEmpty()).flatMap(enumField -> { - final List subEntity = enumField.getSubEntityAttributes().stream() - .map(fieldName -> enumField.name().toLowerCase() - + FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR + fieldName) - .collect(Collectors.toList()); - - return subEntity.stream(); - }).collect(Collectors.toList()); - expectedFieldList.addAll(expectedSubFieldList); - return expectedFieldList; - } - - private A getFieldEnumByName(final ComparisonNode node) { - String enumName = node.getSelector(); - final String[] graph = getSubAttributesFrom(enumName); - if (graph.length != 0) { - enumName = graph[0]; - } - LOGGER.debug("get fieldidentifier by name {} of enum type {}", enumName, enumType); - return Enum.valueOf(enumType, enumName.toUpperCase()); - } - 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 @@ -522,7 +455,7 @@ public final class RSQLUtility { 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( + throw createRSQLParameterUnsupportedException( "field {" + node.getSelector() + "} must be one of the following values {" + Arrays.toString(((FieldValueConverter) fieldName).possibleValues(fieldName)) + "}", null); @@ -775,10 +708,6 @@ public final class RSQLUtility { return cb.not(cb.exists(subquery)); } - private static String[] getSubAttributesFrom(final String property) { - return property.split("\\" + FieldNameProvider.SUB_ATTRIBUTE_SEPERATOR); - } - private static boolean isSimpleField(final String[] split, final boolean isMapKeyField) { return split.length == 1 || (split.length == 2 && isMapKeyField); } @@ -788,8 +717,7 @@ public final class RSQLUtility { return pathOfString(innerFieldPath); } if (innerFieldPath instanceof MapJoin) { - // Currently we support only string key .So below cast - // is safe. + // Currently we support only string key. So below cast is safe. return (Expression) (((MapJoin) pathOfString(innerFieldPath)).value()); } final String valueFieldName = enumField.getSubEntityMapTuple().map(Entry::getValue) diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RsqlParserValidationOracle.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RsqlParserValidationOracle.java index 0c43cfbf6..ca602cf93 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RsqlParserValidationOracle.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/RsqlParserValidationOracle.java @@ -21,7 +21,6 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; import org.eclipse.hawkbit.repository.TargetFields; -import org.eclipse.hawkbit.repository.TargetManagement; import org.eclipse.hawkbit.repository.exception.RSQLParameterSyntaxException; import org.eclipse.hawkbit.repository.exception.RSQLParameterUnsupportedFieldException; import org.eclipse.hawkbit.repository.jpa.rsql.ParseExceptionWrapper.TokenWrapper; @@ -33,8 +32,6 @@ import org.eclipse.hawkbit.repository.rsql.ValidationOracleContext; import org.eclipse.persistence.exceptions.ConversionException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.data.domain.PageRequest; import org.springframework.orm.jpa.JpaSystemException; import org.springframework.util.CollectionUtils; @@ -61,9 +58,6 @@ public class RsqlParserValidationOracle implements RsqlValidationOracle { private static final Logger LOGGER = LoggerFactory.getLogger(RsqlParserValidationOracle.class); - @Autowired - private TargetManagement targetManagement; - @Override public ValidationOracleContext suggest(final String rsqlQuery, final int cursorPosition) { @@ -76,7 +70,7 @@ public class RsqlParserValidationOracle implements RsqlValidationOracle { context.setSyntaxErrorContext(errorContext); try { - targetManagement.findByRsql(PageRequest.of(0, 1), rsqlQuery); + RSQLUtility.validateRsqlFor(rsqlQuery, TargetFields.class); context.setSyntaxError(false); suggestionContext.getSuggestions().addAll(getLogicalOperatorSuggestion(rsqlQuery)); } catch (final RSQLParameterSyntaxException | RSQLParserException ex) { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldValidationTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldValidationTest.java new file mode 100644 index 000000000..168bc7764 --- /dev/null +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldValidationTest.java @@ -0,0 +1,42 @@ +/** + * Copyright (c) 2020 devolo AG and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.repository.jpa.rsql; + +import org.eclipse.hawkbit.repository.TargetFields; +import org.junit.jupiter.api.Test; + +import io.qameta.allure.Description; +import io.qameta.allure.Feature; +import io.qameta.allure.Story; + +@Feature("Component Tests - Repository") +@Story("RSQL target field validation") +public class RSQLTargetFieldValidationTest { + @Test + @Description("Testing allowed RSQL keys based on TargetFields.class") + public void rsqlValidTargetFields() { + final String rsql1 = "ID == '0123' and NAME == abcd and DESCRIPTION == absd" + + " and CREATEDAT =lt= 0123 and LASTMODIFIEDAT =gt= 0123" + + " and CONTROLLERID == 0123 and UPDATESTATUS == PENDING" + + " and IPADDRESS == 0123 and LASTCONTROLLERREQUESTAT == 0123" + " and tag == beta"; + + RSQLUtility.validateRsqlFor(rsql1, TargetFields.class); + + final String rsql2 = "ASSIGNEDDS.name == abcd and ASSIGNEDDS.version == 0123" + + " and INSTALLEDDS.name == abcd and INSTALLEDDS.version == 0123"; + RSQLUtility.validateRsqlFor(rsql2, TargetFields.class); + + final String rsql3 = "ATTRIBUTE.subkey1 == test and ATTRIBUTE.subkey2 == test" + + " and METADATA.metakey1 == abcd and METADATA.metavalue2 == asdfg"; + RSQLUtility.validateRsqlFor(rsql3, TargetFields.class); + + final String rsql4 = "CREATEDAT =lt= ${NOW_TS} and LASTMODIFIEDAT =ge= ${OVERDUE_TS}"; + RSQLUtility.validateRsqlFor(rsql4, TargetFields.class); + } +} 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 c7d0e4cdc..d228aa1f3 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 @@ -8,6 +8,7 @@ */ package org.eclipse.hawkbit.repository.jpa.rsql; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -19,6 +20,9 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.Arrays; +import java.util.List; + import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; import javax.persistence.criteria.Expression; @@ -99,6 +103,46 @@ public class RSQLUtilityTest { private static final TenantConfigurationValue TEST_POLLING_OVERDUE_TIME_INTERVAL = TenantConfigurationValue . builder().value("00:07:37").build(); + @Test + @Description("Testing throwing exception in case of not allowed RSQL key") + public void rsqlUnsupportedFieldExceptionTest() { + final String rsql1 = "wrongfield == abcd"; + assertThatExceptionOfType(RSQLParameterUnsupportedFieldException.class) + .isThrownBy(() -> RSQLUtility.validateRsqlFor(rsql1, TestFieldEnum.class)); + + final String rsql2 = "wrongfield == abcd or TESTFIELD_WITH_SUB_ENTITIES.subentity11 == 0123"; + assertThatExceptionOfType(RSQLParameterUnsupportedFieldException.class) + .isThrownBy(() -> RSQLUtility.validateRsqlFor(rsql2, TestFieldEnum.class)); + } + + @Test + @Description("Testing exception in case of not allowed subkey") + public void rsqlUnsupportedSubkeyThrowException() { + final String rsql1 = "TESTFIELD_WITH_SUB_ENTITIES.unsupported == abcd and TESTFIELD_WITH_SUB_ENTITIES.subentity22 == 0123"; + assertThatExceptionOfType(RSQLParameterUnsupportedFieldException.class) + .isThrownBy(() -> RSQLUtility.validateRsqlFor(rsql1, TestFieldEnum.class)); + + final String rsql2 = "TESTFIELD_WITH_SUB_ENTITIES.unsupported == abcd or TESTFIELD_WITH_SUB_ENTITIES.subentity22 == 0123"; + assertThatExceptionOfType(RSQLParameterUnsupportedFieldException.class) + .isThrownBy(() -> RSQLUtility.validateRsqlFor(rsql2, TestFieldEnum.class)); + + final String rsql3 = "TESTFIELD == abcd or TESTFIELD_WITH_SUB_ENTITIES.unsupported == 0123"; + assertThatExceptionOfType(RSQLParameterUnsupportedFieldException.class) + .isThrownBy(() -> RSQLUtility.validateRsqlFor(rsql3, TestFieldEnum.class)); + } + + @Test + @Description("Testing valid RSQL keys based on TestFieldEnum.class") + public void rsqlFieldValidation() { + final String rsql1 = "TESTFIELD_WITH_SUB_ENTITIES.subentity11 == abcd and TESTFIELD_WITH_SUB_ENTITIES.subentity22 == 0123"; + final String rsql2 = "TESTFIELD_WITH_SUB_ENTITIES.subentity11 == abcd or TESTFIELD_WITH_SUB_ENTITIES.subentity22 == 0123"; + final String rsql3 = "TESTFIELD_WITH_SUB_ENTITIES.subentity11 == abcd and TESTFIELD_WITH_SUB_ENTITIES.subentity22 == 0123 and TESTFIELD == any"; + + RSQLUtility.validateRsqlFor(rsql1, TestFieldEnum.class); + RSQLUtility.validateRsqlFor(rsql2, TestFieldEnum.class); + RSQLUtility.validateRsqlFor(rsql3, TestFieldEnum.class); + } + @Test public void wrongRsqlSyntaxThrowSyntaxException() { final String wrongRSQL = "name==abc;d"; @@ -421,23 +465,32 @@ public class RSQLUtilityTest { } private enum TestFieldEnum implements FieldNameProvider { - TESTFIELD; + TESTFIELD("testfield"), TESTFIELD_WITH_SUB_ENTITIES("testfieldWithSubEntities", "subentity11", "subentity22"); + + private final String fieldName; + private List subEntityAttributes; + + TestFieldEnum(final String fieldName) { + this(fieldName, new String[0]); + } + + TestFieldEnum(final String fieldName, final String... subEntityAttributes) { + this.fieldName = fieldName; + this.subEntityAttributes = (Arrays.asList(subEntityAttributes)); + } - /* - * (non-Javadoc) - * - * @see - * org.eclipse.hawkbit.server.rest.resource.model.FieldNameProvider# - * getFieldName() - */ @Override public String getFieldName() { - return "testfield"; + return this.fieldName; + } + + @Override + public List getSubEntityAttributes() { + return subEntityAttributes; } } private enum TestValueEnum { BUMLUX; } - } diff --git a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetFilterQueryResourceTest.java b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetFilterQueryResourceTest.java index 83a559a80..f97e7c0f1 100644 --- a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetFilterQueryResourceTest.java +++ b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetFilterQueryResourceTest.java @@ -85,7 +85,7 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte @Description("Ensures that deletion is executed if permitted.") public void deleteTargetFilterQueryReturnsOK() throws Exception { final String filterName = "filter_01"; - final TargetFilterQuery filterQuery = createSingleTargetFilterQuery(filterName, "name=test_01"); + final TargetFilterQuery filterQuery = createSingleTargetFilterQuery(filterName, "name==test_01"); mvc.perform(delete(MgmtRestConstants.TARGET_FILTER_V1_REQUEST_MAPPING + "/" + filterQuery.getId())) .andExpect(status().isOk()); @@ -114,8 +114,8 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte @Description("Ensures that update request is reflected by repository.") public void updateTargetFilterQueryQuery() throws Exception { final String filterName = "filter_02"; - final String filterQuery = "name=test_02"; - final String filterQuery2 = "name=test_02_changed"; + final String filterQuery = "name==test_02"; + final String filterQuery2 = "name==test_02_changed"; final String body = new JSONObject().put("query", filterQuery2).toString(); // prepare @@ -137,7 +137,7 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte public void updateTargetFilterQueryName() throws Exception { final String filterName = "filter_03"; final String filterName2 = "filter_03_changed"; - final String filterQuery = "name=test_03"; + final String filterQuery = "name==test_03"; final String body = new JSONObject().put("name", filterName2).toString(); // prepare @@ -162,7 +162,7 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte final String idA = "a"; final String idB = "b"; final String idC = "c"; - final String testQuery = "name=test"; + final String testQuery = "name==test"; createSingleTargetFilterQuery(idA, testQuery); createSingleTargetFilterQuery(idB, testQuery); @@ -191,7 +191,7 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte final String idA = "a"; final String idB = "b"; final String idC = "c"; - final String testQuery = "name=test"; + final String testQuery = "name==test"; createSingleTargetFilterQuery(idA, testQuery); createSingleTargetFilterQuery(idB, testQuery); @@ -217,7 +217,7 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte final String idC = "c"; final String idD = "d"; final String idE = "e"; - final String testQuery = "name=test"; + final String testQuery = "name==test"; createSingleTargetFilterQuery("a", testQuery); createSingleTargetFilterQuery("b", testQuery); @@ -247,7 +247,7 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte @Description("Ensures that a single target filter query can be retrieved via its id.") public void getSingleTarget() throws Exception { // create first a target which can be retrieved by rest interface - final String knownQuery = "name=test01"; + final String knownQuery = "name==test01"; final String knownName = "someName"; final TargetFilterQuery tfq = createSingleTargetFilterQuery(knownName, knownQuery); final String hrefPrefix = "http://localhost" + MgmtRestConstants.TARGET_FILTER_V1_REQUEST_MAPPING + "/" @@ -294,6 +294,18 @@ public class MgmtTargetFilterQueryResourceTest extends AbstractManagementApiInte assertThat(exceptionInfo.getExceptionClass()).isEqualTo(MessageNotReadableException.class.getName()); assertThat(exceptionInfo.getErrorCode()).isEqualTo(SpServerError.SP_REST_BODY_NOT_READABLE.getKey()); } + + @Test + @Description("Ensures that the creation of a target filter query based on an invalid RSQL query results in a HTTP Bad Request error (400).") + public void createTargetFilterWithInvalidQuery() throws Exception { + final String invalidQuery = "name=abc"; + final String body = new JSONObject().put("query", invalidQuery).put("name", "invalidFilter").toString(); + + mvc.perform(post(MgmtRestConstants.TARGET_FILTER_V1_REQUEST_MAPPING).content(body) + .contentType(MediaType.APPLICATION_JSON)).andDo(print()).andExpect(status().isBadRequest()).andReturn(); + + assertThat(targetFilterQueryManagement.count()).isEqualTo(0); + } @Test @Description("Ensures that the assignment of an auto-assign distribution set results in a HTTP Forbidden error (403) " diff --git a/pom.xml b/pom.xml index d6f2ad870..68c92ad8e 100644 --- a/pom.xml +++ b/pom.xml @@ -332,6 +332,7 @@ licenses/LICENSE_HEADER_TEMPLATE_MICROSOFT_18.txt licenses/LICENSE_HEADER_TEMPLATE_MICROSOFT_20.txt licenses/LICENSE_HEADER_TEMPLATE_DEVOLO_19.txt + licenses/LICENSE_HEADER_TEMPLATE_DEVOLO_20.txt licenses/LICENSE_HEADER_TEMPLATE_KIWIGRID_19.txt licenses/LICENSE_HEADER_TEMPLATE_ENAPTER.txt