Skip to content

Commit f60f0cd

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 91b7f46 commit f60f0cd

File tree

2 files changed

+74
-30
lines changed

2 files changed

+74
-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
@@ -375,13 +375,6 @@ FieldName getFieldName(MongoPersistentProperty prop) {
375375

376376
populateProperties(context, mappedEntity, documentAccessor, evaluator, instance);
377377

378-
PersistentPropertyAccessor<?> convertingAccessor = new ConvertingPropertyAccessor<>(accessor, conversionService);
379-
MongoDbPropertyValueProvider valueProvider = new MongoDbPropertyValueProvider(context, documentAccessor, evaluator,
380-
spELContext);
381-
382-
readProperties(context, mappedEntity, convertingAccessor, documentAccessor, valueProvider, evaluator,
383-
Predicates.isTrue());
384-
385378
return accessor.getBean();
386379
}
387380

@@ -518,16 +511,16 @@ private <S> S read(ConversionContext context, MongoPersistentEntity<S> entity, D
518511
EntityInstantiator instantiator = instantiators.getInstantiatorFor(entity);
519512
S instance = instantiator.createInstance(entity, provider);
520513

521-
if (entity.requiresPropertyPopulation()) {
522-
return populateProperties(context, entity, documentAccessor, evaluator, instance);
523-
}
524-
525-
return instance;
514+
return populateProperties(context, entity, documentAccessor, evaluator, instance);
526515
}
527516

528517
private <S> S populateProperties(ConversionContext context, MongoPersistentEntity<S> entity,
529518
DocumentAccessor documentAccessor, SpELExpressionEvaluator evaluator, S instance) {
530519

520+
if (!entity.requiresPropertyPopulation()) {
521+
return instance;
522+
}
523+
531524
PersistentPropertyAccessor<S> accessor = new ConvertingPropertyAccessor<>(entity.getPropertyAccessor(instance),
532525
conversionService);
533526

@@ -578,7 +571,9 @@ private Object readIdValue(ConversionContext context, SpELExpressionEvaluator ev
578571
String expression = idProperty.getSpelExpression();
579572
Object resolvedValue = expression != null ? evaluator.evaluate(expression) : rawId;
580573

581-
return resolvedValue != null ? readValue(context.forProperty(idProperty), resolvedValue, idProperty.getTypeInformation()) : null;
574+
return resolvedValue != null
575+
? readValue(context.forProperty(idProperty), resolvedValue, idProperty.getTypeInformation())
576+
: null;
582577
}
583578

584579
private void readProperties(ConversionContext context, MongoPersistentEntity<?> entity,
@@ -634,9 +629,8 @@ private DbRefResolverCallback getDbRefResolverCallback(ConversionContext context
634629
}
635630

636631
@Nullable
637-
private Object readAssociation(Association<MongoPersistentProperty> association,
638-
DocumentAccessor documentAccessor, DbRefProxyHandler handler, DbRefResolverCallback callback,
639-
ConversionContext context) {
632+
private Object readAssociation(Association<MongoPersistentProperty> association, DocumentAccessor documentAccessor,
633+
DbRefProxyHandler handler, DbRefResolverCallback callback, ConversionContext context) {
640634

641635
MongoPersistentProperty property = association.getInverse();
642636
Object value = documentAccessor.get(property);
@@ -659,7 +653,7 @@ private Object readAssociation(Association<MongoPersistentProperty> association,
659653
} else {
660654

661655
return dbRefResolver.resolveReference(property,
662-
new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)),
656+
new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)),
663657
referenceLookupDelegate, context.forProperty(property)::convert);
664658
}
665659
}
@@ -2435,8 +2429,6 @@ class ProjectingConversionContext extends DefaultConversionContext {
24352429
this.returnedTypeDescriptor = projection;
24362430
}
24372431

2438-
2439-
24402432
@Override
24412433
public ConversionContext forProperty(String name) {
24422434

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

Lines changed: 63 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.math.BigDecimal;
2424
import java.math.BigInteger;
2525
import java.net.URL;
26+
import java.nio.ByteBuffer;
2627
import java.time.LocalDate;
2728
import java.time.LocalDateTime;
2829
import java.time.temporal.ChronoUnit;
@@ -31,6 +32,7 @@
3132
import java.util.function.Function;
3233
import java.util.stream.Stream;
3334

35+
import org.assertj.core.data.Percentage;
3436
import org.bson.BsonUndefined;
3537
import org.bson.types.Binary;
3638
import org.bson.types.Code;
@@ -129,7 +131,8 @@ class MappingMongoConverterUnitTests {
129131
@BeforeEach
130132
void beforeEach() {
131133

132-
MongoCustomConversions conversions = new MongoCustomConversions();
134+
MongoCustomConversions conversions = new MongoCustomConversions(
135+
Arrays.asList(new ByteBufferToDoubleHolderConverter()));
133136

134137
mappingContext = new MongoMappingContext();
135138
mappingContext.setApplicationContext(context);
@@ -1579,7 +1582,7 @@ void shouldWriteEntityWithGeoCircleCorrectly() {
15791582
assertThat(document.get("circle")).isInstanceOf(org.bson.Document.class);
15801583
assertThat(document.get("circle")).isEqualTo((Object) new org.bson.Document("center",
15811584
new org.bson.Document("x", circle.getCenter().getX()).append("y", circle.getCenter().getY()))
1582-
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
1585+
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
15831586
}
15841587

15851588
@Test // DATAMONGO-858
@@ -1612,7 +1615,7 @@ void shouldWriteEntityWithGeoSphereCorrectly() {
16121615
assertThat(document.get("sphere")).isInstanceOf(org.bson.Document.class);
16131616
assertThat(document.get("sphere")).isEqualTo((Object) new org.bson.Document("center",
16141617
new org.bson.Document("x", sphere.getCenter().getX()).append("y", sphere.getCenter().getY()))
1615-
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
1618+
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
16161619
}
16171620

16181621
@Test // DATAMONGO-858
@@ -1630,7 +1633,7 @@ void shouldWriteEntityWithGeoSphereWithMetricDistanceCorrectly() {
16301633
assertThat(document.get("sphere")).isInstanceOf(org.bson.Document.class);
16311634
assertThat(document.get("sphere")).isEqualTo((Object) new org.bson.Document("center",
16321635
new org.bson.Document("x", sphere.getCenter().getX()).append("y", sphere.getCenter().getY()))
1633-
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
1636+
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
16341637
}
16351638

16361639
@Test // DATAMONGO-858
@@ -1663,7 +1666,7 @@ void shouldWriteEntityWithGeoShapeCorrectly() {
16631666
assertThat(document.get("shape")).isInstanceOf(org.bson.Document.class);
16641667
assertThat(document.get("shape")).isEqualTo((Object) new org.bson.Document("center",
16651668
new org.bson.Document("x", sphere.getCenter().getX()).append("y", sphere.getCenter().getY()))
1666-
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
1669+
.append("radius", radius.getNormalizedValue()).append("metric", radius.getMetric().toString()));
16671670
}
16681671

16691672
@Test // DATAMONGO-858
@@ -2673,8 +2676,8 @@ void usesCustomConverterForPropertiesUsingTypesImplementingMapOnRead() {
26732676
converter.afterPropertiesSet();
26742677

26752678
org.bson.Document source = new org.bson.Document("typeImplementingMap",
2676-
new org.bson.Document("1st", "one").append("2nd", 2)).append("_class",
2677-
TypeWrappingTypeImplementingMap.class.getName());
2679+
new org.bson.Document("1st", "one").append("2nd", 2))
2680+
.append("_class", TypeWrappingTypeImplementingMap.class.getName());
26782681

26792682
TypeWrappingTypeImplementingMap target = converter.read(TypeWrappingTypeImplementingMap.class, source);
26802683

@@ -2862,8 +2865,8 @@ void projectShouldReadNestedInterfaceProjection() {
28622865
.and((target, underlyingType) -> !converter.conversions.isSimpleType(target)),
28632866
mappingContext);
28642867

2865-
EntityProjection<WithNestedInterfaceProjection, Person> projection = introspector.introspect(WithNestedInterfaceProjection.class,
2866-
Person.class);
2868+
EntityProjection<WithNestedInterfaceProjection, Person> projection = introspector
2869+
.introspect(WithNestedInterfaceProjection.class, Person.class);
28672870
WithNestedInterfaceProjection person = converter.project(projection, source);
28682871

28692872
assertThat(person.getFirstname()).isEqualTo("spring");
@@ -2881,14 +2884,35 @@ void projectShouldReadNestedDtoProjection() {
28812884
.and((target, underlyingType) -> !converter.conversions.isSimpleType(target)),
28822885
mappingContext);
28832886

2884-
EntityProjection<WithNestedDtoProjection, Person> projection = introspector.introspect(WithNestedDtoProjection.class,
2885-
Person.class);
2887+
EntityProjection<WithNestedDtoProjection, Person> projection = introspector
2888+
.introspect(WithNestedDtoProjection.class, Person.class);
28862889
WithNestedDtoProjection person = converter.project(projection, source);
28872890

28882891
assertThat(person.getFirstname()).isEqualTo("spring");
28892892
assertThat(person.getAddress().getStreet()).isEqualTo("data");
28902893
}
28912894

2895+
@Test // GH-4626
2896+
void projectShouldReadDtoProjectionPropertiesOnlyOnce() {
2897+
2898+
ByteBuffer number = ByteBuffer.allocate(8);
2899+
number.putDouble(1.2d);
2900+
number.flip();
2901+
2902+
org.bson.Document source = new org.bson.Document("number", number);
2903+
2904+
EntityProjectionIntrospector introspector = EntityProjectionIntrospector.create(converter.getProjectionFactory(),
2905+
EntityProjectionIntrospector.ProjectionPredicate.typeHierarchy()
2906+
.and((target, underlyingType) -> !converter.conversions.isSimpleType(target)),
2907+
mappingContext);
2908+
2909+
EntityProjection<DoubleHolderDto, WithDoubleHolder> projection = introspector.introspect(DoubleHolderDto.class,
2910+
WithDoubleHolder.class);
2911+
DoubleHolderDto result = converter.project(projection, source);
2912+
2913+
assertThat(result.number.number).isCloseTo(1.2, Percentage.withPercentage(1));
2914+
}
2915+
28922916
@Test // GH-2860
28932917
void projectShouldReadProjectionWithNestedEntity() {
28942918

@@ -3289,11 +3313,13 @@ interface WithNestedProjection {
32893313

32903314
interface WithNestedInterfaceProjection {
32913315
String getFirstname();
3316+
32923317
AddressProjection getAddress();
32933318
}
32943319

32953320
interface WithNestedDtoProjection {
32963321
String getFirstname();
3322+
32973323
AddressDto getAddress();
32983324
}
32993325

@@ -4342,4 +4368,30 @@ static class ComplexIdAndNoAnnotation {
43424368
ComplexId id;
43434369
String value;
43444370
}
4371+
4372+
@ReadingConverter
4373+
static class ByteBufferToDoubleHolderConverter implements Converter<ByteBuffer, DoubleHolder> {
4374+
4375+
@Override
4376+
public DoubleHolder convert(ByteBuffer source) {
4377+
return new DoubleHolder(source.getDouble());
4378+
}
4379+
}
4380+
4381+
record DoubleHolder(double number) {
4382+
4383+
}
4384+
4385+
static class WithDoubleHolder {
4386+
DoubleHolder number;
4387+
}
4388+
4389+
static class DoubleHolderDto {
4390+
DoubleHolder number;
4391+
4392+
public DoubleHolderDto(DoubleHolder number) {
4393+
this.number = number;
4394+
}
4395+
}
4396+
43454397
}

0 commit comments

Comments
 (0)