Skip to content

Commit

Permalink
HandlerExecutionChain prevents re-adding the interceptors array to th…
Browse files Browse the repository at this point in the history 
…e list (and declares varargs now)

Issue: SPR-12566
(cherry picked from commit 6b3023c)
  • Loading branch information
@jhoeller
jhoeller committed Dec 29, 2014
1 parent e118086 commit 37713ae
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 60 deletions.
29 changes: 15 additions & 14 deletions ...g-webmvc-portlet/src/main/java/org/springframework/web/portlet/HandlerExecutionChain.java
View file
@@ -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.
Expand All @@ -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.
Expand All @@ -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);
}

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

/**
Expand Down
95 changes: 50 additions & 45 deletions spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerExecutionChain.java
View file
@@ -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.
Expand All @@ -19,13 +19,14 @@
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.
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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();
Expand All @@ -76,6 +77,7 @@ public HandlerExecutionChain(Object handler, HandlerInterceptor[] interceptors)
}
}


/**
* Return the handler object to execute.
* @return the handler object
Expand All @@ -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;
}

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

Expand All @@ -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);
}
}
}
}
Expand All @@ -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()}.
*/
Expand Down
7 changes: 6 additions & 1 deletion spring-webmvc/src/test/java/org/springframework/web/servlet/HandlerExecutionChainTests.java
View file
@@ -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.
Expand All @@ -18,6 +18,7 @@

import org.junit.Before;
import org.junit.Test;

import org.springframework.mock.web.test.MockHttpServletRequest;
import org.springframework.mock.web.test.MockHttpServletResponse;

Expand Down Expand Up @@ -45,6 +46,7 @@ public class HandlerExecutionChainTests {

private AsyncHandlerInterceptor interceptor3;


@Before
public void setup() {
this.request = new MockHttpServletRequest();
Expand All @@ -59,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();
Expand Down

0 comments on commit 37713ae

Please sign in to comment.