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

additionalProperties behavior for non-object instances #103

Closed
handrews opened this issue Oct 19, 2016 · 7 comments
Closed

additionalProperties behavior for non-object instances #103

handrews opened this issue Oct 19, 2016 · 7 comments
Milestone

Comments

@handrews
Copy link
Contributor

This is a pretty subtle wording issue, and easily fixed, if we agree on the desirabilities of the underlying principles.

The wording for validating the additionalProperties keyword against an object (independent of its children) has changed from the following in v4 (fge-00):

Successful validation of an object instance against these three
keywords depends on the value of "additionalProperties":

if its value is boolean true or a schema, validation succeeds;

if its value is boolean false, the algorithm to determine
validation success is described below.

To this in v5 (wright-00):

The value of "additionalProperties" MUST be a boolean or a schema.

If "additionalProperties" is absent, it may be considered present
with an empty schema as a value.

If "additionalProperties" is true, validation always succeeds.

If "additionalProperties" is false, validation succeeds only if the
instance is an object and all properties on the instance were covered
by "properties" and/or "patternProperties".

If "additionalProperties" is an object, validate the value as a
schema to all of the properties that weren't validated by
"properties" nor "patternProperties".

Oddly, the wording of "additionalItems" has not been changed to have this problem- it still preserves the behavior it had in v4.

The old wording had the following properties, which are lost in the new wording:

  • Whether or not the object itself validated could be calculated independent of the validation outcomes of its children.
  • The validation effects of properties, patternProperties, and additionalProperties with respect to the object itself were independent:
    • properties did not affect validation - as an object validation keyword, it always succeeds
    • patternProperties did not affect validation - as an object validation keyword, it always succeeds
    • additionalProperties always fails for the value false and always succeeds otherwise, no matter the value of the other two keywords

This language change significantly affects the theoretical model and the principles discussed in #55 as it is no longer possible to return a validation result of the object without calculating the validation results of its children.

I previously complained about the confusion of separating child validation from object validation but I was completely wrong to do so. Once I started working through a lot of schema algebra in an effort to solve the reuse-with-additionalProperties-false problem, the value of the separation became very apparent.

The correct solution is to more clearly correlate the object and child validation without tying them together. The v4 wording read strangely because it appeared to completely ignore child values, rather than explaining how and why object (and array) validation is independent of child validation, and fit it into the overall framework of many independent validation decisions which are only combined after their evaluation.

@awwright
Copy link
Member

You pasted the wording of the I-D, but can you explain what the problem is, maybe give a test case?

@handrews
Copy link
Contributor Author

@awwright Yeah, that wasn't overly clear, and actually I focused on the wrong thing because of how confusing the wording in v4 actually is :-P Your v5 language is much more clear so part of this is just that I noticed the implication for a change. Anyway...

There's only one part that I (still) think is a problem in the new wording, which is:

validation succeeds only if the instance is an object and...

This means that the schema {"additonalProperties": false} fails to validate 42, which is incorrect. It should not be possible for a non-object to fail additionalProperties validation.

@awwright
Copy link
Member

awwright commented Oct 19, 2016

Yeah, that's definitely not the intended behavior.

It should have gone something like "Validation fails if instance is an object and not..." or "If instance is an object, validation succeeds if..."

And that's probably entirely my fault, I reworded the entire section.

@awwright awwright added the bug label Oct 19, 2016
@awwright awwright added this to the draft-6 milestone Oct 19, 2016
@awwright awwright changed the title New additionalProperties language breaks linearity and independence of parent/child validation additionalProperties behavior for non-object instances Oct 19, 2016
@epoberezkin
Copy link
Member

@handrews It seems resolved, no?

@awwright
Copy link
Member

I still need to fix the language

@handrews
Copy link
Contributor Author

@awwright I submitted a pull request for the language which also makes the style of specifying array items and object properties the same. Previously, properties had been reorganized but items was still glommed together as it was in Draft 04.

@handrews
Copy link
Contributor Author

handrews commented Nov 4, 2016

Resolved by #111

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

No branches or pull requests

3 participants