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.

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

Introduce SimpleUrlHandlerMapping constructor that accepts an order and a Map #23362

Closed
joshlong opened this issue Jul 26, 2019 · 5 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@joshlong
Copy link
Member

I use SimpleUrlHandlerMapping for reactive websockets and I either end up instantiating the bean and then calling .setUrlMap(...) and setOrder(...); Or I create an anonymous subclass and call those setters in that. Either way, tedious. Please add a constructor so it can be a one-liner :-)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 26, 2019
@sbrannen sbrannen added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement labels Jul 26, 2019
@sbrannen
Copy link
Member

If we introduce explicit constructors for org.springframework.web.reactive.handler.SimpleUrlHandlerMapping, we should do the exact same for org.springframework.web.servlet.handler.SimpleUrlHandlerMapping.

@sbrannen sbrannen changed the title Please support a constructor for SimpleUrlHandlerMapping that accepts an order and a Map Introduce SimpleUrlHandlerMapping constructor that accepts an order and a Map Jul 26, 2019
@sbrannen sbrannen added this to the 5.2 RC1 milestone Jul 26, 2019
@sbrannen sbrannen removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 26, 2019
@sbrannen sbrannen self-assigned this Jul 26, 2019
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jul 26, 2019

The spring-webmvc variant has a lot more properties which makes picking urlMap and order out of all of them a bit more random. On the other hand if we were to pick one, urlMap does make sense on both sides. Maybe then order can be left to being expressed with @Order and just have a one-arg constructor on both sides with urlMap?

@sbrannen
Copy link
Member

The spring-webmvc variant has a lot more properties which makes picking urlMap and order out of all of them a bit more random. On the other hand if we were to pick one, urlMap does make sense on both sides.

I agree that picking urlMap for both makes sense. Supporting Properties directly in the constructor probably doesn't make much sense for programmatic configuration (i.e., in @Bean methods), since properties are more often used with XML configuration where constructor arguments are likely not used.

Maybe then order can be left to being expressed with @Order and just have a one-arg constructor on both sides with urlMap?

That unfortunately won't work since AnnotationAwareOrderComparator (which is used to sort the handler mappings in DispatcherServlet#initHandlerMappings(ApplicationContext)) gives Ordered precedence over @Order, effectively ignoring @Order for AbstractHandlerMapping implementations such as SimpleUrlHandlerMapping.

I think it would be OK to introduce the following constructors for both types.

  • SimpleUrlHandlerMapping()
  • SimpleUrlHandlerMapping(Map<String, ?> urlMap)
  • SimpleUrlHandlerMapping(Map<String, ?> urlMap, int order)

Thoughts?

@joshlong
Copy link
Member Author

I'd very much like that.

@sbrannen
Copy link
Member

sbrannen commented Jul 28, 2019

FWIW, DefaultServletHandlerConfigurer.buildHandlerMapping(), ViewControllerRegistry.buildHandlerMapping(), and ResourceHandlerRegistry.getHandlerMapping() in Spring MVC could also benefit from a SimpleUrlHandlerMapping(Map<String, ?> urlMap, int order) constructor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants