CDRIVER-655 return error on repeated calls to mongoc_bulk_operation_execute (#1937)

Author: kevinAlbs

NEWS

@@ -26,6 +26,7 @@ Unreleased (2.0.0)
 * `authMechanismProperties` is now validated and returns a client error for invalid or unsupported fields when the authentication mechanism is specified for:
   * GSSAPI: supported fields are SERVICE_NAME, CANONICALIZE_HOST_NAME, SERVICE_REALM, and SERVICE_HOST.
   * MONGODB-AWS: supported fields are AWS_SESSION_TOKEN.
+* Calling `mongoc_bulk_operation_execute` on the same `mongoc_bulk_operation_t` repeatedly is an error. Previously this was only discouraged in documentation.
 
 ## Removals
 

src/libmongoc/doc/mongoc_bulk_operation_execute.rst

@@ -15,7 +15,9 @@ Synopsis
 
 This function executes all operations queued into the bulk operation. Unless ``ordered: false`` was specified in the ``opts`` passed to :symbol:`mongoc_collection_create_bulk_operation_with_opts()`, then forward progress will be stopped upon the first error.
 
-It is only valid to call :symbol:`mongoc_bulk_operation_execute()` once. The ``mongoc_bulk_operation_t`` must be destroyed afterwards.
+It is only valid to call :symbol:`mongoc_bulk_operation_execute()` once on the same :symbol:`mongoc_bulk_operation_t`. Calling repeatedly results in error. Call :symbol:`mongoc_bulk_operation_destroy` to destroy ``bulk`` afterwards.
+
+.. versionchanged:: 2.0.0 Calling :symbol:`mongoc_bulk_operation_execute()` repeatedly results in an error.
 
 .. warning::
 

src/libmongoc/src/mongoc/mongoc-bulk-operation.c

@@ -740,8 +740,8 @@ mongoc_bulk_operation_execute (mongoc_bulk_operation_t *bulk, /* IN */
    cluster = &bulk->client->cluster;
 
    if (bulk->executed) {
-      _mongoc_write_result_destroy (&bulk->result);
-      _mongoc_write_result_init (&bulk->result);
+      _mongoc_set_error (error, MONGOC_ERROR_COMMAND, MONGOC_ERROR_COMMAND_INVALID_ARG, "bulk write already executed");
+      GOTO (err);
    }
 
    bulk->executed = true;

src/libmongoc/tests/test-mongoc-bulk.c

@@ -3753,51 +3753,76 @@ test_bulk_max_batch_size (void)
 static void
 test_bulk_new (void)
 {
-   mongoc_bulk_operation_t *bulk;
    mongoc_collection_t *collection;
    mongoc_client_t *client;
    bson_error_t error;
    bson_t empty = BSON_INITIALIZER;
    uint32_t r;
 
    client = test_framework_new_default_client ();
-   BSON_ASSERT (client);
+   ASSERT (client);
 
    collection = get_test_collection (client, "bulk_new");
-   BSON_ASSERT (collection);
+   ASSERT (collection);
 
-   bulk = mongoc_bulk_operation_new (true);
-   mongoc_bulk_operation_destroy (bulk);
-
-   bulk = mongoc_bulk_operation_new (true);
-
-   r = mongoc_bulk_operation_execute (bulk, NULL, &error);
-   BSON_ASSERT (!r);
-   ASSERT_CMPINT (error.domain, ==, MONGOC_ERROR_COMMAND);
-   ASSERT_CMPINT (error.code, ==, MONGOC_ERROR_COMMAND_INVALID_ARG);
+   // Can create and destroy:
+   {
+      mongoc_bulk_operation_t *bulk = mongoc_bulk_operation_new (true);
+      mongoc_bulk_operation_destroy (bulk);
+   }
 
-   mongoc_bulk_operation_set_database (bulk, "test");
-   r = mongoc_bulk_operation_execute (bulk, NULL, &error);
-   BSON_ASSERT (!r);
-   ASSERT_CMPINT (error.domain, ==, MONGOC_ERROR_COMMAND);
-   ASSERT_CMPINT (error.code, ==, MONGOC_ERROR_COMMAND_INVALID_ARG);
+   // Execute without a client is an error:
+   {
+      mongoc_bulk_operation_t *bulk = mongoc_bulk_operation_new (true);
+      r = mongoc_bulk_operation_execute (bulk, NULL, &error);
+      ASSERT (!r);
+      ASSERT_ERROR_CONTAINS (error, MONGOC_ERROR_COMMAND, MONGOC_ERROR_COMMAND_INVALID_ARG, "requires a client");
+      mongoc_bulk_operation_destroy (bulk);
+   }
 
-   mongoc_bulk_operation_set_collection (bulk, "test");
-   r = mongoc_bulk_operation_execute (bulk, NULL, &error);
-   BSON_ASSERT (!r);
-   ASSERT_CMPINT (error.domain, ==, MONGOC_ERROR_COMMAND);
-   ASSERT_CMPINT (error.code, ==, MONGOC_ERROR_COMMAND_INVALID_ARG);
+   // Execute with a database and no client is an error:
+   {
+      mongoc_bulk_operation_t *bulk = mongoc_bulk_operation_new (true);
+      mongoc_bulk_operation_set_database (bulk, "test");
+      r = mongoc_bulk_operation_execute (bulk, NULL, &error);
+      ASSERT (!r);
+      ASSERT_ERROR_CONTAINS (error, MONGOC_ERROR_COMMAND, MONGOC_ERROR_COMMAND_INVALID_ARG, "requires a client");
+      mongoc_bulk_operation_destroy (bulk);
+   }
 
-   mongoc_bulk_operation_set_client (bulk, client);
-   r = mongoc_bulk_operation_execute (bulk, NULL, &error);
-   BSON_ASSERT (!r);
-   ASSERT_CMPINT (error.domain, ==, MONGOC_ERROR_COMMAND);
-   ASSERT_CMPINT (error.code, ==, MONGOC_ERROR_COMMAND_INVALID_ARG);
+   // Execute with a database and collection and no client is an error:
+   {
+      mongoc_bulk_operation_t *bulk = mongoc_bulk_operation_new (true);
+      mongoc_bulk_operation_set_database (bulk, "test");
+      mongoc_bulk_operation_set_collection (bulk, "test");
+      r = mongoc_bulk_operation_execute (bulk, NULL, &error);
+      ASSERT (!r);
+      ASSERT_ERROR_CONTAINS (error, MONGOC_ERROR_COMMAND, MONGOC_ERROR_COMMAND_INVALID_ARG, "requires a client");
+      mongoc_bulk_operation_destroy (bulk);
+   }
 
-   mongoc_bulk_operation_insert (bulk, &empty);
-   ASSERT_OR_PRINT (mongoc_bulk_operation_execute (bulk, NULL, &error), error);
+   // Execute with no operations is an error:
+   {
+      mongoc_bulk_operation_t *bulk = mongoc_bulk_operation_new (true);
+      mongoc_bulk_operation_set_database (bulk, "test");
+      mongoc_bulk_operation_set_collection (bulk, "test");
+      mongoc_bulk_operation_set_client (bulk, client);
+      r = mongoc_bulk_operation_execute (bulk, NULL, &error);
+      ASSERT (!r);
+      ASSERT_ERROR_CONTAINS (error, MONGOC_ERROR_COMMAND, MONGOC_ERROR_COMMAND_INVALID_ARG, "empty bulk write");
+      mongoc_bulk_operation_destroy (bulk);
+   }
 
-   mongoc_bulk_operation_destroy (bulk);
+   // Execute with operations is OK:
+   {
+      mongoc_bulk_operation_t *bulk = mongoc_bulk_operation_new (true);
+      mongoc_bulk_operation_set_database (bulk, "test");
+      mongoc_bulk_operation_set_collection (bulk, "test");
+      mongoc_bulk_operation_set_client (bulk, client);
+      mongoc_bulk_operation_insert (bulk, &empty);
+      ASSERT_OR_PRINT (mongoc_bulk_operation_execute (bulk, NULL, &error), error);
+      mongoc_bulk_operation_destroy (bulk);
+   }
 
    mongoc_collection_drop (collection, NULL);
 
@@ -4766,6 +4791,23 @@ test_bulk_write_set_client_updates_operation_id_when_client_changes (void)
    mock_server_destroy (mock_server);
 }
 
+static void
+test_multiple_execution (void)
+{
+   mongoc_client_t *client = test_framework_new_default_client ();
+   mongoc_collection_t *coll = get_test_collection (client, "test_multiple_execution");
+   bson_error_t error;
+   mongoc_bulk_operation_t *bulk = mongoc_collection_create_bulk_operation (coll, true, NULL);
+   mongoc_bulk_operation_insert (bulk, tmp_bson ("{}"));
+   ASSERT_OR_PRINT (mongoc_bulk_operation_execute (bulk, NULL, &error), error);
+   ASSERT (!mongoc_bulk_operation_execute (bulk, NULL, &error));
+   ASSERT_ERROR_CONTAINS (error, MONGOC_ERROR_COMMAND, MONGOC_ERROR_COMMAND_INVALID_ARG, "bulk write already executed");
+   mongoc_bulk_operation_destroy (bulk);
+   mongoc_collection_destroy (coll);
+   mongoc_client_destroy (client);
+}
+
+
 void
 test_bulk_install (TestSuite *suite)
 {
@@ -4944,4 +4986,5 @@ test_bulk_install (TestSuite *suite)
    TestSuite_AddMockServerTest (suite,
                                 "/BulkOperation/set_client_updates_operation_id_when_client_changes",
                                 test_bulk_write_set_client_updates_operation_id_when_client_changes);
+   TestSuite_AddLive (suite, "/BulkOperation/multiple_execution", test_multiple_execution);
 }