Support for multiple ServletModules/GuiceFilters and injectors in the same JVM.

Maintains backwards compatibility with apps that don't use GuiceServletContextListener and warns where appropriate.

git-svn-id: https://google-guice.googlecode.com/svn/trunk@1187 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/servlet/src/com/google/inject/servlet/GuiceFilter.java b/servlet/src/com/google/inject/servlet/GuiceFilter.java
index bd2a773..7a3d6ea 100644
--- a/servlet/src/com/google/inject/servlet/GuiceFilter.java
+++ b/servlet/src/com/google/inject/servlet/GuiceFilter.java
@@ -18,7 +18,6 @@
 
 import com.google.inject.Inject;
 import com.google.inject.OutOfScopeException;
-import com.google.inject.Stage;
 import java.io.IOException;
 import java.lang.ref.WeakReference;
 import java.util.logging.Logger;
@@ -70,29 +69,22 @@
   static volatile WeakReference<ServletContext> servletContext =
       new WeakReference<ServletContext>(null);
 
-  private static final String MULTIPLE_INJECTORS_ERROR =
-      "Multiple injectors detected. Please install only one"
-          + " ServletModule in your web application. While you may "
-          + "have more than one injector, you should only configure"
-          + " guice-servlet in one of them. (Hint: look for legacy "
-          + "ServetModules). You typically see this error if are not"
-          + " using " + GuiceServletContextListener.class.getSimpleName()
-          + " as described in the documentation.";
+  private static final String MULTIPLE_INJECTORS_WARNING =
+      "Multiple Servlet injectors detected. This is a warning "
+      + "indicating that you have more than one "
+      + GuiceFilter.class.getSimpleName() + " running "
+      + "in your web application. If this is deliberate, you may safely "
+      + "ignore this message. If this is NOT deliberate however, "
+      + "your application may not work as expected.";
 
   //VisibleForTesting
   @Inject
-  static void setPipeline(FilterPipeline pipeline, Stage stage) {
+  static void setPipeline(FilterPipeline pipeline) {
 
-    // Multiple injectors with Servlet pipelines?!
-    // We don't throw an exception, to allow for legacy
-    // tests that don't have a tearDown that calls GuiceFilter#destroy(),
-    // and so we don't force people to use the static filter pipeline
-    // (e.g. Jetty/OpenGSE filters constructed in Guice, rather than via web.xml)
+    // This can happen if you create many injectors and they all have their own
+    // servlet module. This is legal, caveat a small warning.
     if (GuiceFilter.pipeline instanceof ManagedFilterPipeline) {
-        Logger.getLogger(GuiceFilter.class.getName()).warning(MULTIPLE_INJECTORS_ERROR);
-
-        // TODO(dhanji): should we return early here and refuse to overwrite
-        // the existing pipleine? That may break some broken apps =)
+        Logger.getLogger(GuiceFilter.class.getName()).warning(MULTIPLE_INJECTORS_WARNING);
     }
 
     // We overwrite the default pipeline
diff --git a/servlet/src/com/google/inject/servlet/GuiceServletContextListener.java b/servlet/src/com/google/inject/servlet/GuiceServletContextListener.java
index 70810ca..957d22f 100644
--- a/servlet/src/com/google/inject/servlet/GuiceServletContextListener.java
+++ b/servlet/src/com/google/inject/servlet/GuiceServletContextListener.java
@@ -36,11 +36,16 @@
   static final String INJECTOR_NAME = Injector.class.getName();
 
   public void contextInitialized(ServletContextEvent servletContextEvent) {
-    ServletContext servletContext = servletContextEvent.getServletContext();
+    final ServletContext servletContext = servletContextEvent.getServletContext();
 
     // Set the Servletcontext early for those people who are using this class.
+    // NOTE(dhanji): This use of the servletContext is deprecated.
     GuiceFilter.servletContext = new WeakReference<ServletContext>(servletContext);
-    servletContext.setAttribute(INJECTOR_NAME, getInjector());
+
+    Injector injector = getInjector();
+    injector.getInstance(InternalServletModule.BackwardsCompatibleServletContextProvider.class)
+        .set(servletContext);
+    servletContext.setAttribute(INJECTOR_NAME, injector);
   }
 
   public void contextDestroyed(ServletContextEvent servletContextEvent) {
diff --git a/servlet/src/com/google/inject/servlet/InternalServletModule.java b/servlet/src/com/google/inject/servlet/InternalServletModule.java
index f3a9981..5b7064f 100644
--- a/servlet/src/com/google/inject/servlet/InternalServletModule.java
+++ b/servlet/src/com/google/inject/servlet/InternalServletModule.java
@@ -1,8 +1,11 @@
 package com.google.inject.servlet;
 
 import com.google.inject.AbstractModule;
+import com.google.inject.Provider;
 import com.google.inject.Provides;
+import com.google.inject.Singleton;
 import java.util.Map;
+import java.util.logging.Logger;
 import javax.servlet.ServletContext;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
@@ -22,6 +25,37 @@
  */
 final class InternalServletModule extends AbstractModule {
 
+  /**
+   * Special Provider that tries to obtain an injected servlet context, specific
+   * to the current injector, failing which, it falls back to the static singleton
+   * instance that is available in the legacy Guice Servlet.
+   */
+  @Singleton
+  static class BackwardsCompatibleServletContextProvider implements Provider<ServletContext> {
+    private ServletContext injectedServletContext;
+
+    // This setter is called by the GuiceServletContextListener
+    void set(ServletContext injectedServletContext) {
+      this.injectedServletContext = injectedServletContext;
+    }
+
+    public ServletContext get() {
+      if (null != injectedServletContext) {
+        return injectedServletContext;
+      }
+
+      Logger.getLogger(InternalServletModule.class.getName())
+          .warning("You are attempting to use a deprecated API (specifically,"
+          + " attempting to @Inject ServletContext inside an eagerly created"
+          + " singleton. While we allow this for backwards compatibility, be"
+          + " warned that this MAY have unexpected behavior if you have more"
+          + " than one injector (with ServletModule) running in the same JVM."
+          + " Please consult the Guice documentation at"
+          + " http://code.google.com/p/google-guice for more information.");
+      return GuiceFilter.getServletContext();
+    }
+  }
+
   @Override
   protected void configure() {
     bindScope(RequestScoped.class, REQUEST);
@@ -31,11 +65,14 @@
 
     // inject the pipeline into GuiceFilter so it can route requests correctly
     // Unfortunate staticness... =(
+    // NOTE(dhanji): This is maintained for legacy purposes.
     requestStaticInjection(GuiceFilter.class);
 
     bind(ManagedFilterPipeline.class);
     bind(ManagedServletPipeline.class);
     bind(FilterPipeline.class).to(ManagedFilterPipeline.class).asEagerSingleton();
+
+    bind(ServletContext.class).toProvider(BackwardsCompatibleServletContextProvider.class);
   }
 
   @Provides @RequestScoped HttpServletRequest provideHttpServletRequest() {
@@ -50,10 +87,6 @@
     return GuiceFilter.getRequest().getSession();
   }
 
-  @Provides ServletContext provideServletContext() {
-    return GuiceFilter.getServletContext();
-  }
-
   @SuppressWarnings({"unchecked"})
   @Provides @RequestScoped @RequestParameters Map<String, String[]> provideRequestParameters() {
     return GuiceFilter.getRequest().getParameterMap();
diff --git a/servlet/test/com/google/inject/servlet/MultipleServletInjectorsTest.java b/servlet/test/com/google/inject/servlet/MultipleServletInjectorsTest.java
new file mode 100644
index 0000000..6e63b72
--- /dev/null
+++ b/servlet/test/com/google/inject/servlet/MultipleServletInjectorsTest.java
@@ -0,0 +1,98 @@
+// Copyright 2010 Google Inc. All Rights Reserved.
+
+package com.google.inject.servlet;
+
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.http.HttpServlet;
+import junit.framework.TestCase;
+
+import static com.google.inject.servlet.GuiceServletContextListener.INJECTOR_NAME;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.eq;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.isA;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+
+/**
+ * This gorgeous test asserts that multiple servlet pipelines can
+ * run in the SAME JVM. booya.
+ *
+ * @author dhanji@google.com (Dhanji R. Prasanna)
+ */
+public class MultipleServletInjectorsTest extends TestCase {
+
+  private Injector injectorOne;
+  private Injector injectorTwo;
+
+  public final void testTwoInjectors() {
+    ServletContext fakeContextOne = createMock(ServletContext.class);
+    ServletContext fakeContextTwo = createMock(ServletContext.class);
+
+    fakeContextOne.setAttribute(eq(INJECTOR_NAME), isA(Injector.class));
+    expectLastCall().once();
+
+    fakeContextTwo.setAttribute(eq(INJECTOR_NAME), isA(Injector.class));
+    expectLastCall().once();
+
+    replay(fakeContextOne);
+
+    // Simulate the start of a servlet container.
+    new GuiceServletContextListener() {
+
+      @Override
+      protected Injector getInjector() {
+        // Cache this injector in the test for later testing...
+        return injectorOne = Guice.createInjector(new ServletModule() {
+
+          @Override
+          protected void configureServlets() {
+            // This creates a ManagedFilterPipeline internally...
+            serve("/*").with(DummyServlet.class);
+          }
+        });
+      }
+    }.contextInitialized(new ServletContextEvent(fakeContextOne));
+
+    ServletContext contextOne = injectorOne.getInstance(ServletContext.class);
+    assertNotNull(contextOne);
+
+    // Now simulate a second injector with a slightly different config.
+    replay(fakeContextTwo);
+    new GuiceServletContextListener() {
+
+      @Override
+      protected Injector getInjector() {
+        return injectorTwo = Guice.createInjector(new ServletModule() {
+
+          @Override
+          protected void configureServlets() {
+            // This creates a ManagedFilterPipeline internally...
+            filter("/8").through(DummyFilterImpl.class);
+
+            serve("/*").with(HttpServlet.class);
+          }
+        });
+      }
+    }.contextInitialized(new ServletContextEvent(fakeContextTwo));
+
+    ServletContext contextTwo = injectorTwo.getInstance(ServletContext.class);
+
+    // Make sure they are different.
+    assertNotNull(contextTwo);
+    assertNotSame(contextOne, contextTwo);
+
+    // Make sure they are as expected
+    assertSame(fakeContextOne, contextOne);
+    assertSame(fakeContextTwo, contextTwo);
+
+    // Make sure they are consistent.
+    assertSame(contextOne, injectorOne.getInstance(ServletContext.class));
+    assertSame(contextTwo, injectorTwo.getInstance(ServletContext.class));
+
+    verify(fakeContextOne, fakeContextTwo);
+  }
+}