Skip to content

Commit cd99d17

Browse files
dondonzdondonz
authored
Merge pull request #3824 from graphql-java/make-strict-runtime-wiring-default
Change runtime wiring redefinition checks to be strict by default
2 parents bac784b + 8d5436a commit cd99d17

File tree

5 files changed

+195
-54
lines changed

5 files changed

+195
-54
lines changed

‎src/main/java/graphql/schema/idl/RuntimeWiring.java

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ public static class Builder {
183183
private final Map<String, SchemaDirectiveWiring> registeredDirectiveWiring = new LinkedHashMap<>();
184184
private final List<SchemaDirectiveWiring> directiveWiring = new ArrayList<>();
185185
private WiringFactory wiringFactory = new NoopWiringFactory();
186-
private boolean strictMode = false;
186+
private boolean strictMode = true;
187187
private GraphqlFieldVisibility fieldVisibility = DEFAULT_FIELD_VISIBILITY;
188188
private GraphQLCodeRegistry codeRegistry = GraphQLCodeRegistry.newCodeRegistry().build();
189189
private GraphqlTypeComparatorRegistry comparatorRegistry = GraphqlTypeComparatorRegistry.AS_IS_REGISTRY;
@@ -192,11 +192,24 @@ private Builder() {
192192
ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS.forEach(this::scalar);
193193
}
194194

195+
/**
196+
* This sets strict mode as true or false. If strictMode is true, if things get defined twice, for example, it will throw a {@link StrictModeWiringException}.
197+
*
198+
* @return this builder
199+
*/
200+
public Builder strictMode(boolean strictMode) {
201+
this.strictMode = strictMode;
202+
return this;
203+
}
204+
195205
/**
196206
* This puts the builder into strict mode, so if things get defined twice, for example, it will throw a {@link StrictModeWiringException}.
197207
*
198208
* @return this builder
209+
*
210+
* @deprecated strictMode default value changed to true, use {@link #strictMode(boolean)} instead
199211
*/
212+
@Deprecated(since = "2025-03-22", forRemoval = true)
200213
public Builder strictMode() {
201214
this.strictMode = true;
202215
return this;
@@ -300,13 +313,23 @@ public Builder type(String typeName, UnaryOperator<TypeRuntimeWiring.Builder> bu
300313
public Builder type(TypeRuntimeWiring typeRuntimeWiring) {
301314
String typeName = typeRuntimeWiring.getTypeName();
302315
Map<String, DataFetcher> typeDataFetchers = dataFetchers.computeIfAbsent(typeName, k -> new LinkedHashMap<>());
316+
317+
Map<String, DataFetcher> additionalFieldDataFetchers = typeRuntimeWiring.getFieldDataFetchers();
303318
if (strictMode && !typeDataFetchers.isEmpty()) {
304-
throw new StrictModeWiringException(format("The type %s has already been defined", typeName));
319+
// Check if the existing type wiring contains overlapping DataFetcher definitions
320+
for (String fieldName : additionalFieldDataFetchers.keySet()) {
321+
if (typeDataFetchers.containsKey(fieldName)) {
322+
throw new StrictModeWiringException(format("The field %s on type %s has already been defined", fieldName, typeName));
323+
}
324+
}
305325
}
306-
typeDataFetchers.putAll(typeRuntimeWiring.getFieldDataFetchers());
326+
typeDataFetchers.putAll(additionalFieldDataFetchers);
307327

308328
DataFetcher<?> defaultDataFetcher = typeRuntimeWiring.getDefaultDataFetcher();
309329
if (defaultDataFetcher != null) {
330+
if (strictMode && defaultDataFetchers.containsKey(typeName)) {
331+
throw new StrictModeWiringException(format("The type %s already has a default data fetcher defined", typeName));
332+
}
310333
defaultDataFetchers.put(typeName, defaultDataFetcher);
311334
}
312335

‎src/main/java/graphql/schema/idl/TypeRuntimeWiring.java

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,20 @@
1616

1717
/**
1818
* A type runtime wiring is a specification of the data fetchers and possible type resolver for a given type name.
19-
*
19+
* <br>
2020
* This is used by {@link RuntimeWiring} to wire together a functional {@link GraphQLSchema}
2121
*/
2222
@PublicApi
2323
public class TypeRuntimeWiring {
2424

25-
private final static AtomicBoolean DEFAULT_STRICT_MODE = new AtomicBoolean(false);
25+
private final static AtomicBoolean DEFAULT_STRICT_MODE = new AtomicBoolean(true);
2626

2727
/**
28-
* By default {@link TypeRuntimeWiring} builders are not in strict mode, but you can set a JVM wide value
29-
* so that any created will be.
28+
* By default {@link TypeRuntimeWiring} builders are in strict mode, but you can set a JVM wide value too
3029
*
3130
* @param strictMode the desired strict mode state
3231
*
33-
* @see Builder#strictMode()
32+
* @see Builder#strictMode(boolean)
3433
*/
3534
public static void setStrictModeJvmWide(boolean strictMode) {
3635
DEFAULT_STRICT_MODE.set(strictMode);
@@ -127,6 +126,20 @@ public Builder typeName(String typeName) {
127126
*
128127
* @return this builder
129128
*/
129+
public Builder strictMode(boolean strictMode) {
130+
this.strictMode = strictMode;
131+
return this;
132+
}
133+
134+
/**
135+
* This puts the builder into strict mode, so if things get defined twice, for example, it
136+
* will throw a {@link StrictModeWiringException}.
137+
*
138+
* @return this builder
139+
*
140+
* @deprecated use {@link #strictMode(boolean)} instead
141+
*/
142+
@Deprecated(since = "2025-03-22", forRemoval = true)
130143
public Builder strictMode() {
131144
this.strictMode = true;
132145
return this;

‎src/main/java/graphql/schema/idl/errors/StrictModeWiringException.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import graphql.schema.idl.TypeRuntimeWiring;
77

88
/**
9-
* An exception that is throw when {@link RuntimeWiring.Builder#strictMode()} or {@link TypeRuntimeWiring.Builder#strictMode()} is true and
9+
* An exception that is throw when {@link RuntimeWiring.Builder#strictMode(boolean)} or {@link TypeRuntimeWiring.Builder#strictMode(boolean)} is true and
1010
* something gets redefined.
1111
*/
1212
@PublicApi

‎src/test/groovy/graphql/schema/idl/RuntimeWiringTest.groovy

Lines changed: 129 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -193,26 +193,23 @@ class RuntimeWiringTest extends Specification {
193193
newWiring.fieldVisibility == fieldVisibility
194194
}
195195

196-
def "strict mode can stop certain redefinitions"() {
196+
def "strict mode, on by default, can stop certain redefinitions"() {
197197
DataFetcher DF1 = env -> "x"
198198
DataFetcher DF2 = env -> "x"
199199
TypeResolver TR1 = env -> null
200200
EnumValuesProvider EVP1 = name -> null
201201

202202
when:
203203
RuntimeWiring.newRuntimeWiring()
204-
.strictMode()
205204
.type(TypeRuntimeWiring.newTypeWiring("Foo").dataFetcher("foo", DF1))
206-
.type(TypeRuntimeWiring.newTypeWiring("Foo").dataFetcher("bar", DF1))
207-
205+
.type(TypeRuntimeWiring.newTypeWiring("Foo").dataFetcher("foo", DF1)) // Cannot redefine the same field's datafetcher
208206

209207
then:
210208
def e1 = thrown(StrictModeWiringException)
211-
e1.message == "The type Foo has already been defined"
209+
e1.message == "The field foo on type Foo has already been defined"
212210

213211
when:
214212
RuntimeWiring.newRuntimeWiring()
215-
.strictMode()
216213
.type(TypeRuntimeWiring.newTypeWiring("Foo").typeResolver(TR1))
217214
.type(TypeRuntimeWiring.newTypeWiring("Foo").typeResolver(TR1))
218215

@@ -222,7 +219,6 @@ class RuntimeWiringTest extends Specification {
222219

223220
when:
224221
RuntimeWiring.newRuntimeWiring()
225-
.strictMode()
226222
.type(TypeRuntimeWiring.newTypeWiring("Foo").enumValues(EVP1))
227223
.type(TypeRuntimeWiring.newTypeWiring("Foo").enumValues(EVP1))
228224
then:
@@ -231,15 +227,13 @@ class RuntimeWiringTest extends Specification {
231227

232228
when:
233229
RuntimeWiring.newRuntimeWiring()
234-
.strictMode()
235230
.scalar(Scalars.GraphQLString)
236231
then:
237232
def e4 = thrown(StrictModeWiringException)
238233
e4.message == "The scalar String is already defined"
239234

240235
when:
241236
TypeRuntimeWiring.newTypeWiring("Foo")
242-
.strictMode()
243237
.defaultDataFetcher(DF1)
244238
.defaultDataFetcher(DF2)
245239

@@ -248,34 +242,148 @@ class RuntimeWiringTest extends Specification {
248242
e5.message == "The type Foo has already has a default data fetcher defined"
249243
}
250244

251-
def "overwrite default data fetchers if they are null"() {
252-
245+
def "strict mode, on by default, permits a type to be defined more than once as long as elements are not overlapping"() {
253246
DataFetcher DF1 = env -> "x"
254247
DataFetcher DF2 = env -> "x"
255-
DataFetcher DEFAULT_DF = env -> null
256-
DataFetcher DEFAULT_DF2 = env -> null
248+
TypeResolver TR1 = env -> null
249+
EnumValuesProvider EVP1 = name -> null
257250

258251
when:
259-
def runtimeWiring = RuntimeWiring.newRuntimeWiring()
260-
.type(TypeRuntimeWiring.newTypeWiring("Foo").defaultDataFetcher(DEFAULT_DF))
252+
// Permit type wiring to be defined more than once, if child DataFetchers are for distinct fields
253+
def runtimeWiring1 = RuntimeWiring.newRuntimeWiring()
261254
.type(TypeRuntimeWiring.newTypeWiring("Foo").dataFetcher("foo", DF1))
262255
.type(TypeRuntimeWiring.newTypeWiring("Foo").dataFetcher("bar", DF2))
263256
.build()
264257

265258
then:
266-
runtimeWiring.getDefaultDataFetcherForType("Foo") == DEFAULT_DF
259+
noExceptionThrown()
260+
runtimeWiring1.getDataFetchers().get("Foo").get("foo") == DF1
261+
runtimeWiring1.getDataFetchers().get("Foo").get("bar") == DF2
262+
263+
when:
264+
// Only one type wiring is allowed per type, do not allow redefinition
265+
RuntimeWiring.newRuntimeWiring()
266+
.type(TypeRuntimeWiring.newTypeWiring("Foo").typeResolver(TR1))
267+
.type(TypeRuntimeWiring.newTypeWiring("Foo").typeResolver(TR1))
268+
269+
then:
270+
def e2 = thrown(StrictModeWiringException)
271+
e2.message == "The type Foo already has a type resolver defined"
272+
273+
when:
274+
// Only one enum values provider is allowed per type, do not allow redefinition
275+
RuntimeWiring.newRuntimeWiring()
276+
.type(TypeRuntimeWiring.newTypeWiring("Foo").enumValues(EVP1))
277+
.type(TypeRuntimeWiring.newTypeWiring("Foo").enumValues(EVP1))
278+
279+
then:
280+
def e3 = thrown(StrictModeWiringException)
281+
e3.message == "The type Foo already has a enum provider defined"
282+
283+
when:
284+
// Only one scalar wiring is allowed per scalar
285+
RuntimeWiring.newRuntimeWiring()
286+
.scalar(Scalars.GraphQLString)
287+
then:
288+
def e4 = thrown(StrictModeWiringException)
289+
e4.message == "The scalar String is already defined"
290+
291+
when:
292+
// Only one default data fetcher is allowed, do not allow redefinition
293+
TypeRuntimeWiring.newTypeWiring("Foo")
294+
.defaultDataFetcher(DF1)
295+
.defaultDataFetcher(DF2)
296+
297+
then:
298+
def e5 = thrown(StrictModeWiringException)
299+
e5.message == "The type Foo has already has a default data fetcher defined"
300+
}
301+
302+
def "strict mode, if set to off, won't stop certain redefinitions"() {
303+
DataFetcher DF1 = env -> "x"
304+
DataFetcher DF2 = env -> "x"
305+
TypeResolver TR1 = env -> null
306+
EnumValuesProvider EVP1 = name -> null
267307

268308
when:
269-
runtimeWiring = RuntimeWiring.newRuntimeWiring()
309+
def runtimeWiring1 = RuntimeWiring.newRuntimeWiring()
310+
.strictMode(false)
311+
.type(TypeRuntimeWiring.newTypeWiring("Foo").dataFetcher("foo", DF1))
312+
.type(TypeRuntimeWiring.newTypeWiring("Foo").dataFetcher("foo", DF2))
313+
.build()
314+
315+
then:
316+
noExceptionThrown()
317+
runtimeWiring1.getDataFetchers().get("Foo").get("foo") == DF2
318+
319+
when:
320+
def runtimeWiring2 = RuntimeWiring.newRuntimeWiring()
321+
.strictMode(false)
322+
.type(TypeRuntimeWiring.newTypeWiring("Foo").typeResolver(TR1))
323+
.type(TypeRuntimeWiring.newTypeWiring("Foo").typeResolver(TR1))
324+
.build()
325+
326+
then:
327+
noExceptionThrown()
328+
runtimeWiring2.typeResolvers.get("Foo") == TR1
329+
330+
when:
331+
def runtimeWiring3 = RuntimeWiring.newRuntimeWiring()
332+
.strictMode(false)
333+
.type(TypeRuntimeWiring.newTypeWiring("Foo").enumValues(EVP1))
334+
.type(TypeRuntimeWiring.newTypeWiring("Foo").enumValues(EVP1))
335+
.build()
336+
337+
then:
338+
noExceptionThrown()
339+
runtimeWiring3.getEnumValuesProviders().get("Foo") == EVP1
340+
341+
when:
342+
def runtimeWiring4 = RuntimeWiring.newRuntimeWiring()
343+
.strictMode(false)
344+
.scalar(Scalars.GraphQLString)
345+
.build()
346+
347+
then:
348+
noExceptionThrown()
349+
runtimeWiring4.scalars.get("String") == Scalars.GraphQLString
350+
351+
when:
352+
def typeRuntimeWiring = TypeRuntimeWiring.newTypeWiring("Foo")
353+
.strictMode(false)
354+
.defaultDataFetcher(DF1)
355+
.defaultDataFetcher(DF2)
356+
.build()
357+
358+
then:
359+
noExceptionThrown()
360+
typeRuntimeWiring.defaultDataFetcher == DF2
361+
}
362+
363+
def "when strict mode on, do not allow default data fetcher redefinition"() {
364+
DataFetcher DF1 = env -> "w"
365+
DataFetcher DEFAULT_DF = env -> "x"
366+
DataFetcher DEFAULT_DF2 = env -> "y"
367+
368+
// Having a datafetcher and a default for the type is ok
369+
when:
370+
def runtimeWiring1 = RuntimeWiring.newRuntimeWiring()
270371
.type(TypeRuntimeWiring.newTypeWiring("Foo").defaultDataFetcher(DEFAULT_DF))
271372
.type(TypeRuntimeWiring.newTypeWiring("Foo").dataFetcher("foo", DF1))
272-
.type(TypeRuntimeWiring.newTypeWiring("Foo").dataFetcher("bar", DF2))
273-
// we can specifically overwrite it later
274-
.type(TypeRuntimeWiring.newTypeWiring("Foo").defaultDataFetcher(DEFAULT_DF2))
275373
.build()
276374

277375
then:
278-
runtimeWiring.getDefaultDataFetcherForType("Foo") == DEFAULT_DF2
376+
runtimeWiring1.getDefaultDataFetcherForType("Foo") == DEFAULT_DF
279377

378+
// Do not permit redefinition of the default datafetcher
379+
when:
380+
RuntimeWiring.newRuntimeWiring()
381+
.type(TypeRuntimeWiring.newTypeWiring("Foo").defaultDataFetcher(DEFAULT_DF))
382+
.type(TypeRuntimeWiring.newTypeWiring("Foo").defaultDataFetcher(DEFAULT_DF2))
383+
.build()
384+
385+
then:
386+
def error = thrown(StrictModeWiringException)
387+
error.message == "The type Foo already has a default data fetcher defined"
280388
}
281389
}

0 commit comments

Comments
 (0)