X Tutup
Skip to content

Adding Locale to Coercing and hence ValueResolver #2912

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.

Sign up for GitHub

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

Merged
bbakerman merged 22 commits into from
Aug 22, 2022
Merged

Adding Locale to Coercing and hence ValueResolver #2912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f56f937
A propose shape change on Coercing
bbakerman Aug 5, 2022
5919da0
deprecation
bbakerman Aug 5, 2022
dffefc4
Added parser new signature
bbakerman Aug 5, 2022
c97b558
Changed ValueResolver so it took Locale
bbakerman Aug 6, 2022
5386876
Changed calls to coerce to use new environment
bbakerman Aug 6, 2022
62ccc28
Changed calls to coerce to NOT use new environment
bbakerman Aug 9, 2022
68b6c2e
Updated tests to use Locale
bbakerman Aug 10, 2022
39fcdab
Formatting of ValuesResolver
bbakerman Aug 10, 2022
6ccfa79
Move to graphql context for coercing and also got the parsing error m…
bbakerman Aug 12, 2022
655faad
Merge remote-tracking branch 'origin/master' into propose_changes_to_…
bbakerman Aug 12, 2022
d6bfb9c
Merged in master
bbakerman Aug 12, 2022
89c8574
Added Locale to Coercing and everywhere that calls it
bbakerman Aug 14, 2022
da0bd0e
now using Locale in the standard scalars
bbakerman Aug 15, 2022
2d74845
Break out legacy ValuesResolver code
bbakerman Aug 15, 2022
68d1070
Some renaming of methods
bbakerman Aug 15, 2022
9d42a8d
Refactoring ValueResolver so that conversion methods are in another c…
bbakerman Aug 15, 2022
c615df1
DirectivesResolver should take context and locale from input
bbakerman Aug 15, 2022
f59454d
Merge remote-tracking branch 'origin/master' into propose_changes_to_…
bbakerman Aug 15, 2022
f003665
Reverting Parser changes - they can go into their own PR
bbakerman Aug 15, 2022
6e23f9b
Merge remote-tracking branch 'origin/master' into propose_changes_to_…
bbakerman Aug 17, 2022
4408cc8
PR feedback
bbakerman Aug 17, 2022
f207309
PR feedback- fixed tests
bbakerman Aug 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/graphql/ExecutionInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ private ExecutionInput(Builder builder) {
this.dataLoaderRegistry = builder.dataLoaderRegistry;
this.cacheControl = builder.cacheControl;
this.executionId = builder.executionId;
this.locale = builder.locale;
this.locale = builder.locale != null ? builder.locale : Locale.getDefault(); // always have a locale in place
this.localContext = builder.localContext;
this.extensions = builder.extensions;
}
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/graphql/GraphQLContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,13 @@ public static GraphQLContext of(Consumer<GraphQLContext.Builder> contextBuilderC
return of(builder.map);
}

/**
* @return a new and empty graphql context object
*/
public static GraphQLContext getDefault() {
return GraphQLContext.newContext().build();
}

/**
* Creates a new GraphqlContext builder
*
Expand Down
9 changes: 8 additions & 1 deletion src/main/java/graphql/analysis/NodeVisitorWithTypeTracking.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package graphql.analysis;

import graphql.GraphQLContext;
import graphql.Internal;
import graphql.execution.CoercedVariables;
import graphql.execution.ConditionalNodes;
Expand Down Expand Up @@ -29,6 +30,7 @@
import graphql.util.TraversalControl;
import graphql.util.TraverserContext;

import java.util.Locale;
import java.util.Map;

import static graphql.Assert.assertNotNull;
Expand Down Expand Up @@ -151,7 +153,12 @@ 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(), CoercedVariables.of(variables));
Map<String, Object> argumentValues = ValuesResolver.getArgumentValues(codeRegistry,
fieldDefinition.getArguments(),
field.getArguments(),
CoercedVariables.of(variables),
GraphQLContext.getDefault(),
Locale.getDefault());
QueryVisitorFieldEnvironment environment = new QueryVisitorFieldEnvironmentImpl(isTypeNameIntrospectionField,
Copy link
Member Author

@bbakerman bbakerman Aug 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entry point come not from a graphql request and hence uses a default graphql context and default locale. Perhaps in some future change these are passed in.

field,
fieldDefinition,
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/graphql/analysis/QueryTraverser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package graphql.analysis;

import graphql.GraphQLContext;
import graphql.PublicApi;
import graphql.execution.CoercedVariables;
import graphql.execution.RawVariables;
Expand All @@ -20,6 +21,7 @@
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;

import static graphql.Assert.assertNotNull;
Expand Down Expand Up @@ -70,7 +72,7 @@ private QueryTraverser(GraphQLSchema schema,
this.fragmentsByName = getOperationResult.fragmentsByName;
this.roots = singletonList(getOperationResult.operationDefinition);
this.rootParentType = getRootTypeFromOperation(getOperationResult.operationDefinition);
this.coercedVariables = ValuesResolver.coerceVariableValues(schema, variableDefinitions, rawVariables);
this.coercedVariables = ValuesResolver.coerceVariableValues(schema, variableDefinitions, rawVariables, GraphQLContext.getDefault(), Locale.getDefault());
Copy link
Member Author

@bbakerman bbakerman Aug 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entry point come not from a graphql request and hence uses a default graphql context and default locale. Perhaps in some future change these are passed in.

}

private QueryTraverser(GraphQLSchema schema,
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/graphql/execution/ConditionalNodes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package graphql.execution;

import graphql.Assert;
import graphql.GraphQLContext;
import graphql.Internal;
import graphql.language.Directive;
import graphql.language.NodeUtil;

import java.util.List;
import java.util.Locale;
import java.util.Map;

import static graphql.Directives.IncludeDirective;
Expand All @@ -30,7 +32,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(), CoercedVariables.of(variables));
Map<String, Object> argumentValues = ValuesResolver.getArgumentValues(SkipDirective.getArguments(), foundDirective.getArguments(), CoercedVariables.of(variables), GraphQLContext.getDefault(), Locale.getDefault());
Copy link
Member Author

@bbakerman bbakerman Aug 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entry point come not from a graphql request and hence uses a default graphql context and default locale. Perhaps in some future change these are passed in.

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
2 changes: 1 addition & 1 deletion src/main/java/graphql/execution/Execution.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public CompletableFuture<ExecutionResult> execute(Document document, GraphQLSche

CoercedVariables coercedVariables;
try {
coercedVariables = ValuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables);
coercedVariables = ValuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables, executionInput.getGraphQLContext(), executionInput.getLocale());
Copy link
Member Author

@bbakerman bbakerman Aug 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

context and locale take from ExecutionInput

} catch (RuntimeException rte) {
if (rte instanceof GraphQLError) {
return completedFuture(new ExecutionResultImpl((GraphQLError) rte));
Expand Down
8 changes: 7 additions & 1 deletion src/main/java/graphql/execution/ExecutionStepInfoFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,13 @@ public ExecutionStepInfo newExecutionStepInfoForSubField(ExecutionContext execut
GraphQLOutputType fieldType = fieldDefinition.getType();
List<Argument> fieldArgs = mergedField.getArguments();
GraphQLCodeRegistry codeRegistry = executionContext.getGraphQLSchema().getCodeRegistry();
Supplier<Map<String, Object>> argumentValues = FpKit.intraThreadMemoize(() -> ValuesResolver.getArgumentValues(codeRegistry, fieldDefinition.getArguments(), fieldArgs, executionContext.getCoercedVariables()));
Supplier<Map<String, Object>> argumentValuesSupplier = () -> ValuesResolver.getArgumentValues(codeRegistry,
fieldDefinition.getArguments(),
fieldArgs,
executionContext.getCoercedVariables(),
executionContext.getGraphQLContext(),
executionContext.getLocale());
Supplier<Map<String, Object>> argumentValues = FpKit.intraThreadMemoize(argumentValuesSupplier);

ResultPath newPath = parentInfo.getPath().segment(mergedField.getResultKey());

Expand Down
16 changes: 13 additions & 3 deletions src/main/java/graphql/execution/ExecutionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,11 @@ protected CompletableFuture<FetchedValue> fetchField(ExecutionContext executionC

// DataFetchingFieldSelectionSet and QueryDirectives is a supplier of sorts - eg a lazy pattern
DataFetchingFieldSelectionSet fieldCollector = DataFetchingFieldSelectionSetImpl.newCollector(executionContext.getGraphQLSchema(), fieldType, normalizedFieldSupplier);
QueryDirectives queryDirectives = new QueryDirectivesImpl(field, executionContext.getGraphQLSchema(), executionContext.getVariables());
QueryDirectives queryDirectives = new QueryDirectivesImpl(field,
executionContext.getGraphQLSchema(),
executionContext.getCoercedVariables().toMap(),
executionContext.getGraphQLContext(),
executionContext.getLocale());


DataFetchingEnvironment environment = newDataFetchingEnvironment(executionContext)
Expand Down Expand Up @@ -590,7 +594,7 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext,
protected CompletableFuture<ExecutionResult> completeValueForScalar(ExecutionContext executionContext, ExecutionStrategyParameters parameters, GraphQLScalarType scalarType, Object result) {
Object serialized;
try {
serialized = scalarType.getCoercing().serialize(result);
serialized = scalarType.getCoercing().serialize(result, executionContext.getGraphQLContext(), executionContext.getLocale());
} catch (CoercingSerializeException e) {
serialized = handleCoercionProblem(executionContext, parameters, e);
}
Expand Down Expand Up @@ -820,7 +824,13 @@ protected ExecutionStepInfo createExecutionStepInfo(ExecutionContext executionCo
if (!fieldArgDefs.isEmpty()) {
List<Argument> fieldArgs = field.getArguments();
GraphQLCodeRegistry codeRegistry = executionContext.getGraphQLSchema().getCodeRegistry();
argumentValues = FpKit.intraThreadMemoize(() -> ValuesResolver.getArgumentValues(codeRegistry, fieldArgDefs, fieldArgs, executionContext.getCoercedVariables()));
Supplier<Map<String, Object>> argValuesSupplier = () -> ValuesResolver.getArgumentValues(codeRegistry,
fieldArgDefs,
fieldArgs,
executionContext.getCoercedVariables(),
executionContext.getGraphQLContext(),
executionContext.getLocale());
argumentValues = FpKit.intraThreadMemoize(argValuesSupplier);
}


Expand Down
Loading
X Tutup