Skip to content

Commit 5f260fc

Browse files
christophstroblchristophstrobl
committed
Fix reference sorting via explicit sort clause
1 parent 8d4306b commit 5f260fc

File tree

4 files changed

+142
-31
lines changed

4 files changed

+142
-31
lines changed

Diff for: ‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ReferenceLookupDelegate.java

+98-28
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ public final class ReferenceLookupDelegate {
6363
private final SpELContext spELContext;
6464
private final ParameterBindingDocumentCodec codec;
6565

66+
/**
67+
* Create a new {@link ReferenceLookupDelegate}.
68+
*
69+
* @param mappingContext must not be {@literal null}.
70+
* @param spELContext must not be {@literal null}.
71+
*/
6672
public ReferenceLookupDelegate(
6773
MappingContext<? extends MongoPersistentEntity<?>, MongoPersistentProperty> mappingContext,
6874
SpELContext spELContext) {
@@ -75,8 +81,18 @@ public ReferenceLookupDelegate(
7581
this.codec = new ParameterBindingDocumentCodec();
7682
}
7783

84+
/**
85+
* Read the reference expressed by the given property.
86+
*
87+
* @param property the reference defining property. Must not be {@literal null}. THe
88+
* @param value the source value identifying to the referenced entity. Must not be {@literal null}.
89+
* @param lookupFunction to execute a lookup query. Must not be {@literal null}.
90+
* @param entityReader the callback to convert raw source values into actual domain types. Must not be
91+
* {@literal null}.
92+
* @return can be {@literal null}.
93+
*/
7894
@Nullable
79-
Object readReference(MongoPersistentProperty property, Object value, LookupFunction lookupFunction,
95+
public Object readReference(MongoPersistentProperty property, Object value, LookupFunction lookupFunction,
8096
MongoEntityReader entityReader) {
8197

8298
DocumentReferenceQuery filter = computeFilter(property, value, spELContext);
@@ -98,10 +114,12 @@ Object readReference(MongoPersistentProperty property, Object value, LookupFunct
98114
private ReferenceCollection computeReferenceContext(MongoPersistentProperty property, Object value,
99115
SpELContext spELContext) {
100116

117+
// Use the first value as a reference for others in case of collection like
101118
if (value instanceof Iterable) {
102119
value = ((Iterable<?>) value).iterator().next();
103120
}
104121

122+
// handle DBRef value
105123
if (value instanceof DBRef) {
106124
return ReferenceCollection.fromDBRef((DBRef) value);
107125
}
@@ -110,21 +128,21 @@ private ReferenceCollection computeReferenceContext(MongoPersistentProperty prop
110128

111129
if (value instanceof Document) {
112130

113-
Document ref = (Document) value;
131+
Document documentPointer = (Document) value;
114132

115133
if (property.isDocumentReference()) {
116134

117135
ParameterBindingContext bindingContext = bindingContext(property, value, spELContext);
118136
DocumentReference documentReference = property.getDocumentReference();
119137

120138
String targetDatabase = parseValueOrGet(documentReference.db(), bindingContext,
121-
() -> ref.get("db", String.class));
139+
() -> documentPointer.get("db", String.class));
122140
String targetCollection = parseValueOrGet(documentReference.collection(), bindingContext,
123-
() -> ref.get("collection", collection));
141+
() -> documentPointer.get("collection", collection));
124142
return new ReferenceCollection(targetDatabase, targetCollection);
125143
}
126144

127-
return new ReferenceCollection(ref.getString("db"), ref.get("collection", collection));
145+
return new ReferenceCollection(documentPointer.getString("db"), documentPointer.get("collection", collection));
128146
}
129147

130148
if (property.isDocumentReference()) {
@@ -141,19 +159,34 @@ private ReferenceCollection computeReferenceContext(MongoPersistentProperty prop
141159
return new ReferenceCollection(null, collection);
142160
}
143161

162+
/**
163+
* Use the given {@link ParameterBindingContext} to compute potential expressions against the value.
164+
*
165+
* @param value must not be {@literal null}.
166+
* @param bindingContext must not be {@literal null}.
167+
* @param defaultValue
168+
* @param <T>
169+
* @return can be {@literal null}.
170+
*/
171+
@Nullable
144172
@SuppressWarnings("unchecked")
145173
private <T> T parseValueOrGet(String value, ParameterBindingContext bindingContext, Supplier<T> defaultValue) {
146174

147175
if (!StringUtils.hasText(value)) {
148176
return defaultValue.get();
149177
}
150178

179+
// parameter binding requires a document, since we do not have one, construct it.
151180
if (!BsonUtils.isJsonDocument(value) && value.contains("?#{")) {
152181
String s = "{ 'target-value' : " + value + "}";
153182
T evaluated = (T) codec.decode(s, bindingContext).get("target-value");
154183
return evaluated != null ? evaluated : defaultValue.get();
155184
}
156185

186+
if(BsonUtils.isJsonDocument(value)) {
187+
return (T) codec.decode(value, bindingContext);
188+
}
189+
157190
T evaluated = (T) bindingContext.evaluateExpression(value);
158191
return evaluated != null ? evaluated : defaultValue.get();
159192
}
@@ -165,8 +198,8 @@ ParameterBindingContext bindingContext(MongoPersistentProperty property, Object
165198
}
166199

167200
ValueProvider valueProviderFor(Object source) {
168-
return (index) -> {
169201

202+
return (index) -> {
170203
if (source instanceof Document) {
171204
return Streamable.of(((Document) source).values()).toList().get(index);
172205
}
@@ -183,13 +216,22 @@ EvaluationContext evaluationContextFor(MongoPersistentProperty property, Object
183216
return ctx;
184217
}
185218

219+
/**
220+
* Compute the query to retrieve linked documents.
221+
*
222+
* @param property must not be {@literal null}.
223+
* @param value must not be {@literal null}.
224+
* @param spELContext must not be {@literal null}.
225+
* @return never {@literal null}.
226+
*/
186227
@SuppressWarnings("unchecked")
187228
DocumentReferenceQuery computeFilter(MongoPersistentProperty property, Object value, SpELContext spELContext) {
188229

189230
DocumentReference documentReference = property.getDocumentReference();
190231
String lookup = documentReference.lookup();
191232

192-
Document sort = parseValueOrGet(documentReference.sort(), bindingContext(property, value, spELContext), () -> null);
233+
Document sort = parseValueOrGet(documentReference.sort(), bindingContext(property, value, spELContext),
234+
() -> new Document());
193235

194236
if (property.isCollectionLike() && value instanceof Collection) {
195237

@@ -219,45 +261,59 @@ DocumentReferenceQuery computeFilter(MongoPersistentProperty property, Object va
219261
return new SingleDocumentReferenceQuery(codec.decode(lookup, bindingContext(property, value, spELContext)), sort);
220262
}
221263

264+
/**
265+
* {@link DocumentReferenceQuery} implementation fetching a single {@link Document}.
266+
*/
222267
static class SingleDocumentReferenceQuery implements DocumentReferenceQuery {
223268

224-
Document filter;
225-
Document sort;
269+
private final Document query;
270+
private final Document sort;
271+
272+
public SingleDocumentReferenceQuery(Document query, Document sort) {
226273

227-
public SingleDocumentReferenceQuery(Document filter, Document sort) {
228-
this.filter = filter;
274+
this.query = query;
229275
this.sort = sort;
230276
}
231277

232278
@Override
233279
public Bson getQuery() {
234-
return filter;
280+
return query;
281+
}
282+
283+
@Override
284+
public Document getSort() {
285+
return sort;
235286
}
236287

237288
@Override
238289
public Iterable<Document> apply(MongoCollection<Document> collection) {
239290

240-
Document result = collection.find(getQuery()).limit(1).first();
291+
Document result = collection.find(getQuery()).sort(getSort()).limit(1).first();
241292
return result != null ? Collections.singleton(result) : Collections.emptyList();
242293
}
243294
}
244295

296+
/**
297+
* {@link DocumentReferenceQuery} implementation to retrieve linked {@link Document documents} stored inside a
298+
* {@link Map} structure. Restores the original map order by matching individual query documents against the actual
299+
* values.
300+
*/
245301
static class MapDocumentReferenceQuery implements DocumentReferenceQuery {
246302

247-
private final Document filter;
303+
private final Document query;
248304
private final Document sort;
249305
private final Map<Object, Document> filterOrderMap;
250306

251-
public MapDocumentReferenceQuery(Document filter, Document sort, Map<Object, Document> filterOrderMap) {
307+
public MapDocumentReferenceQuery(Document query, Document sort, Map<Object, Document> filterOrderMap) {
252308

253-
this.filter = filter;
309+
this.query = query;
254310
this.sort = sort;
255311
this.filterOrderMap = filterOrderMap;
256312
}
257313

258314
@Override
259315
public Bson getQuery() {
260-
return filter;
316+
return query;
261317
}
262318

263319
@Override
@@ -283,33 +339,38 @@ public Iterable<Document> restoreOrder(Iterable<Document> documents) {
283339
}
284340
}
285341

342+
/**
343+
* {@link DocumentReferenceQuery} implementation to retrieve linked {@link Document documents} stored inside a
344+
* {@link Collection} like structure. Restores the original order by matching individual query documents against the
345+
* actual values.
346+
*/
286347
static class ListDocumentReferenceQuery implements DocumentReferenceQuery {
287348

288-
private final Document filter;
349+
private final Document query;
289350
private final Document sort;
290351

291-
public ListDocumentReferenceQuery(Document filter, Document sort) {
352+
public ListDocumentReferenceQuery(Document query, Document sort) {
292353

293-
this.filter = filter;
354+
this.query = query;
294355
this.sort = sort;
295356
}
296357

297358
@Override
298359
public Iterable<Document> restoreOrder(Iterable<Document> documents) {
299360

300-
if (filter.containsKey("$or")) {
301-
List<Document> ors = filter.get("$or", List.class);
302-
List<Document> target = documents instanceof List ? (List<Document>) documents
303-
: Streamable.of(documents).toList();
304-
return target.stream().sorted((o1, o2) -> compareAgainstReferenceIndex(ors, o1, o2))
305-
.collect(Collectors.toList());
361+
List<Document> target = documents instanceof List ? (List<Document>) documents
362+
: Streamable.of(documents).toList();
363+
364+
if (!sort.isEmpty() || !query.containsKey("$or")) {
365+
return target;
306366
}
307367

308-
return documents;
368+
List<Document> ors = query.get("$or", List.class);
369+
return target.stream().sorted((o1, o2) -> compareAgainstReferenceIndex(ors, o1, o2)).collect(Collectors.toList());
309370
}
310371

311372
public Document getQuery() {
312-
return filter;
373+
return query;
313374
}
314375

315376
@Override
@@ -333,9 +394,18 @@ int compareAgainstReferenceIndex(List<Document> referenceList, Document document
333394
}
334395
}
335396

397+
/**
398+
* The function that can execute a given {@link DocumentReferenceQuery} within the {@link ReferenceCollection} to
399+
* obtain raw results.
400+
*/
336401
@FunctionalInterface
337402
interface LookupFunction {
338403

404+
/**
405+
* @param referenceQuery never {@literal null}.
406+
* @param referenceCollection never {@literal null}.
407+
* @return never {@literal null}.
408+
*/
339409
Iterable<Document> apply(DocumentReferenceQuery referenceQuery, ReferenceCollection referenceCollection);
340410
}
341411
}

Diff for: ‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/BsonUtils.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,13 @@ public static String toJson(@Nullable Document source) {
349349
* @since 3.0
350350
*/
351351
public static boolean isJsonDocument(@Nullable String value) {
352-
return StringUtils.hasText(value) && (value.startsWith("{") && value.endsWith("}"));
352+
353+
if(!StringUtils.hasText(value)) {
354+
return false;
355+
}
356+
357+
String potentialJson = value.trim();
358+
return potentialJson.startsWith("{") && potentialJson.endsWith("}");
353359
}
354360

355361
/**

Diff for: ‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateDocumentReferenceTests.java

+32-1
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,33 @@ void readCollectionObjectReferenceFromDocumentDeclaringCollectionName() {
380380
new ObjectRefOfDocumentWithEmbeddedCollectionName("ref-1", "me-the-1-referenced-object"));
381381
}
382382

383+
@Test // GH-3602
384+
void useOrderFromAnnotatedSort() {
385+
386+
String rootCollectionName = template.getCollectionName(CollectionRefRoot.class);
387+
String refCollectionName = template.getCollectionName(SimpleObjectRef.class);
388+
Document refSource1 = new Document("_id", "ref-1").append("value", "me-the-1-referenced-object");
389+
Document refSource2 = new Document("_id", "ref-2").append("value", "me-the-2-referenced-object");
390+
Document refSource3 = new Document("_id", "ref-3").append("value", "me-the-3-referenced-object");
391+
Document source = new Document("_id", "id-1").append("value", "v1").append("simpleSortedValueRef",
392+
Arrays.asList("ref-1", "ref-3", "ref-2"));
393+
394+
template.execute(db -> {
395+
396+
db.getCollection(refCollectionName).insertOne(refSource1);
397+
db.getCollection(refCollectionName).insertOne(refSource2);
398+
db.getCollection(refCollectionName).insertOne(refSource3);
399+
db.getCollection(rootCollectionName).insertOne(source);
400+
return null;
401+
});
402+
403+
CollectionRefRoot result = template.findOne(query(where("id").is("id-1")), CollectionRefRoot.class);
404+
assertThat(result.getSimpleSortedValueRef()).containsExactly(
405+
new SimpleObjectRef("ref-3", "me-the-3-referenced-object"),
406+
new SimpleObjectRef("ref-2", "me-the-2-referenced-object"),
407+
new SimpleObjectRef("ref-1", "me-the-1-referenced-object"));
408+
}
409+
383410
@Test // GH-3602
384411
void readObjectReferenceFromDocumentNotRelatingToTheIdProperty() {
385412

@@ -857,7 +884,8 @@ void updateDerivedMappingFromLookup() {
857884

858885
template.save(book);
859886

860-
template.update(Book.class).matching(where("id").is(book.id)).apply(new Update().set("publisher", publisher)).first();
887+
template.update(Book.class).matching(where("id").is(book.id)).apply(new Update().set("publisher", publisher))
888+
.first();
861889

862890
Document target = template.execute(db -> {
863891
return db.getCollection(template.getCollectionName(Book.class)).find(Filters.eq("_id", book.id)).first();
@@ -930,6 +958,9 @@ static class CollectionRefRoot {
930958
@DocumentReference(lookup = "{ '_id' : '?#{#target}' }") //
931959
List<SimpleObjectRef> simpleValueRef;
932960

961+
@DocumentReference(lookup = "{ '_id' : '?#{#target}' }", sort = "{ '_id' : -1 } ") //
962+
List<SimpleObjectRef> simpleSortedValueRef;
963+
933964
@DocumentReference(lookup = "{ '_id' : '?#{#target}' }") //
934965
Map<String, SimpleObjectRef> mapValueRef;
935966

Diff for: ‎src/main/asciidoc/reference/mapping.adoc

+5-1
Original file line numberDiff line numberDiff line change
@@ -902,6 +902,10 @@ It is possible to alter resolution defaults (listed below) via the attributes of
902902
| The single document lookup query evaluating placeholders via SpEL expressions using `#target` as the marker for a given source value. `Collection` like or `Map` properties combine individual lookups via an `$or` operator.
903903
| An `_id` field based query (`{ '_id' : ?#{#target} }`) using the loaded source value.
904904

905+
| `sort`
906+
| Used for sorting result documents on server side.
907+
| None by default. Result order of `Collection` like properties is restored based on the used lookup query.
908+
905909
| `lazy`
906910
| If set to `true` value resolution is delayed upon first access of the property.
907911
| Resolves properties eagerly by default.
@@ -1182,7 +1186,7 @@ We know it is tempting to use all kinds of MongoDB query operators in the lookup
11821186
* Mind that resolution takes time and consider a lazy strategy.
11831187
* A collection of document references is bulk loaded using an `$or` operator. +
11841188
The original element order is restored in memory which cannot be done when using MongoDB query operators.
1185-
In this case Results will be ordered as they are received from the store.
1189+
In this case Results will be ordered as they are received from the store or via the provided `@DocumentReference(sort = ...)` attribute.
11861190
11871191
And a few more general remarks:
11881192

0 commit comments

Comments
 (0)