CDRIVER-2213 findAndModify respects writeConcern opt

ajdavis · ajdavis · commit ab0606aadb61 · 2017-09-01T13:55:23.000-04:00
diff --git a/NEWS b/NEWS
@@ -1,6 +1,8 @@
 mongo-c-driver 1.8.0
 ====================
 
+  * mongoc_collection_find_and_modify_with_opts now respects a "writeConcern"
+    field in the "extra" BSON document in its mongoc_find_and_modify_opts_t.
   * The command functions mongoc_client_read_write_command_with_opts,
     mongoc_database_read_write_command_with_opts, and
     mongoc_collection_read_write_command_with_opts now ignore the "read_prefs"
diff --git a/examples/find_and_modify_with_opts/opts.c b/examples/find_and_modify_with_opts/opts.c
@@ -7,6 +7,7 @@ fam_opts (mongoc_collection_t *collection)
    bson_t *update;
    bson_error_t error;
    bson_t query = BSON_INITIALIZER;
+   mongoc_write_concern_t *wc;
    bson_t extra = BSON_INITIALIZER;
    bool success;
 
@@ -25,6 +26,11 @@ fam_opts (mongoc_collection_t *collection)
    /* Abort if the operation takes too long. */
    mongoc_find_and_modify_opts_set_max_time_ms (opts, 100);
 
+   /* Set write concern w: 2 */
+   wc = mongoc_write_concern_new ();
+   mongoc_write_concern_set_w (wc, 2);
+   mongoc_write_concern_append (wc, &extra);
+
    /* Some future findAndModify option the driver doesn't support conveniently
     */
    BSON_APPEND_INT32 (&extra, "futureOption", 42);
@@ -48,5 +54,6 @@ fam_opts (mongoc_collection_t *collection)
    bson_destroy (&extra);
    bson_destroy (update);
    bson_destroy (&query);
+   mongoc_write_concern_destroy (wc);
    mongoc_find_and_modify_opts_destroy (opts);
 }
diff --git a/src/mongoc/mongoc-cmd.c b/src/mongoc/mongoc-cmd.c
@@ -20,6 +20,8 @@
 #include "mongoc-trace-private.h"
 #include "mongoc-client-private.h"
 #include "mongoc-write-concern-private.h"
+/* For strcasecmp on Windows */
+#include "mongoc-util-private.h"
 
 
 void
@@ -67,11 +69,16 @@ mongoc_cmd_parts_append_opts (mongoc_cmd_parts_t *parts,
                               int max_wire_version,
                               bson_error_t *error)
 {
+   bool is_fam;
+
    ENTRY;
 
    /* not yet assembled */
    BSON_ASSERT (!parts->assembled.command);
 
+   is_fam =
+      !strcasecmp (_mongoc_get_command_name (parts->body), "findandmodify");
+
    while (bson_iter_next (iter)) {
       if (BSON_ITER_IS_KEY (iter, "collation")) {
          if (max_wire_version < WIRE_VERSION_COLLATION) {
@@ -91,7 +98,8 @@ mongoc_cmd_parts_append_opts (mongoc_cmd_parts_t *parts,
             RETURN (false);
          }
 
-         if (max_wire_version < WIRE_VERSION_CMD_WRITE_CONCERN) {
+         if ((is_fam && max_wire_version < WIRE_VERSION_FAM_WRITE_CONCERN) ||
+             (!is_fam && max_wire_version < WIRE_VERSION_CMD_WRITE_CONCERN)) {
             continue;
          }
 
diff --git a/src/mongoc/mongoc-collection.c b/src/mongoc/mongoc-collection.c
@@ -2526,19 +2526,22 @@ mongoc_collection_find_and_modify_with_opts (
       BSON_APPEND_INT32 (&command, "maxTimeMS", opts->max_time_ms);
    }
 
-   if (server_stream->sd->max_wire_version >= WIRE_VERSION_FAM_WRITE_CONCERN) {
-      if (!mongoc_write_concern_is_valid (collection->write_concern)) {
-         bson_set_error (error,
-                         MONGOC_ERROR_COMMAND,
-                         MONGOC_ERROR_COMMAND_INVALID_ARG,
-                         "The write concern is invalid.");
-         bson_destroy (&command);
-         mongoc_server_stream_cleanup (server_stream);
-         RETURN (false);
-      }
+   if (!bson_has_field (&opts->extra, "writeConcern")) {
+      if (server_stream->sd->max_wire_version >=
+          WIRE_VERSION_FAM_WRITE_CONCERN) {
+         if (!mongoc_write_concern_is_valid (collection->write_concern)) {
+            bson_set_error (error,
+                            MONGOC_ERROR_COMMAND,
+                            MONGOC_ERROR_COMMAND_INVALID_ARG,
+                            "The write concern is invalid.");
+            bson_destroy (&command);
+            mongoc_server_stream_cleanup (server_stream);
+            RETURN (false);
+         }
 
-      if (mongoc_write_concern_is_acknowledged (collection->write_concern)) {
-         _BSON_APPEND_WRITE_CONCERN (&command, collection->write_concern);
+         if (mongoc_write_concern_is_acknowledged (collection->write_concern)) {
+            _BSON_APPEND_WRITE_CONCERN (&command, collection->write_concern);
+         }
       }
    }
 
diff --git a/tests/test-mongoc-find-and-modify.c b/tests/test-mongoc-find-and-modify.c
@@ -206,7 +206,8 @@ test_find_and_modify_write_concern (int wire_version)
          "{ 'findAndModify' : 'test_find_and_modify', "
          "'query' : { 'superduper' : 77889 },"
          "'update' : { '$set' : { 'superduper' : 1234 } },"
-         "'new' : true }");
+         "'new' : true,"
+         "'writeConcern' : { '$exists': false } }");
    }
 
    mock_server_replies_simple (request, "{ 'value' : null, 'ok' : 1 }");
@@ -416,6 +417,71 @@ test_find_and_modify_opts (void)
 }
 
 
+static void
+test_find_and_modify_opts_write_concern (void)
+{
+   mongoc_write_concern_t *w2;
+   mongoc_write_concern_t *w3;
+   mock_server_t *server;
+   mongoc_client_t *client;
+   mongoc_collection_t *collection;
+   bson_error_t error;
+   mongoc_find_and_modify_opts_t *opts;
+   bson_t extra = BSON_INITIALIZER;
+   future_t *future;
+   request_t *request;
+
+   w2 = mongoc_write_concern_new ();
+   mongoc_write_concern_set_w (w2, 2);
+   w3 = mongoc_write_concern_new ();
+   mongoc_write_concern_set_w (w3, 3);
+
+   server = mock_server_with_autoismaster (WIRE_VERSION_FAM_WRITE_CONCERN);
+   mock_server_run (server);
+
+   client = mongoc_client_new_from_uri (mock_server_get_uri (server));
+   collection = mongoc_client_get_collection (client, "db", "collection");
+
+   opts = mongoc_find_and_modify_opts_new ();
+   mongoc_write_concern_append (w2, &extra);
+   BSON_ASSERT (mongoc_find_and_modify_opts_append (opts, &extra));
+   bson_destroy (&extra);
+
+   future = future_collection_find_and_modify_with_opts (
+      collection, tmp_bson ("{}"), opts, NULL, &error);
+   request = mock_server_receives_command (
+      server,
+      "db",
+      MONGOC_QUERY_NONE,
+      "{'findAndModify': 'collection', 'writeConcern': {'w': 2}}");
+   mock_server_replies_ok_and_destroys (request);
+   ASSERT_OR_PRINT (future_get_bool (future), error);
+   future_destroy (future);
+
+   /* opts overrides collection */
+   mongoc_collection_set_write_concern (collection, w3);
+   future = future_collection_find_and_modify_with_opts (
+      collection, tmp_bson ("{}"), opts, NULL, &error);
+
+   /* still w: 2 */
+   request = mock_server_receives_command (
+      server,
+      "db",
+      MONGOC_QUERY_NONE,
+      "{'findAndModify': 'collection', 'writeConcern': {'w': 2}}");
+   mock_server_replies_ok_and_destroys (request);
+   ASSERT_OR_PRINT (future_get_bool (future), error);
+   future_destroy (future);
+
+   mongoc_find_and_modify_opts_destroy (opts);
+   mongoc_collection_destroy (collection);
+   mongoc_client_destroy (client);
+   mock_server_destroy (server);
+   mongoc_write_concern_destroy (w2);
+   mongoc_write_concern_destroy (w3);
+}
+
+
 static void
 test_find_and_modify_collation (int wire)
 {
@@ -518,6 +584,9 @@ test_find_and_modify_install (TestSuite *suite)
                       should_run_fam_wc);
    TestSuite_AddMockServerTest (
       suite, "/find_and_modify/opts", test_find_and_modify_opts);
+   TestSuite_AddMockServerTest (suite,
+                                "/find_and_modify/opts/write_concern",
+                                test_find_and_modify_opts_write_concern);
    TestSuite_AddMockServerTest (suite,
                                 "/find_and_modify/collation/ok",
                                 test_find_and_modify_collation_ok);
