Merge pull request #295 from t1/#282-error-code-annotation

#282: get error extension `code` from exception name or annotation
smallrye · Jun 19, 2020 · e5213cc · e5213cc
2 parents aa97c94 + 8cbb4e5
commit e5213cc
Show file tree

Hide file tree

Showing 15 changed files with 208 additions and 25 deletions.
diff --git a/client/generator/pom.xml b/client/generator/pom.xml
@@ -67,12 +67,6 @@
             <version>${project.version}</version>
             <scope>provided</scope>
         </dependency>
-        <dependency>
-            <groupId>io.smallrye</groupId>
-            <artifactId>smallrye-graphql-api</artifactId>
-            <version>${project.version}</version>
-            <scope>provided</scope>
-        </dependency>
         <dependency>
             <groupId>com.graphql-java</groupId>
             <artifactId>graphql-java</artifactId>

diff --git a/server/api/src/main/java/io/smallrye/graphql/api/ErrorCode.java b/server/api/src/main/java/io/smallrye/graphql/api/ErrorCode.java
@@ -0,0 +1,14 @@
+package io.smallrye.graphql.api;
+
+import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+@Retention(RUNTIME)
+@Target({ TYPE, ANNOTATION_TYPE })
+public @interface ErrorCode {
+    String value();
+}
diff --git a/...lementation/src/main/java/io/smallrye/graphql/execution/error/ExecutionErrorsService.java b/...lementation/src/main/java/io/smallrye/graphql/execution/error/ExecutionErrorsService.java
@@ -1,5 +1,7 @@
 package io.smallrye.graphql.execution.error;
 
+import static java.util.Locale.UK;
+
 import java.io.StringReader;
 import java.util.List;
 import java.util.Map;
@@ -20,6 +22,7 @@
 import graphql.ExceptionWhileDataFetching;
 import graphql.GraphQLError;
 import graphql.validation.ValidationError;
+import io.smallrye.graphql.api.ErrorCode;
 
 /**
  * Help to create the exceptions
@@ -50,10 +53,7 @@ private JsonObject toJsonError(GraphQLError error) {
 
             JsonObjectBuilder resultBuilder = jsonBuilderFactory.createObjectBuilder(jsonErrors);
 
-            Optional<JsonObject> optionalExtensions = getOptionalExtensions(error);
-            if (optionalExtensions.isPresent()) {
-                resultBuilder.add(EXTENSIONS, optionalExtensions.get());
-            }
+            getOptionalExtensions(error).ifPresent(jsonObject -> resultBuilder.add(EXTENSIONS, jsonObject));
             return resultBuilder.build();
         }
     }
@@ -86,12 +86,26 @@ private Optional<JsonObject> getDataFetchingExtensions(ExceptionWhileDataFetchin
 
         addKeyValue(objectBuilder, EXCEPTION, exception.getClass().getName());
         addKeyValue(objectBuilder, CLASSIFICATION, error.getErrorType().toString());
+        addKeyValue(objectBuilder, CODE, toErrorCode(exception));
         Map<String, Object> extensions = error.getExtensions();
         populateCustomExtensions(objectBuilder, extensions);
 
         return Optional.of(objectBuilder.build());
     }
 
+    private String toErrorCode(Throwable exception) {
+        // TODO change to use the Model and the model needs to get this using Jandex
+        ErrorCode annotation = exception.getClass().getAnnotation(ErrorCode.class);
+        return (annotation == null)
+                ? camelToKebab(exception.getClass().getSimpleName().replaceAll("Exception$", ""))
+                : annotation.value();
+    }
+
+    private static String camelToKebab(String input) {
+        return String.join("-", input.split("(?=\\p{javaUpperCase})"))
+                .toLowerCase(UK);
+    }
+
     private void populateCustomExtensions(JsonObjectBuilder objectBuilder, Map<String, Object> extensions) {
         if (extensions != null) {
             for (Map.Entry<String, Object> entry : extensions.entrySet()) {
@@ -121,5 +135,6 @@ private void addKeyValue(JsonObjectBuilder objectBuilder, String key, String val
     private static final String QUERYPATH = "queryPath";
     private static final String CLASSIFICATION = "classification";
     private static final String EXTENSIONS = "extensions";
+    private static final String CODE = "code";
 
 }
diff --git a/...ntation/src/test/java/io/smallrye/graphql/execution/error/ExecutionErrorsServiceTest.java b/...ntation/src/test/java/io/smallrye/graphql/execution/error/ExecutionErrorsServiceTest.java
@@ -1,23 +1,24 @@
 package io.smallrye.graphql.execution.error;
 
+import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
 import javax.json.JsonArray;
 import javax.json.JsonObject;
 
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import graphql.ExceptionWhileDataFetching;
+import graphql.GraphQLError;
 import graphql.GraphqlErrorException;
 import graphql.execution.ExecutionPath;
 import graphql.language.SourceLocation;
 import graphql.validation.ValidationError;
 import graphql.validation.ValidationErrorType;
+import io.smallrye.graphql.api.ErrorCode;
 
 /**
  * Test for {@link ExecutionErrorsService}
@@ -26,55 +27,88 @@
  */
 class ExecutionErrorsServiceTest {
 
-    private ExecutionErrorsService executionErrorsService;
-
-    @BeforeEach
-    void init() {
-        executionErrorsService = new ExecutionErrorsService();
-    }
+    private final ExecutionErrorsService executionErrorsService = new ExecutionErrorsService();
 
     @Test
     void testToJsonErrors_WhenExceptionWhileDataFetchingErrorCaught_ShouldReturnJsonBodyWithCustomExtensions() {
         // Given
         Map<String, Object> extensions = new HashMap<>();
-        extensions.put("errorCode", "OPERATION_FAILED");
+        extensions.put("code", "OPERATION_FAILED");
         GraphqlErrorException graphqlErrorException = GraphqlErrorException.newErrorException()
                 .extensions(extensions)
                 .build();
         ExceptionWhileDataFetching exceptionWhileDataFetching = new ExceptionWhileDataFetching(ExecutionPath.rootPath(),
                 graphqlErrorException, new SourceLocation(1, 1));
 
         // When
-        JsonArray jsonArray = executionErrorsService.toJsonErrors(Collections.singletonList(exceptionWhileDataFetching));
+        JsonArray jsonArray = executionErrorsService.toJsonErrors(singletonList(exceptionWhileDataFetching));
 
         // Then
         JsonObject extensionJsonObject = jsonArray.getJsonObject(0).getJsonObject("extensions");
         assertThat(extensionJsonObject.getString("exception")).isEqualTo("graphql.GraphqlErrorException");
         assertThat(extensionJsonObject.getString("classification")).isEqualTo("DataFetchingException");
-        assertThat(extensionJsonObject.getString("errorCode")).isEqualTo("OPERATION_FAILED");
+        assertThat(extensionJsonObject.getString("code")).isEqualTo("OPERATION_FAILED");
     }
 
     @Test
     void testToJsonErrors_WhenExceptionWhileValidationErrorCaught_ShouldReturnJsonBodyWithCustomExtensions() {
         // Given
         Map<String, Object> extensions = new HashMap<>();
-        extensions.put("errorCode", "OPERATION_FAILED");
+        extensions.put("code", "OPERATION_FAILED");
         ValidationError validationError = ValidationError.newValidationError()
                 .validationErrorType(ValidationErrorType.UnknownDirective)
                 .description("TestDescription")
-                .queryPath(Collections.singletonList("Test-Path"))
+                .queryPath(singletonList("Test-Path"))
                 .extensions(extensions)
                 .build();
 
         // When
-        JsonArray jsonArray = executionErrorsService.toJsonErrors(Collections.singletonList(validationError));
+        JsonArray jsonArray = executionErrorsService.toJsonErrors(singletonList(validationError));
 
         // Then
         JsonObject extensionJsonObject = jsonArray.getJsonObject(0).getJsonObject("extensions");
         assertThat(extensionJsonObject.getString("description")).isEqualTo("TestDescription");
         assertThat(extensionJsonObject.getString("validationErrorType")).isEqualTo("UnknownDirective");
         assertThat(extensionJsonObject.getJsonArray("queryPath").getString(0)).isEqualTo("Test-Path");
         assertThat(extensionJsonObject.getString("classification")).isEqualTo("ValidationError");
-        assertThat(extensionJsonObject.getString("errorCode")).isEqualTo("OPERATION_FAILED");
+        assertThat(extensionJsonObject.getString("code")).isEqualTo("OPERATION_FAILED");
+    }
+
+    @Test
+    void shouldMapExceptionNameToCode() {
+        class DummyBusinessException extends RuntimeException {
+            public DummyBusinessException(String message) {
+                super(message);
+            }
+        }
+
+        JsonArray jsonArray = whenConverting(new DummyBusinessException("dummy-message"));
+
+        JsonObject extensions = jsonArray.getJsonObject(0).getJsonObject("extensions");
+        assertThat(extensions.getString("exception")).isEqualTo(DummyBusinessException.class.getName());
+        assertThat(extensions.getString("code", null)).isEqualTo("dummy-business");
+    }
+
+    @Test
+    void shouldMapClassAnnotationErrorCode() {
+        @ErrorCode("dummy-code")
+        class DummyBusinessException extends RuntimeException {
+            public DummyBusinessException(String message) {
+                super(message);
+            }
+        }
+
+        JsonArray jsonArray = whenConverting(new DummyBusinessException("dummy-message"));
+
+        JsonObject extensions = jsonArray.getJsonObject(0).getJsonObject("extensions");
+        assertThat(extensions.getString("exception")).isEqualTo(DummyBusinessException.class.getName());
+        assertThat(extensions.getString("code", null)).isEqualTo("dummy-code");
+    }
+
+    private JsonArray whenConverting(RuntimeException exception) {
+        ExecutionPath path = ExecutionPath.parse("/foo/bar");
+        SourceLocation location = new SourceLocation(12, 34);
+        GraphQLError graphQLError = new GraphQLExceptionWhileDataFetching(path, exception, location);
+        return executionErrorsService.toJsonErrors(singletonList(graphQLError));
     }
 }
diff --git a/server/tck/src/test/java/io/smallrye/graphql/SmallRyeGraphQLArchiveProcessor.java b/server/tck/src/test/java/io/smallrye/graphql/SmallRyeGraphQLArchiveProcessor.java
@@ -28,6 +28,7 @@
 import org.jboss.shrinkwrap.resolver.api.maven.Maven;
 
 import io.smallrye.graphql.test.apps.async.api.AsyncApi;
+import io.smallrye.graphql.test.apps.error.api.ErrorApi;
 import io.smallrye.graphql.test.apps.profile.api.ProfileGraphQLApi;
 import io.smallrye.graphql.test.apps.scalars.api.AdditionalScalarsApi;
 
@@ -70,6 +71,7 @@ public void process(Archive<?> applicationArchive, TestClass testClass) {
             testDeployment.addPackage(ProfileGraphQLApi.class.getPackage());
             testDeployment.addPackage(AdditionalScalarsApi.class.getPackage());
             testDeployment.addPackage(AsyncApi.class.getPackage());
+            testDeployment.addPackage(ErrorApi.class.getPackage());
         }
     }
 }
diff --git a/server/tck/src/test/java/io/smallrye/graphql/test/apps/error/api/ErrorApi.java b/server/tck/src/test/java/io/smallrye/graphql/test/apps/error/api/ErrorApi.java
@@ -0,0 +1,31 @@
+package io.smallrye.graphql.test.apps.error.api;
+
+import org.eclipse.microprofile.graphql.GraphQLApi;
+import org.eclipse.microprofile.graphql.Query;
+
+import io.smallrye.graphql.api.ErrorCode;
+
+@GraphQLApi
+public class ErrorApi {
+    @Query
+    public String unsupportedOperation() {
+        throw new UnsupportedOperationException("dummy-message");
+    }
+
+    public static class CustomBusinessException extends RuntimeException {
+    }
+
+    @Query
+    public String customBusinessException() {
+        throw new CustomBusinessException();
+    }
+
+    @ErrorCode("some-business-error-code")
+    public static class AnnotatedCustomBusinessException extends RuntimeException {
+    }
+
+    @Query
+    public String annotatedCustomBusinessException() {
+        throw new AnnotatedCustomBusinessException();
+    }
+}
diff --git a/server/tck/src/test/resources/tests/errors/annotatedCustomException/input.graphql b/server/tck/src/test/resources/tests/errors/annotatedCustomException/input.graphql
@@ -0,0 +1,3 @@
+{
+  annotatedCustomBusinessException
+}
diff --git a/server/tck/src/test/resources/tests/errors/annotatedCustomException/output.json b/server/tck/src/test/resources/tests/errors/annotatedCustomException/output.json
@@ -0,0 +1,24 @@
+{
+    "errors": [
+        {
+            "message": "Unexpected failure in the system. Jarvis is working to fix it.",
+            "locations": [
+                {
+                    "line": 2,
+                    "column": 3
+                }
+            ],
+            "path": [
+                "annotatedCustomBusinessException"
+            ],
+            "extensions": {
+                "exception": "io.smallrye.graphql.test.apps.error.api.ErrorApi$AnnotatedCustomBusinessException",
+                "classification": "DataFetchingException",
+                "code": "some-business-error-code"
+            }
+        }
+    ],
+    "data": {
+        "annotatedCustomBusinessException": null
+    }
+}
diff --git a/server/tck/src/test/resources/tests/errors/annotatedCustomException/test.properties b/server/tck/src/test/resources/tests/errors/annotatedCustomException/test.properties
@@ -0,0 +1,4 @@
+## error extension `code` derived from the `@GraphQlErrorCode` annotation on a custom exception
+
+ignore=false
+priority=100
diff --git a/server/tck/src/test/resources/tests/errors/builtInException/input.graphql b/server/tck/src/test/resources/tests/errors/builtInException/input.graphql
@@ -0,0 +1,3 @@
+{
+  unsupportedOperation
+}
diff --git a/server/tck/src/test/resources/tests/errors/builtInException/output.json b/server/tck/src/test/resources/tests/errors/builtInException/output.json
@@ -0,0 +1,24 @@
+{
+    "errors": [
+        {
+            "message": "Unexpected failure in the system. Jarvis is working to fix it.",
+            "locations": [
+                {
+                    "line": 2,
+                    "column": 3
+                }
+            ],
+            "path": [
+                "unsupportedOperation"
+            ],
+            "extensions": {
+                "exception": "java.lang.UnsupportedOperationException",
+                "classification": "DataFetchingException",
+                "code": "unsupported-operation"
+            }
+        }
+    ],
+    "data": {
+        "unsupportedOperation": null
+    }
+}
diff --git a/server/tck/src/test/resources/tests/errors/builtInException/test.properties b/server/tck/src/test/resources/tests/errors/builtInException/test.properties
@@ -0,0 +1,4 @@
+## error extension `code` derived from the class name of a jdk exception
+
+ignore=false
+priority=100
diff --git a/server/tck/src/test/resources/tests/errors/customException/input.graphql b/server/tck/src/test/resources/tests/errors/customException/input.graphql
@@ -0,0 +1,3 @@
+{
+  customBusinessException
+}
diff --git a/server/tck/src/test/resources/tests/errors/customException/output.json b/server/tck/src/test/resources/tests/errors/customException/output.json
@@ -0,0 +1,24 @@
+{
+    "errors": [
+        {
+            "message": "Unexpected failure in the system. Jarvis is working to fix it.",
+            "locations": [
+                {
+                    "line": 2,
+                    "column": 3
+                }
+            ],
+            "path": [
+                "customBusinessException"
+            ],
+            "extensions": {
+                "exception": "io.smallrye.graphql.test.apps.error.api.ErrorApi$CustomBusinessException",
+                "classification": "DataFetchingException",
+                "code": "custom-business"
+            }
+        }
+    ],
+    "data": {
+        "customBusinessException": null
+    }
+}
diff --git a/server/tck/src/test/resources/tests/errors/customException/test.properties b/server/tck/src/test/resources/tests/errors/customException/test.properties
@@ -0,0 +1,4 @@
+## error extension `code` derived from the class name of a custom exception
+
+ignore=false
+priority=100