-
Notifications
You must be signed in to change notification settings - Fork 1.2k
This adds support for QueryAppliedDirective on operations and documents #4297
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
Changes from all commits
9f65d67
c897548
0f689b0
1740162
1951ed6
2e7c4fc
8f049fe
17e7ccd
fdd9568
9da09ed
b47540c
7b92be4
601195a
93b53e4
bdf5163
0752dc1
878478b
47d3101
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 |
|---|---|---|
|
|
@@ -11,14 +11,15 @@ | |
| import graphql.Profiler; | ||
| import graphql.PublicApi; | ||
| import graphql.collect.ImmutableKit; | ||
| import graphql.execution.directives.OperationDirectivesResolver; | ||
| import graphql.execution.directives.QueryAppliedDirective; | ||
| import graphql.execution.incremental.IncrementalCallState; | ||
| import graphql.execution.instrumentation.Instrumentation; | ||
| import graphql.execution.instrumentation.InstrumentationState; | ||
| import graphql.language.Document; | ||
| import graphql.language.FragmentDefinition; | ||
| import graphql.language.OperationDefinition; | ||
| import graphql.normalized.ExecutableNormalizedOperation; | ||
| import graphql.normalized.ExecutableNormalizedOperationFactory; | ||
| import graphql.schema.GraphQLSchema; | ||
| import graphql.util.FpKit; | ||
| import graphql.util.LockKit; | ||
|
|
@@ -34,6 +35,9 @@ | |
| import java.util.function.Consumer; | ||
| import java.util.function.Supplier; | ||
|
|
||
| import static graphql.normalized.ExecutableNormalizedOperationFactory.Options; | ||
| import static graphql.normalized.ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation; | ||
|
|
||
| @SuppressWarnings("TypeParameterUnusedInFormals") | ||
| @PublicApi | ||
| public class ExecutionContext { | ||
|
|
@@ -73,6 +77,8 @@ public class ExecutionContext { | |
| private final ResultNodesInfo resultNodesInfo = new ResultNodesInfo(); | ||
| private final EngineRunningState engineRunningState; | ||
|
|
||
| private final Supplier<Map<OperationDefinition, ImmutableList<QueryAppliedDirective>>> allOperationsDirectives; | ||
| private final Supplier<Map<String, ImmutableList<QueryAppliedDirective>>> operationDirectives; | ||
| private final Profiler profiler; | ||
|
|
||
| ExecutionContext(ExecutionContextBuilder builder) { | ||
|
|
@@ -99,10 +105,27 @@ public class ExecutionContext { | |
| this.localContext = builder.localContext; | ||
| this.executionInput = builder.executionInput; | ||
| this.dataLoaderDispatcherStrategy = builder.dataLoaderDispatcherStrategy; | ||
| this.queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables)); | ||
| this.propagateErrorsOnNonNullContractFailure = builder.propagateErrorsOnNonNullContractFailure; | ||
|
Member
Author
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. rejigged this code to a maker method |
||
| this.engineRunningState = builder.engineRunningState; | ||
| this.profiler = builder.profiler; | ||
| // lazy loading for performance | ||
| this.queryTree = mkExecutableNormalizedOperation(); | ||
| this.allOperationsDirectives = builder.allOperationsDirectives; | ||
| this.operationDirectives = mkOpDirectives(builder.allOperationsDirectives); | ||
| } | ||
|
|
||
| private Supplier<ExecutableNormalizedOperation> mkExecutableNormalizedOperation() { | ||
| return FpKit.interThreadMemoize(() -> { | ||
| Options options = Options.defaultOptions().graphQLContext(graphQLContext).locale(locale); | ||
|
Member
Author
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. fixed a bug here - graphql context and locale was never respected before |
||
| return createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables, options); | ||
| }); | ||
| } | ||
|
|
||
| private Supplier<Map<String, ImmutableList<QueryAppliedDirective>>> mkOpDirectives(Supplier<Map<OperationDefinition, ImmutableList<QueryAppliedDirective>>> allOperationsDirectives) { | ||
| return FpKit.interThreadMemoize(() -> { | ||
| List<QueryAppliedDirective> list = allOperationsDirectives.get().get(operationDefinition); | ||
|
Member
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. A JSpecify observation - this class hasn't yet been annotated, but if it was, it would be clear this is not null, that an operation definition always exists |
||
| return OperationDirectivesResolver.toAppliedDirectivesByName(list); | ||
| }); | ||
| } | ||
|
|
||
| public ExecutionId getExecutionId() { | ||
|
|
@@ -137,6 +160,21 @@ public OperationDefinition getOperationDefinition() { | |
| return operationDefinition; | ||
| } | ||
|
|
||
| /** | ||
| * @return the map of {@link QueryAppliedDirective}s by name that were on this executing operation | ||
| */ | ||
| public Map<String, ImmutableList<QueryAppliedDirective>> getOperationDirectives() { | ||
| return operationDirectives.get(); | ||
| } | ||
|
|
||
| /** | ||
| * @return the map of all the {@link QueryAppliedDirective}s that were on the {@link Document} including | ||
| * {@link OperationDefinition}s that are not currently executing. | ||
| */ | ||
| public Map<OperationDefinition, ImmutableList<QueryAppliedDirective>> getAllOperationDirectives() { | ||
| return allOperationsDirectives.get(); | ||
| } | ||
|
|
||
| public CoercedVariables getCoercedVariables() { | ||
| return coercedVariables; | ||
| } | ||
|
|
@@ -156,7 +194,7 @@ public Supplier<NormalizedVariables> getNormalizedVariables() { | |
| * @deprecated use {@link #getGraphQLContext()} instead | ||
| */ | ||
| @Deprecated(since = "2021-07-05") | ||
| @SuppressWarnings({ "unchecked", "TypeParameterUnusedInFormals" }) | ||
| @SuppressWarnings({"unchecked", "TypeParameterUnusedInFormals"}) | ||
| public @Nullable <T> T getContext() { | ||
| return (T) context; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| import graphql.Internal; | ||
| import graphql.Profiler; | ||
| import graphql.collect.ImmutableKit; | ||
| import graphql.execution.directives.QueryAppliedDirective; | ||
| import graphql.execution.instrumentation.Instrumentation; | ||
| import graphql.execution.instrumentation.InstrumentationState; | ||
| import graphql.language.Document; | ||
|
|
@@ -19,6 +20,7 @@ | |
| import org.dataloader.DataLoaderRegistry; | ||
| import org.jspecify.annotations.Nullable; | ||
|
|
||
| import java.util.Collections; | ||
| import java.util.Locale; | ||
| import java.util.Map; | ||
| import java.util.function.Supplier; | ||
|
|
@@ -55,6 +57,7 @@ public class ExecutionContextBuilder { | |
| EngineRunningState engineRunningState; | ||
| ResponseMapFactory responseMapFactory = ResponseMapFactory.DEFAULT; | ||
| Profiler profiler; | ||
| Supplier<Map<OperationDefinition, ImmutableList<QueryAppliedDirective>>> allOperationsDirectives = Collections::emptyMap; | ||
|
|
||
| /** | ||
| * @return a new builder of {@link graphql.execution.ExecutionContext}s | ||
|
|
@@ -168,6 +171,7 @@ public ExecutionContextBuilder root(Object root) { | |
|
|
||
| /** | ||
| * @param variables map of already coerced variables | ||
| * | ||
| * @return this builder | ||
| * | ||
| * @deprecated use {@link #coercedVariables(CoercedVariables)} instead | ||
|
|
@@ -246,13 +250,6 @@ public ExecutionContextBuilder propagapropagateErrorsOnNonNullContractFailureeEr | |
| return this; | ||
| } | ||
|
|
||
|
|
||
| public ExecutionContext build() { | ||
| // preconditions | ||
| assertNotNull(executionId, "You must provide a query identifier"); | ||
| return new ExecutionContext(this); | ||
| } | ||
|
|
||
|
Member
Author
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. move down |
||
| public ExecutionContextBuilder engineRunningState(EngineRunningState engineRunningState) { | ||
| this.engineRunningState = engineRunningState; | ||
| return this; | ||
|
|
@@ -262,4 +259,16 @@ public ExecutionContextBuilder profiler(Profiler profiler) { | |
| this.profiler = profiler; | ||
| return this; | ||
| } | ||
|
|
||
| public ExecutionContextBuilder operationDirectives(Supplier<Map<OperationDefinition, ImmutableList<QueryAppliedDirective>>> allOperationsDirectives) { | ||
| this.allOperationsDirectives = allOperationsDirectives; | ||
| return this; | ||
| } | ||
|
|
||
|
|
||
| public ExecutionContext build() { | ||
| // preconditions | ||
| assertNotNull(executionId, "You must provide a query identifier"); | ||
| return new ExecutionContext(this); | ||
| } | ||
|
Member
Author
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. moved build down to the bottom for code OCD reasons |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,10 @@ | |
| import com.google.common.collect.BiMap; | ||
| import com.google.common.collect.HashBiMap; | ||
| import com.google.common.collect.ImmutableBiMap; | ||
| import com.google.common.collect.ImmutableList; | ||
| import graphql.GraphQLContext; | ||
| import graphql.Internal; | ||
| import graphql.collect.ImmutableKit; | ||
| import graphql.execution.CoercedVariables; | ||
| import graphql.execution.ValuesResolver; | ||
| import graphql.language.Directive; | ||
|
|
@@ -61,4 +63,34 @@ private void buildArguments(GraphQLDirective.Builder directiveBuilder, | |
| } | ||
| }); | ||
| } | ||
|
|
||
| public ImmutableList<QueryAppliedDirective> toAppliedDirectives(List<Directive> directives, GraphQLSchema schema, CoercedVariables variables, GraphQLContext graphQLContext, Locale locale) { | ||
| BiMap<GraphQLDirective, Directive> directivesMap = resolveDirectives(directives, schema, variables, graphQLContext, locale); | ||
| return ImmutableKit.map(directivesMap.keySet(), this::toAppliedDirective); | ||
| } | ||
|
|
||
| /** | ||
| * This helps us remodel the applied GraphQLDirective back to the better modelled and named {@link QueryAppliedDirective} | ||
| * | ||
| * @param directive the directive to remodel | ||
| * | ||
| * @return a QueryAppliedDirective | ||
| */ | ||
| public QueryAppliedDirective toAppliedDirective(GraphQLDirective directive) { | ||
|
Member
Author
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. These methods used to be in QueryDirectivesImpl but I moved them here to be common and used now in two places |
||
| QueryAppliedDirective.Builder builder = QueryAppliedDirective.newDirective(); | ||
| builder.name(directive.getName()); | ||
| for (GraphQLArgument argument : directive.getArguments()) { | ||
| builder.argument(toAppliedArgument(argument)); | ||
| } | ||
| return builder.build(); | ||
| } | ||
|
|
||
| public QueryAppliedDirectiveArgument toAppliedArgument(GraphQLArgument argument) { | ||
| return QueryAppliedDirectiveArgument.newArgument() | ||
| .name(argument.getName()) | ||
| .type(argument.getType()) | ||
| .inputValueWithState(argument.getArgumentValue()) | ||
| .build(); | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| package graphql.execution.directives; | ||
|
|
||
| import com.google.common.collect.ImmutableList; | ||
| import com.google.common.collect.ImmutableMap; | ||
| import graphql.GraphQLContext; | ||
| import graphql.Internal; | ||
| import graphql.execution.CoercedVariables; | ||
| import graphql.language.Document; | ||
| import graphql.language.OperationDefinition; | ||
| import graphql.schema.GraphQLSchema; | ||
| import graphql.util.FpKit; | ||
| import org.jspecify.annotations.NullMarked; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Locale; | ||
| import java.util.Map; | ||
|
|
||
| @Internal | ||
| @NullMarked | ||
| public class OperationDirectivesResolver { | ||
|
Member
Author
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. The new extracting class is Internal - exposed later via ExecutionContext and ENO |
||
|
|
||
| private final DirectivesResolver directivesResolver = new DirectivesResolver(); | ||
|
|
||
| public ImmutableMap<OperationDefinition, ImmutableList<QueryAppliedDirective>> resolveDirectives(Document document, GraphQLSchema schema, CoercedVariables variables, GraphQLContext graphQLContext, Locale locale) { | ||
| ImmutableMap.Builder<OperationDefinition, ImmutableList<QueryAppliedDirective>> builder = ImmutableMap.builder(); | ||
| for (OperationDefinition operationDefinition : document.getDefinitionsOfType(OperationDefinition.class)) { | ||
| builder.put(operationDefinition, resolveDirectives(operationDefinition, schema, variables, graphQLContext, locale)); | ||
| } | ||
| return builder.build(); | ||
| } | ||
|
|
||
| public ImmutableList<QueryAppliedDirective> resolveDirectives(OperationDefinition operationDefinition, GraphQLSchema schema, CoercedVariables variables, GraphQLContext graphQLContext, Locale locale) { | ||
| return directivesResolver.toAppliedDirectives( | ||
| operationDefinition.getDirectives(), | ||
| schema, | ||
| variables, | ||
| graphQLContext, | ||
| locale | ||
| ); | ||
| } | ||
|
|
||
| public ImmutableMap<String, ImmutableList<QueryAppliedDirective>> resolveDirectivesByName(OperationDefinition operationDefinition, GraphQLSchema schema, CoercedVariables variables, GraphQLContext graphQLContext, Locale locale) { | ||
| List<QueryAppliedDirective> list = resolveDirectives(operationDefinition, schema, variables, graphQLContext, locale); | ||
| return toAppliedDirectivesByName(list); | ||
| } | ||
|
|
||
| public static ImmutableMap<String, ImmutableList<QueryAppliedDirective>> toAppliedDirectivesByName(List<QueryAppliedDirective> queryAppliedDirectives) { | ||
| Map<String, ImmutableList<QueryAppliedDirective>> immutableListMap = FpKit.groupingBy(queryAppliedDirectives, QueryAppliedDirective::getName); | ||
| return ImmutableMap.copyOf(immutableListMap); | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,7 +80,7 @@ private void computeValuesLazily() { | |
|
|
||
| ImmutableList.Builder<QueryAppliedDirective> appliedDirectiveBuilder = ImmutableList.builder(); | ||
| for (GraphQLDirective resolvedDirective : resolvedDirectives) { | ||
| QueryAppliedDirective appliedDirective = toAppliedDirective(resolvedDirective); | ||
| QueryAppliedDirective appliedDirective = directivesResolver.toAppliedDirective(resolvedDirective); | ||
| appliedDirectiveBuilder.add(appliedDirective); | ||
| gqlDirectiveCounterParts.put(resolvedDirective, appliedDirective); | ||
| } | ||
|
|
@@ -125,23 +125,6 @@ private void computeValuesLazily() { | |
| }); | ||
| } | ||
|
|
||
| private QueryAppliedDirective toAppliedDirective(GraphQLDirective directive) { | ||
| QueryAppliedDirective.Builder builder = QueryAppliedDirective.newDirective(); | ||
| builder.name(directive.getName()); | ||
| for (GraphQLArgument argument : directive.getArguments()) { | ||
| builder.argument(toAppliedArgument(argument)); | ||
| } | ||
| return builder.build(); | ||
| } | ||
|
|
||
| private QueryAppliedDirectiveArgument toAppliedArgument(GraphQLArgument argument) { | ||
| return QueryAppliedDirectiveArgument.newArgument() | ||
| .name(argument.getName()) | ||
| .type(argument.getType()) | ||
| .inputValueWithState(argument.getArgumentValue()) | ||
| .build(); | ||
| } | ||
|
|
||
|
Member
Author
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. moved methods so they can be re-used |
||
|
|
||
| @Override | ||
| public Map<Field, List<GraphQLDirective>> getImmediateDirectivesByField() { | ||
|
|
||
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.
This is a memoizing supplier for performance reasons. if you never ask for the directives - no work is done - only the allocation of a
interThreadMemoizeobject