Quarkus: GraphQL schema generation does not handle sealed classes

[aritabrito]

Describe the bug
Sealed classes are ignored when generating the schema, causing issues with inner classes with same name.

Expected behavior

sealed class SealedClassA {
       data class Response(val propertyA: String)
}
sealed class SealedClassB {
       data class Response(val propertyB: String)
}

Generated schema:

type Response extends SealedClassA {
       propertyA: String
}
type ClassOne extends SealedClassB {
       propertyB: String
}

Actual behavior
Generated schema:

type Response {
       propertyA: String
}

[quarkusbot]

/cc @phillip-kruger, @jmartisk

[jmartisk]

I don't think that the class being sealed or not is really relevant here. The problem here is that you have two GraphQL types that map to the same name, because we use the unqualified class name by default. Perhaps we should improve the code to detect that and throw an error... Anyway would it help if you differentiated your classes with @Name("CUSTOM-NAME") to mitigate the name clash?

[aritabrito]

The error handling would be a nice thing to add since it's very hard to understand the root cause.
This is the error I currently get: _Validation error of type FieldUndefined_.
I've tried to use the @Name tag, but the name is not being overridden and the class name is the one used in the schema.

[phillip-kruger]

Do you have a reproducer ?

[aritabrito]

@phillip-kruger I don't, sorry

[phillip-kruger]

If you can create one, that would help a lot

[jmartisk]

I'll try and create something (I already did for a bit of experimenting), at least we'll make sure we're all on the same page

[jmartisk]

btw I think you might be the first person ever trying the GraphQL extension with Kotlin... This thing will definitely need some more care.

[jmartisk]

https://github.com/jmartisk/graphql-kotlin-experiment
This is what I understood from the description that you're trying to do. I added the @Name annotations in SealedA.kt and SealedB.kt to demonstrate that after differentiating the two classes, it works properly:

@Name("RESPONSE_A")
class Response : SealedClassA { .... }

@Name("RESPONSE_B")
class Response : SealedClassB { .... }

the schema now is:

type RESPONSE_A {
  propertyA: String
  propertyInResponseA: String
}

type RESPONSE_B {
  propertyB: String
  propertyInResponseB: String
}

Without the annotations, both response classes are wrongly mapped to the same GraphQL type, and thus one of them stops working properly

[aritabrito]

Sorry, I cannot try with your demo at the moment, but will do that later.
The main difference I see is in the model, I have only one file like this, let's say SealedClasses.kt:

sealed class SealedClassA {
   @Name("RESPONSE_A")
   data class Response {
      lateinit var propertyInResponseA: String
   }
}

sealed class SealedClassB {
   @Name("RESPONSE_B")
   data class Response {
      lateinit var propertyInResponseB: String
   }
}

[aritabrito]

I managed to make it work in your demonstration. Thanks!

[jmartisk]

Great to hear it's working. I reported https://github.com/smallrye/smallrye-graphql/issues/482 to fix the issue where we don't throw an error if multiple classes map to the same GraphQL type

[aritabrito]

Anyway, not being able to use the @Name tag on our project, due to not wanting to add new dependencies, it would be nice to have a way of fixing this other than having data classes with different names.

[jmartisk]

@aritabrito Not sure what you mean, if you're using GraphQL then you must have the org.eclipse.microprofile.graphql package available anyway, and @Name is in that package?!

[aritabrito]

It's in two different services, one just for the dtos where I have the sealed classes. That's the one without the package

[phillip-kruger]

You can also use jsonbproperty to name it

[jmartisk]

You can also use jsonbproperty to name it

That AFAIK only affects the output JSON, not the schema, so the schema would still be invalid?

Anyway, I think another problem here is that if a model class is nested, we only use the inner class' name for the type, perhaps we should translate it to OuterClass_InnerClass?? I think that would solve @aritabrito 's problem, the Response classes in the description are nested.
It wouldn't solve the problem when the outer AND inner classes are named the same BUT are in a different package, but that's not the case here AFAICS

[aritabrito]

Yep, that would solve the issue! 👍

[phillip-kruger]

I added a config option that allow the naming strategy a while back, you can even use the full package name, or inner classes can be pretended with their parent class

[phillip-kruger]

Try setting quarkus.smallrye-graphql.auto-name-strategy=MergeInnerClass

[phillip-kruger]

Or Full to get the whole package

[jmartisk]

Oh I didn't know that, ok. Note that this is only available since Quarkus 1.9.0

[aritabrito]

Wow! It worked! Thanks a lot 💯