Skip to content

Code-first support of annotated controllers #526

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

Closed
itzg opened this issue Nov 7, 2022 · 5 comments
Closed

Code-first support of annotated controllers #526

itzg opened this issue Nov 7, 2022 · 5 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@itzg
Copy link

itzg commented Nov 7, 2022

With modern Spring, especially with Spring Boot, development of external application interfaces is very commonly declared via Java/annotation based configuration, such as Spring MVC and annotation-based WebFlux. Robust OpenAPI schema generation can be performed with Springfox.

As such, the presence of annotated controller support gave the initial impression that a familiar, Spring-like code-first approach was supported. As of https://github.com/spring-projects/spring-graphql/releases/tag/v1.0.2 that is not the case -- Spring Boot auto-configuration of GraphQL is completely disabled if GraphQL schema files are not present.

I see there has been various discussions and offered application solutions for code-first:

The solutions appear to be fairly complex and indirectly derive the schema from the runtime wiring. The mechanics of constructing the runtime wiring by the AnnotatedControllerConfigurer seem more appealing since it is already traversing the surface of type-safe declarations of GraphQL integration points.

FWIW, I encountered this "limitation" during an exercise to introduce GraphQL into our product and use the exercise as an opportunity to steer the platform away from Quarkus to Spring. It turns out that Quarkus has first-class (and only) support for code-first declaration of GraphQL, which is built upon Smallrye GraphQL.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 7, 2022
@bclozel bclozel added the status: waiting-for-feedback We need additional information before we can continue label Nov 7, 2022
@bclozel
Copy link
Member

bclozel commented Nov 7, 2022

Hello @itzg

As explained in this comment, we expect developer to contribute a GraphQlSource bean to the application context if they wish take control over how the schema is created. Now "code first approach" can cover a lot of different solutions depending on the opinion.

As you've pointed out, AnnotatedControllerConfigurer looks at @SchemaMapping annotations and registers data fetchers accordingly. But I don't think this currently ensures that the arrangement is complete for the schema schema.

FWIW, I encountered this "limitation" during an exercise to introduce GraphQL into our product and use the exercise as an opportunity to steer the platform away from Quarkus to Spring. It turns out that Quarkus has first-class (and only) support for code-first declaration of GraphQL, which is built upon Smallrye GraphQL.

As you've probably seen, Spring for GraphQL did not choose the code first approach and we don't think we should revisit this choice.

It would be interesting indeed to see how a solution similar to Springfox would work in this case. I'm not sure our annotation model would be enough to generate a complete and consistent schema. What have you tried so far? Have you found limitations or extension points that we could provide to make this use case easier?

@itzg
Copy link
Author

itzg commented Nov 7, 2022

Thank you for your prompt response @bclozel !

As you've pointed out, AnnotatedControllerConfigurer looks at @schemamapping annotations and registers data fetchers accordingly. But I don't think this currently ensures that the arrangement is complete for the schema schema.

Agreed, it's not complete since it doesn't (yet) extract the property level structure of the return type, arguments and similar introspection.

As you've probably seen, Spring for GraphQL did not choose the code first approach and we don't think we should revisit this choice.

That's reasonable. Mainly I wanted to find out if this was a "forever choice" or just a not-in-the-next major release or two.

It would be interesting indeed to see how a solution similar to Springfox would work in this case. I'm not sure our annotation model would be enough to generate a complete and consistent schema.

Agreed. More annotations would be needed to convey the deeper/precise intent. Basically I was hoping/expecting the concepts in graphql-spring-annotations were to be folded into spring-graphql. That project seems to be quite out of date, so it itself didn't not seem viable for a production grade requirement.

What have you tried so far? Have you found limitations or extension points that we could provide to make this use case easier?

Admittedly not much and also admittedly it was the graphql-java setup of the TypeRegistry and its relationship to a pristine GraphQLSchema that blocked me more. I hadn't yet looked at all of the linked samples that those other issue authors mentioned, so I still have some homework to do.

My only specific suggestions at this point are

  • some way to further hook into and extend the AnnotatedControllerConfigurer such as getting at the MappingInfo gathered by the findHandlerMethods()
  • not for this project, but FYI the Spring Boot starter project's GraphQlAutoConfiguration has a @ConditionalOnGraphQlSchema that assumes GraphQlSource.schemaResourceBuilder() will always be used. I was able to "defeat" the negative condition by exposing an empty GraphQlSourceBuilderCustomizer bean to trigger the conditional.

Having said that, perhaps my expectation of using GraphQlAutoConfiguration in some part is where I went off track in the first place. That auto-configuration seems to be misnamed since it is tailored towards schema resource setup. I should probably focus on creating the required beans directly.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 7, 2022
@bclozel
Copy link
Member

bclozel commented Nov 7, 2022

not for this project, but FYI the Spring Boot starter project's GraphQlAutoConfiguration has a @ConditionalOnGraphQlSchema that assumes GraphQlSource.schemaResourceBuilder() will always be used. I was able to "defeat" the negative condition by exposing an empty GraphQlSourceBuilderCustomizer bean to trigger the conditional.

Good point. We discussed that with @rstoyanchev recently. We noticed that the auto-configuration is quite opinionated and doesn't necessarily support the code-first approaches as it could. GraphQlSource.builder() vs. GraphQlSource.schemaResourceBuilder() offer different paths, and the auto-configuration configure additional things with the latter only. We could make this use case easier in the future and promote new concepts to Spring for GraphQL, but at this point we don't have much use cases to work with and this could mean breaking the current API.

As a side note, Spring for GraphQL is very much tailored for schema-first as we think the GraphQL schema must be carefully designed and communicates a lot to the consumers. We don't want to prevent code-first approaches with Spring for GraphQL, but we have limited resources to dedicate to this particular theme, given all the other features requested by the community.

@itzg
Copy link
Author

itzg commented Nov 7, 2022

Thank you for the confirmation and clarification.

@itzg itzg closed this as completed Nov 7, 2022
@rstoyanchev rstoyanchev changed the title [Enhancement] Code-first support of annotated controllers Code-first support of annotated controllers Nov 10, 2022
@bclozel
Copy link
Member

bclozel commented Nov 10, 2022

Thanks @itzg for the follow up replies, I've created spring-projects/spring-boot#33096 to improve the @ConditionalOnGraphQlSchema.

@bclozel bclozel added status: declined A suggestion or change that we don't feel we should currently apply and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

3 participants