From 37713ae9dd8213ada99e1fb2fe73d58fa2f3f289 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 29 Dec 2014 18:58:41 +0100 Subject: [PATCH] HandlerExecutionChain prevents re-adding the interceptors array to the list (and declares varargs now) Issue: SPR-12566 (cherry picked from commit 6b3023c) --- .../web/portlet/HandlerExecutionChain.java | 29 +++--- .../web/servlet/HandlerExecutionChain.java | 95 ++++++++++--------- .../servlet/HandlerExecutionChainTests.java | 7 +- 3 files changed, 71 insertions(+), 60 deletions(-) diff --git a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/HandlerExecutionChain.java b/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/HandlerExecutionChain.java index 80c64e29545..e09a0198684 100644 --- a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/HandlerExecutionChain.java +++ b/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/HandlerExecutionChain.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2014 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. @@ -21,6 +21,7 @@ import java.util.Arrays; import java.util.List; import org.springframework.util.CollectionUtils; +import org.springframework.util.ObjectUtils; /** * Handler execution chain, consisting of handler object and any handler interceptors. @@ -45,7 +46,7 @@ public class HandlerExecutionChain { * @param handler the handler object to execute */ public HandlerExecutionChain(Object handler) { - this(handler, null); + this(handler, (HandlerInterceptor[]) null); } /** @@ -54,7 +55,7 @@ public class HandlerExecutionChain { * @param interceptors the array of interceptors to apply * (in the given order) before the handler itself executes */ - public HandlerExecutionChain(Object handler, HandlerInterceptor[] interceptors) { + public HandlerExecutionChain(Object handler, HandlerInterceptor... interceptors) { if (handler instanceof HandlerExecutionChain) { HandlerExecutionChain originalChain = (HandlerExecutionChain) handler; this.handler = originalChain.getHandler(); @@ -78,25 +79,25 @@ public class HandlerExecutionChain { } public void addInterceptor(HandlerInterceptor interceptor) { - initInterceptorList(); - this.interceptorList.add(interceptor); + initInterceptorList().add(interceptor); } - public void addInterceptors(HandlerInterceptor[] interceptors) { - if (interceptors != null) { - initInterceptorList(); - this.interceptorList.addAll(Arrays.asList(interceptors)); + public void addInterceptors(HandlerInterceptor... interceptors) { + if (!ObjectUtils.isEmpty(interceptors)) { + initInterceptorList().addAll(Arrays.asList(interceptors)); } } - private void initInterceptorList() { + private List initInterceptorList() { if (this.interceptorList == null) { this.interceptorList = new ArrayList(); + if (this.interceptors != null) { + // An interceptor array specified through the constructor + this.interceptorList.addAll(Arrays.asList(this.interceptors)); + } } - if (this.interceptors != null) { - this.interceptorList.addAll(Arrays.asList(this.interceptors)); - this.interceptors = null; - } + this.interceptors = null; + return this.interceptorList; } /** diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerExecutionChain.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerExecutionChain.java index dbbe8946baf..6fab48f2027 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerExecutionChain.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerExecutionChain.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2014 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,13 +19,14 @@ package org.springframework.web.servlet; import java.util.ArrayList; import java.util.Arrays; import java.util.List; - import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.util.CollectionUtils; +import org.springframework.util.ObjectUtils; /** * Handler execution chain, consisting of handler object and any handler interceptors. @@ -53,7 +54,7 @@ public class HandlerExecutionChain { * @param handler the handler object to execute */ public HandlerExecutionChain(Object handler) { - this(handler, null); + this(handler, (HandlerInterceptor[]) null); } /** @@ -62,7 +63,7 @@ public class HandlerExecutionChain { * @param interceptors the array of interceptors to apply * (in the given order) before the handler itself executes */ - public HandlerExecutionChain(Object handler, HandlerInterceptor[] interceptors) { + public HandlerExecutionChain(Object handler, HandlerInterceptor... interceptors) { if (handler instanceof HandlerExecutionChain) { HandlerExecutionChain originalChain = (HandlerExecutionChain) handler; this.handler = originalChain.getHandler(); @@ -76,6 +77,7 @@ public class HandlerExecutionChain { } } + /** * Return the handler object to execute. * @return the handler object @@ -85,25 +87,25 @@ public class HandlerExecutionChain { } public void addInterceptor(HandlerInterceptor interceptor) { - initInterceptorList(); - this.interceptorList.add(interceptor); + initInterceptorList().add(interceptor); } - public void addInterceptors(HandlerInterceptor[] interceptors) { - if (interceptors != null) { - initInterceptorList(); - this.interceptorList.addAll(Arrays.asList(interceptors)); + public void addInterceptors(HandlerInterceptor... interceptors) { + if (!ObjectUtils.isEmpty(interceptors)) { + initInterceptorList().addAll(Arrays.asList(interceptors)); } } - private void initInterceptorList() { + private List initInterceptorList() { if (this.interceptorList == null) { this.interceptorList = new ArrayList(); + if (this.interceptors != null) { + // An interceptor array specified through the constructor + this.interceptorList.addAll(Arrays.asList(this.interceptors)); + } } - if (this.interceptors != null) { - this.interceptorList.addAll(Arrays.asList(this.interceptors)); - this.interceptors = null; - } + this.interceptors = null; + return this.interceptorList; } /** @@ -117,6 +119,7 @@ public class HandlerExecutionChain { return this.interceptors; } + /** * Apply preHandle methods of registered interceptors. * @return {@code true} if the execution chain should proceed with the @@ -124,9 +127,10 @@ public class HandlerExecutionChain { * that this interceptor has already dealt with the response itself. */ boolean applyPreHandle(HttpServletRequest request, HttpServletResponse response) throws Exception { - if (getInterceptors() != null) { - for (int i = 0; i < getInterceptors().length; i++) { - HandlerInterceptor interceptor = getInterceptors()[i]; + HandlerInterceptor[] interceptors = getInterceptors(); + if (!ObjectUtils.isEmpty(interceptors)) { + for (int i = 0; i < interceptors.length; i++) { + HandlerInterceptor interceptor = interceptors[i]; if (!interceptor.preHandle(request, response, this.handler)) { triggerAfterCompletion(request, response, null); return false; @@ -141,12 +145,12 @@ public class HandlerExecutionChain { * Apply postHandle methods of registered interceptors. */ void applyPostHandle(HttpServletRequest request, HttpServletResponse response, ModelAndView mv) throws Exception { - if (getInterceptors() == null) { - return; - } - for (int i = getInterceptors().length - 1; i >= 0; i--) { - HandlerInterceptor interceptor = getInterceptors()[i]; - interceptor.postHandle(request, response, this.handler, mv); + HandlerInterceptor[] interceptors = getInterceptors(); + if (!ObjectUtils.isEmpty(interceptors)) { + for (int i = interceptors.length - 1; i >= 0; i--) { + HandlerInterceptor interceptor = interceptors[i]; + interceptor.postHandle(request, response, this.handler, mv); + } } } @@ -158,16 +162,16 @@ public class HandlerExecutionChain { void triggerAfterCompletion(HttpServletRequest request, HttpServletResponse response, Exception ex) throws Exception { - if (getInterceptors() == null) { - return; - } - for (int i = this.interceptorIndex; i >= 0; i--) { - HandlerInterceptor interceptor = getInterceptors()[i]; - try { - interceptor.afterCompletion(request, response, this.handler, ex); - } - catch (Throwable ex2) { - logger.error("HandlerInterceptor.afterCompletion threw exception", ex2); + HandlerInterceptor[] interceptors = getInterceptors(); + if (!ObjectUtils.isEmpty(interceptors)) { + for (int i = this.interceptorIndex; i >= 0; i--) { + HandlerInterceptor interceptor = interceptors[i]; + try { + interceptor.afterCompletion(request, response, this.handler, ex); + } + catch (Throwable ex2) { + logger.error("HandlerInterceptor.afterCompletion threw exception", ex2); + } } } } @@ -176,22 +180,23 @@ public class HandlerExecutionChain { * Apply afterConcurrentHandlerStarted callback on mapped AsyncHandlerInterceptors. */ void applyAfterConcurrentHandlingStarted(HttpServletRequest request, HttpServletResponse response) { - if (getInterceptors() == null) { - return; - } - for (int i = getInterceptors().length - 1; i >= 0; i--) { - if (interceptors[i] instanceof AsyncHandlerInterceptor) { - try { - AsyncHandlerInterceptor asyncInterceptor = (AsyncHandlerInterceptor) this.interceptors[i]; - asyncInterceptor.afterConcurrentHandlingStarted(request, response, this.handler); - } - catch (Throwable ex) { - logger.error("Interceptor [" + interceptors[i] + "] failed in afterConcurrentHandlingStarted", ex); + HandlerInterceptor[] interceptors = getInterceptors(); + if (!ObjectUtils.isEmpty(interceptors)) { + for (int i = interceptors.length - 1; i >= 0; i--) { + if (interceptors[i] instanceof AsyncHandlerInterceptor) { + try { + AsyncHandlerInterceptor asyncInterceptor = (AsyncHandlerInterceptor) interceptors[i]; + asyncInterceptor.afterConcurrentHandlingStarted(request, response, this.handler); + } + catch (Throwable ex) { + logger.error("Interceptor [" + interceptors[i] + "] failed in afterConcurrentHandlingStarted", ex); + } } } } } + /** * Delegates to the handler's {@code toString()}. */ diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/HandlerExecutionChainTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/HandlerExecutionChainTests.java index a4f9572dc15..b2aac29ac90 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/HandlerExecutionChainTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/HandlerExecutionChainTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -18,6 +18,7 @@ package org.springframework.web.servlet; import org.junit.Before; import org.junit.Test; + import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; @@ -45,6 +46,7 @@ public class HandlerExecutionChainTests { private AsyncHandlerInterceptor interceptor3; + @Before public void setup() { this.request = new MockHttpServletRequest(); @@ -59,9 +61,12 @@ public class HandlerExecutionChainTests { this.chain.addInterceptor(this.interceptor1); this.chain.addInterceptor(this.interceptor2); + assertEquals(2, this.chain.getInterceptors().length); this.chain.addInterceptor(this.interceptor3); + assertEquals(3, this.chain.getInterceptors().length); } + @Test public void successScenario() throws Exception { ModelAndView mav = new ModelAndView();