Skip to content

Commit 053b824

Browse files
bbakermanbbakerman
authored
Merge pull request graphql-java#3211 from graphql-java/deprecate_fields_container_in_code_registry
Deprecate FieldsContainer on code registry
2 parents 657c6a8 + 5e84296 commit 053b824

4 files changed

Lines changed: 78 additions & 6 deletions

File tree

‎src/main/java/graphql/schema/GraphQLCodeRegistry.java‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package graphql.schema;
22

33
import graphql.Assert;
4+
import graphql.DeprecatedAt;
45
import graphql.Internal;
56
import graphql.PublicApi;
67
import graphql.schema.visibility.GraphqlFieldVisibility;
@@ -56,11 +57,29 @@ public GraphqlFieldVisibility getFieldVisibility() {
5657
* @param fieldDefinition the field definition
5758
*
5859
* @return the DataFetcher associated with this field. All fields have data fetchers
60+
*
61+
* @see #getDataFetcher(GraphQLObjectType, GraphQLFieldDefinition)
62+
* @deprecated This is confusing because {@link GraphQLInterfaceType}s cant have data fetchers. At runtime only a {@link GraphQLObjectType}
63+
* can be used to fetch a field. This method allows the mapping to be made, but it is never useful if an interface is passed in.
5964
*/
65+
@Deprecated
66+
@DeprecatedAt("2023-05-13")
6067
public DataFetcher<?> getDataFetcher(GraphQLFieldsContainer parentType, GraphQLFieldDefinition fieldDefinition) {
6168
return getDataFetcherImpl(FieldCoordinates.coordinates(parentType, fieldDefinition), fieldDefinition, dataFetcherMap, systemDataFetcherMap, defaultDataFetcherFactory);
6269
}
6370

71+
/**
72+
* Returns a data fetcher associated with a field within an object type
73+
*
74+
* @param parentType the container type
75+
* @param fieldDefinition the field definition
76+
*
77+
* @return the DataFetcher associated with this field. All fields have data fetchers
78+
*/
79+
public DataFetcher<?> getDataFetcher(GraphQLObjectType parentType, GraphQLFieldDefinition fieldDefinition) {
80+
return getDataFetcherImpl(FieldCoordinates.coordinates(parentType, fieldDefinition), fieldDefinition, dataFetcherMap, systemDataFetcherMap, defaultDataFetcherFactory);
81+
}
82+
6483
/**
6584
* Returns a data fetcher associated with a field located at specified coordinates.
6685
*
@@ -242,11 +261,29 @@ private Builder markChanged(boolean condition) {
242261
* @param fieldDefinition the field definition
243262
*
244263
* @return the DataFetcher associated with this field. All fields have data fetchers
264+
*
265+
* @see #getDataFetcher(GraphQLObjectType, GraphQLFieldDefinition)
266+
* @deprecated This is confusing because {@link GraphQLInterfaceType}s cant have data fetchers. At runtime only a {@link GraphQLObjectType}
267+
* can be used to fetch a field. This method allows the mapping to be made, but it is never useful if an interface is passed in.
245268
*/
269+
@Deprecated
270+
@DeprecatedAt("2023-05-13")
246271
public DataFetcher<?> getDataFetcher(GraphQLFieldsContainer parentType, GraphQLFieldDefinition fieldDefinition) {
247272
return getDataFetcherImpl(FieldCoordinates.coordinates(parentType, fieldDefinition), fieldDefinition, dataFetcherMap, systemDataFetcherMap, defaultDataFetcherFactory);
248273
}
249274

275+
/**
276+
* Returns a data fetcher associated with a field within an object type
277+
*
278+
* @param parentType the container type
279+
* @param fieldDefinition the field definition
280+
*
281+
* @return the DataFetcher associated with this field. All fields have data fetchers
282+
*/
283+
public DataFetcher<?> getDataFetcher(GraphQLObjectType parentType, GraphQLFieldDefinition fieldDefinition) {
284+
return getDataFetcherImpl(FieldCoordinates.coordinates(parentType, fieldDefinition), fieldDefinition, dataFetcherMap, systemDataFetcherMap, defaultDataFetcherFactory);
285+
}
286+
250287
/**
251288
* Returns a data fetcher associated with a field located at specified coordinates.
252289
*
@@ -331,11 +368,31 @@ public Builder dataFetcher(FieldCoordinates coordinates, DataFetcher<?> dataFetc
331368
* @param dataFetcher the data fetcher code for that field
332369
*
333370
* @return this builder
371+
*
372+
* @see #dataFetcher(GraphQLObjectType, GraphQLFieldDefinition, DataFetcher)
373+
* @deprecated This is confusing because {@link GraphQLInterfaceType}s cant have data fetchers. At runtime only a {@link GraphQLObjectType}
374+
* can be used to fetch a field. This method allows the mapping to be made, but it is never useful if an interface is passed in.
334375
*/
376+
@Deprecated
377+
@DeprecatedAt("2023-05-13")
335378
public Builder dataFetcher(GraphQLFieldsContainer parentType, GraphQLFieldDefinition fieldDefinition, DataFetcher<?> dataFetcher) {
336379
return dataFetcher(FieldCoordinates.coordinates(parentType.getName(), fieldDefinition.getName()), dataFetcher);
337380
}
338381

382+
383+
/**
384+
* Sets the data fetcher for a specific field inside an object type
385+
*
386+
* @param parentType the object type
387+
* @param fieldDefinition the field definition
388+
* @param dataFetcher the data fetcher code for that field
389+
*
390+
* @return this builder
391+
*/
392+
public Builder dataFetcher(GraphQLObjectType parentType, GraphQLFieldDefinition fieldDefinition, DataFetcher<?> dataFetcher) {
393+
return dataFetcher(FieldCoordinates.coordinates(parentType.getName(), fieldDefinition.getName()), dataFetcher);
394+
}
395+
339396
/**
340397
* Called to place system data fetchers (eg Introspection fields) into the mix
341398
*

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ public GraphQLFieldDefinition getFieldDefinition() {
130130
public DataFetcher<?> getFieldDataFetcher() {
131131
assertNotNull(fieldDefinition, () -> "An output field must be in context to call this method");
132132
assertNotNull(fieldsContainer, () -> "An output field container must be in context to call this method");
133-
return codeRegistry.getDataFetcher(fieldsContainer, fieldDefinition);
133+
return codeRegistry.getDataFetcher(FieldCoordinates.coordinates(fieldsContainer, fieldDefinition), fieldDefinition);
134134
}
135135

136136
@Override

‎src/test/groovy/graphql/analysis/values/ValueTraverserTest.groovy‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import graphql.schema.GraphQLInputObjectType
1717
import graphql.schema.GraphQLInputSchemaElement
1818
import graphql.schema.GraphQLList
1919
import graphql.schema.GraphQLNamedSchemaElement
20+
import graphql.schema.GraphQLObjectType
2021
import graphql.schema.GraphQLScalarType
2122
import graphql.schema.idl.SchemaDirectiveWiring
2223
import graphql.schema.idl.SchemaDirectiveWiringEnvironment
@@ -861,8 +862,12 @@ type Profile {
861862
GraphQLFieldDefinition onField(SchemaDirectiveWiringEnvironment<GraphQLFieldDefinition> env) {
862863
GraphQLFieldsContainer fieldsContainer = env.getFieldsContainer()
863864
GraphQLFieldDefinition fieldDefinition = env.getFieldDefinition()
865+
if (! fieldsContainer instanceof GraphQLObjectType) {
866+
return fieldDefinition
867+
}
868+
GraphQLObjectType containingObjectType = env.getFieldsContainer() as GraphQLObjectType
864869

865-
final DataFetcher<?> originalDF = env.getCodeRegistry().getDataFetcher(fieldsContainer, fieldDefinition)
870+
final DataFetcher<?> originalDF = env.getCodeRegistry().getDataFetcher(containingObjectType, fieldDefinition)
866871
final DataFetcher<?> newDF = { DataFetchingEnvironment originalEnv ->
867872
ValueVisitor visitor = new ValueVisitor() {
868873
@Override
@@ -884,7 +889,7 @@ type Profile {
884889
return originalDF.get(newEnv);
885890
}
886891

887-
env.getCodeRegistry().dataFetcher(fieldsContainer, fieldDefinition, newDF)
892+
env.getCodeRegistry().dataFetcher(containingObjectType, fieldDefinition, newDF)
888893

889894
return fieldDefinition
890895
}

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -325,9 +325,14 @@ class SchemaGeneratorDirectiveHelperTest extends Specification {
325325
@Override
326326
GraphQLFieldDefinition onField(SchemaDirectiveWiringEnvironment<GraphQLFieldDefinition> directiveEnv) {
327327
GraphQLFieldDefinition field = directiveEnv.getElement()
328+
def container = directiveEnv.fieldsContainer
329+
if (!container instanceof GraphQLObjectType) {
330+
return field
331+
}
328332
//
329333
// we use the non shortcut path to the data fetcher here so prove it still works
330-
def fetcher = directiveEnv.getCodeRegistry().getDataFetcher(directiveEnv.fieldsContainer, field)
334+
335+
def fetcher = directiveEnv.getCodeRegistry().getDataFetcher(container as GraphQLObjectType, field)
331336
def newFetcher = wrapDataFetcher(fetcher, { dfEnv, value ->
332337
def directiveName = directiveEnv.appliedDirective.name
333338
if (directiveName == "uppercase") {
@@ -484,10 +489,15 @@ class SchemaGeneratorDirectiveHelperTest extends Specification {
484489
@Override
485490
GraphQLFieldDefinition onField(SchemaDirectiveWiringEnvironment<GraphQLFieldDefinition> environment) {
486491
GraphQLFieldDefinition element = environment.getElement()
487-
return wrapField(environment.fieldsContainer, element, environment.getBuildContext(), environment.getCodeRegistry())
492+
493+
def container = environment.fieldsContainer
494+
if (! container instanceof GraphQLObjectType) {
495+
return element
496+
}
497+
return wrapField(container as GraphQLObjectType, element, environment.getBuildContext(), environment.getCodeRegistry())
488498
}
489499

490-
private GraphQLFieldDefinition wrapField(GraphQLFieldsContainer parentType, GraphQLFieldDefinition field, Map<String, Object> contextMap, GraphQLCodeRegistry.Builder codeRegistry) {
500+
private GraphQLFieldDefinition wrapField(GraphQLObjectType parentType, GraphQLFieldDefinition field, Map<String, Object> contextMap, GraphQLCodeRegistry.Builder codeRegistry) {
491501
def originalFetcher = codeRegistry.getDataFetcher(parentType, field)
492502

493503
String key = mkFieldKey(parentType.getName(), field.getName())

0 commit comments

Comments
 (0)