From 003bfdc27ddc571142bcc322093aa847d799e2ff Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Mon, 28 Feb 2022 09:24:45 +1100 Subject: [PATCH 1/6] Another WIP PR that shows how we could transition to the new idea of passing in state to the instrumentation fields --- .../graphql/execution/ExecutionStrategy.java | 14 +++++------ .../SubscriptionExecutionStrategy.java | 4 ++- .../ChainedInstrumentation.java | 25 +++++++++++++++++++ .../instrumentation/Instrumentation.java | 25 +++++++++++++++++++ 4 files changed, 60 insertions(+), 8 deletions(-) diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index 3b0d58fa4e..c4ec22b9cc 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -204,7 +204,7 @@ protected CompletableFuture resolveFieldWithInfo(ExecutionContex Instrumentation instrumentation = executionContext.getInstrumentation(); InstrumentationContext fieldCtx = instrumentation.beginField( - new InstrumentationFieldParameters(executionContext, executionStepInfo) + new InstrumentationFieldParameters(executionContext, executionStepInfo), executionContext.getInstrumentationState() ); CompletableFuture fetchFieldFuture = fetchField(executionContext, parameters); @@ -270,7 +270,7 @@ protected CompletableFuture fetchField(ExecutionContext executionC Instrumentation instrumentation = executionContext.getInstrumentation(); InstrumentationFieldFetchParameters instrumentationFieldFetchParams = new InstrumentationFieldFetchParameters(executionContext, environment, parameters, dataFetcher instanceof TrivialDataFetcher); - InstrumentationContext fetchCtx = instrumentation.beginFieldFetch(instrumentationFieldFetchParams); + InstrumentationContext fetchCtx = instrumentation.beginFieldFetch(instrumentationFieldFetchParams, executionContext.getInstrumentationState()); CompletableFuture fetchedValue; dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, instrumentationFieldFetchParams); @@ -354,9 +354,9 @@ protected CompletableFuture handleFetchingException(ExecutionContext exec private CompletableFuture asyncHandleException(DataFetcherExceptionHandler handler, DataFetcherExceptionHandlerParameters handlerParameters, ExecutionContext executionContext) { //noinspection unchecked - return handler.handleException(handlerParameters) - .thenApply(handlerResult -> (T) DataFetcherResult.newResult().errors(handlerResult.getErrors()).build() - ); + return handler.handleException(handlerParameters) + .thenApply(handlerResult -> (T) DataFetcherResult.newResult().errors(handlerResult.getErrors()).build() + ); } /** @@ -385,7 +385,7 @@ protected FieldValueInfo completeField(ExecutionContext executionContext, Execut Instrumentation instrumentation = executionContext.getInstrumentation(); InstrumentationFieldCompleteParameters instrumentationParams = new InstrumentationFieldCompleteParameters(executionContext, parameters, () -> executionStepInfo, fetchedValue); InstrumentationContext ctxCompleteField = instrumentation.beginFieldComplete( - instrumentationParams + instrumentationParams, executionContext.getInstrumentationState() ); NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, executionStepInfo); @@ -519,7 +519,7 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext, Instrumentation instrumentation = executionContext.getInstrumentation(); InstrumentationContext completeListCtx = instrumentation.beginFieldListComplete( - instrumentationParams + instrumentationParams, executionContext.getInstrumentationState() ); List fieldValueInfos = new ArrayList<>(size.orElse(1)); diff --git a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java index 4fd3499627..c6551576db 100644 --- a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java +++ b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java @@ -123,7 +123,9 @@ private CompletableFuture executeSubscriptionEvent(ExecutionCon ExecutionStepInfo subscribedFieldStepInfo = createSubscribedFieldStepInfo(executionContext, newParameters); InstrumentationFieldParameters i13nFieldParameters = new InstrumentationFieldParameters(executionContext, () -> subscribedFieldStepInfo); - InstrumentationContext subscribedFieldCtx = instrumentation.beginSubscribedFieldEvent(i13nFieldParameters); + InstrumentationContext subscribedFieldCtx = instrumentation.beginSubscribedFieldEvent( + i13nFieldParameters, executionContext.getInstrumentationState() + ); FetchedValue fetchedValue = unboxPossibleDataFetcherResult(newExecutionContext, parameters, eventPayload); FieldValueInfo fieldValueInfo = completeField(newExecutionContext, newParameters, fetchedValue); diff --git a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java index 48f22a11fa..d489740678 100644 --- a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java @@ -118,6 +118,11 @@ public InstrumentationContext beginSubscribedFieldEvent(Instrum })); } + @Override + public InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) { + return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginSubscribedFieldEvent(parameters, state))); + } + @Override public InstrumentationContext beginField(InstrumentationFieldParameters parameters) { return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> { @@ -126,6 +131,11 @@ public InstrumentationContext beginField(InstrumentationFieldPa })); } + @Override + public InstrumentationContext beginField(InstrumentationFieldParameters parameters, InstrumentationState state) { + return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginField(parameters, state))); + } + @Override public InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters) { return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> { @@ -134,6 +144,11 @@ public InstrumentationContext beginFieldFetch(InstrumentationFieldFetchP })); } + @Override + public InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginFieldFetch(parameters, getState(it, state)))); + } + @Override public InstrumentationContext beginFieldComplete(InstrumentationFieldCompleteParameters parameters) { return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> { @@ -142,6 +157,11 @@ public InstrumentationContext beginFieldComplete(Instrumentatio })); } + @Override + public InstrumentationContext beginFieldComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginFieldComplete(parameters, getState(it, state)))); + } + @Override public InstrumentationContext beginFieldListComplete(InstrumentationFieldCompleteParameters parameters) { return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> { @@ -150,6 +170,11 @@ public InstrumentationContext beginFieldListComplete(Instrument })); } + @Override + public InstrumentationContext beginFieldListComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginFieldListComplete(parameters, getState(it, state)))); + } + @Override public ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters) { for (Instrumentation instrumentation : instrumentations) { diff --git a/src/main/java/graphql/execution/instrumentation/Instrumentation.java b/src/main/java/graphql/execution/instrumentation/Instrumentation.java index 8d64509a59..82532b59df 100644 --- a/src/main/java/graphql/execution/instrumentation/Instrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/Instrumentation.java @@ -114,10 +114,15 @@ default InstrumentationState createState(InstrumentationCreateStateParameters pa * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends */ + @Deprecated default InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters) { return noOp(); } + default InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) { + return beginSubscribedFieldEvent(parameters.withNewState(state)); + } + /** * This is called just before a field is resolved into a value. * @@ -125,8 +130,13 @@ default InstrumentationContext beginSubscribedFieldEvent(Instru * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends */ + @Deprecated InstrumentationContext beginField(InstrumentationFieldParameters parameters); + default InstrumentationContext beginField(InstrumentationFieldParameters parameters, InstrumentationState state) { + return beginField(parameters.withNewState(state)); + } + /** * This is called just before a field {@link DataFetcher} is invoked. * @@ -134,8 +144,13 @@ default InstrumentationContext beginSubscribedFieldEvent(Instru * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends */ + @Deprecated InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters); + default InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + return beginFieldFetch(parameters.withNewState(state)); + } + /** * This is called just before the complete field is started. @@ -144,10 +159,15 @@ default InstrumentationContext beginSubscribedFieldEvent(Instru * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends */ + @Deprecated default InstrumentationContext beginFieldComplete(InstrumentationFieldCompleteParameters parameters) { return noOp(); } + default InstrumentationContext beginFieldComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + return beginFieldComplete(parameters.withNewState(state)); + } + /** * This is called just before the complete field list is started. * @@ -155,10 +175,15 @@ default InstrumentationContext beginFieldComplete(Instrumentati * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends */ + @Deprecated default InstrumentationContext beginFieldListComplete(InstrumentationFieldCompleteParameters parameters) { return noOp(); } + default InstrumentationContext beginFieldListComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + return beginFieldListComplete(parameters.withNewState(state)); + } + /** * This is called to instrument a {@link graphql.ExecutionInput} before it is used to parse, validate * and execute a query, allowing you to adjust what query input parameters are used From 3cb1003f1c396592b0dfce3b958fa1013c607c0f Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Mon, 28 Feb 2022 10:01:54 +1100 Subject: [PATCH 2/6] Opps didnt write it right --- .../execution/instrumentation/ChainedInstrumentation.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java index d489740678..c0bea68285 100644 --- a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java @@ -120,7 +120,7 @@ public InstrumentationContext beginSubscribedFieldEvent(Instrum @Override public InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) { - return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginSubscribedFieldEvent(parameters, state))); + return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginSubscribedFieldEvent(parameters, getState(it, state)))); } @Override @@ -133,7 +133,7 @@ public InstrumentationContext beginField(InstrumentationFieldPa @Override public InstrumentationContext beginField(InstrumentationFieldParameters parameters, InstrumentationState state) { - return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginField(parameters, state))); + return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginField(parameters, getState(it, state)))); } @Override From 41598ddb35b08d3b75427c38c1c33646932be88f Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Tue, 22 Mar 2022 16:38:56 +1100 Subject: [PATCH 3/6] Start of work to have instrumentation state parameters passed in explicitly --- src/main/java/graphql/GraphQL.java | 21 +- .../execution/AsyncExecutionStrategy.java | 2 +- .../AsyncSerialExecutionStrategy.java | 6 +- .../java/graphql/execution/Execution.java | 6 +- .../graphql/execution/ExecutionStrategy.java | 19 +- .../SubscriptionExecutionStrategy.java | 12 +- ...ecutionStrategyInstrumentationContext.java | 28 ++ .../instrumentation/Instrumentation.java | 269 +++++++++++++++++- .../SimpleInstrumentationContext.java | 16 +- ...rumentationExecuteOperationParameters.java | 14 + .../InstrumentationExecutionParameters.java | 15 + ...umentationExecutionStrategyParameters.java | 15 + ...nstrumentationFieldCompleteParameters.java | 16 ++ .../InstrumentationFieldFetchParameters.java | 3 + .../InstrumentationFieldParameters.java | 15 + .../InstrumentationValidationParameters.java | 3 + 16 files changed, 430 insertions(+), 30 deletions(-) diff --git a/src/main/java/graphql/GraphQL.java b/src/main/java/graphql/GraphQL.java index e4f54168ec..91246aab6e 100644 --- a/src/main/java/graphql/GraphQL.java +++ b/src/main/java/graphql/GraphQL.java @@ -46,6 +46,7 @@ import static graphql.Assert.assertNotNull; import static graphql.execution.ExecutionIdProvider.DEFAULT_EXECUTION_ID_PROVIDER; import static graphql.execution.instrumentation.SimpleInstrumentationContext.completeInstrumentationCtxCF; +import static graphql.execution.instrumentation.SimpleInstrumentationContext.nonNullCtx; /** * This class is where all graphql-java query execution begins. It combines the objects that are needed @@ -512,14 +513,14 @@ public CompletableFuture executeAsync(ExecutionInput executionI InstrumentationState instrumentationState = instrumentation.createState(new InstrumentationCreateStateParameters(this.graphQLSchema, executionInput)); InstrumentationExecutionParameters inputInstrumentationParameters = new InstrumentationExecutionParameters(executionInput, this.graphQLSchema, instrumentationState); - executionInput = instrumentation.instrumentExecutionInput(executionInput, inputInstrumentationParameters); + executionInput = instrumentation.instrumentExecutionInput(executionInput, inputInstrumentationParameters, instrumentationState); CompletableFuture beginExecutionCF = new CompletableFuture<>(); InstrumentationExecutionParameters instrumentationParameters = new InstrumentationExecutionParameters(executionInput, this.graphQLSchema, instrumentationState); - InstrumentationContext executionInstrumentation = instrumentation.beginExecution(instrumentationParameters); + InstrumentationContext executionInstrumentation = nonNullCtx(instrumentation.beginExecution(instrumentationParameters, instrumentationState)); executionInstrumentation.onDispatched(beginExecutionCF); - GraphQLSchema graphQLSchema = instrumentation.instrumentSchema(this.graphQLSchema, instrumentationParameters); + GraphQLSchema graphQLSchema = instrumentation.instrumentSchema(this.graphQLSchema, instrumentationParameters, instrumentationState); CompletableFuture executionResult = parseValidateAndExecute(executionInput, graphQLSchema, instrumentationState); // @@ -527,7 +528,7 @@ public CompletableFuture executeAsync(ExecutionInput executionI executionResult = executionResult.whenComplete(completeInstrumentationCtxCF(executionInstrumentation, beginExecutionCF)); // // allow instrumentation to tweak the result - executionResult = executionResult.thenCompose(result -> instrumentation.instrumentExecutionResult(result, instrumentationParameters)); + executionResult = executionResult.thenCompose(result -> instrumentation.instrumentExecutionResult(result, instrumentationParameters, instrumentationState)); return executionResult; } catch (AbortExecutionException abortException) { return CompletableFuture.completedFuture(abortException.toExecutionResult()); @@ -598,27 +599,27 @@ private PreparsedDocumentEntry parseAndValidate(AtomicReference private ParseAndValidateResult parse(ExecutionInput executionInput, GraphQLSchema graphQLSchema, InstrumentationState instrumentationState) { InstrumentationExecutionParameters parameters = new InstrumentationExecutionParameters(executionInput, graphQLSchema, instrumentationState); - InstrumentationContext parseInstrumentation = instrumentation.beginParse(parameters); + InstrumentationContext parseInstrumentationCtx = nonNullCtx(instrumentation.beginParse(parameters, instrumentationState)); CompletableFuture documentCF = new CompletableFuture<>(); - parseInstrumentation.onDispatched(documentCF); + parseInstrumentationCtx.onDispatched(documentCF); ParseAndValidateResult parseResult = ParseAndValidate.parse(executionInput); if (parseResult.isFailure()) { - parseInstrumentation.onCompleted(null, parseResult.getSyntaxException()); + parseInstrumentationCtx.onCompleted(null, parseResult.getSyntaxException()); return parseResult; } else { documentCF.complete(parseResult.getDocument()); - parseInstrumentation.onCompleted(parseResult.getDocument(), null); + parseInstrumentationCtx.onCompleted(parseResult.getDocument(), null); DocumentAndVariables documentAndVariables = parseResult.getDocumentAndVariables(); - documentAndVariables = instrumentation.instrumentDocumentAndVariables(documentAndVariables, parameters); + documentAndVariables = instrumentation.instrumentDocumentAndVariables(documentAndVariables, parameters, instrumentationState); return ParseAndValidateResult.newResult() .document(documentAndVariables.getDocument()).variables(documentAndVariables.getVariables()).build(); } } private List validate(ExecutionInput executionInput, Document document, GraphQLSchema graphQLSchema, InstrumentationState instrumentationState) { - InstrumentationContext> validationCtx = instrumentation.beginValidation(new InstrumentationValidationParameters(executionInput, document, graphQLSchema, instrumentationState)); + InstrumentationContext> validationCtx = nonNullCtx(instrumentation.beginValidation(new InstrumentationValidationParameters(executionInput, document, graphQLSchema, instrumentationState), instrumentationState)); CompletableFuture> cf = new CompletableFuture<>(); validationCtx.onDispatched(cf); diff --git a/src/main/java/graphql/execution/AsyncExecutionStrategy.java b/src/main/java/graphql/execution/AsyncExecutionStrategy.java index 3800657680..abdda65bd5 100644 --- a/src/main/java/graphql/execution/AsyncExecutionStrategy.java +++ b/src/main/java/graphql/execution/AsyncExecutionStrategy.java @@ -43,7 +43,7 @@ public CompletableFuture execute(ExecutionContext executionCont Instrumentation instrumentation = executionContext.getInstrumentation(); InstrumentationExecutionStrategyParameters instrumentationParameters = new InstrumentationExecutionStrategyParameters(executionContext, parameters); - ExecutionStrategyInstrumentationContext executionStrategyCtx = instrumentation.beginExecutionStrategy(instrumentationParameters); + ExecutionStrategyInstrumentationContext executionStrategyCtx = ExecutionStrategyInstrumentationContext.nonNullCtx(instrumentation.beginExecutionStrategy(instrumentationParameters, executionContext.getInstrumentationState())); MergedSelectionSet fields = parameters.getFields(); Set fieldNames = fields.keySet(); diff --git a/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java b/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java index 889156ce85..3e5ee2f67f 100644 --- a/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java +++ b/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java @@ -10,6 +10,8 @@ import java.util.List; import java.util.concurrent.CompletableFuture; +import static graphql.execution.instrumentation.SimpleInstrumentationContext.nonNullCtx; + /** * Async non-blocking execution, but serial: only one field at the the time will be resolved. * See {@link AsyncExecutionStrategy} for a non serial (parallel) execution of every field. @@ -31,7 +33,9 @@ public CompletableFuture execute(ExecutionContext executionCont Instrumentation instrumentation = executionContext.getInstrumentation(); InstrumentationExecutionStrategyParameters instrumentationParameters = new InstrumentationExecutionStrategyParameters(executionContext, parameters); - InstrumentationContext executionStrategyCtx = instrumentation.beginExecutionStrategy(instrumentationParameters); + InstrumentationContext executionStrategyCtx = nonNullCtx(instrumentation.beginExecutionStrategy(instrumentationParameters, + executionContext.getInstrumentationState()) + ); MergedSelectionSet fields = parameters.getFields(); ImmutableList fieldNames = ImmutableList.copyOf(fields.keySet()); diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 01e6e85034..6ef6296dab 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -30,7 +30,7 @@ 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.execution.instrumentation.SimpleInstrumentationContext.nonNullCtx; import static graphql.language.OperationDefinition.Operation.MUTATION; import static graphql.language.OperationDefinition.Operation.QUERY; import static graphql.language.OperationDefinition.Operation.SUBSCRIPTION; @@ -102,7 +102,7 @@ public CompletableFuture execute(Document document, GraphQLSche InstrumentationExecutionParameters parameters = new InstrumentationExecutionParameters( executionInput, graphQLSchema, instrumentationState ); - executionContext = instrumentation.instrumentExecutionContext(executionContext, parameters); + executionContext = instrumentation.instrumentExecutionContext(executionContext, parameters, instrumentationState); return executeOperation(executionContext, executionInput.getRoot(), executionContext.getOperationDefinition()); } @@ -110,7 +110,7 @@ public CompletableFuture execute(Document document, GraphQLSche private CompletableFuture executeOperation(ExecutionContext executionContext, Object root, OperationDefinition operationDefinition) { InstrumentationExecuteOperationParameters instrumentationParams = new InstrumentationExecuteOperationParameters(executionContext); - InstrumentationContext executeOperationCtx = instrumentation.beginExecuteOperation(instrumentationParams); + InstrumentationContext executeOperationCtx = nonNullCtx(instrumentation.beginExecuteOperation(instrumentationParams, executionContext.getInstrumentationState())); OperationDefinition.Operation operation = operationDefinition.getOperation(); GraphQLObjectType operationRootType; diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index faea70cbe4..698f3a9c44 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -59,6 +59,7 @@ import static graphql.execution.FieldValueInfo.CompleteValueType.NULL; import static graphql.execution.FieldValueInfo.CompleteValueType.OBJECT; import static graphql.execution.FieldValueInfo.CompleteValueType.SCALAR; +import static graphql.execution.instrumentation.SimpleInstrumentationContext.nonNullCtx; import static graphql.schema.DataFetchingEnvironmentImpl.newDataFetchingEnvironment; import static graphql.schema.GraphQLTypeUtil.isEnum; import static graphql.schema.GraphQLTypeUtil.isList; @@ -203,9 +204,9 @@ protected CompletableFuture resolveFieldWithInfo(ExecutionContex Supplier executionStepInfo = FpKit.intraThreadMemoize(() -> createExecutionStepInfo(executionContext, parameters, fieldDef, null)); Instrumentation instrumentation = executionContext.getInstrumentation(); - InstrumentationContext fieldCtx = instrumentation.beginField( + InstrumentationContext fieldCtx = nonNullCtx(instrumentation.beginField( new InstrumentationFieldParameters(executionContext, executionStepInfo), executionContext.getInstrumentationState() - ); + )); CompletableFuture fetchFieldFuture = fetchField(executionContext, parameters); CompletableFuture result = fetchFieldFuture.thenApply((fetchedValue) -> @@ -270,10 +271,12 @@ protected CompletableFuture fetchField(ExecutionContext executionC Instrumentation instrumentation = executionContext.getInstrumentation(); InstrumentationFieldFetchParameters instrumentationFieldFetchParams = new InstrumentationFieldFetchParameters(executionContext, environment, parameters, dataFetcher instanceof TrivialDataFetcher); - InstrumentationContext fetchCtx = instrumentation.beginFieldFetch(instrumentationFieldFetchParams, executionContext.getInstrumentationState()); + InstrumentationContext fetchCtx = nonNullCtx(instrumentation.beginFieldFetch(instrumentationFieldFetchParams, + executionContext.getInstrumentationState()) + ); CompletableFuture fetchedValue; - dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, instrumentationFieldFetchParams); + dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, instrumentationFieldFetchParams, executionContext.getInstrumentationState()); ExecutionId executionId = executionContext.getExecutionId(); try { Object fetchedValueRaw = dataFetcher.get(environment); @@ -384,9 +387,9 @@ protected FieldValueInfo completeField(ExecutionContext executionContext, Execut Instrumentation instrumentation = executionContext.getInstrumentation(); InstrumentationFieldCompleteParameters instrumentationParams = new InstrumentationFieldCompleteParameters(executionContext, parameters, () -> executionStepInfo, fetchedValue); - InstrumentationContext ctxCompleteField = instrumentation.beginFieldComplete( + InstrumentationContext ctxCompleteField = nonNullCtx(instrumentation.beginFieldComplete( instrumentationParams, executionContext.getInstrumentationState() - ); + )); NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, executionStepInfo); @@ -518,9 +521,9 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext, InstrumentationFieldCompleteParameters instrumentationParams = new InstrumentationFieldCompleteParameters(executionContext, parameters, () -> executionStepInfo, iterableValues); Instrumentation instrumentation = executionContext.getInstrumentation(); - InstrumentationContext completeListCtx = instrumentation.beginFieldListComplete( + InstrumentationContext completeListCtx = nonNullCtx(instrumentation.beginFieldListComplete( instrumentationParams, executionContext.getInstrumentationState() - ); + )); List fieldValueInfos = new ArrayList<>(size.orElse(1)); int index = 0; diff --git a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java index c6551576db..abd71569e2 100644 --- a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java +++ b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java @@ -20,6 +20,7 @@ import java.util.function.Function; import static graphql.Assert.assertTrue; +import static graphql.execution.instrumentation.SimpleInstrumentationContext.nonNullCtx; import static java.util.Collections.singletonMap; /** @@ -48,7 +49,10 @@ public CompletableFuture execute(ExecutionContext executionCont Instrumentation instrumentation = executionContext.getInstrumentation(); InstrumentationExecutionStrategyParameters instrumentationParameters = new InstrumentationExecutionStrategyParameters(executionContext, parameters); - ExecutionStrategyInstrumentationContext executionStrategyCtx = instrumentation.beginExecutionStrategy(instrumentationParameters); + ExecutionStrategyInstrumentationContext executionStrategyCtx = ExecutionStrategyInstrumentationContext.nonNullCtx(instrumentation.beginExecutionStrategy( + instrumentationParameters, + executionContext.getInstrumentationState() + )); CompletableFuture> sourceEventStream = createSourceEventStream(executionContext, parameters); @@ -123,9 +127,9 @@ private CompletableFuture executeSubscriptionEvent(ExecutionCon ExecutionStepInfo subscribedFieldStepInfo = createSubscribedFieldStepInfo(executionContext, newParameters); InstrumentationFieldParameters i13nFieldParameters = new InstrumentationFieldParameters(executionContext, () -> subscribedFieldStepInfo); - InstrumentationContext subscribedFieldCtx = instrumentation.beginSubscribedFieldEvent( + InstrumentationContext subscribedFieldCtx = nonNullCtx(instrumentation.beginSubscribedFieldEvent( i13nFieldParameters, executionContext.getInstrumentationState() - ); + )); FetchedValue fetchedValue = unboxPossibleDataFetcherResult(newExecutionContext, parameters, eventPayload); FieldValueInfo fieldValueInfo = completeField(newExecutionContext, newParameters, fetchedValue); @@ -141,7 +145,7 @@ private CompletableFuture executeSubscriptionEvent(ExecutionCon InstrumentationExecutionParameters i13nExecutionParameters = new InstrumentationExecutionParameters( executionContext.getExecutionInput(), executionContext.getGraphQLSchema(), executionContext.getInstrumentationState()); - overallResult = overallResult.thenCompose(executionResult -> instrumentation.instrumentExecutionResult(executionResult, i13nExecutionParameters)); + overallResult = overallResult.thenCompose(executionResult -> instrumentation.instrumentExecutionResult(executionResult, i13nExecutionParameters, executionContext.getInstrumentationState())); return overallResult; } diff --git a/src/main/java/graphql/execution/instrumentation/ExecutionStrategyInstrumentationContext.java b/src/main/java/graphql/execution/instrumentation/ExecutionStrategyInstrumentationContext.java index da11626bc3..47186ea11b 100644 --- a/src/main/java/graphql/execution/instrumentation/ExecutionStrategyInstrumentationContext.java +++ b/src/main/java/graphql/execution/instrumentation/ExecutionStrategyInstrumentationContext.java @@ -1,10 +1,13 @@ package graphql.execution.instrumentation; import graphql.ExecutionResult; +import graphql.Internal; import graphql.PublicSpi; import graphql.execution.FieldValueInfo; +import javax.annotation.Nonnull; import java.util.List; +import java.util.concurrent.CompletableFuture; @PublicSpi public interface ExecutionStrategyInstrumentationContext extends InstrumentationContext { @@ -17,4 +20,29 @@ default void onFieldValuesException() { } + /** + * This creates a no-op {@link InstrumentationContext} if the one pass in is null + * + * @param nullableContext a {@link InstrumentationContext} that can be null + * @param for two + * + * @return a non null {@link InstrumentationContext} that maybe a no-op + */ + @Nonnull + @Internal + static ExecutionStrategyInstrumentationContext nonNullCtx(ExecutionStrategyInstrumentationContext nullableContext) { + return nullableContext == null ? NOOP : nullableContext; + } + + @Internal + ExecutionStrategyInstrumentationContext NOOP = new ExecutionStrategyInstrumentationContext() { + @Override + public void onDispatched(CompletableFuture result) { + } + + @Override + public void onCompleted(ExecutionResult result, Throwable t) { + } + }; + } diff --git a/src/main/java/graphql/execution/instrumentation/Instrumentation.java b/src/main/java/graphql/execution/instrumentation/Instrumentation.java index 82532b59df..0b1cd9a7e2 100644 --- a/src/main/java/graphql/execution/instrumentation/Instrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/Instrumentation.java @@ -17,6 +17,8 @@ import graphql.schema.GraphQLSchema; import graphql.validation.ValidationError; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -43,7 +45,10 @@ public interface Instrumentation { * to allow them to have per execution request state * * @return a state object that is passed to each method + * + * @deprecated use {@link #createState(InstrumentationCreateStateParameters)} instead */ + @Deprecated default InstrumentationState createState() { return null; } @@ -56,56 +61,143 @@ default InstrumentationState createState() { * * @return a state object that is passed to each method */ + @Nullable default InstrumentationState createState(InstrumentationCreateStateParameters parameters) { return createState(); } /** - * This is called right at the start of query execution and its the first step in the instrumentation chain. + * This is called right at the start of query execution, and it's the first step in the instrumentation chain. * * @param parameters the parameters to this step * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends + * + * @deprecated use {@link #beginExecution(InstrumentationExecutionParameters, InstrumentationState)} instead */ + @Deprecated + @Nonnull InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters); + /** + * This is called right at the start of query execution, and it's the first step in the instrumentation chain. + * + * @param parameters the parameters to this step + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) + */ + @Nullable + default InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, InstrumentationState state) { + return beginExecution(parameters.withNewState(state)); + } + /** * This is called just before a query is parsed. * * @param parameters the parameters to this step * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends + * + * @deprecated use {@link #beginParse(InstrumentationExecutionParameters, InstrumentationState)} instead */ + @Deprecated + @Nonnull InstrumentationContext beginParse(InstrumentationExecutionParameters parameters); + /** + * This is called just before a query is parsed. + * + * @param parameters the parameters to this step + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) + */ + @Nullable + default InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, InstrumentationState state) { + return beginParse(parameters.withNewState(state)); + } + /** * This is called just before the parsed query document is validated. * * @param parameters the parameters to this step * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends + * + * @deprecated use {@link #beginValidation(InstrumentationValidationParameters, InstrumentationState)} instead */ + @Deprecated + @Nonnull InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters); + /** + * This is called just before the parsed query document is validated. + * + * @param parameters the parameters to this step + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) + */ + @Nullable + default InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, InstrumentationState state) { + return beginValidation(parameters.withNewState(state)); + } + /** * This is called just before the execution of the query operation is started. * * @param parameters the parameters to this step * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends + * + * @deprecated use {@link #beginExecuteOperation(InstrumentationExecuteOperationParameters, InstrumentationState)} instead */ + @Deprecated + @Nonnull InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters); + /** + * This is called just before the execution of the query operation is started. + * + * @param parameters the parameters to this step + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) + */ + @Nullable + default InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, InstrumentationState state) { + return beginExecuteOperation(parameters.withNewState(state)); + } + /** * This is called each time an {@link graphql.execution.ExecutionStrategy} is invoked, which may be multiple times * per query as the engine recursively descends down over the query. * * @param parameters the parameters to this step * - * @return a non null {@link InstrumentationContext} object that will be called back when the step ends + * @return a non null {@link ExecutionStrategyInstrumentationContext} object that will be called back when the step ends + * + * @deprecated use {@link #beginExecutionStrategy(InstrumentationExecutionStrategyParameters, InstrumentationState)} instead */ + @Deprecated + @Nonnull ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters); + /** + * This is called each time an {@link graphql.execution.ExecutionStrategy} is invoked, which may be multiple times + * per query as the engine recursively descends down over the query. + * + * @param parameters the parameters to this step + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a nullable {@link ExecutionStrategyInstrumentationContext} object that will be called back when the step ends (assuming it's not null) + */ + @Nullable + default ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + return beginExecutionStrategy(parameters.withNewState(state)); + } + /** * This is called each time a subscription field produces a new reactive stream event value and it needs to be mapped over via the graphql field subselection. @@ -113,12 +205,24 @@ default InstrumentationState createState(InstrumentationCreateStateParameters pa * @param parameters the parameters to this step * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends + * + * @deprecated use {@link #beginSubscribedFieldEvent(InstrumentationFieldParameters, InstrumentationState)} instead */ @Deprecated + @Nonnull default InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters) { return noOp(); } + /** + * This is called each time a subscription field produces a new reactive stream event value and it needs to be mapped over via the graphql field subselection. + * + * @param parameters the parameters to this step + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) + */ + @Nullable default InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) { return beginSubscribedFieldEvent(parameters.withNewState(state)); } @@ -129,10 +233,22 @@ default InstrumentationContext beginSubscribedFieldEvent(Instru * @param parameters the parameters to this step * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends + * + * @deprecated use {@link #beginField(InstrumentationFieldParameters, InstrumentationState)} instead */ @Deprecated + @Nonnull InstrumentationContext beginField(InstrumentationFieldParameters parameters); + /** + * This is called just before a field is resolved into a value. + * + * @param parameters the parameters to this step + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) + */ + @Nullable default InstrumentationContext beginField(InstrumentationFieldParameters parameters, InstrumentationState state) { return beginField(parameters.withNewState(state)); } @@ -143,10 +259,22 @@ default InstrumentationContext beginField(InstrumentationFieldP * @param parameters the parameters to this step * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends + * + * @deprecated use {@link #beginFieldFetch(InstrumentationFieldFetchParameters, InstrumentationState)} instead */ @Deprecated + @Nonnull InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters); + /** + * This is called just before a field {@link DataFetcher} is invoked. + * + * @param parameters the parameters to this step + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) + */ + @Nullable default InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { return beginFieldFetch(parameters.withNewState(state)); } @@ -158,12 +286,24 @@ default InstrumentationContext beginFieldFetch(InstrumentationFieldFetch * @param parameters the parameters to this step * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends + * + * @deprecated use {@link #beginFieldComplete(InstrumentationFieldCompleteParameters, InstrumentationState)} instead */ @Deprecated + @Nonnull default InstrumentationContext beginFieldComplete(InstrumentationFieldCompleteParameters parameters) { return noOp(); } + /** + * This is called just before the complete field is started. + * + * @param parameters the parameters to this step + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) + */ + @Nullable default InstrumentationContext beginFieldComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { return beginFieldComplete(parameters.withNewState(state)); } @@ -174,12 +314,24 @@ default InstrumentationContext beginFieldComplete(Instrumentati * @param parameters the parameters to this step * * @return a non null {@link InstrumentationContext} object that will be called back when the step ends + * + * @deprecated use {@link #beginFieldListComplete(InstrumentationFieldCompleteParameters, InstrumentationState)} instead */ @Deprecated + @Nonnull default InstrumentationContext beginFieldListComplete(InstrumentationFieldCompleteParameters parameters) { return noOp(); } + /** + * This is called just before the complete field list is started. + * + * @param parameters the parameters to this step + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) + */ + @Nullable default InstrumentationContext beginFieldListComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { return beginFieldListComplete(parameters.withNewState(state)); } @@ -192,11 +344,30 @@ default InstrumentationContext beginFieldListComplete(Instrumen * @param parameters the parameters describing the field to be fetched * * @return a non null instrumented ExecutionInput, the default is to return to the same object + * + * @deprecated use {@link #instrumentExecutionInput(ExecutionInput, InstrumentationExecutionParameters, InstrumentationState)} instead */ + @Deprecated + @Nonnull default ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters) { return executionInput; } + /** + * This is called to instrument a {@link graphql.ExecutionInput} before it is used to parse, validate + * and execute a query, allowing you to adjust what query input parameters are used + * + * @param executionInput the execution input to be used + * @param parameters the parameters describing the field to be fetched + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a non null instrumented ExecutionInput, the default is to return to the same object + */ + @Nonnull + default ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters, InstrumentationState state) { + return instrumentExecutionInput(executionInput, parameters.withNewState(state)); + } + /** * This is called to instrument a {@link graphql.language.Document} and variables before it is used allowing you to adjust the query AST if you so desire * @@ -204,11 +375,29 @@ default ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, I * @param parameters the parameters describing the execution * * @return a non null instrumented DocumentAndVariables, the default is to return to the same objects + * + * @deprecated use {@link #instrumentDocumentAndVariables(DocumentAndVariables, InstrumentationExecutionParameters, InstrumentationState)} instead */ + @Deprecated + @Nonnull default DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters) { return documentAndVariables; } + /** + * This is called to instrument a {@link graphql.language.Document} and variables before it is used allowing you to adjust the query AST if you so desire + * + * @param documentAndVariables the document and variables to be used + * @param parameters the parameters describing the execution + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a non null instrumented DocumentAndVariables, the default is to return to the same objects + */ + @Nonnull + default DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters, InstrumentationState state) { + return instrumentDocumentAndVariables(documentAndVariables, parameters.withNewState(state)); + } + /** * This is called to instrument a {@link graphql.schema.GraphQLSchema} before it is used to parse, validate * and execute a query, allowing you to adjust what types are used. @@ -217,11 +406,30 @@ default DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables * @param parameters the parameters describing the field to be fetched * * @return a non null instrumented GraphQLSchema, the default is to return to the same object + * + * @deprecated use {@link #instrumentSchema(GraphQLSchema, InstrumentationExecutionParameters, InstrumentationState)} instead */ + @Deprecated + @Nonnull default GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExecutionParameters parameters) { return schema; } + /** + * This is called to instrument a {@link graphql.schema.GraphQLSchema} before it is used to parse, validate + * and execute a query, allowing you to adjust what types are used. + * + * @param schema the schema to be used + * @param parameters the parameters describing the field to be fetched + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a non null instrumented GraphQLSchema, the default is to return to the same object + */ + @Nonnull + default GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExecutionParameters parameters, InstrumentationState state) { + return instrumentSchema(schema, parameters.withNewState(state)); + } + /** * This is called to instrument a {@link ExecutionContext} before it is used to execute a query, * allowing you to adjust the base data used. @@ -230,11 +438,30 @@ default GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExec * @param parameters the parameters describing the field to be fetched * * @return a non null instrumented ExecutionContext, the default is to return to the same object + * + * @deprecated use {@link #instrumentExecutionContext(ExecutionContext, InstrumentationExecutionParameters)} instead */ + @Deprecated + @Nonnull default ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, InstrumentationExecutionParameters parameters) { return executionContext; } + /** + * This is called to instrument a {@link ExecutionContext} before it is used to execute a query, + * allowing you to adjust the base data used. + * + * @param executionContext the execution context to be used + * @param parameters the parameters describing the field to be fetched + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a non null instrumented ExecutionContext, the default is to return to the same object + */ + @Nonnull + default ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, InstrumentationExecutionParameters parameters, InstrumentationState state) { + return instrumentExecutionContext(executionContext, parameters.withNewState(state)); + } + /** * This is called to instrument a {@link DataFetcher} just before it is used to fetch a field, allowing you @@ -246,11 +473,32 @@ default ExecutionContext instrumentExecutionContext(ExecutionContext executionCo * @param parameters the parameters describing the field to be fetched * * @return a non null instrumented DataFetcher, the default is to return to the same object + * + * @deprecated use {@link #instrumentDataFetcher(DataFetcher, InstrumentationFieldFetchParameters, InstrumentationState)} instead */ + @Deprecated + @Nonnull default DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters) { return dataFetcher; } + /** + * This is called to instrument a {@link DataFetcher} just before it is used to fetch a field, allowing you + * to adjust what information is passed back or record information about specific data fetches. Note + * the same data fetcher instance maybe presented to you many times and that data fetcher + * implementations widely vary. + * + * @param dataFetcher the data fetcher about to be used + * @param parameters the parameters describing the field to be fetched + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a non null instrumented DataFetcher, the default is to return to the same object + */ + @Nonnull + default DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + return instrumentDataFetcher(dataFetcher, parameters.withNewState(state)); + } + /** * This is called to allow instrumentation to instrument the execution result in some way * @@ -258,9 +506,26 @@ default DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, Instrum * @param parameters the parameters to this step * * @return a new execution result completable future + * + * @deprecated use {@link #instrumentExecutionResult(ExecutionResult, InstrumentationExecutionParameters, InstrumentationState)} instead */ + @Deprecated + @Nonnull default CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters) { return CompletableFuture.completedFuture(executionResult); } + /** + * This is called to allow instrumentation to instrument the execution result in some way + * + * @param executionResult {@link java.util.concurrent.CompletableFuture} of the result to instrument + * @param parameters the parameters to this step + * @param state the state created during the call to {@link #createState(InstrumentationCreateStateParameters)} + * + * @return a new execution result completable future + */ + @Nonnull + default CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, InstrumentationState state) { + return instrumentExecutionResult(executionResult, parameters.withNewState(state)); + } } diff --git a/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java b/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java index 575b143b14..10b9cb5933 100644 --- a/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java +++ b/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java @@ -2,6 +2,7 @@ import graphql.PublicApi; +import javax.annotation.Nonnull; import java.util.concurrent.CompletableFuture; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -26,6 +27,19 @@ public static InstrumentationContext noOp() { return (InstrumentationContext) NO_OP; } + /** + * This creates a no-op {@link InstrumentationContext} if the one pass in is null + * + * @param nullableContext a {@link InstrumentationContext} that can be null + * @param for two + * + * @return a non null {@link InstrumentationContext} that maybe a no-op + */ + @Nonnull + public static InstrumentationContext nonNullCtx(InstrumentationContext nullableContext) { + return nullableContext == null ? noOp() : nullableContext; + } + private final BiConsumer codeToRunOnComplete; private final Consumer> codeToRunOnDispatch; @@ -86,7 +100,7 @@ public static SimpleInstrumentationContext whenCompleted(BiConsumer for two + * + * @return the state created previously during a call to {@link Instrumentation#createState(InstrumentationCreateStateParameters)} + * + * @deprecated state is now passed in direct to instrumentation methods + */ + @Deprecated public T getInstrumentationState() { //noinspection unchecked return (T) instrumentationState; diff --git a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationExecutionParameters.java b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationExecutionParameters.java index 957d9e2c48..743f21515a 100644 --- a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationExecutionParameters.java +++ b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationExecutionParameters.java @@ -41,7 +41,10 @@ public InstrumentationExecutionParameters(ExecutionInput executionInput, GraphQL * @param instrumentationState the new state for this parameters object * * @return a new parameters object with the new state + * + * @deprecated state is now passed in direct to instrumentation methods */ + @Deprecated public InstrumentationExecutionParameters withNewState(InstrumentationState instrumentationState) { return new InstrumentationExecutionParameters(this.getExecutionInput(), this.schema, instrumentationState); } @@ -60,6 +63,7 @@ public String getOperation() { /** * @param for two + * * @return the legacy context * * @deprecated use {@link #getGraphQLContext()} instead @@ -78,6 +82,17 @@ public Map getVariables() { return variables; } + /** + * Previously the instrumentation parameters had access to the state created via {@link Instrumentation#createState(InstrumentationCreateStateParameters)} but now + * to save object allocations, the state is passed directly into instrumentation methods + * + * @param for two + * + * @return the state created previously during a call to {@link Instrumentation#createState(InstrumentationCreateStateParameters)} + * + * @deprecated state is now passed in direct to instrumentation methods + */ + @Deprecated @SuppressWarnings("TypeParameterUnusedInFormals") public T getInstrumentationState() { //noinspection unchecked diff --git a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationExecutionStrategyParameters.java b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationExecutionStrategyParameters.java index a018832c29..dee0bbf3e5 100644 --- a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationExecutionStrategyParameters.java +++ b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationExecutionStrategyParameters.java @@ -3,6 +3,7 @@ import graphql.PublicApi; import graphql.execution.ExecutionContext; import graphql.execution.ExecutionStrategyParameters; +import graphql.execution.instrumentation.Instrumentation; import graphql.execution.instrumentation.InstrumentationState; /** @@ -31,7 +32,10 @@ private InstrumentationExecutionStrategyParameters(ExecutionContext executionCon * @param instrumentationState the new state for this parameters object * * @return a new parameters object with the new state + * + * @deprecated state is now passed in direct to instrumentation methods */ + @Deprecated public InstrumentationExecutionStrategyParameters withNewState(InstrumentationState instrumentationState) { return new InstrumentationExecutionStrategyParameters(executionContext, executionStrategyParameters, instrumentationState); } @@ -44,6 +48,17 @@ public ExecutionStrategyParameters getExecutionStrategyParameters() { return executionStrategyParameters; } + /** + * Previously the instrumentation parameters had access to the state created via {@link Instrumentation#createState(InstrumentationCreateStateParameters)} but now + * to save object allocations, the state is passed directly into instrumentation methods + * + * @param for two + * + * @return the state created previously during a call to {@link Instrumentation#createState(InstrumentationCreateStateParameters)} + * + * @deprecated state is now passed in direct to instrumentation methods + */ + @Deprecated @SuppressWarnings("TypeParameterUnusedInFormals") public T getInstrumentationState() { //noinspection unchecked diff --git a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java index 6a04fc18ca..b7221468dd 100644 --- a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java +++ b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java @@ -4,6 +4,7 @@ import graphql.execution.ExecutionContext; import graphql.execution.ExecutionStepInfo; import graphql.execution.ExecutionStrategyParameters; +import graphql.execution.instrumentation.Instrumentation; import graphql.execution.instrumentation.InstrumentationState; import graphql.schema.GraphQLFieldDefinition; @@ -36,8 +37,12 @@ public InstrumentationFieldCompleteParameters(ExecutionContext executionContext, * Returns a cloned parameters object with the new state * * @param instrumentationState the new state for this parameters object + * * @return a new parameters object with the new state + * + * @deprecated state is now passed in direct to instrumentation methods */ + @Deprecated public InstrumentationFieldCompleteParameters withNewState(InstrumentationState instrumentationState) { return new InstrumentationFieldCompleteParameters( this.executionContext, executionStrategyParameters, this.executionStepInfo, this.fetchedValue, instrumentationState); @@ -69,6 +74,17 @@ public Object getFetchedValue() { return fetchedValue; } + /** + * Previously the instrumentation parameters had access to the state created via {@link Instrumentation#createState(InstrumentationCreateStateParameters)} but now + * to save object allocations, the state is passed directly into instrumentation methods + * + * @param for two + * + * @return the state created previously during a call to {@link Instrumentation#createState(InstrumentationCreateStateParameters)} + * + * @deprecated state is now passed in direct to instrumentation methods + */ + @Deprecated @SuppressWarnings("TypeParameterUnusedInFormals") public T getInstrumentationState() { //noinspection unchecked diff --git a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldFetchParameters.java b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldFetchParameters.java index 256ad1bf53..76fc770c51 100644 --- a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldFetchParameters.java +++ b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldFetchParameters.java @@ -36,7 +36,10 @@ private InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext * @param instrumentationState the new state for this parameters object * * @return a new parameters object with the new state + * + * @deprecated state is now passed in direct to instrumentation methods */ + @Deprecated @Override public InstrumentationFieldFetchParameters withNewState(InstrumentationState instrumentationState) { return new InstrumentationFieldFetchParameters( diff --git a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldParameters.java b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldParameters.java index 90efbdc119..a6d6f0bfea 100644 --- a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldParameters.java +++ b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldParameters.java @@ -32,8 +32,12 @@ public InstrumentationFieldParameters(ExecutionContext executionContext, Supplie * Returns a cloned parameters object with the new state * * @param instrumentationState the new state for this parameters object + * * @return a new parameters object with the new state + * + * @deprecated state is now passed in direct to instrumentation methods */ + @Deprecated public InstrumentationFieldParameters withNewState(InstrumentationState instrumentationState) { return new InstrumentationFieldParameters( this.executionContext, this.executionStepInfo, instrumentationState); @@ -52,6 +56,17 @@ public ExecutionStepInfo getExecutionStepInfo() { return executionStepInfo.get(); } + /** + * Previously the instrumentation parameters had access to the state created via {@link Instrumentation#createState(InstrumentationCreateStateParameters)} but now + * to save object allocations, the state is passed directly into instrumentation methods + * + * @param for two + * + * @return the state created previously during a call to {@link Instrumentation#createState(InstrumentationCreateStateParameters)} + * + * @deprecated state is now passed in direct to instrumentation methods + */ + @Deprecated @SuppressWarnings("TypeParameterUnusedInFormals") public T getInstrumentationState() { //noinspection unchecked diff --git a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationValidationParameters.java b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationValidationParameters.java index 54827ab01d..230749b70c 100644 --- a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationValidationParameters.java +++ b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationValidationParameters.java @@ -25,7 +25,10 @@ public InstrumentationValidationParameters(ExecutionInput executionInput, Docume * @param instrumentationState the new state for this parameters object * * @return a new parameters object with the new state + * + * @deprecated state is now passed in direct to instrumentation methods */ + @Deprecated @Override public InstrumentationValidationParameters withNewState(InstrumentationState instrumentationState) { return new InstrumentationValidationParameters( From 105439aa3d5133c020b4ccc3413317209a5c8bdf Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Tue, 22 Mar 2022 17:05:51 +1100 Subject: [PATCH 4/6] Javadoc error --- .../instrumentation/ExecutionStrategyInstrumentationContext.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/graphql/execution/instrumentation/ExecutionStrategyInstrumentationContext.java b/src/main/java/graphql/execution/instrumentation/ExecutionStrategyInstrumentationContext.java index 47186ea11b..44a75ffc0b 100644 --- a/src/main/java/graphql/execution/instrumentation/ExecutionStrategyInstrumentationContext.java +++ b/src/main/java/graphql/execution/instrumentation/ExecutionStrategyInstrumentationContext.java @@ -24,7 +24,6 @@ default void onFieldValuesException() { * This creates a no-op {@link InstrumentationContext} if the one pass in is null * * @param nullableContext a {@link InstrumentationContext} that can be null - * @param for two * * @return a non null {@link InstrumentationContext} that maybe a no-op */ From 5be3fc3a46964d00150679f9e8f6aa423677e3fe Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Thu, 24 Mar 2022 22:33:24 +1100 Subject: [PATCH 5/6] Added ChainedInstrumentation updates and deprecated all the old methods. --- src/main/java/graphql/GraphQL.java | 4 + .../java/graphql/collect/ImmutableKit.java | 25 +++ .../ChainedInstrumentation.java | 189 ++++++++++++------ .../instrumentation/Instrumentation.java | 28 ++- .../NoContextChainedInstrumentation.java | 109 ++++++++++ .../SimpleInstrumentationContext.java | 10 +- .../graphql/collect/ImmutableKitTest.groovy | 10 + .../SubscriptionExecutionStrategyTest.groovy | 4 +- .../AllNullTestingInstrumentation.groovy | 148 ++++++++++++++ .../ChainedInstrumentationStateTest.groovy | 181 ++++++++++------- .../InstrumentationTest.groovy | 140 ++++++++++++- ...vy => LegacyTestingInstrumentation.groovy} | 5 +- .../ModernTestingInstrumentation.groovy | 143 +++++++++++++ .../NamedInstrumentation.groovy | 95 +++++++++ ...NoContextChainedInstrumentationTest.groovy | 87 ++++++++ .../PreparsedDocumentProviderTest.groovy | 6 +- 16 files changed, 1033 insertions(+), 151 deletions(-) create mode 100644 src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java create mode 100644 src/test/groovy/graphql/execution/instrumentation/AllNullTestingInstrumentation.groovy rename src/test/groovy/graphql/execution/instrumentation/{TestingInstrumentation.groovy => LegacyTestingInstrumentation.groovy} (97%) create mode 100644 src/test/groovy/graphql/execution/instrumentation/ModernTestingInstrumentation.groovy create mode 100644 src/test/groovy/graphql/execution/instrumentation/NamedInstrumentation.groovy create mode 100644 src/test/groovy/graphql/execution/instrumentation/NoContextChainedInstrumentationTest.groovy diff --git a/src/main/java/graphql/GraphQL.java b/src/main/java/graphql/GraphQL.java index 91246aab6e..84fdbd9a28 100644 --- a/src/main/java/graphql/GraphQL.java +++ b/src/main/java/graphql/GraphQL.java @@ -16,6 +16,7 @@ import graphql.execution.instrumentation.Instrumentation; import graphql.execution.instrumentation.InstrumentationContext; import graphql.execution.instrumentation.InstrumentationState; +import graphql.execution.instrumentation.NoContextChainedInstrumentation; import graphql.execution.instrumentation.SimpleInstrumentation; import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation; import graphql.execution.instrumentation.parameters.InstrumentationCreateStateParameters; @@ -664,6 +665,9 @@ private static Instrumentation checkInstrumentationDefaultState(Instrumentation if (instrumentation instanceof DataLoaderDispatcherInstrumentation) { return instrumentation; } + if (instrumentation instanceof NoContextChainedInstrumentation) { + return instrumentation; + } if (instrumentation == null) { return new DataLoaderDispatcherInstrumentation(); } diff --git a/src/main/java/graphql/collect/ImmutableKit.java b/src/main/java/graphql/collect/ImmutableKit.java index 3c741401c3..d6b2a5025c 100644 --- a/src/main/java/graphql/collect/ImmutableKit.java +++ b/src/main/java/graphql/collect/ImmutableKit.java @@ -87,6 +87,31 @@ public static ImmutableList map(Iterable iterable, Functi return builder.build(); } + /** + * This will map an iterable of items but drop any that are null from the mapped list + * + * This is more efficient than `c.stream().map().collect()` because it does not create the intermediate objects needed + * for the flexible style. Benchmarking has shown this to outperform `stream()`. + * + * @param iterable the iterable to map + * @param mapper the mapper function + * @param for two + * @param for result + * + * @return a map immutable list of results + */ + public static ImmutableList mapAndDropNulls(Iterable iterable, Function mapper) { + ImmutableList.Builder builder = ImmutableList.builder(); + for (T t : iterable) { + R r = mapper.apply(t); + if (r != null) { + builder.add(r); + } + } + return builder.build(); + } + + /** * This constructs a new Immutable list from an existing collection and adds a new element to it. * diff --git a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java index c0bea68285..59b2d1012c 100644 --- a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java @@ -1,6 +1,7 @@ package graphql.execution.instrumentation; import com.google.common.collect.ImmutableList; +import graphql.Assert; import graphql.ExecutionInput; import graphql.ExecutionResult; import graphql.PublicApi; @@ -19,6 +20,7 @@ import graphql.schema.DataFetcher; import graphql.schema.GraphQLSchema; import graphql.validation.ValidationError; +import org.jetbrains.annotations.NotNull; import java.util.Arrays; import java.util.LinkedHashMap; @@ -27,7 +29,7 @@ import java.util.concurrent.CompletableFuture; import static graphql.Assert.assertNotNull; -import static graphql.collect.ImmutableKit.map; +import static graphql.collect.ImmutableKit.mapAndDropNulls; /** * This allows you to chain together a number of {@link graphql.execution.instrumentation.Instrumentation} implementations @@ -42,7 +44,7 @@ public class ChainedInstrumentation implements Instrumentation { // This class is inspired from https://github.com/leangen/graphql-spqr/blob/master/src/main/java/io/leangen/graphql/GraphQLRuntime.java#L80 - private final ImmutableList instrumentations; + protected final ImmutableList instrumentations; public ChainedInstrumentation(List instrumentations) { this.instrumentations = ImmutableList.copyOf(assertNotNull(instrumentations)); @@ -59,7 +61,7 @@ public List getInstrumentations() { return instrumentations; } - private InstrumentationState getState(Instrumentation instrumentation, InstrumentationState parametersInstrumentationState) { + protected InstrumentationState getSpecificState(Instrumentation instrumentation, InstrumentationState parametersInstrumentationState) { ChainedInstrumentationState chainedInstrumentationState = (ChainedInstrumentationState) parametersInstrumentationState; return chainedInstrumentationState.getState(instrumentation); } @@ -70,167 +72,230 @@ public InstrumentationState createState(InstrumentationCreateStateParameters par } @Override - public InstrumentationContext beginExecution(final InstrumentationExecutionParameters parameters) { - return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - return instrumentation.beginExecution(parameters.withNewState(state)); + public InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters) { + // these assert methods have been left in so that we truly never call these methods, either in production nor in tests + // later when the deprecated methods are removed, this will disappear. + return Assert.assertShouldNeverHappen("The deprecated " + "beginExecution" + " was called"); + } + + @Override + public InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, InstrumentationState state) { + return new ChainedInstrumentationContext<>(mapAndDropNulls(instrumentations, instrumentation -> { + InstrumentationState specificState = getSpecificState(instrumentation, state); + return instrumentation.beginExecution(parameters, specificState); })); } @Override public InstrumentationContext beginParse(InstrumentationExecutionParameters parameters) { - return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - return instrumentation.beginParse(parameters.withNewState(state)); + return Assert.assertShouldNeverHappen("The deprecated " + "beginParse" + " was called"); + } + + @Override + public InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, InstrumentationState state) { + return new ChainedInstrumentationContext<>(mapAndDropNulls(instrumentations, instrumentation -> { + InstrumentationState specificState = getSpecificState(instrumentation, state); + return instrumentation.beginParse(parameters, specificState); })); } @Override public InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters) { - return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - return instrumentation.beginValidation(parameters.withNewState(state)); + return Assert.assertShouldNeverHappen("The deprecated " + "beginValidation" + " was called"); + } + + @Override + public InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, InstrumentationState state) { + return new ChainedInstrumentationContext<>(mapAndDropNulls(instrumentations, instrumentation -> { + InstrumentationState specificState = getSpecificState(instrumentation, state); + return instrumentation.beginValidation(parameters, specificState); })); } @Override public InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters) { - return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - return instrumentation.beginExecuteOperation(parameters.withNewState(state)); + return Assert.assertShouldNeverHappen("The deprecated " + "beginExecuteOperation" + " was called"); + } + + @Override + public InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, InstrumentationState state) { + return new ChainedInstrumentationContext<>(mapAndDropNulls(instrumentations, instrumentation -> { + InstrumentationState specificState = getSpecificState(instrumentation, state); + return instrumentation.beginExecuteOperation(parameters, specificState); })); + } @Override public ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters) { - return new ChainedExecutionStrategyInstrumentationContext(map(instrumentations, instrumentation -> { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - return instrumentation.beginExecutionStrategy(parameters.withNewState(state)); - })); + return Assert.assertShouldNeverHappen("The deprecated " + "beginExecutionStrategy" + " was called"); } + @Override + public ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + return new ChainedExecutionStrategyInstrumentationContext(mapAndDropNulls(instrumentations, instrumentation -> { + InstrumentationState specificState = getSpecificState(instrumentation, state); + return instrumentation.beginExecutionStrategy(parameters, specificState); + })); + } @Override public InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters) { - return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - return instrumentation.beginSubscribedFieldEvent(parameters.withNewState(state)); - })); + return Assert.assertShouldNeverHappen("The deprecated " + "beginSubscribedFieldEvent" + " was called"); } @Override public InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) { - return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginSubscribedFieldEvent(parameters, getState(it, state)))); + return new ChainedInstrumentationContext<>(mapAndDropNulls(instrumentations, instrumentation -> { + InstrumentationState specificState = getSpecificState(instrumentation, state); + return instrumentation.beginSubscribedFieldEvent(parameters, specificState); + })); } @Override public InstrumentationContext beginField(InstrumentationFieldParameters parameters) { - return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - return instrumentation.beginField(parameters.withNewState(state)); - })); + return Assert.assertShouldNeverHappen("The deprecated " + "beginField" + " was called"); } @Override public InstrumentationContext beginField(InstrumentationFieldParameters parameters, InstrumentationState state) { - return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginField(parameters, getState(it, state)))); + return new ChainedInstrumentationContext<>(mapAndDropNulls(instrumentations, instrumentation -> { + InstrumentationState specificState = getSpecificState(instrumentation, state); + return instrumentation.beginField(parameters, specificState); + })); } @Override public InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters) { - return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - return instrumentation.beginFieldFetch(parameters.withNewState(state)); - })); + return Assert.assertShouldNeverHappen("The deprecated " + "beginFieldFetch" + " was called"); } @Override public InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { - return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginFieldFetch(parameters, getState(it, state)))); + return new ChainedInstrumentationContext<>(mapAndDropNulls(instrumentations, instrumentation -> { + InstrumentationState specificState = getSpecificState(instrumentation, state); + return instrumentation.beginFieldFetch(parameters, specificState); + })); } @Override public InstrumentationContext beginFieldComplete(InstrumentationFieldCompleteParameters parameters) { - return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - return instrumentation.beginFieldComplete(parameters.withNewState(state)); - })); + return Assert.assertShouldNeverHappen("The deprecated " + "beginFieldComplete" + " was called"); } @Override public InstrumentationContext beginFieldComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { - return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginFieldComplete(parameters, getState(it, state)))); + return new ChainedInstrumentationContext<>(mapAndDropNulls(instrumentations, instrumentation -> { + InstrumentationState specificState = getSpecificState(instrumentation, state); + return instrumentation.beginFieldComplete(parameters, specificState); + })); } @Override public InstrumentationContext beginFieldListComplete(InstrumentationFieldCompleteParameters parameters) { - return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - return instrumentation.beginFieldListComplete(parameters.withNewState(state)); - })); + return Assert.assertShouldNeverHappen("The deprecated " + "beginFieldListComplete" + " was called"); } @Override public InstrumentationContext beginFieldListComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { - return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginFieldListComplete(parameters, getState(it, state)))); + return new ChainedInstrumentationContext<>(mapAndDropNulls(instrumentations, it -> { + InstrumentationState specificState = getSpecificState(it, state); + return it.beginFieldListComplete(parameters, specificState); + })); } @Override public ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters) { + return Assert.assertShouldNeverHappen("The deprecated " + "instrumentExecutionInput" + " was called"); + } + + @NotNull + @Override + public ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters, InstrumentationState state) { for (Instrumentation instrumentation : instrumentations) { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - executionInput = instrumentation.instrumentExecutionInput(executionInput, parameters.withNewState(state)); + InstrumentationState specificState = getSpecificState(instrumentation, state); + executionInput = instrumentation.instrumentExecutionInput(executionInput, parameters, specificState); } return executionInput; } @Override public DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters) { + return Assert.assertShouldNeverHappen("The deprecated " + "instrumentDocumentAndVariables" + " was called"); + } + + @NotNull + @Override + public DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters, InstrumentationState state) { for (Instrumentation instrumentation : instrumentations) { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - documentAndVariables = instrumentation.instrumentDocumentAndVariables(documentAndVariables, parameters.withNewState(state)); + InstrumentationState specificState = getSpecificState(instrumentation, state); + documentAndVariables = instrumentation.instrumentDocumentAndVariables(documentAndVariables, parameters, specificState); } return documentAndVariables; } @Override public GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExecutionParameters parameters) { + return Assert.assertShouldNeverHappen("The deprecated " + "instrumentSchema" + " was called"); + } + + @NotNull + @Override + public GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExecutionParameters parameters, InstrumentationState state) { for (Instrumentation instrumentation : instrumentations) { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - schema = instrumentation.instrumentSchema(schema, parameters.withNewState(state)); + InstrumentationState specificState = getSpecificState(instrumentation, state); + schema = instrumentation.instrumentSchema(schema, parameters, specificState); } return schema; } @Override public ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, InstrumentationExecutionParameters parameters) { + return Assert.assertShouldNeverHappen("The deprecated " + "instrumentExecutionContext" + " was called"); + } + + @NotNull + @Override + public ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, InstrumentationExecutionParameters parameters, InstrumentationState state) { for (Instrumentation instrumentation : instrumentations) { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - executionContext = instrumentation.instrumentExecutionContext(executionContext, parameters.withNewState(state)); + InstrumentationState specificState = getSpecificState(instrumentation, state); + executionContext = instrumentation.instrumentExecutionContext(executionContext, parameters, specificState); } return executionContext; } @Override public DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters) { + return Assert.assertShouldNeverHappen("The deprecated " + "instrumentDataFetcher" + " was called"); + } + + @NotNull + @Override + public DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters, InstrumentationState state) { for (Instrumentation instrumentation : instrumentations) { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); - dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, parameters.withNewState(state)); + InstrumentationState specificState = getSpecificState(instrumentation, state); + dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, parameters, specificState); } return dataFetcher; } @Override public CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters) { + return Assert.assertShouldNeverHappen("The deprecated " + "instrumentExecutionResult" + " was called"); + } + + @NotNull + @Override + public CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, InstrumentationState state) { CompletableFuture> resultsFuture = Async.eachSequentially(instrumentations, (instrumentation, index, prevResults) -> { - InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState()); + InstrumentationState specificState = getSpecificState(instrumentation, state); ExecutionResult lastResult = prevResults.size() > 0 ? prevResults.get(prevResults.size() - 1) : executionResult; - return instrumentation.instrumentExecutionResult(lastResult, parameters.withNewState(state)); + return instrumentation.instrumentExecutionResult(lastResult, parameters, specificState); }); return resultsFuture.thenApply((results) -> results.isEmpty() ? executionResult : results.get(results.size() - 1)); } - private static class ChainedInstrumentationState implements InstrumentationState { + static class ChainedInstrumentationState implements InstrumentationState { private final Map instrumentationStates; @@ -249,8 +314,8 @@ private static class ChainedInstrumentationContext implements Instrumentation private final ImmutableList> contexts; - ChainedInstrumentationContext(List> contexts) { - this.contexts = ImmutableList.copyOf(contexts); + ChainedInstrumentationContext(ImmutableList> contexts) { + this.contexts = contexts; } @Override @@ -268,8 +333,8 @@ private static class ChainedExecutionStrategyInstrumentationContext implements E private final ImmutableList contexts; - ChainedExecutionStrategyInstrumentationContext(List contexts) { - this.contexts = ImmutableList.copyOf(contexts); + ChainedExecutionStrategyInstrumentationContext(ImmutableList contexts) { + this.contexts = contexts; } @Override diff --git a/src/main/java/graphql/execution/instrumentation/Instrumentation.java b/src/main/java/graphql/execution/instrumentation/Instrumentation.java index 0b1cd9a7e2..460855114c 100644 --- a/src/main/java/graphql/execution/instrumentation/Instrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/Instrumentation.java @@ -77,7 +77,9 @@ default InstrumentationState createState(InstrumentationCreateStateParameters pa */ @Deprecated @Nonnull - InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters); + default InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters) { + return noOp(); + } /** * This is called right at the start of query execution, and it's the first step in the instrumentation chain. @@ -103,7 +105,9 @@ default InstrumentationContext beginExecution(InstrumentationEx */ @Deprecated @Nonnull - InstrumentationContext beginParse(InstrumentationExecutionParameters parameters); + default InstrumentationContext beginParse(InstrumentationExecutionParameters parameters) { + return noOp(); + } /** * This is called just before a query is parsed. @@ -129,7 +133,9 @@ default InstrumentationContext beginParse(InstrumentationExecutionPara */ @Deprecated @Nonnull - InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters); + default InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters) { + return noOp(); + } /** * This is called just before the parsed query document is validated. @@ -155,7 +161,9 @@ default InstrumentationContext> beginValidation(Instrument */ @Deprecated @Nonnull - InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters); + default InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters) { + return noOp(); + } /** * This is called just before the execution of the query operation is started. @@ -182,7 +190,9 @@ default InstrumentationContext beginExecuteOperation(Instrument */ @Deprecated @Nonnull - ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters); + default ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters) { + return ExecutionStrategyInstrumentationContext.NOOP; + } /** * This is called each time an {@link graphql.execution.ExecutionStrategy} is invoked, which may be multiple times @@ -238,7 +248,9 @@ default InstrumentationContext beginSubscribedFieldEvent(Instru */ @Deprecated @Nonnull - InstrumentationContext beginField(InstrumentationFieldParameters parameters); + default InstrumentationContext beginField(InstrumentationFieldParameters parameters) { + return noOp(); + } /** * This is called just before a field is resolved into a value. @@ -264,7 +276,9 @@ default InstrumentationContext beginField(InstrumentationFieldP */ @Deprecated @Nonnull - InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters); + default InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters) { + return noOp(); + } /** * This is called just before a field {@link DataFetcher} is invoked. diff --git a/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java new file mode 100644 index 0000000000..72e0783b0c --- /dev/null +++ b/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java @@ -0,0 +1,109 @@ +package graphql.execution.instrumentation; + +import graphql.ExecutionResult; +import graphql.PublicApi; +import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters; +import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters; +import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters; +import graphql.execution.instrumentation.parameters.InstrumentationFieldCompleteParameters; +import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters; +import graphql.execution.instrumentation.parameters.InstrumentationFieldParameters; +import graphql.execution.instrumentation.parameters.InstrumentationValidationParameters; +import graphql.language.Document; +import graphql.validation.ValidationError; + +import java.util.List; +import java.util.function.BiConsumer; + +/** + * This version of {@link ChainedInstrumentation} will call a list of {@link Instrumentation}s + * but it will never back on the returned {@link InstrumentationContext} objects, hence it is only suitable to + * certain use cases. + * + * Only use this class if you are optimising for memory usage as scale. In most cases the {@link ChainedInstrumentation} + * will do the job required with all the instrumentation features used however some users require the fastest performance and lowest memory + * usage at scale and this class can be used. + * + * At scale, the fact that the graphql engine holds the {@link InstrumentationContext} objects in memory for a (relatively) long time + * (the length of the request or the length of a large field fetch) means that memory pressure can grow + * and objects move into longer tenure GC pools. Holding these contexts is also not necessary if the instrumentation never needs to know when a + * certain execution step finishes. + * + * The {@link InstrumentationContext} is used ot know when an execution step has completed, so instrumentations that do + * timings say need to use this callback mechanism. Putting such an instrumentation into {@link NoContextChainedInstrumentation} would + * be a mistake because no callback will occur. Therefore, use of this class is reserved for very specific us cases. You are fore-warned. + * + * This class never holds onto the returned {@link InstrumentationContext} objects and always returns null + * as itself. + */ +@PublicApi +public class NoContextChainedInstrumentation extends ChainedInstrumentation { + + public NoContextChainedInstrumentation(List instrumentations) { + super(instrumentations); + } + + public NoContextChainedInstrumentation(Instrumentation... instrumentations) { + super(instrumentations); + } + + private T runAll(InstrumentationState state, BiConsumer stateConsumer) { + for (Instrumentation instrumentation : instrumentations) { + InstrumentationState specificState = getSpecificState(instrumentation, state); + stateConsumer.accept(instrumentation, specificState); + } + return null; + } + + @Override + public InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, InstrumentationState state) { + return runAll(state, (instrumentation, specificState) -> instrumentation.beginExecution(parameters, specificState)); + } + + @Override + public InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, InstrumentationState state) { + return runAll(state, (instrumentation, specificState) -> instrumentation.beginParse(parameters, specificState)); + } + + @Override + public InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, InstrumentationState state) { + return runAll(state, (instrumentation, specificState) -> instrumentation.beginValidation(parameters, specificState)); + } + + @Override + public InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, InstrumentationState state) { + return runAll(state, (instrumentation, specificState) -> instrumentation.beginExecuteOperation(parameters, specificState)); + } + + @Override + public ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + return runAll(state, (instrumentation, specificState) -> instrumentation.beginExecutionStrategy(parameters, specificState)); + } + + @Override + public InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) { + return runAll(state, (instrumentation, specificState) -> instrumentation.beginSubscribedFieldEvent(parameters, specificState)); + } + + @Override + public InstrumentationContext beginField(InstrumentationFieldParameters parameters, InstrumentationState state) { + return runAll(state, (instrumentation, specificState) -> instrumentation.beginField(parameters, specificState)); + } + + @Override + public InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + return runAll(state, (instrumentation, specificState) -> instrumentation.beginFieldFetch(parameters, specificState)); + } + + @Override + public InstrumentationContext beginFieldComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + return runAll(state, (instrumentation, specificState) -> instrumentation.beginFieldComplete(parameters, specificState)); + } + + @Override + public InstrumentationContext beginFieldListComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + return runAll(state, (instrumentation, specificState) -> instrumentation.beginFieldListComplete(parameters, specificState)); + } + + // relies on the other methods from ChainedInstrumentation which this does not change +} diff --git a/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java b/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java index 10b9cb5933..b696cc225e 100644 --- a/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java +++ b/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java @@ -13,7 +13,15 @@ @PublicApi public class SimpleInstrumentationContext implements InstrumentationContext { - private static final InstrumentationContext NO_OP = new SimpleInstrumentationContext<>(); + private static final InstrumentationContext NO_OP = new InstrumentationContext() { + @Override + public void onDispatched(CompletableFuture result) { + } + + @Override + public void onCompleted(Object result, Throwable t) { + } + }; /** * A context that does nothing diff --git a/src/test/groovy/graphql/collect/ImmutableKitTest.groovy b/src/test/groovy/graphql/collect/ImmutableKitTest.groovy index 684e48f9be..e2fb0dfde7 100644 --- a/src/test/groovy/graphql/collect/ImmutableKitTest.groovy +++ b/src/test/groovy/graphql/collect/ImmutableKitTest.groovy @@ -29,6 +29,16 @@ class ImmutableKitTest extends Specification { outputList instanceof ImmutableList } + def "can map a collection with nulls"() { + when: + def outputList = ImmutableKit.mapAndDropNulls(["quick", "brown", "NULL", "fox"], { + word -> (word == "NULL") ? null : word.reverse() + }) + then: + outputList == ["kciuq", "nworb", "xof"] + outputList instanceof ImmutableList + } + def "can add to lists"() { def list = ["a"] diff --git a/src/test/groovy/graphql/execution/SubscriptionExecutionStrategyTest.groovy b/src/test/groovy/graphql/execution/SubscriptionExecutionStrategyTest.groovy index 035117e497..4a8031a805 100644 --- a/src/test/groovy/graphql/execution/SubscriptionExecutionStrategyTest.groovy +++ b/src/test/groovy/graphql/execution/SubscriptionExecutionStrategyTest.groovy @@ -8,7 +8,7 @@ import graphql.GraphQLError import graphql.GraphqlErrorBuilder import graphql.TestUtil import graphql.TypeMismatchError -import graphql.execution.instrumentation.TestingInstrumentation +import graphql.execution.instrumentation.LegacyTestingInstrumentation import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters import graphql.execution.pubsub.CapturingSubscriber import graphql.execution.pubsub.Message @@ -577,7 +577,7 @@ class SubscriptionExecutionStrategyTest extends Specification { } def instrumentResultCalls = [] - TestingInstrumentation instrumentation = new TestingInstrumentation() { + LegacyTestingInstrumentation instrumentation = new LegacyTestingInstrumentation() { @Override CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters) { instrumentResultCalls.add("instrumentExecutionResult") diff --git a/src/test/groovy/graphql/execution/instrumentation/AllNullTestingInstrumentation.groovy b/src/test/groovy/graphql/execution/instrumentation/AllNullTestingInstrumentation.groovy new file mode 100644 index 0000000000..87ef7800f0 --- /dev/null +++ b/src/test/groovy/graphql/execution/instrumentation/AllNullTestingInstrumentation.groovy @@ -0,0 +1,148 @@ +package graphql.execution.instrumentation + +import graphql.ExecutionInput +import graphql.ExecutionResult +import graphql.execution.ExecutionContext +import graphql.execution.instrumentation.parameters.InstrumentationCreateStateParameters +import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters +import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters +import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters +import graphql.execution.instrumentation.parameters.InstrumentationFieldCompleteParameters +import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters +import graphql.execution.instrumentation.parameters.InstrumentationFieldParameters +import graphql.execution.instrumentation.parameters.InstrumentationValidationParameters +import graphql.language.Document +import graphql.schema.DataFetcher +import graphql.schema.DataFetchingEnvironment +import graphql.schema.GraphQLSchema +import graphql.validation.ValidationError + +import java.util.concurrent.CompletableFuture + +class AllNullTestingInstrumentation implements Instrumentation { + + InstrumentationState instrumentationState = new InstrumentationState() {} + List executionList = [] + List dfInvocations = [] + List dfClasses = [] + Map capturedData = [:] + + @Override + InstrumentationState createState(InstrumentationCreateStateParameters parameters) { + return instrumentationState + } + + @Override + InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + executionList << "start:execution" + return null + } + + @Override + InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + executionList << "start:parse" + return null + } + + @Override + InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, InstrumentationState state) { + assert state == instrumentationState + executionList << "start:validation" + return null + } + + @Override + ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + assert state == instrumentationState + executionList << "start:execution-strategy" + return null + } + + @Override + InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, InstrumentationState state) { + assert state == instrumentationState + executionList << "start:execute-operation" + return null + } + + @Override + InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) { + assert state == instrumentationState + executionList << "start:subscribed-field-event-$parameters.field.name" + return null + } + + @Override + InstrumentationContext beginField(InstrumentationFieldParameters parameters, InstrumentationState state) { + assert state == instrumentationState + executionList << "start:field-$parameters.field.name" + return null + } + + @Override + InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + assert state == instrumentationState + executionList << "start:fetch-$parameters.field.name" + return null + } + + @Override + InstrumentationContext beginFieldComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + assert state == instrumentationState + executionList << "start:complete-$parameters.field.name" + return null + } + + @Override + InstrumentationContext beginFieldListComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + assert state == instrumentationState + executionList << "start:complete-list-$parameters.field.name" + return null + } + + @Override + ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return executionInput + } + + @Override + DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return documentAndVariables + } + + @Override + GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return schema + } + + @Override + ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return executionContext + } + + @Override + DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + assert state == instrumentationState + dfClasses.add(dataFetcher.getClass()) + return new DataFetcher() { + @Override + Object get(DataFetchingEnvironment environment) { + dfInvocations.add(environment) + dataFetcher.get(environment) + } + } + } + + @Override + CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return CompletableFuture.completedFuture(executionResult) + } +} + diff --git a/src/test/groovy/graphql/execution/instrumentation/ChainedInstrumentationStateTest.groovy b/src/test/groovy/graphql/execution/instrumentation/ChainedInstrumentationStateTest.groovy index ddfd62f160..90c858e9c4 100644 --- a/src/test/groovy/graphql/execution/instrumentation/ChainedInstrumentationStateTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/ChainedInstrumentationStateTest.groovy @@ -19,89 +19,97 @@ import java.util.concurrent.CompletableFuture class ChainedInstrumentationStateTest extends Specification { - class NamedInstrumentationState implements InstrumentationState { - String name - } - // each implementation gives out a state object with its name - // and then asserts it gets it back with that name - class NamedInstrumentation extends TestingInstrumentation { - String name + def "basic chaining and state management"() { - NamedInstrumentation(String name) { - instrumentationState = new NamedInstrumentationState(name: name) - this.name = name - } + def a = new NamedInstrumentation("A") + def b = new NamedInstrumentation("B") + def c = new NamedInstrumentation("C") + def chainedInstrumentation = new ChainedInstrumentation([ + a, + b, + c, + ]) - @Override - InstrumentationState createState() { - return instrumentationState + def query = """ + query HeroNameAndFriendsQuery { + hero { + id + } } + """ - def assertState(InstrumentationState instrumentationState) { - assert instrumentationState instanceof NamedInstrumentationState - assert (instrumentationState as NamedInstrumentationState).name == this.name - } + def expected = [ + "start:execution", - @Override - InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters) { - assertState(parameters.getInstrumentationState()) - return super.beginExecution(parameters) - } + "start:parse", + "end:parse", - @Override - InstrumentationContext beginParse(InstrumentationExecutionParameters parameters) { - assertState(parameters.getInstrumentationState()) - return super.beginParse(parameters) - } + "start:validation", + "end:validation", - @Override - InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters) { - assertState(parameters.getInstrumentationState()) - return super.beginValidation(parameters) - } + "start:execute-operation", - @Override - ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters) { - assertState(parameters.getInstrumentationState()) - return super.beginExecutionStrategy(parameters) - } + "start:execution-strategy", - @Override - InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters) { - assertState(parameters.getInstrumentationState()) - return super.beginExecuteOperation(parameters) - } + "start:field-hero", + "start:fetch-hero", + "end:fetch-hero", + "start:complete-hero", - @Override - InstrumentationContext beginField(InstrumentationFieldParameters parameters) { - assertState(parameters.getInstrumentationState()) - return super.beginField(parameters) - } + "start:execution-strategy", - @Override - InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters) { - assertState(parameters.getInstrumentationState()) - return super.beginFieldFetch(parameters) - } + "start:field-id", + "start:fetch-id", + "end:fetch-id", + "start:complete-id", + "end:complete-id", + "end:field-id", - @Override - DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters) { - assertState(parameters.getInstrumentationState()) - return super.instrumentDataFetcher(dataFetcher, parameters) - } + "end:execution-strategy", + + "end:complete-hero", + "end:field-hero", + + "end:execution-strategy", + + "end:execute-operation", + + "end:execution", + ] + + + when: + def strategy = new AsyncExecutionStrategy() + def graphQL = GraphQL + .newGraphQL(StarWarsSchema.starWarsSchema) + .queryExecutionStrategy(strategy) + .instrumentation(chainedInstrumentation) + .build() + + graphQL.execute(query) + + then: + + a.executionList == expected + b.executionList == expected + c.executionList == expected + + assertCalls(a) + assertCalls(b) + assertCalls(c) - @Override - CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters) { - assertState(parameters.getInstrumentationState()) - return super.instrumentExecutionResult(executionResult, parameters) - } } - def "basic chaining and state management"() { + def "basic chaining and state management when null returned"() { def a = new NamedInstrumentation("A") - def b = new NamedInstrumentation("B") + def b = new NamedInstrumentation("B") { + @Override + InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, InstrumentationState state) { + return null // just this method + } + } def c = new NamedInstrumentation("C") def chainedInstrumentation = new ChainedInstrumentation([ a, @@ -156,6 +164,46 @@ class ChainedInstrumentationStateTest extends Specification { "end:execution", ] + def expectedWhenReturningNull = [ + "start:execution", + + "start:parse", + "end:parse", + + // overridden above + //"start:validation", + //"end:validation", + + "start:execute-operation", + + "start:execution-strategy", + + "start:field-hero", + "start:fetch-hero", + "end:fetch-hero", + "start:complete-hero", + + "start:execution-strategy", + + "start:field-id", + "start:fetch-id", + "end:fetch-id", + "start:complete-id", + "end:complete-id", + "end:field-id", + + "end:execution-strategy", + + "end:complete-hero", + "end:field-hero", + + "end:execution-strategy", + + "end:execute-operation", + + "end:execution", + ] + when: def strategy = new AsyncExecutionStrategy() @@ -170,13 +218,12 @@ class ChainedInstrumentationStateTest extends Specification { then: a.executionList == expected - b.executionList == expected + b.executionList == expectedWhenReturningNull c.executionList == expected assertCalls(a) assertCalls(b) assertCalls(c) - } def "empty chain"() { diff --git a/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy b/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy index f8b469e45b..f5e74eb510 100644 --- a/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy @@ -80,7 +80,7 @@ class InstrumentationTest extends Specification { ] when: - def instrumentation = new TestingInstrumentation() + def instrumentation = new LegacyTestingInstrumentation() instrumentation.useOnDispatch = true def graphQL = GraphQL @@ -124,7 +124,7 @@ class InstrumentationTest extends Specification { } """ - def instrumentation = new TestingInstrumentation() { + def instrumentation = new LegacyTestingInstrumentation() { @Override DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters) { return new DataFetcher() { @@ -163,7 +163,7 @@ class InstrumentationTest extends Specification { final AtomicBoolean goSignal = new AtomicBoolean() @Override - ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters) { + ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { System.out.println(String.format("t%s setting go signal off", Thread.currentThread().getId())) goSignal.set(false) return new ExecutionStrategyInstrumentationContext() { @@ -181,7 +181,7 @@ class InstrumentationTest extends Specification { } @Override - DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters) { + DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters, InstrumentationState state) { System.out.println(String.format("t%s instrument DF for %s", Thread.currentThread().getId(), parameters.environment.getExecutionStepInfo().getPath())) return new DataFetcher() { @@ -196,7 +196,6 @@ class InstrumentationTest extends Specification { return value }) } - } } } @@ -251,17 +250,16 @@ class InstrumentationTest extends Specification { } ''' - def instrumentation = new TestingInstrumentation() { + def instrumentation = new ModernTestingInstrumentation() { @Override - DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters) { + DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters, InstrumentationState state) { this.capturedData["originalDoc"] = AstPrinter.printAst(documentAndVariables.getDocument()) this.capturedData["originalVariables"] = documentAndVariables.getVariables() def newDoc = new Parser().parseDocument(newQuery) def newVars = [var: "1001"] documentAndVariables.transform({ builder -> builder.document(newDoc).variables(newVars) }) } - } def graphQL = GraphQL @@ -278,4 +276,130 @@ class InstrumentationTest extends Specification { instrumentation.capturedData["originalDoc"] == query instrumentation.capturedData["originalVariables"] == variables } + + def "an instrumentation can return null and graphql calling code can handle it when inside a chain"() { + given: + + def query = """ + { + hero { + id + } + } + """ + + // + // for testing purposes we must use AsyncExecutionStrategy under the covers to get such + // serial behaviour. The Instrumentation of a parallel strategy would be much different + // and certainly harder to test + + def expected = [ + "start:execution", + // because a null context was returned, there is no onDispatched and end + //"onDispatched:execution", + "start:parse", + "onDispatched:parse", + "end:parse", + "start:validation", + "onDispatched:validation", + "end:validation", + "start:execute-operation", + "start:execution-strategy", + "start:field-hero", + "start:fetch-hero", + "onDispatched:fetch-hero", + "end:fetch-hero", + "start:complete-hero", + "start:execution-strategy", + "start:field-id", + "start:fetch-id", + "onDispatched:fetch-id", + "end:fetch-id", + "start:complete-id", + "onDispatched:complete-id", + "end:complete-id", + "onDispatched:field-id", + "end:field-id", + "onDispatched:execution-strategy", + "end:execution-strategy", + "onDispatched:complete-hero", + "end:complete-hero", + "onDispatched:field-hero", + "end:field-hero", + "onDispatched:execution-strategy", + "end:execution-strategy", + "onDispatched:execute-operation", + "end:execute-operation", + //"end:execution", + ] + when: + + def instrumentation = new ModernTestingInstrumentation() { + @Override + InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, InstrumentationState state) { + executionList.add("start:execution") + return null + } + } + instrumentation.useOnDispatch = true + + def graphQL = GraphQL + .newGraphQL(StarWarsSchema.starWarsSchema) + .queryExecutionStrategy(new AsyncExecutionStrategy()) + .instrumentation(instrumentation) + .build() + + graphQL.execute(query) + + then: + instrumentation.executionList == expected + } + + def "an instrumentation can return null and graphql calling code can handle it when not inside a chain"() { + + given: + + def query = '''query Q($var: String!) { + human(id: $var) { + id + name + } + } + ''' + + def instrumentation = new AllNullTestingInstrumentation() + + def graphQL = GraphQL + .newGraphQL(StarWarsSchema.starWarsSchema) + .instrumentation(instrumentation) + .doNotAddDefaultInstrumentations() // important, otherwise a chained one wil be used + .build() + + when: + def variables = [var: "1001"] + def er = graphQL.execute(ExecutionInput.newExecutionInput().query(query).variables(variables)) // Luke + + then: + er.data == [human: [id: "1001", name: 'Darth Vader']] + + def expected = [ + "start:execution", + "start:parse", + "start:validation", + "start:execute-operation", + "start:execution-strategy", + "start:field-human", + "start:fetch-human", + "start:complete-human", + "start:execution-strategy", + "start:field-id", + "start:fetch-id", + "start:complete-id", + "start:field-name", + "start:fetch-name", + "start:complete-name", + ] + + instrumentation.executionList == expected + } } diff --git a/src/test/groovy/graphql/execution/instrumentation/TestingInstrumentation.groovy b/src/test/groovy/graphql/execution/instrumentation/LegacyTestingInstrumentation.groovy similarity index 97% rename from src/test/groovy/graphql/execution/instrumentation/TestingInstrumentation.groovy rename to src/test/groovy/graphql/execution/instrumentation/LegacyTestingInstrumentation.groovy index 5b1fce2d8a..5e24cbf09f 100644 --- a/src/test/groovy/graphql/execution/instrumentation/TestingInstrumentation.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/LegacyTestingInstrumentation.groovy @@ -18,7 +18,10 @@ import graphql.validation.ValidationError import java.util.concurrent.CompletableFuture -class TestingInstrumentation implements Instrumentation { +/** + * This class overrides the old deprecated methods and shows that they still can be called + */ +class LegacyTestingInstrumentation implements Instrumentation { def instrumentationState = new InstrumentationState() {} List executionList = [] diff --git a/src/test/groovy/graphql/execution/instrumentation/ModernTestingInstrumentation.groovy b/src/test/groovy/graphql/execution/instrumentation/ModernTestingInstrumentation.groovy new file mode 100644 index 0000000000..81648969f7 --- /dev/null +++ b/src/test/groovy/graphql/execution/instrumentation/ModernTestingInstrumentation.groovy @@ -0,0 +1,143 @@ +package graphql.execution.instrumentation + +import graphql.ExecutionInput +import graphql.ExecutionResult +import graphql.execution.ExecutionContext +import graphql.execution.instrumentation.parameters.InstrumentationCreateStateParameters +import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters +import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters +import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters +import graphql.execution.instrumentation.parameters.InstrumentationFieldCompleteParameters +import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters +import graphql.execution.instrumentation.parameters.InstrumentationFieldParameters +import graphql.execution.instrumentation.parameters.InstrumentationValidationParameters +import graphql.language.Document +import graphql.schema.DataFetcher +import graphql.schema.DataFetchingEnvironment +import graphql.schema.GraphQLSchema +import graphql.validation.ValidationError + +import java.util.concurrent.CompletableFuture + +/** + * This class overrides the new methods that take state directly as a parameter + */ +class ModernTestingInstrumentation implements Instrumentation { + + InstrumentationState instrumentationState = new InstrumentationState() {} + List executionList = [] + List throwableList = [] + List dfInvocations = [] + List dfClasses = [] + Map capturedData = [:] + boolean useOnDispatch = false + + @Override + InstrumentationState createState(InstrumentationCreateStateParameters parameters) { + return instrumentationState + } + + @Override + InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + new TestingInstrumentContext("execution", executionList, throwableList, useOnDispatch) + } + + @Override + InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return new TestingInstrumentContext("parse", executionList, throwableList, useOnDispatch) + } + + @Override + InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return new TestingInstrumentContext("validation", executionList, throwableList, useOnDispatch) + } + + @Override + ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return new TestingExecutionStrategyInstrumentationContext("execution-strategy", executionList, throwableList, useOnDispatch) + } + + @Override + InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return new TestingInstrumentContext("execute-operation", executionList, throwableList, useOnDispatch) + } + + @Override + InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return new TestingInstrumentContext("subscribed-field-event-$parameters.field.name", executionList, throwableList, useOnDispatch) + } + + @Override + InstrumentationContext beginField(InstrumentationFieldParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return new TestingInstrumentContext("field-$parameters.field.name", executionList, throwableList, useOnDispatch) + } + + @Override + InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return new TestingInstrumentContext("fetch-$parameters.field.name", executionList, throwableList, useOnDispatch) + } + + @Override + InstrumentationContext beginFieldComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return new TestingInstrumentContext("complete-$parameters.field.name", executionList, throwableList, useOnDispatch) + } + + @Override + InstrumentationContext beginFieldListComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return new TestingInstrumentContext("complete-list-$parameters.field.name", executionList, throwableList, useOnDispatch) + } + + @Override + ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return executionInput + } + + @Override + DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return documentAndVariables + } + + @Override + GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return schema + } + + @Override + ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return executionContext + } + + @Override + DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + assert state == instrumentationState + dfClasses.add(dataFetcher.getClass()) + return new DataFetcher() { + @Override + Object get(DataFetchingEnvironment environment) { + dfInvocations.add(environment) + dataFetcher.get(environment) + } + } + } + + @Override + CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return CompletableFuture.completedFuture(executionResult) + } +} + diff --git a/src/test/groovy/graphql/execution/instrumentation/NamedInstrumentation.groovy b/src/test/groovy/graphql/execution/instrumentation/NamedInstrumentation.groovy new file mode 100644 index 0000000000..4ed707cfba --- /dev/null +++ b/src/test/groovy/graphql/execution/instrumentation/NamedInstrumentation.groovy @@ -0,0 +1,95 @@ +package graphql.execution.instrumentation + +import graphql.ExecutionResult +import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters +import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters +import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters +import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters +import graphql.execution.instrumentation.parameters.InstrumentationFieldParameters +import graphql.execution.instrumentation.parameters.InstrumentationValidationParameters +import graphql.language.Document +import graphql.schema.DataFetcher +import graphql.validation.ValidationError + +import java.util.concurrent.CompletableFuture + +// each implementation gives out a state object with its name +// and then asserts it gets it back with that name + +class NamedInstrumentation extends ModernTestingInstrumentation { + String name + + + NamedInstrumentation(String name) { + instrumentationState = new NamedInstrumentationState(name: name) + this.name = name + } + + @Override + InstrumentationState createState() { + return instrumentationState + } + + def assertState(InstrumentationState instrumentationState) { + assert instrumentationState instanceof NamedInstrumentationState + assert (instrumentationState as NamedInstrumentationState).name == this.name + } + + @Override + InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, InstrumentationState state) { + assertState(state) + return super.beginExecution(parameters, state) + } + + @Override + InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, InstrumentationState state) { + assertState(state) + return super.beginParse(parameters, state) + } + + @Override + InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, InstrumentationState state) { + assertState(state) + return super.beginValidation(parameters, state) + } + + @Override + ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + assertState(state) + return super.beginExecutionStrategy(parameters, state) + } + + @Override + InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, InstrumentationState state) { + assertState(state) + return super.beginExecuteOperation(parameters, state) + } + + @Override + InstrumentationContext beginField(InstrumentationFieldParameters parameters, InstrumentationState state) { + assertState(state) + return super.beginField(parameters, state) + } + + @Override + InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + assertState(state) + return super.beginFieldFetch(parameters, state) + } + + @Override + DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + assertState(state) + return super.instrumentDataFetcher(dataFetcher, parameters, state) + } + + @Override + CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, InstrumentationState state) { + assertState(state) + return super.instrumentExecutionResult(executionResult, parameters, state) + } +} + +class NamedInstrumentationState implements InstrumentationState { + String name +} diff --git a/src/test/groovy/graphql/execution/instrumentation/NoContextChainedInstrumentationTest.groovy b/src/test/groovy/graphql/execution/instrumentation/NoContextChainedInstrumentationTest.groovy new file mode 100644 index 0000000000..a76c1e51d6 --- /dev/null +++ b/src/test/groovy/graphql/execution/instrumentation/NoContextChainedInstrumentationTest.groovy @@ -0,0 +1,87 @@ +package graphql.execution.instrumentation + +import graphql.GraphQL +import graphql.StarWarsSchema +import graphql.execution.AsyncExecutionStrategy +import spock.lang.Specification + +class NoContextChainedInstrumentationTest extends Specification { + + def "basic chaining and state management with no contexts"() { + + def a = new NamedInstrumentation("A") + def b = new NamedInstrumentation("B") + def c = new NamedInstrumentation("C") + def noContextChainedInstrumentation = new NoContextChainedInstrumentation([ + a, + b, + c, + ]) + + def query = """ + query HeroNameAndFriendsQuery { + hero { + id + } + } + """ + + // no end: statements becaue the context is never called + def expected = [ + "start:execution", + + "start:parse", + + "start:validation", + + "start:execute-operation", + + "start:execution-strategy", + + "start:field-hero", + "start:fetch-hero", + "start:complete-hero", + + "start:execution-strategy", + + "start:field-id", + "start:fetch-id", + "start:complete-id", + ] + + + when: + def strategy = new AsyncExecutionStrategy() + def graphQL = GraphQL + .newGraphQL(StarWarsSchema.starWarsSchema) + .queryExecutionStrategy(strategy) + .instrumentation(noContextChainedInstrumentation) + .build() + + graphQL.execute(query) + + then: + + a.executionList == expected + b.executionList == expected + c.executionList == expected + + assertCalls(a) + assertCalls(b) + assertCalls(c) + } + + + private void assertCalls(NamedInstrumentation instrumentation) { + assert instrumentation.dfInvocations[0].getFieldDefinition().name == 'hero' + assert instrumentation.dfInvocations[0].getExecutionStepInfo().getPath().toList() == ['hero'] + assert instrumentation.dfInvocations[0].getExecutionStepInfo().getUnwrappedNonNullType().name == 'Character' + assert !instrumentation.dfInvocations[0].getExecutionStepInfo().isNonNullType() + + assert instrumentation.dfInvocations[1].getFieldDefinition().name == 'id' + assert instrumentation.dfInvocations[1].getExecutionStepInfo().getPath().toList() == ['hero', 'id'] + assert instrumentation.dfInvocations[1].getExecutionStepInfo().getUnwrappedNonNullType().name == 'String' + assert instrumentation.dfInvocations[1].getExecutionStepInfo().isNonNullType() + } + +} diff --git a/src/test/groovy/graphql/execution/preparsed/PreparsedDocumentProviderTest.groovy b/src/test/groovy/graphql/execution/preparsed/PreparsedDocumentProviderTest.groovy index 435536115a..5bc311c843 100644 --- a/src/test/groovy/graphql/execution/preparsed/PreparsedDocumentProviderTest.groovy +++ b/src/test/groovy/graphql/execution/preparsed/PreparsedDocumentProviderTest.groovy @@ -7,7 +7,7 @@ import graphql.StarWarsSchema import graphql.execution.AsyncExecutionStrategy import graphql.execution.instrumentation.InstrumentationContext import graphql.execution.instrumentation.SimpleInstrumentation -import graphql.execution.instrumentation.TestingInstrumentation +import graphql.execution.instrumentation.LegacyTestingInstrumentation import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters import graphql.language.Document import graphql.parser.Parser @@ -100,8 +100,8 @@ class PreparsedDocumentProviderTest extends Specification { when: - def instrumentation = new TestingInstrumentation() - def instrumentationPreparsed = new TestingInstrumentation() + def instrumentation = new LegacyTestingInstrumentation() + def instrumentationPreparsed = new LegacyTestingInstrumentation() def preparsedCache = new TestingPreparsedDocumentProvider() def strategy = new AsyncExecutionStrategy() From edaa3c8c251fa51b7adb75bc069380296531b6a9 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Fri, 29 Apr 2022 10:18:25 +1000 Subject: [PATCH 6/6] Use jetbrains annotations and also fix merge conflict --- .../ChainedInstrumentation.java | 3 +- .../instrumentation/Instrumentation.java | 50 +++++++++---------- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java index dcd8ccbfe0..9a405015ce 100644 --- a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java @@ -1,8 +1,8 @@ package graphql.execution.instrumentation; import com.google.common.collect.ImmutableList; -import graphql.Assert; import com.google.common.collect.Maps; +import graphql.Assert; import graphql.ExecutionInput; import graphql.ExecutionResult; import graphql.PublicApi; @@ -24,7 +24,6 @@ import org.jetbrains.annotations.NotNull; import java.util.Arrays; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; diff --git a/src/main/java/graphql/execution/instrumentation/Instrumentation.java b/src/main/java/graphql/execution/instrumentation/Instrumentation.java index 460855114c..1f19018284 100644 --- a/src/main/java/graphql/execution/instrumentation/Instrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/Instrumentation.java @@ -16,9 +16,9 @@ import graphql.schema.DataFetcher; import graphql.schema.GraphQLSchema; import graphql.validation.ValidationError; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -61,7 +61,7 @@ default InstrumentationState createState() { * * @return a state object that is passed to each method */ - @Nullable + @Nullable default InstrumentationState createState(InstrumentationCreateStateParameters parameters) { return createState(); } @@ -76,7 +76,7 @@ default InstrumentationState createState(InstrumentationCreateStateParameters pa * @deprecated use {@link #beginExecution(InstrumentationExecutionParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters) { return noOp(); } @@ -104,7 +104,7 @@ default InstrumentationContext beginExecution(InstrumentationEx * @deprecated use {@link #beginParse(InstrumentationExecutionParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default InstrumentationContext beginParse(InstrumentationExecutionParameters parameters) { return noOp(); } @@ -132,7 +132,7 @@ default InstrumentationContext beginParse(InstrumentationExecutionPara * @deprecated use {@link #beginValidation(InstrumentationValidationParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters) { return noOp(); } @@ -160,7 +160,7 @@ default InstrumentationContext> beginValidation(Instrument * @deprecated use {@link #beginExecuteOperation(InstrumentationExecuteOperationParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters) { return noOp(); } @@ -189,7 +189,7 @@ default InstrumentationContext beginExecuteOperation(Instrument * @deprecated use {@link #beginExecutionStrategy(InstrumentationExecutionStrategyParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters) { return ExecutionStrategyInstrumentationContext.NOOP; } @@ -219,7 +219,7 @@ default ExecutionStrategyInstrumentationContext beginExecutionStrategy(Instrumen * @deprecated use {@link #beginSubscribedFieldEvent(InstrumentationFieldParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters) { return noOp(); } @@ -247,7 +247,7 @@ default InstrumentationContext beginSubscribedFieldEvent(Instru * @deprecated use {@link #beginField(InstrumentationFieldParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default InstrumentationContext beginField(InstrumentationFieldParameters parameters) { return noOp(); } @@ -275,7 +275,7 @@ default InstrumentationContext beginField(InstrumentationFieldP * @deprecated use {@link #beginFieldFetch(InstrumentationFieldFetchParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters) { return noOp(); } @@ -304,7 +304,7 @@ default InstrumentationContext beginFieldFetch(InstrumentationFieldFetch * @deprecated use {@link #beginFieldComplete(InstrumentationFieldCompleteParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default InstrumentationContext beginFieldComplete(InstrumentationFieldCompleteParameters parameters) { return noOp(); } @@ -332,7 +332,7 @@ default InstrumentationContext beginFieldComplete(Instrumentati * @deprecated use {@link #beginFieldListComplete(InstrumentationFieldCompleteParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default InstrumentationContext beginFieldListComplete(InstrumentationFieldCompleteParameters parameters) { return noOp(); } @@ -362,7 +362,7 @@ default InstrumentationContext beginFieldListComplete(Instrumen * @deprecated use {@link #instrumentExecutionInput(ExecutionInput, InstrumentationExecutionParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters) { return executionInput; } @@ -377,7 +377,7 @@ default ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, I * * @return a non null instrumented ExecutionInput, the default is to return to the same object */ - @Nonnull + @NotNull default ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters, InstrumentationState state) { return instrumentExecutionInput(executionInput, parameters.withNewState(state)); } @@ -393,7 +393,7 @@ default ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, I * @deprecated use {@link #instrumentDocumentAndVariables(DocumentAndVariables, InstrumentationExecutionParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters) { return documentAndVariables; } @@ -407,7 +407,7 @@ default DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables * * @return a non null instrumented DocumentAndVariables, the default is to return to the same objects */ - @Nonnull + @NotNull default DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters, InstrumentationState state) { return instrumentDocumentAndVariables(documentAndVariables, parameters.withNewState(state)); } @@ -424,7 +424,7 @@ default DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables * @deprecated use {@link #instrumentSchema(GraphQLSchema, InstrumentationExecutionParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExecutionParameters parameters) { return schema; } @@ -439,7 +439,7 @@ default GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExec * * @return a non null instrumented GraphQLSchema, the default is to return to the same object */ - @Nonnull + @NotNull default GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExecutionParameters parameters, InstrumentationState state) { return instrumentSchema(schema, parameters.withNewState(state)); } @@ -456,7 +456,7 @@ default GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExec * @deprecated use {@link #instrumentExecutionContext(ExecutionContext, InstrumentationExecutionParameters)} instead */ @Deprecated - @Nonnull + @NotNull default ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, InstrumentationExecutionParameters parameters) { return executionContext; } @@ -471,7 +471,7 @@ default ExecutionContext instrumentExecutionContext(ExecutionContext executionCo * * @return a non null instrumented ExecutionContext, the default is to return to the same object */ - @Nonnull + @NotNull default ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, InstrumentationExecutionParameters parameters, InstrumentationState state) { return instrumentExecutionContext(executionContext, parameters.withNewState(state)); } @@ -491,7 +491,7 @@ default ExecutionContext instrumentExecutionContext(ExecutionContext executionCo * @deprecated use {@link #instrumentDataFetcher(DataFetcher, InstrumentationFieldFetchParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters) { return dataFetcher; } @@ -508,7 +508,7 @@ default DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, Instrum * * @return a non null instrumented DataFetcher, the default is to return to the same object */ - @Nonnull + @NotNull default DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters, InstrumentationState state) { return instrumentDataFetcher(dataFetcher, parameters.withNewState(state)); } @@ -524,7 +524,7 @@ default DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, Instrum * @deprecated use {@link #instrumentExecutionResult(ExecutionResult, InstrumentationExecutionParameters, InstrumentationState)} instead */ @Deprecated - @Nonnull + @NotNull default CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters) { return CompletableFuture.completedFuture(executionResult); } @@ -538,7 +538,7 @@ default CompletableFuture instrumentExecutionResult(ExecutionRe * * @return a new execution result completable future */ - @Nonnull + @NotNull default CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, InstrumentationState state) { return instrumentExecutionResult(executionResult, parameters.withNewState(state)); }