ExpediaGroup / graphql-kotlin

Libraries for running GraphQL in Kotlin
https://opensource.expediagroup.com/graphql-kotlin/
Apache License 2.0
1.74k stars 351 forks source link

8.0.0 upgrade to 8.1.0 breaks Spring WebMVC Compatibility in Fastjson Implementation #2047

Closed TiKevin83 closed 1 month ago

TiKevin83 commented 1 month ago

Library Version works on 8.0.0, broken on 8.1.0

Describe the bug Before the fastjson upgrade commit (v8.0.0), we were successfully using graphql-kotlin-spring-server in a Spring WebMVC project. After v8.1.0, the added GraphQLServerCodecConfiguration.kt now has an @EnableWebFlux annotation, which conflicts with projects using WebMVC. We use WebMVC in a modern project due to maturity issues with some of the nuances of reactive streams vs traditional threads.

To Reproduce

Build any graphql spring server with an @EnableWebMvcannotation. Will throw error Caused by: java.lang.IllegalStateException: The Java/XML config for Spring MVC and Spring WebFlux cannot both be enabled, e.g. via @EnableWebMvc and @EnableWebFlux, in the same application.

Expected behavior Spring WebMVC can be used alongside graphql-kotlin-spring-server

samuelAndalon commented 1 month ago

can you provide a minimal project with repro steps, are you using servlet 3.1 ?

grantaveryatgfs commented 1 month ago

Hi @samuelAndalon, I'm working on the same issue with @TiKevin83

Here's a minimal reproduction of the issue on 8.1.0: https://github.com/grantaveryatgfs/graphql-kotlin-spring-server-webflux-error-reproduction

To clarify, the issue shows up like this if you don't have spring.main.allow-bean-definition-overriding=true set:

Description:

The bean 'requestMappingHandlerMapping', defined in class path resource [org/springframework/web/reactive/config/DelegatingWebFluxConfiguration.class], could not be registered. A bean with that name has already been defined in class path resource [org/springframework/web/servlet/config/annotation/DelegatingWebMvcConfiguration.class] and overriding is disabled.

Action:

Consider renaming one of the beans or enabling overriding by setting spring.main.allow-bean-definition-overriding=true

If we enable bean overriding, we get the error that he mentioned:

Error creating bean with name 'org.springframework.web.reactive.config.DelegatingWebFluxConfiguration': The Java/XML config for Spring MVC and Spring WebFlux cannot both be enabled, e.g. via @EnableWebMvc and @EnableWebFlux, in the same application.

Thanks.

samuelAndalon commented 1 month ago

I am having a hard time understanding this issue, yes I can repro the @EnableWebMvc vs @EnableWebFlux issue, but I can't even send a graphql request to the project you are sharing, it returns 404, the spring server works with web flux, not with spring web, I am interested in seeing how your mvc project works with our graphql kotlin spring server.

grantaveryatgfs commented 1 month ago

but I can't even send a graphql request to the project you are sharing, it returns 404

Hm good point, I perhaps made the reproduction project a bit too bare bones. I've updated the repo to be fully functional with WebMvc and the GraphQL API, see the README for instructions to reproduce each scenario: https://github.com/grantaveryatgfs/graphql-kotlin-spring-server-webflux-error-reproduction/blob/main/README.md

Screenshot 2024-10-18 at 4 43 17 PM

I am interested in seeing how your mvc project works with our graphql kotlin spring server.

I do see that the library isn't necessarily designed for WebMvc support (https://opensource.expediagroup.com/graphql-kotlin/docs/server/spring-server/spring-overview/#webflux-vs-webmvc), but I hope this shows that it is a very viable use-case. Our codebase started out on WebFlux, but we've since moved away from it once virtual threads became supported by Spring, as that gives us the majority of the technical advantage of WebFlux without the difficulty of the reactive-stream-based programming paradigm.

Feel free to check out this file to see the main modifications we have made to make the library work with a Servlet application: https://github.com/grantaveryatgfs/graphql-kotlin-spring-server-webflux-error-reproduction/blob/main/src/main/kotlin/common/graphql/GraphQLConfiguration.kt

samuelAndalon commented 1 month ago

try https://github.com/ExpediaGroup/graphql-kotlin/releases/tag/8.2.0

samuelAndalon commented 1 month ago

interesting to see graphql-kotlin with mvc, I did perform some benchmarks at some point, web flux with reactor netty vs servlet and virtual threads, netty is still faster and uses less memory.

grantaveryatgfs commented 1 month ago

Thanks, yup https://github.com/ExpediaGroup/graphql-kotlin/releases/tag/8.2.0 works great with our project!

I did perform some benchmarks at some point, web flux with reactor netty vs servlet and virtual threads, netty is still faster and uses less memory.

Interesting, our benchmarking last spring showed the opposite. I'll note that we had multiple other reasons for moving off of WebFlux, including that you only get practical value if your whole application is non-blocking, and the corresponding reactive database systems like R2DBC were not mature enough for our use-case compared to JDBC.