[v24.2.x] CORE-6861 Schema Registry: verbose compat checks for JSON #23299
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds a helper for constructing singleton `raw_compatibility_result`'s. (cherry picked from commit 81fea0a)
Define `operator==` and `operator<<` for `compatibility_result` to allow using it in `BOOST_REQUIRE_EQ` in tests. (cherry picked from commit 7a80f8c)
Add all json incompatibility types that we expect to report with their descriptions matching the reference implementation. (cherry picked from commit 9c7ccc5)
Pure refactor, no behaviour change intended. Make a small change to the method to make the diff in the following commit simpler to read and reason about. (cherry picked from commit a89af69)
Pure refactor, no behaviour change intended. Make a small change to the method to make the diff in the following commit simpler to read and reason about. (cherry picked from commit ea4af04)
Pure refactor, no behaviour change intended. Make a small change to the method to make the diff in the following commit simpler to read and reason about. New tests are also added (they passed before too). (cherry picked from commit ff98239)
Adapt the json compatibility check code to report the exact reason of
the incompatibility that is causing the compatibility check to fail.
Tests are adapted to now make checks explicitly on the reported
incompatibilities instead of just the boolean of whether the change is
compatible or not.
Note that because our json compatibility checks are expressed as forward
compatibility checks and not backward compatibility checks, the names of
the error types are always the inverse of what the surrounding code and
comments express. For example, we report the `additional_items_removed`
error when `newer` has `additionalItems` but `older` does not.
Some general patterns in this change are:
* return false --> add a specific error
* return true --> return a non-error result
* return is_superset(...) --> depending on how the reference
implementation works one of the following is chosen:
- Forward the result:
return is_superset(...)
- Merge the result and continue to gather more:
res.merge(is_superset(...))
- Merge the result and add a more specific error:
res.merge(is_superset(...));
res.emplace(...)
- Only add a specific error if the result has an error:
res.emplace(...)
(cherry picked from commit 337e067)
Add a smoketest to further test that multiple (as many as possible) errors are reported for a complex schema. This is checked in both the forward and backward direction. (cherry picked from commit ca18cfb)
Extend the verbose compatibility message test to include json as well. (cherry picked from commit 6b16be3)
pgellert
approved these changes
Sep 12, 2024
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of PR #23208