CDRIVER-4374 only allow extra fields when matching root docs

jmikola · jmikola · commit dcde2077eb02 · 2022-05-04T16:49:55.000-04:00
diff --git a/src/libmongoc/tests/bsonutil/bson-match.c b/src/libmongoc/tests/bsonutil/bson-match.c
@@ -353,7 +353,7 @@ bson_matcher_match (bson_matcher_t *matcher,
                     const bson_val_t *expected,
                     const bson_val_t *actual,
                     const char *path,
-                    bool allow_extra,
+                    bool array_of_root_docs,
                     bson_error_t *error)
 {
    bool ret = false;
@@ -421,12 +421,8 @@ bson_matcher_match (bson_matcher_t *matcher,
          }
 
          path_child = bson_strdup_printf ("%s.%s", path, key);
-         if (!bson_matcher_match (matcher,
-                                  expected_val,
-                                  actual_val,
-                                  path_child,
-                                  allow_extra,
-                                  error)) {
+         if (!bson_matcher_match (
+                matcher, expected_val, actual_val, path_child, false, error)) {
             bson_val_destroy (expected_val);
             bson_val_destroy (actual_val);
             bson_free (path_child);
@@ -439,14 +435,20 @@ bson_matcher_match (bson_matcher_t *matcher,
 
       expected_keys = bson_count_keys (expected_bson);
       actual_keys = bson_count_keys (actual_bson);
-      if (!is_root) {
-         if (expected_keys < actual_keys && !allow_extra) {
-            MATCH_ERR ("expected %" PRIu32 " keys in document, got: %" PRIu32,
-                       expected_keys,
-                       actual_keys);
-            goto done;
-         }
+
+      /* Unified test format spec: "When matching root-level documents, test
+       * runners MUST permit the actual document to contain additional fields
+       * not present in the expected document.""
+       *
+       * This logic must also handle the case where `expected` is one of any
+       * number of root documents within an array (i.e. cursor result). */
+      if (!(is_root || array_of_root_docs) && expected_keys < actual_keys) {
+         MATCH_ERR ("expected %" PRIu32 " keys in document, got: %" PRIu32,
+                    expected_keys,
+                    actual_keys);
+         goto done;
       }
+
       ret = true;
       goto done;
    }
@@ -467,8 +469,8 @@ bson_matcher_match (bson_matcher_t *matcher,
 
       actual_bson = bson_val_to_array (actual);
       actual_keys = bson_count_keys (actual_bson);
-      if ((expected_keys > actual_keys) ||
-          (expected_keys < actual_keys && !allow_extra)) {
+
+      if (expected_keys != actual_keys) {
          MATCH_ERR ("expected array of size %" PRIu32
                     ", but got array of size: %" PRIu32,
                     expected_keys,
@@ -498,7 +500,7 @@ bson_matcher_match (bson_matcher_t *matcher,
                                   expected_val,
                                   actual_val,
                                   path_child,
-                                  allow_extra,
+                                  (is_root && array_of_root_docs),
                                   error)) {
             bson_val_destroy (expected_val);
             bson_val_destroy (actual_val);
@@ -539,12 +541,12 @@ bson_matcher_match (bson_matcher_t *matcher,
 bool
 bson_match (const bson_val_t *expected,
             const bson_val_t *actual,
-            bool allow_extra,
+            bool array_of_root_docs,
             bson_error_t *error)
 {
    bson_matcher_t *matcher = bson_matcher_new ();
-   bool matched =
-      bson_matcher_match (matcher, expected, actual, "", allow_extra, error);
+   bool matched = bson_matcher_match (
+      matcher, expected, actual, "", array_of_root_docs, error);
    bson_matcher_destroy (matcher);
    return matched;
 }
diff --git a/src/libmongoc/tests/bsonutil/bson-match.h b/src/libmongoc/tests/bsonutil/bson-match.h
@@ -24,7 +24,7 @@
 bool
 bson_match (const bson_val_t *expected,
             const bson_val_t *actual,
-            bool allow_extra,
+            bool array_of_root_docs,
             bson_error_t *error);
 
 /* A bson_matcher_t may be used to extend the default matching behavior. */
@@ -58,7 +58,7 @@ bson_matcher_match (bson_matcher_t *matcher,
                     const bson_val_t *expected,
                     const bson_val_t *actual,
                     const char *path,
-                    bool allow_extra,
+                    bool array_of_root_docs,
                     bson_error_t *error);
 
 void
diff --git a/src/libmongoc/tests/unified/entity-map.c b/src/libmongoc/tests/unified/entity-map.c
@@ -353,7 +353,7 @@ entity_client_new (entity_map_t *em, bson_t *bson, bson_error_t *error)
       }
    } else if (use_multiple_mongoses != NULL) {
       if (!test_framework_uri_apply_multi_mongos (
-               uri, *use_multiple_mongoses, error)) {
+             uri, *use_multiple_mongoses, error)) {
          goto done;
       }
    }
@@ -396,7 +396,8 @@ entity_client_new (entity_map_t *em, bson_t *bson, bson_error_t *error)
          } else if (0 == strcmp (event_type, "commandSucceededEvent")) {
             mongoc_apm_set_command_succeeded_cb (callbacks, command_succeeded);
          } else if (is_unsupported_event_type (event_type)) {
-            MONGOC_DEBUG ("Skipping observing unsupported event type: %s", event_type);
+            MONGOC_DEBUG ("Skipping observing unsupported event type: %s",
+                          event_type);
             continue;
          } else {
             test_set_error (error, "Unexpected event type: %s", event_type);
@@ -1288,7 +1289,7 @@ bool
 entity_map_match (entity_map_t *em,
                   const bson_val_t *expected,
                   const bson_val_t *actual,
-                  bool allow_extra,
+                  bool array_of_root_docs,
                   bson_error_t *error)
 {
    bson_matcher_t *matcher;
@@ -1299,7 +1300,8 @@ entity_map_match (entity_map_t *em,
       matcher, "$$sessionLsid", special_session_lsid, em);
    bson_matcher_add_special (
       matcher, "$$matchesEntity", special_matches_entity, em);
-   ret = bson_matcher_match (matcher, expected, actual, "", allow_extra, error);
+   ret = bson_matcher_match (
+      matcher, expected, actual, "", array_of_root_docs, error);
    bson_matcher_destroy (matcher);
    return ret;
 }
diff --git a/src/libmongoc/tests/unified/result.c b/src/libmongoc/tests/unified/result.c
@@ -33,6 +33,7 @@ struct _result_t {
       For a read operation, reply is an optional server reply. */
    bson_t *reply;
    char *str;
+   bool array_of_root_docs;
 };
 
 result_t *
@@ -305,6 +306,7 @@ result_from_cursor (result_t *result, mongoc_cursor_t *cursor)
    val = bson_val_from_array (documents);
 
    _result_init (result, val, (bson_t *) reply, &error);
+   result->array_of_root_docs = true;
    bson_destroy (documents);
    bson_val_destroy (val);
 }
@@ -361,9 +363,13 @@ result_check (result_t *result,
             error, "expected result, but got error: %s", result->error.message);
          goto done;
       }
-      if (!entity_map_match (em, expect_result, result->value, false, error)) {
+      if (!entity_map_match (em,
+                             expect_result,
+                             result->value,
+                             result->array_of_root_docs,
+                             error)) {
          test_set_error (error,
-                         "checking expectResult: %s",
+                         "checking expectResult: %s\n",
                          bson_val_to_json (expect_result));
          goto done;
       }
@@ -527,7 +533,12 @@ result_check (result_t *result,
             goto done;
          }
 
-         if (!bson_match (error_expect_result, result->value, true, error)) {
+         /* TODO: expectError.expectResult is not used for cursor-bearing
+          * operations, so array_of_root_docs should always be false */
+         if (!bson_match (error_expect_result,
+                          result->value,
+                          result->array_of_root_docs,
+                          error)) {
             test_diagnostics_error_info (
                "checking error.expectResult: %s",
                bson_val_to_json (error_expect_result));
diff --git a/src/libmongoc/tests/unified/runner.c b/src/libmongoc/tests/unified/runner.c
@@ -1073,7 +1073,7 @@ test_check_event (test_t *test,
       actual_val = bson_val_from_bson (actual->command);
 
       if (!entity_map_match (
-             test->entity_map, expected_val, actual_val, true, error)) {
+             test->entity_map, expected_val, actual_val, false, error)) {
          bson_val_destroy (expected_val);
          bson_val_destroy (actual_val);
          goto done;
@@ -1104,7 +1104,7 @@ test_check_event (test_t *test,
       bson_val_t *expected_val = bson_val_from_bson (expected_reply);
       bson_val_t *actual_val = bson_val_from_bson (actual->reply);
       if (!entity_map_match (
-             test->entity_map, expected_val, actual_val, true, error)) {
+             test->entity_map, expected_val, actual_val, false, error)) {
          bson_val_destroy (expected_val);
          bson_val_destroy (actual_val);
          goto done;
