diff --git a/pom.xml b/pom.xml index 555e0846..8e1c131b 100644 --- a/pom.xml +++ b/pom.xml @@ -14,10 +14,10 @@ UTF-8 11 - 1.8.21 - 1.7.3 - 2.16.0 - 21.3 + 2.0.20 + 1.9.0 + 2.17.0 + 22.3 1.0.4 ${java.version} @@ -83,7 +83,7 @@ 3.29.2-GA provided - + org.springframework spring-aop @@ -91,7 +91,7 @@ provided - + cglib cglib-nodep @@ -134,8 +134,8 @@ org.jetbrains.kotlin kotlin-stdlib - + org.jetbrains annotations @@ -240,7 +240,8 @@ org.codehaus.mojo build-helper-maven-plugin - 3.5.0 + + 3.4.0 add-test-source diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolver.kt index af88f6aa..addf61d7 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolver.kt @@ -1,10 +1,6 @@ package graphql.kickstart.tools.resolver import graphql.kickstart.tools.* -import graphql.kickstart.tools.GenericType -import graphql.kickstart.tools.ResolverError -import graphql.kickstart.tools.ResolverInfo -import graphql.kickstart.tools.TypeClassMatcher import graphql.kickstart.tools.util.JavaType import graphql.language.FieldDefinition import graphql.schema.DataFetcher @@ -29,12 +25,15 @@ internal abstract class FieldResolver( /** * Add source resolver depending on whether or not this is a resolver method */ - protected fun getSourceResolver(): SourceResolver { + protected fun createSourceResolver(): SourceResolver { return if (this.search.source != null) { - { this.search.source } + SourceResolver { _, _ -> this.search.source } } else { - { environment -> - val source = environment.getSource() + SourceResolver { environment, sourceObject -> + // if source object is known, environment is null as an optimization (LightDataFetcher) + val source = sourceObject + ?: environment?.getSource() + ?: throw ResolverError("Expected DataFetchingEnvironment and source object to not be null!") if (!this.genericType.isAssignableFrom(source.javaClass)) { throw ResolverError("Expected source object to be an instance of '${this.genericType.getRawClass().name}' but instead got '${source.javaClass.name}'") @@ -46,4 +45,7 @@ internal abstract class FieldResolver( } } -internal typealias SourceResolver = (DataFetchingEnvironment) -> Any +fun interface SourceResolver { + + fun resolve(environment: DataFetchingEnvironment?, source: Any?): Any +} diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt index 20e51a24..e2b635be 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt @@ -8,6 +8,9 @@ import graphql.kickstart.tools.util.JavaType import graphql.language.FieldDefinition import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment +import graphql.schema.GraphQLFieldDefinition +import graphql.schema.LightDataFetcher +import java.util.function.Supplier /** * @author Nick Weedon @@ -37,7 +40,7 @@ internal class MapFieldResolver( } override fun createDataFetcher(): DataFetcher<*> { - return MapFieldResolverDataFetcher(getSourceResolver(), field.name) + return MapFieldResolverDataFetcher(createSourceResolver(), field.name) } override fun scanForMatches(): List { @@ -50,14 +53,17 @@ internal class MapFieldResolver( internal class MapFieldResolverDataFetcher( private val sourceResolver: SourceResolver, private val key: String -) : DataFetcher { +) : LightDataFetcher { - override fun get(environment: DataFetchingEnvironment): Any? { - val resolvedSourceObject = sourceResolver(environment) - if (resolvedSourceObject is Map<*, *>) { - return resolvedSourceObject[key] + override fun get(fieldDefinition: GraphQLFieldDefinition, sourceObject: Any?, environmentSupplier: Supplier): Any? { + if (sourceObject is Map<*, *>) { + return sourceObject[key] } else { throw RuntimeException("MapFieldResolver attempt to fetch a field from an object instance that was not a map") } } + + override fun get(environment: DataFetchingEnvironment): Any? { + return get(environment.fieldDefinition, sourceResolver.resolve(environment, null)) { environment } + } } diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index c76023d5..76fc7f19 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -2,22 +2,23 @@ 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 import graphql.kickstart.tools.util.JavaType import graphql.kickstart.tools.util.coroutineScope -import graphql.kickstart.tools.util.isTrivialDataFetcher import graphql.kickstart.tools.util.unwrap import graphql.language.* import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment +import graphql.schema.GraphQLFieldDefinition import graphql.schema.GraphQLTypeUtil.isScalar +import graphql.schema.LightDataFetcher import kotlinx.coroutines.future.future import org.slf4j.LoggerFactory import java.lang.reflect.InvocationTargetException import java.lang.reflect.Method import java.util.* +import java.util.function.Supplier import kotlin.coroutines.intrinsics.suspendCoroutineUninterceptedOrReturn import kotlin.reflect.full.valueParameters import kotlin.reflect.jvm.javaType @@ -35,13 +36,9 @@ internal class MethodFieldResolver( private val log = LoggerFactory.getLogger(javaClass) - private val additionalLastArgument = - try { - (method.kotlinFunction?.valueParameters?.size - ?: method.parameterCount) == (field.inputValueDefinitions.size + getIndexOffset() + 1) - } catch (e: InternalError) { - method.parameterCount == (field.inputValueDefinitions.size + getIndexOffset() + 1) - } + private val isSuspendFunction = method.isSuspendFunction() + private val numberOfParameters = method.kotlinFunction?.valueParameters?.size ?: method.parameterCount + private val hasAdditionalParameter = numberOfParameters == (field.inputValueDefinitions.size + getIndexOffset() + 1) override fun createDataFetcher(): DataFetcher<*> { val args = mutableListOf() @@ -53,6 +50,7 @@ internal class MethodFieldResolver( args.add { environment -> val source = environment.getSource() + ?: throw ResolverError("Expected source object to not be null!") if (!expectedType.isAssignableFrom(source.javaClass)) { throw ResolverError("Source type (${source.javaClass.name}) is not expected type (${expectedType.name})!") } @@ -97,7 +95,7 @@ internal class MethodFieldResolver( } // Add DataFetchingEnvironment/Context argument - if (this.additionalLastArgument) { + if (this.hasAdditionalParameter) { 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 -> @@ -114,15 +112,18 @@ internal class MethodFieldResolver( environment.getContext() // TODO: remove deprecated use in next major release } } + GraphQLContext::class.java -> args.add { environment -> environment.graphQlContext } else -> args.add { environment -> environment } } } - return if (args.isEmpty() && isTrivialDataFetcher(this.method)) { - TrivialMethodFieldResolverDataFetcher(getSourceResolver(), this.method, args, options) + return if (numberOfParameters > 0 || isSuspendFunction) { + // requires arguments and environment or is a suspend function + MethodFieldResolverDataFetcher(createSourceResolver(), this.method, args, options, isSuspendFunction) } else { - MethodFieldResolverDataFetcher(getSourceResolver(), this.method, args, options) + // if there are no parameters an optimized version of the data fetcher can be used + LightMethodFieldResolverDataFetcher(createSourceResolver(), this.method, options) } } @@ -139,19 +140,23 @@ internal class MethodFieldResolver( return when (type) { is ListType -> List::class.java.isAssignableFrom(this.genericType.getRawClass(genericParameterType)) && isConcreteScalarType(environment, type.type, this.genericType.unwrapGenericType(genericParameterType)) + is TypeName -> environment.graphQLSchema?.getType(type.name)?.let { isScalar(it) && type.name != "ID" } ?: false + is NonNullType -> isConcreteScalarType(environment, type.type, genericParameterType) else -> false } } override fun scanForMatches(): List { - val unwrappedGenericType = genericType.unwrapGenericType(try { - method.kotlinFunction?.returnType?.javaType ?: method.genericReturnType - } catch (e: InternalError) { - method.genericReturnType - }) + val unwrappedGenericType = genericType.unwrapGenericType( + try { + method.kotlinFunction?.returnType?.javaType ?: method.genericReturnType + } catch (e: InternalError) { + method.genericReturnType + } + ) val returnValueMatch = TypeClassMatcher.PotentialMatch.returnValue(field.type, unwrappedGenericType, genericType, SchemaClassScanner.ReturnValueReference(method)) return field.inputValueDefinitions.mapIndexed { i, inputDefinition -> @@ -183,68 +188,92 @@ internal class MethodFieldResolver( override fun toString() = "MethodFieldResolver{method=$method}" } -internal open class MethodFieldResolverDataFetcher( +internal class MethodFieldResolverDataFetcher( private val sourceResolver: SourceResolver, - method: Method, + private val method: Method, private val args: List, - private val options: SchemaParserOptions + private val options: SchemaParserOptions, + private val isSuspendFunction: Boolean ) : DataFetcher { - private val resolverMethod = method - private val isSuspendFunction = try { - method.kotlinFunction?.isSuspend == true - } catch (e: InternalError) { - false - } - - private class CompareGenericWrappers { - companion object : Comparator { - override fun compare(w1: GenericWrapper, w2: GenericWrapper): Int = when { - w1.type.isAssignableFrom(w2.type) -> 1 - else -> -1 - } - } - } - override fun get(environment: DataFetchingEnvironment): Any? { - val source = sourceResolver(environment) + val source = sourceResolver.resolve(environment, null) val args = this.args.map { it(environment) }.toTypedArray() return if (isSuspendFunction) { environment.coroutineScope().future(options.coroutineContextProvider.provide()) { - invokeSuspend(source, resolverMethod, args)?.transformWithGenericWrapper(environment) + invokeSuspend(source, method, args)?.transformWithGenericWrapper(options.genericWrappers) { environment } } } else { - invoke(resolverMethod, source, args)?.transformWithGenericWrapper(environment) + invoke(method, source, args)?.transformWithGenericWrapper(options.genericWrappers) { environment } } } - private fun Any.transformWithGenericWrapper(environment: DataFetchingEnvironment): Any? { - return options.genericWrappers - .asSequence() - .filter { it.type.isInstance(this) } - .sortedWith(CompareGenericWrappers) - .firstOrNull() - ?.transformer?.invoke(this, environment) ?: this + /** + * Function that returns the object used to fetch the data. It can be a DataFetcher or an entity. + */ + @Suppress("unused") + fun getWrappedFetchingObject(environment: DataFetchingEnvironment): Any { + return sourceResolver.resolve(environment, null) + } +} + +/** + * Similar to [MethodFieldResolverDataFetcher] but for light data fetchers which do not require the environment to be supplied unless generic wrappers are used. + */ +internal class LightMethodFieldResolverDataFetcher( + private val sourceResolver: SourceResolver, + private val method: Method, + private val options: SchemaParserOptions +) : LightDataFetcher { + + override fun get(fieldDefinition: GraphQLFieldDefinition?, sourceObject: Any?, environmentSupplier: Supplier): Any? { + val source = sourceResolver.resolve(null, sourceObject) + + return invoke(method, source, emptyArray())?.transformWithGenericWrapper(options.genericWrappers, environmentSupplier) + } + + override fun get(environment: DataFetchingEnvironment): Any? { + return get(environment.fieldDefinition, sourceResolver.resolve(environment, null)) { environment } } /** - * Function that returns the object used to fetch the data. - * It can be a DataFetcher or an entity. + * Function that returns the object used to fetch the data. It can be a DataFetcher or an entity. */ @Suppress("unused") - open fun getWrappedFetchingObject(environment: DataFetchingEnvironment): Any { - return sourceResolver(environment) + fun getWrappedFetchingObject(environment: DataFetchingEnvironment): Any { + return sourceResolver.resolve(environment, null) } } -internal class TrivialMethodFieldResolverDataFetcher( - sourceResolver: SourceResolver, - method: Method, - args: List, - options: SchemaParserOptions -) : MethodFieldResolverDataFetcher(sourceResolver, method, args, options), - TrivialDataFetcher // just to mark it for tracing and optimizations +private fun Any.transformWithGenericWrapper( + genericWrappers: List, + environmentSupplier: Supplier +): Any { + return genericWrappers + .asSequence() + .filter { it.type.isInstance(this) } + .sortedWith(CompareGenericWrappers) + .firstOrNull() + ?.transformer?.invoke(this, environmentSupplier.get()) ?: this +} + +private class CompareGenericWrappers { + companion object : Comparator { + override fun compare(w1: GenericWrapper, w2: GenericWrapper): Int = when { + w1.type.isAssignableFrom(w2.type) -> 1 + else -> -1 + } + } +} + +private fun Method.isSuspendFunction(): Boolean { + return try { + this.kotlinFunction?.isSuspend == true + } catch (e: InternalError) { + false + } +} private suspend inline fun invokeSuspend(target: Any, resolverMethod: Method, args: Array): Any? { return suspendCoroutineUninterceptedOrReturn { continuation -> @@ -256,7 +285,7 @@ private fun invoke(method: Method, instance: Any, args: Array): Any? { try { return method.invoke(instance, *args) } catch (e: InvocationTargetException) { - throw e.cause ?: RuntimeException("Unknown error occurred while invoking resolver method") + throw e.cause ?: RuntimeException("Unknown error occurred while invoking resolver method") } } diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt index dba3e9dd..6151181a 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt @@ -6,7 +6,10 @@ import graphql.kickstart.tools.TypeClassMatcher import graphql.language.FieldDefinition import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment +import graphql.schema.GraphQLFieldDefinition +import graphql.schema.LightDataFetcher import java.lang.reflect.Field +import java.util.function.Supplier /** * @author Andrew Potter @@ -19,7 +22,7 @@ internal class PropertyFieldResolver( ) : FieldResolver(field, search, options, property.declaringClass) { override fun createDataFetcher(): DataFetcher<*> { - return PropertyFieldResolverDataFetcher(getSourceResolver(), property) + return PropertyFieldResolverDataFetcher(createSourceResolver(), property) } override fun scanForMatches(): List { @@ -28,7 +31,8 @@ internal class PropertyFieldResolver( field.type, property.genericType, genericType, - SchemaClassScanner.FieldTypeReference(property.toString())) + SchemaClassScanner.FieldTypeReference(property.toString()) + ) ) } @@ -38,9 +42,13 @@ internal class PropertyFieldResolver( internal class PropertyFieldResolverDataFetcher( private val sourceResolver: SourceResolver, private val field: Field -) : DataFetcher { +) : LightDataFetcher { + + override fun get(fieldDefinition: GraphQLFieldDefinition, sourceObject: Any?, environmentSupplier: Supplier): Any? { + return field.get(sourceResolver.resolve(null, sourceObject)) + } override fun get(environment: DataFetchingEnvironment): Any? { - return field.get(sourceResolver(environment)) + return get(environment.fieldDefinition, sourceResolver.resolve(environment, null)) { environment } } } diff --git a/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt b/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt index ba5e22a5..e61e88c9 100644 --- a/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt +++ b/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt @@ -61,23 +61,4 @@ internal fun getDocumentation(node: AbstractNode<*>, options: SchemaParserOption .trimIndent() } -/** - * Simple heuristic to check is a method is a trivial data fetcher. - * - * Requirements are: - * prefixed with get - * must have zero parameters - */ -internal fun isTrivialDataFetcher(method: Method): Boolean { - return (method.parameterCount == 0 - && ( - method.name.startsWith("get") - || isBooleanGetter(method))) -} - -private fun isBooleanGetter(method: Method) = (method.name.startsWith("is") - && (method.returnType == java.lang.Boolean::class.java) - || method.returnType == Boolean::class.java) - internal fun String.snakeToCamelCase(): String = split("_").joinToString(separator = "") { it.replaceFirstChar(Char::titlecase) } - diff --git a/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt b/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt index 912b3501..79e787cf 100644 --- a/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt @@ -10,6 +10,7 @@ import graphql.schema.idl.SchemaDirectiveWiringEnvironment import org.junit.Test class DirectiveTest { + @Test fun `should apply @uppercase directive on field`() { val schema = SchemaParser.newParser() diff --git a/src/test/kotlin/graphql/kickstart/tools/EndToEndSpecHelper.kt b/src/test/kotlin/graphql/kickstart/tools/EndToEndSpecHelper.kt index 329f9ed8..b728f7a0 100644 --- a/src/test/kotlin/graphql/kickstart/tools/EndToEndSpecHelper.kt +++ b/src/test/kotlin/graphql/kickstart/tools/EndToEndSpecHelper.kt @@ -141,8 +141,8 @@ input ItemSearchInput { } input NewItemInput { - name: String! @deprecated - type: Type! @deprecated(reason: "This is a reason") + name: String @deprecated + type: Type @deprecated(reason: "This is a reason") } enum Type { @@ -536,4 +536,3 @@ val uploadScalar: GraphQLScalarType = GraphQLScalarType.newScalar() throw CoercingParseLiteralException("Must use variables to specify Upload values") } }).build() - diff --git a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt index 2b0ff09b..32473510 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt @@ -7,14 +7,16 @@ import graphql.GraphQL private val mapper = ObjectMapper() fun assertNoGraphQlErrors(gql: GraphQL, args: Map = mapOf(), context: Map = mapOf(), closure: () -> String): Map { - val result = gql.execute(ExecutionInput.newExecutionInput() - .query(closure.invoke()) - .graphQLContext(context) - .root(context) - .variables(args)) + val result = gql.execute( + ExecutionInput.newExecutionInput() + .query(closure.invoke()) + .graphQLContext(context) + .root(context) + .variables(args) + ) if (result.errors.isNotEmpty()) { - throw AssertionError("GraphQL result contained errors!\n${result.errors.map { mapper.writeValueAsString(it) }.joinToString { "\n" }}") + throw AssertionError("GraphQL result contained errors!\n${result.errors.map { it.message }.joinToString("\n")}") } return result.getData() as Map diff --git a/src/test/kotlin/graphql/kickstart/tools/util/UtilsTest.kt b/src/test/kotlin/graphql/kickstart/tools/util/UtilsTest.kt index e8af532e..14cd8dd9 100644 --- a/src/test/kotlin/graphql/kickstart/tools/util/UtilsTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/util/UtilsTest.kt @@ -1,8 +1,5 @@ package graphql.kickstart.tools.util -import org.junit.Assert -import org.junit.Test - class UtilsTest { @Suppress("unused") @@ -26,19 +23,4 @@ class UtilsTest { private class UtilsTestTrivialDataFetcherBean { val isBooleanPrimitive = false } - - @Test - fun isTrivialDataFetcher() { - val clazz = Bean::class.java - - Assert.assertTrue(isTrivialDataFetcher(clazz.getMethod("getterValid"))) - Assert.assertFalse(isTrivialDataFetcher(clazz.getMethod("getterWithArgument", String::class.java))) - Assert.assertFalse(isTrivialDataFetcher(clazz.getMethod("notAGetter"))) - - Assert.assertFalse(isTrivialDataFetcher(clazz.getMethod("isString"))) - Assert.assertTrue(isTrivialDataFetcher(clazz.getMethod("isJavaBoolean"))) - Assert.assertTrue(isTrivialDataFetcher(clazz.getMethod("isKotlinBoolean"))) - - Assert.assertTrue(isTrivialDataFetcher(UtilsTestTrivialDataFetcherBean::class.java.getMethod("isBooleanPrimitive"))) - } }