diff --git a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java index ebf2758e3..f736bde96 100644 --- a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java @@ -198,13 +198,39 @@ public CompletableFuture createStateAsync(InstrumentationC if (instrumentations.isEmpty()) { return FieldFetchingInstrumentationContext.NOOP; } - BiFunction mapper = (instrumentation, specificState) -> instrumentation.beginFieldFetching(parameters, specificState); ChainedInstrumentationState chainedInstrumentationState = (ChainedInstrumentationState) state; if (instrumentations.size() == 1) { - return mapper.apply(instrumentations.get(0), chainedInstrumentationState.getState(0)); + return instrumentations.get(0).beginFieldFetching(parameters, chainedInstrumentationState.getState(0)); + } + return chainedFieldFetchingCtx(parameters, chainedInstrumentationState); + } + + private FieldFetchingInstrumentationContext chainedFieldFetchingCtx(InstrumentationFieldFetchParameters parameters, ChainedInstrumentationState chainedInstrumentationState) { + @Nullable FieldFetchingInstrumentationContext firstContext = null; + 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)); + if (context == null || context == FieldFetchingInstrumentationContext.NOOP) { + continue; + } + if (firstContext == null) { + firstContext = context; + continue; + } + if (builder == null) { + builder = ImmutableList.builder(); + builder.add(firstContext); + } + builder.add(context); } - ImmutableList objects = chainedMapAndDropNulls(chainedInstrumentationState, mapper); - return new ChainedFieldFetchingInstrumentationContext(objects); + if (builder != null) { + return new ChainedFieldFetchingInstrumentationContext(builder.build()); + } + if (firstContext != null) { + return firstContext; + } + return FieldFetchingInstrumentationContext.NOOP; } @Override @@ -414,4 +440,3 @@ private interface ChainedInstrumentationFunction { } - diff --git a/src/main/java/graphql/execution/instrumentation/Instrumentation.java b/src/main/java/graphql/execution/instrumentation/Instrumentation.java index 2bfb6eadc..441bf049b 100644 --- a/src/main/java/graphql/execution/instrumentation/Instrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/Instrumentation.java @@ -241,6 +241,9 @@ default InstrumentationContext beginFieldFetch(InstrumentationFieldFetch @Nullable default FieldFetchingInstrumentationContext beginFieldFetching(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { InstrumentationContext ctx = beginFieldFetch(parameters, state); + if (ctx == noOp()) { + return FieldFetchingInstrumentationContext.NOOP; + } return FieldFetchingInstrumentationContext.adapter(ctx); } diff --git a/src/test/groovy/graphql/execution/instrumentation/InstrumentationDefaultMethodsTest.groovy b/src/test/groovy/graphql/execution/instrumentation/InstrumentationDefaultMethodsTest.groovy new file mode 100644 index 000000000..716094570 --- /dev/null +++ b/src/test/groovy/graphql/execution/instrumentation/InstrumentationDefaultMethodsTest.groovy @@ -0,0 +1,169 @@ +package graphql.execution.instrumentation + +import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters +import spock.lang.Specification + +class InstrumentationDefaultMethodsTest extends Specification { + + def "default begin field fetching does not allocate an adapter for inherited no-op"() { + given: + def instrumentation = new Instrumentation() {} + + when: + def context = instrumentation.beginFieldFetching(null, null) + + then: + context.is(FieldFetchingInstrumentationContext.NOOP) + } + + def "simple performant instrumentation begin field fetching does not allocate an adapter for inherited no-op"() { + when: + def context = SimplePerformantInstrumentation.INSTANCE.beginFieldFetching(null, null) + + then: + context.is(FieldFetchingInstrumentationContext.NOOP) + } + + def "default begin field fetching does not allocate an adapter when deprecated override returns no-op"() { + given: + def instrumentation = new Instrumentation() { + @Override + InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + return SimpleInstrumentationContext.noOp() + } + } + + when: + def context = instrumentation.beginFieldFetching(null, null) + + then: + context.is(FieldFetchingInstrumentationContext.NOOP) + } + + def "default begin field fetching still adapts deprecated begin field fetch overrides"() { + given: + def events = [] + def instrumentation = new Instrumentation() { + @Override + InstrumentationContext beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + return new InstrumentationContext() { + @Override + void onDispatched() { + events.add("dispatched") + } + + @Override + void onCompleted(Object result, Throwable t) { + events.add(result) + } + } + } + } + + when: + def context = instrumentation.beginFieldFetching(null, null) + context.onDispatched() + context.onFetchedValue("ignored") + context.onCompleted("completed", null) + + then: + events == ["dispatched", "completed"] + } + + def "chained field fetching does not allocate a chained context when all entries are no-op"() { + given: + def instrumentation = new ChainedInstrumentation(new Instrumentation() {}, SimplePerformantInstrumentation.INSTANCE) + def state = instrumentation.createStateAsync(null).join() + + when: + def context = instrumentation.beginFieldFetching(null, state) + + then: + context.is(FieldFetchingInstrumentationContext.NOOP) + } + + def "chained field fetching returns the only real context directly"() { + given: + def realContext = Mock(FieldFetchingInstrumentationContext) + def instrumentation = new ChainedInstrumentation( + new Instrumentation() {}, + instrumentationReturning(realContext), + SimplePerformantInstrumentation.INSTANCE + ) + def state = instrumentation.createStateAsync(null).join() + + when: + def context = instrumentation.beginFieldFetching(null, state) + + then: + context.is(realContext) + } + + def "chained field fetching treats null contexts as no-op entries"() { + given: + def instrumentation = new ChainedInstrumentation(new Instrumentation() {}, instrumentationReturning(null)) + def state = instrumentation.createStateAsync(null).join() + + when: + def context = instrumentation.beginFieldFetching(null, state) + + then: + context.is(FieldFetchingInstrumentationContext.NOOP) + } + + def "chained field fetching keeps multiple real contexts and skips canonical no-ops"() { + given: + def events = [] + def instrumentation = new ChainedInstrumentation( + new Instrumentation() {}, + instrumentationReturning(recordingContext("first", events)), + SimplePerformantInstrumentation.INSTANCE, + instrumentationReturning(recordingContext("second", events)) + ) + def state = instrumentation.createStateAsync(null).join() + + when: + def context = instrumentation.beginFieldFetching(null, state) + context.onDispatched() + context.onFetchedValue("value") + context.onCompleted("complete", null) + + then: + events == [ + "first-dispatched", + "second-dispatched", + "first-fetched-value", + "second-fetched-value", + "first-completed-complete", + "second-completed-complete", + ] + } + + private static Instrumentation instrumentationReturning(FieldFetchingInstrumentationContext context) { + return new Instrumentation() { + @Override + FieldFetchingInstrumentationContext beginFieldFetching(InstrumentationFieldFetchParameters parameters, InstrumentationState state) { + return context + } + } + } + + private static FieldFetchingInstrumentationContext recordingContext(String name, List events) { + return new FieldFetchingInstrumentationContext() { + @Override + void onDispatched() { + events.add(name + "-dispatched") + } + + @Override + void onFetchedValue(Object fetchedValue) { + events.add(name + "-fetched-" + fetchedValue) + } + + @Override + void onCompleted(Object result, Throwable t) { + events.add(name + "-completed-" + result) + } + } + } +}