diff --git a/README.md b/README.md index 74f397b3..30e9bc42 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,7 @@ endpoints declared in an OpenAPI 3.1.x specification. Handlers are pure function - [Highlights](#highlights) - [Maven artifact](#maven-artifact) - [Quick start](#quick-start) + - [Multiple specs](#multiple-specs) - [Spec loading](#spec-loading) - [JSON mapping](#json-mapping) - [Body parsers and response writers](#body-parsers-and-response-writers) @@ -160,6 +161,46 @@ public class YourServerLauncher { } ``` +### Multiple specs + +A single server instance can host more than one OpenAPI spec — useful for running a v1/v2 API +side-by-side, or a public and an internal admin surface in the same process. Each `addSpec()` call +registers one spec and its handlers as an independent binding. The JDK `HttpServer` routes +incoming requests to the binding whose `basePath` (the path component of `servers[0].url`) best +matches the request URI. + +`operationId`s and security-scheme names only need to be unique _within_ a single spec — two +specs can each declare a `getCustomer` operation without conflict. + +``` java +OpenApiServer server = OpenApiServer.builder() + .port(8080) + .addSpec(v1Spec, Map.of( + "getCustomer", new V1GetCustomer(), + "listCustomers", new V1ListCustomers())) + .addSpec(v2Spec, Map.of( + "getCustomer", new V2GetCustomer(), + "listCustomers", new V2ListCustomers(), + "createCustomer", new V2CreateCustomer())) + .build(); +``` + +Each spec that declares `securitySchemes` gets its own validator map via the three-argument +overload: + +``` java +OpenApiServer.builder() + .port(8080) + .addSpec(v1Spec, v1Handlers, Map.of( + "bearerAuth", (req, cred) -> jwt.verify(((BearerCredential) cred).token()))) + .addSpec(v2Spec, v2Handlers, Map.of( + "apiKeyAuth", (req, cred) -> apiKeyStore.lookup(((ApiKeyCredential) cred).value()))) + .build(); +``` + +Mixing `addSpec()` with the legacy `spec()`/`handlers()`/`securityValidator()` methods in the +same builder call is rejected at build time with `IllegalStateException`. + ## Spec loading `Spec.fromClasspath(Class, String)` is the recommended way to load a spec packaged with your diff --git a/docs/superpowers/plans/2026-05-22-multiple-specs.md b/docs/superpowers/plans/2026-05-22-multiple-specs.md new file mode 100644 index 00000000..65e018ee --- /dev/null +++ b/docs/superpowers/plans/2026-05-22-multiple-specs.md @@ -0,0 +1,860 @@ +# Multiple OpenAPI specs — 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:** Allow `OpenApiServer` to serve more than one `Spec` on a single process, with per-spec scoping of `operationId`s and security-scheme names, while keeping the existing single-spec Builder API source-compatible. + +**Architecture:** Each registered spec becomes a package-private `SpecBinding` record (spec + handlers + validators + derived `DefaultValidator` + `Router`). At runtime, the server creates one `HttpContext` per binding at the spec's `basePath`, with a filter chain wired against that binding's maps. JDK `HttpServer`'s longest-prefix-match handles routing across bindings. The legacy single-spec Builder methods stay and synthesise one implicit `addSpec()` bundle at `build()` time; mixing them with explicit `addSpec()` calls is rejected. + +**Tech Stack:** Java 25, `com.sun.net.httpserver.HttpServer`, JUnit 5, AssertJ, Mockito. Maven build (`mvn test` / `mvn verify`). + +**Spec reference:** `docs/superpowers/specs/2026-05-22-multiple-specs-design.md`. + +--- + +## File structure + +**New files:** + +- `src/main/java/com/retailsvc/http/internal/SpecBinding.java` — package-private record bundling one spec's runtime state. +- `src/test/java/com/retailsvc/http/MultiSpecServerTest.java` — unit tests for the new multi-spec Builder behaviour. +- `src/test/java/com/retailsvc/http/MultiSpecServerIT.java` — integration test mounting two derived specs at different base paths under a real `HttpServer`. +- `src/test/java/com/retailsvc/http/support/SpecFixtures.java` — small test helper that reads `openapi.json`, deep-clones the parsed `Map`, mutates `servers[0].url`, and returns a `Spec`. Reused across the new tests; **no new OpenAPI fixture files**. + +**Modified files:** + +- `src/main/java/com/retailsvc/http/OpenApiServer.java` — constructor takes `List` instead of single `Spec`/handlers/validators; one `HttpContext` per binding; cross-binding basePath conflict check; Builder gets `addSpec()` plus synthesis logic. + +No other production files need to change. Existing filters (`RequestPreparationFilter`, `SecurityFilter`, `DispatchHandler`, `ExceptionFilter`) keep their current constructors — we instantiate one per binding instead of one per server. + +--- + +## Phased approach + +The plan is split into two phases to keep risk small: + +- **Phase 1 — Internal refactor (no public API change):** introduce `SpecBinding`, change `OpenApiServer`'s constructor to a `List` of size 1, keep the existing Builder surface untouched. All 440 existing tests must continue passing without modification. +- **Phase 2 — Multi-spec public API:** add `addSpec()` builder methods, synthesis-of-legacy-calls, cross-binding validation, new tests. + +Each phase ends green (compile + tests pass) and is committable on its own. + +--- + +## Phase 1 — Internal refactor + +### Task 1: Introduce `SpecBinding` record + +**Files:** +- Create: `src/main/java/com/retailsvc/http/internal/SpecBinding.java` + +- [x] **Step 1: Write `SpecBinding`** + +```java +package com.retailsvc.http.internal; + +import com.retailsvc.http.RequestHandler; +import com.retailsvc.http.SchemeValidator; +import com.retailsvc.http.spec.Spec; +import com.retailsvc.http.validate.DefaultValidator; +import java.util.Map; + +/** + * Bundles everything the server needs to serve one OpenAPI spec: the spec itself, the handler map + * keyed by {@code operationId}, the security-scheme validator map, and the derived {@link Router} + * and {@link DefaultValidator}. One {@code SpecBinding} drives exactly one {@code HttpContext}. + */ +public record SpecBinding( + Spec spec, + Map handlers, + Map securityValidators, + DefaultValidator validator, + Router router) { + + public SpecBinding { + handlers = Map.copyOf(handlers); + securityValidators = Map.copyOf(securityValidators); + } + + /** Builds a binding by deriving the {@link Router} and {@link DefaultValidator} from the spec. */ + public static SpecBinding of( + Spec spec, + Map handlers, + Map securityValidators) { + return new SpecBinding( + spec, + handlers, + securityValidators, + new DefaultValidator(spec::resolveSchema), + new Router(spec.operations())); + } +} +``` + +- [x] **Step 2: Verify compilation** + +Run: `mvn -q test-compile` +Expected: BUILD SUCCESS (the record isn't used yet). + +- [x] **Step 3: Commit** + +```bash +git add src/main/java/com/retailsvc/http/internal/SpecBinding.java +git commit -m "refactor: Add SpecBinding record for per-spec server state" +``` + +--- + +### Task 2: Rewire `OpenApiServer` constructor around a single-element `List` + +**Files:** +- Modify: `src/main/java/com/retailsvc/http/OpenApiServer.java` + +The goal here is purely structural: the constructor stops taking a `Spec` and instead takes `List`. The Builder still produces a list of size 1. No public API change yet. + +- [x] **Step 1: Update `OpenApiServer` constructor signature and body** + +Replace the constructor (currently lines 73–157) with a version that loops over bindings: + +```java +OpenApiServer( + java.util.List bindings, + Map bodyMappers, + HandlerConfig handlerConfig, + int port, + InetAddress bindAddress, + int shutdownTimeoutSeconds, + SSLContext sslContext) + throws IOException { + + requireNonNull(bindings, "bindings must not be null"); + if (bindings.isEmpty()) { + throw new IllegalStateException("at least one spec binding is required"); + } + requireNonNull(bodyMappers, "bodyMappers must not be null"); + ExceptionHandler exceptionHandler = handlerConfig.exceptionHandler(); + + long t0 = System.currentTimeMillis(); + + InetSocketAddress socketAddress = + (bindAddress == null) + ? new InetSocketAddress(port) + : new InetSocketAddress(bindAddress, port); + if (sslContext != null) { + HttpsServer https = HttpsServer.create(socketAddress, 0); + https.setHttpsConfigurator(new TlsHttpsConfigurator(sslContext)); + this.httpServer = https; + } else { + this.httpServer = HttpServer.create(socketAddress, 0); + } + httpServer.setExecutor(newThreadPerTaskExecutor(ofVirtual().name("http-", 0).factory())); + + ResponseRenderer renderer = new ResponseRenderer(bodyMappers); + + boolean anyBindingAtRoot = false; + for (SpecBinding binding : bindings) { + String basePath = Optional.ofNullable(binding.spec().basePath()).orElse("/"); + anyBindingAtRoot |= "/".equals(basePath); + Map operationsById = + binding.spec().operations().stream() + .collect(Collectors.toUnmodifiableMap(Operation::operationId, op -> op)); + HttpContext ctx = httpServer.createContext(basePath); + ctx.getFilters() + .add( + new RequestPreparationFilter( + binding.spec(), + binding.router(), + binding.validator(), + bodyMappers, + exceptionHandler, + renderer, + handlerConfig.afterHooks())); + ctx.getFilters() + .add( + new SecurityFilter( + operationsById, + binding.spec().securitySchemes(), + binding.spec().security(), + binding.securityValidators(), + handlerConfig.externalAuth())); + ctx.setHandler( + new DispatchHandler( + binding.handlers(), + handlerConfig.interceptors(), + handlerConfig.decorators(), + renderer)); + } + + for (Map.Entry e : handlerConfig.extras().entrySet()) { + HttpContext extraCtx = httpServer.createContext(e.getKey()); + extraCtx.getFilters().add(new ExceptionFilter(exceptionHandler, renderer)); + extraCtx.setHandler(new ExtraRouteAdapter(e.getValue(), renderer)); + } + + if (!anyBindingAtRoot) { + httpServer.createContext("/", new NotFoundHandler()); + } + httpServer.start(); + + this.shutdownTimeoutSeconds = shutdownTimeoutSeconds; + + String host = httpServer.getAddress().getHostString(); + String displayHost = host.contains(":") ? "[" + host + "]" : host; + LOG.info( + "Server started ({}:{}) in {}ms", + displayHost, + httpServer.getAddress().getPort(), + System.currentTimeMillis() - t0); +} +``` + +- [x] **Step 2: Drop the now-unused `handlers` field from `HandlerConfig`** + +In `OpenApiServer.java`, change the `HandlerConfig` record to remove `handlers` (it's per-binding now): + +```java +record HandlerConfig( + List interceptors, + List decorators, + ExceptionHandler exceptionHandler, + Map extras, + boolean externalAuth, + List afterHooks) {} +``` + +Also drop the per-binding `securityValidators` from `HandlerConfig` (they live on each `SpecBinding` now). Update every callsite of `HandlerConfig` accordingly. + +- [x] **Step 3: Update `Builder.build()` to construct one binding and pass `List.of(binding)`** + +In `Builder.build()` (currently lines 364–402), replace the construction of `HandlerConfig` and the call to `new OpenApiServer(...)` with: + +```java +public OpenApiServer build() throws IOException { + requireNonNull(spec, "Spec must not be null"); + requireNonNull(handlers, "handlers must not be null"); + String basePath = Optional.ofNullable(spec.basePath()).orElse("/"); + for (String path : extras.keySet()) { + if (path.equals(basePath)) { + throw new IllegalStateException( + "extra handler path " + path + " conflicts with spec basePath " + basePath); + } + } + if (!externalAuth) { + validateSecurityWiring(spec, securityValidators); + } + validateHandlerWiring(spec, handlers); + Map resolved = resolveBodyMappers(bodyMappers); + ExceptionHandler effectiveExceptionHandler = + exceptionHandler != null ? exceptionHandler : Handlers.defaultExceptionHandler(); + HandlerConfig handlerConfig = + new HandlerConfig( + interceptors, + decorators, + effectiveExceptionHandler, + extras, + externalAuth, + List.copyOf(afterHooks)); + SpecBinding binding = SpecBinding.of(spec, handlers, securityValidators); + int resolvedPort = resolvePort(); + SSLContext sslContext = + httpsCertChain != null ? PemSslContext.load(httpsCertChain, httpsPrivateKey) : null; + return new OpenApiServer( + java.util.List.of(binding), + resolved, + handlerConfig, + resolvedPort, + bindAddress, + shutdownTimeoutSeconds, + sslContext); +} +``` + +- [x] **Step 4: Run the full unit test suite** + +Run: `mvn -q test` +Expected: BUILD SUCCESS. All 440 existing tests pass with no edits. (If anything fails, it's a refactoring mistake — fix before moving on.) + +- [x] **Step 5: Run the integration test suite** + +Run: `mvn -q verify -DskipITs=false` +Expected: BUILD SUCCESS. + +- [x] **Step 6: Commit** + +```bash +git add src/main/java/com/retailsvc/http/OpenApiServer.java +git commit -m "refactor: Drive OpenApiServer from a list of SpecBinding" +``` + +--- + +## Phase 2 — Public multi-spec API + +### Task 3: Test fixture helper for deriving specs without new resource files + +**Files:** +- Create: `src/test/java/com/retailsvc/http/support/SpecFixtures.java` + +This avoids adding any new `openapi*.json|yaml` files — per project memory, fixtures are minimised. The helper reads the existing `/openapi.json`, deep-clones the parsed `Map`, and lets a caller override `servers[0].url`. + +- [x] **Step 1: Write the helper** + +Created with proper imports (no inline FQNs per `feedback_no_inline_fqn.md`) and deep-clone logic. + +- [x] **Step 2: Verify it compiles** + +Ran: `mvn -q test-compile` +Result: BUILD SUCCESS. Compiled class at `target/test-classes/com/retailsvc/http/support/SpecFixtures.class`. + +- [x] **Step 3: Commit** + +``` +git add src/test/java/com/retailsvc/http/support/SpecFixtures.java +git commit -m "test: Add SpecFixtures helper to derive multiple specs from one fixture" +``` + +Committed as: `abe2369` (Google Java Formatter reformatted javadoc wrapping; pre-commit hooks passed). + +--- + +### Task 4: Failing test — `addSpec()` accepts multiple bindings with distinct base paths + +**Files:** +- Create: `src/test/java/com/retailsvc/http/MultiSpecServerTest.java` + +- [x] **Step 1: Write the failing test** + +```java +package com.retailsvc.http; + +import static java.net.HttpURLConnection.HTTP_OK; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.retailsvc.http.spec.Spec; +import com.retailsvc.http.support.SpecFixtures; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class MultiSpecServerTest { + + @Test + void servesTwoBindingsOnDistinctBasePaths() throws Exception { + Spec v1 = SpecFixtures.specAt("http://localhost/api/v1"); + Spec v2 = SpecFixtures.specAt("http://localhost/api/v2"); + + RequestHandler v1Handler = req -> Response.ok(Map.of("version", "v1")); + RequestHandler v2Handler = req -> Response.ok(Map.of("version", "v2")); + + Map v1Handlers = handlersFor(v1, v1Handler); + Map v2Handlers = handlersFor(v2, v2Handler); + + try (OpenApiServer server = + OpenApiServer.builder() + .port(0) + .addSpec(v1, v1Handlers) + .addSpec(v2, v2Handlers) + .useExternalAuthentication() + .build()) { + + int port = server.listenPort(); + assertThat(get("http://localhost:" + port + "/api/v1/ping").statusCode()).isEqualTo(HTTP_OK); + assertThat(get("http://localhost:" + port + "/api/v2/ping").statusCode()).isEqualTo(HTTP_OK); + } + } + + private static Map handlersFor(Spec spec, RequestHandler shared) { + java.util.LinkedHashMap out = new java.util.LinkedHashMap<>(); + spec.operations().forEach(op -> out.put(op.operationId(), shared)); + return out; + } + + private static HttpResponse get(String url) throws Exception { + return HttpClient.newHttpClient() + .send( + HttpRequest.newBuilder(URI.create(url)).GET().build(), + HttpResponse.BodyHandlers.ofString()); + } +} +``` + +Note: the test uses `useExternalAuthentication()` to side-step per-spec validator wiring — the security branch is covered separately in Task 8. It uses the existing `/ping` operation from `openapi.json` (which is GET-only and unsecured per the fixture). If the fixture's `/ping` requires auth, switch to the first GET-only unsecured operation in the spec — discoverable by inspecting `src/test/resources/openapi.json`. + +- [x] **Step 2: Run the test to verify it fails** + +Run: `mvn -q test -Dtest=MultiSpecServerTest#servesTwoBindingsOnDistinctBasePaths` +Expected: COMPILE FAILURE — `addSpec` does not yet exist on `Builder`. + +- [x] **Step 3: Do NOT commit yet** — proceed to Task 5 to make it pass. + +--- + +### Task 5: Add `addSpec()` builder methods + +**Files:** +- Modify: `src/main/java/com/retailsvc/http/OpenApiServer.java` + +- [x] **Step 1: Add bindings list and `addSpec` methods to `Builder`** + +Inside `Builder`, alongside the existing fields, add: + +```java +private final java.util.List bindings = new java.util.ArrayList<>(); + +/** + * Registers an OpenAPI {@link Spec} with the handlers and security validators that serve it. May + * be called more than once; each binding becomes its own {@link com.sun.net.httpserver.HttpContext} + * at the spec's {@code basePath}. {@code operationId}s and security-scheme names only need to be + * unique within a single spec. + */ +public Builder addSpec( + Spec spec, + Map handlers, + Map securityValidators) { + requireNonNull(spec, "spec must not be null"); + requireNonNull(handlers, "handlers must not be null"); + requireNonNull(securityValidators, "securityValidators must not be null"); + bindings.add(SpecBinding.of(spec, handlers, securityValidators)); + return this; +} + +/** Convenience overload for specs that declare no security schemes. */ +public Builder addSpec(Spec spec, Map handlers) { + return addSpec(spec, handlers, java.util.Map.of()); +} +``` + +- [x] **Step 2: Update `build()` to consume bindings** + +Replace the `build()` method body so it either: +- uses the explicit `bindings` list when `addSpec()` was called, OR +- synthesises a single binding from the legacy `spec`/`handlers`/`securityValidators` fields. + +Mixing both forms is rejected. Full replacement of `build()`: + +```java +public OpenApiServer build() throws IOException { + boolean usedLegacy = spec != null || handlers != null; + boolean usedAddSpec = !bindings.isEmpty(); + if (usedLegacy && usedAddSpec) { + throw new IllegalStateException( + "use either spec()/handler()/securityValidator() or addSpec(), not both"); + } + java.util.List effectiveBindings; + if (usedAddSpec) { + effectiveBindings = java.util.List.copyOf(bindings); + } else { + requireNonNull(spec, "Spec must not be null"); + requireNonNull(handlers, "handlers must not be null"); + effectiveBindings = java.util.List.of(SpecBinding.of(spec, handlers, securityValidators)); + } + + // Per-binding wiring validation. + for (SpecBinding b : effectiveBindings) { + if (!externalAuth) { + validateSecurityWiring(b.spec(), b.securityValidators()); + } + validateHandlerWiring(b.spec(), b.handlers()); + } + + // Cross-binding: duplicate basePath rejection. + java.util.Map seenBasePaths = new java.util.LinkedHashMap<>(); + for (SpecBinding b : effectiveBindings) { + String bp = Optional.ofNullable(b.spec().basePath()).orElse("/"); + String existingTitle = seenBasePaths.putIfAbsent(bp, b.spec().info().title()); + if (existingTitle != null) { + throw new IllegalStateException( + "duplicate basePath '" + + bp + + "' across specs: '" + + existingTitle + + "' and '" + + b.spec().info().title() + + "'"); + } + } + + // Extras must not collide with any binding's basePath. + for (String path : extras.keySet()) { + if (seenBasePaths.containsKey(path)) { + throw new IllegalStateException( + "extra handler path " + path + " conflicts with spec basePath " + path); + } + } + + Map resolved = resolveBodyMappers(bodyMappers); + ExceptionHandler effectiveExceptionHandler = + exceptionHandler != null ? exceptionHandler : Handlers.defaultExceptionHandler(); + HandlerConfig handlerConfig = + new HandlerConfig( + interceptors, + decorators, + effectiveExceptionHandler, + extras, + externalAuth, + List.copyOf(afterHooks)); + int resolvedPort = resolvePort(); + SSLContext sslContext = + httpsCertChain != null ? PemSslContext.load(httpsCertChain, httpsPrivateKey) : null; + return new OpenApiServer( + effectiveBindings, + resolved, + handlerConfig, + resolvedPort, + bindAddress, + shutdownTimeoutSeconds, + sslContext); +} +``` + +- [x] **Step 3: Run the new test** + +Run: `mvn -q test -Dtest=MultiSpecServerTest#servesTwoBindingsOnDistinctBasePaths` +Expected: PASS. + +- [x] **Step 4: Run the full suite to confirm nothing regressed** + +Run: `mvn -q test` +Expected: BUILD SUCCESS. + +- [x] **Step 5: Commit** + +```bash +git add src/main/java/com/retailsvc/http/OpenApiServer.java \ + src/test/java/com/retailsvc/http/MultiSpecServerTest.java +git commit -m "feat: Add addSpec() builder method for multi-spec servers" +``` + +--- + +### Task 6: Test — identical `operationId`s across bindings dispatch to their own handler + +**Files:** +- Modify: `src/test/java/com/retailsvc/http/MultiSpecServerTest.java` + +- [x] **Step 1: Add the test** + +```java +@Test +void identicalOperationIdsAcrossBindingsDispatchIndependently() throws Exception { + Spec v1 = SpecFixtures.specAt("http://localhost/api/v1"); + Spec v2 = SpecFixtures.specAt("http://localhost/api/v2"); + + java.util.concurrent.atomic.AtomicInteger v1Hits = new java.util.concurrent.atomic.AtomicInteger(); + java.util.concurrent.atomic.AtomicInteger v2Hits = new java.util.concurrent.atomic.AtomicInteger(); + + Map v1Handlers = + handlersFor(v1, req -> { v1Hits.incrementAndGet(); return Response.ok(Map.of("v", 1)); }); + Map v2Handlers = + handlersFor(v2, req -> { v2Hits.incrementAndGet(); return Response.ok(Map.of("v", 2)); }); + + try (OpenApiServer server = + OpenApiServer.builder() + .port(0) + .addSpec(v1, v1Handlers) + .addSpec(v2, v2Handlers) + .useExternalAuthentication() + .build()) { + + int port = server.listenPort(); + get("http://localhost:" + port + "/api/v1/ping"); + get("http://localhost:" + port + "/api/v2/ping"); + + assertThat(v1Hits.get()).isEqualTo(1); + assertThat(v2Hits.get()).isEqualTo(1); + } +} +``` + +- [x] **Step 2: Run it** + +Run: `mvn -q test -Dtest=MultiSpecServerTest#identicalOperationIdsAcrossBindingsDispatchIndependently` +Expected: PASS (no production change needed — `Map` per binding already gives us this). + +- [x] **Step 3: Commit** + +```bash +git add src/test/java/com/retailsvc/http/MultiSpecServerTest.java +git commit -m "test: Verify identical operationIds across bindings dispatch independently" +``` + +--- + +### Task 7: Test — duplicate basePath rejected at build + +**Files:** +- Modify: `src/test/java/com/retailsvc/http/MultiSpecServerTest.java` + +- [x] **Step 1: Add the test** + +```java +@Test +void rejectsTwoBindingsWithSameBasePath() { + Spec a = SpecFixtures.specAt("http://localhost/api/v1"); + Spec b = SpecFixtures.specAt("http://localhost/api/v1"); + Map handlersA = handlersFor(a, req -> Response.ok(Map.of())); + Map handlersB = handlersFor(b, req -> Response.ok(Map.of())); + + assertThatThrownBy( + () -> + OpenApiServer.builder() + .port(0) + .addSpec(a, handlersA) + .addSpec(b, handlersB) + .useExternalAuthentication() + .build()) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("duplicate basePath") + .hasMessageContaining("/api/v1"); +} +``` + +- [x] **Step 2: Run it** + +Run: `mvn -q test -Dtest=MultiSpecServerTest#rejectsTwoBindingsWithSameBasePath` +Expected: PASS. + +- [x] **Step 3: Commit** + +```bash +git add src/test/java/com/retailsvc/http/MultiSpecServerTest.java +git commit -m "test: Reject two bindings on the same basePath" +``` + +--- + +### Task 8: Test — per-binding security wiring is enforced independently + +**Files:** +- Modify: `src/test/java/com/retailsvc/http/MultiSpecServerTest.java` + +- [x] **Step 1: Add the test** + +This test confirms that when a spec references a security scheme, the corresponding `SchemeValidator` must be supplied in *that binding's* validator map — not somewhere else. It uses the existing fixture which (per repo convention) declares at least one secured operation. + +```java +@Test +void missingValidatorOnOneBindingFailsIndependently() { + // Only run if the test spec has any security requirements; otherwise this assertion is vacuous. + Spec v1 = SpecFixtures.specAt("http://localhost/api/v1"); + boolean specHasSecurity = !v1.securitySchemes().isEmpty(); + org.junit.jupiter.api.Assumptions.assumeTrue( + specHasSecurity, "test spec has no security schemes to exercise"); + + Map handlers = handlersFor(v1, req -> Response.ok(Map.of())); + + assertThatThrownBy( + () -> + OpenApiServer.builder() + .port(0) + .addSpec(v1, handlers, Map.of()) // empty validators on purpose + .build()) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("no SchemeValidator registered for security scheme"); +} +``` + +- [x] **Step 2: Run it** + +Run: `mvn -q test -Dtest=MultiSpecServerTest#missingValidatorOnOneBindingFailsIndependently` +Expected: PASS (the assumption skips if the fixture is unsecured; otherwise the wiring check fires). + +- [x] **Step 3: Commit** + +```bash +git add src/test/java/com/retailsvc/http/MultiSpecServerTest.java +git commit -m "test: Per-binding security validator wiring is enforced" +``` + +--- + +### Task 9: Test — mixing legacy methods with `addSpec()` is rejected + +**Files:** +- Modify: `src/test/java/com/retailsvc/http/MultiSpecServerTest.java` + +- [x] **Step 1: Add the test** + +```java +@Test +void rejectsMixingLegacySpecMethodsWithAddSpec() { + Spec a = SpecFixtures.specAt("http://localhost/api/v1"); + Spec b = SpecFixtures.specAt("http://localhost/api/v2"); + Map handlersA = handlersFor(a, req -> Response.ok(Map.of())); + Map handlersB = handlersFor(b, req -> Response.ok(Map.of())); + + assertThatThrownBy( + () -> + OpenApiServer.builder() + .port(0) + .spec(a) + .handlers(handlersA) + .addSpec(b, handlersB) + .useExternalAuthentication() + .build()) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("use either spec()/handler()/securityValidator() or addSpec()"); +} +``` + +- [x] **Step 2: Run it** + +Run: `mvn -q test -Dtest=MultiSpecServerTest#rejectsMixingLegacySpecMethodsWithAddSpec` +Expected: PASS. + +- [x] **Step 3: Commit** + +```bash +git add src/test/java/com/retailsvc/http/MultiSpecServerTest.java +git commit -m "test: Reject mixing legacy spec() with addSpec()" +``` + +--- + +### Task 10: Integration test — two bindings under a real `HttpServer` + +**Files:** +- Create: `src/test/java/com/retailsvc/http/MultiSpecServerIT.java` + +This runs under `mvn verify` (Failsafe) and exercises real HTTP traffic. + +- [x] **Step 1: Write the IT** + +```java +package com.retailsvc.http; + +import static java.net.HttpURLConnection.HTTP_NOT_FOUND; +import static java.net.HttpURLConnection.HTTP_OK; +import static org.assertj.core.api.Assertions.assertThat; + +import com.retailsvc.http.spec.Spec; +import com.retailsvc.http.support.SpecFixtures; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.util.LinkedHashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class MultiSpecServerIT { + + @Test + void twoBindingsServeTrafficAndUnknownPathReturns404() throws Exception { + Spec v1 = SpecFixtures.specAt("http://localhost/api/v1"); + Spec v2 = SpecFixtures.specAt("http://localhost/api/v2"); + + Map v1Handlers = handlersFor(v1, req -> Response.ok(Map.of("v", 1))); + Map v2Handlers = handlersFor(v2, req -> Response.ok(Map.of("v", 2))); + + try (OpenApiServer server = + OpenApiServer.builder() + .port(0) + .addSpec(v1, v1Handlers) + .addSpec(v2, v2Handlers) + .useExternalAuthentication() + .build()) { + + int port = server.listenPort(); + assertThat(get("http://localhost:" + port + "/api/v1/ping").body()).contains("\"v\":1"); + assertThat(get("http://localhost:" + port + "/api/v2/ping").body()).contains("\"v\":2"); + assertThat(get("http://localhost:" + port + "/").statusCode()).isEqualTo(HTTP_NOT_FOUND); + } + } + + private static Map handlersFor(Spec spec, RequestHandler shared) { + Map out = new LinkedHashMap<>(); + spec.operations().forEach(op -> out.put(op.operationId(), shared)); + return out; + } + + private static HttpResponse get(String url) throws Exception { + return HttpClient.newHttpClient() + .send( + HttpRequest.newBuilder(URI.create(url)).GET().build(), + HttpResponse.BodyHandlers.ofString()); + } +} +``` + +- [x] **Step 2: Run it** + +Run: `mvn -q verify` +Expected: BUILD SUCCESS. The IT runs under Failsafe (matches `*IT.java`). + +- [x] **Step 3: Commit** + +```bash +git add src/test/java/com/retailsvc/http/MultiSpecServerIT.java +git commit -m "test: Integration test for multi-spec server under real HttpServer" +``` + +--- + +### Task 11: README — document `addSpec()` + +**Files:** +- Modify: `README.md` + +- [x] **Step 1: Find the existing single-spec example and add a multi-spec section beneath it** + +Use Read to locate the current "Quick start" or builder example in `README.md`, then add a "Multiple specs" subsection containing the example from §4 of the design doc (the `v1Spec` / `v2Spec` `addSpec` example). Keep the wording aligned with the design doc — the public contract is that each call is one binding and namespaces are per-spec. + +- [x] **Step 2: Run pre-commit** + +Run: `pre-commit run --files README.md` +Expected: PASS (editorconfig, trailing whitespace, end-of-file). + +- [x] **Step 3: Commit** + +```bash +git add README.md +git commit -m "docs: Document addSpec() for multi-spec servers" +``` + +--- + +### Task 12: Final verification + +- [ ] **Step 1: Full unit + integration run** + +Run: `mvn -q verify` +Expected: BUILD SUCCESS. All existing 440 tests plus the new tests pass. Coverage report at `target/site/jacoco/`. + +- [ ] **Step 2: Sonar pre-push scan** (per project memory `feedback_sonar_pre_push.md`) + +Per memory, SonarLint MCP is blind to worktrees (`feedback_sonarlint_blind_to_worktrees.md`); rely on the CI Sonar scan for the branch. Push and let CI report. + +- [ ] **Step 3: Push the branch** + +Per memory `reference_gh_token.md`, gh CLI can't open PRs in this repo. Push the branch and ask the user to open the PR manually: + +```bash +git push -u origin feat/multiple-specs +``` + +Then post a one-line summary asking the user to open the PR. + +--- + +## Self-review + +**Spec coverage** + +- §3 constraint 1 (per-spec namespace) — Task 6 (identical operationIds dispatch independently). +- §3 constraint 2 (backward-compatible builder + reject mixing) — Task 9. +- §3 constraint 3 (duplicate basePath rejection only) — Task 7. +- §3 constraint 4 (global hooks) — implicit; no per-spec hook plumbing added. `HandlerConfig` keeps `interceptors`/`decorators`/`afterHooks`/`exceptionHandler` at server scope (Task 2). +- §4 public API (`addSpec` x2 overloads, mix-rejection error) — Task 5. +- §5 `SpecBinding` record and per-binding HttpContext — Tasks 1, 2. +- §6 filters become per-binding instances — Task 2 (instantiates filters inside the binding loop). +- §7 error cases — Tasks 7, 8, 9 cover duplicate basePath, missing validator, mixed-form. Task 5 implements the "extras vs basePath" cross-check. The "no bindings" case is covered by the `requireNonNull(spec, ...)` path in `build()` (legacy synthesis still requires `spec` non-null) plus the empty-bindings guard in the `OpenApiServer` constructor (Task 2). +- §8 testing — Tasks 4, 6, 7, 8, 9, 10. No new fixture files (Task 3 helper). +- §9 migration — Tasks 2 & 5 keep the legacy builder methods source-compatible; the existing test suite acts as regression. + +**Placeholder scan** — no TBDs, no "add appropriate error handling", every code step contains the actual code. The Task 8 assumption (`assumeTrue`) is conditional rather than placeholder — it intentionally skips if the fixture has no security; if so, the case is already covered by the existing `OpenApiServerTest` security wiring tests against the same fixture. + +**Type consistency** — `SpecBinding.of(spec, handlers, validators)` factory used consistently in Tasks 2 and 5. `HandlerConfig` record fields aligned between Task 2 (record change) and Task 5 (callsite). `bindings` field added in Task 5, never referenced before. `effectiveBindings` local variable name used only within `build()`. diff --git a/docs/superpowers/specs/2026-05-22-multiple-specs-design.md b/docs/superpowers/specs/2026-05-22-multiple-specs-design.md new file mode 100644 index 00000000..7bf885fa --- /dev/null +++ b/docs/superpowers/specs/2026-05-22-multiple-specs-design.md @@ -0,0 +1,173 @@ +# Multiple OpenAPI specs per server — design + +Status: proposed +Date: 2026-05-22 +Branch: `feat/multiple-specs` + +## 1. Motivation + +`OpenApiServer` today serves exactly one OpenAPI specification. Two real use cases push against that limit: + +- **Versioned APIs side-by-side.** During a v1 → v2 migration window the same service may need to expose `/api/v1/*` and `/api/v2/*` simultaneously, each driven by its own spec file. +- **Distinct APIs in one process.** A public API and an internal admin API (different specs, different base paths) sometimes share a deployable for infrastructure reasons. + +Both cases share the same property: each spec has its own base path and is otherwise independent. This document describes the smallest viable change that supports them. + +## 2. Goals and non-goals + +**Goals** + +- Allow callers to register more than one `Spec` on a single `OpenApiServer`. +- Scope `operationId` and security-scheme names *per spec*, so v1 and v2 can both declare `getCustomer` or `bearerAuth` without colliding. +- Keep existing single-spec callers source-compatible. +- Reuse the JDK `HttpServer` longest-prefix-match for routing across base paths. + +**Non-goals** + +- Merging multiple specs into a single composite document. +- Per-spec interceptors, response decorators, after-response hooks, or exception handlers. These remain global to the server; callers needing per-spec branching can switch on the request path or resolved `operationId` inside the hook. +- Cross-spec `$ref` resolution. +- Hot reload or runtime mutation of bindings. + +## 3. Decided constraints + +These were settled during brainstorming and frame the design: + +1. **Per-spec namespace.** `operationId`s and security-scheme names need only be unique within a single spec. +2. **Backward-compatible builder.** Existing `Builder.spec(Spec)`, `Builder.handler(opId, h)`, and `Builder.securityValidator(name, v)` keep working. They are sugar for one implicit `addSpec()` bundle. Mixing them with explicit `addSpec()` calls in the same builder is rejected at build time. +3. **Duplicate basePath is the only path conflict rejected.** Nested prefixes (`/` plus `/api`) are legal — the JDK `HttpServer` resolves them by longest-prefix match. +4. **Global hooks.** `RequestInterceptor`, `ResponseDecorator`, `AfterResponseHook`, `ExceptionHandler`, and extra `/`-style contexts stay registered once on the `Builder` and apply to every binding. + +## 4. Public API + +One new builder method, with a convenience overload: + +```java +public Builder addSpec( + Spec spec, + Map handlers, + Map securityValidators); + +public Builder addSpec(Spec spec, Map handlers); +``` + +The two-argument overload is equivalent to passing `Map.of()` for `securityValidators` and is rejected by per-binding validation if the spec actually references any security scheme. `addSpec()` may be called more than once; each call adds one spec binding. + +Existing single-spec methods remain on the Builder and behave as before, but at `build()` they are folded into one implicit `addSpec(spec, handlers, validators)` bundle. Mixing the implicit (single-spec) form with explicit `addSpec()` calls fails fast: + +``` +IllegalStateException: use either spec()/handler()/securityValidator() or addSpec(), not both +``` + +Global hooks (`requestInterceptor`, `responseDecorator`, `afterResponseHook`, `exceptionHandler`, `extraHandler`) keep their existing Builder methods unchanged. + +### Example — multi-spec + +```java +OpenApiServer server = OpenApiServer.builder() + .port(8080) + .addSpec(v1Spec, Map.of( + "getCustomer", new V1GetCustomer(), + "listCustomers", new V1ListCustomers())) + .addSpec(v2Spec, Map.of( + "getCustomer", new V2GetCustomer(), + "listCustomers", new V2ListCustomers(), + "createCustomer", new V2CreateCustomer())) + .requestInterceptor(loggingInterceptor) + .build(); +``` + +### Example — single spec (unchanged) + +```java +OpenApiServer server = OpenApiServer.builder() + .port(8080) + .spec(spec) + .handler("getCustomer", new GetCustomer()) + .handler("listCustomers", new ListCustomers()) + .build(); +``` + +## 5. Internal architecture + +A new package-private record captures everything needed to serve one spec: + +```java +record SpecBinding( + Spec spec, + Map handlers, + Map securityValidators, + DefaultValidator validator, // built from spec::resolveSchema + Router router) { // built from spec +} +``` + +`OpenApiServer` stores `List bindings` instead of the current single `spec` / `handlers` / `securityValidators` fields. + +### Build-time wiring + +At `Builder.build()`: + +1. Each `addSpec()` call (or the synthesised single bundle from the legacy methods) becomes one `SpecBinding`. +2. **Per-binding validation** — identical to today's checks, scoped to each binding: + - `validateHandlerWiring(binding.spec(), binding.handlers())` — every `operationId` in the spec has a registered handler, and there are no unknown handler keys. + - `validateSecurityWiring(binding.spec(), binding.securityValidators())` — every security scheme referenced by the spec has a registered validator. +3. **Cross-binding validation** — reject duplicate `spec.basePath()` across bindings, listing the conflicting spec titles (`spec.info().title()`). +4. At least one binding must be present. Empty server is rejected with the existing "Spec must not be null" semantics, adapted to mention bindings. + +### Runtime wiring + +On server start, the server creates **one `HttpContext` per binding** at `binding.spec().basePath()`, with that binding's filter chain attached. The catch-all `/` context for 404s is only registered if no binding owns `/`. JDK `HttpServer` routes incoming requests to the longest-prefix-matching context, so each request lands in exactly one binding's chain. + +## 6. Filter chain + +The three existing filters become per-binding instances. They close over a `SpecBinding` instead of the top-level builder state: + +- **`ExceptionFilter`** — logic unchanged. Continues to use the *global* `ExceptionHandler`. One instance per binding (cheap). +- **`RequestPreparationFilter`** — uses `binding.validator()` and `binding.router()` directly. Still stores the resolved `operationId` on the exchange as today. +- **`DispatchHandler`** — looks up the handler in `binding.handlers()`. No global handler map exists. + +Global hooks (`RequestInterceptor`, `ResponseDecorator`, `AfterResponseHook`) wrap dispatch the same way they do today; there is still only one instance of each, passed into each binding's filter chain at construction. + +## 7. Error handling and edge cases + +| Scenario | Outcome | +| --- | --- | +| Two bindings with the same `basePath` | `build()` throws `IllegalStateException` listing the conflicting spec titles. | +| Legacy single-spec methods mixed with `addSpec()` in the same builder | `build()` throws `IllegalStateException` directing the caller to pick one form. | +| No bindings registered | `build()` throws — adapts the existing "Spec must not be null" message. | +| Unknown `operationId` (handler missing or extra) | `build()` throws, naming the spec the mismatch belongs to. | +| Unknown security scheme | `build()` throws, naming the spec the scheme belongs to. | +| Request to a path no binding covers | Existing catch-all `/` 404 context handles it. | +| One binding at `/`, another at `/api/v1` | Legal. `/api/v1/*` routes to the v1 binding; everything else to the `/` binding. | +| Two specs with overlapping path templates inside disjoint base paths | Impossible by construction — JDK `HttpContext` dispatches by base path first, then each binding's `Router` matches templates within its own subtree. | + +## 8. Testing + +Per project memory, do not add new OpenAPI spec fixture files unless strictly necessary. Reuse `src/test/resources/openapi.json` (and its YAML mirror) by parsing it into a `Map`, deep-cloning, and mutating `servers[0].url` per binding before calling `Spec.from(map)` twice. This naturally gives two bindings with identical `operationId`s, which is exactly the per-spec-namespace property we want to exercise. + +**Unit (`OpenApiServerTest`)** — new camelCase methods, static AssertJ imports: + +- Two bindings with disjoint base paths both serve traffic correctly. +- Identical `operationId`s across bindings dispatch to their own handler (per-spec namespace). +- Duplicate `basePath` across bindings is rejected at `build()`. +- Mixing legacy single-spec methods with `addSpec()` is rejected at `build()`. +- Per-binding `validateHandlerWiring` and `validateSecurityWiring` fail independently. +- Single-spec callers using legacy methods continue to work unchanged (regression net for the "sugar" claim). + +**Integration (`MultiSpecServerIT`)** — boots a real `HttpServer`: + +- Two bindings derived from the same fixture, mounted at `/api/v1` and `/api/v2`. +- Each route hits its own handler under live HTTP. +- A request to an uncovered path returns 404 via the catch-all context. + +## 9. Migration + +No code change is required for existing single-spec callers. The legacy `spec()` / `handler()` / `securityValidator()` builder methods stay on the Builder, with the same semantics — they now compose into one implicit `addSpec()` bundle inside `build()`. Documentation will introduce `addSpec()` as the canonical form for new code while keeping the single-spec example in the README valid. + +## 10. Out of scope (future work) + +- Per-spec global hooks (interceptor/decorator/exception handler). +- Composite spec endpoint (e.g. serving a merged `/openapi.json` listing all bindings). +- Spec hot reload. +- Cross-spec `$ref` resolution. diff --git a/src/main/java/com/retailsvc/http/OpenApiServer.java b/src/main/java/com/retailsvc/http/OpenApiServer.java index 671b269e..e1823b83 100644 --- a/src/main/java/com/retailsvc/http/OpenApiServer.java +++ b/src/main/java/com/retailsvc/http/OpenApiServer.java @@ -11,8 +11,8 @@ import com.retailsvc.http.internal.PemSslContext; import com.retailsvc.http.internal.RequestPreparationFilter; import com.retailsvc.http.internal.ResponseRenderer; -import com.retailsvc.http.internal.Router; import com.retailsvc.http.internal.SecurityFilter; +import com.retailsvc.http.internal.SpecBinding; import com.retailsvc.http.internal.TextTypeMapper; import com.retailsvc.http.internal.TlsHttpsConfigurator; import com.retailsvc.http.internal.gson.GsonJsonMapper; @@ -20,7 +20,6 @@ import com.retailsvc.http.spec.Spec; import com.retailsvc.http.spec.security.SecurityRequirement; import com.retailsvc.http.spec.security.SecurityScheme; -import com.retailsvc.http.validate.DefaultValidator; import com.sun.net.httpserver.HttpContext; import com.sun.net.httpserver.HttpServer; import com.sun.net.httpserver.HttpsServer; @@ -60,17 +59,15 @@ public class OpenApiServer implements AutoCloseable { /** Internal grouping of handler-related configuration to keep the constructor signature small. */ record HandlerConfig( - Map handlers, List interceptors, List decorators, ExceptionHandler exceptionHandler, Map extras, - Map securityValidators, boolean externalAuth, List afterHooks) {} OpenApiServer( - Spec spec, + List bindings, Map bodyMappers, HandlerConfig handlerConfig, int port, @@ -79,41 +76,79 @@ record HandlerConfig( SSLContext sslContext) throws IOException { - requireNonNull(spec, "Spec must not be null"); + requireNonNull(bindings, "bindings must not be null"); + if (bindings.isEmpty()) { + throw new IllegalStateException("at least one spec binding is required"); + } requireNonNull(bodyMappers, "bodyMappers must not be null"); - requireNonNull(handlerConfig.handlers(), "handlers must not be null"); - ExceptionHandler exceptionHandler = handlerConfig.exceptionHandler(); long t0 = System.currentTimeMillis(); - Router router = new Router(spec.operations()); - Map operationsById = - spec.operations().stream() - .collect(Collectors.toUnmodifiableMap(Operation::operationId, op -> op)); - DefaultValidator validator = new DefaultValidator(spec::resolveSchema); + ExceptionHandler exceptionHandler = handlerConfig.exceptionHandler(); InetSocketAddress socketAddress = (bindAddress == null) ? new InetSocketAddress(port) : new InetSocketAddress(bindAddress, port); + this.httpServer = createHttpServer(socketAddress, sslContext); + httpServer.setExecutor(newThreadPerTaskExecutor(ofVirtual().name("http-", 0).factory())); + + ResponseRenderer renderer = new ResponseRenderer(bodyMappers); + boolean anyBindingAtRoot = + wireBindings(httpServer, bindings, bodyMappers, handlerConfig, exceptionHandler, renderer); + wireExtras(httpServer, anyBindingAtRoot, handlerConfig.extras(), exceptionHandler, renderer); + + httpServer.start(); + this.shutdownTimeoutSeconds = shutdownTimeoutSeconds; + logStartup(t0); + } + + private static HttpServer createHttpServer(InetSocketAddress addr, SSLContext sslContext) + throws IOException { if (sslContext != null) { - HttpsServer https = HttpsServer.create(socketAddress, 0); + HttpsServer https = HttpsServer.create(addr, 0); https.setHttpsConfigurator(new TlsHttpsConfigurator(sslContext)); - this.httpServer = https; - } else { - this.httpServer = HttpServer.create(socketAddress, 0); + return https; } - httpServer.setExecutor(newThreadPerTaskExecutor(ofVirtual().name("http-", 0).factory())); + return HttpServer.create(addr, 0); + } - ResponseRenderer renderer = new ResponseRenderer(bodyMappers); + @SuppressWarnings("java:S107") + private static boolean wireBindings( + HttpServer httpServer, + List bindings, + Map bodyMappers, + HandlerConfig handlerConfig, + ExceptionHandler exceptionHandler, + ResponseRenderer renderer) { + boolean anyBindingAtRoot = false; + for (SpecBinding binding : bindings) { + String basePath = Optional.ofNullable(binding.spec().basePath()).orElse("/"); + anyBindingAtRoot |= "/".equals(basePath); + wireBinding( + httpServer, basePath, binding, bodyMappers, handlerConfig, exceptionHandler, renderer); + } + return anyBindingAtRoot; + } - String basePath = Optional.ofNullable(spec.basePath()).orElse("/"); + @SuppressWarnings("java:S107") + private static void wireBinding( + HttpServer httpServer, + String basePath, + SpecBinding binding, + Map bodyMappers, + HandlerConfig handlerConfig, + ExceptionHandler exceptionHandler, + ResponseRenderer renderer) { + Map operationsById = + binding.spec().operations().stream() + .collect(Collectors.toUnmodifiableMap(Operation::operationId, op -> op)); HttpContext ctx = httpServer.createContext(basePath); ctx.getFilters() .add( new RequestPreparationFilter( - spec, - router, - validator, + binding.spec(), + binding.router(), + binding.validator(), bodyMappers, exceptionHandler, renderer, @@ -122,28 +157,37 @@ record HandlerConfig( .add( new SecurityFilter( operationsById, - spec.securitySchemes(), - spec.security(), - handlerConfig.securityValidators(), + binding.spec().securitySchemes(), + binding.spec().security(), + binding.securityValidators(), handlerConfig.externalAuth())); ctx.setHandler( new DispatchHandler( - handlerConfig.handlers(), + binding.handlers(), handlerConfig.interceptors(), handlerConfig.decorators(), renderer)); + } - if (!"/".equals(basePath)) { - ExtrasRouter extrasRouter = new ExtrasRouter(handlerConfig.extras(), renderer); - HttpContext extrasCtx = httpServer.createContext("/", extrasRouter); - extrasCtx.getFilters().add(new ExceptionFilter(exceptionHandler, renderer)); - } else if (!handlerConfig.extras().isEmpty()) { - throw new IllegalStateException("extras cannot be registered when basePath is '/'"); + private static void wireExtras( + HttpServer httpServer, + boolean anyBindingAtRoot, + Map extras, + ExceptionHandler exceptionHandler, + ResponseRenderer renderer) { + if (anyBindingAtRoot) { + if (!extras.isEmpty()) { + throw new IllegalStateException( + "extras cannot be registered when a binding owns basePath '/'"); + } + return; } - httpServer.start(); - - this.shutdownTimeoutSeconds = shutdownTimeoutSeconds; + ExtrasRouter extrasRouter = new ExtrasRouter(extras, renderer); + HttpContext extrasCtx = httpServer.createContext("/", extrasRouter); + extrasCtx.getFilters().add(new ExceptionFilter(exceptionHandler, renderer)); + } + private void logStartup(long t0) { String host = httpServer.getAddress().getHostString(); String displayHost = host.contains(":") ? "[" + host + "]" : host; LOG.info( @@ -207,6 +251,7 @@ public static final class Builder { private final LinkedHashMap extras = new LinkedHashMap<>(); private final Map securityValidators = new LinkedHashMap<>(); private boolean externalAuth = false; + private final List bindings = new ArrayList<>(); private Builder() {} @@ -285,6 +330,28 @@ public Builder useExternalAuthentication() { return this; } + /** + * Registers an OpenAPI {@link Spec} with the handlers and security validators that serve it. + * May be called more than once; each binding becomes its own {@link + * com.sun.net.httpserver.HttpContext} at the spec's {@code basePath}. {@code operationId}s and + * security-scheme names only need to be unique within a single spec. + */ + public Builder addSpec( + Spec spec, + Map handlers, + Map securityValidators) { + requireNonNull(spec, "spec must not be null"); + requireNonNull(handlers, "handlers must not be null"); + requireNonNull(securityValidators, "securityValidators must not be null"); + bindings.add(SpecBinding.of(spec, handlers, securityValidators)); + return this; + } + + /** Convenience overload for specs that declare no security schemes. */ + public Builder addSpec(Spec spec, Map handlers) { + return addSpec(spec, handlers, Map.of()); + } + public Builder exceptionHandler(ExceptionHandler exceptionHandler) { this.exceptionHandler = exceptionHandler; return this; @@ -359,37 +426,27 @@ public Builder extraRoute(String path, RequestHandler handler) { } public OpenApiServer build() throws IOException { - requireNonNull(spec, "Spec must not be null"); - requireNonNull(handlers, "handlers must not be null"); - String basePath = Optional.ofNullable(spec.basePath()).orElse("/"); - for (String path : extras.keySet()) { - if (path.equals(basePath)) { - throw new IllegalStateException( - "extra handler path " + path + " conflicts with spec basePath " + basePath); - } - } - if (!externalAuth) { - validateSecurityWiring(spec, securityValidators); - } - validateHandlerWiring(spec, handlers); + List effectiveBindings = resolveBindings(); + validateBindings(effectiveBindings); + Map seenBasePaths = rejectDuplicateBasePaths(effectiveBindings); + rejectExtrasOnBasePath(seenBasePaths); + Map resolved = resolveBodyMappers(bodyMappers); ExceptionHandler effectiveExceptionHandler = exceptionHandler != null ? exceptionHandler : Handlers.defaultExceptionHandler(); HandlerConfig handlerConfig = new HandlerConfig( - handlers, interceptors, decorators, effectiveExceptionHandler, extras, - Map.copyOf(securityValidators), externalAuth, List.copyOf(afterHooks)); int resolvedPort = resolvePort(); SSLContext sslContext = httpsCertChain != null ? PemSslContext.load(httpsCertChain, httpsPrivateKey) : null; return new OpenApiServer( - spec, + effectiveBindings, resolved, handlerConfig, resolvedPort, @@ -398,6 +455,62 @@ public OpenApiServer build() throws IOException { sslContext); } + private List resolveBindings() { + boolean usedLegacy = spec != null || handlers != null || !securityValidators.isEmpty(); + boolean usedAddSpec = !bindings.isEmpty(); + if (usedLegacy && usedAddSpec) { + throw new IllegalStateException( + "use either spec()/handler()/securityValidator() or addSpec(), not both"); + } + if (usedAddSpec) { + return List.copyOf(bindings); + } + requireNonNull(spec, "Spec must not be null"); + requireNonNull(handlers, "handlers must not be null"); + return List.of(SpecBinding.of(spec, handlers, securityValidators)); + } + + private void validateBindings(List effectiveBindings) { + for (SpecBinding b : effectiveBindings) { + if (!externalAuth) { + validateSecurityWiring(b.spec(), b.securityValidators()); + } + validateHandlerWiring(b.spec(), b.handlers()); + } + } + + private static Map rejectDuplicateBasePaths(List bindings) { + Map seenBasePaths = new LinkedHashMap<>(); + for (SpecBinding b : bindings) { + String bp = Optional.ofNullable(b.spec().basePath()).orElse("/"); + String existingTitle = seenBasePaths.putIfAbsent(bp, b.spec().info().title()); + if (existingTitle != null) { + throw new IllegalStateException( + "duplicate basePath '" + + bp + + "' across specs: '" + + existingTitle + + "' and '" + + b.spec().info().title() + + "'"); + } + } + return seenBasePaths; + } + + private void rejectExtrasOnBasePath(Map seenBasePaths) { + for (String path : extras.keySet()) { + if (seenBasePaths.containsKey(path)) { + throw new IllegalStateException( + "extra handler path '" + + path + + "' conflicts with basePath of spec '" + + seenBasePaths.get(path) + + "'"); + } + } + } + private int resolvePort() { if (port != null) { return port; diff --git a/src/main/java/com/retailsvc/http/internal/SpecBinding.java b/src/main/java/com/retailsvc/http/internal/SpecBinding.java new file mode 100644 index 00000000..d31610b2 --- /dev/null +++ b/src/main/java/com/retailsvc/http/internal/SpecBinding.java @@ -0,0 +1,38 @@ +package com.retailsvc.http.internal; + +import com.retailsvc.http.RequestHandler; +import com.retailsvc.http.SchemeValidator; +import com.retailsvc.http.spec.Spec; +import com.retailsvc.http.validate.DefaultValidator; +import java.util.Map; + +/** + * Bundles everything the server needs to serve one OpenAPI spec: the spec itself, the handler map + * keyed by {@code operationId}, the security-scheme validator map, and the derived {@link Router} + * and {@link DefaultValidator}. One {@code SpecBinding} drives exactly one {@code HttpContext}. + */ +public record SpecBinding( + Spec spec, + Map handlers, + Map securityValidators, + DefaultValidator validator, + Router router) { + + public SpecBinding { + handlers = Map.copyOf(handlers); + securityValidators = Map.copyOf(securityValidators); + } + + /** Builds a binding by deriving the {@link Router} and {@link DefaultValidator} from the spec. */ + public static SpecBinding of( + Spec spec, + Map handlers, + Map securityValidators) { + return new SpecBinding( + spec, + handlers, + securityValidators, + new DefaultValidator(spec::resolveSchema), + new Router(spec.operations())); + } +} diff --git a/src/test/java/com/retailsvc/http/MultiSpecClasspathLoadingTest.java b/src/test/java/com/retailsvc/http/MultiSpecClasspathLoadingTest.java new file mode 100644 index 00000000..1b6fa06f --- /dev/null +++ b/src/test/java/com/retailsvc/http/MultiSpecClasspathLoadingTest.java @@ -0,0 +1,112 @@ +package com.retailsvc.http; + +import static java.net.HttpURLConnection.HTTP_OK; +import static org.assertj.core.api.Assertions.assertThat; + +import com.retailsvc.http.spec.Spec; +import com.retailsvc.http.support.SpecAnchor; +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.net.URL; +import java.net.URLClassLoader; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.jar.JarEntry; +import java.util.jar.JarOutputStream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class MultiSpecClasspathLoadingTest { + + @Test + void loadsTwoSpecsFromDirectoryClasspath() throws Exception { + Spec v1 = Spec.fromClasspath(SpecAnchor.class, "/schemas/v1/openapi.json"); + Spec v2 = Spec.fromClasspath(SpecAnchor.class, "/schemas/v2/openapi.json"); + + assertThat(v1.basePath()).isEqualTo("/api/v1"); + assertThat(v2.basePath()).isEqualTo("/api/v2"); + + bootAndAssertBothPingsReturnOk(v1, v2); + } + + @Test + void loadsTwoSpecsFromJarClasspath(@TempDir Path tmp) throws Exception { + Path jarFile = buildSchemasJar(tmp); + + try (URLClassLoader cl = + new URLClassLoader(new URL[] {jarFile.toUri().toURL()}, getClass().getClassLoader())) { + Class anchor = Class.forName("com.retailsvc.http.support.SpecAnchor", true, cl); + + Spec v1 = Spec.fromClasspath(anchor, "/schemas/v1/openapi.json"); + Spec v2 = Spec.fromClasspath(anchor, "/schemas/v2/openapi.json"); + + assertThat(v1.basePath()).isEqualTo("/api/v1"); + assertThat(v2.basePath()).isEqualTo("/api/v2"); + + bootAndAssertBothPingsReturnOk(v1, v2); + } + } + + private static void bootAndAssertBothPingsReturnOk(Spec v1, Spec v2) throws Exception { + Map v1Handlers = handlersFor(v1, req -> Response.ok(Map.of("v", 1))); + Map v2Handlers = handlersFor(v2, req -> Response.ok(Map.of("v", 2))); + + try (OpenApiServer server = + OpenApiServer.builder() + .port(0) + .addSpec(v1, v1Handlers) + .addSpec(v2, v2Handlers) + .useExternalAuthentication() + .build()) { + + int port = server.listenPort(); + HttpResponse r1 = get("http://localhost:" + port + "/api/v1/ping"); + HttpResponse r2 = get("http://localhost:" + port + "/api/v2/ping"); + assertThat(r1.statusCode()).isEqualTo(HTTP_OK); + assertThat(r1.body()).contains("\"v\":1"); + assertThat(r2.statusCode()).isEqualTo(HTTP_OK); + assertThat(r2.body()).contains("\"v\":2"); + } + } + + private static Path buildSchemasJar(Path tmp) throws IOException { + Path jar = tmp.resolve("specs.jar"); + try (JarOutputStream out = new JarOutputStream(Files.newOutputStream(jar))) { + copyToJar(out, "schemas/v1/openapi.json"); + copyToJar(out, "schemas/v2/openapi.json"); + copyToJar(out, "com/retailsvc/http/support/SpecAnchor.class"); + } + return jar; + } + + private static void copyToJar(JarOutputStream out, String resourcePath) throws IOException { + try (InputStream in = + MultiSpecClasspathLoadingTest.class.getClassLoader().getResourceAsStream(resourcePath)) { + if (in == null) { + throw new IllegalStateException("missing test resource: " + resourcePath); + } + out.putNextEntry(new JarEntry(resourcePath)); + in.transferTo(out); + out.closeEntry(); + } + } + + private static Map handlersFor(Spec spec, RequestHandler shared) { + Map out = new LinkedHashMap<>(); + spec.operations().forEach(op -> out.put(op.operationId(), shared)); + return out; + } + + private static HttpResponse get(String url) throws Exception { + return HttpClient.newHttpClient() + .send( + HttpRequest.newBuilder(URI.create(url)).GET().build(), + HttpResponse.BodyHandlers.ofString()); + } +} diff --git a/src/test/java/com/retailsvc/http/MultiSpecServerIT.java b/src/test/java/com/retailsvc/http/MultiSpecServerIT.java new file mode 100644 index 00000000..f2133d9f --- /dev/null +++ b/src/test/java/com/retailsvc/http/MultiSpecServerIT.java @@ -0,0 +1,58 @@ +package com.retailsvc.http; + +import static java.net.HttpURLConnection.HTTP_NOT_FOUND; +import static java.net.HttpURLConnection.HTTP_OK; +import static org.assertj.core.api.Assertions.assertThat; + +import com.retailsvc.http.spec.Spec; +import com.retailsvc.http.support.SpecFixtures; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.util.LinkedHashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class MultiSpecServerIT { + + @Test + void twoBindingsServeTrafficAndUnknownPathReturns404() throws Exception { + Spec v1 = SpecFixtures.specAt("http://localhost/api/v1"); + Spec v2 = SpecFixtures.specAt("http://localhost/api/v2"); + + Map v1Handlers = handlersFor(v1, req -> Response.ok(Map.of("v", 1))); + Map v2Handlers = handlersFor(v2, req -> Response.ok(Map.of("v", 2))); + + try (OpenApiServer server = + OpenApiServer.builder() + .port(0) + .addSpec(v1, v1Handlers) + .addSpec(v2, v2Handlers) + .useExternalAuthentication() + .build()) { + + int port = server.listenPort(); + HttpResponse v1Resp = get("http://localhost:" + port + "/api/v1/data"); + HttpResponse v2Resp = get("http://localhost:" + port + "/api/v2/data"); + assertThat(v1Resp.statusCode()).isEqualTo(HTTP_OK); + assertThat(v1Resp.body()).contains("\"v\":1"); + assertThat(v2Resp.statusCode()).isEqualTo(HTTP_OK); + assertThat(v2Resp.body()).contains("\"v\":2"); + assertThat(get("http://localhost:" + port + "/nope").statusCode()).isEqualTo(HTTP_NOT_FOUND); + } + } + + private static Map handlersFor(Spec spec, RequestHandler shared) { + Map out = new LinkedHashMap<>(); + spec.operations().forEach(op -> out.put(op.operationId(), shared)); + return out; + } + + private static HttpResponse get(String url) throws Exception { + return HttpClient.newHttpClient() + .send( + HttpRequest.newBuilder(URI.create(url)).GET().build(), + HttpResponse.BodyHandlers.ofString()); + } +} diff --git a/src/test/java/com/retailsvc/http/MultiSpecServerTest.java b/src/test/java/com/retailsvc/http/MultiSpecServerTest.java new file mode 100644 index 00000000..1956f80e --- /dev/null +++ b/src/test/java/com/retailsvc/http/MultiSpecServerTest.java @@ -0,0 +1,154 @@ +package com.retailsvc.http; + +import static java.net.HttpURLConnection.HTTP_OK; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.retailsvc.http.spec.Spec; +import com.retailsvc.http.support.SpecFixtures; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; + +class MultiSpecServerTest { + + @Test + void servesTwoBindingsOnDistinctBasePaths() throws Exception { + Spec v1 = SpecFixtures.specAt("http://localhost/api/v1"); + Spec v2 = SpecFixtures.specAt("http://localhost/api/v2"); + + RequestHandler v1Handler = req -> Response.ok(Map.of("version", "v1")); + RequestHandler v2Handler = req -> Response.ok(Map.of("version", "v2")); + + try (OpenApiServer server = + OpenApiServer.builder() + .port(0) + .addSpec(v1, handlersFor(v1, v1Handler)) + .addSpec(v2, handlersFor(v2, v2Handler)) + .useExternalAuthentication() + .build()) { + + int port = server.listenPort(); + HttpResponse v1Resp = get("http://localhost:" + port + "/api/v1/data"); + HttpResponse v2Resp = get("http://localhost:" + port + "/api/v2/data"); + assertThat(v1Resp.statusCode()).isEqualTo(HTTP_OK); + assertThat(v1Resp.body()).contains("v1"); + assertThat(v2Resp.statusCode()).isEqualTo(HTTP_OK); + assertThat(v2Resp.body()).contains("v2"); + } + } + + @Test + void identicalOperationIdsAcrossBindingsDispatchIndependently() throws Exception { + Spec v1 = SpecFixtures.specAt("http://localhost/api/v1"); + Spec v2 = SpecFixtures.specAt("http://localhost/api/v2"); + + AtomicInteger v1Hits = new AtomicInteger(); + AtomicInteger v2Hits = new AtomicInteger(); + + Map v1Handlers = + handlersFor( + v1, + req -> { + v1Hits.incrementAndGet(); + return Response.ok(Map.of("v", 1)); + }); + Map v2Handlers = + handlersFor( + v2, + req -> { + v2Hits.incrementAndGet(); + return Response.ok(Map.of("v", 2)); + }); + + try (OpenApiServer server = + OpenApiServer.builder() + .port(0) + .addSpec(v1, v1Handlers) + .addSpec(v2, v2Handlers) + .useExternalAuthentication() + .build()) { + + int port = server.listenPort(); + get("http://localhost:" + port + "/api/v1/data"); + get("http://localhost:" + port + "/api/v2/data"); + + assertThat(v1Hits.get()).isEqualTo(1); + assertThat(v2Hits.get()).isEqualTo(1); + } + } + + @Test + void rejectsTwoBindingsWithSameBasePath() { + Spec a = SpecFixtures.specAt("http://localhost/api/v1"); + Spec b = SpecFixtures.specAt("http://localhost/api/v1"); + Map handlersA = handlersFor(a, req -> Response.ok(Map.of())); + Map handlersB = handlersFor(b, req -> Response.ok(Map.of())); + + OpenApiServer.Builder builder = + OpenApiServer.builder() + .port(0) + .addSpec(a, handlersA) + .addSpec(b, handlersB) + .useExternalAuthentication(); + + assertThatThrownBy(builder::build) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("duplicate basePath") + .hasMessageContaining("/api/v1"); + } + + @Test + void missingValidatorOnOneBindingFailsIndependently() { + Spec v1 = SpecFixtures.specAt("http://localhost/api/v1"); + Assumptions.assumeTrue( + !v1.securitySchemes().isEmpty(), "test spec has no security schemes to exercise"); + + Map handlers = handlersFor(v1, req -> Response.ok(Map.of())); + + OpenApiServer.Builder builder = OpenApiServer.builder().port(0).addSpec(v1, handlers, Map.of()); + + assertThatThrownBy(builder::build) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("no SchemeValidator registered for security scheme"); + } + + @Test + void rejectsMixingLegacySpecMethodsWithAddSpec() { + Spec a = SpecFixtures.specAt("http://localhost/api/v1"); + Spec b = SpecFixtures.specAt("http://localhost/api/v2"); + Map handlersA = handlersFor(a, req -> Response.ok(Map.of())); + Map handlersB = handlersFor(b, req -> Response.ok(Map.of())); + + OpenApiServer.Builder builder = + OpenApiServer.builder() + .port(0) + .spec(a) + .handlers(handlersA) + .addSpec(b, handlersB) + .useExternalAuthentication(); + + assertThatThrownBy(builder::build) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("use either spec()/handler()/securityValidator() or addSpec()"); + } + + private static Map handlersFor(Spec spec, RequestHandler shared) { + Map out = new LinkedHashMap<>(); + spec.operations().forEach(op -> out.put(op.operationId(), shared)); + return out; + } + + private static HttpResponse get(String url) throws Exception { + return HttpClient.newHttpClient() + .send( + HttpRequest.newBuilder(URI.create(url)).GET().build(), + HttpResponse.BodyHandlers.ofString()); + } +} diff --git a/src/test/java/com/retailsvc/http/support/SpecAnchor.java b/src/test/java/com/retailsvc/http/support/SpecAnchor.java new file mode 100644 index 00000000..2c92502a --- /dev/null +++ b/src/test/java/com/retailsvc/http/support/SpecAnchor.java @@ -0,0 +1,9 @@ +package com.retailsvc.http.support; + +/** + * Marker class used by classpath-loading tests as the {@code Class} argument to {@link + * com.retailsvc.http.spec.Spec#fromClasspath(Class, String)}. + */ +public final class SpecAnchor { + private SpecAnchor() {} +} diff --git a/src/test/java/com/retailsvc/http/support/SpecFixtures.java b/src/test/java/com/retailsvc/http/support/SpecFixtures.java new file mode 100644 index 00000000..75fcc936 --- /dev/null +++ b/src/test/java/com/retailsvc/http/support/SpecFixtures.java @@ -0,0 +1,81 @@ +package com.retailsvc.http.support; + +import com.google.gson.Gson; +import com.retailsvc.http.spec.Spec; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.Reader; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +/** + * Test-only helper: loads {@code /openapi.json} from the classpath, deep-clones the parsed map, and + * re-points {@code servers[0].url} so callers can derive multiple {@link Spec} instances from a + * single fixture file. + */ +public final class SpecFixtures { + + private SpecFixtures() {} + + /** Loads the test spec and rewrites {@code servers[0].url} to {@code newServerUrl}. */ + public static Spec specAt(String newServerUrl) { + Map raw = readBaseSpec(); + @SuppressWarnings("unchecked") + List> servers = (List>) raw.get("servers"); + if (servers == null || servers.isEmpty()) { + throw new IllegalStateException("test spec has no servers entry"); + } + servers.get(0).put("url", newServerUrl); + return Spec.from(raw); + } + + private static Map readBaseSpec() { + try (InputStream in = SpecFixtures.class.getResourceAsStream("/openapi.json")) { + if (in == null) { + throw new IllegalStateException("/openapi.json not found on test classpath"); + } + return parse(in); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + @SuppressWarnings("unchecked") + private static Map parse(InputStream in) { + Gson gson = new Gson(); + try (Reader reader = new InputStreamReader(in, StandardCharsets.UTF_8)) { + Map root = gson.fromJson(reader, Map.class); + return deepClone(root); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + private static Map deepClone(Map in) { + Map out = new LinkedHashMap<>(in.size()); + for (var e : in.entrySet()) { + out.put(e.getKey(), cloneValue(e.getValue())); + } + return out; + } + + @SuppressWarnings("unchecked") + private static Object cloneValue(Object v) { + if (v instanceof Map m) { + return deepClone((Map) m); + } + if (v instanceof List l) { + List out = new ArrayList<>(l.size()); + for (Object item : l) { + out.add(cloneValue(item)); + } + return out; + } + return v; + } +} diff --git a/src/test/resources/schemas/v1/openapi.json b/src/test/resources/schemas/v1/openapi.json new file mode 100644 index 00000000..8ed1c4fa --- /dev/null +++ b/src/test/resources/schemas/v1/openapi.json @@ -0,0 +1,27 @@ +{ + "openapi": "3.1.0", + "info": { + "title": "Multi-spec test API v1", + "version": "1.0.0" + }, + "servers": [ + {"url": "/api/v1"} + ], + "paths": { + "/ping": { + "get": { + "operationId": "v1-ping", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": {"type": "object"} + } + } + } + } + } + } + } +} diff --git a/src/test/resources/schemas/v2/openapi.json b/src/test/resources/schemas/v2/openapi.json new file mode 100644 index 00000000..7a024c63 --- /dev/null +++ b/src/test/resources/schemas/v2/openapi.json @@ -0,0 +1,27 @@ +{ + "openapi": "3.1.0", + "info": { + "title": "Multi-spec test API v2", + "version": "1.0.0" + }, + "servers": [ + {"url": "/api/v2"} + ], + "paths": { + "/ping": { + "get": { + "operationId": "v2-ping", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": {"type": "object"} + } + } + } + } + } + } + } +}