diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt index d2961e4c..42dd965b 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt @@ -6,7 +6,6 @@ import graphql.kickstart.tools.relay.RelayConnectionFactory import graphql.kickstart.tools.resolver.MissingResolverDataFetcherProvider import graphql.kickstart.tools.util.JavaType import graphql.kickstart.tools.util.ParameterizedTypeImpl -import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment import graphql.schema.visibility.GraphqlFieldVisibility import kotlinx.coroutines.Dispatchers @@ -24,8 +23,6 @@ data class SchemaParserOptions internal constructor( val contextClass: Class<*>?, val genericWrappers: List, val allowUnimplementedResolvers: Boolean, - @Deprecated("Use missingResolverDataFetcherProvider instead.") - val missingResolverDataFetcher: DataFetcher?, val missingResolverDataFetcherProvider: MissingResolverDataFetcherProvider?, val objectMapperProvider: PerFieldObjectMapperProvider, val proxyHandlers: List, @@ -55,7 +52,6 @@ data class SchemaParserOptions internal constructor( private val genericWrappers: MutableList = mutableListOf() private var useDefaultGenericWrappers = true private var allowUnimplementedResolvers = false - private var missingResolverDataFetcher: DataFetcher? = null private var missingResolverDataFetcherProvider: MissingResolverDataFetcherProvider? = null private var objectMapperProvider: PerFieldObjectMapperProvider = PerFieldConfiguringObjectMapperProvider() private val proxyHandlers: MutableList = mutableListOf(Spring4AopProxyHandler(), GuiceAopProxyHandler(), JavassistProxyHandler(), WeldProxyHandler()) @@ -93,11 +89,6 @@ data class SchemaParserOptions internal constructor( this.allowUnimplementedResolvers = allowUnimplementedResolvers } - @Deprecated("Use missingResolverDataFetcherProvider instead.") - fun missingResolverDataFetcher(missingResolverDataFetcher: DataFetcher?) = this.apply { - this.missingResolverDataFetcher = missingResolverDataFetcher - } - fun missingResolverDataFetcherProvider(missingResolverDataFetcherProvider: MissingResolverDataFetcherProvider?) = this.apply { this.missingResolverDataFetcherProvider = missingResolverDataFetcherProvider } @@ -205,7 +196,6 @@ data class SchemaParserOptions internal constructor( contextClass, wrappers, allowUnimplementedResolvers, - missingResolverDataFetcher, missingResolverDataFetcherProvider, objectMapperProvider, proxyHandlers, diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt index 4b3a83d7..561096b6 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt @@ -77,10 +77,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions, private va } private fun missingFieldResolver(field: FieldDefinition): FieldResolver? { - @Suppress("DEPRECATION") - return if (options.allowUnimplementedResolvers - || options.missingResolverDataFetcher != null - || options.missingResolverDataFetcherProvider != null) { + return if (options.allowUnimplementedResolvers || options.missingResolverDataFetcherProvider != null) { if (options.allowUnimplementedResolvers) { log.warn("Missing resolver for field: $field") } diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index 9050738c..a3ce80be 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -20,7 +20,6 @@ import kotlinx.coroutines.ensureActive import kotlinx.coroutines.future.future import org.apache.commons.lang3.reflect.TypeUtils import org.reactivestreams.Publisher -import org.slf4j.LoggerFactory import java.lang.reflect.InvocationTargetException import java.lang.reflect.Method import java.util.* @@ -41,8 +40,6 @@ internal class MethodFieldResolver( private val scalarNames: Set ) : FieldResolver(field, search, options) { - private val log = LoggerFactory.getLogger(javaClass) - private val isSuspendFunction = method.isSuspendFunction() private val numberOfParameters = method.parameterCountWithoutContinuation() @@ -105,23 +102,17 @@ internal class MethodFieldResolver( val firstTrailingParameter = field.inputValueDefinitions.size + search.firstArgumentIndex method.parameterTypes.slice(firstTrailingParameter until numberOfParameters).forEach { parameterType -> when (parameterType) { + GraphQLContext::class.java -> args.add { environment -> environment.graphQlContext } + options.contextClass -> args.add { environment -> - val context: Any? = environment.graphQlContext[options.contextClass] - if (context != null) { - context - } else { - log.warn( - "Generic context class has been deprecated by graphql-java. " + - "To continue using a custom context class as the last parameter in resolver methods " + - "please insert it into the GraphQLContext map when building the ExecutionInput. " + - "This warning will become an error in the future." - ) - @Suppress("DEPRECATION") - environment.getContext() // TODO: remove deprecated use in next major release - } + environment.graphQlContext.get(parameterType) ?: throw ResolverError( + "Resolver method '${method.declaringClass.name}.${method.name}' has a parameter of the context class " + + "'${parameterType.name}', but the GraphQLContext has no value for it. Put it in the GraphQLContext under " + + "the class key when building the ExecutionInput, for example " + + "graphQLContext(mapOf(${parameterType.simpleName}::class.java to context))." + ) } - GraphQLContext::class.java -> args.add { environment -> environment.graphQlContext } else -> args.add { environment -> environment } } } diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MissingFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MissingFieldResolver.kt index 2b86457e..652af687 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MissingFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MissingFieldResolver.kt @@ -11,11 +11,8 @@ internal class MissingFieldResolver( options: SchemaParserOptions ) : FieldResolver(field, FieldResolverScanner.Search(Any::class.java, MissingResolverInfo(), null), options) { - @Suppress("DEPRECATION") private val missingResolverDataFetcherProvider: MissingResolverDataFetcherProvider = options.missingResolverDataFetcherProvider - ?: MissingResolverDataFetcherProvider { - _, options -> options.missingResolverDataFetcher ?: DataFetcher { TODO("Schema resolver not implemented") } - } + ?: MissingResolverDataFetcherProvider { _, _ -> DataFetcher { TODO("Schema resolver not implemented") } } override fun scanForMatches(): List = listOf() override fun createDataFetcher(): DataFetcher<*> = missingResolverDataFetcherProvider.createDataFetcher(field, options) } diff --git a/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt b/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt index bcd52629..5c8c98fb 100644 --- a/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt +++ b/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt @@ -9,8 +9,6 @@ import graphql.schema.idl.TypeUtil import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import org.apache.commons.lang3.reflect.TypeUtils -import org.slf4j.LoggerFactory -import java.lang.invoke.MethodHandles import java.lang.reflect.Method import java.lang.reflect.ParameterizedType import java.lang.reflect.Proxy @@ -22,8 +20,6 @@ import kotlin.reflect.jvm.kotlinFunction * @author Andrew Potter */ -private val log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()) - internal typealias GraphQLRootResolver = GraphQLResolver internal typealias JavaType = java.lang.reflect.Type @@ -70,17 +66,6 @@ internal fun DataFetchingEnvironment.coroutineScope(): CoroutineScope { return scope } - @Suppress("DEPRECATION") - val context: Any? = this.getContext() - if (context is CoroutineScope) { - log.warn( - "Passing a CoroutineScope as the context object has been deprecated by graphql-java. " + - "Please insert it into the GraphQLContext map under the CoroutineScope class key when building the ExecutionInput. " + - "Support for the context object will be removed in the next major release." - ) - return context - } - return CoroutineScope(Dispatchers.Default) } diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt index 2373c0a5..607ef081 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt @@ -19,6 +19,8 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.channels.ReceiveChannel +import org.junit.Assert.assertThrows +import org.junit.Assert.assertTrue import org.junit.Test import org.reactivestreams.Publisher import org.reactivestreams.tck.TestEnvironment @@ -98,7 +100,7 @@ class MethodFieldResolverDataFetcherTest { } @Test - fun `data fetcher executes suspend function in CoroutineScope from legacy context`() { + fun `data fetcher does not execute suspend function in CoroutineScope from legacy context`() { // setup val scopeClass = ScopeClass() @@ -107,7 +109,7 @@ class MethodFieldResolverDataFetcherTest { // expect @Suppress("UNCHECKED_CAST") val future = resolver.get(createEnvironment(DataClass(), legacyContext = scopeClass.scope)) as CompletableFuture - assert(future.get()) + assertEquals(future.get(), false) } class ScopeClass : GraphQLResolver { @@ -116,7 +118,7 @@ class MethodFieldResolverDataFetcherTest { @Suppress("UNUSED_PARAMETER") suspend fun isActive(data: DataClass): Boolean { - return coroutineContext[Job] == job.children.first() + return coroutineContext[Job] == job.children.firstOrNull() } } @@ -275,6 +277,47 @@ class MethodFieldResolverDataFetcherTest { assertEquals(resolver.get(createEnvironment(DataClass(), context = context)), true) } + @Test + fun `data fetcher fails if custom context is missing from GraphQLContext`() { + val options = SchemaParserOptions.newOptions().contextClass(ContextClass::class).build() + val resolver = createFetcher("active", options = options, resolver = object : GraphQLResolver { + fun isActive(dataClass: DataClass, ctx: ContextClass?): Boolean { + return ctx != null + } + }) + + val error = assertThrows(ResolverError::class.java) { resolver.get(createEnvironment(DataClass())) } + + assertTrue(error.message!!.contains("'${ContextClass::class.java.name}'")) + assertTrue(error.message!!.contains(".isActive'")) + assertTrue(error.message!!.contains("GraphQLContext")) + } + + @Test + fun `data fetcher fails if custom context is only passed as the legacy context object`() { + val options = SchemaParserOptions.newOptions().contextClass(ContextClass::class).build() + val resolver = createFetcher("active", options = options, resolver = object : GraphQLResolver { + fun isActive(dataClass: DataClass, ctx: ContextClass): Boolean { + return true + } + }) + + assertThrows(ResolverError::class.java) { resolver.get(createEnvironment(DataClass(), legacyContext = ContextClass())) } + } + + @Test + fun `data fetcher passes GraphQLContext if it is the custom context class`() { + val context = GraphQLContext.newContext().build() + val options = SchemaParserOptions.newOptions().contextClass(GraphQLContext::class).build() + val resolver = createFetcher("active", options = options, resolver = object : GraphQLResolver { + fun isActive(dataClass: DataClass, ctx: GraphQLContext): Boolean { + return ctx == context + } + }) + + assertEquals(resolver.get(createEnvironment(DataClass(), context = context)), true) + } + @Test fun `data fetcher passes custom context and environment if method has both as extra arguments`() { val customContext = ContextClass() diff --git a/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt index 3959216c..9272f70e 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt @@ -58,46 +58,6 @@ class MissingFieldResolverTest { )) } - @Test - @Suppress("DEPRECATION") - fun `should call missing resolver data fetcher if provided`() { - val schema = SchemaParser.newParser() - .schemaString( - """ - type Query { - implementedField(input: String): String - missingField(input: Int): Int - } - """ - ) - .resolvers(object : GraphQLQueryResolver { - fun implementedField(input: Optional) = input.toString() - }) - .options(SchemaParserOptions.newOptions() - .missingResolverDataFetcher(TestMissingResolverDataFetcher()) - .build()) - .build() - .makeExecutableSchema() - .assertEveryFieldResolves() - - val gql = GraphQL.newGraphQL(schema).build() - - val result = gql.execute( - """ - query { - implementedField(input: "test-value") - missingField(input: 1) - } - """) - - val expected = mapOf( - "implementedField" to "Optional[test-value]", - "missingField" to 1 - ) - - assertEquals(result.getData(), expected) - } - @Test fun `should call missing resolver data fetcher provider if provided`() { val missingResolverDataFetcherProvider = TestMissingResolverDataFetcherProvider();