Merge pull request 'fix(ext-openapi): one shared answer per status, not a numbered family' (#24) from fix/openapi/one-shared-answer-per-status into master

Hoisting named a shared response after its status and disambiguated with a
counter, so a document with three wordings for 403 grew Forbidden, Forbidden2
and Forbidden3 in its components. Numbered names say nothing and move as soon
as a route is added.

The answer a status is usually given is now the one hoisted, under that
status's own name, and a route that answers the same status differently keeps
its wording inline where it belongs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

flash-extensions/flash-ext-jackson-core/README.md

@@ -38,6 +38,18 @@ public record NewUser(@NotBlank @Size(max = 80) String name, @Email String email
 Supported: `@NotNull`, `@NotBlank`, `@NotEmpty`, `@Size`, `@Min`, `@Max`, `@Email`, `@Pattern`.
 Jakarta semantics: only `@NotNull` rejects null, every other constraint passes it.
 
+A failure reads `<field> <message>`, and the message is the constraint's own when it sets one —
+which is how the person who sent the request is told something better than a regex:
+
+```java
+@Pattern(regexp = "[A-Za-z0-9_][A-Za-z0-9_.-]{0,254}", message = "uses up to 255 letters, digits, _, . and -")
+String key
+```
+
+```json
+{"error": "key uses up to 255 letters, digits, _, . and -", "status": 422}
+```
+
 The constraints of a type are compiled the first time it is seen and kept in a `ClassValue`,
 beside the class itself — no map, no lock. A check reads the field through an exact-signature
 `MethodHandle`: no boxing, no argument array, no iterator, and nothing allocated at all unless

flash-extensions/flash-ext-jackson-core/src/main/java/dev/relism/flash/ext/jackson/Validator.java

@@ -178,25 +178,28 @@ public final class Validator {
         Class<?> type = field.getType();
         MethodHandle ref = type.isPrimitive() ? null : asReference(getter);
 
-        if (field.isAnnotationPresent(NotNull.class) && ref != null)
-            checks.add(Check.reference(Check.NOT_NULL, name, "must not be null", ref));
+        NotNull notNull = field.getAnnotation(NotNull.class);
+        if (notNull != null && ref != null)
+            checks.add(Check.reference(Check.NOT_NULL, name, said(notNull.message(), "must not be null"), ref));
 
-        if (field.isAnnotationPresent(NotBlank.class) && ref != null)
-            checks.add(Check.reference(Check.NOT_BLANK, name, "must not be blank", ref));
+        NotBlank notBlank = field.getAnnotation(NotBlank.class);
+        if (notBlank != null && ref != null)
+            checks.add(Check.reference(Check.NOT_BLANK, name, said(notBlank.message(), "must not be blank"), ref));
 
-        if (field.isAnnotationPresent(NotEmpty.class) && ref != null)
-            checks.add(Check.reference(Check.NOT_EMPTY, name, "must not be empty", ref));
+        NotEmpty notEmpty = field.getAnnotation(NotEmpty.class);
+        if (notEmpty != null && ref != null)
+            checks.add(Check.reference(Check.NOT_EMPTY, name, said(notEmpty.message(), "must not be empty"), ref));
 
         Size size = field.getAnnotation(Size.class);
         if (size != null && ref != null)
-            checks.add(Check.size(name, sizeMessage(size), ref, size.min(), size.max()));
+            checks.add(Check.size(name, said(size.message(), sizeMessage(size)), ref, size.min(), size.max()));
 
         Min min = field.getAnnotation(Min.class);
         Max max = field.getAnnotation(Max.class);
         if (min != null || max != null) {
             long lo = min != null ? min.value() : Long.MIN_VALUE;
             long hi = max != null ? max.value() : Long.MAX_VALUE;
-            String message = rangeMessage(min, max);
+            String message = said(min != null ? min.message() : max.message(), rangeMessage(min, max));
             if (isIntegralPrimitive(type)) {
                 checks.add(Check.rangePrimitive(name, message, asLong(getter), lo, hi));
             } else if (Number.class.isAssignableFrom(type) && ref != null) {
@@ -204,14 +207,15 @@ public final class Validator {
             }
         }
 
-        if (field.isAnnotationPresent(Email.class) && ref != null)
-            checks.add(Check.reference(Check.EMAIL, name, "must be a well-formed email address", ref));
+        Email email = field.getAnnotation(Email.class);
+        if (email != null && ref != null)
+            checks.add(Check.reference(Check.EMAIL, name, said(email.message(), "must be a well-formed email address"), ref));
 
         Pattern pattern = field.getAnnotation(Pattern.class);
         if (pattern != null && ref != null) {
             // ponytail: the one allocating check — Pattern.matcher() per call. The regex itself is
             // compiled once here; swap for a structural check if a hot route ever needs it.
-            checks.add(Check.pattern(name, "must match " + pattern.regexp(), ref,
+            checks.add(Check.pattern(name, said(pattern.message(), "must match " + pattern.regexp()), ref,
                     java.util.regex.Pattern.compile(pattern.regexp())));
         }
     }
@@ -228,6 +232,15 @@ public final class Validator {
         return getter.asType(MethodType.methodType(long.class, Object.class));
     }
 
+    /**
+     * What a failure says: the constraint's own {@code message} when it sets one, and otherwise a
+     * plain description of the rule. Jakarta's defaults are bundle keys in braces, and a key is
+     * not something to put in front of whoever sent the request.
+     */
+    private static String said(String message, String otherwise) {
+        return message == null || message.isBlank() || message.startsWith("{") ? otherwise : message;
+    }
+
     private static String sizeMessage(Size size) {
         if (size.min() == 0) return "size must be at most " + size.max();
         if (size.max() == Integer.MAX_VALUE) return "size must be at least " + size.min();

flash-extensions/flash-ext-jackson-core/src/test/java/dev/relism/flash/ext/jackson/ValidatorTest.java

@@ -114,4 +114,15 @@ class ValidatorTest {
         assertTrue(validator.isEmpty());
         assertDoesNotThrow(() -> validator.verify(new Plain(null)));
     }
+
+    record Keyed(@jakarta.validation.constraints.Pattern(regexp = "[a-z.]+",
+            message = "uses lowercase letters and dots") String key) {}
+
+    @org.junit.jupiter.api.Test
+    void a_constraint_says_what_it_wants_in_its_own_words() {
+        ValidationException refused = org.junit.jupiter.api.Assertions.assertThrows(
+                ValidationException.class, () -> Validator.check(new Keyed("Not A Key")));
+
+        org.junit.jupiter.api.Assertions.assertEquals("key uses lowercase letters and dots", refused.getMessage());
+    }
 }

flash-extensions/flash-ext-jackson-json/src/test/java/dev/relism/flash/ext/jackson/json/ConstraintsInTheDocumentTest.java

@@ -33,7 +33,7 @@ class ConstraintsInTheDocumentTest {
 
     @GET("/accounts")
     @ApiOperation(summary = "List accounts")
-    @APIResponse(responseCode = "200", content = @Content(contentType = ContentType.JSON, schema = Account.class))
+    @APIResponse(responseCode = "200", content = @Content(schema = Account.class))
     public static class ListAccounts extends RequestHandler {
         @Override public Object handle(Request request, Response response) {
             return new Account("alice", "a@b.com", 30);

flash-extensions/flash-ext-openapi/README.md

@@ -26,11 +26,25 @@ Every class-based route is documented, annotated or not. Read off the code:
 - **path parameters**, from `/{id}` in the route
 - **the request body**, from the handler's own body type (see below)
 - **the response schema**, from what `handle` returns — an object, a `List<T>`, a `Map<String, T>`
+- **the media types**: `@Consumes` for what it reads, `@Produces` for what it answers, which are
+  two different questions — taking a JSON body does not make the answer JSON
 - **error responses**, in the one shape Flash answers failures with: `{"error": "...", "status": 404}`
 - **security and rate limiting**, from the extensions that enforce them
 
 Annotations add what the code cannot say: prose, extra statuses, examples. They never repeat it.
 
+## Leaving a route out
+
+```java
+@GET("/healthz")
+@Undocumented
+public final class Health extends RequestHandler { ... }
+```
+
+Every route is documented, so a document never lies by omission. `@Undocumented` says a route is
+not part of the API — a health check, an internal callback, something on its way out. On a base
+class it leaves out every handler written against it.
+
 ## Request bodies
 
 A handler that extends `BodyHandler` — `JsonHandler` and `XmlHandler`, and anything else that
@@ -53,8 +67,8 @@ requestBody:
       schema: { $ref: '#/components/schemas/NewUser' }
 ```
 
-The media type comes from `@Consumes` on the base class, so an XML handler documents itself as
-XML without a word from the route.
+The media type of the body comes from `@Consumes` on the base class, so an XML handler documents
+itself as XML without a word from the route.
 
 For a handler that reads the body by hand, or to describe it as something else, declare it:
 
@@ -83,13 +97,17 @@ The success response is inferred. Declare one only to say more:
 @APIResponse(responseCode = "200", description = "User found",
              content = @Content(schema = UserDto.class, example = "{\"id\":\"usr-1\"}"))
 @APIResponse(responseCode = "409", description = "That email is taken")
-@APIResponse(responseCode = "204", content = @Content(contentType = ContentType.NONE))
+@APIResponse(responseCode = "204", description = "Deleted")
 ```
 
 - `content.schema` omitted on a 2xx: the handler's return type.
-- Any 4xx or 5xx without an explicit schema: Flash's error object, referenced from `components`.
+- Any 4xx or 5xx without an explicit schema: Flash's error object, referenced from `components` —
+  in JSON, which is what Flash answers a failure with whatever the route produces.
 - `content.array = true` wraps whichever schema was chosen.
-- `contentType = NONE` documents a response with no body.
+- No schema and nothing to infer one from: a response with no body, which is what a 204 is.
+
+The media type of every answer is the handler's `@Produces`, JSON when nothing says otherwise. It
+is not on `@Content`: one handler answers in one format, and a status code does not change that.
 
 **A response several operations share is written once.** Identical answers — the 401 of every
 guarded route, the 429 of every limited one — become `components.responses` entries referenced by

flash-extensions/flash-ext-openapi/src/main/java/dev/relism/flash/ext/openapi/Content.java

@@ -1,19 +1,20 @@
 package dev.relism.flash.ext.openapi;
 
-import dev.relism.flash.http.ContentType;
-
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 /**
- * OpenAPI response content descriptor.
+ * What a response carries: the schema, and an example of it.
+ *
+ * <p>The media type is not here — it is the handler's, declared with
+ * {@link dev.relism.flash.routing.Produces @Produces} and JSON when nothing says otherwise. A
+ * response with no schema, and no return type to infer one from, is documented without a body.
  */
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.TYPE, ElementType.FIELD, ElementType.METHOD})
 public @interface Content {
-    ContentType contentType() default ContentType.JSON;
     Class<?> schema() default Void.class;
     boolean array() default false;
 

flash-extensions/flash-ext-openapi/src/main/java/dev/relism/flash/ext/openapi/OpenApiBuilder.java

@@ -5,6 +5,7 @@ import dev.relism.flash.http.HttpMethod;
 import dev.relism.flash.http.HttpStatus;
 import dev.relism.flash.models.BodyHandler;
 import dev.relism.flash.routing.Consumes;
+import dev.relism.flash.routing.Produces;
 import dev.relism.flash.routing.Route;
 
 import java.lang.annotation.Annotation;
@@ -64,8 +65,13 @@ public final class OpenApiBuilder {
     public OpenApiBuilder description(String description) { this.description = description; return this; }
     void setContributorRegistry(OpenApiContributorRegistry registry) { this.contributorRegistry = registry; }
 
-    /** Documents one route. {@code op} is optional: a route without it is still an operation. */
+    /**
+     * Documents one route. {@code op} is optional: a route without it is still an operation.
+     * A handler marked {@link Undocumented} is left out entirely.
+     */
     public void addOperation(Route route, ApiOperation op, Class<?> handlerClass) {
+        if (handlerClass.isAnnotationPresent(Undocumented.class)) return;
+
         String path = normalizePath(route.path());
         String method = route.method().name().toLowerCase(Locale.ROOT);
 
@@ -230,15 +236,13 @@ public final class OpenApiBuilder {
         response.put("description", declared.description().isEmpty() ? reasonFor(status) : declared.description());
 
         Content content = declared.content();
-        if (content.contentType() == ContentType.NONE) return response;
-
         Map<String, Object> schema = schemaFor(content, status, handlerClass);
-        if (schema == null) return response;
+        if (schema == null) return response;   // nothing to describe: a response with no body
 
         Map<String, Object> media = new LinkedHashMap<>();
         media.put("schema", schema);
         if (!content.example().isEmpty()) media.put("example", content.example());
-        response.put("content", Map.of(mediaTypeOf(content.contentType()), media));
+        response.put("content", Map.of(mediaTypeOf(producedBy(handlerClass)), media));
         return response;
     }
 
@@ -259,10 +263,21 @@ public final class OpenApiBuilder {
     private Map<String, Object> inferredResponse(Class<?> handlerClass) {
         Map<String, Object> response = new LinkedHashMap<>(Map.of("description", reasonFor(200)));
         Map<String, Object> schema = returnSchema(handlerClass);
-        if (schema != null) response.put("content", Map.of(mediaTypeOf(ContentType.JSON), Map.of("schema", schema)));
+        if (schema != null) {
+            response.put("content", Map.of(mediaTypeOf(producedBy(handlerClass)), Map.of("schema", schema)));
+        }
         return response;
     }
 
+    /**
+     * What this handler answers in. {@code @Consumes} says what it reads, which is a different
+     * question: taking a JSON body does not make the answer JSON.
+     */
+    private static ContentType producedBy(Class<?> handlerClass) {
+        Produces produces = handlerClass.getAnnotation(Produces.class);
+        return produces == null ? ContentType.JSON : produces.value();
+    }
+
     /** The schema of whatever {@code handle} gives back, or null when it says nothing useful. */
     private Map<String, Object> returnSchema(Class<?> handlerClass) {
         Type returned = returnTypeOf(handlerClass);
@@ -336,7 +351,7 @@ public final class OpenApiBuilder {
         // A status a contributor added is a failure Flash answers in its own shape.
         if (status >= 400 && !response.containsKey("content")) {
             response.put("content", Map.of(mediaTypeOf(ContentType.JSON), Map.of("schema", errorSchema())));
-        }
+        }   // Flash answers a failure in JSON whatever the route produces
     }
 
     private void applyContributorSecurity(Map<String, Object> operation, Class<?> handlerClass,
@@ -368,53 +383,49 @@ public final class OpenApiBuilder {
     // ── Shared responses ──────────────────────────────────────────────────────
 
     /**
-     * Whatever answer more than one operation gives identically is written once under
-     * {@code components.responses} and referenced. Authentication and rate limiting say the same
-     * thing on every route they guard; the document should say it once.
+     * The answer a status is usually given is written once under {@code components.responses} and
+     * referenced. Authentication and rate limiting say the same thing on every route they guard;
+     * the document should say it once, under that status's own name. A route that answers the same
+     * status differently keeps its own wording, inline, rather than pushing a second name into the
+     * components.
      */
     @SuppressWarnings("unchecked")
     private Map<String, Object> hoistSharedResponses(Map<String, Object> renderedPaths) {
-        Map<Object, Integer> seen = new HashMap<>();
+        Map<String, Map<Object, Integer>> seen = new LinkedHashMap<>();
         for (Object pathItem : renderedPaths.values()) {
             for (Object operation : ((Map<String, Object>) pathItem).values()) {
                 Map<String, Object> responses = (Map<String, Object>) ((Map<String, Object>) operation).get("responses");
-                responses.forEach((status, response) -> seen.merge(key(status, response), 1, Integer::sum));
+                responses.forEach((status, response) ->
+                        seen.computeIfAbsent(status, s -> new LinkedHashMap<>()).merge(response, 1, Integer::sum));
             }
         }
 
-        Map<Object, String> names = new LinkedHashMap<>();
         Map<String, Object> shared = new LinkedHashMap<>();
+        Map<String, Object> hoisted = new LinkedHashMap<>();   // status to the one body that is shared
+        seen.forEach((status, bodies) -> {
+            Map.Entry<Object, Integer> commonest = bodies.entrySet().stream()
+                    .max(Map.Entry.comparingByValue()).orElseThrow();
+            if (commonest.getValue() < 2) return;
+            String name = reasonFor(Integer.parseInt(status)).replace(" ", "");
+            if (name.isEmpty() || shared.containsKey(name)) name = "Status" + status;
+            shared.put(name, commonest.getKey());
+            hoisted.put(status, name);
+        });
+
         for (Object pathItem : renderedPaths.values()) {
             for (Object operation : ((Map<String, Object>) pathItem).values()) {
                 Map<String, Object> responses = (Map<String, Object>) ((Map<String, Object>) operation).get("responses");
                 for (var response : responses.entrySet()) {
-                    Object key = key(response.getKey(), response.getValue());
-                    if (seen.getOrDefault(key, 0) < 2) continue;
-
-                    String name = names.get(key);
-                    if (name == null) {
-                        name = uniqueName(reasonFor(Integer.parseInt(response.getKey())), shared);
-                        names.put(key, name);
-                        shared.put(name, response.getValue());
+                    String name = (String) hoisted.get(response.getKey());
+                    if (name != null && shared.get(name).equals(response.getValue())) {
+                        response.setValue(Map.of("$ref", RESPONSE_REF + name));
                     }
-                    response.setValue(Map.of("$ref", RESPONSE_REF + name));
                 }
             }
         }
         return shared;
     }
 
-    private static Object key(String status, Object response) {
-        return status + response;
-    }
-
-    private static String uniqueName(String reason, Map<String, Object> taken) {
-        String base = reason.isEmpty() ? "Response" : reason.replace(" ", "");
-        String name = base;
-        for (int i = 2; taken.containsKey(name); i++) name = base + i;
-        return name;
-    }
-
     // ── Odds and ends ─────────────────────────────────────────────────────────
 
     private static Map<String, Object> arrayOf(Map<String, Object> items) {

flash-extensions/flash-ext-openapi/src/main/java/dev/relism/flash/ext/openapi/Undocumented.java

@@ -0,0 +1,21 @@
+package dev.relism.flash.ext.openapi;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Inherited;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Keeps a route out of the published document.
+ *
+ * <p>Every class-based route is documented, which is what stops a document from lying by
+ * omission. Some routes are not part of the API anyway — a health check, an internal callback,
+ * something on its way out — and this says so, once, where the handler is.
+ *
+ * <p>Inherited: on a base class it leaves out every handler written against it.
+ */
+@Inherited
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
+public @interface Undocumented {}

flash-extensions/flash-ext-openapi/src/test/java/dev/relism/flash/ext/openapi/OpenApiBuilderTest.java

@@ -23,7 +23,7 @@ class OpenApiBuilderTest {
     @GET("/users/{id}")
     @ApiOperation(summary = "Get user")
     @Parameter(name = "expand", in = ParameterIn.QUERY, required = false, type = SchemaType.STRING, examples = {"roles", "permissions"})
-    @APIResponse(responseCode = "200", description = "User found", content = @Content(contentType = ContentType.JSON, schema = UserDto.class))
+    @APIResponse(responseCode = "200", description = "User found", content = @Content(schema = UserDto.class))
     static class GetUserHandler extends RequestHandler {
         @Override
         public Object handle(Request request, Response response) {
@@ -33,7 +33,7 @@ class OpenApiBuilderTest {
 
     @GET("/users")
     @ApiOperation(summary = "List users")
-    @APIResponse(responseCode = "200", content = @Content(contentType = ContentType.JSON, schema = UserDto.class, array = true))
+    @APIResponse(responseCode = "200", content = @Content(schema = UserDto.class, array = true))
     static class ListUsersHandler extends RequestHandler {
         @Override
         public Object handle(Request request, Response response) {
@@ -43,7 +43,7 @@ class OpenApiBuilderTest {
 
     @GET("/ping")
     @ApiOperation(summary = "Ping")
-    @APIResponse(responseCode = "204", description = "No content", content = @Content(contentType = ContentType.NONE))
+    @APIResponse(responseCode = "204", description = "No content")
     static class PingHandler extends RequestHandler {
         @Override
         public Object handle(Request request, Response response) {
@@ -169,6 +169,26 @@ class OpenApiBuilderTest {
         @Override public Object handle(Request request, Response response) { return null; }
     }
 
+    @GET("/xml")
+    @dev.relism.flash.routing.Produces(ContentType.XML)
+    @ApiOperation(summary = "Answers XML")
+    static class XmlAnswerHandler extends RequestHandler {
+        @Override public UserDto handle(Request request, Response response) { return null; }
+    }
+
+    @Test
+    void the_media_type_of_an_answer_is_the_handler_s_own() {
+        OpenApiBuilder b = new OpenApiBuilder();
+        b.addOperation(OpenApiBuilder.routeOf(XmlAnswerHandler.class), XmlAnswerHandler.class.getAnnotation(ApiOperation.class), XmlAnswerHandler.class);
+
+        Map<String, Object> get = getOperation(b.build(), "/xml", "get");
+        Map<String, Object> responses = cast(get.get("responses"));
+        Map<String, Object> ok = cast(responses.get("200"));
+        Map<String, Object> content = cast(ok.get("content"));
+
+        assertTrue(content.containsKey("application/xml"), content.keySet().toString());
+    }
+
     @Test
     void a_typed_handler_documents_its_body_without_saying_the_type_twice() {
         OpenApiBuilder b = new OpenApiBuilder();
@@ -200,6 +220,24 @@ class OpenApiBuilderTest {
         assertEquals("array", schema.get("type"));
     }
 
+    @GET("/internal")
+    @Undocumented
+    static class InternalHandler extends RequestHandler {
+        @Override public Object handle(Request request, Response response) { return null; }
+    }
+
+    @Test
+    void a_route_that_says_it_is_not_part_of_the_api_is_left_out() {
+        OpenApiBuilder b = new OpenApiBuilder();
+        b.addOperation(OpenApiBuilder.routeOf(InternalHandler.class), null, InternalHandler.class);
+        b.addOperation(OpenApiBuilder.routeOf(BareHandler.class), null, BareHandler.class);
+
+        Map<String, Object> paths = cast(b.build().get("paths"));
+
+        assertFalse(paths.containsKey("/internal"));
+        assertTrue(paths.containsKey("/bare"), "the others are still documented");
+    }
+
     @Test
     void a_route_with_no_annotations_is_still_documented() {
         OpenApiBuilder b = new OpenApiBuilder();

flash/src/main/java/dev/relism/flash/routing/Produces.java

@@ -0,0 +1,26 @@
+package dev.relism.flash.routing;
+
+import dev.relism.flash.http.ContentType;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Inherited;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * The media type a handler answers in.
+ *
+ * <p>Independent of {@link Consumes}: what a request carries says nothing about what the answer
+ * is written as, and a handler that takes JSON may well answer something else. Declare it on the
+ * handler, or once on a base class every handler of that kind extends.
+ *
+ * <p>Descriptive, like {@link Consumes}: tooling reads it, the router does not. What actually
+ * serializes a response is the middleware the route runs under.
+ */
+@Inherited
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
+public @interface Produces {
+    ContentType value();
+}