CDRIVER-3316 retryable reads test follow-up

Author: kevinAlbs

src/libmongoc/doc/distinct-mapreduce.rst

@@ -34,7 +34,10 @@ First define the ``map`` and ``reduce`` functions:
    :language: c
    :caption: constants.c
 
-Run the ``mapReduce`` command:
+Run the ``mapReduce`` command. Use the generic command helpers (e.g. :symbol:`mongoc_database_command_simple()`).
+Do not the read command helpers (e.g. :symbol:`mongoc_database_read_command_with_opts()`) because they are considered
+retryable read operations. If retryable reads are enabled, those operations will retry once on a retryable error,
+giving undesirable behavior for ``mapReduce``.
 
 .. literalinclude:: ../examples/basic_aggregation/map-reduce-basic.c
    :language: c

src/libmongoc/tests/json-test-operations.c

@@ -1326,7 +1326,7 @@ find (mongoc_collection_t *collection,
    return true;
 }
 
-/* TODO: merge into single_write */
+
 static bool
 find_one (mongoc_collection_t *collection,
           const bson_t *test,
@@ -1607,23 +1607,17 @@ list_databases (mongoc_client_t *client,
                 const bson_t *test,
                 const bson_t *operation,
                 mongoc_client_session_t *session,
-                const mongoc_read_prefs_t *read_prefs,
-                bson_t *reply,
-                bool name_only)
+                bson_t *reply)
 {
    mongoc_cursor_t *cursor;
-   bson_t cmd = BSON_INITIALIZER;
+   bson_t opts;
 
    BSON_ASSERT (client);
-   BSON_APPEND_INT32 (&cmd, "listDatabases", 1);
-
-   if (name_only) {
-      BSON_APPEND_BOOL (&cmd, "nameOnly", true);
-   }
+   bson_init (&opts);
+   append_session (session, &opts);
 
-   /* ignore client read prefs */
-   cursor = _mongoc_cursor_array_new (client, "admin", &cmd, NULL, "databases");
-   bson_destroy (&cmd);
+   cursor = mongoc_client_find_databases_with_opts (client, &opts);
+   bson_destroy (&opts);
 
    check_cursor (cursor, test, operation);
    mongoc_cursor_destroy (cursor);
@@ -1632,80 +1626,73 @@ list_databases (mongoc_client_t *client,
 }
 
 
-/* TODO: use find_indexes/collection/databases helpers */
 static bool
-list_indexes (mongoc_collection_t *collection,
-              const bson_t *test,
-              const bson_t *operation,
-              mongoc_client_session_t *session,
-              const mongoc_read_prefs_t *read_prefs,
-              bson_t *reply,
-              bool name_only)
+list_database_names (mongoc_client_t *client,
+                     const bson_t *test,
+                     const bson_t *operation,
+                     mongoc_client_session_t *session,
+                     bson_t *reply)
 {
-   mongoc_cursor_t *cursor;
-   bson_t cmd = BSON_INITIALIZER;
-   bson_t child;
+   char **database_names;
+   bson_t opts;
    bson_error_t error;
 
-   BSON_ASSERT (collection);
+   BSON_ASSERT (client);
+   bson_init (&opts);
+   append_session (session, &opts);
 
-   bson_append_utf8 (&cmd,
-                     "listIndexes",
-                     -1,
-                     collection->collection,
-                     collection->collectionlen);
+   database_names =
+      mongoc_client_get_database_names_with_opts (client, &opts, &error);
+   bson_destroy (&opts);
 
-   if (name_only) {
-      BSON_APPEND_BOOL (&cmd, "nameOnly", true);
-   }
+   check_result (
+      test, operation, database_names != NULL, NULL /* result */, &error);
+   bson_init (reply);
+   bson_strfreev (database_names);
+   return true;
+}
 
-   BSON_APPEND_DOCUMENT_BEGIN (&cmd, "cursor", &child);
-   bson_append_document_end (&cmd, &child);
 
-   /* No read preference. Index Enumeration Spec: "run listIndexes on the
-    * primary node in replicaSet mode". */
-   cursor = _mongoc_cursor_cmd_new (
-      collection->client, collection->ns, &cmd, NULL, NULL, NULL, NULL);
+static bool
+list_indexes (mongoc_collection_t *collection,
+              const bson_t *test,
+              const bson_t *operation,
+              mongoc_client_session_t *session,
+              bson_t *reply)
+{
+   bson_t opts;
+   mongoc_cursor_t *cursor;
 
-   if (!mongoc_cursor_error (cursor, &error)) {
-      _mongoc_cursor_prime (cursor);
-   }
+   BSON_ASSERT (collection);
 
-   bson_destroy (&cmd);
+   bson_init (&opts);
+   append_session (session, &opts);
 
+   cursor = mongoc_collection_find_indexes_with_opts (collection, &opts);
    check_cursor (cursor, test, operation);
    mongoc_cursor_destroy (cursor);
    bson_init (reply);
+   bson_destroy (&opts);
    return true;
 }
 
 
 static bool
-list_collections (mongoc_client_t *client,
+list_collections (mongoc_database_t *db,
                   const bson_t *test,
                   const bson_t *operation,
                   mongoc_client_session_t *session,
-                  const mongoc_read_prefs_t *read_prefs,
-                  bson_t *reply,
-                  bool name_only)
+                  bson_t *reply)
 {
    mongoc_cursor_t *cursor;
-   bson_t cmd = BSON_INITIALIZER;
+   bson_t opts;
 
-   BSON_APPEND_INT32 (&cmd, "listCollections", 1);
+   bson_init (&opts);
+   append_session (session, &opts);
 
-   if (name_only) {
-      BSON_APPEND_BOOL (&cmd, "nameOnly", true);
-   }
+   cursor = mongoc_database_find_collections_with_opts (db, &opts);
 
-   /* Enumerate Collections Spec: "run listCollections on the primary node in
-    * replicaset mode" */
-   cursor =
-      _mongoc_cursor_cmd_new (client, "admin", &cmd, NULL, NULL, NULL, NULL);
-   if (cursor->error.domain == 0) {
-      _mongoc_cursor_prime (cursor);
-   }
-   bson_destroy (&cmd);
+   bson_destroy (&opts);
 
    check_cursor (cursor, test, operation);
    mongoc_cursor_destroy (cursor);
@@ -1714,6 +1701,33 @@ list_collections (mongoc_client_t *client,
    return true;
 }
 
+static bool
+list_collection_names (mongoc_database_t *db,
+                       const bson_t *test,
+                       const bson_t *operation,
+                       mongoc_client_session_t *session,
+                       bson_t *reply)
+{
+   char **collection_names;
+   bson_t opts;
+   bson_error_t error;
+
+   bson_init (&opts);
+   append_session (session, &opts);
+
+   collection_names =
+      mongoc_database_get_collection_names_with_opts (db, &opts, &error);
+
+   bson_destroy (&opts);
+
+   check_result (
+      test, operation, collection_names != NULL, NULL /* result */, &error);
+   bson_init (reply);
+   bson_strfreev (collection_names);
+
+   return true;
+}
+
 
 static bool
 gridfs_download (mongoc_database_t *db,
@@ -1745,19 +1759,6 @@ gridfs_download (mongoc_database_t *db,
    return true;
 }
 
-/* The download_by_name functionality is part of the Advanced API for GridFS
- * and the C Driver hasn't implemented the Advanced API yet. This is a
- * placeholder to be used when the download_by_name is implemented. */
-static bool
-gridfs_download_by_name ()
-{
-   test_error ("The download_by_name functionality is part of the Advanced API "
-               "for GridFS and the C Driver hasn't implemented the Advanced "
-               "API yet.");
-   return false;
-}
-
-
 bool
 json_test_operation (json_test_ctx_t *ctx,
                      const bson_t *test,
@@ -1837,18 +1838,17 @@ json_test_operation (json_test_ctx_t *ctx,
          res = find_one (c, test, operation, session, read_prefs, reply);
       } else if (!strcmp (op_name, "aggregate")) {
          res = aggregate (c, test, operation, session, read_prefs, reply);
-      } else if (!strcmp (op_name, "listIndexNames")) {
-         res =
-            list_indexes (c, test, operation, session, read_prefs, reply, true);
       } else if (!strcmp (op_name, "listIndexes")) {
-         res = list_indexes (
-            c, test, operation, session, read_prefs, reply, false);
+         res = list_indexes (c, test, operation, session, reply);
       } else if (!strcmp (op_name, "watch")) {
          bson_t pipeline = BSON_INITIALIZER;
          mongoc_change_stream_destroy (ctx->change_stream);
          ctx->change_stream = mongoc_collection_watch (c, &pipeline, NULL);
          bson_init (reply);
          bson_destroy (&pipeline);
+      } else if (!strcmp (op_name, "mapReduce") ||
+                 !strcmp (op_name, "listIndexNames")) {
+         test_error ("operation not implemented in libmongoc");
       } else {
          test_error ("unrecognized collection operation name %s", op_name);
       }
@@ -1857,19 +1857,18 @@ json_test_operation (json_test_ctx_t *ctx,
          res = db_aggregate (db, test, operation, session, read_prefs, reply);
       } else if (!strcmp (op_name, "runCommand")) {
          res = command (db, test, operation, session, read_prefs, reply);
-      } else if (!strcmp (op_name, "listCollections") ||
-                 !strcmp (op_name, "listCollectionObjects")) {
-         res = list_collections (
-            c->client, test, operation, session, read_prefs, reply, false);
+      } else if (!strcmp (op_name, "listCollections")) {
+         res = list_collections (db, test, operation, session, reply);
       } else if (!strcmp (op_name, "listCollectionNames")) {
-         res = list_collections (
-            c->client, test, operation, session, read_prefs, reply, true);
+         res = list_collection_names (db, test, operation, session, reply);
       } else if (!strcmp (op_name, "watch")) {
          bson_t pipeline = BSON_INITIALIZER;
          mongoc_change_stream_destroy (ctx->change_stream);
          ctx->change_stream = mongoc_database_watch (db, &pipeline, NULL);
          bson_init (reply);
          bson_destroy (&pipeline);
+      } else if (!strcmp (op_name, "listCollectionObjects")) {
+         test_error ("listCollectionObjects is not implemented in libmongoc");
       } else {
          test_error ("unrecognized database operation name %s", op_name);
       }
@@ -1905,19 +1904,18 @@ json_test_operation (json_test_ctx_t *ctx,
          test_error ("unrecognized session operation name %s", op_name);
       }
    } else if (!strcmp (obj_name, "client")) {
-      if (!strcmp (op_name, "listDatabases") ||
-          !strcmp (op_name, "listDatabaseObjects")) {
-         res = list_databases (
-            c->client, test, operation, session, read_prefs, reply, false);
+      if (!strcmp (op_name, "listDatabases")) {
+         res = list_databases (c->client, test, operation, session, reply);
       } else if (!strcmp (op_name, "listDatabaseNames")) {
-         res = list_databases (
-            c->client, test, operation, session, read_prefs, reply, true);
+         res = list_database_names (c->client, test, operation, session, reply);
       } else if (!strcmp (op_name, "watch")) {
          bson_t pipeline = BSON_INITIALIZER;
          mongoc_change_stream_destroy (ctx->change_stream);
          ctx->change_stream = mongoc_client_watch (c->client, &pipeline, NULL);
          bson_init (reply);
          bson_destroy (&pipeline);
+      } else if (!strcmp (op_name, "listDatabaseObjects")) {
+         test_error ("listDatabaseObjects is not implemented in libmongoc");
       } else {
          test_error ("unrecognized client operation name %s", op_name);
       }
@@ -1926,8 +1924,8 @@ json_test_operation (json_test_ctx_t *ctx,
          res =
             gridfs_download (db, test, operation, session, read_prefs, reply);
       } else if (!strcmp (op_name, "download_by_name")) {
-         res = gridfs_download_by_name ();
-         bson_init (reply);
+         test_error ("download_by_name is part of the optional advanced API "
+                     "and not implemented in libmongoc");
       } else {
          test_error ("unrecognized gridfs operation name %s", op_name);
       }

src/libmongoc/tests/json-test.c

@@ -1225,7 +1225,6 @@ _should_skip_due_to_server_39704 (const bson_t *test)
       }
    }
 
-
    return false;
 }
 
@@ -1391,6 +1390,61 @@ json_test_config_cleanup (json_test_config_t *config)
 }
 
 
+/* Tests on unsupported operations are automatically skipped with a message
+ * indicating why. */
+static bson_t *
+_skip_if_unsupported (const char *test_name, bson_t *original)
+{
+   int i;
+   bool skip = false;
+   const char *unsupported_tests[] = {
+      "/retryable_reads/gridfs-downloadByName",
+      "/retryable_reads/gridfs-downloadByName-serverErrors",
+      "/retryable_reads/listCollectionObjects",
+      "/retryable_reads/listCollectionObjects-serverErrors",
+      "/retryable_reads/listDatabaseObjects",
+      "/retryable_reads/listDatabaseObjects-serverErrors",
+      "/retryable_reads/listIndexNames",
+      "/retryable_reads/listIndexNames-serverErrors",
+      "/retryable_reads/mapReduce"};
+
+   for (i = 0; i < sizeof (unsupported_tests) / sizeof (unsupported_tests[0]);
+        i++) {
+      if (0 == strcmp (test_name, unsupported_tests[i])) {
+         skip = true;
+         break;
+      }
+   }
+
+   if (skip) {
+      /* Modify the test file to give all entries in "tests" a skipReason */
+      bson_t *modified = bson_new ();
+      bson_t modified_tests;
+      bson_iter_t iter;
+
+      bson_copy_to_excluding_noinit (original, modified, "tests", NULL);
+      BSON_APPEND_ARRAY_BEGIN (modified, "tests", &modified_tests);
+      BSON_ASSERT (bson_iter_init_find (&iter, original, "tests"));
+      for (bson_iter_recurse (&iter, &iter); bson_iter_next (&iter);) {
+         bson_t original_test;
+         bson_t modified_test;
+
+         bson_iter_bson (&iter, &original_test);
+         BSON_APPEND_DOCUMENT_BEGIN (
+            &modified_tests, bson_iter_key (&iter), &modified_test);
+         bson_concat (&modified_test, &original_test);
+         BSON_APPEND_UTF8 (&modified_test,
+                           "skipReason",
+                           "libmongoc does not support required operation.");
+         bson_append_document_end (&modified_tests, &modified_test);
+      }
+      bson_append_array_end (modified, &modified_tests);
+      bson_destroy (original);
+      return modified;
+   }
+   return original;
+}
+
 /*
  *-----------------------------------------------------------------------
  *
@@ -1432,6 +1486,7 @@ _install_json_test_suite_with_check (TestSuite *suite,
       BSON_ASSERT (ext);
       ext[0] = '\0';
 
+      test = _skip_if_unsupported (skip_json, test);
       /* list of "check" functions that decide whether to skip the test */
       va_start (ap, callback);
       _V_TestSuite_AddFull (suite,