CDRIVER-4184 don't emit closed on invalid topology (#884)

Author: kevinAlbs

src/libmongoc/src/mongoc/mongoc-topology.c

@@ -563,7 +563,12 @@ mongoc_topology_destroy (mongoc_topology_t *topology)
       bson_mutex_destroy (&topology->apm_mutex);
       mongoc_cond_destroy (&topology->srv_polling_cond);
    }
-   _mongoc_topology_description_monitor_closed (&topology->description);
+
+   if (topology->valid) {
+      /* Do not emit a topology_closed event. A topology opening event was not
+       * emitted. */
+      _mongoc_topology_description_monitor_closed (&topology->description);
+   }
 
    mongoc_uri_destroy (topology->uri);
    mongoc_topology_description_destroy (&topology->description);

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

@@ -728,6 +728,127 @@ prose_test_9_pooled (void *unused)
    _prose_test_9 (true);
 }
 
+/* cb_stats_t tracks counters for the test_invalid_topology_pooled and
+ * test_invalid_topology_single tests. */
+typedef struct {
+   int num_topology_opening;
+   int num_topology_closed;
+   int num_server_opening;
+   int num_server_closed;
+} cb_stats_t;
+
+/* invalid_topology_opening is used as a callback for the
+ * test_invalid_topology_pooled and test_invalid_topology_single tests. */
+static void
+invalid_topology_opening (const mongoc_apm_topology_opening_t *event) {
+   cb_stats_t *stats = (cb_stats_t*) mongoc_apm_topology_opening_get_context (event);
+   stats->num_topology_opening++;
+}
+
+/* invalid_topology_closed is used as a callback for the
+ * test_invalid_topology_pooled and test_invalid_topology_single tests. */
+static void
+invalid_topology_closed (const mongoc_apm_topology_closed_t *event) {
+   cb_stats_t *stats = (cb_stats_t*) mongoc_apm_topology_closed_get_context (event);
+   stats->num_topology_closed++;
+}
+
+/* invalid_server_closed is used as a callback for the
+ * test_invalid_topology_pooled and test_invalid_topology_single tests. */
+static void
+invalid_server_closed (const mongoc_apm_server_closed_t *event) {
+   cb_stats_t *stats = (cb_stats_t*) mongoc_apm_server_closed_get_context (event);
+   stats->num_server_closed++;
+}
+
+/* invalid_server_opening is used as a callback for the
+ * test_invalid_topology_pooled and test_invalid_topology_single tests. */
+static void
+invalid_server_opening (const mongoc_apm_server_opening_t *event) {
+   cb_stats_t *stats = (cb_stats_t*) mongoc_apm_server_opening_get_context (event);
+   stats->num_server_opening++;
+}
+
+/* CDRIVER-4184 Test that an invalid topology does not emit a topology_closed
+ * event. */
+static void
+test_invalid_topology_pooled (void *unused) {
+   mongoc_client_pool_t *pool;
+   mongoc_client_t *client;
+   mongoc_uri_t *uri;
+   mongoc_apm_callbacks_t *cbs;
+   cb_stats_t stats = {0};
+
+   /* TXT record for test20.test.build.10gen.cc resolves to "loadBalanced=true". */
+   uri = mongoc_uri_new ("mongodb+srv://test20.test.build.10gen.cc/?replicaSet=rs0");
+   pool = mongoc_client_pool_new (uri);
+   cbs = mongoc_apm_callbacks_new ();
+   mongoc_apm_set_topology_opening_cb (cbs, invalid_topology_opening);
+   mongoc_apm_set_topology_closed_cb (cbs, invalid_topology_closed);
+   mongoc_apm_set_server_opening_cb (cbs, invalid_server_opening);
+   mongoc_apm_set_server_closed_cb (cbs, invalid_server_closed);
+   mongoc_client_pool_set_apm_callbacks (pool, cbs, &stats);
+
+   ASSERT_CMPINT(stats.num_topology_opening, ==, 0);
+
+   /* Pop a client to attempt to start monitoring. Monitoring emits the topology_opening event on valid topologies. */
+   client = mongoc_client_pool_pop (pool);
+   mongoc_client_pool_push (pool, client);
+
+   mongoc_apm_callbacks_destroy (cbs);
+   mongoc_client_pool_destroy (pool);
+   mongoc_uri_destroy (uri);
+
+   ASSERT_CMPINT(stats.num_topology_opening, ==, 0);
+   ASSERT_CMPINT(stats.num_server_opening, ==, 0);
+   ASSERT_CMPINT(stats.num_server_closed, ==, 0);
+   ASSERT_CMPINT(stats.num_topology_closed, ==, 0);
+}
+
+/* CDRIVER-4184 Test that an invalid topology does not emit a topology_closed
+ * event. */
+static void
+test_invalid_topology_single (void *unused) {
+   mongoc_client_t *client;
+   mongoc_uri_t *uri;
+   mongoc_apm_callbacks_t *cbs;
+   cb_stats_t stats = {0};
+   bson_error_t error;
+   mongoc_server_description_t *sd;
+
+   /* TXT records for test20.test.build.10gen.cc resolve to loadBalanced=true. */
+   uri = mongoc_uri_new ("mongodb+srv://test20.test.build.10gen.cc/?replicaSet=true");
+   client = mongoc_client_new_from_uri (uri);
+   cbs = mongoc_apm_callbacks_new ();
+   mongoc_apm_set_topology_opening_cb (cbs, invalid_topology_opening);
+   mongoc_apm_set_topology_closed_cb (cbs, invalid_topology_closed);
+   mongoc_apm_set_server_opening_cb (cbs, invalid_server_opening);
+   mongoc_apm_set_server_closed_cb (cbs, invalid_server_closed);
+   mongoc_client_set_apm_callbacks (client, cbs, &stats);
+
+   ASSERT_CMPINT(stats.num_topology_opening, ==, 0);
+
+   /* Perform server selection. Server selection emits the topology_opening
+    * event on valid topologies. */
+   sd = mongoc_client_select_server (
+      client, false /* for_writes */, NULL /* read_prefs */, &error);
+   ASSERT_ERROR_CONTAINS (
+      error,
+      MONGOC_ERROR_SERVER_SELECTION,
+      MONGOC_ERROR_SERVER_SELECTION_FAILURE,
+      "URI with \"loadbalanced\" enabled must not contain option \"replicaset\"");
+   ASSERT (!sd);
+
+   mongoc_apm_callbacks_destroy (cbs);
+   mongoc_client_destroy (client);
+   mongoc_uri_destroy (uri);
+
+   ASSERT_CMPINT(stats.num_topology_opening, ==, 0);
+   ASSERT_CMPINT(stats.num_server_opening, ==, 0);
+   ASSERT_CMPINT(stats.num_server_closed, ==, 0);
+   ASSERT_CMPINT(stats.num_topology_closed, ==, 0);
+}
+
 void
 test_dns_install (TestSuite *suite)
 {
@@ -770,4 +891,20 @@ test_dns_install (TestSuite *suite)
       NULL,
       NULL,
       test_dns_check_loadbalanced);
+
+   TestSuite_AddFull (
+      suite,
+      "/initial_dns_seedlist_discovery/load-balanced/invalid_topology/pooled",
+      test_invalid_topology_pooled,
+      NULL,
+      NULL,
+      test_dns_check_loadbalanced);
+
+   TestSuite_AddFull (
+      suite,
+      "/initial_dns_seedlist_discovery/load-balanced/invalid_topology/single",
+      test_invalid_topology_single,
+      NULL,
+      NULL,
+      test_dns_check_loadbalanced);
 }