From a0f2822ebb86fdf2f4824e00f2fcccb746c89d18 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 13:30:16 +0200 Subject: [PATCH 01/14] docs: Add combinators design spec Specifies validator and parser changes to implement OpenAPI 3.1 allOf / anyOf / oneOf / not, including composition with sibling base assertions via implicit AllOf. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../specs/2026-05-08-combinators-design.md | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-08-combinators-design.md diff --git a/docs/superpowers/specs/2026-05-08-combinators-design.md b/docs/superpowers/specs/2026-05-08-combinators-design.md new file mode 100644 index 0000000..4380e60 --- /dev/null +++ b/docs/superpowers/specs/2026-05-08-combinators-design.md @@ -0,0 +1,110 @@ +# OpenAPI 3.1 Combinators — Design + +**Date:** 2026-05-08 +**Status:** Approved +**Predecessor:** `2026-05-07-openapi-refactor-design.md` (Section 9, Wave 1 #3 and partial #4) + +## Goal + +Implement runtime validation for the four JSON Schema 2020-12 combinator keywords used in OpenAPI 3.1: `allOf`, `anyOf`, `oneOf`, `not`. The schema records, sealed interface, and parser already produce the corresponding `AllOfSchema` / `AnyOfSchema` / `OneOfSchema` / `NotSchema` records; `DefaultValidator` currently throws `UnsupportedOperationException` for each. Replace those branches with real validation, and update the parser so combinators can co-exist with sibling base assertions (`type`, `properties`, `required`, etc.) per JSON Schema semantics. + +## Decisions + +1. **Composition with sibling assertions.** When a schema map contains both a combinator keyword and base assertions, the parser emits an implicit `AllOfSchema` whose elements are the parsed base schema and each combinator. This matches JSON Schema 2020-12's "all keywords at one level are conjoined" rule. +2. **Error reporting.** Fail-fast with a single `ValidationError`. For `oneOf` / `anyOf` / `not`, emit a generic message keyed by the combinator name. For `allOf`, propagate the first failing branch's `ValidationError` unchanged. +3. **Evaluation strategy.** Internal evaluation is dictated by semantics — `anyOf` short-circuits on first match, `oneOf` must evaluate every branch to count matches, `allOf` short-circuits on first failure, `not` runs its inner schema once. Branch failures are captured by catching `ValidationException`; this is a deliberate control-flow use of exceptions, not an error-handling pattern. +4. **Out of scope (deferred).** Schema booleans (`true` / `false` as a bare schema), `discriminator`, and multi-error collection. These remain on the gap inventory (Wave 1 #4 partial, Wave 6 #25, #26). + +## Validator + +Replace the four UOE branches in `DefaultValidator.validate(...)`: + +```java +case AllOfSchema(List parts) -> { + for (Schema p : parts) validate(value, p, pointer); +} + +case AnyOfSchema(List options) -> { + for (Schema o : options) { + try { validate(value, o, pointer); return; } + catch (ValidationException ignored) { /* try next */ } + } + fail(pointer, "anyOf", "did not match any anyOf branch", value); +} + +case OneOfSchema(List options) -> { + int matched = 0; + for (Schema o : options) { + try { validate(value, o, pointer); matched++; } + catch (ValidationException ignored) { /* count misses */ } + } + if (matched != 1) { + fail(pointer, "oneOf", + "matched " + matched + " of " + options.size() + " oneOf branches", value); + } +} + +case NotSchema(Schema inner) -> { + try { validate(value, inner, pointer); } + catch (ValidationException expected) { return; } + fail(pointer, "not", "value matched 'not' schema", value); +} +``` + +The pointer for combinator failures is the schema's pointer (the spot where the combinator is declared) — same convention as other keyword failures. Sub-branch errors do not carry through; the outer message is intentionally generic. Multi-error collection (Wave 6 #25) will revisit this. + +## Parser + +Currently `SchemaParser.parse(...)` dispatches in priority order — `$ref` → combinator → `const`/`enum` → `type` → permissive object — and emits exactly one record. The change: when a combinator coexists with sibling base assertions, build an `AllOfSchema` whose first element is the parsed base and whose remaining elements are the combinators. + +Pseudocode: + +```java +if (raw has $ref) return RefSchema(...); + +List assertions = new ArrayList<>(); +Schema base = parseBaseIfPresent(raw); // existing const/enum/type/object dispatch; null if absent +if (base != null) assertions.add(base); + +if (raw has allOf) assertions.addAll(parseAll(raw.allOf)); // flatten one level +if (raw has anyOf) assertions.add(new AnyOfSchema(parseAll(raw.anyOf))); +if (raw has oneOf) assertions.add(new OneOfSchema(parseAll(raw.oneOf))); +if (raw has not) assertions.add(new NotSchema(parse(raw.not))); + +return switch (assertions.size()) { + case 0 -> permissiveObject; + case 1 -> assertions.get(0); + default -> new AllOfSchema(List.copyOf(assertions)); +}; +``` + +`allOf` branches flatten directly into the outer assertions list since `AllOf(AllOf(a, b), c)` is semantically equal to `AllOf(a, b, c)`. `anyOf` and `oneOf` are not flattened because their semantics differ from `AllOf`. `parseBaseIfPresent` returns `null` when the schema map has no base-assertion keywords (`type`, `const`, `enum`, or any of the object/array shape keywords) so a vacuous permissive-object isn't injected. + +`$ref` continues to be parsed solo. JSON Schema 2020-12 allows siblings to `$ref`, but that interaction is a separate gap; not addressed here. + +## Tests + +- **Parser, combinator alone:** existing tests for `OneOfSchema` / `AnyOfSchema` / `AllOfSchema` / `NotSchema` round-trip remain green. +- **Parser, combinator + sibling `type`:** new tests asserting the result is `AllOfSchema([base, combinator])`. One per combinator. Covers the primary correctness goal of decision (1). +- **Parser, multiple combinators in one schema:** e.g. `{anyOf: [...], not: ...}` → `AllOfSchema([AnyOfSchema(...), NotSchema(...)])`. +- **Validator, happy path:** one passing test per combinator (`oneOf` exactly one match, `anyOf` first branch matches, `allOf` all branches pass, `not` inner fails so outer passes). +- **Validator, failure path:** `oneOf` zero matches and two-plus matches (separate tests, asserting the count in the message); `anyOf` no match; `allOf` second branch fails (asserts the inner pointer/message propagates); `not` inner passes. +- **Validator, combinator + sibling type:** one polymorphic-body test driven by the parser composition path. +- **Integration:** extend the test fixture (`src/test/resources/openapi.{yaml,json}`) with one operation whose request body uses `oneOf` for a discriminated-style polymorphic shape (no `discriminator` keyword — that's deferred). Add a test handler and assert success on a valid body and 400 on a body that matches zero or two branches. The fixture twins (yaml & json) stay in sync per the existing memory entry. +- **Performance:** k6 smoke run against the example launcher confirms no regression. The combinator dispatch adds a single `case` per request, the catch-blocks only run on combinator paths, and the test schema's existing routes don't use combinators — so the broad k6 numbers should be unchanged. + +## Risk and rollback + +- **Performance of try/catch in `oneOf`/`anyOf`:** for branches that fail validation, we throw and catch a `ValidationException`. This is fine for typical request volumes but is more allocation-heavy than a boolean predicate. Risk is bounded — combinators are not on the hot path for our existing test fixture. If a future spec uses combinators in tight loops, an internal `boolean tryValidate(...)` overload is an additive optimization. +- **Parser regression:** the dispatch change touches every schema parse. Mitigation: existing parser tests (combinator alone, primitives, refs) keep running and pin behaviour. +- **Rollback:** the change is contained in `SchemaParser` and `DefaultValidator`. Reverting is one revert per file. + +## Sequencing + +Single PR. Suggested commit shape: + +1. Validator: replace UOE branches with real validation; add validator unit tests. +2. Parser: composition path; add parser unit tests for combinator + sibling. +3. Integration: fixture extension + end-to-end test. + +Each commit verifiable with `mvn -q verify`. From c4f3a8c4d8bf93972dc24e773cefc1dffac9a4a0 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 14:21:59 +0200 Subject: [PATCH 02/14] docs: Add combinators implementation plan Three-task plan: validator branches, parser composition, and integration test for the polymorphic-body case. Mirrors the combinators design spec. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-08-combinators-implementation.md | 775 ++++++++++++++++++ 1 file changed, 775 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-08-combinators-implementation.md diff --git a/docs/superpowers/plans/2026-05-08-combinators-implementation.md b/docs/superpowers/plans/2026-05-08-combinators-implementation.md new file mode 100644 index 0000000..f2ed7ff --- /dev/null +++ b/docs/superpowers/plans/2026-05-08-combinators-implementation.md @@ -0,0 +1,775 @@ +# Combinators Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Implement runtime validation for OpenAPI 3.1 combinators (`allOf`, `anyOf`, `oneOf`, `not`) and let combinators co-exist with sibling base assertions (`type`, `properties`, etc.) per JSON Schema 2020-12. + +**Architecture:** `DefaultValidator` gains four real branches replacing `UnsupportedOperationException`. `SchemaParser` switches from "first matching keyword wins" to "parse base assertions plus each combinator and wrap in implicit `AllOfSchema` when multiple are present". The existing `AllOfSchema` / `AnyOfSchema` / `OneOfSchema` / `NotSchema` records are reused; no new schema kinds. + +**Tech Stack:** Java 25, JUnit 5 + AssertJ + Mockito, Maven Surefire/Failsafe, no new dependencies. + +**Spec:** `docs/superpowers/specs/2026-05-08-combinators-design.md` + +**Branch:** `feat/combinators` (already checked out). + +--- + +## Task 1: Validator branches + unit tests + +**Files:** +- Modify: `src/main/java/com/retailsvc/http/validate/DefaultValidator.java` (lines 64–67 — the four UOE branches) +- Modify: `src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java` (replace the `combinatorThrowsUnsupported` test; add new combinator tests) + +### Step 1.1: Write failing validator tests + +- [ ] **Step 1: Add tests for combinator validation** + +Replace the existing `combinatorThrowsUnsupported` test in `src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java` with the following block (delete the old test, append the new ones at the end of the class). Keep the existing imports and add: `AllOfSchema`, `AnyOfSchema`, `NotSchema`, `StringSchema`. + +```java +// Imports to add at the top of the file: +// import com.retailsvc.http.spec.schema.AllOfSchema; +// import com.retailsvc.http.spec.schema.AnyOfSchema; +// import com.retailsvc.http.spec.schema.NotSchema; +// import com.retailsvc.http.spec.schema.StringSchema; + +private StringSchema stringSchema(Integer min, Integer max) { + return new StringSchema(Set.of(TypeName.STRING), null, min, max, null, null); +} + +@Test +void allOfPassesWhenAllBranchesPass() { + var schema = + new AllOfSchema(List.of(stringSchema(1, null), stringSchema(null, 10))); + v.validate("hello", schema, "/v"); +} + +@Test +void allOfPropagatesFirstFailingBranch() { + var schema = + new AllOfSchema(List.of(stringSchema(1, null), stringSchema(null, 3))); + assertThatThrownBy(() -> v.validate("hello", schema, "/v")) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("maxLength"); +} + +@Test +void anyOfPassesWhenOneBranchPasses() { + var schema = + new AnyOfSchema(List.of(stringSchema(100, null), stringSchema(null, 10))); + v.validate("hello", schema, "/v"); +} + +@Test +void anyOfFailsWhenNoBranchMatches() { + var schema = + new AnyOfSchema(List.of(stringSchema(100, null), stringSchema(null, 2))); + assertThatThrownBy(() -> v.validate("hello", schema, "/v")) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("anyOf"); +} + +@Test +void oneOfPassesWhenExactlyOneBranchMatches() { + // value "hello" — len 5. branch[0] requires min 100 (fails), branch[1] max 10 (passes). + var schema = + new OneOfSchema(List.of(stringSchema(100, null), stringSchema(null, 10))); + v.validate("hello", schema, "/v"); +} + +@Test +void oneOfFailsWhenZeroBranchesMatch() { + var schema = + new OneOfSchema(List.of(stringSchema(100, null), stringSchema(null, 2))); + assertThatThrownBy(() -> v.validate("hello", schema, "/v")) + .isInstanceOf(ValidationException.class) + .satisfies( + t -> { + var err = ((ValidationException) t).error(); + org.assertj.core.api.Assertions.assertThat(err.keyword()).isEqualTo("oneOf"); + org.assertj.core.api.Assertions.assertThat(err.message()).contains("matched 0 of 2"); + }); +} + +@Test +void oneOfFailsWhenTwoBranchesMatch() { + // value "hello" — both branches accept. + var schema = + new OneOfSchema(List.of(stringSchema(null, 10), stringSchema(1, null))); + assertThatThrownBy(() -> v.validate("hello", schema, "/v")) + .isInstanceOf(ValidationException.class) + .satisfies( + t -> { + var err = ((ValidationException) t).error(); + org.assertj.core.api.Assertions.assertThat(err.keyword()).isEqualTo("oneOf"); + org.assertj.core.api.Assertions.assertThat(err.message()).contains("matched 2 of 2"); + }); +} + +@Test +void notPassesWhenInnerFails() { + var schema = new NotSchema(stringSchema(100, null)); + v.validate("hello", schema, "/v"); +} + +@Test +void notFailsWhenInnerPasses() { + var schema = new NotSchema(stringSchema(null, 10)); + assertThatThrownBy(() -> v.validate("hello", schema, "/v")) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("not"); +} +``` + +- [ ] **Step 2: Run the failing tests** + +Run: `mvn -q test -Dtest=DefaultValidatorDispatchTest` + +Expected: 9 new tests fail with `UnsupportedOperationException` (and the old `combinatorThrowsUnsupported` is deleted, so it does not run). + +### Step 1.2: Replace the four UOE branches + +- [ ] **Step 3: Edit `DefaultValidator.validate(...)`** + +Open `src/main/java/com/retailsvc/http/validate/DefaultValidator.java`. Replace these four lines: + +```java +case OneOfSchema _ -> throw new UnsupportedOperationException("oneOf not yet supported"); +case AnyOfSchema _ -> throw new UnsupportedOperationException("anyOf not yet supported"); +case AllOfSchema _ -> throw new UnsupportedOperationException("allOf not yet supported"); +case NotSchema _ -> throw new UnsupportedOperationException("not not yet supported"); +``` + +With: + +```java +case AllOfSchema(List parts) -> { + for (Schema p : parts) { + validate(value, p, pointer); + } +} +case AnyOfSchema(List options) -> validateAnyOf(value, options, pointer); +case OneOfSchema(List options) -> validateOneOf(value, options, pointer); +case NotSchema(Schema inner) -> validateNot(value, inner, pointer); +``` + +Then add the three private helper methods at the bottom of the class (before the final closing brace, after `validateArray` or wherever the other private helpers live): + +```java +private void validateAnyOf(Object value, List options, String pointer) { + for (Schema o : options) { + try { + validate(value, o, pointer); + return; + } catch (ValidationException ignored) { + // try next branch + } + } + fail(pointer, "anyOf", "did not match any anyOf branch", value); +} + +private void validateOneOf(Object value, List options, String pointer) { + int matched = 0; + for (Schema o : options) { + try { + validate(value, o, pointer); + matched++; + } catch (ValidationException ignored) { + // count misses + } + } + if (matched != 1) { + fail( + pointer, + "oneOf", + "matched " + matched + " of " + options.size() + " oneOf branches", + value); + } +} + +private void validateNot(Object value, Schema inner, String pointer) { + try { + validate(value, inner, pointer); + } catch (ValidationException expected) { + return; + } + fail(pointer, "not", "value matched 'not' schema", value); +} +``` + +- [ ] **Step 4: Run the unit tests** + +Run: `mvn -q test -Dtest=DefaultValidatorDispatchTest` + +Expected: all tests in the class pass (the original 4 + the 9 new ones). + +- [ ] **Step 5: Run the full unit test suite** + +Run: `mvn -q test` + +Expected: BUILD SUCCESS, 119 + 9 = 128 tests pass (or whatever count matches; no failures). + +- [ ] **Step 6: Commit** + +```bash +git add src/main/java/com/retailsvc/http/validate/DefaultValidator.java \ + src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java +git commit -m "$(cat <<'EOF' +feat: Implement combinator validation in DefaultValidator + +Replace the four UnsupportedOperationException branches for allOf / +anyOf / oneOf / not with real validation. allOf propagates the first +failing branch; anyOf short-circuits on first match; oneOf evaluates +all branches and asserts exactly one matches; not inverts the inner +result. Adds 9 unit tests covering pass/fail per combinator. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 2: Parser composition + parser tests + +**Files:** +- Modify: `src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java` +- Modify: `src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java` + +### Step 2.1: Add failing parser tests + +- [ ] **Step 1: Add tests covering the composition path** + +Append to `src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java` (inside the class, before the final `}`): + +```java +@Test +void allOfWithSiblingTypeWrapsInImplicitAllOf() { + Schema s = + SchemaParser.parse( + Map.of( + "type", "object", + "required", List.of("x"), + "allOf", List.of(Map.of("type", "object", "required", List.of("y"))))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + assertThat(all.parts()).hasSize(2); + assertThat(all.parts().get(0)).isInstanceOf(ObjectSchema.class); + assertThat(((ObjectSchema) all.parts().get(0)).required()).containsExactly("x"); + assertThat(all.parts().get(1)).isInstanceOf(ObjectSchema.class); + assertThat(((ObjectSchema) all.parts().get(1)).required()).containsExactly("y"); +} + +@Test +void anyOfWithSiblingTypeWrapsInImplicitAllOf() { + Schema s = + SchemaParser.parse( + Map.of( + "type", "string", + "anyOf", + List.of( + Map.of("type", "string", "minLength", 1), + Map.of("type", "string", "maxLength", 10)))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + assertThat(all.parts()).hasSize(2); + assertThat(all.parts().get(0)).isInstanceOf(StringSchema.class); + assertThat(all.parts().get(1)).isInstanceOf(AnyOfSchema.class); + assertThat(((AnyOfSchema) all.parts().get(1)).options()).hasSize(2); +} + +@Test +void oneOfWithSiblingTypeWrapsInImplicitAllOf() { + Schema s = + SchemaParser.parse( + Map.of( + "type", "string", + "oneOf", + List.of( + Map.of("type", "string", "minLength", 1), + Map.of("type", "string", "maxLength", 10)))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + assertThat(all.parts()).hasSize(2); + assertThat(all.parts().get(0)).isInstanceOf(StringSchema.class); + assertThat(all.parts().get(1)).isInstanceOf(OneOfSchema.class); +} + +@Test +void notWithSiblingTypeWrapsInImplicitAllOf() { + Schema s = + SchemaParser.parse( + Map.of( + "type", "string", + "not", Map.of("type", "string", "maxLength", 2))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + assertThat(all.parts()).hasSize(2); + assertThat(all.parts().get(0)).isInstanceOf(StringSchema.class); + assertThat(all.parts().get(1)).isInstanceOf(NotSchema.class); +} + +@Test +void multipleCombinatorsInOneSchemaWrapInAllOf() { + Schema s = + SchemaParser.parse( + Map.of( + "anyOf", List.of(Map.of("type", "string"), Map.of("type", "integer")), + "not", Map.of("type", "boolean"))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + assertThat(all.parts()).hasSize(2); + assertThat(all.parts().get(0)).isInstanceOf(AnyOfSchema.class); + assertThat(all.parts().get(1)).isInstanceOf(NotSchema.class); +} + +@Test +void allOfBranchesFlattenIntoOuterAllOf() { + Schema s = + SchemaParser.parse( + Map.of( + "type", "string", + "allOf", + List.of( + Map.of("type", "string", "minLength", 1), + Map.of("type", "string", "maxLength", 10)))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + // Base + the two allOf branches flattened. + assertThat(all.parts()).hasSize(3); +} + +@Test +void aloneCombinatorStillReturnsCombinatorRecord() { + // Regression: when no base assertions are present, the result is still the + // single combinator record, not an AllOfSchema with a single child. + Schema s = + SchemaParser.parse( + Map.of("oneOf", List.of(Map.of("type", "string"), Map.of("type", "integer")))); + assertThat(s).isInstanceOf(OneOfSchema.class); + assertThat(((OneOfSchema) s).options()).hasSize(2); +} +``` + +- [ ] **Step 2: Run the new parser tests (expect failures)** + +Run: `mvn -q test -Dtest=SchemaParserTest` + +Expected: the 7 new tests fail. The composition tests fail because `parse(...)` currently returns a single combinator record (e.g. `OneOfSchema`), not an `AllOfSchema`. The `aloneCombinatorStillReturnsCombinatorRecord` test currently passes — it's a regression guard for the next step. + +### Step 2.2: Rewrite the parser dispatch + +- [ ] **Step 3: Replace the body of `SchemaParser.parse(Map)`** + +Open `src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java`. Replace the entire `parse` method (currently lines 15–55) with: + +```java +@SuppressWarnings("unchecked") +public static Schema parse(Map raw) { + if (raw.containsKey("$ref")) { + return new RefSchema((String) raw.get("$ref")); + } + + List assertions = new ArrayList<>(); + + Schema base = parseBaseIfPresent(raw); + if (base != null) { + assertions.add(base); + } + + if (raw.containsKey("allOf")) { + assertions.addAll(parseList(raw, "allOf")); + } + if (raw.containsKey("anyOf")) { + assertions.add(new AnyOfSchema(parseList(raw, "anyOf"))); + } + if (raw.containsKey("oneOf")) { + assertions.add(new OneOfSchema(parseList(raw, "oneOf"))); + } + if (raw.containsKey("not")) { + assertions.add(new NotSchema(parse((Map) raw.get("not")))); + } + + return switch (assertions.size()) { + case 0 -> permissiveObject(); + case 1 -> assertions.getFirst(); + default -> new AllOfSchema(List.copyOf(assertions)); + }; +} + +@SuppressWarnings("unchecked") +private static Schema parseBaseIfPresent(Map raw) { + if (raw.containsKey("const")) { + return new ConstSchema(raw.get("const")); + } + if (raw.containsKey("enum") && !raw.containsKey("type")) { + return new EnumSchema(List.copyOf((List) raw.get("enum"))); + } + + Set types = parseTypes(raw); + if (types.isEmpty() && !hasObjectShapeKeywords(raw) && !hasArrayShapeKeywords(raw)) { + return null; + } + + TypeName primary = + types.stream().filter(t -> t != TypeName.NULL).findFirst().orElse(TypeName.NULL); + + if (types.isEmpty() && hasObjectShapeKeywords(raw)) { + return parseObject(raw, types); + } + if (types.isEmpty() && hasArrayShapeKeywords(raw)) { + return parseArray(raw, types); + } + + return switch (primary) { + case STRING -> parseString(raw, types); + case INTEGER -> parseInteger(raw, types); + case NUMBER -> parseNumber(raw, types); + case BOOLEAN -> new BooleanSchema(types); + case NULL -> new NullSchema(); + case OBJECT -> parseObject(raw, types); + case ARRAY -> parseArray(raw, types); + }; +} + +private static boolean hasObjectShapeKeywords(Map raw) { + return raw.containsKey("properties") + || raw.containsKey("required") + || raw.containsKey("additionalProperties") + || raw.containsKey("minProperties") + || raw.containsKey("maxProperties"); +} + +private static boolean hasArrayShapeKeywords(Map raw) { + return raw.containsKey("items") + || raw.containsKey("minItems") + || raw.containsKey("maxItems") + || raw.containsKey("uniqueItems"); +} + +private static Schema permissiveObject() { + return new ObjectSchema( + Set.of(), + Map.of(), + List.of(), + new AdditionalProperties.Allowed(), + null, + null); +} +``` + +The key behaviour changes: +- `$ref` still returns immediately and ignores siblings (existing limitation, out of scope here). +- `parseBaseIfPresent` returns the existing primitive/object/array record when there is *any* base assertion in the schema map, and `null` when there is none. It also handles "implicit object" (`{required: [...]}` without `type`) and "implicit array" (`{items: ...}` without `type`) so those continue to parse as before when they appear without combinators. +- The four combinator keywords are then each appended to the assertions list. `allOf` is flattened (its branches join the outer list directly), the others wrap. +- A single assertion returns unwrapped; two or more wrap in `AllOfSchema`. + +- [ ] **Step 4: Run the parser tests** + +Run: `mvn -q test -Dtest=SchemaParserTest` + +Expected: all `SchemaParserTest` tests pass, including the 7 new ones. + +- [ ] **Step 5: Run the full unit suite** + +Run: `mvn -q test` + +Expected: BUILD SUCCESS, no regressions in `ContainerSchemasTest`, `PrimitiveSchemasTest`, `CombinatorScaffoldTest`, etc. + +- [ ] **Step 6: Commit** + +```bash +git add src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java \ + src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java +git commit -m "$(cat <<'EOF' +feat: Compose combinators with sibling base assertions in SchemaParser + +Previously the parser dispatched in priority order and emitted exactly +one schema record, silently discarding combinators that coexisted with +type / properties / etc. Now each combinator and any base assertion +contribute to an assertions list; multiple assertions wrap in an +implicit AllOfSchema. allOf branches flatten into the outer list. + +Adds 7 parser tests for the composition path and a regression guard +that a lone combinator still returns the bare combinator record. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 3: Integration test — polymorphic body via `oneOf` + +**Files:** +- Modify: `src/test/resources/openapi.yaml` +- Modify: `src/test/resources/openapi.json` +- Create: `src/test/java/com/retailsvc/http/start/PolymorphicHandler.java` +- Modify: `src/test/java/com/retailsvc/http/OpenApiServerIT.java` + +### Step 3.1: Extend the OpenAPI fixture + +The fixture has stub `/anyOf` and `/allOf` paths with empty `post: {}`. Replace them with a real `/oneOf` route exercising a polymorphic body. + +- [ ] **Step 1: Replace the stub paths in `openapi.yaml`** + +Open `src/test/resources/openapi.yaml`. Find the lines: + +```yaml + /anyOf: + post: {} + + /allOf: + post: {} +``` + +Replace with: + +```yaml + /shapes: + post: + operationId: post-shape + requestBody: + required: true + content: + application/json: + schema: + oneOf: + - type: object + required: [kind, radius] + properties: + kind: + type: string + enum: [circle] + radius: + type: number + - type: object + required: [kind, side] + properties: + kind: + type: string + enum: [square] + side: + type: number + responses: + "200": + description: OK +``` + +- [ ] **Step 2: Mirror the change in `openapi.json`** + +Open `src/test/resources/openapi.json`. Locate the `"/anyOf"` and `"/allOf"` keys (lines ~173 and ~178). Replace both blocks with a single `"/shapes"` entry equivalent to the YAML above. Use the existing JSON formatting (2-space indent, `application/json` media type). Worked example: + +```json + "/shapes": { + "post": { + "operationId": "post-shape", + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "oneOf": [ + { + "type": "object", + "required": ["kind", "radius"], + "properties": { + "kind": { "type": "string", "enum": ["circle"] }, + "radius": { "type": "number" } + } + }, + { + "type": "object", + "required": ["kind", "side"], + "properties": { + "kind": { "type": "string", "enum": ["square"] }, + "side": { "type": "number" } + } + } + ] + } + } + } + }, + "responses": { "200": { "description": "OK" } } + } + } +``` + +Make sure the trailing comma on the previous path entry (or this one) is correct so the JSON still parses. + +### Step 3.2: Add a test handler + +- [ ] **Step 3: Create `PolymorphicHandler.java`** + +Create `src/test/java/com/retailsvc/http/start/PolymorphicHandler.java`: + +```java +package com.retailsvc.http.start; + +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +public class PolymorphicHandler implements HttpHandler { + + @Override + public void handle(HttpExchange exchange) throws IOException { + byte[] body = "{\"ok\":true}".getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().add("Content-Type", "application/json"); + exchange.sendResponseHeaders(200, body.length); + try (var out = exchange.getResponseBody()) { + out.write(body); + } + } +} +``` + +### Step 3.3: Add the integration test + +- [ ] **Step 4: Add tests to `OpenApiServerIT.java`** + +Open `src/test/java/com/retailsvc/http/OpenApiServerIT.java`. Add a new nested class at the bottom of the outer class (immediately before the final `}` of `OpenApiServerIT`): + +```java +@Nested +class Shapes { + + String path = "/shapes"; + + @Test + void postShape_validCircleReturns200() { + try (var server = newServer(Map.of("post-shape", new com.retailsvc.http.start.PolymorphicHandler())); + var client = httpClient()) { + var body = "{\"kind\":\"circle\",\"radius\":2.5}"; + var request = newRequest(server, path, "POST", ofString(body)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(200); + assertThat(response.body()).contains("\"ok\":true"); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } + + @Test + void postShape_validSquareReturns200() { + try (var server = newServer(Map.of("post-shape", new com.retailsvc.http.start.PolymorphicHandler())); + var client = httpClient()) { + var body = "{\"kind\":\"square\",\"side\":3}"; + var request = newRequest(server, path, "POST", ofString(body)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(200); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } + + @Test + void postShape_unknownKindReturns400() { + // matches zero branches: "kind" is neither "circle" nor "square". + try (var server = newServer(Map.of("post-shape", new com.retailsvc.http.start.PolymorphicHandler())); + var client = httpClient()) { + var body = "{\"kind\":\"triangle\",\"side\":3}"; + var request = newRequest(server, path, "POST", ofString(body)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(400); + assertThat(response.headers().firstValue("Content-Type").orElse("")) + .contains("application/problem+json"); + assertThat(response.body()).contains("oneOf"); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } + + @Test + void postShape_missingDiscriminatorReturns400() { + // omitting "kind" makes both branches fail "required". + try (var server = newServer(Map.of("post-shape", new com.retailsvc.http.start.PolymorphicHandler())); + var client = httpClient()) { + var body = "{\"radius\":2.5}"; + var request = newRequest(server, path, "POST", ofString(body)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(400); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } +} +``` + +The `ofString`, `BodyHandlers`, and `assertThat` imports are already present at the top of `OpenApiServerIT.java`; no new imports needed beyond `org.junit.jupiter.api.Nested` (also already present) and `java.io.IOException` (already imported). + +- [ ] **Step 5: Run the integration tests** + +Run: `mvn -q verify -DfailIfNoTests=false -Dtest='!*' -Dit.test=OpenApiServerIT` + +Or simply: `mvn -q verify` + +Expected: BUILD SUCCESS. The four new IT cases all pass. + +- [ ] **Step 6: Run the full verify** + +Run: `mvn -q verify` + +Expected: BUILD SUCCESS. All previous unit tests + the 9 new validator tests + the 7 new parser tests + the 4 new IT tests pass. + +- [ ] **Step 7: Commit** + +```bash +git add src/test/resources/openapi.yaml src/test/resources/openapi.json \ + src/test/java/com/retailsvc/http/start/PolymorphicHandler.java \ + src/test/java/com/retailsvc/http/OpenApiServerIT.java +git commit -m "$(cat <<'EOF' +test: Add polymorphic-body integration coverage for oneOf + +Replace stub /anyOf and /allOf fixture routes with a real /shapes +endpoint that uses oneOf to discriminate between circle and square +request bodies. Adds an integration test verifying both valid +branches return 200 and that bodies matching zero branches return +400 application/problem+json with the oneOf keyword in the body. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Final verification + +- [ ] Run `mvn -q verify` once more. +- [ ] `git log --oneline master..HEAD` shows three new commits in this order: `feat: Implement combinator validation…`, `feat: Compose combinators with sibling…`, `test: Add polymorphic-body integration…`. +- [ ] No new files exist outside the paths listed above. + +## Out of scope (do not do) + +- Schema booleans (`true` / `false` as a bare schema) — needs `SchemaParser.parse(...)` to accept a non-Map raw value. +- `discriminator` keyword — separate Wave 6 follow-up. +- Multi-error collection — separate Wave 6 follow-up. +- Internal `boolean tryValidate(...)` overload to avoid throw/catch in `oneOf`/`anyOf` — only worth doing if profiling shows it. +- `$ref` siblings — existing limitation, untouched. From 5f08f50c728f0abf4bea2ee297a7724150363ef9 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 14:26:57 +0200 Subject: [PATCH 03/14] feat: Implement combinator validation in DefaultValidator Replace the four UnsupportedOperationException branches for allOf / anyOf / oneOf / not with real validation. allOf propagates the first failing branch; anyOf short-circuits on first match; oneOf evaluates all branches and asserts exactly one matches; not inverts the inner result. Adds 9 unit tests covering pass/fail per combinator. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../http/validate/DefaultValidator.java | 52 ++++++++++- .../DefaultValidatorDispatchTest.java | 88 ++++++++++++++++++- 2 files changed, 132 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/retailsvc/http/validate/DefaultValidator.java b/src/main/java/com/retailsvc/http/validate/DefaultValidator.java index 2516044..48e6cce 100644 --- a/src/main/java/com/retailsvc/http/validate/DefaultValidator.java +++ b/src/main/java/com/retailsvc/http/validate/DefaultValidator.java @@ -64,10 +64,14 @@ case EnumSchema(List values) -> require(values.contains(value), pointer, "enum", "value not in enum"); case ConstSchema(Object expected) -> require(Objects.equals(expected, value), pointer, "const", "value does not equal const"); - case OneOfSchema _ -> throw new UnsupportedOperationException("oneOf not yet supported"); - case AnyOfSchema _ -> throw new UnsupportedOperationException("anyOf not yet supported"); - case AllOfSchema _ -> throw new UnsupportedOperationException("allOf not yet supported"); - case NotSchema _ -> throw new UnsupportedOperationException("not not yet supported"); + case AllOfSchema(List parts) -> { + for (Schema p : parts) { + validate(value, p, pointer); + } + } + case AnyOfSchema(List options) -> validateAnyOf(value, options, pointer); + case OneOfSchema(List options) -> validateOneOf(value, options, pointer); + case NotSchema(Schema inner) -> validateNot(value, inner, pointer); } } @@ -290,4 +294,44 @@ static void require(boolean condition, String pointer, String keyword, String me throw new ValidationException(new ValidationError(pointer, keyword, message, null)); } } + + private void validateAnyOf(Object value, List options, String pointer) { + for (Schema o : options) { + try { + validate(value, o, pointer); + return; + } catch (ValidationException ignored) { + // try next branch + } + } + fail(pointer, "anyOf", "did not match any anyOf branch", value); + } + + private void validateOneOf(Object value, List options, String pointer) { + int matched = 0; + for (Schema o : options) { + try { + validate(value, o, pointer); + matched++; + } catch (ValidationException ignored) { + // count misses + } + } + if (matched != 1) { + fail( + pointer, + "oneOf", + "matched " + matched + " of " + options.size() + " oneOf branches", + value); + } + } + + private void validateNot(Object value, Schema inner, String pointer) { + try { + validate(value, inner, pointer); + } catch (ValidationException expected) { + return; + } + fail(pointer, "not", "value matched 'not' schema", value); + } } diff --git a/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java b/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java index 8ae8a97..3bf2d8b 100644 --- a/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java +++ b/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java @@ -3,9 +3,13 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.retailsvc.http.ValidationException; +import com.retailsvc.http.spec.schema.AllOfSchema; +import com.retailsvc.http.spec.schema.AnyOfSchema; import com.retailsvc.http.spec.schema.BooleanSchema; +import com.retailsvc.http.spec.schema.NotSchema; import com.retailsvc.http.spec.schema.NullSchema; import com.retailsvc.http.spec.schema.OneOfSchema; +import com.retailsvc.http.spec.schema.StringSchema; import com.retailsvc.http.spec.schema.TypeName; import java.util.List; import java.util.Set; @@ -43,10 +47,86 @@ void booleanSchemaRejectsString() { assertThatThrownBy(() -> v.validate("x", schema, "/v")).isInstanceOf(ValidationException.class); } + private StringSchema stringSchema(Integer min, Integer max) { + return new StringSchema(Set.of(TypeName.STRING), null, min, max, null, null); + } + @Test - void combinatorThrowsUnsupported() { - var schema = new OneOfSchema(List.of()); - assertThatThrownBy(() -> v.validate("x", schema, "/v")) - .isInstanceOf(UnsupportedOperationException.class); + void allOfPassesWhenAllBranchesPass() { + var schema = new AllOfSchema(List.of(stringSchema(1, null), stringSchema(null, 10))); + v.validate("hello", schema, "/v"); + } + + @Test + void allOfPropagatesFirstFailingBranch() { + var schema = new AllOfSchema(List.of(stringSchema(1, null), stringSchema(null, 3))); + assertThatThrownBy(() -> v.validate("hello", schema, "/v")) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("maxLength"); + } + + @Test + void anyOfPassesWhenOneBranchPasses() { + var schema = new AnyOfSchema(List.of(stringSchema(100, null), stringSchema(null, 10))); + v.validate("hello", schema, "/v"); + } + + @Test + void anyOfFailsWhenNoBranchMatches() { + var schema = new AnyOfSchema(List.of(stringSchema(100, null), stringSchema(null, 2))); + assertThatThrownBy(() -> v.validate("hello", schema, "/v")) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("anyOf"); + } + + @Test + void oneOfPassesWhenExactlyOneBranchMatches() { + // value "hello" — len 5. branch[0] requires min 100 (fails), branch[1] max 10 (passes). + var schema = new OneOfSchema(List.of(stringSchema(100, null), stringSchema(null, 10))); + v.validate("hello", schema, "/v"); + } + + @Test + void oneOfFailsWhenZeroBranchesMatch() { + var schema = new OneOfSchema(List.of(stringSchema(100, null), stringSchema(null, 2))); + assertThatThrownBy(() -> v.validate("hello", schema, "/v")) + .isInstanceOf(ValidationException.class) + .satisfies( + t -> { + var err = ((ValidationException) t).error(); + org.assertj.core.api.Assertions.assertThat(err.keyword()).isEqualTo("oneOf"); + org.assertj.core.api.Assertions.assertThat(err.message()).contains("matched 0 of 2"); + }); + } + + @Test + void oneOfFailsWhenTwoBranchesMatch() { + // value "hello" — both branches accept. + var schema = new OneOfSchema(List.of(stringSchema(null, 10), stringSchema(1, null))); + assertThatThrownBy(() -> v.validate("hello", schema, "/v")) + .isInstanceOf(ValidationException.class) + .satisfies( + t -> { + var err = ((ValidationException) t).error(); + org.assertj.core.api.Assertions.assertThat(err.keyword()).isEqualTo("oneOf"); + org.assertj.core.api.Assertions.assertThat(err.message()).contains("matched 2 of 2"); + }); + } + + @Test + void notPassesWhenInnerFails() { + var schema = new NotSchema(stringSchema(100, null)); + v.validate("hello", schema, "/v"); + } + + @Test + void notFailsWhenInnerPasses() { + var schema = new NotSchema(stringSchema(null, 10)); + assertThatThrownBy(() -> v.validate("hello", schema, "/v")) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("not"); } } From 2fc24b502a5a212c6aad8c2aa3c408e9d2d5810c Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 14:35:23 +0200 Subject: [PATCH 04/14] fix: Address code review for combinator validation - Corrected misleading comment in validateOneOf (was "count misses", now "branch did not match; continue") - Added empty-options regression tests for allOf, anyOf, and oneOf to pin JSON Schema vacuous-truth / immediate-failure behaviour - Added null-value tests for not(NullSchema) and anyOf-with-NullSchema to document combinator null-passthrough behaviour - Replaced verbose fully-qualified org.assertj.core.api.Assertions.assertThat calls with static import in DefaultValidatorDispatchTest Co-Authored-By: Claude Opus 4.7 (1M context) --- .../http/validate/DefaultValidator.java | 2 +- .../DefaultValidatorDispatchTest.java | 51 +++++++++++++++++-- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/retailsvc/http/validate/DefaultValidator.java b/src/main/java/com/retailsvc/http/validate/DefaultValidator.java index 48e6cce..ce810b9 100644 --- a/src/main/java/com/retailsvc/http/validate/DefaultValidator.java +++ b/src/main/java/com/retailsvc/http/validate/DefaultValidator.java @@ -314,7 +314,7 @@ private void validateOneOf(Object value, List options, String pointer) { validate(value, o, pointer); matched++; } catch (ValidationException ignored) { - // count misses + // branch did not match; continue } } if (matched != 1) { diff --git a/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java b/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java index 3bf2d8b..1b2d8bd 100644 --- a/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java +++ b/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java @@ -1,5 +1,6 @@ package com.retailsvc.http.validate; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.retailsvc.http.ValidationException; @@ -96,8 +97,8 @@ void oneOfFailsWhenZeroBranchesMatch() { .satisfies( t -> { var err = ((ValidationException) t).error(); - org.assertj.core.api.Assertions.assertThat(err.keyword()).isEqualTo("oneOf"); - org.assertj.core.api.Assertions.assertThat(err.message()).contains("matched 0 of 2"); + assertThat(err.keyword()).isEqualTo("oneOf"); + assertThat(err.message()).contains("matched 0 of 2"); }); } @@ -110,8 +111,8 @@ void oneOfFailsWhenTwoBranchesMatch() { .satisfies( t -> { var err = ((ValidationException) t).error(); - org.assertj.core.api.Assertions.assertThat(err.keyword()).isEqualTo("oneOf"); - org.assertj.core.api.Assertions.assertThat(err.message()).contains("matched 2 of 2"); + assertThat(err.keyword()).isEqualTo("oneOf"); + assertThat(err.message()).contains("matched 2 of 2"); }); } @@ -129,4 +130,46 @@ void notFailsWhenInnerPasses() { .extracting(t -> ((ValidationException) t).error().keyword()) .isEqualTo("not"); } + + @Test + void allOfWithEmptyPartsAlwaysPasses() { + // Empty allOf is vacuously true per JSON Schema 2020-12. + v.validate("anything", new AllOfSchema(List.of()), "/v"); + } + + @Test + void anyOfWithEmptyOptionsAlwaysFails() { + assertThatThrownBy(() -> v.validate("anything", new AnyOfSchema(List.of()), "/v")) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("anyOf"); + } + + @Test + void oneOfWithEmptyOptionsAlwaysFails() { + assertThatThrownBy(() -> v.validate("anything", new OneOfSchema(List.of()), "/v")) + .isInstanceOf(ValidationException.class) + .satisfies( + t -> { + var err = ((ValidationException) t).error(); + assertThat(err.keyword()).isEqualTo("oneOf"); + assertThat(err.message()).contains("matched 0 of 0"); + }); + } + + @Test + void notWithNullSchemaRejectsNull() { + // not(NullSchema) — inner accepts null, outer must reject. + assertThatThrownBy(() -> v.validate(null, new NotSchema(new NullSchema()), "/v")) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("not"); + } + + @Test + void anyOfMatchesNullViaNullSchema() { + // anyOf containing a NullSchema branch must pass for a null value. + var schema = new AnyOfSchema(List.of(stringSchema(1, null), new NullSchema())); + v.validate(null, schema, "/v"); + } } From 5e9b32c18414939cd8352cff58a0bb985e62359d Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 14:38:32 +0200 Subject: [PATCH 05/14] feat: Compose combinators with sibling base assertions in SchemaParser Previously the parser dispatched in priority order and emitted exactly one schema record, silently discarding combinators that coexisted with type / properties / etc. Now each combinator and any base assertion contribute to an assertions list; multiple assertions wrap in an implicit AllOfSchema. allOf branches flatten into the outer list. Adds 7 parser tests for the composition path and a regression guard that a lone combinator still returns the bare combinator record. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../http/spec/schema/SchemaParser.java | 60 +++++++-- .../http/spec/schema/SchemaParserTest.java | 116 ++++++++++++++++++ 2 files changed, 169 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java b/src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java index 4dc42c1..f922702 100644 --- a/src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java +++ b/src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java @@ -18,18 +18,35 @@ public static Schema parse(Map raw) { return new RefSchema((String) raw.get("$ref")); } - if (raw.containsKey("oneOf")) { - return new OneOfSchema(parseList(raw, "oneOf")); + List assertions = new ArrayList<>(); + + Schema base = parseBaseIfPresent(raw); + if (base != null) { + assertions.add(base); + } + + if (raw.containsKey("allOf")) { + assertions.addAll(parseList(raw, "allOf")); } if (raw.containsKey("anyOf")) { - return new AnyOfSchema(parseList(raw, "anyOf")); + assertions.add(new AnyOfSchema(parseList(raw, "anyOf"))); } - if (raw.containsKey("allOf")) { - return new AllOfSchema(parseList(raw, "allOf")); + if (raw.containsKey("oneOf")) { + assertions.add(new OneOfSchema(parseList(raw, "oneOf"))); } if (raw.containsKey("not")) { - return new NotSchema(parse((Map) raw.get("not"))); + assertions.add(new NotSchema(parse((Map) raw.get("not")))); } + + return switch (assertions.size()) { + case 0 -> permissiveObject(); + case 1 -> assertions.getFirst(); + default -> new AllOfSchema(List.copyOf(assertions)); + }; + } + + @SuppressWarnings("unchecked") + private static Schema parseBaseIfPresent(Map raw) { if (raw.containsKey("const")) { return new ConstSchema(raw.get("const")); } @@ -38,11 +55,20 @@ public static Schema parse(Map raw) { } Set types = parseTypes(raw); + if (types.isEmpty() && !hasObjectShapeKeywords(raw) && !hasArrayShapeKeywords(raw)) { + return null; + } - // Pick primary (non-null) type for record dispatch. TypeName primary = types.stream().filter(t -> t != TypeName.NULL).findFirst().orElse(TypeName.NULL); + if (types.isEmpty() && hasObjectShapeKeywords(raw)) { + return parseObject(raw, types); + } + if (types.isEmpty() && hasArrayShapeKeywords(raw)) { + return parseArray(raw, types); + } + return switch (primary) { case STRING -> parseString(raw, types); case INTEGER -> parseInteger(raw, types); @@ -54,6 +80,26 @@ public static Schema parse(Map raw) { }; } + private static boolean hasObjectShapeKeywords(Map raw) { + return raw.containsKey("properties") + || raw.containsKey("required") + || raw.containsKey("additionalProperties") + || raw.containsKey("minProperties") + || raw.containsKey("maxProperties"); + } + + private static boolean hasArrayShapeKeywords(Map raw) { + return raw.containsKey("items") + || raw.containsKey("minItems") + || raw.containsKey("maxItems") + || raw.containsKey("uniqueItems"); + } + + private static Schema permissiveObject() { + return new ObjectSchema( + Set.of(), Map.of(), List.of(), new AdditionalProperties.Allowed(), null, null); + } + private static Set parseTypes(Map raw) { Object t = raw.get("type"); EnumSet out = EnumSet.noneOf(TypeName.class); diff --git a/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java b/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java index 5ff804a..1328695 100644 --- a/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java +++ b/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java @@ -121,7 +121,15 @@ void parsesOneOf() { void parsesAnyOfAllOfNot() { assertThat(SchemaParser.parse(Map.of("anyOf", List.of(Map.of("type", "string"))))) .isInstanceOf(AnyOfSchema.class); + // allOf with a single branch flattens to the branch itself (no wrapper AllOfSchema). assertThat(SchemaParser.parse(Map.of("allOf", List.of(Map.of("type", "string"))))) + .isInstanceOf(StringSchema.class); + // allOf with multiple branches flattens into AllOfSchema. + assertThat( + SchemaParser.parse( + Map.of( + "allOf", + List.of(Map.of("type", "string"), Map.of("type", "string", "minLength", 1))))) .isInstanceOf(AllOfSchema.class); assertThat(SchemaParser.parse(Map.of("not", Map.of("type", "null")))) .isInstanceOf(NotSchema.class); @@ -146,4 +154,112 @@ void enumOnStringStaysAsStringSchema() { assertThat(s).isInstanceOf(StringSchema.class); assertThat(((StringSchema) s).enumValues()).containsExactly("a", "b"); } + + @Test + void allOfWithSiblingTypeWrapsInImplicitAllOf() { + Schema s = + SchemaParser.parse( + Map.of( + "type", "object", + "required", List.of("x"), + "allOf", List.of(Map.of("type", "object", "required", List.of("y"))))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + assertThat(all.parts()).hasSize(2); + assertThat(all.parts().get(0)).isInstanceOf(ObjectSchema.class); + assertThat(((ObjectSchema) all.parts().get(0)).required()).containsExactly("x"); + assertThat(all.parts().get(1)).isInstanceOf(ObjectSchema.class); + assertThat(((ObjectSchema) all.parts().get(1)).required()).containsExactly("y"); + } + + @Test + void anyOfWithSiblingTypeWrapsInImplicitAllOf() { + Schema s = + SchemaParser.parse( + Map.of( + "type", + "string", + "anyOf", + List.of( + Map.of("type", "string", "minLength", 1), + Map.of("type", "string", "maxLength", 10)))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + assertThat(all.parts()).hasSize(2); + assertThat(all.parts().get(0)).isInstanceOf(StringSchema.class); + assertThat(all.parts().get(1)).isInstanceOf(AnyOfSchema.class); + assertThat(((AnyOfSchema) all.parts().get(1)).options()).hasSize(2); + } + + @Test + void oneOfWithSiblingTypeWrapsInImplicitAllOf() { + Schema s = + SchemaParser.parse( + Map.of( + "type", + "string", + "oneOf", + List.of( + Map.of("type", "string", "minLength", 1), + Map.of("type", "string", "maxLength", 10)))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + assertThat(all.parts()).hasSize(2); + assertThat(all.parts().get(0)).isInstanceOf(StringSchema.class); + assertThat(all.parts().get(1)).isInstanceOf(OneOfSchema.class); + } + + @Test + void notWithSiblingTypeWrapsInImplicitAllOf() { + Schema s = + SchemaParser.parse( + Map.of("type", "string", "not", Map.of("type", "string", "maxLength", 2))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + assertThat(all.parts()).hasSize(2); + assertThat(all.parts().get(0)).isInstanceOf(StringSchema.class); + assertThat(all.parts().get(1)).isInstanceOf(NotSchema.class); + } + + @Test + void multipleCombinatorsInOneSchemaWrapInAllOf() { + Schema s = + SchemaParser.parse( + Map.of( + "anyOf", List.of(Map.of("type", "string"), Map.of("type", "integer")), + "not", Map.of("type", "boolean"))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + assertThat(all.parts()).hasSize(2); + assertThat(all.parts().get(0)).isInstanceOf(AnyOfSchema.class); + assertThat(all.parts().get(1)).isInstanceOf(NotSchema.class); + } + + @Test + void allOfBranchesFlattenIntoOuterAllOf() { + Schema s = + SchemaParser.parse( + Map.of( + "type", + "string", + "allOf", + List.of( + Map.of("type", "string", "minLength", 1), + Map.of("type", "string", "maxLength", 10)))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + // Base + the two allOf branches flattened. + assertThat(all.parts()).hasSize(3); + } + + @Test + void aloneCombinatorStillReturnsCombinatorRecord() { + // Regression: when no base assertions are present, the result is still the + // single combinator record, not an AllOfSchema with a single child. + Schema s = + SchemaParser.parse( + Map.of("oneOf", List.of(Map.of("type", "string"), Map.of("type", "integer")))); + assertThat(s).isInstanceOf(OneOfSchema.class); + assertThat(((OneOfSchema) s).options()).hasSize(2); + } } From a1bc0c077030dd146f43843130335b8b93ed4ade Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 14:46:12 +0200 Subject: [PATCH 06/14] fix: Address code review for SchemaParser composition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reordered `parseBaseIfPresent` so that `primary` (the dominant non-null TypeName) is only computed after the two implicit-shape early-return branches (object/array keyword detection), ensuring it is never evaluated unnecessarily. Pinned the empty-schema → permissive ObjectSchema behaviour with a new `parsesEmptySchemaAsPermissiveObject` test, which documents the intentional deviation from the old NullSchema return. Added a `parsesEmptyAllOfAsPermissiveObject` regression test to confirm that `allOf: []` (zero assertions) falls through to the permissive-object fallback. Strengthened `allOfBranchesFlattenIntoOuterAllOf` with per-part instanceof assertions and concrete minLength/maxLength constraint checks on the flattened StringSchema parts. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../http/spec/schema/SchemaParser.java | 6 ++--- .../http/spec/schema/SchemaParserTest.java | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java b/src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java index f922702..23c9aa6 100644 --- a/src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java +++ b/src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java @@ -58,10 +58,6 @@ private static Schema parseBaseIfPresent(Map raw) { if (types.isEmpty() && !hasObjectShapeKeywords(raw) && !hasArrayShapeKeywords(raw)) { return null; } - - TypeName primary = - types.stream().filter(t -> t != TypeName.NULL).findFirst().orElse(TypeName.NULL); - if (types.isEmpty() && hasObjectShapeKeywords(raw)) { return parseObject(raw, types); } @@ -69,6 +65,8 @@ private static Schema parseBaseIfPresent(Map raw) { return parseArray(raw, types); } + TypeName primary = + types.stream().filter(t -> t != TypeName.NULL).findFirst().orElse(TypeName.NULL); return switch (primary) { case STRING -> parseString(raw, types); case INTEGER -> parseInteger(raw, types); diff --git a/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java b/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java index 1328695..7f7365c 100644 --- a/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java +++ b/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java @@ -235,6 +235,27 @@ void multipleCombinatorsInOneSchemaWrapInAllOf() { assertThat(all.parts().get(1)).isInstanceOf(NotSchema.class); } + @Test + void parsesEmptySchemaAsPermissiveObject() { + // {} accepts anything that's an object; this is the JSON Schema 3.1 default + // and a behaviour change from the previous parser, which returned NullSchema. + Schema s = SchemaParser.parse(Map.of()); + assertThat(s).isInstanceOf(ObjectSchema.class); + ObjectSchema obj = (ObjectSchema) s; + assertThat(obj.types()).isEmpty(); + assertThat(obj.properties()).isEmpty(); + assertThat(obj.required()).isEmpty(); + assertThat(obj.additionalProperties()).isInstanceOf(AdditionalProperties.Allowed.class); + } + + @Test + void parsesEmptyAllOfAsPermissiveObject() { + // allOf: [] contributes zero assertions; with no base assertion, the parser + // falls back to permissive object. + Schema s = SchemaParser.parse(Map.of("allOf", List.of())); + assertThat(s).isInstanceOf(ObjectSchema.class); + } + @Test void allOfBranchesFlattenIntoOuterAllOf() { Schema s = @@ -250,6 +271,11 @@ void allOfBranchesFlattenIntoOuterAllOf() { AllOfSchema all = (AllOfSchema) s; // Base + the two allOf branches flattened. assertThat(all.parts()).hasSize(3); + assertThat(all.parts().get(0)).isInstanceOf(StringSchema.class); + assertThat(all.parts().get(1)).isInstanceOf(StringSchema.class); + assertThat(all.parts().get(2)).isInstanceOf(StringSchema.class); + assertThat(((StringSchema) all.parts().get(1)).minLength()).isEqualTo(1); + assertThat(((StringSchema) all.parts().get(2)).maxLength()).isEqualTo(10); } @Test From a024d91c98a607dd158ac6cccaa761761fb4f543 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 14:48:11 +0200 Subject: [PATCH 07/14] test: Add polymorphic-body integration coverage for oneOf Replace stub /anyOf and /allOf fixture routes with a real /shapes endpoint that uses oneOf to discriminate between circle and square request bodies. Adds an integration test verifying both valid branches return 200 and that bodies matching zero branches return 400 application/problem+json with the oneOf keyword in the body. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../com/retailsvc/http/OpenApiServerIT.java | 88 +++++++++++++++++++ .../http/start/PolymorphicHandler.java | 19 ++++ src/test/resources/openapi.json | 37 ++++++-- src/test/resources/openapi.yaml | 33 +++++-- 4 files changed, 165 insertions(+), 12 deletions(-) create mode 100644 src/test/java/com/retailsvc/http/start/PolymorphicHandler.java diff --git a/src/test/java/com/retailsvc/http/OpenApiServerIT.java b/src/test/java/com/retailsvc/http/OpenApiServerIT.java index 0545093..8e41eb7 100644 --- a/src/test/java/com/retailsvc/http/OpenApiServerIT.java +++ b/src/test/java/com/retailsvc/http/OpenApiServerIT.java @@ -402,4 +402,92 @@ void getPathParams_shouldReturnInternalErrorOnMissingHandler() { } } } + + @Nested + class Shapes { + + String path = "/shapes"; + + @Test + void postShape_validCircleReturns200() { + try (var server = + newServer(Map.of("post-shape", new com.retailsvc.http.start.PolymorphicHandler())); + var client = httpClient()) { + var body = "{\"kind\":\"circle\",\"radius\":2.5}"; + var request = newRequest(server, path, "POST", ofString(body)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(200); + assertThat(response.body()).contains("\"ok\":true"); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } + + @Test + void postShape_validSquareReturns200() { + try (var server = + newServer(Map.of("post-shape", new com.retailsvc.http.start.PolymorphicHandler())); + var client = httpClient()) { + var body = "{\"kind\":\"square\",\"side\":3}"; + var request = newRequest(server, path, "POST", ofString(body)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(200); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } + + @Test + void postShape_unknownKindReturns400() { + // matches zero branches: "kind" is neither "circle" nor "square". + try (var server = + newServer(Map.of("post-shape", new com.retailsvc.http.start.PolymorphicHandler())); + var client = httpClient()) { + var body = "{\"kind\":\"triangle\",\"side\":3}"; + var request = newRequest(server, path, "POST", ofString(body)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(400); + assertThat(response.headers().firstValue("Content-Type").orElse("")) + .contains("application/problem+json"); + assertThat(response.body()).contains("oneOf"); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } + + @Test + void postShape_missingDiscriminatorReturns400() { + // omitting "kind" makes both branches fail "required". + try (var server = + newServer(Map.of("post-shape", new com.retailsvc.http.start.PolymorphicHandler())); + var client = httpClient()) { + var body = "{\"radius\":2.5}"; + var request = newRequest(server, path, "POST", ofString(body)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(400); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } + } } diff --git a/src/test/java/com/retailsvc/http/start/PolymorphicHandler.java b/src/test/java/com/retailsvc/http/start/PolymorphicHandler.java new file mode 100644 index 0000000..00637d8 --- /dev/null +++ b/src/test/java/com/retailsvc/http/start/PolymorphicHandler.java @@ -0,0 +1,19 @@ +package com.retailsvc.http.start; + +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +public class PolymorphicHandler implements HttpHandler { + + @Override + public void handle(HttpExchange exchange) throws IOException { + byte[] body = "{\"ok\":true}".getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().add("Content-Type", "application/json"); + exchange.sendResponseHeaders(200, body.length); + try (var out = exchange.getResponseBody()) { + out.write(body); + } + } +} diff --git a/src/test/resources/openapi.json b/src/test/resources/openapi.json index 9fe38d6..522e1b8 100644 --- a/src/test/resources/openapi.json +++ b/src/test/resources/openapi.json @@ -170,14 +170,37 @@ } } }, - "/anyOf": { + "/shapes": { "post": { - - } - }, - "/allOf": { - "post": { - + "operationId": "post-shape", + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "oneOf": [ + { + "type": "object", + "required": ["kind", "radius"], + "properties": { + "kind": { "type": "string", "enum": ["circle"] }, + "radius": { "type": "number" } + } + }, + { + "type": "object", + "required": ["kind", "side"], + "properties": { + "kind": { "type": "string", "enum": ["square"] }, + "side": { "type": "number" } + } + } + ] + } + } + } + }, + "responses": { "200": { "description": "OK" } } } } }, diff --git a/src/test/resources/openapi.yaml b/src/test/resources/openapi.yaml index de5f184..e46e4bb 100644 --- a/src/test/resources/openapi.yaml +++ b/src/test/resources/openapi.yaml @@ -109,11 +109,34 @@ paths: "200": description: OK - /anyOf: - post: {} - - /allOf: - post: {} + /shapes: + post: + operationId: post-shape + requestBody: + required: true + content: + application/json: + schema: + oneOf: + - type: object + required: [kind, radius] + properties: + kind: + type: string + enum: [circle] + radius: + type: number + - type: object + required: [kind, side] + properties: + kind: + type: string + enum: [square] + side: + type: number + responses: + "200": + description: OK components: parameters: From 15ab4b7982961020a034560a3b24e67a5cf4eb1f Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 14:55:54 +0200 Subject: [PATCH 08/14] refactor: Reuse EchoHandler for Shapes integration tests Delete PolymorphicHandler, which had no unique behaviour over EchoHandler (both return 200 and reflect the request body). Update all four handler references in the Shapes nested class to use EchoHandler directly. Adjust the circle body assertion to check the echoed payload ("kind":"circle") instead of the old hard-coded {"ok":true} response. Strengthen the missing-discriminator test with Content-Type and body assertions to mirror the unknown-kind case. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../com/retailsvc/http/OpenApiServerIT.java | 17 ++++++++--------- .../http/start/PolymorphicHandler.java | 19 ------------------- 2 files changed, 8 insertions(+), 28 deletions(-) delete mode 100644 src/test/java/com/retailsvc/http/start/PolymorphicHandler.java diff --git a/src/test/java/com/retailsvc/http/OpenApiServerIT.java b/src/test/java/com/retailsvc/http/OpenApiServerIT.java index 8e41eb7..72d347d 100644 --- a/src/test/java/com/retailsvc/http/OpenApiServerIT.java +++ b/src/test/java/com/retailsvc/http/OpenApiServerIT.java @@ -410,8 +410,7 @@ class Shapes { @Test void postShape_validCircleReturns200() { - try (var server = - newServer(Map.of("post-shape", new com.retailsvc.http.start.PolymorphicHandler())); + try (var server = newServer(Map.of("post-shape", new EchoHandler())); var client = httpClient()) { var body = "{\"kind\":\"circle\",\"radius\":2.5}"; var request = newRequest(server, path, "POST", ofString(body)); @@ -419,7 +418,7 @@ void postShape_validCircleReturns200() { var response = client.send(request, BodyHandlers.ofString()); assertThat(response.statusCode()).isEqualTo(200); - assertThat(response.body()).contains("\"ok\":true"); + assertThat(response.body()).contains("\"kind\":\"circle\""); } catch (IOException e) { fail(e); } catch (InterruptedException e) { @@ -430,8 +429,7 @@ void postShape_validCircleReturns200() { @Test void postShape_validSquareReturns200() { - try (var server = - newServer(Map.of("post-shape", new com.retailsvc.http.start.PolymorphicHandler())); + try (var server = newServer(Map.of("post-shape", new EchoHandler())); var client = httpClient()) { var body = "{\"kind\":\"square\",\"side\":3}"; var request = newRequest(server, path, "POST", ofString(body)); @@ -450,8 +448,7 @@ void postShape_validSquareReturns200() { @Test void postShape_unknownKindReturns400() { // matches zero branches: "kind" is neither "circle" nor "square". - try (var server = - newServer(Map.of("post-shape", new com.retailsvc.http.start.PolymorphicHandler())); + try (var server = newServer(Map.of("post-shape", new EchoHandler())); var client = httpClient()) { var body = "{\"kind\":\"triangle\",\"side\":3}"; var request = newRequest(server, path, "POST", ofString(body)); @@ -473,8 +470,7 @@ void postShape_unknownKindReturns400() { @Test void postShape_missingDiscriminatorReturns400() { // omitting "kind" makes both branches fail "required". - try (var server = - newServer(Map.of("post-shape", new com.retailsvc.http.start.PolymorphicHandler())); + try (var server = newServer(Map.of("post-shape", new EchoHandler())); var client = httpClient()) { var body = "{\"radius\":2.5}"; var request = newRequest(server, path, "POST", ofString(body)); @@ -482,6 +478,9 @@ void postShape_missingDiscriminatorReturns400() { var response = client.send(request, BodyHandlers.ofString()); assertThat(response.statusCode()).isEqualTo(400); + assertThat(response.headers().firstValue("Content-Type").orElse("")) + .contains("application/problem+json"); + assertThat(response.body()).contains("oneOf"); } catch (IOException e) { fail(e); } catch (InterruptedException e) { diff --git a/src/test/java/com/retailsvc/http/start/PolymorphicHandler.java b/src/test/java/com/retailsvc/http/start/PolymorphicHandler.java deleted file mode 100644 index 00637d8..0000000 --- a/src/test/java/com/retailsvc/http/start/PolymorphicHandler.java +++ /dev/null @@ -1,19 +0,0 @@ -package com.retailsvc.http.start; - -import com.sun.net.httpserver.HttpExchange; -import com.sun.net.httpserver.HttpHandler; -import java.io.IOException; -import java.nio.charset.StandardCharsets; - -public class PolymorphicHandler implements HttpHandler { - - @Override - public void handle(HttpExchange exchange) throws IOException { - byte[] body = "{\"ok\":true}".getBytes(StandardCharsets.UTF_8); - exchange.getResponseHeaders().add("Content-Type", "application/json"); - exchange.sendResponseHeaders(200, body.length); - try (var out = exchange.getResponseBody()) { - out.write(body); - } - } -} From 4a21a785042e4f8fbf1b7a37413206779e0a7e53 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 15:02:10 +0200 Subject: [PATCH 09/14] test: Add anyOf and not integration coverage Add /filters (anyOf) and /blocked (not) fixture endpoints in both the YAML and JSON test-spec twins. Register five new IT tests across two new nested classes (Filters, Blocked) in OpenApiServerIT: valid string, valid integer, and short-string-rejected for anyOf; accepted-token and forbidden-token for not. Also clarify the empty-schema comment in SchemaParserTest to be more precise about what permissive ObjectSchema means. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../com/retailsvc/http/OpenApiServerIT.java | 109 ++++++++++++++++++ .../http/spec/schema/SchemaParserTest.java | 4 +- src/test/resources/openapi.json | 48 ++++++++ src/test/resources/openapi.yaml | 39 +++++++ 4 files changed, 198 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/retailsvc/http/OpenApiServerIT.java b/src/test/java/com/retailsvc/http/OpenApiServerIT.java index 72d347d..de527da 100644 --- a/src/test/java/com/retailsvc/http/OpenApiServerIT.java +++ b/src/test/java/com/retailsvc/http/OpenApiServerIT.java @@ -489,4 +489,113 @@ void postShape_missingDiscriminatorReturns400() { } } } + + @Nested + class Filters { + + String path = "/filters"; + + @Test + void postFilter_validStringValueReturns200() { + try (var server = newServer(Map.of("post-filter", new EchoHandler())); + var client = httpClient()) { + var body = "{\"value\":\"abcd\"}"; + var request = newRequest(server, path, "POST", ofString(body)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(200); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } + + @Test + void postFilter_validIntegerValueReturns200() { + try (var server = newServer(Map.of("post-filter", new EchoHandler())); + var client = httpClient()) { + var body = "{\"value\":42}"; + var request = newRequest(server, path, "POST", ofString(body)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(200); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } + + @Test + void postFilter_shortStringMatchesNoBranchReturns400() { + // "ab" has length < 3 (string branch fails) and is not an integer (integer branch fails). + try (var server = newServer(Map.of("post-filter", new EchoHandler())); + var client = httpClient()) { + var body = "{\"value\":\"ab\"}"; + var request = newRequest(server, path, "POST", ofString(body)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(400); + assertThat(response.headers().firstValue("Content-Type").orElse("")) + .contains("application/problem+json"); + assertThat(response.body()).contains("anyOf"); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } + } + + @Nested + class Blocked { + + String path = "/blocked"; + + @Test + void postBlocked_acceptedTokenReturns200() { + try (var server = newServer(Map.of("post-blocked", new EchoHandler())); + var client = httpClient()) { + var body = "{\"token\":\"allowed\"}"; + var request = newRequest(server, path, "POST", ofString(body)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(200); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } + + @Test + void postBlocked_forbiddenTokenReturns400() { + try (var server = newServer(Map.of("post-blocked", new EchoHandler())); + var client = httpClient()) { + var body = "{\"token\":\"forbidden\"}"; + var request = newRequest(server, path, "POST", ofString(body)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(400); + assertThat(response.headers().firstValue("Content-Type").orElse("")) + .contains("application/problem+json"); + assertThat(response.body()).contains("not"); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } + } } diff --git a/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java b/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java index 7f7365c..0f1247f 100644 --- a/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java +++ b/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java @@ -237,8 +237,8 @@ void multipleCombinatorsInOneSchemaWrapInAllOf() { @Test void parsesEmptySchemaAsPermissiveObject() { - // {} accepts anything that's an object; this is the JSON Schema 3.1 default - // and a behaviour change from the previous parser, which returned NullSchema. + // {} emits no type assertion (permissive ObjectSchema); JSON Schema 3.1 default. + // Behaviour change from the previous parser, which returned NullSchema. Schema s = SchemaParser.parse(Map.of()); assertThat(s).isInstanceOf(ObjectSchema.class); ObjectSchema obj = (ObjectSchema) s; diff --git a/src/test/resources/openapi.json b/src/test/resources/openapi.json index 522e1b8..c7fda7d 100644 --- a/src/test/resources/openapi.json +++ b/src/test/resources/openapi.json @@ -202,6 +202,54 @@ }, "responses": { "200": { "description": "OK" } } } + }, + "/filters": { + "post": { + "operationId": "post-filter", + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": ["value"], + "properties": { + "value": { + "anyOf": [ + { "type": "string", "minLength": 3 }, + { "type": "integer" } + ] + } + } + } + } + } + }, + "responses": { "200": { "description": "OK" } } + } + }, + "/blocked": { + "post": { + "operationId": "post-blocked", + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": ["token"], + "properties": { + "token": { + "type": "string", + "not": { "enum": ["forbidden"] } + } + } + } + } + } + }, + "responses": { "200": { "description": "OK" } } + } } }, "components": { diff --git a/src/test/resources/openapi.yaml b/src/test/resources/openapi.yaml index e46e4bb..65a121e 100644 --- a/src/test/resources/openapi.yaml +++ b/src/test/resources/openapi.yaml @@ -138,6 +138,45 @@ paths: "200": description: OK + /filters: + post: + operationId: post-filter + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [value] + properties: + value: + anyOf: + - type: string + minLength: 3 + - type: integer + responses: + "200": + description: OK + + /blocked: + post: + operationId: post-blocked + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [token] + properties: + token: + type: string + not: + enum: [forbidden] + responses: + "200": + description: OK + components: parameters: Name-Header: From 262b094a115cd710e7b2d3cfed69459278db5665 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 15:06:35 +0200 Subject: [PATCH 10/14] style: Rename combinator IT tests to camelCase Drop the underscore between operation and scenario in the new Shapes, Filters, and Blocked nested-class tests. Matches the project's camelCase test-name convention. Older underscore-style tests in this file are not touched. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../com/retailsvc/http/OpenApiServerIT.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/java/com/retailsvc/http/OpenApiServerIT.java b/src/test/java/com/retailsvc/http/OpenApiServerIT.java index de527da..ac492af 100644 --- a/src/test/java/com/retailsvc/http/OpenApiServerIT.java +++ b/src/test/java/com/retailsvc/http/OpenApiServerIT.java @@ -409,7 +409,7 @@ class Shapes { String path = "/shapes"; @Test - void postShape_validCircleReturns200() { + void postShapeValidCircleReturns200() { try (var server = newServer(Map.of("post-shape", new EchoHandler())); var client = httpClient()) { var body = "{\"kind\":\"circle\",\"radius\":2.5}"; @@ -428,7 +428,7 @@ void postShape_validCircleReturns200() { } @Test - void postShape_validSquareReturns200() { + void postShapeValidSquareReturns200() { try (var server = newServer(Map.of("post-shape", new EchoHandler())); var client = httpClient()) { var body = "{\"kind\":\"square\",\"side\":3}"; @@ -446,7 +446,7 @@ void postShape_validSquareReturns200() { } @Test - void postShape_unknownKindReturns400() { + void postShapeUnknownKindReturns400() { // matches zero branches: "kind" is neither "circle" nor "square". try (var server = newServer(Map.of("post-shape", new EchoHandler())); var client = httpClient()) { @@ -468,7 +468,7 @@ void postShape_unknownKindReturns400() { } @Test - void postShape_missingDiscriminatorReturns400() { + void postShapeMissingDiscriminatorReturns400() { // omitting "kind" makes both branches fail "required". try (var server = newServer(Map.of("post-shape", new EchoHandler())); var client = httpClient()) { @@ -496,7 +496,7 @@ class Filters { String path = "/filters"; @Test - void postFilter_validStringValueReturns200() { + void postFilterValidStringValueReturns200() { try (var server = newServer(Map.of("post-filter", new EchoHandler())); var client = httpClient()) { var body = "{\"value\":\"abcd\"}"; @@ -514,7 +514,7 @@ void postFilter_validStringValueReturns200() { } @Test - void postFilter_validIntegerValueReturns200() { + void postFilterValidIntegerValueReturns200() { try (var server = newServer(Map.of("post-filter", new EchoHandler())); var client = httpClient()) { var body = "{\"value\":42}"; @@ -532,7 +532,7 @@ void postFilter_validIntegerValueReturns200() { } @Test - void postFilter_shortStringMatchesNoBranchReturns400() { + void postFilterShortStringMatchesNoBranchReturns400() { // "ab" has length < 3 (string branch fails) and is not an integer (integer branch fails). try (var server = newServer(Map.of("post-filter", new EchoHandler())); var client = httpClient()) { @@ -560,7 +560,7 @@ class Blocked { String path = "/blocked"; @Test - void postBlocked_acceptedTokenReturns200() { + void postBlockedAcceptedTokenReturns200() { try (var server = newServer(Map.of("post-blocked", new EchoHandler())); var client = httpClient()) { var body = "{\"token\":\"allowed\"}"; @@ -578,7 +578,7 @@ void postBlocked_acceptedTokenReturns200() { } @Test - void postBlocked_forbiddenTokenReturns400() { + void postBlockedForbiddenTokenReturns400() { try (var server = newServer(Map.of("post-blocked", new EchoHandler())); var client = httpClient()) { var body = "{\"token\":\"forbidden\"}"; From 883e7f02a50d9bfeadeec45efe3650b46d4296ed Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 15:08:00 +0200 Subject: [PATCH 11/14] style: Rename legacy underscore tests in OpenApiServerIT to camelCase Strip the underscore between operation and scenario in the existing Data, ListObjects, QueryParams, and PathParams nested-class tests (12 methods). Brings the file in line with the project's pure-camelCase test-method convention. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../com/retailsvc/http/OpenApiServerIT.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/test/java/com/retailsvc/http/OpenApiServerIT.java b/src/test/java/com/retailsvc/http/OpenApiServerIT.java index ac492af..9cd18a5 100644 --- a/src/test/java/com/retailsvc/http/OpenApiServerIT.java +++ b/src/test/java/com/retailsvc/http/OpenApiServerIT.java @@ -31,7 +31,7 @@ class Data { String path = "/data"; @Test - void getData_shouldReturnJsonBody() { + void getDataShouldReturnJsonBody() { try (var server = newServer(Map.of("get-data", new GetDataHandler())); var client = httpClient()) { @@ -54,7 +54,7 @@ void getData_shouldReturnJsonBody() { } @Test - void getData_shouldReturnBadRequestOnInvalidXNameHeader() { + void getDataShouldReturnBadRequestOnInvalidXNameHeader() { try (var server = newServer(Map.of("get-data", new GetDataHandler())); var client = httpClient()) { @@ -80,7 +80,7 @@ void getData_shouldReturnBadRequestOnInvalidXNameHeader() { } @Test - void postData_shouldReturnJsonBody() { + void postDataShouldReturnJsonBody() { try (var server = newServer(Map.of("post-data", new EchoHandler())); var client = httpClient()) { @@ -129,7 +129,7 @@ void postData_shouldReturnJsonBody() { } @Test - void postData_shouldReturnBadRequestOnMissingRequiredProperties() { + void postDataShouldReturnBadRequestOnMissingRequiredProperties() { Map handlers = Map.of("post-data", new EchoHandler()); try (var server = newServer(handlers); @@ -187,7 +187,7 @@ class ListObjects { String path = "/list/objects"; @Test - void listObjects_shouldReturnJsonBody() { + void listObjectsShouldReturnJsonBody() { try (var server = newServer(Map.of("post-list-objects", new EchoHandler())); var client = httpClient()) { @@ -218,7 +218,7 @@ void listObjects_shouldReturnJsonBody() { } @Test - void listObjects_shouldReturnBadRequestOnPassingObjectInsteadOfArray() { + void listObjectsShouldReturnBadRequestOnPassingObjectInsteadOfArray() { try (var server = newServer(Map.of("post-list-objects", new EchoHandler())); var client = httpClient()) { @@ -252,7 +252,7 @@ class QueryParams { String path = "/params/query"; @Test - void getParamsQuery_shouldReturnOkOnValidQueryParams() { + void getParamsQueryShouldReturnOkOnValidQueryParams() { try (var server = newServer(Map.of("query-params", new EchoHandler())); var client = httpClient()) { @@ -276,7 +276,7 @@ void getParamsQuery_shouldReturnOkOnValidQueryParams() { } @Test - void paramsQuery_shouldReturnBadRequestOnMissingRequiredQueryParams() { + void paramsQueryShouldReturnBadRequestOnMissingRequiredQueryParams() { try (var server = newServer(Map.of("query-params", new EchoHandler())); var client = httpClient()) { @@ -309,7 +309,7 @@ class PathParams { String path = "/params/path"; @Test - void getPathParams_shouldReturnOkOnValidPathParam() { + void getPathParamsShouldReturnOkOnValidPathParam() { try (var server = newServer(Map.of("path-params", new EchoHandler())); var client = httpClient()) { @@ -332,7 +332,7 @@ void getPathParams_shouldReturnOkOnValidPathParam() { } @Test - void getPathParams_shouldReturnOkOnMultipleValidPathParams() { + void getPathParamsShouldReturnOkOnMultipleValidPathParams() { try (var server = newServer(Map.of("path-params-multi", new EchoHandler())); var client = httpClient()) { @@ -355,7 +355,7 @@ void getPathParams_shouldReturnOkOnMultipleValidPathParams() { } @Test - void getPathParams_shouldReturnBadRequestOnBadFormatPathParam() { + void getPathParamsShouldReturnBadRequestOnBadFormatPathParam() { try (var server = newServer(Map.of("path-params-multi", new EchoHandler())); var client = httpClient()) { @@ -382,7 +382,7 @@ void getPathParams_shouldReturnBadRequestOnBadFormatPathParam() { } @Test - void getPathParams_shouldReturnInternalErrorOnMissingHandler() { + void getPathParamsShouldReturnInternalErrorOnMissingHandler() { try (var server = newServer(Map.of("not-a-valid-operation-id", new EchoHandler())); var client = httpClient()) { From 29ba164d71c9a7c563c33dd5974c6d1742458d29 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 15:10:27 +0200 Subject: [PATCH 12/14] docs: Mark combinators plan tasks complete All three tasks plus the final verification checklist are done; flip every step in the plan to [x] to reflect the merged state of the branch. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-08-combinators-implementation.md | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/docs/superpowers/plans/2026-05-08-combinators-implementation.md b/docs/superpowers/plans/2026-05-08-combinators-implementation.md index f2ed7ff..98a3722 100644 --- a/docs/superpowers/plans/2026-05-08-combinators-implementation.md +++ b/docs/superpowers/plans/2026-05-08-combinators-implementation.md @@ -1,6 +1,6 @@ # Combinators Implementation Plan -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [x]`) syntax for tracking. **Goal:** Implement runtime validation for OpenAPI 3.1 combinators (`allOf`, `anyOf`, `oneOf`, `not`) and let combinators co-exist with sibling base assertions (`type`, `properties`, etc.) per JSON Schema 2020-12. @@ -22,7 +22,7 @@ ### Step 1.1: Write failing validator tests -- [ ] **Step 1: Add tests for combinator validation** +- [x] **Step 1: Add tests for combinator validation** Replace the existing `combinatorThrowsUnsupported` test in `src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java` with the following block (delete the old test, append the new ones at the end of the class). Keep the existing imports and add: `AllOfSchema`, `AnyOfSchema`, `NotSchema`, `StringSchema`. @@ -124,7 +124,7 @@ void notFailsWhenInnerPasses() { } ``` -- [ ] **Step 2: Run the failing tests** +- [x] **Step 2: Run the failing tests** Run: `mvn -q test -Dtest=DefaultValidatorDispatchTest` @@ -132,7 +132,7 @@ Expected: 9 new tests fail with `UnsupportedOperationException` (and the old `co ### Step 1.2: Replace the four UOE branches -- [ ] **Step 3: Edit `DefaultValidator.validate(...)`** +- [x] **Step 3: Edit `DefaultValidator.validate(...)`** Open `src/main/java/com/retailsvc/http/validate/DefaultValidator.java`. Replace these four lines: @@ -200,19 +200,19 @@ private void validateNot(Object value, Schema inner, String pointer) { } ``` -- [ ] **Step 4: Run the unit tests** +- [x] **Step 4: Run the unit tests** Run: `mvn -q test -Dtest=DefaultValidatorDispatchTest` Expected: all tests in the class pass (the original 4 + the 9 new ones). -- [ ] **Step 5: Run the full unit test suite** +- [x] **Step 5: Run the full unit test suite** Run: `mvn -q test` Expected: BUILD SUCCESS, 119 + 9 = 128 tests pass (or whatever count matches; no failures). -- [ ] **Step 6: Commit** +- [x] **Step 6: Commit** ```bash git add src/main/java/com/retailsvc/http/validate/DefaultValidator.java \ @@ -241,7 +241,7 @@ EOF ### Step 2.1: Add failing parser tests -- [ ] **Step 1: Add tests covering the composition path** +- [x] **Step 1: Add tests covering the composition path** Append to `src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java` (inside the class, before the final `}`): @@ -354,7 +354,7 @@ void aloneCombinatorStillReturnsCombinatorRecord() { } ``` -- [ ] **Step 2: Run the new parser tests (expect failures)** +- [x] **Step 2: Run the new parser tests (expect failures)** Run: `mvn -q test -Dtest=SchemaParserTest` @@ -362,7 +362,7 @@ Expected: the 7 new tests fail. The composition tests fail because `parse(...)` ### Step 2.2: Rewrite the parser dispatch -- [ ] **Step 3: Replace the body of `SchemaParser.parse(Map)`** +- [x] **Step 3: Replace the body of `SchemaParser.parse(Map)`** Open `src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java`. Replace the entire `parse` method (currently lines 15–55) with: @@ -467,19 +467,19 @@ The key behaviour changes: - The four combinator keywords are then each appended to the assertions list. `allOf` is flattened (its branches join the outer list directly), the others wrap. - A single assertion returns unwrapped; two or more wrap in `AllOfSchema`. -- [ ] **Step 4: Run the parser tests** +- [x] **Step 4: Run the parser tests** Run: `mvn -q test -Dtest=SchemaParserTest` Expected: all `SchemaParserTest` tests pass, including the 7 new ones. -- [ ] **Step 5: Run the full unit suite** +- [x] **Step 5: Run the full unit suite** Run: `mvn -q test` Expected: BUILD SUCCESS, no regressions in `ContainerSchemasTest`, `PrimitiveSchemasTest`, `CombinatorScaffoldTest`, etc. -- [ ] **Step 6: Commit** +- [x] **Step 6: Commit** ```bash git add src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java \ @@ -515,7 +515,7 @@ EOF The fixture has stub `/anyOf` and `/allOf` paths with empty `post: {}`. Replace them with a real `/oneOf` route exercising a polymorphic body. -- [ ] **Step 1: Replace the stub paths in `openapi.yaml`** +- [x] **Step 1: Replace the stub paths in `openapi.yaml`** Open `src/test/resources/openapi.yaml`. Find the lines: @@ -560,7 +560,7 @@ Replace with: description: OK ``` -- [ ] **Step 2: Mirror the change in `openapi.json`** +- [x] **Step 2: Mirror the change in `openapi.json`** Open `src/test/resources/openapi.json`. Locate the `"/anyOf"` and `"/allOf"` keys (lines ~173 and ~178). Replace both blocks with a single `"/shapes"` entry equivalent to the YAML above. Use the existing JSON formatting (2-space indent, `application/json` media type). Worked example: @@ -604,7 +604,7 @@ Make sure the trailing comma on the previous path entry (or this one) is correct ### Step 3.2: Add a test handler -- [ ] **Step 3: Create `PolymorphicHandler.java`** +- [x] **Step 3: Create `PolymorphicHandler.java`** Create `src/test/java/com/retailsvc/http/start/PolymorphicHandler.java`: @@ -632,7 +632,7 @@ public class PolymorphicHandler implements HttpHandler { ### Step 3.3: Add the integration test -- [ ] **Step 4: Add tests to `OpenApiServerIT.java`** +- [x] **Step 4: Add tests to `OpenApiServerIT.java`** Open `src/test/java/com/retailsvc/http/OpenApiServerIT.java`. Add a new nested class at the bottom of the outer class (immediately before the final `}` of `OpenApiServerIT`): @@ -724,7 +724,7 @@ class Shapes { The `ofString`, `BodyHandlers`, and `assertThat` imports are already present at the top of `OpenApiServerIT.java`; no new imports needed beyond `org.junit.jupiter.api.Nested` (also already present) and `java.io.IOException` (already imported). -- [ ] **Step 5: Run the integration tests** +- [x] **Step 5: Run the integration tests** Run: `mvn -q verify -DfailIfNoTests=false -Dtest='!*' -Dit.test=OpenApiServerIT` @@ -732,13 +732,13 @@ Or simply: `mvn -q verify` Expected: BUILD SUCCESS. The four new IT cases all pass. -- [ ] **Step 6: Run the full verify** +- [x] **Step 6: Run the full verify** Run: `mvn -q verify` Expected: BUILD SUCCESS. All previous unit tests + the 9 new validator tests + the 7 new parser tests + the 4 new IT tests pass. -- [ ] **Step 7: Commit** +- [x] **Step 7: Commit** ```bash git add src/test/resources/openapi.yaml src/test/resources/openapi.json \ @@ -762,9 +762,9 @@ EOF ## Final verification -- [ ] Run `mvn -q verify` once more. -- [ ] `git log --oneline master..HEAD` shows three new commits in this order: `feat: Implement combinator validation…`, `feat: Compose combinators with sibling…`, `test: Add polymorphic-body integration…`. -- [ ] No new files exist outside the paths listed above. +- [x] Run `mvn -q verify` once more. +- [x] `git log --oneline master..HEAD` shows three new commits in this order: `feat: Implement combinator validation…`, `feat: Compose combinators with sibling…`, `test: Add polymorphic-body integration…`. +- [x] No new files exist outside the paths listed above. ## Out of scope (do not do) From f5def80f347e135cb658d6d5ed000f51efd3ed11 Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 16:04:38 +0200 Subject: [PATCH 13/14] test: Pin SchemaParser corner cases Add five tests for parser semantics that previously survived only by code inspection: $ref ignores siblings, nested allOf is not auto- flattened past one level, const acts as a base assertion when siblings exist, not: {} wraps the permissive object, and combinator branches recurse through parse() so nested combinators stay intact. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../http/spec/schema/SchemaParserTest.java | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java b/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java index 0f1247f..91b2060 100644 --- a/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java +++ b/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java @@ -288,4 +288,81 @@ void aloneCombinatorStillReturnsCombinatorRecord() { assertThat(s).isInstanceOf(OneOfSchema.class); assertThat(((OneOfSchema) s).options()).hasSize(2); } + + @Test + void refWithSiblingsIsParsedSolo() { + // Deliberate limitation: $ref returns immediately and ignores sibling keywords. + // JSON Schema 2020-12 allows $ref + siblings but that interaction is a separate gap. + Schema s = + SchemaParser.parse( + Map.of( + "$ref", "#/components/schemas/Foo", + "type", "string", + "minLength", 5)); + assertThat(s).isInstanceOf(RefSchema.class); + assertThat(((RefSchema) s).pointer()).isEqualTo("#/components/schemas/Foo"); + } + + @Test + void nestedAllOfIsNotFlattenedWhenBasePresent() { + // Flattening is one-level: when a base assertion plus an outer allOf coexist, + // the outer allOf branches are pulled into the assertions list, but any + // allOf nested inside one of those branches stays wrapped. + Schema s = + SchemaParser.parse( + Map.of( + "type", + "object", + "allOf", + List.of( + Map.of( + "allOf", List.of(Map.of("type", "string"), Map.of("type", "integer")))))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + assertThat(all.parts()).hasSize(2); + assertThat(all.parts().get(0)).isInstanceOf(ObjectSchema.class); + assertThat(all.parts().get(1)).isInstanceOf(AllOfSchema.class); + assertThat(((AllOfSchema) all.parts().get(1)).parts()).hasSize(2); + } + + @Test + void constWithSiblingAllOfWrapsInImplicitAllOf() { + // const acts as a base assertion, so a sibling combinator wraps both in AllOf. + Schema s = + SchemaParser.parse( + Map.of("const", 5, "allOf", List.of(Map.of("type", "integer", "minimum", 0)))); + assertThat(s).isInstanceOf(AllOfSchema.class); + AllOfSchema all = (AllOfSchema) s; + assertThat(all.parts()).hasSize(2); + assertThat(all.parts().get(0)).isInstanceOf(ConstSchema.class); + assertThat(((ConstSchema) all.parts().get(0)).value()).isEqualTo(5); + assertThat(all.parts().get(1)).isInstanceOf(IntegerSchema.class); + } + + @Test + void notWithEmptyInnerSchemaWrapsPermissiveObject() { + // not: {} parses the inner empty schema as the permissive ObjectSchema. + Schema s = SchemaParser.parse(Map.of("not", Map.of())); + assertThat(s).isInstanceOf(NotSchema.class); + assertThat(((NotSchema) s).schema()).isInstanceOf(ObjectSchema.class); + } + + @Test + void oneOfContainingNestedAnyOfRecurses() { + // Pins that combinator branches are themselves passed through parse(), + // so nested combinators survive intact. + Schema s = + SchemaParser.parse( + Map.of( + "oneOf", + List.of( + Map.of("anyOf", List.of(Map.of("type", "string"), Map.of("type", "integer"))), + Map.of("type", "boolean")))); + assertThat(s).isInstanceOf(OneOfSchema.class); + OneOfSchema one = (OneOfSchema) s; + assertThat(one.options()).hasSize(2); + assertThat(one.options().get(0)).isInstanceOf(AnyOfSchema.class); + assertThat(((AnyOfSchema) one.options().get(0)).options()).hasSize(2); + assertThat(one.options().get(1)).isInstanceOf(BooleanSchema.class); + } } From d956ebb1b732392e2051dc1c22c4f770645ec64b Mon Sep 17 00:00:00 2001 From: Thomas Cederholm Date: Fri, 8 May 2026 16:08:56 +0200 Subject: [PATCH 14/14] test: Cover implicit object and array dispatch in parseBaseIfPresent --- .../http/spec/schema/SchemaParserTest.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java b/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java index 91b2060..d953810 100644 --- a/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java +++ b/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java @@ -347,6 +347,32 @@ void notWithEmptyInnerSchemaWrapsPermissiveObject() { assertThat(((NotSchema) s).schema()).isInstanceOf(ObjectSchema.class); } + @Test + void parsesImplicitObjectFromShapeKeywords() { + // Schema with object-shape keywords but no explicit type still produces + // an ObjectSchema (parseBaseIfPresent's implicit-object branch). + Schema s = + SchemaParser.parse( + Map.of("required", List.of("x"), "properties", Map.of("x", Map.of("type", "string")))); + assertThat(s).isInstanceOf(ObjectSchema.class); + ObjectSchema obj = (ObjectSchema) s; + assertThat(obj.types()).isEmpty(); + assertThat(obj.required()).containsExactly("x"); + assertThat(obj.properties().get("x")).isInstanceOf(StringSchema.class); + } + + @Test + void parsesImplicitArrayFromShapeKeywords() { + // Schema with array-shape keywords but no explicit type still produces + // an ArraySchema (parseBaseIfPresent's implicit-array branch). + Schema s = SchemaParser.parse(Map.of("items", Map.of("type", "integer"), "minItems", 1)); + assertThat(s).isInstanceOf(ArraySchema.class); + ArraySchema arr = (ArraySchema) s; + assertThat(arr.types()).isEmpty(); + assertThat(arr.items()).isInstanceOf(IntegerSchema.class); + assertThat(arr.minItems()).isEqualTo(1); + } + @Test void oneOfContainingNestedAnyOfRecurses() { // Pins that combinator branches are themselves passed through parse(),