jsonschema icon indicating copy to clipboard operation
jsonschema copied to clipboard

Ref v2 Spec

Open Julian opened this issue 7 years ago • 3 comments

This is some random notes on a new version of the ref resolution API. Feel free to leave comments, although some of the below might be overly terse chicken scratch

  • [ ] It should probably be easier to pre-resolve all refs (maybe that should even be the default) rather than lazy/forced resolution
  • [ ] Keep in mind async use cases
  • [ ] Too many methods bruh. Just 2? Resolver().resolve(ref) + Resolver().for_scope(url)?
  • [ ] Take hyperlink.URLs, not strs
  • [ ] What should be the thing for users who want to resolve relative to files on disk?
  • [ ] Move the resolver argument to extend

Julian avatar Jul 19 '17 12:07 Julian

Hey. I was looking at some of the tests that are currently skipped as known failures [1]

I poked this a little and realised that the id field isn't handled when it is is encountered in fragment resolution. E.g. in [2], there is a self-reference {"$ref": "#/definitions/baz/definitions/bar"} where under /definitions/baz the scope is updated.

I haven't had time to work out if there are any ill-effects but one possible solution could be to return a new scope during fragment resolution if an id field is present. Is this something that is worth exploring separately or do you still plan to rework the resolver?

[1] https://github.com/Julian/jsonschema/blob/master/jsonschema/tests/test_jsonschema_test_suite.py#L311-L318

[2] https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/master/tests/draft4/refRemote.json#L103-L135

bsmithers avatar Nov 05 '17 18:11 bsmithers

I am all for an incremental fix if it's small and self contained! Would definitely be appreciated. Just have to be careful to preserve backwards compatibility for any custom defined RefResolvers, but assuming that's not an issue, patch would be great! And we can worry about making things cleaner as a separate item (this ticket).

Julian avatar Nov 08 '17 23:11 Julian

I've just been thinking about the problem of following $id declarations during ref resolution and came across this issue.

I don't think that we should recursively dereference schemas. Whilst it's certainly a simpler approach (at least, conceptually), I think the extra cost should be avoided (especially if there are schemas with lazy reference paths (though I'm not sure if currently jsonschema does this, as even in the case of oneOf we need to ensure only one subschema validates (and hence we have to dereference it anyway)).

I think a better approach would be to follow the $id uri from within the resolution process, such as the approach taken here. I'm not convinced that we could (or should) just use this repository (for a start, it doesn't support the newer $id keyword, but the approach is valid enough (we already use context managers to track the current scope).

I'll give it some thought.

agoose77 avatar Jul 05 '18 21:07 agoose77