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

Define "applicability" and "attachment" #424

Merged
merged 5 commits into from
Oct 16, 2017

Conversation

handrews
Copy link
Contributor

@handrews handrews commented Sep 25, 2017

This is a bit more groundwork for the rewrite, and is relevant
to issue #423.

NOTE: This has been updated from its original approach to move the concept into the validation specification!

The commits have been broken up with individual messages into useful chunks:

  1. Just move the metadata section
  2. Split out and re-word "definitions"
  3. Generalize and move applicability from hyper-schema to validation/annotation
  4. Group annotation keywords & re-indent without changing
  5. Update annotation keywords with multi-value behavior

Rework the whole applicability/attachement concept from the
hyper-schema specification into the validations specification,
and establish it as the basis for working with annotation keywords.

Update the annotation kewyords with clarifications on how to
handle values across multiple applicable schemas.

Update the hyper-schema specification to reference the now more
generic description of validation interaction in the validation
spec itself, with additional clarifications about how to handle
combinign keywords from multiple applicable schemas.

Split the "Metadata" section into a re-use section for just the
"definitions" keyword, and a section on annotation and extensibility
which defines applicability/attachement and defines the simple
annotation vocabulary that was already present in the validation
spec.

Move all of that after "format" and "String-encoding non-JSON data"
as those sections are both more closely aligned with validation
than with either re-use or annotations.

handrews added a commit to handrews/json-schema-spec that referenced this pull request Sep 25, 2017
This consolidates all of the URI Template resolution sections into
one section.  The new section is at the top level, as it applies
to both LDO keywords ("href", "anchor", and all of the algorithm
modifiers) and to a schema keyword ("base").

As much of the algorithm as possible is now given in pseudocode,
although whether this is the most readable form is debatable.
It is certainly more concise, and forced detailed consideration
of some steps which had been under-specified, particularly with
respect to "hrefSchema".

The main change is that the input data for "hrefSchema" is
prepopulated only with instance data that validates against
all applicable subschemas within "hrefSchema".  Expressing
this properly requires the "applicability" concept from PR json-schema-org#424.

Previously, the only mention of how to exclude a field from being
used to pre-populate input was the advice to set the "properties"
subschema to "false".  However, this does not hold up once you
start using more complex schemas.
handrews added a commit to handrews/json-schema-spec that referenced this pull request Sep 25, 2017
This consolidates all of the URI Template resolution sections into
one section.  The new section is at the top level, as it applies
to both LDO keywords ("href", "anchor", and all of the algorithm
modifiers) and to a schema keyword ("base").

As much of the algorithm as possible is now given in pseudocode,
although whether this is the most readable form is debatable.
It is certainly more concise, and forced detailed consideration
of some steps which had been under-specified, particularly with
respect to "hrefSchema".

The main changes are that setting any applicable subschema for
a template variable to "false" in "hrefSchema" excludes that
variable from accepting input, and that the input data for
"hrefSchema" is prepopulated only with instance data that
validates against all applicable subschemas within "hrefSchema".
Expressing this properly requires the "applicability" concept
from PR json-schema-org#424.

Previously, the only mention of how to exclude a field from being
used to pre-populate input was the advice to set the "properties"
subschema to "false".  However, this does not hold up once you
start using more complex schemas.
@handrews handrews added this to the draft-07 (wright-*-02) milestone Sep 26, 2017
@handrews handrews force-pushed the applicability branch 2 times, most recently from ff4fdfa to 210ade2 Compare September 27, 2017 01:40
@handrews handrews requested a review from dlax September 27, 2017 01:41
Copy link
Member

@dlax dlax left a comment

Choose a reason for hiding this comment

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

I like it!

When multiple subschemas are applicable to a given sub-instance, all "link"
arrays MUST be concatenated, in any order, into a single array. Each object
in the resulting array MUST retain its own list of applicable "base" values
from the same schema and any parent schemas.
Copy link
Member

Choose a reason for hiding this comment

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

This paragraph makes me think back about #124. So if links was an object, what does concatenation would mean?
Might be an argument in favor of keeping the array...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If links becomes an object, then it's an object of arrays, so for each relation that appears in multiple subschemas, you would concatenate the arrays for that relation to produce the array in the resulting combined value.

Copy link
Member

Choose a reason for hiding this comment

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

True. I realized about that after writing the comment.

<t>
A validation implementation MAY choose to implement determining subschema
applicability and providing access to the value(s) of applicable annotation
kewords. Implementation of this feature MAY instead be done separately.
Copy link
Member

Choose a reason for hiding this comment

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

typo: kewords

kewords. Implementation of this feature MAY instead be done separately.
</t>
<t>
Since many subschemas can be applicable to any single sub-isntance, each
Copy link
Member

Choose a reason for hiding this comment

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

typo: isntance

Copy link
Member

Choose a reason for hiding this comment

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

@handrews That one is still there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dammit... I distinctly remember searching for this, but I must have done it in hyperschema and forgotten about validation. Fixed now. For reals. I think.

available as an array in which order is not significant and items need not
be unique.
</t>
<section title="Combinatoric and conditoinal schemas">
Copy link
Member

Choose a reason for hiding this comment

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

typo: conditoinal

@dlax
Copy link
Member

dlax commented Sep 27, 2017

@handrews

The commits have been broken up with individual messages into useful chunks

Thanks for this, it really makes reviewing pleasant as far as I am concerned.

@handrews
Copy link
Contributor Author

The commits have been broken up with individual messages into useful chunks

Thanks for this, it really makes reviewing pleasant as far as I am concerned.

You're welcome! I do try to make it easy when I see how it can be split up (I just tend to see that criteria a bit differently sometimes, it seems).

BTW, I finally figured out why I keep having so much trouble with spelling typos: the vim spell-checker ignores text within elements in XML. It will check text in attributes, but not element contents. I DO NOT UNDERSTAND WHY. But at least I finally know why they keep getting through. (although apparently I forgot to turn it on at all this time as it would have caught the "conditional" at least).

@awwright
Copy link
Member

I'm still looking at this.
I don't see any obvious problems, but maybe some cases where language could be clarified. Maybe I just need to read the paragraphs in context.

@handrews
Copy link
Contributor Author

@awwright language suggestions welcome. I am also working on a test suite format for this to add to the test suite repo, so I think that will help clarify as well.

@handrews handrews force-pushed the applicability branch 3 times, most recently from d5cd621 to d9c913a Compare September 28, 2017 22:45
@handrews
Copy link
Contributor Author

@dlax I think I fixed all the typos. I merged them into the existing commits because then they are less distracting for other people reading for the first time.

@Relequestual
Copy link
Member

Relequestual commented Oct 3, 2017

Just to make sure I understand the applicability and attachment element...

It's mainly to do with c55678a#diff-2391a2ef934e9aed11cbf87a65eb61c9R1172

In that, annotation key words are only attached to a sub-instance if the sub-schema validates successfully for the sub-instance?

(please do ping me on slack when you reply to this so I pick it up right away)

@handrews
Copy link
Contributor Author

handrews commented Oct 3, 2017

@Relequestual yup, that's correct

@handrews
Copy link
Contributor Author

handrews commented Oct 8, 2017

@awwright @Relequestual please let me know if your concerns have been addressed, and if not, what actionable objections you have.

This is a fundamental piece of defining how JSON Schema works as a multi-vocabulary media type, and in particular how validation serves as a base for applying other vocabularies.

The format and string-encoded content keywords have validation
semantics (even if only optionally), so they should go between
the required validation keywords and the metadata keywords.
In preparation for the applicability text, split out "definitions"
into its own section about re-use, and rename the remainder of
the old metadata section to focus on annotation of schemas with
keywords or new vocabularies.

Rework the "definitions" section text, but leave the other
sections alone until the next commit.
Generalize this and move it into validation, updating the
hyper-schema spec to reference it.
...with some intro text, and re-indentation, but no changes
to the contents of the re-indented sections.
With the applicability/attachment concept, we now have a way
to talk about what happens when multiple annotation values
are present.  Add notes to each keyword as needed.
@handrews
Copy link
Contributor Author

I'm still looking at this.
I don't see any obvious problems, but maybe some cases where language could be clarified. Maybe I just need to read the paragraphs in context.

@awwright has this cleared up with more reading? Or do you have actionable feedback such as indicating the unclear language? It's been two weeks since this comment and I'm not sure how to move forward.

@handrews
Copy link
Contributor Author

Given that this has two approvals, I am going to go ahead and merge. @awwright I am happy to accept further issues on this to work on it more, but it's been nearly 3 weeks since your "I'm thinking" comment and neither mentioning you here nor asking offline has produced anything actionable. We have plenty of time before anything will be published so this is not irreversible.

@handrews handrews merged commit a5beb33 into json-schema-org:master Oct 16, 2017
@handrews handrews deleted the applicability branch October 16, 2017 16:45
handrews added a commit to handrews/json-schema-spec that referenced this pull request Oct 16, 2017
This consolidates all of the URI Template resolution sections into
one section.  The new section is at the top level, as it applies
to both LDO keywords ("href", "anchor", and all of the algorithm
modifiers) and to a schema keyword ("base").

As much of the algorithm as possible is now given in pseudocode,
although whether this is the most readable form is debatable.
It is certainly more concise, and forced detailed consideration
of some steps which had been under-specified, particularly with
respect to "hrefSchema".

The main changes are that setting any applicable subschema for
a template variable to "false" in "hrefSchema" excludes that
variable from accepting input, and that the input data for
"hrefSchema" is prepopulated only with instance data that
validates against all applicable subschemas within "hrefSchema".
Expressing this properly requires the "applicability" concept
from PR json-schema-org#424.

Previously, the only mention of how to exclude a field from being
used to pre-populate input was the advice to set the "properties"
subschema to "false".  However, this does not hold up once you
start using more complex schemas.
@gregsdennis gregsdennis added clarification Items that need to be clarified in the specification and removed Type: Maintenance labels Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Items that need to be clarified in the specification Priority: High
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants