diff --git a/modules/cpr/src/main/java/org/atmosphere/cpr/AsynchronousProcessor.java b/modules/cpr/src/main/java/org/atmosphere/cpr/AsynchronousProcessor.java index d145e391d4d..e5b89e0de5a 100755 --- a/modules/cpr/src/main/java/org/atmosphere/cpr/AsynchronousProcessor.java +++ b/modules/cpr/src/main/java/org/atmosphere/cpr/AsynchronousProcessor.java @@ -28,6 +28,7 @@ import javax.servlet.http.HttpSession; import java.io.IOException; import java.util.Collection; +import java.util.LinkedList; import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; @@ -189,38 +190,41 @@ Action action(AtmosphereRequest req, AtmosphereResponse res) throws IOException, } // handler interceptor lists - Action a = invokeInterceptors(handlerWrapper.interceptors, resource, tracing); + LinkedList invokedInterceptors = handlerWrapper.interceptors; + Action a = invokeInterceptors(invokedInterceptors, resource, tracing); if (a.type() != Action.TYPE.CONTINUE && a.type() != Action.TYPE.SKIP_ATMOSPHEREHANDLER) { return a; } - // Remap occured. - if (req.getAttribute(FrameworkConfig.NEW_MAPPING) != null) { - req.removeAttribute(FrameworkConfig.NEW_MAPPING); - handlerWrapper = map(req); - if (handlerWrapper == null) { - logger.debug("Remap {}", resource.uuid()); - throw new AtmosphereMappingException("Invalid state. No AtmosphereHandler maps request for " + req.getRequestURI()); + try { + // Remap occured. + if (req.getAttribute(FrameworkConfig.NEW_MAPPING) != null) { + req.removeAttribute(FrameworkConfig.NEW_MAPPING); + handlerWrapper = map(req); + if (handlerWrapper == null) { + logger.debug("Remap {}", resource.uuid()); + throw new AtmosphereMappingException("Invalid state. No AtmosphereHandler maps request for " + req.getRequestURI()); + } + resource = configureWorkflow(resource, handlerWrapper, req, res); + resource.setBroadcaster(handlerWrapper.broadcaster); } - resource = configureWorkflow(resource, handlerWrapper, req, res); - resource.setBroadcaster(handlerWrapper.broadcaster); - } - //Unit test mock the request and will throw NPE. - boolean skipAtmosphereHandler = req.getAttribute(SKIP_ATMOSPHEREHANDLER.name()) != null - ? (Boolean) req.getAttribute(SKIP_ATMOSPHEREHANDLER.name()) : Boolean.FALSE; - if (!skipAtmosphereHandler) { - try { - logger.trace("\t Last: {}", handlerWrapper.atmosphereHandler.getClass().getName()); - handlerWrapper.atmosphereHandler.onRequest(resource); - } catch (IOException t) { - resource.onThrowable(t); - throw t; + //Unit test mock the request and will throw NPE. + boolean skipAtmosphereHandler = req.getAttribute(SKIP_ATMOSPHEREHANDLER.name()) != null + ? (Boolean) req.getAttribute(SKIP_ATMOSPHEREHANDLER.name()) : Boolean.FALSE; + if (!skipAtmosphereHandler) { + try { + logger.trace("\t Last: {}", handlerWrapper.atmosphereHandler.getClass().getName()); + handlerWrapper.atmosphereHandler.onRequest(resource); + } catch (IOException t) { + resource.onThrowable(t); + throw t; + } } + } finally{ + postInterceptors(handlerWrapper != null? handlerWrapper.interceptors: invokedInterceptors, resource); } - postInterceptors(handlerWrapper.interceptors, resource); - Action action = resource.action(); if (supportSession() && allowSessionTimeoutRemoval() && action.type().equals(Action.TYPE.SUSPEND)) { // Do not allow times out. diff --git a/modules/cpr/src/test/java/org/atmosphere/cpr/AtmosphereInterceptorTest.java b/modules/cpr/src/test/java/org/atmosphere/cpr/AtmosphereInterceptorTest.java index ba4e1396791..7a479cb6bef 100644 --- a/modules/cpr/src/test/java/org/atmosphere/cpr/AtmosphereInterceptorTest.java +++ b/modules/cpr/src/test/java/org/atmosphere/cpr/AtmosphereInterceptorTest.java @@ -16,6 +16,7 @@ package org.atmosphere.cpr; import org.atmosphere.interceptor.InvokationOrder; +import org.mockito.Mockito; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -24,10 +25,12 @@ import javax.servlet.ServletException; import java.io.IOException; import java.util.Enumeration; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import static org.mockito.Mockito.mock; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; public class AtmosphereInterceptorTest { @@ -318,4 +321,37 @@ public String toString() { assertEquals(framework.getAtmosphereHandlers().get("/" + AtmosphereFramework.MAPPING_REGEX).interceptors.removeFirst().toString(), "CORS Interceptor Support"); assertEquals(framework.getAtmosphereHandlers().get("/" + AtmosphereFramework.MAPPING_REGEX).interceptors.getFirst().toString(), "XXX"); } + + @Test + public void postInspectOnThrown() throws Exception{ + AtmosphereHandler handler = mock(AtmosphereHandler.class); + Mockito.doThrow(new RuntimeException()).when(handler).onRequest(Mockito.any(AtmosphereResource.class)); + framework.addAtmosphereHandler("/*", handler); + + final AtomicBoolean postInspected = new AtomicBoolean(false); + framework.interceptor(new AtmosphereInterceptor() { + @Override + public void configure(AtmosphereConfig config) { + } + + @Override + public void destroy() { + } + + @Override + public Action inspect(AtmosphereResource r) { + return Action.CONTINUE; + } + + @Override + public void postInspect(AtmosphereResource r) { + postInspected.set(true); + } + }); + + try{ + processor.service(mock(AtmosphereRequestImpl.class), AtmosphereResponseImpl.newInstance()); + } catch(Throwable t){} + assertTrue(postInspected.get()); + } }