Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CORE-6836] schema_registry/json: Support Internal References #22815

Closed
wants to merge 4 commits into from
Closed

[CORE-6836] schema_registry/json: Support Internal References #22815

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aed3fee
schema_registry/json: Encapsulate refs in the impl
BenPope Aug 1, 2024
d43f488
schema_registry/json: Introduce as_string_view
BenPope Aug 2, 2024
87393e6
schema_registry/json: Introduce a context
BenPope Aug 2, 2024
955f062
schema_registry/json: Support internal references
BenPope Aug 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions src/v/pandaproxy/schema_registry/json.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "json/chunked_input_stream.h"
#include "json/document.h"
#include "json/ostreamwrapper.h"
#include "json/pointer.h"
#include "json/schema.h"
#include "json/stringbuffer.h"
#include "json/writer.h"
Expand Down Expand Up @@ -182,6 +183,26 @@ class schema_context {
explicit schema_context(json_schema_definition::impl const& schema)
: _schema{schema} {}

json::Document::ValueType const& resolve(std::string_view ref) const {
// Internal reference
if (ref.starts_with("#")) {
json::Pointer ptr{
ref.data() + 1,
ref.length() - 1,
};
if (auto* p = ptr.Get(_schema.doc); p) {
return *p;
}
throw as_exception(error_info{
error_code::schema_invalid,
fmt::format("Reference not found: '{}'", ref)});
}

throw as_exception(error_info{
error_code::schema_invalid,
fmt::format("External references not supported: '{}'", ref)});
}

Comment on lines +187 to +205
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By internal references, do you mean non-cross-schema-version references in general or references starting with # specifically? Looking at https://json-schema.org/understanding-json-schema/structuring#dollarref, it's possible to have "internal" references (ie. a reference to a schema within the same schema document) that don't start with #. Do we want to support those kinds of references as well (eg. "$ref": "/schemas/address")?

private:
json_schema_definition::impl const& _schema;
};
Expand Down Expand Up @@ -499,10 +520,13 @@ json_type_list normalized_type(json::Value const& v) {
return ret;
}

// helper to convert a boolean to a schema
// helper to convert a boolean to a schema, and traverse references
json::Value::ConstObject
get_schema(schema_context const&, json::Value const& v) {
get_schema(schema_context const& ctx, json::Value const& v) {
if (v.IsObject()) {
if (auto it = v.FindMember("$ref"); it != v.MemberEnd()) {
return ctx.resolve(as_string_view(it->value)).GetObject();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be the ok for draft4/7 ($ref overwrites the schema), but reading the spec seems like for 2019/20 the result should be merged on top of the schema.

A schema can reference another schema using the $ref keyword. The value of $ref is a URI-reference that is resolved against the schema's [Base URI](https://json-schema.org/understanding-json-schema/structuring#base-uri). When evaluating a $ref, an implementation uses the resolved identifier to retrieve the referenced schema and applies that schema to the [instance](https://json-schema.org/learn/glossary#instance).
Draft-specific info
In Draft 4-7, $ref behaves a little differently. When an object contains a $ref property, the object is considered a reference, not a schema. Therefore, any other properties you put in that object will not be treated as JSON Schema keywords and will be ignored by the validator. $ref can only be used where a schema is expected.

Then again, I suspect our reference implementation does not implement the merge behavior.
I'll check

}
return v.GetObject();
}

Expand Down Expand Up @@ -1444,10 +1468,7 @@ bool is_superset(
}

for (auto not_yet_handled_keyword : {
"definitions",
"dependencies",
// draft 6 unhandled keywords:
"$ref",
// draft 2019-09 unhandled keywords:
"dependentRequired",
"dependentSchemas",
Expand Down
25 changes: 25 additions & 0 deletions src/v/pandaproxy/schema_registry/test/test_json_schema.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,31 @@ static constexpr auto compatibility_test_cases = std::to_array<
= R"({"$schema": "http://json-schema.org/draft-06/schema#"})",
.reader_is_compatible_with_writer = true,
},
// refs
{
.reader_schema = R"({"type": "string"})",
.writer_schema = R"({
"$ref": "#/definitions/a_ref",
"definitions": {
"a_ref": {
"type": "string"
}
}
})",
.reader_is_compatible_with_writer = true,
},
{
.reader_schema = R"({
"$ref": "#/$defs/a_ref",
"$defs": {
"a_ref": {
"type": "string"
}
}
})",
.writer_schema = R"({"type": "string"})",
.reader_is_compatible_with_writer = true,
},
});
SEASTAR_THREAD_TEST_CASE(test_compatibility_check) {
store_fixture f;
Expand Down
Loading