From 1f7f58fe3f7645e17742c6fa43edcb2815def527 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 17 Apr 2022 19:01:21 +1000 Subject: [PATCH 01/13] Add raw and coerced variables --- .../graphql/execution/CoercedVariables.java | 30 +++++++++++++++++++ .../java/graphql/execution/RawVariables.java | 30 +++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 src/main/java/graphql/execution/CoercedVariables.java create mode 100644 src/main/java/graphql/execution/RawVariables.java diff --git a/src/main/java/graphql/execution/CoercedVariables.java b/src/main/java/graphql/execution/CoercedVariables.java new file mode 100644 index 0000000000..159f3c7328 --- /dev/null +++ b/src/main/java/graphql/execution/CoercedVariables.java @@ -0,0 +1,30 @@ +package graphql.execution; + +import graphql.Internal; +import graphql.collect.ImmutableMapWithNullValues; + +import java.util.Map; + +/** + * Holds coerced variables + */ +@Internal +public class CoercedVariables { + private final ImmutableMapWithNullValues coercedVariables; + + public CoercedVariables(Map coercedVariables) { + this.coercedVariables = ImmutableMapWithNullValues.copyOf(coercedVariables); + } + + public Map getCoercedVariables() { + return coercedVariables; + } + + public boolean containsKey(String key) { + return coercedVariables.containsKey(key); + } + + public Object get(String key) { + return coercedVariables.get(key); + } +} diff --git a/src/main/java/graphql/execution/RawVariables.java b/src/main/java/graphql/execution/RawVariables.java new file mode 100644 index 0000000000..fe590091d2 --- /dev/null +++ b/src/main/java/graphql/execution/RawVariables.java @@ -0,0 +1,30 @@ +package graphql.execution; + +import graphql.Internal; +import graphql.collect.ImmutableMapWithNullValues; + +import java.util.Map; + +/** + * Holds raw variables, which not have been coerced yet + */ +@Internal +public class RawVariables { + private final ImmutableMapWithNullValues rawVariables; + + public RawVariables(Map rawVariables) { + this.rawVariables = ImmutableMapWithNullValues.copyOf(rawVariables); + } + + public Map getRawVariables() { + return rawVariables; + } + + public boolean containsKey(String key) { + return rawVariables.containsKey(key); + } + + public Object get(String key) { + return rawVariables.get(key); + } +} From 5ff6353fb94232b99974e9b6cedb5df7f6c1d6a4 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 17 Apr 2022 19:48:57 +1000 Subject: [PATCH 02/13] Add RawVariables to ExecutionInput --- src/main/java/graphql/ExecutionInput.java | 44 ++++++++++++++----- .../graphql/execution/CoercedVariables.java | 2 +- .../java/graphql/execution/RawVariables.java | 2 +- src/test/groovy/graphql/GraphQLTest.groovy | 2 - 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/src/main/java/graphql/ExecutionInput.java b/src/main/java/graphql/ExecutionInput.java index 777b2d4ab7..0d240929d9 100644 --- a/src/main/java/graphql/ExecutionInput.java +++ b/src/main/java/graphql/ExecutionInput.java @@ -2,6 +2,7 @@ import graphql.cachecontrol.CacheControl; import graphql.execution.ExecutionId; +import graphql.execution.RawVariables; import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentationState; import org.dataloader.DataLoaderRegistry; @@ -24,7 +25,7 @@ public class ExecutionInput { private final GraphQLContext graphQLContext; private final Object localContext; private final Object root; - private final Map variables; + private final RawVariables rawVariables; private final Map extensions; private final DataLoaderRegistry dataLoaderRegistry; private final CacheControl cacheControl; @@ -39,7 +40,7 @@ private ExecutionInput(Builder builder) { this.context = builder.context; this.graphQLContext = assertNotNull(builder.graphQLContext); this.root = builder.root; - this.variables = builder.variables; + this.rawVariables = builder.rawVariables; this.dataLoaderRegistry = builder.dataLoaderRegistry; this.cacheControl = builder.cacheControl; this.executionId = builder.executionId; @@ -97,10 +98,18 @@ public Object getRoot() { } /** - * @return a map of variables that can be referenced via $syntax in the query + * @return a map of raw variables that can be referenced via $syntax in the query. Retaining for backwards compatibility */ + @Deprecated public Map getVariables() { - return variables; + return rawVariables.getMap(); + } + + /** + * @return a map of raw variables that can be referenced via $syntax in the query. + */ + public RawVariables getRawVariables() { + return rawVariables; } /** @@ -158,7 +167,7 @@ public ExecutionInput transform(Consumer builderConsumer) { .root(this.root) .dataLoaderRegistry(this.dataLoaderRegistry) .cacheControl(this.cacheControl) - .variables(this.variables) + .rawVariables(this.rawVariables) .extensions(this.extensions) .executionId(this.executionId) .locale(this.locale); @@ -176,7 +185,7 @@ public String toString() { ", context=" + context + ", graphQLContext=" + graphQLContext + ", root=" + root + - ", variables=" + variables + + ", rawVariables=" + rawVariables + ", dataLoaderRegistry=" + dataLoaderRegistry + ", executionId= " + executionId + ", locale= " + locale + @@ -209,7 +218,7 @@ public static class Builder { private Object context = graphQLContext; // we make these the same object on purpose - legacy code will get the same object if this change nothing private Object localContext; private Object root; - private Map variables = Collections.emptyMap(); + private RawVariables rawVariables = new RawVariables(Collections.emptyMap()); public Map extensions = Collections.emptyMap(); // // this is important - it allows code to later known if we never really set a dataloader and hence it can optimize @@ -242,7 +251,6 @@ public Builder executionId(ExecutionId executionId) { return this; } - /** * Sets the locale to use for this operation * @@ -351,8 +359,24 @@ public Builder root(Object root) { return this; } - public Builder variables(Map variables) { - this.variables = assertNotNull(variables, () -> "variables map can't be null"); + /** + * The legacy variables builder + * + * @param rawVariables the map of raw variables + * + * @return this builder + * + * @deprecated - use {@link RawVariables} to hold raw variables + */ + @Deprecated + public Builder variables(Map rawVariables) { + assertNotNull(rawVariables, () -> "variables map can't be null"); + this.rawVariables = new RawVariables(rawVariables); + return this; + } + + public Builder rawVariables(RawVariables rawVariables) { + this.rawVariables = assertNotNull(rawVariables, () -> "raw variables map can't be null"); return this; } diff --git a/src/main/java/graphql/execution/CoercedVariables.java b/src/main/java/graphql/execution/CoercedVariables.java index 159f3c7328..e484c435a2 100644 --- a/src/main/java/graphql/execution/CoercedVariables.java +++ b/src/main/java/graphql/execution/CoercedVariables.java @@ -16,7 +16,7 @@ public CoercedVariables(Map coercedVariables) { this.coercedVariables = ImmutableMapWithNullValues.copyOf(coercedVariables); } - public Map getCoercedVariables() { + public Map getMap() { return coercedVariables; } diff --git a/src/main/java/graphql/execution/RawVariables.java b/src/main/java/graphql/execution/RawVariables.java index fe590091d2..b8a987a6c5 100644 --- a/src/main/java/graphql/execution/RawVariables.java +++ b/src/main/java/graphql/execution/RawVariables.java @@ -16,7 +16,7 @@ public RawVariables(Map rawVariables) { this.rawVariables = ImmutableMapWithNullValues.copyOf(rawVariables); } - public Map getRawVariables() { + public Map getMap() { return rawVariables; } diff --git a/src/test/groovy/graphql/GraphQLTest.groovy b/src/test/groovy/graphql/GraphQLTest.groovy index 33c5f78868..482bfb2cd1 100644 --- a/src/test/groovy/graphql/GraphQLTest.groovy +++ b/src/test/groovy/graphql/GraphQLTest.groovy @@ -973,8 +973,6 @@ many lines'''] then: def assEx = thrown(AssertException) assEx.message.contains("variables map can't be null") - - } def "query can't be null via ExecutionInput"() { From 2614e4beda14d6e74f931ac16cf7e2a76c45bab8 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 17 Apr 2022 20:02:12 +1000 Subject: [PATCH 03/13] Add more ExecutionInput tests --- .../groovy/graphql/ExecutionInputTest.groovy | 49 +++++++++++++++++++ src/test/groovy/graphql/GraphQLTest.groovy | 11 +++++ 2 files changed, 60 insertions(+) diff --git a/src/test/groovy/graphql/ExecutionInputTest.groovy b/src/test/groovy/graphql/ExecutionInputTest.groovy index 423fb8994e..94739e31e2 100644 --- a/src/test/groovy/graphql/ExecutionInputTest.groovy +++ b/src/test/groovy/graphql/ExecutionInputTest.groovy @@ -2,6 +2,7 @@ package graphql import graphql.cachecontrol.CacheControl import graphql.execution.ExecutionId +import graphql.execution.RawVariables import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment import org.dataloader.DataLoaderRegistry @@ -17,6 +18,7 @@ class ExecutionInputTest extends Specification { def root = "root" def context = "context" def variables = [key: "value"] + def rawVariables = new RawVariables(variables) def "build works"() { when: @@ -42,6 +44,28 @@ class ExecutionInputTest extends Specification { executionInput.extensions == [some: "map"] } + def "build works with raw variables"() { + when: + def executionInput = ExecutionInput.newExecutionInput().query(query) + .dataLoaderRegistry(registry) + .cacheControl(cacheControl) + .rawVariables(rawVariables) + .root(root) + .graphQLContext({ it.of(["a": "b"]) }) + .locale(Locale.GERMAN) + .extensions([some: "map"]) + .build() + then: + executionInput.graphQLContext.get("a") == "b" + executionInput.root == root + executionInput.rawVariables == rawVariables + executionInput.dataLoaderRegistry == registry + executionInput.cacheControl == cacheControl + executionInput.query == query + executionInput.locale == Locale.GERMAN + executionInput.extensions == [some: "map"] + } + def "map context build works"() { when: def executionInput = ExecutionInput.newExecutionInput().query(query) @@ -111,6 +135,31 @@ class ExecutionInputTest extends Specification { executionInput.query == "new query" } + def "transform works and copies values with raw variables"() { + when: + def executionInputOld = ExecutionInput.newExecutionInput().query(query) + .dataLoaderRegistry(registry) + .cacheControl(cacheControl) + .rawVariables(rawVariables) + .extensions([some: "map"]) + .root(root) + .graphQLContext({ it.of(["a": "b"]) }) + .locale(Locale.GERMAN) + .build() + def graphQLContext = executionInputOld.getGraphQLContext() + def executionInput = executionInputOld.transform({ bldg -> bldg.query("new query") }) + + then: + executionInput.graphQLContext == graphQLContext + executionInput.root == root + executionInput.rawVariables == rawVariables + executionInput.dataLoaderRegistry == registry + executionInput.cacheControl == cacheControl + executionInput.locale == Locale.GERMAN + executionInput.extensions == [some: "map"] + executionInput.query == "new query" + } + def "defaults query into builder as expected"() { when: def executionInput = ExecutionInput.newExecutionInput("{ q }").build() diff --git a/src/test/groovy/graphql/GraphQLTest.groovy b/src/test/groovy/graphql/GraphQLTest.groovy index 482bfb2cd1..dc831bfc75 100644 --- a/src/test/groovy/graphql/GraphQLTest.groovy +++ b/src/test/groovy/graphql/GraphQLTest.groovy @@ -975,6 +975,17 @@ many lines'''] assEx.message.contains("variables map can't be null") } + def "raw variables map can't be null via ExecutionInput"() { + given: + + when: + def input = newExecutionInput().query('query($var:String){ hello(arg: $var) }').rawVariables(null).build() + + then: + def assEx = thrown(AssertException) + assEx.message.contains("raw variables map can't be null") + } + def "query can't be null via ExecutionInput"() { given: From 0c51d2ef202a7b4766b8646956ea6c14f95046d6 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 17 Apr 2022 21:06:14 +1000 Subject: [PATCH 04/13] Add coerced variables to ExecutionContext --- .../graphql/execution/ExecutionContext.java | 17 +++-- .../execution/ExecutionContextBuilder.java | 17 +++-- .../ExecutionContextBuilderTest.groovy | 67 +++++++++++++++++++ 3 files changed, 92 insertions(+), 9 deletions(-) diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index a5019fe1d6..9a6bb1f3a7 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -9,7 +9,6 @@ import graphql.PublicApi; import graphql.cachecontrol.CacheControl; import graphql.collect.ImmutableKit; -import graphql.collect.ImmutableMapWithNullValues; import graphql.execution.instrumentation.Instrumentation; import graphql.execution.instrumentation.InstrumentationState; import graphql.language.Document; @@ -43,7 +42,7 @@ public class ExecutionContext { private final ImmutableMap fragmentsByName; private final OperationDefinition operationDefinition; private final Document document; - private final ImmutableMapWithNullValues variables; + private final CoercedVariables coercedVariables; private final Object root; private final Object context; private final GraphQLContext graphQLContext; @@ -66,7 +65,7 @@ public class ExecutionContext { this.mutationStrategy = builder.mutationStrategy; this.subscriptionStrategy = builder.subscriptionStrategy; this.fragmentsByName = builder.fragmentsByName; - this.variables = ImmutableMapWithNullValues.copyOf(builder.variables); + this.coercedVariables = builder.coercedVariables; this.document = builder.document; this.operationDefinition = builder.operationDefinition; this.context = builder.context; @@ -80,7 +79,7 @@ public class ExecutionContext { this.errors.set(builder.errors); this.localContext = builder.localContext; this.executionInput = builder.executionInput; - queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, variables)); + queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables.getMap())); } @@ -116,8 +115,16 @@ public OperationDefinition getOperationDefinition() { return operationDefinition; } + /** + * @deprecated use {@link #getCoercedVariables()} instead + */ + @Deprecated public Map getVariables() { - return variables; + return coercedVariables.getMap(); + } + + public CoercedVariables getCoercedVariables() { + return coercedVariables; } /** diff --git a/src/main/java/graphql/execution/ExecutionContextBuilder.java b/src/main/java/graphql/execution/ExecutionContextBuilder.java index 1affeabf0c..1ac67a032c 100644 --- a/src/main/java/graphql/execution/ExecutionContextBuilder.java +++ b/src/main/java/graphql/execution/ExecutionContextBuilder.java @@ -9,7 +9,6 @@ import graphql.PublicApi; import graphql.cachecontrol.CacheControl; import graphql.collect.ImmutableKit; -import graphql.collect.ImmutableMapWithNullValues; import graphql.execution.instrumentation.Instrumentation; import graphql.execution.instrumentation.InstrumentationState; import graphql.language.Document; @@ -18,6 +17,7 @@ import graphql.schema.GraphQLSchema; import org.dataloader.DataLoaderRegistry; +import java.util.Collections; import java.util.Locale; import java.util.Map; @@ -39,7 +39,7 @@ public class ExecutionContextBuilder { Object root; Document document; OperationDefinition operationDefinition; - ImmutableMapWithNullValues variables = ImmutableMapWithNullValues.emptyMap(); + CoercedVariables coercedVariables = new CoercedVariables(Collections.emptyMap()); ImmutableMap fragmentsByName = ImmutableKit.emptyMap(); DataLoaderRegistry dataLoaderRegistry; CacheControl cacheControl; @@ -86,7 +86,7 @@ public ExecutionContextBuilder() { root = other.getRoot(); document = other.getDocument(); operationDefinition = other.getOperationDefinition(); - variables = ImmutableMapWithNullValues.copyOf(other.getVariables()); + coercedVariables = other.getCoercedVariables(); fragmentsByName = ImmutableMap.copyOf(other.getFragmentsByName()); dataLoaderRegistry = other.getDataLoaderRegistry(); cacheControl = other.getCacheControl(); @@ -151,8 +151,17 @@ public ExecutionContextBuilder root(Object root) { return this; } + /** + * @deprecated use {@link #coercedVariables(CoercedVariables)} instead + */ + @Deprecated public ExecutionContextBuilder variables(Map variables) { - this.variables = ImmutableMapWithNullValues.copyOf(variables); + this.coercedVariables = new CoercedVariables(variables); + return this; + } + + public ExecutionContextBuilder coercedVariables(CoercedVariables coercedVariables) { + this.coercedVariables = coercedVariables; return this; } diff --git a/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy b/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy index 762a85a343..cff0fef5bd 100644 --- a/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy +++ b/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy @@ -82,4 +82,71 @@ class ExecutionContextBuilderTest extends Specification { executionContext.dataLoaderRegistry == dataLoaderRegistry executionContext.cacheControl == cacheControl } + + def "builds the correct ExecutionContext with coerced variables"() { + given: + ExecutionContextBuilder executionContextBuilder = new ExecutionContextBuilder() + + Instrumentation instrumentation = Mock(Instrumentation) + executionContextBuilder.instrumentation(instrumentation) + + ExecutionStrategy queryStrategy = Mock(ExecutionStrategy) + executionContextBuilder.queryStrategy(queryStrategy) + + ExecutionStrategy mutationStrategy = Mock(ExecutionStrategy) + executionContextBuilder.mutationStrategy(mutationStrategy) + + ExecutionStrategy subscriptionStrategy = Mock(ExecutionStrategy) + executionContextBuilder.subscriptionStrategy(subscriptionStrategy) + + GraphQLSchema schema = Mock(GraphQLSchema) + executionContextBuilder.graphQLSchema(schema) + + def executionId = ExecutionId.generate() + executionContextBuilder.executionId(executionId) + + def context = "context" + executionContextBuilder.context(context) + + def graphQLContext = GraphQLContext.newContext().build() + executionContextBuilder.graphQLContext(graphQLContext) + + def root = "root" + executionContextBuilder.root(root) + + Document document = new Parser().parseDocument("query myQuery(\$var: String){...MyFragment} fragment MyFragment on Query{foo}") + def operation = document.definitions[0] as OperationDefinition + def fragment = document.definitions[1] as FragmentDefinition + executionContextBuilder.operationDefinition(operation) + + executionContextBuilder.fragmentsByName([MyFragment: fragment]) + + def coercedVariables = new CoercedVariables([var: 'value']) + executionContextBuilder.coercedVariables(coercedVariables) + + def dataLoaderRegistry = new DataLoaderRegistry() + executionContextBuilder.dataLoaderRegistry(dataLoaderRegistry) + + def cacheControl = CacheControl.newCacheControl() + executionContextBuilder.cacheControl(cacheControl) + + when: + def executionContext = executionContextBuilder.build() + + then: + executionContext.executionId == executionId + executionContext.instrumentation == instrumentation + executionContext.graphQLSchema == schema + executionContext.queryStrategy == queryStrategy + executionContext.mutationStrategy == mutationStrategy + executionContext.subscriptionStrategy == subscriptionStrategy + executionContext.root == root + executionContext.context == context + executionContext.graphQLContext == graphQLContext + executionContext.coercedVariables == coercedVariables + executionContext.getFragmentsByName() == [MyFragment: fragment] + executionContext.operationDefinition == operation + executionContext.dataLoaderRegistry == dataLoaderRegistry + executionContext.cacheControl == cacheControl + } } From c98a615679075e2abc47eb3e422e63b3eb6bd4b7 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Mon, 18 Apr 2022 10:14:48 +1000 Subject: [PATCH 05/13] Add ExecutionContext transformer test and tidy up --- .../ExecutionContextBuilderTest.groovy | 177 +++++++++--------- 1 file changed, 86 insertions(+), 91 deletions(-) diff --git a/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy b/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy index cff0fef5bd..cee709437b 100644 --- a/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy +++ b/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy @@ -13,58 +13,39 @@ import spock.lang.Specification class ExecutionContextBuilderTest extends Specification { + Instrumentation instrumentation = Mock(Instrumentation) + ExecutionStrategy queryStrategy = Mock(ExecutionStrategy) + ExecutionStrategy mutationStrategy = Mock(ExecutionStrategy) + ExecutionStrategy subscriptionStrategy = Mock(ExecutionStrategy) + GraphQLSchema schema = Mock(GraphQLSchema) + def executionId = ExecutionId.generate() + def context = "context" + def graphQLContext = GraphQLContext.newContext().build() + def root = "root" + Document document = new Parser().parseDocument("query myQuery(\$var: String){...MyFragment} fragment MyFragment on Query{foo}") + def operation = document.definitions[0] as OperationDefinition + def fragment = document.definitions[1] as FragmentDefinition + def dataLoaderRegistry = new DataLoaderRegistry() + def cacheControl = CacheControl.newCacheControl() def "builds the correct ExecutionContext"() { - given: - ExecutionContextBuilder executionContextBuilder = new ExecutionContextBuilder() - - Instrumentation instrumentation = Mock(Instrumentation) - executionContextBuilder.instrumentation(instrumentation) - - ExecutionStrategy queryStrategy = Mock(ExecutionStrategy) - executionContextBuilder.queryStrategy(queryStrategy) - - ExecutionStrategy mutationStrategy = Mock(ExecutionStrategy) - executionContextBuilder.mutationStrategy(mutationStrategy) - - ExecutionStrategy subscriptionStrategy = Mock(ExecutionStrategy) - executionContextBuilder.subscriptionStrategy(subscriptionStrategy) - - GraphQLSchema schema = Mock(GraphQLSchema) - executionContextBuilder.graphQLSchema(schema) - - def executionId = ExecutionId.generate() - executionContextBuilder.executionId(executionId) - - def context = "context" - executionContextBuilder.context(context) - - def graphQLContext = GraphQLContext.newContext().build() - executionContextBuilder.graphQLContext(graphQLContext) - - def root = "root" - executionContextBuilder.root(root) - - Document document = new Parser().parseDocument("query myQuery(\$var: String){...MyFragment} fragment MyFragment on Query{foo}") - def operation = document.definitions[0] as OperationDefinition - def fragment = document.definitions[1] as FragmentDefinition - executionContextBuilder.operationDefinition(operation) - - executionContextBuilder.fragmentsByName([MyFragment: fragment]) - - def variables = Collections.emptyMap() - executionContextBuilder.variables(variables) - - executionContextBuilder.variables([var: 'value']) - - def dataLoaderRegistry = new DataLoaderRegistry() - executionContextBuilder.dataLoaderRegistry(dataLoaderRegistry) - - def cacheControl = CacheControl.newCacheControl() - executionContextBuilder.cacheControl(cacheControl) - when: - def executionContext = executionContextBuilder.build() + def executionContext = new ExecutionContextBuilder() + .instrumentation(instrumentation) + .queryStrategy(queryStrategy) + .mutationStrategy(mutationStrategy) + .subscriptionStrategy(subscriptionStrategy) + .graphQLSchema(schema) + .executionId(executionId) + .context(context) + .graphQLContext(graphQLContext) + .root(root) + .operationDefinition(operation) + .fragmentsByName([MyFragment: fragment]) + .variables([var: 'value']) + .dataLoaderRegistry(dataLoaderRegistry) + .cacheControl(cacheControl) + .build() then: executionContext.executionId == executionId @@ -85,53 +66,67 @@ class ExecutionContextBuilderTest extends Specification { def "builds the correct ExecutionContext with coerced variables"() { given: - ExecutionContextBuilder executionContextBuilder = new ExecutionContextBuilder() - - Instrumentation instrumentation = Mock(Instrumentation) - executionContextBuilder.instrumentation(instrumentation) - - ExecutionStrategy queryStrategy = Mock(ExecutionStrategy) - executionContextBuilder.queryStrategy(queryStrategy) - - ExecutionStrategy mutationStrategy = Mock(ExecutionStrategy) - executionContextBuilder.mutationStrategy(mutationStrategy) - - ExecutionStrategy subscriptionStrategy = Mock(ExecutionStrategy) - executionContextBuilder.subscriptionStrategy(subscriptionStrategy) - - GraphQLSchema schema = Mock(GraphQLSchema) - executionContextBuilder.graphQLSchema(schema) - - def executionId = ExecutionId.generate() - executionContextBuilder.executionId(executionId) - - def context = "context" - executionContextBuilder.context(context) - - def graphQLContext = GraphQLContext.newContext().build() - executionContextBuilder.graphQLContext(graphQLContext) - - def root = "root" - executionContextBuilder.root(root) - - Document document = new Parser().parseDocument("query myQuery(\$var: String){...MyFragment} fragment MyFragment on Query{foo}") - def operation = document.definitions[0] as OperationDefinition - def fragment = document.definitions[1] as FragmentDefinition - executionContextBuilder.operationDefinition(operation) - - executionContextBuilder.fragmentsByName([MyFragment: fragment]) - def coercedVariables = new CoercedVariables([var: 'value']) - executionContextBuilder.coercedVariables(coercedVariables) - def dataLoaderRegistry = new DataLoaderRegistry() - executionContextBuilder.dataLoaderRegistry(dataLoaderRegistry) + when: + def executionContext = new ExecutionContextBuilder() + .instrumentation(instrumentation) + .queryStrategy(queryStrategy) + .mutationStrategy(mutationStrategy) + .subscriptionStrategy(subscriptionStrategy) + .graphQLSchema(schema) + .executionId(executionId) + .context(context) + .graphQLContext(graphQLContext) + .root(root) + .operationDefinition(operation) + .fragmentsByName([MyFragment: fragment]) + .coercedVariables(coercedVariables) + .dataLoaderRegistry(dataLoaderRegistry) + .cacheControl(cacheControl) + .build() + + then: + executionContext.executionId == executionId + executionContext.instrumentation == instrumentation + executionContext.graphQLSchema == schema + executionContext.queryStrategy == queryStrategy + executionContext.mutationStrategy == mutationStrategy + executionContext.subscriptionStrategy == subscriptionStrategy + executionContext.root == root + executionContext.context == context + executionContext.graphQLContext == graphQLContext + executionContext.coercedVariables == coercedVariables + executionContext.getFragmentsByName() == [MyFragment: fragment] + executionContext.operationDefinition == operation + executionContext.dataLoaderRegistry == dataLoaderRegistry + executionContext.cacheControl == cacheControl + } - def cacheControl = CacheControl.newCacheControl() - executionContextBuilder.cacheControl(cacheControl) + def "transform works and copies values with coerced variables"() { + given: + def oldCoercedVariables = new CoercedVariables(Collections.emptyMap()) + def executionContextOld = new ExecutionContextBuilder() + .instrumentation(instrumentation) + .queryStrategy(queryStrategy) + .mutationStrategy(mutationStrategy) + .subscriptionStrategy(subscriptionStrategy) + .graphQLSchema(schema) + .executionId(executionId) + .context(context) + .graphQLContext(graphQLContext) + .root(root) + .operationDefinition(operation) + .coercedVariables(oldCoercedVariables) + .fragmentsByName([MyFragment: fragment]) + .dataLoaderRegistry(dataLoaderRegistry) + .cacheControl(cacheControl) + .build() when: - def executionContext = executionContextBuilder.build() + def coercedVariables = new CoercedVariables([var: 'value']) + def executionContext = executionContextOld.transform(builder -> builder + .coercedVariables(coercedVariables)) then: executionContext.executionId == executionId From 409c6bbb608b051fd12f9f356208bf6b2b97238d Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Mon, 18 Apr 2022 10:41:00 +1000 Subject: [PATCH 06/13] Add coerced and raw variables to Execution --- src/main/java/graphql/execution/Execution.java | 10 +++++----- .../graphql/execution/nextgen/ExecutionHelper.java | 12 ++++++------ .../groovy/graphql/execution/ExecutionTest.groovy | 8 ++++---- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 01e6e85034..f79fc32d6c 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -30,7 +30,6 @@ import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder; import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo; import static graphql.execution.ExecutionStrategyParameters.newParameters; -import static graphql.execution.nextgen.Common.getOperationRootType; import static graphql.language.OperationDefinition.Operation.MUTATION; import static graphql.language.OperationDefinition.Operation.QUERY; import static graphql.language.OperationDefinition.Operation.SUBSCRIPTION; @@ -62,12 +61,13 @@ public CompletableFuture execute(Document document, GraphQLSche Map fragmentsByName = getOperationResult.fragmentsByName; OperationDefinition operationDefinition = getOperationResult.operationDefinition; - Map inputVariables = executionInput.getVariables(); + RawVariables inputVariables = executionInput.getRawVariables(); List variableDefinitions = operationDefinition.getVariableDefinitions(); - Map coercedVariables; + CoercedVariables coercedVariables; try { - coercedVariables = valuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables); + // DZ TODO change after updating ValuesResolver#coercedVariableValues to return CoercedVariables + coercedVariables = new CoercedVariables(valuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables.getMap())); } catch (RuntimeException rte) { if (rte instanceof GraphQLError) { return completedFuture(new ExecutionResultImpl((GraphQLError) rte)); @@ -88,7 +88,7 @@ public CompletableFuture execute(Document document, GraphQLSche .localContext(executionInput.getLocalContext()) .root(executionInput.getRoot()) .fragmentsByName(fragmentsByName) - .variables(coercedVariables) + .coercedVariables(coercedVariables) .document(document) .operationDefinition(operationDefinition) .dataLoaderRegistry(executionInput.getDataLoaderRegistry()) diff --git a/src/main/java/graphql/execution/nextgen/ExecutionHelper.java b/src/main/java/graphql/execution/nextgen/ExecutionHelper.java index ac3453dda9..e052aa4a63 100644 --- a/src/main/java/graphql/execution/nextgen/ExecutionHelper.java +++ b/src/main/java/graphql/execution/nextgen/ExecutionHelper.java @@ -2,12 +2,14 @@ import graphql.ExecutionInput; import graphql.Internal; +import graphql.execution.CoercedVariables; import graphql.execution.ExecutionContext; import graphql.execution.ExecutionId; import graphql.execution.ExecutionStepInfo; import graphql.execution.FieldCollector; import graphql.execution.FieldCollectorParameters; import graphql.execution.MergedSelectionSet; +import graphql.execution.RawVariables; import graphql.execution.ResultPath; import graphql.execution.ValuesResolver; import graphql.execution.instrumentation.InstrumentationState; @@ -49,11 +51,11 @@ public ExecutionData createExecutionData(Document document, OperationDefinition operationDefinition = getOperationResult.operationDefinition; ValuesResolver valuesResolver = new ValuesResolver(); - Map inputVariables = executionInput.getVariables(); + RawVariables inputVariables = executionInput.getRawVariables(); List variableDefinitions = operationDefinition.getVariableDefinitions(); - Map coercedVariables; - coercedVariables = valuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables); + // DZ TODO update after changing type of coerceVariableValues + CoercedVariables coercedVariables = new CoercedVariables(valuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables.getMap())); ExecutionContext executionContext = newExecutionContextBuilder() .executionId(executionId) @@ -63,7 +65,7 @@ public ExecutionData createExecutionData(Document document, .graphQLContext(executionInput.getGraphQLContext()) .root(executionInput.getRoot()) .fragmentsByName(fragmentsByName) - .variables(coercedVariables) + .coercedVariables(coercedVariables) .document(document) .operationDefinition(operationDefinition) .build(); @@ -71,7 +73,6 @@ public ExecutionData createExecutionData(Document document, ExecutionData executionData = new ExecutionData(); executionData.executionContext = executionContext; return executionData; - } public FieldSubSelection getFieldSubSelection(ExecutionContext executionContext) { @@ -95,6 +96,5 @@ public FieldSubSelection getFieldSubSelection(ExecutionContext executionContext) .executionInfo(executionInfo) .build(); return fieldSubSelection; - } } diff --git a/src/test/groovy/graphql/execution/ExecutionTest.groovy b/src/test/groovy/graphql/execution/ExecutionTest.groovy index b4af50a5b2..c2539ef6c6 100644 --- a/src/test/groovy/graphql/execution/ExecutionTest.groovy +++ b/src/test/groovy/graphql/execution/ExecutionTest.groovy @@ -99,7 +99,7 @@ class ExecutionTest extends Specification { subscriptionStrategy.execute == 1 } - def "Update query strategy when instrumenting exection context" (){ + def "Update query strategy when instrumenting execution context" (){ given: def query = ''' query { @@ -114,12 +114,12 @@ class ExecutionTest extends Specification { def instrumentation = new SimpleInstrumentation() { @Override - public ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, - InstrumentationExecutionParameters parameters) { + ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, + InstrumentationExecutionParameters parameters) { return ExecutionContextBuilder.newExecutionContextBuilder(executionContext) .queryStrategy(queryStrategyUpdatedToDuringExecutionContextInstrument) - .build(); + .build() } } From 7c38af4edd3658527b658394afcde44be0a43251 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Mon, 18 Apr 2022 11:23:54 +1000 Subject: [PATCH 07/13] Add coerced and raw variables to ExecutableNormalizedOperationFactory --- .../graphql/execution/ExecutionContext.java | 2 +- .../ExecutableNormalizedOperationFactory.java | 23 +-- ...tableNormalizedOperationFactoryTest.groovy | 180 +++++++++--------- ...ormalizedOperationToAstCompilerTest.groovy | 3 +- src/test/java/benchmark/NQBenchmark1.java | 3 +- src/test/java/benchmark/NQBenchmark2.java | 5 +- 6 files changed, 111 insertions(+), 105 deletions(-) diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index 9a6bb1f3a7..372e97cf25 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -79,7 +79,7 @@ public class ExecutionContext { this.errors.set(builder.errors); this.localContext = builder.localContext; this.executionInput = builder.executionInput; - queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables.getMap())); + queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables)); } diff --git a/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java b/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java index b9d679a689..6bb9f9eb6c 100644 --- a/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java +++ b/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java @@ -6,8 +6,10 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import graphql.Internal; +import graphql.execution.CoercedVariables; import graphql.execution.ConditionalNodes; import graphql.execution.MergedField; +import graphql.execution.RawVariables; import graphql.execution.ValuesResolver; import graphql.execution.nextgen.Common; import graphql.introspection.Introspection; @@ -40,7 +42,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.Predicate; import static graphql.Assert.assertNotNull; import static graphql.Assert.assertShouldNeverHappen; @@ -61,23 +62,22 @@ public class ExecutableNormalizedOperationFactory { public static ExecutableNormalizedOperation createExecutableNormalizedOperation(GraphQLSchema graphQLSchema, Document document, String operationName, - Map coercedVariableValues) { + CoercedVariables coercedVariableValues) { NodeUtil.GetOperationResult getOperationResult = NodeUtil.getOperation(document, operationName); return new ExecutableNormalizedOperationFactory().createNormalizedQueryImpl(graphQLSchema, getOperationResult.operationDefinition, getOperationResult.fragmentsByName, coercedVariableValues, null); } - public static ExecutableNormalizedOperation createExecutableNormalizedOperation(GraphQLSchema graphQLSchema, OperationDefinition operationDefinition, Map fragments, - Map coercedVariableValues) { + CoercedVariables coercedVariableValues) { return new ExecutableNormalizedOperationFactory().createNormalizedQueryImpl(graphQLSchema, operationDefinition, fragments, coercedVariableValues, null); } public static ExecutableNormalizedOperation createExecutableNormalizedOperationWithRawVariables(GraphQLSchema graphQLSchema, Document document, String operationName, - Map rawVariables) { + RawVariables rawVariables) { NodeUtil.GetOperationResult getOperationResult = NodeUtil.getOperation(document, operationName); return new ExecutableNormalizedOperationFactory().createExecutableNormalizedOperationImplWithRawVariables(graphQLSchema, getOperationResult.operationDefinition, getOperationResult.fragmentsByName, rawVariables); } @@ -85,13 +85,14 @@ public static ExecutableNormalizedOperation createExecutableNormalizedOperationW private ExecutableNormalizedOperation createExecutableNormalizedOperationImplWithRawVariables(GraphQLSchema graphQLSchema, OperationDefinition operationDefinition, Map fragments, - Map rawVariables + RawVariables rawVariables ) { List variableDefinitions = operationDefinition.getVariableDefinitions(); - Map coerceVariableValues = valuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, rawVariables); - Map normalizedVariableValues = valuesResolver.getNormalizedVariableValues(graphQLSchema, variableDefinitions, rawVariables); - return createNormalizedQueryImpl(graphQLSchema, operationDefinition, fragments, coerceVariableValues, normalizedVariableValues); + // DZ TODO change after coerceVariableValues update + CoercedVariables coercedVariableValues = new CoercedVariables(valuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, rawVariables.getMap())); + Map normalizedVariableValues = valuesResolver.getNormalizedVariableValues(graphQLSchema, variableDefinitions, rawVariables.getMap()); + return createNormalizedQueryImpl(graphQLSchema, operationDefinition, fragments, coercedVariableValues, normalizedVariableValues); } /** @@ -100,13 +101,13 @@ private ExecutableNormalizedOperation createExecutableNormalizedOperationImplWit private ExecutableNormalizedOperation createNormalizedQueryImpl(GraphQLSchema graphQLSchema, OperationDefinition operationDefinition, Map fragments, - Map coercedVariableValues, + CoercedVariables coercedVariableValues, @Nullable Map normalizedVariableValues) { FieldCollectorNormalizedQueryParams parameters = FieldCollectorNormalizedQueryParams .newParameters() .fragments(fragments) .schema(graphQLSchema) - .coercedVariables(coercedVariableValues) + .coercedVariables(coercedVariableValues.getMap()) .normalizedVariables(normalizedVariableValues) .build(); diff --git a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy index b39225a4c1..96ba4e660c 100644 --- a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy +++ b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy @@ -2,7 +2,9 @@ package graphql.normalized import graphql.GraphQL import graphql.TestUtil +import graphql.execution.CoercedVariables import graphql.execution.MergedField +import graphql.execution.RawVariables import graphql.language.Document import graphql.language.Field import graphql.language.FragmentDefinition @@ -106,8 +108,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -192,8 +194,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -272,8 +274,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -323,8 +325,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTree(tree) expect: @@ -366,8 +368,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTree(tree) expect: @@ -416,8 +418,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTree(tree) expect: @@ -479,8 +481,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -525,8 +527,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTree(tree) expect: @@ -569,8 +571,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTree(tree) expect: @@ -613,8 +615,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTree(tree) expect: @@ -645,8 +647,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTree(tree) expect: @@ -696,8 +698,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -734,8 +736,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTree(tree) expect: @@ -779,7 +781,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) def dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -819,7 +821,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) def dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -867,7 +869,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) def dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -935,8 +937,8 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) def subFooField = (document.getDefinitions()[1] as FragmentDefinition).getSelectionSet().getSelections()[0] as Field - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def fieldToNormalizedField = tree.getFieldToNormalizedField() expect: @@ -978,8 +980,8 @@ type Dog implements Animal{ def petsField = (document.getDefinitions()[0] as OperationDefinition).getSelectionSet().getSelections()[0] as Field def idField = petsField.getSelectionSet().getSelections()[0] as Field - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def fieldToNormalizedField = tree.getFieldToNormalizedField() @@ -1027,8 +1029,8 @@ type Dog implements Animal{ def schemaField = selections[2] as Field def typeField = selections[3] as Field - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def fieldToNormalizedField = tree.getFieldToNormalizedField() expect: @@ -1084,13 +1086,13 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: printedTree == ['-Query.pet: Pet', - '--[Dog, Cat].name: String']; + '--[Dog, Cat].name: String'] } def "same result key but different field"() { @@ -1127,14 +1129,14 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTree(tree) expect: printedTree == ['Query.pet', 'name: Dog.otherField', - 'Cat.name']; + 'Cat.name'] } def "normalized field to MergedField is build"() { @@ -1155,20 +1157,20 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) - def normalizedFieldToMergedField = tree.getNormalizedFieldToMergedField(); - Traverser traverser = Traverser.depthFirst({ it.getChildren() }); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def normalizedFieldToMergedField = tree.getNormalizedFieldToMergedField() + Traverser traverser = Traverser.depthFirst({ it.getChildren() }) List result = new ArrayList<>() when: traverser.traverse(tree.getTopLevelFields(), new TraverserVisitorStub() { @Override TraversalControl enter(TraverserContext context) { - ExecutableNormalizedField normalizedField = context.thisNode(); + ExecutableNormalizedField normalizedField = context.thisNode() result.add(normalizedFieldToMergedField[normalizedField]) - return TraversalControl.CONTINUE; + return TraversalControl.CONTINUE } - }); + }) then: result.size() == 4 @@ -1193,10 +1195,10 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def coordinatesToNormalizedFields = tree.coordinatesToNormalizedFields then: @@ -1294,8 +1296,8 @@ schema { Document document = TestUtil.parseQuery(mutation) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, [:]) + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -1343,7 +1345,7 @@ schema { assertValidQuery(graphQLSchema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() def variables = [ var1: [bar: 123], var2: [foo: "foo", input2: [bar: 123]] @@ -1351,7 +1353,7 @@ schema { // the normalized arg value should be the same regardless of how the value was provided def expectedNormalizedArgValue = [foo: new NormalizedInputValue("String", parseValue('"foo"')), input2: new NormalizedInputValue("Input2", [bar: new NormalizedInputValue("Int", parseValue("123"))])] when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, variables) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(variables)) def topLevelField = tree.getTopLevelFields().get(0) def secondField = topLevelField.getChildren().get(0) def arg1 = secondField.getNormalizedArgument("arg1") @@ -1393,7 +1395,7 @@ schema { def dependencyGraph = new ExecutableNormalizedOperationFactory() def variables = [:] when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, variables) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(variables)) then: def topLevelField = tree.getTopLevelFields().get(0) @@ -1432,7 +1434,7 @@ schema { otherVar: null, ] when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, variables) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(variables)) then: def topLevelField = tree.getTopLevelFields().get(0) @@ -1482,9 +1484,9 @@ schema { ] assertValidQuery(graphQLSchema, query, variables) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, variables) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(variables)) def topLevelField = tree.getTopLevelFields().get(0) def arg1 = topLevelField.getNormalizedArgument("arg1") def arg2 = topLevelField.getNormalizedArgument("arg2") @@ -1535,9 +1537,9 @@ schema { ] assertValidQuery(graphQLSchema, query, variables) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, variables) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(variables)) def topLevelField = tree.getTopLevelFields().get(0) def arg1 = topLevelField.getNormalizedArgument("arg1") def arg2 = topLevelField.getNormalizedArgument("arg2") @@ -1590,9 +1592,9 @@ schema { ''' assertValidQuery(graphQLSchema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(Collections.emptyMap())) then: tree.normalizedFieldToMergedField.size() == 3 @@ -1648,9 +1650,9 @@ schema { ''' assertValidQuery(graphQLSchema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(Collections.emptyMap())) println String.join("\n", printTree(tree)) /** @@ -1694,9 +1696,9 @@ schema { ''' assertValidQuery(graphQLSchema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) then: @@ -1765,9 +1767,9 @@ schema { ''' assertValidQuery(schema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -1829,9 +1831,9 @@ schema { ''' assertValidQuery(schema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -1886,9 +1888,9 @@ schema { ''' assertValidQuery(schema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -1961,9 +1963,9 @@ schema { ''' assertValidQuery(schema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2023,9 +2025,9 @@ schema { ''' assertValidQuery(schema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2065,9 +2067,9 @@ schema { ''' assertValidQuery(schema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2108,9 +2110,9 @@ schema { ''' assertValidQuery(schema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2151,9 +2153,9 @@ schema { ''' assertValidQuery(schema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2226,9 +2228,9 @@ schema { ''' assertValidQuery(schema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2302,9 +2304,9 @@ schema { ''' assertValidQuery(schema, query) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, [:]) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2364,9 +2366,9 @@ schema { def variables = ["true": Boolean.TRUE, "false": Boolean.FALSE] assertValidQuery(graphQLSchema, query, variables) Document document = TestUtil.parseQuery(query) - ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory(); + ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, variables) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(variables)) println String.join("\n", printTree(tree)) def printedTree = printTree(tree) @@ -2394,7 +2396,7 @@ schema { assertValidQuery(graphQLSchema, query) Document document = TestUtil.parseQuery(query) when: - def tree = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, [:]) + def tree = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(Collections.emptyMap())) println String.join("\n", printTree(tree)) def printedTree = printTree(tree) diff --git a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy index 8d036db2d1..92ef468bba 100644 --- a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy +++ b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy @@ -2,6 +2,7 @@ package graphql.normalized import graphql.GraphQL import graphql.TestUtil +import graphql.execution.RawVariables import graphql.language.AstPrinter import graphql.language.AstSorter import graphql.language.Document @@ -2044,7 +2045,7 @@ class ExecutableNormalizedOperationToAstCompilerTest extends Specification { Document originalDocument = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - return dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, originalDocument, null, variables) + return dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, originalDocument, null, new RawVariables(variables)) } private List createNormalizedFields(GraphQLSchema schema, String query, Map variables = [:]) { diff --git a/src/test/java/benchmark/NQBenchmark1.java b/src/test/java/benchmark/NQBenchmark1.java index 1ab7aa9e6d..0ba79cac10 100644 --- a/src/test/java/benchmark/NQBenchmark1.java +++ b/src/test/java/benchmark/NQBenchmark1.java @@ -2,6 +2,7 @@ import com.google.common.base.Charsets; import com.google.common.io.Resources; +import graphql.execution.CoercedVariables; import graphql.language.Document; import graphql.normalized.ExecutableNormalizedOperation; import graphql.normalized.ExecutableNormalizedOperationFactory; @@ -83,7 +84,7 @@ public void benchMarkThroughput(MyState myState, Blackhole blackhole ) { } private void runImpl(MyState myState, Blackhole blackhole) { - ExecutableNormalizedOperation executableNormalizedOperation = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(myState.schema, myState.document, null, Collections.emptyMap()); + ExecutableNormalizedOperation executableNormalizedOperation = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(myState.schema, myState.document, null, new CoercedVariables(Collections.emptyMap())); blackhole.consume(executableNormalizedOperation); } diff --git a/src/test/java/benchmark/NQBenchmark2.java b/src/test/java/benchmark/NQBenchmark2.java index 65862a6f86..ba5790efdb 100644 --- a/src/test/java/benchmark/NQBenchmark2.java +++ b/src/test/java/benchmark/NQBenchmark2.java @@ -3,6 +3,7 @@ import com.google.common.base.Charsets; import com.google.common.collect.ImmutableListMultimap; import com.google.common.io.Resources; +import graphql.execution.CoercedVariables; import graphql.language.Document; import graphql.language.Field; import graphql.normalized.ExecutableNormalizedField; @@ -77,7 +78,7 @@ private String readFromClasspath(String file) throws IOException { @BenchmarkMode(Mode.AverageTime) @OutputTimeUnit(TimeUnit.MILLISECONDS) public ExecutableNormalizedOperation benchMarkAvgTime(MyState myState) throws ExecutionException, InterruptedException { - ExecutableNormalizedOperation executableNormalizedOperation = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(myState.schema, myState.document, null, Collections.emptyMap()); + ExecutableNormalizedOperation executableNormalizedOperation = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(myState.schema, myState.document, null, new CoercedVariables(Collections.emptyMap())); // System.out.println("fields size:" + normalizedQuery.getFieldToNormalizedField().size()); return executableNormalizedOperation; } @@ -85,7 +86,7 @@ public ExecutableNormalizedOperation benchMarkAvgTime(MyState myState) throws Ex public static void main(String[] args) { MyState myState = new MyState(); myState.setup(); - ExecutableNormalizedOperation executableNormalizedOperation = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(myState.schema, myState.document, null, Collections.emptyMap()); + ExecutableNormalizedOperation executableNormalizedOperation = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(myState.schema, myState.document, null, new CoercedVariables(Collections.emptyMap())); // System.out.println(printTree(normalizedQuery)); ImmutableListMultimap fieldToNormalizedField = executableNormalizedOperation.getFieldToNormalizedField(); System.out.println(fieldToNormalizedField.size()); From 86af8713c2e1deef0cf9362e62aae95191f266be Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Mon, 18 Apr 2022 12:57:27 +1000 Subject: [PATCH 08/13] Add raw and coerced variables to ValuesResolver and update all method calls --- .../analysis/NodeVisitorWithTypeTracking.java | 3 +- .../java/graphql/analysis/QueryTraverser.java | 4 +- .../graphql/execution/ConditionalNodes.java | 2 +- .../java/graphql/execution/Execution.java | 3 +- .../execution/ExecutionStepInfoFactory.java | 2 +- .../graphql/execution/ExecutionStrategy.java | 2 +- .../graphql/execution/ValuesResolver.java | 47 +++++++++---------- .../directives/DirectivesResolver.java | 3 +- .../execution/nextgen/ExecutionHelper.java | 3 +- .../execution/nextgen/ValueFetcher.java | 2 +- .../ExecutableNormalizedOperationFactory.java | 7 ++- 11 files changed, 37 insertions(+), 41 deletions(-) diff --git a/src/main/java/graphql/analysis/NodeVisitorWithTypeTracking.java b/src/main/java/graphql/analysis/NodeVisitorWithTypeTracking.java index 4c66d6e772..1302393280 100644 --- a/src/main/java/graphql/analysis/NodeVisitorWithTypeTracking.java +++ b/src/main/java/graphql/analysis/NodeVisitorWithTypeTracking.java @@ -1,6 +1,7 @@ package graphql.analysis; import graphql.Internal; +import graphql.execution.CoercedVariables; import graphql.execution.ConditionalNodes; import graphql.execution.ValuesResolver; import graphql.introspection.Introspection; @@ -154,7 +155,7 @@ public TraversalControl visitField(Field field, TraverserContext context) boolean isTypeNameIntrospectionField = fieldDefinition == schema.getIntrospectionTypenameFieldDefinition(); GraphQLFieldsContainer fieldsContainer = !isTypeNameIntrospectionField ? (GraphQLFieldsContainer) unwrapAll(parentEnv.getOutputType()) : null; GraphQLCodeRegistry codeRegistry = schema.getCodeRegistry(); - Map argumentValues = valuesResolver.getArgumentValues(codeRegistry, fieldDefinition.getArguments(), field.getArguments(), variables); + Map argumentValues = valuesResolver.getArgumentValues(codeRegistry, fieldDefinition.getArguments(), field.getArguments(), new CoercedVariables(variables)); QueryVisitorFieldEnvironment environment = new QueryVisitorFieldEnvironmentImpl(isTypeNameIntrospectionField, field, fieldDefinition, diff --git a/src/main/java/graphql/analysis/QueryTraverser.java b/src/main/java/graphql/analysis/QueryTraverser.java index a05e780214..6f064e0b56 100644 --- a/src/main/java/graphql/analysis/QueryTraverser.java +++ b/src/main/java/graphql/analysis/QueryTraverser.java @@ -1,6 +1,7 @@ package graphql.analysis; import graphql.PublicApi; +import graphql.execution.RawVariables; import graphql.execution.ValuesResolver; import graphql.language.Document; import graphql.language.FragmentDefinition; @@ -61,7 +62,8 @@ private QueryTraverser(GraphQLSchema schema, } private Map coerceVariables(Map rawVariables, List variableDefinitions) { - return new ValuesResolver().coerceVariableValues(schema, variableDefinitions, rawVariables); + // DZ TODO change return type after refactoring this class + return new ValuesResolver().coerceVariableValues(schema, variableDefinitions, new RawVariables(rawVariables)).getMap(); } private QueryTraverser(GraphQLSchema schema, diff --git a/src/main/java/graphql/execution/ConditionalNodes.java b/src/main/java/graphql/execution/ConditionalNodes.java index bd13855318..b1afe580ad 100644 --- a/src/main/java/graphql/execution/ConditionalNodes.java +++ b/src/main/java/graphql/execution/ConditionalNodes.java @@ -34,7 +34,7 @@ public boolean shouldInclude(Map variables, List dire private boolean getDirectiveResult(Map variables, List directives, String directiveName, boolean defaultValue) { Directive foundDirective = NodeUtil.findNodeByName(directives, directiveName); if (foundDirective != null) { - Map argumentValues = valuesResolver.getArgumentValues(SkipDirective.getArguments(), foundDirective.getArguments(), variables); + Map argumentValues = valuesResolver.getArgumentValues(SkipDirective.getArguments(), foundDirective.getArguments(), new CoercedVariables(variables)); Object flag = argumentValues.get("if"); Assert.assertTrue(flag instanceof Boolean, () -> String.format("The '%s' directive MUST have a value for the 'if' argument", directiveName)); return (Boolean) flag; diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index f79fc32d6c..5e113f1621 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -66,8 +66,7 @@ public CompletableFuture execute(Document document, GraphQLSche CoercedVariables coercedVariables; try { - // DZ TODO change after updating ValuesResolver#coercedVariableValues to return CoercedVariables - coercedVariables = new CoercedVariables(valuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables.getMap())); + coercedVariables = valuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables); } catch (RuntimeException rte) { if (rte instanceof GraphQLError) { return completedFuture(new ExecutionResultImpl((GraphQLError) rte)); diff --git a/src/main/java/graphql/execution/ExecutionStepInfoFactory.java b/src/main/java/graphql/execution/ExecutionStepInfoFactory.java index 31a2df65dd..57b8044a79 100644 --- a/src/main/java/graphql/execution/ExecutionStepInfoFactory.java +++ b/src/main/java/graphql/execution/ExecutionStepInfoFactory.java @@ -27,7 +27,7 @@ public ExecutionStepInfo newExecutionStepInfoForSubField(ExecutionContext execut GraphQLOutputType fieldType = fieldDefinition.getType(); List fieldArgs = mergedField.getArguments(); GraphQLCodeRegistry codeRegistry = executionContext.getGraphQLSchema().getCodeRegistry(); - Supplier> argumentValues = FpKit.intraThreadMemoize(() -> valuesResolver.getArgumentValues(codeRegistry, fieldDefinition.getArguments(), fieldArgs, executionContext.getVariables())); + Supplier> argumentValues = FpKit.intraThreadMemoize(() -> valuesResolver.getArgumentValues(codeRegistry, fieldDefinition.getArguments(), fieldArgs, executionContext.getCoercedVariables())); ResultPath newPath = parentInfo.getPath().segment(mergedField.getResultKey()); diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index 5f5467292d..48f77e5964 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -817,7 +817,7 @@ protected ExecutionStepInfo createExecutionStepInfo(ExecutionContext executionCo if (!fieldArgDefs.isEmpty()) { List fieldArgs = field.getArguments(); GraphQLCodeRegistry codeRegistry = executionContext.getGraphQLSchema().getCodeRegistry(); - argumentValues = FpKit.intraThreadMemoize(() -> valuesResolver.getArgumentValues(codeRegistry, fieldArgDefs, fieldArgs, executionContext.getVariables())); + argumentValues = FpKit.intraThreadMemoize(() -> valuesResolver.getArgumentValues(codeRegistry, fieldArgDefs, fieldArgs, executionContext.getCoercedVariables())); } diff --git a/src/main/java/graphql/execution/ValuesResolver.java b/src/main/java/graphql/execution/ValuesResolver.java index 23c51a5525..835b4cbe79 100644 --- a/src/main/java/graphql/execution/ValuesResolver.java +++ b/src/main/java/graphql/execution/ValuesResolver.java @@ -87,9 +87,9 @@ public enum ValueMode { * * @return coerced variable values as a map */ - public Map coerceVariableValues(GraphQLSchema schema, - List variableDefinitions, - Map rawVariables) throws CoercingParseValueException, NonNullableValueCoercedAsNullException { + public CoercedVariables coerceVariableValues(GraphQLSchema schema, + List variableDefinitions, + RawVariables rawVariables) throws CoercingParseValueException, NonNullableValueCoercedAsNullException { return externalValueToInternalValueForVariables(schema, variableDefinitions, rawVariables); } @@ -105,7 +105,7 @@ public Map coerceVariableValues(GraphQLSchema schema, */ public Map getNormalizedVariableValues(GraphQLSchema schema, List variableDefinitions, - Map rawVariables) { + RawVariables rawVariables) { GraphqlFieldVisibility fieldVisibility = schema.getCodeRegistry().getFieldVisibility(); Map result = new LinkedHashMap<>(); for (VariableDefinition variableDefinition : variableDefinitions) { @@ -145,7 +145,7 @@ public Map getNormalizedVariableValues(GraphQLSche */ public Map getArgumentValues(List argumentTypes, List arguments, - Map coercedVariables) { + CoercedVariables coercedVariables) { return getArgumentValuesImpl(DEFAULT_FIELD_VISIBILITY, argumentTypes, arguments, coercedVariables); } @@ -189,7 +189,7 @@ public Map getNormalizedArgumentValues(List getArgumentValues(GraphQLCodeRegistry codeRegistry, List argumentTypes, List arguments, - Map coercedVariables) { + CoercedVariables coercedVariables) { return getArgumentValuesImpl(codeRegistry.getFieldVisibility(), argumentTypes, arguments, coercedVariables); } @@ -248,7 +248,7 @@ public static Object valueToInternalValue(InputValueWithState inputValueWithStat return inputValueWithState.getValue(); } if (inputValueWithState.isLiteral()) { - return new ValuesResolver().literalToInternalValue(fieldVisibility, type, (Value) inputValueWithState.getValue(), emptyMap()); + return new ValuesResolver().literalToInternalValue(fieldVisibility, type, (Value) inputValueWithState.getValue(), new CoercedVariables(emptyMap())); } if (inputValueWithState.isExternal()) { return new ValuesResolver().externalValueToInternalValue(fieldVisibility, type, inputValueWithState.getValue()); @@ -316,9 +316,7 @@ private Object externalValueToLiteralForList(GraphqlFieldVisibility fieldVisibil GraphQLInputType wrappedType = (GraphQLInputType) listType.getWrappedType(); List result = FpKit.toListOrSingletonList(value) .stream() - .map(val -> { - return externalValueToLiteral(fieldVisibility, val, wrappedType, valueMode); - }) + .map(val -> externalValueToLiteral(fieldVisibility, val, wrappedType, valueMode)) .collect(toList()); if (valueMode == NORMALIZED) { return result; @@ -384,9 +382,9 @@ private Object externalValueToLiteralForObject(GraphqlFieldVisibility fieldVisib /** * performs validation too */ - private Map externalValueToInternalValueForVariables(GraphQLSchema schema, - List variableDefinitions, - Map rawVariables) { + private CoercedVariables externalValueToInternalValueForVariables(GraphQLSchema schema, + List variableDefinitions, + RawVariables rawVariables) { GraphqlFieldVisibility fieldVisibility = schema.getCodeRegistry().getFieldVisibility(); Map coercedValues = new LinkedHashMap<>(); for (VariableDefinition variableDefinition : variableDefinitions) { @@ -399,7 +397,7 @@ private Map externalValueToInternalValueForVariables(GraphQLSche boolean hasValue = rawVariables.containsKey(variableName); Object value = rawVariables.get(variableName); if (!hasValue && defaultValue != null) { - Object coercedDefaultValue = literalToInternalValue(fieldVisibility, variableType, defaultValue, Collections.emptyMap()); + Object coercedDefaultValue = literalToInternalValue(fieldVisibility, variableType, defaultValue, new CoercedVariables(Collections.emptyMap())); coercedValues.put(variableName, coercedDefaultValue); } else if (isNonNull(variableType) && (!hasValue || value == null)) { throw new NonNullableValueCoercedAsNullException(variableDefinition, variableType); @@ -423,15 +421,14 @@ private Map externalValueToInternalValueForVariables(GraphQLSche } } - return coercedValues; + return new CoercedVariables(coercedValues); } private Map getArgumentValuesImpl(GraphqlFieldVisibility fieldVisibility, List argumentTypes, List arguments, - Map coercedVariables - ) { + CoercedVariables coercedVariables) { if (argumentTypes.isEmpty()) { return Collections.emptyMap(); } @@ -474,7 +471,6 @@ private Map getArgumentValuesImpl(GraphqlFieldVisibility fieldVi } return coercedValues; - } private Map argumentMap(List arguments) { @@ -679,11 +675,10 @@ private List literalToNormalizedValueForList(GraphqlFieldVisibility fiel public Object literalToInternalValue(GraphqlFieldVisibility fieldVisibility, GraphQLType type, Value inputValue, - Map coercedVariables) { + CoercedVariables coercedVariables) { if (inputValue instanceof VariableReference) { - Object variableValue = coercedVariables.get(((VariableReference) inputValue).getName()); - return variableValue; + return coercedVariables.get(((VariableReference) inputValue).getName()); } if (inputValue instanceof NullValue) { return null; @@ -709,9 +704,9 @@ public Object literalToInternalValue(GraphqlFieldVisibility fieldVisibility, /** * no validation */ - private Object literalToInternalValueForScalar(Value inputValue, GraphQLScalarType scalarType, Map variables) { + private Object literalToInternalValueForScalar(Value inputValue, GraphQLScalarType scalarType, CoercedVariables coercedVariables) { // the CoercingParseLiteralException exception that could happen here has been validated earlier via ValidationUtil - return scalarType.getCoercing().parseLiteral(inputValue, variables); + return scalarType.getCoercing().parseLiteral(inputValue, coercedVariables.getMap()); } /** @@ -720,7 +715,7 @@ private Object literalToInternalValueForScalar(Value inputValue, GraphQLScalarTy private Object literalToInternalValueForList(GraphqlFieldVisibility fieldVisibility, GraphQLList graphQLList, Value value, - Map coercedVariables) { + CoercedVariables coercedVariables) { if (value instanceof ArrayValue) { ArrayValue arrayValue = (ArrayValue) value; @@ -744,7 +739,7 @@ private Object literalToInternalValueForList(GraphqlFieldVisibility fieldVisibil private Object literalToInternalValueForInputObject(GraphqlFieldVisibility fieldVisibility, GraphQLInputObjectType type, ObjectValue inputValue, - Map coercedVariables) { + CoercedVariables coercedVariables) { Map coercedValues = new LinkedHashMap<>(); Map inputFieldsByName = mapObjectValueFieldsByName(inputValue); @@ -813,7 +808,7 @@ private Object defaultValueToInternalValue(GraphqlFieldVisibility fieldVisibilit } if (defaultValue.isLiteral()) { // default value literals can't reference variables, this is why the variables are empty - return literalToInternalValue(fieldVisibility, type, (Value) defaultValue.getValue(), Collections.emptyMap()); + return literalToInternalValue(fieldVisibility, type, (Value) defaultValue.getValue(), new CoercedVariables(Collections.emptyMap())); } if (defaultValue.isExternal()) { // performs validation too diff --git a/src/main/java/graphql/execution/directives/DirectivesResolver.java b/src/main/java/graphql/execution/directives/DirectivesResolver.java index 40295d756c..13b518b1a1 100644 --- a/src/main/java/graphql/execution/directives/DirectivesResolver.java +++ b/src/main/java/graphql/execution/directives/DirectivesResolver.java @@ -2,6 +2,7 @@ import com.google.common.collect.ImmutableMap; import graphql.Internal; +import graphql.execution.CoercedVariables; import graphql.execution.ValuesResolver; import graphql.language.Directive; import graphql.schema.GraphQLArgument; @@ -38,7 +39,7 @@ public Map resolveDirectives(List directive } private void buildArguments(GraphQLDirective.Builder directiveBuilder, GraphQLCodeRegistry codeRegistry, GraphQLDirective protoType, Directive fieldDirective, Map variables) { - Map argumentValues = valuesResolver.getArgumentValues(codeRegistry, protoType.getArguments(), fieldDirective.getArguments(), variables); + Map argumentValues = valuesResolver.getArgumentValues(codeRegistry, protoType.getArguments(), fieldDirective.getArguments(), new CoercedVariables(variables)); directiveBuilder.clearArguments(); protoType.getArguments().forEach(protoArg -> { if (argumentValues.containsKey(protoArg.getName())) { diff --git a/src/main/java/graphql/execution/nextgen/ExecutionHelper.java b/src/main/java/graphql/execution/nextgen/ExecutionHelper.java index e052aa4a63..5d26e1b65e 100644 --- a/src/main/java/graphql/execution/nextgen/ExecutionHelper.java +++ b/src/main/java/graphql/execution/nextgen/ExecutionHelper.java @@ -54,8 +54,7 @@ public ExecutionData createExecutionData(Document document, RawVariables inputVariables = executionInput.getRawVariables(); List variableDefinitions = operationDefinition.getVariableDefinitions(); - // DZ TODO update after changing type of coerceVariableValues - CoercedVariables coercedVariables = new CoercedVariables(valuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables.getMap())); + CoercedVariables coercedVariables = valuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables); ExecutionContext executionContext = newExecutionContextBuilder() .executionId(executionId) diff --git a/src/main/java/graphql/execution/nextgen/ValueFetcher.java b/src/main/java/graphql/execution/nextgen/ValueFetcher.java index f808062e37..65c38a54c6 100644 --- a/src/main/java/graphql/execution/nextgen/ValueFetcher.java +++ b/src/main/java/graphql/execution/nextgen/ValueFetcher.java @@ -123,7 +123,7 @@ public CompletableFuture fetchValue(ExecutionContext executionCont GraphQLCodeRegistry codeRegistry = executionContext.getGraphQLSchema().getCodeRegistry(); GraphQLFieldsContainer parentType = getFieldsContainer(executionInfo); - Supplier> argumentValues = FpKit.intraThreadMemoize(() -> valuesResolver.getArgumentValues(codeRegistry, fieldDef.getArguments(), field.getArguments(), executionContext.getVariables())); + Supplier> argumentValues = FpKit.intraThreadMemoize(() -> valuesResolver.getArgumentValues(codeRegistry, fieldDef.getArguments(), field.getArguments(), executionContext.getCoercedVariables())); QueryDirectivesImpl queryDirectives = new QueryDirectivesImpl(sameFields, executionContext.getGraphQLSchema(), executionContext.getVariables()); diff --git a/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java b/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java index 6bb9f9eb6c..485da4d844 100644 --- a/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java +++ b/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java @@ -89,9 +89,8 @@ private ExecutableNormalizedOperation createExecutableNormalizedOperationImplWit ) { List variableDefinitions = operationDefinition.getVariableDefinitions(); - // DZ TODO change after coerceVariableValues update - CoercedVariables coercedVariableValues = new CoercedVariables(valuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, rawVariables.getMap())); - Map normalizedVariableValues = valuesResolver.getNormalizedVariableValues(graphQLSchema, variableDefinitions, rawVariables.getMap()); + CoercedVariables coercedVariableValues = valuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, rawVariables); + Map normalizedVariableValues = valuesResolver.getNormalizedVariableValues(graphQLSchema, variableDefinitions, rawVariables); return createNormalizedQueryImpl(graphQLSchema, operationDefinition, fragments, coercedVariableValues, normalizedVariableValues); } @@ -306,7 +305,7 @@ private ExecutableNormalizedField createNF(FieldCollectorNormalizedQueryParams p String fieldName = field.getName(); GraphQLFieldDefinition fieldDefinition = Introspection.getFieldDef(parameters.getGraphQLSchema(), objectTypes.iterator().next(), fieldName); - Map argumentValues = valuesResolver.getArgumentValues(fieldDefinition.getArguments(), field.getArguments(), parameters.getCoercedVariableValues()); + Map argumentValues = valuesResolver.getArgumentValues(fieldDefinition.getArguments(), field.getArguments(), new CoercedVariables(parameters.getCoercedVariableValues())); Map normalizedArgumentValues = null; if (parameters.getNormalizedVariableValues() != null) { normalizedArgumentValues = valuesResolver.getNormalizedArgumentValues(fieldDefinition.getArguments(), field.getArguments(), parameters.getNormalizedVariableValues()); From f1fe14a1ac00c60fb4b3ecc2199ded3106cead49 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Mon, 18 Apr 2022 12:57:54 +1000 Subject: [PATCH 09/13] Update tests after ValuesResolver refactor --- .../execution/ConditionalNodesTest.groovy | 7 +- .../execution/ValuesResolverTest.groovy | 84 ++++++++----------- 2 files changed, 37 insertions(+), 54 deletions(-) diff --git a/src/test/groovy/graphql/execution/ConditionalNodesTest.groovy b/src/test/groovy/graphql/execution/ConditionalNodesTest.groovy index 55ea94e844..7c76600727 100644 --- a/src/test/groovy/graphql/execution/ConditionalNodesTest.groovy +++ b/src/test/groovy/graphql/execution/ConditionalNodesTest.groovy @@ -1,6 +1,6 @@ package graphql.execution -import graphql.Directives + import graphql.language.Argument import graphql.language.BooleanValue import graphql.language.Directive @@ -8,18 +8,14 @@ import spock.lang.Specification class ConditionalNodesTest extends Specification { - def "should include false for skip = true"() { given: def variables = new LinkedHashMap() ConditionalNodes conditionalNodes = new ConditionalNodes() - conditionalNodes.valuesResolver = Mock(ValuesResolver) def argument = Argument.newArgument("if", new BooleanValue(true)).build() def directives = [Directive.newDirective().name("skip").arguments([argument]).build()] - conditionalNodes.valuesResolver.getArgumentValues(Directives.SkipDirective.getArguments(), [argument], variables) >> ["if": true] - expect: !conditionalNodes.shouldInclude(variables, directives) } @@ -31,6 +27,5 @@ class ConditionalNodesTest extends Specification { expect: conditionalNodes.shouldInclude(variables, []) - } } diff --git a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy index 783fb1cdc1..03b6a8616b 100644 --- a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy +++ b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy @@ -39,16 +39,15 @@ class ValuesResolverTest extends Specification { ValuesResolver resolver = new ValuesResolver() - @Unroll def "getVariableValues: simple variable input #inputValue"() { given: def schema = TestUtil.schemaWithInputType(inputType) VariableDefinition variableDefinition = new VariableDefinition("variable", variableType, null) when: - def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], [variable: inputValue]) + def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: inputValue])) then: - resolvedValues['variable'] == outputValue + resolvedValues.get('variable') == outputValue where: inputType | variableType | inputValue || outputValue @@ -56,7 +55,6 @@ class ValuesResolverTest extends Specification { GraphQLString | new TypeName("String") | 'someString' || 'someString' GraphQLBoolean | new TypeName("Boolean") | 'true' || true GraphQLFloat | new TypeName("Float") | '42.43' || 42.43d - } def "getVariableValues: map object as variable input"() { @@ -76,9 +74,9 @@ class ValuesResolverTest extends Specification { VariableDefinition variableDefinition = new VariableDefinition("variable", new TypeName("Person")) when: - def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], [variable: inputValue]) + def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: inputValue])) then: - resolvedValues['variable'] == outputValue + resolvedValues.get('variable') == outputValue where: inputValue || outputValue [name: 'a', id: 123] || [name: 'a', id: 123] @@ -116,7 +114,7 @@ class ValuesResolverTest extends Specification { when: def obj = new Person('a', 123) - resolver.coerceVariableValues(schema, [variableDefinition], [variable: obj]) + resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: obj])) then: thrown(CoercingParseValueException) } @@ -127,10 +125,9 @@ class ValuesResolverTest extends Specification { VariableDefinition variableDefinition = new VariableDefinition("variable", new ListType(new TypeName("String"))) String value = "world" when: - def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], [variable: value]) + def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: value])) then: - resolvedValues['variable'] == ['world'] - + resolvedValues.get('variable') == ['world'] } def "getVariableValues: list value gets resolved to a list when the type is a List"() { @@ -139,10 +136,9 @@ class ValuesResolverTest extends Specification { VariableDefinition variableDefinition = new VariableDefinition("variable", new ListType(new TypeName("String"))) List value = ["hello","world"] when: - def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], [variable: value]) + def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: value])) then: - resolvedValues['variable'] == ['hello','world'] - + resolvedValues.get('variable') == ['hello','world'] } def "getVariableValues: array value gets resolved to a list when the type is a List"() { @@ -151,16 +147,14 @@ class ValuesResolverTest extends Specification { VariableDefinition variableDefinition = new VariableDefinition("variable", new ListType(new TypeName("String"))) String[] value = ["hello","world"] as String[] when: - def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], [variable: value]) + def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: value])) then: - resolvedValues['variable'] == ['hello','world'] - + resolvedValues.get('variable') == ['hello','world'] } - def "getArgumentValues: resolves argument with variable reference"() { given: - def variables = [var: 'hello'] + def variables = new CoercedVariables([var: 'hello']) def fieldArgument = newArgument().name("arg").type(GraphQLString).build() def argument = new Argument("arg", new VariableReference("var")) @@ -181,8 +175,8 @@ class ValuesResolverTest extends Specification { def argument = new Argument("arg", new VariableReference("var")) when: - def variables = [:] - def values = resolver.getArgumentValues([fieldArgument], [argument], variables as Map) + def variables = new CoercedVariables(Collections.emptyMap()) + def values = resolver.getArgumentValues([fieldArgument], [argument], variables) then: values['arg'] == 'hello' @@ -212,7 +206,7 @@ class ValuesResolverTest extends Specification { when: def argument = new Argument("arg", inputValue) - def values = resolver.getArgumentValues([fieldArgument], [argument], [:]) + def values = resolver.getArgumentValues([fieldArgument], [argument], new CoercedVariables(Collections.emptyMap())) then: values['arg'] == outputValue @@ -260,7 +254,7 @@ class ValuesResolverTest extends Specification { when: def argument = new Argument("arg", inputValue) - def values = resolver.getArgumentValues([fieldArgument], [argument], [:]) + def values = resolver.getArgumentValues([fieldArgument], [argument], new CoercedVariables(Collections.emptyMap())) then: values['arg'] == outputValue @@ -308,7 +302,7 @@ class ValuesResolverTest extends Specification { def fieldArgument1 = newArgument().name("arg1").type(enumType).build() def fieldArgument2 = newArgument().name("arg2").type(enumType).build() when: - def values = resolver.getArgumentValues([fieldArgument1, fieldArgument2], [argument1, argument2], [:]) + def values = resolver.getArgumentValues([fieldArgument1, fieldArgument2], [argument1, argument2], new CoercedVariables(Collections.emptyMap())) then: values['arg1'] == 'PLUTO' @@ -325,11 +319,10 @@ class ValuesResolverTest extends Specification { def fieldArgument = newArgument().name("arg").type(list(GraphQLBoolean)).build() when: - def values = resolver.getArgumentValues([fieldArgument], [argument], [:]) + def values = resolver.getArgumentValues([fieldArgument], [argument], new CoercedVariables(Collections.emptyMap())) then: values['arg'] == [true, false] - } def "getArgumentValues: resolves single value literal to a list when type is a list "() { @@ -340,11 +333,10 @@ class ValuesResolverTest extends Specification { def fieldArgument = newArgument().name("arg").type(list(GraphQLString)).build() when: - def values = resolver.getArgumentValues([fieldArgument], [argument], [:]) + def values = resolver.getArgumentValues([fieldArgument], [argument], new CoercedVariables(Collections.emptyMap())) then: values['arg'] == ['world'] - } def "getVariableValues: enum as variable input"() { @@ -359,14 +351,13 @@ class ValuesResolverTest extends Specification { VariableDefinition variableDefinition = new VariableDefinition("variable", new TypeName("Test")) when: - def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], [variable: inputValue]) + def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: inputValue])) then: - resolvedValues['variable'] == outputValue + resolvedValues.get('variable') == outputValue where: inputValue || outputValue "A_TEST" || "A_TEST" "VALUE_TEST" || 1 - } @Unroll @@ -388,20 +379,18 @@ class ValuesResolverTest extends Specification { VariableDefinition variableDefinition = new VariableDefinition("variable", new TypeName("InputObject")) when: - def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], [variable: inputValue]) + def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: inputValue])) then: - resolvedValues['variable'] == outputValue + resolvedValues.get('variable') == outputValue where: inputValue || outputValue [intKey: 10] || [intKey: 10, stringKey: 'defaultString'] [intKey: 10, stringKey: null] || [intKey: 10, stringKey: null] - } def "getVariableInput: Missing InputObject fields which are non-null cause error"() { - given: def inputObjectType = newInputObject() .name("InputObject") @@ -417,7 +406,7 @@ class ValuesResolverTest extends Specification { VariableDefinition variableDefinition = new VariableDefinition("variable", new TypeName("InputObject")) when: - resolver.coerceVariableValues(schema, [variableDefinition], [variable: inputValue]) + resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: inputValue])) then: thrown(GraphQLException) @@ -436,10 +425,10 @@ class ValuesResolverTest extends Specification { VariableDefinition barVarDef = new VariableDefinition("bar", new TypeName("String")) when: - def resolvedValues = resolver.coerceVariableValues(schema, [fooVarDef, barVarDef], InputValue) + def resolvedValues = resolver.coerceVariableValues(schema, [fooVarDef, barVarDef], new RawVariables(InputValue)) then: - resolvedValues == outputValue + resolvedValues.getMap() == outputValue where: InputValue || outputValue @@ -454,7 +443,7 @@ class ValuesResolverTest extends Specification { VariableDefinition fooVarDef = new VariableDefinition("foo", new NonNullType(new TypeName("String"))) when: - resolver.coerceVariableValues(schema, [fooVarDef], [:]) + resolver.coerceVariableValues(schema, [fooVarDef], new RawVariables(Collections.emptyMap())) then: thrown(GraphQLException) @@ -470,14 +459,14 @@ class ValuesResolverTest extends Specification { def defaultValueForBar = new StringValue("defaultValueForBar") VariableDefinition barVarDef = new VariableDefinition("bar", new TypeName("String"), defaultValueForBar) - def variableValuesMap = ["foo": null, "bar": "barValue"] + def variableValuesMap = new RawVariables(["foo": null, "bar": "barValue"]) when: def resolvedVars = resolver.coerceVariableValues(schema, [fooVarDef, barVarDef], variableValuesMap) then: - resolvedVars['foo'] == null - resolvedVars['bar'] == "barValue" + resolvedVars.get('foo') == null + resolvedVars.get('bar') == "barValue" } def "coerceVariableValues: if variableType is a Non-Nullable type and value is null, throw a query error"() { @@ -487,8 +476,7 @@ class ValuesResolverTest extends Specification { def defaultValueForFoo = new StringValue("defaultValueForFoo") VariableDefinition fooVarDef = new VariableDefinition("foo", new NonNullType(new TypeName("String")), defaultValueForFoo) - - def variableValuesMap = ["foo": null] + def variableValuesMap = new RawVariables(["foo": null]) when: resolver.coerceVariableValues(schema, [fooVarDef], variableValuesMap) @@ -506,7 +494,7 @@ class ValuesResolverTest extends Specification { def type = new ListType(new NonNullType(new TypeName("String"))) VariableDefinition fooVarDef = new VariableDefinition("foo", type, defaultValueForFoo) - def variableValuesMap = ["foo": [null]] + def variableValuesMap = new RawVariables(["foo": [null]]) when: resolver.coerceVariableValues(schema, [fooVarDef], variableValuesMap) @@ -528,8 +516,8 @@ class ValuesResolverTest extends Specification { def argument = new Argument("arg", NullValue.newNullValue().build()) when: - def variables = [:] - def values = resolver.getArgumentValues([fieldArgument], [argument], variables as Map) + def variables = new CoercedVariables(Collections.emptyMap()) + def values = resolver.getArgumentValues([fieldArgument], [argument], variables) then: values['arg'] == null @@ -545,7 +533,7 @@ class ValuesResolverTest extends Specification { def argument = new Argument("arg", new VariableReference("var")) when: - def variables = ["var": null] + def variables = new CoercedVariables(["var": null]) def values = resolver.getArgumentValues([fieldArgument], [argument], variables) then: @@ -562,7 +550,7 @@ class ValuesResolverTest extends Specification { def argument = new Argument("arg", new VariableReference("var")) when: - def variables = ["var": null] + def variables = new CoercedVariables(["var": null]) resolver.getArgumentValues([fieldArgument], [argument], variables) then: From 61198b5b1b589aa75f93ca74d236855a9088820e Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Mon, 18 Apr 2022 15:26:13 +1000 Subject: [PATCH 10/13] Add coerced variables to Query Traverser --- .../java/graphql/analysis/QueryTraverser.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/graphql/analysis/QueryTraverser.java b/src/main/java/graphql/analysis/QueryTraverser.java index 6f064e0b56..be3e5bfac0 100644 --- a/src/main/java/graphql/analysis/QueryTraverser.java +++ b/src/main/java/graphql/analysis/QueryTraverser.java @@ -1,6 +1,7 @@ package graphql.analysis; import graphql.PublicApi; +import graphql.execution.CoercedVariables; import graphql.execution.RawVariables; import graphql.execution.ValuesResolver; import graphql.language.Document; @@ -43,7 +44,7 @@ public class QueryTraverser { private final Collection roots; private final GraphQLSchema schema; private final Map fragmentsByName; - private final Map variables; + private CoercedVariables coercedVariables; private final GraphQLCompositeType rootParentType; @@ -51,19 +52,18 @@ private QueryTraverser(GraphQLSchema schema, Document document, String operation, Map variables) { - assertNotNull(document, () -> "document can't be null"); + assertNotNull(document, () -> "document can't be null"); NodeUtil.GetOperationResult getOperationResult = NodeUtil.getOperation(document, operation); List variableDefinitions = getOperationResult.operationDefinition.getVariableDefinitions(); this.schema = assertNotNull(schema, () -> "schema can't be null"); this.fragmentsByName = getOperationResult.fragmentsByName; this.roots = singletonList(getOperationResult.operationDefinition); this.rootParentType = getRootTypeFromOperation(getOperationResult.operationDefinition); - this.variables = coerceVariables(assertNotNull(variables, () -> "variables can't be null"), variableDefinitions); + this.coercedVariables = coerceVariables(assertNotNull(variables, () -> "variables can't be null"), variableDefinitions); } - private Map coerceVariables(Map rawVariables, List variableDefinitions) { - // DZ TODO change return type after refactoring this class - return new ValuesResolver().coerceVariableValues(schema, variableDefinitions, new RawVariables(rawVariables)).getMap(); + private CoercedVariables coerceVariables(Map rawVariables, List variableDefinitions) { + return new ValuesResolver().coerceVariableValues(schema, variableDefinitions, new RawVariables(rawVariables)); } private QueryTraverser(GraphQLSchema schema, @@ -72,11 +72,11 @@ private QueryTraverser(GraphQLSchema schema, Map fragmentsByName, Map variables) { this.schema = assertNotNull(schema, () -> "schema can't be null"); - this.variables = assertNotNull(variables, () -> "variables can't be null"); assertNotNull(root, () -> "root can't be null"); this.roots = Collections.singleton(root); this.rootParentType = assertNotNull(rootParentType, () -> "rootParentType can't be null"); this.fragmentsByName = assertNotNull(fragmentsByName, () -> "fragmentsByName can't be null"); + this.coercedVariables = new CoercedVariables(assertNotNull(variables, () -> "variables can't be null")); } public Object visitDepthFirst(QueryVisitor queryVisitor) { @@ -183,7 +183,7 @@ private Object visitImpl(QueryVisitor visitFieldCallback, Boolean preOrder) { } NodeTraverser nodeTraverser = new NodeTraverser(rootVars, this::childrenOf); - NodeVisitorWithTypeTracking nodeVisitorWithTypeTracking = new NodeVisitorWithTypeTracking(preOrderCallback, postOrderCallback, variables, schema, fragmentsByName); + NodeVisitorWithTypeTracking nodeVisitorWithTypeTracking = new NodeVisitorWithTypeTracking(preOrderCallback, postOrderCallback, coercedVariables.getMap(), schema, fragmentsByName); return nodeTraverser.depthFirst(nodeVisitorWithTypeTracking, roots); } From d87ea6d45e36f90649b57d609ae7c89597cd6461 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sat, 14 May 2022 18:19:00 +1000 Subject: [PATCH 11/13] Create empty versions of variables for convenience, rename getMap to toMap --- src/main/java/graphql/ExecutionInput.java | 4 +- .../java/graphql/analysis/QueryTraverser.java | 2 +- .../graphql/execution/CoercedVariables.java | 7 +- .../graphql/execution/ExecutionContext.java | 2 +- .../execution/ExecutionContextBuilder.java | 2 +- .../java/graphql/execution/RawVariables.java | 7 +- .../graphql/execution/ValuesResolver.java | 6 +- .../ExecutableNormalizedOperationFactory.java | 2 +- .../ExecutionContextBuilderTest.groovy | 2 +- .../execution/ValuesResolverTest.groovy | 18 ++--- ...tableNormalizedOperationFactoryTest.groovy | 76 +++++++++---------- src/test/java/benchmark/NQBenchmark1.java | 2 +- src/test/java/benchmark/NQBenchmark2.java | 4 +- 13 files changed, 72 insertions(+), 62 deletions(-) diff --git a/src/main/java/graphql/ExecutionInput.java b/src/main/java/graphql/ExecutionInput.java index 0d240929d9..3f8d1476b5 100644 --- a/src/main/java/graphql/ExecutionInput.java +++ b/src/main/java/graphql/ExecutionInput.java @@ -102,7 +102,7 @@ public Object getRoot() { */ @Deprecated public Map getVariables() { - return rawVariables.getMap(); + return rawVariables.toMap(); } /** @@ -218,7 +218,7 @@ public static class Builder { private Object context = graphQLContext; // we make these the same object on purpose - legacy code will get the same object if this change nothing private Object localContext; private Object root; - private RawVariables rawVariables = new RawVariables(Collections.emptyMap()); + private RawVariables rawVariables = RawVariables.emptyVariables(); public Map extensions = Collections.emptyMap(); // // this is important - it allows code to later known if we never really set a dataloader and hence it can optimize diff --git a/src/main/java/graphql/analysis/QueryTraverser.java b/src/main/java/graphql/analysis/QueryTraverser.java index be3e5bfac0..7b5ff7735e 100644 --- a/src/main/java/graphql/analysis/QueryTraverser.java +++ b/src/main/java/graphql/analysis/QueryTraverser.java @@ -183,7 +183,7 @@ private Object visitImpl(QueryVisitor visitFieldCallback, Boolean preOrder) { } NodeTraverser nodeTraverser = new NodeTraverser(rootVars, this::childrenOf); - NodeVisitorWithTypeTracking nodeVisitorWithTypeTracking = new NodeVisitorWithTypeTracking(preOrderCallback, postOrderCallback, coercedVariables.getMap(), schema, fragmentsByName); + NodeVisitorWithTypeTracking nodeVisitorWithTypeTracking = new NodeVisitorWithTypeTracking(preOrderCallback, postOrderCallback, coercedVariables.toMap(), schema, fragmentsByName); return nodeTraverser.depthFirst(nodeVisitorWithTypeTracking, roots); } diff --git a/src/main/java/graphql/execution/CoercedVariables.java b/src/main/java/graphql/execution/CoercedVariables.java index e484c435a2..47b403f7ff 100644 --- a/src/main/java/graphql/execution/CoercedVariables.java +++ b/src/main/java/graphql/execution/CoercedVariables.java @@ -3,6 +3,7 @@ import graphql.Internal; import graphql.collect.ImmutableMapWithNullValues; +import java.util.Collections; import java.util.Map; /** @@ -16,7 +17,7 @@ public CoercedVariables(Map coercedVariables) { this.coercedVariables = ImmutableMapWithNullValues.copyOf(coercedVariables); } - public Map getMap() { + public Map toMap() { return coercedVariables; } @@ -27,4 +28,8 @@ public boolean containsKey(String key) { public Object get(String key) { return coercedVariables.get(key); } + + public static CoercedVariables emptyVariables() { + return new CoercedVariables(Collections.emptyMap()); + } } diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index 372e97cf25..bf774165a3 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -120,7 +120,7 @@ public OperationDefinition getOperationDefinition() { */ @Deprecated public Map getVariables() { - return coercedVariables.getMap(); + return coercedVariables.toMap(); } public CoercedVariables getCoercedVariables() { diff --git a/src/main/java/graphql/execution/ExecutionContextBuilder.java b/src/main/java/graphql/execution/ExecutionContextBuilder.java index 1ac67a032c..7df56db553 100644 --- a/src/main/java/graphql/execution/ExecutionContextBuilder.java +++ b/src/main/java/graphql/execution/ExecutionContextBuilder.java @@ -39,7 +39,7 @@ public class ExecutionContextBuilder { Object root; Document document; OperationDefinition operationDefinition; - CoercedVariables coercedVariables = new CoercedVariables(Collections.emptyMap()); + CoercedVariables coercedVariables = CoercedVariables.emptyVariables(); ImmutableMap fragmentsByName = ImmutableKit.emptyMap(); DataLoaderRegistry dataLoaderRegistry; CacheControl cacheControl; diff --git a/src/main/java/graphql/execution/RawVariables.java b/src/main/java/graphql/execution/RawVariables.java index b8a987a6c5..ad4d04db41 100644 --- a/src/main/java/graphql/execution/RawVariables.java +++ b/src/main/java/graphql/execution/RawVariables.java @@ -3,6 +3,7 @@ import graphql.Internal; import graphql.collect.ImmutableMapWithNullValues; +import java.util.Collections; import java.util.Map; /** @@ -16,7 +17,7 @@ public RawVariables(Map rawVariables) { this.rawVariables = ImmutableMapWithNullValues.copyOf(rawVariables); } - public Map getMap() { + public Map toMap() { return rawVariables; } @@ -27,4 +28,8 @@ public boolean containsKey(String key) { public Object get(String key) { return rawVariables.get(key); } + + public static RawVariables emptyVariables() { + return new RawVariables(Collections.emptyMap()); + } } diff --git a/src/main/java/graphql/execution/ValuesResolver.java b/src/main/java/graphql/execution/ValuesResolver.java index 835b4cbe79..fc601ed55f 100644 --- a/src/main/java/graphql/execution/ValuesResolver.java +++ b/src/main/java/graphql/execution/ValuesResolver.java @@ -397,7 +397,7 @@ private CoercedVariables externalValueToInternalValueForVariables(GraphQLSchema boolean hasValue = rawVariables.containsKey(variableName); Object value = rawVariables.get(variableName); if (!hasValue && defaultValue != null) { - Object coercedDefaultValue = literalToInternalValue(fieldVisibility, variableType, defaultValue, new CoercedVariables(Collections.emptyMap())); + Object coercedDefaultValue = literalToInternalValue(fieldVisibility, variableType, defaultValue, CoercedVariables.emptyVariables()); coercedValues.put(variableName, coercedDefaultValue); } else if (isNonNull(variableType) && (!hasValue || value == null)) { throw new NonNullableValueCoercedAsNullException(variableDefinition, variableType); @@ -706,7 +706,7 @@ public Object literalToInternalValue(GraphqlFieldVisibility fieldVisibility, */ private Object literalToInternalValueForScalar(Value inputValue, GraphQLScalarType scalarType, CoercedVariables coercedVariables) { // the CoercingParseLiteralException exception that could happen here has been validated earlier via ValidationUtil - return scalarType.getCoercing().parseLiteral(inputValue, coercedVariables.getMap()); + return scalarType.getCoercing().parseLiteral(inputValue, coercedVariables.toMap()); } /** @@ -808,7 +808,7 @@ private Object defaultValueToInternalValue(GraphqlFieldVisibility fieldVisibilit } if (defaultValue.isLiteral()) { // default value literals can't reference variables, this is why the variables are empty - return literalToInternalValue(fieldVisibility, type, (Value) defaultValue.getValue(), new CoercedVariables(Collections.emptyMap())); + return literalToInternalValue(fieldVisibility, type, (Value) defaultValue.getValue(), CoercedVariables.emptyVariables()); } if (defaultValue.isExternal()) { // performs validation too diff --git a/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java b/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java index 485da4d844..cce09956f5 100644 --- a/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java +++ b/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java @@ -106,7 +106,7 @@ private ExecutableNormalizedOperation createNormalizedQueryImpl(GraphQLSchema gr .newParameters() .fragments(fragments) .schema(graphQLSchema) - .coercedVariables(coercedVariableValues.getMap()) + .coercedVariables(coercedVariableValues.toMap()) .normalizedVariables(normalizedVariableValues) .build(); diff --git a/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy b/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy index cee709437b..047e794ef7 100644 --- a/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy +++ b/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy @@ -105,7 +105,7 @@ class ExecutionContextBuilderTest extends Specification { def "transform works and copies values with coerced variables"() { given: - def oldCoercedVariables = new CoercedVariables(Collections.emptyMap()) + def oldCoercedVariables = CoercedVariables.emptyVariables() def executionContextOld = new ExecutionContextBuilder() .instrumentation(instrumentation) .queryStrategy(queryStrategy) diff --git a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy index 03b6a8616b..570e88741d 100644 --- a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy +++ b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy @@ -175,7 +175,7 @@ class ValuesResolverTest extends Specification { def argument = new Argument("arg", new VariableReference("var")) when: - def variables = new CoercedVariables(Collections.emptyMap()) + def variables = CoercedVariables.emptyVariables() def values = resolver.getArgumentValues([fieldArgument], [argument], variables) then: @@ -206,7 +206,7 @@ class ValuesResolverTest extends Specification { when: def argument = new Argument("arg", inputValue) - def values = resolver.getArgumentValues([fieldArgument], [argument], new CoercedVariables(Collections.emptyMap())) + def values = resolver.getArgumentValues([fieldArgument], [argument], CoercedVariables.emptyVariables()) then: values['arg'] == outputValue @@ -254,7 +254,7 @@ class ValuesResolverTest extends Specification { when: def argument = new Argument("arg", inputValue) - def values = resolver.getArgumentValues([fieldArgument], [argument], new CoercedVariables(Collections.emptyMap())) + def values = resolver.getArgumentValues([fieldArgument], [argument], CoercedVariables.emptyVariables()) then: values['arg'] == outputValue @@ -302,7 +302,7 @@ class ValuesResolverTest extends Specification { def fieldArgument1 = newArgument().name("arg1").type(enumType).build() def fieldArgument2 = newArgument().name("arg2").type(enumType).build() when: - def values = resolver.getArgumentValues([fieldArgument1, fieldArgument2], [argument1, argument2], new CoercedVariables(Collections.emptyMap())) + def values = resolver.getArgumentValues([fieldArgument1, fieldArgument2], [argument1, argument2], CoercedVariables.emptyVariables()) then: values['arg1'] == 'PLUTO' @@ -319,7 +319,7 @@ class ValuesResolverTest extends Specification { def fieldArgument = newArgument().name("arg").type(list(GraphQLBoolean)).build() when: - def values = resolver.getArgumentValues([fieldArgument], [argument], new CoercedVariables(Collections.emptyMap())) + def values = resolver.getArgumentValues([fieldArgument], [argument], CoercedVariables.emptyVariables()) then: values['arg'] == [true, false] @@ -333,7 +333,7 @@ class ValuesResolverTest extends Specification { def fieldArgument = newArgument().name("arg").type(list(GraphQLString)).build() when: - def values = resolver.getArgumentValues([fieldArgument], [argument], new CoercedVariables(Collections.emptyMap())) + def values = resolver.getArgumentValues([fieldArgument], [argument], CoercedVariables.emptyVariables()) then: values['arg'] == ['world'] @@ -428,7 +428,7 @@ class ValuesResolverTest extends Specification { def resolvedValues = resolver.coerceVariableValues(schema, [fooVarDef, barVarDef], new RawVariables(InputValue)) then: - resolvedValues.getMap() == outputValue + resolvedValues.toMap() == outputValue where: InputValue || outputValue @@ -443,7 +443,7 @@ class ValuesResolverTest extends Specification { VariableDefinition fooVarDef = new VariableDefinition("foo", new NonNullType(new TypeName("String"))) when: - resolver.coerceVariableValues(schema, [fooVarDef], new RawVariables(Collections.emptyMap())) + resolver.coerceVariableValues(schema, [fooVarDef], RawVariables.emptyVariables()) then: thrown(GraphQLException) @@ -516,7 +516,7 @@ class ValuesResolverTest extends Specification { def argument = new Argument("arg", NullValue.newNullValue().build()) when: - def variables = new CoercedVariables(Collections.emptyMap()) + def variables = CoercedVariables.emptyVariables() def values = resolver.getArgumentValues([fieldArgument], [argument], variables) then: diff --git a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy index 96ba4e660c..3199148d1e 100644 --- a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy +++ b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy @@ -109,7 +109,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -195,7 +195,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -275,7 +275,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -326,7 +326,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTree(tree) expect: @@ -369,7 +369,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTree(tree) expect: @@ -419,7 +419,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTree(tree) expect: @@ -482,7 +482,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -528,7 +528,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTree(tree) expect: @@ -572,7 +572,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTree(tree) expect: @@ -616,7 +616,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTree(tree) expect: @@ -648,7 +648,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTree(tree) expect: @@ -699,7 +699,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -737,7 +737,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTree(tree) expect: @@ -781,7 +781,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) def dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -821,7 +821,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) def dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -869,7 +869,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) def dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -938,7 +938,7 @@ type Dog implements Animal{ def subFooField = (document.getDefinitions()[1] as FragmentDefinition).getSelectionSet().getSelections()[0] as Field ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def fieldToNormalizedField = tree.getFieldToNormalizedField() expect: @@ -981,7 +981,7 @@ type Dog implements Animal{ def idField = petsField.getSelectionSet().getSelections()[0] as Field ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def fieldToNormalizedField = tree.getFieldToNormalizedField() @@ -1030,7 +1030,7 @@ type Dog implements Animal{ def typeField = selections[3] as Field ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def fieldToNormalizedField = tree.getFieldToNormalizedField() expect: @@ -1087,7 +1087,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -1130,7 +1130,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTree(tree) expect: @@ -1158,7 +1158,7 @@ type Dog implements Animal{ Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def normalizedFieldToMergedField = tree.getNormalizedFieldToMergedField() Traverser traverser = Traverser.depthFirst({ it.getChildren() }) List result = new ArrayList<>() @@ -1198,7 +1198,7 @@ type Dog implements Animal{ ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def coordinatesToNormalizedFields = tree.coordinatesToNormalizedFields then: @@ -1297,7 +1297,7 @@ schema { Document document = TestUtil.parseQuery(mutation) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() - def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, new CoercedVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperation(graphQLSchema, document, null, CoercedVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) expect: @@ -1594,7 +1594,7 @@ schema { Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, RawVariables.emptyVariables()) then: tree.normalizedFieldToMergedField.size() == 3 @@ -1652,7 +1652,7 @@ schema { Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, RawVariables.emptyVariables()) println String.join("\n", printTree(tree)) /** @@ -1698,7 +1698,7 @@ schema { Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, RawVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, graphQLSchema) then: @@ -1769,7 +1769,7 @@ schema { Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, RawVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -1833,7 +1833,7 @@ schema { Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, RawVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -1890,7 +1890,7 @@ schema { Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, RawVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -1965,7 +1965,7 @@ schema { Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, RawVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2027,7 +2027,7 @@ schema { Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, RawVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2069,7 +2069,7 @@ schema { Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, RawVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2112,7 +2112,7 @@ schema { Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, RawVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2155,7 +2155,7 @@ schema { Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, RawVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2230,7 +2230,7 @@ schema { Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, RawVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2306,7 +2306,7 @@ schema { Document document = TestUtil.parseQuery(query) ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory() when: - def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, new RawVariables(Collections.emptyMap())) + def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, RawVariables.emptyVariables()) def printedTree = printTreeWithLevelInfo(tree, schema) then: @@ -2396,7 +2396,7 @@ schema { assertValidQuery(graphQLSchema, query) Document document = TestUtil.parseQuery(query) when: - def tree = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, new RawVariables(Collections.emptyMap())) + def tree = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables(graphQLSchema, document, null, RawVariables.emptyVariables()) println String.join("\n", printTree(tree)) def printedTree = printTree(tree) diff --git a/src/test/java/benchmark/NQBenchmark1.java b/src/test/java/benchmark/NQBenchmark1.java index 0ba79cac10..f96dd27447 100644 --- a/src/test/java/benchmark/NQBenchmark1.java +++ b/src/test/java/benchmark/NQBenchmark1.java @@ -84,7 +84,7 @@ public void benchMarkThroughput(MyState myState, Blackhole blackhole ) { } private void runImpl(MyState myState, Blackhole blackhole) { - ExecutableNormalizedOperation executableNormalizedOperation = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(myState.schema, myState.document, null, new CoercedVariables(Collections.emptyMap())); + ExecutableNormalizedOperation executableNormalizedOperation = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(myState.schema, myState.document, null, CoercedVariables.emptyVariables()); blackhole.consume(executableNormalizedOperation); } diff --git a/src/test/java/benchmark/NQBenchmark2.java b/src/test/java/benchmark/NQBenchmark2.java index ba5790efdb..931e1160f9 100644 --- a/src/test/java/benchmark/NQBenchmark2.java +++ b/src/test/java/benchmark/NQBenchmark2.java @@ -78,7 +78,7 @@ private String readFromClasspath(String file) throws IOException { @BenchmarkMode(Mode.AverageTime) @OutputTimeUnit(TimeUnit.MILLISECONDS) public ExecutableNormalizedOperation benchMarkAvgTime(MyState myState) throws ExecutionException, InterruptedException { - ExecutableNormalizedOperation executableNormalizedOperation = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(myState.schema, myState.document, null, new CoercedVariables(Collections.emptyMap())); + ExecutableNormalizedOperation executableNormalizedOperation = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(myState.schema, myState.document, null, CoercedVariables.emptyVariables()); // System.out.println("fields size:" + normalizedQuery.getFieldToNormalizedField().size()); return executableNormalizedOperation; } @@ -86,7 +86,7 @@ public ExecutableNormalizedOperation benchMarkAvgTime(MyState myState) throws Ex public static void main(String[] args) { MyState myState = new MyState(); myState.setup(); - ExecutableNormalizedOperation executableNormalizedOperation = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(myState.schema, myState.document, null, new CoercedVariables(Collections.emptyMap())); + ExecutableNormalizedOperation executableNormalizedOperation = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(myState.schema, myState.document, null, CoercedVariables.emptyVariables()); // System.out.println(printTree(normalizedQuery)); ImmutableListMultimap fieldToNormalizedField = executableNormalizedOperation.getFieldToNormalizedField(); System.out.println(fieldToNormalizedField.size()); From c0e10be4d1142192ec18d0aa592294623e913c1a Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 15 May 2022 09:51:36 +1000 Subject: [PATCH 12/13] Add tests where both map and boxed variables are set --- .../groovy/graphql/ExecutionInputTest.groovy | 76 +++++++++++++++++ .../ExecutionContextBuilderTest.groovy | 84 +++++++++++++++++++ 2 files changed, 160 insertions(+) diff --git a/src/test/groovy/graphql/ExecutionInputTest.groovy b/src/test/groovy/graphql/ExecutionInputTest.groovy index 94739e31e2..712d74a60e 100644 --- a/src/test/groovy/graphql/ExecutionInputTest.groovy +++ b/src/test/groovy/graphql/ExecutionInputTest.groovy @@ -66,6 +66,29 @@ class ExecutionInputTest extends Specification { executionInput.extensions == [some: "map"] } + def "build works, if both variables and rawVariables are set, the latest value set takes precedence"() { + when: + def executionInput = ExecutionInput.newExecutionInput().query(query) + .dataLoaderRegistry(registry) + .cacheControl(cacheControl) + .variables(variables) + .rawVariables(rawVariables) + .root(root) + .graphQLContext({ it.of(["a": "b"]) }) + .locale(Locale.GERMAN) + .extensions([some: "map"]) + .build() + then: + executionInput.rawVariables == rawVariables + executionInput.graphQLContext.get("a") == "b" + executionInput.root == root + executionInput.dataLoaderRegistry == registry + executionInput.cacheControl == cacheControl + executionInput.query == query + executionInput.locale == Locale.GERMAN + executionInput.extensions == [some: "map"] + } + def "map context build works"() { when: def executionInput = ExecutionInput.newExecutionInput().query(query) @@ -160,6 +183,59 @@ class ExecutionInputTest extends Specification { executionInput.query == "new query" } + def "transform works and sets raw variables"() { + when: + def executionInputOld = ExecutionInput.newExecutionInput().query(query) + .dataLoaderRegistry(registry) + .cacheControl(cacheControl) + .extensions([some: "map"]) + .root(root) + .graphQLContext({ it.of(["a": "b"]) }) + .locale(Locale.GERMAN) + .build() + def graphQLContext = executionInputOld.getGraphQLContext() + def executionInput = executionInputOld.transform({ bldg -> bldg + .query("new query") + .rawVariables(rawVariables) }) + + then: + executionInput.graphQLContext == graphQLContext + executionInput.root == root + executionInput.rawVariables == rawVariables + executionInput.dataLoaderRegistry == registry + executionInput.cacheControl == cacheControl + executionInput.locale == Locale.GERMAN + executionInput.extensions == [some: "map"] + executionInput.query == "new query" + } + + def "transform works and sets values, if both variables and rawVariables are set, latest value set takes precedence"() { + when: + def executionInputOld = ExecutionInput.newExecutionInput().query(query) + .dataLoaderRegistry(registry) + .cacheControl(cacheControl) + .extensions([some: "map"]) + .root(root) + .graphQLContext({ it.of(["a": "b"]) }) + .locale(Locale.GERMAN) + .build() + def graphQLContext = executionInputOld.getGraphQLContext() + def executionInput = executionInputOld.transform({ bldg -> bldg + .query("new query") + .variables(variables) + .rawVariables(rawVariables) }) + + then: + executionInput.graphQLContext == graphQLContext + executionInput.root == root + executionInput.rawVariables == rawVariables + executionInput.dataLoaderRegistry == registry + executionInput.cacheControl == cacheControl + executionInput.locale == Locale.GERMAN + executionInput.extensions == [some: "map"] + executionInput.query == "new query" + } + def "defaults query into builder as expected"() { when: def executionInput = ExecutionInput.newExecutionInput("{ q }").build() diff --git a/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy b/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy index 047e794ef7..747160bd2d 100644 --- a/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy +++ b/src/test/groovy/graphql/execution/ExecutionContextBuilderTest.groovy @@ -103,6 +103,46 @@ class ExecutionContextBuilderTest extends Specification { executionContext.cacheControl == cacheControl } + def "builds the correct ExecutionContext, if both variables and coercedVariables are set, latest value set takes precedence"() { + given: + def coercedVariables = new CoercedVariables([var: 'value']) + + when: + def executionContext = new ExecutionContextBuilder() + .instrumentation(instrumentation) + .queryStrategy(queryStrategy) + .mutationStrategy(mutationStrategy) + .subscriptionStrategy(subscriptionStrategy) + .graphQLSchema(schema) + .executionId(executionId) + .context(context) + .graphQLContext(graphQLContext) + .root(root) + .operationDefinition(operation) + .fragmentsByName([MyFragment: fragment]) + .variables([var: 'value']) + .coercedVariables(coercedVariables) + .dataLoaderRegistry(dataLoaderRegistry) + .cacheControl(cacheControl) + .build() + + then: + executionContext.executionId == executionId + executionContext.instrumentation == instrumentation + executionContext.graphQLSchema == schema + executionContext.queryStrategy == queryStrategy + executionContext.mutationStrategy == mutationStrategy + executionContext.subscriptionStrategy == subscriptionStrategy + executionContext.root == root + executionContext.context == context + executionContext.graphQLContext == graphQLContext + executionContext.coercedVariables == coercedVariables + executionContext.getFragmentsByName() == [MyFragment: fragment] + executionContext.operationDefinition == operation + executionContext.dataLoaderRegistry == dataLoaderRegistry + executionContext.cacheControl == cacheControl + } + def "transform works and copies values with coerced variables"() { given: def oldCoercedVariables = CoercedVariables.emptyVariables() @@ -144,4 +184,48 @@ class ExecutionContextBuilderTest extends Specification { executionContext.dataLoaderRegistry == dataLoaderRegistry executionContext.cacheControl == cacheControl } + + def "transform copies values, if both variables and coercedVariables set, latest value set takes precedence"() { + given: + def oldCoercedVariables = CoercedVariables.emptyVariables() + def executionContextOld = new ExecutionContextBuilder() + .instrumentation(instrumentation) + .queryStrategy(queryStrategy) + .mutationStrategy(mutationStrategy) + .subscriptionStrategy(subscriptionStrategy) + .graphQLSchema(schema) + .executionId(executionId) + .context(context) + .graphQLContext(graphQLContext) + .root(root) + .operationDefinition(operation) + .variables([:]) + .coercedVariables(oldCoercedVariables) + .fragmentsByName([MyFragment: fragment]) + .dataLoaderRegistry(dataLoaderRegistry) + .cacheControl(cacheControl) + .build() + + when: + def coercedVariables = new CoercedVariables([var: 'value']) + def executionContext = executionContextOld.transform(builder -> builder + .variables([var: 'value']) + .coercedVariables(coercedVariables)) + + then: + executionContext.executionId == executionId + executionContext.instrumentation == instrumentation + executionContext.graphQLSchema == schema + executionContext.queryStrategy == queryStrategy + executionContext.mutationStrategy == mutationStrategy + executionContext.subscriptionStrategy == subscriptionStrategy + executionContext.root == root + executionContext.context == context + executionContext.graphQLContext == graphQLContext + executionContext.coercedVariables == coercedVariables + executionContext.getFragmentsByName() == [MyFragment: fragment] + executionContext.operationDefinition == operation + executionContext.dataLoaderRegistry == dataLoaderRegistry + executionContext.cacheControl == cacheControl + } } From 7bf2bd5611043967efa2796ac989975cb18e26bf Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Tue, 24 May 2022 16:27:19 +1000 Subject: [PATCH 13/13] Revert to one variable builder for ExecutionInput and tidy up --- src/main/java/graphql/ExecutionInput.java | 15 +-- .../graphql/execution/CoercedVariables.java | 4 +- .../execution/ExecutionContextBuilder.java | 1 - .../java/graphql/execution/RawVariables.java | 4 +- .../groovy/graphql/ExecutionInputTest.groovy | 105 +----------------- src/test/groovy/graphql/GraphQLTest.groovy | 13 --- 6 files changed, 11 insertions(+), 131 deletions(-) diff --git a/src/main/java/graphql/ExecutionInput.java b/src/main/java/graphql/ExecutionInput.java index 3f8d1476b5..eb81404e92 100644 --- a/src/main/java/graphql/ExecutionInput.java +++ b/src/main/java/graphql/ExecutionInput.java @@ -98,9 +98,8 @@ public Object getRoot() { } /** - * @return a map of raw variables that can be referenced via $syntax in the query. Retaining for backwards compatibility + * @return a map of raw variables that can be referenced via $syntax in the query. */ - @Deprecated public Map getVariables() { return rawVariables.toMap(); } @@ -167,7 +166,7 @@ public ExecutionInput transform(Consumer builderConsumer) { .root(this.root) .dataLoaderRegistry(this.dataLoaderRegistry) .cacheControl(this.cacheControl) - .rawVariables(this.rawVariables) + .variables(this.rawVariables.toMap()) .extensions(this.extensions) .executionId(this.executionId) .locale(this.locale); @@ -360,26 +359,18 @@ public Builder root(Object root) { } /** - * The legacy variables builder + * Adds raw (not coerced) variables * * @param rawVariables the map of raw variables * * @return this builder - * - * @deprecated - use {@link RawVariables} to hold raw variables */ - @Deprecated public Builder variables(Map rawVariables) { assertNotNull(rawVariables, () -> "variables map can't be null"); this.rawVariables = new RawVariables(rawVariables); return this; } - public Builder rawVariables(RawVariables rawVariables) { - this.rawVariables = assertNotNull(rawVariables, () -> "raw variables map can't be null"); - return this; - } - public Builder extensions(Map extensions) { this.extensions = assertNotNull(extensions, () -> "extensions map can't be null"); return this; diff --git a/src/main/java/graphql/execution/CoercedVariables.java b/src/main/java/graphql/execution/CoercedVariables.java index 47b403f7ff..25aad43b27 100644 --- a/src/main/java/graphql/execution/CoercedVariables.java +++ b/src/main/java/graphql/execution/CoercedVariables.java @@ -1,9 +1,9 @@ package graphql.execution; import graphql.Internal; +import graphql.collect.ImmutableKit; import graphql.collect.ImmutableMapWithNullValues; -import java.util.Collections; import java.util.Map; /** @@ -30,6 +30,6 @@ public Object get(String key) { } public static CoercedVariables emptyVariables() { - return new CoercedVariables(Collections.emptyMap()); + return new CoercedVariables(ImmutableKit.emptyMap()); } } diff --git a/src/main/java/graphql/execution/ExecutionContextBuilder.java b/src/main/java/graphql/execution/ExecutionContextBuilder.java index 7df56db553..4f5dac8ac8 100644 --- a/src/main/java/graphql/execution/ExecutionContextBuilder.java +++ b/src/main/java/graphql/execution/ExecutionContextBuilder.java @@ -17,7 +17,6 @@ import graphql.schema.GraphQLSchema; import org.dataloader.DataLoaderRegistry; -import java.util.Collections; import java.util.Locale; import java.util.Map; diff --git a/src/main/java/graphql/execution/RawVariables.java b/src/main/java/graphql/execution/RawVariables.java index ad4d04db41..fca5d1014d 100644 --- a/src/main/java/graphql/execution/RawVariables.java +++ b/src/main/java/graphql/execution/RawVariables.java @@ -1,9 +1,9 @@ package graphql.execution; import graphql.Internal; +import graphql.collect.ImmutableKit; import graphql.collect.ImmutableMapWithNullValues; -import java.util.Collections; import java.util.Map; /** @@ -30,6 +30,6 @@ public Object get(String key) { } public static RawVariables emptyVariables() { - return new RawVariables(Collections.emptyMap()); + return new RawVariables(ImmutableKit.emptyMap()); } } diff --git a/src/test/groovy/graphql/ExecutionInputTest.groovy b/src/test/groovy/graphql/ExecutionInputTest.groovy index 712d74a60e..9ab6a8930a 100644 --- a/src/test/groovy/graphql/ExecutionInputTest.groovy +++ b/src/test/groovy/graphql/ExecutionInputTest.groovy @@ -18,7 +18,6 @@ class ExecutionInputTest extends Specification { def root = "root" def context = "context" def variables = [key: "value"] - def rawVariables = new RawVariables(variables) def "build works"() { when: @@ -37,51 +36,7 @@ class ExecutionInputTest extends Specification { executionInput.graphQLContext.get("a") == "b" executionInput.root == root executionInput.variables == variables - executionInput.dataLoaderRegistry == registry - executionInput.cacheControl == cacheControl - executionInput.query == query - executionInput.locale == Locale.GERMAN - executionInput.extensions == [some: "map"] - } - - def "build works with raw variables"() { - when: - def executionInput = ExecutionInput.newExecutionInput().query(query) - .dataLoaderRegistry(registry) - .cacheControl(cacheControl) - .rawVariables(rawVariables) - .root(root) - .graphQLContext({ it.of(["a": "b"]) }) - .locale(Locale.GERMAN) - .extensions([some: "map"]) - .build() - then: - executionInput.graphQLContext.get("a") == "b" - executionInput.root == root - executionInput.rawVariables == rawVariables - executionInput.dataLoaderRegistry == registry - executionInput.cacheControl == cacheControl - executionInput.query == query - executionInput.locale == Locale.GERMAN - executionInput.extensions == [some: "map"] - } - - def "build works, if both variables and rawVariables are set, the latest value set takes precedence"() { - when: - def executionInput = ExecutionInput.newExecutionInput().query(query) - .dataLoaderRegistry(registry) - .cacheControl(cacheControl) - .variables(variables) - .rawVariables(rawVariables) - .root(root) - .graphQLContext({ it.of(["a": "b"]) }) - .locale(Locale.GERMAN) - .extensions([some: "map"]) - .build() - then: - executionInput.rawVariables == rawVariables - executionInput.graphQLContext.get("a") == "b" - executionInput.root == root + executionInput.rawVariables.toMap() == variables executionInput.dataLoaderRegistry == registry executionInput.cacheControl == cacheControl executionInput.query == query @@ -158,58 +113,7 @@ class ExecutionInputTest extends Specification { executionInput.query == "new query" } - def "transform works and copies values with raw variables"() { - when: - def executionInputOld = ExecutionInput.newExecutionInput().query(query) - .dataLoaderRegistry(registry) - .cacheControl(cacheControl) - .rawVariables(rawVariables) - .extensions([some: "map"]) - .root(root) - .graphQLContext({ it.of(["a": "b"]) }) - .locale(Locale.GERMAN) - .build() - def graphQLContext = executionInputOld.getGraphQLContext() - def executionInput = executionInputOld.transform({ bldg -> bldg.query("new query") }) - - then: - executionInput.graphQLContext == graphQLContext - executionInput.root == root - executionInput.rawVariables == rawVariables - executionInput.dataLoaderRegistry == registry - executionInput.cacheControl == cacheControl - executionInput.locale == Locale.GERMAN - executionInput.extensions == [some: "map"] - executionInput.query == "new query" - } - - def "transform works and sets raw variables"() { - when: - def executionInputOld = ExecutionInput.newExecutionInput().query(query) - .dataLoaderRegistry(registry) - .cacheControl(cacheControl) - .extensions([some: "map"]) - .root(root) - .graphQLContext({ it.of(["a": "b"]) }) - .locale(Locale.GERMAN) - .build() - def graphQLContext = executionInputOld.getGraphQLContext() - def executionInput = executionInputOld.transform({ bldg -> bldg - .query("new query") - .rawVariables(rawVariables) }) - - then: - executionInput.graphQLContext == graphQLContext - executionInput.root == root - executionInput.rawVariables == rawVariables - executionInput.dataLoaderRegistry == registry - executionInput.cacheControl == cacheControl - executionInput.locale == Locale.GERMAN - executionInput.extensions == [some: "map"] - executionInput.query == "new query" - } - - def "transform works and sets values, if both variables and rawVariables are set, latest value set takes precedence"() { + def "transform works and sets variables"() { when: def executionInputOld = ExecutionInput.newExecutionInput().query(query) .dataLoaderRegistry(registry) @@ -222,13 +126,12 @@ class ExecutionInputTest extends Specification { def graphQLContext = executionInputOld.getGraphQLContext() def executionInput = executionInputOld.transform({ bldg -> bldg .query("new query") - .variables(variables) - .rawVariables(rawVariables) }) + .variables(variables) }) then: executionInput.graphQLContext == graphQLContext executionInput.root == root - executionInput.rawVariables == rawVariables + executionInput.rawVariables.toMap() == variables executionInput.dataLoaderRegistry == registry executionInput.cacheControl == cacheControl executionInput.locale == Locale.GERMAN diff --git a/src/test/groovy/graphql/GraphQLTest.groovy b/src/test/groovy/graphql/GraphQLTest.groovy index dc831bfc75..caa4fe9d10 100644 --- a/src/test/groovy/graphql/GraphQLTest.groovy +++ b/src/test/groovy/graphql/GraphQLTest.groovy @@ -975,17 +975,6 @@ many lines'''] assEx.message.contains("variables map can't be null") } - def "raw variables map can't be null via ExecutionInput"() { - given: - - when: - def input = newExecutionInput().query('query($var:String){ hello(arg: $var) }').rawVariables(null).build() - - then: - def assEx = thrown(AssertException) - assEx.message.contains("raw variables map can't be null") - } - def "query can't be null via ExecutionInput"() { given: @@ -995,8 +984,6 @@ many lines'''] then: def assEx = thrown(AssertException) assEx.message.contains("query can't be null") - - } def "query must be set via ExecutionInput"() {