Author: dhanji
Date: Tue Jun 16 11:47:41 2009
New Revision: 1015

Modified:
    trunk/servlet/src/com/google/inject/servlet/GuiceFilter.java
    trunk/servlet/src/com/google/inject/servlet/ManagedServletPipeline.java
     
trunk/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java
     
trunk/servlet/test/com/google/inject/servlet/ServletPipelineRequestDispatcherTest.java

Log:
Andrew McLaughlin's patch for making forward() pass the original request  
URI. Fixes a bug with JSP servlet etc., not being able to know the target  
of the original request.

Modified: trunk/servlet/src/com/google/inject/servlet/GuiceFilter.java
==============================================================================
--- trunk/servlet/src/com/google/inject/servlet/GuiceFilter.java        
(original)
+++ trunk/servlet/src/com/google/inject/servlet/GuiceFilter.java        Tue Jun 
16  
11:47:41 2009
@@ -77,7 +77,7 @@

      // Multiple injectors with Servlet pipelines?!
      // We don't throw an exception in DEVELOPMENT stage, to allow for  
legacy
-    // tests that don't have a tearDown that calls GuiceFilter#reset().
+    // tests that don't have a tearDown that calls GuiceFilter#destroy().
      if (GuiceFilter.pipeline instanceof ManagedFilterPipeline) {
        if (Stage.PRODUCTION.equals(stage)) {
          throw new RuntimeException(MULTIPLE_INJECTORS_ERROR);

Modified:  
trunk/servlet/src/com/google/inject/servlet/ManagedServletPipeline.java
==============================================================================
--- trunk/servlet/src/com/google/inject/servlet/ManagedServletPipeline.java     
 
(original)
+++ trunk/servlet/src/com/google/inject/servlet/ManagedServletPipeline.java     
 
Tue Jun 16 11:47:41 2009
@@ -35,6 +35,8 @@
  import javax.servlet.ServletRequest;
  import javax.servlet.ServletResponse;
  import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;

  /**
   * A wrapping dispatcher for servlets, in much the same way as {...@link  
ManagedFilterPipeline} is for
@@ -112,13 +114,12 @@
     * the given path or null if no mapping was found.
     */
    RequestDispatcher getRequestDispatcher(String path) {
+    final String newRequestUri = path;
      for (final ServletDefinition servletDefinition : servletDefinitions) {
        if (servletDefinition.shouldServe(path)) {
          return new RequestDispatcher() {
-
            public void forward(ServletRequest servletRequest,  
ServletResponse servletResponse)
                throws ServletException, IOException {
-
              Preconditions.checkState(!servletResponse.isCommitted(),
                  "Response has been committed--you can only call forward  
before"
                  + " committing the response (hint: don't flush buffers)");
@@ -126,8 +127,23 @@
              // clear buffer before forwarding
              servletResponse.resetBuffer();

+            ServletRequest requestToProcess;
+            if (servletRequest instanceof HttpServletRequest) {
+               requestToProcess =
+                   new HttpServletRequestWrapper((HttpServletRequest)  
servletRequest) {
+                     public String getRequestURI() {
+                       return newRequestUri;
+                     }
+                   };
+            } else {
+              // This should never happen, but instead of throwing an  
exception
+              // we will allow a happy case pass thru for maximum  
tolerance to
+              // legacy (and internal) code.
+              requestToProcess = servletRequest;
+            }
+
              // now dispatch to the servlet
-            servletDefinition.doService(servletRequest, servletResponse);
+            servletDefinition.doService(requestToProcess, servletResponse);
            }

            public void include(ServletRequest servletRequest,  
ServletResponse servletResponse)

Modified:  
trunk/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java
==============================================================================
---  
trunk/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java 
 
(original)
+++  
trunk/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java 
 
Tue Jun 16 11:47:41 2009
@@ -4,22 +4,23 @@
  import com.google.inject.Injector;
  import com.google.inject.Key;
  import com.google.inject.Singleton;
-
-import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.verify;
-
  import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
  import javax.servlet.Filter;
  import javax.servlet.FilterChain;
  import javax.servlet.FilterConfig;
  import javax.servlet.ServletException;
  import javax.servlet.ServletRequest;
  import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServlet;
  import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
  import junit.framework.TestCase;
-
+import org.easymock.EasyMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import org.easymock.IMocksControl;

  /**
   *
@@ -33,12 +34,14 @@
  public class FilterDispatchIntegrationTest extends TestCase {
      private static int inits, doFilters, destroys;

+  private IMocksControl control;
+
    @Override
    public final void setUp() {
      inits = 0;
      doFilters = 0;
      destroys = 0;
-
+    control = EasyMock.createControl();
      GuiceFilter.reset();
    }

@@ -55,25 +58,44 @@
          // These filters should never fire
          filter("/index/*").through(Key.get(TestFilter.class));
          filter("*.jsp").through(Key.get(TestFilter.class));
+
+        // Bind a servlet
+        serve("*.html").with(TestServlet.class);
        }
      });

      final FilterPipeline pipeline =  
injector.getInstance(FilterPipeline.class);
      pipeline.initPipeline(null);

-    //create ourselves a mock request with test URI
-    HttpServletRequest requestMock = createMock(HttpServletRequest.class);
+    // create ourselves a mock request with test URI
+    HttpServletRequest requestMock =  
control.createMock(HttpServletRequest.class);

      expect(requestMock.getServletPath())
              .andReturn("/index.html")
              .anyTimes();
+    expect(requestMock.getRequestURI())
+            .andReturn("/index.html")
+            .anyTimes();
+
+    HttpServletResponse responseMock =  
control.createMock(HttpServletResponse.class);
+    expect(responseMock.isCommitted())
+        .andReturn(false)
+        .anyTimes();
+    responseMock.resetBuffer();
+    expectLastCall().anyTimes();
+
+    FilterChain filterChain = control.createMock(FilterChain.class);

      //dispatch request
-    replay(requestMock);
-    pipeline.dispatch(requestMock, null, createMock(FilterChain.class));
+    control.replay();
+    pipeline.dispatch(requestMock, responseMock, filterChain);
      pipeline.destroyPipeline();
+    control.verify();

-    verify(requestMock);
+    TestServlet servlet = injector.getInstance(TestServlet.class);
+    assertEquals(2, servlet.processedUris.size());
+    assertTrue(servlet.processedUris.contains("/index.html"));
+    assertTrue(servlet.processedUris.contains(TestServlet.FORWARD_TO));

      assert inits == 5 && doFilters == 3 && destroys == 5 : "lifecycle  
states did not"
            + " fire correct number of times-- inits: " + inits + "; dos: "  
+ doFilters
@@ -99,18 +121,19 @@
      pipeline.initPipeline(null);

      //create ourselves a mock request with test URI
-    HttpServletRequest requestMock = createMock(HttpServletRequest.class);
+    HttpServletRequest requestMock =  
control.createMock(HttpServletRequest.class);

      expect(requestMock.getServletPath())
              .andReturn("/index.xhtml")
              .anyTimes();

      //dispatch request
-    replay(requestMock);
-    pipeline.dispatch(requestMock, null, createMock(FilterChain.class));
+    FilterChain filterChain = control.createMock(FilterChain.class);
+    filterChain.doFilter(requestMock, null);
+    control.replay();
+    pipeline.dispatch(requestMock, null, filterChain);
      pipeline.destroyPipeline();
-
-    verify(requestMock);
+    control.verify();

      assert inits == 5 && doFilters == 0 && destroys == 5 : "lifecycle  
states did not "
            + "fire correct number of times-- inits: " + inits + "; dos: " +  
doFilters
@@ -126,7 +149,6 @@
        protected void configureServlets() {
          filterRegex("/[A-Za-z]*").through(TestFilter.class);
          filterRegex("/index").through(TestFilter.class);
-
          //these filters should never fire
          filterRegex("\\w").through(Key.get(TestFilter.class));
        }
@@ -136,18 +158,19 @@
      pipeline.initPipeline(null);

      //create ourselves a mock request with test URI
-    HttpServletRequest requestMock = createMock(HttpServletRequest.class);
+    HttpServletRequest requestMock =  
control.createMock(HttpServletRequest.class);

      expect(requestMock.getServletPath())
              .andReturn("/index")
              .anyTimes();

-    //dispatch request
-    replay(requestMock);
-    pipeline.dispatch(requestMock, null, createMock(FilterChain.class));
+    // dispatch request
+    FilterChain filterChain = control.createMock(FilterChain.class);
+    filterChain.doFilter(requestMock, null);
+    control.replay();
+    pipeline.dispatch(requestMock, null, filterChain);
      pipeline.destroyPipeline();
-
-    verify(requestMock);
+    control.verify();

      assert inits == 3 && doFilters == 2 && destroys == 3 : "lifecycle  
states did not fire "
          + "correct number of times-- inits: " + inits + "; dos: " +  
doFilters
@@ -168,6 +191,30 @@

      public void destroy() {
        destroys++;
+    }
+  }
+
+  @Singleton
+  public static class TestServlet extends HttpServlet {
+    public static final String FORWARD_FROM = "/index.html";
+    public static final String FORWARD_TO = "/forwarded.html";
+    public List<String> processedUris = new ArrayList<String>();
+
+    protected void service(HttpServletRequest httpServletRequest,  
HttpServletResponse httpServletResponse)
+        throws ServletException, IOException {
+      String requestUri = httpServletRequest.getRequestURI();
+      processedUris.add(requestUri);
+
+      // If the client is requesting /index.html then we forward to  
/forwarded.html
+      if (FORWARD_FROM.equals(requestUri)) {
+        httpServletRequest.getRequestDispatcher(FORWARD_TO)
+            .forward(httpServletRequest, httpServletResponse);
+      }
+    }
+
+    public void service(ServletRequest servletRequest, ServletResponse  
servletResponse)
+        throws ServletException, IOException {
+      service((HttpServletRequest) servletRequest, (HttpServletResponse)  
servletResponse);
      }
    }
  }

Modified:  
trunk/servlet/test/com/google/inject/servlet/ServletPipelineRequestDispatcherTest.java
==============================================================================
---  
trunk/servlet/test/com/google/inject/servlet/ServletPipelineRequestDispatcherTest.java
   
(original)
+++  
trunk/servlet/test/com/google/inject/servlet/ServletPipelineRequestDispatcherTest.java
   
Tue Jun 16 11:47:41 2009
@@ -24,6 +24,7 @@
  import com.google.inject.internal.Maps;
  import com.google.inject.internal.Sets;
  import java.io.IOException;
+import java.util.ArrayList;
  import java.util.Date;
  import java.util.HashMap;
  import java.util.List;
@@ -130,11 +131,11 @@
      mockResponse.resetBuffer();
      expectLastCall().once();

-    final boolean[] run = new boolean[1];
+    final List<String> paths = new ArrayList<String>();
      final HttpServlet mockServlet = new HttpServlet() {
        protected void service(HttpServletRequest request,  
HttpServletResponse httpServletResponse)
            throws ServletException, IOException {
-        run[0] = true;
+        paths.add(request.getRequestURI());

          final Object o = request.getAttribute(A_KEY);
          assertEquals("Wrong attrib returned - " + o, A_VALUE, o);
@@ -170,7 +171,7 @@
      assertNotNull(dispatcher);
      dispatcher.forward(mockRequest, mockResponse);

-    assertTrue("Include did not dispatch to our servlet!", run[0]);
+    assertTrue("Include did not dispatch to our servlet!",  
paths.contains(pattern));

      verify(injector, mockRequest, mockResponse, mockBinding);
    }

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"google-guice-dev" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/google-guice-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to