CDRIVER-3850 allow URI to accept timeoutMS

samantharitter · samantharitter · commit 6bacf922669a · 2021-03-29T10:41:20.000-04:00
diff --git a/src/libmongoc/doc/mongoc_uri_t.rst b/src/libmongoc/doc/mongoc_uri_t.rst
@@ -97,7 +97,8 @@ MONGOC_URI_APPNAME                         appname                           Emp
 MONGOC_URI_TLS                             tls                               Empty (not set, same as false)    {true|false}, indicating if TLS must be used. (See also :symbol:`mongoc_client_set_ssl_opts` and :symbol:`mongoc_client_pool_set_ssl_opts`.)
 MONGOC_URI_COMPRESSORS                     compressors                       Empty (no compressors)            Comma separated list of compressors, if any, to use to compress the wire protocol messages. Snappy, zlib, and zstd are optional build time dependencies, and enable the "snappy", "zlib", and "zstd" values respectively.
 MONGOC_URI_CONNECTTIMEOUTMS                connecttimeoutms                  10,000 ms (10 seconds)            This setting applies to new server connections. It is also used as the socket timeout for server discovery and monitoring operations.
-MONGOC_URI_SOCKETTIMEOUTMS                 sockettimeoutms                   300,000 ms (5 minutes)            The time in milliseconds to attempt to send or receive on a socket before the attempt times out.
+MONGOC_URI_SOCKETTIMEOUTMS                 sockettimeoutms                   300,000 ms (5 minutes)            Deprecated in favor of MONGOC_URI_TIMEOUTMS. The time in milliseconds to attempt to send or receive on a socket before the attempt times out.
+MONGOC_URI_TIMEOUTMS                       timeoutms                         Empty (no timeout)                The time limit for the full execution of an operation.
 MONGOC_URI_REPLICASET                      replicaset                        Empty (no replicaset)             The name of the Replica Set that the driver should connect to.
 MONGOC_URI_ZLIBCOMPRESSIONLEVEL            zlibcompressionlevel              -1                                When the MONGOC_URI_COMPRESSORS includes "zlib" this options configures the zlib compression level, when the zlib compressor is used to compress client data.
 ========================================== ================================= ================================= ============================================================================================================================================================================================================================================
@@ -193,7 +194,7 @@ MONGOC_URI_MAXPOOLSIZE                     maxpoolsize                       The
 MONGOC_URI_MINPOOLSIZE                     minpoolsize                       Deprecated. This option's behavior does not match its name, and its actual behavior will likely hurt performance.
 MONGOC_URI_MAXIDLETIMEMS                   maxidletimems                     Not implemented.
 MONGOC_URI_WAITQUEUEMULTIPLE               waitqueuemultiple                 Not implemented.
-MONGOC_URI_WAITQUEUETIMEOUTMS              waitqueuetimeoutms                The maximum time to wait for a client to become available from the pool.
+MONGOC_URI_WAITQUEUETIMEOUTMS              waitqueuetimeoutms                Deprecated in favor of MONGOC_URI_TIMEOUTMS. The maximum time to wait for a client to become available from the pool.
 ========================================== ================================= =========================================================================================================================================================================================================================
 
 .. _mongoc_uri_t_write_concern_options:
@@ -211,7 +212,7 @@ MONGOC_URI_W                               w                                 Det
                                                                              * majority = For replica sets, if you specify the special majority value to w option, write operations will only return successfully after a majority of the configured replica set members have acknowledged the write operation.
                                                                              * n = For replica sets, if you specify a number n greater than 1, operations with this write concern return only after n members of the set have acknowledged the write. If you set n to a number that is greater than the number of available set members or members that hold data, MongoDB will wait, potentially indefinitely, for these members to become available.
                                                                              * tags = For replica sets, you can specify a tag set to require that all members of the set that have these tags configured return confirmation of the write operation.
-MONGOC_URI_WTIMEOUTMS                      wtimeoutms                        The time in milliseconds to wait for replication to succeed, as specified in the w option, before timing out. When wtimeoutMS is 0, write operations will never time out.
+MONGOC_URI_WTIMEOUTMS                      wtimeoutms                        Deprecated in favor of MONGOC_URI_TIMEOUTMS. The time in milliseconds to wait for replication to succeed, as specified in the w option, before timing out. When wtimeoutMS is 0, write operations will never time out.
 MONGOC_URI_JOURNAL                         journal                           Controls whether write operations will wait until the mongod acknowledges the write operations and commits the data to the on disk journal.
 
                                                                              * true  = Enables journal commit acknowledgement write concern. Equivalent to specifying the getLastError command with the j option enabled.
diff --git a/src/libmongoc/src/mongoc/mongoc-client.c b/src/libmongoc/src/mongoc/mongoc-client.c
@@ -1156,8 +1156,7 @@ _mongoc_client_new_from_uri (mongoc_topology_t *topology)
       BSON_ASSERT (mongoc_client_set_appname (client, appname));
    }
 
-   /* TODO CDRIVER-3850 pull initial timeoutMS value from the URI */
-   client->timeout_ms = MONGOC_TIMEOUTMS_UNSET;
+   client->timeout_ms = mongoc_uri_get_option_as_int64 (client->uri, MONGOC_URI_TIMEOUTMS, MONGOC_TIMEOUTMS_UNSET);
 
    mongoc_cluster_init (&client->cluster, client->uri, client);
 
diff --git a/src/libmongoc/src/mongoc/mongoc-uri.c b/src/libmongoc/src/mongoc/mongoc-uri.c
@@ -727,21 +727,23 @@ mongoc_uri_option_is_int32 (const char *key)
           !strcasecmp (key, MONGOC_URI_HEARTBEATFREQUENCYMS) ||
           !strcasecmp (key, MONGOC_URI_SERVERSELECTIONTIMEOUTMS) ||
           !strcasecmp (key, MONGOC_URI_SOCKETCHECKINTERVALMS) ||
-          !strcasecmp (key, MONGOC_URI_SOCKETTIMEOUTMS) ||
           !strcasecmp (key, MONGOC_URI_LOCALTHRESHOLDMS) ||
           !strcasecmp (key, MONGOC_URI_MAXPOOLSIZE) ||
           !strcasecmp (key, MONGOC_URI_MAXSTALENESSSECONDS) ||
           !strcasecmp (key, MONGOC_URI_MINPOOLSIZE) ||
           !strcasecmp (key, MONGOC_URI_MAXIDLETIMEMS) ||
           !strcasecmp (key, MONGOC_URI_WAITQUEUEMULTIPLE) ||
+          !strcasecmp (key, MONGOC_URI_ZLIBCOMPRESSIONLEVEL) ||
+          /* deprecated options */
           !strcasecmp (key, MONGOC_URI_WAITQUEUETIMEOUTMS) ||
-          !strcasecmp (key, MONGOC_URI_ZLIBCOMPRESSIONLEVEL);
+          !strcasecmp (key, MONGOC_URI_SOCKETTIMEOUTMS);
 }
 
 bool
 mongoc_uri_option_is_int64 (const char *key)
 {
-   return !strcasecmp (key, MONGOC_URI_WTIMEOUTMS);
+   return !strcasecmp (key, MONGOC_URI_WTIMEOUTMS) ||
+          !strcasecmp (key, MONGOC_URI_TIMEOUTMS);
 }
 
 bool
@@ -1131,7 +1133,8 @@ mongoc_uri_apply_options (mongoc_uri_t *uri,
                                MONGOC_ERROR_COMMAND,
                                MONGOC_ERROR_COMMAND_INVALID_ARG,
                                "Failed to set %s to %d",
-                               canon, bval);
+                               canon,
+                               bval);
                return false;
             }
          } else {
@@ -2503,6 +2506,30 @@ mongoc_uri_get_option_as_int32 (const mongoc_uri_t *uri,
    return (int32_t) retval;
 }
 
+
+static void
+_mongoc_uri_warn_for_bad_int_option_combos (const mongoc_uri_t *uri,
+					    const char *option)
+{
+   /* Warn for deprecated option combinations */
+   if (!strcasecmp (option, MONGOC_URI_TIMEOUTMS)) {
+      if (mongoc_uri_get_option_as_int64 (uri, MONGOC_URI_WAITQUEUETIMEOUTMS, -1) > 0 ||
+	  mongoc_uri_get_option_as_int64 (uri, MONGOC_URI_SOCKETTIMEOUTMS, -1) > 0 ||
+	  mongoc_uri_get_option_as_int64 (uri, MONGOC_URI_WTIMEOUTMS, -1) > 0) {
+	 MONGOC_WARNING ("Setting a deprecated timeout option %s in combination with timeoutMS", option);
+      }
+   }
+
+   if (!strcasecmp (option, MONGOC_URI_WAITQUEUETIMEOUTMS) ||
+       !strcasecmp (option, MONGOC_URI_SOCKETTIMEOUTMS) ||
+       !strcasecmp (option, MONGOC_URI_WTIMEOUTMS)) {
+      if (mongoc_uri_get_option_as_int64 (uri, MONGOC_URI_TIMEOUTMS, -1) > 0) {
+	 MONGOC_WARNING ("Setting a deprecated timeout option %s in combination with timeoutMS", option);
+      }
+   }
+}
+
+
 /*
  *--------------------------------------------------------------------------
  *
@@ -2624,6 +2651,9 @@ _mongoc_uri_set_option_as_int32_with_error (mongoc_uri_t *uri,
          return false;
       }
    }
+
+   _mongoc_uri_warn_for_bad_int_option_combos (uri, option);
+   
    option_lowercase = lowercase_str_new (option);
    if (!bson_append_int32 (&uri->options, option_lowercase, -1, value)) {
       bson_free (option_lowercase);
@@ -2817,6 +2847,16 @@ _mongoc_uri_set_option_as_int64_with_error (mongoc_uri_t *uri,
 
    option = mongoc_uri_canonicalize_option (option_orig);
 
+   /* timeoutMS may not be a negative number. */
+   if (!bson_strcasecmp (option, MONGOC_URI_TIMEOUTMS) && value < 0) {
+      MONGOC_URI_ERROR (
+         error,
+         "Invalid \"%s\" of %lld: must be a non-negative integer",
+         option_orig,
+         value);
+      return false;
+   }
+
    if ((options = mongoc_uri_get_options (uri)) &&
        bson_iter_init_find_case (&iter, options, option)) {
       if (BSON_ITER_HOLDS_INT64 (&iter)) {
@@ -2833,6 +2873,8 @@ _mongoc_uri_set_option_as_int64_with_error (mongoc_uri_t *uri,
       }
    }
 
+   _mongoc_uri_warn_for_bad_int_option_combos (uri, option);
+
    option_lowercase = lowercase_str_new (option);
    if (!bson_append_int64 (&uri->options, option_lowercase, -1, value)) {
       bson_free (option_lowercase);
diff --git a/src/libmongoc/src/mongoc/mongoc-uri.h b/src/libmongoc/src/mongoc/mongoc-uri.h
@@ -63,6 +63,7 @@
 #define MONGOC_URI_SLAVEOK "slaveok"
 #define MONGOC_URI_SOCKETCHECKINTERVALMS "socketcheckintervalms"
 #define MONGOC_URI_SOCKETTIMEOUTMS "sockettimeoutms"
+#define MONGOC_URI_TIMEOUTMS "timeoutms"
 #define MONGOC_URI_TLS "tls"
 #define MONGOC_URI_TLSCERTIFICATEKEYFILE "tlscertificatekeyfile"
 #define MONGOC_URI_TLSCERTIFICATEKEYFILEPASSWORD "tlscertificatekeyfilepassword"
diff --git a/src/libmongoc/tests/test-mongoc-client.c b/src/libmongoc/tests/test-mongoc-client.c
@@ -3860,12 +3860,14 @@ test_mongoc_client_timeout_ms (void)
 
    client = mongoc_client_new ("mongodb://localhost");
 
-   /* no timeout returns -1 */
+   /* No timeout returns -1 */
    BSON_ASSERT (mongoc_client_get_timeout_ms (client) ==
                 MONGOC_TIMEOUTMS_UNSET);
+   mongoc_client_destroy (client);
 
-   /* TODO CDRIVER-3850 implement additional tests once timeout can be set via
-    * URI */
+   /* Client gets timeout through URI */
+   client = mongoc_client_new ("mongodb://localhost/?timeoutms=100");
+   BSON_ASSERT (mongoc_client_get_timeout_ms (client) == 100);
 
    mongoc_client_destroy (client);
 }
diff --git a/src/libmongoc/tests/test-mongoc-collection.c b/src/libmongoc/tests/test-mongoc-collection.c
@@ -6407,21 +6407,17 @@ test_fam_no_error_on_retry (void *unused)
 static void
 test_timeout_ms (void)
 {
-   mongoc_client_t *client = mongoc_client_new ("mongodb://host/db_name");
+   mongoc_client_t *client = mongoc_client_new ("mongodb://localhost/?timeoutms=100");
    mongoc_database_t *db = NULL;
    mongoc_collection_t *coll =
       mongoc_client_get_collection (client, "db", "test");
    mongoc_collection_t *coll2 = NULL;
    bool res;
    bson_error_t error;
 
-   /* TODO CDRIVER-3850 no timeout MS returns client's timeout */
-   /* BSON_ASSERT (mongoc_client_set_timeout_ms (client, 10, &error));
-      ASSERT_CMPINT (mongoc_collection_get_timeout_ms (coll), ==,
-      mongoc_client_get_timeout_ms (client)); */
-
-   BSON_ASSERT (mongoc_collection_get_timeout_ms (coll) ==
-                MONGOC_TIMEOUTMS_UNSET);
+   /* no timeoutMS returns client's timeoutMS */
+   ASSERT_CMPINT (mongoc_collection_get_timeout_ms (coll), ==,
+		  mongoc_client_get_timeout_ms (client));
 
    /* negative timeouts are invalid */
    res = mongoc_collection_set_timeout_ms (coll, -1, &error);
diff --git a/src/libmongoc/tests/test-mongoc-database.c b/src/libmongoc/tests/test-mongoc-database.c
@@ -1206,17 +1206,14 @@ test_get_default_database (void)
 static void
 test_timeout_ms (void)
 {
-   mongoc_client_t *client = mongoc_client_new ("mongodb://host/db_name");
-   mongoc_database_t *db = mongoc_client_get_default_database (client);
+   mongoc_client_t *client = mongoc_client_new ("mongodb://localhost/?timeoutms=100");
+   mongoc_database_t *db = mongoc_client_get_database (client, "test");
    bool res;
    bson_error_t error;
 
-   /* TODO CDRIVER-3850 no timeout MS returns client's timeout */
-   /* BSON_ASSERT (mongoc_client_set_timeout_ms (client, 10, &error));
-      ASSERT_CMPINT (mongoc_database_get_timeout_ms (db), ==,
-      mongoc_client_get_timeout_ms (client));*/
-
-   BSON_ASSERT (mongoc_database_get_timeout_ms (db) == MONGOC_TIMEOUTMS_UNSET);
+   /* no timeoutMS returns client's timeoutMS */
+   ASSERT_CMPINT (mongoc_database_get_timeout_ms (db), ==,
+		  mongoc_client_get_timeout_ms (client));
 
    /* negative timeouts are invalid */
    res = mongoc_database_set_timeout_ms (db, -1, &error);
diff --git a/src/libmongoc/tests/test-mongoc-uri.c b/src/libmongoc/tests/test-mongoc-uri.c

Original file line number	Diff line number	Diff line change
`@@ -1156,8 +1156,7 @@ _mongoc_client_new_from_uri (mongoc_topology_t *topology)`
`1156`	`1156`	`BSON_ASSERT (mongoc_client_set_appname (client, appname));`
`1157`	`1157`	`}`
`1158`	`1158`
`1159`		`- /* TODO CDRIVER-3850 pull initial timeoutMS value from the URI */`
`1160`		`- client->timeout_ms = MONGOC_TIMEOUTMS_UNSET;`
	`1159`	`+ client->timeout_ms = mongoc_uri_get_option_as_int64 (client->uri, MONGOC_URI_TIMEOUTMS, MONGOC_TIMEOUTMS_UNSET);`
`1161`	`1160`
`1162`	`1161`	`mongoc_cluster_init (&client->cluster, client->uri, client);`
`1163`	`1162`