Skip to content

fix: mark InstrumentationState callback params as @Nullable #4435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andimarek merged 2 commits into from
Aug 23, 2026
Merged

fix: mark InstrumentationState callback params as @Nullable #4435

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
79423f0
fix: mark InstrumentationState callback params as @Nullable
arimu1 Aug 5, 2026
b7bb74e
Propagate nullable chained instrumentation state
andimarek Aug 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Propagate nullable chained instrumentation state
  • Loading branch information
@andimarek
andimarek committed Aug 22, 2026
commit b7bb74e223703badf9a1678bb9cb6132050c24ec
42 changes: 21 additions & 21 deletions src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ private static ChainedInstrumentationState asChainedState(@Nullable Instrumentat
return (ChainedInstrumentationState) assertNotNull(state);
}

private <T> @Nullable InstrumentationContext<T> chainedCtx(@Nullable InstrumentationState state, BiFunction<Instrumentation, InstrumentationState, InstrumentationContext<T>> mapper) {
private <T> @Nullable InstrumentationContext<T> chainedCtx(@Nullable InstrumentationState state, BiFunction<Instrumentation, @Nullable InstrumentationState, InstrumentationContext<T>> 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()) {
Expand All @@ -89,35 +89,35 @@ private static ChainedInstrumentationState asChainedState(@Nullable Instrumentat
return new ChainedInstrumentationContext<>(chainedMapAndDropNulls(chainedInstrumentationState, mapper));
}

private <T> T chainedInstrument(@Nullable InstrumentationState state, T input, ChainedInstrumentationFunction<Instrumentation, InstrumentationState, T, T> mapper) {
private <T> T chainedInstrument(@Nullable InstrumentationState state, T input, ChainedInstrumentationFunction<Instrumentation, T, T> 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 <T> ImmutableList<T> chainedMapAndDropNulls(@Nullable InstrumentationState state, BiFunction<Instrumentation, InstrumentationState, T> mapper) {
protected <T> ImmutableList<T> chainedMapAndDropNulls(@Nullable InstrumentationState state, BiFunction<Instrumentation, @Nullable InstrumentationState, @Nullable T> mapper) {
ChainedInstrumentationState chainedInstrumentationState = asChainedState(state);
ImmutableList.Builder<T> 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);
}
}
return result.build();
}

protected void chainedConsume(@Nullable InstrumentationState state, BiConsumer<Instrumentation, InstrumentationState> stateConsumer) {
protected void chainedConsume(@Nullable InstrumentationState state, BiConsumer<Instrumentation, @Nullable InstrumentationState> 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);
}
}
Expand Down Expand Up @@ -159,7 +159,7 @@ public CompletableFuture<InstrumentationState> createStateAsync(InstrumentationC
if (instrumentations.isEmpty()) {
return ExecutionStrategyInstrumentationContext.NOOP;
}
BiFunction<Instrumentation, InstrumentationState, ExecutionStrategyInstrumentationContext> mapper = (instrumentation, specificState) -> instrumentation.beginExecutionStrategy(parameters, specificState);
BiFunction<Instrumentation, @Nullable InstrumentationState, @Nullable ExecutionStrategyInstrumentationContext> mapper = (instrumentation, specificState) -> instrumentation.beginExecutionStrategy(parameters, specificState);
ChainedInstrumentationState chainedInstrumentationState = asChainedState(state);
if (instrumentations.size() == 1) {
return mapper.apply(instrumentations.get(0), chainedInstrumentationState.getState(0));
Expand All @@ -172,7 +172,7 @@ public CompletableFuture<InstrumentationState> createStateAsync(InstrumentationC
if (instrumentations.isEmpty()) {
return ExecuteObjectInstrumentationContext.NOOP;
}
BiFunction<Instrumentation, InstrumentationState, ExecuteObjectInstrumentationContext> mapper = (instrumentation, specificState) -> instrumentation.beginExecuteObject(parameters, specificState);
BiFunction<Instrumentation, @Nullable InstrumentationState, @Nullable ExecuteObjectInstrumentationContext> mapper = (instrumentation, specificState) -> instrumentation.beginExecuteObject(parameters, specificState);
ChainedInstrumentationState chainedInstrumentationState = asChainedState(state);
if (instrumentations.size() == 1) {
return mapper.apply(instrumentations.get(0), chainedInstrumentationState.getState(0));
Expand Down Expand Up @@ -219,7 +219,7 @@ private FieldFetchingInstrumentationContext chainedFieldFetchingCtx(Instrumentat
ImmutableList.Builder<FieldFetchingInstrumentationContext> 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;
}
Expand Down Expand Up @@ -278,38 +278,38 @@ 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<ExecutionResult> instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, @Nullable InstrumentationState state) {
ImmutableList<Map.Entry<Instrumentation, InstrumentationState>> entries = chainedMapAndDropNulls(state, AbstractMap.SimpleEntry::new);
ImmutableList<Map.Entry<Instrumentation, @Nullable InstrumentationState>> entries = chainedMapAndDropNulls(state, AbstractMap.SimpleEntry::new);
CompletableFuture<List<ExecutionResult>> 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);
});
return resultsFuture.thenApply((results) -> results.isEmpty() ? executionResult : results.get(results.size() - 1));
}

static class ChainedInstrumentationState implements InstrumentationState {
private final List<InstrumentationState> instrumentationStates;
private final List<@Nullable InstrumentationState> instrumentationStates;

private ChainedInstrumentationState(List<InstrumentationState> 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<InstrumentationState> combineAll(List<Instrumentation> instrumentations, InstrumentationCreateStateParameters parameters) {
Async.CombinedBuilder<InstrumentationState> 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<InstrumentationState> 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);
Expand Down Expand Up @@ -443,8 +443,8 @@ public void onCompleted(@Nullable Object result, @Nullable Throwable t) {
}

@FunctionalInterface
private interface ChainedInstrumentationFunction<I, S, V, R> {
R apply(I instrumentation, S state, V value);
private interface ChainedInstrumentationFunction<I, V, R> {
R apply(I instrumentation, @Nullable InstrumentationState state, V value);
}


Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public NoContextChainedInstrumentation(Instrumentation... instrumentations) {
super(instrumentations);
}

private <T> @Nullable T runAll(@Nullable InstrumentationState state, BiConsumer<Instrumentation, InstrumentationState> stateConsumer) {
private <T> @Nullable T runAll(@Nullable InstrumentationState state, BiConsumer<Instrumentation, @Nullable InstrumentationState> stateConsumer) {
chainedConsume(state, stateConsumer);
return null;
}
Expand Down
45 changes: 21 additions & 24 deletions src/test/groovy/graphql/execution/instrumentation/InstrumentationDefaultMethodsTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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<ExecutionResult> 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
Expand All @@ -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) {
Expand Down
26 changes: 26 additions & 0 deletions src/test/kotlin/graphql/execution/instrumentation/KotlinInstrumentationStateFixtures.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package graphql.execution.instrumentation

import graphql.ExecutionResult
import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters

class KotlinStatelessInstrumentation : SimplePerformantInstrumentation() {
val seenStates = mutableListOf<InstrumentationState?>()

override fun beginExecution(
parameters: InstrumentationExecutionParameters,
state: InstrumentationState?,
): InstrumentationContext<ExecutionResult> {
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
}
}
Loading