Skip to content

Commit 2b1bec7

Browse files
oryan-blockclaude
andcommitted
Fix subscription resolver return type validation
The Publisher return type check for subscriptions was applied whenever the resolver instance implemented GraphQLSubscriptionResolver, so a class that is also a query or mutation resolver lost all of its non-Publisher methods and failed with "No method found". The check and the missing method note now depend on the root type being scanned, which RootResolverInfo records. The checks were also inverted: they accepted supertypes of Publisher and CompletableFuture rather than subtypes, so Publisher implementations such as Flux were rejected. Accept Publisher subtypes, futures of them (Future or CompletionStage, with class, parameterized or wildcard type arguments) and ReceiveChannel subtypes that have a wrapper registered for that exact type. Methods erased to Object, such as suspend functions, are still accepted since their return type can't be checked here. Generic wrappers only unwrap their exact type, so subscription return types are now matched against the schema as a Publisher of their events. Otherwise a Flux<Item> return, or a covariant override of a Publisher<Item> method, would map the Item type to the Flux class. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 349d56a commit 2b1bec7

6 files changed

Lines changed: 333 additions & 22 deletions

File tree

‎src/main/kotlin/graphql/kickstart/tools/ResolverInfo.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ internal class MultiResolverInfo(
6868

6969
internal class RootResolverInfo(
7070
val resolvers: List<GraphQLRootResolver>,
71-
private val options: SchemaParserOptions
71+
private val options: SchemaParserOptions,
72+
val isSubscription: Boolean = false
7273
) : ResolverInfo() {
7374
override fun getFieldSearches() =
7475
resolvers.map { FieldResolverScanner.Search(getRealResolverClass(it, options), this, it) }

‎src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ internal class SchemaClassScanner(
543543

544544
private val queryResolverInfo = RootResolverInfo(queryResolvers, options)
545545
private val mutationResolverInfo = RootResolverInfo(mutationResolvers, options)
546-
private val subscriptionResolverInfo = RootResolverInfo(subscriptionResolvers, options)
546+
private val subscriptionResolverInfo = RootResolverInfo(subscriptionResolvers, options, isSubscription = true)
547547

548548
val query = createRootType("query", queryDefinition, queryName, true, queryResolvers, GraphQLQueryResolver::class.java, queryResolverInfo)
549549
val mutation = createRootType("mutation", mutationDefinition, mutationName, rootInfo.isMutationRequired(), mutationResolvers, GraphQLMutationResolver::class.java, mutationResolverInfo)

‎src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package graphql.kickstart.tools.resolver
22

33
import graphql.GraphQLContext
44
import graphql.Scalars
5-
import graphql.kickstart.tools.GraphQLSubscriptionResolver
65
import graphql.kickstart.tools.ResolverInfo
76
import graphql.kickstart.tools.RootResolverInfo
87
import graphql.kickstart.tools.SchemaParserOptions
@@ -13,10 +12,10 @@ import graphql.schema.DataFetchingEnvironment
1312
import kotlinx.coroutines.channels.ReceiveChannel
1413
import org.apache.commons.lang3.ClassUtils
1514
import org.apache.commons.lang3.reflect.FieldUtils
15+
import org.apache.commons.lang3.reflect.TypeUtils
1616
import org.reactivestreams.Publisher
1717
import org.slf4j.LoggerFactory
1818
import java.lang.reflect.*
19-
import java.util.concurrent.CompletableFuture
2019
import kotlin.reflect.full.valueParameters
2120
import kotlin.reflect.jvm.javaType
2221
import kotlin.reflect.jvm.kotlinFunction
@@ -124,27 +123,27 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
124123
// to avoid issues with duplicate method declarations
125124
.filter { it.declaringClass != Object::class.java }
126125
// subscription resolvers must return a publisher
127-
.filter { search.source !is GraphQLSubscriptionResolver || resolverMethodReturnsPublisher(it) }
126+
.filter { !search.isSubscription || resolverMethodReturnsPublisher(it) }
128127
.toList()
129128
}
130129

131130
private fun resolverMethodReturnsPublisher(method: Method) =
132-
method.returnType.isAssignableFrom(Publisher::class.java)
131+
// suspend functions and unbounded type variables are erased to Object, so the actual return type is unknown here
132+
method.returnType == Any::class.java
133+
|| Publisher::class.java.isAssignableFrom(method.returnType)
133134
|| resolverMethodReturnsPublisherFuture(method)
134135
|| receiveChannelToPublisherWrapper(method)
135136

136137
private fun resolverMethodReturnsPublisherFuture(method: Method) =
137-
method.returnType.isAssignableFrom(CompletableFuture::class.java)
138-
&& method.genericReturnType is ParameterizedType
139-
&& (method.genericReturnType as ParameterizedType).actualTypeArguments
140-
.any {
141-
it is ParameterizedType && it.unwrap().isAssignableFrom(Publisher::class.java)
142-
}
138+
method.genericReturnType.futureValueType()
139+
?.let { if (it is WildcardType) it.upperBounds.first() else it }
140+
?.let { TypeUtils.isAssignable(it, Publisher::class.java) } == true
143141

144142
private fun receiveChannelToPublisherWrapper(method: Method) =
145-
method.returnType.isAssignableFrom(ReceiveChannel::class.java)
143+
ReceiveChannel::class.java.isAssignableFrom(method.returnType)
146144
&& options.genericWrappers.any { wrapper ->
147145
val isReceiveChannelWrapper = wrapper.type == method.returnType
146+
// lambdas are erased to return Object, so this only checks that the transformer can return a Publisher
148147
val hasPublisherTransformer = wrapper
149148
.transformer.javaClass
150149
.declaredMethods
@@ -199,7 +198,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
199198

200199
searches.forEach { search ->
201200
signatures.addAll(getMissingMethodSignatures(field, search, isBoolean, scannedProperties))
202-
isSubscription = isSubscription || search.source is GraphQLSubscriptionResolver
201+
isSubscription = isSubscription || search.isSubscription
203202
}
204203

205204
val sourceName = field.sourceLocation?.sourceName ?: "<unknown>"
@@ -241,7 +240,9 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
241240
val resolverInfo: ResolverInfo,
242241
val source: Any?,
243242
val requiredFirstParameterType: Class<*>? = null
244-
)
243+
) {
244+
val isSubscription get() = resolverInfo is RootResolverInfo && resolverInfo.isSubscription
245+
}
245246
}
246247

247248
internal class FieldResolverError(msg: String) : RuntimeException(msg)

‎src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import graphql.kickstart.tools.*
66
import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper
77
import graphql.kickstart.tools.util.JavaType
88
import graphql.kickstart.tools.util.coroutineScope
9+
import graphql.kickstart.tools.util.futureValueType
10+
import graphql.kickstart.tools.util.typeArgument
911
import graphql.kickstart.tools.util.unwrap
1012
import graphql.language.*
1113
import graphql.schema.DataFetcher
@@ -14,6 +16,8 @@ import graphql.schema.GraphQLFieldDefinition
1416
import graphql.schema.GraphQLTypeUtil.isScalar
1517
import graphql.schema.LightDataFetcher
1618
import kotlinx.coroutines.future.future
19+
import org.apache.commons.lang3.reflect.TypeUtils
20+
import org.reactivestreams.Publisher
1721
import org.slf4j.LoggerFactory
1822
import java.lang.reflect.InvocationTargetException
1923
import java.lang.reflect.Method
@@ -150,20 +154,26 @@ internal class MethodFieldResolver(
150154
}
151155

152156
override fun scanForMatches(): List<TypeClassMatcher.PotentialMatch> {
153-
val unwrappedGenericType = genericType.unwrapGenericType(
154-
try {
155-
method.kotlinFunction?.returnType?.javaType ?: method.genericReturnType
156-
} catch (e: InternalError) {
157-
method.genericReturnType
158-
}
159-
)
157+
val returnType = try {
158+
method.kotlinFunction?.returnType?.javaType ?: method.genericReturnType
159+
} catch (e: InternalError) {
160+
method.genericReturnType
161+
}
162+
val unwrappedGenericType = genericType.unwrapGenericType(if (search.isSubscription) returnType.asPublisher() else returnType)
160163
val returnValueMatch = TypeClassMatcher.PotentialMatch.returnValue(field.type, unwrappedGenericType, genericType, SchemaClassScanner.ReturnValueReference(method))
161164

162165
return field.inputValueDefinitions.mapIndexed { i, inputDefinition ->
163166
TypeClassMatcher.PotentialMatch.parameterType(inputDefinition.type, getMethodParameterType(i)!!, genericType, SchemaClassScanner.MethodParameterReference(method, i))
164167
} + listOf(returnValueMatch)
165168
}
166169

170+
// generic wrappers only match their exact type, so a Publisher implementation (e.g. Flux), or a future of one,
171+
// is matched as a plain Publisher of its events
172+
private fun JavaType.asPublisher(): JavaType =
173+
(futureValueType() ?: this).typeArgument(Publisher::class.java)
174+
?.let { TypeUtils.parameterize(Publisher::class.java, it) }
175+
?: this
176+
167177
private fun getIndexOffset(): Int {
168178
return if (resolverInfo is DataClassTypeResolverInfo && !method.declaringClass.isAssignableFrom(resolverInfo.dataClassType)) {
169179
1

‎src/main/kotlin/graphql/kickstart/tools/util/Utils.kt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@ import graphql.language.*
66
import graphql.schema.DataFetchingEnvironment
77
import kotlinx.coroutines.CoroutineScope
88
import kotlinx.coroutines.Dispatchers
9+
import org.apache.commons.lang3.reflect.TypeUtils
910
import java.lang.reflect.Method
1011
import java.lang.reflect.ParameterizedType
1112
import java.lang.reflect.Proxy
13+
import java.util.concurrent.CompletionStage
14+
import java.util.concurrent.Future
1215

1316
/**
1417
* @author Andrew Potter
@@ -37,6 +40,12 @@ internal fun JavaType.unwrap(): Class<out Any> =
3740
this as Class<*>
3841
}
3942

43+
internal fun JavaType.typeArgument(type: Class<*>): JavaType? =
44+
TypeUtils.getTypeArguments(this, type)?.get(type.typeParameters.first())
45+
46+
internal fun JavaType.futureValueType(): JavaType? =
47+
typeArgument(CompletionStage::class.java) ?: typeArgument(Future::class.java)
48+
4049
internal fun DataFetchingEnvironment.coroutineScope(): CoroutineScope {
4150
val context: Any? = this.getContext()
4251
return if (context is CoroutineScope) context else CoroutineScope(Dispatchers.Default)

0 commit comments

Comments
 (0)