Merge pull request #1510 from jmartisk/main-issue-1485

add parser options and upgrade to graphql-java 19
smallrye · Aug 10, 2022 · 6c24699 · 6c24699
2 parents d20e7f6 + b836591
commit 6c24699
Show file tree

Hide file tree

Showing 19 changed files with 396 additions and 9 deletions.
diff --git a/pom.xml b/pom.xml
@@ -29,13 +29,13 @@
         <version.smallrye-context-propagation>2.0.0-RC1</version.smallrye-context-propagation>
         <version.smallrye-stork>1.1.2</version.smallrye-stork>
         <version.opentracing>0.33.0</version.opentracing>
+
         <version.yasson>2.0.4</version.yasson>
         <version.jakarta-validation>3.0.2</version.jakarta-validation>
         <version.jakarta-annotation>2.1.1</version.jakarta-annotation>
         <version.jakarta.servlet>5.0.0</version.jakarta.servlet>
         <version.jakarta.websocket>2.0.0</version.jakarta.websocket>
-
-        <version.graphql-java>18.1</version.graphql-java>
+        <version.graphql-java>19.0</version.graphql-java>
         <verison.io.micrometer>1.9.3</verison.io.micrometer>
         <version.vertx>4.3.3</version.vertx>
 

diff --git a/server/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/config/ConfigKey.java b/server/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/config/ConfigKey.java
@@ -22,5 +22,12 @@ public interface ConfigKey extends org.eclipse.microprofile.graphql.ConfigKey {
     public static final String ERROR_EXTENSION_FIELDS = "smallrye.graphql.errorExtensionFields";
     public static final String FIELD_VISIBILITY = "smallrye.graphql.fieldVisibility";
     public static final String UNWRAP_EXCEPTIONS = "smallrye.graphql.unwrapExceptions";
+    public static final String PARSER_CAPTURE_IGNORED_CHARS = "smallrye.graphql.parser.capture.ignoredChars";
+    public static final String PARSER_CAPTURE_LINE_COMMENTS = "smallrye.graphql.parser.capture.lineComments";
+    public static final String PARSER_CAPTURE_SOURCE_LOCATION = "smallrye.graphql.parser.capture.sourceLocation";
+    public static final String PARSER_MAX_TOKENS = "smallrye.graphql.parser.maxTokens";
+    public static final String PARSER_MAX_WHITESPACE_TOKENS = "smallrye.graphql.parser.maxWhitespaceTokens";
+    public static final String INSTRUMENTATION_QUERY_COMPLEXITY = "smallrye.graphql.instrumentation.queryComplexity";
+    public static final String INSTRUMENTATION_QUERY_DEPTH = "smallrye.graphql.instrumentation.queryDepth";
 
-}
+}
diff --git a/...r/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/config/MicroProfileConfig.java b/...r/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/config/MicroProfileConfig.java
@@ -34,6 +34,13 @@ public class MicroProfileConfig implements Config {
     private String fieldVisibility;
     private Optional<List<String>> unwrapExceptions;
     private Optional<List<String>> errorExtensionFields;
+    private Optional<Integer> parserMaxTokens;
+    private Optional<Integer> parserMaxWhitespaceTokens;
+    private Optional<Boolean> parserCaptureSourceLocation;
+    private Optional<Boolean> parserCaptureLineComments;
+    private Optional<Boolean> parserCaptureIgnoredChars;
+    private Optional<Integer> queryComplexityInstrumentation;
+    private Optional<Integer> queryDepthInstrumentation;
 
     @Override
     public String getName() {
@@ -168,6 +175,55 @@ public LogPayloadOption logPayload() {
         return logPayload;
     }
 
+    @Override
+    public Optional<Boolean> isParserCaptureIgnoredChars() {
+        if (parserCaptureIgnoredChars == null) {
+            org.eclipse.microprofile.config.Config microProfileConfig = ConfigProvider.getConfig();
+            parserCaptureIgnoredChars = microProfileConfig.getOptionalValue(ConfigKey.PARSER_CAPTURE_IGNORED_CHARS,
+                    Boolean.class);
+        }
+        return parserCaptureIgnoredChars;
+    }
+
+    @Override
+    public Optional<Boolean> isParserCaptureLineComments() {
+        if (parserCaptureLineComments == null) {
+            org.eclipse.microprofile.config.Config microProfileConfig = ConfigProvider.getConfig();
+            parserCaptureLineComments = microProfileConfig.getOptionalValue(ConfigKey.PARSER_CAPTURE_LINE_COMMENTS,
+                    Boolean.class);
+        }
+        return parserCaptureLineComments;
+    }
+
+    @Override
+    public Optional<Boolean> isParserCaptureSourceLocation() {
+        if (parserCaptureSourceLocation == null) {
+            org.eclipse.microprofile.config.Config microProfileConfig = ConfigProvider.getConfig();
+            parserCaptureSourceLocation = microProfileConfig.getOptionalValue(ConfigKey.PARSER_CAPTURE_SOURCE_LOCATION,
+                    Boolean.class);
+        }
+        return parserCaptureSourceLocation;
+    }
+
+    @Override
+    public Optional<Integer> getParserMaxTokens() {
+        if (parserMaxTokens == null) {
+            org.eclipse.microprofile.config.Config microProfileConfig = ConfigProvider.getConfig();
+            parserMaxTokens = microProfileConfig.getOptionalValue(ConfigKey.PARSER_MAX_TOKENS, Integer.class);
+        }
+        return parserMaxTokens;
+    }
+
+    @Override
+    public Optional<Integer> getParserMaxWhitespaceTokens() {
+        if (parserMaxWhitespaceTokens == null) {
+            org.eclipse.microprofile.config.Config microProfileConfig = ConfigProvider.getConfig();
+            parserMaxWhitespaceTokens = microProfileConfig.getOptionalValue(ConfigKey.PARSER_MAX_WHITESPACE_TOKENS,
+                    Integer.class);
+        }
+        return parserMaxWhitespaceTokens;
+    }
+
     @Override
     public String getFieldVisibility() {
         if (fieldVisibility == null) {
@@ -194,6 +250,26 @@ public Optional<List<String>> getErrorExtensionFields() {
         return errorExtensionFields;
     }
 
+    @Override
+    public Optional<Integer> getQueryComplexityInstrumentation() {
+        if (queryComplexityInstrumentation == null) {
+            org.eclipse.microprofile.config.Config microProfileConfig = ConfigProvider.getConfig();
+            queryComplexityInstrumentation = microProfileConfig.getOptionalValue(ConfigKey.INSTRUMENTATION_QUERY_COMPLEXITY,
+                    Integer.class);
+        }
+        return queryComplexityInstrumentation;
+    }
+
+    @Override
+    public Optional<Integer> getQueryDepthInstrumentation() {
+        if (queryDepthInstrumentation == null) {
+            org.eclipse.microprofile.config.Config microProfileConfig = ConfigProvider.getConfig();
+            queryDepthInstrumentation = microProfileConfig.getOptionalValue(ConfigKey.INSTRUMENTATION_QUERY_DEPTH,
+                    Integer.class);
+        }
+        return queryDepthInstrumentation;
+    }
+
     @Override
     public <T> T getConfigValue(String key, Class<T> type, T defaultValue) {
         org.eclipse.microprofile.config.Config microProfileConfig = ConfigProvider.getConfig();
@@ -252,6 +328,26 @@ public void setLogPayload(LogPayloadOption logPayload) {
         this.logPayload = logPayload;
     }
 
+    public void setParserCaptureIgnoredChars(Optional<Boolean> parserCaptureIgnoredChars) {
+        this.parserCaptureIgnoredChars = parserCaptureIgnoredChars;
+    }
+
+    public void setParserCaptureLineComments(Optional<Boolean> parserCaptureLineComments) {
+        this.parserCaptureLineComments = parserCaptureLineComments;
+    }
+
+    public void setParserCaptureSourceLocation(Optional<Boolean> parserCaptureSourceLocation) {
+        this.parserCaptureSourceLocation = parserCaptureSourceLocation;
+    }
+
+    public void setParserMaxTokens(Optional<Integer> parserMaxTokens) {
+        this.parserMaxTokens = parserMaxTokens;
+    }
+
+    public void setParserMaxWhitespaceTokens(Optional<Integer> parserMaxWhitespaceTokens) {
+        this.parserMaxWhitespaceTokens = parserMaxWhitespaceTokens;
+    }
+
     public void setFieldVisibility(String fieldVisibility) {
         this.fieldVisibility = fieldVisibility;
     }
@@ -316,6 +412,14 @@ public void setIncludeIntrospectionTypesInSchema(Boolean includeIntrospectionTyp
         this.includeIntrospectionTypesInSchema = includeIntrospectionTypesInSchema;
     }
 
+    public void setQueryComplexityInstrumentation(Optional<Integer> queryComplexityInstrumentation) {
+        this.queryComplexityInstrumentation = queryComplexityInstrumentation;
+    }
+
+    public void getQueryDepthInstrumentation(Optional<Integer> queryDepthInstrumentation) {
+        this.queryDepthInstrumentation = queryDepthInstrumentation;
+    }
+
     private Optional<List<String>> mergeList(Optional<List<String>> currentList, Optional<List<String>> deprecatedList) {
 
         List<String> combined = new ArrayList<>();
@@ -333,10 +437,6 @@ private Optional<List<String>> mergeList(Optional<List<String>> currentList, Opt
         }
     }
 
-    private String getStringConfigValue(String key) {
-        return getStringConfigValue(key, null);
-    }
-
     private String getStringConfigValue(String key, String defaultValue) {
         return getConfigValue(key, String.class, defaultValue);
     }

diff --git a/server/implementation/src/main/java/io/smallrye/graphql/execution/ExecutionService.java b/server/implementation/src/main/java/io/smallrye/graphql/execution/ExecutionService.java
@@ -2,6 +2,7 @@
 
 import static io.smallrye.graphql.SmallRyeGraphQLServerLogging.log;
 
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -20,9 +21,14 @@
 import graphql.ExecutionInput.Builder;
 import graphql.ExecutionResult;
 import graphql.GraphQL;
+import graphql.analysis.MaxQueryComplexityInstrumentation;
+import graphql.analysis.MaxQueryDepthInstrumentation;
 import graphql.execution.ExecutionId;
 import graphql.execution.ExecutionStrategy;
 import graphql.execution.SubscriptionExecutionStrategy;
+import graphql.execution.instrumentation.ChainedInstrumentation;
+import graphql.execution.instrumentation.Instrumentation;
+import graphql.parser.ParserOptions;
 import graphql.schema.GraphQLSchema;
 import io.smallrye.graphql.bootstrap.DataFetcherFactory;
 import io.smallrye.graphql.execution.context.SmallRyeContext;
@@ -245,9 +251,24 @@ private <K, T> DataLoaderRegistry getDataLoaderRegistry(List<Operation> operatio
     private GraphQL getGraphQL() {
         if (this.graphQL == null) {
             if (graphQLSchema != null) {
+                Config config = Config.get();
+                setParserOptions(config);
+
                 GraphQL.Builder graphqlBuilder = GraphQL.newGraphQL(graphQLSchema);
                 graphqlBuilder = graphqlBuilder.defaultDataFetcherExceptionHandler(new ExceptionHandler());
-                graphqlBuilder = graphqlBuilder.instrumentation(queryCache);
+
+                List<Instrumentation> chainedList = new ArrayList<>();
+
+                if (config.getQueryComplexityInstrumentation().isPresent()) {
+                    chainedList.add(new MaxQueryComplexityInstrumentation(config.getQueryComplexityInstrumentation().get()));
+                }
+                if (config.getQueryDepthInstrumentation().isPresent()) {
+                    chainedList.add(new MaxQueryDepthInstrumentation(config.getQueryDepthInstrumentation().get()));
+                }
+                chainedList.add(queryCache);
+                // TODO: Allow users to add custome instumentations 
+                graphqlBuilder = graphqlBuilder.instrumentation(new ChainedInstrumentation(chainedList));
+
                 graphqlBuilder = graphqlBuilder.preparsedDocumentProvider(queryCache);
 
                 if (queryExecutionStrategy != null) {
@@ -273,4 +294,30 @@ private GraphQL getGraphQL() {
         return this.graphQL;
 
     }
+
+    private void setParserOptions(Config config) {
+        if (config.hasParserOptions()) {
+            ParserOptions.Builder parserOptionsBuilder = ParserOptions.newParserOptions();
+            if (config.isParserCaptureIgnoredChars().isPresent()) {
+                parserOptionsBuilder = parserOptionsBuilder
+                        .captureIgnoredChars(config.isParserCaptureIgnoredChars().get());
+            }
+            if (config.isParserCaptureLineComments().isPresent()) {
+                parserOptionsBuilder = parserOptionsBuilder
+                        .captureLineComments(config.isParserCaptureLineComments().get());
+            }
+            if (config.isParserCaptureSourceLocation().isPresent()) {
+                parserOptionsBuilder = parserOptionsBuilder
+                        .captureSourceLocation(config.isParserCaptureSourceLocation().get());
+            }
+            if (config.getParserMaxTokens().isPresent()) {
+                parserOptionsBuilder = parserOptionsBuilder.maxTokens(config.getParserMaxTokens().get());
+            }
+            if (config.getParserMaxWhitespaceTokens().isPresent()) {
+                parserOptionsBuilder = parserOptionsBuilder
+                        .maxWhitespaceTokens(config.getParserMaxWhitespaceTokens().get());
+            }
+            ParserOptions.setDefaultParserOptions(parserOptionsBuilder.build());
+        }
+    }
 }
diff --git a/server/implementation/src/main/java/io/smallrye/graphql/spi/config/Config.java b/server/implementation/src/main/java/io/smallrye/graphql/spi/config/Config.java
@@ -163,6 +163,42 @@ default LogPayloadOption logPayload() {
         return LogPayloadOption.off;
     }
 
+    default Optional<Boolean> isParserCaptureIgnoredChars() {
+        return Optional.empty();
+    }
+
+    default Optional<Boolean> isParserCaptureLineComments() {
+        return Optional.empty();
+    }
+
+    default Optional<Boolean> isParserCaptureSourceLocation() {
+        return Optional.empty();
+    }
+
+    default Optional<Integer> getParserMaxTokens() {
+        return Optional.empty();
+    }
+
+    default Optional<Integer> getParserMaxWhitespaceTokens() {
+        return Optional.empty();
+    }
+
+    default boolean hasParserOptions() {
+        return isParserCaptureIgnoredChars().isPresent()
+                || isParserCaptureLineComments().isPresent()
+                || isParserCaptureSourceLocation().isPresent()
+                || getParserMaxTokens().isPresent()
+                || getParserMaxWhitespaceTokens().isPresent();
+    }
+
+    default Optional<Integer> getQueryComplexityInstrumentation() {
+        return Optional.empty();
+    }
+
+    default Optional<Integer> getQueryDepthInstrumentation() {
+        return Optional.empty();
+    }
+
     default String getFieldVisibility() {
         return FIELD_VISIBILITY_DEFAULT;
     }

diff --git a/server/tck/src/test/resources/META-INF/microprofile-config.properties b/server/tck/src/test/resources/META-INF/microprofile-config.properties
@@ -5,4 +5,5 @@ smallrye.graphql.logPayload=true
 
 mp.graphql.showErrorMessage=java.security.AccessControlException,io.smallrye.graphql.test.apps.exceptionlist.*
 mp.graphql.hideErrorMessage=java.io.IOException,io.smallrye.graphql.test.apps.exceptionlist.*
-smallrye.graphql.errorExtensionFields=exception,classification,code,description,validationErrorType,queryPath
+smallrye.graphql.errorExtensionFields=exception,classification,code,description,validationErrorType,queryPath
+smallrye.graphql.parser.capture.sourceLocation=false
diff --git a/server/tck/src/test/resources/overrides/createNewNullNamedHero/output2.json b/server/tck/src/test/resources/overrides/createNewNullNamedHero/output2.json
@@ -0,0 +1,14 @@
+{
+    "errors": [
+        {
+            "message": "Validation error (WrongType@[createNewHero]) : argument 'hero.name' with value 'NullValue{}' must not be null",
+            "locations": [
+                {
+                    "line": 2,
+                    "column": 19
+                }
+            ]
+        }
+    ],
+    "data": null
+}
diff --git a/server/tck/src/test/resources/overrides/createNewUnnamedHero/output2.json b/server/tck/src/test/resources/overrides/createNewUnnamedHero/output2.json
@@ -0,0 +1,14 @@
+{
+    "errors": [
+        {
+            "message": "Validation error (WrongType@[createNewHero]) : argument 'hero' with value 'ObjectValue{objectFields=[ObjectField{name='realName', value=StringValue{value='John Smith'}}]}' is missing required fields '[name]'",
+            "locations": [
+                {
+                    "line": 2,
+                    "column": 19
+                }
+            ]
+        }
+    ],
+    "data": null
+}
diff --git a/server/tck/src/test/resources/overrides/invalidDataTypeValue/output3.json b/server/tck/src/test/resources/overrides/invalidDataTypeValue/output3.json
@@ -0,0 +1,16 @@
+{
+    "errors": [
+        {
+            "message": "argument 'powerLevel' with value 'StringValue{value='Unlimited'}' is not a valid 'Int'",
+            "locations": [
+                {
+                    "line": 2,
+                    "column": 37
+                }
+            ]
+        }
+    ],
+    "data": {
+        "updateItemPowerLevel": null
+    }
+}
diff --git a/server/tck/src/test/resources/overrides/invalidEnumValue/output2.json b/server/tck/src/test/resources/overrides/invalidEnumValue/output2.json
@@ -0,0 +1,14 @@
+{
+    "errors": [
+        {
+            "message": "Validation error (WrongType@[createNewHero]) : argument 'hero.tshirtSize' with value 'EnumValue{name='XLTall'}' is not a valid 'ShirtSize' - Expected enum literal value not in allowable values -  'EnumValue{name='XLTall'}'.",
+            "locations": [
+                {
+                    "line": 3,
+                    "column": 19
+                }
+            ]
+        }
+    ],
+    "data": null
+}
diff --git a/server/tck/src/test/resources/overrides/invalidLocalDateFormattedValue/output3.json b/server/tck/src/test/resources/overrides/invalidLocalDateFormattedValue/output3.json
@@ -0,0 +1,16 @@
+{
+    "errors": [
+        {
+            "message": "argument 'date' with value 'StringValue{value='Today'}' is not a valid 'Date'",
+            "locations": [
+                {
+                    "line": 2,
+                    "column": 49
+                }
+            ]
+        }
+    ],
+    "data": {
+        "checkInWithCorrectDateFormat": null
+    }
+}