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..98a3722 --- /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 (`- [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. + +**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 + +- [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`. + +```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"); +} +``` + +- [x] **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 + +- [x] **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); +} +``` + +- [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). + +- [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). + +- [x] **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 + +- [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 `}`): + +```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); +} +``` + +- [x] **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 + +- [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: + +```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`. + +- [x] **Step 4: Run the parser tests** + +Run: `mvn -q test -Dtest=SchemaParserTest` + +Expected: all `SchemaParserTest` tests pass, including the 7 new ones. + +- [x] **Step 5: Run the full unit suite** + +Run: `mvn -q test` + +Expected: BUILD SUCCESS, no regressions in `ContainerSchemasTest`, `PrimitiveSchemasTest`, `CombinatorScaffoldTest`, etc. + +- [x] **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. + +- [x] **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 +``` + +- [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: + +```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 + +- [x] **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 + +- [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`): + +```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). + +- [x] **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. + +- [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. + +- [x] **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 + +- [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) + +- 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. 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`. 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..23c9aa6 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,18 @@ public static Schema parse(Map raw) { } Set types = parseTypes(raw); + if (types.isEmpty() && !hasObjectShapeKeywords(raw) && !hasArrayShapeKeywords(raw)) { + return null; + } + if (types.isEmpty() && hasObjectShapeKeywords(raw)) { + return parseObject(raw, types); + } + if (types.isEmpty() && hasArrayShapeKeywords(raw)) { + return parseArray(raw, types); + } - // Pick primary (non-null) type for record dispatch. 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); @@ -54,6 +78,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/main/java/com/retailsvc/http/validate/DefaultValidator.java b/src/main/java/com/retailsvc/http/validate/DefaultValidator.java index 2516044..ce810b9 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) { + // branch did not match; continue + } + } + 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/OpenApiServerIT.java b/src/test/java/com/retailsvc/http/OpenApiServerIT.java index 0545093..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()) { @@ -402,4 +402,200 @@ void getPathParams_shouldReturnInternalErrorOnMissingHandler() { } } } + + @Nested + class Shapes { + + String path = "/shapes"; + + @Test + void postShapeValidCircleReturns200() { + 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)); + + var response = client.send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(200); + assertThat(response.body()).contains("\"kind\":\"circle\""); + } catch (IOException e) { + fail(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail(e); + } + } + + @Test + void postShapeValidSquareReturns200() { + 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)); + + 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 postShapeUnknownKindReturns400() { + // matches zero branches: "kind" is neither "circle" nor "square". + 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)); + + 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 postShapeMissingDiscriminatorReturns400() { + // omitting "kind" makes both branches fail "required". + 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)); + + 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); + } + } + } + + @Nested + class Filters { + + String path = "/filters"; + + @Test + void postFilterValidStringValueReturns200() { + 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 postFilterValidIntegerValueReturns200() { + 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 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()) { + 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 postBlockedAcceptedTokenReturns200() { + 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 postBlockedForbiddenTokenReturns400() { + 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 5ff804a..d953810 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,241 @@ 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 parsesEmptySchemaAsPermissiveObject() { + // {} 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; + 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 = + 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); + 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 + 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); + } + + @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 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(), + // 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); + } } diff --git a/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java b/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java index 8ae8a97..1b2d8bd 100644 --- a/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java +++ b/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java @@ -1,11 +1,16 @@ 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; +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 +48,128 @@ 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(); + assertThat(err.keyword()).isEqualTo("oneOf"); + 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(); + assertThat(err.keyword()).isEqualTo("oneOf"); + 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"); + } + + @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"); } } diff --git a/src/test/resources/openapi.json b/src/test/resources/openapi.json index 9fe38d6..c7fda7d 100644 --- a/src/test/resources/openapi.json +++ b/src/test/resources/openapi.json @@ -170,14 +170,85 @@ } } }, - "/anyOf": { + "/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" } } } }, - "/allOf": { + "/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" } } } } }, diff --git a/src/test/resources/openapi.yaml b/src/test/resources/openapi.yaml index de5f184..65a121e 100644 --- a/src/test/resources/openapi.yaml +++ b/src/test/resources/openapi.yaml @@ -109,11 +109,73 @@ paths: "200": description: OK - /anyOf: - 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 - /allOf: - post: {} + /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: