-
Notifications
You must be signed in to change notification settings - Fork 455
CDRIVER-4394 Add explicit encryption support and tests for range indexes #1152
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests for int LGTM
Let libmongocrypt do the error checking
Matches public API docs
Destroying is not necessary for primitive types like int/double. But prevents a leak if a heap allocated types (e.g. string) is passed.
Produces a consistent error if range_opts are unset for both explicit helpers
It is not required by callers
There is no need to yet. Once there is a stable version of libmongocrypt 1.7.0, will bump the dependency.
} | ||
} | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest that we have newline characters at the end of files.
static ree_fixture * | ||
range_explicit_encryption_setup (const char *typeStr) | ||
{ | ||
ree_fixture *reef = (ree_fixture *) bson_malloc0 (sizeof (ree_fixture)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In C, casting from void *
is not necessary, so I suggest that we don't cast on malloc. Reasoning can be found here: https://stackoverflow.com/a/605858
test_range_explicit_encryption_case3 (void *ctx) | ||
{ | ||
// Case 3: can find encrypted range and return the minimum | ||
const char *typeStr = (const char *) ctx; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Casting from void *
is not necessary, so I suggest we remove the cast.
test_range_explicit_encryption_case4 (void *ctx) | ||
{ | ||
// Case 4: can find encrypted range with an open range query | ||
const char *typeStr = (const char *) ctx; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Casting from void * is not necessary, so I suggest we remove the cast.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review. LGTM.
Summary
Adds support for explicit encryption with the Range Index.
mongoc_client_encryption_encrypt_range_opts
and pass to libmongocrypt.mongoc_client_encryption_encrypt_expression
.ASSERT_BSONVALUE_EQ
.Background & Motivation
See mongodb/specifications@4de4e3c for a description of the API change and prose tests.
Range Index requires MongoDB server 6.2+.
The API is marked experimental due to this comment.