Schema-Builder update Quarkus 3.12 to 3.12 affects schema and startup

[steffenthomasEMP]

Hey smallrye-team,
I start a discussion, but I was ambivalent to create an issue, instead.

While upgrading the quarkus-platform-bom dependency from 3.12.3. to 3.13.3 the application couldn't start. The reason was invalid schema creation by schema-builder.

tl;dr
Reproducer: https://github.com/steffenthomasEMP/graphql-quarkus3.13
There are 3 branches.

• 'main' is broken,
• 'working_version' is working but with version 3.12.3,
• 'working_version_3.13' is working with version 3.13.3 but with annotation changes for schema building.

And here is the Log-Output for the broken schema definition that was bit confusing (in main branch):

Caused by: graphql.schema.validation.InvalidSchemaException: invalid schema:
object type 'LandingPageResult' does not implement interface 'ResultBase' because field 'diagnostics' is defined as 'Diagnostics' type and not as 'Diagnostics!' type

long story
We've build a rather complex schema with a lot of type inheritances. In the reproducer I've tried to break it down to the core part of the problem. Also, there is magic lombok involved for creating a type hierarchie of builders (it is historically grown and we are not super happy with this solution any more...but anyways).
In branch 'working_version' with version 3.12.3 of quarkus-platform (and thus 2.8.6 of smallrye-graphql, with graphql-java in version 21.3) the schema is been created as (pruned by irrelevant parts):

interface CategoryQueryResult implements QueryResult {
  "Diagnostics of the search result."
  diagnostics: Diagnostics!
}

interface QueryResult {
  "Diagnostics of the search result."
  diagnostics: Diagnostics!
}

interface ResultBase implements QueryResult {
  "Diagnostics of the search result."
  diagnostics: Diagnostics!
  id: String!
}

type LandingPageResult implements CategoryQueryResult & QueryResult & ResultBase {
  "Diagnostics of the search result."
  diagnostics: Diagnostics!
  id: String!
}

Branch 'working_version_3.13' in contrast (with 2.9.1 of smallrye-graphql and 22.1 of graphql-java) renders following schema:

interface CategoryQueryResult implements QueryResult {
  "Diagnostics of the search result."
  diagnostics: Diagnostics
}

interface QueryResult {
  "Diagnostics of the search result."
  diagnostics: Diagnostics
}

interface ResultBase implements QueryResult {
  "Diagnostics of the search result."
  diagnostics: Diagnostics
  id: ID
}

type LandingPageResult implements CategoryQueryResult & QueryResult & ResultBase {
  diagnostics: Diagnostics!
  id: ID!
}

The ID fields we are fine with...that was kind of wrong in old impl. But now, in order to have a running application, we need to mark the fields in the interfaces as nullable to have these fields non-nullable in the concrete type (see 'diagnostic'-field or 'id' itself). Also the description is not beeing traversed down to the concrete type anymore (which is also unpractical for our needs -> means, c/p the descriptions from interfaces to concrete types).
We doubt that the schema is correct in the latter case (to reference https://graphql.org/learn/schema/#interfaces)

What do you think?
Maybe we also have an implementation issue...maybe we've workarounded schema definitions that has been fixed in newer versions?
I hope, the reproducer can help. If there is anything missing (maybe I pruned too much in code) just let me know.

Best,
Steffen

[mskacelik]

Hi Steffen,

Thanks for starting the discussion! We've analyzed the code and identified the cause of the issue.

Commit fb1c3f3 inadvertently switched the order in which the Jandex API checks methods. Now, it checks interface methods first, followed by class methods.

This change leads to a situation where the @NonNull annotation on a field in a super-interface is overwritten by a field in the subclass that lacks the annotation–since Lombok generates public getters and setters, but the field itself (with the annotation) in the superclass is private, the subclass's getter doesn't inherit the annotation.

Proposed Solution:

We've found a potential solution that fixes the issue in the reproducer. By adding @Getter(onMethod_={@NonNull}) to both fields in the subclass, we can explicitly include the @NonNull annotation on the generated getter methods.

Also, to address a potential type conflict, there needs to be added @Id annotation to ResultBase#getId. The current setup has a type mismatch: String in ResultBase vs. @Id (mapping to an ID type) in LandingPageResult.

[steffenthomasEMP]

Hi Marek,
thank you for the proposed solution, I will try it out. Thank you for your help :)

Best,
Steffen