Skip to content
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

fix behavior when updating values where the value was previously null #961

Merged
merged 1 commit into from
May 6, 2024

Conversation

paulovictorv
Copy link
Contributor

@paulovictorv paulovictorv commented May 6, 2024

Context

When trying to update nested documents, if the value was previously null, a NPE was raised. This was because the code inside NitriteDocument::merge wasn't null checking the value after taking it out of the map.

Fix

Added a null check. If the value returned from get is null, I assumed we should follow the default behavior and just replace it with whatever was provided by the method. If not, we then use merge. While fixing this bug I noticed that there's no unit tests for NitriteDocument::merge but they are covered indirectly via the CollectionUpdateTest. I added a new test case to the existing suite.

The code works but I'm up for comments - I'm not sure about the full implications of doing this fix so LMK if there's something I should fix or do differently :)

@anidotnet
Copy link
Contributor

Thanks for the contribution.

@anidotnet anidotnet merged commit f4c4644 into nitrite:main May 6, 2024
10 checks passed
@paulovictorv
Copy link
Contributor Author

@anidotnet when are you planning to release this fix?

@anidotnet
Copy link
Contributor

anidotnet commented May 7, 2024

I'll publish the snapshot version by today and release version possibly by this month end.

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

Successfully merging this pull request may close these issues.

2 participants