spring icon indicating copy to clipboard operation
spring copied to clipboard

@PushStateNavigation path registration should be able to exclude paths

Open tsuoanttila opened this issue 8 years ago • 7 comments

Consider the example from vaadin/framework#10578

Vaadin UI with @PushStateNavigation wants to catch all requests that fall under the root path /**. The user wants to also expose a REST API /api/**.

There should be a way to tell the Vaadin path registration to not catch the REST API path. (Or tell Spring that if someone wants to handle something else, it's fine).

tsuoanttila avatar Feb 01 '18 13:02 tsuoanttila

@snicoll any insight on how to configure so that any user defined path take preference over the Vaadin Spring registered ones?

tsuoanttila avatar Feb 01 '18 13:02 tsuoanttila

I am afraid that's not my expertise area but I am sure @bclozel could guide you (you may need to provide more details though).

snicoll avatar Feb 02 '18 09:02 snicoll

How are those REST endpoints defined? Are they implemented using Vaadin infrastructure or are they Spring MVC Controllers?

Spring MVC deals with that using the HandlerMapping beans, ordered. It seems that Vaadin Spring is using a SimpleUrlHandlerMapping, mapped at Integer.MIN_VALUE + 1 (so in front of any other HandlerMapping that could have been defined by Spring MVC) and forwards everything to a ServletForwardingController.

If those REST endpoints are defined in Vaadin, then there's nothing we can do at the Spring MVC level since it's already forwarded to the Vaadin Servlet. If they're defined as Spring MVC Controllers, then I'm wondering how things are supposed to work since the vaadinUiForwardingHandlerMapping is ordered so early.

bclozel avatar Feb 02 '18 09:02 bclozel

Thanks for the quick response. I think the idea of the user is to have for example Rest API of a Repository exposed. So I quess that falls under the Spring MVC dealing with it. So the main question is, how can we fix the too eager registration of Vaadin paths to not step on the toes of Spring?

tsuoanttila avatar Feb 02 '18 10:02 tsuoanttila

So this issue is actually about supporting both Spring MVC and Vaadin in the same Spring Boot application?

If that's the case, this is a much bigger issue, and asks more questions:

  • is there already an existing arrangement with Spring Framework + Vaadin proper?
  • do you intend to allow all Spring MVC-related support as well? (Security, Spring Data REST, etc)
  • is Vaadin leveraging Spring Boot's error handling support (error pages) or using its own? In that case, the Spring MVC parts of an app would deal with errors in a different way than the rest
  • is Vaadin serving static resources by itself? Do you intend to leverage Spring MVC resource handling features?

You could try ordering your SimpleUrlHandlerMapping at 1 instead of Integer.MIN_VALUE + 1 (so right after 0, which is the default position used by the RequestMappingHandlerMapping that maps requests to controllers).

By doing so:

  • you wouldn't be leveraging Spring MVC's resource handling features since the resource handler mapping is ordered quite late, at Integer.MAX_VALUE -1
  • you'd still expose existing applications to more possible issues; if any HandlerMapping ordered before yours is mapped to a more general pattern than yours, there's no guarantee that the request will be handled by Vaadin. For example, Spring MVC's static resources handler will return HTTP 404 if it can't find a resource, effectively denying all chances to the following handler mappings (if mapped to the same pattern)

If until now you didn't allow Spring MVC living next to Vaadin in Spring Boot apps, this is a rather important change and requires a lot of testing and thinking, since the ecosystem around Spring MVC is huge and allowing it in Vaadin apps might have unintended consequences.

bclozel avatar Feb 02 '18 13:02 bclozel

That's pretty much the answer I was afraid of and actually expecting. I'll need to test a few example projects to see how much that change would affect. If the user is not using the PushState navigation feature mentioned here, the mapped path for UIs are not so much of the catch-all kind, but more specific and then Spring parts will manage most error messages anyway and the use of APIs from Spring is quite easy.

Will consult some of our more Spring oriented people and get their opinions and come up with some plan. Any larger integration changes will most likely not end up in this spring integration package since we have a new version in the making. Will remind them to pay attention to this issue as well to do it better.

tsuoanttila avatar Feb 02 '18 13:02 tsuoanttila

@tsuoanttila I think this is also the bug that affects using Spring Security in FW8 Bakery?

johannesh2 avatar Apr 20 '18 11:04 johannesh2