Browse Source

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
issue/4755
Christoph Strobl 2 years ago committed by Mark Paluch
parent
commit
e2397450b0
No known key found for this signature in database
GPG Key ID: 55BC6374BAA9D973
  1. 26
      spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ReferenceLoader.java
  2. 48
      spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ReferenceLookupDelegate.java
  3. 42
      spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/ReferenceLookupDelegateUnitTests.java

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

@ -21,6 +21,7 @@ import java.util.Iterator; @@ -21,6 +21,7 @@ import java.util.Iterator;
import org.bson.Document;
import org.bson.conversions.Bson;
import org.springframework.data.mongodb.core.convert.ReferenceResolver.ReferenceCollection;
import org.springframework.data.mongodb.core.mapping.FieldName;
import org.springframework.lang.Nullable;
import com.mongodb.client.MongoCollection;
@ -126,5 +127,30 @@ public interface ReferenceLoader { @@ -126,5 +127,30 @@ public interface ReferenceLoader {
}
};
}
/**
* @return a {@link DocumentReferenceQuery} that will not match any documents.
* @since 4.3
*/
static DocumentReferenceQuery forNoResult() {
return NoResultsFilter.INSTANCE;
}
}
/**
* A dedicated {@link DocumentReferenceQuery} that will not match any documents.
*
* @since 4.3
*/
enum NoResultsFilter implements DocumentReferenceQuery {
INSTANCE;
private static final Document NO_RESULTS_PREDICATE = new Document(FieldName.ID.name(),
new Document("$exists", false));
@Override
public Bson getQuery() {
return NO_RESULTS_PREDICATE;
}
}
}

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

@ -34,10 +34,10 @@ import org.bson.conversions.Bson; @@ -34,10 +34,10 @@ import org.bson.conversions.Bson;
import org.springframework.data.mapping.context.MappingContext;
import org.springframework.data.mapping.model.SpELContext;
import org.springframework.data.mongodb.core.convert.ReferenceLoader.DocumentReferenceQuery;
import org.springframework.data.mongodb.core.convert.ReferenceLoader.NoResultsFilter;
import org.springframework.data.mongodb.core.convert.ReferenceResolver.MongoEntityReader;
import org.springframework.data.mongodb.core.convert.ReferenceResolver.ReferenceCollection;
import org.springframework.data.mongodb.core.mapping.DocumentReference;
import org.springframework.data.mongodb.core.mapping.FieldName;
import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty;
import org.springframework.data.mongodb.util.BsonUtils;
@ -49,6 +49,7 @@ import org.springframework.data.util.Streamable; @@ -49,6 +49,7 @@ import org.springframework.data.util.Streamable;
import org.springframework.expression.EvaluationContext;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;
import com.mongodb.DBRef;
@ -65,9 +66,6 @@ import com.mongodb.client.MongoCollection; @@ -65,9 +66,6 @@ import com.mongodb.client.MongoCollection;
*/
public final class ReferenceLookupDelegate {
private static final Document NO_RESULTS_PREDICATE = new Document(FieldName.ID.name(),
new Document("$exists", false));
private final MappingContext<? extends MongoPersistentEntity<?>, MongoPersistentProperty> mappingContext;
private final SpELContext spELContext;
private final ParameterBindingDocumentCodec codec;
@ -104,18 +102,15 @@ public final class ReferenceLookupDelegate { @@ -104,18 +102,15 @@ public final class ReferenceLookupDelegate {
public Object readReference(MongoPersistentProperty property, Object source, LookupFunction lookupFunction,
MongoEntityReader entityReader) {
Object value = source instanceof DocumentReferenceSource documentReferenceSource ? documentReferenceSource.getTargetSource()
Object value = source instanceof DocumentReferenceSource documentReferenceSource
? documentReferenceSource.getTargetSource()
: source;
// GH-4612 no need to query database if target collection is empty
if (value != null && property.isCollectionLike() && (value instanceof Collection<?> c) && c.isEmpty()) {
return new ArrayList<>();
}
DocumentReferenceQuery filter = computeFilter(property, source, spELContext);
ReferenceCollection referenceCollection = computeReferenceContext(property, value, spELContext);
Iterable<Document> result = retrieveRawDocuments(property, source, lookupFunction, value);
Iterable<Document> result = lookupFunction.apply(filter, referenceCollection);
if (result == null) {
return null;
}
if (property.isCollectionLike()) {
return entityReader.read(result, property.getTypeInformation());
@ -129,6 +124,19 @@ public final class ReferenceLookupDelegate { @@ -129,6 +124,19 @@ public final class ReferenceLookupDelegate {
return resultValue != null ? entityReader.read(resultValue, property.getTypeInformation()) : null;
}
@Nullable
private Iterable<Document> retrieveRawDocuments(MongoPersistentProperty property, Object source,
LookupFunction lookupFunction, Object value) {
DocumentReferenceQuery filter = computeFilter(property, source, spELContext);
if (filter instanceof NoResultsFilter) {
return Collections.emptyList();
}
ReferenceCollection referenceCollection = computeReferenceContext(property, value, spELContext);
return lookupFunction.apply(filter, referenceCollection);
}
private ReferenceCollection computeReferenceContext(MongoPersistentProperty property, Object value,
SpELContext spELContext) {
@ -278,7 +286,7 @@ public final class ReferenceLookupDelegate { @@ -278,7 +286,7 @@ public final class ReferenceLookupDelegate {
Collection<Object> objects = (Collection<Object>) value;
if (objects.isEmpty()) {
return new ListDocumentReferenceQuery(NO_RESULTS_PREDICATE, sort);
return DocumentReferenceQuery.forNoResult();
}
List<Document> ors = new ArrayList<>(objects.size());
@ -293,11 +301,11 @@ public final class ReferenceLookupDelegate { @@ -293,11 +301,11 @@ public final class ReferenceLookupDelegate {
if (property.isMap() && value instanceof Map) {
Set<Entry<Object, Object>> entries = ((Map<Object, Object>) value).entrySet();
if (entries.isEmpty()) {
return new MapDocumentReferenceQuery(NO_RESULTS_PREDICATE, sort, Collections.emptyMap());
if(ObjectUtils.isEmpty(value)) {
return DocumentReferenceQuery.forNoResult();
}
Set<Entry<Object, Object>> entries = ((Map<Object, Object>) value).entrySet();
Map<Object, Document> filterMap = new LinkedHashMap<>(entries.size());
for (Entry<Object, Object> entry : entries) {
@ -411,8 +419,7 @@ public final class ReferenceLookupDelegate { @@ -411,8 +419,7 @@ public final class ReferenceLookupDelegate {
public Iterable<Document> restoreOrder(Iterable<Document> documents) {
Map<String, Object> targetMap = new LinkedHashMap<>();
List<Document> collected = documents instanceof List<Document> list ? list
: Streamable.of(documents).toList();
List<Document> collected = documents instanceof List<Document> list ? list : Streamable.of(documents).toList();
for (Entry<Object, Document> filterMapping : filterOrderMap.entrySet()) {
@ -444,8 +451,7 @@ public final class ReferenceLookupDelegate { @@ -444,8 +451,7 @@ public final class ReferenceLookupDelegate {
@Override
public Iterable<Document> restoreOrder(Iterable<Document> documents) {
List<Document> target = documents instanceof List<Document> list ? list
: Streamable.of(documents).toList();
List<Document> target = documents instanceof List<Document> list ? list : Streamable.of(documents).toList();
if (!sort.isEmpty() || !query.containsKey("$or")) {
return target;

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

@ -15,6 +15,11 @@ @@ -15,6 +15,11 @@
*/
package org.springframework.data.mongodb.core.convert;
import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;
import java.util.Collections;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@ -28,11 +33,6 @@ import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; @@ -28,11 +33,6 @@ import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty;
import org.springframework.expression.EvaluationContext;
import java.util.Collections;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.*;
/**
* Unit tests for {@link ReferenceLookupDelegate}.
*
@ -53,16 +53,6 @@ class ReferenceLookupDelegateUnitTests { @@ -53,16 +53,6 @@ class ReferenceLookupDelegateUnitTests {
lookupDelegate = new ReferenceLookupDelegate(mappingContext, spELContext);
}
@Test // GH-4612
void shouldResolveEmptyListOnEmptyTargetCollection() {
MongoPersistentProperty property = mock(MongoPersistentProperty.class);
ReferenceLookupDelegate.LookupFunction lookupFunction = mock(ReferenceLookupDelegate.LookupFunction.class);
when(property.isCollectionLike()).thenReturn(true);
lookupDelegate.readReference(property, Collections.emptyList(), lookupFunction, entityReader);
verify(lookupFunction, never()).apply(any(), any());
}
@Test // GH-3842
void shouldComputePlainStringTargetCollection() {
@ -82,4 +72,26 @@ class ReferenceLookupDelegateUnitTests { @@ -82,4 +72,26 @@ class ReferenceLookupDelegateUnitTests {
return Collections.emptyList();
}, entityReader);
}
@Test // GH-4612
void shouldResolveEmptyListOnEmptyTargetCollection() {
MongoPersistentProperty property = mock(MongoPersistentProperty.class);
ReferenceLookupDelegate.LookupFunction lookupFunction = mock(ReferenceLookupDelegate.LookupFunction.class);
when(property.isCollectionLike()).thenReturn(true);
lookupDelegate.readReference(property, Collections.emptyList(), lookupFunction, entityReader);
verify(lookupFunction, never()).apply(any(), any());
}
@Test // GH-4612
void shouldResolveEmptyMapOnEmptyTargetCollection() {
MongoPersistentProperty property = mock(MongoPersistentProperty.class);
ReferenceLookupDelegate.LookupFunction lookupFunction = mock(ReferenceLookupDelegate.LookupFunction.class);
when(property.isMap()).thenReturn(true);
lookupDelegate.readReference(property, Collections.emptyMap(), lookupFunction, entityReader);
verify(lookupFunction, never()).apply(any(), any());
}
}

Loading…
Cancel
Save