Skip to content

Commit

Permalink
schema_registry/json: remove compat checks for "patternProperites"
Browse files Browse the repository at this point in the history
the legacy software does not directly check patternProperties.
this seems likely an oversight, but for now this check is removed
  • Loading branch information
andijcr committed Aug 28, 2024
1 parent 745d56c commit bfa034d
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 41 deletions.
41 changes: 8 additions & 33 deletions src/v/pandaproxy/schema_registry/json.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
36 changes: 28 additions & 8 deletions src/v/pandaproxy/schema_registry/test/test_json_schema.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"(
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit bfa034d

Please sign in to comment.