From 69c9e0f482f90528f3e01d57a198ae31bb5d6ead Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 1 Sep 2023 12:19:59 +0200 Subject: [PATCH] Consistently use the same reading strategies to read associations. Return the value to set instead of calling the accessor directly. Remove duplicate calls to resolve associations. See #4491 --- .../core/convert/MappingMongoConverter.java | 74 +++++++------------ 1 file changed, 28 insertions(+), 46 deletions(-) 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 3610d65532..ec5d866461 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 @@ -507,7 +507,6 @@ private S read(ConversionContext context, MongoPersistentEntity entity, D S instance = instantiator.createInstance(entity, provider); if (entity.requiresPropertyPopulation()) { - return populateProperties(context, entity, documentAccessor, evaluator, instance); } @@ -586,14 +585,18 @@ private void readProperties(ConversionContext context, MongoPersistentEntity ConversionContext propertyContext = context.forProperty(prop); MongoDbPropertyValueProvider valueProviderToUse = valueProvider.withContext(propertyContext); - if (prop.isAssociation() && !entity.isCreatorArgument(prop)) { + if (prop.isAssociation()) { if (callback == null) { callback = getDbRefResolverCallback(propertyContext, documentAccessor, evaluator); } - readAssociation(prop.getRequiredAssociation(), accessor, documentAccessor, dbRefProxyHandler, callback, - propertyContext, evaluator); + Object value = readAssociation(prop.getRequiredAssociation(), documentAccessor, dbRefProxyHandler, callback, + propertyContext); + + if (value != null) { + accessor.setProperty(prop, value); + } continue; } @@ -608,17 +611,6 @@ private void readProperties(ConversionContext context, MongoPersistentEntity continue; } - if (prop.isAssociation()) { - - if (callback == null) { - callback = getDbRefResolverCallback(propertyContext, documentAccessor, evaluator); - } - - readAssociation(prop.getRequiredAssociation(), accessor, documentAccessor, dbRefProxyHandler, callback, - propertyContext, evaluator); - continue; - } - accessor.setProperty(prop, valueProviderToUse.getPropertyValue(prop)); } } @@ -630,9 +622,10 @@ private DbRefResolverCallback getDbRefResolverCallback(ConversionContext context (prop, bson, e, path) -> MappingMongoConverter.this.getValueInternal(context, prop, bson, e)); } - private void readAssociation(Association association, PersistentPropertyAccessor accessor, + @Nullable + private Object readAssociation(Association association, DocumentAccessor documentAccessor, DbRefProxyHandler handler, DbRefResolverCallback callback, - ConversionContext context, SpELExpressionEvaluator evaluator) { + ConversionContext context) { MongoPersistentProperty property = association.getInverse(); Object value = documentAccessor.get(property); @@ -645,30 +638,27 @@ private void readAssociation(Association association, P if (conversionService.canConvert(DocumentPointer.class, property.getActualType())) { if (value == null) { - return; + return null; } DocumentPointer pointer = () -> value; // collection like special treatment - accessor.setProperty(property, conversionService.convert(pointer, property.getActualType())); + return conversionService.convert(pointer, property.getActualType()); } else { - accessor.setProperty(property, - dbRefResolver.resolveReference(property, + return dbRefResolver.resolveReference(property, new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)), - referenceLookupDelegate, context.forProperty(property)::convert)); + referenceLookupDelegate, context.forProperty(property)::convert); } - return; } if (value == null) { - return; + return null; } if (value instanceof DBRef dbref) { - accessor.setProperty(property, dbRefResolver.resolveDbRef(property, dbref, callback, handler)); - return; + return dbRefResolver.resolveDbRef(property, dbref, callback, handler); } /* @@ -679,18 +669,18 @@ private void readAssociation(Association association, P if (value instanceof Document document) { if (property.isMap()) { if (document.isEmpty() || peek(document.values()) instanceof DBRef) { - accessor.setProperty(property, dbRefResolver.resolveDbRef(property, null, callback, handler)); + return dbRefResolver.resolveDbRef(property, null, callback, handler); } else { - accessor.setProperty(property, readMap(context, document, property.getTypeInformation())); + return readMap(context, document, property.getTypeInformation()); } } else { - accessor.setProperty(property, read(property.getActualType(), document)); + return read(property.getActualType(), document); } } else if (value instanceof Collection collection && !collection.isEmpty() && peek(collection) instanceof Document) { - accessor.setProperty(property, readCollectionOrArray(context, collection, property.getTypeInformation())); + return readCollectionOrArray(context, collection, property.getTypeInformation()); } else { - accessor.setProperty(property, dbRefResolver.resolveDbRef(property, null, callback, handler)); + return dbRefResolver.resolveDbRef(property, null, callback, handler); } } @@ -1978,26 +1968,14 @@ public T getPropertyValue(MongoPersistentProperty property) { ConversionContext propertyContext = context.forProperty(property); - if (property.isDbReference() && property.getDBRef().lazy()) { - - Object rawRefValue = accessor.get(property); - if (rawRefValue == null) { - return null; - } + if (property.isAssociation()) { DbRefResolverCallback callback = new DefaultDbRefResolverCallback(accessor.getDocument(), context.getPath(), evaluator, (prop, bson, evaluator, path) -> MappingMongoConverter.this.getValueInternal(context, prop, bson, evaluator)); - DBRef dbref = rawRefValue instanceof DBRef dbRef ? dbRef : null; - return (T) dbRefResolver.resolveDbRef(property, dbref, callback, dbRefProxyHandler); - } - - if (property.isDocumentReference()) { - - return (T) dbRefResolver.resolveReference(property, - new DocumentReferenceSource(accessor.getDocument(), accessor.get(property)), referenceLookupDelegate, - context::convert); + return (T) readAssociation(property.getRequiredAssociation(), accessor, dbRefProxyHandler, callback, + propertyContext); } if (property.isUnwrapped()) { @@ -2006,6 +1984,10 @@ public T getPropertyValue(MongoPersistentProperty property) { mappingContext.getRequiredPersistentEntity(property)); } + if (!accessor.hasValue(property)) { + return null; + } + return super.getPropertyValue(property); } }