From bf7745160c264156efa80c894e612ab13acbf0f5 Mon Sep 17 00:00:00 2001 From: SirWayne Date: Tue, 2 Feb 2016 09:26:22 +0100 Subject: [PATCH] - Fix conversion for boolean for sql query. If boolean is not wellformed a rsql syntax error is thrown - Escape % for sql query# - Remove operator like ("=li=") - Extend not equal operator with not like jpa query Signed-off-by: SirWayne --- .../rsql/RSQLParameterSyntaxException.java | 15 ++++ .../hawkbit/repository/rsql/RSQLUtility.java | 81 ++++++++++++------- .../rsql/RSQLDistributionSetFieldTest.java | 4 +- .../rsql/RSQLSoftwareModuleFieldTest.java | 5 +- .../repository/rsql/RSQLUtilityTest.java | 49 +++++------ .../hawkbit/rest/resource/TargetResource.java | 5 -- 6 files changed, 99 insertions(+), 60 deletions(-) diff --git a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/rsql/RSQLParameterSyntaxException.java b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/rsql/RSQLParameterSyntaxException.java index 324de8eba..aa43f1bbb 100644 --- a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/rsql/RSQLParameterSyntaxException.java +++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/rsql/RSQLParameterSyntaxException.java @@ -43,4 +43,19 @@ public class RSQLParameterSyntaxException extends SpServerRtException { public RSQLParameterSyntaxException(final Throwable cause) { super(SpServerError.SP_REST_RSQL_SEARCH_PARAM_SYNTAX, cause); } + + /** + * Creates a new RSQLParameterSyntaxException with + * {@link SpServerError#SP_REST_RSQL_SEARCH_PARAM_SYNTAX} error. + * + * @param message + * the message of the exception + * @param cause + * the cause (which is saved for later retrieval by the + * getCause() method). (A null value is permitted, and indicates + * that the cause is nonexistent or unknown.) + */ + public RSQLParameterSyntaxException(final String message, final Throwable cause) { + super(message, SpServerError.SP_REST_RSQL_SEARCH_PARAM_SYNTAX, cause); + } } diff --git a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/rsql/RSQLUtility.java b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/rsql/RSQLUtility.java index 7b50a4c3e..5dd4cfd1d 100644 --- a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/rsql/RSQLUtility.java +++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/rsql/RSQLUtility.java @@ -78,6 +78,7 @@ public final class RSQLUtility { * private constructor due utility class. */ private RSQLUtility() { + } /** @@ -164,13 +165,14 @@ public final class RSQLUtility { private final Root root; private final CriteriaBuilder cb; private final Class enumType; + private final SimpleTypeConverter simpleTypeConverter; private JpqQueryRSQLVisitor(final Root root, final CriteriaBuilder cb, final Class enumType) { this.root = root; this.cb = cb; this.enumType = enumType; - this.simpleTypeConverter = new SimpleTypeConverter(); + simpleTypeConverter = new SimpleTypeConverter(); } @Override @@ -328,7 +330,6 @@ public final class RSQLUtility { return Enum.valueOf(enumType, enumName.toUpperCase()); } - @SuppressWarnings({ "rawtypes", "unchecked" }) 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 @@ -345,20 +346,38 @@ public final class RSQLUtility { return transformEnumValue(node, value, javaType); } if (fieldName instanceof FieldValueConverter) { - 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; - } + 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(); + 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; } } @@ -397,38 +416,32 @@ public final class RSQLUtility { singleList.add(mapPredicate); } - final Path mappedFieldPath = getMapValueFieldPath(enumField, fieldPath); - - addPredicateByOperator(node, fieldPath, transformedValues, transformedValue, value, singleList, - mappedFieldPath); + addOperatorPredicate(node, getMapValueFieldPath(enumField, fieldPath), transformedValues, transformedValue, + value, singleList); return Collections.unmodifiableList(singleList); } - private void addPredicateByOperator(final ComparisonNode node, final Path fieldPath, + private void addOperatorPredicate(final ComparisonNode node, final Path fieldPath, final List transformedValues, final Object transformedValue, final String value, - final List singleList, final Path mappedFieldPath) { + final List singleList) { switch (node.getOperator().getSymbol()) { - case "=li=": - singleList.add( - cb.like(cb.upper(pathOfString(mappedFieldPath)), transformedValue.toString().toUpperCase())); - break; case "==": - singleList.add(getEqualToPredicate(transformedValue, mappedFieldPath)); + singleList.add(getEqualToPredicate(transformedValue, fieldPath)); break; case "!=": - singleList.add(cb.notEqual(mappedFieldPath, transformedValue)); + singleList.add(getNotEqualToPredicate(transformedValue, fieldPath)); break; case "=gt=": - singleList.add(cb.greaterThan(pathOfString(mappedFieldPath), value)); + singleList.add(cb.greaterThan(pathOfString(fieldPath), value)); break; case "=ge=": - singleList.add(cb.greaterThanOrEqualTo(pathOfString(mappedFieldPath), value)); + singleList.add(cb.greaterThanOrEqualTo(pathOfString(fieldPath), value)); break; case "=lt=": - singleList.add(cb.lessThan(pathOfString(mappedFieldPath), value)); + singleList.add(cb.lessThan(pathOfString(fieldPath), value)); break; case "=le=": - singleList.add(cb.lessThanOrEqualTo(pathOfString(mappedFieldPath), value)); + singleList.add(cb.lessThanOrEqualTo(pathOfString(fieldPath), value)); break; case "=in=": singleList.add(fieldPath.in(transformedValues)); @@ -464,12 +477,24 @@ public final class RSQLUtility { private Predicate getEqualToPredicate(final Object transformedValue, final Path fieldPath) { if (transformedValue instanceof String) { - final String preFormattedValue = ((String) transformedValue).replace(LIKE_WILDCARD, '%'); + final String preFormattedValue = escapeValueToSQL((String) transformedValue); return cb.like(cb.upper(pathOfString(fieldPath)), preFormattedValue.toUpperCase()); } return cb.equal(fieldPath, transformedValue); } + private Predicate getNotEqualToPredicate(final Object transformedValue, final Path fieldPath) { + if (transformedValue instanceof String) { + final String preFormattedValue = escapeValueToSQL((String) transformedValue); + return cb.notLike(cb.upper(pathOfString(fieldPath)), preFormattedValue.toUpperCase()); + } + return cb.notEqual(fieldPath, transformedValue); + } + + private String escapeValueToSQL(final String transformedValue) { + return transformedValue.replace("%", "\\%").replace(LIKE_WILDCARD, '%'); + } + @SuppressWarnings("unchecked") private Path pathOfString(final Path path) { return (Path) path; diff --git a/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/rsql/RSQLDistributionSetFieldTest.java b/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/rsql/RSQLDistributionSetFieldTest.java index c4c5a181a..6bfdb89aa 100644 --- a/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/rsql/RSQLDistributionSetFieldTest.java +++ b/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/rsql/RSQLDistributionSetFieldTest.java @@ -44,7 +44,7 @@ public class RSQLDistributionSetFieldTest extends AbstractIntegrationTest { final DistributionSet ds2 = TestDataUtil .generateDistributionSets("NewDS", 3, softwareManagement, distributionSetManagement).get(0); - ds2.setDescription("DS2"); + ds2.setDescription("DS%"); ds2.getMetadata().add(new DistributionSetMetadata("metaKey", ds2, "value")); distributionSetManagement.updateDistributionSet(ds2); @@ -77,6 +77,7 @@ public class RSQLDistributionSetFieldTest extends AbstractIntegrationTest { public void testFilterByParameterDescription() { assertRSQLQuery(DistributionSetFields.DESCRIPTION.name() + "==DS", 1); assertRSQLQuery(DistributionSetFields.DESCRIPTION.name() + "==DS*", 2); + assertRSQLQuery(DistributionSetFields.DESCRIPTION.name() + "==DS%", 1); assertRSQLQuery(DistributionSetFields.DESCRIPTION.name() + "==noExist*", 0); assertRSQLQuery(DistributionSetFields.DESCRIPTION.name() + "=in=(DS,notexist)", 1); assertRSQLQuery(DistributionSetFields.DESCRIPTION.name() + "=out=(DS,notexist)", 3); @@ -99,7 +100,6 @@ public class RSQLDistributionSetFieldTest extends AbstractIntegrationTest { assertRSQLQuery(DistributionSetFields.COMPLETE.name() + "==noExist*", 0); fail(); } catch (final RSQLParameterSyntaxException e) { - // excepted } assertRSQLQuery(DistributionSetFields.COMPLETE.name() + "=in=(true)", 4); assertRSQLQuery(DistributionSetFields.COMPLETE.name() + "=out=(true)", 0); diff --git a/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/rsql/RSQLSoftwareModuleFieldTest.java b/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/rsql/RSQLSoftwareModuleFieldTest.java index 8f2fba6da..55e4386cd 100644 --- a/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/rsql/RSQLSoftwareModuleFieldTest.java +++ b/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/rsql/RSQLSoftwareModuleFieldTest.java @@ -61,6 +61,7 @@ public class RSQLSoftwareModuleFieldTest extends AbstractIntegrationTest { public void testFilterByParameterName() { assertRSQLQuery(SoftwareModuleFields.NAME.name() + "==agent-hub", 1); assertRSQLQuery(SoftwareModuleFields.NAME.name() + "==agent-hub*", 2); + assertRSQLQuery(SoftwareModuleFields.NAME.name() + "!=agent-hub*", 2); assertRSQLQuery(SoftwareModuleFields.NAME.name() + "==noExist*", 0); assertRSQLQuery(SoftwareModuleFields.NAME.name() + "=in=(agent-hub,notexist)", 1); assertRSQLQuery(SoftwareModuleFields.NAME.name() + "=out=(agent-hub,notexist)", 3); @@ -94,12 +95,12 @@ public class RSQLSoftwareModuleFieldTest extends AbstractIntegrationTest { } @Test - @Description("") + @Description("Test filter software module by metadata") public void testFilterByMetadata() { assertRSQLQuery(SoftwareModuleFields.METADATA.name() + ".metaKey==metaValue", 1); assertRSQLQuery(SoftwareModuleFields.METADATA.name() + ".metaKey==*v*", 2); assertRSQLQuery(SoftwareModuleFields.METADATA.name() + ".metaKey==noExist*", 0); - assertRSQLQuery(SoftwareModuleFields.METADATA.name() + ".metaKey=in=(metaValue,notexist)", 1); + assertRSQLQuery(SoftwareModuleFields.METADATA.name() + ".metaKey=in=(metaValue,value)", 2); assertRSQLQuery(SoftwareModuleFields.METADATA.name() + ".metaKey=out=(metaValue,notexist)", 1); assertRSQLQuery(SoftwareModuleFields.METADATA.name() + ".notExist==metaValue", 0); diff --git a/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/rsql/RSQLUtilityTest.java b/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/rsql/RSQLUtilityTest.java index 85092b560..3b923d167 100644 --- a/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/rsql/RSQLUtilityTest.java +++ b/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/rsql/RSQLUtilityTest.java @@ -156,7 +156,7 @@ public class RSQLUtilityTest { } @Test - public void correctRsqlBuildsNotEqualPredicate() { + public void correctRsqlBuildsNotLikePredicate() { reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); final String correctRsql = "name!=abc"; when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock); @@ -164,13 +164,37 @@ public class RSQLUtilityTest { when(criteriaBuilderMock.equal(any(Root.class), anyString())).thenReturn(mock(Predicate.class)); when(criteriaBuilderMock. greaterThanOrEqualTo(any(Expression.class), any(String.class))) .thenReturn(mock(Predicate.class)); + when(criteriaBuilderMock.upper(eq(pathOfString(baseSoftwareModuleRootMock)))) + .thenReturn(pathOfString(baseSoftwareModuleRootMock)); // test RSQLUtility.parse(correctRsql, SoftwareModuleFields.class).toPredicate(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); // verfication verify(criteriaBuilderMock, times(1)).and(any(Predicate.class)); - verify(criteriaBuilderMock, times(1)).notEqual(eq(baseSoftwareModuleRootMock), eq("abc")); + verify(criteriaBuilderMock, times(1)).notLike(eq(pathOfString(baseSoftwareModuleRootMock)), + eq("abc".toUpperCase())); + } + + @Test + public void correctRsqlBuildsLikePredicateWithPercentage() { + reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); + final String correctRsql = "name==a%"; + when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock); + when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class); + when(criteriaBuilderMock.equal(any(Root.class), anyString())).thenReturn(mock(Predicate.class)); + when(criteriaBuilderMock. greaterThanOrEqualTo(any(Expression.class), any(String.class))) + .thenReturn(mock(Predicate.class)); + when(criteriaBuilderMock.upper(eq(pathOfString(baseSoftwareModuleRootMock)))) + .thenReturn(pathOfString(baseSoftwareModuleRootMock)); + // test + RSQLUtility.parse(correctRsql, SoftwareModuleFields.class).toPredicate(baseSoftwareModuleRootMock, + criteriaQueryMock, criteriaBuilderMock); + + // verfication + verify(criteriaBuilderMock, times(1)).and(any(Predicate.class)); + verify(criteriaBuilderMock, times(1)).like(eq(pathOfString(baseSoftwareModuleRootMock)), + eq("a\\%".toUpperCase())); } @Test @@ -191,27 +215,6 @@ public class RSQLUtilityTest { verify(criteriaBuilderMock, times(1)).lessThan(eq(pathOfString(baseSoftwareModuleRootMock)), eq("abc")); } - @Test - public void correctRsqlBuildsLikePredicate() { - reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); - final String correctRsql = "name=li=abc"; - when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock); - when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class); - when(criteriaBuilderMock.equal(any(Root.class), anyString())).thenReturn(mock(Predicate.class)); - when(criteriaBuilderMock. greaterThanOrEqualTo(any(Expression.class), any(String.class))) - .thenReturn(mock(Predicate.class)); - when(criteriaBuilderMock.upper(eq(pathOfString(baseSoftwareModuleRootMock)))) - .thenReturn(pathOfString(baseSoftwareModuleRootMock)); - // test - RSQLUtility.parse(correctRsql, SoftwareModuleFields.class).toPredicate(baseSoftwareModuleRootMock, - criteriaQueryMock, criteriaBuilderMock); - - // verfication - verify(criteriaBuilderMock, times(1)).and(any(Predicate.class)); - verify(criteriaBuilderMock, times(1)).like(eq(pathOfString(baseSoftwareModuleRootMock)), - eq("abc".toUpperCase())); - } - @Test public void correctRsqlWithEnumValue() { reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock); diff --git a/hawkbit-rest-resource/src/main/java/org/eclipse/hawkbit/rest/resource/TargetResource.java b/hawkbit-rest-resource/src/main/java/org/eclipse/hawkbit/rest/resource/TargetResource.java index 7b78b0f21..f50111db8 100644 --- a/hawkbit-rest-resource/src/main/java/org/eclipse/hawkbit/rest/resource/TargetResource.java +++ b/hawkbit-rest-resource/src/main/java/org/eclipse/hawkbit/rest/resource/TargetResource.java @@ -15,8 +15,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import javax.persistence.EntityManager; - import org.eclipse.hawkbit.repository.ActionFields; import org.eclipse.hawkbit.repository.ActionStatusFields; import org.eclipse.hawkbit.repository.DeploymentManagement; @@ -76,9 +74,6 @@ public class TargetResource { @Autowired private DeploymentManagement deploymentManagement; - @Autowired - private EntityManager entityManager; - /** * Handles the GET request of retrieving a single target within SP. *