HandlerExecutionChain prevents re-adding the interceptors array to the list (and declares varargs now)

Issue: SPR-12566

Author: jhoeller

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.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 HandlerExecutionChain(Object handler) {
 	 * @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 Object getHandler() {
 	}
 
 	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<HandlerInterceptor> initInterceptorList() {
 		if (this.interceptorList == null) {
 			this.interceptorList = new ArrayList<HandlerInterceptor>();
+			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;
 	}
 
 	/**

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.
@@ -26,6 +26,7 @@
 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 HandlerExecutionChain(Object handler) {
 	 * @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 HandlerExecutionChain(Object handler, HandlerInterceptor[] interceptors)
 		}
 	}
 
+
 	/**
 	 * Return the handler object to execute.
 	 * @return the handler object
@@ -85,25 +87,25 @@ public Object getHandler() {
 	}
 
 	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<HandlerInterceptor> initInterceptorList() {
 		if (this.interceptorList == null) {
 			this.interceptorList = new ArrayList<HandlerInterceptor>();
+			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,16 +119,18 @@ public HandlerInterceptor[] getInterceptors() {
 		return this.interceptors;
 	}
 
+
 	/**
 	 * Apply preHandle methods of registered interceptors.
 	 * @return {@code true} if the execution chain should proceed with the
 	 * next interceptor or the handler itself. Else, DispatcherServlet assumes
 	 * 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 @@ boolean applyPreHandle(HttpServletRequest request, HttpServletResponse response)
 	 * 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 @@ void applyPostHandle(HttpServletRequest request, HttpServletResponse response, M
 	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 @@ void triggerAfterCompletion(HttpServletRequest request, HttpServletResponse resp
 	 * 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()}.
 	 */

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.
@@ -46,6 +46,7 @@ public class HandlerExecutionChainTests {
 
 	private AsyncHandlerInterceptor interceptor3;
 
+
 	@Before
 	public void setup() {
 		this.request = new MockHttpServletRequest();
@@ -60,9 +61,12 @@ public void setup() {
 
 		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();