Skip to content

Commit 19d128c

Browse files
bbakermanbbakerman
authored
Merge pull request #3754 from graphql-java/singeton_property_data_fetcher
Singleton property data fetcher
2 parents 1098c53 + b13b7c8 commit 19d128c

17 files changed

Lines changed: 476 additions & 115 deletions

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,18 @@ public class DataFetcherFactories {
2020
* @return a data fetcher factory that always returns the provided data fetcher
2121
*/
2222
public static <T> DataFetcherFactory<T> useDataFetcher(DataFetcher<T> dataFetcher) {
23-
return fieldDefinition -> dataFetcher;
23+
//noinspection deprecation
24+
return new DataFetcherFactory<>() {
25+
@Override
26+
public DataFetcher<T> get(DataFetcherFactoryEnvironment environment) {
27+
return dataFetcher;
28+
}
29+
30+
@Override
31+
public DataFetcher<T> get(GraphQLFieldDefinition fieldDefinition) {
32+
return dataFetcher;
33+
}
34+
};
2435
}
2536

2637
/**
@@ -32,7 +43,7 @@ public static <T> DataFetcherFactory<T> useDataFetcher(DataFetcher<T> dataFetche
3243
*
3344
* @return a new data fetcher that wraps the provided data fetcher
3445
*/
35-
public static DataFetcher wrapDataFetcher(DataFetcher delegateDataFetcher, BiFunction<DataFetchingEnvironment, Object, Object> mapFunction) {
46+
public static DataFetcher<?> wrapDataFetcher(DataFetcher<?> delegateDataFetcher, BiFunction<DataFetchingEnvironment, Object, Object> mapFunction) {
3647
return environment -> {
3748
Object value = delegateDataFetcher.get(environment);
3849
if (value instanceof CompletionStage) {

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,23 @@ public interface DataFetcherFactory<T> {
1919
* @param environment the environment that needs the data fetcher
2020
*
2121
* @return a data fetcher
22+
*
23+
* @deprecated This method will go away at some point and {@link DataFetcherFactory#get(GraphQLFieldDefinition)} will be used
2224
*/
25+
@Deprecated(since = "2024-11-26")
2326
DataFetcher<T> get(DataFetcherFactoryEnvironment environment);
2427

28+
/**
29+
* Returns a {@link graphql.schema.DataFetcher} given the field definition
30+
* which is cheaper in object allocation terms.
31+
*
32+
* @param fieldDefinition the field that needs the data fetcher
33+
*
34+
* @return a data fetcher
35+
*/
36+
37+
default DataFetcher<T> get(GraphQLFieldDefinition fieldDefinition) {
38+
return null;
39+
}
40+
2541
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,12 @@
55
/**
66
* This is passed to a {@link graphql.schema.DataFetcherFactory} when it is invoked to
77
* get a {@link graphql.schema.DataFetcher}
8+
*
9+
* @deprecated This class will go away at some point in the future since its pointless wrapper
10+
* of a {@link GraphQLFieldDefinition}
811
*/
912
@PublicApi
13+
@Deprecated(since = "2024-11-26")
1014
public class DataFetcherFactoryEnvironment {
1115
private final GraphQLFieldDefinition fieldDefinition;
1216

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ public boolean hasDataFetcher(FieldCoordinates coordinates) {
8484
return hasDataFetcherImpl(coordinates, dataFetcherMap, systemDataFetcherMap);
8585
}
8686

87+
@SuppressWarnings("deprecation")
8788
private static DataFetcher<?> getDataFetcherImpl(FieldCoordinates coordinates, GraphQLFieldDefinition fieldDefinition, Map<FieldCoordinates, DataFetcherFactory<?>> dataFetcherMap, Map<String, DataFetcherFactory<?>> systemDataFetcherMap, DataFetcherFactory<?> defaultDataFetcherFactory) {
8889
assertNotNull(coordinates);
8990
assertNotNull(fieldDefinition);
@@ -95,9 +96,15 @@ private static DataFetcher<?> getDataFetcherImpl(FieldCoordinates coordinates, G
9596
dataFetcherFactory = defaultDataFetcherFactory;
9697
}
9798
}
98-
return dataFetcherFactory.get(newDataFetchingFactoryEnvironment()
99-
.fieldDefinition(fieldDefinition)
100-
.build());
99+
// call direct from the field - cheaper to not make a new environment object
100+
DataFetcher<?> dataFetcher = dataFetcherFactory.get(fieldDefinition);
101+
if (dataFetcher == null) {
102+
DataFetcherFactoryEnvironment factoryEnvironment = newDataFetchingFactoryEnvironment()
103+
.fieldDefinition(fieldDefinition)
104+
.build();
105+
dataFetcher = dataFetcherFactory.get(factoryEnvironment);
106+
}
107+
return dataFetcher;
101108
}
102109

103110
private static boolean hasDataFetcherImpl(FieldCoordinates coords, Map<FieldCoordinates, DataFetcherFactory<?>> dataFetcherMap, Map<String, DataFetcherFactory<?>> systemDataFetcherMap) {
@@ -149,7 +156,7 @@ private static TypeResolver getTypeResolverForUnion(GraphQLUnionType parentType,
149156
if (typeResolver == null) {
150157
typeResolver = parentType.getTypeResolver();
151158
}
152-
return assertNotNull(typeResolver, "There must be a type resolver for union %s",parentType.getName());
159+
return assertNotNull(typeResolver, "There must be a type resolver for union %s", parentType.getName());
153160
}
154161

155162
/**
@@ -189,7 +196,7 @@ public static class Builder {
189196
private final Map<String, DataFetcherFactory<?>> systemDataFetcherMap = new LinkedHashMap<>();
190197
private final Map<String, TypeResolver> typeResolverMap = new HashMap<>();
191198
private GraphqlFieldVisibility fieldVisibility = DEFAULT_FIELD_VISIBILITY;
192-
private DataFetcherFactory<?> defaultDataFetcherFactory = env -> PropertyDataFetcher.fetching(env.getFieldDefinition().getName());
199+
private DataFetcherFactory<?> defaultDataFetcherFactory = SingletonPropertyDataFetcher.singletonFactory();
193200
private boolean changed = false;
194201

195202
private Builder() {

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package graphql.schema;
2+
3+
import java.util.function.Supplier;
4+
5+
/**
6+
* The {@link SingletonPropertyDataFetcher} is much like the {@link PropertyDataFetcher} except
7+
* that it is designed to only ever fetch properties via the name of the field passed in.
8+
* <p>
9+
* This uses the same code as {@link PropertyDataFetcher} and hence is also controlled
10+
* by static methods such as {@link PropertyDataFetcher#setUseNegativeCache(boolean)}
11+
*
12+
* @param <T> for two
13+
*/
14+
public class SingletonPropertyDataFetcher<T> implements LightDataFetcher<T> {
15+
16+
private static final SingletonPropertyDataFetcher<Object> SINGLETON_FETCHER = new SingletonPropertyDataFetcher<>();
17+
18+
private static final DataFetcherFactory<?> SINGLETON_FETCHER_FACTORY = environment -> SINGLETON_FETCHER;
19+
20+
/**
21+
* This returns the same singleton {@link LightDataFetcher} that fetches property values
22+
* based on the name of the field that iis passed into it.
23+
*
24+
* @return a singleton property data fetcher
25+
*/
26+
public static LightDataFetcher<?> singleton() {
27+
return SINGLETON_FETCHER;
28+
}
29+
30+
/**
31+
* This returns the same singleton {@link DataFetcherFactory} that returns the value of {@link #singleton()}
32+
*
33+
* @return a singleton data fetcher factory
34+
*/
35+
public static DataFetcherFactory<?> singletonFactory() {
36+
return SINGLETON_FETCHER_FACTORY;
37+
}
38+
39+
private SingletonPropertyDataFetcher() {
40+
}
41+
42+
@Override
43+
public T get(GraphQLFieldDefinition fieldDefinition, Object sourceObject, Supplier<DataFetchingEnvironment> environmentSupplier) throws Exception {
44+
return fetchImpl(fieldDefinition, sourceObject, environmentSupplier);
45+
}
46+
47+
@Override
48+
public T get(DataFetchingEnvironment environment) throws Exception {
49+
return fetchImpl(environment.getFieldDefinition(), environment.getSource(), () -> environment);
50+
}
51+
52+
private T fetchImpl(GraphQLFieldDefinition fieldDefinition, Object source, Supplier<DataFetchingEnvironment> environmentSupplier) {
53+
if (source == null) {
54+
return null;
55+
}
56+
// this is the same code that PropertyDataFetcher uses and hence unit tests for it include this one
57+
//noinspection unchecked
58+
return (T) PropertyDataFetcherHelper.getPropertyValue(fieldDefinition.getName(), source, fieldDefinition.getType(), environmentSupplier);
59+
}
60+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import graphql.schema.DataFetcher;
99
import graphql.schema.GraphQLScalarType;
1010
import graphql.schema.PropertyDataFetcher;
11+
import graphql.schema.SingletonPropertyDataFetcher;
1112
import graphql.schema.TypeResolver;
1213

1314
@PublicApi
@@ -44,7 +45,7 @@ public boolean providesDataFetcher(FieldWiringEnvironment environment) {
4445

4546
@Override
4647
public DataFetcher getDataFetcher(FieldWiringEnvironment environment) {
47-
return new PropertyDataFetcher(environment.getFieldDefinition().getName());
48+
return SingletonPropertyDataFetcher.singleton();
4849
}
4950

5051
@Override

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

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import graphql.schema.GraphQLUnionType;
5858
import graphql.schema.GraphqlTypeComparatorRegistry;
5959
import graphql.schema.PropertyDataFetcher;
60+
import graphql.schema.SingletonPropertyDataFetcher;
6061
import graphql.schema.TypeResolver;
6162
import graphql.schema.TypeResolverProxy;
6263
import graphql.schema.idl.errors.NotAnInputTypeError;
@@ -801,23 +802,27 @@ GraphQLFieldDefinition buildField(BuildContext buildCtx, TypeDefinition<?> paren
801802
// if they have already wired in a fetcher - then leave it alone
802803
FieldCoordinates coordinates = FieldCoordinates.coordinates(parentType.getName(), fieldDefinition.getName());
803804
if (!buildCtx.getCodeRegistry().hasDataFetcher(coordinates)) {
804-
DataFetcherFactory<?> dataFetcherFactory = buildDataFetcherFactory(buildCtx,
805+
Optional<DataFetcherFactory<?>> dataFetcherFactory = buildDataFetcherFactory(buildCtx,
805806
parentType,
806807
fieldDef,
807808
fieldType,
808809
appliedDirectives.first,
809810
appliedDirectives.second);
810-
buildCtx.getCodeRegistry().dataFetcher(coordinates, dataFetcherFactory);
811+
812+
// if the dataFetcherFactory is empty, then it must have been the code registry default one
813+
// and hence we don't need to make a "map entry" in the code registry since it will be defaulted
814+
// anyway
815+
dataFetcherFactory.ifPresent(fetcherFactory -> buildCtx.getCodeRegistry().dataFetcher(coordinates, fetcherFactory));
811816
}
812817
return directivesObserve(buildCtx, fieldDefinition);
813818
}
814819

815-
private DataFetcherFactory<?> buildDataFetcherFactory(BuildContext buildCtx,
816-
TypeDefinition<?> parentType,
817-
FieldDefinition fieldDef,
818-
GraphQLOutputType fieldType,
819-
List<GraphQLDirective> directives,
820-
List<GraphQLAppliedDirective> appliedDirectives) {
820+
private Optional<DataFetcherFactory<?>> buildDataFetcherFactory(BuildContext buildCtx,
821+
TypeDefinition<?> parentType,
822+
FieldDefinition fieldDef,
823+
GraphQLOutputType fieldType,
824+
List<GraphQLDirective> directives,
825+
List<GraphQLAppliedDirective> appliedDirectives) {
821826
String fieldName = fieldDef.getName();
822827
String parentTypeName = parentType.getName();
823828
TypeDefinitionRegistry typeRegistry = buildCtx.getTypeRegistry();
@@ -847,16 +852,18 @@ private DataFetcherFactory<?> buildDataFetcherFactory(BuildContext buildCtx,
847852
if (dataFetcher == null) {
848853
DataFetcherFactory<?> codeRegistryDFF = codeRegistry.getDefaultDataFetcherFactory();
849854
if (codeRegistryDFF != null) {
850-
return codeRegistryDFF;
855+
// this will use the default of the code registry when its
856+
// asked for at runtime
857+
return Optional.empty();
851858
}
852-
dataFetcher = dataFetcherOfLastResort(wiringEnvironment);
859+
dataFetcher = dataFetcherOfLastResort();
853860
}
854861
}
855862
}
856863
}
857864
dataFetcherFactory = DataFetcherFactories.useDataFetcher(dataFetcher);
858865
}
859-
return dataFetcherFactory;
866+
return Optional.of(dataFetcherFactory);
860867
}
861868

862869
GraphQLArgument buildArgument(BuildContext buildCtx, InputValueDefinition valueDefinition) {
@@ -1087,9 +1094,8 @@ private Optional<OperationTypeDefinition> getOperationNamed(String name, Map<Str
10871094
return Optional.ofNullable(operationTypeDefs.get(name));
10881095
}
10891096

1090-
private DataFetcher<?> dataFetcherOfLastResort(FieldWiringEnvironment environment) {
1091-
String fieldName = environment.getFieldDefinition().getName();
1092-
return new PropertyDataFetcher(fieldName);
1097+
private DataFetcher<?> dataFetcherOfLastResort() {
1098+
return SingletonPropertyDataFetcher.singleton();
10931099
}
10941100

10951101
private List<Directive> directivesOf(List<? extends TypeDefinition<?>> typeDefinitions) {

‎src/test/groovy/graphql/DataFetcherTest.groovy‎

Lines changed: 64 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package graphql
22

33

4+
import graphql.schema.GraphQLFieldDefinition
45
import graphql.schema.GraphQLOutputType
56
import graphql.schema.PropertyDataFetcher
7+
import graphql.schema.SingletonPropertyDataFetcher
68
import spock.lang.Specification
79

810
import static graphql.Scalars.GraphQLBoolean
@@ -55,43 +57,95 @@ class DataFetcherTest extends Specification {
5557

5658
}
5759

58-
def env(GraphQLOutputType type) {
59-
newDataFetchingEnvironment().source(dataHolder).fieldType(type).build()
60+
def env(String propertyName, GraphQLOutputType type) {
61+
GraphQLFieldDefinition fieldDefinition = mkField(propertyName, type)
62+
newDataFetchingEnvironment().source(dataHolder).fieldType(type).fieldDefinition(fieldDefinition).build()
63+
}
64+
65+
def mkField(String propertyName, GraphQLOutputType type) {
66+
GraphQLFieldDefinition.newFieldDefinition().name(propertyName).type(type).build()
6067
}
6168

6269
def "get property value"() {
6370
given:
64-
def environment = env(GraphQLString)
71+
def environment = env("property", GraphQLString)
72+
def field = mkField("property", GraphQLString)
6573
when:
66-
def result = new PropertyDataFetcher("property").get(environment)
74+
def result = fetcher.get(environment)
6775
then:
6876
result == "propertyValue"
77+
78+
when:
79+
result = fetcher.get(field, dataHolder, { environment })
80+
then:
81+
result == "propertyValue"
82+
83+
where:
84+
fetcher | _
85+
new PropertyDataFetcher("property") | _
86+
SingletonPropertyDataFetcher.singleton() | _
6987
}
7088

7189
def "get Boolean property value"() {
7290
given:
73-
def environment = env(GraphQLBoolean)
91+
def environment = env("booleanField", GraphQLBoolean)
92+
def field = mkField("booleanField", GraphQLBoolean)
93+
94+
when:
95+
def result = fetcher.get(environment)
96+
then:
97+
result == true
98+
7499
when:
75-
def result = new PropertyDataFetcher("booleanField").get(environment)
100+
result = fetcher.get(field, dataHolder, { environment })
76101
then:
77102
result == true
103+
104+
where:
105+
fetcher | _
106+
new PropertyDataFetcher("booleanField") | _
107+
SingletonPropertyDataFetcher.singleton() | _
78108
}
79109

80110
def "get Boolean property value with get"() {
81111
given:
82-
def environment = env(GraphQLBoolean)
112+
def environment = env("booleanFieldWithGet", GraphQLBoolean)
113+
def field = mkField("booleanFieldWithGet", GraphQLBoolean)
114+
115+
when:
116+
def result = fetcher.get(environment)
117+
then:
118+
result == false
119+
83120
when:
84-
def result = new PropertyDataFetcher("booleanFieldWithGet").get(environment)
121+
result = fetcher.get(field, dataHolder, { environment })
85122
then:
86123
result == false
124+
125+
where:
126+
fetcher | _
127+
new PropertyDataFetcher("booleanFieldWithGet") | _
128+
SingletonPropertyDataFetcher.singleton() | _
87129
}
88130

89131
def "get public field value as property"() {
90132
given:
91-
def environment = env(GraphQLString)
133+
def environment = env("publicField", GraphQLString)
134+
def field = mkField("publicField", GraphQLString)
135+
136+
when:
137+
def result = fetcher.get(environment)
138+
then:
139+
result == "publicValue"
140+
92141
when:
93-
def result = new PropertyDataFetcher("publicField").get(environment)
142+
result = fetcher.get(field, dataHolder, { environment })
94143
then:
95144
result == "publicValue"
145+
146+
where:
147+
fetcher | _
148+
new PropertyDataFetcher("publicField") | _
149+
SingletonPropertyDataFetcher.singleton() | _
96150
}
97151
}

0 commit comments

Comments
 (0)