Skip to content

Deprecate WebApplicationType and introduce alternative #16412

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
bclozel opened this issue Apr 2, 2019 · 5 comments
Closed

Deprecate WebApplicationType and introduce alternative #16412

bclozel opened this issue Apr 2, 2019 · 5 comments
Assignees
Labels
status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@bclozel
Copy link
Member

bclozel commented Apr 2, 2019

The WebApplicationType enum currently allows developers (and Spring Boot itself) to flag an application as a particular type. This can be done automatically given signals such as classpath or a specific choice of context. This can be done manually through setting a configuration property.

This is useful when auto-detection is not picking up the choice expected by the user, for example:

  • Having both spring-boot-starter-web and spring-boot-starter-webflux on the classpath will configure a Spring MVC application; changing that flag allows to switch to a WebFlux application
  • Having spring-boot-starter-web on the classpath will start a server, but developers might just want a simple CLI application without any server

In #16021, the introduction of RSocket will require a new type of application. RSocket applications aren't necessarily web applications:

  • Having both spring-boot-starter-webflux and spring-boot-starter-rsocket will start a WebFlux application and configure an RSocket endpoint on the server
  • Having only spring-boot-starter-rsocket should start a plain RSocket server; this does not qualify as a web application

Because we've got now more experience about this, we should consider now renaming that enum and reorganize our infrastructure around it. The goal behind this type is to say whether an application needs a server and if so, which type of server. Spring Boot is tying the chosen server to the application context.

We could have a new ServerType enum, with:

  • ServerType.SERVLET, or ServerType.SERVLET_CONTAINER?
  • ServerType.REACTIVE, or ServerType.REACTIVE_WEB (as RSocket is reactive too)?
  • ServerType.RSOCKET
  • ServerType.NONE
@bclozel bclozel added type: enhancement A general enhancement for: team-attention An issue we'd like other members of the team to review status: pending-design-work Needs design work before any code can be developed labels Apr 2, 2019
@bclozel bclozel added this to the 2.2.0.M2 milestone Apr 2, 2019
@bclozel bclozel self-assigned this Apr 2, 2019
@bclozel
Copy link
Member Author

bclozel commented Apr 2, 2019

If we apply this change, this should be reflected in #16411 since the SpringBootEnvironment should get that new enum instead.

@wilkinsona
Copy link
Member

If I understand things correctly, RSocket complicates things quite a bit. Previously, the different types were only possible individually. For example, you couldn't have both a reactive and servlet application. With RSocket, you could have a server that is just RSocket, just reactive web, or reactive web and RSocket. The proposed enum doesn't have a value for the last of these three options.

I wonder if we need to be thinking about being able to combine types instead. Perhaps the type needs to be an EnumSet? We'd then need to reject combinations that are not supported such as servlet and reactive.

Anything we do here needs to be considered with #14814 in mind. Combining types doesn't feel like it will help…

@bclozel
Copy link
Member Author

bclozel commented Apr 3, 2019

In the case of RSocket, you can have:

  • an RSocket application, with an RSocket server (using websocket or TCP as transport)
  • a WebFlux application, with a RSocket endpoint (using websocket transport only); IMO, this is rather similar to having a WebFlux application with a standard websocket endpoint. Or a Spring MVC application with websocket and STOMP.

So I do think RSocket is blurring a bit the lines, but a simple enum would still work, as a WebFlux application with RSocket support in it is still a WebFlux application.

@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Apr 5, 2019
@bclozel bclozel added status: declined A suggestion or change that we don't feel we should currently apply and removed status: pending-design-work Needs design work before any code can be developed status: declined A suggestion or change that we don't feel we should currently apply labels Apr 5, 2019
@bclozel
Copy link
Member Author

bclozel commented Apr 5, 2019

Instead of creating a new application type for RSocket and rename the WebApplicationType enum, we'll experiment and see if we can:

  • create the RSocket server as its own bean and not tie it to the application context (like web servers currently)
  • configure a websocket endpoint if a WebFlux server is configured
  • opt-in to create an RSocket server when defining a configuration property

So developers should be able to change Spring Boot's opinion by providing a configuration property for the RSocket port, or setting the WebApplicationType. This should give more freedom and cover all use cases.

@bclozel bclozel removed this from the 2.2.0.M2 milestone Apr 11, 2019
@bclozel bclozel added the status: declined A suggestion or change that we don't feel we should currently apply label Apr 11, 2019
@bclozel
Copy link
Member Author

bclozel commented Apr 11, 2019

Confirmed that the solution described in the last comment works. Closing this issue as a result.

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 type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants