schema_registry/json: remove compat checks for "patternProperites"

the legacy software does not directly check patternProperties. this seems likely an oversight, but for now this check is removed
vbotbuildovich · Aug 28, 2024 · d67bbcc · d67bbcc
1 parent b6201a1
commit d67bbcc
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 41 deletions.
diff --git a/src/v/pandaproxy/schema_registry/json.cc b/src/v/pandaproxy/schema_registry/json.cc
@@ -1111,35 +1111,6 @@ bool is_object_properties_superset(
     return true;
 }
 
-bool is_object_pattern_properties_superset(
-  context const& ctx, json::Value const& older, json::Value const& newer) {
-    // check that every pattern property in newer["patternProperties"]
-    // appears in older["patternProperties"] and is compatible with the schema
-
-    // "patternProperties" is a map of <pattern, schema>
-    auto newer_pattern_properties = get_object_or_empty(
-      ctx.newer, newer, "patternProperties");
-    auto older_pattern_properties = get_object_or_empty(
-      ctx.older, older, "patternProperties");
-
-    // TODO O(n^2) lookup
-    for (auto const& [pattern, schema] : newer_pattern_properties) {
-        // search for pattern in older_pattern_properties and check schemas
-        auto older_pp_it = older_pattern_properties.FindMember(pattern);
-        if (older_pp_it == older_pattern_properties.MemberEnd()) {
-            // pattern not in older["patternProperties"], not compatible
-            return false;
-        }
-
-        if (!is_superset(ctx, older_pp_it->value, schema)) {
-            // not compatible
-            return false;
-        }
-    }
-
-    return true;
-}
-
 bool is_object_required_superset(
   context const& ctx, json::Value const& older, json::Value const& newer) {
     // to pass the check, a required property from newer has to be present in
@@ -1250,10 +1221,14 @@ bool is_object_superset(
         // newer)
         return false;
     }
-    if (!is_object_pattern_properties_superset(ctx, older, newer)) {
-        // pattern properties checks are not compatible
-        return false;
-    }
+
+    // note: to match the behavior of the legacy software,
+    // ```
+    // if(!is_object_pattern_properties_superset(ctx, older, newer))
+    //   return false;
+    // ```
+    // is omitted
+
     if (!is_object_required_superset(ctx, older, newer)) {
         // required properties are not compatible
         return false;

diff --git a/src/v/pandaproxy/schema_registry/test/test_json_schema.cc b/src/v/pandaproxy/schema_registry/test/test_json_schema.cc
@@ -458,14 +458,6 @@ static constexpr auto compatibility_test_cases = std::to_array<
     = R"({"type": "object", "properties": {"aaaa": {"type": "string"}}})",
     .reader_is_compatible_with_writer = false,
   },
-  // object checks: patternProperties need to be compatible
-  {
-    .reader_schema
-    = R"({"type": "object", "patternProperties": {"^a": {"type": "integer"}}})",
-    .writer_schema
-    = R"({"type": "object", "patternProperties": {"^a": {"type": "number"}}})",
-    .reader_is_compatible_with_writer = false,
-  },
   // object checks: dependencies removed
   {
     .reader_schema = R"(
@@ -1220,6 +1212,34 @@ static constexpr auto compatibility_test_cases = std::to_array<
 
     .reader_is_compatible_with_writer = true,
   },
+  // object: "patternProperties" is not involved in compat checks
+  {
+    .reader_schema = R"(
+{
+  "type": "object",
+  "patternProperties": {
+    "^a": {
+      "type": "boolean"
+    },
+    "^b": {
+      "type": "integer"
+    }
+  }
+})",
+    .writer_schema = R"(
+{
+  "type": "object",
+  "patternProperties": {
+    "^a": {
+      "type": "null"
+    },
+    "^c": {
+      "type": "string"
+    }
+  }
+})",
+    .reader_is_compatible_with_writer = true,
+  },
 });
 SEASTAR_THREAD_TEST_CASE(test_compatibility_check) {
     store_fixture f;