:thread: Question regarding simple mutation we're trying to model, which turns out it doesn't seem s...

# graphql-kotlin

[Yannick Lazzari]

🧵 Question regarding simple mutation we're trying to model, which turns out it doesn't seem so simple after all and generating an error like

kotlin.reflect.jvm.internal.KotlinReflectionInternalError: This callable does not support a default call

depending on how you call it. Details inside thread.

[Yannick Lazzari]

Disclaimer: the example below is not the actual implementation, but a simplified made up use case for illustrative purposes.

[Yannick Lazzari]

So, let's say you have this simply input type:

data class UserInput(id: ID, firstName: String? = null, lastName: String? = null)

and mutation:

fun updateUser(user: UserInput): Boolean = true

giving us schema:

type Mutation {
  updateUser(user: User!): Boolean!
}

input UserInput {
  id: ID!
  firstName: String
  lastName: String
}

So, a mutation where you can either update a user's first name, or last name, or both, or nothing.

[Yannick Lazzari]

If you call it with all fields specified, it works:

mutation {
  updateUser(
    user: {
      id: "123"
      firstName: "John"
      lastName: "Doe"
    }
  )
}

but if you just specify either only the first name (or last name):

mutation {
  updateUser(
    user: {
      id: "123"
      firstName: "John"
    }
  )
}

it fails with something that looks like (stripped down version of the stacktrace):

kotlin.reflect.jvm.internal.KotlinReflectionInternalError: This callable does not support a default call: ...
Original Stack Trace:
		at kotlin.reflect.jvm.internal.KCallableImpl.callDefaultMethod$kotlin_reflection(KCallableImpl.kt:164) ~[kotlin-reflect-1.6.21.jar:1.6.21-release-334(1.6.21)]
		at kotlin.reflect.jvm.internal.KCallableImpl.callBy(KCallableImpl.kt:112) ~[kotlin-reflect-1.6.21.jar:1.6.21-release-334(1.6.21)]
		at com.expediagroup.graphql.generator.execution.ConvertArgumentValueKt.mapToKotlinObject(convertArgumentValue.kt:121) ~[graphql-kotlin-schema-generator-6.2.2.jar:6.2.2]
		at com.expediagroup.graphql.generator.execution.ConvertArgumentValueKt.convertValue(convertArgumentValue.kt:76) ~[graphql-kotlin-schema-generator-6.2.2.jar:6.2.2]
		at com.expediagroup.graphql.generator.execution.ConvertArgumentValueKt.convertArgumentValue(convertArgumentValue.kt:54) ~[graphql-kotlin-schema-generator-6.2.2.jar:6.2.2]
		at com.expediagroup.graphql.generator.execution.FunctionDataFetcher.mapParameterToValue(FunctionDataFetcher.kt:101) ~[graphql-kotlin-schema-generator-6.2.2.jar:6.2.2]
		at com.expediagroup.graphql.server.spring.execution.SpringDataFetcher.mapParameterToValue(SpringDataFetcher.kt:48) ~[graphql-kotlin-spring-server-6.2.2.jar:6.2.2]
		at com.expediagroup.graphql.generator.execution.FunctionDataFetcher.getParameters(FunctionDataFetcher.kt:79) ~[graphql-kotlin-schema-generator-6.2.2.jar:6.2.2]
		at com.expediagroup.graphql.generator.execution.FunctionDataFetcher.get(FunctionDataFetcher.kt:58) ~[graphql-kotlin-schema-generator-6.2.2.jar:6.2.2]
		at graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation.lambda$instrumentDataFetcher$0(DataLoaderDispatcherInstrumentation.java:86) ~[graphql-java-19.1.jar:?]

[Yannick Lazzari]

Changing the id field in the data class to a String instead, it actually works.

[Dariusz Kuc]

guessing its https://github.com/ExpediaGroup/graphql-kotlin/blob/master/generator/graphql-kotli[…]iagroup/graphql/generator/execution/ConvertArgumentValueTest.kt

[Dariusz Kuc]

its an issue with inline value classes

[Yannick Lazzari]

I also saw issue 1528 which I thought may help (though I didn't it would), so I updated to that version but same error

[Yannick Lazzari]

Is there way to declare the field as string in the code but expose the type as ID instead, such that we can keep the same schema and adjust the implementation?

[Dariusz Kuc]

can you bump up your Kotlin version to 1.7 and see if it works

[Yannick Lazzari]

I thought from the the info further changes needed to be done in the codebase of GraphQL Kotlin to leverage Kotlin 1.7 and then we could upgrade too. But I can try.

[Dariusz Kuc]

Is there way to declare the field as string in the code but expose the type as ID instead, such that we can keep the same schema and adjust the implementation?

pretty sure you could do it through hooks but it will be a manual setup

[Yannick Lazzari]

I'm assuming you are referring to things documented in Custom Types to set that up manually?

[Dariusz Kuc]

i was thinking more about https://opensource.expediagroup.com/graphql-kotlin/docs/schema-generator/customizing-schemas/generator-config#schemageneratorhooks but custom types might work - give it a try

[Yannick Lazzari]

Cool. We'll give all those options a try.

[Yannick Lazzari]

Is there an issue in GraphQL Kotlin that we can watch, that corresponds to the test case you shared above?

[Dariusz Kuc]

we have an issue to update to 1.7 but don't think there is one specifically for inline value classes and 1.6

[Yannick Lazzari]

Ok. So then the issue to watch would be the upgrade to 1.7, which if I understand correctly, should fix this problem at the same time, as in it either fixes itself by virtue of the upgrade, or it's implicit that the maintainers know to fix it as part of that. I guess my point is, is it worth creating a separate issue that simply states the problem with 1.6, which can then be closed by the other which is about upgrading to 1.7? (I hope I'm clear!)

[Dariusz Kuc]

I'd say go ahead and create the issue

[Yannick Lazzari]

Ok. Will do. Thanks a lot for your help @Dariusz Kuc!

[Dariusz Kuc]

underlying issue is https://youtrack.jetbrains.com/issue/KT-27598 which was fixed in 1.7

[Yannick Lazzari]

Yup. It was actually called out in the GraphQL Kotlin docs, so I had seen that (also linked to from the test case code you shared). Thanks!

[Shane Myrick]

I think this might still work today

data class UserInput(
  @GraphQLType("ID")
  id: String,
  firstName: String? = null,
  lastName: String? = null
)

And you shouldn’t have to manually register the ID type since that is a built-in scalar

At least that would work for output, but I don’t know for sure if it would work for input

[Dariusz Kuc]

We dont have a problem with output types :) as for the input -> have no idea how this would be treated by the converter

[Yannick Lazzari]

Spoiler alert: bumping Kotlin to 1.7 and keeping the same GraphQL Kotlin version that we were using (6,2.0) seems to work! I think I'll still create the issue for traceability as there is an issue that people may face. I will let maintainers decide how to best close it.

[Dariusz Kuc]

👍 thanks for letting us know

[Yannick Lazzari]

Here's the issue I created: https://github.com/ExpediaGroup/graphql-kotlin/issues/1564. Hopefully it's clear enough.

[Samuel Vazquez]

kind of late to the party 😄 , we added updating to kotlin 1.7 to the 7.x.x milestone

https://github.com/ExpediaGroup/graphql-kotlin/issues/1549

https://github.com/ExpediaGroup/graphql-kotlin/milestone/5

[Samuel Vazquez]

will find some time to do the update, glad to hear manually overriding the kotlin version in your application solved it 👍 (never had a chance of actually checking it and just trusted the PR when the fixed it)

[Samuel Vazquez]

we might need to update the 6.x.x documentation as well, by mentioning that inline classes wont work neither if your are using an scalar type in an input type with optional fields as siblings

[Yannick Lazzari]

I think it would definitely be useful to include something in the docs to that effect. I don't know that the example that I put in the issue is that uncommon. Surely others will run into it. And it's a simple fix too now that Kotlin 1.7 is officially out, but definitely not the first thing that pops to mind when faced with the error. You just to need know!

[Samuel Vazquez]

this is fixed/merged now, next release will include it

[Yannick Lazzari]

Thanks!