MappingMongoConverter eagerly fetches and converts lazy DbRef to change them afterwards by proxies [DATAMONGO-1287]

[spring-projects-issues]

Jordi Llach Fernandez opened DATAMONGO-1287 and commented

The problem occurs whenever an association against a document contains an annotation @DBRef(lazy=true) , AND this association is defined as a parameter in the constructor of the parent document.
If the association is not defined as a parameter in the constructor everything works fine because MappingMongoConverter will use an instance of ConvertingPropertyAccessor to inject those proxies where needed later, in the "associations" phase.

MappingMongoConverter.read(MongoPersistentEntity<S>, DBObject, ObjectPath) creates a MongoDbPropertyValueProvider which is passed as a parameter to the EntityInstantiator in order to load and convert the constructor parameters of each document. These constructor parameters are retrieved in the EntityInstantiator one by one through MongoDbPropertyValueProvider's method getParameterValue(Parameter), if the parameter does not have any SpEL expression the call flow goes down to MongoDbPropertyValueProvider.getPropertyValue(MongoPersistentProperty) which will use MappingMongoConverter.readValue(Object, TypeInformation<?>, ObjectPath) to retrieve the value of the parameter, BUT this method ALWAYS resolves the DBRef (no matter if it's lazy or not).

The issue is not visible by inspecting the returned object because the "association phase" in MappingMongoConverter.read(MongoPersistentEntity<S>, DBObject, ObjectPath) does not skip association properties already set before, and thus it fetches and converts AGAIN DBRef and sets proxies to lazy DBRef properties already resolved making the bug "invisible"

---

Issue Links:

• DATAMONGO-1271 Provide read lifecycle events when loading DBRefs
  ("is depended on by")

• DATAMONGO-1319 Lazily loaded DBRef fields fail to resolve DBRefs contained within

• DATAMONGO-2004 Lazily resolve DBRefs during constructor creation of the enclosing entity

Referenced from: pull request #335

Backported to: 1.8.2 (Gosling SR2), 1.7.3 (Fowler SR3)

2 votes, 3 watchers

[spring-projects-issues]

Jordi Llach Fernandez commented

I found this issue when preparing the tests for https://jira.spring.io/browse/DATAMONGO-1271

[spring-projects-issues]

Christoph Strobl commented

good catch Jordi Llach Fernandez. Thanks!

[spring-projects-issues]

Jordi Llach Fernandez commented

Hi Christoph,
I could be wrong and haven't got much time to take a deeper look at https://github.com/spring-projects/spring-data-mongodb/pull/335/commits but I think that ONLY one bug is solved, to me any DBRef(lazy=true) in the constructor is still eagerly fetched/resolved.

And by the way I think that this bug is the reason why in https://jira.spring.io/browse/DATAMONGO-1332 a stackoverflow error is thrown no matter if DBRefs are lazy or not because both are eagerly fetched.

J

[spring-projects-issues]

Jordi Llach Fernandez commented

When DBRef(lazy=true) are resolved in ReflectionEntityInstantiator NONE of the possible ParameterValueProvider received knows how to deal with lazy DBRref.
In my case the call flow ends in PersistentEntityParameterValueProvider which in turn uses MappingMongoConverter$MongoDbPropertyValueProvider, which in turn delegates to MappingMongoConverter.readValue(Object value, TypeInformation<?> type, ObjectPath path) whom by calling potentiallyReadOrResolveDbRef eagerly resolves the DBRef.

The point is that DefaultDbRefResolver class used in the association phase performed by MappingMongoConverter is the only one who knows how to deal with DbRef(lazy=true)

That's the reason why in my PR I modified how the ParameterValueProvider is built in MappingMongoConverter's method getParameterProvider(MongoPersistentEntity entity, DBObject source, DefaultSpELExpressionEvaluator evaluator, ObjectPath path) in order to build a specialized MongoDbPropertyValueProvider which by default does not deal with DBRef(lazy=true)

[spring-projects-issues]

Jordi Llach Fernandez commented

Another thing that is banging my head is how you avoid the double fetch of NON lazy DBRef

if (value == null|| (entity.getPersistenceConstructor().hasParameters() && entity.isConstructorArgument(property)
&& accessor.getProperty(property) != null)) return;

Why are you checking if the property is there with the object accessor ? In the worst case the code will try to fetch again a DBRef pointing nowhere already dealt by the entity instantiator

Thanks!

[spring-projects-issues]

Christoph Strobl commented

thanks Jordi Llach Fernandez true - totally overlooked the fact with the accessor - removing the accessor seems to be a good idea.
Concerning usage of lazy dbref as a constructor arg, I stronlgly think that those should be fetched eagerly in any case or just not be used within the constructor. I'll check DATAMONGO-1332 as well to see what happens there.

[spring-projects-issues]

Jordi Llach Fernandez commented

About the usage of lazy dbref as a constructor arg, that decision is up to the SpringData team for sure, but I encourage you to add a side note about it to the docs.
I am quite sure that other devs like me assume that their usage in constructors is fine(do not involve eagerly fetching)

[spring-projects-issues]

Christoph Strobl commented

Jordi Llach Fernandez, I just wanted to let you know that we've altered the lazy DBRef resolution of constructor arguments (DATAMONGO-2004) for the upcoming 2.1 release

[spring-projects-issues]

Jordi Llach Fernandez commented

Thanks for the feedback. I really appreciate it