From 30cd5fa0349f65818fc0b8212d1acda633233698 Mon Sep 17 00:00:00 2001 From: Ben Hutton Date: Wed, 30 Mar 2022 09:46:52 +0100 Subject: [PATCH 1/3] Add CREF about ambiguous behaviour of additionalProperties Fixes #1172 Must see new issue relating to the behaviour of annotation collection for resolving in the next draft. --- jsonschema-core.xml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/jsonschema-core.xml b/jsonschema-core.xml index 00738358..5622cc38 100644 --- a/jsonschema-core.xml +++ b/jsonschema-core.xml @@ -2480,6 +2480,28 @@ checking the names in "properties" and the patterns in "patternProperties" against the instance property set. Implementations that do not support annotation collection MUST do so. + + In defining this option, it seems there is the potential for + ambiguity in the output format. The ambiguity does not affect validation results, + but it does affect the resulting output format. + The ambiguity allows for multiple valid output results depending on whether annotations + are used or a solution that "produces the same effect" as draft-07. It is understood + that annotations from failing schemas are dropped. See + https://github.com/json-schema-org/community/discussions/57 for details. + We discussed the possible solutions at length (https://github.com/json-schema-org/json-schema-spec/issues/1172). + We concluded that we need to re-evaluate the annotation collection system + and its use for validation purposes, but we also agreed not to do so for this release. + This confusion stemmed from the understanding of "evaluated" and the impact on + annotation collection. + It was proposed that we could read a different interpretation (https://github.com/json-schema-org/json-schema-spec/issues/1172#issuecomment-1049686478) + however the previous interpretation is still valid. + The only consensus we could reach for this release was to note all this + in a CREF, which you're now reading, and acknowledge that both results + as described in https://github.com/json-schema-org/community/discussions/57. + If you encounter the same issue regarding ambiguity of the output format, + please do reach out to us, either via emails as per editors of this specification, + or other official JSON Schema channels like our Slack or Twitter. + From ef73bc87165147439ea78b2423ab7384e50a5dca Mon Sep 17 00:00:00 2001 From: Ben Hutton Date: Fri, 8 Apr 2022 15:53:07 +0100 Subject: [PATCH 2/3] Remove detail about ambiguity and reasoning. Link out to pending ADR --- jsonschema-core.xml | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/jsonschema-core.xml b/jsonschema-core.xml index 5622cc38..b4e2efa2 100644 --- a/jsonschema-core.xml +++ b/jsonschema-core.xml @@ -2486,21 +2486,10 @@ but it does affect the resulting output format. The ambiguity allows for multiple valid output results depending on whether annotations are used or a solution that "produces the same effect" as draft-07. It is understood - that annotations from failing schemas are dropped. See - https://github.com/json-schema-org/community/discussions/57 for details. - We discussed the possible solutions at length (https://github.com/json-schema-org/json-schema-spec/issues/1172). - We concluded that we need to re-evaluate the annotation collection system - and its use for validation purposes, but we also agreed not to do so for this release. - This confusion stemmed from the understanding of "evaluated" and the impact on - annotation collection. - It was proposed that we could read a different interpretation (https://github.com/json-schema-org/json-schema-spec/issues/1172#issuecomment-1049686478) - however the previous interpretation is still valid. - The only consensus we could reach for this release was to note all this - in a CREF, which you're now reading, and acknowledge that both results - as described in https://github.com/json-schema-org/community/discussions/57. - If you encounter the same issue regarding ambiguity of the output format, - please do reach out to us, either via emails as per editors of this specification, - or other official JSON Schema channels like our Slack or Twitter. + that annotations from failing schemas are dropped. + See our + [Decision Record](https://github.com/json-schema-org/json-schema-spec/tree/HEAD/adr/2022-04-08-cref-for-ambiguity-and-fix-later-gh-spec-issue-1172.md) + for further details. From 6b8542ca68b382db7fbc15aa928822fd0a95f127 Mon Sep 17 00:00:00 2001 From: Ben Hutton Date: Fri, 8 Apr 2022 16:00:19 +0100 Subject: [PATCH 3/3] Add folder for architectural decision records (ADRs) Add first spec related ADR about the handling of additionalProperties ambiguity in 2019-09 and 2020-12 for the patch release by adding a CREF --- ...iguity-and-fix-later-gh-spec-issue-1172.md | 87 +++++++++++++++++++ adr/README.md | 15 ++++ adr/template.md | 72 +++++++++++++++ 3 files changed, 174 insertions(+) create mode 100644 adr/2022-04-08-cref-for-ambiguity-and-fix-later-gh-spec-issue-1172.md create mode 100644 adr/README.md create mode 100644 adr/template.md diff --git a/adr/2022-04-08-cref-for-ambiguity-and-fix-later-gh-spec-issue-1172.md b/adr/2022-04-08-cref-for-ambiguity-and-fix-later-gh-spec-issue-1172.md new file mode 100644 index 00000000..cd7fb61d --- /dev/null +++ b/adr/2022-04-08-cref-for-ambiguity-and-fix-later-gh-spec-issue-1172.md @@ -0,0 +1,87 @@ +# Acknowledge ambiguity in additionalProperties behaviour and fix after patch release + +* Status: proposed +* Deciders: @relequestual @gregsdennis, @jdesrosiers, @karenetheridge +* Date: 2022-04-08 + +https://github.com/json-schema-org/json-schema-spec/issues/1172 +https://github.com/json-schema-org/community/discussions/57 +https://github.com/json-schema-org/json-schema-spec/pull/1203 + +## Context and Problem Statement + +When we changed the specification to use annotations as the context in which some keywords behave, we included a clause that implementations which didn't use annotations may implement or optimize the processing of `additionalProperties` in another way which produces the same effect as the prior behaviour. +This section created an ambiguity in terms of the resulting output format, but not validation. + +We needed to decide on how to proceed for the patch release of the 2020-12 version of the specification. + +The two above links are to a GitHub Discussion and a GitHub Issue detailing the problems. +Details with an example of the problem can be seen in the Discussion's opening post specifically. + +## Decision Drivers + +* The "patch release" should not change anything functionally +* Annotations as they are, are confusing to users, implementers, and specification editors alike +* Patch release is behind schedule +* There are currently no tests for the output format +* It's hard to see any immediate consensus on changing the annotation based behaviour + +## Considered Options + +* [Leaving it "as is" and do nothing](https://github.com/json-schema-org/community/discussions/57#discussioncomment-1413777) +* [Pick one](https://github.com/json-schema-org/community/discussions/57#discussioncomment-1416683) of the behaviours +* [Revert back to draft-07 behaviour](https://github.com/json-schema-org/community/discussions/57#discussioncomment-1453723) +* [Reinterpret how we understand annotation collection](https://github.com/json-schema-org/json-schema-spec/issues/1172#issuecomment-1049686478) to allow reading annotations within the same schema object regardless of assertion results +* [Acknowledge and accept that two approaches and results are allowable](https://github.com/json-schema-org/community/issues/161#issue-1173742930) +* Redefine annotation collection behaviour and/or how `additionalProperties` works + +## Decision Outcome + +Chosen option: "Acknowledge and accept that two approaches and results are allowable", because + +* Leaving it "as is" will continue to cause confusion +* The change is non-functional which is required for the patch release +* The patch release is behind schedule +* Finding consensus of other solutions proved to be difficult +* There's no test suite for the output format, so it's not easy to see unintended consequences of a functional change +* We need to properly re-evaluate annotation collection and how annotations are used by other keywords + +### Positive Consequences + +* Patch release can move forward +* Validation result is not impacted +* Confusion is at least seen and acknowledged +* Implementations which pick either approach are seen to be compliant + +### Negative Consequences + +* May have an impact for downstream tools which process full output data +* A test suite (not yet developed) which covers this situation needs to allow for multiple valid answers + +## Pros and Cons of the Options + +### Leaving it "as is" and do nothing + +Agree to do nothing and hope for the best. There isn't any downstream tooling yet anyway. + +* Good, because no functional change +* Good, because no impact on downstream tooling +* Bad, because leaves a known ambiguity in the specification + +### Pick one / Revert to draft-07 behaviour / Reinterpret annotation collection + +* Good, because ambiguity is removed +* Good, because not many tools will be effected +* Bad, because it can be seen as a functional change (not really allowed for the patch release) +* Bad, because it can break existing implementations and downstream tools +* Bad, because without a test suite it's hard to see unexpected consequences + +## Links + +* Issue: [Ambiguous behaviour of additionalProperties when invalid](https://github.com/json-schema-org/json-schema-spec/issues/1172) +* Discussion: [The meaning of "additionalProperties" has changed](https://github.com/json-schema-org/community/discussions/57) +* Resolving Pull Request: [Add CREF about ambiguous behaviour of additionalProperties](https://github.com/json-schema-org/json-schema-spec/pull/1203) +* Alternative solution proposal: [Resolve contradictions in the described behaviour of "additionalProperties" and "items"](https://github.com/json-schema-org/json-schema-spec/pull/1154) + +* [Result of discussing](https://github.com/json-schema-org/json-schema-spec/issues/1172#issuecomment-1063962900) on an Open Community Working Meeting call - @jdesrosiers proposed a less controversial and more agreeable solution to add a comment that both are allowable +* [Related GitHub Discussion](https://github.com/json-schema-org/community/discussions/67) on alternative behaviour for `unevaluated*` keywords diff --git a/adr/README.md b/adr/README.md new file mode 100644 index 00000000..331cd6bf --- /dev/null +++ b/adr/README.md @@ -0,0 +1,15 @@ +# Architectural Decision Log + +This log lists the architectural decisions for the JSON Schema specification. + + + +* [ADR-2022-04-08](2022-04-08-cref-for-ambiguity-and-fix-later-gh-spec-issue-1172.md) - Acknowledge ambiguity in additionalProperties behaviour and fix after patch release + + + +You can find the ADR for using ADRs in our [community repo ADR log](https://github.com/json-schema-org/community/tree/HEAD/docs/adr). + +For new ADRs, please use [template.md](template.md) as basis. +More information on MADR is available at . +General information about architectural decision records is available at . diff --git a/adr/template.md b/adr/template.md new file mode 100644 index 00000000..25696bbe --- /dev/null +++ b/adr/template.md @@ -0,0 +1,72 @@ +# [short title of solved problem and solution] + +* Status: [proposed | rejected | accepted | deprecated | … | superseded by [ADR-0005](0005-example.md)] +* Deciders: [list everyone involved in the decision] +* Date: [YYYY-MM-DD when the decision was last updated] + +Technical Story: [description | ticket/issue URL] + +## Context and Problem Statement + +[Describe the context and problem statement, e.g., in free form using two to three sentences. You may want to articulate the problem in form of a question.] + +## Decision Drivers + +* [driver 1, e.g., a force, facing concern, …] +* [driver 2, e.g., a force, facing concern, …] +* … + +## Considered Options + +* [option 1] +* [option 2] +* [option 3] +* … + +## Decision Outcome + +Chosen option: "[option 1]", because [justification. e.g., only option, which meets k.o. criterion decision driver | which resolves force force | … | comes out best (see below)]. + +### Positive Consequences + +* [e.g., improvement of quality attribute satisfaction, follow-up decisions required, …] +* … + +### Negative Consequences + +* [e.g., compromising quality attribute, follow-up decisions required, …] +* … + +## Pros and Cons of the Options + +### [option 1] + +[example | description | pointer to more information | …] + +* Good, because [argument a] +* Good, because [argument b] +* Bad, because [argument c] +* … + +### [option 2] + +[example | description | pointer to more information | …] + +* Good, because [argument a] +* Good, because [argument b] +* Bad, because [argument c] +* … + +### [option 3] + +[example | description | pointer to more information | …] + +* Good, because [argument a] +* Good, because [argument b] +* Bad, because [argument c] +* … + +## Links + +* [Link type] [Link to ADR] +* …