diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ReferenceLookupDelegate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ReferenceLookupDelegate.java index a0e5a6f2bb..088b27e70b 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ReferenceLookupDelegate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ReferenceLookupDelegate.java @@ -116,7 +116,7 @@ public ReferenceLookupDelegate( } if (!result.iterator().hasNext()) { - return null; + return property.isMap() ? Collections.emptyMap() : null; } Object resultValue = result.iterator().next(); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DefaultDbRefResolverUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DefaultDbRefResolverUnitTests.java index 75c7cc4366..ae54c8afa4 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DefaultDbRefResolverUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DefaultDbRefResolverUnitTests.java @@ -21,10 +21,12 @@ import java.util.Arrays; import java.util.Collections; +import java.util.Map; import org.bson.Document; import org.bson.types.ObjectId; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; @@ -35,6 +37,8 @@ import org.mockito.quality.Strictness; import org.springframework.dao.InvalidDataAccessApiUsageException; +import org.springframework.data.mapping.context.MappingContext; +import org.springframework.data.mapping.model.SpELContext; import org.springframework.data.mongodb.MongoDatabaseFactory; import org.springframework.data.mongodb.core.DocumentTestUtils; import org.springframework.data.mongodb.core.MongoExceptionTranslator; @@ -43,6 +47,9 @@ import com.mongodb.client.FindIterable; import com.mongodb.client.MongoCollection; import com.mongodb.client.MongoDatabase; +import org.springframework.data.mongodb.core.mapping.DocumentReference; +import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; +import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; /** * Unit tests for {@link DefaultDbRefResolver}. @@ -58,6 +65,8 @@ class DefaultDbRefResolverUnitTests { @Mock MongoDatabase dbMock; @Mock MongoCollection collectionMock; @Mock FindIterable cursorMock; + @Mock MappingContext, MongoPersistentProperty> mappingContext; + @Mock SpELContext spELContext; private DefaultDbRefResolver resolver; @BeforeEach @@ -91,7 +100,7 @@ void bulkFetchShouldLoadDbRefsCorrectly() { } @Test // DATAMONGO-1194 - void bulkFetchShouldThrowExceptionWhenUsingDifferntCollectionsWithinSetOfReferences() { + void bulkFetchShouldThrowExceptionWhenUsingDifferentCollectionsWithinSetOfReferences() { DBRef ref1 = new DBRef("collection-1", new ObjectId()); DBRef ref2 = new DBRef("collection-2", new ObjectId()); @@ -134,4 +143,75 @@ void bulkFetchContainsDuplicates() { assertThat(resolver.bulkFetch(Arrays.asList(ref1, ref2))).containsExactly(document, document); } + + @Test // GH-5065 + @DisplayName("GH-5065: Empty Map with @DocumentReference annotation should deserialize to an empty map.") + void resolveEmptyMapIsNotNull() { + DocumentReference documentReference = mock(DocumentReference.class); + when(documentReference.lookup()).thenReturn("{ '_id' : ?#{#target} }"); + when(documentReference.sort()).thenReturn(""); + when(documentReference.lazy()).thenReturn(false); + MongoPersistentProperty property = mock(MongoPersistentProperty.class); + when(property.isCollectionLike()).thenReturn(false); + when(property.isMap()).thenReturn(true); + when(property.isDocumentReference()).thenReturn(true); + when(property.getDocumentReference()).thenReturn(documentReference); + DocumentReferenceSource source = mock(DocumentReferenceSource.class); + when(source.getTargetSource()).thenReturn(Document.parse("{}")); + ReferenceLookupDelegate lookupDelegate = new ReferenceLookupDelegate(mappingContext, spELContext); + + ReferenceResolver.MongoEntityReader entityReader = mock(ReferenceResolver.MongoEntityReader.class); + + Object target = resolver.resolveReference(property, source, lookupDelegate, entityReader); + + verify(property, times(3)).isMap(); + verify(property, times(2)).isDocumentReference(); + verify(property, times(2)).getDocumentReference(); + verify(property, times(3)).isCollectionLike(); + verify(documentReference, times(1)).lookup(); + verify(documentReference, times(1)).sort(); + verify(documentReference, times(1)).lazy(); + verify(source, times(3)).getTargetSource(); + verifyNoMoreInteractions(documentReference, property, source); // Make sure we only call the properties we mocked. + assertThat(target) + .isNotNull() + .isInstanceOf(Map.class); + } + + @Test // GH-5065 + @DisplayName("GH-5065: Lazy loaded empty Map with @DocumentReference annotation should deserialize to an empty map with a non-null values property.") + void resolveLazyLoadedEmptyMapIsNotNull() { + DocumentReference documentReference = mock(DocumentReference.class); + when(documentReference.lookup()).thenReturn("{ '_id' : ?#{#target} }"); + when(documentReference.sort()).thenReturn(""); + when(documentReference.lazy()).thenReturn(true); + MongoPersistentProperty property = mock(MongoPersistentProperty.class); + when(property.isCollectionLike()).thenReturn(false); + when(property.isMap()).thenReturn(true); + when(property.isDocumentReference()).thenReturn(true); + when(property.getDocumentReference()).thenReturn(documentReference); + //noinspection rawtypes,unchecked + when(property.getType()).thenReturn((Class) Map.class); + DocumentReferenceSource source = mock(DocumentReferenceSource.class); + when(source.getTargetSource()).thenReturn(Document.parse("{}")); + ReferenceLookupDelegate lookupDelegate = new ReferenceLookupDelegate(mappingContext, spELContext); + + ReferenceResolver.MongoEntityReader entityReader = mock(ReferenceResolver.MongoEntityReader.class); + + Object target = resolver.resolveReference(property, source, lookupDelegate, entityReader); + + verify(property, times(1)).isMap(); + verify(property, times(1)).isDocumentReference(); + verify(property, times(1)).getDocumentReference(); + verify(property, times(1)).isCollectionLike(); + verify(property, times(1)).getType(); + verify(documentReference, times(1)).lazy(); + verify(source, times(1)).getTargetSource(); + verifyNoMoreInteractions(documentReference, property, source); // Make sure we only call the properties we mocked. + + assertThat(target) + .isNotNull() + .isInstanceOf(Map.class) + .asInstanceOf(MAP).values().isNotNull(); + } } 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 be8dbced9e..ee1b031a5d 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 @@ -34,6 +34,8 @@ import org.bson.Document; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.data.annotation.Id; @@ -41,6 +43,7 @@ import org.springframework.data.mongodb.core.SimpleMongoClientDatabaseFactory; import org.springframework.data.mongodb.core.convert.MongoCustomConversions.MongoConverterConfigurationAdapter; import org.springframework.data.mongodb.core.mapping.DBRef; +import org.springframework.data.mongodb.core.mapping.DocumentReference; import org.springframework.data.mongodb.core.mapping.MongoMappingContext; import org.springframework.data.mongodb.test.util.Client; @@ -478,4 +481,64 @@ public String toString() { + ", localTime=" + this.getLocalTime() + ", localDateTime=" + this.getLocalDateTime() + ")"; } } + + @Nested // GH-5065 + @DisplayName("GH-5065: Empty Map Tests") + class EmptyMapTests { + + @Test // GH-5065 + @DisplayName("Passing test to indicate that the problem is not a more generic issue with maps in general.") + void readsEmptyMapCorrectly() { + org.bson.Document document = org.bson.Document.parse("{\"map\":{}}"); + EmptyMapDocument target = converter.read(EmptyMapDocument.class, document); + assertThat(target.map).isNotNull().isEmpty(); + } + + @Test // GH-5065 + @DisplayName("Converter should read an empty object as an empty map when using @DocumentReference.") + void readsEmptyMapWithDocumentReferenceCorrectly() { + org.bson.Document document = org.bson.Document.parse("{\"map\":{}}"); + DocumentReferenceEmptyMapDocument target = converter.read(DocumentReferenceEmptyMapDocument.class, document); + assertThat(target.map).isNotNull().isEmpty(); + } + + @Test // GH-5065 + @DisplayName("Converter should read an empty object as an empty map with a valis values property when using @DocumentReference(lazy = true).") + void readsEmptyMapWithLazyLoadedDocumentReferenceCorrectly() { + org.bson.Document document = org.bson.Document.parse("{\"map\":{}}"); + LazyDocumentReferenceEmptyMapDocument target = converter.read(LazyDocumentReferenceEmptyMapDocument.class, document); + assertThat(target.map).isNotNull(); + assertThat(target.map.values()).isNotNull(); + } + + static class EmptyMapDocument { + + Map map; + + public EmptyMapDocument(Map map) { + this.map = map; + } + } + + static class DocumentReferenceEmptyMapDocument { + + @DocumentReference + Map map; + + public DocumentReferenceEmptyMapDocument(Map map) { + this.map = map; + } + } + + static class LazyDocumentReferenceEmptyMapDocument { + + @DocumentReference(lazy = true) + Map map; + + public LazyDocumentReferenceEmptyMapDocument(Map map) { + this.map = map; + } + } + } + } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/ReferenceLookupDelegateUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/ReferenceLookupDelegateUnitTests.java index 384cffaad4..c7d9f3494a 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/ReferenceLookupDelegateUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/ReferenceLookupDelegateUnitTests.java @@ -19,8 +19,11 @@ import static org.mockito.Mockito.*; import java.util.Collections; +import java.util.Map; +import org.bson.Document; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -94,4 +97,35 @@ void shouldResolveEmptyMapOnEmptyTargetCollection() { lookupDelegate.readReference(property, Collections.emptyMap(), lookupFunction, entityReader); verify(lookupFunction, never()).apply(any(), any()); } + + @Test // GH-5065 + @DisplayName("GH-5065: Empty Map with @DocumentReference annotation should deserialize to an empty map.") + void shouldResolveEmptyMapOnEmptyDocumentReferenceMapProperty() { + DocumentReference documentReference = mock(DocumentReference.class); + when(documentReference.lookup()).thenReturn("{ '_id' : ?#{#target} }"); + when(documentReference.sort()).thenReturn(""); + MongoPersistentProperty property = mock(MongoPersistentProperty.class); + when(property.isMap()).thenReturn(true); + when(property.isDocumentReference()).thenReturn(true); + when(property.getDocumentReference()).thenReturn(documentReference); + when(property.isCollectionLike()).thenReturn(false); + DocumentReferenceSource source = mock(DocumentReferenceSource.class); + when(source.getTargetSource()).thenReturn(Document.parse("{}")); + ReferenceLookupDelegate.LookupFunction lookupFunction = mock(ReferenceLookupDelegate.LookupFunction.class); + + Object target = lookupDelegate.readReference(property, source, lookupFunction, entityReader); + + verify(lookupFunction, never()).apply(any(), any()); // Since we mocked it, make sure it is never called. + verify(property, times(2)).isMap(); + verify(property, times(1)).isDocumentReference(); + verify(property, times(1)).getDocumentReference(); + verify(property, times(2)).isCollectionLike(); + verify(documentReference, times(1)).lookup(); + verify(documentReference, times(1)).sort(); + verifyNoMoreInteractions(documentReference, property, source); // Make sure we only call the properties we mocked. + assertThat(target) + .isNotNull() + .isInstanceOf(Map.class); + } + }