search

line / armeria

Your go-to microservice framework for any situation, from the creator of Netty et al. You can build any type of microservice leveraging your favorite technologies, including gRPC, Thrift, Kotlin, Retrofit, Reactive Streams, Spring Boot and Dropwizard.
https://armeria.dev
Apache License 2.0
4.81k stars 915 forks source link

`armeria-spring-boot3-starter` does not work when the dependency includes `spring-webflux` #4873

Open be-hase opened 1 year ago

be-hase commented 1 year ago

Overview

armeria-spring-boot3-starter does not work when the dependency includes spring-webflux.

This problem occurs when using armeria-spring-boot3-starter (also armeria-spring-boot2-starter) in conjunction with a library that uses Spring WebFlux's WebClient.

Why

ArmeriaAutoConfiguration is disabled when WebApplication.Type is REACTIVE. https://github.com/line/armeria/blob/main/spring/boot3-autoconfigure/src/main/java/com/linecorp/armeria/spring/ArmeriaAutoConfiguration.java#L43

On the other hand, spring boot sets WebApplication.Type as REACTIVE under the following conditions

I tried to submit a PR to remove the NonReactiveWebApplicationCondition, but this looks like it is needed for the armeria-spring-boot3-webflux-starter.

Therefore, I will first make an ISSUE and ask for feedback.

Simple Reproduction Code

https://github.com/be-hase/armeria-issues-202305/tree/main/example/reactive-condition-issue

minwoox commented 1 year ago

Hi! Thanks for the report. I have a couple of questions:

no servlet is in the dependencies spring-webflux is in the dependencies

be-hase commented 1 year ago

Is this an and condition that means WebApplication.Type is set to REACTIVE when both conditions are met?

yes. I think this spring code would be helpful. https://github.com/spring-projects/spring-boot/blob/87a704369234db614ea62d0453b7df7f39d8981c/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/WebApplicationType.java#L60-L71

Does this work in the same way for the Spring starter?

Sorry. Which spring starter are you intending?

To add a little more detail, you can workaround by explicitly specifying WebApplication.Type as None as follows. https://github.com/be-hase/armeria-issues-202305/blob/main/example/reactive-condition-issue/src/main/resources/application.yml#L2

ikhoon commented 1 year ago

NonReactiveWebApplicationCondition has been added to give priority to REACTIVE when both armeria-boot-autoconfigure and armeria-boot-webflux-autonconfigure are in the dependencies. #3102 However, looking at the problem presented, NonReactiveWebApplicationCondition doesn't cover all corner cases.

As you said Spring WebApplicationType determined according to the existence of reactive.DispatcherHandler is used to choose the server type even if the server type is not used. It is possible that users do not want to use the dependency for a server but just add it for dependencies.

To resolve this ambiguity, I propose to add armeria.server.type properties to ArmeriaSettings so that explicitly users choose their server type if the two auto-configurations are conflicted, for example:

armeria.server.type: [reactive | mvc]
jrhee17 commented 1 year ago

To resolve this ambiguity, I propose to add armeria.server.type properties to ArmeriaSettings

I think the only down-side with this approach is that users might want to set up two spring servers (one mvc and one webflux) on the same jvm (but with different ports). In this case, I guess the armeria integration can't be used for both server types. Having said this, I guess this case should be rare enough so we can worry about this later if this is actually an issue.

be-hase commented 1 year ago

How about including in the auto configuration condition whether or not the ArmeriaReactiveWebServerFactory is present in the classpath?

ikhoon commented 1 year ago

I think the only down-side with this approach is that users might want to set up two spring servers (one mvc and one webflux) on the same jvm (but with different ports).

MVC and WebFlux integration have their own server lifecycle. We need to integrate the two modules to use only one server instance to support it. It may be handled in a separate issue. https://github.com/line/armeria/blob/dbbe4ca0dea6bf776231d2ec04644235d98ab43e/spring/boot3-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaReactiveWebServerFactory.java#L130 https://github.com/line/armeria/blob/f2116739018c15f230dd8f0e5a4dd5db1fefc888/spring/boot3-autoconfigure/src/main/java/com/linecorp/armeria/spring/ArmeriaServerGracefulShutdownLifecycle.java#L31-L31

How about including in the auto configuration condition whether or not the ArmeriaReactiveWebServerFactory is present in the classpath?

It is a possible solution but the auto-configuration is working on an implicit convention and users can't override the default behavior.

It should be nicer to provide a customization point to override the default behavior.