From 92a2f1bd74b62b9d77bd6db825fea5c5ef8dd968 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 11 Nov 2015 11:28:27 +0100 Subject: [PATCH 1/3] DATAMONGO-1290 - @Query annotation with byte[] parameter does not work. Prepare issue branch. --- pom.xml | 2 +- spring-data-mongodb-cross-store/pom.xml | 4 ++-- spring-data-mongodb-distribution/pom.xml | 2 +- spring-data-mongodb-log4j/pom.xml | 2 +- spring-data-mongodb/pom.xml | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index 41512eaee8..3701bb1c90 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 1.9.0.BUILD-SNAPSHOT + 1.9.0.DATAMONGO-1290-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-cross-store/pom.xml b/spring-data-mongodb-cross-store/pom.xml index fd36debedd..50acfe4b41 100644 --- a/spring-data-mongodb-cross-store/pom.xml +++ b/spring-data-mongodb-cross-store/pom.xml @@ -6,7 +6,7 @@ org.springframework.data spring-data-mongodb-parent - 1.9.0.BUILD-SNAPSHOT + 1.9.0.DATAMONGO-1290-SNAPSHOT ../pom.xml @@ -48,7 +48,7 @@ org.springframework.data spring-data-mongodb - 1.9.0.BUILD-SNAPSHOT + 1.9.0.DATAMONGO-1290-SNAPSHOT diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index 28c91bc332..7880e7e4d3 100644 --- a/spring-data-mongodb-distribution/pom.xml +++ b/spring-data-mongodb-distribution/pom.xml @@ -13,7 +13,7 @@ org.springframework.data spring-data-mongodb-parent - 1.9.0.BUILD-SNAPSHOT + 1.9.0.DATAMONGO-1290-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-log4j/pom.xml b/spring-data-mongodb-log4j/pom.xml index dfe146ff96..350a777d3f 100644 --- a/spring-data-mongodb-log4j/pom.xml +++ b/spring-data-mongodb-log4j/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 1.9.0.BUILD-SNAPSHOT + 1.9.0.DATAMONGO-1290-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index 0fcdb2f39f..c317a76114 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -11,7 +11,7 @@ org.springframework.data spring-data-mongodb-parent - 1.9.0.BUILD-SNAPSHOT + 1.9.0.DATAMONGO-1290-SNAPSHOT ../pom.xml From fdc2acd68ed76552fd25b6897ccdc76f0a43f458 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 12 Nov 2015 11:50:25 +0100 Subject: [PATCH 2/3] DATAMONGO-1290 - Convert byte[] parameter in @Query to $binary representation. We now convert non quoted binary parameters to the $binary format. This allows using them along with the @Query annotation --- .../query/StringBasedMongoQuery.java | 12 ++++++++++ .../query/StringBasedMongoQueryUnitTests.java | 23 +++++++++++++++++++ spring-data-mongodb/template.mf | 1 + 3 files changed, 36 insertions(+) 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 f0391683f4..a3c976be25 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 @@ -21,6 +21,9 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.xml.bind.DatatypeConverter; + +import org.bson.BSON; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.data.mongodb.core.MongoOperations; @@ -224,6 +227,15 @@ private String getParameterValueForBinding(ConvertingParameterAccessor accessor, return (String) value; } + if (value instanceof byte[]) { + + String base64representation = DatatypeConverter.printBase64Binary((byte[]) value); + if (!binding.isQuoted()) { + return "{ '$binary' : '" + base64representation + "', '$type' : " + BSON.B_GENERAL + "}"; + } + return base64representation; + } + return JSON.serialize(value); } 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 e22e5f8f98..c7f018c494 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 @@ -24,6 +24,9 @@ import java.util.List; import java.util.Map; +import javax.xml.bind.DatatypeConverter; + +import org.bson.BSON; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -343,6 +346,23 @@ public void shouldSupportExpressionsInCustomQueriesWithMultipleNestedObjects() t assertThat(query.getQueryObject(), is(reference.getQueryObject())); } + /** + * @see DATAMONGO-1290 + */ + @Test + public void shouldSupportNonQuotedBinaryDataReplacement() throws Exception { + + byte[] binaryData = "Matthews".getBytes("UTF-8"); + ConvertingParameterAccessor accesor = StubParameterAccessor.getAccessor(converter, binaryData); + StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastnameAsBinary", byte[].class); + + org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor); + org.springframework.data.mongodb.core.query.Query reference = new BasicQuery("{'lastname' : { '$binary' : '" + + DatatypeConverter.printBase64Binary(binaryData) + "', '$type' : " + BSON.B_GENERAL + "}}"); + + assertThat(query.getQueryObject(), is(reference.getQueryObject())); + } + private StringBasedMongoQuery createQueryForMethod(String name, Class... parameters) throws Exception { Method method = SampleRepository.class.getMethod(name, parameters); @@ -355,6 +375,9 @@ private interface SampleRepository { @Query("{ 'lastname' : ?0 }") Person findByLastname(String lastname); + @Query("{ 'lastname' : ?0 }") + Person findByLastnameAsBinary(byte[] lastname); + @Query("{ 'lastname' : '?0' }") Person findByLastnameQuoted(String lastname); diff --git a/spring-data-mongodb/template.mf b/spring-data-mongodb/template.mf index 1966fc8dbe..5706242415 100644 --- a/spring-data-mongodb/template.mf +++ b/spring-data-mongodb/template.mf @@ -16,6 +16,7 @@ Import-Template: javax.tools.*;version="0", javax.net.*;version="0", javax.validation.*;version="${validation:[=.=.=.=,+1.0.0)}";resolution:=optional, + javax.xml.bind.*;version=0, org.aopalliance.*;version="[1.0.0, 2.0.0)";resolution:=optional, org.bson.*;version="0", org.objenesis.*;version="${objenesis:[=.=.=, +1.0.0)}";resolution:=optional, From 9eb2e63c594a12b8a1c3f859a3216b49a7435f97 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 18 Nov 2015 13:20:13 +0100 Subject: [PATCH 3/3] DATAMONGO-1290 - Move parameter binding for String based queries. Moved parameter binding for string based queries into separate class. --- .../ExpressionEvaluatingParameterBinder.java | 230 ++++++++++++++++++ .../query/StringBasedMongoQuery.java | 133 +--------- 2 files changed, 241 insertions(+), 122 deletions(-) create mode 100644 spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ExpressionEvaluatingParameterBinder.java diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ExpressionEvaluatingParameterBinder.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ExpressionEvaluatingParameterBinder.java new file mode 100644 index 0000000000..df1fde90a6 --- /dev/null +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ExpressionEvaluatingParameterBinder.java @@ -0,0 +1,230 @@ +/* + * Copyright 2015 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mongodb.repository.query; + +import java.util.Collections; +import java.util.List; + +import javax.xml.bind.DatatypeConverter; + +import org.bson.BSON; +import org.springframework.data.mongodb.repository.query.StringBasedMongoQuery.ParameterBinding; +import org.springframework.data.repository.query.EvaluationContextProvider; +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.Expression; +import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; +import org.springframework.util.StringUtils; + +import com.mongodb.util.JSON; + +/** + * {@link ExpressionEvaluatingParameterBinder} allows to evaluate, convert and bind parameters to placholders within a + * {@link String}. + * + * @author Christoph Strobl + * @author Thomas Darimont + * @since 1.9 + */ +class ExpressionEvaluatingParameterBinder { + + private final SpelExpressionParser expressionParser; + private final EvaluationContextProvider evaluationContextProvider; + + /** + * Creates new {@link ExpressionEvaluatingParameterBinder} + * + * @param expressionParser must not be {@literal null}. + * @param evaluationContextProvider must not be {@literal null}. + */ + public ExpressionEvaluatingParameterBinder(SpelExpressionParser expressionParser, + EvaluationContextProvider evaluationContextProvider) { + + Assert.notNull(expressionParser, "ExpressionParser must not be null!"); + Assert.notNull(evaluationContextProvider, "EvaluationContextProvider must not be null!"); + + this.expressionParser = expressionParser; + this.evaluationContextProvider = evaluationContextProvider; + } + + /** + * Bind values provided by {@link MongoParameterAccessor} to placeholders in {@literal raw} while consisdering + * potential conversions and parameter types. + * + * @param raw + * @param accessor + * @param bindingContext + * @return {@literal null} if given {@literal raw} value is empty. + */ + public String bind(String raw, MongoParameterAccessor accessor, BindingContext bindingContext) { + + if (!StringUtils.hasText(raw)) { + return null; + } + + return replacePlaceholders(raw, accessor, bindingContext); + } + + /** + * Replaced the parameter place-holders with the actual parameter values from the given {@link ParameterBinding}s. + * + * @param input + * @param accessor + * @param parameters + * @param bindings + * @return + */ + private String replacePlaceholders(String input, MongoParameterAccessor accessor, BindingContext bindingContext) { + + if (!bindingContext.hasBindings()) { + return input; + } + + boolean isCompletlyParameterizedQuery = input.matches("^\\?\\d+$"); + + StringBuilder result = new StringBuilder(input); + + for (ParameterBinding binding : bindingContext.getBindings()) { + + String parameter = binding.getParameter(); + int idx = result.indexOf(parameter); + + if (idx != -1) { + String valueForBinding = getParameterValueForBinding(accessor, bindingContext.getParameters(), 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); + } + } + + return result.toString(); + } + + /** + * Returns the serialized value to be used for the given {@link ParameterBinding}. + * + * @param accessor + * @param parameters + * @param binding + * @return + */ + private String getParameterValueForBinding(MongoParameterAccessor accessor, MongoParameters parameters, + ParameterBinding binding) { + + Object value = binding.isExpression() ? evaluateExpression(binding.getExpression(), parameters, + accessor.getValues()) : accessor.getBindableValue(binding.getParameterIndex()); + + if (value instanceof String && binding.isQuoted()) { + return (String) value; + } + + if (value instanceof byte[]) { + + String base64representation = DatatypeConverter.printBase64Binary((byte[]) value); + if (!binding.isQuoted()) { + return "{ '$binary' : '" + base64representation + "', '$type' : " + BSON.B_GENERAL + "}"; + } + return base64representation; + } + + return JSON.serialize(value); + } + + /** + * Evaluates the given {@code expressionString}. + * + * @param expressionString + * @param parameters + * @param parameterValues + * @return + */ + private Object evaluateExpression(String expressionString, MongoParameters parameters, Object[] parameterValues) { + + EvaluationContext evaluationContext = evaluationContextProvider.getEvaluationContext(parameters, parameterValues); + Expression expression = expressionParser.parseExpression(expressionString); + + return expression.getValue(evaluationContext, Object.class); + } + + /** + * @author Christoph Strobl + * @since 1.9 + */ + static class BindingContext { + + final MongoParameters parameters; + final List bindings; + + /** + * Creates new {@link BindingContext}. + * + * @param parameters + * @param bindings + */ + public BindingContext(MongoParameters parameters, List bindings) { + + this.parameters = parameters; + this.bindings = bindings; + } + + /** + * @return {@literal true} when list of bindings is not empty. + */ + boolean hasBindings() { + return !CollectionUtils.isEmpty(bindings); + } + + /** + * Get unmodifiable list of {@link ParameterBinding}s. + * + * @return never {@literal null}. + */ + public List getBindings() { + return Collections.unmodifiableList(bindings); + } + + /** + * Get the associated {@link MongoParameters}. + * + * @return + */ + public MongoParameters getParameters() { + return parameters; + } + + } +} 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 a3c976be25..33d2e15e7d 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 @@ -21,17 +21,13 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import javax.xml.bind.DatatypeConverter; - -import org.bson.BSON; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.data.mongodb.core.MongoOperations; import org.springframework.data.mongodb.core.query.BasicQuery; import org.springframework.data.mongodb.core.query.Query; +import org.springframework.data.mongodb.repository.query.ExpressionEvaluatingParameterBinder.BindingContext; import org.springframework.data.repository.query.EvaluationContextProvider; -import org.springframework.expression.EvaluationContext; -import org.springframework.expression.Expression; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -59,8 +55,7 @@ public class StringBasedMongoQuery extends AbstractMongoQuery { private final boolean isDeleteQuery; private final List queryParameterBindings; private final List fieldSpecParameterBindings; - private final SpelExpressionParser expressionParser; - private final EvaluationContextProvider evaluationContextProvider; + private final ExpressionEvaluatingParameterBinder parameterBinder; /** * Creates a new {@link StringBasedMongoQuery} for the given {@link MongoQueryMethod} and {@link MongoOperations}. @@ -92,9 +87,6 @@ public StringBasedMongoQuery(String query, MongoQueryMethod method, MongoOperati Assert.notNull(query, "Query must not be null!"); Assert.notNull(expressionParser, "SpelExpressionParser must not be null!"); - this.expressionParser = expressionParser; - this.evaluationContextProvider = evaluationContextProvider; - this.queryParameterBindings = new ArrayList(); this.query = BINDING_PARSER.parseAndCollectParameterBindingsFromQueryIntoBindings(query, this.queryParameterBindings); @@ -109,6 +101,8 @@ public StringBasedMongoQuery(String query, MongoQueryMethod method, MongoOperati if (isCountQuery && isDeleteQuery) { throw new IllegalArgumentException(String.format(COUND_AND_DELETE, method)); } + + this.parameterBinder = new ExpressionEvaluatingParameterBinder(expressionParser, evaluationContextProvider); } /* @@ -118,21 +112,15 @@ public StringBasedMongoQuery(String query, MongoQueryMethod method, MongoOperati @Override protected Query createQuery(ConvertingParameterAccessor accessor) { - String queryString = replacePlaceholders(query, accessor, queryParameterBindings); + String queryString = parameterBinder.bind(this.query, accessor, new BindingContext(getQueryMethod() + .getParameters(), queryParameterBindings)); + String fieldsString = parameterBinder.bind(this.fieldSpec, accessor, new BindingContext(getQueryMethod() + .getParameters(), fieldSpecParameterBindings)); - Query query = null; - - if (fieldSpec != null) { - String fieldString = replacePlaceholders(fieldSpec, accessor, fieldSpecParameterBindings); - query = new BasicQuery(queryString, fieldString); - } else { - query = new BasicQuery(queryString); - } - - query.with(accessor.getSort()); + Query query = new BasicQuery(queryString, fieldsString).with(accessor.getSort()); if (LOG.isDebugEnabled()) { - LOG.debug(String.format("Created query %s", query.getQueryObject())); + LOG.debug(String.format("Created query %s for %s fields.", query.getQueryObject(), query.getFieldsObject())); } return query; @@ -156,105 +144,6 @@ protected boolean isDeleteQuery() { return this.isDeleteQuery; } - /** - * Replaced the parameter place-holders with the actual parameter values from the given {@link ParameterBinding}s. - * - * @param input - * @param accessor - * @param bindings - * @return - */ - 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) { - - String parameter = binding.getParameter(); - int idx = result.indexOf(parameter); - - if (idx != -1) { - 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); - } - } - - return result.toString(); - } - - /** - * Returns the serialized value to be used for the given {@link ParameterBinding}. - * - * @param accessor - * @param binding - * @return - */ - private String getParameterValueForBinding(ConvertingParameterAccessor accessor, ParameterBinding binding) { - - Object value = binding.isExpression() ? evaluateExpression(binding.getExpression(), accessor.getValues()) - : accessor.getBindableValue(binding.getParameterIndex()); - - if (value instanceof String && binding.isQuoted()) { - return (String) value; - } - - if (value instanceof byte[]) { - - String base64representation = DatatypeConverter.printBase64Binary((byte[]) value); - if (!binding.isQuoted()) { - return "{ '$binary' : '" + base64representation + "', '$type' : " + BSON.B_GENERAL + "}"; - } - return base64representation; - } - - return JSON.serialize(value); - } - - /** - * Evaluates the given {@code expressionString}. - * - * @param expressionString - * @param parameterValues - * @return - */ - private Object evaluateExpression(String expressionString, Object[] parameterValues) { - - EvaluationContext evaluationContext = evaluationContextProvider - .getEvaluationContext(getQueryMethod().getParameters(), parameterValues); - Expression expression = expressionParser.parseExpression(expressionString); - - return expression.getValue(evaluationContext, Object.class); - } - /** * A parser that extracts the parameter bindings from a given query string. * @@ -429,7 +318,7 @@ private static int getIndexOfExpressionParameter(String input, int position) { * * @author Thomas Darimont */ - private static class ParameterBinding { + static class ParameterBinding { private final int parameterIndex; private final boolean quoted;