CDRIVER-3769 Deprecate unimplemented URI options (#1770)

* Removes maxidletimems and waitqueuemultiple from the schema of allowed int URI options
* Tests that maxidletimems and waitqueuemultiple warn as unsupported options
* Replaces conflicting spec tests with modified libmongoc-specific versions

Author: Micah Scott @ MongoDB

src/libmongoc/src/mongoc/mongoc-uri.c

@@ -720,9 +720,12 @@ mongoc_uri_option_is_int32 (const char *key)
           !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_WAITQUEUETIMEOUTMS) || !strcasecmp (key, MONGOC_URI_ZLIBCOMPRESSIONLEVEL) ||
           !strcasecmp (key, MONGOC_URI_SRVMAXHOSTS);
+   /* Not including deprecated unimplemented options:
+    * - MONGOC_URI_MAXIDLETIMEMS
+    * - MONGOC_URI_WAITQUEUEMULTIPLE
+    */
 }
 
 bool
@@ -742,7 +745,7 @@ mongoc_uri_option_is_bool (const char *key)
           !strcasecmp (key, MONGOC_URI_TLSALLOWINVALIDHOSTNAMES) ||
           !strcasecmp (key, MONGOC_URI_TLSDISABLECERTIFICATEREVOCATIONCHECK) ||
           !strcasecmp (key, MONGOC_URI_TLSDISABLEOCSPENDPOINTCHECK) || !strcasecmp (key, MONGOC_URI_LOADBALANCED) ||
-          /* deprecated options */
+          /* deprecated options with canonical equivalents */
           !strcasecmp (key, MONGOC_URI_SSL) || !strcasecmp (key, MONGOC_URI_SSLALLOWINVALIDCERTIFICATES) ||
           !strcasecmp (key, MONGOC_URI_SSLALLOWINVALIDHOSTNAMES);
 }
@@ -754,7 +757,7 @@ mongoc_uri_option_is_utf8 (const char *key)
           !strcasecmp (key, MONGOC_URI_READPREFERENCE) || !strcasecmp (key, MONGOC_URI_SERVERMONITORINGMODE) ||
           !strcasecmp (key, MONGOC_URI_SRVSERVICENAME) || !strcasecmp (key, MONGOC_URI_TLSCERTIFICATEKEYFILE) ||
           !strcasecmp (key, MONGOC_URI_TLSCERTIFICATEKEYFILEPASSWORD) || !strcasecmp (key, MONGOC_URI_TLSCAFILE) ||
-          /* deprecated options */
+          /* deprecated options with canonical equivalents */
           !strcasecmp (key, MONGOC_URI_SSLCLIENTCERTIFICATEKEYFILE) ||
           !strcasecmp (key, MONGOC_URI_SSLCLIENTCERTIFICATEKEYPASSWORD) ||
           !strcasecmp (key, MONGOC_URI_SSLCERTIFICATEAUTHORITYFILE);

src/libmongoc/tests/json-test.c

@@ -1716,21 +1716,8 @@ run_json_general_test (const json_test_config_t *config)
          continue;
       }
 
-      if (config->skips) {
-         test_skip_t *iter;
-         bool should_skip = false;
-
-         for (iter = config->skips; iter->description != NULL; iter++) {
-            if (0 == strcmp (description, iter->description)) {
-               should_skip = true;
-               break;
-            }
-         }
-
-         if (should_skip) {
-            fprintf (stderr, " - %s SKIPPED, due to reason: %s\n", description, iter->reason);
-            continue;
-         }
+      if (test_should_be_skipped (config->skips, description)) {
+         continue;
       }
 
       uri = (config->uri_str != NULL) ? mongoc_uri_new (config->uri_str) : test_framework_get_uri ();
@@ -1980,3 +1967,30 @@ install_json_test_suite (TestSuite *suite, const char *base, const char *subdir,
 {
    install_json_test_suite_with_check (suite, base, subdir, callback, TestSuite_CheckLive);
 }
+
+
+/*
+ *-----------------------------------------------------------------------
+ *
+ * test_should_be_skipped --
+ *
+ *      Check a test description string against a list of description strings for
+ *      tests that should be skipped. "skips" is an optional NULL terminated array.
+ *
+ *      If the test should be skipped, returns 'true' and logs a reason to stderr.
+ *
+ *-----------------------------------------------------------------------
+ */
+bool
+test_should_be_skipped (const test_skip_t *skips, const char *description)
+{
+   if (skips) {
+      for (const test_skip_t *iter = skips; iter->description != NULL; iter++) {
+         if (0 == strcmp (description, iter->description)) {
+            fprintf (stderr, "  - %s SKIPPED, due to reason: %s\n", description, iter->reason);
+            return true;
+         }
+      }
+   }
+   return false;
+}

src/libmongoc/tests/json-test.h

@@ -115,4 +115,7 @@ check_scenario_version (const bson_t *scenario);
 void
 check_outcome_collection (mongoc_collection_t *collection, bson_t *test);
 
+bool
+test_should_be_skipped (const test_skip_t *skips, const char *description);
+
 #endif

src/libmongoc/tests/json/connection_uri/additional-nonspec-tests.json

@@ -49,7 +49,9 @@
       ],
       "auth": null,
       "options": {
-        "authmechanismproperties": { "SERVICE_NAME": "49" }
+        "authmechanismproperties": {
+          "SERVICE_NAME": "49"
+        }
       }
     },
     {
@@ -115,8 +117,8 @@
         "source": "$external",
         "mechanism": "GSSAPI",
         "mechanism_properties": {
-            "SERVICE_NAME": "mongodb",
-            "CANONICALIZE_HOST_NAME": true
+          "SERVICE_NAME": "mongodb",
+          "CANONICALIZE_HOST_NAME": true
         }
       }
     },
@@ -130,28 +132,59 @@
         "source": "$external",
         "mechanism": "GSSAPI",
         "mechanism_properties": {
-            "SERVICE_NAME": "abc",
-            "CANONICALIZE_HOST_NAME": true
+          "SERVICE_NAME": "abc",
+          "CANONICALIZE_HOST_NAME": true
         }
       }
-   },
-   {
-    "description": "Username containing percent encoded multi-byte UTF-8 is valid",
-    "uri": "mongodb://%E2%98%83",
-    "valid": true,
-    "hosts": [
-      {
-        "type": "hostname",
-        "host": "☃",
-        "port": null
+    },
+    {
+      "description": "Username containing percent encoded multi-byte UTF-8 is valid",
+      "uri": "mongodb://%E2%98%83",
+      "valid": true,
+      "hosts": [
+        {
+          "type": "hostname",
+          "host": "☃",
+          "port": null
+        }
+      ]
+    },
+    {
+      "description": "Username containing percent encoded multi-byte UTF-8 is valid",
+      "uri": "mongodb://%E2%D8%83",
+      "valid": false,
+      "warning": true
+    },
+    {
+      "description": "Valid connection and timeout options are parsed correctly, libmongoc version without maxIdleTimeMS",
+      "uri": "mongodb://example.com/?appname=URI-OPTIONS-SPEC-TEST&connectTimeoutMS=20000&heartbeatFrequencyMS=5000&localThresholdMS=3000&replicaSet=uri-options-spec&retryWrites=true&serverSelectionTimeoutMS=15000&socketTimeoutMS=7500",
+      "valid": true,
+      "warning": false,
+      "hosts": null,
+      "auth": null,
+      "options": {
+        "appname": "URI-OPTIONS-SPEC-TEST",
+        "connectTimeoutMS": 20000,
+        "heartbeatFrequencyMS": 5000,
+        "localThresholdMS": 3000,
+        "replicaSet": "uri-options-spec",
+        "retryWrites": true,
+        "serverSelectionTimeoutMS": 15000,
+        "socketTimeoutMS": 7500
+      }
+    },
+    {
+      "description": "Valid connection pool options are parsed correctly, libmongoc version without maxIdleTimeMS",
+      "uri": "mongodb://example.com/?waitQueueTimeoutMS=50000&maxPoolSize=5&minPoolSize=3",
+      "valid": true,
+      "warning": false,
+      "hosts": null,
+      "auth": null,
+      "options": {
+        "waitQueueTimeoutMS": 50000,
+        "maxPoolSize": 5,
+        "minPoolSize": 3
       }
-    ]
-  },
-  {
-    "description": "Username containing percent encoded multi-byte UTF-8 is valid",
-    "uri": "mongodb://%E2%D8%83",
-    "valid": false,
-    "warning": true
-  }
+    }
   ]
 }

src/libmongoc/tests/test-mongoc-connection-uri.c

@@ -104,7 +104,6 @@ run_uri_test (const char *uri_string, bool valid, const bson_t *hosts, const bso
       /* CDRIVER-3167 */
       if ((mongoc_uri_get_option_as_int32 (uri, MONGOC_URI_CONNECTTIMEOUTMS, 0) < 0) ||
           (mongoc_uri_get_option_as_int32 (uri, MONGOC_URI_LOCALTHRESHOLDMS, 0) < 0) ||
-          (mongoc_uri_get_option_as_int32 (uri, MONGOC_URI_MAXIDLETIMEMS, 0) < 0) ||
           (mongoc_uri_get_option_as_int32 (uri, MONGOC_URI_SERVERSELECTIONTIMEOUTMS, 0) < 0) ||
           (mongoc_uri_get_option_as_int32 (uri, MONGOC_URI_SOCKETTIMEOUTMS, 0) < 0)) {
          MONGOC_WARNING ("Invalid negative timeout");
@@ -240,11 +239,18 @@ run_uri_test (const char *uri_string, bool valid, const bson_t *hosts, const bso
 static void
 test_connection_uri_cb (void *scenario_vp)
 {
+   static const test_skip_t skips[] = {
+      {.description = "Valid connection pool options are parsed correctly",
+       .reason = "libmongoc does not support maxIdleTimeMS"},
+      {.description = "Valid connection and timeout options are parsed correctly",
+       .reason = "libmongoc does not support maxIdleTimeMS"},
+      {.description = NULL},
+   };
+
    bson_iter_t iter;
    bson_iter_t descendent;
    bson_iter_t tests_iter;
    bson_iter_t warning_iter;
-   const char *uri_string = NULL;
    bson_t hosts;
    bson_t auth;
    bson_t options;
@@ -262,23 +268,17 @@ test_connection_uri_cb (void *scenario_vp)
 
       bson_iter_bson (&tests_iter, &test_case);
 
-      if (test_suite_debug_output ()) {
-         bson_iter_t test_case_iter;
-
-         ASSERT (bson_iter_recurse (&tests_iter, &test_case_iter));
-         if (bson_iter_find (&test_case_iter, "description")) {
-            const char *description = bson_iter_utf8 (&test_case_iter, NULL);
-            ASSERT (bson_iter_find_case (&test_case_iter, "uri"));
+      const char *description = bson_lookup_utf8 (&test_case, "description");
+      if (test_should_be_skipped (skips, description)) {
+         continue;
+      }
 
-            printf ("  - %s: '%s'\n", description, bson_iter_utf8 (&test_case_iter, 0));
-            fflush (stdout);
-         } else {
-            fprintf (stderr, "Couldn't find `description` field in testcase\n");
-            BSON_ASSERT (0);
-         }
+      const char *uri_string = bson_lookup_utf8 (&test_case, "uri");
+      if (test_suite_debug_output ()) {
+         printf ("  - %s: '%s'\n", description, uri_string);
+         fflush (stdout);
       }
 
-      uri_string = bson_lookup_utf8 (&test_case, "uri");
       /* newer spec test replaces both "auth" and "options" with "credential"
        */
       if (bson_has_field (&test_case, "credential")) {

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

@@ -1988,10 +1988,6 @@ test_mongoc_uri_duplicates (void)
    ASSERT_LOG_DUPE (MONGOC_URI_LOCALTHRESHOLDMS);
    BSON_ASSERT (mongoc_uri_get_option_as_int32 (uri, MONGOC_URI_LOCALTHRESHOLDMS, 0) == 2);
 
-   RECREATE_URI (MONGOC_URI_MAXIDLETIMEMS "=1&" MONGOC_URI_MAXIDLETIMEMS "=2");
-   ASSERT_LOG_DUPE (MONGOC_URI_MAXIDLETIMEMS);
-   BSON_ASSERT (mongoc_uri_get_option_as_int32 (uri, MONGOC_URI_MAXIDLETIMEMS, 0) == 2);
-
    RECREATE_URI (MONGOC_URI_MAXPOOLSIZE "=1&" MONGOC_URI_MAXPOOLSIZE "=2");
    ASSERT_LOG_DUPE (MONGOC_URI_MAXPOOLSIZE);
    BSON_ASSERT (mongoc_uri_get_option_as_int32 (uri, MONGOC_URI_MAXPOOLSIZE, 0) == 2);
@@ -2093,10 +2089,6 @@ test_mongoc_uri_duplicates (void)
    wc = mongoc_uri_get_write_concern (uri);
    BSON_ASSERT (mongoc_write_concern_get_w (wc) == MONGOC_WRITE_CONCERN_W_MAJORITY);
 
-   RECREATE_URI (MONGOC_URI_WAITQUEUEMULTIPLE "=1&" MONGOC_URI_WAITQUEUEMULTIPLE "=2");
-   ASSERT_LOG_DUPE (MONGOC_URI_WAITQUEUEMULTIPLE);
-   BSON_ASSERT (mongoc_uri_get_option_as_int32 (uri, MONGOC_URI_WAITQUEUEMULTIPLE, 0) == 2);
-
    RECREATE_URI (MONGOC_URI_WAITQUEUETIMEOUTMS "=1&" MONGOC_URI_WAITQUEUETIMEOUTMS "=2");
    ASSERT_LOG_DUPE (MONGOC_URI_WAITQUEUETIMEOUTMS);
    BSON_ASSERT (mongoc_uri_get_option_as_int32 (uri, MONGOC_URI_WAITQUEUETIMEOUTMS, 0) == 2);
@@ -2301,6 +2293,38 @@ test_parses_long_ipv6 (void)
    }
 }
 
+void
+test_uri_depr (void)
+{
+   // Test behavior of deprecated URI options.
+   // Regression test for CDRIVER-3769 Deprecate unimplemented URI options
+
+   // Test an unsupported option warns.
+   {
+      capture_logs (true);
+      mongoc_uri_t *uri = mongoc_uri_new ("mongodb://host/?foo=bar");
+      ASSERT_CAPTURED_LOG ("uri", MONGOC_LOG_LEVEL_WARNING, "Unsupported");
+      capture_logs (false);
+      mongoc_uri_destroy (uri);
+   }
+   // Test that waitQueueMultiple warns.
+   {
+      capture_logs (true);
+      mongoc_uri_t *uri = mongoc_uri_new ("mongodb://host/?waitQueueMultiple=123");
+      ASSERT_CAPTURED_LOG ("uri", MONGOC_LOG_LEVEL_WARNING, "Unsupported");
+      capture_logs (false);
+      mongoc_uri_destroy (uri);
+   }
+   // Test that maxIdleTimeMS warns.
+   {
+      capture_logs (true);
+      mongoc_uri_t *uri = mongoc_uri_new ("mongodb://host/?maxIdleTimeMS=123");
+      ASSERT_CAPTURED_LOG ("uri", MONGOC_LOG_LEVEL_WARNING, "Unsupported");
+      capture_logs (false);
+      mongoc_uri_destroy (uri);
+   }
+}
+
 void
 test_uri_install (TestSuite *suite)
 {
@@ -2328,4 +2352,5 @@ test_uri_install (TestSuite *suite)
    TestSuite_Add (suite, "/Uri/one_tls_option_enables_tls", test_one_tls_option_enables_tls);
    TestSuite_Add (suite, "/Uri/options_casing", test_casing_options);
    TestSuite_Add (suite, "/Uri/parses_long_ipv6", test_parses_long_ipv6);
+   TestSuite_Add (suite, "/Uri/depr", test_uri_depr);
 }