Skip to content

Behavior breaking change deserializing property with null value #4571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pierre-gautier opened this issue Nov 28, 2023 · 6 comments
Closed
Labels
type: documentation A documentation update

Comments

@pierre-gautier
Copy link

Using spring-data-mongodb 4.1.5, when deserializaing a property with null value in database to an instanciated collection, the collection was not set to null.

With version 4.2.0, the collection is set to null.

While I do understand the intention and tends to be aligned with it, it's still a breaking change that may have numerous impacts in production.

WDYT ?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 28, 2023
@christophstrobl
Copy link
Member

Thank you for reporting! Do you by any chance have a complete minimal sample (something that we can unzip or git clone, build, and deploy) that reproduces the problem? That would help us quite a bit triaging the issue.

@pierre-gautier
Copy link
Author

I don't but I'll spend some time on it

@pierre-gautier
Copy link
Author

Would it be enough if I told you that the difference happens in MappingMongoConverter#readProperties ?
In 4.1.5 on line 610 documentAccessor.hasValue(prop) returns false while it returns true in 4.2.0.
Digging down it seems the issue comes from BsonUtils#hasValue that has changed here.

@christophstrobl
Copy link
Member

@pierre-gautier Thanks again for bringing this to our attention!
The current behaviour is the intended one, using the MongoDB Document as source of truth for collection initialization.
We'll add a couple of tests to the current test suite to verify the intended behaviour and extended the reference documentation to cover collection property initialization.

@mp911de mp911de linked a pull request Nov 30, 2023 that will close this issue
@mp911de mp911de added type: documentation A documentation update and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 30, 2023
@mp911de mp911de added this to the 4.2.1 (2023.1.1) milestone Nov 30, 2023
mp911de added a commit that referenced this issue Nov 30, 2023
Refactor fixture creation for easier readability. Tweak documentation wording.

See #4571
Original pull request: #4574
mp911de pushed a commit that referenced this issue Nov 30, 2023
Update the reference documentation about collection initialization on read, add the required tests to make sure it behaves as expected and simplify BeanUtils value presence check.

Closes #4571
Original pull request: #4574
mp911de added a commit that referenced this issue Nov 30, 2023
Refactor fixture creation for easier readability. Tweak documentation wording.

See #4571
Original pull request: #4574
@pierre-gautier
Copy link
Author

Thanks for your answer.
As I said before I'm actually quite OK with this philosophy.
But I need to confirm that you are aware about the fact that it is an important behavior change and upgrading from 4.1.5 to 4.2.0 will cause NPEs in most applications using initialized collections, are you OK with that ?

@mp911de
Copy link
Member

mp911de commented Dec 1, 2023

We should have introduced this change much earlier as the change is more honest. You can always intercept values in a constructor or, if you use @Access(PROPERTY), then you can intercept collection null values in setters.

Alternatively, you can make use of lifecycle events to post-process the Document values that are null for a collection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants