search

Netflix / dgs-framework

GraphQL for Java with Spring Boot made easy.
https://netflix.github.io/dgs
Apache License 2.0
3.03k stars 286 forks source link

Scalar backed by value class inside input type fail to deserialize #1879

Open stengvac opened 2 months ago

stengvac commented 2 months ago

Hello,

lets say we have value class

@JvmInline
value class ValueClass(val value: String)

we use DGS codegen to generate input and outputs for our server side and we used few value classes for Scalars. In version 8.2.0 it was possible to use scalar in generated input type as one of its fields. Top level value class did not work.

Expected behavior

Allow to register converters for conversionService in DefaultInputObjectMapper. It is possible to register custom input object mapper, but as value class is nested in input type, it is not possible to simply register converter only for some types, it require to implement whole DefaultInputObjectMapper or I did miss some configuration.

I have tried with DGS 8.5.5

Actual behavior

With registered Coercing<ValueClass, String>

Code like this worked:

// generated by DGS codegen
data class MyInput(
 val value: ValueClass,
)

@DgsQuery
fun myQuery(@InputArgument input: MyInput): String

This changed with higher versions of DGS as DefaultInputObjectMapper got reworked. Now registered coercing is still called, but converter for value class is not found in DefaultInputObjectMapper.

Stack

org.springframework.core.convert.ConverterNotFoundException: No converter found capable of converting from type [ValueClass] to type [java.lang.String]
    at org.springframework.core.convert.support.GenericConversionService.handleConverterNotFound(GenericConversionService.java:294)
    at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:185)
    at com.netflix.graphql.dgs.internal.DefaultInputObjectMapper.maybeConvert(DefaultInputObjectMapper.kt:118)
    at com.netflix.graphql.dgs.internal.DefaultInputObjectMapper.mapToKotlinObject(DefaultInputObjectMapper.kt:94)
    at com.netflix.graphql.dgs.internal.DefaultInputObjectMapper$Converter.convert(DefaultInputObjectMapper.kt:68)
    at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:41)
    at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:182)
    at com.netflix.graphql.dgs.internal.DefaultInputObjectMapper.maybeConvert(DefaultInputObjectMapper.kt:118)
    at com.netflix.graphql.dgs.internal.DefaultInputObjectMapper.mapToKotlinObject(DefaultInputObjectMapper.kt:94)
    at com.netflix.graphql.dgs.internal.method.InputObjectMapperConverter.convert(InputObjectMapperConverter.kt:41)
    at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:41)
    at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:182)
    at com.netflix.graphql.dgs.internal.method.AbstractInputArgumentResolver.convertValue(AbstractInputArgumentResolver.kt:79)
    at com.netflix.graphql.dgs.internal.method.AbstractInputArgumentResolver.resolveArgument(AbstractInputArgumentResolver.kt:48)
    at com.netflix.graphql.dgs.internal.method.ArgumentResolverComposite.resolveArgument(ArgumentResolverComposite.kt:39)
    at com.netflix.graphql.dgs.internal.DataFetcherInvoker.invokeKotlinMethod(DataFetcherInvoker.kt:114)
    at com.netflix.graphql.dgs.internal.DataFetcherInvoker.get(DataFetcherInvoker.kt:76)
    at graphql.schema.DataFetcherFactories.lambda$wrapDataFetcher$2(DataFetcherFactories.java:37)
    at com.netflix.graphql.dgs.metrics.micrometer.DgsGraphQLMetricsInstrumentation.instrumentDataFetcher$lambda$6(DgsGraphQLMetricsInstrumentation.kt:149)
    at graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation.lambda$instrumentDataFetcher$0(DataLoaderDispatcherInstrumentation.java:90)
    at graphql.execution.ExecutionStrategy.invokeDataFetcher(ExecutionStrategy.java:329)
    at graphql.execution.ExecutionStrategy.fetchField(ExecutionStrategy.java:305)
    at graphql.execution.ExecutionStrategy.fetchField(ExecutionStrategy.java:243)
    at graphql.execution.ExecutionStrategy.resolveFieldWithInfo(ExecutionStrategy.java:214)
    at graphql.execution.ExecutionStrategy.resolveField(ExecutionStrategy.java:186)
    at graphql.execution.AsyncSerialExecutionStrategy.lambda$execute$1(AsyncSerialExecutionStrategy.java:56)
    at graphql.execution.Async.eachSequentiallyImpl(Async.java:162)
    at graphql.execution.Async.eachSequentially(Async.java:151)
    at graphql.execution.AsyncSerialExecutionStrategy.execute(AsyncSerialExecutionStrategy.java:51)
    at graphql.execution.Execution.executeOperation(Execution.java:162)
    at graphql.execution.Execution.execute(Execution.java:104)
    at graphql.GraphQL.execute(GraphQL.java:568)
    at graphql.GraphQL.lambda$parseValidateAndExecute$13(GraphQL.java:487)
    at java.base/java.util.concurrent.CompletableFuture.uniComposeStage(CompletableFuture.java:1187)
    at java.base/java.util.concurrent.CompletableFuture.thenCompose(CompletableFuture.java:2309)
    at graphql.GraphQL.parseValidateAndExecute(GraphQL.java:482)
    at graphql.GraphQL.lambda$executeAsync$9(GraphQL.java:440)
    at java.base/java.util.concurrent.CompletableFuture.uniComposeStage(CompletableFuture.java:1187)
    at java.base/java.util.concurrent.CompletableFuture.thenCompose(CompletableFuture.java:2309)
    at graphql.GraphQL.executeAsync(GraphQL.java:428)
    at com.netflix.graphql.dgs.internal.BaseDgsQueryExecutor.baseExecute(BaseDgsQueryExecutor.kt:123)
    at com.netflix.graphql.dgs.internal.DefaultDgsQueryExecutor.execute(DefaultDgsQueryExecutor.kt:84)
    at com.netflix.graphql.dgs.mvc.DgsRestController$executeQuery$executionResult$1.invoke(DgsRestController.kt:215)
    at com.netflix.graphql.dgs.mvc.DgsRestController$executeQuery$executionResult$1.invoke(DgsRestController.kt:213)
    at com.netflix.graphql.dgs.internal.utils.TimeTracer.logTime(TimeTracer.kt:24)
    at com.netflix.graphql.dgs.mvc.DgsRestController.executeQuery(DgsRestController.kt:213)
    at com.netflix.graphql.dgs.mvc.DgsRestController.graphql(DgsRestController.kt:135)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
    at java.base/java.lang.reflect.Method.invoke(Method.java:577)

Steps to reproduce

Create value class. Use it as field in input type. Call query/mutation with such input.

stengvac commented 2 months ago

Or maybe it could be considered feature.

kilink commented 2 months ago

I think this was already reported in #355, although I suppose you consider it a different problem since it's reporting usage at the top level. It worked before we switched to using the ConversionService? I'd have to look into why that is the case, it likely just worked by chance.

stengvac commented 2 months ago

Yep I am aware, that top level does not work so we went for input object, which worked.

Will value classes be supported or do we have to swap to non value class? I just want to know what to expect and make adjustments in our code base. Thx.