Repository: graphql-java-kickstart/graphql-java-tools
Pull request #566 (2021-08-17 17:44:02)

Update custom context class option to be compatible with new GraphQLContext class
Also minimize use of deprecated APIs. 

## Checklist
- [x] Pull requests follows the [contribution guide](https://github.com/graphql-java-kickstart/graphql-java-tools/wiki/Contribution-guide)
- [x] New or modified functionality is covered by tests

## Description
Following https://github.com/graphql-java/graphql-java/pull/2368 the custom context class option is deprecated in GraphQL Java, this PR does the same for Tools by using the "official" `GraphQLContext` class ~~and deprecating the `contextClass` option~~.

The `contextClass` option is still available as a value in the new context object with the context class as the key:
Clients that want to keep using this option will have to adjust accordingly:

```
var executionInput = ExecutionInput.newExecutionInput()
            .query(query)
            .variables(variables)
            .graphQLContext(Map.of(CustomContext.class, context));
```

`environment.<GraphQLContext>getContext();` -> `environment.getGraphQLContext().<CustomContext>get(CustomContext.class);`



Patch:
diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt
index 945e2fc..2472bef 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/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt
index c961301..8bace14 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 fa60173..efd60cf 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
@@ -13,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
@@ -30,9 +32,12 @@ 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 ?: 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)
         }
@@ -94,7 +99,21 @@ 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 ->
+                    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."
+                        )
+                        environment.getContext() // TODO: remove deprecated use in next major release
+                    }
+                }
+                GraphQLContext::class.java -> args.add { environment -> environment.graphQlContext }
                 else -> args.add { environment -> environment }
             }
         }


Test patch:
diff --git a/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt b/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt
index cb47e09..b5c4ee7 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 746f287..3b56047 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<Item> { subscriber ->
-            subscriber.onNext(env.getContext<OnItemCreatedContext>().newItem)
+            subscriber.onNext(env.graphQlContext["newItem"])
 //            subscriber.onComplete()
         }
 
     fun onItemCreatedCoroutineChannel(env: DataFetchingEnvironment): ReceiveChannel<Item> {
         val channel = Channel<Item>(1)
-        channel.offer(env.getContext<OnItemCreatedContext>().newItem)
+        channel.trySend(env.graphQlContext["newItem"])
         return channel
     }
 
     suspend fun onItemCreatedCoroutineChannelAndSuspendFunction(env: DataFetchingEnvironment): ReceiveChannel<Item> {
         return coroutineScope {
             val channel = Channel<Item>(1)
-            channel.offer(env.getContext<OnItemCreatedContext>().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 5d4dc51..d162c6b 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<ExecutionResult>
@@ -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<ExecutionResult>
@@ -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<ExecutionResult>
diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt
index 564c7e3..f6701d0 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<String>(10)
 
         init {
-            channel.offer("A")
-            channel.offer("B")
+            channel.trySend("A")
+            channel.trySend("B")
         }
 
         @Suppress("UNUSED_PARAMETER")
@@ -176,21 +177,34 @@ 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<DataClass> {
+        val context = GraphQLContext.newContext().build()
+        val resolver = createFetcher("active", resolver = object : GraphQLResolver<DataClass> {
             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 context = GraphQLContext.newContext().build()
+        val resolver = createFetcher("active", resolver = object : GraphQLResolver<DataClass> {
+            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 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<DataClass> {
             fun isActive(dataClass: DataClass, ctx: ContextClass): Boolean {
-                return ctx == context
+                return ctx == customContext
             }
         })
 
@@ -243,7 +257,7 @@ class MethodFieldResolverDataFetcherTest {
         private val channel = Channel<String>(10)
 
         init {
-            channel.offer("A")
+            channel.trySend("A")
             channel.close(IllegalStateException("Channel error"))
         }
 
@@ -281,11 +295,11 @@ class MethodFieldResolverDataFetcherTest {
         return FieldResolverScanner(options).findFieldResolver(field, resolverInfo).createDataFetcher()
     }
 
-    private fun createEnvironment(source: Any = Object(), arguments: Map<String, Any> = emptyMap(), context: Any? = null): DataFetchingEnvironment {
+    private fun createEnvironment(source: Any = Object(), arguments: Map<String, Any> = 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 bef07c8..4dff5e0 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 8cb3c71..dfbc6d5 100644
--- a/src/test/kotlin/graphql/kickstart/tools/ResolverMethodsTest.java
+++ b/src/test/kotlin/graphql/kickstart/tools/ResolverMethodsTest.java
@@ -31,7 +31,6 @@ public class ResolverMethodsTest {
         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 0a4b892..ece86bc 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<String>? = 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<String>? = 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 955f7da..2b0ff09 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<String, Any> = mapOf(), context: Any = Object(), closure: () -> String): Map<String, Any> {
+fun assertNoGraphQlErrors(gql: GraphQL, args: Map<String, Any> = mapOf(), context: Map<Any, Any> = mapOf(), closure: () -> String): Map<String, Any> {
     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 f92d879..fc75399 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<Int> = 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)
     }
 }