Revision: 1135 Author: dhanji Date: Tue Dec 29 17:05:45 2009 Log: Fix for issue #455 where path info was a memoized provided incorrectly to request-dispatcher produced requests.
Now we also expose the servlet context in ServletModule for people who want it. http://code.google.com/p/google-guice/source/detail?r=1135 Modified: /trunk/common.xml /trunk/servlet/src/com/google/inject/servlet/ManagedServletPipeline.java /trunk/servlet/src/com/google/inject/servlet/ServletDefinition.java /trunk/servlet/src/com/google/inject/servlet/ServletModule.java /trunk/servlet/src/com/google/inject/servlet/UriPatternType.java /trunk/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java /trunk/servlet/test/com/google/inject/servlet/ServletDefinitionPathsTest.java /trunk/servlet/test/com/google/inject/servlet/ServletDispatchIntegrationTest.java /trunk/servlet/test/com/google/inject/servlet/ServletPipelineRequestDispatcherTest.java ======================================= --- /trunk/common.xml Sun Sep 27 19:51:24 2009 +++ /trunk/common.xml Tue Dec 29 17:05:45 2009 @@ -65,6 +65,7 @@ destdir="${build.dir}/test" source="1.5" target="1.5"> <classpath path="${build.dir}/classes"/> + <classpath path="${build.dir}/test"/> <classpath refid="compile.classpath"/> </javac> <copy toDir="${build.dir}/test"> ======================================= --- /trunk/servlet/src/com/google/inject/servlet/ManagedServletPipeline.java Mon Oct 12 16:03:32 2009 +++ /trunk/servlet/src/com/google/inject/servlet/ManagedServletPipeline.java Tue Dec 29 17:05:45 2009 @@ -133,28 +133,34 @@ ServletRequest requestToProcess; if (servletRequest instanceof HttpServletRequest) { - requestToProcess = - new HttpServletRequestWrapper((HttpServletRequest) servletRequest) { - public String getRequestURI() { - return newRequestUri; - } - }; + requestToProcess = new RequestDispatcherRequestWrapper(servletRequest, 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; } + + servletRequest.setAttribute(REQUEST_DISPATCHER_REQUEST, Boolean.TRUE); // now dispatch to the servlet - servletDefinition.doService(requestToProcess, servletResponse); + try { + servletDefinition.doService(requestToProcess, servletResponse); + } finally { + servletRequest.removeAttribute(REQUEST_DISPATCHER_REQUEST); + } } public void include(ServletRequest servletRequest, ServletResponse servletResponse) throws ServletException, IOException { + servletRequest.setAttribute(REQUEST_DISPATCHER_REQUEST, Boolean.TRUE); // route to the target servlet - servletDefinition.doService(servletRequest, servletResponse); + try { + servletDefinition.doService(servletRequest, servletResponse); + } finally { + servletRequest.removeAttribute(REQUEST_DISPATCHER_REQUEST); + } } }; } @@ -163,4 +169,24 @@ //otherwise, can't process return null; } -} + + /** + * A Marker constant attribute that when present in the request indicates to Guice servlet that + * this request has been generated by a request dispatcher rather than the servlet pipeline. + */ + public static final String REQUEST_DISPATCHER_REQUEST = "$$Guice$RDR"; + + private static class RequestDispatcherRequestWrapper extends HttpServletRequestWrapper { + private final String newRequestUri; + + public RequestDispatcherRequestWrapper(ServletRequest servletRequest, String newRequestUri) { + super((HttpServletRequest) servletRequest); + this.newRequestUri = newRequestUri; + } + + @Override + public String getRequestURI() { + return newRequestUri; + } + } +} ======================================= --- /trunk/servlet/src/com/google/inject/servlet/ServletDefinition.java Mon Jun 22 20:16:48 2009 +++ /trunk/servlet/src/com/google/inject/servlet/ServletDefinition.java Tue Dec 29 17:05:45 2009 @@ -35,6 +35,8 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; +import static com.google.inject.servlet.ManagedServletPipeline.REQUEST_DISPATCHER_REQUEST; + /** * An internal representation of a servlet definition mapped to a particular URI pattern. Also * performs the request dispatch to that servlet. How nice and OO =) @@ -156,7 +158,6 @@ void doService(final ServletRequest servletRequest, ServletResponse servletResponse) throws ServletException, IOException { - //noinspection OverlyComplexAnonymousInnerClass HttpServletRequest request = new HttpServletRequestWrapper( (HttpServletRequest) servletRequest) { private String path; @@ -168,7 +169,7 @@ @Override public String getPathInfo() { - if (!pathInfoComputed) { + if (!isPathInfoComputed()) { final int servletPathLength = getServletPath().length(); pathInfo = getRequestURI().substring(getContextPath().length()).replaceAll("[/]{2,}", "/") .substring(servletPathLength); @@ -184,6 +185,18 @@ return pathInfo; } + + // NOTE(dhanji): These two are a bit of a hack to help ensure that request dipatcher-sent + // requests don't use the same path info that was memoized for the original request. + private boolean isPathInfoComputed() { + return pathInfoComputed + && !(null != servletRequest.getAttribute(REQUEST_DISPATCHER_REQUEST)); + } + + private boolean isPathComputed() { + return pathComputed + && !(null != servletRequest.getAttribute(REQUEST_DISPATCHER_REQUEST)); + } @Override public String getServletPath() { @@ -199,7 +212,7 @@ // Memoizer pattern. private String computePath() { - if (!pathComputed) { + if (!isPathComputed()) { String servletPath = super.getServletPath(); path = patternMatcher.extractPath(servletPath); pathComputed = true; ======================================= --- /trunk/servlet/src/com/google/inject/servlet/ServletModule.java Fri Oct 30 06:19:19 2009 +++ /trunk/servlet/src/com/google/inject/servlet/ServletModule.java Tue Dec 29 17:05:45 2009 @@ -21,6 +21,7 @@ import com.google.inject.internal.Lists; import java.util.Map; import javax.servlet.Filter; +import javax.servlet.ServletContext; import javax.servlet.http.HttpServlet; /** @@ -252,6 +253,16 @@ protected final ServletKeyBindingBuilder serveRegex(String regex, String... regexes) { return servletsModuleBuilder.serveRegex(Lists.newArrayList(regex, regexes)); } + + /** + * This method only works if you are using the {...@linkplain GuiceServletContextListener} to + * create your injector. Otherwise, it returns null. + * @return The current servlet context. + * @since 2.1 + */ + protected final ServletContext getServletContext() { + return GuiceFilter.getServletContext(); + } /** * See the EDSL examples at {...@link ServletModule#configureServlets()} ======================================= --- /trunk/servlet/src/com/google/inject/servlet/UriPatternType.java Fri Jun 26 17:59:38 2009 +++ /trunk/servlet/src/com/google/inject/servlet/UriPatternType.java Tue Dec 29 17:05:45 2009 @@ -94,7 +94,7 @@ } /** - * Matchers URIs using a regular expression. + * Matches URIs using a regular expression. * * @author [email protected] (Dhanji R. Prasanna) */ ======================================= --- /trunk/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java Tue Jun 16 11:47:41 2009 +++ /trunk/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java Tue Dec 29 17:05:45 2009 @@ -18,9 +18,11 @@ import javax.servlet.http.HttpServletResponse; import junit.framework.TestCase; import org.easymock.EasyMock; +import org.easymock.IMocksControl; + +import static com.google.inject.servlet.ManagedServletPipeline.REQUEST_DISPATCHER_REQUEST; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.expectLastCall; -import org.easymock.IMocksControl; /** * @@ -77,6 +79,9 @@ .andReturn("/index.html") .anyTimes(); + requestMock.setAttribute(REQUEST_DISPATCHER_REQUEST, true); + requestMock.removeAttribute(REQUEST_DISPATCHER_REQUEST); + HttpServletResponse responseMock = control.createMock(HttpServletResponse.class); expect(responseMock.isCommitted()) .andReturn(false) ======================================= --- /trunk/servlet/test/com/google/inject/servlet/ServletDefinitionPathsTest.java Wed Jun 24 09:11:56 2009 +++ /trunk/servlet/test/com/google/inject/servlet/ServletDefinitionPathsTest.java Tue Dec 29 17:05:45 2009 @@ -19,9 +19,9 @@ import com.google.inject.Binding; import com.google.inject.Injector; import com.google.inject.Key; -import com.google.inject.spi.BindingScopingVisitor; import com.google.inject.internal.Maps; import com.google.inject.internal.Sets; +import com.google.inject.spi.BindingScopingVisitor; import java.io.IOException; import java.util.HashMap; import javax.servlet.ServletException; @@ -29,10 +29,12 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import junit.framework.TestCase; + +import static com.google.inject.servlet.ManagedServletPipeline.REQUEST_DISPATCHER_REQUEST; +import static org.easymock.EasyMock.anyObject; import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; -import static org.easymock.EasyMock.anyObject; /** * Ensures servlet spec compliance for CGI-style variables and general @@ -177,6 +179,8 @@ expect(request.getContextPath()) .andReturn(contextPath); + expect(request.getAttribute(REQUEST_DISPATCHER_REQUEST)).andReturn(null); + replay(injector, binding, request); ServletDefinition servletDefinition = new ServletDefinition(mapping, Key.get(HttpServlet.class), @@ -270,6 +274,8 @@ expect(request.getContextPath()) .andReturn(contextPath); + expect(request.getAttribute(REQUEST_DISPATCHER_REQUEST)).andReturn(null); + replay(injector, binding, request); ServletDefinition servletDefinition = new ServletDefinition(mapping, Key.get(HttpServlet.class), ======================================= --- /trunk/servlet/test/com/google/inject/servlet/ServletDispatchIntegrationTest.java Fri Sep 4 02:08:15 2009 +++ /trunk/servlet/test/com/google/inject/servlet/ServletDispatchIntegrationTest.java Tue Dec 29 17:05:45 2009 @@ -32,6 +32,8 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import junit.framework.TestCase; + +import static com.google.inject.servlet.ManagedServletPipeline.REQUEST_DISPATCHER_REQUEST; import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; @@ -204,19 +206,27 @@ @Singleton public static class ForwardedServlet extends HttpServlet { + static int forwardedTo = 0; + + // Reset for test. + public ForwardedServlet() { + forwardedTo = 0; + } + public void service(ServletRequest servletRequest, ServletResponse servletResponse) throws IOException, ServletException { final HttpServletRequest request = (HttpServletRequest) servletRequest; - System.out.println(request.getRequestURI()); - } + assertTrue((Boolean) request.getAttribute(REQUEST_DISPATCHER_REQUEST)); + forwardedTo++; + } } public void testForwardUsingRequestDispatcher() throws IOException, ServletException { Guice.createInjector(new ServletModule() { @Override protected void configureServlets() { - serve("/*").with(ForwardingServlet.class); + serve("/").with(ForwardingServlet.class); serve("/blah.jsp").with(ForwardedServlet.class); } }); @@ -227,12 +237,20 @@ .andReturn("/") .anyTimes(); + requestMock.setAttribute(REQUEST_DISPATCHER_REQUEST, true); + expect(requestMock.getAttribute(REQUEST_DISPATCHER_REQUEST)).andReturn(true); + requestMock.removeAttribute(REQUEST_DISPATCHER_REQUEST); + expect(responseMock.isCommitted()).andReturn(false); + responseMock.resetBuffer(); replay(requestMock, responseMock); new GuiceFilter() .doFilter(requestMock, responseMock, createMock(FilterChain.class)); + + assertEquals("Incorrect number of forwards", 1, ForwardedServlet.forwardedTo); + verify(requestMock, responseMock); } } ======================================= --- /trunk/servlet/test/com/google/inject/servlet/ServletPipelineRequestDispatcherTest.java Wed Jun 24 09:11:56 2009 +++ /trunk/servlet/test/com/google/inject/servlet/ServletPipelineRequestDispatcherTest.java Tue Dec 29 17:05:45 2009 @@ -20,10 +20,10 @@ import com.google.inject.Injector; import com.google.inject.Key; import com.google.inject.TypeLiteral; -import com.google.inject.spi.BindingScopingVisitor; import com.google.inject.internal.ImmutableList; import com.google.inject.internal.Maps; import com.google.inject.internal.Sets; +import com.google.inject.spi.BindingScopingVisitor; import java.io.IOException; import java.util.ArrayList; import java.util.Date; @@ -36,13 +36,15 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import junit.framework.TestCase; + +import static com.google.inject.servlet.ManagedServletPipeline.REQUEST_DISPATCHER_REQUEST; +import static org.easymock.EasyMock.anyObject; import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.eq; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.expectLastCall; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; -import static org.easymock.EasyMock.anyObject; /** * Tests forwarding and inclusion (RequestDispatcher actions from the @@ -64,11 +66,15 @@ final Injector injector = createMock(Injector.class); final Binding binding = createMock(Binding.class); - final HttpServletRequest mockRequest = createMock(HttpServletRequest.class); - - expect(mockRequest.getAttribute(A_KEY)) + final HttpServletRequest requestMock = createMock(HttpServletRequest.class); + + expect(requestMock.getAttribute(A_KEY)) .andReturn(A_VALUE); + + requestMock.setAttribute(REQUEST_DISPATCHER_REQUEST, true); + requestMock.removeAttribute(REQUEST_DISPATCHER_REQUEST); + final boolean[] run = new boolean[1]; final HttpServlet mockServlet = new HttpServlet() { protected void service(HttpServletRequest request, HttpServletResponse httpServletResponse) @@ -99,7 +105,7 @@ expect(injector.getInstance(servetDefsKey)) .andReturn(ImmutableList.of(servletDefinition)); - replay(injector, binding, mockRequest, mockBinding); + replay(injector, binding, requestMock, mockBinding); // Have to init the Servlet before we can dispatch to it. servletDefinition.init(null, injector, @@ -110,11 +116,11 @@ .getRequestDispatcher(pattern); assertNotNull(dispatcher); - dispatcher.include(mockRequest, createMock(HttpServletResponse.class)); + dispatcher.include(requestMock, createMock(HttpServletResponse.class)); assertTrue("Include did not dispatch to our servlet!", run[0]); - verify(injector, mockRequest, mockBinding); + verify(injector, requestMock, mockBinding); } public final void testForwardToManagedServlet() throws IOException, ServletException { @@ -125,12 +131,16 @@ final Injector injector = createMock(Injector.class); final Binding binding = createMock(Binding.class); - final HttpServletRequest mockRequest = createMock(HttpServletRequest.class); + final HttpServletRequest requestMock = createMock(HttpServletRequest.class); final HttpServletResponse mockResponse = createMock(HttpServletResponse.class); - expect(mockRequest.getAttribute(A_KEY)) + expect(requestMock.getAttribute(A_KEY)) .andReturn(A_VALUE); + + requestMock.setAttribute(REQUEST_DISPATCHER_REQUEST, true); + requestMock.removeAttribute(REQUEST_DISPATCHER_REQUEST); + expect(mockResponse.isCommitted()) .andReturn(false); @@ -167,7 +177,7 @@ expect(injector.getInstance(servetDefsKey)) .andReturn(ImmutableList.of(servletDefinition)); - replay(injector, binding, mockRequest, mockResponse, mockBinding); + replay(injector, binding, requestMock, mockResponse, mockBinding); // Have to init the Servlet before we can dispatch to it. servletDefinition.init(null, injector, @@ -177,11 +187,11 @@ .getRequestDispatcher(pattern); assertNotNull(dispatcher); - dispatcher.forward(mockRequest, mockResponse); + dispatcher.forward(requestMock, mockResponse); assertTrue("Include did not dispatch to our servlet!", paths.contains(pattern)); - verify(injector, mockRequest, mockResponse, mockBinding); + verify(injector, requestMock, mockResponse, mockBinding); } public final void testForwardToManagedServletFailureOnCommittedBuffer() -- 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.
