Skip to content

Commit 39b95dd

Browse files
andimarekandimarek
committed
Avoid chained no-op field fetch contexts
1 parent b8fbcb3 commit 39b95dd

2 files changed

Lines changed: 115 additions & 5 deletions

File tree

‎src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,13 +198,39 @@ public CompletableFuture<InstrumentationState> createStateAsync(InstrumentationC
198198
if (instrumentations.isEmpty()) {
199199
return FieldFetchingInstrumentationContext.NOOP;
200200
}
201-
BiFunction<Instrumentation, InstrumentationState, FieldFetchingInstrumentationContext> mapper = (instrumentation, specificState) -> instrumentation.beginFieldFetching(parameters, specificState);
202201
ChainedInstrumentationState chainedInstrumentationState = (ChainedInstrumentationState) state;
203202
if (instrumentations.size() == 1) {
204-
return mapper.apply(instrumentations.get(0), chainedInstrumentationState.getState(0));
203+
return instrumentations.get(0).beginFieldFetching(parameters, chainedInstrumentationState.getState(0));
204+
}
205+
return chainedFieldFetchingCtx(parameters, chainedInstrumentationState);
206+
}
207+
208+
private FieldFetchingInstrumentationContext chainedFieldFetchingCtx(InstrumentationFieldFetchParameters parameters, ChainedInstrumentationState chainedInstrumentationState) {
209+
@Nullable FieldFetchingInstrumentationContext firstContext = null;
210+
ImmutableList.Builder<FieldFetchingInstrumentationContext> builder = null;
211+
for (int i = 0; i < instrumentations.size(); i++) {
212+
Instrumentation instrumentation = instrumentations.get(i);
213+
FieldFetchingInstrumentationContext context = instrumentation.beginFieldFetching(parameters, chainedInstrumentationState.getState(i));
214+
if (context == null || context == FieldFetchingInstrumentationContext.NOOP) {
215+
continue;
216+
}
217+
if (firstContext == null) {
218+
firstContext = context;
219+
continue;
220+
}
221+
if (builder == null) {
222+
builder = ImmutableList.builder();
223+
builder.add(firstContext);
224+
}
225+
builder.add(context);
205226
}
206-
ImmutableList<FieldFetchingInstrumentationContext> objects = chainedMapAndDropNulls(chainedInstrumentationState, mapper);
207-
return new ChainedFieldFetchingInstrumentationContext(objects);
227+
if (builder != null) {
228+
return new ChainedFieldFetchingInstrumentationContext(builder.build());
229+
}
230+
if (firstContext != null) {
231+
return firstContext;
232+
}
233+
return FieldFetchingInstrumentationContext.NOOP;
208234
}
209235

210236
@Override
@@ -414,4 +440,3 @@ private interface ChainedInstrumentationFunction<I, S, V, R> {
414440

415441

416442
}
417-

‎src/test/groovy/graphql/execution/instrumentation/InstrumentationDefaultMethodsTest.groovy‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,4 +69,89 @@ class InstrumentationDefaultMethodsTest extends Specification {
6969
then:
7070
events == ["dispatched", "completed"]
7171
}
72+
73+
def "chained field fetching does not allocate a chained context when all entries are no-op"() {
74+
given:
75+
def instrumentation = new ChainedInstrumentation(new Instrumentation() {}, SimplePerformantInstrumentation.INSTANCE)
76+
def state = instrumentation.createStateAsync(null).join()
77+
78+
when:
79+
def context = instrumentation.beginFieldFetching(null, state)
80+
81+
then:
82+
context.is(FieldFetchingInstrumentationContext.NOOP)
83+
}
84+
85+
def "chained field fetching returns the only real context directly"() {
86+
given:
87+
def realContext = Mock(FieldFetchingInstrumentationContext)
88+
def instrumentation = new ChainedInstrumentation(
89+
new Instrumentation() {},
90+
instrumentationReturning(realContext),
91+
SimplePerformantInstrumentation.INSTANCE
92+
)
93+
def state = instrumentation.createStateAsync(null).join()
94+
95+
when:
96+
def context = instrumentation.beginFieldFetching(null, state)
97+
98+
then:
99+
context.is(realContext)
100+
}
101+
102+
def "chained field fetching keeps multiple real contexts and skips canonical no-ops"() {
103+
given:
104+
def events = []
105+
def instrumentation = new ChainedInstrumentation(
106+
new Instrumentation() {},
107+
instrumentationReturning(recordingContext("first", events)),
108+
SimplePerformantInstrumentation.INSTANCE,
109+
instrumentationReturning(recordingContext("second", events))
110+
)
111+
def state = instrumentation.createStateAsync(null).join()
112+
113+
when:
114+
def context = instrumentation.beginFieldFetching(null, state)
115+
context.onDispatched()
116+
context.onFetchedValue("value")
117+
context.onCompleted("complete", null)
118+
119+
then:
120+
events == [
121+
"first-dispatched",
122+
"second-dispatched",
123+
"first-fetched-value",
124+
"second-fetched-value",
125+
"first-completed-complete",
126+
"second-completed-complete",
127+
]
128+
}
129+
130+
private static Instrumentation instrumentationReturning(FieldFetchingInstrumentationContext context) {
131+
return new Instrumentation() {
132+
@Override
133+
FieldFetchingInstrumentationContext beginFieldFetching(InstrumentationFieldFetchParameters parameters, InstrumentationState state) {
134+
return context
135+
}
136+
}
137+
}
138+
139+
private static FieldFetchingInstrumentationContext recordingContext(String name, List<String> events) {
140+
return new FieldFetchingInstrumentationContext() {
141+
@Override
142+
void onDispatched() {
143+
events.add(name + "-dispatched")
144+
}
145+
146+
@Override
147+
void onFetchedValue(Object fetchedValue) {
148+
events.add(name + "-fetched-" + fetchedValue)
149+
}
150+
151+
@Override
152+
void onCompleted(Object result, Throwable t) {
153+
events.add(name + "-completed-" + result)
154+
}
155+
}
156+
}
72157
}

0 commit comments

Comments
 (0)