diff --git a/pom.xml b/pom.xml index 9cc6d6e9b7..af6be81333 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 4.0.0-SNAPSHOT + 4.0.x-GH-4098-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index c28a240d2c..f063592c6f 100644 --- a/spring-data-mongodb-benchmarks/pom.xml +++ b/spring-data-mongodb-benchmarks/pom.xml @@ -7,7 +7,7 @@ org.springframework.data spring-data-mongodb-parent - 4.0.0-SNAPSHOT + 4.0.x-GH-4098-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index 5dedcf81ed..af7b89df0c 100644 --- a/spring-data-mongodb-distribution/pom.xml +++ b/spring-data-mongodb-distribution/pom.xml @@ -15,7 +15,7 @@ org.springframework.data spring-data-mongodb-parent - 4.0.0-SNAPSHOT + 4.0.x-GH-4098-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index 9cd2578964..02438c440d 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -12,7 +12,7 @@ org.springframework.data spring-data-mongodb-parent - 4.0.0-SNAPSHOT + 4.0.x-GH-4098-SNAPSHOT ../pom.xml 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 af84e780c6..36030ec5a6 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 @@ -39,7 +39,6 @@ import org.bson.conversions.Bson; import org.bson.json.JsonReader; import org.bson.types.ObjectId; - import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.context.ApplicationContext; @@ -188,7 +187,7 @@ protected ConversionContext getConversionContext(ObjectPath path) { Assert.notNull(path, "ObjectPath must not be null"); - return new ConversionContext(this, conversions, path, this::readDocument, this::readCollectionOrArray, + return new DefaultConversionContext(this, conversions, path, this::readDocument, this::readCollectionOrArray, this::readMap, this::readDBRef, this::getPotentiallyConvertedSimpleRead); } @@ -385,7 +384,46 @@ private Object doReadOrProject(ConversionContext context, Bson source, TypeInfor return readDocument(context, source, typeHint); } - class ProjectingConversionContext extends ConversionContext { + static class AssociationConversionContext implements ConversionContext { + + private final ConversionContext delegate; + + public AssociationConversionContext(ConversionContext delegate) { + this.delegate = delegate; + } + + @Override + public S convert(Object source, TypeInformation typeHint, ConversionContext context) { + return delegate.convert(source, typeHint, context); + } + + @Override + public ConversionContext withPath(ObjectPath currentPath) { + return new AssociationConversionContext(delegate.withPath(currentPath)); + } + + @Override + public ObjectPath getPath() { + return delegate.getPath(); + } + + @Override + public CustomConversions getCustomConversions() { + return delegate.getCustomConversions(); + } + + @Override + public MongoConverter getSourceConverter() { + return delegate.getSourceConverter(); + } + + @Override + public boolean resolveIdsInContext() { + return true; + } + } + + class ProjectingConversionContext extends DefaultConversionContext { private final EntityProjection returnedTypeDescriptor; @@ -401,12 +439,13 @@ class ProjectingConversionContext extends ConversionContext { } @Override - public ConversionContext forProperty(String name) { + public DefaultConversionContext forProperty(String name) { EntityProjection property = returnedTypeDescriptor.findProperty(name); if (property == null) { - return new ConversionContext(sourceConverter, conversions, path, MappingMongoConverter.this::readDocument, - collectionConverter, mapConverter, dbRefConverter, elementConverter); + return new DefaultConversionContext(sourceConverter, conversions, path, + MappingMongoConverter.this::readDocument, collectionConverter, mapConverter, dbRefConverter, + elementConverter); } return new ProjectingConversionContext(sourceConverter, conversions, path, collectionConverter, mapConverter, @@ -414,7 +453,7 @@ public ConversionContext forProperty(String name) { } @Override - public ConversionContext withPath(ObjectPath currentPath) { + public DefaultConversionContext withPath(ObjectPath currentPath) { return new ProjectingConversionContext(sourceConverter, conversions, currentPath, collectionConverter, mapConverter, dbRefConverter, elementConverter, returnedTypeDescriptor); } @@ -529,7 +568,7 @@ private S read(ConversionContext context, MongoPersistentEntity entity, D SpELExpressionEvaluator evaluator = new DefaultSpELExpressionEvaluator(bson, spELContext); DocumentAccessor documentAccessor = new DocumentAccessor(bson); - if (hasIdentifier(bson)) { + if (context.resolveIdsInContext() && hasIdentifier(bson)) { S existing = findContextualEntity(context, entity, bson); if (existing != null) { return existing; @@ -632,7 +671,7 @@ private void readProperties(ConversionContext context, MongoPersistentEntity continue; } - ConversionContext propertyContext = context.forProperty(prop.getName()); + ConversionContext propertyContext = context.forProperty(prop); MongoDbPropertyValueProvider valueProviderToUse = valueProvider.withContext(propertyContext); if (prop.isAssociation() && !entity.isConstructorArgument(prop)) { @@ -706,7 +745,7 @@ private void readAssociation(Association association, P accessor.setProperty(property, dbRefResolver.resolveReference(property, new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)), - referenceLookupDelegate, context::convert)); + referenceLookupDelegate, context.forProperty(property)::convert)); } return; } @@ -1935,13 +1974,13 @@ public T getPropertyValue(MongoPersistentProperty property) { return null; } - CustomConversions conversions = context.conversions; + CustomConversions conversions = context.getCustomConversions(); if (conversions.hasValueConverter(property)) { return (T) conversions.getPropertyValueConversions().getValueConverter(property).read(value, - new MongoConversionContext(property, context.sourceConverter)); + new MongoConversionContext(property, context.getSourceConverter())); } - ConversionContext contextToUse = context.forProperty(property.getName()); + ConversionContext contextToUse = context.forProperty(property); return (T) contextToUse.convert(value, property.getTypeInformation()); } @@ -2158,13 +2197,49 @@ public TypeDescriptor toTypeDescriptor() { } } + interface ConversionContext { + + default S convert(Object source, TypeInformation typeHint) { + return convert(source, typeHint, this); + } + + S convert(Object source, TypeInformation typeHint, ConversionContext context); + + ConversionContext withPath(ObjectPath currentPath); + + ObjectPath getPath(); + + default ConversionContext forProperty(String name) { + return this; + } + + default ConversionContext forProperty(@Nullable PersistentProperty property) { + + if (property != null) { + if (property.isAssociation()) { + return new AssociationConversionContext(forProperty(property.getName())); + } + return forProperty(property.getName()); + } + return this; + } + + default boolean resolveIdsInContext() { + return false; + } + + CustomConversions getCustomConversions(); + + MongoConverter getSourceConverter(); + } + /** * Conversion context holding references to simple {@link ValueConverter} and {@link ContainerValueConverter}. * Entrypoint for recursive conversion of {@link Document} and other types. * * @since 3.2 */ - protected static class ConversionContext { + protected static class DefaultConversionContext implements ConversionContext { final MongoConverter sourceConverter; final org.springframework.data.convert.CustomConversions conversions; @@ -2175,7 +2250,7 @@ protected static class ConversionContext { final ContainerValueConverter dbRefConverter; final ValueConverter elementConverter; - ConversionContext(MongoConverter sourceConverter, + DefaultConversionContext(MongoConverter sourceConverter, org.springframework.data.convert.CustomConversions customConversions, ObjectPath path, ContainerValueConverter documentConverter, ContainerValueConverter> collectionConverter, ContainerValueConverter mapConverter, ContainerValueConverter dbRefConverter, @@ -2199,7 +2274,8 @@ protected static class ConversionContext { * @return the converted object. */ @SuppressWarnings("unchecked") - public S convert(Object source, TypeInformation typeHint) { + public S convert(Object source, TypeInformation typeHint, + ConversionContext context) { Assert.notNull(source, "Source must not be null"); Assert.notNull(typeHint, "TypeInformation must not be null"); @@ -2219,18 +2295,18 @@ public S convert(Object source, TypeInformation } if (typeHint.isCollectionLike() || typeHint.getType().isAssignableFrom(Collection.class)) { - return (S) collectionConverter.convert(this, (Collection) source, typeHint); + return (S) collectionConverter.convert(context, (Collection) source, typeHint); } } if (typeHint.isMap()) { if (ClassUtils.isAssignable(Document.class, typeHint.getType())) { - return (S) documentConverter.convert(this, BsonUtils.asBson(source), typeHint); + return (S) documentConverter.convert(context, BsonUtils.asBson(source), typeHint); } if (BsonUtils.supportsBson(source)) { - return (S) mapConverter.convert(this, BsonUtils.asBson(source), typeHint); + return (S) mapConverter.convert(context, BsonUtils.asBson(source), typeHint); } throw new IllegalArgumentException( @@ -2238,7 +2314,7 @@ public S convert(Object source, TypeInformation } if (source instanceof DBRef) { - return (S) dbRefConverter.convert(this, (DBRef) source, typeHint); + return (S) dbRefConverter.convert(context, (DBRef) source, typeHint); } if (source instanceof Collection) { @@ -2247,31 +2323,41 @@ public S convert(Object source, TypeInformation } if (BsonUtils.supportsBson(source)) { - return (S) documentConverter.convert(this, BsonUtils.asBson(source), typeHint); + return (S) documentConverter.convert(context, BsonUtils.asBson(source), typeHint); } return (S) elementConverter.convert(source, typeHint); } + @Override + public CustomConversions getCustomConversions() { + return conversions; + } + + @Override + public MongoConverter getSourceConverter() { + return sourceConverter; + } + /** - * Create a new {@link ConversionContext} with {@link ObjectPath currentPath} applied. + * Create a new {@link DefaultConversionContext} with {@link ObjectPath currentPath} applied. * * @param currentPath must not be {@literal null}. - * @return a new {@link ConversionContext} with {@link ObjectPath currentPath} applied. + * @return a new {@link DefaultConversionContext} with {@link ObjectPath currentPath} applied. */ - public ConversionContext withPath(ObjectPath currentPath) { + public DefaultConversionContext withPath(ObjectPath currentPath) { Assert.notNull(currentPath, "ObjectPath must not be null"); - return new ConversionContext(sourceConverter, conversions, currentPath, documentConverter, collectionConverter, - mapConverter, dbRefConverter, elementConverter); + return new DefaultConversionContext(sourceConverter, conversions, currentPath, documentConverter, + collectionConverter, mapConverter, dbRefConverter, elementConverter); } public ObjectPath getPath() { return path; } - public ConversionContext forProperty(String name) { + public DefaultConversionContext forProperty(String name) { return this; } 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 44f6d700da..3f8c65d637 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 @@ -45,7 +45,6 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; - import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.ConversionNotSupportedException; import org.springframework.beans.factory.annotation.Autowired; @@ -2823,6 +2822,15 @@ public org.bson.Document write(@Nullable String domainValue, MongoConversionCont assertThat(read.viaRegisteredConverter).isEqualTo("spring"); } + @Test // GH-4098 + void resolvesCyclicNonAssociationValueFromSource/* and does not attempt to be smart and look up id values in context */() { + + org.bson.Document source = new org.bson.Document("_id", "id-1").append("value", "v1").append("cycle", + new org.bson.Document("_id", "id-1").append("value", "v2")); + + assertThat(converter.read(Cyclic.class, source).cycle.value).isEqualTo("v2"); + } + static class GenericType { T content; } @@ -3546,12 +3554,10 @@ static class WithFieldWrite { @org.springframework.data.mongodb.core.mapping.Field( write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Integer writeAlways; - @org.springframework.data.mongodb.core.mapping.DBRef - @org.springframework.data.mongodb.core.mapping.Field( + @org.springframework.data.mongodb.core.mapping.DBRef @org.springframework.data.mongodb.core.mapping.Field( write = org.springframework.data.mongodb.core.mapping.Field.Write.NON_NULL) Person writeNonNullPerson; - @org.springframework.data.mongodb.core.mapping.DBRef - @org.springframework.data.mongodb.core.mapping.Field( + @org.springframework.data.mongodb.core.mapping.DBRef @org.springframework.data.mongodb.core.mapping.Field( write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Person writeAlwaysPerson; } @@ -3665,4 +3671,12 @@ static class Author { } + @Data + static class Cyclic { + + @Id String id; + String value; + Cyclic cycle; + } + }