Skip to content

$ref across schema versions clarification #845

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

Closed
wants to merge 2 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions jsonschema-core.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1767,6 +1767,10 @@
URI to identify more than one schema. When multiple schemas try to identify
as the same URI, validators SHOULD raise an error condition.
</t>
<t>
As with all schemas, a referenced schema SHOULD be processed according to its define "$schema"
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this ought (OUGHT?) to be MUST, except that I don't think we ever actually say that $schema MUST be respected at all? But we do say that $vocabulary MUST be respected at least to the point of the implementation failing in the face of required but unsupported vocabularies. So that sort-of implies a MUST for respecting $schema. Maybe?


Looking at this now, it occurs to me that how a schema is loaded and processed is a very fundamental thing that deserves more emphasis than we are giving it. Loading a referenced schema is an important use case, but generally loading schemas is the same whether you found the schema directly, through $ref, or through hypermedia. As @awwright noted in #808, $ref is not really a special case for this.

I think we should have a section about loading schemas that covers the general load-and-process model, and notes any specifics needed for $ref and for hypermedia. My initial thought was to put this early in the spec (general considerations?) but you really need to understand the core vocabulary to do all of the steps.

So I say we should add a new top-level section between section 8 (Core Vocabulary) and section 9 (Applicator Vocabulary) on schema loading. It should include most or all of:

8.2.4.5 Loading a referenced schema (but just Loading a schema)
8.2.4.6 Dereferencing (maybe tweak to cover other ways you get a schema other than just $ref)
8.2.4.3 Guarding against infinite recursion
8.2.4.4 References to possible non-schemas
8.1.4 Detecting a meta-schema
11 Usage in hypermedia (demote to subsection?)

While this sounds like a lot, it's mostly just moving stuff around.

I've gotten several questions that basically boil down to "How do I load schemas?" Consolidating this information would make an essential process much more clear and emphasize its importance. It would also trim the extremely long Core Vocabulary section and avoid some of the really deep nesting that causes key sections to not show up in the Table of Contents right now.

Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

(maybe I should have put all that in the issue, can move if we'd prefer)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds reasonable. As such, want to create a new issue for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK- yeah I'm happy with you merging this after taking into account @gregsdennis 's comment, and I'll file the above so that we can consider it separately.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd probably go as far as to say "Implementations MAY throw an error if a $schema is not provided"... but hey =]

Copy link
Member Author

@Relequestual Relequestual Feb 5, 2020

Choose a reason for hiding this comment

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

I'll hold this as unresolved till I see a related new issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Relequestual no, it's not fair- there is no requirement to provide $schema. A linter can raise an issue, but an implementation? Definitely not.

Copy link
Contributor

Choose a reason for hiding this comment

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

JSON Schema implementations should generally be forgiving. If the spec specifically allows something (and saying that $schema SHOULD be present is more or less equivalent to saying that it MAY be absent) then all implementations should always accept that thing even if we are clearly indicating that doing so is a bad idea.

Things that are bad ideas but neither outright forbidden (requiring an error) nor explicitly have undefined or implementation-defined behavior (allowing an error as that is one possible behavior) need to be allowed. Not warnings, not errors, allowed. A linter can do "warnings" for those things as a separate piece.

Copy link
Member Author

Choose a reason for hiding this comment

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

Apologies, it wasn't clear enough that suggesting implementers throw an error on lack of $schema was not a serious suggestion.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Relequestual ah, I see. I usually read these right after waking up in hopes of catching you and Phil at the end of your days, so subtleties are lost on me.

and "$vocabulary" values, if defined.
</t>
</section>
<section title="Dereferencing">
<t>
Expand Down