From a2127a4da9bc4292c16abee05f84839e0360e2bb Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 8 Mar 2023 14:36:14 +0100 Subject: [PATCH] Allow reading already resolved references. This commit adds the ability to read (eg. by an aggregation $lookup) already fully resolved references between documents. No proxy will be created for lazy loading references and we'll also skip the additional server roundtrip to load the reference by its id. Closes #4312 Original pull request: #4323 --- .../core/convert/MappingMongoConverter.java | 27 +++- .../convert/MappingMongoConverterTests.java | 118 ++++++++++++++++++ 2 files changed, 143 insertions(+), 2 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java index fe1882dfd..d9c2be9b2 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java @@ -649,9 +649,32 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return; } - DBRef dbref = value instanceof DBRef ? (DBRef) value : null; + if (value instanceof DBRef dbref) { + accessor.setProperty(property, dbRefResolver.resolveDbRef(property, dbref, callback, handler)); + return; + } - accessor.setProperty(property, dbRefResolver.resolveDbRef(property, dbref, callback, handler)); + /* + * The value might be a pre resolved full document (eg. resulting from an aggregation $lookup). + * In this case we try to map that object to the target type without an additional step ($dbref resolution server roundtrip) + * in between. + */ + if (value instanceof Document document) { + if(property.isMap()) { + if(document.isEmpty() || document.values().iterator().next() instanceof DBRef) { + accessor.setProperty(property, dbRefResolver.resolveDbRef(property, null, callback, handler)); + } else { + accessor.setProperty(property, readMap(context, document, property.getTypeInformation())); + } + } else { + accessor.setProperty(property, read(property.getActualType(), document)); + } + } else if (value instanceof Collection collection && collection.size() > 0 + && collection.iterator().next() instanceof Document) { + accessor.setProperty(property, readCollectionOrArray(context, collection, property.getTypeInformation())); + } else { + accessor.setProperty(property, dbRefResolver.resolveDbRef(property, null, callback, handler)); + } } @Nullable diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterTests.java index 03492f169..a6f26565a 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterTests.java @@ -33,6 +33,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import org.bson.Document; import org.junit.jupiter.api.BeforeEach; @@ -110,6 +111,98 @@ public class MappingMongoConverterTests { verify(dbRefResolver).bulkFetch(any()); } + @Test // GH-4312 + void conversionShouldAllowReadingAlreadyResolvedReferences() { + + Document sampleSource = new Document("_id", "sample-1").append("value", "one"); + Document source = new Document("_id", "id-1").append("sample", sampleSource); + + WithSingleValueDbRef read = converter.read(WithSingleValueDbRef.class, source); + + assertThat(read.sample).isEqualTo(converter.read(Sample.class, sampleSource)); + verifyNoInteractions(dbRefResolver); + } + + @Test // GH-4312 + void conversionShouldAllowReadingAlreadyResolvedListOfReferences() { + + Document sample1Source = new Document("_id", "sample-1").append("value", "one"); + Document sample2Source = new Document("_id", "sample-2").append("value", "two"); + Document source = new Document("_id", "id-1").append("lazyList", List.of(sample1Source, sample2Source)); + + WithLazyDBRef read = converter.read(WithLazyDBRef.class, source); + + assertThat(read.lazyList).containsExactly(converter.read(Sample.class, sample1Source), + converter.read(Sample.class, sample2Source)); + verifyNoInteractions(dbRefResolver); + } + + @Test // GH-4312 + void conversionShouldAllowReadingAlreadyResolvedMapOfReferences() { + + Document sample1Source = new Document("_id", "sample-1").append("value", "one"); + Document sample2Source = new Document("_id", "sample-2").append("value", "two"); + Document source = new Document("_id", "id-1").append("sampleMap", + new Document("s1", sample1Source).append("s2", sample2Source)); + + WithMapValueDbRef read = converter.read(WithMapValueDbRef.class, source); + + assertThat(read.sampleMap) // + .containsEntry("s1", converter.read(Sample.class, sample1Source)) // + .containsEntry("s2", converter.read(Sample.class, sample2Source)); + verifyNoInteractions(dbRefResolver); + } + + @Test // GH-4312 + void conversionShouldAllowReadingAlreadyResolvedMapOfLazyReferences() { + + Document sample1Source = new Document("_id", "sample-1").append("value", "one"); + Document sample2Source = new Document("_id", "sample-2").append("value", "two"); + Document source = new Document("_id", "id-1").append("sampleMapLazy", + new Document("s1", sample1Source).append("s2", sample2Source)); + + WithMapValueDbRef read = converter.read(WithMapValueDbRef.class, source); + + assertThat(read.sampleMapLazy) // + .containsEntry("s1", converter.read(Sample.class, sample1Source)) // + .containsEntry("s2", converter.read(Sample.class, sample2Source)); + verifyNoInteractions(dbRefResolver); + } + + @Test // GH-4312 + void resolvesLazyDBRefMapOnAccess() { + + client.getDatabase(DATABASE).getCollection("samples") + .insertMany(Arrays.asList(new Document("_id", "sample-1").append("value", "one"), + new Document("_id", "sample-2").append("value", "two"))); + + Document source = new Document("_id", "id-1").append("sampleMapLazy", + new Document("s1", new com.mongodb.DBRef("samples", "sample-1")).append("s2", + new com.mongodb.DBRef("samples", "sample-2"))); + + WithMapValueDbRef target = converter.read(WithMapValueDbRef.class, source); + + verify(dbRefResolver).resolveDbRef(any(), isNull(), any(), any()); + + assertThat(target.sampleMapLazy).isInstanceOf(LazyLoadingProxy.class); + assertThat(target.getSampleMapLazy()).containsEntry("s1", new Sample("sample-1", "one")).containsEntry("s2", + new Sample("sample-2", "two")); + + verify(dbRefResolver).bulkFetch(any()); + } + + @Test // GH-4312 + void conversionShouldAllowReadingAlreadyResolvedLazyReferences() { + + Document sampleSource = new Document("_id", "sample-1").append("value", "one"); + Document source = new Document("_id", "id-1").append("sampleLazy", sampleSource); + + WithSingleValueDbRef read = converter.read(WithSingleValueDbRef.class, source); + + assertThat(read.sampleLazy).isEqualTo(converter.read(Sample.class, sampleSource)); + verifyNoInteractions(dbRefResolver); + } + @Test // DATAMONGO-2004 void resolvesLazyDBRefConstructorArgOnAccess() { @@ -164,6 +257,31 @@ public class MappingMongoConverterTests { } } + @Data + public static class WithSingleValueDbRef { + + @Id // + String id; + + @DBRef // + Sample sample; + + @DBRef(lazy = true) // + Sample sampleLazy; + } + + @Data + public static class WithMapValueDbRef { + + @Id String id; + + @DBRef // + Map sampleMap; + + @DBRef(lazy = true) // + Map sampleMapLazy; + } + public static class WithLazyDBRefAsConstructorArg { @Id String id;