RUST-1687 Add human_readable_serialization option to Collection

[maiconpavi]

…find_one_and_replace_common

This was a solution proposed by @isabelatkinson for the problem described in the following PR and the Task

Regards,
Maicon Pavi.

[isabelatkinson]

Hi @maiconpavi, thank you for making this pull request! Just wanted to let you know that because this is not a trivial addition to our API, the team will need to have some discussion around design/testing. I will let you know how we want to proceed with these changes once we come to an agreement.

[isabelatkinson]

Hi @maiconpavi apologies for the delays and back-and-forth on this! After further discussion and a bug report from another user, we decided to proceed with your original changes despite the possibility of breaking user code. These changes were merged in #919. We still would like to add the ability to make human_readable configurable, however, as a mitigation for users who may have been relying upon update using a human-readable serializer. To ensure consistency across methods, we decided that this option should be added at the collection level. This would entail adding a human_readable_serialization optional boolean field to CollectionOptions and applying it in Collection methods that serialize user data.

Please let me know if you'd be interested in updating this PR to the design I've described and adding a few tests; otherwise, I'd be happy to take it on myself. We plan to release the fix from #919 and the collection-level option together. Thank you again for bringing this issue to our attention!

[maiconpavi]

Hi @isabelatkinson, I'm interested to make this change. I will make in this week. Thank you.

Regards,
Maicon Pavi.

[maiconpavi]

Hi @isabelatkinson,

When you say serialize user data this includes Update and Insert operations?
like the document to be inserted and the update part of the update operation.

Regards,
Maicon Pavi.

[isabelatkinson]

Hi @maiconpavi, this option should apply to any method on Collection that serializes the user's data into BSON. That includes the following methods:

• insert_one
• insert_many
• replace_one
• find_one_and_replace

The changes in #924 (which I will be merging in soon) should give you an idea of how to apply the option for the replace methods. The insert methods will be a bit different, as we don't serialize insert documents until we build the insert command here. to_raw_document_buf does not accept any serializer options, so you'll need to go through to_document_with_options first in order to apply the option if human_readable_serialization is true.

[maiconpavi]

Hi @isabelatkinson
Thanks for the patience, i just committed the solution proposed.

Regards,
Maicon Pavi.

[isabelatkinson]

Thanks for your work on this! I have a few suggestions.

[maiconpavi]

Hello @isabelatkinson
Thanks for the suggestions, i committed the improvements that you suggested.
Maybe it would be interesting to add the serialization options to the bson so that it doesn't have this performance penalty, i can do that, what do you think?

Regards,
Maicon Pavi.

[isabelatkinson]

Hi @maiconpavi, the code changes look great! I just rebased onto main to resolve merge conflicts and fixed up a few small test-related issues. Once CI passes I will approve this and tag in the other team member for review. Thanks for your work on this PR!

Maybe it would be interesting to add the serialization options to the bson so that it doesn't have this performance penalty

human_readable is configurable on the regular Bson type because it's somewhat of a hybrid between a human-readable and non-human-readable format: the Rust-specific Bson enum is certainly human-readable, but it gets serialized to BSON bytes that are not human-readable. On the other hand, RawBson (and by extension RawDocumentBuf) is not human-readable data type because it is just the raw BSON bytes, so it would not make as much sense to provide configuration.

[abr-egn]

LGTM!