From f2ab42cb8008c910b2b70427a55bf2785020216c Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Sat, 20 Jun 2015 17:27:11 +0200 Subject: [PATCH] DATAMONGO-1244 - Improved handling of expression parameters in StringBasedMongoQuery. Replaced regex based parsing of dynamic expression based parameters with custom parsing to make sure we also support complex nested expression objects. Previously we only supported simple named or positional expressions. Since MongoDBs JSON based query language uses deeply nested objects to express queries, we needed to improve the handling here. Manual parsing is tedious and more verbose than regex based parsing but it gives us more control over the whole parsing process. We also dynamically adjust the quoting so that we only output quoted parameters if necessary. This enables to express complex filtering queries the use Spring Security constructors like: ``` @Query("{id: ?#{ hasRole('ROLE_ADMIN') ? {$exists:true} : principal.id}}") List findAllForCurrentUserById(); ``` Original pull request: #306. --- .../query/StringBasedMongoQuery.java | 87 +++++++++++++++---- .../query/StringBasedMongoQueryUnitTests.java | 40 ++++++++- 2 files changed, 110 insertions(+), 17 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java index 465b22b05..e75531415 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java @@ -161,13 +161,14 @@ public class StringBasedMongoQuery extends AbstractMongoQuery { * @param bindings * @return */ - private String replacePlaceholders(String input, ConvertingParameterAccessor accessor, - List bindings) { + private String replacePlaceholders(String input, ConvertingParameterAccessor accessor, List bindings) { if (bindings.isEmpty()) { return input; } + boolean isCompletlyParameterizedQuery = input.matches("^\\?\\d+$"); + StringBuilder result = new StringBuilder(input); for (ParameterBinding binding : bindings) { @@ -176,7 +177,30 @@ public class StringBasedMongoQuery extends AbstractMongoQuery { int idx = result.indexOf(parameter); if (idx != -1) { - result.replace(idx, idx + parameter.length(), getParameterValueForBinding(accessor, binding)); + String valueForBinding = getParameterValueForBinding(accessor, binding); + + // if the value to bind is an object literal we need to remove the quoting around + // the expression insertion point. + boolean shouldPotentiallyRemoveQuotes = valueForBinding.startsWith("{") && !isCompletlyParameterizedQuery; + + int start = idx; + int end = idx + parameter.length(); + + if (shouldPotentiallyRemoveQuotes) { + + // is the insertion point actually surrounded by quotes? + char beforeStart = result.charAt(start - 1); + char afterEnd = result.charAt(end); + + if ((beforeStart == '\'' || beforeStart == '"') && (afterEnd == '\'' || afterEnd == '"')) { + + // skip preceeding and following quote + start -= 1; + end += 1; + } + } + + result.replace(start, end, valueForBinding); } } @@ -211,8 +235,8 @@ public class StringBasedMongoQuery extends AbstractMongoQuery { */ private Object evaluateExpression(String expressionString, Object[] parameterValues) { - EvaluationContext evaluationContext = evaluationContextProvider - .getEvaluationContext(getQueryMethod().getParameters(), parameterValues); + EvaluationContext evaluationContext = evaluationContextProvider.getEvaluationContext(getQueryMethod() + .getParameters(), parameterValues); Expression expression = expressionParser.parseExpression(expressionString); return expression.getValue(evaluationContext, Object.class); } @@ -226,11 +250,16 @@ public class StringBasedMongoQuery extends AbstractMongoQuery { INSTANCE; + private static final String EXPRESSION_PARAM_QUOTE = "'"; + private static final String EXPRESSION_PARAM_PREFIX = "?expr"; + private static final String INDEX_BASED_EXPRESSION_PARAM_START = "?#{"; + private static final String NAME_BASED_EXPRESSION_PARAM_START = ":#{"; + private static final char CURRLY_BRACE_OPEN = '{'; + private static final char CURRLY_BRACE_CLOSE = '}'; private static final String PARAMETER_PREFIX = "_param_"; private static final String PARSEABLE_PARAMETER = "\"" + PARAMETER_PREFIX + "$1\""; private static final Pattern PARAMETER_BINDING_PATTERN = Pattern.compile("\\?(\\d+)"); private static final Pattern PARSEABLE_BINDING_PATTERN = Pattern.compile("\"?" + PARAMETER_PREFIX + "(\\d+)\"?"); - private static final Pattern PARAMETER_EXPRESSION_PATTERN = Pattern.compile("((:|\\?)#\\{([^}]+)\\})"); private final static int PARAMETER_INDEX_GROUP = 1; @@ -261,28 +290,54 @@ public class StringBasedMongoQuery extends AbstractMongoQuery { private String transformQueryAndCollectExpressionParametersIntoBindings(String input, List bindings) { - Matcher matcher = PARAMETER_EXPRESSION_PATTERN.matcher(input); - StringBuilder result = new StringBuilder(); - int lastPos = 0; + int startIndex = 0; + int currentPos = 0; int exprIndex = 0; - while (matcher.find()) { + while (currentPos < input.length()) { + int indexOfExpressionParameter = input.indexOf(INDEX_BASED_EXPRESSION_PARAM_START, currentPos); - int startOffSet = matcher.start(); + if (indexOfExpressionParameter < 0) { + indexOfExpressionParameter = input.indexOf(NAME_BASED_EXPRESSION_PARAM_START, currentPos); + } - result.append(input.subSequence(lastPos, startOffSet)); - result.append("'?expr").append(exprIndex).append("'"); + if (indexOfExpressionParameter < 0) { + // no expression parameter found + break; + } - lastPos = matcher.end(); + int exprStart = indexOfExpressionParameter + 3; + currentPos = exprStart; - bindings.add(new ParameterBinding(exprIndex, true, matcher.group(3))); + // eat parameter expression + int curlyBraceOpenCnt = 1; + while (curlyBraceOpenCnt > 0) { + char c = input.charAt(currentPos++); + switch (c) { + case CURRLY_BRACE_OPEN: + curlyBraceOpenCnt++; + break; + case CURRLY_BRACE_CLOSE: + curlyBraceOpenCnt--; + break; + default: + ; + } + } + + result.append(input.subSequence(startIndex, indexOfExpressionParameter)); + result.append(EXPRESSION_PARAM_QUOTE).append(EXPRESSION_PARAM_PREFIX).append(exprIndex) + .append(EXPRESSION_PARAM_QUOTE); + bindings.add(new ParameterBinding(exprIndex, true, input.substring(exprStart, currentPos - 1))); + + startIndex = currentPos; exprIndex++; } - result.append(input.subSequence(lastPos, input.length())); + result.append(input.subSequence(currentPos, input.length())); return result.toString(); } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java index 28990ae37..ddd026896 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java @@ -310,6 +310,38 @@ public class StringBasedMongoQueryUnitTests { assertThat(query.getQueryObject(), is(reference.getQueryObject())); } + + /** + * @see DATAMONGO-1244 + */ + @Test + public void shouldSupportExpressionsInCustomQueriesWithNestedObject() throws Exception { + + ConvertingParameterAccessor accesor = StubParameterAccessor.getAccessor(converter, true, "param1", "param2"); + StringBasedMongoQuery mongoQuery = createQueryForMethod("findByQueryWithExpressionAndNestedObject", boolean.class, String.class); + + org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor); + org.springframework.data.mongodb.core.query.Query reference = new BasicQuery("{ \"id\" : { \"$exists\" : true}}"); + + assertThat(query.getQueryObject(), is(reference.getQueryObject())); + } + + /** + * @see DATAMONGO-1244 + */ + @Test + public void shouldSupportExpressionsInCustomQueriesWithMultipleNestedObjects() throws Exception { + + ConvertingParameterAccessor accesor = StubParameterAccessor.getAccessor(converter, true, "param1", "param2"); + StringBasedMongoQuery mongoQuery = createQueryForMethod("findByQueryWithExpressionAndMultipleNestedObjects", boolean.class, String.class, String.class); + + org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor); + org.springframework.data.mongodb.core.query.Query reference = new BasicQuery("{ \"id\" : { \"$exists\" : true} , \"foo\" : 42 , \"bar\" : { \"$exists\" : false}}"); + + assertThat(query.getQueryObject(), is(reference.getQueryObject())); + } + + private StringBasedMongoQuery createQueryForMethod(String name, Class... parameters) throws Exception { Method method = SampleRepository.class.getMethod(name, parameters); @@ -355,7 +387,13 @@ public class StringBasedMongoQueryUnitTests { @Query("{ ?0 : ?1}") Object methodWithPlaceholderInKeyOfJsonStructure(String keyReplacement, String valueReplacement); - @Query(value = "{'lastname': ?#{[0]} }") + @Query("{'lastname': ?#{[0]} }") List findByQueryWithExpression(String param0); + + @Query("{'id':?#{ [0] ? { $exists :true} : [1] }}") + List findByQueryWithExpressionAndNestedObject(boolean param0, String param1); + + @Query("{'id':?#{ [0] ? { $exists :true} : [1] }, 'foo':42, 'bar': ?#{ [0] ? { $exists :false} : [1] }}") + List findByQueryWithExpressionAndMultipleNestedObjects(boolean param0, String param1, String param2); } }