vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
607 stars 166 forks source link

Including flow server as a dependency breaks custom spring webmvc configuration #9005

Closed masc3d closed 3 years ago

masc3d commented 4 years ago

Description of the bug

Merely including flow-server as a dependency breaks custom spring config via @EnableWebMvc

Minimal reproducible example

Here's a custom webmvc example for serving static resources

@Configuration
@Lazy(false)
@EnableWebMvc
class WebConfiguration : WebMvcConfigurer {
    override fun addResourceHandlers(registry: ResourceHandlerRegistry) {
        super.addResourceHandlers(registry.apply {
            // resource handler for static swagger-ui
            addResourceHandler("/rs/**")
                .addResourceLocations("classpath:/webapp/rs/")

            // resource handler for images
            addResourceHandler("/img/**")
                .addResourceLocations("classpath:/webapp/img/")

            // resource handler for icons
            addResourceHandler("/icon/**")
                .addResourceLocations("classpath:/webapp/icon/")
        });
    }

    override fun addViewControllers(registry: ViewControllerRegistry) {
        super.addViewControllers(
            registry.apply {
                // redirect(s) to index-html
                addRedirectViewController("/rs", "/rs/index.html")
                addRedirectViewController("/rs/", "/rs/index.html")
                addRedirectViewController("/rs/internal", "/rs/internal/index.html")
                addRedirectViewController("/rs/internal/", "/rs/internal/index.html")
            }
        )
    }
}

Expected behavior

Custom webmvc config should not be adversely affected. Also, when not including vaadin-spring or excluding its auto configuration, vaadin should not interfere with spring at all.

Actual behavior

Just by including flow-server as a dependency custom webmvc setup stops working and fetching a static resource will yield:

Resolved [org.springframework.web.HttpRequestMethodNotSupportedException: Request method 'GET' not supported]
15:07:18.006 [mt-6] DEBUG o.s.web.servlet.DispatcherServlet - Completed 405 METHOD_NOT_ALLOWED

Versions:

- Vaadin / Flow version: 17.0.1
- Java version: 14.0.2
- OS version: macos-10.15.6
- Browser version (if applicable): -
- Application Server (if applicable): -
- IDE (if applicable): IJ

Sidenotes

haijian-vaadin commented 4 years ago

@masc3d do you have an example project that we can use to reproduce the problem?

masc3d commented 4 years ago

sure. https://github.com/masc3d/vaadin-issue-9005

start via :bootRun then go to http://localhost:8080/img/yes.png will yield this error. remove flow-server from build.gradle.kts and it will work.

most simple vaadin project generated via https://start.spring.io is broken as well. seems like spring support needs plenty of love.

masc3d commented 4 years ago

what is ccdm @haijian-vaadin

haijian-vaadin commented 4 years ago

Hi @masc3d, ccdm stands for the new Vaadin for TypeScript programming model that we introduced in Vaadin 15, basically in addition to Java, it allows you to create UI in TypeScript (A bit more details here in this blog post if you are interested).

The current implementation relies on Spring, it has a Spring controller to handle the auto generated client side post calls, that's most likely the reason it breaks the WEBMVC configuration for you.

Thank you so much for your example application, just that I'm not familiar with kotlin and gradle. Could you help to describe what exactly is the problem you are having, is it that the application cannot start or the images cannot be loaded or sth else?

most simple vaadin project generated via https://start.spring.io is broken as well.

This sounds very serious, can you help to elaborate what configuration do you use there and how is it broken?

masc3d commented 4 years ago

Could you help to describe what exactly is the problem you are having, is it that the application cannot start or the images cannot be loaded or sth else?

the latter. you can run the sample project with ./gradlew :bootRun on command line within project folder. (or gradle.bat :bootRun on windows respectively)

masc3d commented 4 years ago

Hi @masc3d, ccdm stands for the new Vaadin for TypeScript

I don't see how this issue relates to typescript or even UI in any way.

masc3d commented 3 years ago

could you please correct the tags for this issue @haijian-vaadin.

this is not a ccdm issue, but a bug for vaadin-spring which qualifies for at least severity major and blocker for any spring project which requires a custom spring webmvc config, which is not that uncommon.

haijian-vaadin commented 3 years ago

Hi @masc3d, sorry for getting back late to this. Sure, I will remove the tag and let someone with better knowledge on Spring MVC, Kotlin, and Gradle to take a look at the issue.

caalador commented 3 years ago

I tested the given WebMvcConfiguration on a SpringBoot project downloaded from start.spring.io and noted that this only seems to fail when running the kotlin version. Having the project as Java or Groovy with the exact same pieces the image is served, only in the kotlin version is the RouteNotFoundError page show.

@mvysny do you have any idea about this as I have no experience with kotlin?

Attached the 3 demos (mvn spring-boot:run for each) java_demo.zip groovy_demo.zip kotlin_demo.zip

mvysny commented 3 years ago

I was able to reproduce this behavior with the plain Java and Maven project, when running the app from Intellij via running the Application.main() method:

skeleton-starter-flow-spring.zip

Navigating to http://localhost:8080/img/tesla.jpg yields an error page:

Whitelabel Error Page

This application has no explicit mapping for /error, so you are seeing this as a fallback.
Fri Oct 09 10:22:20 EEST 2020
There was an unexpected error (type=Method Not Allowed, status=405).
Request method 'GET' not supported
org.springframework.web.HttpRequestMethodNotSupportedException: Request method 'GET' not supported
    at org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping.handleNoMatch(RequestMappingInfoHandlerMapping.java:213)
    at org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.lookupHandlerMethod(AbstractHandlerMethodMapping.java:422)
    at org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.getHandlerInternal(AbstractHandlerMethodMapping.java:367)
    at org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping.getHandlerInternal(RequestMappingInfoHandlerMapping.java:110)
    at org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping.getHandlerInternal(RequestMappingInfoHandlerMapping.java:59)
    at org.springframework.web.servlet.handler.AbstractHandlerMapping.getHandler(AbstractHandlerMapping.java:395)
    at org.springframework.web.servlet.DispatcherServlet.getHandler(DispatcherServlet.java:1234)
    at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1016)
    at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:943)
    at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006)
    at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:626)
    at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:733)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53)
mvysny commented 3 years ago

@caalador I think you used Vaadin 14.x while the reporter uses Vaadin 17.x. Also the Kotlin compiler doesn't seem to be configured properly in pom.xml, which causes the Kotlin classes to not to be compiled at all, which causes them to not to take any effect in the runtime.

caalador commented 3 years ago

Good point. Living on the edge makes on expect that any download would use SNAPSHOT versions =_=

caalador commented 3 years ago

But yes. using the Java_demo and updating the version to 18.0-SNAPSHOT makes the serving of the file from localhost:8080/img/yes.png fails with the exception

Whitelabel Error Page
This application has no explicit mapping for /error, so you are seeing this as a fallback.

Fri Oct 09 12:11:43 EEST 2020
There was an unexpected error (type=Method Not Allowed, status=405).

Due to the GET request being not allowed. Removing @EnableWebMvc and setting the registry precedence does fix the issue e.g. registry.setOrder(Ordered.HIGHEST_PRECEDENCE);, but the ordering then again breaks Vaadin at the same time as it fixes the static file serving.

knoobie commented 3 years ago

Could this be related to https://github.com/vaadin/spring/issues/602?

caalador commented 3 years ago

I feel the problem is the same as vaadin/spring#602 but the main issue here is that having only flow-server without the Spring add-on breaks the mvc functionality. Dropping the @EnableWebMvc annotation makes it work when there is only flow-server, but I guess this is now a problem due to the VaadinConnectController etc. which after 15 (flow-server 3.0) bring in spring dependencies in the provided scope. @haijian-vaadin would there be some possibility to move these away from the server as they are quite Spring specific. minimal.zip

haijian-vaadin commented 3 years ago

@caalador unfortunately no. The whole Endpoint concept since Vaadin 15 (flow-server 3.0) depends on Spring.

mvysny commented 3 years ago

The whole Endpoint concept since Vaadin 15 (flow-server 3.0) depends on Spring.

Endpoint? As in the backend for Vaadin Fusion? @haijian-vaadin Does that mean that Fusion requires Spring and won't work with plain Servlets?

haijian-vaadin commented 3 years ago

@mvysny yes, that's the current implementation.

mvysny commented 3 years ago

Luckily the new implementation will be made independent from Spring; that could also hopefully fix this issue.

haijian-vaadin commented 3 years ago

This indeed is related to our Vaadin for TypeScript feature, the solution could be that we register the VaadinConnectController conditionally (when there is an Endpoint). I will make a PR for this.

haijian-vaadin commented 3 years ago

Hi @masc3d, can you try to remove the @EnableWebMvc annotation and see it works for you? I tried the minimal project provided by @caalador, seems it's working that way.

After a bit of debugging and researching, I found that in a Spring Boot project, you don't need to add @EnableWebMvc explicitly, it's done in Spring Boot via EnableWebMvcConfiguration. Actually having a @EnableWebMvc annotation will disable the auto-configuration in Spring Boot(see this article). Since it seems you are also using Spring Boot, I'd recommend removing the @EnableWebMvc annotation unless you have a strong reason to keep it.

Why it's breaking the image request is that Vaadin registers a RestController for posting mapping with path /{endpoint}/{method}, and also a configuration class to modify the mapping with a configurable prefix (default value is connect). So normally in a Spring Boot project, the RestController provided by Vaadin maps to connect/{endpoint}/{method}, which will rarely conflict with other handlers. But when adding @EnableWebMvc into the application, Spring Boot auto-configuration is disabled, so the prefix is not added, and the request to img/yes.png happens to match /{endpoint}/{method}, that's why you get the Method Not Allowed error 😂. You can verify that by putting you img file one layer deeper e.g. img/abc/yes.png, and the request the img (http://localhost:8080/img/abc/yes.png) will work.

masc3d commented 3 years ago

yes, may work for simple examples, but compat with @EnableWebMvc would still be preferable for those who require it, as it's also suggested in spring-boot docs

masc3d commented 3 years ago

does it fix @EnableWebMvc? otherwise I would open another issue for it @haijian-vaadin

haijian-vaadin commented 3 years ago

@masc3d, it works if you don't use Vaadin endpoint and @EnableWebMvc at the same time. We will document it as a limitation that our Endpoint for now depends on Spring Boot auto-configuration.

masc3d commented 3 years ago

how would I migrate to >vaadin-14 then in a project which requires @EnableWebMvc?

masc3d commented 3 years ago

instead of documenting the limitation, it would presumably be more helpful to document how vaadin can be setup with @EnableWebMvc @haijian-vaadin

haijian-vaadin commented 3 years ago

Hi @masc3d, for migrations, @EnableWebMvc should work just fine. What doesn't work now is when you start using @Endpoint explicitly. e.g. having a class like

@Endpoint
@Service
public class ServiceEndpoint {
  public hello(){
  }
}

@Endpoint is for supporting the new TypeScript UIs, if you don't have that, everything should work the same as before.

masc3d commented 3 years ago

thanks @haijian-vaadin, I will check. which version(s) of vaadin are going to have the fix?

haijian-vaadin commented 3 years ago

In the coming Vaadin 17/18 releases (17.0.8, 18.0.0.alpha2), the new 17.0.8 release could be expected next week.

haijian-vaadin commented 3 years ago

Hi @masc3d, just to check, do you have vaadin-spring in your real project?

masc3d commented 3 years ago

Hi @masc3d, just to check, do you have vaadin-spring in your real project?

yes

haijian-vaadin commented 3 years ago

Hi @masc3d, an update, we just released a new version of Flow 4.0.4 with the fix to this issue. A new Vaadin 17 release is coming next week.

masc3d commented 3 years ago

I can confirm vaadin-17.0.8 appears to work well with @EnableWebMvc. that's great @haijian-vaadin, thanks everyone

haijian-vaadin commented 3 years ago

That's great to hear, thanks again for your feedback @masc3d.

nero404 commented 2 months ago

I have this problem in 24.4.6 when im using @EnableWebMvc, 24.3.X is fine. Any workaround?

knoobie commented 2 months ago

Excluding Hilla might work

nero404 commented 2 months ago

Thx for answer. I sloved my problem by using vaadin.exclude-urls