Skip to content

Commit 50de1d6

Browse files
mp911demp911de
committed
Use consistently ParameterBindingDocumentCodec to parse queries and aggregations in AOT-generated code.
We now use ParameterBindingDocumentCodec instead of Document.parse(…) to reinstate lenient MQL parsing and to align with reflective behavior. Previously, we've used Document.parse(…) requiring a stricter syntax for e.g. values. Closes #5018
1 parent 4812760 commit 50de1d6

File tree

9 files changed

+41
-67
lines changed

9 files changed

+41
-67
lines changed

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/aot/MongoAotRepositoryFragmentSupport.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ protected MongoAotRepositoryFragmentSupport(MongoOperations mongoOperations, Rep
101101
it -> valueExpressions.createValueContextProvider(mongoParameters.get().get(it))));
102102
}
103103

104+
protected Document parse(String json) {
105+
return CODEC.decode(json);
106+
}
107+
104108
protected Document bindParameters(Method method, String source, Object... args) {
105109

106110
expandGeoShapes(args);

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/aot/MongoCodeBlocks.java‎

Lines changed: 6 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@
1515
*/
1616
package org.springframework.data.mongodb.repository.aot;
1717

18-
import java.util.Iterator;
19-
import java.util.Map;
20-
import java.util.Map.Entry;
2118
import java.util.regex.Pattern;
2219

2320
import org.bson.Document;
@@ -171,10 +168,10 @@ static CodeBlock asDocument(String source, String argNames) {
171168
Builder builder = CodeBlock.builder();
172169
if (!StringUtils.hasText(source)) {
173170
builder.add("new $T()", Document.class);
174-
} else if (!containsPlaceholder(source)) {
175-
builder.add("$T.parse($S)", Document.class, source);
176-
} else {
171+
} else if (containsPlaceholder(source)) {
177172
builder.add("bindParameters(ExpressionMarker.class.getEnclosingMethod(), $S, $L);\n", source, argNames);
173+
} else {
174+
builder.add("parse($S)", source);
178175
}
179176
return builder.build();
180177
}
@@ -185,47 +182,15 @@ static CodeBlock renderExpressionToDocument(@Nullable String source, String vari
185182
Builder builder = CodeBlock.builder();
186183
if (!StringUtils.hasText(source)) {
187184
builder.addStatement("$1T $2L = new $1T()", Document.class, variableName);
188-
} else if (!containsPlaceholder(source)) {
189-
builder.addStatement("$1T $2L = $1T.parse($3S)", Document.class, variableName, source);
190-
} else {
185+
} else if (containsPlaceholder(source)) {
191186
builder.add("$T $L = bindParameters(ExpressionMarker.class.getEnclosingMethod(), $S, $L);\n", Document.class,
192187
variableName, source, argNames);
188+
} else {
189+
builder.addStatement("$1T $2L = parse($3S)", Document.class, variableName, source);
193190
}
194191
return builder.build();
195192
}
196193

197-
static CodeBlock renderArgumentMap(Map<String, CodeBlock> arguments) {
198-
199-
Builder builder = CodeBlock.builder();
200-
builder.add("argumentMap(");
201-
Iterator<Entry<String, CodeBlock>> iterator = arguments.entrySet().iterator();
202-
while (iterator.hasNext()) {
203-
Entry<String, CodeBlock> next = iterator.next();
204-
builder.add("$S, ", next.getKey());
205-
builder.add(next.getValue());
206-
if (iterator.hasNext()) {
207-
builder.add(", ");
208-
}
209-
}
210-
builder.add(")");
211-
return builder.build();
212-
}
213-
214-
static CodeBlock renderArgumentArray(Map<String, CodeBlock> arguments) {
215-
216-
Builder builder = CodeBlock.builder();
217-
builder.add("arguments(");
218-
Iterator<CodeBlock> iterator = arguments.values().iterator();
219-
while (iterator.hasNext()) {
220-
builder.add(iterator.next());
221-
if (iterator.hasNext()) {
222-
builder.add(", ");
223-
}
224-
}
225-
builder.add(")");
226-
return builder.build();
227-
}
228-
229194
static CodeBlock evaluateNumberPotentially(String value, Class<? extends Number> targetType,
230195
AotQueryMethodGenerationContext context) {
231196
try {

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/aot/QueryBlocks.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -258,13 +258,14 @@ private CodeBlock renderExpressionToQuery() {
258258
String source = this.source.getQuery().getQueryString();
259259
if (!StringUtils.hasText(source)) {
260260
return CodeBlock.of("new $T(new $T())", BasicQuery.class, Document.class);
261+
} else if (MongoCodeBlocks.containsPlaceholder(source)) {
262+
Builder builder = CodeBlock.builder();
263+
builder.add("createQuery(ExpressionMarker.class.getEnclosingMethod(), $S, $L)", source, parameterNames);
264+
return builder.build();
261265
}
262-
if (!MongoCodeBlocks.containsPlaceholder(source)) {
263-
return CodeBlock.of("new $T($T.parse($S))", BasicQuery.class, Document.class, source);
266+
else {
267+
return CodeBlock.of("new $T(parse($S))", BasicQuery.class, source);
264268
}
265-
Builder builder = CodeBlock.builder();
266-
builder.add("createQuery(ExpressionMarker.class.getEnclosingMethod(), $S, $L)", source, parameterNames);
267-
return builder.build();
268269
}
269270
}
270271
}

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/aot/VectorSearchBlocks.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ private ExpressionSnippet getSort() {
165165
builder.add("($T) ($L) -> {\n", AggregationOperation.class, ctx);
166166
builder.indent();
167167

168-
builder.add("$1T $4L = $5L.getMappedObject($1T.parse($2S), $3T.class);\n", Document.class, filter.getSortString(),
168+
builder.add("$1T $4L = $5L.getMappedObject(parse($2S), $3T.class);\n", Document.class, filter.getSortString(),
169169
context.getActualReturnType().getType(), mappedSort, ctx);
170170
builder.add("return new $1T($2S, $3L.append(\"__score__\", -1));\n", Document.class, "$sort", mappedSort);
171171
builder.unindent();

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ public void encode(final BsonWriter writer, final Document document, final Encod
168168
}
169169

170170
// Spring Data Customization START
171-
public Document decode(@Nullable String json, Object[] values) {
171+
public Document decode(@Nullable String json, Object... values) {
172172

173173
return decode(json, new ParameterBindingContext((index) -> values[index], new SpelExpressionParser(),
174174
() -> EvaluationContextProvider.DEFAULT.getEvaluationContext(values)));
@@ -221,7 +221,7 @@ public Document decode(final BsonReader reader, final DecoderContext decoderCont
221221
return document;
222222
} else if (bindingReader.currentValue instanceof String stringValue) {
223223
try {
224-
return decode(stringValue, new Object[0]);
224+
return decode(stringValue);
225225
} catch (JsonParseException jsonParseException) {
226226
throw new IllegalArgumentException("Expression result is not a valid json document", jsonParseException);
227227
}

‎spring-data-mongodb/src/test/java/example/aot/UserRepository.java‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,7 @@
2626
import java.util.stream.Stream;
2727

2828
import org.springframework.data.annotation.Id;
29-
import org.springframework.data.domain.Limit;
30-
import org.springframework.data.domain.Page;
31-
import org.springframework.data.domain.Pageable;
32-
import org.springframework.data.domain.Range;
33-
import org.springframework.data.domain.Score;
34-
import org.springframework.data.domain.ScrollPosition;
35-
import org.springframework.data.domain.SearchResults;
36-
import org.springframework.data.domain.Similarity;
37-
import org.springframework.data.domain.Slice;
38-
import org.springframework.data.domain.Sort;
39-
import org.springframework.data.domain.Vector;
40-
import org.springframework.data.domain.Window;
29+
import org.springframework.data.domain.*;
4130
import org.springframework.data.geo.Box;
4231
import org.springframework.data.geo.Circle;
4332
import org.springframework.data.geo.Distance;
@@ -297,6 +286,9 @@ public interface UserRepository extends CrudRepository<User, String> {
297286
"{ '$project': { '_id' : '$last_name' } }" }, collation = "no_collation")
298287
List<String> findAllLastnamesWithCollation();
299288

289+
@Aggregation("{ $group : { _id : $customerId, total : { $sum : 1 } } }")
290+
List<OrdersPerCustomer> totalOrdersPerCustomer(Sort sort);
291+
300292
// Vector Search
301293

302294
@VectorSearch(indexName = "embedding.vector_cos", filter = "{lastname: ?0}", numCandidates = "#{10+10}",
@@ -362,4 +354,7 @@ public int hashCode() {
362354
return Objects.hash(lastname, names);
363355
}
364356
}
357+
358+
record OrdersPerCustomer(Object id, long total) {
359+
}
365360
}

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/aot/MongoRepositoryContributorTests.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
import org.bson.BsonString;
3232
import org.bson.Document;
33+
import org.bson.json.JsonParseException;
3334
import org.junit.jupiter.api.AfterEach;
3435
import org.junit.jupiter.api.BeforeAll;
3536
import org.junit.jupiter.api.BeforeEach;
@@ -688,6 +689,16 @@ void testAggregationWithCollation() {
688689
.withMessageContaining("'locale' is invalid");
689690
}
690691

692+
@Test // GH-5018
693+
void aggregationIsParsedLeniently() {
694+
695+
List<UserRepository.OrdersPerCustomer> result = fragment.totalOrdersPerCustomer(Sort.by("_id"));
696+
assertThat(result).hasSize(1);
697+
698+
assertThatExceptionOfType(JsonParseException.class)
699+
.isThrownBy(() -> Document.parse("{ $group : { _id : $customerId, total : { $sum : 1 } } }"));
700+
}
701+
691702
@Test // GH-5004
692703
void testNear() {
693704

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/aot/QueryMethodContributionUnitTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ void rendersVectorSearchOrderByWithScoreLast() throws NoSuchMethodException {
388388
.containsSubsequence("var $sort = ", //
389389
"(ctx) -> {", //
390390
"mappedSort = ctx.getMappedObject(", //
391-
"Document.parse(\"{\\\"firstname\\\": 1}\")", //
391+
"parse(\"{\\\"firstname\\\": 1}\")", //
392392
"Document(\"$sort\", mappedSort.append(\"__score__\", -1))");
393393
}
394394

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@
1515
*/
1616
package org.springframework.data.mongodb.util.json;
1717

18-
import static org.assertj.core.api.Assertions.assertThat;
19-
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
18+
import static org.assertj.core.api.Assertions.*;
2019

2120
import java.nio.charset.StandardCharsets;
2221
import java.util.Arrays;
@@ -31,6 +30,7 @@
3130
import org.bson.Document;
3231
import org.bson.codecs.DecoderContext;
3332
import org.junit.jupiter.api.Test;
33+
3434
import org.springframework.data.expression.ValueExpressionParser;
3535
import org.springframework.data.spel.EvaluationContextProvider;
3636
import org.springframework.data.spel.ExpressionDependencies;
@@ -635,9 +635,7 @@ void shouldParseUUIDasStandardRepresentation() {
635635
}
636636

637637
private static Document parse(String json, Object... args) {
638-
639-
ParameterBindingJsonReader reader = new ParameterBindingJsonReader(json, args);
640-
return new ParameterBindingDocumentCodec().decode(reader, DecoderContext.builder().build());
638+
return new ParameterBindingDocumentCodec().decode(json, args);
641639
}
642640

643641
// DATAMONGO-2545

0 commit comments

Comments
 (0)