Netflix / dgs-framework

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

bug: Graphql Assertion throws exception when validating when using Kotlin Coroutines #834

Closed BCantos17 closed 2 years ago

BCantos17 commented 2 years ago

Expected behavior

Validation passes and app starts as usual

Actual behavior

Looks to have failed because of the dollar sign when the method has the suspend keyword. Exception is thrown with this stacktrace

Caused by: graphql.AssertException: Name must be non-null, non-empty and match [_A-Za-z][_0-9A-Za-z]* - was 'session$suspendImpl'
at graphql.Assert.assertValidName(Assert.java:117) ~[graphql-java-17.3.jar:na]
at graphql.schema.FieldCoordinates.assertValidNames(FieldCoordinates.java:54) ~[graphql-java-17.3.jar:na]
at graphql.schema.GraphQLCodeRegistry$Builder.dataFetcher(GraphQLCodeRegistry.java:366) ~[graphql-java-17.3.jar:na]
at graphql.schema.GraphQLCodeRegistry$Builder.dataFetcher(GraphQLCodeRegistry.java:323) ~[graphql-java-17.3.jar:na]
at com.netflix.graphql.dgs.internal.DgsSchemaProvider.registerDataFetcher(DgsSchemaProvider.kt:276) ~[graphql-dgs-4.9.16.jar:4.9.16]
at com.netflix.graphql.dgs.internal.DgsSchemaProvider.findDataFetchers$lambda-12$lambda-11$lambda-10(DgsSchemaProvider.kt:209) ~[graphql-dgs-4.9.16.jar:4.9.16]
at org.springframework.core.annotation.TypeMappedAnnotations$AggregatesSpliterator.tryAdvance(TypeMappedAnnotations.java:599) ~[spring-core-5.3.15.jar:5.3.15]
at org.springframework.core.annotation.TypeMappedAnnotations$AggregatesSpliterator.tryAdvance(TypeMappedAnnotations.java:566) ~[spring-core-5.3.15.jar:5.3.15]
at java.base/java.util.Spliterator.forEachRemaining(Spliterator.java:332) ~[na:na]
at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762) ~[na:na]
at com.netflix.graphql.dgs.internal.DgsSchemaProvider.findDataFetchers(DgsSchemaProvider.kt:208) ~[graphql-dgs-4.9.16.jar:4.9.16]
at com.netflix.graphql.dgs.internal.DgsSchemaProvider.schema(DgsSchemaProvider.kt:112) ~[graphql-dgs-4.9.16.jar:4.9.16]
at com.netflix.graphql.dgs.autoconfig.DgsAutoConfiguration.schema(DgsAutoConfiguration.kt:169) ~[graphql-dgs-spring-boot-oss-autoconfigure-4.9.16.jar:4.9.16]
at com.netflix.graphql.dgs.autoconfig.DgsAutoConfiguration$$EnhancerBySpringCGLIB$$22543706.CGLIB$schema$0(<generated>) ~[graphql-dgs-spring-boot-oss-autoconfigure-4.9.16.jar:4.9.16]
at com.netflix.graphql.dgs.autoconfig.DgsAutoConfiguration$$EnhancerBySpringCGLIB$$22543706$$FastClassBySpringCGLIB$$d7467a1a.invoke(<generated>) ~[graphql-dgs-spring-boot-oss-autoconfigure-4.9.16.jar:4.9.16]
at org.springframework.cglib.proxy.MethodProxy.invokeSuper(MethodProxy.java:244) ~[spring-core-5.3.15.jar:5.3.15]
at org.springframework.context.annotation.ConfigurationClassEnhancer$BeanMethodInterceptor.intercept(ConfigurationClassEnhancer.java:331) ~[spring-context-5.3.15.jar:5.3.15]
at com.netflix.graphql.dgs.autoconfig.DgsAutoConfiguration$$EnhancerBySpringCGLIB$$22543706.schema(<generated>) ~[graphql-dgs-spring-boot-oss-autoconfigure-4.9.16.jar:4.9.16]
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) ~[na:na]
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
at java.base/java.lang.reflect.Method.invoke(Method.java:568) ~[na:na]
at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:154) ~[spring-beans-5.3.15.jar:5.3.15]
... 89 common frames omitted

Steps to reproduce

graphql.schema

type Query {
    session(id: String): Session
}

type Mutation {
    setSession(id: String, name: String): Boolean
}

type Session {
    id: String
    name: String
}

fetcher class

@DgsComponent
class SessionFetcher(
    private val redisOperations: ReactiveRedisOperations<String, Session>
) {

    @DgsQuery
    suspend fun session(id: String): Session? {
        return redisOperations.opsForValue().getAndAwait(id)
    }

    @DgsMutation
    suspend fun setSession(id: String, name: String): Boolean {
        return redisOperations.opsForValue().setAndAwait(id, Session(id, name))
    }
}

redis configuration

@Configuration
class RedisConfig {
    @Bean
    @Primary
    fun connectionFactory(): ReactiveRedisConnectionFactory {
        return LettuceConnectionFactory("localhost", 6379)
    }

    @Bean
    fun redisOperations(connectionFactory: LettuceConnectionFactory, objectMapper: ObjectMapper): ReactiveRedisOperations<String, Session> {
        val valueSerializer = Jackson2JsonRedisSerializer(Session::class.java).apply {
            setObjectMapper(objectMapper)
        }
        return ReactiveRedisTemplate(
            connectionFactory,
            newSerializationContext<String, Session>(StringRedisSerializer())
                .value(valueSerializer)
                .build()
        )
    }
}

session class

class Session(
    val id: String,
    val name: String
)

build.gradle.kts

import org.jetbrains.kotlin.gradle.tasks.KotlinCompile

plugins {
id("org.springframework.boot") version "2.6.3"
id("io.spring.dependency-management") version "1.0.11.RELEASE"
kotlin("jvm") version "1.6.10"
kotlin("plugin.spring") version "1.6.10"
}

group = "com.company"
version = "0.0.1-SNAPSHOT"
java.sourceCompatibility = JavaVersion.VERSION_16

repositories {
mavenCentral()
}

dependencies {
    implementation(platform("com.netflix.graphql.dgs:graphql-dgs-platform-dependencies:latest.release"))
    implementation("com.netflix.graphql.dgs:graphql-dgs-webflux-starter")
    implementation("org.jetbrains.kotlin:kotlin-reflect")
    implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk8")
    implementation("org.springframework.boot:spring-boot-starter-webflux")
    implementation("org.springframework.boot:spring-boot-starter-actuator")
    implementation("org.springframework.boot:spring-boot-starter-data-redis-reactive:2.6.3")
    testImplementation("org.springframework.boot:spring-boot-starter-test")
}

tasks.withType<KotlinCompile> {
kotlinOptions {
freeCompilerArgs = listOf("-Xjsr305=strict")
jvmTarget = "16"
}
}

tasks.withType<Test> {
useJUnitPlatform()
}

noticed there were resources that states coroutines were supported found here https://github.com/hantsy/spring-graphql-sample/tree/master/dgs-kotlin-co https://github.com/Netflix/dgs-framework/issues/413

but I could not find any leads which points to an answer. This works without coroutines if I take out the suspend key words and just use java reactors (Mono and Flux and removing the AndAwait) but using coroutines returns this exception.

Not sure if this is entirely a bug or if I am doing something wrong. If I did something wrong, any guidance would be much appreciated

berngp commented 2 years ago

Looks like a bug, thanks for reporting,

gaplo917 commented 2 years ago

Hi @berngp @BCantos17, I did a simple test that can confirm it fails in 4.9.16 but pass in 4.9.15 for the kotlin coroutine suspend fun support.

For someone encountering this impact could fallback to 4.9.15.

4.9.15

Screenshot 2022-01-26 at 6 14 52 PM

4.9.16

Screenshot 2022-01-26 at 6 18 03 PM

I suspect the following changes cause this regression

https://github.com/Netflix/dgs-framework/pull/808/files#diff-6206801457e356a62d795ef56a39762a8ea68bf5879cb063c86fd2406501aab4R200

gaplo917 commented 2 years ago

For more deep dive, I confirmed in JVM debugger that ReflectionUtils.getUniqueDeclaredMethods(javaClass).asSequence() would get a static function, I suggest to add a filter for this case.

4.9.15 Screenshot 2022-01-26 at 6 30 39 PM

4.9.16 Screenshot 2022-01-26 at 6 24 37 PM

berngp commented 2 years ago

@gaplo917 thanks for the amazing walkthrough! Will look into a fix so we can get it in in the next release.