Repository navigation
Fix suspend resolver context and Kotlin extension resolvers - #835
Merged
Merged
Conversation
A suspend resolver taking GraphQLContext or the custom context class as its last argument failed at runtime with "argument type mismatch". MethodFieldResolver picked the type of the last JVM parameter to decide what to pass, but for a suspend function that is the Continuation, so it fell through to passing the DataFetchingEnvironment. It now looks at the last counted parameter instead. Kotlin extension functions in a GraphQLResolver could not be used as resolver methods because parameters were counted with kotlinFunction.valueParameters, which leaves out the extension receiver. They were either skipped or matched with the wrong arity and failed at runtime. FieldResolverScanner and MethodFieldResolver now count every parameter except the instance, which matches the JVM parameters minus the Continuation, so the receiver takes the source object. An extension receiver is only accepted in that source position. Extension helpers on root resolvers and data classes used to be matched when their arity happened to fit, and then either collided with the real resolver method at build time or failed with the wrong number of arguments at query time. They are now skipped during matching. Fixes #318 Fixes #272 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
# Conflicts: # src/test/kotlin/graphql/kickstart/tools/FieldResolverScannerTest.kt
Count JVM parameters minus the suspend Continuation instead of going through kotlin-reflect's parameter list, and read the last parameter from parameterTypes at that index. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #318
Fixes #272
Checklist
Description
A suspend resolver whose last parameter is the custom context class, like
suspend fun myQuery(context: MyCustomContextType): Booleanfrom #318, builds fine but fails every time it's queried. The reporter got aClassCastException, on master it'sIllegalArgumentException: argument type mismatch. Same thing withGraphQLContextas the last parameter.MethodFieldResolver#createDataFetcherpicks what to pass for that trailing argument withwhen (method.parameterTypes.last()). For a suspend function the last JVM parameter is theContinuation, so neither thecontextClassnor theGraphQLContextbranch matched and theelsepassed theDataFetchingEnvironment. It now looks atmethod.parameterTypes[numberOfParameters - 1], which is the last parameter before theContinuation.Kotlin extension functions in a resolver, like
fun Service.healthy()in aGraphQLResolver<Service>from #272, worked in 5.2.0 and fail since 5.6.0.FieldResolverScanner#getMethodParameterCountandMethodFieldResolver.numberOfParameterscounted parameters withkotlinFunction.valueParameters, which leaves out the extension receiver. Sofun Service.healthy()counted 0 parameters instead of 1. Depending on the arity, the extension was either skipped (aFieldResolverError, or the data class's own getter was used instead) or matched with the wrong count and failed with "wrong number of arguments" at query time. Both now use a sharedparameterCountWithoutContinuation()util, which is the JVM parameter count minus theContinuationfor suspend functions, so the receiver takes the source object. The last parameter is read fromparameterTypesat that count too, otherwise a suspend extension and a non-suspend one were checked differently.An extension receiver is only accepted as the source though. When there's no source parameter to match (root resolvers and data classes),
verifyMethodArgumentsnow rejects extension functions. Without that, the new count would let a helper likefun DataFetchingEnvironment.user()on a second root resolver collide with the realUserQuery#user(id), and the build would fail with "Found more than one matching resolver".I looked at #418 too but didn't fix it. With
contextClassset,myMutation(String id, String field, CustomContext context)is accepted formyMutation(id, field, persist)because the context class is allowed as the last parameter. A build-time check could reject resolvers that work today.contextClasscan be any type, likeMapor a class Jackson can build from the argument value, so a schema argument of that type in the last slot is valid (e.g.contextClass(Map::class)with a Java resolver taking aMapinput). Limiting the check toDataFetchingEnvironmentandGraphQLContextwould miss thecontextClasscase the issue is about, and whether to check only the last slot or every slot is a call for maintainers. I also didn't touch how suspend resolvers are started (#419).Behaviour change: suspend resolvers whose last parameter is
GraphQLContextor the configuredcontextClassnow get that object instead of failing. Public extension functionsfun T.field()in aGraphQLResolver<T>are now matched as resolver methods, so if the data class has a getter with the same name, or the resolver also hasgetField(t: T), the extension now wins. Extension helpers on root resolvers and data classes used to be matched when their arity happened to fit, and then either collided with the real resolver method at build time or failed with "wrong number of arguments" at query time. They're now skipped during matching. So a field whose only match was such a helper now fails at build time withFieldResolverError(unlessallowUnimplementedResolversis set) instead of failing every time it's queried. The last parameter is now checked by its raw class, so a Kotlin resolver with a parameterized context type likectx: Map<String, Any>andcontextClass(Map::class)is now accepted, same as Java resolvers already were. Java resolvers and regular Kotlin methods count parameters the same as before.🤖 Generated with Claude Code