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

fixing the find or delete serde inconsistency #948

Merged
merged 3 commits into from
Sep 18, 2023

Conversation

mlokr
Copy link
Contributor

@mlokr mlokr commented Aug 31, 2023

Hello,

I found an inconsistency in the way mongo documents are deserialized when using serde(with) and find_one_and_delete. My fix might nor be ideal because it breaks

< operation::find_and_modify::test::handle_no_value_delete                                                                                                                                                                                    
< operation::find_and_modify::test::handle_no_value_replace                      
< operation::find_and_modify::test::handle_no_value_update

The tests no longer return Err(_) but just Ok(None). I am not sure how I can assert that Field is present but can be null, when deserializing Option<T> and could not find anything either.

I added a tests to verify my changes fix the issue (see changes)

@abr-egn abr-egn self-assigned this Sep 7, 2023
@abr-egn
Copy link
Contributor

abr-egn commented Sep 7, 2023

Hi!

Thanks for reporting this! I particularly appreciate the included test case. At a first guess, the issue here is similar to others we've fixed recently where the helper function you're using has different logic for human-readable vs non-human-readable formats, and the driver code inadvertently serializes with one and deserializes with the other.

I'm going to reproduce the issue on my end and, if that's the case, it should be straightforward to adapt your PR to fix this without breaking the tests you note :)

@abr-egn abr-egn force-pushed the find-and-modify-serde-fix branch from 9d26e88 to b1a2e5b Compare September 13, 2023 15:11
@abr-egn
Copy link
Contributor

abr-egn commented Sep 13, 2023

I couldn't find a way to distinguish between null and "missing" in autogenerated Serde impls either; fortunately, we can just swap the Bson for a RawBson and check directly :)

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.

3 participants