Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Align DispatcherServlet defaults for Java and XML #25209

Closed
sdeleuze opened this issue Jun 8, 2020 · 6 comments
Closed

Align DispatcherServlet defaults for Java and XML #25209

sdeleuze opened this issue Jun 8, 2020 · 6 comments
Assignees
@sdeleuze
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another

Comments

@sdeleuze
Copy link
Contributor

sdeleuze commented Jun 8, 2020 •
edited
Loading

In order to improve GraalVM compatibility (by requiring less reflection configuration) and for the sake of consistency, we should probably register these 4 missing beans in WebMvcConfigurationSupport:

  • org.springframework.web.servlet.i18n.AcceptHeaderLocaleResolver
  • org.springframework.web.servlet.theme.FixedThemeResolver
  • org.springframework.web.servlet.support.SessionFlashMapManager
  • org.springframework.web.servlet.view.DefaultRequestToViewNameTranslator

We should also make sure the same is done on XML side and should then make DispatcherServlet.properties parsing lazy as most use case will not use it.

Finally, we should check that no Spring Boot configuration update is needed.
@sdeleuze sdeleuze added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jun 8, 2020
@sdeleuze sdeleuze added this to the 5.3 M1 milestone Jun 8, 2020
@sdeleuze sdeleuze self-assigned this Jun 8, 2020
This was referenced Jun 8, 2020
Closed
Closed
@sdeleuze sdeleuze mentioned this issue Jun 16, 2020
Closed
sdeleuze added a commit that referenced this issue Jun 16, 2020
@sdeleuze
Make DispatcherServlet.properties loading lazy
e2944c3 
With #25209, DispatcherServlet.properties loading and parsing
will be useless for most of use cases, and it requires
configuration on GraalVM native images.

The purpose of this issue to make such loading and parsing lazy,
only invoked in getDefaultStrategies() if needed.

Closes gh-25257
@mdeinum
Copy link
Contributor

mdeinum commented Jun 16, 2020

I have a branch that includes these additional beans in both Java and XML configuration. I can provide a pull request if you like?

I was still pondering on the lazy loading. This kind of mechanism is used on other places as well, would it make sense to extract to loading of default strategies to a standalone class, for reuse in other places as well?

@sdeleuze
Copy link
Contributor Author

@mdeinum Sure thanks, feel free to create a PR for that. I already implemented lazy loading via e2944c3.

@sdeleuze
Copy link
Contributor Author

After a deeper look I think there is a 4th bean to add: org.springframework.web.servlet.view.DefaultRequestToViewNameTranslator.

mdeinum added a commit to mdeinum/spring-framework that referenced this issue Jun 17, 2020
@mdeinum
Align DispatcherServlet defaults for Java and XML
5a80d5f 
Prior to this commit some of the default strategies defined
for the DispatcherServlet weren't included in the default
configuration for both Java and XML configuration.

The following default beans have been added to the configuration
with the name as expected by the DispatcherServlet:

. org.springframework.web.servlet.i18n.AcceptHeaderLocaleResolver
. org.springframework.web.servlet.theme.FixedThemeResolver
. org.springframework.web.servlet.support.SessionFlashMapManager
. org.springframework.web.servlet.view.DefaultRequestToViewNameTranslator

Fixes: spring-projectsgh-25209
@mdeinum mdeinum mentioned this issue Jun 17, 2020
Closed
@mdeinum
Copy link
Contributor

mdeinum commented Jun 17, 2020

Pull request added.

I considered adding a test to check if all the defaults have been added (1 for XML and 1 for Java), not sure if that would make sense or be helpful?

@sdeleuze sdeleuze changed the title Add missing DispatcherServlet default beans to WebMvcConfigurationSupport Align DispatcherServlet defaults for Java and XML Jun 18, 2020
@sdeleuze
Copy link
Contributor Author

I think this is ok as provided in your PR, thanks.

@wilkinsona wilkinsona mentioned this issue Jun 19, 2020
Closed
kenny5he pushed a commit to kenny5he/spring-framework that referenced this issue Jun 21, 2020
@sdeleuze @kenny5he
Make DispatcherServlet.properties loading lazy
807afc8 
With spring-projects#25209, DispatcherServlet.properties loading and parsing
will be useless for most of use cases, and it requires
configuration on GraalVM native images.

The purpose of this issue to make such loading and parsing lazy,
only invoked in getDefaultStrategies() if needed.

Closes spring-projectsgh-25257
kenny5he pushed a commit to kenny5he/spring-framework that referenced this issue Jun 21, 2020
@mdeinum @kenny5he
Align DispatcherServlet defaults for Java and XML
29ee53f 
Prior to this commit some of the default strategies defined
for the DispatcherServlet weren't included in the default
configuration for both Java and XML configuration.

The following default beans have been added to the configuration
with the name as expected by the DispatcherServlet:

- org.springframework.web.servlet.i18n.AcceptHeaderLocaleResolver
- org.springframework.web.servlet.theme.FixedThemeResolver
- org.springframework.web.servlet.support.SessionFlashMapManager
- org.springframework.web.servlet.view.DefaultRequestToViewNameTranslator

Closes spring-projectsgh-25209
@rstoyanchev rstoyanchev added the status: superseded An issue that has been superseded by another label Jun 22, 2020
@rstoyanchev rstoyanchev removed this from the 5.3 M1 milestone Jun 22, 2020
@rstoyanchev
Copy link
Contributor

Superseded by #25260.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sdeleuze sdeleuze

Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Align DispatcherServlet defaults for Java and XML mdeinum/spring-framework
3 participants
@sdeleuze @rstoyanchev @mdeinum