google / guice

Guice (pronounced 'juice') is a lightweight dependency injection framework for Java 11 and above, brought to you by Google.
https://github.com/google/guice
Apache License 2.0
12.48k stars 1.67k forks source link

Guice can't create async servlet #650

Open gissuebot opened 10 years ago

gissuebot commented 10 years ago

From swapii on August 30, 2011 07:39:51

I've connect Guice to my project, write web.xml and everything gone right until I wan't to realize async servlet. I using Tomcat 7.0.20 and Guice 3.0. Servlets spec version is 3.0.

Test Comet servlet:

@Singleton @WebServlet(name = "comet", urlPatterns = {"/comet"}, asyncSupported = true) public class CometServlet extends HttpServlet {

        private class TestThread implements Runnable {

                private AsyncContext context;

                public TestThread(AsyncContext ctx) {                         context = ctx;                 }

                @Override                 public void run() {                         try {                                 Thread.sleep(5000);                                 context.getResponse().getWriter().write("TEST!");                         } catch (InterruptedException e) {                                 e.printStackTrace();                         } catch (IOException e) {                                 e.printStackTrace();                         }                 }         }

        @Override         protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {                 AsyncContext ctx = req.startAsync(req, resp);                 new Thread(new TestThread(ctx)).start();         }          }

Servlet created, it placed at "/comet" path, but async not working and error was printed:

HTTP Status 500

java.lang.IllegalStateException: Not supported.         org.apache.catalina.connector.Request.startAsync(Request.java:1638)         org.apache.catalina.connector.RequestFacade.startAsync(RequestFacade.java:1031)         javax.servlet.ServletRequestWrapper.startAsync(ServletRequestWrapper.java:379)         javax.servlet.ServletRequestWrapper.startAsync(ServletRequestWrapper.java:379)         ru.unimobi.cinema.server.server.servlets.CometServlet.service(CometServlet.java:46)         javax.servlet.http.HttpServlet.service(HttpServlet.java:722)         com.google.inject.servlet.ServletDefinition.doService(ServletDefinition.java:216)         com.google.inject.servlet.ServletDefinition.service(ServletDefinition.java:141)         com.google.inject.servlet.ManagedServletPipeline.service(ManagedServletPipeline.java:93)         com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:63)         com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:134)         com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:59)         com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:122)         com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:110)

I think this is bug, because I write web.xml directly without Guice and it working as expected.

Original issue: http://code.google.com/p/google-guice/issues/detail?id=650

gissuebot commented 10 years ago

From igor.petrouk on January 20, 2012 12:55:47

Could you please suggest a workaround for this.

gissuebot commented 10 years ago

From igor.petrouk on January 24, 2012 01:00:27

Check out this Issue https://code.google.com/p/google-guice/issues/detail?id=678 . You can use the class I have provided in the patch. It allows to inject Guice managed objects to servlets managed my the container. So you can use your class without @Singleton, just as a regular servlet mapped with annotation. Make sure you have asyncSupport=true in all filters in the path to your servlet

bcmedeiros commented 9 years ago

I'm doing this inside the servlet class:

@WebServlet(value = "/*", asyncSupported=true)
public class PortalServlet extends HttpServlet {
    ...
    @Override
    public void init() throws ServletException {
        Injector injector = (Injector) getServletContext().getAttribute(Injector.class.getName());
        injector.injectMembers(this);
        super.init();
    }

Dirty workaround, I know, but I can't see anything better. You can also extract this code to a common superclass.

nanodeath commented 7 years ago

It's disappointing to discover Guice has decent Servlet integration, only to not be able to use it properly (i.e. constructor injection) due to a 5-year-old bug. Can someone who knows how to fix this please weigh in here? Is there a workaround that allows us to use constructor injection? Or can we get a pointer to the area of code that would need to be changed?

mcculls commented 7 years ago

We're using constructor-injection without any problems, but then we let Guice manage creation of our servlets.

AFAICT the issue here is they want container managed instances (in this case a servlet annotated with @WebServlet created by the container) to be further injected by Guice... so either the container would ask Guice for an instance of the servlet, which would allow constructor injection (and the container would then further configure it as necessary) - or the container would create the instance itself and pass it to Guice for further configuration (via the init method or similar), which would not allow constructor injection.

The second case doesn't require modifications to the container or Guice - simply pass the relevant injector in via the servlet context, retrieve it in the init method, and use injectMembers(this). The first case assumes the container provides a way to hook into how it creates servlet instances, in which case you could write a hook that asks the injector for the instance instead of simply new'ing it. Again this can be done without any changes to Guice (assuming such a container hook exists)

nanodeath commented 7 years ago

Thanks @mcculls. I'm not very familiar with the Servlet API, but there might be a container-specific way to achieve this -- Tomcat has an InstanceManager interface (implemented by DefaultInstanceManager) that might be overridable, and used to construct Servlet instances. However, even if this was possible, such coupling to a particular container implementation is probably a non-starter for people (and rather unpleasant to me).

At this point, I'm thinking that trying to use constructor injection with Servlets is not really using them properly -- using the init() method is the typical route, plus the injectMembers(this) approach you mentioned. Guess that's what I'll do.

That said, I think the original requester's issue was that the Guice's ServletModule() API offers no analog for @WebServlet(asyncSupported=true). That's still something that would be good to see.

cstamas commented 6 years ago

I think this issue, at least it's title is wrong: "Guice can't create async servlet" is not true.

Guice can create async servlet (just don't forget to set <async-supported>true</async-supported> on guice filter in web.xml).

Just FTR, as I got puzzled by this opened issue, while creating a POC that basically proved the opposite of this issue.

atais commented 4 years ago

There is yet another way to register the async servlet in guice context. It works with constructor injection and no annotations.

// initialize all your beans, with servlets
val i = Guice.createInjector(new AbstractModule(){...})

// Guice context listener just needs to register the beans to servlet context
class AppServletContextListener extends GuiceServletContextListener {
  def getInjector: Injector = i.createChildInjector(new ServletRegistrator(i))
}

class ServletRegistration(i: Injector) extends ServletModule with ScalaServletModule {
  override def configureServlets(): Unit = {
    serve("/sync-home").`with`(classOf[SyncHomeServlet])

    // async
    val registration = getServletContext.addServlet(classOf[AsyncHomeServlet].getSimpleName, classOf[AsyncHomeServlet])
    registration.setAsyncSupported(true)
    registration.addMapping("/async-home")
  }
}