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

Clean-up of stable features and removal of unstable features #1444

Open
3 of 12 tasks
gregsdennis opened this issue Sep 20, 2023 · 5 comments
Open
3 of 12 tasks

Clean-up of stable features and removal of unstable features #1444

gregsdennis opened this issue Sep 20, 2023 · 5 comments

Comments

@gregsdennis
Copy link
Member

gregsdennis commented Sep 20, 2023

In What current features should be considered unstable? we determined that most everything available in 2020-12 will be considered stable, with some things having a few changes.

This issue will catalog the things we need to do to resolve that discussion.

  • Scrutinize $schema and $id to ensure that everything is right because we can't change it if it's wrong
  • [Issue] [PR] Remove initial resolution requirement from $dynamicRef (@jdesrosiers)
  • [PR] Remove $anchor behavior from $dynamicAnchor (@jdesrosiers)
  • [PR] Extract propertyDependencies (see also Should we have a supplementary spec template for proposals? #1443) (@gregsdennis & @jdesrosiers)
  • [PR] Extract output (@gregsdennis)
  • [PR] Add language that makes it clear using annotations any mechanism used to describe keyword interaction is for illustration purposes and not a prescriptive implementation architecture (@gregsdennis)
  • [PR] Update interaction description for if/then/else to use annotation mechanism (@gregsdennis)
  • Update interaction description for contains/minContains/maxContains to use annotation mechanism (requires Reconsider allowing "contains" to apply to objects #1358 first)
  • Ensure interaction description for unevaluated* (which currently uses annotation mechanism) is clear
  • Check if/then/else for clarity using evaluation-result dynamic dependency mechanism
  • Remove annotation dependency descriptions from keywords where it's not required (basically everything except unevaluated*)
  • Create clear separation between the two referencing systems ($ref vs $dynamicRef)

I think that's everything. Edit this comment to add more. I'd like to keep everything here instead of having to search below.

If you start work on one of these, please edit the issue and put your name next to what you're working on so that effort isn't duplicated.

It should also be noted that this is not intended to be everything we need for stable release; only those things covered in the discussion.

@gregsdennis
Copy link
Member Author

I checked the following for $schema and it all seems mostly good to me:

  • Core 7.1 (lexical scope)
  • Core 8.1 and subsections (keyword definition)
  • Core 9.1.3 (processing as a meta-schema)
  • Core 9.3.1 (bundling)
  • Core 9.3.2 (changing dialects) - this may have some redundant language with 8.1.1 (dialect determination) and could probably use a link to that section
  • Core 9.3.3 (meta-schema validation of schemas that change dialects)

There is also #1442 where we're discussing a possible tweak to the meta-schema validation language (9.3.3), but I don't think the concept is going to change.

@gregsdennis
Copy link
Member Author

Checking for $id raises some questions:

  • Core 8.2.1
    • Fragments are now disallowed
    • $id defines a new schema resource
    • relative IRIs in subschemas are to be resolved against the parent's IRI
  • Core 8.2.1.1
    • Schema document roots SHOULD have an $id (maybe relax this? we have 9.1.1 Initial Base IRI to determine an identifier. also this isn't a requirement of implementors, but of authors, which... can we put a SHOULD requirement on authors? how would an implementation enforce that?)
  • Core 8.2.3
    • Duplicate identifiers SHOULD raise an error, otherwise behavior is undefined and not considered interoperable
  • Core 9.1.1
    • This section seems to make a distinction between a schema and the document that contains it. In particular, there is a difference between the base IRI of a document (where it's found) and that of a schema (defined by $id). I think the language here could be cleaned up to make that more explicit (if that is what it's trying to say).
    • Short of a schema declaring $id, the schema's base IRI would fall back to the base IRI of its parent, which may be another schema or ultimately the document which contains it.
  • Core 9.2.1
    • Pointers that cross resource boundaries are not explicitly disallowed, but as guidance to schema authors, the spec recommends using the value in $id (which defines the schema boundary) rather than a pointer that goes through it in order to support the case where a schema could be decomposed into its resources. We've had some discussion around this, but I'm not sure if we want to change it. It seems fine as is to me.
  • Core 9.3.1
    • Each schema resource in a bundled schema MUST use $id.
    • (There's a typo in the third paragraph: "identifer" vs "identifier")
  • Core 9.4.2
    • References into unknown keywords cannot reliably or interoperably be expected to resolve to a schema because implementations cannot be expected to search for $id as an identifier (might be just data).
    • This section might actually be a candidate for removal since we're disallowing unknown keywords now. There may be parts we want to keep, though.
  • Core 13
    • Servers need to protect against bad actors overwriting schemas by attempting to register new schemas against known $ids.

Any other references are minor usages or examples that don't describe the behavior of $id.


It should also be noted that a lot of these sections have references to $dynamic* operating within the same referencing system as $id/$anchor/$ref. Part of the "removing bookending" and "removing initial resolution step" work is also "disconnecting$dynamicRef and $ref referencing systems." These sections will need to be revisited to address that. (Added to the list above)

@jdesrosiers
Copy link
Member

I think the identification stuff needs a lot of attention. Probably after the initial release, I'd like to look into completely reworking this area.

Schema document roots SHOULD have an $id (maybe relax this? we have 9.1.1 Initial Base IRI to determine an identifier. also this isn't a requirement of implementors, but of authors, which... can we put a SHOULD requirement on authors? how would an implementation enforce that?)

Agree. I say relax it to make it informal or remove it entirely.

Pointers that cross resource boundaries are not explicitly disallowed, but as guidance to schema authors, the spec recommends using the value in $id (which defines the schema boundary) rather than a pointer that goes through it in order to support the case where a schema could be decomposed into its resources. We've had some discussion around this, but I'm not sure if we want to change it. It seems fine as is to me.

We identified that the behavior of pointers crossing schema resource boundaries needs to specified as "undefined" rather than "allowed" for compatibility reasons. (I feel like we talked about this in more detail than the short convo I linked to, but this is what I found.)

This section might actually be a candidate for removal since we're disallowing unknown keywords now.

That makes sense to me.

@gregsdennis
Copy link
Member Author

Probably after the initial release, I'd like to look into completely reworking this area.

Remember that we can't break things, so it's important to get this right with the first release.

That said, I'm uninterested in re-hashing old conversations, if that's what you're getting at.

@jdesrosiers
Copy link
Member

I wasn't referring to any old conversations or any behavior changes. I just want to clean up the way the spec presents the concepts. That's why I said it can be done after the initial release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Awaiting PR
Development

No branches or pull requests

3 participants
@jdesrosiers @gregsdennis and others