Skip to content

Commit c566004

Browse files
gregsdennisgregsdennis
committed
add propertyDependencies adr
1 parent 25aafc7 commit c566004

File tree

1 file changed

+314
-0
lines changed

1 file changed

+314
-0
lines changed

‎proposals/propertyDependencies-adr.md

+314
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,314 @@
1+
# [short title of solved problem and solution]
2+
3+
* Status: proposed
4+
* Deciders: @gregsdennis, @jdesrosiers, @relequestual
5+
* Date: 2022-04-07
6+
7+
Technical Story:
8+
9+
- Issue discussing feature - https://github.com/json-schema-org/json-schema-spec/issues/1082
10+
- PR to add to the spec - https://github.com/json-schema-org/json-schema-spec/issues/1082
11+
- ADR to extract from the spec and use feature life cycle - https://github.com/json-schema-org/json-schema-spec/pull/1505
12+
13+
## Context and Problem Statement
14+
15+
A common need in JSON Schema is to select between one schema or another to
16+
validate an instance based on the value of some property in the JSON instance.
17+
There are a several patterns people use to accomplish this, but they all have
18+
significant [problems](#problems).
19+
20+
OpenAPI solves this problem with the `discriminator` keyword. However, their
21+
approach is more oriented toward code generation concerns, is poorly specified
22+
when it comes to validation, and is coupled to OpenAPI concepts that don't exist
23+
is JSON Schema. Therefore, it's necessary to define something new rather than
24+
adopt or redefine `discriminator`.
25+
26+
## Decision Drivers
27+
28+
- Ease of use
29+
- Readability
30+
- Coverage of most common use cases
31+
- Coverage of all use cases
32+
- Ease of implementation
33+
34+
## Considered Options
35+
36+
All of the following options have the same validation result as the following schema.
37+
38+
```json
39+
{
40+
"if": {
41+
"properties": {
42+
"foo": { "const": "aaa" }
43+
},
44+
"required": ["foo"]
45+
},
46+
"then": { "$ref": "#/$defs/foo-aaa" }
47+
}
48+
```
49+
50+
51+
### Option 1
52+
53+
The `dependentSchemas` keyword is very close to what is needed except it checks
54+
for the presence of a property rather than it's value. This option builds on
55+
that concept to solve this problem.
56+
57+
```json
58+
{
59+
"propertyDependencies": {
60+
"foo": {
61+
"aaa": { "$ref": "#/$defs/foo-aaa" }
62+
}
63+
}
64+
}
65+
```
66+
67+
### Option 2
68+
69+
This version uses an array of objects. Each object is a collection of the
70+
variables needed to express a property dependency. This doesn't fit the style of
71+
JSON Schema. There aren't any keywords remotely like this. It's also still too
72+
verbose. It's a little more intuitive than `if`/`then` and definitely less error
73+
prone.
74+
75+
```jsonschema
76+
{
77+
"propertyDependencies": [
78+
{
79+
"propertyName": "foo",
80+
"propertySchema": { "const": "aaa" },
81+
"apply": { "$ref": "#/$defs/foo-aaa" }
82+
},
83+
{
84+
"propertyName": "foo",
85+
"propertySchema": { "const": "bbb" },
86+
"apply": { "$ref": "#/$defs/foo-bbb" }
87+
}
88+
]
89+
}
90+
```
91+
92+
### Option 3
93+
94+
A slight variation on that example is to make it a map of keyword to dependency
95+
object. It's still too verbose.
96+
97+
```jsonschema
98+
{
99+
"propertyDependencies": {
100+
"foo": [
101+
{
102+
"propertySchema": { "const": "aaa" },
103+
"apply": { "$ref": "#/$defs/foo-aaa" }
104+
},
105+
{
106+
"propertySchema": { "const": "bbb" },
107+
"apply": { "$ref": "#/$defs/foo-bbb" }
108+
}
109+
]
110+
}
111+
}
112+
```
113+
114+
### Option 4
115+
116+
This one is a little more consistent with the JSON Schema style (poor keyword
117+
naming aside), but otherwise has all the same problems as the other examples.
118+
119+
```jsonschema
120+
{
121+
"allOf": [
122+
{
123+
"propertyDependencyName": "foo",
124+
"propertyDependencySchema": { "const": "aaa" },
125+
"propertyDependencyApply": { "$ref": "#/$defs/foo-aaa" }
126+
},
127+
{
128+
"propertyDependencyName": "foo",
129+
"propertyDependencySchema": { "const": "bbb" },
130+
"propertyDependencyApply": { "$ref": "#/$defs/foo-bbb" }
131+
}
132+
]
133+
}
134+
```
135+
136+
### Option 4
137+
138+
This one is a variation of `if` that combines `if`, `properties`, and `required`
139+
to reduce boilerplate. It's also essentially a variation of the previous example
140+
with better names. This avoids to error proneness problem, but it's still too
141+
verbose.
142+
143+
```jsonschema
144+
{
145+
"allOf": [
146+
{
147+
"ifProperties": {
148+
"foo": { "const": "aaa" }
149+
},
150+
"then": { "$ref": "#/$defs/foo-aaa" }
151+
},
152+
{
153+
"ifProperties": {
154+
"foo": { "const": "bbb" }
155+
},
156+
"then": { "$ref": "#/$defs/foo-aaa" }
157+
}
158+
]
159+
}
160+
```
161+
162+
### Option 6
163+
164+
All of the previous alternatives use a schema as the discriminator. This
165+
alternative is a little less powerful in that it can only match on exact values,
166+
but it successfully addresses the problems we're concerned about with the
167+
current approaches. The only issue with this alternative is that it's not as
168+
intuitive as the chosen solution.
169+
170+
```jsonschema
171+
{
172+
"propertyDependencies": {
173+
"foo": [
174+
["aaa", { "$ref": "#/$defs/foo-aaa" }],
175+
["bbb", { "$ref": "#/$defs/foo-bbb" }]
176+
]
177+
}
178+
}
179+
```
180+
181+
182+
## Decision Outcome
183+
184+
Option 1 was chosen because it satisfies the most common use cases while being
185+
sufficiently readable and easy to implement, even though it does not satisfy
186+
_all_ use cases, such as those where the property value is not a string. As
187+
these cases are significantly less common, the requirement to support all use
188+
cases carried a lower priority.
189+
190+
### Positive Consequences <!-- optional -->
191+
192+
- Some level of built-in support for a `discriminator`-like keyword that aligns
193+
with the existing operation of JSON Schema.
194+
195+
### Negative Consequences <!-- optional -->
196+
197+
- Properties with non-string values cannot be supported using this keyword and
198+
the `allOf`-`if`-`then` pattern must still be used.
199+
200+
## Pros and Cons of the Options <!-- optional -->
201+
202+
### [option 1]
203+
204+
[example | description | pointer to more information | …] <!-- optional -->
205+
206+
* Good, because it handle the most common use case: string property values
207+
* Good, because all property values are grouped together
208+
* Good, because it's less verbose
209+
* Bad, because it doesn't handle non-string property values
210+
211+
### [option 2]
212+
213+
* Good, because it supports all use cases
214+
* Bad, because properties are not naturally grouped together
215+
* Bad, because it's quite verbose
216+
* Bad, because we have no precedent for a keyword which explicitly defines its own properties. This would be new operational functionality, which we try to avoid if we can.
217+
218+
### [option 3]
219+
220+
* Good, because it supports all use cases
221+
* Good, because all property values are grouped together
222+
* Bad, because it's quite verbose
223+
* Bad, because we have no precedent for a keyword which explicitly defines its own properties. This would be new operational functionality, which we try to avoid if we can.
224+
225+
### [option 4]
226+
227+
* Good, because it supports all use cases
228+
* Bad, because properties are not naturally grouped together
229+
* Bad, because it's very verbose
230+
* Bad, because it introduces a lot of inter-keyword dependencies, which we'd have to exhaustively define
231+
232+
### [option 5]
233+
234+
* Good, because it supports all use cases
235+
* Good, because it's a familiar syntax
236+
* Bad, because properties are not naturally grouped together
237+
* Bad, because it's very verbose
238+
* Bad, because `ifProperties` is very niche. Will this spawn a new series of `if*` keywords? How would it interact with `if`?
239+
240+
### [option 6]
241+
242+
* Good, because it supports all use cases
243+
* Bad, because it's an unintuitive syntax and easy to get wrong
244+
* Bad, because properties are not naturally grouped together
245+
246+
## Links <!-- optional -->
247+
248+
* [Link type] [Link to ADR] <!-- example: Refined by [ADR-0005](0005-example.md) -->
249+
* … <!-- numbers of links can vary -->
250+
251+
252+
## [Appendix] Problems With Existing Patterns {#problems}
253+
254+
### `oneOf`/`anyOf`
255+
256+
The pattern of using `oneOf` to describe a choice between two schemas has become
257+
ubiquitous.
258+
259+
```jsonschema
260+
{
261+
"oneOf": [
262+
{ "$ref": "#/$defs/aaa" },
263+
{ "$ref": "#/$defs/bbb" }
264+
]
265+
}
266+
```
267+
268+
However, this pattern has several shortcomings. The main problem is that it
269+
tends to produce confusing error messages. Some implementations employ
270+
heuristics to guess the user's intent and provide better messaging, but that's
271+
not wide-spread or consistent behavior, nor is it expected or required from
272+
implementations.
273+
274+
This pattern is also inefficient. Generally, there is a single value in the
275+
object that determines which alternative to chose, but the `oneOf` pattern has
276+
no way to specify what that value is and therefore needs to evaluate the entire
277+
schema. This is made worse in that every alternative needs to be fully validated
278+
to ensure that only one of the alternative passes and all the others fail. This
279+
last problem can be avoided by using `anyOf` instead, but that pattern is much
280+
less used.
281+
282+
### `if`/`then`
283+
284+
We can describe this kind of constraint more efficiently and with with better
285+
error messaging by using `if`/`then`. This allows the user to explicitly specify
286+
the constraint to be used to select which alternative the schema should be used
287+
to validate the schema. However, this pattern has problems of it's own. It's
288+
verbose, error prone, and not particularly intuitive, which leads most people to
289+
avoid it.
290+
291+
```jsonschema
292+
{
293+
"allOf": [
294+
{
295+
"if": {
296+
"properties": {
297+
"foo": { "const": "aaa" }
298+
},
299+
"required": ["foo"]
300+
},
301+
"then": { "$ref": "#/$defs/foo-aaa" }
302+
},
303+
{
304+
"if": {
305+
"properties": {
306+
"foo": { "const": "bbb" }
307+
},
308+
"required": ["foo"]
309+
},
310+
"then": { "$ref": "#/$defs/foo-bbb" }
311+
}
312+
]
313+
}
314+
```

0 commit comments

Comments
 (0)