Skip to content
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

Bugfix cherry pick: RawVariables and CoercedVariables are public API (#2868) #2870

Merged
merged 1 commit into from Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/graphql/ExecutionInput.java
Expand Up @@ -367,7 +367,7 @@ public Builder root(Object root) {
*/
public Builder variables(Map<String, Object> rawVariables) {
assertNotNull(rawVariables, () -> "variables map can't be null");
this.rawVariables = new RawVariables(rawVariables);
this.rawVariables = RawVariables.of(rawVariables);
return this;
}

Expand Down
Expand Up @@ -155,7 +155,7 @@ public TraversalControl visitField(Field field, TraverserContext<Node> context)
boolean isTypeNameIntrospectionField = fieldDefinition == schema.getIntrospectionTypenameFieldDefinition();
GraphQLFieldsContainer fieldsContainer = !isTypeNameIntrospectionField ? (GraphQLFieldsContainer) unwrapAll(parentEnv.getOutputType()) : null;
GraphQLCodeRegistry codeRegistry = schema.getCodeRegistry();
Map<String, Object> argumentValues = valuesResolver.getArgumentValues(codeRegistry, fieldDefinition.getArguments(), field.getArguments(), new CoercedVariables(variables));
Map<String, Object> argumentValues = valuesResolver.getArgumentValues(codeRegistry, fieldDefinition.getArguments(), field.getArguments(), CoercedVariables.of(variables));
QueryVisitorFieldEnvironment environment = new QueryVisitorFieldEnvironmentImpl(isTypeNameIntrospectionField,
field,
fieldDefinition,
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/graphql/analysis/QueryTraverser.java
Expand Up @@ -257,7 +257,7 @@ public Builder document(Document document) {
*/
public Builder variables(Map<String, Object> variables) {
assertNotNull(variables, () -> "variables can't be null");
this.rawVariables = new RawVariables(variables);
this.rawVariables = RawVariables.of(variables);
return this;
}

Expand Down Expand Up @@ -326,7 +326,7 @@ public QueryTraverser build() {
// When traversing with an arbitrary root, there is no variable definition context available
// Thus, the variables must have already been coerced
// Retaining this builder for backwards compatibility
return new QueryTraverser(schema, root, rootParentType, fragmentsByName, new CoercedVariables(rawVariables.toMap()));
return new QueryTraverser(schema, root, rootParentType, fragmentsByName, CoercedVariables.of(rawVariables.toMap()));
}
return new QueryTraverser(schema, root, rootParentType, fragmentsByName, coercedVariables);
}
Expand Down
10 changes: 7 additions & 3 deletions src/main/java/graphql/execution/CoercedVariables.java
@@ -1,15 +1,15 @@
package graphql.execution;

import graphql.Internal;
import graphql.PublicApi;
import graphql.collect.ImmutableKit;
import graphql.collect.ImmutableMapWithNullValues;

import java.util.Map;

/**
* Holds coerced variables
* Holds coerced variables, that is their values are now in a canonical form.
*/
@Internal
@PublicApi
public class CoercedVariables {
private final ImmutableMapWithNullValues<String, Object> coercedVariables;

Expand All @@ -32,4 +32,8 @@ public Object get(String key) {
public static CoercedVariables emptyVariables() {
return new CoercedVariables(ImmutableKit.emptyMap());
}

public static CoercedVariables of(Map<String, Object> coercedVariables) {
return new CoercedVariables(coercedVariables);
}
}
2 changes: 1 addition & 1 deletion src/main/java/graphql/execution/ConditionalNodes.java
Expand Up @@ -34,7 +34,7 @@ public boolean shouldInclude(Map<String, Object> variables, List<Directive> dire
private boolean getDirectiveResult(Map<String, Object> variables, List<Directive> directives, String directiveName, boolean defaultValue) {
Directive foundDirective = NodeUtil.findNodeByName(directives, directiveName);
if (foundDirective != null) {
Map<String, Object> argumentValues = valuesResolver.getArgumentValues(SkipDirective.getArguments(), foundDirective.getArguments(), new CoercedVariables(variables));
Map<String, Object> argumentValues = valuesResolver.getArgumentValues(SkipDirective.getArguments(), foundDirective.getArguments(), CoercedVariables.of(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;
Expand Down
Expand Up @@ -155,7 +155,7 @@ public ExecutionContextBuilder root(Object root) {
*/
@Deprecated
public ExecutionContextBuilder variables(Map<String, Object> variables) {
this.coercedVariables = new CoercedVariables(variables);
this.coercedVariables = CoercedVariables.of(variables);
return this;
}

Expand Down
12 changes: 8 additions & 4 deletions src/main/java/graphql/execution/RawVariables.java
@@ -1,15 +1,15 @@
package graphql.execution;

import graphql.Internal;
import graphql.PublicApi;
import graphql.collect.ImmutableKit;
import graphql.collect.ImmutableMapWithNullValues;

import java.util.Map;

/**
* Holds raw variables, which not have been coerced yet
* Holds raw variables, which have not been coerced yet into {@link CoercedVariables}
*/
@Internal
@PublicApi
public class RawVariables {
private final ImmutableMapWithNullValues<String, Object> rawVariables;

Expand All @@ -30,6 +30,10 @@ public Object get(String key) {
}

public static RawVariables emptyVariables() {
return new RawVariables(ImmutableKit.emptyMap());
return RawVariables.of(ImmutableKit.emptyMap());
}

public static RawVariables of(Map<String, Object> rawVariables) {
return new RawVariables(rawVariables);
}
}
4 changes: 2 additions & 2 deletions src/main/java/graphql/execution/ValuesResolver.java
Expand Up @@ -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(), new CoercedVariables(emptyMap()));
return new ValuesResolver().literalToInternalValue(fieldVisibility, type, (Value<?>) inputValueWithState.getValue(), CoercedVariables.emptyVariables());
}
if (inputValueWithState.isExternal()) {
return new ValuesResolver().externalValueToInternalValue(fieldVisibility, type, inputValueWithState.getValue());
Expand Down Expand Up @@ -421,7 +421,7 @@ private CoercedVariables externalValueToInternalValueForVariables(GraphQLSchema
}
}

return new CoercedVariables(coercedValues);
return CoercedVariables.of(coercedValues);
}


Expand Down
Expand Up @@ -39,7 +39,7 @@ public Map<String, GraphQLDirective> resolveDirectives(List<Directive> directive
}

private void buildArguments(GraphQLDirective.Builder directiveBuilder, GraphQLCodeRegistry codeRegistry, GraphQLDirective protoType, Directive fieldDirective, Map<String, Object> variables) {
Map<String, Object> argumentValues = valuesResolver.getArgumentValues(codeRegistry, protoType.getArguments(), fieldDirective.getArguments(), new CoercedVariables(variables));
Map<String, Object> argumentValues = valuesResolver.getArgumentValues(codeRegistry, protoType.getArguments(), fieldDirective.getArguments(), CoercedVariables.of(variables));
directiveBuilder.clearArguments();
protoType.getArguments().forEach(protoArg -> {
if (argumentValues.containsKey(protoArg.getName())) {
Expand Down
Expand Up @@ -306,7 +306,7 @@ private ExecutableNormalizedField createNF(FieldCollectorNormalizedQueryParams p
String fieldName = field.getName();
GraphQLFieldDefinition fieldDefinition = Introspection.getFieldDef(parameters.getGraphQLSchema(), objectTypes.iterator().next(), fieldName);

Map<String, Object> argumentValues = valuesResolver.getArgumentValues(fieldDefinition.getArguments(), field.getArguments(), new CoercedVariables(parameters.getCoercedVariableValues()));
Map<String, Object> argumentValues = valuesResolver.getArgumentValues(fieldDefinition.getArguments(), field.getArguments(),CoercedVariables.of(parameters.getCoercedVariableValues()));
Map<String, NormalizedInputValue> normalizedArgumentValues = null;
if (parameters.getNormalizedVariableValues() != null) {
normalizedArgumentValues = valuesResolver.getNormalizedArgumentValues(fieldDefinition.getArguments(), field.getArguments(), parameters.getNormalizedVariableValues());
Expand Down
Expand Up @@ -66,7 +66,7 @@ class ExecutionContextBuilderTest extends Specification {

def "builds the correct ExecutionContext with coerced variables"() {
given:
def coercedVariables = new CoercedVariables([var: 'value'])
def coercedVariables = CoercedVariables.of([var: 'value'])

when:
def executionContext = new ExecutionContextBuilder()
Expand Down Expand Up @@ -105,7 +105,7 @@ class ExecutionContextBuilderTest extends Specification {

def "builds the correct ExecutionContext, if both variables and coercedVariables are set, latest value set takes precedence"() {
given:
def coercedVariables = new CoercedVariables([var: 'value'])
def coercedVariables = CoercedVariables.of([var: 'value'])

when:
def executionContext = new ExecutionContextBuilder()
Expand Down Expand Up @@ -164,7 +164,7 @@ class ExecutionContextBuilderTest extends Specification {
.build()

when:
def coercedVariables = new CoercedVariables([var: 'value'])
def coercedVariables = CoercedVariables.of([var: 'value'])
def executionContext = executionContextOld.transform(builder -> builder
.coercedVariables(coercedVariables))

Expand Down Expand Up @@ -207,7 +207,7 @@ class ExecutionContextBuilderTest extends Specification {
.build()

when:
def coercedVariables = new CoercedVariables([var: 'value'])
def coercedVariables = CoercedVariables.of([var: 'value'])
def executionContext = executionContextOld.transform(builder -> builder
.variables([var: 'value'])
.coercedVariables(coercedVariables))
Expand Down
32 changes: 16 additions & 16 deletions src/test/groovy/graphql/execution/ValuesResolverTest.groovy
Expand Up @@ -45,7 +45,7 @@ class ValuesResolverTest extends Specification {
def schema = TestUtil.schemaWithInputType(inputType)
VariableDefinition variableDefinition = new VariableDefinition("variable", variableType, null)
when:
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: inputValue]))
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], RawVariables.of([variable: inputValue]))
then:
resolvedValues.get('variable') == outputValue

Expand Down Expand Up @@ -74,7 +74,7 @@ class ValuesResolverTest extends Specification {
VariableDefinition variableDefinition = new VariableDefinition("variable", new TypeName("Person"))

when:
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: inputValue]))
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], RawVariables.of([variable: inputValue]))
then:
resolvedValues.get('variable') == outputValue
where:
Expand Down Expand Up @@ -114,7 +114,7 @@ class ValuesResolverTest extends Specification {

when:
def obj = new Person('a', 123)
resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: obj]))
resolver.coerceVariableValues(schema, [variableDefinition], RawVariables.of([variable: obj]))
then:
thrown(CoercingParseValueException)
}
Expand All @@ -125,7 +125,7 @@ class ValuesResolverTest extends Specification {
VariableDefinition variableDefinition = new VariableDefinition("variable", new ListType(new TypeName("String")))
String value = "world"
when:
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: value]))
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], RawVariables.of([variable: value]))
then:
resolvedValues.get('variable') == ['world']
}
Expand All @@ -136,7 +136,7 @@ class ValuesResolverTest extends Specification {
VariableDefinition variableDefinition = new VariableDefinition("variable", new ListType(new TypeName("String")))
List<String> value = ["hello","world"]
when:
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: value]))
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], RawVariables.of([variable: value]))
then:
resolvedValues.get('variable') == ['hello','world']
}
Expand All @@ -147,14 +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], new RawVariables([variable: value]))
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], RawVariables.of([variable: value]))
then:
resolvedValues.get('variable') == ['hello','world']
}

def "getArgumentValues: resolves argument with variable reference"() {
given:
def variables = new CoercedVariables([var: 'hello'])
def variables = CoercedVariables.of([var: 'hello'])
def fieldArgument = newArgument().name("arg").type(GraphQLString).build()
def argument = new Argument("arg", new VariableReference("var"))

Expand Down Expand Up @@ -351,7 +351,7 @@ class ValuesResolverTest extends Specification {
VariableDefinition variableDefinition = new VariableDefinition("variable", new TypeName("Test"))

when:
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: inputValue]))
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], RawVariables.of([variable: inputValue]))
then:
resolvedValues.get('variable') == outputValue
where:
Expand Down Expand Up @@ -379,7 +379,7 @@ class ValuesResolverTest extends Specification {
VariableDefinition variableDefinition = new VariableDefinition("variable", new TypeName("InputObject"))

when:
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: inputValue]))
def resolvedValues = resolver.coerceVariableValues(schema, [variableDefinition], RawVariables.of([variable: inputValue]))

then:
resolvedValues.get('variable') == outputValue
Expand All @@ -406,7 +406,7 @@ class ValuesResolverTest extends Specification {
VariableDefinition variableDefinition = new VariableDefinition("variable", new TypeName("InputObject"))

when:
resolver.coerceVariableValues(schema, [variableDefinition], new RawVariables([variable: inputValue]))
resolver.coerceVariableValues(schema, [variableDefinition], RawVariables.of([variable: inputValue]))

then:
thrown(GraphQLException)
Expand All @@ -425,7 +425,7 @@ class ValuesResolverTest extends Specification {
VariableDefinition barVarDef = new VariableDefinition("bar", new TypeName("String"))

when:
def resolvedValues = resolver.coerceVariableValues(schema, [fooVarDef, barVarDef], new RawVariables(InputValue))
def resolvedValues = resolver.coerceVariableValues(schema, [fooVarDef, barVarDef], RawVariables.of(InputValue))

then:
resolvedValues.toMap() == outputValue
Expand Down Expand Up @@ -459,7 +459,7 @@ class ValuesResolverTest extends Specification {
def defaultValueForBar = new StringValue("defaultValueForBar")
VariableDefinition barVarDef = new VariableDefinition("bar", new TypeName("String"), defaultValueForBar)

def variableValuesMap = new RawVariables(["foo": null, "bar": "barValue"])
def variableValuesMap = RawVariables.of(["foo": null, "bar": "barValue"])

when:
def resolvedVars = resolver.coerceVariableValues(schema, [fooVarDef, barVarDef], variableValuesMap)
Expand All @@ -476,7 +476,7 @@ class ValuesResolverTest extends Specification {
def defaultValueForFoo = new StringValue("defaultValueForFoo")
VariableDefinition fooVarDef = new VariableDefinition("foo", new NonNullType(new TypeName("String")), defaultValueForFoo)

def variableValuesMap = new RawVariables(["foo": null])
def variableValuesMap = RawVariables.of(["foo": null])

when:
resolver.coerceVariableValues(schema, [fooVarDef], variableValuesMap)
Expand All @@ -494,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 = new RawVariables(["foo": [null]])
def variableValuesMap = RawVariables.of(["foo": [null]])

when:
resolver.coerceVariableValues(schema, [fooVarDef], variableValuesMap)
Expand Down Expand Up @@ -533,7 +533,7 @@ class ValuesResolverTest extends Specification {
def argument = new Argument("arg", new VariableReference("var"))

when:
def variables = new CoercedVariables(["var": null])
def variables = CoercedVariables.of(["var": null])
def values = resolver.getArgumentValues([fieldArgument], [argument], variables)

then:
Expand All @@ -550,7 +550,7 @@ class ValuesResolverTest extends Specification {
def argument = new Argument("arg", new VariableReference("var"))

when:
def variables = new CoercedVariables(["var": null])
def variables = CoercedVariables.of(["var": null])
resolver.getArgumentValues([fieldArgument], [argument], variables)

then:
Expand Down