Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support default parameters for DgsData methods #1403

Merged
merged 2 commits into from Feb 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -19,14 +19,20 @@ package com.netflix.graphql.dgs.internal
import com.netflix.graphql.dgs.internal.method.ArgumentResolverComposite
import graphql.schema.DataFetcher
import graphql.schema.DataFetchingEnvironment
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.reactor.mono
import org.springframework.core.BridgeMethodResolver
import org.springframework.core.CoroutinesUtils
import org.springframework.core.KotlinDetector
import org.springframework.core.MethodParameter
import org.springframework.core.ParameterNameDiscoverer
import org.springframework.core.annotation.SynthesizingMethodParameter
import org.springframework.util.CollectionUtils
import org.springframework.util.ReflectionUtils
import java.lang.reflect.InvocationTargetException
import java.lang.reflect.Method
import kotlin.reflect.KFunction
import kotlin.reflect.KParameter
import kotlin.reflect.full.callSuspendBy
import kotlin.reflect.jvm.kotlinFunction

class DataFetcherInvoker internal constructor(
private val dgsComponent: Any,
Expand All @@ -36,7 +42,7 @@ class DataFetcherInvoker internal constructor(
) : DataFetcher<Any?> {

private val bridgedMethod: Method = BridgeMethodResolver.findBridgedMethod(method)
private val isSuspending: Boolean = KotlinDetector.isSuspendingFunction(bridgedMethod)
private val kotlinFunction: KFunction<*>? = bridgedMethod.kotlinFunction

private val methodParameters: List<MethodParameter> = bridgedMethod.parameters.map { parameter ->
val methodParameter = SynthesizingMethodParameter.forParameter(parameter)
Expand All @@ -53,6 +59,10 @@ class DataFetcherInvoker internal constructor(
return ReflectionUtils.invokeMethod(bridgedMethod, dgsComponent)
}

if (kotlinFunction != null) {
return invokeKotlinMethod(kotlinFunction, environment)
}

val args = arrayOfNulls<Any?>(methodParameters.size)

for ((idx, parameter) in methodParameters.withIndex()) {
Expand All @@ -62,10 +72,40 @@ class DataFetcherInvoker internal constructor(
args[idx] = resolvers.resolveArgument(parameter, environment)
}

return if (isSuspending) {
CoroutinesUtils.invokeSuspendingFunction(bridgedMethod, dgsComponent, *args)
return ReflectionUtils.invokeMethod(bridgedMethod, dgsComponent, *args)
}

private fun invokeKotlinMethod(kFunc: KFunction<*>, dfe: DataFetchingEnvironment): Any? {
val parameters = kFunc.parameters
val argsByName = CollectionUtils.newLinkedHashMap<KParameter, Any?>(parameters.size)

val paramSeq = if (parameters[0].kind == KParameter.Kind.INSTANCE) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain what lines #82 and #83 are doing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are checking if the KFunction takes an instance parameter, and supplying it if so; that is, the target object we want to call the method on, in this case the DGS component. That said, I can't really think if a scenario where we won't have an instance parameter, but we are kind of forced to deal with the possibility.

argsByName[parameters[0]] = dgsComponent
parameters.asSequence().drop(1)
} else {
ReflectionUtils.invokeMethod(bridgedMethod, dgsComponent, *args)
parameters.asSequence()
}

for ((kParameter, parameter) in paramSeq.zip(methodParameters.asSequence())) {
if (!resolvers.supportsParameter(parameter)) {
throw IllegalStateException(formatArgumentError(parameter, "No suitable resolver"))
}
val value = resolvers.resolveArgument(parameter, dfe)
if (value == null && kParameter.isOptional && !kParameter.type.isMarkedNullable) {
continue
}
argsByName[kParameter] = value
}

if (kFunc.isSuspend) {
return mono(Dispatchers.Unconfined) {
kFunc.callSuspendBy(argsByName)
}.onErrorMap(InvocationTargetException::class.java) { it.targetException }
}
return try {
kFunc.callBy(argsByName)
} catch (ex: Exception) {
ReflectionUtils.handleReflectionException(ex)
}
}

Expand Down
Expand Up @@ -1361,7 +1361,6 @@ internal class InputArgumentTest {
assertThat(executionResult).isNotNull
assertThat(executionResult.errors).isEmpty()
assertThat(executionResult.isDataPresent).isTrue
val data = executionResult.getData<Map<String, *>>()
}

@Test
Expand Down Expand Up @@ -2053,6 +2052,36 @@ internal class InputArgumentTest {
assertThat(data).hasEntrySatisfying("strings") { assertThat(it).isEqualTo("Ok") }
}

@Test
fun `@InputArgument on a list of input types with Kotlin default argument`() {
val schema = """
type Query {
hello(person:[Person]): String
}

input Person {
name:String
}
""".trimIndent()

val fetcher = object {
@DgsData(parentType = "Query", field = "hello")
fun someFetcher(@InputArgument("person") person: List<Person> = emptyList()): String {
assertThat(person).isEmpty()
return "Hello, Nobody"
}
}

withComponents("helloFetcher" to fetcher)

val build = GraphQL.newGraphQL(provider.schema(schema)).build()
val executionResult = build.execute("""{ hello }""")
assertThat(executionResult.errors).isEmpty()
assertThat(executionResult.isDataPresent).isTrue
val data = executionResult.getData<Map<String, *>>()
assertThat(data["hello"]).isEqualTo("Hello, Nobody")
}

private fun withComponents(vararg components: Pair<String, Any>) {
every { applicationContextMock.getBeansWithAnnotation(DgsComponent::class.java) } returns mapOf(*components)
}
Expand Down