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

Test for "$ref" causing "definitions" or annotations to be ignored #198

Closed
handrews opened this issue Nov 8, 2017 · 7 comments
Closed
Labels
question A non-bug or non-feature request (that typically should be redirected to another medium).

Comments

@handrews
Copy link
Contributor

handrews commented Nov 8, 2017

We test that "$ref" overrides an assertion, but not that it causes non-assertion keywords to be ignored. Should we test that?

@handrews handrews added the question A non-bug or non-feature request (that typically should be redirected to another medium). label Nov 8, 2017
@handrews
Copy link
Contributor Author

@epoberezkin brought up in #197 the idea of instead changing the spec to allow definitions here as it is not a normal keyword. This is worth considering and I'll file it for draft-08 as soon as I wrap up draft-07 in the next few days. I'll link the spec issue in here.

@Julian
Copy link
Member

Julian commented Nov 18, 2017

(I replied on the ticket that my reading at the time was that this was already allowed in the spec :), but if we decide it isn't, then yeah we should test this.)

@handrews
Copy link
Contributor Author

Agreed with @Julian in #197 to close that, which means that we'll definitely just work on clarifying it in draft-08. This goes more in the direction @epoberezkin wanted as well, so we've got a pretty good consensus past and present.

No new tests required here so I'm closing this. Once we clarify it, there may be new test cases to add but we'll handle that then.

@awwright
Copy link
Member

The one issue I'm running into is this means I'd have to store arbitrary JSON documents (that aren't schemas, only references) in the schema registry in the off chance that one of the references pulls up an embedded schema by property path.

@handrews
Copy link
Contributor Author

@awwright can you elaborate on that? When I implemented references, I just kept a map of URIs (including fragments) to schemas. I wrote $schema and id/$id values into the in-memory schemas so that I could easily determine the current meta-schema and base URI when using referenced schemas. I don't recall any cases where I needed to store any other JSON documents.

@awwright
Copy link
Member

awwright commented Feb 14, 2018

The issue with $ref is, since everything else is supposed to be ignored, it's impossible to tell which values are a schema, in order to enter them into a registry.

"definitions" is a little more than just a reserved keyword, it's asserting all the children will be schemas. That's useful if you're scanning a schema for sub-schemas.

If we have a document like

{
   definitions: { "a": { type:"string" } },
   $ref: "#/type/a"
}

There's only one schema inside this JSON document, and it is { type:"string" }. It's URI would supposedly be {}#/definitions/a (where {} is an anonymous suffix, to invent the concept of an anonymous URI), but the {} document isn't in the registry because it's not a schema.

I suppose I could make an exception and just rewrite this case to look like

{
   definitions: { "a": { type:"string" } },
   allOf: [ { $ref: "#/type/a" } ]
}

(and I probably will)

But the question is should implementations be required to support cases where the root document is a mere reference and not a schema? (Actually, this question is json-schema-org/json-schema-spec#505)

@handrews
Copy link
Contributor Author

Your "exception" is exactly what we're doing for draft-08 per json-schema-org/json-schema-spec#514 and json-schema-org/json-schema-spec#523, so I'd say just do that. Don't put a ton of effort into implementing a soon-to-be-obsolete corner case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question A non-bug or non-feature request (that typically should be redirected to another medium).
Projects
None yet
Development

No branches or pull requests

3 participants