Skip to content

Commit 34693e6

Browse files
committed
Perform hasDataLoaderRegistrations check at runtime
Closes gh-1020
1 parent 06675e0 commit 34693e6

File tree

4 files changed

+54
-29
lines changed

4 files changed

+54
-29
lines changed

spring-graphql/src/main/java/org/springframework/graphql/execution/DataLoaderRegistrar.java

+12-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -30,6 +30,17 @@
3030
*/
3131
public interface DataLoaderRegistrar {
3232

33+
34+
/**
35+
* Whether the registrar has any {@code DataLoader} registrations to make.
36+
* @since 1.2.8
37+
*/
38+
default boolean hasRegistrations() {
39+
DataLoaderRegistry registry = DataLoaderRegistry.newRegistry().build();
40+
registerDataLoaders(registry, GraphQLContext.newContext().build());
41+
return !registry.getDataLoaders().isEmpty();
42+
}
43+
3344
/**
3445
* Callback that provides access to the {@link DataLoaderRegistry} from the
3546
* the {@link graphql.ExecutionInput}.

spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultBatchLoaderRegistry.java

+5
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,11 @@ public <K, V> RegistrationSpec<K, V> forName(String name) {
9090
return new DefaultRegistrationSpec<>(name);
9191
}
9292

93+
@Override
94+
public boolean hasRegistrations() {
95+
return (!this.loaders.isEmpty() || !this.mappedLoaders.isEmpty());
96+
}
97+
9398
@Override
9499
public void registerDataLoaders(DataLoaderRegistry registry, GraphQLContext context) {
95100
BatchLoaderContextProvider contextProvider = () -> context;

spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultExecutionGraphQlService.java

+28-20
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.springframework.graphql.ExecutionGraphQlResponse;
3535
import org.springframework.graphql.ExecutionGraphQlService;
3636
import org.springframework.graphql.support.DefaultExecutionGraphQlResponse;
37+
import org.springframework.lang.Nullable;
3738
import org.springframework.util.ClassUtils;
3839
import org.springframework.util.ReflectionUtils;
3940

@@ -57,7 +58,8 @@ public class DefaultExecutionGraphQlService implements ExecutionGraphQlService {
5758

5859
private final List<DataLoaderRegistrar> dataLoaderRegistrars = new ArrayList<>();
5960

60-
private boolean hasDataLoaderRegistrations;
61+
@Nullable
62+
private Boolean hasDataLoaderRegistrations;
6163

6264
private final boolean isDefaultExecutionIdProvider;
6365

@@ -80,13 +82,6 @@ public DefaultExecutionGraphQlService(GraphQlSource graphQlSource) {
8082
*/
8183
public void addDataLoaderRegistrar(DataLoaderRegistrar registrar) {
8284
this.dataLoaderRegistrars.add(registrar);
83-
this.hasDataLoaderRegistrations = (this.hasDataLoaderRegistrations || hasRegistrations(registrar));
84-
}
85-
86-
private static boolean hasRegistrations(DataLoaderRegistrar registrar) {
87-
DataLoaderRegistry registry = DataLoaderRegistry.newRegistry().build();
88-
registrar.registerDataLoaders(registry, GraphQLContext.newContext().build());
89-
return !registry.getDataLoaders().isEmpty();
9085
}
9186

9287

@@ -104,28 +99,41 @@ public final Mono<ExecutionGraphQlResponse> execute(ExecutionGraphQlRequest requ
10499
ContextSnapshotFactoryHelper.saveInstance(factory, graphQLContext);
105100
factory.captureFrom(contextView).updateContext(graphQLContext);
106101

107-
ExecutionInput updatedExecutionInput =
108-
(this.hasDataLoaderRegistrations ? registerDataLoaders(executionInput) : executionInput);
102+
ExecutionInput executionInputToUse = registerDataLoaders(executionInput);
109103

110-
return Mono.fromFuture(this.graphQlSource.graphQl().executeAsync(updatedExecutionInput))
111-
.map((result) -> new DefaultExecutionGraphQlResponse(updatedExecutionInput, result));
104+
return Mono.fromFuture(this.graphQlSource.graphQl().executeAsync(executionInputToUse))
105+
.map((result) -> new DefaultExecutionGraphQlResponse(executionInputToUse, result));
112106
});
113107
}
114108

115109
private ExecutionInput registerDataLoaders(ExecutionInput executionInput) {
116-
GraphQLContext graphQLContext = executionInput.getGraphQLContext();
117-
DataLoaderRegistry existingRegistry = executionInput.getDataLoaderRegistry();
118-
if (existingRegistry == this.emptyDataLoaderRegistryInstance) {
119-
DataLoaderRegistry newRegistry = DataLoaderRegistry.newRegistry().build();
120-
applyDataLoaderRegistrars(newRegistry, graphQLContext);
121-
executionInput = executionInput.transform((builder) -> builder.dataLoaderRegistry(newRegistry));
110+
if (this.hasDataLoaderRegistrations == null) {
111+
this.hasDataLoaderRegistrations = initHasDataLoaderRegistrations();
122112
}
123-
else {
124-
applyDataLoaderRegistrars(existingRegistry, graphQLContext);
113+
if (this.hasDataLoaderRegistrations) {
114+
GraphQLContext graphQLContext = executionInput.getGraphQLContext();
115+
DataLoaderRegistry existingRegistry = executionInput.getDataLoaderRegistry();
116+
if (existingRegistry == this.emptyDataLoaderRegistryInstance) {
117+
DataLoaderRegistry newRegistry = DataLoaderRegistry.newRegistry().build();
118+
applyDataLoaderRegistrars(newRegistry, graphQLContext);
119+
executionInput = executionInput.transform((builder) -> builder.dataLoaderRegistry(newRegistry));
120+
}
121+
else {
122+
applyDataLoaderRegistrars(existingRegistry, graphQLContext);
123+
}
125124
}
126125
return executionInput;
127126
}
128127

128+
private boolean initHasDataLoaderRegistrations() {
129+
for (DataLoaderRegistrar registrar : this.dataLoaderRegistrars) {
130+
if (registrar.hasRegistrations()) {
131+
return true;
132+
}
133+
}
134+
return false;
135+
}
136+
129137
private void applyDataLoaderRegistrars(DataLoaderRegistry registry, GraphQLContext graphQLContext) {
130138
this.dataLoaderRegistrars.forEach((registrar) -> registrar.registerDataLoaders(registry, graphQLContext));
131139
}

spring-graphql/src/test/java/org/springframework/graphql/execution/DefaultExecutionGraphQlServiceTests.java

+9-8
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -41,26 +41,27 @@ public class DefaultExecutionGraphQlServiceTests {
4141

4242
@Test
4343
void customDataLoaderRegistry() {
44-
DefaultBatchLoaderRegistry batchLoaderRegistry = new DefaultBatchLoaderRegistry();
45-
batchLoaderRegistry.forTypePair(Book.class, Author.class)
46-
.registerBatchLoader((books, batchLoaderEnvironment) -> Flux.empty());
47-
4844
GraphQlSource graphQlSource = GraphQlSetup.schemaContent("type Query { greeting: String }")
4945
.queryFetcher("greeting", (env) -> "hi")
5046
.toGraphQlSource();
5147

48+
BatchLoaderRegistry batchLoaderRegistry = new DefaultBatchLoaderRegistry();
5249
DefaultExecutionGraphQlService graphQlService = new DefaultExecutionGraphQlService(graphQlSource);
5350
graphQlService.addDataLoaderRegistrar(batchLoaderRegistry);
5451

55-
DataLoaderRegistry myRegistry = new DataLoaderRegistry();
52+
// gh-1020: register loader after adding the registry to DefaultExecutionGraphQlService
53+
batchLoaderRegistry.forTypePair(Book.class, Author.class)
54+
.registerBatchLoader((books, batchLoaderEnvironment) -> Flux.empty());
55+
56+
DataLoaderRegistry dataLoaderRegistry = new DataLoaderRegistry();
5657

5758
ExecutionGraphQlRequest request = TestExecutionRequest.forDocument("{ greeting }");
58-
request.configureExecutionInput((input, builder) -> builder.dataLoaderRegistry(myRegistry).build());
59+
request.configureExecutionInput((input, builder) -> builder.dataLoaderRegistry(dataLoaderRegistry).build());
5960

6061
ExecutionGraphQlResponse response = graphQlService.execute(request).block();
6162
Map<?, ?> data = response.getExecutionResult().getData();
6263
assertThat(data).isEqualTo(Map.of("greeting", "hi"));
63-
assertThat(myRegistry.getDataLoaders()).hasSize(1);
64+
assertThat(dataLoaderRegistry.getDataLoaders()).hasSize(1);
6465
}
6566

6667
}

0 commit comments

Comments
 (0)