DispatcherPortlet does not forward event exceptions to the render phase by default

jhoeller · jhoeller · commit 79b86a7e3ad3 · 2012-07-05T00:19:32.000+02:00
Issue: SPR-9287
diff --git a/org.springframework.web.portlet/src/main/java/org/springframework/web/portlet/DispatcherPortlet.java b/org.springframework.web.portlet/src/main/java/org/springframework/web/portlet/DispatcherPortlet.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2012 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -123,7 +123,7 @@
  * as loaded by {@link org.springframework.web.context.ContextLoaderListener},
  * if any, will be shared.
  *
- * <p>Thanks to Rainer Schmitz and Nick Lothian for their suggestions!
+ * <p>Thanks to Rainer Schmitz, Nick Lothian and Eric Dalquist for their suggestions!
  *
  * @author William G. Thompson, Jr.
  * @author John A. Lewis
@@ -241,6 +241,12 @@ public class DispatcherPortlet extends FrameworkPortlet {
 	/** Detect all ViewResolvers or just expect "viewResolver" bean? */
 	private boolean detectAllViewResolvers = true;
 
+	/** Whether exceptions thrown during doAction should be forwarded to doRender */
+	private boolean forwardActionException = true;
+
+	/** Whether exceptions thrown during doEvent should be forwarded to doRender */
+	private boolean forwardEventException = false;
+
 	/** URL that points to the ViewRendererServlet */
 	private String viewRendererUrl = DEFAULT_VIEW_RENDERER_URL;
 
@@ -305,6 +311,28 @@ public void setDetectAllViewResolvers(boolean detectAllViewResolvers) {
 		this.detectAllViewResolvers = detectAllViewResolvers;
 	}
 
+	/**
+	 * Set whether to forward exceptions thrown during the action phase
+	 * to the render phase via a session attribute.
+	 * <p>Default is true. Turn this off if you want the portlet container
+	 * to provide immediate exception handling for action requests.
+	 * @see #exposeActionException(javax.portlet.PortletRequest, javax.portlet.StateAwareResponse, Exception)
+	 */
+	public void setForwardActionException(boolean forwardActionException) {
+		this.forwardActionException = forwardActionException;
+	}
+
+	/**
+	 * Set whether to forward exceptions thrown during the event phase
+	 * to the render phase via a session attribute.
+	 * <p>Default is false. Turn this on if you want the {@link DispatcherPortlet}
+	 * to forward the exception to the render phase, similar to what it does
+	 * for {@link #setForwardActionException action exceptions} by default.
+	 */
+	public void setForwardEventException(boolean forwardEventException) {
+		this.forwardEventException = forwardEventException;
+	}
+
 	/**
 	 * Set the URL to the ViewRendererServlet. That servlet is used to
 	 * ultimately render all views in the portlet application.
@@ -648,12 +676,17 @@ protected void doActionService(ActionRequest request, ActionResponse response) t
 			// Trigger after-completion for thrown exception.
 			triggerAfterActionCompletion(mappedHandler, interceptorIndex, processedRequest, response, ex);
 			// Forward the exception to the render phase to be displayed.
-			try {
-				exposeActionException(request, response, ex);
-				logger.debug("Caught exception during action phase - forwarding to render phase", ex);
+			if (this.forwardActionException) {
+				try {
+					exposeActionException(request, response, ex);
+					logger.debug("Caught exception during action phase - forwarding to render phase", ex);
+				}
+				catch (IllegalStateException ex2) {
+					// Probably sendRedirect called... need to rethrow exception immediately.
+					throw ex;
+				}
 			}
-			catch (IllegalStateException ex2) {
-				// Probably sendRedirect called... need to rethrow exception immediately.
+			else {
 				throw ex;
 			}
 		}
@@ -921,12 +954,17 @@ protected void doEventService(EventRequest request, EventResponse response) thro
 			// Trigger after-completion for thrown exception.
 			triggerAfterEventCompletion(mappedHandler, interceptorIndex, request, response, ex);
 			// Forward the exception to the render phase to be displayed.
-			try {
-				exposeActionException(request, response, ex);
-				logger.debug("Caught exception during event phase - forwarding to render phase", ex);
+			if (this.forwardEventException) {
+				try {
+					exposeActionException(request, response, ex);
+					logger.debug("Caught exception during event phase - forwarding to render phase", ex);
+				}
+				catch (IllegalStateException ex2) {
+					// Probably sendRedirect called... need to rethrow exception immediately.
+					throw ex;
+				}
 			}
-			catch (IllegalStateException ex2) {
-				// Probably sendRedirect called... need to rethrow exception immediately.
+			else {
 				throw ex;
 			}
 		}
@@ -963,8 +1001,7 @@ protected ActionRequest checkMultipart(ActionRequest request) throws MultipartEx
 	 * Return the HandlerExecutionChain for this request.
 	 * Try all handler mappings in order.
 	 * @param request current portlet request
-	 * @param cache whether to cache the HandlerExecutionChain in a request attribute
-	 * @return the HandlerExceutionChain, or null if no handler could be found
+	 * @return the HandlerExecutionChain, or null if no handler could be found
 	 */
 	protected HandlerExecutionChain getHandler(PortletRequest request) throws Exception {
 		for (HandlerMapping hm : this.handlerMappings) {
diff --git a/org.springframework.web.portlet/src/test/java/org/springframework/web/portlet/DispatcherPortletTests.java b/org.springframework.web.portlet/src/test/java/org/springframework/web/portlet/DispatcherPortletTests.java
@@ -1,18 +1,18 @@
 /*
- * Copyright 2002-2008 the original author or authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+* Copyright 2002-2012 the original author or authors.
+*
+* Licensed under the Apache License, Version 2.0 (the "License");
+* you may not use this file except in compliance with the License.
+* You may obtain a copy of the License at
+*
+* http://www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
 
 package org.springframework.web.portlet;
 
@@ -24,7 +24,6 @@
 import javax.portlet.PortletMode;
 import javax.portlet.PortletSecurityException;
 import javax.portlet.PortletSession;
-import javax.portlet.UnavailableException;
 
 import junit.framework.TestCase;
 
@@ -35,6 +34,9 @@
 import org.springframework.context.i18n.LocaleContextHolder;
 import org.springframework.mock.web.portlet.MockActionRequest;
 import org.springframework.mock.web.portlet.MockActionResponse;
+import org.springframework.mock.web.portlet.MockEvent;
+import org.springframework.mock.web.portlet.MockEventRequest;
+import org.springframework.mock.web.portlet.MockEventResponse;
 import org.springframework.mock.web.portlet.MockPortletConfig;
 import org.springframework.mock.web.portlet.MockPortletContext;
 import org.springframework.mock.web.portlet.MockPortletSession;
@@ -106,7 +108,7 @@ public void testDispatcherPortlets() {
 				(FrameworkPortlet.PORTLET_CONTEXT_PREFIX + "simple").equals(simpleDispatcherPortlet.getPortletContextAttributeName()));
 		assertTrue("Context published",
 				simpleDispatcherPortlet.getPortletApplicationContext() ==
-				getPortletContext().getAttribute(FrameworkPortlet.PORTLET_CONTEXT_PREFIX + "simple"));
+						getPortletContext().getAttribute(FrameworkPortlet.PORTLET_CONTEXT_PREFIX + "simple"));
 
 		assertTrue("Correct namespace", "test".equals(complexDispatcherPortlet.getNamespace()));
 		assertTrue("Correct attribute",
@@ -139,6 +141,56 @@ public void testSimpleInvalidActionRequest() throws Exception {
 		assertTrue(exceptionParam.startsWith(NoHandlerFoundException.class.getName()));
 	}
 
+
+	public void testSimpleInvalidActionRequestWithoutHandling() throws Exception {
+		MockActionRequest request = new MockActionRequest();
+		MockActionResponse response = new MockActionResponse();
+		request.setParameter("action", "invalid");
+		simpleDispatcherPortlet.setForwardActionException(false);
+		try {
+			simpleDispatcherPortlet.processAction(request, response);
+			fail("Should have thrown a " + NoHandlerFoundException.class);
+		}
+		catch (NoHandlerFoundException ex) {
+			// expected
+		}
+	}
+
+	public void testSimpleValidEventRequest() throws Exception {
+		MockEvent event = new MockEvent("test-event");
+		MockEventRequest request = new MockEventRequest(event);
+		MockEventResponse response = new MockEventResponse();
+		request.setParameter("action", "form");
+		simpleDispatcherPortlet.processEvent(request, response);
+		assertEquals("test-event", response.getRenderParameter("event"));
+	}
+
+	public void testSimpleInvalidEventRequest() throws Exception {
+		MockEvent event = new MockEvent("test-event");
+		MockEventRequest request = new MockEventRequest(event);
+		MockEventResponse response = new MockEventResponse();
+		request.setParameter("action", "invalid");
+		try {
+			simpleDispatcherPortlet.processEvent(request, response);
+			fail("Should have thrown a " + NoHandlerFoundException.class);
+		}
+		catch (NoHandlerFoundException ex) {
+			// expected
+		}
+	}
+
+	public void testSimpleInvalidEventRequestWithHandling() throws Exception {
+		MockEvent event = new MockEvent("event");
+		MockEventRequest request = new MockEventRequest(event);
+		MockEventResponse response = new MockEventResponse();
+		request.setParameter("action", "invalid");
+		simpleDispatcherPortlet.setForwardEventException(true);
+		simpleDispatcherPortlet.processEvent(request, response);
+		String exceptionParam = response.getRenderParameter(DispatcherPortlet.ACTION_EXCEPTION_RENDER_PARAMETER);
+		assertNotNull(exceptionParam);
+		assertTrue(exceptionParam.startsWith(NoHandlerFoundException.class.getName()));
+	}
+
 	public void testSimpleFormViewNoBindOnNewForm() throws Exception {
 		MockRenderRequest request = new MockRenderRequest();
 		MockRenderResponse response = new MockRenderResponse();
@@ -202,7 +254,7 @@ public void testSimpleRequiredSessionFormWithoutSession() throws Exception {
 			fail("Should have thrown PortletSessionRequiredException");
 		}
 		catch (PortletSessionRequiredException ex) {
-			// expected
+// expected
 		}
 	}
 
@@ -247,7 +299,7 @@ public void testSimpleInvalidRenderRequest() throws Exception {
 			fail("Should have thrown UnavailableException");
 		}
 		catch (NoHandlerFoundException ex) {
-			// expected
+// expected
 		}
 	}
 
@@ -431,7 +483,7 @@ public void testExistingMultipartRequest() throws Exception {
 		request.setPortletMode(PortletMode.EDIT);
 		ComplexPortletApplicationContext.MockMultipartResolver multipartResolver =
 				(ComplexPortletApplicationContext.MockMultipartResolver)
-				complexDispatcherPortlet.getPortletApplicationContext().getBean("portletMultipartResolver");
+						complexDispatcherPortlet.getPortletApplicationContext().getBean("portletMultipartResolver");
 		MultipartActionRequest multipartRequest = multipartResolver.resolveMultipart(request);
 		complexDispatcherPortlet.processAction(multipartRequest, response);
 		multipartResolver.cleanupMultipart(multipartRequest);
@@ -455,7 +507,7 @@ public void testActionRequestHandledEvent() throws Exception {
 		complexDispatcherPortlet.processAction(request, response);
 		ComplexPortletApplicationContext.TestApplicationListener listener =
 				(ComplexPortletApplicationContext.TestApplicationListener)
-				complexDispatcherPortlet.getPortletApplicationContext().getBean("testListener");
+						complexDispatcherPortlet.getPortletApplicationContext().getBean("testListener");
 		assertEquals(1, listener.counter);
 	}
 
@@ -465,7 +517,7 @@ public void testRenderRequestHandledEvent() throws Exception {
 		complexDispatcherPortlet.doDispatch(request, response);
 		ComplexPortletApplicationContext.TestApplicationListener listener =
 				(ComplexPortletApplicationContext.TestApplicationListener)
-				complexDispatcherPortlet.getPortletApplicationContext().getBean("testListener");
+						complexDispatcherPortlet.getPortletApplicationContext().getBean("testListener");
 		assertEquals(1, listener.counter);
 	}
 
@@ -477,7 +529,7 @@ public void testPublishEventsOff() throws Exception {
 		complexDispatcherPortlet.processAction(request, response);
 		ComplexPortletApplicationContext.TestApplicationListener listener =
 				(ComplexPortletApplicationContext.TestApplicationListener)
-				complexDispatcherPortlet.getPortletApplicationContext().getBean("testListener");
+						complexDispatcherPortlet.getPortletApplicationContext().getBean("testListener");
 		assertEquals(0, listener.counter);
 	}
 
@@ -611,7 +663,7 @@ public void testPortletHandlerAdapterActionRequest() throws Exception {
 		complexDispatcherPortlet.processAction(request, response);
 		assertEquals("myPortlet action called", response.getRenderParameter("result"));
 		ComplexPortletApplicationContext.MyPortlet myPortlet =
-			(ComplexPortletApplicationContext.MyPortlet) complexDispatcherPortlet.getPortletApplicationContext().getBean("myPortlet");
+				(ComplexPortletApplicationContext.MyPortlet) complexDispatcherPortlet.getPortletApplicationContext().getBean("myPortlet");
 		assertEquals("complex", myPortlet.getPortletConfig().getPortletName());
 		assertEquals(getPortletContext(), myPortlet.getPortletConfig().getPortletContext());
 		assertEquals(complexDispatcherPortlet.getPortletContext(), myPortlet.getPortletConfig().getPortletContext());
@@ -626,12 +678,12 @@ public void testPortletHandlerAdapterRenderRequest() throws Exception {
 		complexDispatcherPortlet.doDispatch(request, response);
 		assertEquals("myPortlet was here", response.getContentAsString());
 		ComplexPortletApplicationContext.MyPortlet myPortlet =
-			(ComplexPortletApplicationContext.MyPortlet)
-			complexDispatcherPortlet.getPortletApplicationContext().getBean("myPortlet");
+				(ComplexPortletApplicationContext.MyPortlet)
+						complexDispatcherPortlet.getPortletApplicationContext().getBean("myPortlet");
 		assertEquals("complex", myPortlet.getPortletConfig().getPortletName());
 		assertEquals(getPortletContext(), myPortlet.getPortletConfig().getPortletContext());
 		assertEquals(complexDispatcherPortlet.getPortletContext(),
-						 myPortlet.getPortletConfig().getPortletContext());
+				myPortlet.getPortletConfig().getPortletContext());
 		complexDispatcherPortlet.destroy();
 		assertNull(myPortlet.getPortletConfig());
 	}
@@ -769,18 +821,18 @@ public void testRuntimeExceptionInUnmappedHandler() throws Exception {
 	}
 
 	public void testGetMessage() {
-		String message  = complexDispatcherPortlet.getPortletApplicationContext().getMessage("test", null, Locale.ENGLISH);
+		String message = complexDispatcherPortlet.getPortletApplicationContext().getMessage("test", null, Locale.ENGLISH);
 		assertEquals("test message", message);
 	}
 
 	public void testGetMessageOtherLocale() {
-		String message  = complexDispatcherPortlet.getPortletApplicationContext().getMessage("test", null, Locale.CANADA);
+		String message = complexDispatcherPortlet.getPortletApplicationContext().getMessage("test", null, Locale.CANADA);
 		assertEquals("Canadian & test message", message);
 	}
 
 	public void testGetMessageWithArgs() {
 		Object[] args = new String[] {"this", "that"};
-		String message  = complexDispatcherPortlet.getPortletApplicationContext().getMessage("test.args", args, Locale.ENGLISH);
+		String message = complexDispatcherPortlet.getPortletApplicationContext().getMessage("test.args", args, Locale.ENGLISH);
 		assertEquals("test this and that", message);
 	}
 
@@ -793,7 +845,7 @@ public void testPortletApplicationContextLookup() {
 			fail("Should have thrown IllegalStateException");
 		}
 		catch (IllegalStateException ex) {
-			// expected
+// expected
 		}
 		portletContext.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE,
 				new StaticWebApplicationContext());
@@ -813,7 +865,7 @@ public void testValidActionRequestWithExistingThreadLocalRequestContext() throws
 		request.setParameter("action", "form");
 		request.setParameter("age", "29");
 
-		// see RequestContextListener.requestInitialized()
+// see RequestContextListener.requestInitialized()
 		try {
 			LocaleContextHolder.setLocale(request.getLocale());
 			RequestContextHolder.setRequestAttributes(new PortletRequestAttributes(request));
@@ -837,7 +889,7 @@ public void testValidRenderRequestWithExistingThreadLocalRequestContext() throws
 		MockRenderResponse response = new MockRenderResponse();
 		request.addPreferredLocale(Locale.GERMAN);
 
-		// see RequestContextListener.requestInitialized()
+// see RequestContextListener.requestInitialized()
 		try {
 			LocaleContextHolder.setLocale(request.getLocale());
 			RequestContextHolder.setRequestAttributes(new PortletRequestAttributes(request));
@@ -863,7 +915,7 @@ public void testInvalidActionRequestWithExistingThreadLocalRequestContext() thro
 		MockActionResponse response = new MockActionResponse();
 		request.addPreferredLocale(Locale.GERMAN);
 
-		// see RequestContextListener.requestInitialized()
+// see RequestContextListener.requestInitialized()
 		try {
 			LocaleContextHolder.setLocale(request.getLocale());
 			RequestContextHolder.setRequestAttributes(new PortletRequestAttributes(request));
@@ -890,7 +942,7 @@ public void testInvalidRenderRequestWithExistingThreadLocalRequestContext() thro
 		MockRenderResponse response = new MockRenderResponse();
 		request.addPreferredLocale(Locale.GERMAN);
 
-		// see RequestContextListener.requestInitialized()
+// see RequestContextListener.requestInitialized()
 		try {
 			LocaleContextHolder.setLocale(request.getLocale());
 			RequestContextHolder.setRequestAttributes(new PortletRequestAttributes(request));
@@ -903,7 +955,7 @@ public void testInvalidRenderRequestWithExistingThreadLocalRequestContext() thro
 				fail("should have failed to find a handler and raised an NoHandlerFoundExceptionException");
 			}
 			catch (NoHandlerFoundException ex) {
-				// expected
+// expected
 			}
 
 			assertSame(servletLocaleContext, LocaleContextHolder.getLocaleContext());
diff --git a/org.springframework.web.portlet/src/test/java/org/springframework/web/portlet/SimplePortletApplicationContext.java b/org.springframework.web.portlet/src/test/java/org/springframework/web/portlet/SimplePortletApplicationContext.java
