Merge pull request graphql-java#3754 from graphql-java/singeton_property_data_fetcher

Singleton property data fetcher

Author: bbakerman

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

@@ -20,7 +20,18 @@ public class DataFetcherFactories {
      * @return a data fetcher factory that always returns the provided data fetcher
      */
     public static <T> DataFetcherFactory<T> useDataFetcher(DataFetcher<T> dataFetcher) {
-        return fieldDefinition -> dataFetcher;
+        //noinspection deprecation
+        return new DataFetcherFactory<>() {
+            @Override
+            public DataFetcher<T> get(DataFetcherFactoryEnvironment environment) {
+                return dataFetcher;
+            }
+
+            @Override
+            public DataFetcher<T> get(GraphQLFieldDefinition fieldDefinition) {
+                return dataFetcher;
+            }
+        };
     }
 
     /**
@@ -32,7 +43,7 @@ public static <T> DataFetcherFactory<T> useDataFetcher(DataFetcher<T> dataFetche
      *
      * @return a new data fetcher that wraps the provided data fetcher
      */
-    public static DataFetcher wrapDataFetcher(DataFetcher delegateDataFetcher, BiFunction<DataFetchingEnvironment, Object, Object> mapFunction) {
+    public static DataFetcher<?> wrapDataFetcher(DataFetcher<?> delegateDataFetcher, BiFunction<DataFetchingEnvironment, Object, Object> mapFunction) {
         return environment -> {
             Object value = delegateDataFetcher.get(environment);
             if (value instanceof CompletionStage) {

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

@@ -19,7 +19,23 @@ public interface DataFetcherFactory<T> {
      * @param environment the environment that needs the data fetcher
      *
      * @return a data fetcher
+     *
+     * @deprecated This method will go away at some point and {@link DataFetcherFactory#get(GraphQLFieldDefinition)} will be used
      */
+    @Deprecated(since = "2024-11-26")
     DataFetcher<T> get(DataFetcherFactoryEnvironment environment);
 
+    /**
+     * Returns a {@link graphql.schema.DataFetcher} given the field definition
+     * which is cheaper in object allocation terms.
+     *
+     * @param fieldDefinition the field that needs the data fetcher
+     *
+     * @return a data fetcher
+     */
+
+    default DataFetcher<T> get(GraphQLFieldDefinition fieldDefinition) {
+        return null;
+    }
+
 }

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

@@ -5,8 +5,12 @@
 /**
  * This is passed to a {@link graphql.schema.DataFetcherFactory} when it is invoked to
  * get a {@link graphql.schema.DataFetcher}
+ *
+ * @deprecated This class will go away at some point in the future since its pointless wrapper
+ * of a {@link GraphQLFieldDefinition}
  */
 @PublicApi
+@Deprecated(since = "2024-11-26")
 public class DataFetcherFactoryEnvironment {
     private final GraphQLFieldDefinition fieldDefinition;
 

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

@@ -84,6 +84,7 @@ public boolean hasDataFetcher(FieldCoordinates coordinates) {
         return hasDataFetcherImpl(coordinates, dataFetcherMap, systemDataFetcherMap);
     }
 
+    @SuppressWarnings("deprecation")
     private static DataFetcher<?> getDataFetcherImpl(FieldCoordinates coordinates, GraphQLFieldDefinition fieldDefinition, Map<FieldCoordinates, DataFetcherFactory<?>> dataFetcherMap, Map<String, DataFetcherFactory<?>> systemDataFetcherMap, DataFetcherFactory<?> defaultDataFetcherFactory) {
         assertNotNull(coordinates);
         assertNotNull(fieldDefinition);
@@ -95,9 +96,15 @@ private static DataFetcher<?> getDataFetcherImpl(FieldCoordinates coordinates, G
                 dataFetcherFactory = defaultDataFetcherFactory;
             }
         }
-        return dataFetcherFactory.get(newDataFetchingFactoryEnvironment()
-                .fieldDefinition(fieldDefinition)
-                .build());
+        // call direct from the field - cheaper to not make a new environment object
+        DataFetcher<?> dataFetcher = dataFetcherFactory.get(fieldDefinition);
+        if (dataFetcher == null) {
+            DataFetcherFactoryEnvironment factoryEnvironment = newDataFetchingFactoryEnvironment()
+                    .fieldDefinition(fieldDefinition)
+                    .build();
+            dataFetcher = dataFetcherFactory.get(factoryEnvironment);
+        }
+        return dataFetcher;
     }
 
     private static boolean hasDataFetcherImpl(FieldCoordinates coords, Map<FieldCoordinates, DataFetcherFactory<?>> dataFetcherMap, Map<String, DataFetcherFactory<?>> systemDataFetcherMap) {
@@ -149,7 +156,7 @@ private static TypeResolver getTypeResolverForUnion(GraphQLUnionType parentType,
         if (typeResolver == null) {
             typeResolver = parentType.getTypeResolver();
         }
-        return assertNotNull(typeResolver, "There must be a type resolver for union %s",parentType.getName());
+        return assertNotNull(typeResolver, "There must be a type resolver for union %s", parentType.getName());
     }
 
     /**
@@ -189,7 +196,7 @@ public static class Builder {
         private final Map<String, DataFetcherFactory<?>> systemDataFetcherMap = new LinkedHashMap<>();
         private final Map<String, TypeResolver> typeResolverMap = new HashMap<>();
         private GraphqlFieldVisibility fieldVisibility = DEFAULT_FIELD_VISIBILITY;
-        private DataFetcherFactory<?> defaultDataFetcherFactory = env -> PropertyDataFetcher.fetching(env.getFieldDefinition().getName());
+        private DataFetcherFactory<?> defaultDataFetcherFactory = SingletonPropertyDataFetcher.singletonFactory();
         private boolean changed = false;
 
         private Builder() {

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

@@ -0,0 +1,60 @@
+package graphql.schema;
+
+import java.util.function.Supplier;
+
+/**
+ * The {@link SingletonPropertyDataFetcher} is much like the {@link PropertyDataFetcher} except
+ * that it is designed to only ever fetch properties via the name of the field passed in.
+ * <p>
+ * This uses the same code as {@link PropertyDataFetcher} and hence is also controlled
+ * by static methods such as {@link PropertyDataFetcher#setUseNegativeCache(boolean)}
+ *
+ * @param <T> for two
+ */
+public class SingletonPropertyDataFetcher<T> implements LightDataFetcher<T> {
+
+    private static final SingletonPropertyDataFetcher<Object> SINGLETON_FETCHER = new SingletonPropertyDataFetcher<>();
+
+    private static final DataFetcherFactory<?> SINGLETON_FETCHER_FACTORY = environment -> SINGLETON_FETCHER;
+
+    /**
+     * This returns the same singleton {@link LightDataFetcher} that fetches property values
+     * based on the name of the field that iis passed into it.
+     *
+     * @return a singleton property data fetcher
+     */
+    public static LightDataFetcher<?> singleton() {
+        return SINGLETON_FETCHER;
+    }
+
+    /**
+     * This returns the same singleton {@link DataFetcherFactory} that returns the value of {@link #singleton()}
+     *
+     * @return a singleton data fetcher factory
+     */
+    public static DataFetcherFactory<?> singletonFactory() {
+        return SINGLETON_FETCHER_FACTORY;
+    }
+
+    private SingletonPropertyDataFetcher() {
+    }
+
+    @Override
+    public T get(GraphQLFieldDefinition fieldDefinition, Object sourceObject, Supplier<DataFetchingEnvironment> environmentSupplier) throws Exception {
+        return fetchImpl(fieldDefinition, sourceObject, environmentSupplier);
+    }
+
+    @Override
+    public T get(DataFetchingEnvironment environment) throws Exception {
+        return fetchImpl(environment.getFieldDefinition(), environment.getSource(), () -> environment);
+    }
+
+    private T fetchImpl(GraphQLFieldDefinition fieldDefinition, Object source, Supplier<DataFetchingEnvironment> environmentSupplier) {
+        if (source == null) {
+            return null;
+        }
+        // this is the same code that PropertyDataFetcher uses and hence unit tests for it include this one
+        //noinspection unchecked
+        return (T) PropertyDataFetcherHelper.getPropertyValue(fieldDefinition.getName(), source, fieldDefinition.getType(), environmentSupplier);
+    }
+}

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

@@ -8,6 +8,7 @@
 import graphql.schema.DataFetcher;
 import graphql.schema.GraphQLScalarType;
 import graphql.schema.PropertyDataFetcher;
+import graphql.schema.SingletonPropertyDataFetcher;
 import graphql.schema.TypeResolver;
 
 @PublicApi
@@ -44,7 +45,7 @@ public boolean providesDataFetcher(FieldWiringEnvironment environment) {
 
     @Override
     public DataFetcher getDataFetcher(FieldWiringEnvironment environment) {
-        return new PropertyDataFetcher(environment.getFieldDefinition().getName());
+        return SingletonPropertyDataFetcher.singleton();
     }
 
     @Override

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

@@ -57,6 +57,7 @@
 import graphql.schema.GraphQLUnionType;
 import graphql.schema.GraphqlTypeComparatorRegistry;
 import graphql.schema.PropertyDataFetcher;
+import graphql.schema.SingletonPropertyDataFetcher;
 import graphql.schema.TypeResolver;
 import graphql.schema.TypeResolverProxy;
 import graphql.schema.idl.errors.NotAnInputTypeError;
@@ -801,23 +802,27 @@ GraphQLFieldDefinition buildField(BuildContext buildCtx, TypeDefinition<?> paren
         // if they have already wired in a fetcher - then leave it alone
         FieldCoordinates coordinates = FieldCoordinates.coordinates(parentType.getName(), fieldDefinition.getName());
         if (!buildCtx.getCodeRegistry().hasDataFetcher(coordinates)) {
-            DataFetcherFactory<?> dataFetcherFactory = buildDataFetcherFactory(buildCtx,
+            Optional<DataFetcherFactory<?>> dataFetcherFactory = buildDataFetcherFactory(buildCtx,
                     parentType,
                     fieldDef,
                     fieldType,
                     appliedDirectives.first,
                     appliedDirectives.second);
-            buildCtx.getCodeRegistry().dataFetcher(coordinates, dataFetcherFactory);
+
+            // if the dataFetcherFactory is empty, then it must have been the code registry default one
+            // and hence we don't need to make a "map entry" in the code registry since it will be defaulted
+            // anyway
+            dataFetcherFactory.ifPresent(fetcherFactory -> buildCtx.getCodeRegistry().dataFetcher(coordinates, fetcherFactory));
         }
         return directivesObserve(buildCtx, fieldDefinition);
     }
 
-    private DataFetcherFactory<?> buildDataFetcherFactory(BuildContext buildCtx,
-                                                          TypeDefinition<?> parentType,
-                                                          FieldDefinition fieldDef,
-                                                          GraphQLOutputType fieldType,
-                                                          List<GraphQLDirective> directives,
-                                                          List<GraphQLAppliedDirective> appliedDirectives) {
+    private Optional<DataFetcherFactory<?>> buildDataFetcherFactory(BuildContext buildCtx,
+                                                                    TypeDefinition<?> parentType,
+                                                                    FieldDefinition fieldDef,
+                                                                    GraphQLOutputType fieldType,
+                                                                    List<GraphQLDirective> directives,
+                                                                    List<GraphQLAppliedDirective> appliedDirectives) {
         String fieldName = fieldDef.getName();
         String parentTypeName = parentType.getName();
         TypeDefinitionRegistry typeRegistry = buildCtx.getTypeRegistry();
@@ -847,16 +852,18 @@ private DataFetcherFactory<?> buildDataFetcherFactory(BuildContext buildCtx,
                         if (dataFetcher == null) {
                             DataFetcherFactory<?> codeRegistryDFF = codeRegistry.getDefaultDataFetcherFactory();
                             if (codeRegistryDFF != null) {
-                                return codeRegistryDFF;
+                                // this will use the default of the code registry when its
+                                // asked for at runtime
+                                return Optional.empty();
                             }
-                            dataFetcher = dataFetcherOfLastResort(wiringEnvironment);
+                            dataFetcher = dataFetcherOfLastResort();
                         }
                     }
                 }
             }
             dataFetcherFactory = DataFetcherFactories.useDataFetcher(dataFetcher);
         }
-        return dataFetcherFactory;
+        return Optional.of(dataFetcherFactory);
     }
 
     GraphQLArgument buildArgument(BuildContext buildCtx, InputValueDefinition valueDefinition) {
@@ -1087,9 +1094,8 @@ private Optional<OperationTypeDefinition> getOperationNamed(String name, Map<Str
         return Optional.ofNullable(operationTypeDefs.get(name));
     }
 
-    private DataFetcher<?> dataFetcherOfLastResort(FieldWiringEnvironment environment) {
-        String fieldName = environment.getFieldDefinition().getName();
-        return new PropertyDataFetcher(fieldName);
+    private DataFetcher<?> dataFetcherOfLastResort() {
+        return SingletonPropertyDataFetcher.singleton();
     }
 
     private List<Directive> directivesOf(List<? extends TypeDefinition<?>> typeDefinitions) {

src/test/groovy/graphql/DataFetcherTest.groovy

@@ -1,8 +1,10 @@
 package graphql
 
 
+import graphql.schema.GraphQLFieldDefinition
 import graphql.schema.GraphQLOutputType
 import graphql.schema.PropertyDataFetcher
+import graphql.schema.SingletonPropertyDataFetcher
 import spock.lang.Specification
 
 import static graphql.Scalars.GraphQLBoolean
@@ -55,43 +57,95 @@ class DataFetcherTest extends Specification {
 
     }
 
-    def env(GraphQLOutputType type) {
-        newDataFetchingEnvironment().source(dataHolder).fieldType(type).build()
+    def env(String propertyName, GraphQLOutputType type) {
+        GraphQLFieldDefinition fieldDefinition = mkField(propertyName, type)
+        newDataFetchingEnvironment().source(dataHolder).fieldType(type).fieldDefinition(fieldDefinition).build()
+    }
+
+    def mkField(String propertyName, GraphQLOutputType type) {
+        GraphQLFieldDefinition.newFieldDefinition().name(propertyName).type(type).build()
     }
 
     def "get property value"() {
         given:
-        def environment = env(GraphQLString)
+        def environment = env("property", GraphQLString)
+        def field = mkField("property", GraphQLString)
         when:
-        def result = new PropertyDataFetcher("property").get(environment)
+        def result = fetcher.get(environment)
         then:
         result == "propertyValue"
+
+        when:
+        result = fetcher.get(field, dataHolder, { environment })
+        then:
+        result == "propertyValue"
+
+        where:
+        fetcher                                  | _
+        new PropertyDataFetcher("property")      | _
+        SingletonPropertyDataFetcher.singleton() | _
     }
 
     def "get Boolean property value"() {
         given:
-        def environment = env(GraphQLBoolean)
+        def environment = env("booleanField", GraphQLBoolean)
+        def field = mkField("booleanField", GraphQLBoolean)
+
+        when:
+        def result = fetcher.get(environment)
+        then:
+        result == true
+
         when:
-        def result = new PropertyDataFetcher("booleanField").get(environment)
+        result = fetcher.get(field, dataHolder, { environment })
         then:
         result == true
+
+        where:
+        fetcher                                  | _
+        new PropertyDataFetcher("booleanField")  | _
+        SingletonPropertyDataFetcher.singleton() | _
     }
 
     def "get Boolean property value with get"() {
         given:
-        def environment = env(GraphQLBoolean)
+        def environment = env("booleanFieldWithGet", GraphQLBoolean)
+        def field = mkField("booleanFieldWithGet", GraphQLBoolean)
+
+        when:
+        def result = fetcher.get(environment)
+        then:
+        result == false
+
         when:
-        def result = new PropertyDataFetcher("booleanFieldWithGet").get(environment)
+        result = fetcher.get(field, dataHolder, { environment })
         then:
         result == false
+
+        where:
+        fetcher                                        | _
+        new PropertyDataFetcher("booleanFieldWithGet") | _
+        SingletonPropertyDataFetcher.singleton()       | _
     }
 
     def "get public field value as property"() {
         given:
-        def environment = env(GraphQLString)
+        def environment = env("publicField", GraphQLString)
+        def field = mkField("publicField", GraphQLString)
+
+        when:
+        def result = fetcher.get(environment)
+        then:
+        result == "publicValue"
+
         when:
-        def result = new PropertyDataFetcher("publicField").get(environment)
+        result = fetcher.get(field, dataHolder, { environment })
         then:
         result == "publicValue"
+
+        where:
+        fetcher                                  | _
+        new PropertyDataFetcher("publicField")   | _
+        SingletonPropertyDataFetcher.singleton() | _
     }
 }