Skip to content

RUST-1826 Use serde attribute to remove empty write concerns #1392

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

Merged
merged 4 commits into from
Jun 10, 2025

Conversation

JamieTsai1024
Copy link
Collaborator

@JamieTsai1024 JamieTsai1024 commented Jun 9, 2025

Replaced remove_empty_write_concern macro with serde attribute #[serde(skip_serializing_if = "write_concern_is_empty")] to remove empty write concerns before serialization.

Updated structs

Applied the new serde attribute to write_concern fields in the following structs that previously used remove_empty_write_concern:

  • TransactionOptions
  • DeleteOptions
  • AggregateOptions
  • CreateIndexOptions
  • DropCollectionOptions
  • CreateCollectionOptions
  • DropDatabaseOptions
  • FindAndModifyOptions
  • DropIndexOptions

Also updated additional structs with write_concern fields that serialized empty values before:

  • BulkWriteOptions
  • InsertOneOptions
  • ReplaceOptions
  • FindOneAndDeleteOptions
  • FindOneAndReplaceOptions
  • FindOneAndUpdateOptions

Special cases
Certains structs with write_concern fields do not use the serde attribute for one of the following reasons:

  • Fields are added manually only if not empty
    • E.x. UpdateOptions
  • The struct itself does not implement serialization (Serialize)
    • E.x. CollectionOrDatabaseOptions, DatabaseOptions, GridFsBucketOptions, AbortTransaction

@JamieTsai1024 JamieTsai1024 marked this pull request as ready for review June 9, 2025 18:47
@JamieTsai1024 JamieTsai1024 requested a review from a team as a code owner June 9, 2025 18:47
Copy link
Contributor

@isabelatkinson isabelatkinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look great! There are just a few CI issues to address:

  • The check-cargo-deny and compile-on-msrv tasks can both be fixed by merging with the latest commits on main
  • The check-rustfmt lint can be fixed by running rustfmt +nightly --unstable-features src/**/*.rs

Copy link
Contributor

@isabelatkinson isabelatkinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work!

@JamieTsai1024 JamieTsai1024 merged commit 65ce423 into mongodb:main Jun 10, 2025
18 checks passed
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