Skip to content

Commit 410d38d

Browse files
mp911demp911de
committed
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
1 parent 788a5c2 commit 410d38d

File tree

2 files changed

+75
-30
lines changed

2 files changed

+75
-30
lines changed

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

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -363,13 +363,6 @@ String getFieldName(MongoPersistentProperty prop) {
363363

364364
populateProperties(context, mappedEntity, documentAccessor, evaluator, instance);
365365

366-
PersistentPropertyAccessor<?> convertingAccessor = new ConvertingPropertyAccessor<>(accessor, conversionService);
367-
MongoDbPropertyValueProvider valueProvider = new MongoDbPropertyValueProvider(context, documentAccessor, evaluator,
368-
spELContext);
369-
370-
readProperties(context, mappedEntity, convertingAccessor, documentAccessor, valueProvider, evaluator,
371-
Predicates.isTrue());
372-
373366
return accessor.getBean();
374367
}
375368

@@ -506,16 +499,16 @@ private <S> S read(ConversionContext context, MongoPersistentEntity<S> entity, D
506499
EntityInstantiator instantiator = instantiators.getInstantiatorFor(entity);
507500
S instance = instantiator.createInstance(entity, provider);
508501

509-
if (entity.requiresPropertyPopulation()) {
510-
return populateProperties(context, entity, documentAccessor, evaluator, instance);
511-
}
512-
513-
return instance;
502+
return populateProperties(context, entity, documentAccessor, evaluator, instance);
514503
}
515504

516505
private <S> S populateProperties(ConversionContext context, MongoPersistentEntity<S> entity,
517506
DocumentAccessor documentAccessor, SpELExpressionEvaluator evaluator, S instance) {
518507

508+
if (!entity.requiresPropertyPopulation()) {
509+
return instance;
510+
}
511+
519512
PersistentPropertyAccessor<S> accessor = new ConvertingPropertyAccessor<>(entity.getPropertyAccessor(instance),
520513
conversionService);
521514

@@ -566,7 +559,9 @@ private Object readIdValue(ConversionContext context, SpELExpressionEvaluator ev
566559
String expression = idProperty.getSpelExpression();
567560
Object resolvedValue = expression != null ? evaluator.evaluate(expression) : rawId;
568561

569-
return resolvedValue != null ? readValue(context.forProperty(idProperty), resolvedValue, idProperty.getTypeInformation()) : null;
562+
return resolvedValue != null
563+
? readValue(context.forProperty(idProperty), resolvedValue, idProperty.getTypeInformation())
564+
: null;
570565
}
571566

572567
private void readProperties(ConversionContext context, MongoPersistentEntity<?> entity,
@@ -622,9 +617,8 @@ private DbRefResolverCallback getDbRefResolverCallback(ConversionContext context
622617
}
623618

624619
@Nullable
625-
private Object readAssociation(Association<MongoPersistentProperty> association,
626-
DocumentAccessor documentAccessor, DbRefProxyHandler handler, DbRefResolverCallback callback,
627-
ConversionContext context) {
620+
private Object readAssociation(Association<MongoPersistentProperty> association, DocumentAccessor documentAccessor,
621+
DbRefProxyHandler handler, DbRefResolverCallback callback, ConversionContext context) {
628622

629623
MongoPersistentProperty property = association.getInverse();
630624
Object value = documentAccessor.get(property);
@@ -647,7 +641,7 @@ private Object readAssociation(Association<MongoPersistentProperty> association,
647641
} else {
648642

649643
return dbRefResolver.resolveReference(property,
650-
new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)),
644+
new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)),
651645
referenceLookupDelegate, context.forProperty(property)::convert);
652646
}
653647
}
@@ -2426,8 +2420,6 @@ class ProjectingConversionContext extends DefaultConversionContext {
24262420
this.returnedTypeDescriptor = projection;
24272421
}
24282422

2429-
2430-
24312423
@Override
24322424
public ConversionContext forProperty(String name) {
24332425

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

Lines changed: 64 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,14 @@
2828
import java.math.BigDecimal;
2929
import java.math.BigInteger;
3030
import java.net.URL;
31+
import java.nio.ByteBuffer;
3132
import java.time.LocalDate;
3233
import java.time.LocalDateTime;
3334
import java.time.temporal.ChronoUnit;
3435
import java.util.*;
3536

37+
import org.assertj.core.data.Percentage;
38+
import org.bson.BsonUndefined;
3639
import org.bson.types.Binary;
3740
import org.bson.types.Code;
3841
import org.bson.types.Decimal128;
@@ -125,7 +128,8 @@ class MappingMongoConverterUnitTests {
125128
@BeforeEach
126129
void beforeEach() {
127130

128-
MongoCustomConversions conversions = new MongoCustomConversions();
131+
MongoCustomConversions conversions = new MongoCustomConversions(
132+
Arrays.asList(new ByteBufferToDoubleHolderConverter()));
129133

130134
mappingContext = new MongoMappingContext();
131135
mappingContext.setApplicationContext(context);
@@ -1437,7 +1441,7 @@ void shouldWriteEntityWithGeoCircleCorrectly() {
14371441
assertThat(document.get("circle")).isInstanceOf(org.bson.Document.class);
14381442
assertThat(document.get("circle")).isEqualTo((Object) new org.bson.Document("center",
14391443
new org.bson.Document("x", circle.getCenter().getX()).append("y", circle.getCenter().getY()))
1440-
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
1444+
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
14411445
}
14421446

14431447
@Test // DATAMONGO-858
@@ -1470,7 +1474,7 @@ void shouldWriteEntityWithGeoSphereCorrectly() {
14701474
assertThat(document.get("sphere")).isInstanceOf(org.bson.Document.class);
14711475
assertThat(document.get("sphere")).isEqualTo((Object) new org.bson.Document("center",
14721476
new org.bson.Document("x", sphere.getCenter().getX()).append("y", sphere.getCenter().getY()))
1473-
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
1477+
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
14741478
}
14751479

14761480
@Test // DATAMONGO-858
@@ -1488,7 +1492,7 @@ void shouldWriteEntityWithGeoSphereWithMetricDistanceCorrectly() {
14881492
assertThat(document.get("sphere")).isInstanceOf(org.bson.Document.class);
14891493
assertThat(document.get("sphere")).isEqualTo((Object) new org.bson.Document("center",
14901494
new org.bson.Document("x", sphere.getCenter().getX()).append("y", sphere.getCenter().getY()))
1491-
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
1495+
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
14921496
}
14931497

14941498
@Test // DATAMONGO-858
@@ -1521,7 +1525,7 @@ void shouldWriteEntityWithGeoShapeCorrectly() {
15211525
assertThat(document.get("shape")).isInstanceOf(org.bson.Document.class);
15221526
assertThat(document.get("shape")).isEqualTo((Object) new org.bson.Document("center",
15231527
new org.bson.Document("x", sphere.getCenter().getX()).append("y", sphere.getCenter().getY()))
1524-
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
1528+
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
15251529
}
15261530

15271531
@Test // DATAMONGO-858
@@ -2531,8 +2535,8 @@ void usesCustomConverterForPropertiesUsingTypesImplementingMapOnRead() {
25312535
converter.afterPropertiesSet();
25322536

25332537
org.bson.Document source = new org.bson.Document("typeImplementingMap",
2534-
new org.bson.Document("1st", "one").append("2nd", 2)).append("_class",
2535-
TypeWrappingTypeImplementingMap.class.getName());
2538+
new org.bson.Document("1st", "one").append("2nd", 2))
2539+
.append("_class", TypeWrappingTypeImplementingMap.class.getName());
25362540

25372541
TypeWrappingTypeImplementingMap target = converter.read(TypeWrappingTypeImplementingMap.class, source);
25382542

@@ -2720,8 +2724,8 @@ void projectShouldReadNestedInterfaceProjection() {
27202724
.and((target, underlyingType) -> !converter.conversions.isSimpleType(target)),
27212725
mappingContext);
27222726

2723-
EntityProjection<WithNestedInterfaceProjection, Person> projection = introspector.introspect(WithNestedInterfaceProjection.class,
2724-
Person.class);
2727+
EntityProjection<WithNestedInterfaceProjection, Person> projection = introspector
2728+
.introspect(WithNestedInterfaceProjection.class, Person.class);
27252729
WithNestedInterfaceProjection person = converter.project(projection, source);
27262730

27272731
assertThat(person.getFirstname()).isEqualTo("spring");
@@ -2739,14 +2743,35 @@ void projectShouldReadNestedDtoProjection() {
27392743
.and((target, underlyingType) -> !converter.conversions.isSimpleType(target)),
27402744
mappingContext);
27412745

2742-
EntityProjection<WithNestedDtoProjection, Person> projection = introspector.introspect(WithNestedDtoProjection.class,
2743-
Person.class);
2746+
EntityProjection<WithNestedDtoProjection, Person> projection = introspector
2747+
.introspect(WithNestedDtoProjection.class, Person.class);
27442748
WithNestedDtoProjection person = converter.project(projection, source);
27452749

27462750
assertThat(person.getFirstname()).isEqualTo("spring");
27472751
assertThat(person.getAddress().getStreet()).isEqualTo("data");
27482752
}
27492753

2754+
@Test // GH-4626
2755+
void projectShouldReadDtoProjectionPropertiesOnlyOnce() {
2756+
2757+
ByteBuffer number = ByteBuffer.allocate(8);
2758+
number.putDouble(1.2d);
2759+
number.flip();
2760+
2761+
org.bson.Document source = new org.bson.Document("number", number);
2762+
2763+
EntityProjectionIntrospector introspector = EntityProjectionIntrospector.create(converter.getProjectionFactory(),
2764+
EntityProjectionIntrospector.ProjectionPredicate.typeHierarchy()
2765+
.and((target, underlyingType) -> !converter.conversions.isSimpleType(target)),
2766+
mappingContext);
2767+
2768+
EntityProjection<DoubleHolderDto, WithDoubleHolder> projection = introspector.introspect(DoubleHolderDto.class,
2769+
WithDoubleHolder.class);
2770+
DoubleHolderDto result = converter.project(projection, source);
2771+
2772+
assertThat(result.number.number).isCloseTo(1.2, Percentage.withPercentage(1));
2773+
}
2774+
27502775
@Test // GH-2860
27512776
void projectShouldReadProjectionWithNestedEntity() {
27522777

@@ -3022,11 +3047,13 @@ interface WithNestedProjection {
30223047

30233048
interface WithNestedInterfaceProjection {
30243049
String getFirstname();
3050+
30253051
AddressProjection getAddress();
30263052
}
30273053

30283054
interface WithNestedDtoProjection {
30293055
String getFirstname();
3056+
30303057
AddressDto getAddress();
30313058
}
30323059

@@ -3925,4 +3952,30 @@ static class ComplexIdAndNoAnnotation {
39253952
ComplexId id;
39263953
String value;
39273954
}
3955+
3956+
@ReadingConverter
3957+
static class ByteBufferToDoubleHolderConverter implements Converter<ByteBuffer, DoubleHolder> {
3958+
3959+
@Override
3960+
public DoubleHolder convert(ByteBuffer source) {
3961+
return new DoubleHolder(source.getDouble());
3962+
}
3963+
}
3964+
3965+
record DoubleHolder(double number) {
3966+
3967+
}
3968+
3969+
static class WithDoubleHolder {
3970+
DoubleHolder number;
3971+
}
3972+
3973+
static class DoubleHolderDto {
3974+
DoubleHolder number;
3975+
3976+
public DoubleHolderDto(DoubleHolder number) {
3977+
this.number = number;
3978+
}
3979+
}
3980+
39283981
}

0 commit comments

Comments
 (0)