Skip to content

Conversation

mdb-ad
Copy link
Contributor

@mdb-ad mdb-ad commented Aug 11, 2025

No description provided.

@mdb-ad mdb-ad changed the title Text search explicit encryption API and prose tests CDRIVER-6044 Text search explicit encryption API and prose tests Aug 28, 2025
@mdb-ad mdb-ad marked this pull request as ready for review August 28, 2025 06:32
@mdb-ad mdb-ad requested a review from a team as a code owner August 28, 2025 06:32
@mdb-ad mdb-ad requested review from Julia-Garland and kevinAlbs and removed request for Julia-Garland August 28, 2025 06:32
@kevinAlbs kevinAlbs self-requested a review October 10, 2025 19:58
@@ -0,0 +1,53 @@
:man_page: mongoc_encrypt_text_opts_t

mongoc_encrypt_text_opts_t
Copy link
Collaborator

Choose a reason for hiding this comment

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

See the make-docs task for docs warnings causing the task to fail:

mongoc_encrypt_text_opts_t.rst: WARNING: document isn't included in any toctree

Docs can be built with instructions from CONTRIBUTING.md:

uv run --frozen --with 'sphinx-autobuild' sphinx-autobuild -b html src/libmongoc/doc/ src/libmongoc/doc/html --re-ignore ".*.pickle" --re-ignore ".*.doctree" -j auto

typedef struct {
bool set;
int32_t value;
} mc_optional_int32_t;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
} mc_optional_int32_t;
} mcd_optional_int32_t;

Suggest giving this a different name than the mc_optional_int32_t symbol in libmongocrypt. The PHP driver builds the C driver and libmongocrypt side-by-side, so this may cause redefinition errors. Quoting MONGOCRYPT-845:

libmongoc, libbson, and libmongocrypt are all compiled statically into the PHP extension

typedef struct {
bool set;
bool value;
} mc_optional_bool;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
} mc_optional_bool;
} mcd_optional_bool;

Alternatively: use the existing mongoc_optional_t (which I would rather name mongoc_optional_bool_t, but it is a public type).


// Setters for text opts
void
mongoc_encrypt_text_opts_set_prefix(mongoc_encrypt_text_opts_t *opts, mongoc_encrypt_text_prefix_opts_t *popts)
Copy link
Collaborator

Choose a reason for hiding this comment

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

On this function and other setters, use const since argument is only copied:

Suggested change
mongoc_encrypt_text_opts_set_prefix(mongoc_encrypt_text_opts_t *opts, mongoc_encrypt_text_prefix_opts_t *popts)
mongoc_encrypt_text_opts_set_prefix(mongoc_encrypt_text_opts_t *opts, const mongoc_encrypt_text_prefix_opts_t *popts)

mongoc_client_encryption_encrypt_opts_set_text_opts(mongoc_client_encryption_encrypt_opts_t *opts,
const mongoc_encrypt_text_opts_t *text_opts)
{
BSON_ASSERT_PARAM(opts);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
BSON_ASSERT_PARAM(opts);
BSON_ASSERT_PARAM(opts);
BSON_ASSERT_PARAM(text_opts);

typedef struct _mongoc_client_encryption_encrypt_range_opts_t mongoc_client_encryption_encrypt_range_opts_t;
typedef struct _encrypt_text_per_index_opts_t mongoc_encrypt_text_prefix_opts_t;
typedef struct _encrypt_text_per_index_opts_t mongoc_encrypt_text_suffix_opts_t;
typedef struct _encrypt_text_per_index_opts_t mongoc_encrypt_text_substring_opts_t;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggest avoiding defining prefix/suffix/substring as a typedef of the same struct: _encrypt_text_per_index_opts_t. I would rather avoid exposing the same struct is used for all three in public API. Future options might make these prefix/suffix/substring no longer be the same.

Consider instead:

typedef struct _mongoc_encrypt_text_prefix_opts_t mongoc_encrypt_text_prefix_opts_t;
typedef struct _mongoc_encrypt_text_suffix_opts_t mongoc_encrypt_text_suffix_opts_t;
typedef struct _mongoc_encrypt_text_substring_opts_t mongoc_encrypt_text_substring_opts_t;

Internally, share the same definition for convenience:

// In mongoc-client-side-encryption.c:

struct _encrypt_text_per_index_opts_t {
   mc_optional_int32_t str_max_length;
   mc_optional_int32_t str_max_query_length;
   mc_optional_int32_t str_min_query_length;
};

struct _mongoc_encrypt_text_prefix_opts_t {
   struct _encrypt_text_per_index_opts_t per_index_opts;
};

struct _mongoc_encrypt_text_suffix_opts_t {
   struct _encrypt_text_per_index_opts_t per_index_opts;
};

struct _mongoc_encrypt_text_substring_opts_t {
   struct _encrypt_text_per_index_opts_t per_index_opts;
};

mongoc_client_encryption_encrypt_opts_t *eo = mongoc_client_encryption_encrypt_opts_new();
mongoc_client_encryption_encrypt_opts_set_keyid(eo, &eef->key1ID);
mongoc_client_encryption_encrypt_opts_set_algorithm(eo, MONGOC_ENCRYPT_ALGORITHM_TEXTPREVIEW);
mongoc_client_encryption_encrypt_opts_set_query_type(eo, MONGOC_ENCRYPT_QUERY_TYPE_PREFIXPREVIEW);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
mongoc_client_encryption_encrypt_opts_set_query_type(eo, MONGOC_ENCRYPT_QUERY_TYPE_PREFIXPREVIEW);
mongoc_client_encryption_encrypt_opts_set_query_type(eo, MONGOC_ENCRYPT_QUERY_TYPE_SUBSTRINGPREVIEW);

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