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.

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

[DRAFT] Provide richer error info for logical combination schemas #67

Closed
wants to merge 1 commit into from

Conversation

garethsb
Copy link
Contributor

@garethsb garethsb commented Aug 15, 2019

Follow-up to #52.

If a schema includes an "allOf", "anyOf" or "oneOf" near to the root, error messages are almost useless unless they go beyond "one of the subschemas is required to validate" and identify what the failures in each of the subschemas were. Gathering this error info would allow heuristics to be applied to identify the 'best match' (see json-schema-org/json-schema-spec#632), or most nearly matching subschema.

This is a DRAFT pull request to gain feedback as to whether such an approach would fit into this library.

@garethsb
Copy link
Contributor Author

garethsb commented Aug 16, 2019

I suspect it would be a good idea to align error_info with the Output Format included in JSON-Schema draft08.

@garethsb
Copy link
Contributor Author

garethsb commented Aug 16, 2019

I think the draft08 "absoluteKeywordLocation" can be generated from the schema locations and JSON pointers held in the root_schema. However, in the current implementation, during validation I think a schema cannot get these (they are stored in root_schema as std::map keys). I propose to hold these inside the schema objects either as well as, or instead of (and use std::set with custom Comparator for example), in the root_schema.

On the other hand, the "keywordLocation" needs to include $ref explicitly based on how the validation is driven, which I think is best accomplished by adding a schema json_pointer to the validate function arguments.

@garethsb
Copy link
Contributor Author

Another question to answer before I really get stuck into this is whether all the different 'draft08' validation output levels (flag, basic, detail, verbose) make sense for this implementation and whether the gathering of annotations when the document is valid is also useful or not?

@pboettch
Copy link
Owner

You are asking good questions here. I haven't looked at draf8 at all. I can't be of much help in this discussion for the moment.

Copy link
Owner

@pboettch pboettch left a comment

Choose a reason for hiding this comment

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

Your changes look good to me.

And if it improves error reporting then it is a good thing.

@garethsb
Copy link
Contributor Author

garethsb commented Aug 19, 2019

Thanks for the positive response, @pboettch. I've been diving into the definition of Output Formatting in the latest JSON Schema draft, and it makes a lot of sense.

I think we can end up with an implementation that is able to produce the final validation output in any of the four forms. There will need to be some structural changes to the code that will ensure the output includes all failures in the case of multiple failures and on the other hand, allow "short circuit" validation for performance in all cases where that's possible. Short circuiting has two forms:

  • Positive form: Skipping subsequent "anyOf" items after one succeeds
  • Negative form: Skipping subsequent "allOf" items (and keyword siblings) after one fails (and after more than one "oneOf" also)
    The second form of short circuiting should only be performed for Flag output.

On the other hand, the "keywordLocation" needs to include $ref explicitly based on how the validation is driven, which I think is best accomplished by adding a schema json_pointer to the validate function arguments.

I note that the statement in json-schema-org/json-schema-spec#779 (comment) reflects how I was thinking of tweaking the implementation of schema_ref to achieve the above!

(FWIW, I think most of the above will bring much better error reporting without the need to adopt the other changes proposed in draft08 yet.)

@pboettch pboettch mentioned this pull request Nov 12, 2019
@lkersting
Copy link
Contributor

Is there still work to include something like this?

@pboettch
Copy link
Owner

I haven't checked the newer drafts/standards but back then I saw that error-reporting was defined as well. To make it clearer independently of the chosen programming language.

@garethsb
Copy link
Contributor Author

I haven't had time to investigate the progress of the spec or bring this to a conclusion, sorry. It's still a good idea! 💡

@sam20908
Copy link

Is there any updates on this? This would be appreciated

@pboettch
Copy link
Owner

Is there any updates on this? This would be appreciated

Currently not, but if you have time to help, this would equally be appreciated.

@pboettch pboettch deleted the branch pboettch:master June 6, 2022 08:29
@pboettch pboettch closed this Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants