Skip to content

Commit a622d0d

Browse files
committed
PR feedback
1 parent 5b04714 commit a622d0d

3 files changed

Lines changed: 18 additions & 16 deletions

File tree

src/main/java/graphql/execution/instrumentation/dataloader/DataLoaderDispatchingContextKeys.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ private DataLoaderDispatchingContextKeys() {
5050
*
5151
* @param graphQLContext
5252
*/
53-
public static void enableDataLoaderChaining(GraphQLContext graphQLContext) {
54-
graphQLContext.put(ENABLE_DATA_LOADER_CHAINING, true);
53+
public static void setEnableDataLoaderChaining(GraphQLContext graphQLContext, boolean enabled) {
54+
graphQLContext.put(ENABLE_DATA_LOADER_CHAINING, enabled);
5555
}
5656

5757

src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategy.java

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -179,12 +179,7 @@ public PerLevelDataLoaderDispatchStrategy(ExecutionContext executionContext) {
179179
this.executionContext = executionContext;
180180

181181
GraphQLContext graphQLContext = executionContext.getGraphQLContext();
182-
Long batchWindowNs = graphQLContext.get(DataLoaderDispatchingContextKeys.DELAYED_DATA_LOADER_BATCH_WINDOW_SIZE_NANO_SECONDS);
183-
if (batchWindowNs != null) {
184-
this.batchWindowNs = batchWindowNs;
185-
} else {
186-
this.batchWindowNs = DEFAULT_BATCH_WINDOW_NANO_SECONDS_DEFAULT;
187-
}
182+
this.batchWindowNs = graphQLContext.getOrDefault(DataLoaderDispatchingContextKeys.DELAYED_DATA_LOADER_BATCH_WINDOW_SIZE_NANO_SECONDS, DEFAULT_BATCH_WINDOW_NANO_SECONDS_DEFAULT);
188183

189184
this.delayedDataLoaderDispatchExecutor = new InterThreadMemoizedSupplier<>(() -> {
190185
DelayedDataLoaderDispatcherExecutorFactory delayedDataLoaderDispatcherExecutorFactory = graphQLContext.get(DataLoaderDispatchingContextKeys.DELAYED_DATA_LOADER_DISPATCHING_EXECUTOR_FACTORY);
@@ -194,8 +189,7 @@ public PerLevelDataLoaderDispatchStrategy(ExecutionContext executionContext) {
194189
return defaultDelayedDLCFBatchWindowScheduler.get();
195190
});
196191

197-
Boolean enableDataLoaderChaining = graphQLContext.get(DataLoaderDispatchingContextKeys.ENABLE_DATA_LOADER_CHAINING);
198-
this.enableDataLoaderChaining = enableDataLoaderChaining != null && enableDataLoaderChaining;
192+
this.enableDataLoaderChaining = graphQLContext.getBoolean(DataLoaderDispatchingContextKeys.ENABLE_DATA_LOADER_CHAINING, false);
199193
}
200194

201195
@Override

src/test/groovy/graphql/ChainedDataLoaderTest.groovy

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import java.util.concurrent.TimeUnit
1818
import java.util.concurrent.atomic.AtomicInteger
1919

2020
import static graphql.ExecutionInput.newExecutionInput
21+
import static graphql.execution.instrumentation.dataloader.DataLoaderDispatchingContextKeys.setEnableDataLoaderChaining
2122
import static java.util.concurrent.CompletableFuture.supplyAsync
2223

2324
class ChainedDataLoaderTest extends Specification {
@@ -73,7 +74,7 @@ class ChainedDataLoaderTest extends Specification {
7374

7475
def query = "{ dogName catName } "
7576
def ei = newExecutionInput(query).dataLoaderRegistry(dataLoaderRegistry).build()
76-
DataLoaderDispatchingContextKeys.enableDataLoaderChaining(ei.graphQLContext)
77+
setEnableDataLoaderChaining(ei.graphQLContext, true)
7778

7879
when:
7980
def efCF = graphQL.executeAsync(ei)
@@ -164,7 +165,8 @@ class ChainedDataLoaderTest extends Specification {
164165
def graphQL = GraphQL.newGraphQL(schema).build()
165166

166167
def query = "{ hello helloDelayed} "
167-
def ei = newExecutionInput(query).graphQLContext([(DataLoaderDispatchingContextKeys.ENABLE_DATA_LOADER_CHAINING): true]).dataLoaderRegistry(dataLoaderRegistry).build()
168+
def ei = newExecutionInput(query).dataLoaderRegistry(dataLoaderRegistry).build()
169+
setEnableDataLoaderChaining(ei.graphQLContext, true)
168170

169171
when:
170172
def efCF = graphQL.executeAsync(ei)
@@ -255,7 +257,8 @@ class ChainedDataLoaderTest extends Specification {
255257
def graphQL = GraphQL.newGraphQL(schema).build()
256258

257259
def query = "{ foo } "
258-
def ei = newExecutionInput(query).graphQLContext([(DataLoaderDispatchingContextKeys.ENABLE_DATA_LOADER_CHAINING): true]).dataLoaderRegistry(dataLoaderRegistry).build()
260+
def ei = newExecutionInput(query).dataLoaderRegistry(dataLoaderRegistry).build()
261+
setEnableDataLoaderChaining(ei.graphQLContext, true)
259262

260263
when:
261264
def efCF = graphQL.executeAsync(ei)
@@ -323,7 +326,8 @@ class ChainedDataLoaderTest extends Specification {
323326
def graphQL = GraphQL.newGraphQL(schema).build()
324327

325328
def query = "{ dogName catName } "
326-
def ei = newExecutionInput(query).graphQLContext([(DataLoaderDispatchingContextKeys.ENABLE_DATA_LOADER_CHAINING): true]).dataLoaderRegistry(dataLoaderRegistry).build()
329+
def ei = newExecutionInput(query).dataLoaderRegistry(dataLoaderRegistry).build()
330+
setEnableDataLoaderChaining(ei.graphQLContext, true)
327331

328332
when:
329333
def efCF = graphQL.executeAsync(ei)
@@ -382,7 +386,10 @@ class ChainedDataLoaderTest extends Specification {
382386
def graphQL = GraphQL.newGraphQL(schema).build()
383387

384388
def query = "{ foo bar } "
385-
def ei = newExecutionInput(query).graphQLContext([(DataLoaderDispatchingContextKeys.ENABLE_DATA_LOADER_CHAINING): true]).dataLoaderRegistry(dataLoaderRegistry).build()
389+
390+
def eiBuilder = ExecutionInput.newExecutionInput(query)
391+
def ei = eiBuilder.dataLoaderRegistry(dataLoaderRegistry).build()
392+
setEnableDataLoaderChaining(ei.graphQLContext, true);
386393

387394
// make the window 250ms
388395
DataLoaderDispatchingContextKeys.setDelayedDataLoaderBatchWindowSizeNanoSeconds(ei.graphQLContext, 1_000_000L * 250)
@@ -432,7 +439,8 @@ class ChainedDataLoaderTest extends Specification {
432439
def graphQL = GraphQL.newGraphQL(schema).build()
433440

434441
def query = "{ foo } "
435-
def ei = newExecutionInput(query).graphQLContext([(DataLoaderDispatchingContextKeys.ENABLE_DATA_LOADER_CHAINING): true]).dataLoaderRegistry(dataLoaderRegistry).build()
442+
def ei = newExecutionInput(query).dataLoaderRegistry(dataLoaderRegistry).build()
443+
setEnableDataLoaderChaining(ei.graphQLContext, true);
436444

437445

438446
ScheduledExecutorService scheduledExecutorService = Mock()

0 commit comments

Comments
 (0)