feat: Add extension, public method for coercing DataFetchingEnvironment arguments with Kotlin reflection#2169
feat: Add extension, public method for coercing DataFetchingEnvironment arguments with Kotlin reflection#2169JordanJLopez wants to merge 4 commits intomasterfrom
Conversation
…ng arguments into typed objects using Kotlin reflection
There was a problem hiding this comment.
Pull request overview
Adds a public DataFetchingEnvironment extension to coerce environment.arguments into typed Kotlin input objects using the same Kotlin-reflection constructor path as FunctionDataFetcher, avoiding Jackson Kotlin metadata/constructor discovery issues (notably with Jackson 3 + older Kotlin metadata).
Changes:
- Introduces
DataFetchingEnvironment.getArgumentsAs(KClass)/getArgumentsAs<T>()extension in the schema generator. - Extracts/introduces an internal
convertInputMaphelper and extends tests around map-to-Kotlin-object coercion, including pre-coerced custom scalar values. - Documents the recommended migration away from
ObjectMapper.convertValue(environment.arguments).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| website/docs/schema-generator/execution/data-fetching-environment.md | Documents the new getArgumentsAs extension and why it’s preferred over ObjectMapper.convertValue. |
| generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/extensions/DataFetchingEnvironmentExtensions.kt | Adds getArgumentsAs extensions that coerce environment.arguments via Kotlin reflection. |
| generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/convertArgumentValue.kt | Adds convertInputMap helper and (currently) changes convertArgumentValue visibility. |
| generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/extensions/DataFetchingEnvironmentExtensionsTest.kt | Unit tests for getArgumentsAs behavior (defaults, @GraphQLName, pass-through of pre-coerced values). |
| generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/execution/ConvertArgumentValueTest.kt | Adds direct tests for convertInputMap conversion behavior. |
| generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/test/integration/PlainKotlinInputWithCustomScalarTest.kt | Integration-style tests validating end-to-end custom scalar coercion and map conversion behavior. |
Comments suppressed due to low confidence (1)
generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/convertArgumentValue.kt:56
convertArgumentValuewas changed frominternalto a public top-level function. This unexpectedly expands the library's public API surface (and effectively commits to supporting this signature/behavior). If this wasn't intentional, revert it back tointernal; if it is intended to be public API, it should be documented as such (including KDoc contract/expectations).
fun convertArgumentValue(
argumentName: String,
param: KParameter,
argumentMap: Map<String, Any?>
): Any? {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/convertArgumentValue.kt:56
convertArgumentValueis now public (fun) instead ofinternal. Unless you intentionally want to add/maintain this as a supported public API, it should remaininternalto avoid expanding the library surface area and committing to its behavior/signature for external consumers.
fun convertArgumentValue(
argumentName: String,
param: KParameter,
argumentMap: Map<String, Any?>
): Any? {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/convertArgumentValue.kt:56
convertArgumentValueappears to have been unintentionally made public (it was previouslyinternal). This expands the library’s public API surface and makes it harder to change this helper without breaking consumers. If external use isn’t intended, restoreinternalvisibility; if it is intended to be public, please add KDoc/README docs and treat it as a supported API (incl. semantic versioning implications).
fun convertArgumentValue(
argumentName: String,
param: KParameter,
argumentMap: Map<String, Any?>
): Any? {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…entire environment.arguments
📝 Description
Jackson 3's
jackson-module-kotlinfails to construct Kotlin classes that lack proper creatormetadata when using
ObjectMapper.convertValue. The failure path is inReflectionCache.valueCreatorFromJava:when
Constructor.kotlinFunctionreturnsnullfor a class (which occurs for classes compiledwith older Kotlin metadata under a newer
kotlin-reflectruntime),valueClassAwareKotlinFunctionpropagates
nullup throughvalueCreatorFromJava, causingKotlinValueInstantiatorto fallback to a Java creator path that finds nothing — resulting in "no Creators... cannot deserialize
from Object value".
FunctionDataFetcherhas never had this problem because it usesKClass.primaryConstructor(Kotlin-side reflection) directly. This PR exposes that same coercion path as a public API.
Changes:
DataFetchingEnvironment.getArgumentsAs<T>()/getArgumentsAs(KClass)extension ingraphql-kotlin-schema-generator'sextensionspackage — coerces the full arguments mapconvertInputMap(map, KClass)public — coerces a single extracted argument value,for use in instrumentation or library code that inspects fields dynamically
@GraphQLNameor Kotlin parameter names and correctlypass through values already coerced by graphql-java's scalar pipeline
Migration:
objectMapper.convertValue<MyArgs>(environment.arguments)with
environment.getArgumentsAs<MyArgs>()objectMapper.convertValue<MyInput>(rawMap)with
convertInputMap(rawMap, MyInput::class)🔗 Related Issues