From 596571059e389b0e7728e0c4a3b9aae2d727a57b Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 5 Jul 2012 00:19:01 +0200 Subject: [PATCH] DispatcherPortlet does not forward event exceptions to the render phase by default Issue: SPR-9287 --- .../web/portlet/DispatcherPortlet.java | 64 +++++++-- .../web/portlet/DispatcherPortletTests.java | 122 +++++++++++++----- .../SimplePortletApplicationContext.java | 11 +- 3 files changed, 147 insertions(+), 50 deletions(-) diff --git a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/DispatcherPortlet.java b/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/DispatcherPortlet.java index 98029b9a6b..8985f91acb 100644 --- a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/DispatcherPortlet.java +++ b/spring-webmvc-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 @@ import org.springframework.web.servlet.ViewResolver; * as loaded by {@link org.springframework.web.context.ContextLoaderListener}, * if any, will be shared. * - *

Thanks to Rainer Schmitz and Nick Lothian for their suggestions! + *

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 class DispatcherPortlet extends FrameworkPortlet { this.detectAllViewResolvers = detectAllViewResolvers; } + /** + * Set whether to forward exceptions thrown during the action phase + * to the render phase via a session attribute. + *

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. + *

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 @@ public class DispatcherPortlet extends FrameworkPortlet { // 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 @@ public class DispatcherPortlet extends FrameworkPortlet { // 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,7 +1001,7 @@ public class DispatcherPortlet extends FrameworkPortlet { * Return the HandlerExecutionChain for this request. * Try all handler mappings in order. * @param request current portlet request - * @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/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/DispatcherPortletTests.java b/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/DispatcherPortletTests.java index 299ff54f6f..b624737c1b 100644 --- a/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/DispatcherPortletTests.java +++ b/spring-webmvc-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.PortletException; 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.LocaleContext; 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 class DispatcherPortletTests extends TestCase { (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 class DispatcherPortletTests extends TestCase { 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 class DispatcherPortletTests extends TestCase { fail("Should have thrown PortletSessionRequiredException"); } catch (PortletSessionRequiredException ex) { - // expected +// expected } } @@ -247,7 +299,7 @@ public class DispatcherPortletTests extends TestCase { fail("Should have thrown UnavailableException"); } catch (NoHandlerFoundException ex) { - // expected +// expected } } @@ -431,7 +483,7 @@ public class DispatcherPortletTests extends TestCase { 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 class DispatcherPortletTests extends TestCase { 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 class DispatcherPortletTests extends TestCase { 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 class DispatcherPortletTests extends TestCase { 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 class DispatcherPortletTests extends TestCase { 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 class DispatcherPortletTests extends TestCase { 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 class DispatcherPortletTests extends TestCase { } 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 class DispatcherPortletTests extends TestCase { 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 class DispatcherPortletTests extends TestCase { 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 class DispatcherPortletTests extends TestCase { 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 class DispatcherPortletTests extends TestCase { 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 class DispatcherPortletTests extends TestCase { 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 class DispatcherPortletTests extends TestCase { fail("should have failed to find a handler and raised an NoHandlerFoundExceptionException"); } catch (NoHandlerFoundException ex) { - // expected +// expected } assertSame(servletLocaleContext, LocaleContextHolder.getLocaleContext()); diff --git a/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/SimplePortletApplicationContext.java b/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/SimplePortletApplicationContext.java index c3d4efd3c7..748414ff48 100644 --- a/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/SimplePortletApplicationContext.java +++ b/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/SimplePortletApplicationContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2007 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. @@ -19,6 +19,8 @@ package org.springframework.web.portlet; import java.io.IOException; import java.util.Map; +import javax.portlet.EventRequest; +import javax.portlet.EventResponse; import javax.portlet.RenderRequest; import javax.portlet.RenderResponse; @@ -31,6 +33,7 @@ import org.springframework.beans.factory.support.ManagedMap; import org.springframework.validation.BindException; import org.springframework.web.portlet.context.StaticPortletApplicationContext; import org.springframework.web.portlet.handler.ParameterHandlerMapping; +import org.springframework.web.portlet.mvc.EventAwareController; import org.springframework.web.portlet.mvc.SimpleFormController; /** @@ -86,7 +89,7 @@ public class SimplePortletApplicationContext extends StaticPortletApplicationCon } - public static class TestFormController extends SimpleFormController { + public static class TestFormController extends SimpleFormController implements EventAwareController { TestFormController() { super(); @@ -124,6 +127,10 @@ public class SimplePortletApplicationContext extends StaticPortletApplicationCon private void writeResponse(RenderResponse response, TestBean testBean, boolean finished) throws IOException { response.getWriter().write((finished ? "finished" : "") + (testBean.getAge() + 5)); } + + public void handleEventRequest(EventRequest request, EventResponse response) throws Exception { + response.setRenderParameter("event", request.getEvent().getName()); + } } }