Skip to content

Commit dd9f0bc

Browse files
christophstroblmp911de
christophstrobl
authored and
mp911de
committed
Polishing.
Wrap no results predicate in dedicated lookup filter and make sure to omit loading attempt also for map properties. See #4612 Original pull request: #4613
1 parent 521fcb1 commit dd9f0bc

File tree

3 files changed

+80
-36
lines changed

3 files changed

+80
-36
lines changed

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ReferenceLoader.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.bson.Document;
2222
import org.bson.conversions.Bson;
2323
import org.springframework.data.mongodb.core.convert.ReferenceResolver.ReferenceCollection;
24+
import org.springframework.data.mongodb.core.mapping.FieldName;
2425
import org.springframework.lang.Nullable;
2526

2627
import com.mongodb.client.MongoCollection;
@@ -126,5 +127,30 @@ public Iterable<Document> apply(MongoCollection<Document> collection) {
126127
}
127128
};
128129
}
130+
131+
/**
132+
* @return a {@link DocumentReferenceQuery} that will not match any documents.
133+
* @since 4.3
134+
*/
135+
static DocumentReferenceQuery forNoResult() {
136+
return NoResultsFilter.INSTANCE;
137+
}
138+
}
139+
140+
/**
141+
* A dedicated {@link DocumentReferenceQuery} that will not match any documents.
142+
*
143+
* @since 4.3
144+
*/
145+
enum NoResultsFilter implements DocumentReferenceQuery {
146+
INSTANCE;
147+
148+
private static final Document NO_RESULTS_PREDICATE = new Document(FieldName.ID.name(),
149+
new Document("$exists", false));
150+
151+
@Override
152+
public Bson getQuery() {
153+
return NO_RESULTS_PREDICATE;
154+
}
129155
}
130156
}

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

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@
3434
import org.springframework.data.mapping.context.MappingContext;
3535
import org.springframework.data.mapping.model.SpELContext;
3636
import org.springframework.data.mongodb.core.convert.ReferenceLoader.DocumentReferenceQuery;
37+
import org.springframework.data.mongodb.core.convert.ReferenceLoader.NoResultsFilter;
3738
import org.springframework.data.mongodb.core.convert.ReferenceResolver.MongoEntityReader;
3839
import org.springframework.data.mongodb.core.convert.ReferenceResolver.ReferenceCollection;
3940
import org.springframework.data.mongodb.core.mapping.DocumentReference;
40-
import org.springframework.data.mongodb.core.mapping.FieldName;
4141
import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
4242
import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty;
4343
import org.springframework.data.mongodb.util.BsonUtils;
@@ -49,6 +49,7 @@
4949
import org.springframework.expression.EvaluationContext;
5050
import org.springframework.lang.Nullable;
5151
import org.springframework.util.Assert;
52+
import org.springframework.util.ObjectUtils;
5253
import org.springframework.util.StringUtils;
5354

5455
import com.mongodb.DBRef;
@@ -65,9 +66,6 @@
6566
*/
6667
public final class ReferenceLookupDelegate {
6768

68-
private static final Document NO_RESULTS_PREDICATE = new Document(FieldName.ID.name(),
69-
new Document("$exists", false));
70-
7169
private final MappingContext<? extends MongoPersistentEntity<?>, MongoPersistentProperty> mappingContext;
7270
private final SpELContext spELContext;
7371
private final ParameterBindingDocumentCodec codec;
@@ -104,18 +102,15 @@ public ReferenceLookupDelegate(
104102
public Object readReference(MongoPersistentProperty property, Object source, LookupFunction lookupFunction,
105103
MongoEntityReader entityReader) {
106104

107-
Object value = source instanceof DocumentReferenceSource documentReferenceSource ? documentReferenceSource.getTargetSource()
105+
Object value = source instanceof DocumentReferenceSource documentReferenceSource
106+
? documentReferenceSource.getTargetSource()
108107
: source;
109108

110-
// GH-4612 no need to query database if target collection is empty
111-
if (value != null && property.isCollectionLike() && (value instanceof Collection<?> c) && c.isEmpty()) {
112-
return new ArrayList<>();
113-
}
114-
115-
DocumentReferenceQuery filter = computeFilter(property, source, spELContext);
116-
ReferenceCollection referenceCollection = computeReferenceContext(property, value, spELContext);
109+
Iterable<Document> result = retrieveRawDocuments(property, source, lookupFunction, value);
117110

118-
Iterable<Document> result = lookupFunction.apply(filter, referenceCollection);
111+
if (result == null) {
112+
return null;
113+
}
119114

120115
if (property.isCollectionLike()) {
121116
return entityReader.read(result, property.getTypeInformation());
@@ -129,6 +124,19 @@ public Object readReference(MongoPersistentProperty property, Object source, Loo
129124
return resultValue != null ? entityReader.read(resultValue, property.getTypeInformation()) : null;
130125
}
131126

127+
@Nullable
128+
private Iterable<Document> retrieveRawDocuments(MongoPersistentProperty property, Object source,
129+
LookupFunction lookupFunction, Object value) {
130+
131+
DocumentReferenceQuery filter = computeFilter(property, source, spELContext);
132+
if (filter instanceof NoResultsFilter) {
133+
return Collections.emptyList();
134+
}
135+
136+
ReferenceCollection referenceCollection = computeReferenceContext(property, value, spELContext);
137+
return lookupFunction.apply(filter, referenceCollection);
138+
}
139+
132140
private ReferenceCollection computeReferenceContext(MongoPersistentProperty property, Object value,
133141
SpELContext spELContext) {
134142

@@ -278,7 +286,7 @@ DocumentReferenceQuery computeFilter(MongoPersistentProperty property, Object so
278286
Collection<Object> objects = (Collection<Object>) value;
279287

280288
if (objects.isEmpty()) {
281-
return new ListDocumentReferenceQuery(NO_RESULTS_PREDICATE, sort);
289+
return DocumentReferenceQuery.forNoResult();
282290
}
283291

284292
List<Document> ors = new ArrayList<>(objects.size());
@@ -293,11 +301,11 @@ DocumentReferenceQuery computeFilter(MongoPersistentProperty property, Object so
293301

294302
if (property.isMap() && value instanceof Map) {
295303

296-
Set<Entry<Object, Object>> entries = ((Map<Object, Object>) value).entrySet();
297-
if (entries.isEmpty()) {
298-
return new MapDocumentReferenceQuery(NO_RESULTS_PREDICATE, sort, Collections.emptyMap());
304+
if(ObjectUtils.isEmpty(value)) {
305+
return DocumentReferenceQuery.forNoResult();
299306
}
300307

308+
Set<Entry<Object, Object>> entries = ((Map<Object, Object>) value).entrySet();
301309
Map<Object, Document> filterMap = new LinkedHashMap<>(entries.size());
302310

303311
for (Entry<Object, Object> entry : entries) {
@@ -411,8 +419,7 @@ public Bson getSort() {
411419
public Iterable<Document> restoreOrder(Iterable<Document> documents) {
412420

413421
Map<String, Object> targetMap = new LinkedHashMap<>();
414-
List<Document> collected = documents instanceof List<Document> list ? list
415-
: Streamable.of(documents).toList();
422+
List<Document> collected = documents instanceof List<Document> list ? list : Streamable.of(documents).toList();
416423

417424
for (Entry<Object, Document> filterMapping : filterOrderMap.entrySet()) {
418425

@@ -444,8 +451,7 @@ public ListDocumentReferenceQuery(Document query, Document sort) {
444451
@Override
445452
public Iterable<Document> restoreOrder(Iterable<Document> documents) {
446453

447-
List<Document> target = documents instanceof List<Document> list ? list
448-
: Streamable.of(documents).toList();
454+
List<Document> target = documents instanceof List<Document> list ? list : Streamable.of(documents).toList();
449455

450456
if (!sort.isEmpty() || !query.containsKey("$or")) {
451457
return target;

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/ReferenceLookupDelegateUnitTests.java‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@
1515
*/
1616
package org.springframework.data.mongodb.core.convert;
1717

18+
import static org.assertj.core.api.Assertions.*;
19+
import static org.mockito.Mockito.*;
20+
21+
import java.util.Collections;
22+
1823
import org.junit.jupiter.api.BeforeEach;
1924
import org.junit.jupiter.api.Test;
2025
import org.junit.jupiter.api.extension.ExtendWith;
@@ -28,11 +33,6 @@
2833
import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty;
2934
import org.springframework.expression.EvaluationContext;
3035

31-
import java.util.Collections;
32-
33-
import static org.assertj.core.api.Assertions.assertThat;
34-
import static org.mockito.Mockito.*;
35-
3636
/**
3737
* Unit tests for {@link ReferenceLookupDelegate}.
3838
*
@@ -53,16 +53,6 @@ void beforeEach() {
5353
lookupDelegate = new ReferenceLookupDelegate(mappingContext, spELContext);
5454
}
5555

56-
@Test // GH-4612
57-
void shouldResolveEmptyListOnEmptyTargetCollection() {
58-
MongoPersistentProperty property = mock(MongoPersistentProperty.class);
59-
ReferenceLookupDelegate.LookupFunction lookupFunction = mock(ReferenceLookupDelegate.LookupFunction.class);
60-
61-
when(property.isCollectionLike()).thenReturn(true);
62-
lookupDelegate.readReference(property, Collections.emptyList(), lookupFunction, entityReader);
63-
verify(lookupFunction, never()).apply(any(), any());
64-
}
65-
6656
@Test // GH-3842
6757
void shouldComputePlainStringTargetCollection() {
6858

@@ -82,4 +72,26 @@ void shouldComputePlainStringTargetCollection() {
8272
return Collections.emptyList();
8373
}, entityReader);
8474
}
75+
76+
@Test // GH-4612
77+
void shouldResolveEmptyListOnEmptyTargetCollection() {
78+
79+
MongoPersistentProperty property = mock(MongoPersistentProperty.class);
80+
ReferenceLookupDelegate.LookupFunction lookupFunction = mock(ReferenceLookupDelegate.LookupFunction.class);
81+
82+
when(property.isCollectionLike()).thenReturn(true);
83+
lookupDelegate.readReference(property, Collections.emptyList(), lookupFunction, entityReader);
84+
verify(lookupFunction, never()).apply(any(), any());
85+
}
86+
87+
@Test // GH-4612
88+
void shouldResolveEmptyMapOnEmptyTargetCollection() {
89+
90+
MongoPersistentProperty property = mock(MongoPersistentProperty.class);
91+
ReferenceLookupDelegate.LookupFunction lookupFunction = mock(ReferenceLookupDelegate.LookupFunction.class);
92+
93+
when(property.isMap()).thenReturn(true);
94+
lookupDelegate.readReference(property, Collections.emptyMap(), lookupFunction, entityReader);
95+
verify(lookupFunction, never()).apply(any(), any());
96+
}
8597
}

0 commit comments

Comments
 (0)