fix #299: add ErrorExtensionProvider via ServiceLoader to allow custo…

…m GraphQL error `extension` fields; use it for `code` and `exception` (class name)
smallrye · May 13, 2022 · 79d507b · 79d507b
1 parent 1d83c1d
commit 79d507b
Show file tree

Hide file tree

Showing 12 changed files with 120 additions and 42 deletions.
diff --git a/server/api/src/main/java/io/smallrye/graphql/api/ErrorExtensionProvider.java b/server/api/src/main/java/io/smallrye/graphql/api/ErrorExtensionProvider.java
@@ -0,0 +1,13 @@
+package io.smallrye.graphql.api;
+
+import jakarta.json.JsonValue;
+
+/**
+ * To add you own GraphQL error <code>extension</code> fields, you can add your own implementations
+ * of this class via the {@link java.util.ServiceLoader ServiceLoader} mechanism.
+ */
+public interface ErrorExtensionProvider {
+    String getKey();
+
+    JsonValue mapValueFrom(Throwable exception);
+}
diff --git a/server/implementation/src/main/java/io/smallrye/graphql/execution/ExecutionResponse.java b/server/implementation/src/main/java/io/smallrye/graphql/execution/ExecutionResponse.java
@@ -59,9 +59,7 @@ public JsonObject getExecutionResultAsJsonObject() {
         // Extensions
         returnObjectBuilder = addExtensionsToResponse(returnObjectBuilder, executionResult);
 
-        JsonObject jsonResponse = returnObjectBuilder.build();
-
-        return jsonResponse;
+        return returnObjectBuilder.build();
     }
 
     public String getExecutionResultAsString() {
@@ -75,10 +73,8 @@ private JsonObjectBuilder addErrorsToResponse(JsonObjectBuilder returnObjectBuil
             if (!jsonArray.isEmpty()) {
                 returnObjectBuilder = returnObjectBuilder.add(ERRORS, jsonArray);
             }
-            return returnObjectBuilder;
-        } else {
-            return returnObjectBuilder;
         }
+        return returnObjectBuilder;
     }
 
     private JsonObjectBuilder addDataToResponse(JsonObjectBuilder returnObjectBuilder, ExecutionResult executionResult) {
@@ -123,7 +119,7 @@ private JsonObject buildExtensions(final Map<Object, Object> extensions) {
      * GraphQL returns a limited set of values ({@code Collection}, {@code Map}, {@code Number}, {@code Boolean}, {@code Enum}),
      * so the json value is build by hand.
      * Additionally, {@code JsonB} is used as a fallback if an different type is encountered.
-     * 
+     *
      * @param pojo a java object, limited to {@code Collection}, {@code Map}, {@code Number}, {@code Boolean} and {@code Enum}
      * @return the json value
      */

diff --git a/...ntation/src/main/java/io/smallrye/graphql/execution/error/ErrorCodeExtensionProvider.java b/...ntation/src/main/java/io/smallrye/graphql/execution/error/ErrorCodeExtensionProvider.java
@@ -0,0 +1,36 @@
+package io.smallrye.graphql.execution.error;
+
+import static java.util.Locale.ROOT;
+
+import jakarta.json.Json;
+import jakarta.json.JsonValue;
+
+import io.smallrye.graphql.api.ErrorExtensionProvider;
+import io.smallrye.graphql.schema.model.ErrorInfo;
+import io.smallrye.graphql.spi.config.Config;
+
+public class ErrorCodeExtensionProvider implements ErrorExtensionProvider {
+    @Override
+    public String getKey() {
+        return Config.ERROR_EXTENSION_CODE;
+    }
+
+    @Override
+    public JsonValue mapValueFrom(Throwable exception) {
+        return Json.createValue(errorCode(exception));
+    }
+
+    private String errorCode(Throwable exception) {
+        ErrorInfo errorInfo = ErrorInfoMap.getErrorInfo(exception.getClass().getName());
+        if (errorInfo == null) {
+            return camelToKebab(exception.getClass().getSimpleName().replaceAll("Exception$", ""));
+        } else {
+            return errorInfo.getErrorCode();
+        }
+    }
+
+    private static String camelToKebab(String input) {
+        return String.join("-", input.split("(?=\\p{javaUpperCase})"))
+                .toLowerCase(ROOT);
+    }
+}
diff --git a/server/implementation/src/main/java/io/smallrye/graphql/execution/error/ErrorInfoMap.java b/server/implementation/src/main/java/io/smallrye/graphql/execution/error/ErrorInfoMap.java
@@ -7,7 +7,7 @@
 
 /**
  * Here we create a mapping of all error info that we know about
- * 
+ *
  * @author Phillip Kruger ([email protected])
  */
 public class ErrorInfoMap {
@@ -23,10 +23,6 @@ public static void register(Map<String, ErrorInfo> map) {
         }
     }
 
-    public static boolean hasErrorInfo(String className) {
-        return errorInfoMap.containsKey(className);
-    }
-
     public static ErrorInfo getErrorInfo(String className) {
         if (errorInfoMap.containsKey(className)) {
             return errorInfoMap.get(className);

diff --git a/...rc/main/java/io/smallrye/graphql/execution/error/ExceptionNameErrorExtensionProvider.java b/...rc/main/java/io/smallrye/graphql/execution/error/ExceptionNameErrorExtensionProvider.java
@@ -0,0 +1,19 @@
+package io.smallrye.graphql.execution.error;
+
+import jakarta.json.Json;
+import jakarta.json.JsonString;
+
+import io.smallrye.graphql.api.ErrorExtensionProvider;
+import io.smallrye.graphql.spi.config.Config;
+
+public class ExceptionNameErrorExtensionProvider implements ErrorExtensionProvider {
+    @Override
+    public String getKey() {
+        return Config.ERROR_EXTENSION_EXCEPTION;
+    }
+
+    @Override
+    public JsonString mapValueFrom(Throwable exception) {
+        return Json.createValue(exception.getClass().getName());
+    }
+}
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,11 +1,10 @@
 package io.smallrye.graphql.execution.error;
 
-import static java.util.Locale.UK;
-
 import java.io.StringReader;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.ServiceLoader;
 
 import jakarta.json.Json;
 import jakarta.json.JsonArray;
@@ -15,14 +14,15 @@
 import jakarta.json.JsonObjectBuilder;
 import jakarta.json.JsonReader;
 import jakarta.json.JsonReaderFactory;
+import jakarta.json.JsonValue;
 import jakarta.json.bind.Jsonb;
 import jakarta.json.bind.JsonbBuilder;
 import jakarta.json.bind.JsonbConfig;
 
 import graphql.ExceptionWhileDataFetching;
 import graphql.GraphQLError;
 import graphql.validation.ValidationError;
-import io.smallrye.graphql.schema.model.ErrorInfo;
+import io.smallrye.graphql.api.ErrorExtensionProvider;
 import io.smallrye.graphql.spi.config.Config;
 
 /**
@@ -38,7 +38,7 @@ public class ExecutionErrorsService {
             .withNullValues(Boolean.TRUE)
             .withFormatting(Boolean.TRUE));
 
-    private Config config = Config.get();
+    private final Config config = Config.get();
 
     public JsonArray toJsonErrors(List<GraphQLError> errors) {
         JsonArrayBuilder arrayBuilder = jsonBuilderFactory.createArrayBuilder();
@@ -71,7 +71,6 @@ private Optional<JsonObject> getOptionalExtensions(GraphQLError error) {
     }
 
     private Optional<JsonObject> getValidationExtensions(ValidationError error) {
-
         if (config.getErrorExtensionFields().isPresent()) {
             JsonObjectBuilder objectBuilder = jsonBuilderFactory.createObjectBuilder();
             addKeyValue(objectBuilder, Config.ERROR_EXTENSION_DESCRIPTION, error.getDescription());
@@ -90,9 +89,8 @@ private Optional<JsonObject> getDataFetchingExtensions(ExceptionWhileDataFetchin
             Throwable exception = error.getException();
 
             JsonObjectBuilder objectBuilder = jsonBuilderFactory.createObjectBuilder();
-            addKeyValue(objectBuilder, Config.ERROR_EXTENSION_EXCEPTION, exception.getClass().getName());
             addKeyValue(objectBuilder, Config.ERROR_EXTENSION_CLASSIFICATION, error.getErrorType().toString());
-            addKeyValue(objectBuilder, Config.ERROR_EXTENSION_CODE, toErrorCode(exception));
+            addErrorExtensions(objectBuilder, exception);
             Map<String, Object> extensions = error.getExtensions();
             populateCustomExtensions(objectBuilder, extensions);
 
@@ -101,24 +99,15 @@ private Optional<JsonObject> getDataFetchingExtensions(ExceptionWhileDataFetchin
         return Optional.empty();
     }
 
-    private String toErrorCode(Throwable exception) {
-        String exceptionClassName = exception.getClass().getName();
-        if (ErrorInfoMap.hasErrorInfo(exceptionClassName)) {
-            ErrorInfo errorInfo = ErrorInfoMap.getErrorInfo(exceptionClassName);
-            return errorInfo.getErrorCode();
-        }
-        return camelToKebab(exception.getClass().getSimpleName().replaceAll("Exception$", ""));
-    }
-
-    private static String camelToKebab(String input) {
-        return String.join("-", input.split("(?=\\p{javaUpperCase})"))
-                .toLowerCase(UK);
+    private void addErrorExtensions(JsonObjectBuilder objectBuilder, Throwable exception) {
+        ServiceLoader.load(ErrorExtensionProvider.class)
+                .forEach(provider -> addKeyValue(objectBuilder, provider.getKey(), provider.mapValueFrom(exception)));
     }
 
     private void populateCustomExtensions(JsonObjectBuilder objectBuilder, Map<String, Object> extensions) {
         if (extensions != null) {
             for (Map.Entry<String, Object> entry : extensions.entrySet()) {
-                if (!config.getErrorExtensionFields().isPresent()
+                if (config.getErrorExtensionFields().isEmpty()
                         || (config.getErrorExtensionFields().isPresent()
                                 && config.getErrorExtensionFields().get().contains(entry.getKey()))) {
                     addKeyValue(objectBuilder, entry.getKey(), entry.getValue().toString());
@@ -139,7 +128,10 @@ private JsonArray toJsonArray(List<?> list) {
     }
 
     private void addKeyValue(JsonObjectBuilder objectBuilder, String key, String value) {
+        addKeyValue(objectBuilder, key, Json.createValue(value));
+    }
 
+    private void addKeyValue(JsonObjectBuilder objectBuilder, String key, JsonValue value) {
         if (config.getErrorExtensionFields().isPresent()) {
             List<String> fieldsThatShouldBeIncluded = config.getErrorExtensionFields().get();
             if (fieldsThatShouldBeIncluded.contains(key)) {

diff --git a/...ation/src/main/resources/META-INF/services/io.smallrye.graphql.api.ErrorExtensionProvider b/...ation/src/main/resources/META-INF/services/io.smallrye.graphql.api.ErrorExtensionProvider
@@ -0,0 +1,2 @@
+io.smallrye.graphql.execution.error.ErrorCodeExtensionProvider
+io.smallrye.graphql.execution.error.ExceptionNameErrorExtensionProvider
diff --git a/server/implementation/src/test/java/io/smallrye/graphql/execution/MutinyTest.java b/server/implementation/src/test/java/io/smallrye/graphql/execution/MutinyTest.java
@@ -11,6 +11,8 @@
 import org.jboss.jandex.IndexView;
 import org.junit.jupiter.api.Test;
 
+import io.smallrye.graphql.test.mutiny.CustomException;
+
 public class MutinyTest extends ExecutionTestBase {
 
     protected IndexView getIndex() {
@@ -37,10 +39,11 @@ public void testFailureQuery() {
 
         assertNotNull(errors);
         assertEquals(errors.size(), 1);
-
-        String code = errors.get(0).asJsonObject().getJsonObject("extensions").getString("code");
-
-        assertEquals(code, "custom-error", "expected error code: custom-error");
+        var extensions = errors.get(0).asJsonObject().getJsonObject("extensions");
+        assertEquals("custom-error", extensions.getString("code"), "error code");
+        assertEquals(CustomException.class.getName(), extensions.getString("exception"), "exception");
+        assertEquals("DataFetchingException", extensions.getString("classification"), "classification");
+        assertEquals(CustomException.class.getSimpleName().length(), extensions.getInt("test-extension"), "test extension");
     }
 
     private static final String TEST_QUERY = "{\n" +

diff --git a/server/implementation/src/test/java/io/smallrye/graphql/execution/TestConfig.java b/server/implementation/src/test/java/io/smallrye/graphql/execution/TestConfig.java
@@ -10,7 +10,7 @@
 
 /**
  * Implements the config for testing
- * 
+ *
  * @author Phillip Kruger ([email protected])
  */
 public class TestConfig implements Config {
@@ -32,9 +32,9 @@ public LogPayloadOption logPayload() {
 
     @Override
     public Optional<List<String>> getErrorExtensionFields() {
-        return Optional
-                .of(Arrays.asList(new String[] { "exception", "classification", "code", "description",
-                        "validationErrorType", "queryPath" }));
+        return Optional.of(Arrays.asList(
+                "exception", "classification", "code", "description",
+                "validationErrorType", "queryPath", "test-extension"));
     }
 
     @Override
@@ -45,6 +45,7 @@ public boolean isIncludeDirectivesInSchema() {
     @Override
     public <T> T getConfigValue(String key, Class<T> type, T defaultValue) {
         if (key.equals(TestEventingService.KEY)) {
+            //noinspection unchecked
             return (T) Boolean.TRUE;
         }
         return defaultValue;

diff --git a/...mplementation/src/test/java/io/smallrye/graphql/execution/TestErrorExtensionProvider.java b/...mplementation/src/test/java/io/smallrye/graphql/execution/TestErrorExtensionProvider.java
@@ -0,0 +1,18 @@
+package io.smallrye.graphql.execution;
+
+import jakarta.json.Json;
+import jakarta.json.JsonNumber;
+
+import io.smallrye.graphql.api.ErrorExtensionProvider;
+
+public class TestErrorExtensionProvider implements ErrorExtensionProvider {
+    @Override
+    public String getKey() {
+        return "test-extension";
+    }
+
+    @Override
+    public JsonNumber mapValueFrom(Throwable exception) {
+        return Json.createValue(exception.getClass().getSimpleName().length());
+    }
+}
diff --git a/...er/implementation/src/test/java/io/smallrye/graphql/test/mutiny/MutinyBookGraphQLApi.java b/...er/implementation/src/test/java/io/smallrye/graphql/test/mutiny/MutinyBookGraphQLApi.java
@@ -21,11 +21,12 @@ public Uni<Book> getUniBook(String name) {
     }
 
     @Query("failedBook")
-    public Uni<Book> failedBook(String name) {
+    public Uni<Book> failedBook(@SuppressWarnings("unused") String name) {
         return Uni.createFrom().failure(new CustomException());
     }
 
-    private static Map<String, Book> BOOKS = new HashMap<>();
+    private static final Map<String, Book> BOOKS = new HashMap<>();
+
     static {
         Book book1 = new Book("0-571-05686-5", "Lord of the Flies", LocalDate.of(1954, Month.SEPTEMBER, 17), "William Golding");
         BOOKS.put(book1.title, book1);

diff --git a/...ation/src/test/resources/META-INF/services/io.smallrye.graphql.api.ErrorExtensionProvider b/...ation/src/test/resources/META-INF/services/io.smallrye.graphql.api.ErrorExtensionProvider
@@ -0,0 +1 @@
+io.smallrye.graphql.execution.TestErrorExtensionProvider
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		io.smallrye.graphql.execution.error.ErrorCodeExtensionProvider
		io.smallrye.graphql.execution.error.ExceptionNameErrorExtensionProvider
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		io.smallrye.graphql.execution.TestErrorExtensionProvider