Skip to content

DRIVERS-2604 deduplicate encrypted fields files #1400

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 8 commits into from
Apr 24, 2023

Conversation

kevinAlbs
Copy link
Contributor

@kevinAlbs kevinAlbs commented Apr 11, 2023

Summary

  • Replace use of encryptedFields-Range-* files with range-encryptedFields-*.
  • Replace references to encryptedDouble and encryptedDecimal with encryptedDoubleNoPrecision and encryptedDecimalNoPrecision to match the field name in the range-encryptedFields-* files.

There is no expected test coverage or behavior change.

Changes tested with C driver in this patch build: https://spruce.mongodb.com/version/6436ce262a60edecd5b8f761

Background & Motivation

Motivated by this comment: #1396 (comment)
The range-encryptedFields-*.json files and the encryptedFields-Range-*.json files are almost identical.

14c5099 adds contention to encryptedFields-Range-*.json. The server response to listCollections includes the default value chosen for contention. encryptedFields-Range-*.json is used both for collection creation and for the command_started event assertions. Without adding contention to the expected command_started event, the test assertion fails since the actual event contains the default contention.

9b2df75 uses $numberLong consistently for sparsity. The server accepts both $numberLong and $numberInt. The only motivation was for consistency.

Please complete the following before merging:

  • [ ] Update changelog. N/A. Only test files changed
  • Make sure there are generated JSON files from the YAML test files.
  • Test changes in at least one language driver. Tested in C driver
  • [ ] Test these changes against all server versions and topologies (including standalone, replica set, sharded clusters, and serverless). Tested in C driver on replica set only.

@kevinAlbs kevinAlbs force-pushed the remove-duplicate-encryptedFields branch from 32b6d35 to e303013 Compare April 12, 2023 15:18
@kevinAlbs kevinAlbs changed the title Remove duplicate encrypted fields DRIVERS-2604 deduplicate encrypted fields files Apr 12, 2023
@kevinAlbs kevinAlbs marked this pull request as ready for review April 12, 2023 18:23
@kevinAlbs kevinAlbs requested a review from a team as a code owner April 12, 2023 18:23
@kevinAlbs kevinAlbs requested review from katcharov and jmikola and removed request for a team and katcharov April 12, 2023 18:23
Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

I only glossed over the regenerated test files, but the template and data file changes LGTM. Thanks for cleaning this up!

If contention is not present, the server will provide a default value.
The default value will be included in the `command_started_event`.
Adding `contention` to the `range-encryptedFields-*` files enable the same YAML to be used for `encrypted_fields` and the `command_started_event` in specification tests.
Server accepts either. Use numberLong for consistency.
@kevinAlbs kevinAlbs force-pushed the remove-duplicate-encryptedFields branch from e303013 to a338dc7 Compare April 21, 2023 21:03
kevinAlbs added a commit to kevinAlbs/mongo-c-driver that referenced this pull request Apr 24, 2023
@kevinAlbs kevinAlbs merged commit c4f3fab into mongodb:master Apr 24, 2023
kevinAlbs added a commit to mongodb/mongo-c-driver that referenced this pull request Apr 24, 2023
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