vaadin / spring

Spring integration for Vaadin
https://vaadin.com/start
177 stars 101 forks source link

How to use SpringServlet with web.xml and custom WebApplicationInitializer #560

Closed mvysny closed 4 years ago

mvysny commented 4 years ago

EDIT: since the usecase case is quite specific I think it has no sense to cover all such cases. It should be enough to provide a proper javadocs for the second parameter of SpringServlet. This is definitely a bug and needs to be fixed. Also please refer to https://github.com/vaadin/spring/issues/560#issuecomment-586204776: extend javadocs with explanation that SpringServlet is not supposed to be used directly.

Previously with Vaadin 8, the SpringServlet had zero-arg constructor and would look up ApplicationContext via WebApplicationContextUtils.getWebApplicationContext(). However, the new SpringServlet class in the new version of vaadin-spring (namely 12.1.2) always requires two parameters: ApplicationContext and boolean.

Please provide an example project using web.xml and Spring Security filter for this use-case.

denis-anisimov commented 4 years ago

Is it a real common usecase to use SpringServlet ? SpringServlet has always been considered as an internal class and never supposed to be used directly. If you don't use Spring boot app where the servlet is instantiated for you out of the box then you may use Spring MVC to do this. And you don't have to care about SpringServlet again:

https://github.com/vaadin/flow-and-components-documentation/blob/0cf34180120ec3531bc9dc521f25a9f08b1fbe5a/documentation/spring/tutorial-spring-basic-mvc.asciidoc

Do you have another usecase which can't be covered by these two cases?

mvysny commented 4 years ago

Yes, I have a customer with long web.xml and a custom servlet, and we're migrating using MPR to Vaadin 14.

Current solution is to have

public class MyWebApplicationInitializer extends AbstractContextLoaderInitializer {

    @Override
    protected WebApplicationContext createRootApplicationContext() {
        XmlWebApplicationContext context = new XmlWebApplicationContext();
        context.setConfigLocation('/WEB-INF/applicationContext.xml');
        ApplicationServlet.APP_CONTEXT = context;
        return context;
    }
...

and then

public class ApplicationServlet extends SpringServlet {

    ApplicationServlet() {
        super(APP_CONTEXT, true);
    }

    public static volatile ApplicationContext APP_CONTEXT;
}
denis-anisimov commented 4 years ago

So your usecase is the real need to have a customized SpringServlet ? I think it's quite specific and rare usecase......

The reason why I ask is: as I said SpringServlet has never been considered to use directly. That's why it has a CTOR which is not really obvious.

And the reason why this CTOR has two parameters: avoid code duplication. The second parameter has been added at some point because of fix to one issue. In the previous version it is not used : https://github.com/vaadin/spring/blob/10.0/vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringServlet.java#L53

The one -arg CTOR is preserved to keep backward compatibility . But this is a code duplication which I didn't like and decided to have only two arg CTOR since the value of parameter is known in the code where the servlet is instantiated.

The problem is : for some reason the introduced parameter is not documented . Apparently it's my bad though I remember that I spent some time to write cumbersome javadocs for this parameter. Don't know why they disappeared in the end.

So we definitely should document the parameter.

I'm not sure about the example though: it should be possible to solve anything without extending SpringServlet , so I don't consider this as a common usecase. On the other hand if it's strongly needed then:

So I don't think an example is a good solution here. It should be solved somehow in other way.

mvysny commented 4 years ago

So your usecase is the real need to have a customized SpringServlet ? I think it's quite specific and rare usecase......

My use-case is to instantiate Vaadin's SpringServlet from web.xml, exactly as it was possible to do so from Vaadin 8. The customer is not using Spring Boot.

as I said SpringServlet has never been considered to use directly.

No problem. What is the suggested way to bootstrap Spring-specific VaadinServlet with Spring-specific Instantiator and other Spring-specific things from web.xml please?

denis-anisimov commented 4 years ago

My use-case is to instantiate Vaadin's SpringServlet from web.xml, exactly as it was possible to do so from Vaadin 8. The customer is not using Spring Boot.

https://docs.spring.io/spring/docs/current/spring-framework-reference/web.html#mvc-container-config says:

In a Servlet 3.0+ environment, you have the option of configuring the Servlet container programmatically as an alternative or in combination with a web.xml

https://github.com/vaadin/flow-and-components-documentation/blob/0cf34180120ec3531bc9dc521f25a9f08b1fbe5a/documentation/spring/tutorial-spring-basic-mvc.asciidoc

shows how to extend VaadinMVCWebAppInitializer which should instantiate SpringServlet out of the box regardless of web.xml .

As a result web.xml is technically is an extra detail here. According to Spring docs VaadinMVCWebAppInitializer should work with or without web.xml.

If you don't need a custom SpringServlet then there is already a way to do what you want.

mvysny commented 4 years ago

Thank you for your kind reply :+1:

Unfortunately the customer has other servlets and filters in the web.xml, not just Vaadin ones, so it is not possible to remove web.xml.

The customer already has his own MyWebApplicationInitializer which extends AbstractContextLoaderInitializer and creates the XmlWebApplicationContext(). Is it possible to use this custom MyWebApplicationInitializer side-by-side with VaadinMVCWebAppInitializer please?

EDIT: Ah right, I could simply add the dynamic servlet registration into MyWebApplicationInitializer as follows perhaps:

public class MyWebApplicationInitializer extends AbstractContextLoaderInitializer {

    @Override
    protected WebApplicationContext createRootApplicationContext() {
        XmlWebApplicationContext context = new XmlWebApplicationContext();
        context.setConfigLocation('/WEB-INF/applicationContext.xml'); 
        ServletRegistration.Dynamic registration =
            servletContext.addServlet("dispatcher",
                new SpringServlet(context, true));
        registration.setLoadOnStartup(1);
        registration.addMapping("/*");
        return context;
    }
}
denis-anisimov commented 4 years ago

Unfortunately the customer has other servlets and filters in the web.xml, not just Vaadin ones, so it is not possible to remove web.xml.

Hm, I never said that web.xml should be removed. It should be possible to use VaadinMVCWebAppInitializer with web.xml or without it.

But I don't know whether this is possible to use VaadinMVCWebAppInitializer along with MyWebApplicationInitializer.

denis-anisimov commented 4 years ago

In the latest example you still need to instantiate SpringServlet and you need to know the second parameter meaning.

Again: this is quite specific corner case .

I think it should be enough to provide javadocs for the second parameter of SpringServlet.

mvysny commented 4 years ago

The javadocs for second parameter sounds good. However, a programmer coming from Vaadin 8 expects that SpringServlet is public. It would be really great to clarify this in SpringServlet's javadoc that the class is not public anymore and cannot be used from web.xml anymore, and provide link to further documentation.

denis-anisimov commented 4 years ago

That makes sense.

archiecobbs commented 2 years ago

FYI for anyone interested in this issue, I'm working on a servlet for Vaadin+Spring that doesn't require Spring Boot and supports traditional web.xml, etc.

It's still a work in progress but is functional. Source code is here.

joelpop commented 1 month ago

FYI for anyone interested in this issue, I'm working on a servlet for Vaadin+Spring that doesn't require Spring Boot and supports traditional web.xml, etc.

It's still a work in progress but is functional. Source code is here.

Updated URL: https://github.com/archiecobbs/dellroad-stuff/tree/2.x-branch/vaadin/vaadin23/src/main/java/org/dellroad/stuff/vaadin23/servlet