DATAMONGO-1603 - Fix Placeholder not replaced correctly in @Query.

Fix issues when placeholders are appended with other chars eg. '?0xyz' or have been reused multiple times within the query. Additional tests and fixes for complex quoted replacements eg. in regex query. Rely on placeholder quotation indication instead of binding one. Might be misleading when placeholder is used more than once.

Original pull request: #441.
This commit is contained in:
Christoph Strobl
2017-02-08 15:11:13 +01:00
committed by Mark Paluch
parent 9d4f34a6ff
commit 5925b5133c
3 changed files with 106 additions and 8 deletions

View File

@@ -126,7 +126,7 @@ class ExpressionEvaluatingParameterBinder {
buffer.append(placeholder.getSuffix());
}
if (binding.isQuoted() || placeholder.isQuoted()) {
if (placeholder.isQuoted()) {
postProcessQuotedBinding(buffer, valueForBinding,
!binding.isExpression() ? accessor.getBindableValue(binding.getParameterIndex()) : null,
binding.isExpression());
@@ -247,7 +247,8 @@ class ExpressionEvaluatingParameterBinder {
regex.append("|");
regex.append("(" + Pattern.quote(binding.getParameter()) + ")");
regex.append("(\\W?['\"])?"); // potential quotation char (as in { foo : '?0' }).
regex.append("([\\w.]*");
regex.append("(\\W?['\"]|\\w*')?)");
}
return Pattern.compile(regex.substring(1));
@@ -265,15 +266,22 @@ class ExpressionEvaluatingParameterBinder {
if (matcher.groupCount() > 1) {
String rawPlaceholder = matcher.group(parameterIndex * 2 + 1);
String suffix = matcher.group(parameterIndex * 2 + 2);
String rawPlaceholder = matcher.group(parameterIndex * 3 + 1);
String suffix = matcher.group(parameterIndex * 3 + 2);
if (!StringUtils.hasText(rawPlaceholder)) {
rawPlaceholder = matcher.group();
suffix = "" + rawPlaceholder.charAt(rawPlaceholder.length() - 1);
if(rawPlaceholder.matches(".*\\d$")) {
suffix = "";
} else {
int index = rawPlaceholder.replaceAll("[^\\?0-9]*$", "").length() - 1;
if (index > 0 && rawPlaceholder.length() > index) {
suffix = rawPlaceholder.substring(index+1);
}
}
if (QuotedString.endsWithQuote(rawPlaceholder)) {
rawPlaceholder = QuotedString.unquoteSuffix(rawPlaceholder);
rawPlaceholder = rawPlaceholder.substring(0, rawPlaceholder.length() - (StringUtils.hasText(suffix) ? suffix.length() : 1));
}
}
@@ -284,6 +292,7 @@ class ExpressionEvaluatingParameterBinder {
return Placeholder.of(parameterIndex, rawPlaceholder, quoted,
quoted ? QuotedString.unquoteSuffix(suffix) : suffix);
}
return Placeholder.of(parameterIndex, rawPlaceholder, false, null);
}
return Placeholder.of(parameterIndex, matcher.group(), false, null);

View File

@@ -339,8 +339,7 @@ public class StringBasedMongoQuery extends AbstractMongoQuery {
while (valueMatcher.find()) {
int paramIndex = Integer.parseInt(valueMatcher.group(PARAMETER_INDEX_GROUP));
boolean quoted = (source.startsWith("'") && source.endsWith("'"))
|| (source.startsWith("\"") && source.endsWith("\""));
boolean quoted = source.startsWith("'") || source.startsWith("\"");
bindings.add(new ParameterBinding(paramIndex, quoted));
}

View File

@@ -437,6 +437,78 @@ public class StringBasedMongoQueryUnitTests {
assertThat(query.getQueryObject(), is((DBObject) new BasicDBObject("lastname", Pattern.compile("^(calamity)"))));
}
@Test // DATAMONGO-1603
public void shouldAllowReuseOfPlaceholderWithinQuery() throws Exception {
StringBasedMongoQuery mongoQuery = createQueryForMethod("findByReusingPlaceholdersMultipleTimes", String.class,
String.class);
ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, "calamity", "regalia");
org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor);
assertThat(query.getQueryObject(), is((DBObject) new BasicDBObject().append("arg0", "calamity")
.append("arg1", "regalia").append("arg2", "calamity")));
}
@Test // DATAMONGO-1603
public void shouldAllowReuseOfQuotedPlaceholderWithinQuery() throws Exception {
StringBasedMongoQuery mongoQuery = createQueryForMethod("findByReusingPlaceholdersMultipleTimesWhenQuoted",
String.class, String.class);
ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, "calamity", "regalia");
org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor);
assertThat(query.getQueryObject(), is((DBObject) new BasicDBObject().append("arg0", "calamity")
.append("arg1", "regalia").append("arg2", "calamity")));
}
@Test // DATAMONGO-1603
public void shouldAllowReuseOfQuotedPlaceholderWithinQueryAndIncludeSuffixCorrectly() throws Exception {
StringBasedMongoQuery mongoQuery = createQueryForMethod(
"findByReusingPlaceholdersMultipleTimesWhenQuotedAndSomeStuffAppended", String.class, String.class);
ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, "calamity", "regalia");
org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor);
assertThat(query.getQueryObject(), is((DBObject) new BasicDBObject().append("arg0", "calamity")
.append("arg1", "regalia").append("arg2", "calamitys")));
}
@Test // DATAMONGO-1603
public void shouldAllowQuotedParameterWithSuffixAppended() throws Exception {
StringBasedMongoQuery mongoQuery = createQueryForMethod("findByWhenQuotedAndSomeStuffAppended", String.class,
String.class);
ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, "calamity", "regalia");
org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor);
assertThat(query.getQueryObject(),
is((DBObject) new BasicDBObject().append("arg0", "calamity").append("arg1", "regalias")));
}
@Test // DATAMONGO-1603
public void shouldCaptureReplacementWithComplexSuffixCorrectly() throws Exception {
StringBasedMongoQuery mongoQuery = createQueryForMethod("findByMultiRegex", String.class);
ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, "calamity");
org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor);
assertThat(query.getQueryObject(), is((DBObject) JSON.parse(
"{ \"$or\" : [ { \"firstname\" : { \"$regex\" : \".*calamity.*\" , \"$options\" : \"i\"}} , { \"lastname\" : { \"$regex\" : \".*calamityxyz.*\" , \"$options\" : \"i\"}}]}")));
}
@Test // DATAMONGO-1603
public void shouldAllowPlaceholderReuseInQuotedValue() throws Exception {
StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastnameRegex", String.class, String.class);
ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, "calamity", "regalia");
org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor);
assertThat(query.getQueryObject(),
is((DBObject) JSON.parse("{ 'lastname' : { '$regex' : '^(calamity|John regalia|regalia)'} }")));
}
private StringBasedMongoQuery createQueryForMethod(String name, Class<?>... parameters) throws Exception {
Method method = SampleRepository.class.getMethod(name, parameters);
@@ -460,6 +532,9 @@ public class StringBasedMongoQueryUnitTests {
@Query("{ 'lastname' : { '$regex' : '^(?0)'} }")
Person findByLastnameRegex(String lastname);
@Query("{'$or' : [{'firstname': {'$regex': '.*?0.*', '$options': 'i'}}, {'lastname' : {'$regex': '.*?0xyz.*', '$options': 'i'}} ]}")
Person findByMultiRegex(String arg0);
@Query("{ 'address' : ?0 }")
Person findByAddress(Address address);
@@ -510,5 +585,20 @@ public class StringBasedMongoQueryUnitTests {
@Query("{ 'arg0' : ?0 }")
List<Person> findByWithBsonArgument(DBObject arg0);
@Query("{ 'arg0' : ?0, 'arg1' : ?1, 'arg2' : ?0 }")
List<Person> findByReusingPlaceholdersMultipleTimes(String arg0, String arg1);
@Query("{ 'arg0' : ?0, 'arg1' : ?1, 'arg2' : '?0' }")
List<Person> findByReusingPlaceholdersMultipleTimesWhenQuoted(String arg0, String arg1);
@Query("{ 'arg0' : '?0', 'arg1' : ?1, 'arg2' : '?0s' }")
List<Person> findByReusingPlaceholdersMultipleTimesWhenQuotedAndSomeStuffAppended(String arg0, String arg1);
@Query("{ 'arg0' : '?0', 'arg1' : '?1s' }")
List<Person> findByWhenQuotedAndSomeStuffAppended(String arg0, String arg1);
@Query("{ 'lastname' : { '$regex' : '^(?0|John ?1|?1)'} }") // use spel or some regex string this is fucking bad
Person findByLastnameRegex(String lastname, String alternative);
}
}