From 8611c5165ac7fb62c0ba7400a683592a15e0ef08 Mon Sep 17 00:00:00 2001 From: Oryan Date: Mon, 12 Oct 2026 15:30:13 -0400 Subject: [PATCH 1/7] Call methods of JDK proxied resolvers through the proxy's interfaces Resolvers behind a JDK dynamic proxy, e.g. Spring AOP without proxyTargetClass, are scanned as their target class, but the proxy only implements the target's interfaces, so every field failed at runtime with "object of type jdk.proxy2.$ProxyN is not an instance of ...". Each resolver method is now called through the interface method it implements, so the proxy's interceptors still run, and build() fails with a clear error if no interface declares it. Static methods are still called directly. Only the first matching resolver of a non-root field is checked, so a supertype resolver's method that a more specific resolver takes precedence over doesn't fail the build. Fixes #822 Co-Authored-By: Claude Opus 5.5 --- .../tools/resolver/FieldResolverScanner.kt | 54 ++- .../tools/resolver/MethodFieldResolver.kt | 8 +- .../kickstart/tools/ProxiedResolverTest.kt | 371 ++++++++++++++++++ .../kickstart/tools/SchemaParserTest.kt | 20 - 4 files changed, 424 insertions(+), 29 deletions(-) create mode 100644 src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt index d787ef6d..681f10f1 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt @@ -34,7 +34,11 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { val searches = resolverInfo.getFieldSearches() val scanProperties = field.inputValueDefinitions.isEmpty() - val found = searches.mapNotNull { search -> findFieldResolver(field, search, scanProperties) } + val found = searches.asSequence() + .mapNotNull { search -> findFieldResolver(field, search, scanProperties) } + // only root fields must match a single resolver. Other fields use the first match, so the methods it shadows, + // e.g. a supertype resolver's, are never called and aren't checked + .let { if (resolverInfo is RootResolverInfo) it.toList() else it.take(1).toList() } if (resolverInfo is RootResolverInfo && found.size > 1) { throw FieldResolverError("Found more than one matching resolver for field '$field': $found") @@ -46,7 +50,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { private fun findFieldResolver(field: FieldDefinition, search: Search, scanProperties: Boolean): FieldResolver? { val method = findResolverMethod(field, search) if (method != null) { - return MethodFieldResolver(field, search, options, method.apply(trySetAccessible(field, search.type))) + return MethodFieldResolver(field, search, options, method.apply(trySetAccessible(field, search.type)), findInvocableMethod(field, search, method)) } if (scanProperties) { @@ -63,6 +67,42 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { return null } + /** + * A resolver behind a JDK dynamic proxy, e.g. Spring AOP without proxyTargetClass, is scanned as its target class, but the + * proxy only implements the target's interfaces. So its methods are called through the interface method they implement, + * which also runs the proxy's interceptors. + */ + private fun findInvocableMethod(field: FieldDefinition, search: Search, method: Method): Method { + val source = search.source + if (source == null || Modifier.isStatic(method.modifiers) || method.declaringClass.isInstance(source)) { + return method + } + + val type = search.type.unwrap() + return ClassUtils.getAllInterfaces(type) + .filter { it.isInstance(source) } + .flatMap { it.methods.asList() } + .find { !Modifier.isStatic(it.modifiers) && method.implements(it, type) } + ?.apply(trySetAccessible(field, search.type)) + ?: throw FieldResolverError( + "Resolver method '${method.declaringClass.name}.${method.name}' for field '${field.name}' as defined in schema ${getSourceLocation(field)} " + + "can't be called on ${source.javaClass.name}, which is not an instance of ${method.declaringClass.name}. " + + "This happens with JDK dynamic proxies, which only implement interfaces. " + + "Declare the method on an interface the resolver implements, or use class-based proxies (e.g. proxyTargetClass = true in Spring)." + ) + } + + // the implementation of a generic interface method takes the type arguments, e.g. echo(String) for echo(T) of Api, + // and the compiler bridges the erased echo(Object) to it. Kotlin also unboxes them, e.g. increment(int) for Api + private fun Method.implements(interfaceMethod: Method, type: Class<*>): Boolean = + name == interfaceMethod.name + && parameterCount == interfaceMethod.parameterCount + && parameterTypes.indices.all { i -> + val interfaceParameterType = interfaceMethod.genericParameterTypes[i] + val resolvedType = TypeUtils.getRawType(interfaceParameterType, type) ?: interfaceMethod.parameterTypes[i] + resolvedType == if (interfaceParameterType is TypeVariable<*>) ClassUtils.primitiveToWrapper(parameterTypes[i]) else parameterTypes[i] + } + private fun trySetAccessible(field: FieldDefinition, type: JavaType): AccessibleObject.() -> Unit = { try { isAccessible = true @@ -200,14 +240,16 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { isSubscription = isSubscription || search.isSubscription } - val sourceName = field.sourceLocation?.sourceName ?: "" - val sourceLocation = field.sourceLocation?.let { "$sourceName:${it.line}" } ?: "" - - return "No method${if (scannedProperties) " or field" else ""} found as defined in schema $sourceLocation with any of the following signatures " + + return "No method${if (scannedProperties) " or field" else ""} found as defined in schema ${getSourceLocation(field)} with any of the following signatures " + "(with or without one of $allowedLastArgumentTypes as the last argument), in priority order:\n${signatures.joinToString("\n ")}" + if (isSubscription) "\n\nNote that a Subscription data fetcher must return a Publisher of events" else "" } + private fun getSourceLocation(field: FieldDefinition): String { + val sourceName = field.sourceLocation?.sourceName ?: "" + return field.sourceLocation?.let { "$sourceName:${it.line}" } ?: "" + } + private fun getMissingMethodSignatures(field: FieldDefinition, search: Search, isBoolean: Boolean, scannedProperties: Boolean): List { val baseType = search.type.unwrap() val signatures = mutableListOf() diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index 0a9e65d6..71063ffe 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -40,7 +40,9 @@ internal class MethodFieldResolver( field: FieldDefinition, search: FieldResolverScanner.Search, options: SchemaParserOptions, - val method: Method + val method: Method, + // the method that's called, which is the interface method that [method] implements if the resolver is a JDK dynamic proxy + private val invocableMethod: Method ) : FieldResolver(field, search, options) { private val log = LoggerFactory.getLogger(javaClass) @@ -136,10 +138,10 @@ internal class MethodFieldResolver( return if (numberOfParameters > 0 || isSuspendFunction) { // requires arguments and environment or is a suspend function - MethodFieldResolverDataFetcher(createSourceResolver(), this.method, args, options, isSuspendFunction, valueClass) + MethodFieldResolverDataFetcher(createSourceResolver(), invocableMethod, args, options, isSuspendFunction, valueClass) } else { // if there are no parameters an optimized version of the data fetcher can be used - LightMethodFieldResolverDataFetcher(createSourceResolver(), this.method, options, valueClass) + LightMethodFieldResolverDataFetcher(createSourceResolver(), invocableMethod, options, valueClass) } } diff --git a/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt new file mode 100644 index 00000000..a7ddcb16 --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt @@ -0,0 +1,371 @@ +package graphql.kickstart.tools + +import graphql.ExceptionWhileDataFetching +import graphql.GraphQL +import graphql.kickstart.tools.resolver.FieldResolverError +import graphql.schema.DataFetchingEnvironment +import kotlinx.coroutines.delay +import org.aopalliance.intercept.MethodInterceptor +import org.junit.Assert.assertThrows +import org.junit.Test +import org.springframework.aop.framework.ProxyFactory +import java.lang.reflect.Proxy + +class ProxiedResolverTest { + + private val invocations = mutableListOf() + + // stands in for the interceptors Spring adds to a proxy, e.g. for @PreAuthorize or @Transactional + private val recordingInterceptor = MethodInterceptor { invocation -> + invocations.add("${invocation.method.declaringClass.simpleName}.${invocation.method.name}") + invocation.proceed() + } + + private val querySchema = """ + type Query { + test: String + greeting(name: String!): String + suspendTest: String + } + """ + + @Test + fun `JDK proxied resolver methods are called through the interface the proxy implements`() { + val gql = graphQL(querySchema, jdkProxy(Query())) + + val data = assertNoGraphQlErrors(gql) { + """ + query { + test + } + """ + } + + assertEquals(data, mapOf("test" to "ok")) + assertEquals(invocations, listOf("QueryApi.test")) + } + + @Test + fun `JDK proxied resolver methods with arguments are called through the interface the proxy implements`() { + val gql = graphQL(querySchema, jdkProxy(Query())) + + val data = assertNoGraphQlErrors(gql) { + """ + query { + greeting(name: "Rex") + } + """ + } + + assertEquals(data, mapOf("greeting" to "Hello, Rex, from greeting")) + assertEquals(invocations, listOf("QueryApi.greeting")) + } + + @Test + fun `JDK proxied resolver suspend functions are called through the interface the proxy implements`() { + val gql = graphQL(querySchema, jdkProxy(Query())) + + val data = assertNoGraphQlErrors(gql) { + """ + query { + suspendTest + } + """ + } + + assertEquals(data, mapOf("suspendTest" to "ok")) + assertEquals(invocations, listOf("QueryApi.suspendTest")) + } + + @Test + fun `JDK proxied resolver interceptors can reject the call`() { + val gql = graphQL(querySchema, jdkProxy(Query()) { throw IllegalStateException("Access denied") }) + + val result = gql.execute( + """ + query { + test + } + """) + + assertEquals(result.getData(), mapOf("test" to null)) + assertEquals(result.errors.map { (it as ExceptionWhileDataFetching).exception.message }, listOf("Access denied")) + } + + interface QueryApi { + fun test(): String + fun greeting(name: String, env: DataFetchingEnvironment): String + suspend fun suspendTest(): String + } + + class Query : GraphQLQueryResolver, QueryApi { + override fun test() = "ok" + override fun greeting(name: String, env: DataFetchingEnvironment) = "Hello, $name, from ${env.field.name}" + override suspend fun suspendTest(): String { + // suspends, so the result reaches graphql-java through the continuation instead of the proxy's return value + delay(1) + return "ok" + } + } + + private val petSchema = """ + type Query { + pet: Pet + } + + type Pet { + name: String + greeting: String + } + """ + + @Test + fun `JDK proxied field resolver methods are called through the interface the proxy implements`() { + val gql = graphQL(petSchema, PetQuery(), jdkProxy(PetResolver())) + + val data = assertNoGraphQlErrors(gql) { + """ + query { + pet { + name + greeting + } + } + """ + } + + assertEquals(data, mapOf("pet" to mapOf("name" to "Rex", "greeting" to "Hello, Rex"))) + assertEquals(invocations, listOf("GreetingApi.greeting")) + } + + @Test + fun `JDK proxied supertype resolver methods are called through the interface the proxy implements`() { + val gql = graphQL(petSchema, PetQuery(), jdkProxy(NamedResolver())) + + val data = assertNoGraphQlErrors(gql) { + """ + query { + pet { + greeting + } + } + """ + } + + assertEquals(data, mapOf("pet" to mapOf("greeting" to "Hello, Rex"))) + assertEquals(invocations, listOf("NamedGreetingApi.greeting")) + } + + @Test + fun `parser ignores JDK proxied supertype resolver methods when a more specific resolver takes precedence`() { + val gql = graphQL(petSchema, PetQuery(), PetResolver(), jdkProxy(ClassOnlyNamedResolver())) + + val data = assertNoGraphQlErrors(gql) { + """ + query { + pet { + greeting + } + } + """ + } + + assertEquals(data, mapOf("pet" to mapOf("greeting" to "Hello, Rex"))) + } + + interface Named { + val name: String + } + + class Pet(override val name: String) : Named + + class PetQuery : GraphQLQueryResolver { + fun pet() = Pet("Rex") + } + + interface GreetingApi { + fun greeting(pet: Pet): String + } + + class PetResolver : GraphQLResolver, GreetingApi { + override fun greeting(pet: Pet) = "Hello, ${pet.name}" + } + + interface NamedGreetingApi { + fun greeting(named: Named): String + } + + class NamedResolver : GraphQLResolver, NamedGreetingApi { + override fun greeting(named: Named) = "Hello, ${named.name}" + } + + class ClassOnlyNamedResolver : GraphQLResolver { + fun greeting(named: Named) = "Hi, ${named.name}" + } + + @Test + fun `JDK proxied resolver methods implementing a generic interface are called through the interface the proxy implements`() { + val gql = graphQL( + """ + type Query { + echo(value: String!): String + increment(value: Int!): Int + } + """, + jdkProxy(GenericQuery())) + + val data = assertNoGraphQlErrors(gql) { + """ + query { + echo(value: "ok") + increment(value: 1) + } + """ + } + + assertEquals(data, mapOf("echo" to "ok", "increment" to 2)) + assertEquals(invocations, listOf("EchoApi.echo", "IncrementApi.increment")) + } + + interface EchoApi { + fun echo(value: T): T + } + + interface IncrementApi { + fun increment(value: T): T + } + + // compiled to echo(String) and increment(int), which the interfaces' echo(Object) and increment(Object) are bridged to + class GenericQuery : GraphQLQueryResolver, EchoApi, IncrementApi { + override fun echo(value: String) = value + override fun increment(value: Int) = value + 1 + } + + @Test + fun `JDK proxied resolver methods declared on a non-public interface are called through the interface the proxy implements`() { + val gql = graphQL( + """ + type Query { + test: String + } + """, + jdkProxy(PrivateInterfaceQuery())) + + val data = assertNoGraphQlErrors(gql) { + """ + query { + test + } + """ + } + + assertEquals(data, mapOf("test" to "ok")) + assertEquals(invocations, listOf("PrivateQueryApi.test")) + } + + private interface PrivateQueryApi { + fun test(): String + } + + private class PrivateInterfaceQuery : GraphQLQueryResolver, PrivateQueryApi { + override fun test() = "ok" + } + + @Test + fun `JDK proxied resolver static methods are called directly`() { + val gql = graphQL( + """ + type Query { + test: String + } + """, + jdkProxy(StaticQuery())) + + val data = assertNoGraphQlErrors(gql) { + """ + query { + test + } + """ + } + + assertEquals(data, mapOf("test" to "static")) + } + + class StaticQuery : GraphQLQueryResolver { + companion object { + @JvmStatic + fun test() = "static" + } + } + + @Test + fun `parser should throw descriptive exception when a JDK proxied resolver method isn't declared on an interface`() { + val resolver = jdkProxy(ClassOnlyQuery()) + + val error = assertThrows(FieldResolverError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + test: String + } + """) + .resolvers(resolver) + .build() + } + + val queryClass = ClassOnlyQuery::class.java.name + assertEquals(error.message, "Resolver method '$queryClass.test' for field 'test' as defined in schema :3 can't be called on " + + "${resolver.javaClass.name}, which is not an instance of $queryClass. This happens with JDK dynamic proxies, which only implement " + + "interfaces. Declare the method on an interface the resolver implements, or use class-based proxies (e.g. proxyTargetClass = true in Spring).") + } + + class ClassOnlyQuery : GraphQLQueryResolver { + fun test() = "ok" + } + + @Test + fun `class based proxied resolver methods are called through the proxy`() { + val resolver = ProxyFactory(OpenQuery()).apply { + isProxyTargetClass = true + addAdvice(recordingInterceptor) + }.proxy as GraphQLQueryResolver + + val gql = graphQL( + """ + type Query { + test: String + } + """, + resolver) + + val data = assertNoGraphQlErrors(gql) { + """ + query { + test + } + """ + } + + assertEquals(data, mapOf("test" to "ok")) + assertEquals(invocations, listOf("OpenQuery.test")) + } + + // CGLIB can only intercept methods it can override + open class OpenQuery : GraphQLQueryResolver { + open fun test() = "ok" + } + + private fun jdkProxy(resolver: GraphQLResolver<*>, interceptor: MethodInterceptor = recordingInterceptor) = + (ProxyFactory(resolver).apply { addAdvice(interceptor) }.proxy as GraphQLResolver<*>) + .also { assert(Proxy.isProxyClass(it.javaClass)) } + + private fun graphQL(schema: String, vararg resolvers: GraphQLResolver<*>) = GraphQL.newGraphQL( + SchemaParser.newParser() + .schemaString(schema) + .resolvers(*resolvers) + .build() + .makeExecutableSchema()) + .build() +} diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt index e5e2ba30..e4d7835f 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt @@ -14,7 +14,6 @@ import org.junit.Before import org.junit.Test import org.reactivestreams.Publisher import org.reactivestreams.tck.TestEnvironment -import org.springframework.aop.framework.ProxyFactory import java.io.FileNotFoundException import java.util.concurrent.CompletableFuture import java.util.concurrent.CompletableFuture.completedFuture @@ -220,21 +219,6 @@ class SchemaParserTest { } } - @Test - fun `parser handles spring AOP proxied resolvers by default`() { - val resolver = ProxyFactory(ProxiedResolver()).proxy as GraphQLQueryResolver - - SchemaParser.newParser() - .schemaString( - """ - type Query { - test: [String] - } - """) - .resolvers(resolver) - .build() - } - @Test fun `parser handles enums with overridden toString method`() { SchemaParser.newParser() @@ -702,10 +686,6 @@ class SchemaParserTest { class RootObj - class ProxiedResolver : GraphQLQueryResolver { - fun test(): List = listOf() - } - enum class CustomEnum { FOO { override fun toString(): String { From f5665c3052149efc8ef9f311c104c277b13dddb3 Mon Sep 17 00:00:00 2001 From: Oryan Date: Mon, 12 Oct 2026 15:38:32 -0400 Subject: [PATCH 2/7] Match JDK proxied resolver methods inherited from a generic superclass A resolver method inherited from a generic superclass, e.g. getValue(T) of FooResolver, is erased to getValue(Object), so it didn't match the interface method it implements and build() threw the new FieldResolverError for a supported resolver pattern. Its parameter types are now resolved against the target class too, like the interface's. Also covers the guards that skip interfaces the proxy doesn't implement and static interface methods. Co-Authored-By: Claude Opus 5.5 --- .../tools/resolver/FieldResolverScanner.kt | 9 +- .../kickstart/tools/ProxiedResolverTest.kt | 130 ++++++++++++++++-- 2 files changed, 121 insertions(+), 18 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt index 681f10f1..4b2e3bc5 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt @@ -93,14 +93,17 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { } // the implementation of a generic interface method takes the type arguments, e.g. echo(String) for echo(T) of Api, - // and the compiler bridges the erased echo(Object) to it. Kotlin also unboxes them, e.g. increment(int) for Api + // and the compiler bridges the erased echo(Object) to it. Kotlin also unboxes them, e.g. increment(int) for Api. + // A method inherited from a generic superclass is erased itself, e.g. getValue(Object) for getValue(T), so both sides are + // resolved against the target class private fun Method.implements(interfaceMethod: Method, type: Class<*>): Boolean = name == interfaceMethod.name && parameterCount == interfaceMethod.parameterCount && parameterTypes.indices.all { i -> val interfaceParameterType = interfaceMethod.genericParameterTypes[i] - val resolvedType = TypeUtils.getRawType(interfaceParameterType, type) ?: interfaceMethod.parameterTypes[i] - resolvedType == if (interfaceParameterType is TypeVariable<*>) ClassUtils.primitiveToWrapper(parameterTypes[i]) else parameterTypes[i] + val resolvedInterfaceType = TypeUtils.getRawType(interfaceParameterType, type) ?: interfaceMethod.parameterTypes[i] + val resolvedType = TypeUtils.getRawType(genericParameterTypes[i], type) ?: parameterTypes[i] + resolvedInterfaceType == if (interfaceParameterType is TypeVariable<*>) ClassUtils.primitiveToWrapper(resolvedType) else resolvedType } private fun trySetAccessible(field: FieldDefinition, type: JavaType): AccessibleObject.() -> Unit = { diff --git a/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt index a7ddcb16..3c11fd1b 100644 --- a/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt @@ -241,6 +241,60 @@ class ProxiedResolverTest { override fun increment(value: Int) = value + 1 } + @Test + fun `JDK proxied field resolver methods inherited from a generic superclass are called through the interface the proxy implements`() { + val gql = graphQL(petSchema, PetQuery(), jdkProxy(InheritedPetResolver())) + + val data = assertNoGraphQlErrors(gql) { + """ + query { + pet { + greeting + } + } + """ + } + + assertEquals(data, mapOf("pet" to mapOf("greeting" to "Hello, Rex"))) + assertEquals(invocations, listOf("GenericGreetingApi.greeting")) + } + + interface GenericGreetingApi { + fun greeting(pet: T): String + } + + abstract class GenericGreetingResolver : GraphQLResolver, GenericGreetingApi { + override fun greeting(pet: T) = "Hello, ${pet.name}" + } + + // inherits the erased greeting(Named) + class InheritedPetResolver : GenericGreetingResolver() + + @Test + fun `JDK proxied field resolver methods inherited from a generic superclass are called through a non-generic interface the proxy implements`() { + val gql = graphQL(petSchema, PetQuery(), jdkProxy(MixedPetResolver())) + + val data = assertNoGraphQlErrors(gql) { + """ + query { + pet { + greeting + } + } + """ + } + + assertEquals(data, mapOf("pet" to mapOf("greeting" to "Hello, Rex"))) + assertEquals(invocations, listOf("GreetingApi.greeting")) + } + + abstract class PlainGreetingResolver : GraphQLResolver { + fun greeting(pet: T) = "Hello, ${pet.name}" + } + + // the inherited greeting(Named) implements GreetingApi.greeting(Pet) through a synthetic bridge, which isn't scanned + class MixedPetResolver : PlainGreetingResolver(), GreetingApi + @Test fun `JDK proxied resolver methods declared on a non-public interface are called through the interface the proxy implements`() { val gql = graphQL( @@ -304,15 +358,13 @@ class ProxiedResolverTest { val resolver = jdkProxy(ClassOnlyQuery()) val error = assertThrows(FieldResolverError::class.java) { - SchemaParser.newParser() - .schemaString( - """ - type Query { - test: String - } - """) - .resolvers(resolver) - .build() + parse( + """ + type Query { + test: String + } + """, + resolver) } val queryClass = ClassOnlyQuery::class.java.name @@ -325,6 +377,54 @@ class ProxiedResolverTest { fun test() = "ok" } + @Test + fun `parser should throw descriptive exception when a JDK proxied resolver method is only declared on an interface the proxy doesn't implement`() { + // the target implements QueryApi, but the proxy only exposes the interfaces it's given + val resolver = ProxyFactory().apply { + setTarget(Query()) + setInterfaces(GraphQLQueryResolver::class.java) + }.proxy as GraphQLQueryResolver + + val error = assertThrows(FieldResolverError::class.java) { + parse( + """ + type Query { + test: String + } + """, + resolver) + } + + assertEquals(error.message?.substringBefore(" as defined"), "Resolver method '${Query::class.java.name}.test' for field 'test'") + } + + @Test + fun `parser should throw descriptive exception when a JDK proxied resolver method is only declared as a static interface method`() { + val error = assertThrows(FieldResolverError::class.java) { + parse( + """ + type Query { + test: String + } + """, + jdkProxy(StaticInterfaceQuery())) + } + + assertEquals(error.message?.substringBefore(" as defined"), "Resolver method '${StaticInterfaceQuery::class.java.name}.test' for field 'test'") + } + + interface StaticApi { + companion object { + @JvmStatic + fun test() = "static" + } + } + + // calling the static StaticApi.test() would skip both the proxy and the resolver's own test() + class StaticInterfaceQuery : GraphQLQueryResolver, StaticApi { + fun test() = "ok" + } + @Test fun `class based proxied resolver methods are called through the proxy`() { val resolver = ProxyFactory(OpenQuery()).apply { @@ -361,11 +461,11 @@ class ProxiedResolverTest { (ProxyFactory(resolver).apply { addAdvice(interceptor) }.proxy as GraphQLResolver<*>) .also { assert(Proxy.isProxyClass(it.javaClass)) } - private fun graphQL(schema: String, vararg resolvers: GraphQLResolver<*>) = GraphQL.newGraphQL( - SchemaParser.newParser() - .schemaString(schema) - .resolvers(*resolvers) - .build() - .makeExecutableSchema()) + private fun graphQL(schema: String, vararg resolvers: GraphQLResolver<*>) = + GraphQL.newGraphQL(parse(schema, *resolvers).makeExecutableSchema()).build() + + private fun parse(schema: String, vararg resolvers: GraphQLResolver<*>) = SchemaParser.newParser() + .schemaString(schema) + .resolvers(*resolvers) .build() } From b0ccd5b51b37693c0c619e3129491a7338e519da Mon Sep 17 00:00:00 2001 From: Oryan Date: Sat, 17 Oct 2026 11:59:55 -0400 Subject: [PATCH 3/7] Fold the proxied resolver tests and tidy the shadowed resolver search Co-Authored-By: Claude Opus 5.5 --- .../tools/resolver/FieldResolverScanner.kt | 19 +- .../kickstart/tools/ProxiedResolverTest.kt | 407 +++--------------- 2 files changed, 76 insertions(+), 350 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt index 8a62beca..cf9ed907 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt @@ -34,11 +34,12 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { val searches = resolverInfo.getFieldSearches() val scanProperties = field.inputValueDefinitions.isEmpty() + // root fields must match a single resolver. Other fields use the first match, so the resolvers it shadows, e.g. for a + // supertype, are never called and aren't checked val found = searches.asSequence() .mapNotNull { search -> findFieldResolver(field, search, scanProperties) } - // only root fields must match a single resolver. Other fields use the first match, so the methods it shadows, - // e.g. a supertype resolver's, are never called and aren't checked - .let { if (resolverInfo is RootResolverInfo) it.toList() else it.take(1).toList() } + .let { if (resolverInfo is RootResolverInfo) it else it.take(1) } + .toList() if (resolverInfo is RootResolverInfo && found.size > 1) { throw FieldResolverError("Found more than one matching resolver for field '$field': $found") @@ -81,7 +82,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { val type = search.type.unwrap() return ClassUtils.getAllInterfaces(type) .filter { it.isInstance(source) } - .flatMap { it.methods.asList() } + .flatMap { it.methods.toList() } .find { !Modifier.isStatic(it.modifiers) && method.implements(it, type) } ?.apply(trySetAccessible(field, search.type)) ?: throw FieldResolverError( @@ -97,9 +98,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { // A method inherited from a generic superclass is erased itself, e.g. getValue(Object) for getValue(T), so both sides are // resolved against the target class private fun Method.implements(interfaceMethod: Method, type: Class<*>): Boolean = - name == interfaceMethod.name - && parameterCount == interfaceMethod.parameterCount - && parameterTypes.indices.all { i -> + name == interfaceMethod.name && parameterCount == interfaceMethod.parameterCount && parameterTypes.indices.all { i -> val interfaceParameterType = interfaceMethod.genericParameterTypes[i] val resolvedInterfaceType = TypeUtils.getRawType(interfaceParameterType, type) ?: interfaceMethod.parameterTypes[i] val resolvedType = TypeUtils.getRawType(genericParameterTypes[i], type) ?: parameterTypes[i] @@ -248,10 +247,8 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { if (isSubscription) "\n\nNote that a Subscription data fetcher must return a Publisher of events" else "" } - private fun getSourceLocation(field: FieldDefinition): String { - val sourceName = field.sourceLocation?.sourceName ?: "" - return field.sourceLocation?.let { "$sourceName:${it.line}" } ?: "" - } + private fun getSourceLocation(field: FieldDefinition): String = + field.sourceLocation?.let { "${it.sourceName ?: ""}:${it.line}" } ?: "" private fun getMissingMethodSignatures(field: FieldDefinition, search: Search, isBoolean: Boolean, scannedProperties: Boolean): List { val baseType = search.type.unwrap() diff --git a/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt index 3c11fd1b..f33539b1 100644 --- a/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt @@ -21,84 +21,57 @@ class ProxiedResolverTest { invocation.proceed() } - private val querySchema = """ - type Query { - test: String - greeting(name: String!): String - suspendTest: String - } - """ - @Test fun `JDK proxied resolver methods are called through the interface the proxy implements`() { - val gql = graphQL(querySchema, jdkProxy(Query())) - - val data = assertNoGraphQlErrors(gql) { + val gql = graphQL( """ - query { - test + type Query { + test: String + greeting(name: String!): String + suspendTest: String + increment(value: Int!): Int + staticTest: String } - """ - } - - assertEquals(data, mapOf("test" to "ok")) - assertEquals(invocations, listOf("QueryApi.test")) - } - - @Test - fun `JDK proxied resolver methods with arguments are called through the interface the proxy implements`() { - val gql = graphQL(querySchema, jdkProxy(Query())) + """, + jdkProxy(Query())) val data = assertNoGraphQlErrors(gql) { """ query { + test greeting(name: "Rex") - } - """ - } - - assertEquals(data, mapOf("greeting" to "Hello, Rex, from greeting")) - assertEquals(invocations, listOf("QueryApi.greeting")) - } - - @Test - fun `JDK proxied resolver suspend functions are called through the interface the proxy implements`() { - val gql = graphQL(querySchema, jdkProxy(Query())) - - val data = assertNoGraphQlErrors(gql) { - """ - query { suspendTest + increment(value: 1) + staticTest } """ } - assertEquals(data, mapOf("suspendTest" to "ok")) - assertEquals(invocations, listOf("QueryApi.suspendTest")) + assertEquals(data, mapOf("test" to "ok", "greeting" to "Hello, Rex, from greeting", "suspendTest" to "ok", "increment" to 2, "staticTest" to "static")) + // static methods are called directly, so they skip the interceptors + assertEquals(invocations, listOf("QueryApi.test", "QueryApi.greeting", "QueryApi.suspendTest", "QueryApi.increment")) } @Test fun `JDK proxied resolver interceptors can reject the call`() { - val gql = graphQL(querySchema, jdkProxy(Query()) { throw IllegalStateException("Access denied") }) + val gql = graphQL("type Query { test: String }", jdkProxy(Query()) { throw IllegalStateException("Access denied") }) - val result = gql.execute( - """ - query { - test - } - """) + val result = gql.execute("query { test }") assertEquals(result.getData(), mapOf("test" to null)) assertEquals(result.errors.map { (it as ExceptionWhileDataFetching).exception.message }, listOf("Access denied")) } - interface QueryApi { + // not public, so its methods have to be made accessible + private interface QueryApi { fun test(): String fun greeting(name: String, env: DataFetchingEnvironment): String suspend fun suspendTest(): String + fun increment(value: T): T } - class Query : GraphQLQueryResolver, QueryApi { + // increment(Int) is compiled to increment(int), which the interface's increment(Object) is bridged to + private class Query : GraphQLQueryResolver, QueryApi { override fun test() = "ok" override fun greeting(name: String, env: DataFetchingEnvironment) = "Hello, $name, from ${env.field.name}" override suspend fun suspendTest(): String { @@ -106,311 +79,73 @@ class ProxiedResolverTest { delay(1) return "ok" } - } - - private val petSchema = """ - type Query { - pet: Pet - } - - type Pet { - name: String - greeting: String - } - """ - - @Test - fun `JDK proxied field resolver methods are called through the interface the proxy implements`() { - val gql = graphQL(petSchema, PetQuery(), jdkProxy(PetResolver())) - - val data = assertNoGraphQlErrors(gql) { - """ - query { - pet { - name - greeting - } - } - """ - } - - assertEquals(data, mapOf("pet" to mapOf("name" to "Rex", "greeting" to "Hello, Rex"))) - assertEquals(invocations, listOf("GreetingApi.greeting")) - } - - @Test - fun `JDK proxied supertype resolver methods are called through the interface the proxy implements`() { - val gql = graphQL(petSchema, PetQuery(), jdkProxy(NamedResolver())) - - val data = assertNoGraphQlErrors(gql) { - """ - query { - pet { - greeting - } - } - """ - } - - assertEquals(data, mapOf("pet" to mapOf("greeting" to "Hello, Rex"))) - assertEquals(invocations, listOf("NamedGreetingApi.greeting")) - } - - @Test - fun `parser ignores JDK proxied supertype resolver methods when a more specific resolver takes precedence`() { - val gql = graphQL(petSchema, PetQuery(), PetResolver(), jdkProxy(ClassOnlyNamedResolver())) - - val data = assertNoGraphQlErrors(gql) { - """ - query { - pet { - greeting - } - } - """ - } - - assertEquals(data, mapOf("pet" to mapOf("greeting" to "Hello, Rex"))) - } - - interface Named { - val name: String - } - - class Pet(override val name: String) : Named - - class PetQuery : GraphQLQueryResolver { - fun pet() = Pet("Rex") - } - - interface GreetingApi { - fun greeting(pet: Pet): String - } - - class PetResolver : GraphQLResolver, GreetingApi { - override fun greeting(pet: Pet) = "Hello, ${pet.name}" - } - - interface NamedGreetingApi { - fun greeting(named: Named): String - } - - class NamedResolver : GraphQLResolver, NamedGreetingApi { - override fun greeting(named: Named) = "Hello, ${named.name}" - } - - class ClassOnlyNamedResolver : GraphQLResolver { - fun greeting(named: Named) = "Hi, ${named.name}" - } - - @Test - fun `JDK proxied resolver methods implementing a generic interface are called through the interface the proxy implements`() { - val gql = graphQL( - """ - type Query { - echo(value: String!): String - increment(value: Int!): Int - } - """, - jdkProxy(GenericQuery())) + override fun increment(value: Int) = value + 1 - val data = assertNoGraphQlErrors(gql) { - """ - query { - echo(value: "ok") - increment(value: 1) - } - """ + companion object { + @JvmStatic + fun staticTest() = "static" } - - assertEquals(data, mapOf("echo" to "ok", "increment" to 2)) - assertEquals(invocations, listOf("EchoApi.echo", "IncrementApi.increment")) - } - - interface EchoApi { - fun echo(value: T): T - } - - interface IncrementApi { - fun increment(value: T): T - } - - // compiled to echo(String) and increment(int), which the interfaces' echo(Object) and increment(Object) are bridged to - class GenericQuery : GraphQLQueryResolver, EchoApi, IncrementApi { - override fun echo(value: String) = value - override fun increment(value: Int) = value + 1 } @Test - fun `JDK proxied field resolver methods inherited from a generic superclass are called through the interface the proxy implements`() { - val gql = graphQL(petSchema, PetQuery(), jdkProxy(InheritedPetResolver())) - - val data = assertNoGraphQlErrors(gql) { - """ - query { - pet { - greeting - } - } - """ - } + fun `JDK proxied field resolver methods are called through the interface the proxy implements`() { + val dog = mapOf("name" to "Rex", "greeting" to "Hello, Rex") - assertEquals(data, mapOf("pet" to mapOf("greeting" to "Hello, Rex"))) - assertEquals(invocations, listOf("GenericGreetingApi.greeting")) - } + // resolvers for the data class and for its supertype + assertEquals(queryDog(jdkProxy(LabradorResolver())), dog) + assertEquals(queryDog(jdkProxy(DogResolver())), dog) + // greeting(Dog) is inherited from a class that doesn't implement the interface + assertEquals(queryDog(jdkProxy(BridgedLabradorResolver())), dog) + // LabradorResolver takes precedence, so the supertype resolver's class-only greeting isn't checked + assertEquals(queryDog(jdkProxy(LabradorResolver()), jdkProxy(ClassOnlyDogResolver())), dog) - interface GenericGreetingApi { - fun greeting(pet: T): String + assertEquals(invocations, listOf("GreetingApi.greeting", "GreetingApi.greeting", "LabradorGreetingApi.greeting", "GreetingApi.greeting")) } - abstract class GenericGreetingResolver : GraphQLResolver, GenericGreetingApi { - override fun greeting(pet: T) = "Hello, ${pet.name}" + class DogQuery : GraphQLQueryResolver { + fun dog() = LabradorRetriever("Rex", "black", 3, 1f) } - // inherits the erased greeting(Named) - class InheritedPetResolver : GenericGreetingResolver() - - @Test - fun `JDK proxied field resolver methods inherited from a generic superclass are called through a non-generic interface the proxy implements`() { - val gql = graphQL(petSchema, PetQuery(), jdkProxy(MixedPetResolver())) - - val data = assertNoGraphQlErrors(gql) { - """ - query { - pet { - greeting - } - } - """ - } - - assertEquals(data, mapOf("pet" to mapOf("greeting" to "Hello, Rex"))) - assertEquals(invocations, listOf("GreetingApi.greeting")) + interface GreetingApi { + fun greeting(dog: T): String } - abstract class PlainGreetingResolver : GraphQLResolver { - fun greeting(pet: T) = "Hello, ${pet.name}" + // greeting(T) is erased to greeting(Dog), which the interface's greeting(Object) is bridged to + abstract class GreetingResolver : GraphQLResolver, GreetingApi { + override fun greeting(dog: T) = "Hello, ${dog.name}" } - // the inherited greeting(Named) implements GreetingApi.greeting(Pet) through a synthetic bridge, which isn't scanned - class MixedPetResolver : PlainGreetingResolver(), GreetingApi - - @Test - fun `JDK proxied resolver methods declared on a non-public interface are called through the interface the proxy implements`() { - val gql = graphQL( - """ - type Query { - test: String - } - """, - jdkProxy(PrivateInterfaceQuery())) - - val data = assertNoGraphQlErrors(gql) { - """ - query { - test - } - """ - } + class LabradorResolver : GreetingResolver() - assertEquals(data, mapOf("test" to "ok")) - assertEquals(invocations, listOf("PrivateQueryApi.test")) - } + class DogResolver : GreetingResolver() - private interface PrivateQueryApi { - fun test(): String + abstract class ClassOnlyGreetingResolver : GraphQLResolver { + fun greeting(dog: T) = "Hello, ${dog.name}" } - private class PrivateInterfaceQuery : GraphQLQueryResolver, PrivateQueryApi { - override fun test() = "ok" + interface LabradorGreetingApi { + fun greeting(dog: LabradorRetriever): String } - @Test - fun `JDK proxied resolver static methods are called directly`() { - val gql = graphQL( - """ - type Query { - test: String - } - """, - jdkProxy(StaticQuery())) - - val data = assertNoGraphQlErrors(gql) { - """ - query { - test - } - """ - } - - assertEquals(data, mapOf("test" to "static")) - } + // the inherited greeting(Dog) implements LabradorGreetingApi.greeting(LabradorRetriever) through a synthetic bridge, which isn't scanned + class BridgedLabradorResolver : ClassOnlyGreetingResolver(), LabradorGreetingApi - class StaticQuery : GraphQLQueryResolver { - companion object { - @JvmStatic - fun test() = "static" - } - } + class ClassOnlyDogResolver : ClassOnlyGreetingResolver() @Test - fun `parser should throw descriptive exception when a JDK proxied resolver method isn't declared on an interface`() { + fun `parser should throw descriptive exception when a JDK proxied resolver method can't be called through the proxy`() { val resolver = jdkProxy(ClassOnlyQuery()) - - val error = assertThrows(FieldResolverError::class.java) { - parse( - """ - type Query { - test: String - } - """, - resolver) - } - val queryClass = ClassOnlyQuery::class.java.name - assertEquals(error.message, "Resolver method '$queryClass.test' for field 'test' as defined in schema :3 can't be called on " + + assertEquals(buildError(resolver), "Resolver method '$queryClass.test' for field 'test' as defined in schema :1 can't be called on " + "${resolver.javaClass.name}, which is not an instance of $queryClass. This happens with JDK dynamic proxies, which only implement " + "interfaces. Declare the method on an interface the resolver implements, or use class-based proxies (e.g. proxyTargetClass = true in Spring).") - } - - class ClassOnlyQuery : GraphQLQueryResolver { - fun test() = "ok" - } - @Test - fun `parser should throw descriptive exception when a JDK proxied resolver method is only declared on an interface the proxy doesn't implement`() { - // the target implements QueryApi, but the proxy only exposes the interfaces it's given - val resolver = ProxyFactory().apply { + // the target implements QueryApi, but the proxy only implements the interfaces it's given + val hidingProxy = ProxyFactory().apply { setTarget(Query()) setInterfaces(GraphQLQueryResolver::class.java) }.proxy as GraphQLQueryResolver - - val error = assertThrows(FieldResolverError::class.java) { - parse( - """ - type Query { - test: String - } - """, - resolver) - } - - assertEquals(error.message?.substringBefore(" as defined"), "Resolver method '${Query::class.java.name}.test' for field 'test'") - } - - @Test - fun `parser should throw descriptive exception when a JDK proxied resolver method is only declared as a static interface method`() { - val error = assertThrows(FieldResolverError::class.java) { - parse( - """ - type Query { - test: String - } - """, - jdkProxy(StaticInterfaceQuery())) - } - - assertEquals(error.message?.substringBefore(" as defined"), "Resolver method '${StaticInterfaceQuery::class.java.name}.test' for field 'test'") + assertEquals(buildError(hidingProxy)?.substringBefore(" as defined"), "Resolver method '${Query::class.java.name}.test' for field 'test'") } interface StaticApi { @@ -420,8 +155,8 @@ class ProxiedResolverTest { } } - // calling the static StaticApi.test() would skip both the proxy and the resolver's own test() - class StaticInterfaceQuery : GraphQLQueryResolver, StaticApi { + // calling the static StaticApi.test() instead would skip both the proxy and the resolver's own test() + class ClassOnlyQuery : GraphQLQueryResolver, StaticApi { fun test() = "ok" } @@ -432,21 +167,7 @@ class ProxiedResolverTest { addAdvice(recordingInterceptor) }.proxy as GraphQLQueryResolver - val gql = graphQL( - """ - type Query { - test: String - } - """, - resolver) - - val data = assertNoGraphQlErrors(gql) { - """ - query { - test - } - """ - } + val data = assertNoGraphQlErrors(graphQL("type Query { test: String }", resolver)) { "query { test }" } assertEquals(data, mapOf("test" to "ok")) assertEquals(invocations, listOf("OpenQuery.test")) @@ -461,6 +182,14 @@ class ProxiedResolverTest { (ProxyFactory(resolver).apply { addAdvice(interceptor) }.proxy as GraphQLResolver<*>) .also { assert(Proxy.isProxyClass(it.javaClass)) } + private fun queryDog(vararg resolvers: GraphQLResolver<*>) = + assertNoGraphQlErrors(graphQL("type Query { dog: Dog } type Dog { name: String greeting: String }", DogQuery(), *resolvers)) { + "query { dog { name greeting } }" + }["dog"] + + private fun buildError(resolver: GraphQLResolver<*>) = + assertThrows(FieldResolverError::class.java) { parse("type Query { test: String }", resolver) }.message + private fun graphQL(schema: String, vararg resolvers: GraphQLResolver<*>) = GraphQL.newGraphQL(parse(schema, *resolvers).makeExecutableSchema()).build() From 2e8d3e5bac3c12a73621d58d0da99765c36bf2a4 Mon Sep 17 00:00:00 2001 From: Oryan Date: Sat, 17 Oct 2026 12:18:35 -0400 Subject: [PATCH 4/7] Throw at build when the proxy would call another overload of the resolver method Co-Authored-By: Claude Opus 5.5 --- .../tools/resolver/FieldResolverScanner.kt | 7 ++++++- .../kickstart/tools/ProxiedResolverTest.kt | 20 +++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt index cf9ed907..f40b9840 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt @@ -83,7 +83,12 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { return ClassUtils.getAllInterfaces(type) .filter { it.isInstance(source) } .flatMap { it.methods.toList() } - .find { !Modifier.isStatic(it.modifiers) && method.implements(it, type) } + // the proxy calls whichever method of the target class implements the interface method, so that has to be this method + // and not an overload, e.g. test(int) next to test(Integer) for Api.test(T) + .find { interfaceMethod -> + !Modifier.isStatic(interfaceMethod.modifiers) + && type.methods.filter { !it.isBridge && it.implements(interfaceMethod, type) } == listOf(method) + } ?.apply(trySetAccessible(field, search.type)) ?: throw FieldResolverError( "Resolver method '${method.declaringClass.name}.${method.name}' for field '${field.name}' as defined in schema ${getSourceLocation(field)} " + diff --git a/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt index f33539b1..4c48b2a2 100644 --- a/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt @@ -146,6 +146,10 @@ class ProxiedResolverTest { setInterfaces(GraphQLQueryResolver::class.java) }.proxy as GraphQLQueryResolver assertEquals(buildError(hidingProxy)?.substringBefore(" as defined"), "Resolver method '${Query::class.java.name}.test' for field 'test'") + + // test(Integer) is scanned, but the proxy would call test(int), which implements OverloadApi.test(T) + assertEquals(buildError(jdkProxy(OverloadQuery()), "type Query { test(value: Int!): String }")?.substringBefore(" as defined"), + "Resolver method '${OverloadQuery::class.java.name}.test' for field 'test'") } interface StaticApi { @@ -160,6 +164,18 @@ class ProxiedResolverTest { fun test() = "ok" } + interface OverloadApi { + fun test(value: T): String + } + + abstract class OverloadBase : OverloadApi { + override fun test(value: Int) = "override" + } + + class OverloadQuery : OverloadBase(), GraphQLQueryResolver { + fun test(value: Int?) = "overload" + } + @Test fun `class based proxied resolver methods are called through the proxy`() { val resolver = ProxyFactory(OpenQuery()).apply { @@ -187,8 +203,8 @@ class ProxiedResolverTest { "query { dog { name greeting } }" }["dog"] - private fun buildError(resolver: GraphQLResolver<*>) = - assertThrows(FieldResolverError::class.java) { parse("type Query { test: String }", resolver) }.message + private fun buildError(resolver: GraphQLResolver<*>, schema: String = "type Query { test: String }") = + assertThrows(FieldResolverError::class.java) { parse(schema, resolver) }.message private fun graphQL(schema: String, vararg resolvers: GraphQLResolver<*>) = GraphQL.newGraphQL(parse(schema, *resolvers).makeExecutableSchema()).build() From 44e393200ccef4259d3907ea60dc3132abdec1b0 Mon Sep 17 00:00:00 2001 From: Oryan Date: Sat, 17 Oct 2026 12:19:23 -0400 Subject: [PATCH 5/7] Make the interface method matching easier to read Co-Authored-By: Claude Opus 5.5 --- .../tools/resolver/FieldResolverScanner.kt | 31 ++++++++++++------- .../kickstart/tools/ProxiedResolverTest.kt | 2 +- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt index f40b9840..9b86cb37 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt @@ -51,7 +51,8 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { private fun findFieldResolver(field: FieldDefinition, search: Search, scanProperties: Boolean): FieldResolver? { val method = findResolverMethod(field, search) if (method != null) { - return MethodFieldResolver(field, search, options, method.apply(trySetAccessible(field, search.type)), findInvocableMethod(field, search, method)) + val invocableMethod = findInvocableMethod(field, search, method) + return MethodFieldResolver(field, search, options, method.apply(trySetAccessible(field, search.type)), invocableMethod) } if (scanProperties) { @@ -79,15 +80,15 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { return method } - val type = search.type.unwrap() - return ClassUtils.getAllInterfaces(type) + val targetClass = search.type.unwrap() + return ClassUtils.getAllInterfaces(targetClass) .filter { it.isInstance(source) } .flatMap { it.methods.toList() } // the proxy calls whichever method of the target class implements the interface method, so that has to be this method // and not an overload, e.g. test(int) next to test(Integer) for Api.test(T) .find { interfaceMethod -> !Modifier.isStatic(interfaceMethod.modifiers) - && type.methods.filter { !it.isBridge && it.implements(interfaceMethod, type) } == listOf(method) + && targetClass.methods.filter { !it.isBridge && it.implements(interfaceMethod, targetClass) } == listOf(method) } ?.apply(trySetAccessible(field, search.type)) ?: throw FieldResolverError( @@ -99,16 +100,22 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { } // the implementation of a generic interface method takes the type arguments, e.g. echo(String) for echo(T) of Api, - // and the compiler bridges the erased echo(Object) to it. Kotlin also unboxes them, e.g. increment(int) for Api. - // A method inherited from a generic superclass is erased itself, e.g. getValue(Object) for getValue(T), so both sides are - // resolved against the target class - private fun Method.implements(interfaceMethod: Method, type: Class<*>): Boolean = - name == interfaceMethod.name && parameterCount == interfaceMethod.parameterCount && parameterTypes.indices.all { i -> + // and the compiler bridges the erased echo(Object) to it. A method inherited from a generic superclass is erased itself, + // e.g. getValue(Object) for getValue(T), so both sides are resolved against the target class + private fun Method.implements(interfaceMethod: Method, targetClass: Class<*>): Boolean { + if (name != interfaceMethod.name || parameterCount != interfaceMethod.parameterCount) { + return false + } + + return parameterTypes.indices.all { i -> val interfaceParameterType = interfaceMethod.genericParameterTypes[i] - val resolvedInterfaceType = TypeUtils.getRawType(interfaceParameterType, type) ?: interfaceMethod.parameterTypes[i] - val resolvedType = TypeUtils.getRawType(genericParameterTypes[i], type) ?: parameterTypes[i] - resolvedInterfaceType == if (interfaceParameterType is TypeVariable<*>) ClassUtils.primitiveToWrapper(resolvedType) else resolvedType + val resolvedInterfaceType = TypeUtils.getRawType(interfaceParameterType, targetClass) ?: interfaceMethod.parameterTypes[i] + val resolvedType = (TypeUtils.getRawType(genericParameterTypes[i], targetClass) ?: parameterTypes[i]) + // Kotlin unboxes type arguments, e.g. increment(int) for Api.increment(T) + .let { if (interfaceParameterType is TypeVariable<*>) ClassUtils.primitiveToWrapper(it) else it } + resolvedType == resolvedInterfaceType } + } private fun trySetAccessible(field: FieldDefinition, type: JavaType): AccessibleObject.() -> Unit = { try { diff --git a/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt index 4c48b2a2..20e5ffc8 100644 --- a/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt @@ -22,7 +22,7 @@ class ProxiedResolverTest { } @Test - fun `JDK proxied resolver methods are called through the interface the proxy implements`() { + fun `JDK proxied resolver methods are called through the proxy's interfaces and static methods directly`() { val gql = graphQL( """ type Query { From 76b67394a3bd45402a9116f05c6cbf310a5a8513 Mon Sep 17 00:00:00 2001 From: Oryan Date: Sat, 17 Oct 2026 22:05:41 -0400 Subject: [PATCH 6/7] Warn at build instead of calling JDK proxied resolver methods through the proxy's interfaces Co-Authored-By: Claude --- .../kickstart/tools/SchemaClassScanner.kt | 24 ++ .../tools/resolver/FieldResolverScanner.kt | 66 +----- .../tools/resolver/MethodFieldResolver.kt | 8 +- .../tools/resolver/PropertyFieldResolver.kt | 2 +- .../kickstart/tools/ProxiedResolverTest.kt | 216 ------------------ .../kickstart/tools/SchemaParserTest.kt | 20 ++ 6 files changed, 54 insertions(+), 282 deletions(-) delete mode 100644 src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index edf356bb..82ddadc7 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -3,6 +3,8 @@ package graphql.kickstart.tools import com.fasterxml.jackson.databind.JsonNode import graphql.kickstart.tools.resolver.FieldResolver import graphql.kickstart.tools.resolver.FieldResolverScanner +import graphql.kickstart.tools.resolver.MethodFieldResolver +import graphql.kickstart.tools.resolver.PropertyFieldResolver import graphql.kickstart.tools.util.* import graphql.language.* import graphql.schema.GraphQLScalarType @@ -12,6 +14,7 @@ import org.apache.commons.lang3.reflect.TypeUtils import org.slf4j.LoggerFactory import java.lang.reflect.Member import java.lang.reflect.Method +import java.lang.reflect.Modifier import java.lang.reflect.ParameterizedType /** @@ -524,6 +527,7 @@ internal class SchemaClassScanner( private fun scanResolverInfoForPotentialMatches(type: ObjectTypeDefinition, resolverInfo: ResolverInfo) { type.withExtensions(typeExtensions).flatMap { it.fieldDefinitions }.forEach { field -> val fieldResolver = fieldResolverScanner.findFieldResolver(field, resolverInfo) + warnIfNotCallableOnResolver(type, fieldResolver) fieldResolversByType.getOrPut(type) { mutableMapOf() }[fieldResolver.field] = fieldResolver @@ -533,6 +537,26 @@ internal class SchemaClassScanner( } } + // a JDK dynamic proxy only implements its target's interfaces, so the target's own methods and fields can't be called on it + // TODO throw a FieldResolverError instead in 15.0.0 (#868) + private fun warnIfNotCallableOnResolver(type: ObjectTypeDefinition, fieldResolver: FieldResolver) { + val source = fieldResolver.search.source ?: return + val member: Member = when (fieldResolver) { + is MethodFieldResolver -> fieldResolver.method + is PropertyFieldResolver -> fieldResolver.property + else -> return + } + if (Modifier.isStatic(member.modifiers) || member.declaringClass.isInstance(source)) { + return + } + + log.warn( + "Field '${type.name}.${fieldResolver.field.name}' resolves to ${member.declaringClass.name}.${member.name}, but the resolver is a " + + "${source.javaClass.name}, which isn't a ${member.declaringClass.name}, so every query of the field will fail. This happens " + + "with JDK dynamic proxies, which only implement interfaces. Use class-based proxies instead, e.g. proxyTargetClass = true in Spring." + ) + } + private fun handleFoundType(match: TypeClassMatcher.Match) = when (match) { is TypeClassMatcher.ScalarMatch -> handleFoundScalarType(match.type) is TypeClassMatcher.ValidMatch -> handleFoundType(match.type, match.javaType, match.reference) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt index 9b86cb37..34e5b282 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt @@ -34,12 +34,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { val searches = resolverInfo.getFieldSearches() val scanProperties = field.inputValueDefinitions.isEmpty() - // root fields must match a single resolver. Other fields use the first match, so the resolvers it shadows, e.g. for a - // supertype, are never called and aren't checked - val found = searches.asSequence() - .mapNotNull { search -> findFieldResolver(field, search, scanProperties) } - .let { if (resolverInfo is RootResolverInfo) it else it.take(1) } - .toList() + val found = searches.mapNotNull { search -> findFieldResolver(field, search, scanProperties) } if (resolverInfo is RootResolverInfo && found.size > 1) { throw FieldResolverError("Found more than one matching resolver for field '$field': $found") @@ -51,8 +46,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { private fun findFieldResolver(field: FieldDefinition, search: Search, scanProperties: Boolean): FieldResolver? { val method = findResolverMethod(field, search) if (method != null) { - val invocableMethod = findInvocableMethod(field, search, method) - return MethodFieldResolver(field, search, options, method.apply(trySetAccessible(field, search.type)), invocableMethod) + return MethodFieldResolver(field, search, options, method.apply(trySetAccessible(field, search.type))) } if (scanProperties) { @@ -69,54 +63,6 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { return null } - /** - * A resolver behind a JDK dynamic proxy, e.g. Spring AOP without proxyTargetClass, is scanned as its target class, but the - * proxy only implements the target's interfaces. So its methods are called through the interface method they implement, - * which also runs the proxy's interceptors. - */ - private fun findInvocableMethod(field: FieldDefinition, search: Search, method: Method): Method { - val source = search.source - if (source == null || Modifier.isStatic(method.modifiers) || method.declaringClass.isInstance(source)) { - return method - } - - val targetClass = search.type.unwrap() - return ClassUtils.getAllInterfaces(targetClass) - .filter { it.isInstance(source) } - .flatMap { it.methods.toList() } - // the proxy calls whichever method of the target class implements the interface method, so that has to be this method - // and not an overload, e.g. test(int) next to test(Integer) for Api.test(T) - .find { interfaceMethod -> - !Modifier.isStatic(interfaceMethod.modifiers) - && targetClass.methods.filter { !it.isBridge && it.implements(interfaceMethod, targetClass) } == listOf(method) - } - ?.apply(trySetAccessible(field, search.type)) - ?: throw FieldResolverError( - "Resolver method '${method.declaringClass.name}.${method.name}' for field '${field.name}' as defined in schema ${getSourceLocation(field)} " + - "can't be called on ${source.javaClass.name}, which is not an instance of ${method.declaringClass.name}. " + - "This happens with JDK dynamic proxies, which only implement interfaces. " + - "Declare the method on an interface the resolver implements, or use class-based proxies (e.g. proxyTargetClass = true in Spring)." - ) - } - - // the implementation of a generic interface method takes the type arguments, e.g. echo(String) for echo(T) of Api, - // and the compiler bridges the erased echo(Object) to it. A method inherited from a generic superclass is erased itself, - // e.g. getValue(Object) for getValue(T), so both sides are resolved against the target class - private fun Method.implements(interfaceMethod: Method, targetClass: Class<*>): Boolean { - if (name != interfaceMethod.name || parameterCount != interfaceMethod.parameterCount) { - return false - } - - return parameterTypes.indices.all { i -> - val interfaceParameterType = interfaceMethod.genericParameterTypes[i] - val resolvedInterfaceType = TypeUtils.getRawType(interfaceParameterType, targetClass) ?: interfaceMethod.parameterTypes[i] - val resolvedType = (TypeUtils.getRawType(genericParameterTypes[i], targetClass) ?: parameterTypes[i]) - // Kotlin unboxes type arguments, e.g. increment(int) for Api.increment(T) - .let { if (interfaceParameterType is TypeVariable<*>) ClassUtils.primitiveToWrapper(it) else it } - resolvedType == resolvedInterfaceType - } - } - private fun trySetAccessible(field: FieldDefinition, type: JavaType): AccessibleObject.() -> Unit = { try { isAccessible = true @@ -254,14 +200,14 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { isSubscription = isSubscription || search.isSubscription } - return "No method${if (scannedProperties) " or field" else ""} found as defined in schema ${getSourceLocation(field)} with any of the following signatures " + + val sourceName = field.sourceLocation?.sourceName ?: "" + val sourceLocation = field.sourceLocation?.let { "$sourceName:${it.line}" } ?: "" + + return "No method${if (scannedProperties) " or field" else ""} found as defined in schema $sourceLocation with any of the following signatures " + "(with or without one of $allowedLastArgumentTypes as the last argument), in priority order:\n${signatures.joinToString("\n ")}" + if (isSubscription) "\n\nNote that a Subscription data fetcher must return a Publisher of events" else "" } - private fun getSourceLocation(field: FieldDefinition): String = - field.sourceLocation?.let { "${it.sourceName ?: ""}:${it.line}" } ?: "" - private fun getMissingMethodSignatures(field: FieldDefinition, search: Search, isBoolean: Boolean, scannedProperties: Boolean): List { val baseType = search.type.unwrap() val signatures = mutableListOf() diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index 7745cdc3..c9a1046f 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -39,9 +39,7 @@ internal class MethodFieldResolver( field: FieldDefinition, search: FieldResolverScanner.Search, options: SchemaParserOptions, - val method: Method, - // the method that's called, which is the interface method that [method] implements if the resolver is a JDK dynamic proxy - private val invocableMethod: Method + val method: Method ) : FieldResolver(field, search, options) { private val log = LoggerFactory.getLogger(javaClass) @@ -136,10 +134,10 @@ internal class MethodFieldResolver( return if (numberOfParameters > 0 || isSuspendFunction) { // requires arguments and environment or is a suspend function - MethodFieldResolverDataFetcher(createSourceResolver(), invocableMethod, args, options, isSuspendFunction, valueClass) + MethodFieldResolverDataFetcher(createSourceResolver(), this.method, args, options, isSuspendFunction, valueClass) } else { // if there are no parameters an optimized version of the data fetcher can be used - LightMethodFieldResolverDataFetcher(createSourceResolver(), invocableMethod, options, valueClass) + LightMethodFieldResolverDataFetcher(createSourceResolver(), this.method, options, valueClass) } } diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt index b2b498ce..818186e8 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt @@ -18,7 +18,7 @@ internal class PropertyFieldResolver( field: FieldDefinition, search: FieldResolverScanner.Search, options: SchemaParserOptions, - private val property: Field + val property: Field ) : FieldResolver(field, search, options) { override fun createDataFetcher(): DataFetcher<*> { diff --git a/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt deleted file mode 100644 index 20e5ffc8..00000000 --- a/src/test/kotlin/graphql/kickstart/tools/ProxiedResolverTest.kt +++ /dev/null @@ -1,216 +0,0 @@ -package graphql.kickstart.tools - -import graphql.ExceptionWhileDataFetching -import graphql.GraphQL -import graphql.kickstart.tools.resolver.FieldResolverError -import graphql.schema.DataFetchingEnvironment -import kotlinx.coroutines.delay -import org.aopalliance.intercept.MethodInterceptor -import org.junit.Assert.assertThrows -import org.junit.Test -import org.springframework.aop.framework.ProxyFactory -import java.lang.reflect.Proxy - -class ProxiedResolverTest { - - private val invocations = mutableListOf() - - // stands in for the interceptors Spring adds to a proxy, e.g. for @PreAuthorize or @Transactional - private val recordingInterceptor = MethodInterceptor { invocation -> - invocations.add("${invocation.method.declaringClass.simpleName}.${invocation.method.name}") - invocation.proceed() - } - - @Test - fun `JDK proxied resolver methods are called through the proxy's interfaces and static methods directly`() { - val gql = graphQL( - """ - type Query { - test: String - greeting(name: String!): String - suspendTest: String - increment(value: Int!): Int - staticTest: String - } - """, - jdkProxy(Query())) - - val data = assertNoGraphQlErrors(gql) { - """ - query { - test - greeting(name: "Rex") - suspendTest - increment(value: 1) - staticTest - } - """ - } - - assertEquals(data, mapOf("test" to "ok", "greeting" to "Hello, Rex, from greeting", "suspendTest" to "ok", "increment" to 2, "staticTest" to "static")) - // static methods are called directly, so they skip the interceptors - assertEquals(invocations, listOf("QueryApi.test", "QueryApi.greeting", "QueryApi.suspendTest", "QueryApi.increment")) - } - - @Test - fun `JDK proxied resolver interceptors can reject the call`() { - val gql = graphQL("type Query { test: String }", jdkProxy(Query()) { throw IllegalStateException("Access denied") }) - - val result = gql.execute("query { test }") - - assertEquals(result.getData(), mapOf("test" to null)) - assertEquals(result.errors.map { (it as ExceptionWhileDataFetching).exception.message }, listOf("Access denied")) - } - - // not public, so its methods have to be made accessible - private interface QueryApi { - fun test(): String - fun greeting(name: String, env: DataFetchingEnvironment): String - suspend fun suspendTest(): String - fun increment(value: T): T - } - - // increment(Int) is compiled to increment(int), which the interface's increment(Object) is bridged to - private class Query : GraphQLQueryResolver, QueryApi { - override fun test() = "ok" - override fun greeting(name: String, env: DataFetchingEnvironment) = "Hello, $name, from ${env.field.name}" - override suspend fun suspendTest(): String { - // suspends, so the result reaches graphql-java through the continuation instead of the proxy's return value - delay(1) - return "ok" - } - override fun increment(value: Int) = value + 1 - - companion object { - @JvmStatic - fun staticTest() = "static" - } - } - - @Test - fun `JDK proxied field resolver methods are called through the interface the proxy implements`() { - val dog = mapOf("name" to "Rex", "greeting" to "Hello, Rex") - - // resolvers for the data class and for its supertype - assertEquals(queryDog(jdkProxy(LabradorResolver())), dog) - assertEquals(queryDog(jdkProxy(DogResolver())), dog) - // greeting(Dog) is inherited from a class that doesn't implement the interface - assertEquals(queryDog(jdkProxy(BridgedLabradorResolver())), dog) - // LabradorResolver takes precedence, so the supertype resolver's class-only greeting isn't checked - assertEquals(queryDog(jdkProxy(LabradorResolver()), jdkProxy(ClassOnlyDogResolver())), dog) - - assertEquals(invocations, listOf("GreetingApi.greeting", "GreetingApi.greeting", "LabradorGreetingApi.greeting", "GreetingApi.greeting")) - } - - class DogQuery : GraphQLQueryResolver { - fun dog() = LabradorRetriever("Rex", "black", 3, 1f) - } - - interface GreetingApi { - fun greeting(dog: T): String - } - - // greeting(T) is erased to greeting(Dog), which the interface's greeting(Object) is bridged to - abstract class GreetingResolver : GraphQLResolver, GreetingApi { - override fun greeting(dog: T) = "Hello, ${dog.name}" - } - - class LabradorResolver : GreetingResolver() - - class DogResolver : GreetingResolver() - - abstract class ClassOnlyGreetingResolver : GraphQLResolver { - fun greeting(dog: T) = "Hello, ${dog.name}" - } - - interface LabradorGreetingApi { - fun greeting(dog: LabradorRetriever): String - } - - // the inherited greeting(Dog) implements LabradorGreetingApi.greeting(LabradorRetriever) through a synthetic bridge, which isn't scanned - class BridgedLabradorResolver : ClassOnlyGreetingResolver(), LabradorGreetingApi - - class ClassOnlyDogResolver : ClassOnlyGreetingResolver() - - @Test - fun `parser should throw descriptive exception when a JDK proxied resolver method can't be called through the proxy`() { - val resolver = jdkProxy(ClassOnlyQuery()) - val queryClass = ClassOnlyQuery::class.java.name - assertEquals(buildError(resolver), "Resolver method '$queryClass.test' for field 'test' as defined in schema :1 can't be called on " + - "${resolver.javaClass.name}, which is not an instance of $queryClass. This happens with JDK dynamic proxies, which only implement " + - "interfaces. Declare the method on an interface the resolver implements, or use class-based proxies (e.g. proxyTargetClass = true in Spring).") - - // the target implements QueryApi, but the proxy only implements the interfaces it's given - val hidingProxy = ProxyFactory().apply { - setTarget(Query()) - setInterfaces(GraphQLQueryResolver::class.java) - }.proxy as GraphQLQueryResolver - assertEquals(buildError(hidingProxy)?.substringBefore(" as defined"), "Resolver method '${Query::class.java.name}.test' for field 'test'") - - // test(Integer) is scanned, but the proxy would call test(int), which implements OverloadApi.test(T) - assertEquals(buildError(jdkProxy(OverloadQuery()), "type Query { test(value: Int!): String }")?.substringBefore(" as defined"), - "Resolver method '${OverloadQuery::class.java.name}.test' for field 'test'") - } - - interface StaticApi { - companion object { - @JvmStatic - fun test() = "static" - } - } - - // calling the static StaticApi.test() instead would skip both the proxy and the resolver's own test() - class ClassOnlyQuery : GraphQLQueryResolver, StaticApi { - fun test() = "ok" - } - - interface OverloadApi { - fun test(value: T): String - } - - abstract class OverloadBase : OverloadApi { - override fun test(value: Int) = "override" - } - - class OverloadQuery : OverloadBase(), GraphQLQueryResolver { - fun test(value: Int?) = "overload" - } - - @Test - fun `class based proxied resolver methods are called through the proxy`() { - val resolver = ProxyFactory(OpenQuery()).apply { - isProxyTargetClass = true - addAdvice(recordingInterceptor) - }.proxy as GraphQLQueryResolver - - val data = assertNoGraphQlErrors(graphQL("type Query { test: String }", resolver)) { "query { test }" } - - assertEquals(data, mapOf("test" to "ok")) - assertEquals(invocations, listOf("OpenQuery.test")) - } - - // CGLIB can only intercept methods it can override - open class OpenQuery : GraphQLQueryResolver { - open fun test() = "ok" - } - - private fun jdkProxy(resolver: GraphQLResolver<*>, interceptor: MethodInterceptor = recordingInterceptor) = - (ProxyFactory(resolver).apply { addAdvice(interceptor) }.proxy as GraphQLResolver<*>) - .also { assert(Proxy.isProxyClass(it.javaClass)) } - - private fun queryDog(vararg resolvers: GraphQLResolver<*>) = - assertNoGraphQlErrors(graphQL("type Query { dog: Dog } type Dog { name: String greeting: String }", DogQuery(), *resolvers)) { - "query { dog { name greeting } }" - }["dog"] - - private fun buildError(resolver: GraphQLResolver<*>, schema: String = "type Query { test: String }") = - assertThrows(FieldResolverError::class.java) { parse(schema, resolver) }.message - - private fun graphQL(schema: String, vararg resolvers: GraphQLResolver<*>) = - GraphQL.newGraphQL(parse(schema, *resolvers).makeExecutableSchema()).build() - - private fun parse(schema: String, vararg resolvers: GraphQLResolver<*>) = SchemaParser.newParser() - .schemaString(schema) - .resolvers(*resolvers) - .build() -} diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt index 98b4f24f..0eaa8aef 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt @@ -14,6 +14,7 @@ import org.junit.Before import org.junit.Test import org.reactivestreams.Publisher import org.reactivestreams.tck.TestEnvironment +import org.springframework.aop.framework.ProxyFactory import java.io.FileNotFoundException import java.util.concurrent.CompletableFuture import java.util.concurrent.CompletableFuture.completedFuture @@ -340,6 +341,21 @@ class SchemaParserTest { var name: String? = null } + @Test + fun `parser handles spring AOP proxied resolvers by default`() { + val resolver = ProxyFactory(ProxiedResolver()).proxy as GraphQLQueryResolver + + SchemaParser.newParser() + .schemaString( + """ + type Query { + test: [String] + } + """) + .resolvers(resolver) + .build() + } + @Test fun `parser handles enums with overridden toString method`() { SchemaParser.newParser() @@ -1027,6 +1043,10 @@ class SchemaParserTest { class RootObj + class ProxiedResolver : GraphQLQueryResolver { + fun test(): List = listOf() + } + enum class CustomEnum { FOO { override fun toString(): String { From 191029a62196d22c430fb5db042e7723ac6c382d Mon Sep 17 00:00:00 2001 From: Oryan Date: Sat, 17 Oct 2026 22:10:24 -0400 Subject: [PATCH 7/7] Suggest a getter in the warning for a field the JDK proxy can't read Co-Authored-By: Claude --- .../kotlin/graphql/kickstart/tools/SchemaClassScanner.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 82ddadc7..5459c470 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -12,6 +12,7 @@ import graphql.schema.idl.ScalarInfo import org.apache.commons.lang3.ClassUtils import org.apache.commons.lang3.reflect.TypeUtils import org.slf4j.LoggerFactory +import java.lang.reflect.Field import java.lang.reflect.Member import java.lang.reflect.Method import java.lang.reflect.Modifier @@ -550,10 +551,12 @@ internal class SchemaClassScanner( return } + // class-based proxies don't delegate field reads to the target, so a field needs a getter too + val fix = if (member is Field) "Add a getter for the field and use class-based proxies" else "Use class-based proxies instead" log.warn( "Field '${type.name}.${fieldResolver.field.name}' resolves to ${member.declaringClass.name}.${member.name}, but the resolver is a " + "${source.javaClass.name}, which isn't a ${member.declaringClass.name}, so every query of the field will fail. This happens " + - "with JDK dynamic proxies, which only implement interfaces. Use class-based proxies instead, e.g. proxyTargetClass = true in Spring." + "with JDK dynamic proxies, which only implement interfaces. $fix, e.g. proxyTargetClass = true in Spring." ) }