Skip to content

Add test_example_59 #1143

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 31 commits into from
Nov 16, 2022
Merged
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
21e7653
Add test_example_59
kkloberdanz Nov 9, 2022
d038c92
setup boilerplate
kkloberdanz Nov 9, 2022
12635e6
check cursor for results
kkloberdanz Nov 9, 2022
103bcda
populating cats
kkloberdanz Nov 9, 2022
cacada0
successfully comparing document with control
kkloberdanz Nov 9, 2022
4df0e3c
goto cleanup
kkloberdanz Nov 9, 2022
af7d801
lookup adoptableCatsCount
kkloberdanz Nov 10, 2022
d697998
seeding dogs collection
kkloberdanz Nov 10, 2022
b73bfe3
a little DRYer
kkloberdanz Nov 10, 2022
433376a
use snapshot session
kkloberdanz Nov 10, 2022
8edc763
fmt
kkloberdanz Nov 10, 2022
3fc0e8a
is_equal flag no longer used
kkloberdanz Nov 10, 2022
8138aba
clang-format
kkloberdanz Nov 10, 2022
352afa4
check for error first
kkloberdanz Nov 10, 2022
2985bb3
destroy client
kkloberdanz Nov 10, 2022
5f513ac
Merge branch 'master' into CDRIVER-4295
kkloberdanz Nov 10, 2022
14b86b0
warning for using switch, using if/else instead
kkloberdanz Nov 10, 2022
31d615a
move boiler plate seeding into pet_setup
kkloberdanz Nov 10, 2022
57d9a89
move closer to related function
kkloberdanz Nov 10, 2022
4657565
added suggestions from Kevin
kkloberdanz Nov 14, 2022
ffb07d0
lld is not portable for int64_t, use macro PRId64 instead.
kkloberdanz Nov 14, 2022
5709929
end example before assertion
kkloberdanz Nov 14, 2022
3c62a01
comment on how the aggregation pipeline is being performed
kkloberdanz Nov 14, 2022
c0479ab
missing punctuation
kkloberdanz Nov 14, 2022
036e287
show all example code
kkloberdanz Nov 14, 2022
1069a12
redundant
kkloberdanz Nov 15, 2022
a467c55
Revert "redundant"
kkloberdanz Nov 15, 2022
22890b2
Revert "show all example code"
kkloberdanz Nov 15, 2022
479e472
Update src/libmongoc/tests/test-mongoc-sample-commands.c
kkloberdanz Nov 16, 2022
a5e5a5e
Update src/libmongoc/tests/test-mongoc-sample-commands.c
kkloberdanz Nov 16, 2022
4b242db
Update src/libmongoc/tests/test-mongoc-sample-commands.c
kkloberdanz Nov 16, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
205 changes: 205 additions & 0 deletions src/libmongoc/tests/test-mongoc-sample-commands.c
Original file line number Diff line number Diff line change
Expand Up @@ -2474,6 +2474,210 @@ test_example_56 (mongoc_database_t *db)
}


/* clang-format on */

static bool
insert_pet (mongoc_collection_t *collection, bool is_adoptable)
{
bson_t *doc = NULL;
bson_error_t error;
bool rc;

doc = BCON_NEW ("adoptable", BCON_BOOL (is_adoptable));

rc = mongoc_collection_insert_one (collection, doc, NULL, NULL, &error);
if (!rc) {
MONGOC_ERROR ("insert into pets.%s failed: %s",
mongoc_collection_get_name (collection),
error.message);
goto cleanup;
}

cleanup:
bson_destroy (doc);
return rc;
}


static bool
pet_setup (mongoc_collection_t *cats_collection,
mongoc_collection_t *dogs_collection)
{
bool ok = true;

mongoc_collection_drop (cats_collection, NULL);
mongoc_collection_drop (dogs_collection, NULL);

ok = insert_pet (cats_collection, true);
if (!ok) {
goto done;
}

ok = insert_pet (dogs_collection, true);
if (!ok) {
goto done;
}

ok = insert_pet (dogs_collection, false);
if (!ok) {
goto done;
}
done:
return ok;
}


/*
* Increment 'accumulator' by the amount of adoptable pets in the given
* collection.
*/
static bool
accumulate_adoptable_count (const mongoc_client_session_t *cs,
mongoc_collection_t *collection,
int64_t *accumulator /* OUT */
)
{
bson_t *pipeline = NULL;
mongoc_cursor_t *cursor = NULL;
bool rc = false;
const bson_t *doc = NULL;
bson_error_t error;
bson_iter_t iter;
bson_t opts = BSON_INITIALIZER;

rc = mongoc_client_session_append (cs, &opts, &error);
if (!rc) {
MONGOC_ERROR ("could not apply session options: %s", error.message);
goto cleanup;
}

pipeline = BCON_NEW ("pipeline",
"[",
"{",
"$match",
"{",
"adoptable",
BCON_BOOL (true),
"}",
"}",
"{",
"$count",
BCON_UTF8 ("adoptableCount"),
"}",
"]");

cursor = mongoc_collection_aggregate (
collection, MONGOC_QUERY_NONE, pipeline, &opts, NULL);
bson_destroy (&opts);

rc = mongoc_cursor_next (cursor, &doc);

if (mongoc_cursor_error (cursor, &error)) {
MONGOC_ERROR ("could not get adoptableCount: %s", error.message);
rc = false;
goto cleanup;
}

if (!rc) {
MONGOC_ERROR ("%s", "cursor has no results");
goto cleanup;
}

rc = bson_iter_init_find (&iter, doc, "adoptableCount");
if (rc) {
*accumulator += bson_iter_as_int64 (&iter);
} else {
MONGOC_ERROR ("%s", "missing key: 'adoptableCount'");
goto cleanup;
}

cleanup:
bson_destroy (pipeline);
mongoc_cursor_destroy (cursor);
return rc;
}

/*
* JIRA: https://jira.mongodb.org/browse/DRIVERS-2181
*/
static void
test_example_59 (mongoc_database_t *db)
{
/* Start Example 59 */
mongoc_client_t *client = NULL;
mongoc_client_session_t *cs = NULL;
mongoc_collection_t *cats_collection = NULL;
mongoc_collection_t *dogs_collection = NULL;
int64_t adoptable_pets_count = 0;
bson_error_t error;
mongoc_session_opt_t *session_opts;

client = test_framework_new_default_client ();

cats_collection = mongoc_client_get_collection (client, "pets", "cats");
dogs_collection = mongoc_client_get_collection (client, "pets", "dogs");

/* Seed 'pets.cats' and 'pets.dogs' with example data */
if (!pet_setup (cats_collection, dogs_collection)) {
goto cleanup;
}

/* start a snapshot session */
session_opts = mongoc_session_opts_new ();
mongoc_session_opts_set_snapshot (session_opts, true);
cs = mongoc_client_start_session (client, session_opts, &error);
mongoc_session_opts_destroy (session_opts);
if (!cs) {
MONGOC_ERROR ("Could not start session: %s", error.message);
goto cleanup;
}

/*
* Perform the following aggregation pipeline, and accumulate the count in
* `adoptable_pets_count`.
*
* adoptablePetsCount = db.cats.aggregate(
* [ { "$match": { "adoptable": true } },
* { "$count": "adoptableCatsCount" } ], session=s
* ).next()["adoptableCatsCount"]
*
* adoptablePetsCount += db.dogs.aggregate(
* [ { "$match": { "adoptable": True} },
* { "$count": "adoptableDogsCount" } ], session=s
* ).next()["adoptableDogsCount"]
*
* Remember in order to apply the client session to
* this operation, you must append the client session to the options passed
* to `mongoc_collection_aggregate`, i.e.,
*
* mongoc_client_session_append (cs, &opts, &error);
* cursor = mongoc_collection_aggregate (
* collection, MONGOC_QUERY_NONE, pipeline, &opts, NULL);
*/
accumulate_adoptable_count (cs, cats_collection, &adoptable_pets_count);
accumulate_adoptable_count (cs, dogs_collection, &adoptable_pets_count);

printf ("there are %" PRId64 " adoptable pets\n", adoptable_pets_count);

Copy link
Contributor

@galon1 galon1 Nov 14, 2022

Choose a reason for hiding this comment

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

It might be helpful to add a comment here describing that we are using an aggregation pipeline to return the count, since other drivers have been displaying the aggregation pipeline command used. The comment could also be describing how the aggregation pipeline command is omitted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm glad you brought this up, because that's something I'm curious about. It would be a fair amount of duplicate code and a bit lengthy to inline both calls to accumulate_adoptable_count in this function, but I'm wondering if I should do one of the following:

  1. Include the entire function accumulate_adoptable_count in the example, i.e., place the start example comment just above the declaration of accumulate_adoptable_count, and end example comment stays where it is within test_example_59.
  2. Place the text of accumulate_adoptable_count in a comment in test_example_59.
  3. Explain in a comment what is happening in the function accumulate_adoptable_count purely in English, and omit code in the comment.
  4. Inline all the code from accumulate_adoptable_count within test_example_59.

I'm leaning towards 1., but I'm not sure how that would look when rendered. I'd like to know what everyone thinks about this, since I'm not sure which way would be the cleanest for the purpose of producing an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

P.S. I'm planning on reaching out to Jeffrey Allen, the reporter for the original ticket, regarding this before merging.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be the most helpful to include the entire function, but since that is many more lines of code than the Python example, we could do option 3 and just describe the aggregation pipeline run. There are other pages (like this one in the C documentation that walks through writing an aggregation pipeline, so it might not be necessary to show every single step in accumulate_adoptable_count .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. One detail that would be good to include is that the client session must be appended to the options passed to mongoc_collection_aggregate. If we go with option 3., then I suppose that could be explained also in a comment, but it seems like showing the code for it would be the best.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got some feed back from Jeffery Allen. It sounds like he's in favor of option 3.

(quote from Jeff)

Hey Kyle, thanks for reaching out. So my recollection with that ticket is that the main goal is to emphasize the snapshot functionality in the example. Therefore, I'm inclined to say that we can exclude the agg pipeline from the example, and leave a comment explaining the aggregation. That way, users aren't too overwhelmed by a large example and they'll still be able to see the snapshot behavior in action

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree option 3 would be the clearest, but I'm also not sure about how strict the formatting constraints are. Maybe the docs team could help make the choice.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just saw the addition of the comment! Ignore my comment above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's interesting. I do like when an example is explicit, since this example includes 2 functions, then I suppose no reason why I can't either. I'll make that change for now, and request another review from both you and Kevin to see what you think.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Defer to Jeff's judgement since this example is requested by the documentation team.

/* End Example 59 */

if (adoptable_pets_count != 2) {
MONGOC_ERROR (
"there should be exatly 2 adoptable_pets_count, found: %" PRId64,
adoptable_pets_count);
}

/* Start Example 59 Post */
cleanup:
mongoc_collection_destroy (dogs_collection);
mongoc_collection_destroy (cats_collection);
mongoc_client_session_destroy (cs);
mongoc_client_destroy (client);
/* End Example 59 Post */
}

/* clang-format off */

typedef struct {
bson_mutex_t lock;
mongoc_collection_t *collection;
Expand Down Expand Up @@ -4000,6 +4204,7 @@ test_sample_commands (void)
test_sample_command (test_example_57, 57, db, collection, false);
test_sample_command (test_example_58, 58, db, collection, false);
test_sample_command (test_example_56, 56, db, collection, true);
test_sample_command (test_example_59, 59, db, collection, true);
test_sample_change_stream_command (test_example_change_stream, db);
test_sample_causal_consistency (client);
test_sample_aggregation (db);
Expand Down