From 6c98ae8f9d2b289bc14067344e4850b88d3b3a63 Mon Sep 17 00:00:00 2001 From: Oryan M Date: Tue, 17 Aug 2021 13:34:28 -0400 Subject: [PATCH 1/4] Deprecate costume context class option And minimize use of deprecated api --- .../kickstart/tools/SchemaClassScanner.kt | 2 +- .../graphql/kickstart/tools/SchemaParser.kt | 18 +------ .../kickstart/tools/SchemaParserOptions.kt | 2 + .../tools/resolver/FieldResolverScanner.kt | 11 ++-- .../tools/resolver/MethodFieldResolver.kt | 2 + .../graphql/kickstart/tools/DirectiveTest.kt | 6 +-- .../kickstart/tools/EndToEndSpecHelper.kt | 8 ++- .../graphql/kickstart/tools/EndToEndTest.kt | 6 +-- .../MethodFieldResolverDataFetcherTest.kt | 24 ++++----- .../tools/MethodFieldResolverTest.kt | 5 -- .../kickstart/tools/ResolverMethodsTest.java | 1 - .../kickstart/tools/SchemaParserTest.kt | 52 +++++++------------ .../graphql/kickstart/tools/TestUtils.kt | 4 +- .../graphql/kickstart/tools/util/BiMapTest.kt | 24 ++++----- 14 files changed, 68 insertions(+), 97 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 945e2fc3..2472bef8 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -386,7 +386,7 @@ internal class SchemaClassScanner( val methods = clazz.methods val filteredMethods = methods.filter { - it.name == name || it.name == "get${name.capitalize()}" + it.name == name || it.name == "get${name.replaceFirstChar(Char::titlecase)}" }.sortedBy { it.name.length } return filteredMethods.find { !it.isSynthetic diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt index 46c2be24..673d7010 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt @@ -174,7 +174,7 @@ class SchemaParser internal constructor( .name(inputDefinition.name) .definition(inputDefinition) .description(getDocumentation(inputDefinition, options)) - .defaultValue(buildDefaultValue(inputDefinition.defaultValue)) + .apply { inputDefinition.defaultValue?.let { v -> defaultValueLiteral(v) } } .type(determineInputType(inputDefinition.type, inputObjects, referencingInputObjects)) .withDirectives(*buildDirectives(inputDefinition.directives, Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION)) builder.field(fieldBuilder.build()) @@ -288,7 +288,7 @@ class SchemaParser internal constructor( .definition(argumentDefinition) .description(getDocumentation(argumentDefinition, options)) .type(determineInputType(argumentDefinition.type, inputObjects, setOf())) - .apply { buildDefaultValue(argumentDefinition.defaultValue)?.let { defaultValue(it) } } + .apply { argumentDefinition.defaultValue?.let { defaultValueLiteral(it) } } .withDirectives(*buildDirectives(argumentDefinition.directives, Introspection.DirectiveLocation.ARGUMENT_DEFINITION)) field.argument(argumentBuilder.build()) @@ -364,20 +364,6 @@ class SchemaParser internal constructor( return nonNullValueList[0] } - private fun buildDefaultValue(value: Value<*>?): Any? { - return when (value) { - null -> null - is IntValue -> value.value.toInt() - is FloatValue -> value.value.toDouble() - is StringValue -> value.value - is EnumValue -> value.name - is BooleanValue -> value.isValue - is ArrayValue -> value.values.map { buildDefaultValue(it) } - is ObjectValue -> value.objectFields.associate { it.name to buildDefaultValue(it.value) } - else -> throw SchemaError("Unrecognized default value: $value") - } - } - private fun determineOutputType(typeDefinition: Type<*>, inputObjects: List) = determineType(GraphQLOutputType::class, typeDefinition, permittedTypesForObject, inputObjects) as GraphQLOutputType diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt index c161504f..a48b5dfe 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt @@ -64,10 +64,12 @@ data class SchemaParserOptions internal constructor( private var includeUnusedTypes = false private var useCommentsForDescriptions = true + @Deprecated("Replaced with graphql.GraphQLContext") fun contextClass(contextClass: Class<*>) = this.apply { this.contextClass = contextClass } + @Deprecated("Replaced with graphql.GraphQLContext") fun contextClass(contextClass: KClass<*>) = this.apply { this.contextClass = contextClass.java } diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt index c9613012..8bace142 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt @@ -1,5 +1,6 @@ package graphql.kickstart.tools.resolver +import graphql.GraphQLContext import graphql.Scalars import graphql.kickstart.tools.ResolverInfo import graphql.kickstart.tools.RootResolverInfo @@ -25,7 +26,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { private val log = LoggerFactory.getLogger(javaClass) - private val allowedLastArgumentTypes = listOfNotNull(DataFetchingEnvironment::class.java, options.contextClass) + private val allowedLastArgumentTypes = listOfNotNull(DataFetchingEnvironment::class.java, GraphQLContext::class.java, options.contextClass) fun findFieldResolver(field: FieldDefinition, resolverInfo: ResolverInfo): FieldResolver { val searches = resolverInfo.getFieldSearches() @@ -72,7 +73,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { } } - private fun findResolverMethod(field: FieldDefinition, search: Search): java.lang.reflect.Method? { + private fun findResolverMethod(field: FieldDefinition, search: Search): Method? { val methods = getAllMethods(search.type) val argumentCount = field.inputValueDefinitions.size + if (search.requiredFirstParameterType != null) 1 else 0 val name = field.name @@ -113,7 +114,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { private fun isBoolean(type: GraphQLLangType) = type.unwrap().let { it is TypeName && it.name == Scalars.GraphQLBoolean.name } - private fun verifyMethodArguments(method: java.lang.reflect.Method, requiredCount: Int, search: Search): Boolean { + private fun verifyMethodArguments(method: Method, requiredCount: Int, search: Search): Boolean { val appropriateFirstParameter = if (search.requiredFirstParameterType != null) { method.genericParameterTypes.firstOrNull()?.let { it == search.requiredFirstParameterType || method.declaringClass.typeParameters.contains(it) @@ -130,7 +131,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { return correctParameterCount && appropriateFirstParameter } - private fun getMethodParameterCount(method: java.lang.reflect.Method): Int { + private fun getMethodParameterCount(method: Method): Int { return try { method.kotlinFunction?.valueParameters?.size ?: method.parameterCount } catch (e: InternalError) { @@ -138,7 +139,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { } } - private fun getMethodLastParameter(method: java.lang.reflect.Method): Type? { + private fun getMethodLastParameter(method: Method): Type? { return try { method.kotlinFunction?.valueParameters?.lastOrNull()?.type?.javaType ?: method.parameterTypes.lastOrNull() diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index fa601737..1ddeb5fa 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -1,6 +1,7 @@ package graphql.kickstart.tools.resolver import com.fasterxml.jackson.core.type.TypeReference +import graphql.GraphQLContext import graphql.TrivialDataFetcher import graphql.kickstart.tools.* import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper @@ -95,6 +96,7 @@ internal class MethodFieldResolver( when (this.method.parameterTypes.last()) { null -> throw ResolverError("Expected at least one argument but got none, this is most likely a bug with graphql-java-tools") options.contextClass -> args.add { environment -> environment.getContext() } + GraphQLContext::class.java -> args.add { environment -> environment.graphQlContext } else -> args.add { environment -> environment } } } diff --git a/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt b/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt index cb47e09d..b5c4ee76 100644 --- a/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt @@ -138,9 +138,9 @@ class DirectiveTest { val parentType = environment.fieldsContainer val originalDataFetcher = environment.codeRegistry.getDataFetcher(parentType, field) - val wrappedDataFetcher = DataFetcherFactories.wrapDataFetcher(originalDataFetcher, { _, value -> - (value as? String)?.toUpperCase() - }) + val wrappedDataFetcher = DataFetcherFactories.wrapDataFetcher(originalDataFetcher) { _, value -> + (value as? String)?.uppercase() + } environment.codeRegistry.dataFetcher(parentType, field, wrappedDataFetcher) diff --git a/src/test/kotlin/graphql/kickstart/tools/EndToEndSpecHelper.kt b/src/test/kotlin/graphql/kickstart/tools/EndToEndSpecHelper.kt index 746f2874..3b56047c 100644 --- a/src/test/kotlin/graphql/kickstart/tools/EndToEndSpecHelper.kt +++ b/src/test/kotlin/graphql/kickstart/tools/EndToEndSpecHelper.kt @@ -366,25 +366,23 @@ class Mutation : GraphQLMutationResolver { } } -class OnItemCreatedContext(val newItem: Item) - class Subscription : GraphQLSubscriptionResolver { fun onItemCreated(env: DataFetchingEnvironment) = Publisher { subscriber -> - subscriber.onNext(env.getContext().newItem) + subscriber.onNext(env.graphQlContext["newItem"]) // subscriber.onComplete() } fun onItemCreatedCoroutineChannel(env: DataFetchingEnvironment): ReceiveChannel { val channel = Channel(1) - channel.offer(env.getContext().newItem) + channel.trySend(env.graphQlContext["newItem"]) return channel } suspend fun onItemCreatedCoroutineChannelAndSuspendFunction(env: DataFetchingEnvironment): ReceiveChannel { return coroutineScope { val channel = Channel(1) - channel.offer(env.getContext().newItem) + channel.trySend(env.graphQlContext["newItem"]) channel } } diff --git a/src/test/kotlin/graphql/kickstart/tools/EndToEndTest.kt b/src/test/kotlin/graphql/kickstart/tools/EndToEndTest.kt index 5d4dc51d..d162c6b0 100644 --- a/src/test/kotlin/graphql/kickstart/tools/EndToEndTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/EndToEndTest.kt @@ -75,7 +75,7 @@ class EndToEndTest { val result = gql.execute(ExecutionInput.newExecutionInput() .query(closure.invoke()) - .context(OnItemCreatedContext(newItem)) + .graphQLContext(mapOf("newItem" to newItem)) .variables(mapOf())) val data = result.getData() as Publisher @@ -597,7 +597,7 @@ class EndToEndTest { val result = gql.execute(ExecutionInput.newExecutionInput() .query(closure.invoke()) - .context(OnItemCreatedContext(newItem)) + .graphQLContext(mapOf("newItem" to newItem)) .variables(mapOf())) val data = result.getData() as Publisher @@ -625,7 +625,7 @@ class EndToEndTest { val result = gql.execute(ExecutionInput.newExecutionInput() .query(closure.invoke()) - .context(OnItemCreatedContext(newItem)) + .graphQLContext(mapOf("newItem" to newItem)) .variables(mapOf())) val data = result.getData() as Publisher diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt index 564c7e3e..c043a6cc 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt @@ -1,6 +1,7 @@ package graphql.kickstart.tools import graphql.ExecutionResult +import graphql.GraphQLContext import graphql.execution.* import graphql.execution.instrumentation.SimpleInstrumentation import graphql.kickstart.tools.resolver.FieldResolverError @@ -79,8 +80,8 @@ class MethodFieldResolverDataFetcherTest { val channel = Channel(10) init { - channel.offer("A") - channel.offer("B") + channel.trySend("A") + channel.trySend("B") } @Suppress("UNUSED_PARAMETER") @@ -176,20 +177,19 @@ class MethodFieldResolverDataFetcherTest { @Test fun `data fetcher passes environment if method has extra argument even if context is specified`() { - val options = SchemaParserOptions.newOptions().contextClass(ContextClass::class).build() - val resolver = createFetcher("active", options = options, resolver = object : GraphQLResolver { + val context = GraphQLContext.newContext().build() + val resolver = createFetcher("active", resolver = object : GraphQLResolver { fun isActive(dataClass: DataClass, env: DataFetchingEnvironment): Boolean = env is DataFetchingEnvironment }) - assertEquals(resolver.get(createEnvironment(DataClass(), context = ContextClass())), true) + assertEquals(resolver.get(createEnvironment(DataClass(), context = context)), true) } @Test fun `data fetcher passes context if method has extra argument and context is specified`() { - val context = ContextClass() - val options = SchemaParserOptions.newOptions().contextClass(ContextClass::class).build() - val resolver = createFetcher("active", options = options, resolver = object : GraphQLResolver { - fun isActive(dataClass: DataClass, ctx: ContextClass): Boolean { + val context = GraphQLContext.newContext().build() + val resolver = createFetcher("active", resolver = object : GraphQLResolver { + fun isActive(dataClass: DataClass, ctx: GraphQLContext): Boolean { return ctx == context } }) @@ -243,7 +243,7 @@ class MethodFieldResolverDataFetcherTest { private val channel = Channel(10) init { - channel.offer("A") + channel.trySend("A") channel.close(IllegalStateException("Channel error")) } @@ -281,11 +281,11 @@ class MethodFieldResolverDataFetcherTest { return FieldResolverScanner(options).findFieldResolver(field, resolverInfo).createDataFetcher() } - private fun createEnvironment(source: Any = Object(), arguments: Map = emptyMap(), context: Any? = null): DataFetchingEnvironment { + private fun createEnvironment(source: Any = Object(), arguments: Map = emptyMap(), context: GraphQLContext? = null): DataFetchingEnvironment { return DataFetchingEnvironmentImpl.newDataFetchingEnvironment(buildExecutionContext()) .source(source) .arguments(arguments) - .context(context) + .graphQLContext(context) .build() } diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt index bef07c85..4dff5e0f 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt @@ -45,7 +45,6 @@ class MethodFieldResolverTest { testNull(input: null) } """) - .context(Object()) .root(Object())) assertEquals(result.getData(), mapOf( @@ -88,7 +87,6 @@ class MethodFieldResolverTest { testNull(input: null) } """) - .context(Object()) .root(Object())) assertEquals(result.getData(), mapOf( @@ -124,7 +122,6 @@ class MethodFieldResolverTest { } """) .variables(mapOf("input" to "FooBar")) - .context(Object()) .root(Object())) assertEquals(result.getData(), mapOf("test" to 6)) @@ -156,7 +153,6 @@ class MethodFieldResolverTest { } """) .variables(mapOf("input" to listOf("Foo", "Bar"))) - .context(Object()) .root(Object())) assertEquals(result.getData(), mapOf("test" to 6)) @@ -204,7 +200,6 @@ class MethodFieldResolverTest { } """) .variables(mapOf("input" to listOf("Foo", "Bar"))) - .context(Object()) .root(Object())) assertEquals(result.getData(), mapOf("test" to 6)) diff --git a/src/test/kotlin/graphql/kickstart/tools/ResolverMethodsTest.java b/src/test/kotlin/graphql/kickstart/tools/ResolverMethodsTest.java index 8cb3c712..dfbc6d5e 100644 --- a/src/test/kotlin/graphql/kickstart/tools/ResolverMethodsTest.java +++ b/src/test/kotlin/graphql/kickstart/tools/ResolverMethodsTest.java @@ -31,7 +31,6 @@ public void testOmittedBooleanArgument() { ExecutionResult result = gql .execute(ExecutionInput.newExecutionInput() .query("query { testOmittedBoolean }") - .context(new Object()) .root(new Object())); assertTrue(result.getErrors().isEmpty()); diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt index 0a4b892c..ece86bc4 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt @@ -1,17 +1,12 @@ package graphql.kickstart.tools import graphql.kickstart.tools.resolver.FieldResolverError -import graphql.schema.GraphQLInterfaceType -import graphql.schema.GraphQLObjectType -import graphql.schema.GraphQLArgument -import graphql.schema.GraphQLInputObjectType -import graphql.schema.GraphQLNonNull +import graphql.schema.* import graphql.schema.idl.SchemaDirectiveWiring import graphql.schema.idl.SchemaDirectiveWiringEnvironment +import org.junit.Assert.assertThrows import org.junit.Before -import org.junit.Rule import org.junit.Test -import org.junit.rules.ExpectedException import org.springframework.aop.framework.ProxyFactory import java.io.FileNotFoundException import java.util.concurrent.Future @@ -19,10 +14,6 @@ import java.util.concurrent.Future class SchemaParserTest { private lateinit var builder: SchemaParserBuilder - @Rule - @JvmField - var expectedEx: ExpectedException = ExpectedException.none() - @Before fun setup() { builder = SchemaParser.newParser() @@ -197,27 +188,24 @@ class SchemaParserTest { @Test fun `parser should throw descriptive exception when object is used as input type incorrectly`() { - expectedEx.expect(SchemaError::class.java) - expectedEx.expectMessage("Was a type only permitted for object types incorrectly used as an input type, or vice-versa") - - SchemaParser.newParser() - .schemaString( - """ - type Query { - name(filter: Filter): [String] - } - - type Filter { - filter: String - } - """) - .resolvers(object : GraphQLQueryResolver { - fun name(filter: Filter): List? = null - }) - .build() - .makeExecutableSchema() - - throw AssertionError("should not be called") + assertThrows("Was a type only permitted for object types incorrectly used as an input type, or vice-versa", SchemaError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + name(filter: Filter): [String] + } + + type Filter { + filter: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun name(filter: Filter): List? = null + }) + .build() + .makeExecutableSchema() + } } @Test diff --git a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt index 955f7da5..2b0ff09b 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt @@ -6,10 +6,10 @@ import graphql.GraphQL private val mapper = ObjectMapper() -fun assertNoGraphQlErrors(gql: GraphQL, args: Map = mapOf(), context: Any = Object(), closure: () -> String): Map { +fun assertNoGraphQlErrors(gql: GraphQL, args: Map = mapOf(), context: Map = mapOf(), closure: () -> String): Map { val result = gql.execute(ExecutionInput.newExecutionInput() .query(closure.invoke()) - .context(context) + .graphQLContext(context) .root(context) .variables(args)) diff --git a/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt b/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt index f92d879f..fc753990 100644 --- a/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt @@ -1,7 +1,7 @@ package graphql.kickstart.tools.util -import org.hamcrest.CoreMatchers -import org.junit.Assert +import graphql.kickstart.tools.assertEquals +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import java.util.* @@ -36,12 +36,12 @@ class BiMapTest { val old = bimap.put("bar", 3)!! // old value should have been returned - Assert.assertEquals(2, old.toLong()) + assertEquals(2, old.toLong()) // inverse should have correct keys val expected: Set = HashSet(Arrays.asList(1, 3)) - Assert.assertEquals(expected, bimap.values) - Assert.assertEquals(expected, bimap.inverse().keys) + assertEquals(expected, bimap.values) + assertEquals(expected, bimap.inverse().keys) } @Test @@ -50,12 +50,12 @@ class BiMapTest { bimap["bar"] = 2 bimap["baz"] = 3 val inverse = bimap.inverse() - Assert.assertTrue(inverse.containsKey(1)) - Assert.assertTrue(inverse.containsKey(2)) - Assert.assertTrue(inverse.containsKey(3)) - Assert.assertThat(inverse[1], CoreMatchers.`is`("foo")) - Assert.assertThat(inverse[2], CoreMatchers.`is`("bar")) - Assert.assertThat(inverse[3], CoreMatchers.`is`("baz")) + assertTrue(inverse.containsKey(1)) + assertTrue(inverse.containsKey(2)) + assertTrue(inverse.containsKey(3)) + assertEquals(inverse[1], "foo") + assertEquals(inverse[2], "bar") + assertEquals(inverse[3], "baz") } @Test @@ -65,6 +65,6 @@ class BiMapTest { bimap["baz"] = 3 val values = bimap.values val expected = setOf(1, 2, 3) - Assert.assertEquals(expected, values) + assertEquals(expected, values) } } From 0a2e483382e51dfb5838aeea2b8f23b40ff2550d Mon Sep 17 00:00:00 2001 From: Oryan M Date: Mon, 27 Sep 2021 15:41:40 -0400 Subject: [PATCH 2/4] Do not deprecate custom context option --- .../kotlin/graphql/kickstart/tools/SchemaParserOptions.kt | 2 -- .../kickstart/tools/resolver/MethodFieldResolver.kt | 8 ++++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt index a48b5dfe..c161504f 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt @@ -64,12 +64,10 @@ data class SchemaParserOptions internal constructor( private var includeUnusedTypes = false private var useCommentsForDescriptions = true - @Deprecated("Replaced with graphql.GraphQLContext") fun contextClass(contextClass: Class<*>) = this.apply { this.contextClass = contextClass } - @Deprecated("Replaced with graphql.GraphQLContext") fun contextClass(contextClass: KClass<*>) = this.apply { this.contextClass = contextClass.java } diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index 1ddeb5fa..61ad7313 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -33,7 +33,8 @@ internal class MethodFieldResolver( private val additionalLastArgument = try { - method.kotlinFunction?.valueParameters?.size ?: method.parameterCount == (field.inputValueDefinitions.size + getIndexOffset() + 1) + (method.kotlinFunction?.valueParameters?.size + ?: method.parameterCount) == (field.inputValueDefinitions.size + getIndexOffset() + 1) } catch (e: InternalError) { method.parameterCount == (field.inputValueDefinitions.size + getIndexOffset() + 1) } @@ -95,7 +96,10 @@ internal class MethodFieldResolver( if (this.additionalLastArgument) { when (this.method.parameterTypes.last()) { null -> throw ResolverError("Expected at least one argument but got none, this is most likely a bug with graphql-java-tools") - options.contextClass -> args.add { environment -> environment.getContext() } + options.contextClass -> args.add { environment -> + environment.graphQlContext[options.contextClass] + ?: environment.getContext() // TODO: remove deprecated use in next major release + } GraphQLContext::class.java -> args.add { environment -> environment.graphQlContext } else -> args.add { environment -> environment } } From da2884bde8415d5da83b1a82ea64df6e9d454c4a Mon Sep 17 00:00:00 2001 From: Oryan M Date: Thu, 30 Sep 2021 15:13:12 -0400 Subject: [PATCH 3/4] Add warning and test --- .../tools/resolver/MethodFieldResolver.kt | 14 ++++++++++++-- .../tools/MethodFieldResolverDataFetcherTest.kt | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index 61ad7313..baf117cb 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -14,6 +14,7 @@ import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment import graphql.schema.GraphQLTypeUtil.isScalar import kotlinx.coroutines.future.future +import org.slf4j.LoggerFactory import java.lang.reflect.Method import java.util.* import kotlin.coroutines.intrinsics.suspendCoroutineUninterceptedOrReturn @@ -31,6 +32,8 @@ internal class MethodFieldResolver( val method: Method ) : FieldResolver(field, search, options, search.type) { + private val log = LoggerFactory.getLogger(javaClass) + private val additionalLastArgument = try { (method.kotlinFunction?.valueParameters?.size @@ -97,8 +100,15 @@ internal class MethodFieldResolver( when (this.method.parameterTypes.last()) { null -> throw ResolverError("Expected at least one argument but got none, this is most likely a bug with graphql-java-tools") options.contextClass -> args.add { environment -> - environment.graphQlContext[options.contextClass] - ?: environment.getContext() // TODO: remove deprecated use in next major release + 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 new GraphQLContext class when building the ExecutionInput.") + environment.getContext() // TODO: remove deprecated use in next major release + } } GraphQLContext::class.java -> args.add { environment -> environment.graphQlContext } else -> args.add { environment -> environment } diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt index c043a6cc..f6701d01 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt @@ -197,6 +197,20 @@ class MethodFieldResolverDataFetcherTest { assertEquals(resolver.get(createEnvironment(DataClass(), context = context)), true) } + @Test + fun `data fetcher passes custom context if method has extra argument and custom context is specified as part of GraphQLContext`() { + val customContext = ContextClass() + val context = GraphQLContext.of(mapOf(ContextClass::class.java to customContext)) + 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 == customContext + } + }) + + assertEquals(resolver.get(createEnvironment(DataClass(), context = context)), true) + } + @Test fun `data fetcher marshalls input object if required`() { val name = "correct name" From 71175ec8a2aa5fb959a9c9c46694e9adcef85584 Mon Sep 17 00:00:00 2001 From: Vojtech Polivka Date: Mon, 4 Oct 2021 10:32:05 -0700 Subject: [PATCH 4/4] Update warning message --- .../kickstart/tools/resolver/MethodFieldResolver.kt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index baf117cb..efd60cf7 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -104,9 +104,12 @@ internal class MethodFieldResolver( 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 new GraphQLContext class when building the ExecutionInput.") + 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." + ) environment.getContext() // TODO: remove deprecated use in next major release } }