Handle InvalidJsonException by default. Fix mapper ordering

a-dubaj · Feb 27, 2024 · b107fe1 · b107fe1
1 parent 7e89f23
commit b107fe1
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 1 deletion.
diff --git a/container-core/src/main/java/com/yahoo/restapi/RestApiImpl.java b/container-core/src/main/java/com/yahoo/restapi/RestApiImpl.java
@@ -211,7 +211,11 @@ private static List<ExceptionMapperHolder<?>> combineWithDefaultExceptionMappers
             exceptionMappers.addAll(RestApiMappers.DEFAULT_EXCEPTION_MAPPERS);
         }
         // Topologically sort children before superclasses, so most the specific match is found by iterating through mappers in order.
-        exceptionMappers.sort((a, b) -> (a.type.isAssignableFrom(b.type) ? 1 : 0) + (b.type.isAssignableFrom(a.type) ? -1 : 0));
+        exceptionMappers.sort((l, r) -> {
+            if (l.type.equals(r.type)) return 0;
+            if (l.type.isAssignableFrom(r.type)) return 1;
+            return -1;
+        });
         return exceptionMappers;
     }
 

diff --git a/container-core/src/main/java/com/yahoo/restapi/RestApiMappers.java b/container-core/src/main/java/com/yahoo/restapi/RestApiMappers.java
@@ -1,6 +1,7 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.restapi;
 
+import ai.vespa.json.InvalidJsonException;
 import ai.vespa.json.Json;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -52,6 +53,7 @@ public class RestApiMappers {
                     (context, entity) -> new JacksonJsonResponse<>(200, entity, context.jacksonJsonMapper(), true)));
 
     static List<ExceptionMapperHolder<?>> DEFAULT_EXCEPTION_MAPPERS = List.of(
+            new ExceptionMapperHolder<>(InvalidJsonException.class, (ctx, e) -> ErrorResponse.badRequest(e.getMessage())),
             new ExceptionMapperHolder<>(RestApiException.class, (context, exception) -> exception.response()));
 
     private RestApiMappers() {}

diff --git a/container-core/src/test/java/com/yahoo/restapi/RestApiImplTest.java b/container-core/src/test/java/com/yahoo/restapi/RestApiImplTest.java
@@ -1,6 +1,7 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.restapi;
 
+import ai.vespa.json.InvalidJsonException;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.yahoo.container.jdisc.AclMapping;
 import com.yahoo.container.jdisc.HttpRequestBuilder;
@@ -20,6 +21,8 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.MissingFormatWidthException;
+import java.util.NoSuchElementException;
 import java.util.Set;
 
 import static com.yahoo.jdisc.http.HttpRequest.Method;
@@ -98,6 +101,23 @@ void handles_custom_response_and_exception_mapper() {
         verifyJsonResponse(restApi, Method.GET, "/exception", null, 500, "{\"message\":\"oops division by zero\", \"error-code\":\"INTERNAL_SERVER_ERROR\"}");
     }
 
+    @Test
+    void chooses_most_specific_exception_mapper() {
+        RestApi restApi = RestApi.builder()
+                .addRoute(route("/json").get(ctx -> { throw new InvalidJsonException("oops invalid json"); }))
+                .addRoute(route("/illegal-argument").get(ctx -> { throw new IllegalArgumentException(); }))
+                .addRoute(route("/bad-format").get(ctx -> { throw new MissingFormatWidthException(""); }))
+                .addExceptionMapper(IllegalArgumentException.class, (ctx, exception) -> ErrorResponse.badRequest("oops illegal argument"))
+                .addExceptionMapper(NoSuchElementException.class, (ctx, exception) -> ErrorResponse.badRequest("oops no such element"))
+                .addExceptionMapper(RuntimeException.class, (ctx, exception) -> ErrorResponse.internalServerError("oops runtime"))
+                .addExceptionMapper(MissingFormatWidthException.class, (ctx, exception) -> ErrorResponse.internalServerError("oops bad format width"))
+                .build();
+        // Uses default mapper for `InvalidJsonException` since it's more specific than `IllegalArgumentException`
+        verifyJsonResponse(restApi, Method.GET, "/json", null, 400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"oops invalid json\"}");
+        verifyJsonResponse(restApi, Method.GET, "/illegal-argument", null, 400, "{\"message\":\"oops illegal argument\", \"error-code\":\"BAD_REQUEST\"}");
+        verifyJsonResponse(restApi, Method.GET, "/bad-format", null, 500, "{\"message\":\"oops bad format width\", \"error-code\":\"INTERNAL_SERVER_ERROR\"}");
+    }
+
     @Test
     void method_handler_can_consume_and_produce_json() {
         RestApi.HandlerWithRequestEntity<TestEntity, TestEntity> handler = (context, requestEntity) -> requestEntity;