-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Raw and Coerced Variables refactor #2802
Changes from all commits
1f7f58f
5ff6353
2614e4b
0c51d2e
c98a615
409c6bb
7c38af4
86af871
f1fe14a
61198b5
6240769
88a69c2
d87ea6d
c0e10be
bdd907f
7bf2bd5
84ae2d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
package graphql.analysis; | ||
|
||
import graphql.PublicApi; | ||
import graphql.execution.CoercedVariables; | ||
import graphql.execution.RawVariables; | ||
import graphql.execution.ValuesResolver; | ||
import graphql.language.Document; | ||
import graphql.language.FragmentDefinition; | ||
|
@@ -42,26 +44,26 @@ public class QueryTraverser { | |
private final Collection<? extends Node> roots; | ||
private final GraphQLSchema schema; | ||
private final Map<String, FragmentDefinition> fragmentsByName; | ||
private final Map<String, Object> variables; | ||
private CoercedVariables coercedVariables; | ||
|
||
private final GraphQLCompositeType rootParentType; | ||
|
||
private QueryTraverser(GraphQLSchema schema, | ||
Document document, | ||
String operation, | ||
Map<String, Object> variables) { | ||
assertNotNull(document, () -> "document can't be null"); | ||
assertNotNull(document, () -> "document can't be null"); | ||
NodeUtil.GetOperationResult getOperationResult = NodeUtil.getOperation(document, operation); | ||
List<VariableDefinition> 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now, keep functionality similar to before In the next PR (#2773), |
||
} | ||
|
||
private Map<String, Object> coerceVariables(Map<String, Object> rawVariables, List<VariableDefinition> variableDefinitions) { | ||
return new ValuesResolver().coerceVariableValues(schema, variableDefinitions, rawVariables); | ||
private CoercedVariables coerceVariables(Map<String, Object> rawVariables, List<VariableDefinition> variableDefinitions) { | ||
return new ValuesResolver().coerceVariableValues(schema, variableDefinitions, new RawVariables(rawVariables)); | ||
} | ||
|
||
private QueryTraverser(GraphQLSchema schema, | ||
|
@@ -70,11 +72,11 @@ private QueryTraverser(GraphQLSchema schema, | |
Map<String, FragmentDefinition> fragmentsByName, | ||
Map<String, Object> 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) { | ||
|
@@ -181,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.toMap(), schema, fragmentsByName); | ||
return nodeTraverser.depthFirst(nodeVisitorWithTypeTracking, roots); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
package graphql.execution; | ||
|
||
import graphql.Internal; | ||
import graphql.collect.ImmutableKit; | ||
import graphql.collect.ImmutableMapWithNullValues; | ||
|
||
import java.util.Map; | ||
|
||
/** | ||
* Holds coerced variables | ||
*/ | ||
@Internal | ||
public class CoercedVariables { | ||
private final ImmutableMapWithNullValues<String, Object> coercedVariables; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using this special map as variable values can be null |
||
|
||
public CoercedVariables(Map<String, Object> coercedVariables) { | ||
this.coercedVariables = ImmutableMapWithNullValues.copyOf(coercedVariables); | ||
} | ||
|
||
public Map<String, Object> toMap() { | ||
return coercedVariables; | ||
} | ||
|
||
public boolean containsKey(String key) { | ||
return coercedVariables.containsKey(key); | ||
} | ||
|
||
public Object get(String key) { | ||
return coercedVariables.get(key); | ||
} | ||
|
||
public static CoercedVariables emptyVariables() { | ||
return new CoercedVariables(ImmutableKit.emptyMap()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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<String, FragmentDefinition> fragmentsByName; | ||
private final OperationDefinition operationDefinition; | ||
private final Document document; | ||
private final ImmutableMapWithNullValues<String, Object> variables; | ||
private final CoercedVariables coercedVariables; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
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)); | ||
} | ||
|
||
|
||
|
@@ -116,8 +115,16 @@ public OperationDefinition getOperationDefinition() { | |
return operationDefinition; | ||
} | ||
|
||
/** | ||
* @deprecated use {@link #getCoercedVariables()} instead | ||
*/ | ||
@Deprecated | ||
public Map<String, Object> getVariables() { | ||
return variables; | ||
return coercedVariables.toMap(); | ||
} | ||
|
||
public CoercedVariables getCoercedVariables() { | ||
return coercedVariables; | ||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variables in
ExecutionInput
are always raw