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 045181476..c74d8832a 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 @@ -38,7 +38,6 @@ import org.bson.codecs.DecoderContext; 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; @@ -187,7 +186,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App 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); } @@ -396,7 +395,46 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App 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; @@ -412,12 +450,13 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } @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, @@ -425,7 +464,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } @Override - public ConversionContext withPath(ObjectPath currentPath) { + public DefaultConversionContext withPath(ObjectPath currentPath) { return new ProjectingConversionContext(sourceConverter, conversions, currentPath, collectionConverter, mapConverter, dbRefConverter, elementConverter, returnedTypeDescriptor); } @@ -544,7 +583,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App 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; @@ -647,7 +686,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App continue; } - ConversionContext propertyContext = context.forProperty(prop.getName()); + ConversionContext propertyContext = context.forProperty(prop); MongoDbPropertyValueProvider valueProviderToUse = valueProvider.withContext(propertyContext); if (prop.isAssociation() && !entity.isConstructorArgument(prop)) { @@ -721,7 +760,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App accessor.setProperty(property, dbRefResolver.resolveReference(property, new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)), - referenceLookupDelegate, context::convert)); + referenceLookupDelegate, context.forProperty(property)::convert)); } return; } @@ -1971,13 +2010,13 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return null; } - CustomConversions conversions = context.conversions; + CustomConversions conversions = context.getCustomConversions(); if (conversions.getPropertyValueConversions().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()); } @@ -2201,13 +2240,49 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } } + 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; @@ -2218,7 +2293,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App 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, @@ -2242,7 +2317,8 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App * @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"); @@ -2262,18 +2338,18 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } 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( @@ -2281,7 +2357,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } if (source instanceof DBRef) { - return (S) dbRefConverter.convert(this, (DBRef) source, typeHint); + return (S) dbRefConverter.convert(context, (DBRef) source, typeHint); } if (source instanceof Collection) { @@ -2290,31 +2366,41 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } 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 4a8602f35..446ec6f81 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 @@ -44,7 +44,6 @@ import org.junit.jupiter.api.extension.ExtendWith; 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; @@ -2850,6 +2849,15 @@ class MappingMongoConverterUnitTests { 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; } @@ -3573,12 +3581,10 @@ class MappingMongoConverterUnitTests { @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; } @@ -3692,4 +3698,12 @@ class MappingMongoConverterUnitTests { } + @Data + static class Cyclic { + + @Id String id; + String value; + Cyclic cycle; + } + }