Skip to content

Commit 424cac7

Browse files
bbakermanbbakerman
committed
Use a singleton PropertyFetcher by default - broke out the code into a SingletonPropertyDataFetcher class
1 parent b8bfd4b commit 424cac7

12 files changed

+167
-113
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ private static TypeResolver getTypeResolverForUnion(GraphQLUnionType parentType,
149149
if (typeResolver == null) {
150150
typeResolver = parentType.getTypeResolver();
151151
}
152-
return assertNotNull(typeResolver, "There must be a type resolver for union %s",parentType.getName());
152+
return assertNotNull(typeResolver, "There must be a type resolver for union %s", parentType.getName());
153153
}
154154

155155
/**
@@ -189,7 +189,7 @@ public static class Builder {
189189
private final Map<String, DataFetcherFactory<?>> systemDataFetcherMap = new LinkedHashMap<>();
190190
private final Map<String, TypeResolver> typeResolverMap = new HashMap<>();
191191
private GraphqlFieldVisibility fieldVisibility = DEFAULT_FIELD_VISIBILITY;
192-
private DataFetcherFactory<?> defaultDataFetcherFactory = PropertyDataFetcher.singletonFactory();
192+
private DataFetcherFactory<?> defaultDataFetcherFactory = SingletonPropertyDataFetcher.singletonFactory();
193193
private boolean changed = false;
194194

195195
private Builder() {

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

Lines changed: 5 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -33,35 +33,6 @@
3333
@PublicApi
3434
public class PropertyDataFetcher<T> implements LightDataFetcher<T> {
3535

36-
private static final PropertyDataFetcher<Object> SINGLETON_FETCHER = new PropertyDataFetcher<>() {
37-
@Override
38-
Object fetchImpl(String propertyName, Object source, GraphQLFieldDefinition fieldDefinition, Supplier<DataFetchingEnvironment> environmentSupplier) {
39-
return super.fetchImpl(fieldDefinition.getName(), source, fieldDefinition, environmentSupplier);
40-
}
41-
};
42-
43-
private static final DataFetcherFactory<?> SINGLETON_FETCHER_FACTORY = environment -> SINGLETON_FETCHER;
44-
45-
/**
46-
* This returns the same singleton {@link PropertyDataFetcher} that fetches property values
47-
* based on the name of the field that iis passed into it.
48-
*
49-
* @return a singleton property data fetcher
50-
*/
51-
public static PropertyDataFetcher<?> singleton() {
52-
return SINGLETON_FETCHER;
53-
}
54-
55-
/**
56-
* This returns the same singleton {@link DataFetcherFactory} that returns the value of {@link #singleton()}
57-
*
58-
* @return a singleton data fetcher factory
59-
*/
60-
public static DataFetcherFactory<?> singletonFactory() {
61-
return SINGLETON_FETCHER_FACTORY;
62-
}
63-
64-
6536
private final String propertyName;
6637
private final Function<Object, Object> function;
6738

@@ -82,11 +53,6 @@ private <O> PropertyDataFetcher(Function<O, T> function) {
8253
this.propertyName = null;
8354
}
8455

85-
private PropertyDataFetcher() {
86-
this.function = null;
87-
this.propertyName = null;
88-
}
89-
9056
/**
9157
* Returns a data fetcher that will use the property name to examine the {@link DataFetchingEnvironment#getSource()} object
9258
* for a getter method or field with that name, or if it's a map, it will look up a value using
@@ -143,26 +109,17 @@ public String getPropertyName() {
143109

144110
@Override
145111
public T get(GraphQLFieldDefinition fieldDefinition, Object source, Supplier<DataFetchingEnvironment> environmentSupplier) throws Exception {
146-
return fetchImpl(propertyName, source, fieldDefinition, environmentSupplier);
112+
return getImpl(source, fieldDefinition.getType(), environmentSupplier);
147113
}
148114

149115
@Override
150116
public T get(DataFetchingEnvironment environment) {
151-
return fetchImpl(propertyName, environment.getSource(), environment.getFieldDefinition(), () -> environment);
117+
Object source = environment.getSource();
118+
return getImpl(source, environment.getFieldType(), () -> environment);
152119
}
153120

154-
/**
155-
* This is our implementation of property fetching
156-
*
157-
* @param propertyName the name of the property to fetch in the source object
158-
* @param source the source object to fetch from
159-
* @param fieldDefinition the field definition of the field being fetched for
160-
* @param environmentSupplier a supplied of thee {@link DataFetchingEnvironment}
161-
*
162-
* @return a value of type T
163-
*/
164121
@SuppressWarnings("unchecked")
165-
T fetchImpl(String propertyName, Object source, GraphQLFieldDefinition fieldDefinition, Supplier<DataFetchingEnvironment> environmentSupplier) {
122+
private T getImpl(Object source, GraphQLOutputType fieldDefinition, Supplier<DataFetchingEnvironment> environmentSupplier) {
166123
if (source == null) {
167124
return null;
168125
}
@@ -171,7 +128,7 @@ T fetchImpl(String propertyName, Object source, GraphQLFieldDefinition fieldDefi
171128
return (T) function.apply(source);
172129
}
173130

174-
return (T) PropertyDataFetcherHelper.getPropertyValue(propertyName, source, fieldDefinition.getType(), environmentSupplier);
131+
return (T) PropertyDataFetcherHelper.getPropertyValue(propertyName, source, fieldDefinition, environmentSupplier);
175132
}
176133

177134
/**

‎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 PropertyDataFetcher.singleton();
48+
return SingletonPropertyDataFetcher.singleton();
4849
}
4950

5051
@Override

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

Lines changed: 2 additions & 1 deletion
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;
@@ -1088,7 +1089,7 @@ private Optional<OperationTypeDefinition> getOperationNamed(String name, Map<Str
10881089
}
10891090

10901091
private DataFetcher<?> dataFetcherOfLastResort() {
1091-
return PropertyDataFetcher.singleton();
1092+
return SingletonPropertyDataFetcher.singleton();
10921093
}
10931094

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

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

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package graphql
44
import graphql.schema.GraphQLFieldDefinition
55
import graphql.schema.GraphQLOutputType
66
import graphql.schema.PropertyDataFetcher
7+
import graphql.schema.SingletonPropertyDataFetcher
78
import spock.lang.Specification
89

910
import static graphql.Scalars.GraphQLBoolean
@@ -57,63 +58,94 @@ class DataFetcherTest extends Specification {
5758
}
5859

5960
def env(String propertyName, GraphQLOutputType type) {
60-
def fieldDefinition = GraphQLFieldDefinition.newFieldDefinition().name(propertyName).type(type).build()
61+
GraphQLFieldDefinition fieldDefinition = mkField(propertyName, type)
6162
newDataFetchingEnvironment().source(dataHolder).fieldType(type).fieldDefinition(fieldDefinition).build()
6263
}
6364

65+
def mkField(String propertyName, GraphQLOutputType type) {
66+
GraphQLFieldDefinition.newFieldDefinition().name(propertyName).type(type).build()
67+
}
68+
6469
def "get property value"() {
6570
given:
6671
def environment = env("property", GraphQLString)
72+
def field = mkField("property", GraphQLString)
6773
when:
6874
def result = fetcher.get(environment)
6975
then:
7076
result == "propertyValue"
7177

78+
when:
79+
result = fetcher.get(field, dataHolder, { environment })
80+
then:
81+
result == "propertyValue"
82+
7283
where:
73-
fetcher | _
74-
new PropertyDataFetcher("property") | _
75-
PropertyDataFetcher.singleton() | _
84+
fetcher | _
85+
new PropertyDataFetcher("property") | _
86+
SingletonPropertyDataFetcher.singleton() | _
7687
}
7788

7889
def "get Boolean property value"() {
7990
given:
8091
def environment = env("booleanField", GraphQLBoolean)
92+
def field = mkField("booleanField", GraphQLBoolean)
93+
8194
when:
8295
def result = fetcher.get(environment)
8396
then:
8497
result == true
8598

99+
when:
100+
result = fetcher.get(field, dataHolder, { environment })
101+
then:
102+
result == true
103+
86104
where:
87-
fetcher | _
88-
new PropertyDataFetcher("booleanField") | _
89-
PropertyDataFetcher.singleton() | _
105+
fetcher | _
106+
new PropertyDataFetcher("booleanField") | _
107+
SingletonPropertyDataFetcher.singleton() | _
90108
}
91109

92110
def "get Boolean property value with get"() {
93111
given:
94112
def environment = env("booleanFieldWithGet", GraphQLBoolean)
113+
def field = mkField("booleanFieldWithGet", GraphQLBoolean)
114+
95115
when:
96116
def result = fetcher.get(environment)
97117
then:
98118
result == false
99119

120+
when:
121+
result = fetcher.get(field, dataHolder, { environment })
122+
then:
123+
result == false
124+
100125
where:
101126
fetcher | _
102127
new PropertyDataFetcher("booleanFieldWithGet") | _
103-
PropertyDataFetcher.singleton() | _
128+
SingletonPropertyDataFetcher.singleton() | _
104129
}
105130

106131
def "get public field value as property"() {
107132
given:
108133
def environment = env("publicField", GraphQLString)
134+
def field = mkField("publicField", GraphQLString)
135+
109136
when:
110137
def result = fetcher.get(environment)
111138
then:
112139
result == "publicValue"
113140

141+
when:
142+
result = fetcher.get(field, dataHolder, { environment })
143+
then:
144+
result == "publicValue"
145+
114146
where:
115-
fetcher | _
116-
new PropertyDataFetcher("publicField") | _
117-
PropertyDataFetcher.singleton() | _
147+
fetcher | _
148+
new PropertyDataFetcher("publicField") | _
149+
SingletonPropertyDataFetcher.singleton() | _
118150
}
119151
}

‎src/test/groovy/graphql/LargeSchemaDataFetcherTest.groovy

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@ package graphql
44
import graphql.schema.FieldCoordinates
55
import graphql.schema.GraphQLFieldDefinition
66
import graphql.schema.PropertyDataFetcher
7+
import graphql.schema.SingletonPropertyDataFetcher
78
import spock.lang.Specification
89

910
class LargeSchemaDataFetcherTest extends Specification {
1011

1112
def howManyFields = 100_000
1213

13-
def "large schema with lots of fields has a property data fetcher by default"() {
14+
def "large schema with lots of fields has the same property data fetcher by default"() {
1415
def sdl = """
1516
type Query {
1617
${mkFields()}
@@ -20,8 +21,7 @@ class LargeSchemaDataFetcherTest extends Specification {
2021
when:
2122
def schema = TestUtil.schema(sdl)
2223
def codeRegistry = schema.getCodeRegistry()
23-
24-
def singletonDF = PropertyDataFetcher.singleton()
24+
def singletonDF = SingletonPropertyDataFetcher.singleton()
2525

2626
then:
2727

‎src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import graphql.language.AstPrinter
1313
import graphql.parser.Parser
1414
import graphql.schema.DataFetcher
1515
import graphql.schema.DataFetchingEnvironment
16+
import graphql.schema.LightDataFetcher
1617
import graphql.schema.PropertyDataFetcher
1718
import graphql.schema.StaticDataFetcher
1819
import org.awaitility.Awaitility
@@ -99,7 +100,7 @@ class InstrumentationTest extends Specification {
99100

100101
instrumentation.dfClasses.size() == 2
101102
instrumentation.dfClasses[0] == StaticDataFetcher.class
102-
PropertyDataFetcher.isAssignableFrom(instrumentation.dfClasses[1])
103+
LightDataFetcher.class.isAssignableFrom(instrumentation.dfClasses[1])
103104

104105
instrumentation.dfInvocations.size() == 2
105106

0 commit comments

Comments
 (0)