Skip to content

Commit 6b8542c

Browse files
RelequestualRelequestual
committed
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
1 parent ef73bc8 commit 6b8542c

File tree

3 files changed

+174
-0
lines changed

3 files changed

+174
-0
lines changed

‎adr/2022-04-08-cref-for-ambiguity-and-fix-later-gh-spec-issue-1172.md

+87
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# Acknowledge ambiguity in additionalProperties behaviour and fix after patch release
2+
3+
* Status: proposed
4+
* Deciders: @relequestual @gregsdennis, @jdesrosiers, @karenetheridge
5+
* Date: 2022-04-08
6+
7+
https://github.com/json-schema-org/json-schema-spec/issues/1172
8+
https://github.com/json-schema-org/community/discussions/57
9+
https://github.com/json-schema-org/json-schema-spec/pull/1203
10+
11+
## Context and Problem Statement
12+
13+
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.
14+
This section created an ambiguity in terms of the resulting output format, but not validation.
15+
16+
We needed to decide on how to proceed for the patch release of the 2020-12 version of the specification.
17+
18+
The two above links are to a GitHub Discussion and a GitHub Issue detailing the problems.
19+
Details with an example of the problem can be seen in the Discussion's opening post specifically.
20+
21+
## Decision Drivers <!-- optional -->
22+
23+
* The "patch release" should not change anything functionally
24+
* Annotations as they are, are confusing to users, implementers, and specification editors alike
25+
* Patch release is behind schedule
26+
* There are currently no tests for the output format
27+
* It's hard to see any immediate consensus on changing the annotation based behaviour
28+
29+
## Considered Options
30+
31+
* [Leaving it "as is" and do nothing](https://github.com/json-schema-org/community/discussions/57#discussioncomment-1413777)
32+
* [Pick one](https://github.com/json-schema-org/community/discussions/57#discussioncomment-1416683) of the behaviours
33+
* [Revert back to draft-07 behaviour](https://github.com/json-schema-org/community/discussions/57#discussioncomment-1453723)
34+
* [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
35+
* [Acknowledge and accept that two approaches and results are allowable](https://github.com/json-schema-org/community/issues/161#issue-1173742930)
36+
* Redefine annotation collection behaviour and/or how `additionalProperties` works
37+
38+
## Decision Outcome
39+
40+
Chosen option: "Acknowledge and accept that two approaches and results are allowable", because
41+
42+
* Leaving it "as is" will continue to cause confusion
43+
* The change is non-functional which is required for the patch release
44+
* The patch release is behind schedule
45+
* Finding consensus of other solutions proved to be difficult
46+
* There's no test suite for the output format, so it's not easy to see unintended consequences of a functional change
47+
* We need to properly re-evaluate annotation collection and how annotations are used by other keywords
48+
49+
### Positive Consequences
50+
51+
* Patch release can move forward
52+
* Validation result is not impacted
53+
* Confusion is at least seen and acknowledged
54+
* Implementations which pick either approach are seen to be compliant
55+
56+
### Negative Consequences
57+
58+
* May have an impact for downstream tools which process full output data
59+
* A test suite (not yet developed) which covers this situation needs to allow for multiple valid answers
60+
61+
## Pros and Cons of the Options
62+
63+
### Leaving it "as is" and do nothing
64+
65+
Agree to do nothing and hope for the best. There isn't any downstream tooling yet anyway.
66+
67+
* Good, because no functional change
68+
* Good, because no impact on downstream tooling
69+
* Bad, because leaves a known ambiguity in the specification
70+
71+
### Pick one / Revert to draft-07 behaviour / Reinterpret annotation collection
72+
73+
* Good, because ambiguity is removed
74+
* Good, because not many tools will be effected
75+
* Bad, because it can be seen as a functional change (not really allowed for the patch release)
76+
* Bad, because it can break existing implementations and downstream tools
77+
* Bad, because without a test suite it's hard to see unexpected consequences
78+
79+
## Links
80+
81+
* Issue: [Ambiguous behaviour of additionalProperties when invalid](https://github.com/json-schema-org/json-schema-spec/issues/1172)
82+
* Discussion: [The meaning of "additionalProperties" has changed](https://github.com/json-schema-org/community/discussions/57)
83+
* Resolving Pull Request: [Add CREF about ambiguous behaviour of additionalProperties](https://github.com/json-schema-org/json-schema-spec/pull/1203)
84+
* Alternative solution proposal: [Resolve contradictions in the described behaviour of "additionalProperties" and "items"](https://github.com/json-schema-org/json-schema-spec/pull/1154)
85+
86+
* [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
87+
* [Related GitHub Discussion](https://github.com/json-schema-org/community/discussions/67) on alternative behaviour for `unevaluated*` keywords

‎adr/README.md

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# Architectural Decision Log
2+
3+
This log lists the architectural decisions for the JSON Schema specification.
4+
5+
<!-- adrlog -- Regenerate the content by using "adr-log -i". You can install it via "npm install -g adr-log" -->
6+
7+
* [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
8+
9+
<!-- adrlogstop -->
10+
11+
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).
12+
13+
For new ADRs, please use [template.md](template.md) as basis.
14+
More information on MADR is available at <https://adr.github.io/madr/>.
15+
General information about architectural decision records is available at <https://adr.github.io/>.

‎adr/template.md

+72
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# [short title of solved problem and solution]
2+
3+
* Status: [proposed | rejected | accepted | deprecated | … | superseded by [ADR-0005](0005-example.md)] <!-- optional -->
4+
* Deciders: [list everyone involved in the decision] <!-- optional -->
5+
* Date: [YYYY-MM-DD when the decision was last updated] <!-- optional -->
6+
7+
Technical Story: [description | ticket/issue URL] <!-- optional -->
8+
9+
## Context and Problem Statement
10+
11+
[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.]
12+
13+
## Decision Drivers <!-- optional -->
14+
15+
* [driver 1, e.g., a force, facing concern, …]
16+
* [driver 2, e.g., a force, facing concern, …]
17+
* … <!-- numbers of drivers can vary -->
18+
19+
## Considered Options
20+
21+
* [option 1]
22+
* [option 2]
23+
* [option 3]
24+
* … <!-- numbers of options can vary -->
25+
26+
## Decision Outcome
27+
28+
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)].
29+
30+
### Positive Consequences <!-- optional -->
31+
32+
* [e.g., improvement of quality attribute satisfaction, follow-up decisions required, …]
33+
* …
34+
35+
### Negative Consequences <!-- optional -->
36+
37+
* [e.g., compromising quality attribute, follow-up decisions required, …]
38+
* …
39+
40+
## Pros and Cons of the Options <!-- optional -->
41+
42+
### [option 1]
43+
44+
[example | description | pointer to more information | …] <!-- optional -->
45+
46+
* Good, because [argument a]
47+
* Good, because [argument b]
48+
* Bad, because [argument c]
49+
* … <!-- numbers of pros and cons can vary -->
50+
51+
### [option 2]
52+
53+
[example | description | pointer to more information | …] <!-- optional -->
54+
55+
* Good, because [argument a]
56+
* Good, because [argument b]
57+
* Bad, because [argument c]
58+
* … <!-- numbers of pros and cons can vary -->
59+
60+
### [option 3]
61+
62+
[example | description | pointer to more information | …] <!-- optional -->
63+
64+
* Good, because [argument a]
65+
* Good, because [argument b]
66+
* Bad, because [argument c]
67+
* … <!-- numbers of pros and cons can vary -->
68+
69+
## Links <!-- optional -->
70+
71+
* [Link type] [Link to ADR] <!-- example: Refined by [ADR-0005](0005-example.md) -->
72+
* … <!-- numbers of links can vary -->

0 commit comments

Comments
 (0)