Merge pull request #270 from jmikola/cdriver-851

CDRIVER-851: Negative wtimeout values are invalid

Author: ajdavis

doc/mongoc_write_concern_set_wmajority.page

@@ -21,13 +21,14 @@ mongoc_write_concern_set_wmajority (mongoc_write_concern_t *write_concern,
     <title>Parameters</title>
     <table>
       <tr><td><p>write_concern</p></td><td><p>A <code xref="mongoc_write_concern_t">mongoc_write_concern_t</code>.</p></td></tr>
-      <tr><td><p>wtimeout_msec</p></td><td><p>A int32_t.</p></td></tr>
+      <tr><td><p>wtimeout_msec</p></td><td><p>A positive <code>int32_t</code> or zero.</p></td></tr>
     </table>
   </section>
 
   <section id="description">
     <title>Description</title>
     <p>Sets if the write must have been propagated to a majority of nodes before indicating write success.</p>
+    <p>The timeout specifies how long, in milliseconds, the server should wait before indicating that the write has failed. This is not the same as a socket timeout. A value of zero may be used to indicate no timeout.</p>
   </section>
 
 </page>

doc/mongoc_write_concern_set_wtimeout.page

@@ -22,13 +22,13 @@ mongoc_write_concern_set_wtimeout (mongoc_write_concern_t *write_concern,
     <title>Parameters</title>
     <table>
       <tr><td><p>write_concern</p></td><td><p>A <code xref="mongoc_write_concern_t">mongoc_write_concern_t</code>.</p></td></tr>
-      <tr><td><p>wtimeout_msec</p></td><td><p>A int32_t containing the timeout in milliseconds.</p></td></tr>
+      <tr><td><p>wtimeout_msec</p></td><td><p>A positive <code>int32_t</code> or zero.</p></td></tr>
     </table>
   </section>
 
   <section id="description">
     <title>Description</title>
-    <p>Set the timeout in milliseconds that the server should wait before idicating that the write has failed. This is not the same as a socket timeout.</p>
+    <p>Set the timeout in milliseconds that the server should wait before indicating that the write has failed. This is not the same as a socket timeout. A value of zero may be used to indicate no timeout.</p>
   </section>
 
 </page>

src/mongoc/mongoc-write-concern.c

@@ -198,14 +198,21 @@ mongoc_write_concern_get_wtimeout (const mongoc_write_concern_t *write_concern)
  * @wtimeout_msec: Number of milliseconds before timeout.
  *
  * Sets the number of milliseconds to wait before considering a write
- * request as failed.
+ * request as failed. A value of 0 indicates no write timeout.
+ *
+ * The @wtimeout_msec parameter must be positive or zero. Negative values will
+ * be ignored.
  */
 void
 mongoc_write_concern_set_wtimeout (mongoc_write_concern_t *write_concern,
-                                   int32_t            wtimeout_msec)
+                                   int32_t                 wtimeout_msec)
 {
    BSON_ASSERT (write_concern);
 
+   if (wtimeout_msec < 0) {
+      return;
+   }
+
    if (!_mongoc_write_concern_warn_frozen(write_concern)) {
       write_concern->wtimeout = wtimeout_msec;
    }
@@ -227,7 +234,10 @@ mongoc_write_concern_get_wmajority (const mongoc_write_concern_t *write_concern)
  *
  * Sets the "w" of a write concern to "majority". It is suggested that
  * you provide a reasonable @wtimeout_msec to wait before considering the
- * write request failed.
+ * write request failed. A @wtimeout_msec value of 0 indicates no write timeout.
+ *
+ * The @wtimeout_msec parameter must be positive or zero. Negative values will
+ * be ignored.
  */
 void
 mongoc_write_concern_set_wmajority (mongoc_write_concern_t *write_concern,
@@ -237,7 +247,10 @@ mongoc_write_concern_set_wmajority (mongoc_write_concern_t *write_concern,
 
    if (!_mongoc_write_concern_warn_frozen(write_concern)) {
       write_concern->w = MONGOC_WRITE_CONCERN_W_MAJORITY;
-      write_concern->wtimeout = wtimeout_msec;
+
+      if (wtimeout_msec >= 0) {
+         write_concern->wtimeout = wtimeout_msec;
+      }
    }
 }
 
@@ -415,5 +428,9 @@ _mongoc_write_concern_is_valid (const mongoc_write_concern_t *write_concern)
       return false;
    }
 
+   if (write_concern->wtimeout < 0) {
+      return false;
+   }
+
    return true;
 }

tests/test-mongoc-write-concern.c

@@ -207,6 +207,38 @@ test_write_concern_fsync_and_journal_gle_and_validity (void)
    mongoc_write_concern_destroy(write_concern);
 }
 
+static void
+test_write_concern_wtimeout_validity (void)
+{
+   mongoc_write_concern_t *write_concern = mongoc_write_concern_new();
+
+   /* Test defaults */
+   ASSERT(write_concern);
+   ASSERT(mongoc_write_concern_get_w(write_concern) == MONGOC_WRITE_CONCERN_W_DEFAULT);
+   ASSERT(mongoc_write_concern_get_wtimeout(write_concern) == 0);
+   ASSERT(!mongoc_write_concern_get_wmajority(write_concern));
+
+   /* mongoc_write_concern_set_wtimeout() ignores invalid wtimeout */
+   mongoc_write_concern_set_wtimeout(write_concern, -1);
+   ASSERT(mongoc_write_concern_get_w(write_concern) == MONGOC_WRITE_CONCERN_W_DEFAULT);
+   ASSERT(mongoc_write_concern_get_wtimeout(write_concern) == 0);
+   ASSERT(!mongoc_write_concern_get_wmajority(write_concern));
+   ASSERT(_mongoc_write_concern_is_valid(write_concern));
+
+   /* mongoc_write_concern_set_wmajority() ignores invalid wtimeout */
+   mongoc_write_concern_set_wmajority(write_concern, -1);
+   ASSERT(mongoc_write_concern_get_w(write_concern) == MONGOC_WRITE_CONCERN_W_MAJORITY);
+   ASSERT(mongoc_write_concern_get_wtimeout(write_concern) == 0);
+   ASSERT(mongoc_write_concern_get_wmajority(write_concern));
+   ASSERT(_mongoc_write_concern_is_valid(write_concern));
+
+   /* Manually assigning a negative wtimeout will make the write concern invalid */
+   write_concern->wtimeout = -1;
+   ASSERT(!_mongoc_write_concern_is_valid(write_concern));
+
+   mongoc_write_concern_destroy(write_concern);
+}
+
 
 void
 test_write_concern_install (TestSuite *suite)
@@ -215,4 +247,5 @@ test_write_concern_install (TestSuite *suite)
    TestSuite_Add (suite, "/WriteConcern/bson_omits_defaults", test_write_concern_bson_omits_defaults);
    TestSuite_Add (suite, "/WriteConcern/bson_includes_false_fsync_and_journal", test_write_concern_bson_includes_false_fsync_and_journal);
    TestSuite_Add (suite, "/WriteConcern/fsync_and_journal_gle_and_validity", test_write_concern_fsync_and_journal_gle_and_validity);
+   TestSuite_Add (suite, "/WriteConcern/wtimeout_validity", test_write_concern_wtimeout_validity);
 }