Skip to content

Commit 18d609a

Browse files
committed
docs: Add design spec for validation errors[] array
1 parent 48ec55d commit 18d609a

1 file changed

Lines changed: 189 additions & 0 deletions

File tree

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
# Validation `errors[]` Array — Design
2+
3+
**Date:** 2026-06-08
4+
**Status:** Approved
5+
**Predecessor:** `2026-05-08-combinators-design.md` (Decision 2 deferred multi-error collection); `2026-05-07-openapi-refactor-design.md` §9 Wave 6 (orig #25, "multi-error collection")
6+
7+
## Goal
8+
9+
Make `oneOf` / `anyOf` validation failures actionable. Today a failed combinator produces a single, opaque problem document — e.g. `{"detail":"matched 0 of 2 oneOf branches","pointer":"/offers/0","keyword":"oneOf"}` — that hides the real cause. The per-branch `ValidationError`s are computed inside `checkOneOf` / `checkAnyOf` and then discarded.
10+
11+
This design **retains** the per-branch errors and surfaces them by reshaping the `application/problem+json` document to the RFC 9457 validation idiom: `pointer` and `keyword` move out of the top level into per-entry objects inside an `errors[]` extension array. Each branch failure becomes one entry. Pointers adopt the RFC's `#/…` JSON-Pointer-in-fragment form.
12+
13+
This is a **breaking change** to the wire shape (top-level `pointer` / `keyword` are removed), shipped as a feature. The current document is already RFC 9457-compliant; this change keeps it compliant while aligning with the RFC's own multiple-validation-error example.
14+
15+
### Worked example (the motivating case)
16+
17+
`POST /promotions` where `offers[]` is `oneOf: [UnitOffer, TotalOffer]` and the body sends `"minQuantity": "2"` (a string where the schema types it `number`).
18+
19+
Before:
20+
```json
21+
{ "type":"about:blank", "title":"Bad Request", "status":400,
22+
"detail":"matched 0 of 2 oneOf branches", "pointer":"/offers/0", "keyword":"oneOf" }
23+
```
24+
25+
After: both `UnitOffer` and `TotalOffer` fail fast at the *same* shared leaf (`conditions` precedes `reward` in the body, so neither branch reaches `reward`), producing identical branch errors. After de-duplication (Decision 11) the array collapses to the single actionable cause:
26+
```json
27+
{ "type":"about:blank", "title":"Bad Request", "status":400,
28+
"detail":"matched 0 of 2 oneOf branches",
29+
"errors":[
30+
{"pointer":"#/offers/0/conditions/0/itemSet/minQuantity","keyword":"type","detail":"expected number"}
31+
] }
32+
```
33+
34+
When branches instead fail at *different* places, every distinct cause is listed, deepest first (Decision 10). For `oneOf: [Cat, Dog]` given a body that nearly matches `Cat` (failing deep inside a nested `collar`) but is far from `Dog` (a shallow type error):
35+
```json
36+
{ "type":"about:blank", "title":"Bad Request", "status":400,
37+
"detail":"matched 0 of 2 oneOf branches",
38+
"errors":[
39+
{"pointer":"#/pet/collar/size","keyword":"type","detail":"expected integer"},
40+
{"pointer":"#/pet/bark","keyword":"type","detail":"expected boolean"}
41+
] }
42+
```
43+
`errors[0]` is the deeper `Cat` failure (`/pet/collar/size`, depth 3 — the branch the payload most resembles), ahead of the shallow `Dog` failure (`/pet/bark`, depth 2).
44+
45+
## Decisions
46+
47+
1. **`errors[]` everywhere, not just combinators.** Every validation failure renders an `errors[]` array. A non-combinator failure (e.g. a type mismatch or missing required property) yields a single-entry array; a `oneOf` / `anyOf` failure yields one entry per failed branch. This keeps one consistent shape rather than "an array only when a combinator is involved."
48+
2. **Top-level `pointer` / `keyword` removed.** They live only inside `errors[]` entries now. This is the approved breaking change. Top-level retains the RFC core members plus `detail`.
49+
3. **Top-level `detail` = the failing node's own message.** For a combinator that means the summary (`"matched 0 of 2 oneOf branches"`); for a leaf failure it is that leaf's message (`"expected number"`). The summary is honest because `errors[]` now carries the specifics. We do **not** promote a "best branch" message to the top level (rejected as guesswork; the array already exposes every branch).
50+
4. **Pointer form `#/…`.** Entries express the body location as a JSON Pointer in a URI fragment (`"#/offers/0"`, root = `"#"`), matching the RFC 9457 example (`"#/age"`). Today's plain `/…` form is replaced.
51+
5. **`keyword` retained as a per-entry extension.** Useful to clients; permitted by the RFC even though its illustrative example omits it.
52+
6. **`instance` not added.** It is optional under RFC 9457, so its absence is not a compliance gap, and the default `ExceptionHandler` is `handle(Throwable)` with no request context by design (its javadoc directs context-aware mapping to a `RequestInterceptor`). A request-path `instance` would require breaking that public SPI. Deferred; a `urn:uuid` correlation id is a possible future additive option.
53+
7. **One level of flattening.** `errors[]` entries are the immediate failed branches of the failing combinator (or the single leaf for a non-combinator failure). A branch that is itself a nested combinator contributes one entry carrying its own summary (`detail` = `"matched 0 of N …"`, `pointer` = the nested combinator's location); its sub-branches are not recursively expanded in v1. This bounds output size and matches the fail-fast model — each branch's `check()` already collapses to a single `ValidationError`.
54+
8. **`oneOf` "too many matches" yields an empty `errors[]`.** When `matched ≥ 2`, the problem is ambiguity, not a bad field; there are no failed-branch errors worth listing. The array is empty and therefore omitted, leaving just the summary `detail`. Same for any failure whose node has no sub-errors.
55+
9. **No happy-path cost.** `anyOf` still short-circuits on the first matching branch (no list built). `oneOf` already must evaluate every branch to count matches; the branch-error list is built only on the failure path (`matched != 1`). `ValidationException.CONSTRUCTIONS` stays at zero for valid combinator bodies.
56+
10. **`errors[]` ordered "most likely first."** Entries are sorted by descending failure depth — the number of path segments in the entry's pointer — so the branch that validated the most structure before failing (most likely the branch the client intended) comes first. Ties keep schema order (stable sort). This is a best-effort heuristic, not a guarantee; it is cheap, deterministic, and degrades gracefully (a "wrong" guess still lists a real branch error). Sorting is a presentation concern applied when building entries (see below); the validator keeps `branches` in natural schema order. A single-entry (leaf) or empty (too-many-matches) array is unaffected.
57+
11. **Identical entries are de-duplicated.** When branches share structure they often fail at the exact same leaf (same `pointer` + `keyword` + `detail`) — the motivating promotion payload does. Such exact duplicates collapse to a single entry, keeping the first occurrence (so order from Decision 10 is preserved). Only fully-identical entries collapse; two failures at the same pointer for different reasons both remain. This removes pure noise without losing information.
58+
59+
## Data model
60+
61+
`ValidationError` gains an ordered list of sub-errors (the failed branches). Existing call sites are preserved with a convenience constructor that defaults to no branches.
62+
63+
```java
64+
public record ValidationError(
65+
String pointer, String keyword, String message, Object rejectedValue,
66+
List<ValidationError> branches) {
67+
68+
public ValidationError(String pointer, String keyword, String message, Object rejectedValue) {
69+
this(pointer, keyword, message, rejectedValue, List.of());
70+
}
71+
72+
public ValidationError {
73+
branches = List.copyOf(branches);
74+
}
75+
}
76+
```
77+
78+
- **Leaf error:** `branches` empty. Renders as a single `errors[]` entry built from its own `pointer` / `keyword` / `message`.
79+
- **Combinator error:** `branches` holds each failed branch's `ValidationError`. Renders as one `errors[]` entry per branch.
80+
81+
`ValidationException` is unchanged (its message string still derives from the top-level `pointer` / `keyword` / `message`).
82+
83+
## Validator
84+
85+
`checkOneOf` / `checkAnyOf` in `DefaultValidator` stop discarding branch results.
86+
87+
```java
88+
private Optional<ValidationError> checkAnyOf(Object value, List<Schema> options, String pointer) {
89+
List<ValidationError> failures = new ArrayList<>();
90+
for (Schema o : options) {
91+
Optional<ValidationError> r = check(value, o, pointer);
92+
if (r.isEmpty()) {
93+
return OK; // short-circuit; no list retained on success
94+
}
95+
failures.add(r.get());
96+
}
97+
return Optional.of(new ValidationError(
98+
pointer, "anyOf", "did not match any anyOf branch", value, failures));
99+
}
100+
101+
private Optional<ValidationError> checkOneOf(Object value, List<Schema> options, String pointer) {
102+
int matched = 0;
103+
List<ValidationError> failures = new ArrayList<>();
104+
for (Schema o : options) {
105+
Optional<ValidationError> r = check(value, o, pointer);
106+
if (r.isEmpty()) {
107+
matched++;
108+
} else {
109+
failures.add(r.get());
110+
}
111+
}
112+
if (matched == 1) {
113+
return OK;
114+
}
115+
return Optional.of(new ValidationError(
116+
pointer, "oneOf",
117+
"matched " + matched + " of " + options.size() + " oneOf branches",
118+
value,
119+
matched == 0 ? failures : List.of())); // ≥2 matches → ambiguity, no field-level errors
120+
}
121+
```
122+
123+
`allOf` / `not` / object / array paths are unchanged — they already propagate the real first-failure `ValidationError`, which now simply renders as a single-entry `errors[]`.
124+
125+
## Problem detail and renderer
126+
127+
`ProblemDetail` drops top-level `pointer` / `keyword` and gains an `errors` list. A nested record models each entry.
128+
129+
```java
130+
public record ProblemDetail(
131+
String type, String title, int status, String detail, List<Entry> errors) {
132+
133+
public record Entry(String pointer, String keyword, String detail) {}
134+
135+
public static ProblemDetail forValidation(ValidationError e) {
136+
return new ProblemDetail(DEFAULT_TYPE, "Bad Request", 400, e.message(), entriesOf(e));
137+
}
138+
139+
public static ProblemDetail forBadRequest(BadRequestException e) {
140+
List<Entry> errors = e.pointer()
141+
.map(p -> List.of(new Entry(fragment(p), e.keyword().orElse(null), e.getMessage())))
142+
.orElseGet(List::of);
143+
return new ProblemDetail(DEFAULT_TYPE, titleFor(e.status()), e.status(), e.getMessage(), errors);
144+
}
145+
}
146+
```
147+
148+
- `entriesOf(e)` = one `Entry` per branch when `e.branches()` is non-empty, else a single `Entry` from `e` itself. `fragment(p)` = `"#" + p` (root `""``"#"`).
149+
- When there is more than one entry, sort them by descending pointer depth (segment count, i.e. number of `/` in the pre-fragment pointer), stable on ties (Decision 10), then drop exact `(pointer, keyword, detail)` duplicates keeping the first (Decision 11), so the deepest — most-likely-intended — branch is `errors[0]`.
150+
- A `BadRequestException` with no `pointer` produces an empty `errors` list.
151+
152+
`ProblemDetailRenderer` replaces the two top-level `pointer` / `keyword` appends with an `errors` array writer:
153+
154+
- Emit `"errors":[ … ]` only when the list is non-empty (consistent with the existing null-omission rule).
155+
- Each entry is `{"pointer":"#/…","keyword":"…","detail":"…"}`; omit `keyword` within an entry when null (the `BadRequestException`-without-keyword case). `pointer` and `detail` are always present in an emitted entry.
156+
- Reuse `JsonStrings.appendQuoted` for escaping. No JSON library is introduced; GraalVM-native friendliness is preserved.
157+
158+
`Handlers.defaultExceptionHandler` is unchanged in structure — it still calls `ProblemDetail.forValidation` / `forBadRequest` and renders via `ProblemDetailRenderer`.
159+
160+
## Tests
161+
162+
- **`ValidationError`:** convenience constructor defaults `branches` to empty; canonical constructor copies the list (defensive).
163+
- **`DefaultValidator` (unit):**
164+
- `oneOf` zero matches: top-level message keeps `"matched 0 of N"`; `branches` contains one error per branch with their real pointers/keywords/messages.
165+
- `oneOf` two matches: `branches` is empty (Decision 8).
166+
- `anyOf` no match: `branches` contains every branch's error.
167+
- Happy paths (`oneOf` exactly one, `anyOf` first match): return `OK`, `branches` never built, `ValidationException.CONSTRUCTIONS` unchanged — extend `ValidatorNoThrowOnHappyPathTest`.
168+
- Nested combinator branch surfaces as a single summary entry (Decision 7).
169+
- **`ProblemDetail` / `ProblemDetailRenderer`:** rewrite `ProblemDetailRendererTest` for the new shape — single-entry `errors[]` for a leaf, multi-entry for a combinator, `#/…` pointer form, `keyword` omitted when absent, `errors` omitted when empty. Round-trip the JSON through the test `JsonMapper`.
170+
- **Ordering (Decision 10):** a combinator failure whose branches fail at different depths puts the deepest entry at `errors[0]`; a test asserts the order, including a stable-on-ties case (two branches failing at equal depth keep schema order).
171+
- **De-duplication (Decision 11):** two branches failing at the identical leaf (the promotion-style shared-structure case) collapse to one entry; two failures at the same pointer with different `keyword`/`detail` both remain.
172+
- **`HandlersDefaultExceptionTest`:** update the `containsEntry("pointer", "/email")` / `containsEntry("keyword", …)` assertions to read from `errors[0]` and expect the `#/email` fragment form.
173+
- **`OpenApiServerIT`:** the existing `contains("pointer")` / `contains("keyword")` substring checks still hold (both keys now live inside entries); add one assertion that a `oneOf` body failure returns multiple `errors[]` entries.
174+
- **Integration (the worked example):** extend the test fixture (`src/test/resources/openapi.{yaml,json}`, kept in sync) with a `oneOf` body whose branches fail at different depths, and assert the response lists the deep leaf error. Prefer mutating the existing fixture per the minimize-fixtures convention rather than adding a new file.
175+
176+
## Risk and rollback
177+
178+
- **Breaking wire change.** Any consumer reading top-level `pointer` / `keyword` breaks. Accepted and released as a feature; called out in the changelog/README. The `#/…` pointer form is an additional value change for clients that parsed the old `/…` form.
179+
- **Output size for wide unions.** A `oneOf` with many branches emits one entry per failed branch. Bounded by branch count and one-level flattening (Decision 7); acceptable. If a pathological spec makes this large, a future cap is additive.
180+
- **Scope boundary.** This is *not* full multi-error collection (orig #25): non-combinator failures remain fail-fast (single entry). Whole-request error gathering stays deferred; the `errors[]` shape introduced here is forward-compatible with it.
181+
- **Rollback.** Contained to `ValidationError`, `DefaultValidator` (two methods), `ProblemDetail`, `ProblemDetailRenderer`, and the affected tests. Revert per file.
182+
183+
## Sequencing
184+
185+
Single PR. Suggested commit shape, each verifiable with `mvn -q verify`:
186+
187+
1. `ValidationError` branches field + `DefaultValidator` `checkOneOf` / `checkAnyOf` retaining branch errors; validator unit tests.
188+
2. `ProblemDetail` reshape + `ProblemDetailRenderer` `errors[]` writer; renderer/handler unit tests.
189+
3. Integration fixture + end-to-end test; README/changelog note on the breaking shape.

0 commit comments

Comments
 (0)