refactor: Delete hand-rolled ProblemDetailRenderer, migrate SecurityFilter to ProblemDetail

Author: thced

src/main/java/com/retailsvc/http/OpenApiServer.java

@@ -103,7 +103,8 @@ record HandlerConfig(
                 spec.securitySchemes(),
                 spec.security(),
                 handlerConfig.securityValidators(),
-                handlerConfig.externalAuth()));
+                handlerConfig.externalAuth(),
+                bodyMappers.get(JSON)));
     ctx.setHandler(
         new DispatchHandler(
             handlerConfig.handlers(),

src/main/java/com/retailsvc/http/internal/ProblemDetailRenderer.java

@@ -5,20 +5,21 @@
 
 import com.retailsvc.http.Request;
 import com.retailsvc.http.SchemeValidator;
+import com.retailsvc.http.TypeMapper;
 import com.retailsvc.http.spec.Operation;
 import com.retailsvc.http.spec.security.SecurityRequirement;
 import com.retailsvc.http.spec.security.SecurityScheme;
 import com.sun.net.httpserver.Filter;
 import com.sun.net.httpserver.HttpExchange;
 import java.io.IOException;
-import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 
 public final class SecurityFilter extends Filter {
@@ -28,18 +29,21 @@ public final class SecurityFilter extends Filter {
   private final List<SecurityRequirement> rootSecurity;
   private final Map<String, SchemeValidator> validators;
   private final boolean externalAuth;
+  private final TypeMapper jsonMapper;
 
   public SecurityFilter(
       Map<String, Operation> operationsById,
       Map<String, SecurityScheme> schemes,
       List<SecurityRequirement> rootSecurity,
       Map<String, SchemeValidator> validators,
-      boolean externalAuth) {
+      boolean externalAuth,
+      TypeMapper jsonMapper) {
     this.operationsById = Map.copyOf(operationsById);
     this.schemes = Map.copyOf(schemes);
     this.rootSecurity = List.copyOf(rootSecurity);
     this.validators = Map.copyOf(validators);
     this.externalAuth = externalAuth;
+    this.jsonMapper = Objects.requireNonNull(jsonMapper, "jsonMapper must not be null");
   }
 
   @Override
@@ -119,8 +123,9 @@ private void renderRejection(HttpExchange exchange, List<GroupOutcome.Failed> fa
         failures.stream().max(Comparator.comparing(GroupOutcome.Failed::kind)).orElseThrow();
     String detail = describe(pick);
 
-    byte[] body =
-        ProblemDetailRenderer.render(status, title, detail).getBytes(StandardCharsets.UTF_8);
+    ProblemDetail problemDetail =
+        new ProblemDetail("about:blank", title, status, detail, null, null);
+    byte[] body = jsonMapper.writeTo(problemDetail);
     exchange.getResponseHeaders().add("Content-Type", "application/problem+json");
     if (!anyDenied) {
       LinkedHashSet<String> attempted = new LinkedHashSet<>();

src/main/java/com/retailsvc/http/internal/SecurityFilter.java

@@ -11,6 +11,7 @@
 
 import com.retailsvc.http.Request;
 import com.retailsvc.http.SchemeValidator;
+import com.retailsvc.http.TypeMapper;
 import com.retailsvc.http.spec.HttpMethod;
 import com.retailsvc.http.spec.Operation;
 import com.retailsvc.http.spec.security.SecurityRequirement;
@@ -23,6 +24,7 @@
 import com.sun.net.httpserver.HttpExchange;
 import java.io.ByteArrayOutputStream;
 import java.net.URI;
+import java.nio.charset.StandardCharsets;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -49,7 +51,8 @@ void allowsRequestWhenValidatorReturnsPrincipal() throws Exception {
         Map.of("bearerAuth", (req, cred) -> Optional.of("user-1"));
 
     SecurityFilter filter =
-        new SecurityFilter(Map.of("getX", op), schemes, List.of(), validators, false);
+        new SecurityFilter(
+            Map.of("getX", op), schemes, List.of(), validators, false, mockJsonMapper());
 
     HttpExchange ex = mock(HttpExchange.class);
     Headers headers = new Headers();
@@ -88,7 +91,8 @@ void passesThroughWhenOperationHasNoSecurity() throws Exception {
             Optional.empty()); // inherits root, root is empty
 
     SecurityFilter filter =
-        new SecurityFilter(Map.of("getY", op), Map.of(), List.of(), Map.of(), false);
+        new SecurityFilter(
+            Map.of("getY", op), Map.of(), List.of(), Map.of(), false, mockJsonMapper());
 
     HttpExchange ex = mock(HttpExchange.class);
     Chain chain = mock(Chain.class);
@@ -117,7 +121,8 @@ void missingCredentialReturns401WithBearerChallenge() throws Exception {
             Map.of("bearerAuth", new HttpBearer(Optional.empty())),
             List.of(),
             Map.of("bearerAuth", (req, cred) -> Optional.of("never-called")),
-            false);
+            false,
+            mockJsonMapper());
 
     HttpExchange ex = mock(HttpExchange.class);
     Headers headers = new Headers();
@@ -159,7 +164,8 @@ void deniedValidatorReturns403WithoutChallenge() throws Exception {
             Map.of("bearerAuth", new HttpBearer(Optional.empty())),
             List.of(),
             Map.of("bearerAuth", (req, cred) -> Optional.empty()),
-            false);
+            false,
+            mockJsonMapper());
 
     HttpExchange ex = mock(HttpExchange.class);
     Headers headers = new Headers();
@@ -196,7 +202,8 @@ void apiKeyMissingReturnsApiKeyChallengeHeader() throws Exception {
             Map.of("apiKeyAuth", new ApiKey("X-API-Key", Location.HEADER)),
             List.of(),
             Map.of("apiKeyAuth", (req, cred) -> Optional.of("ok")),
-            false);
+            false,
+            mockJsonMapper());
 
     HttpExchange ex = mock(HttpExchange.class);
     when(ex.getRequestHeaders()).thenReturn(new Headers());
@@ -241,7 +248,8 @@ void andGroupRequiresAllSchemesToSucceed() throws Exception {
             "bearerAuth", (req, cred) -> Optional.of("bearer-principal"));
 
     SecurityFilter filter =
-        new SecurityFilter(Map.of("getX", op), schemes, List.of(), validators, false);
+        new SecurityFilter(
+            Map.of("getX", op), schemes, List.of(), validators, false, mockJsonMapper());
 
     HttpExchange ex = mock(HttpExchange.class);
     Headers headers = new Headers();
@@ -295,7 +303,8 @@ void orFallsBackToSecondGroupWhenFirstDenied() throws Exception {
             "bearerAuth", (req, cred) -> Optional.of("bearer-ok"));
 
     SecurityFilter filter =
-        new SecurityFilter(Map.of("getX", op), schemes, List.of(), validators, false);
+        new SecurityFilter(
+            Map.of("getX", op), schemes, List.of(), validators, false, mockJsonMapper());
 
     HttpExchange ex = mock(HttpExchange.class);
     Headers headers = new Headers();
@@ -339,7 +348,8 @@ void externalAuthBypassesEverything() throws Exception {
             Map.of("bearerAuth", new HttpBearer(Optional.empty())),
             List.of(),
             Map.of(), // NO validators
-            /* externalAuth= */ true);
+            /* externalAuth= */ true,
+            mockJsonMapper());
 
     HttpExchange ex = mock(HttpExchange.class);
     Chain chain = mock(Chain.class);
@@ -351,4 +361,24 @@ void externalAuthBypassesEverything() throws Exception {
   private static Request newMinimalRequest(String operationId) {
     return new Request(new byte[0], null, null, operationId, Map.of(), null, h -> null);
   }
+
+  private static TypeMapper mockJsonMapper() {
+    return new TypeMapper() {
+      @Override
+      public Object readFrom(byte[] body, String contentTypeHeader) {
+        return null;
+      }
+
+      @Override
+      public byte[] writeTo(Object value) {
+        if (value instanceof ProblemDetail pd) {
+          return String.format(
+                  "{\"type\":\"%s\",\"title\":\"%s\",\"status\":%d,\"detail\":\"%s\"}",
+                  pd.type(), pd.title(), pd.status(), pd.detail())
+              .getBytes(StandardCharsets.UTF_8);
+        }
+        return "{}".getBytes(StandardCharsets.UTF_8);
+      }
+    };
+  }
 }