Skip to content

Commit

Permalink
Read DTO projection properties only once.
Browse files Browse the repository at this point in the history 
We ensure to not read DTO properties multiple times if these are already read by their persistence creator.

Closes #4626
  • Loading branch information
@mp911de
mp911de committed Jan 29, 2024
1 parent 788a5c2 commit 410d38d
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 30 deletions.
30 changes: 11 additions & 19 deletions ...db/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,13 +363,6 @@ String getFieldName(MongoPersistentProperty prop) {

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();
}

Expand Down Expand Up @@ -506,16 +499,16 @@ private <S> S read(ConversionContext context, MongoPersistentEntity<S> entity, D
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> S populateProperties(ConversionContext context, MongoPersistentEntity<S> entity,
DocumentAccessor documentAccessor, SpELExpressionEvaluator evaluator, S instance) {

if (!entity.requiresPropertyPopulation()) {
return instance;
}

PersistentPropertyAccessor<S> accessor = new ConvertingPropertyAccessor<>(entity.getPropertyAccessor(instance),
conversionService);

Expand Down Expand Up @@ -566,7 +559,9 @@ private Object readIdValue(ConversionContext context, SpELExpressionEvaluator ev
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,
Expand Down Expand Up @@ -622,9 +617,8 @@ private DbRefResolverCallback getDbRefResolverCallback(ConversionContext context
}

@Nullable
private Object readAssociation(Association<MongoPersistentProperty> association,
DocumentAccessor documentAccessor, DbRefProxyHandler handler, DbRefResolverCallback callback,
ConversionContext context) {
private Object readAssociation(Association<MongoPersistentProperty> association, DocumentAccessor documentAccessor,
DbRefProxyHandler handler, DbRefResolverCallback callback, ConversionContext context) {

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

return dbRefResolver.resolveReference(property,
new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)),
new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)),
referenceLookupDelegate, context.forProperty(property)::convert);
}
}
Expand Down Expand Up @@ -2426,8 +2420,6 @@ class ProjectingConversionContext extends DefaultConversionContext {
this.returnedTypeDescriptor = projection;
}



@Override
public ConversionContext forProperty(String name) {

Expand Down
75 changes: 64 additions & 11 deletions ...st/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,14 @@
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;
import java.util.*;

import org.assertj.core.data.Percentage;
import org.bson.BsonUndefined;
import org.bson.types.Binary;
import org.bson.types.Code;
import org.bson.types.Decimal128;
Expand Down Expand Up @@ -125,7 +128,8 @@ class MappingMongoConverterUnitTests {
@BeforeEach
void beforeEach() {

MongoCustomConversions conversions = new MongoCustomConversions();
MongoCustomConversions conversions = new MongoCustomConversions(
Arrays.asList(new ByteBufferToDoubleHolderConverter()));

mappingContext = new MongoMappingContext();
mappingContext.setApplicationContext(context);
Expand Down Expand Up @@ -1437,7 +1441,7 @@ void shouldWriteEntityWithGeoCircleCorrectly() {
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
Expand Down Expand Up @@ -1470,7 +1474,7 @@ void shouldWriteEntityWithGeoSphereCorrectly() {
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
Expand All @@ -1488,7 +1492,7 @@ void shouldWriteEntityWithGeoSphereWithMetricDistanceCorrectly() {
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
Expand Down Expand Up @@ -1521,7 +1525,7 @@ void shouldWriteEntityWithGeoShapeCorrectly() {
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
Expand Down Expand Up @@ -2531,8 +2535,8 @@ void usesCustomConverterForPropertiesUsingTypesImplementingMapOnRead() {
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);

Expand Down Expand Up @@ -2720,8 +2724,8 @@ void projectShouldReadNestedInterfaceProjection() {
.and((target, underlyingType) -> !converter.conversions.isSimpleType(target)),
mappingContext);

EntityProjection<WithNestedInterfaceProjection, Person> projection = introspector.introspect(WithNestedInterfaceProjection.class,
Person.class);
EntityProjection<WithNestedInterfaceProjection, Person> projection = introspector
.introspect(WithNestedInterfaceProjection.class, Person.class);
WithNestedInterfaceProjection person = converter.project(projection, source);

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

EntityProjection<WithNestedDtoProjection, Person> projection = introspector.introspect(WithNestedDtoProjection.class,
Person.class);
EntityProjection<WithNestedDtoProjection, Person> 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<DoubleHolderDto, WithDoubleHolder> 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() {

Expand Down Expand Up @@ -3022,11 +3047,13 @@ interface WithNestedProjection {

interface WithNestedInterfaceProjection {
String getFirstname();

AddressProjection getAddress();
}

interface WithNestedDtoProjection {
String getFirstname();

AddressDto getAddress();
}

Expand Down Expand Up @@ -3925,4 +3952,30 @@ static class ComplexIdAndNoAnnotation {
ComplexId id;
String value;
}

@ReadingConverter
static class ByteBufferToDoubleHolderConverter implements Converter<ByteBuffer, DoubleHolder> {

@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;
}
}

}

0 comments on commit 410d38d

Please sign in to comment.