refactor: Address SonarQube code smells across new packages

thced · thced · commit 51604e45b029 · 2026-05-08T10:35:19.000+02:00
diff --git a/src/main/java/com/retailsvc/http/Handlers.java b/src/main/java/com/retailsvc/http/Handlers.java
@@ -29,7 +29,7 @@ public static ExceptionHandler defaultExceptionHandler() {
             exchange.sendResponseHeaders(HTTP_BAD_REQUEST, body.length);
             exchange.getResponseBody().write(body);
           }
-          case NotFoundException nf -> exchange.sendResponseHeaders(HTTP_NOT_FOUND, 0);
+          case NotFoundException _ -> exchange.sendResponseHeaders(HTTP_NOT_FOUND, 0);
           case MethodNotAllowedException mna -> {
             String allow = mna.allowed().stream().map(Enum::name).collect(Collectors.joining(", "));
             exchange.getResponseHeaders().add("Allow", allow);
diff --git a/src/main/java/com/retailsvc/http/ValidationException.java b/src/main/java/com/retailsvc/http/ValidationException.java
@@ -3,7 +3,7 @@
 import com.retailsvc.http.validate.ValidationError;
 
 public final class ValidationException extends RuntimeException {
-  private final ValidationError error;
+  private final transient ValidationError error;
 
   public ValidationException(ValidationError error) {
     super(error.pointer() + " [" + error.keyword() + "] " + error.message());
diff --git a/src/main/java/com/retailsvc/http/internal/ProblemDetailRenderer.java b/src/main/java/com/retailsvc/http/internal/ProblemDetailRenderer.java
@@ -14,13 +14,16 @@ public final class ProblemDetailRenderer {
   private static final String PROBLEM_TITLE = "Bad Request";
   private static final int PROBLEM_STATUS = 400;
 
+  /** Initial capacity of the JSON buffer; sized for a typical problem-detail document. */
+  private static final int INITIAL_BUFFER_CAPACITY = 128;
+
   /** Codepoints below this value are control characters and must be unicode-escaped in JSON. */
   private static final int FIRST_PRINTABLE_ASCII = 0x20;
 
   private ProblemDetailRenderer() {}
 
   public static String render(ValidationError error) {
-    StringBuilder out = new StringBuilder(128);
+    StringBuilder out = new StringBuilder(INITIAL_BUFFER_CAPACITY);
     out.append('{');
     appendStringField(out, "type", PROBLEM_TYPE);
     out.append(',');
diff --git a/src/main/java/com/retailsvc/http/spec/Spec.java b/src/main/java/com/retailsvc/http/spec/Spec.java
@@ -18,6 +18,8 @@ public record Spec(
     Map<String, Schema> componentSchemas,
     Map<String, Parameter> componentParameters) {
 
+  private static final String SCHEMA_KEY = "schema";
+
   @SuppressWarnings("unchecked")
   public static Spec from(Map<String, Object> raw) {
     String openapi = (String) raw.get("openapi");
@@ -106,7 +108,7 @@ private static Parameter parseParameter(Map<String, Object> raw) {
         Parameter.Location.valueOf(((String) raw.get("in")).toUpperCase(Locale.ROOT)),
         Boolean.TRUE.equals(raw.get("required")),
         SchemaParser.parse(
-            (Map<String, Object>) raw.getOrDefault("schema", Map.of("type", "string"))));
+            (Map<String, Object>) raw.getOrDefault(SCHEMA_KEY, Map.of("type", "string"))));
   }
 
   @SuppressWarnings("unchecked")
@@ -174,7 +176,7 @@ private static RequestBody parseRequestBody(Map<String, Object> raw) {
           e.getKey(),
           new MediaType(
               SchemaParser.parse(
-                  (Map<String, Object>) mt.getOrDefault("schema", Map.of("type", "object")))));
+                  (Map<String, Object>) mt.getOrDefault(SCHEMA_KEY, Map.of("type", "object")))));
     }
     return new RequestBody(Boolean.TRUE.equals(raw.get("required")), Map.copyOf(content));
   }
@@ -188,10 +190,10 @@ private static Map<String, Response> parseResponses(Map<String, Object> raw) {
       Map<String, MediaType> content = new LinkedHashMap<>();
       for (var ce : contentRaw.entrySet()) {
         Map<String, Object> mt = (Map<String, Object>) ce.getValue();
-        if (mt.containsKey("schema")) {
+        if (mt.containsKey(SCHEMA_KEY)) {
           content.put(
               ce.getKey(),
-              new MediaType(SchemaParser.parse((Map<String, Object>) mt.get("schema"))));
+              new MediaType(SchemaParser.parse((Map<String, Object>) mt.get(SCHEMA_KEY))));
         }
       }
       out.put(e.getKey(), new Response(Map.copyOf(content)));
diff --git a/src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java b/src/main/java/com/retailsvc/http/spec/schema/SchemaParser.java
@@ -10,6 +10,8 @@
 public final class SchemaParser {
   private SchemaParser() {}
 
+  private static final String FORMAT_KEY = "format";
+
   @SuppressWarnings("unchecked")
   public static Schema parse(Map<String, Object> raw) {
     if (raw.containsKey("$ref")) {
@@ -75,7 +77,7 @@ private static StringSchema parseString(Map<String, Object> raw, Set<TypeName> t
         (String) raw.get("pattern"),
         toIntOrNull(raw.get("minLength")),
         toIntOrNull(raw.get("maxLength")),
-        (String) raw.get("format"),
+        (String) raw.get(FORMAT_KEY),
         (List<String>) raw.get("enum"));
   }
 
@@ -87,7 +89,7 @@ private static IntegerSchema parseInteger(Map<String, Object> raw, Set<TypeName>
         toLongOrNull(raw.get("exclusiveMinimum")),
         toLongOrNull(raw.get("exclusiveMaximum")),
         toLongOrNull(raw.get("multipleOf")),
-        (String) raw.get("format"));
+        (String) raw.get(FORMAT_KEY));
   }
 
   private static NumberSchema parseNumber(Map<String, Object> raw, Set<TypeName> types) {
@@ -98,7 +100,7 @@ private static NumberSchema parseNumber(Map<String, Object> raw, Set<TypeName> t
         (Number) raw.get("exclusiveMinimum"),
         (Number) raw.get("exclusiveMaximum"),
         (Number) raw.get("multipleOf"),
-        (String) raw.get("format"));
+        (String) raw.get(FORMAT_KEY));
   }
 
   @SuppressWarnings("unchecked")
@@ -121,13 +123,12 @@ private static ObjectSchema parseObject(Map<String, Object> raw, Set<TypeName> t
 
   @SuppressWarnings("unchecked")
   private static AdditionalProperties parseAdditionalProperties(Object value) {
-    if (value == null || Boolean.TRUE.equals(value)) {
-      return new AdditionalProperties.Allowed();
-    }
-    if (Boolean.FALSE.equals(value)) {
-      return new AdditionalProperties.Forbidden();
-    }
-    return new AdditionalProperties.SchemaConstraint(parse((Map<String, Object>) value));
+    return switch (value) {
+      case null -> new AdditionalProperties.Allowed();
+      case Boolean b when b -> new AdditionalProperties.Allowed();
+      case Boolean b -> new AdditionalProperties.Forbidden();
+      default -> new AdditionalProperties.SchemaConstraint(parse((Map<String, Object>) value));
+    };
   }
 
   @SuppressWarnings("unchecked")
diff --git a/src/main/java/com/retailsvc/http/validate/DefaultValidator.java b/src/main/java/com/retailsvc/http/validate/DefaultValidator.java
@@ -21,6 +21,7 @@
 import java.math.BigDecimal;
 import java.time.LocalDate;
 import java.time.OffsetDateTime;
+import java.time.format.DateTimeParseException;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -46,21 +47,21 @@ public void validate(Object value, Schema schema, String pointer) {
 
     switch (schema) {
       case RefSchema r -> validate(value, refResolver.apply(r.pointer()), pointer);
-      case BooleanSchema b -> validateBoolean(value, pointer);
-      case NullSchema n -> require(value == null, pointer, "type", "expected null");
+      case BooleanSchema _ -> validateBoolean(value, pointer);
+      case NullSchema _ -> require(value == null, pointer, "type", "expected null");
       case StringSchema s -> validateString(value, s, pointer);
       case IntegerSchema i -> validateInteger(value, i, pointer);
       case NumberSchema n -> validateNumber(value, n, pointer);
       case ObjectSchema o -> validateObject(value, o, pointer);
       case ArraySchema a -> validateArray(value, a, pointer);
-      case EnumSchema e ->
-          require(e.values().contains(value), pointer, "enum", "value not in enum");
-      case ConstSchema c ->
-          require(Objects.equals(c.value(), value), pointer, "const", "value does not equal const");
-      case OneOfSchema o -> throw new UnsupportedOperationException("oneOf not yet supported");
-      case AnyOfSchema a -> throw new UnsupportedOperationException("anyOf not yet supported");
-      case AllOfSchema a -> throw new UnsupportedOperationException("allOf not yet supported");
-      case NotSchema n -> throw new UnsupportedOperationException("not not yet supported");
+      case EnumSchema(List<Object> values) ->
+          require(values.contains(value), pointer, "enum", "value not in enum");
+      case ConstSchema(Object expected) ->
+          require(Objects.equals(expected, value), pointer, "const", "value does not equal const");
+      case OneOfSchema _ -> throw new UnsupportedOperationException("oneOf not yet supported");
+      case AnyOfSchema _ -> throw new UnsupportedOperationException("anyOf not yet supported");
+      case AllOfSchema _ -> throw new UnsupportedOperationException("allOf not yet supported");
+      case NotSchema _ -> throw new UnsupportedOperationException("not not yet supported");
     }
   }
 
@@ -93,42 +94,46 @@ private void validateStringFormat(String str, String format, String pointer) {
       case "uuid" -> {
         try {
           UUID.fromString(str);
-        } catch (IllegalArgumentException e) {
+        } catch (IllegalArgumentException _) {
           fail(pointer, "format", "not a valid uuid", str);
         }
       }
       case "date" -> {
         try {
           LocalDate.parse(str);
-        } catch (Exception e) {
+        } catch (DateTimeParseException _) {
           fail(pointer, "format", "not a valid date", str);
         }
       }
       case "date-time" -> {
         try {
           OffsetDateTime.parse(str);
-        } catch (Exception e) {
+        } catch (DateTimeParseException _) {
           fail(pointer, "format", "not a valid date-time", str);
         }
       }
-      default -> {}
+      default -> {
+        /* unknown format ignored — handled in 3.1 follow-up */
+      }
     }
   }
 
   private void validateInteger(Object value, IntegerSchema s, String pointer) {
     long n;
-    if (value instanceof Number num) {
-      n = num.longValue();
-    } else if (value instanceof String str) {
-      try {
-        n = Long.parseLong(str);
-      } catch (NumberFormatException e) {
+    switch (value) {
+      case Number num -> n = num.longValue();
+      case String str -> {
+        try {
+          n = Long.parseLong(str);
+        } catch (NumberFormatException _) {
+          fail(pointer, "type", "expected integer", value);
+          return;
+        }
+      }
+      case null, default -> {
         fail(pointer, "type", "expected integer", value);
         return;
       }
-    } else {
-      fail(pointer, "type", "expected integer", value);
-      return;
     }
 
     if (s.minimum() != null && n < s.minimum()) {
@@ -150,18 +155,20 @@ private void validateInteger(Object value, IntegerSchema s, String pointer) {
 
   private void validateNumber(Object value, NumberSchema s, String pointer) {
     double n;
-    if (value instanceof Number num) {
-      n = num.doubleValue();
-    } else if (value instanceof String str) {
-      try {
-        n = Double.parseDouble(str);
-      } catch (NumberFormatException e) {
+    switch (value) {
+      case Number num -> n = num.doubleValue();
+      case String str -> {
+        try {
+          n = Double.parseDouble(str);
+        } catch (NumberFormatException _) {
+          fail(pointer, "type", "expected number", value);
+          return;
+        }
+      }
+      case null, default -> {
         fail(pointer, "type", "expected number", value);
         return;
       }
-    } else {
-      fail(pointer, "type", "expected number", value);
-      return;
     }
 
     if (s.minimum() != null && n < s.minimum().doubleValue()) {
@@ -219,8 +226,8 @@ private void validateObject(Object value, ObjectSchema s, String pointer) {
         validate(entry.getValue(), propSchema, childPointer);
       } else {
         switch (s.additionalProperties()) {
-          case AdditionalProperties.Allowed a -> {}
-          case AdditionalProperties.Forbidden f ->
+          case AdditionalProperties.Allowed _ -> {}
+          case AdditionalProperties.Forbidden _ ->
               fail(
                   childPointer,
                   "additionalProperties",
diff --git a/src/test/java/com/retailsvc/http/JsonMapperTest.java b/src/test/java/com/retailsvc/http/JsonMapperTest.java
@@ -7,7 +7,7 @@
 class JsonMapperTest {
   @Test
   void usableAsLambda() {
-    JsonMapper m = body -> new String(body);
+    JsonMapper m = String::new;
     assertThat(m.mapFrom("hello".getBytes())).isEqualTo("hello");
   }
 }
diff --git a/src/test/java/com/retailsvc/http/OpenApiServerTest.java b/src/test/java/com/retailsvc/http/OpenApiServerTest.java
@@ -25,7 +25,7 @@ void shouldStartHttpServerWithValidConfiguration() {
 
     assertDoesNotThrow(
         () -> {
-          try (var ignore = new OpenApiServer(validSpec, jsonMapper, handlers, onError, 0)) {
+          try (var _ = new OpenApiServer(validSpec, jsonMapper, handlers, onError, 0)) {
             // also close on exit
           }
         });
diff --git a/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java b/src/test/java/com/retailsvc/http/internal/DispatchHandlerTest.java
@@ -6,14 +6,11 @@
 import com.retailsvc.http.Request;
 import com.sun.net.httpserver.HttpExchange;
 import com.sun.net.httpserver.HttpHandler;
-import java.util.HashMap;
 import java.util.Map;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 
 class DispatchHandlerTest {
-  private final Map<String, Object> attrs = new HashMap<>();
-
   private HttpExchange exchange(String operationId) {
     HttpExchange ex = Mockito.mock(HttpExchange.class);
     Mockito.when(ex.getAttribute(Request.OPERATION_ID)).thenReturn(operationId);
@@ -30,7 +27,7 @@ void invokesRegisteredHandler() throws Exception {
   @Test
   void throwsWhenHandlerMissing() {
     DispatchHandler d = new DispatchHandler(Map.of());
-    assertThatThrownBy(() -> d.handle(exchange("ghost")))
-        .isInstanceOf(MissingOperationHandlerException.class);
+    HttpExchange ex = exchange("ghost");
+    assertThatThrownBy(() -> d.handle(ex)).isInstanceOf(MissingOperationHandlerException.class);
   }
 }
diff --git a/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java b/src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java
@@ -64,7 +64,7 @@ void successPathSetsAttributes() throws Exception {
             List.of(),
             Map.of());
     Spec spec = specWith(op);
-    JsonMapper m = body -> new String(body);
+    JsonMapper m = String::new;
     Filter f =
         new RequestPreparationFilter(
             spec, new Router(spec.operations()), new DefaultValidator(spec::resolveSchema), m);
@@ -90,7 +90,7 @@ void unknownPathThrowsNotFound() {
                 Optional.empty(),
                 List.of(),
                 Map.of()));
-    JsonMapper m = body -> new String(body);
+    JsonMapper m = String::new;
     Filter f =
         new RequestPreparationFilter(
             spec, new Router(spec.operations()), new DefaultValidator(spec::resolveSchema), m);
@@ -111,7 +111,7 @@ void wrongMethodThrowsMethodNotAllowed() {
                 Optional.empty(),
                 List.of(),
                 Map.of()));
-    JsonMapper m = body -> new String(body);
+    JsonMapper m = String::new;
     Filter f =
         new RequestPreparationFilter(
             spec, new Router(spec.operations()), new DefaultValidator(spec::resolveSchema), m);
@@ -133,7 +133,7 @@ void invalidQueryParamThrowsValidation() {
             List.of(new Parameter("q", Parameter.Location.QUERY, true, stringSchema)),
             Map.of());
     Spec spec = specWith(op);
-    JsonMapper m = body -> new String(body);
+    JsonMapper m = String::new;
     Filter f =
         new RequestPreparationFilter(
             spec, new Router(spec.operations()), new DefaultValidator(spec::resolveSchema), m);
diff --git a/src/test/java/com/retailsvc/http/spec/schema/PrimitiveSchemasTest.java b/src/test/java/com/retailsvc/http/spec/schema/PrimitiveSchemasTest.java
@@ -21,7 +21,7 @@ void stringSchemaCarriesAllStringFields() {
   @Test
   void numberSchemaCarriesAllNumericConstraints() {
     NumberSchema n = new NumberSchema(Set.of(TypeName.NUMBER), 0, 100, null, 100, 5, "double");
-    assertThat(n.minimum()).isEqualTo(0);
+    assertThat(n.minimum().intValue()).isZero();
     assertThat(n.maximum()).isEqualTo(100);
     assertThat(n.exclusiveMaximum()).isEqualTo(100);
     assertThat(n.multipleOf()).isEqualTo(5);
diff --git a/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java b/src/test/java/com/retailsvc/http/spec/schema/SchemaParserTest.java
@@ -21,7 +21,7 @@ void parsesIntegerWithFormat() {
     Schema s = SchemaParser.parse(Map.of("type", "integer", "format", "int64", "minimum", 0));
     assertThat(s).isInstanceOf(IntegerSchema.class);
     assertThat(((IntegerSchema) s).format()).isEqualTo("int64");
-    assertThat(((IntegerSchema) s).minimum()).isEqualTo(0L);
+    assertThat(((IntegerSchema) s).minimum().longValue()).isZero();
   }
 
   @Test
diff --git a/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java b/src/test/java/com/retailsvc/http/validate/DefaultValidatorDispatchTest.java
@@ -25,7 +25,8 @@ void nullSchemaAcceptsNull() {
 
   @Test
   void nullSchemaRejectsNonNull() {
-    assertThatThrownBy(() -> v.validate("x", new NullSchema(), "/v"))
+    var schema = new NullSchema();
+    assertThatThrownBy(() -> v.validate("x", schema, "/v"))
         .isInstanceOf(ValidationException.class)
         .extracting(t -> ((ValidationException) t).error().keyword())
         .isEqualTo("type");
@@ -38,13 +39,14 @@ void booleanSchemaAcceptsBoolean() {
 
   @Test
   void booleanSchemaRejectsString() {
-    assertThatThrownBy(() -> v.validate("x", new BooleanSchema(Set.of(TypeName.BOOLEAN)), "/v"))
-        .isInstanceOf(ValidationException.class);
+    var schema = new BooleanSchema(Set.of(TypeName.BOOLEAN));
+    assertThatThrownBy(() -> v.validate("x", schema, "/v")).isInstanceOf(ValidationException.class);
   }
 
   @Test
   void combinatorThrowsUnsupported() {
-    assertThatThrownBy(() -> v.validate("x", new OneOfSchema(List.of()), "/v"))
+    var schema = new OneOfSchema(List.of());
+    assertThatThrownBy(() -> v.validate("x", schema, "/v"))
         .isInstanceOf(UnsupportedOperationException.class);
   }
 }
diff --git a/src/test/java/com/retailsvc/http/validate/ObjectValidationTest.java b/src/test/java/com/retailsvc/http/validate/ObjectValidationTest.java

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ public static ExceptionHandler defaultExceptionHandler() {`
`29`	`29`	`exchange.sendResponseHeaders(HTTP_BAD_REQUEST, body.length);`
`30`	`30`	`exchange.getResponseBody().write(body);`
`31`	`31`	`}`
`32`		`- case NotFoundException nf -> exchange.sendResponseHeaders(HTTP_NOT_FOUND, 0);`
	`32`	`+ case NotFoundException _ -> exchange.sendResponseHeaders(HTTP_NOT_FOUND, 0);`
`33`	`33`	`case MethodNotAllowedException mna -> {`
`34`	`34`	`String allow = mna.allowed().stream().map(Enum::name).collect(Collectors.joining(", "));`
`35`	`35`	`exchange.getResponseHeaders().add("Allow", allow);`
Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@`
`7`	`7`	`class JsonMapperTest {`
`8`	`8`	`@Test`
`9`	`9`	`void usableAsLambda() {`
`10`		`- JsonMapper m = body -> new String(body);`
	`10`	`+ JsonMapper m = String::new;`
`11`	`11`	`assertThat(m.mapFrom("hello".getBytes())).isEqualTo("hello");`
`12`	`12`	`}`
`13`	`13`	`}`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ void shouldStartHttpServerWithValidConfiguration() {`
`25`	`25`
`26`	`26`	`assertDoesNotThrow(`
`27`	`27`	`() -> {`
`28`		`- try (var ignore = new OpenApiServer(validSpec, jsonMapper, handlers, onError, 0)) {`
	`28`	`+ try (var _ = new OpenApiServer(validSpec, jsonMapper, handlers, onError, 0)) {`
`29`	`29`	`// also close on exit`
`30`	`30`	`}`
`31`	`31`	`});`