From f60f0cd306a13b67e014784e33520cbc6403910d Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 29 Jan 2024 09:07:21 +0100 Subject: [PATCH] Read DTO projection properties only once. We ensure to not read DTO properties multiple times if these are already read by their persistence creator. Closes #4626 --- .../core/convert/MappingMongoConverter.java | 30 +++----- .../MappingMongoConverterUnitTests.java | 74 ++++++++++++++++--- 2 files changed, 74 insertions(+), 30 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 b6cbd824d..86d875485 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 @@ -375,13 +375,6 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App populateProperties(context, mappedEntity, documentAccessor, evaluator, instance); - PersistentPropertyAccessor convertingAccessor = new ConvertingPropertyAccessor<>(accessor, conversionService); - MongoDbPropertyValueProvider valueProvider = new MongoDbPropertyValueProvider(context, documentAccessor, evaluator, - spELContext); - - readProperties(context, mappedEntity, convertingAccessor, documentAccessor, valueProvider, evaluator, - Predicates.isTrue()); - return accessor.getBean(); } @@ -518,16 +511,16 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App EntityInstantiator instantiator = instantiators.getInstantiatorFor(entity); S instance = instantiator.createInstance(entity, provider); - if (entity.requiresPropertyPopulation()) { - return populateProperties(context, entity, documentAccessor, evaluator, instance); - } - - return instance; + return populateProperties(context, entity, documentAccessor, evaluator, instance); } private S populateProperties(ConversionContext context, MongoPersistentEntity entity, DocumentAccessor documentAccessor, SpELExpressionEvaluator evaluator, S instance) { + if (!entity.requiresPropertyPopulation()) { + return instance; + } + PersistentPropertyAccessor accessor = new ConvertingPropertyAccessor<>(entity.getPropertyAccessor(instance), conversionService); @@ -578,7 +571,9 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App String expression = idProperty.getSpelExpression(); Object resolvedValue = expression != null ? evaluator.evaluate(expression) : rawId; - return resolvedValue != null ? readValue(context.forProperty(idProperty), resolvedValue, idProperty.getTypeInformation()) : null; + return resolvedValue != null + ? readValue(context.forProperty(idProperty), resolvedValue, idProperty.getTypeInformation()) + : null; } private void readProperties(ConversionContext context, MongoPersistentEntity entity, @@ -634,9 +629,8 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } @Nullable - private Object readAssociation(Association association, - DocumentAccessor documentAccessor, DbRefProxyHandler handler, DbRefResolverCallback callback, - ConversionContext context) { + private Object readAssociation(Association association, DocumentAccessor documentAccessor, + DbRefProxyHandler handler, DbRefResolverCallback callback, ConversionContext context) { MongoPersistentProperty property = association.getInverse(); Object value = documentAccessor.get(property); @@ -659,7 +653,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } else { return dbRefResolver.resolveReference(property, - new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)), + new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)), referenceLookupDelegate, context.forProperty(property)::convert); } } @@ -2435,8 +2429,6 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App this.returnedTypeDescriptor = projection; } - - @Override public ConversionContext forProperty(String name) { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java index 8a73aa2e9..32c7746f9 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java @@ -23,6 +23,7 @@ import static org.springframework.data.mongodb.core.DocumentTestUtils.*; import java.math.BigDecimal; import java.math.BigInteger; import java.net.URL; +import java.nio.ByteBuffer; import java.time.LocalDate; import java.time.LocalDateTime; import java.time.temporal.ChronoUnit; @@ -31,6 +32,7 @@ import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Stream; +import org.assertj.core.data.Percentage; import org.bson.BsonUndefined; import org.bson.types.Binary; import org.bson.types.Code; @@ -129,7 +131,8 @@ class MappingMongoConverterUnitTests { @BeforeEach void beforeEach() { - MongoCustomConversions conversions = new MongoCustomConversions(); + MongoCustomConversions conversions = new MongoCustomConversions( + Arrays.asList(new ByteBufferToDoubleHolderConverter())); mappingContext = new MongoMappingContext(); mappingContext.setApplicationContext(context); @@ -1579,7 +1582,7 @@ class MappingMongoConverterUnitTests { assertThat(document.get("circle")).isInstanceOf(org.bson.Document.class); assertThat(document.get("circle")).isEqualTo((Object) new org.bson.Document("center", new org.bson.Document("x", circle.getCenter().getX()).append("y", circle.getCenter().getY())) - .append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString())); + .append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString())); } @Test // DATAMONGO-858 @@ -1612,7 +1615,7 @@ class MappingMongoConverterUnitTests { assertThat(document.get("sphere")).isInstanceOf(org.bson.Document.class); assertThat(document.get("sphere")).isEqualTo((Object) new org.bson.Document("center", new org.bson.Document("x", sphere.getCenter().getX()).append("y", sphere.getCenter().getY())) - .append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString())); + .append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString())); } @Test // DATAMONGO-858 @@ -1630,7 +1633,7 @@ class MappingMongoConverterUnitTests { assertThat(document.get("sphere")).isInstanceOf(org.bson.Document.class); assertThat(document.get("sphere")).isEqualTo((Object) new org.bson.Document("center", new org.bson.Document("x", sphere.getCenter().getX()).append("y", sphere.getCenter().getY())) - .append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString())); + .append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString())); } @Test // DATAMONGO-858 @@ -1663,7 +1666,7 @@ class MappingMongoConverterUnitTests { assertThat(document.get("shape")).isInstanceOf(org.bson.Document.class); assertThat(document.get("shape")).isEqualTo((Object) new org.bson.Document("center", new org.bson.Document("x", sphere.getCenter().getX()).append("y", sphere.getCenter().getY())) - .append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString())); + .append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString())); } @Test // DATAMONGO-858 @@ -2673,8 +2676,8 @@ class MappingMongoConverterUnitTests { converter.afterPropertiesSet(); org.bson.Document source = new org.bson.Document("typeImplementingMap", - new org.bson.Document("1st", "one").append("2nd", 2)).append("_class", - TypeWrappingTypeImplementingMap.class.getName()); + new org.bson.Document("1st", "one").append("2nd", 2)) + .append("_class", TypeWrappingTypeImplementingMap.class.getName()); TypeWrappingTypeImplementingMap target = converter.read(TypeWrappingTypeImplementingMap.class, source); @@ -2862,8 +2865,8 @@ class MappingMongoConverterUnitTests { .and((target, underlyingType) -> !converter.conversions.isSimpleType(target)), mappingContext); - EntityProjection projection = introspector.introspect(WithNestedInterfaceProjection.class, - Person.class); + EntityProjection projection = introspector + .introspect(WithNestedInterfaceProjection.class, Person.class); WithNestedInterfaceProjection person = converter.project(projection, source); assertThat(person.getFirstname()).isEqualTo("spring"); @@ -2881,14 +2884,35 @@ class MappingMongoConverterUnitTests { .and((target, underlyingType) -> !converter.conversions.isSimpleType(target)), mappingContext); - EntityProjection projection = introspector.introspect(WithNestedDtoProjection.class, - Person.class); + EntityProjection projection = introspector + .introspect(WithNestedDtoProjection.class, Person.class); WithNestedDtoProjection person = converter.project(projection, source); assertThat(person.getFirstname()).isEqualTo("spring"); assertThat(person.getAddress().getStreet()).isEqualTo("data"); } + @Test // GH-4626 + void projectShouldReadDtoProjectionPropertiesOnlyOnce() { + + ByteBuffer number = ByteBuffer.allocate(8); + number.putDouble(1.2d); + number.flip(); + + org.bson.Document source = new org.bson.Document("number", number); + + EntityProjectionIntrospector introspector = EntityProjectionIntrospector.create(converter.getProjectionFactory(), + EntityProjectionIntrospector.ProjectionPredicate.typeHierarchy() + .and((target, underlyingType) -> !converter.conversions.isSimpleType(target)), + mappingContext); + + EntityProjection projection = introspector.introspect(DoubleHolderDto.class, + WithDoubleHolder.class); + DoubleHolderDto result = converter.project(projection, source); + + assertThat(result.number.number).isCloseTo(1.2, Percentage.withPercentage(1)); + } + @Test // GH-2860 void projectShouldReadProjectionWithNestedEntity() { @@ -3289,11 +3313,13 @@ class MappingMongoConverterUnitTests { interface WithNestedInterfaceProjection { String getFirstname(); + AddressProjection getAddress(); } interface WithNestedDtoProjection { String getFirstname(); + AddressDto getAddress(); } @@ -4342,4 +4368,30 @@ class MappingMongoConverterUnitTests { ComplexId id; String value; } + + @ReadingConverter + static class ByteBufferToDoubleHolderConverter implements Converter { + + @Override + public DoubleHolder convert(ByteBuffer source) { + return new DoubleHolder(source.getDouble()); + } + } + + record DoubleHolder(double number) { + + } + + static class WithDoubleHolder { + DoubleHolder number; + } + + static class DoubleHolderDto { + DoubleHolder number; + + public DoubleHolderDto(DoubleHolder number) { + this.number = number; + } + } + }