diff --git a/graphql-dgs-spring-boot-micrometer/src/test/kotlin/com/netflix/graphql/dgs/metrics/micrometer/MicrometerServletSmokeTest.kt b/graphql-dgs-spring-boot-micrometer/src/test/kotlin/com/netflix/graphql/dgs/metrics/micrometer/MicrometerServletSmokeTest.kt index 1cf22c992..9bf006a3c 100644 --- a/graphql-dgs-spring-boot-micrometer/src/test/kotlin/com/netflix/graphql/dgs/metrics/micrometer/MicrometerServletSmokeTest.kt +++ b/graphql-dgs-spring-boot-micrometer/src/test/kotlin/com/netflix/graphql/dgs/metrics/micrometer/MicrometerServletSmokeTest.kt @@ -932,13 +932,6 @@ class MicrometerServletSmokeTest { return CustomDataFetchingExceptionHandler() } - @Bean - open fun reverserDataLoader( - @Qualifier("dataLoaderTaskExecutor") executor: Executor - ): ExampleImplementation.ReverseStringDataLoader { - return ExampleImplementation.ReverseStringDataLoader(executor) - } - @Bean open fun dataLoaderTaskExecutor(): Executor { val executor = ThreadPoolTaskExecutor() 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 index 26d0ea65e..10b8f7d77 100644 --- 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 @@ -16,14 +16,10 @@ package com.netflix.graphql.dgs.internal -import com.netflix.graphql.dgs.DataLoaderInstrumentationExtensionProvider -import com.netflix.graphql.dgs.DgsComponent -import com.netflix.graphql.dgs.DgsDataLoader -import com.netflix.graphql.dgs.DgsDataLoaderOptionsProvider -import com.netflix.graphql.dgs.DgsDataLoaderRegistryConsumer -import com.netflix.graphql.dgs.DgsDispatchPredicate +import com.netflix.graphql.dgs.* import com.netflix.graphql.dgs.exceptions.DgsUnnamedDataLoaderOnFieldException import com.netflix.graphql.dgs.exceptions.InvalidDataLoaderTypeException +import com.netflix.graphql.dgs.exceptions.MultipleDataLoadersDefinedException import com.netflix.graphql.dgs.exceptions.UnsupportedSecuredDataLoaderException import com.netflix.graphql.dgs.internal.utils.DataLoaderNameUtil import jakarta.annotation.PostConstruct @@ -59,6 +55,8 @@ class DgsDataLoaderProvider( private val mappedBatchLoaders = mutableListOf>>() private val mappedBatchLoadersWithContext = mutableListOf>>() + private val loaderMap = mutableMapOf() + fun buildRegistry(): DataLoaderRegistry { return buildRegistryWithContextSupplier { null } } @@ -87,7 +85,8 @@ class DgsDataLoaderProvider( } private fun addDataLoaderFields() { - applicationContext.getBeansWithAnnotation(DgsComponent::class.java).values.forEach { dgsComponent -> + val dataLoaders = applicationContext.getBeansWithAnnotation(DgsComponent::class.java) + dataLoaders.values.forEach { dgsComponent -> val javaClass = AopUtils.getTargetClass(dgsComponent) javaClass.declaredFields.asSequence().filter { it.isAnnotationPresent(DgsDataLoader::class.java) } @@ -230,6 +229,11 @@ class DgsDataLoaderProvider( is MappedBatchLoaderWithContext<*, *> -> createDataLoader(holder.theLoader, holder.annotation, holder.name, contextSupplier, registry, extensionProviders) else -> throw IllegalArgumentException("Data loader ${holder.name} has unknown type") } + // detect and throw an exception if multiple data loaders use the same name + if (registry.keys.contains(holder.name)) { + throw MultipleDataLoadersDefinedException(holder.theLoader.javaClass) + } + if (holder.dispatchPredicate == null) { registry.register(holder.name, loader) } else { diff --git a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/DgsDataLoaderProviderTest.kt b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/DgsDataLoaderProviderTest.kt index da04b98b4..1fd9a39c6 100644 --- a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/DgsDataLoaderProviderTest.kt +++ b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/DgsDataLoaderProviderTest.kt @@ -18,6 +18,7 @@ package com.netflix.graphql.dgs import com.netflix.graphql.dgs.exceptions.DgsUnnamedDataLoaderOnFieldException import com.netflix.graphql.dgs.exceptions.InvalidDataLoaderTypeException +import com.netflix.graphql.dgs.exceptions.MultipleDataLoadersDefinedException import com.netflix.graphql.dgs.internal.DgsDataLoaderProvider import graphql.schema.DataFetchingEnvironmentImpl import org.assertj.core.api.Assertions.assertThat @@ -65,6 +66,16 @@ class DgsDataLoaderProviderTest { } } + @Test + fun detectDuplicateDataLoaders() { + applicationContextRunner.withBean(ExampleBatchLoader::class.java).withBean(ExampleDuplicateBatchLoader::class.java).run { context -> + val provider = context.getBean(DgsDataLoaderProvider::class.java) + assertThrows { + provider.buildRegistry() + } + } + } + @Test fun dataLoaderInvalidType() { @DgsDataLoader diff --git a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/ExampleDuplicateBatchLoader.kt b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/ExampleDuplicateBatchLoader.kt new file mode 100644 index 000000000..b8d715d50 --- /dev/null +++ b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/ExampleDuplicateBatchLoader.kt @@ -0,0 +1,28 @@ +/* + * Copyright 2021 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 + +import org.dataloader.BatchLoader +import java.util.concurrent.CompletableFuture +import java.util.concurrent.CompletionStage + +@DgsDataLoader(name = "exampleLoader") +class ExampleDuplicateBatchLoader : BatchLoader { + override fun load(keys: MutableList?): CompletionStage> { + return CompletableFuture.supplyAsync { mutableListOf("a", "b", "c") } + } +}