Skip to content

Commit

Permalink
DispatcherPortlet does not forward event exceptions to the render pha…
Browse files Browse the repository at this point in the history 
…se by default

Issue: SPR-9287
  • Loading branch information
@jhoeller
jhoeller committed Jul 4, 2012
1 parent 8bd1fd3 commit 5965710
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 50 deletions.
64 changes: 51 additions & 13 deletions spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/DispatcherPortlet.java
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -963,7 +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
* @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) {
Expand Down
122 changes: 87 additions & 35 deletions ...-webmvc-portlet/src/test/java/org/springframework/web/portlet/DispatcherPortletTests.java
View file
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -24,7 +24,6 @@
import javax.portlet.PortletMode;
import javax.portlet.PortletSecurityException;
import javax.portlet.PortletSession;
import javax.portlet.UnavailableException;

import junit.framework.TestCase;

Expand All @@ -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;
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -202,7 +254,7 @@ public void testSimpleRequiredSessionFormWithoutSession() throws Exception {
fail("Should have thrown PortletSessionRequiredException");
}
catch (PortletSessionRequiredException ex) {
// expected
// expected
}
}

Expand Down Expand Up @@ -247,7 +299,7 @@ public void testSimpleInvalidRenderRequest() throws Exception {
fail("Should have thrown UnavailableException");
}
catch (NoHandlerFoundException ex) {
// expected
// expected
}
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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());
Expand All @@ -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());
}
Expand Down Expand Up @@ -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);
}

Expand All @@ -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());
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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());
Expand Down
Loading

0 comments on commit 5965710

Please sign in to comment.