From 79423f0b5fbe99db6a059e5991153df4967e92c7 Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:07:39 +0700 Subject: [PATCH 1/2] fix: mark InstrumentationState callback params as @Nullable createState()/createStateAsync() may return null, and the default SimplePerformantInstrumentation.createState() does. After @NullMarked on instrumentation classes (#4272), unannotated state parameters were treated as non-null in Kotlin, causing NPEs for stateless subclasses. Annotate callback state parameters as @Nullable to match the optional- state runtime contract. ChainedInstrumentation asserts non-null when casting its own materialized ChainedInstrumentationState. Fixes #4433 --- .../MaxQueryComplexityInstrumentation.java | 5 +- .../MaxQueryDepthInstrumentation.java | 2 +- .../ChainedInstrumentation.java | 71 +++++++++++-------- .../instrumentation/Instrumentation.java | 43 +++++------ .../NoContextChainedInstrumentation.java | 30 ++++---- .../SimplePerformantInstrumentation.java | 34 ++++----- .../FieldValidationInstrumentation.java | 2 +- .../tracing/TracingInstrumentation.java | 8 +-- .../InstrumentationDefaultMethodsTest.groovy | 44 ++++++++++++ 9 files changed, 147 insertions(+), 92 deletions(-) diff --git a/src/main/java/graphql/analysis/MaxQueryComplexityInstrumentation.java b/src/main/java/graphql/analysis/MaxQueryComplexityInstrumentation.java index 669f147aa3..1af769338c 100644 --- a/src/main/java/graphql/analysis/MaxQueryComplexityInstrumentation.java +++ b/src/main/java/graphql/analysis/MaxQueryComplexityInstrumentation.java @@ -12,6 +12,7 @@ import graphql.execution.instrumentation.parameters.InstrumentationValidationParameters; import graphql.validation.ValidationError; import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -85,7 +86,7 @@ public CompletableFuture createStateAsync(InstrumentationC } @Override - public InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, InstrumentationState rawState) { + public InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, @Nullable InstrumentationState rawState) { State state = ofState(rawState); // for API backwards compatibility reasons we capture the validation parameters, so we can put them into QueryComplexityInfo state.instrumentationValidationParameters.set(parameters); @@ -93,7 +94,7 @@ public InstrumentationContext> beginValidation(Instrumenta } @Override - public InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters instrumentationExecuteOperationParameters, InstrumentationState rawState) { + public InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters instrumentationExecuteOperationParameters, @Nullable InstrumentationState rawState) { State state = ofState(rawState); QueryComplexityCalculator queryComplexityCalculator = newQueryComplexityCalculator(instrumentationExecuteOperationParameters.getExecutionContext()); int totalComplexity = queryComplexityCalculator.calculate(); diff --git a/src/main/java/graphql/analysis/MaxQueryDepthInstrumentation.java b/src/main/java/graphql/analysis/MaxQueryDepthInstrumentation.java index 6bab51da1a..93cd7400ca 100644 --- a/src/main/java/graphql/analysis/MaxQueryDepthInstrumentation.java +++ b/src/main/java/graphql/analysis/MaxQueryDepthInstrumentation.java @@ -49,7 +49,7 @@ public MaxQueryDepthInstrumentation(int maxDepth, Function beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, InstrumentationState state) { + public InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, @Nullable InstrumentationState state) { QueryTraverser queryTraverser = newQueryTraverser(parameters.getExecutionContext()); int depth = queryTraverser.reducePreOrder((env, acc) -> Math.max(getPathLength(env.getParentEnvironment()), acc), 0); if (depth > maxDepth) { diff --git a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java index f736bde964..568a716274 100644 --- a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java @@ -67,21 +67,30 @@ public List getInstrumentations() { return instrumentations; } - private @Nullable InstrumentationContext chainedCtx(InstrumentationState state, BiFunction> mapper) { + /** + * Chained instrumentation always materializes a {@link ChainedInstrumentationState} via + * {@link #createStateAsync(InstrumentationCreateStateParameters)}. Callback {@code state} is still + * {@link Nullable} to match the optional-state contract of {@link Instrumentation}. + */ + private static ChainedInstrumentationState asChainedState(@Nullable InstrumentationState state) { + return (ChainedInstrumentationState) assertNotNull(state); + } + + private @Nullable InstrumentationContext chainedCtx(@Nullable InstrumentationState state, BiFunction> mapper) { // if we have zero or 1 instrumentations (and 1 is the most common), then we can avoid an object allocation // of the ChainedInstrumentationContext since it won't be needed if (instrumentations.isEmpty()) { return SimpleInstrumentationContext.noOp(); } - ChainedInstrumentationState chainedInstrumentationState = (ChainedInstrumentationState) state; + ChainedInstrumentationState chainedInstrumentationState = asChainedState(state); if (instrumentations.size() == 1) { return mapper.apply(instrumentations.get(0), chainedInstrumentationState.getState(0)); } return new ChainedInstrumentationContext<>(chainedMapAndDropNulls(chainedInstrumentationState, mapper)); } - private T chainedInstrument(InstrumentationState state, T input, ChainedInstrumentationFunction mapper) { - ChainedInstrumentationState chainedInstrumentationState = (ChainedInstrumentationState) state; + private T chainedInstrument(@Nullable InstrumentationState state, T input, ChainedInstrumentationFunction mapper) { + ChainedInstrumentationState chainedInstrumentationState = asChainedState(state); for (int i = 0; i < instrumentations.size(); i++) { Instrumentation instrumentation = instrumentations.get(i); InstrumentationState specificState = chainedInstrumentationState.getState(i); @@ -90,8 +99,8 @@ private T chainedInstrument(InstrumentationState state, T input, ChainedInst return input; } - protected ImmutableList chainedMapAndDropNulls(InstrumentationState state, BiFunction mapper) { - ChainedInstrumentationState chainedInstrumentationState = (ChainedInstrumentationState) state; + protected ImmutableList chainedMapAndDropNulls(@Nullable InstrumentationState state, BiFunction mapper) { + ChainedInstrumentationState chainedInstrumentationState = asChainedState(state); ImmutableList.Builder result = ImmutableList.builderWithExpectedSize(instrumentations.size()); for (int i = 0; i < instrumentations.size(); i++) { Instrumentation instrumentation = instrumentations.get(i); @@ -104,8 +113,8 @@ protected ImmutableList chainedMapAndDropNulls(InstrumentationState state return result.build(); } - protected void chainedConsume(InstrumentationState state, BiConsumer stateConsumer) { - ChainedInstrumentationState chainedInstrumentationState = (ChainedInstrumentationState) state; + protected void chainedConsume(@Nullable InstrumentationState state, BiConsumer stateConsumer) { + ChainedInstrumentationState chainedInstrumentationState = asChainedState(state); for (int i = 0; i < instrumentations.size(); i++) { Instrumentation instrumentation = instrumentations.get(i); InstrumentationState specificState = chainedInstrumentationState.getState(i); @@ -119,39 +128,39 @@ public CompletableFuture createStateAsync(InstrumentationC } @Override - public @Nullable InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginExecution(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginParse(parameters, specificState)); } @Override - public @Nullable InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, @Nullable InstrumentationState state) { return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginValidation(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, @Nullable InstrumentationState state) { return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginExecuteOperation(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginReactiveResults(InstrumentationReactiveResultsParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginReactiveResults(InstrumentationReactiveResultsParameters parameters, @Nullable InstrumentationState state) { return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginReactiveResults(parameters, specificState)); } @Override - public @Nullable ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + public @Nullable ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, @Nullable InstrumentationState state) { if (instrumentations.isEmpty()) { return ExecutionStrategyInstrumentationContext.NOOP; } BiFunction mapper = (instrumentation, specificState) -> instrumentation.beginExecutionStrategy(parameters, specificState); - ChainedInstrumentationState chainedInstrumentationState = (ChainedInstrumentationState) state; + ChainedInstrumentationState chainedInstrumentationState = asChainedState(state); if (instrumentations.size() == 1) { return mapper.apply(instrumentations.get(0), chainedInstrumentationState.getState(0)); } @@ -159,12 +168,12 @@ public CompletableFuture createStateAsync(InstrumentationC } @Override - public @Nullable ExecuteObjectInstrumentationContext beginExecuteObject(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + public @Nullable ExecuteObjectInstrumentationContext beginExecuteObject(InstrumentationExecutionStrategyParameters parameters, @Nullable InstrumentationState state) { if (instrumentations.isEmpty()) { return ExecuteObjectInstrumentationContext.NOOP; } BiFunction mapper = (instrumentation, specificState) -> instrumentation.beginExecuteObject(parameters, specificState); - ChainedInstrumentationState chainedInstrumentationState = (ChainedInstrumentationState) state; + ChainedInstrumentationState chainedInstrumentationState = asChainedState(state); if (instrumentations.size() == 1) { return mapper.apply(instrumentations.get(0), chainedInstrumentationState.getState(0)); } @@ -173,32 +182,32 @@ public CompletableFuture createStateAsync(InstrumentationC @ExperimentalApi @Override - public @Nullable InstrumentationContext beginDeferredField(InstrumentationFieldParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginDeferredField(InstrumentationFieldParameters parameters, @Nullable InstrumentationState state) { return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginDeferredField(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, @Nullable InstrumentationState state) { return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginSubscribedFieldEvent(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginFieldExecution(InstrumentationFieldParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginFieldExecution(InstrumentationFieldParameters parameters, @Nullable InstrumentationState state) { return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginFieldExecution(parameters, specificState)); } @SuppressWarnings("deprecation") @Override - public @Nullable InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, @Nullable InstrumentationState state) { return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginFieldFetch(parameters, specificState)); } @Override - public @Nullable FieldFetchingInstrumentationContext beginFieldFetching(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + public @Nullable FieldFetchingInstrumentationContext beginFieldFetching(InstrumentationFieldFetchParameters parameters, @Nullable InstrumentationState state) { if (instrumentations.isEmpty()) { return FieldFetchingInstrumentationContext.NOOP; } - ChainedInstrumentationState chainedInstrumentationState = (ChainedInstrumentationState) state; + ChainedInstrumentationState chainedInstrumentationState = asChainedState(state); if (instrumentations.size() == 1) { return instrumentations.get(0).beginFieldFetching(parameters, chainedInstrumentationState.getState(0)); } @@ -234,47 +243,47 @@ private FieldFetchingInstrumentationContext chainedFieldFetchingCtx(Instrumentat } @Override - public @Nullable InstrumentationContext beginFieldCompletion(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginFieldCompletion(InstrumentationFieldCompleteParameters parameters, @Nullable InstrumentationState state) { return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginFieldCompletion(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginFieldListCompletion(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginFieldListCompletion(InstrumentationFieldCompleteParameters parameters, @Nullable InstrumentationState state) { return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginFieldListCompletion(parameters, specificState)); } @Override - public ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters, InstrumentationState state) { + public ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return chainedInstrument(state, executionInput, (instrumentation, specificState, accumulator) -> instrumentation.instrumentExecutionInput(accumulator, parameters, specificState)); } @Override - public DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters, InstrumentationState state) { + public DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return chainedInstrument(state, documentAndVariables, (instrumentation, specificState, accumulator) -> instrumentation.instrumentDocumentAndVariables(accumulator, parameters, specificState)); } @Override - public GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExecutionParameters parameters, InstrumentationState state) { + public GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return chainedInstrument(state, schema, (instrumentation, specificState, accumulator) -> instrumentation.instrumentSchema(accumulator, parameters, specificState)); } @Override - public ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, InstrumentationExecutionParameters parameters, InstrumentationState state) { + public ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return chainedInstrument(state, executionContext, (instrumentation, specificState, accumulator) -> instrumentation.instrumentExecutionContext(accumulator, parameters, specificState)); } @Override - public DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + public DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters, @Nullable InstrumentationState state) { return chainedInstrument(state, dataFetcher, (Instrumentation instrumentation, InstrumentationState specificState, DataFetcher accumulator) -> instrumentation.instrumentDataFetcher(accumulator, parameters, specificState)); } @Override - public CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, InstrumentationState state) { + public CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { ImmutableList> entries = chainedMapAndDropNulls(state, AbstractMap.SimpleEntry::new); CompletableFuture> resultsFuture = Async.eachSequentially(entries, (entry, prevResults) -> { Instrumentation instrumentation = entry.getKey(); diff --git a/src/main/java/graphql/execution/instrumentation/Instrumentation.java b/src/main/java/graphql/execution/instrumentation/Instrumentation.java index 441bf049b8..d2415d9587 100644 --- a/src/main/java/graphql/execution/instrumentation/Instrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/Instrumentation.java @@ -73,12 +73,13 @@ default InstrumentationState createState(InstrumentationCreateStateParameters pa * 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 #createStateAsync(InstrumentationCreateStateParameters)} + * @param state the state created during the call to {@link #createStateAsync(InstrumentationCreateStateParameters)}, + * or {@code null} when createState/createStateAsync returns null * * @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) { + default InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @@ -91,7 +92,7 @@ default InstrumentationContext beginExecution(InstrumentationEx * @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) { + default InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @@ -104,7 +105,7 @@ default InstrumentationContext beginParse(InstrumentationExecutionPara * @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) { + default InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @@ -117,7 +118,7 @@ default InstrumentationContext> beginValidation(Instrument * @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) { + default InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @@ -132,7 +133,7 @@ default InstrumentationContext beginExecuteOperation(Instrument * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) */ @Nullable - default InstrumentationContext beginReactiveResults(InstrumentationReactiveResultsParameters parameters, InstrumentationState state) { + default InstrumentationContext beginReactiveResults(InstrumentationReactiveResultsParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @@ -146,7 +147,7 @@ default InstrumentationContext beginReactiveResults(InstrumentationReactiv * @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) { + default ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, @Nullable InstrumentationState state) { return ExecutionStrategyInstrumentationContext.NOOP; } @@ -160,7 +161,7 @@ default ExecutionStrategyInstrumentationContext beginExecutionStrategy(Instrumen * @return a nullable {@link ExecutionStrategyInstrumentationContext} object that will be called back when the step ends (assuming it's not null) */ @Nullable - default ExecuteObjectInstrumentationContext beginExecuteObject(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + default ExecuteObjectInstrumentationContext beginExecuteObject(InstrumentationExecutionStrategyParameters parameters, @Nullable InstrumentationState state) { return ExecuteObjectInstrumentationContext.NOOP; } @@ -176,7 +177,7 @@ default ExecuteObjectInstrumentationContext beginExecuteObject(InstrumentationEx */ @ExperimentalApi @Nullable - default InstrumentationContext beginDeferredField(InstrumentationFieldParameters parameters, InstrumentationState state) { + default InstrumentationContext beginDeferredField(InstrumentationFieldParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @@ -189,7 +190,7 @@ default InstrumentationContext beginDeferredField(InstrumentationFieldPa * @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) { + default InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @@ -202,7 +203,7 @@ default InstrumentationContext beginSubscribedFieldEvent(Instru * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) */ @Nullable - default InstrumentationContext beginFieldExecution(InstrumentationFieldParameters parameters, InstrumentationState state) { + default InstrumentationContext beginFieldExecution(InstrumentationFieldParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @@ -219,7 +220,7 @@ default InstrumentationContext beginFieldExecution(InstrumentationFieldP */ @Deprecated(since = "2024-04-18") @Nullable - default InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + default InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @@ -239,7 +240,7 @@ default InstrumentationContext beginFieldFetch(InstrumentationFieldFetch * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) */ @Nullable - default FieldFetchingInstrumentationContext beginFieldFetching(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + default FieldFetchingInstrumentationContext beginFieldFetching(InstrumentationFieldFetchParameters parameters, @Nullable InstrumentationState state) { InstrumentationContext ctx = beginFieldFetch(parameters, state); if (ctx == noOp()) { return FieldFetchingInstrumentationContext.NOOP; @@ -256,7 +257,7 @@ default FieldFetchingInstrumentationContext beginFieldFetching(InstrumentationFi * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) */ @Nullable - default InstrumentationContext beginFieldCompletion(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + default InstrumentationContext beginFieldCompletion(InstrumentationFieldCompleteParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @@ -269,7 +270,7 @@ default InstrumentationContext beginFieldCompletion(InstrumentationField * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) */ @Nullable - default InstrumentationContext beginFieldListCompletion(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + default InstrumentationContext beginFieldListCompletion(InstrumentationFieldCompleteParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @@ -284,7 +285,7 @@ default InstrumentationContext beginFieldListCompletion(InstrumentationF * @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) { + default ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return executionInput; } @@ -298,7 +299,7 @@ default ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, I * @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) { + default DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return documentAndVariables; } @@ -313,7 +314,7 @@ default DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables * @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) { + default GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return schema; } @@ -328,7 +329,7 @@ default GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExec * @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) { + default ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return executionContext; } @@ -345,7 +346,7 @@ default ExecutionContext instrumentExecutionContext(ExecutionContext executionCo * @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) { + default DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters, @Nullable InstrumentationState state) { return dataFetcher; } @@ -359,7 +360,7 @@ default DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, Instrum * @return a new execution result completable future */ @NonNull - default CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, InstrumentationState state) { + default CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return CompletableFuture.completedFuture(executionResult); } } diff --git a/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java index 719376819a..96187d6bb2 100644 --- a/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java @@ -51,78 +51,78 @@ public NoContextChainedInstrumentation(Instrumentation... instrumentations) { super(instrumentations); } - private @Nullable T runAll(InstrumentationState state, BiConsumer stateConsumer) { + private @Nullable T runAll(@Nullable InstrumentationState state, BiConsumer stateConsumer) { chainedConsume(state, stateConsumer); return null; } @Override - public @Nullable InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginExecution(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginParse(parameters, specificState)); } @Override - public @Nullable InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginValidation(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginExecuteOperation(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginReactiveResults(InstrumentationReactiveResultsParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginReactiveResults(InstrumentationReactiveResultsParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginReactiveResults(parameters, specificState)); } @Override - public @Nullable ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + public @Nullable ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginExecutionStrategy(parameters, specificState)); } @Override - public @Nullable ExecuteObjectInstrumentationContext beginExecuteObject(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + public @Nullable ExecuteObjectInstrumentationContext beginExecuteObject(InstrumentationExecutionStrategyParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginExecuteObject(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginDeferredField(InstrumentationFieldParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginDeferredField(InstrumentationFieldParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginDeferredField(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginSubscribedFieldEvent(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginFieldExecution(InstrumentationFieldParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginFieldExecution(InstrumentationFieldParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginFieldExecution(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginFieldFetch(parameters, specificState)); } @Override - public @Nullable FieldFetchingInstrumentationContext beginFieldFetching(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + public @Nullable FieldFetchingInstrumentationContext beginFieldFetching(InstrumentationFieldFetchParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginFieldFetching(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginFieldCompletion(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginFieldCompletion(InstrumentationFieldCompleteParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginFieldCompletion(parameters, specificState)); } @Override - public @Nullable InstrumentationContext beginFieldListCompletion(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginFieldListCompletion(InstrumentationFieldCompleteParameters parameters, @Nullable InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginFieldListCompletion(parameters, specificState)); } diff --git a/src/main/java/graphql/execution/instrumentation/SimplePerformantInstrumentation.java b/src/main/java/graphql/execution/instrumentation/SimplePerformantInstrumentation.java index a2cb05b592..b09f44645e 100644 --- a/src/main/java/graphql/execution/instrumentation/SimplePerformantInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/SimplePerformantInstrumentation.java @@ -58,87 +58,87 @@ public class SimplePerformantInstrumentation implements Instrumentation { } @Override - public @Nullable InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @Override - public @Nullable InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @Override - public @Nullable InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @Override - public @Nullable InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @Override - public @Nullable ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + public @Nullable ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, @Nullable InstrumentationState state) { return ExecutionStrategyInstrumentationContext.NOOP; } @Override - public @Nullable ExecuteObjectInstrumentationContext beginExecuteObject(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + public @Nullable ExecuteObjectInstrumentationContext beginExecuteObject(InstrumentationExecutionStrategyParameters parameters, @Nullable InstrumentationState state) { return ExecuteObjectInstrumentationContext.NOOP; } @Override - public @Nullable InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @Override - public @Nullable InstrumentationContext beginFieldExecution(InstrumentationFieldParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginFieldExecution(InstrumentationFieldParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @Override - public @Nullable InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @Override - public @Nullable InstrumentationContext beginFieldCompletion(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginFieldCompletion(InstrumentationFieldCompleteParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @Override - public @Nullable InstrumentationContext beginFieldListCompletion(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginFieldListCompletion(InstrumentationFieldCompleteParameters parameters, @Nullable InstrumentationState state) { return noOp(); } @Override - public ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters, InstrumentationState state) { + public ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return executionInput; } @Override - public DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters, InstrumentationState state) { + public DocumentAndVariables instrumentDocumentAndVariables(DocumentAndVariables documentAndVariables, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return documentAndVariables; } @Override - public GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExecutionParameters parameters, InstrumentationState state) { + public GraphQLSchema instrumentSchema(GraphQLSchema schema, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return schema; } @Override - public ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, InstrumentationExecutionParameters parameters, InstrumentationState state) { + public ExecutionContext instrumentExecutionContext(ExecutionContext executionContext, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return executionContext; } @Override - public DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + public DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters, @Nullable InstrumentationState state) { return dataFetcher; } @Override - public CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, InstrumentationState state) { + public CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { return CompletableFuture.completedFuture(executionResult); } } diff --git a/src/main/java/graphql/execution/instrumentation/fieldvalidation/FieldValidationInstrumentation.java b/src/main/java/graphql/execution/instrumentation/fieldvalidation/FieldValidationInstrumentation.java index bea5d52577..536fc25419 100644 --- a/src/main/java/graphql/execution/instrumentation/fieldvalidation/FieldValidationInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/fieldvalidation/FieldValidationInstrumentation.java @@ -40,7 +40,7 @@ public FieldValidationInstrumentation(FieldValidation fieldValidation) { } @Override - public @Nullable InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, InstrumentationState state) { + public @Nullable InstrumentationContext beginExecuteOperation(InstrumentationExecuteOperationParameters parameters, @Nullable InstrumentationState state) { List errors = FieldValidationSupport.validateFieldsAndArguments(fieldValidation, parameters.getExecutionContext()); if (errors != null && !errors.isEmpty()) { throw new AbortExecutionException(errors); diff --git a/src/main/java/graphql/execution/instrumentation/tracing/TracingInstrumentation.java b/src/main/java/graphql/execution/instrumentation/tracing/TracingInstrumentation.java index f522ed1544..1de7082f38 100644 --- a/src/main/java/graphql/execution/instrumentation/tracing/TracingInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/tracing/TracingInstrumentation.java @@ -78,7 +78,7 @@ public TracingInstrumentation(Options options) { } @Override - public CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, InstrumentationState rawState) { + public CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState rawState) { Map currentExt = executionResult.getExtensions(); TracingSupport tracingSupport = ofState(rawState); @@ -89,21 +89,21 @@ public CompletableFuture instrumentExecutionResult(ExecutionRes } @Override - public InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState rawState) { + public InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, @Nullable InstrumentationState rawState) { TracingSupport tracingSupport = ofState(rawState); TracingSupport.TracingContext ctx = tracingSupport.beginField(parameters.getEnvironment(), parameters.isTrivialDataFetcher()); return whenCompleted((result, t) -> ctx.onEnd()); } @Override - public InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, InstrumentationState rawState) { + public InstrumentationContext beginParse(InstrumentationExecutionParameters parameters, @Nullable InstrumentationState rawState) { TracingSupport tracingSupport = ofState(rawState); TracingSupport.TracingContext ctx = tracingSupport.beginParse(); return whenCompleted((result, t) -> ctx.onEnd()); } @Override - public InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, InstrumentationState rawState) { + public InstrumentationContext> beginValidation(InstrumentationValidationParameters parameters, @Nullable InstrumentationState rawState) { TracingSupport tracingSupport = ofState(rawState); TracingSupport.TracingContext ctx = tracingSupport.beginValidation(); return whenCompleted((result, t) -> ctx.onEnd()); diff --git a/src/test/groovy/graphql/execution/instrumentation/InstrumentationDefaultMethodsTest.groovy b/src/test/groovy/graphql/execution/instrumentation/InstrumentationDefaultMethodsTest.groovy index 716094570c..a154befacc 100644 --- a/src/test/groovy/graphql/execution/instrumentation/InstrumentationDefaultMethodsTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/InstrumentationDefaultMethodsTest.groovy @@ -1,6 +1,12 @@ package graphql.execution.instrumentation +import graphql.ExecutionResult +import graphql.GraphQL +import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters +import graphql.schema.idl.RuntimeWiring +import graphql.schema.idl.SchemaGenerator +import graphql.schema.idl.SchemaParser import spock.lang.Specification class InstrumentationDefaultMethodsTest extends Specification { @@ -139,6 +145,44 @@ class InstrumentationDefaultMethodsTest extends Specification { ] } + def "simple performant instrumentation createState is null by default and begin hooks accept null state"() { + when: + def created = SimplePerformantInstrumentation.INSTANCE.createState(null) + def beginCtx = SimplePerformantInstrumentation.INSTANCE.beginExecution(null, null) + + then: + created == null + beginCtx != null + // null state must not throw (optional-state contract; Kotlin sees @Nullable after #4433) + noExceptionThrown() + } + + def "stateless SimplePerformantInstrumentation subclass executes with null state"() { + given: + def seenStates = [] + def instrumentation = new SimplePerformantInstrumentation() { + @Override + InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, InstrumentationState state) { + seenStates << state + return SimpleInstrumentationContext.noOp() + } + } + def typeRegistry = new SchemaParser().parse(""" + type Query { + hello: String + } + """) + def schema = new SchemaGenerator().makeExecutableSchema(typeRegistry, RuntimeWiring.MOCKED_WIRING) + def graphQL = GraphQL.newGraphQL(schema).instrumentation(instrumentation).build() + + when: + def result = graphQL.execute("{ hello }") + + then: + result.errors.isEmpty() + seenStates == [null] + } + private static Instrumentation instrumentationReturning(FieldFetchingInstrumentationContext context) { return new Instrumentation() { @Override From b7bb74e223703badf9a1678bb9cb6132050c24ec Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Sun, 23 Aug 2026 08:56:14 +1000 Subject: [PATCH 2/2] Propagate nullable chained instrumentation state --- .../ChainedInstrumentation.java | 42 ++++++++--------- .../NoContextChainedInstrumentation.java | 2 +- .../InstrumentationDefaultMethodsTest.groovy | 45 +++++++++---------- .../KotlinInstrumentationStateFixtures.kt | 26 +++++++++++ 4 files changed, 69 insertions(+), 46 deletions(-) create mode 100644 src/test/kotlin/graphql/execution/instrumentation/KotlinInstrumentationStateFixtures.kt diff --git a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java index 568a716274..573095f904 100644 --- a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java @@ -76,7 +76,7 @@ private static ChainedInstrumentationState asChainedState(@Nullable Instrumentat return (ChainedInstrumentationState) assertNotNull(state); } - private @Nullable InstrumentationContext chainedCtx(@Nullable InstrumentationState state, BiFunction> mapper) { + private @Nullable InstrumentationContext chainedCtx(@Nullable InstrumentationState state, BiFunction> mapper) { // if we have zero or 1 instrumentations (and 1 is the most common), then we can avoid an object allocation // of the ChainedInstrumentationContext since it won't be needed if (instrumentations.isEmpty()) { @@ -89,23 +89,23 @@ private static ChainedInstrumentationState asChainedState(@Nullable Instrumentat return new ChainedInstrumentationContext<>(chainedMapAndDropNulls(chainedInstrumentationState, mapper)); } - private T chainedInstrument(@Nullable InstrumentationState state, T input, ChainedInstrumentationFunction mapper) { + private T chainedInstrument(@Nullable InstrumentationState state, T input, ChainedInstrumentationFunction mapper) { ChainedInstrumentationState chainedInstrumentationState = asChainedState(state); for (int i = 0; i < instrumentations.size(); i++) { Instrumentation instrumentation = instrumentations.get(i); - InstrumentationState specificState = chainedInstrumentationState.getState(i); + @Nullable InstrumentationState specificState = chainedInstrumentationState.getState(i); input = mapper.apply(instrumentation, specificState, input); } return input; } - protected ImmutableList chainedMapAndDropNulls(@Nullable InstrumentationState state, BiFunction mapper) { + protected ImmutableList chainedMapAndDropNulls(@Nullable InstrumentationState state, BiFunction mapper) { ChainedInstrumentationState chainedInstrumentationState = asChainedState(state); ImmutableList.Builder result = ImmutableList.builderWithExpectedSize(instrumentations.size()); for (int i = 0; i < instrumentations.size(); i++) { Instrumentation instrumentation = instrumentations.get(i); - InstrumentationState specificState = chainedInstrumentationState.getState(i); - T value = mapper.apply(instrumentation, specificState); + @Nullable InstrumentationState specificState = chainedInstrumentationState.getState(i); + @Nullable T value = mapper.apply(instrumentation, specificState); if (value != null) { result.add(value); } @@ -113,11 +113,11 @@ protected ImmutableList chainedMapAndDropNulls(@Nullable InstrumentationS return result.build(); } - protected void chainedConsume(@Nullable InstrumentationState state, BiConsumer stateConsumer) { + protected void chainedConsume(@Nullable InstrumentationState state, BiConsumer stateConsumer) { ChainedInstrumentationState chainedInstrumentationState = asChainedState(state); for (int i = 0; i < instrumentations.size(); i++) { Instrumentation instrumentation = instrumentations.get(i); - InstrumentationState specificState = chainedInstrumentationState.getState(i); + @Nullable InstrumentationState specificState = chainedInstrumentationState.getState(i); stateConsumer.accept(instrumentation, specificState); } } @@ -159,7 +159,7 @@ public CompletableFuture createStateAsync(InstrumentationC if (instrumentations.isEmpty()) { return ExecutionStrategyInstrumentationContext.NOOP; } - BiFunction mapper = (instrumentation, specificState) -> instrumentation.beginExecutionStrategy(parameters, specificState); + BiFunction mapper = (instrumentation, specificState) -> instrumentation.beginExecutionStrategy(parameters, specificState); ChainedInstrumentationState chainedInstrumentationState = asChainedState(state); if (instrumentations.size() == 1) { return mapper.apply(instrumentations.get(0), chainedInstrumentationState.getState(0)); @@ -172,7 +172,7 @@ public CompletableFuture createStateAsync(InstrumentationC if (instrumentations.isEmpty()) { return ExecuteObjectInstrumentationContext.NOOP; } - BiFunction mapper = (instrumentation, specificState) -> instrumentation.beginExecuteObject(parameters, specificState); + BiFunction mapper = (instrumentation, specificState) -> instrumentation.beginExecuteObject(parameters, specificState); ChainedInstrumentationState chainedInstrumentationState = asChainedState(state); if (instrumentations.size() == 1) { return mapper.apply(instrumentations.get(0), chainedInstrumentationState.getState(0)); @@ -219,7 +219,7 @@ private FieldFetchingInstrumentationContext chainedFieldFetchingCtx(Instrumentat ImmutableList.Builder builder = null; for (int i = 0; i < instrumentations.size(); i++) { Instrumentation instrumentation = instrumentations.get(i); - FieldFetchingInstrumentationContext context = instrumentation.beginFieldFetching(parameters, chainedInstrumentationState.getState(i)); + @Nullable FieldFetchingInstrumentationContext context = instrumentation.beginFieldFetching(parameters, chainedInstrumentationState.getState(i)); if (context == null || context == FieldFetchingInstrumentationContext.NOOP) { continue; } @@ -278,16 +278,16 @@ public ExecutionContext instrumentExecutionContext(ExecutionContext executionCon @Override public DataFetcher instrumentDataFetcher(DataFetcher dataFetcher, InstrumentationFieldFetchParameters parameters, @Nullable InstrumentationState state) { - return chainedInstrument(state, dataFetcher, (Instrumentation instrumentation, InstrumentationState specificState, DataFetcher accumulator) -> + return chainedInstrument(state, dataFetcher, (Instrumentation instrumentation, @Nullable InstrumentationState specificState, DataFetcher accumulator) -> instrumentation.instrumentDataFetcher(accumulator, parameters, specificState)); } @Override public CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) { - ImmutableList> entries = chainedMapAndDropNulls(state, AbstractMap.SimpleEntry::new); + ImmutableList> entries = chainedMapAndDropNulls(state, AbstractMap.SimpleEntry::new); CompletableFuture> resultsFuture = Async.eachSequentially(entries, (entry, prevResults) -> { Instrumentation instrumentation = entry.getKey(); - InstrumentationState specificState = entry.getValue(); + @Nullable InstrumentationState specificState = entry.getValue(); ExecutionResult lastResult = !prevResults.isEmpty() ? prevResults.get(prevResults.size() - 1) : executionResult; return instrumentation.instrumentExecutionResult(lastResult, parameters, specificState); }); @@ -295,21 +295,21 @@ public CompletableFuture instrumentExecutionResult(ExecutionRes } static class ChainedInstrumentationState implements InstrumentationState { - private final List instrumentationStates; + private final List<@Nullable InstrumentationState> instrumentationStates; - private ChainedInstrumentationState(List instrumentationStates) { + private ChainedInstrumentationState(List<@Nullable InstrumentationState> instrumentationStates) { this.instrumentationStates = instrumentationStates; } - private InstrumentationState getState(int index) { + private @Nullable InstrumentationState getState(int index) { return instrumentationStates.get(index); } private static CompletableFuture combineAll(List instrumentations, InstrumentationCreateStateParameters parameters) { - Async.CombinedBuilder builder = Async.ofExpectedSize(instrumentations.size()); + Async.CombinedBuilder<@Nullable InstrumentationState> builder = Async.ofExpectedSize(instrumentations.size()); for (Instrumentation instrumentation : instrumentations) { // state can be null including the CF so handle that - CompletableFuture stateCF = Async.orNullCompletedFuture(instrumentation.createStateAsync(parameters)); + CompletableFuture<@Nullable InstrumentationState> stateCF = Async.<@Nullable InstrumentationState>orNullCompletedFuture(instrumentation.createStateAsync(parameters)); builder.add(stateCF); } return builder.await().thenApply(ChainedInstrumentationState::new); @@ -443,8 +443,8 @@ public void onCompleted(@Nullable Object result, @Nullable Throwable t) { } @FunctionalInterface - private interface ChainedInstrumentationFunction { - R apply(I instrumentation, S state, V value); + private interface ChainedInstrumentationFunction { + R apply(I instrumentation, @Nullable InstrumentationState state, V value); } diff --git a/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java index 96187d6bb2..0d2eb84998 100644 --- a/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java @@ -51,7 +51,7 @@ public NoContextChainedInstrumentation(Instrumentation... instrumentations) { super(instrumentations); } - private @Nullable T runAll(@Nullable InstrumentationState state, BiConsumer stateConsumer) { + private @Nullable T runAll(@Nullable InstrumentationState state, BiConsumer stateConsumer) { chainedConsume(state, stateConsumer); return null; } diff --git a/src/test/groovy/graphql/execution/instrumentation/InstrumentationDefaultMethodsTest.groovy b/src/test/groovy/graphql/execution/instrumentation/InstrumentationDefaultMethodsTest.groovy index a154befacc..793de9e2e5 100644 --- a/src/test/groovy/graphql/execution/instrumentation/InstrumentationDefaultMethodsTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/InstrumentationDefaultMethodsTest.groovy @@ -1,8 +1,6 @@ package graphql.execution.instrumentation -import graphql.ExecutionResult import graphql.GraphQL -import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters import graphql.schema.idl.RuntimeWiring import graphql.schema.idl.SchemaGenerator @@ -145,28 +143,10 @@ class InstrumentationDefaultMethodsTest extends Specification { ] } - def "simple performant instrumentation createState is null by default and begin hooks accept null state"() { - when: - def created = SimplePerformantInstrumentation.INSTANCE.createState(null) - def beginCtx = SimplePerformantInstrumentation.INSTANCE.beginExecution(null, null) - - then: - created == null - beginCtx != null - // null state must not throw (optional-state contract; Kotlin sees @Nullable after #4433) - noExceptionThrown() - } - - def "stateless SimplePerformantInstrumentation subclass executes with null state"() { + def "stateless Kotlin instrumentation executes with null state"() { given: - def seenStates = [] - def instrumentation = new SimplePerformantInstrumentation() { - @Override - InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters, InstrumentationState state) { - seenStates << state - return SimpleInstrumentationContext.noOp() - } - } + def fixtureClass = Class.forName("graphql.execution.instrumentation.KotlinStatelessInstrumentation") + def instrumentation = fixtureClass.getDeclaredConstructor().newInstance() as Instrumentation def typeRegistry = new SchemaParser().parse(""" type Query { hello: String @@ -180,7 +160,24 @@ class InstrumentationDefaultMethodsTest extends Specification { then: result.errors.isEmpty() - seenStates == [null] + fixtureClass.getMethod("getSeenStates").invoke(instrumentation) == [null] + } + + def "Kotlin chained instrumentation receives nullable child state"() { + given: + def fixtureClass = Class.forName("graphql.execution.instrumentation.KotlinChainedInstrumentation") + def instrumentation = fixtureClass + .getDeclaredConstructor(Instrumentation) + .newInstance(SimplePerformantInstrumentation.INSTANCE) as ChainedInstrumentation + def state = instrumentation.createStateAsync(null).join() + + when: + def childState = fixtureClass + .getMethod("consumeChildState", InstrumentationState) + .invoke(instrumentation, state) + + then: + childState == null } private static Instrumentation instrumentationReturning(FieldFetchingInstrumentationContext context) { diff --git a/src/test/kotlin/graphql/execution/instrumentation/KotlinInstrumentationStateFixtures.kt b/src/test/kotlin/graphql/execution/instrumentation/KotlinInstrumentationStateFixtures.kt new file mode 100644 index 0000000000..cc816e32d4 --- /dev/null +++ b/src/test/kotlin/graphql/execution/instrumentation/KotlinInstrumentationStateFixtures.kt @@ -0,0 +1,26 @@ +package graphql.execution.instrumentation + +import graphql.ExecutionResult +import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters + +class KotlinStatelessInstrumentation : SimplePerformantInstrumentation() { + val seenStates = mutableListOf() + + override fun beginExecution( + parameters: InstrumentationExecutionParameters, + state: InstrumentationState?, + ): InstrumentationContext { + seenStates.add(state) + return SimpleInstrumentationContext.noOp() + } +} + +class KotlinChainedInstrumentation( + instrumentation: Instrumentation, +) : ChainedInstrumentation(instrumentation) { + fun consumeChildState(state: InstrumentationState?): InstrumentationState? { + var observedState: InstrumentationState? = null + chainedConsume(state) { _, childState -> observedState = childState } + return observedState + } +}