Skip to content

Commit d1b132a

Browse files
bbakermanbbakerman
committed
Merge branch 'singeton_property_data_fetcher' into no_df_if_codereg_default
# Conflicts: # src/main/java/graphql/schema/PropertyDataFetcher.java
2 parents b8b2e26 + 424cac7 commit d1b132a

12 files changed

+167
-123
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ private static TypeResolver getTypeResolverForUnion(GraphQLUnionType parentType,
155155
if (typeResolver == null) {
156156
typeResolver = parentType.getTypeResolver();
157157
}
158-
return assertNotNull(typeResolver, "There must be a type resolver for union %s",parentType.getName());
158+
return assertNotNull(typeResolver, "There must be a type resolver for union %s", parentType.getName());
159159
}
160160

161161
/**
@@ -195,7 +195,7 @@ public static class Builder {
195195
private final Map<String, DataFetcherFactory<?>> systemDataFetcherMap = new LinkedHashMap<>();
196196
private final Map<String, TypeResolver> typeResolverMap = new HashMap<>();
197197
private GraphqlFieldVisibility fieldVisibility = DEFAULT_FIELD_VISIBILITY;
198-
private DataFetcherFactory<?> defaultDataFetcherFactory = PropertyDataFetcher.singletonFactory();
198+
private DataFetcherFactory<?> defaultDataFetcherFactory = SingletonPropertyDataFetcher.singletonFactory();
199199
private boolean changed = false;
200200

201201
private Builder() {

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

Lines changed: 5 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -33,45 +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 = new DataFetcherFactory<Object>() {
44-
@Override
45-
public DataFetcher<Object> get(DataFetcherFactoryEnvironment environment) {
46-
return SINGLETON_FETCHER;
47-
}
48-
49-
@Override
50-
public DataFetcher<Object> getViaField(GraphQLFieldDefinition fieldDefinition) {
51-
return SINGLETON_FETCHER;
52-
}
53-
};
54-
55-
/**
56-
* This returns the same singleton {@link PropertyDataFetcher} that fetches property values
57-
* based on the name of the field that iis passed into it.
58-
*
59-
* @return a singleton property data fetcher
60-
*/
61-
public static PropertyDataFetcher<?> singleton() {
62-
return SINGLETON_FETCHER;
63-
}
64-
65-
/**
66-
* This returns the same singleton {@link DataFetcherFactory} that returns the value of {@link #singleton()}
67-
*
68-
* @return a singleton data fetcher factory
69-
*/
70-
public static DataFetcherFactory<?> singletonFactory() {
71-
return SINGLETON_FETCHER_FACTORY;
72-
}
73-
74-
7536
private final String propertyName;
7637
private final Function<Object, Object> function;
7738

@@ -92,11 +53,6 @@ private <O> PropertyDataFetcher(Function<O, T> function) {
9253
this.propertyName = null;
9354
}
9455

95-
private PropertyDataFetcher() {
96-
this.function = null;
97-
this.propertyName = null;
98-
}
99-
10056
/**
10157
* Returns a data fetcher that will use the property name to examine the {@link DataFetchingEnvironment#getSource()} object
10258
* for a getter method or field with that name, or if it's a map, it will look up a value using
@@ -153,26 +109,17 @@ public String getPropertyName() {
153109

154110
@Override
155111
public T get(GraphQLFieldDefinition fieldDefinition, Object source, Supplier<DataFetchingEnvironment> environmentSupplier) throws Exception {
156-
return fetchImpl(propertyName, source, fieldDefinition, environmentSupplier);
112+
return getImpl(source, fieldDefinition.getType(), environmentSupplier);
157113
}
158114

159115
@Override
160116
public T get(DataFetchingEnvironment environment) {
161-
return fetchImpl(propertyName, environment.getSource(), environment.getFieldDefinition(), () -> environment);
117+
Object source = environment.getSource();
118+
return getImpl(source, environment.getFieldType(), () -> environment);
162119
}
163120

164-
/**
165-
* This is our implementation of property fetching
166-
*
167-
* @param propertyName the name of the property to fetch in the source object
168-
* @param source the source object to fetch from
169-
* @param fieldDefinition the field definition of the field being fetched for
170-
* @param environmentSupplier a supplied of thee {@link DataFetchingEnvironment}
171-
*
172-
* @return a value of type T
173-
*/
174121
@SuppressWarnings("unchecked")
175-
T fetchImpl(String propertyName, Object source, GraphQLFieldDefinition fieldDefinition, Supplier<DataFetchingEnvironment> environmentSupplier) {
122+
private T getImpl(Object source, GraphQLOutputType fieldDefinition, Supplier<DataFetchingEnvironment> environmentSupplier) {
176123
if (source == null) {
177124
return null;
178125
}
@@ -181,7 +128,7 @@ T fetchImpl(String propertyName, Object source, GraphQLFieldDefinition fieldDefi
181128
return (T) function.apply(source);
182129
}
183130

184-
return (T) PropertyDataFetcherHelper.getPropertyValue(propertyName, source, fieldDefinition.getType(), environmentSupplier);
131+
return (T) PropertyDataFetcherHelper.getPropertyValue(propertyName, source, fieldDefinition, environmentSupplier);
185132
}
186133

187134
/**

‎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;
@@ -1094,7 +1095,7 @@ private Optional<OperationTypeDefinition> getOperationNamed(String name, Map<Str
10941095
}
10951096

10961097
private DataFetcher<?> dataFetcherOfLastResort() {
1097-
return PropertyDataFetcher.singleton();
1098+
return SingletonPropertyDataFetcher.singleton();
10981099
}
10991100

11001101
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)