Merge branch 'master' into sbn-3.3.5

Netflix · Nov 6, 2024 · 529ecd9 · 529ecd9
2 parents a0bfbd5 + 8a08df7
commit 529ecd9
Show file tree

Hide file tree

Showing 25 changed files with 482 additions and 151 deletions.
diff --git a/...-autoconfigure/src/main/kotlin/com/netflix/graphql/dgs/autoconfig/DgsAutoConfiguration.kt b/...-autoconfigure/src/main/kotlin/com/netflix/graphql/dgs/autoconfig/DgsAutoConfiguration.kt
@@ -289,7 +289,9 @@ open class DgsAutoConfiguration(
             Duration.parse(configProps.preparsedDocumentProvider.cacheValidityDuration),
         )
 
+    // TODO: Remove when legacy modules are removed. This is also handled in DgsSpringGraphQLAutoConfiguration
     @Bean
+    @ConditionalOnMissingBean
     @ConditionalOnProperty(
         prefix = "$AUTO_CONF_PREFIX.introspection",
         name = ["enabled"],

diff --git a/...igure/src/main/kotlin/com/netflix/graphql/dgs/autoconfig/DgsInputArgumentConfiguration.kt b/...igure/src/main/kotlin/com/netflix/graphql/dgs/autoconfig/DgsInputArgumentConfiguration.kt
@@ -24,6 +24,7 @@ import com.netflix.graphql.dgs.internal.method.DataFetchingEnvironmentArgumentRe
 import com.netflix.graphql.dgs.internal.method.FallbackEnvironmentArgumentResolver
 import com.netflix.graphql.dgs.internal.method.InputArgumentResolver
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean
+import org.springframework.context.ApplicationContext
 import org.springframework.context.annotation.Bean
 import org.springframework.context.annotation.Configuration
 import org.springframework.core.Ordered
@@ -36,7 +37,8 @@ open class DgsInputArgumentConfiguration {
     open fun inputArgumentResolver(inputObjectMapper: InputObjectMapper): ArgumentResolver = InputArgumentResolver(inputObjectMapper)
 
     @Bean
-    open fun dataFetchingEnvironmentArgumentResolver(): ArgumentResolver = DataFetchingEnvironmentArgumentResolver()
+    open fun dataFetchingEnvironmentArgumentResolver(context: ApplicationContext): ArgumentResolver =
+        DataFetchingEnvironmentArgumentResolver(context)
 
     @Bean
     open fun coroutineArgumentResolver(): ArgumentResolver = ContinuationArgumentResolver()

diff --git a/graphql-dgs-spring-graphql-starter/build.gradle.kts b/graphql-dgs-spring-graphql-starter/build.gradle.kts
@@ -19,6 +19,7 @@ dependencies {
     api(project(":graphql-dgs-spring-boot-oss-autoconfigure"))
     api(project(":graphql-dgs-spring-graphql"))
     api(project(":graphql-dgs-client"))
+    api(project(":graphql-dgs-reactive"))
     api(project(":graphql-error-types"))
     api("org.springframework.boot:spring-boot-starter-graphql")
 }
diff --git a/.../netflix/graphql/dgs/springgraphql/autoconfig/DgsSpringGraphQLEnvironmentPostProcessor.kt b/.../netflix/graphql/dgs/springgraphql/autoconfig/DgsSpringGraphQLEnvironmentPostProcessor.kt
@@ -20,15 +20,35 @@ import org.springframework.boot.SpringApplication
 import org.springframework.boot.env.EnvironmentPostProcessor
 import org.springframework.core.env.ConfigurableEnvironment
 import org.springframework.core.env.MapPropertySource
+import org.springframework.core.env.get
 
 class DgsSpringGraphQLEnvironmentPostProcessor : EnvironmentPostProcessor {
+    companion object {
+        private const val SPRING_GRAPHQL_SCHEMA_INTROSPECTION_ENABLED = "spring.graphql.schema.introspection.enabled"
+        private const val DGS_GRAPHQL_INTROSPECTION_ENABLED = "dgs.graphql.introspection.enabled"
+    }
+
     override fun postProcessEnvironment(
         environment: ConfigurableEnvironment,
         application: SpringApplication,
     ) {
         val properties = mutableMapOf<String, Any>()
 
-        properties["spring.graphql.schema.introspection.enabled"] = environment.getProperty("dgs.graphql.introspection.enabled") ?: true
+        if (environment.getProperty(SPRING_GRAPHQL_SCHEMA_INTROSPECTION_ENABLED) != null &&
+            environment.getProperty(DGS_GRAPHQL_INTROSPECTION_ENABLED) != null
+        ) {
+            throw RuntimeException(
+                "Both properties `$SPRING_GRAPHQL_SCHEMA_INTROSPECTION_ENABLED` and `$DGS_GRAPHQL_INTROSPECTION_ENABLED` are explicitly set. Use `$DGS_GRAPHQL_INTROSPECTION_ENABLED` only",
+            )
+        } else if (environment.getProperty(DGS_GRAPHQL_INTROSPECTION_ENABLED) != null) {
+            properties[SPRING_GRAPHQL_SCHEMA_INTROSPECTION_ENABLED] = environment.getProperty(
+                DGS_GRAPHQL_INTROSPECTION_ENABLED,
+            ) ?: true
+        } else {
+            properties[SPRING_GRAPHQL_SCHEMA_INTROSPECTION_ENABLED] =
+                environment[SPRING_GRAPHQL_SCHEMA_INTROSPECTION_ENABLED] ?: true
+        }
+
         properties["spring.graphql.graphiql.enabled"] = environment.getProperty("dgs.graphql.graphiql.enabled") ?: true
         properties["spring.graphql.graphiql.path"] = environment.getProperty("dgs.graphql.graphiql.path") ?: "/graphiql"
         properties["spring.graphql.path"] = environment.getProperty("dgs.graphql.path") ?: "/graphql"

diff --git a/graphql-dgs/src/main/java/com/netflix/graphql/dgs/DgsDataLoader.java b/graphql-dgs/src/main/java/com/netflix/graphql/dgs/DgsDataLoader.java
@@ -17,7 +17,6 @@
 package com.netflix.graphql.dgs;
 
 import com.netflix.graphql.dgs.internal.utils.DataLoaderNameUtil;
-import org.dataloader.registries.DispatchPredicate;
 import org.springframework.stereotype.Component;
 
 import java.lang.annotation.ElementType;
@@ -31,7 +30,7 @@
  * The class or field must implement one of the BatchLoader interfaces.
  * See https://netflix.github.io/dgs/data-loaders/
  */
-@Target({ElementType.TYPE, ElementType.FIELD})
+@Target({ElementType.TYPE, ElementType.FIELD, ElementType.METHOD})
 @Retention(RetentionPolicy.RUNTIME)
 @Component
 @Inherited

diff --git a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/DgsCodeRegistryBuilder.kt b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/DgsCodeRegistryBuilder.kt
@@ -23,6 +23,7 @@ import graphql.schema.DataFetchingEnvironment
 import graphql.schema.FieldCoordinates
 import graphql.schema.GraphQLCodeRegistry
 import graphql.schema.GraphQLFieldDefinition
+import org.springframework.context.ApplicationContext
 
 /**
  * Utility wrapper for [GraphQLCodeRegistry.Builder] which provides
@@ -32,6 +33,7 @@ import graphql.schema.GraphQLFieldDefinition
 class DgsCodeRegistryBuilder(
     private val dataFetcherResultProcessors: List<DataFetcherResultProcessor>,
     private val graphQLCodeRegistry: GraphQLCodeRegistry.Builder,
+    private val ctx: ApplicationContext,
 ) {
     fun dataFetcher(
         coordinates: FieldCoordinates,
@@ -67,7 +69,7 @@ class DgsCodeRegistryBuilder(
             if (dfe is DgsDataFetchingEnvironment) {
                 dfe
             } else {
-                DgsDataFetchingEnvironment(dfe)
+                DgsDataFetchingEnvironment(dfe, ctx)
             }
         return processor.process(result, env)
     }

diff --git a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/DgsDataFetchingEnvironment.kt b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/DgsDataFetchingEnvironment.kt
@@ -22,10 +22,13 @@ import com.netflix.graphql.dgs.exceptions.NoDataLoaderFoundException
 import com.netflix.graphql.dgs.internal.utils.DataLoaderNameUtil
 import graphql.schema.DataFetchingEnvironment
 import org.dataloader.DataLoader
-import java.util.*
+import org.springframework.context.ApplicationContext
+import org.springframework.context.ConfigurableApplicationContext
+import org.springframework.core.type.StandardMethodMetadata
 
 class DgsDataFetchingEnvironment(
     private val dfe: DataFetchingEnvironment,
+    private val ctx: ApplicationContext,
 ) : DataFetchingEnvironment by dfe {
     fun getDfe(): DataFetchingEnvironment = this.dfe
 
@@ -45,14 +48,42 @@ class DgsDataFetchingEnvironment(
                 DataLoaderNameUtil.getDataLoaderName(loaderClass, annotation)
             } else {
                 val loaders = loaderClass.fields.filter { it.isAnnotationPresent(DgsDataLoader::class.java) }
-                if (loaders.size > 1) throw MultipleDataLoadersDefinedException(loaderClass)
-                val loaderField = loaders.firstOrNull() ?: throw NoDataLoaderFoundException(loaderClass)
-                val theAnnotation = loaderField.getAnnotation(DgsDataLoader::class.java)
-                theAnnotation.name
+                if (loaders.isEmpty()) {
+                    // annotation is not on the class, but potentially on the Bean definition
+                    tryGetDataLoaderFromBeanDefinition(loaderClass)
+                } else {
+                    if (loaders.size > 1) throw MultipleDataLoadersDefinedException(loaderClass)
+                    val loaderField = loaders.firstOrNull() ?: throw NoDataLoaderFoundException(loaderClass)
+                    val theAnnotation = loaderField.getAnnotation(DgsDataLoader::class.java)
+                    theAnnotation.name
+                }
             }
+
         return getDataLoader(loaderName) ?: throw NoDataLoaderFoundException("DataLoader with name $loaderName not found")
     }
 
+    private fun tryGetDataLoaderFromBeanDefinition(loaderClass: Class<*>): String {
+        var name = loaderClass.simpleName
+        if (ctx is ConfigurableApplicationContext) {
+            val beansOfType = ctx.beanFactory.getBeansOfType(loaderClass)
+            if (beansOfType.isEmpty()) {
+                throw NoDataLoaderFoundException(loaderClass)
+            }
+            if (beansOfType.size > 1) {
+                throw MultipleDataLoadersDefinedException(loaderClass)
+            }
+            val beanName = beansOfType.keys.first()
+            val beanDefinition = ctx.beanFactory.getBeanDefinition(beanName)
+            if (beanDefinition.source is StandardMethodMetadata) {
+                val methodMetadata = beanDefinition.source as StandardMethodMetadata
+                val method = methodMetadata.introspectedMethod
+                val methodAnnotation = method.getAnnotation(DgsDataLoader::class.java)
+                name = DataLoaderNameUtil.getDataLoaderName(loaderClass, methodAnnotation)
+            }
+        }
+        return name
+    }
+
     /**
      * Check if an argument is explicitly set using "argument.nested.property" or "argument->nested->property" syntax.
      * Note that this requires String splitting which is expensive for hot code paths.

diff --git a/...dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DuplicateEntityFetcherException.kt b/...dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DuplicateEntityFetcherException.kt
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2024 Netflix, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.netflix.graphql.dgs.exceptions
+
+import java.lang.reflect.Method
+
+class DuplicateEntityFetcherException(
+    val entityType: String,
+    val firstEntityFetcherClass: Class<out Any>,
+    val firstEntityFetcherMethod: Method,
+    val secondEntityFetcherClass: Class<out Any>,
+    val secondEntityFetcherMethod: Method,
+) : RuntimeException(
+        "Duplicate EntityFetcherResolver found for entity type $entityType, defined by ${firstEntityFetcherClass.name}.${firstEntityFetcherMethod.name} and ${secondEntityFetcherClass.name}.${secondEntityFetcherMethod.name}",
+    )
diff --git a/...src/main/kotlin/com/netflix/graphql/dgs/exceptions/MultipleDataLoadersDefinedException.kt b/...src/main/kotlin/com/netflix/graphql/dgs/exceptions/MultipleDataLoadersDefinedException.kt
@@ -17,5 +17,5 @@
 package com.netflix.graphql.dgs.exceptions
 
 class MultipleDataLoadersDefinedException(
-    clazz: Class<*>,
-) : RuntimeException("Multiple data loaders found, unable to disambiguate for ${clazz.name}.")
+    vararg classes: Class<*>,
+) : RuntimeException("Multiple data loaders found, unable to disambiguate. [${classes.joinToString { it.name }}].")
diff --git a/...ql-dgs/src/main/kotlin/com/netflix/graphql/dgs/federation/DefaultDgsFederationResolver.kt b/...ql-dgs/src/main/kotlin/com/netflix/graphql/dgs/federation/DefaultDgsFederationResolver.kt
@@ -40,6 +40,7 @@ import org.dataloader.Try
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 import org.springframework.beans.factory.annotation.Autowired
+import org.springframework.context.ApplicationContext
 import org.springframework.util.ReflectionUtils
 import reactor.core.publisher.Mono
 import java.lang.reflect.InvocationTargetException
@@ -59,9 +60,11 @@ open class DefaultDgsFederationResolver() : DgsFederationResolver {
     constructor(
         entityFetcherRegistry: EntityFetcherRegistry,
         dataFetcherExceptionHandler: Optional<DataFetcherExceptionHandler>,
+        applicationContext: ApplicationContext,
     ) : this() {
         this.entityFetcherRegistry = entityFetcherRegistry
-        dgsExceptionHandler = dataFetcherExceptionHandler
+        this.dgsExceptionHandler = dataFetcherExceptionHandler
+        this.applicationContext = applicationContext
     }
 
     /**
@@ -73,6 +76,9 @@ open class DefaultDgsFederationResolver() : DgsFederationResolver {
     @Autowired
     lateinit var dgsExceptionHandler: Optional<DataFetcherExceptionHandler>
 
+    @Autowired
+    lateinit var applicationContext: ApplicationContext
+
     private val entitiesDataFetcher: DataFetcher<Any?> = DataFetcher { env -> dgsEntityFetchers(env) }
 
     override fun entitiesFetcher(): DataFetcher<Any?> = entitiesDataFetcher
@@ -139,7 +145,12 @@ open class DefaultDgsFederationResolver() : DgsFederationResolver {
 
                             val result =
                                 if (parameterTypes.last().isAssignableFrom(DgsDataFetchingEnvironment::class.java)) {
-                                    ReflectionUtils.invokeMethod(method, target, coercedValues, DgsDataFetchingEnvironment(env))
+                                    ReflectionUtils.invokeMethod(
+                                        method,
+                                        target,
+                                        coercedValues,
+                                        DgsDataFetchingEnvironment(env, applicationContext),
+                                    )
                                 } else {
                                     ReflectionUtils.invokeMethod(method, target, coercedValues)
                                 }
@@ -216,6 +227,7 @@ open class DefaultDgsFederationResolver() : DgsFederationResolver {
         val dfe = if (env is DgsDataFetchingEnvironment) env.getDfe() else env
         return DgsDataFetchingEnvironment(
             DataFetchingEnvironmentImpl.newDataFetchingEnvironment(dfe).executionStepInfo(executionStepInfoWithPath).build(),
+            applicationContext,
         )
     }
 

diff --git a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/internal/DgsDataLoaderProvider.kt b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/internal/DgsDataLoaderProvider.kt
@@ -37,6 +37,8 @@ import org.slf4j.LoggerFactory
 import org.springframework.aop.support.AopUtils
 import org.springframework.beans.factory.NoSuchBeanDefinitionException
 import org.springframework.context.ApplicationContext
+import org.springframework.context.ConfigurableApplicationContext
+import org.springframework.core.type.StandardMethodMetadata
 import org.springframework.util.ReflectionUtils
 import java.time.Duration
 import java.util.concurrent.Executors
@@ -63,6 +65,7 @@ class DgsDataLoaderProvider(
         val dispatchPredicate: DispatchPredicate? = null,
     )
 
+    private val dataLoaders = mutableMapOf<String, Class<*>>()
     private val batchLoaders = mutableListOf<LoaderHolder<BatchLoader<*, *>>>()
     private val batchLoadersWithContext = mutableListOf<LoaderHolder<BatchLoaderWithContext<*, *>>>()
     private val mappedBatchLoaders = mutableListOf<LoaderHolder<MappedBatchLoader<*, *>>>()
@@ -120,53 +123,68 @@ class DgsDataLoaderProvider(
                         throw DgsUnnamedDataLoaderOnFieldException(field)
                     }
 
-                    fun <T : Any> createHolder(t: T): LoaderHolder<T> = LoaderHolder(t, annotation, annotation.name)
-                    when (val dataLoader = runCustomizers(field.get(dgsComponent), annotation.name, dgsComponent::class.java)) {
-                        is BatchLoader<*, *> -> batchLoaders.add(createHolder(dataLoader))
-                        is BatchLoaderWithContext<*, *> -> batchLoadersWithContext.add(createHolder(dataLoader))
-                        is MappedBatchLoader<*, *> -> mappedBatchLoaders.add(createHolder(dataLoader))
-                        is MappedBatchLoaderWithContext<*, *> -> mappedBatchLoadersWithContext.add(createHolder(dataLoader))
-                        else -> throw InvalidDataLoaderTypeException(dgsComponent::class.java)
-                    }
+                    addDataLoader(field.get(dgsComponent), annotation.name, dgsComponent::class.java, annotation)
                 }
         }
     }
 
     private fun addDataLoaderComponents() {
         val dataLoaders = applicationContext.getBeansWithAnnotation(DgsDataLoader::class.java)
-        dataLoaders.values.forEach { dgsComponent ->
-            val javaClass = AopUtils.getTargetClass(dgsComponent)
+        dataLoaders.forEach { (beanName, beanInstance) ->
+            val javaClass = AopUtils.getTargetClass(beanInstance)
+
+            // check for class-level annotations
             val annotation = javaClass.getAnnotation(DgsDataLoader::class.java)
-            val predicateField = javaClass.declaredFields.find { it.isAnnotationPresent(DgsDispatchPredicate::class.java) }
-            if (predicateField != null) {
-                ReflectionUtils.makeAccessible(predicateField)
-                val dispatchPredicate = predicateField.get(dgsComponent)
-                if (dispatchPredicate is DispatchPredicate) {
-                    addDataLoaders(dgsComponent, javaClass, annotation, dispatchPredicate)
+            if (annotation != null) {
+                val dataLoaderName = DataLoaderNameUtil.getDataLoaderName(javaClass, annotation)
+                val predicateField = javaClass.declaredFields.find { it.isAnnotationPresent(DgsDispatchPredicate::class.java) }
+                if (predicateField != null) {
+                    ReflectionUtils.makeAccessible(predicateField)
+                    val dispatchPredicate = predicateField.get(beanInstance)
+                    if (dispatchPredicate is DispatchPredicate) {
+                        addDataLoader(beanInstance, dataLoaderName, javaClass, annotation, dispatchPredicate)
+                    }
+                } else {
+                    addDataLoader(beanInstance, dataLoaderName, javaClass, annotation)
                 }
             } else {
-                addDataLoaders(dgsComponent, javaClass, annotation, null)
+                // Check for method-level bean annotations in configuration classes
+                if (applicationContext is ConfigurableApplicationContext) {
+                    val beanDefinition = applicationContext.beanFactory.getBeanDefinition(beanName)
+                    if (beanDefinition.source is StandardMethodMetadata) {
+                        val methodMetadata = beanDefinition.source as StandardMethodMetadata
+                        val method = methodMetadata.introspectedMethod
+                        val methodAnnotation = method.getAnnotation(DgsDataLoader::class.java)
+                        if (methodAnnotation != null) {
+                            val dataLoaderName = DataLoaderNameUtil.getDataLoaderName(javaClass, methodAnnotation)
+                            addDataLoader(beanInstance, dataLoaderName, javaClass, methodAnnotation, null)
+                        }
+                    }
+                }
             }
         }
     }
 
-    private fun <T : Any> addDataLoaders(
-        dgsComponent: T,
-        targetClass: Class<*>,
+    private fun <T : Any> addDataLoader(
+        dataLoader: T,
+        dataLoaderName: String,
+        dgsComponentClass: Class<*>,
         annotation: DgsDataLoader,
-        dispatchPredicate: DispatchPredicate?,
+        dispatchPredicate: DispatchPredicate? = null,
     ) {
-        val name = DataLoaderNameUtil.getDataLoaderName(targetClass, annotation)
+        if (dataLoaders.contains(dataLoaderName)) {
+            throw MultipleDataLoadersDefinedException(dgsComponentClass, dataLoaders.getValue(dataLoaderName))
+        }
+        dataLoaders[dataLoaderName] = dgsComponentClass
 
-        fun <T : Any> createHolder(t: T): LoaderHolder<T> =
-            LoaderHolder(t, annotation, DataLoaderNameUtil.getDataLoaderName(targetClass, annotation), dispatchPredicate)
+        fun <T : Any> createHolder(t: T): LoaderHolder<T> = LoaderHolder(t, annotation, dataLoaderName, dispatchPredicate)
 
-        when (val dataLoader = runCustomizers(dgsComponent, name, dgsComponent::class.java)) {
-            is BatchLoader<*, *> -> batchLoaders.add(createHolder(dataLoader))
-            is BatchLoaderWithContext<*, *> -> batchLoadersWithContext.add(createHolder(dataLoader))
-            is MappedBatchLoader<*, *> -> mappedBatchLoaders.add(createHolder(dataLoader))
-            is MappedBatchLoaderWithContext<*, *> -> mappedBatchLoadersWithContext.add(createHolder(dataLoader))
-            else -> throw InvalidDataLoaderTypeException(dgsComponent::class.java)
+        when (val customizedDataLoader = runCustomizers(dataLoader, dataLoaderName, dgsComponentClass)) {
+            is BatchLoader<*, *> -> batchLoaders.add(createHolder(customizedDataLoader))
+            is BatchLoaderWithContext<*, *> -> batchLoadersWithContext.add(createHolder(customizedDataLoader))
+            is MappedBatchLoader<*, *> -> mappedBatchLoaders.add(createHolder(customizedDataLoader))
+            is MappedBatchLoaderWithContext<*, *> -> mappedBatchLoadersWithContext.add(createHolder(customizedDataLoader))
+            else -> throw InvalidDataLoaderTypeException(dgsComponentClass)
         }
     }