Skip to content

Commit 885d059

Browse files
Revert query modification in json parsing tests.
Add tests and move json string treatment into the ParameterBindingDocumentCodec. Finally add issue references and format code. Original Pull Request: #3907
1 parent a8a0fb5 commit 885d059

File tree

3 files changed

+75
-31
lines changed

3 files changed

+75
-31
lines changed

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/json/ParameterBindingDocumentCodec.java

+8-1
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,15 @@ public Document decode(final BsonReader reader, final DecoderContext decoderCont
217217
// binds just placeholder queries like: `@Query(?0)`
218218
if (bindingReader.currentValue instanceof org.bson.Document) {
219219
return (Document) bindingReader.currentValue;
220+
} else if (bindingReader.currentValue instanceof String) {
221+
try {
222+
return decode((String) bindingReader.currentValue, new Object[0]);
223+
} catch (JsonParseException jsonParseException) {
224+
throw new IllegalArgumentException("Expression result is not a valid json document!", jsonParseException);
225+
}
226+
} else if (bindingReader.currentValue instanceof Map) {
227+
return new Document((Map) bindingReader.currentValue);
220228
}
221-
222229
}
223230

224231
Document document = new Document();

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/json/ParameterBindingJsonReader.java

+1-14
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import org.bson.types.MaxKey;
3737
import org.bson.types.MinKey;
3838
import org.bson.types.ObjectId;
39-
4039
import org.springframework.data.spel.EvaluationContextProvider;
4140
import org.springframework.expression.EvaluationContext;
4241
import org.springframework.expression.spel.standard.SpelExpressionParser;
@@ -120,20 +119,8 @@ public ParameterBindingJsonReader(String json, ParameterBindingContext bindingCo
120119
Matcher matcher = ENTIRE_QUERY_BINDING_PATTERN.matcher(json);
121120
if (matcher.find()) {
122121
BindableValue bindingResult = bindableValueFor(new JsonToken(JsonTokenType.UNQUOTED_STRING, json));
123-
try {
124-
125-
if (bindingResult.getValue() instanceof String) {
126-
currentValue = Document.parse((String)bindingResult.getValue());
127-
} else {
128-
currentValue = bindingResult.getValue();
129-
}
130-
131-
} catch (JsonParseException jsonParseException) {
132-
throw new IllegalArgumentException(
133-
String.format("Resulting value of expression '%s' is not a valid json query", json), jsonParseException);
134-
}
122+
currentValue = bindingResult.getValue();
135123
}
136-
137124
}
138125

139126
// Spring Data Customization END

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/ParameterBindingJsonReaderUnitTests.java

+66-16
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.Date;
2727
import java.util.List;
2828

29+
import org.bson.BsonBinary;
2930
import org.bson.Document;
3031
import org.bson.codecs.DecoderContext;
3132
import org.junit.jupiter.api.Test;
@@ -347,7 +348,7 @@ void evaluatesSpelExpressionDefiningEntireQuery() {
347348
evaluationContext.setRootObject(new DummySecurityObject(new DummyWithId("wonderwoman")));
348349

349350
String json = "?#{ T(" + this.getClass().getName()
350-
+ ").isBatman() ? \"{'_class': { '$eq' : 'region' }}\" : \"{ '$and' : [ {'_class': { '$eq' : 'region' } }, {'user.supervisor': '\"+ principal.id +\"' } ] }\" }";
351+
+ ").isBatman() ? {'_class': { '$eq' : 'region' }} : { '$and' : { {'_class': { '$eq' : 'region' } }, {'user.supervisor': principal.id } } } }";
351352

352353
ParameterBindingJsonReader reader = new ParameterBindingJsonReader(json,
353354
new ParameterBindingContext((index) -> args[index], new SpelExpressionParser(), evaluationContext));
@@ -357,11 +358,66 @@ void evaluatesSpelExpressionDefiningEntireQuery() {
357358
.isEqualTo(new Document("$and", Arrays.asList(new Document("_class", new Document("$eq", "region")),
358359
new Document("user.supervisor", "wonderwoman"))));
359360
}
360-
361-
@Test
361+
362+
@Test // GH-3871
363+
public void capturingExpressionDependenciesShouldNotThrowParseErrorForSpelOnlyJson() {
364+
365+
Object[] args = new Object[] { "1", "2" };
366+
String json = "?#{ true ? { 'name': #name } : { 'name' : #name + 'trouble' } }";
367+
368+
new ParameterBindingDocumentCodec().captureExpressionDependencies(json, (index) -> args[index],
369+
new SpelExpressionParser());
370+
}
371+
372+
@Test // GH-3871
373+
public void bindEntireQueryUsingSpelExpressionWhenEvaluationResultIsJsonString() {
374+
375+
Object[] args = new Object[] { "expected", "unexpected" };
376+
String json = "?#{ true ? \"{ 'name': ?0 }\" : \"{ 'name' : ?1 }\" }";
377+
StandardEvaluationContext evaluationContext = (StandardEvaluationContext) EvaluationContextProvider.DEFAULT
378+
.getEvaluationContext(args);
379+
380+
ParameterBindingJsonReader reader = new ParameterBindingJsonReader(json,
381+
new ParameterBindingContext((index) -> args[index], new SpelExpressionParser(), evaluationContext));
382+
383+
Document target = new ParameterBindingDocumentCodec().decode(reader, DecoderContext.builder().build());
384+
assertThat(target).isEqualTo(new Document("name", "expected"));
385+
}
386+
387+
@Test // GH-3871
388+
public void throwsExceptionWhenbindEntireQueryUsingSpelExpressionResultsInInvalidJsonString() {
389+
390+
Object[] args = new Object[] { "expected", "unexpected" };
391+
String json = "?#{ true ? \"{ 'name': ?0 { }\" : \"{ 'name' : ?1 }\" }";
392+
StandardEvaluationContext evaluationContext = (StandardEvaluationContext) EvaluationContextProvider.DEFAULT
393+
.getEvaluationContext(args);
394+
395+
ParameterBindingJsonReader reader = new ParameterBindingJsonReader(json,
396+
new ParameterBindingContext((index) -> args[index], new SpelExpressionParser(), evaluationContext));
397+
398+
assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> new ParameterBindingDocumentCodec().decode(reader, DecoderContext.builder().build()));
399+
}
400+
401+
@Test // GH-3871
402+
public void bindEntireQueryUsingSpelExpressionWhenEvaluationResultIsJsonStringContainingUUID() {
403+
404+
Object[] args = new Object[] { "UUID('cfbca728-4e39-4613-96bc-f920b5c37e16')", "unexpected" };
405+
String json = "?#{ true ? \"{ 'name': ?0 }\" : \"{ 'name' : ?1 }\" }";
406+
StandardEvaluationContext evaluationContext = (StandardEvaluationContext) EvaluationContextProvider.DEFAULT
407+
.getEvaluationContext(args);
408+
409+
ParameterBindingJsonReader reader = new ParameterBindingJsonReader(json,
410+
new ParameterBindingContext((index) -> args[index], new SpelExpressionParser(), evaluationContext));
411+
412+
Document target = new ParameterBindingDocumentCodec().decode(reader, DecoderContext.builder().build());
413+
414+
assertThat(target.get("name")).isInstanceOf(BsonBinary.class);
415+
}
416+
417+
@Test // GH-3871
362418
void bindEntireQueryUsingSpelExpression() {
363419

364-
Object[] args = new Object[] {"region"};
420+
Object[] args = new Object[] { "region" };
365421
StandardEvaluationContext evaluationContext = (StandardEvaluationContext) EvaluationContextProvider.DEFAULT
366422
.getEvaluationContext(args);
367423
evaluationContext.setRootObject(new DummySecurityObject(new DummyWithId("wonderwoman")));
@@ -377,10 +433,10 @@ void bindEntireQueryUsingSpelExpression() {
377433
new Document("user.supervisor", "wonderwoman"))));
378434
}
379435

380-
@Test
436+
@Test // GH-3871
381437
void bindEntireQueryUsingParameter() {
382438

383-
Object[] args = new Object[] {"{ 'itWorks' : true }"};
439+
Object[] args = new Object[] { "{ 'itWorks' : true }" };
384440
StandardEvaluationContext evaluationContext = (StandardEvaluationContext) EvaluationContextProvider.DEFAULT
385441
.getEvaluationContext(args);
386442

@@ -390,9 +446,7 @@ void bindEntireQueryUsingParameter() {
390446
new ParameterBindingContext((index) -> args[index], new SpelExpressionParser(), evaluationContext));
391447
Document target = new ParameterBindingDocumentCodec().decode(reader, DecoderContext.builder().build());
392448

393-
assertThat(target)
394-
.isEqualTo(new Document("itWorks", true));
395-
449+
assertThat(target).isEqualTo(new Document("itWorks", true));
396450
}
397451

398452
@Test // DATAMONGO-2571
@@ -437,17 +491,13 @@ private static Document parse(String json, Object... args) {
437491
public static boolean isBatman() {
438492
return false;
439493
}
440-
494+
441495
public static String applyFilterByUser(String _class, String username) {
442496
switch (username) {
443497
case "batman":
444-
return "{'_class': { '$eq' : '"
445-
+ _class
446-
+ "' }}";
498+
return "{'_class': { '$eq' : '" + _class + "' }}";
447499
default:
448-
return "{ '$and' : [ {'_class': { '$eq' : '"
449-
+ _class
450-
+ "' } }, {'user.supervisor': '" + username + "' } ] }";
500+
return "{ '$and' : [ {'_class': { '$eq' : '" + _class + "' } }, {'user.supervisor': '" + username + "' } ] }";
451501
}
452502
}
453503

0 commit comments

Comments
 (0)