search

jhthorsen / json-validator

:cop: Validate data against a JSON schema
https://metacpan.org/release/JSON-Validator
56 stars 58 forks source link

Extract $schema from document #152

Closed karenetheridge closed 4 years ago

karenetheridge commented 5 years ago

Also infer the schema from the "$schema" key in the document, when available. Previously, when using a schema document that was downloaded from an external source, only the $args variable was used to determine the schema's version; we did not also look inside the document for a $schema key. Now we do both, deferring the second check until after the document has been downloaded and parsing begins.

replaces parts of #150.

karenetheridge commented 5 years ago

rebased to master after the last PR got merged.

karenetheridge commented 5 years ago

I've pushed some amendments as a new commit, that I'm happy to rebase back into the original commits later (if I squashed them now, all the review comments above are much harder to see in github's UI).

jhthorsen commented 5 years ago

@karenetheridge: Got any feedback on https://github.com/mojolicious/json-validator/pull/152/files#r272815748 ?

karenetheridge commented 5 years ago

@karenetheridge: Got any feedback on https://github.com/mojolicious/json-validator/pull/152/files#r272815748 ?

I missed that before! I will look at ASAP (today/tomorrow).

jhthorsen commented 4 years ago

You might want to have a quick look at https://github.com/mojolicious/json-validator/compare/jhthorsen/strict. I want to make JSON::Validator strict by default now, but doing so without the user noticing - hopefully. I think I will scrap that branch, but wanted to show it for inspiration (?).

jhthorsen commented 4 years ago

The more I look at the code, the more I think I want to get rid of load_and_validate_schema(). I started on another suggestion here which has new_from_schema() which again will find the appropriate "Draft" subclass. I'll let it sit for a bit, but I will hopefully find time to work on this later this week.

jhthorsen commented 4 years ago

I'm going to close this in favor of #189.