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-openapi/README.md

@@ -33,6 +33,18 @@ Every class-based route is documented, annotated or not. Read off the code:
 
 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

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

@@ -65,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);
 
@@ -378,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));
                     }
                 }
             }
+        }
         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

@@ -220,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();