From b6ef3cfad5761b00487eb4ce4cbcf140c56b6511 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 7 Aug 2020 00:32:52 +0200 Subject: [PATCH] Refactor HandlerExecutionChain towards List-centric interceptor storage Closes gh-25500 --- .../web/servlet/HandlerExecutionChain.java | 153 ++++++++---------- .../handler/AbstractHandlerMapping.java | 6 +- .../handler/HandlerMappingIntrospector.java | 8 +- .../servlet/HandlerExecutionChainTests.java | 4 +- .../web/servlet/config/MvcNamespaceTests.java | 98 +++++------ ...MvcConfigurationSupportExtensionTests.java | 27 ++-- .../WebMvcConfigurationSupportTests.java | 15 +- .../servlet/handler/HandlerMappingTests.java | 7 +- .../PathMatchingUrlHandlerMappingTests.java | 11 +- .../handler/SimpleUrlHandlerMappingTests.java | 7 +- ...RequestMappingInfoHandlerMappingTests.java | 3 +- .../method/annotation/CrossOriginTests.java | 11 +- 12 files changed, 162 insertions(+), 188 deletions(-) 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 b737f5a01a..21e270bd15 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-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -17,6 +17,8 @@ package org.springframework.web.servlet; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import javax.servlet.http.HttpServletRequest; @@ -27,7 +29,6 @@ import org.apache.commons.logging.LogFactory; import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; -import org.springframework.util.ObjectUtils; /** * Handler execution chain, consisting of handler object and any handler interceptors. @@ -43,11 +44,7 @@ public class HandlerExecutionChain { private final Object handler; - @Nullable - private HandlerInterceptor[] interceptors; - - @Nullable - private List interceptorList; + private final List interceptorList = new ArrayList<>(); private int interceptorIndex = -1; @@ -67,17 +64,26 @@ public class HandlerExecutionChain { * (in the given order) before the handler itself executes */ public HandlerExecutionChain(Object handler, @Nullable HandlerInterceptor... interceptors) { + this(handler, (interceptors != null ? Arrays.asList(interceptors) : Collections.emptyList())); + } + + /** + * Create a new HandlerExecutionChain. + * @param handler the handler object to execute + * @param interceptorList the list of interceptors to apply + * (in the given order) before the handler itself executes + * @since 5.3 + */ + public HandlerExecutionChain(Object handler, List interceptorList) { if (handler instanceof HandlerExecutionChain) { HandlerExecutionChain originalChain = (HandlerExecutionChain) handler; this.handler = originalChain.getHandler(); - this.interceptorList = new ArrayList<>(); - CollectionUtils.mergeArrayIntoCollection(originalChain.getInterceptors(), this.interceptorList); - CollectionUtils.mergeArrayIntoCollection(interceptors, this.interceptorList); + this.interceptorList.addAll(originalChain.interceptorList); } else { this.handler = handler; - this.interceptors = interceptors; } + this.interceptorList.addAll(interceptorList); } @@ -88,30 +94,25 @@ public class HandlerExecutionChain { return this.handler; } + /** + * Add the given interceptor to the end of this chain. + */ public void addInterceptor(HandlerInterceptor interceptor) { - initInterceptorList().add(interceptor); + this.interceptorList.add(interceptor); } + /** + * Add the given interceptor at the specified index of this chain. + */ public void addInterceptor(int index, HandlerInterceptor interceptor) { - initInterceptorList().add(index, interceptor); + this.interceptorList.add(index, interceptor); } + /** + * Add the given interceptors to the end of this chain. + */ public void addInterceptors(HandlerInterceptor... interceptors) { - if (!ObjectUtils.isEmpty(interceptors)) { - CollectionUtils.mergeArrayIntoCollection(interceptors, initInterceptorList()); - } - } - - private List initInterceptorList() { - if (this.interceptorList == null) { - this.interceptorList = new ArrayList<>(); - if (this.interceptors != null) { - // An interceptor array specified through the constructor - CollectionUtils.mergeArrayIntoCollection(this.interceptors, this.interceptorList); - } - } - this.interceptors = null; - return this.interceptorList; + CollectionUtils.mergeArrayIntoCollection(interceptors, this.interceptorList); } /** @@ -120,10 +121,17 @@ public class HandlerExecutionChain { */ @Nullable public HandlerInterceptor[] getInterceptors() { - if (this.interceptors == null && this.interceptorList != null) { - this.interceptors = this.interceptorList.toArray(new HandlerInterceptor[0]); - } - return this.interceptors; + return (!this.interceptorList.isEmpty() ? this.interceptorList.toArray(new HandlerInterceptor[0]) : null); + } + + /** + * Return the list of interceptors to apply (in the given order). + * @return the list of HandlerInterceptors instances (potentially empty) + * @since 5.3 + */ + public List getInterceptorList() { + return (!this.interceptorList.isEmpty() ? Collections.unmodifiableList(this.interceptorList) : + Collections.emptyList()); } @@ -134,16 +142,13 @@ public class HandlerExecutionChain { * that this interceptor has already dealt with the response itself. */ boolean applyPreHandle(HttpServletRequest request, HttpServletResponse response) throws Exception { - 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; - } - this.interceptorIndex = i; + for (int i = 0; i < this.interceptorList.size(); i++) { + HandlerInterceptor interceptor = this.interceptorList.get(i); + if (!interceptor.preHandle(request, response, this.handler)) { + triggerAfterCompletion(request, response, null); + return false; } + this.interceptorIndex = i; } return true; } @@ -154,12 +159,9 @@ public class HandlerExecutionChain { void applyPostHandle(HttpServletRequest request, HttpServletResponse response, @Nullable ModelAndView mv) throws Exception { - 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); - } + for (int i = this.interceptorList.size() - 1; i >= 0; i--) { + HandlerInterceptor interceptor = this.interceptorList.get(i); + interceptor.postHandle(request, response, this.handler, mv); } } @@ -168,19 +170,14 @@ public class HandlerExecutionChain { * Will just invoke afterCompletion for all interceptors whose preHandle invocation * has successfully completed and returned true. */ - void triggerAfterCompletion(HttpServletRequest request, HttpServletResponse response, @Nullable Exception ex) - throws Exception { - - 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); - } + void triggerAfterCompletion(HttpServletRequest request, HttpServletResponse response, @Nullable Exception ex) { + for (int i = this.interceptorIndex; i >= 0; i--) { + HandlerInterceptor interceptor = this.interceptorList.get(i); + try { + interceptor.afterCompletion(request, response, this.handler, ex); + } + catch (Throwable ex2) { + logger.error("HandlerInterceptor.afterCompletion threw exception", ex2); } } } @@ -189,16 +186,16 @@ public class HandlerExecutionChain { * Apply afterConcurrentHandlerStarted callback on mapped AsyncHandlerInterceptors. */ void applyAfterConcurrentHandlingStarted(HttpServletRequest request, HttpServletResponse response) { - 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); + for (int i = this.interceptorList.size() - 1; i >= 0; i--) { + HandlerInterceptor interceptor = this.interceptorList.get(i); + if (interceptor instanceof AsyncHandlerInterceptor) { + try { + AsyncHandlerInterceptor asyncInterceptor = (AsyncHandlerInterceptor) interceptor; + asyncInterceptor.afterConcurrentHandlingStarted(request, response, this.handler); + } + catch (Throwable ex) { + if (logger.isErrorEnabled()) { + logger.error("Interceptor [" + interceptor + "] failed in afterConcurrentHandlingStarted", ex); } } } @@ -207,23 +204,11 @@ public class HandlerExecutionChain { /** - * Delegates to the handler and interceptors' {@code toString()}. + * Delegates to the handler's {@code toString()} implementation. */ @Override public String toString() { - Object handler = getHandler(); - StringBuilder sb = new StringBuilder(); - sb.append("HandlerExecutionChain with [").append(handler).append("] and "); - if (this.interceptorList != null) { - sb.append(this.interceptorList.size()); - } - else if (this.interceptors != null) { - sb.append(this.interceptors.length); - } - else { - sb.append(0); - } - return sb.append(" interceptors").toString(); + return "HandlerExecutionChain with [" + getHandler() + "] and " + this.interceptorList.size() + " interceptors"; } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java index fe55060172..1a12cb6010 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java @@ -652,13 +652,13 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport HandlerExecutionChain chain, @Nullable CorsConfiguration config) { if (CorsUtils.isPreFlightRequest(request)) { - HandlerInterceptor[] interceptors = chain.getInterceptors(); - chain = new HandlerExecutionChain(new PreFlightHandler(config), interceptors); + List interceptors = chain.getInterceptorList(); + return new HandlerExecutionChain(new PreFlightHandler(config), interceptors); } else { chain.addInterceptor(0, new CorsInterceptor(config)); + return chain; } - return chain; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/HandlerMappingIntrospector.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/HandlerMappingIntrospector.java index ac7a2689b5..98c9f848ec 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/HandlerMappingIntrospector.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/HandlerMappingIntrospector.java @@ -169,11 +169,9 @@ public class HandlerMappingIntrospector if (handler == null) { continue; } - if (handler.getInterceptors() != null) { - for (HandlerInterceptor interceptor : handler.getInterceptors()) { - if (interceptor instanceof CorsConfigurationSource) { - return ((CorsConfigurationSource) interceptor).getCorsConfiguration(wrapper); - } + for (HandlerInterceptor interceptor : handler.getInterceptorList()) { + if (interceptor instanceof CorsConfigurationSource) { + return ((CorsConfigurationSource) interceptor).getCorsConfiguration(wrapper); } } if (handler.getHandler() instanceof CorsConfigurationSource) { 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 5ece77d1e2..efe279e4fb 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-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -65,9 +65,7 @@ public class HandlerExecutionChainTests { this.chain.addInterceptor(this.interceptor1); this.chain.addInterceptor(this.interceptor2); - assertThat(this.chain.getInterceptors().length).isEqualTo(2); this.chain.addInterceptor(this.interceptor3); - assertThat(this.chain.getInterceptors().length).isEqualTo(3); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java index d2b99f2f63..2a1c372751 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java @@ -238,9 +238,9 @@ public class MvcNamespaceTests { MockHttpServletResponse response = new MockHttpServletResponse(); HandlerExecutionChain chain = mapping.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(1); - assertThat(chain.getInterceptors()[0] instanceof ConversionServiceExposingInterceptor).isTrue(); - ConversionServiceExposingInterceptor interceptor = (ConversionServiceExposingInterceptor) chain.getInterceptors()[0]; + assertThat(chain.getInterceptorList().size()).isEqualTo(1); + assertThat(chain.getInterceptorList().get(0) instanceof ConversionServiceExposingInterceptor).isTrue(); + ConversionServiceExposingInterceptor interceptor = (ConversionServiceExposingInterceptor) chain.getInterceptorList().get(0); interceptor.preHandle(request, response, handlerMethod); assertThat(request.getAttribute(ConversionService.class.getName())).isSameAs(appContext.getBean(ConversionService.class)); @@ -278,9 +278,9 @@ public class MvcNamespaceTests { MockHttpServletResponse response = new MockHttpServletResponse(); HandlerExecutionChain chain = mapping.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(1); - assertThat(chain.getInterceptors()[0] instanceof ConversionServiceExposingInterceptor).isTrue(); - ConversionServiceExposingInterceptor interceptor = (ConversionServiceExposingInterceptor) chain.getInterceptors()[0]; + assertThat(chain.getInterceptorList().size()).isEqualTo(1); + assertThat(chain.getInterceptorList().get(0) instanceof ConversionServiceExposingInterceptor).isTrue(); + ConversionServiceExposingInterceptor interceptor = (ConversionServiceExposingInterceptor) chain.getInterceptorList().get(0); interceptor.preHandle(request, response, handler); assertThat(request.getAttribute(ConversionService.class.getName())).isSameAs(appContext.getBean("conversionService")); @@ -330,23 +330,23 @@ public class MvcNamespaceTests { request.addParameter("theme", "green"); HandlerExecutionChain chain = mapping.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(4); - assertThat(chain.getInterceptors()[0] instanceof ConversionServiceExposingInterceptor).isTrue(); - assertThat(chain.getInterceptors()[1] instanceof LocaleChangeInterceptor).isTrue(); - assertThat(chain.getInterceptors()[2] instanceof ThemeChangeInterceptor).isTrue(); - assertThat(chain.getInterceptors()[3] instanceof UserRoleAuthorizationInterceptor).isTrue(); + assertThat(chain.getInterceptorList().size()).isEqualTo(4); + assertThat(chain.getInterceptorList().get(0) instanceof ConversionServiceExposingInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(1) instanceof LocaleChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(2) instanceof ThemeChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(3) instanceof UserRoleAuthorizationInterceptor).isTrue(); request.setRequestURI("/admin/users"); chain = mapping.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(2); + assertThat(chain.getInterceptorList().size()).isEqualTo(2); request.setRequestURI("/logged/accounts/12345"); chain = mapping.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(3); + assertThat(chain.getInterceptorList().size()).isEqualTo(3); request.setRequestURI("/foo/logged"); chain = mapping.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(3); + assertThat(chain.getInterceptorList().size()).isEqualTo(3); } @Test @@ -391,7 +391,7 @@ public class MvcNamespaceTests { assertThat(chain.getHandler() instanceof ResourceHttpRequestHandler).isTrue(); MockHttpServletResponse response = new MockHttpServletResponse(); - for (HandlerInterceptor interceptor : chain.getInterceptors()) { + for (HandlerInterceptor interceptor : chain.getInterceptorList()) { interceptor.preHandle(request, response, chain.getHandler()); } ModelAndView mv = adapter.handle(request, response, chain.getHandler()); @@ -555,13 +555,13 @@ public class MvcNamespaceTests { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); HandlerExecutionChain chain = mapping.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(3); - assertThat(chain.getInterceptors()[0] instanceof ConversionServiceExposingInterceptor).isTrue(); - assertThat(chain.getInterceptors()[1] instanceof LocaleChangeInterceptor).isTrue(); - assertThat(chain.getInterceptors()[2] instanceof ThemeChangeInterceptor).isTrue(); - LocaleChangeInterceptor interceptor = (LocaleChangeInterceptor) chain.getInterceptors()[1]; + assertThat(chain.getInterceptorList().size()).isEqualTo(3); + assertThat(chain.getInterceptorList().get(0) instanceof ConversionServiceExposingInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(1) instanceof LocaleChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(2) instanceof ThemeChangeInterceptor).isTrue(); + LocaleChangeInterceptor interceptor = (LocaleChangeInterceptor) chain.getInterceptorList().get(1); assertThat(interceptor.getParamName()).isEqualTo("lang"); - ThemeChangeInterceptor interceptor2 = (ThemeChangeInterceptor) chain.getInterceptors()[2]; + ThemeChangeInterceptor interceptor2 = (ThemeChangeInterceptor) chain.getInterceptorList().get(2); assertThat(interceptor2.getParamName()).isEqualTo("style"); } @@ -581,10 +581,10 @@ public class MvcNamespaceTests { request.setMethod("GET"); HandlerExecutionChain chain = mapping.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(3); - assertThat(chain.getInterceptors()[0] instanceof ConversionServiceExposingInterceptor).isTrue(); - assertThat(chain.getInterceptors()[1] instanceof LocaleChangeInterceptor).isTrue(); - assertThat(chain.getInterceptors()[2] instanceof ThemeChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().size()).isEqualTo(3); + assertThat(chain.getInterceptorList().get(0) instanceof ConversionServiceExposingInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(1) instanceof LocaleChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(2) instanceof ThemeChangeInterceptor).isTrue(); SimpleUrlHandlerMapping mapping2 = appContext.getBean(SimpleUrlHandlerMapping.class); assertThat(mapping2).isNotNull(); @@ -594,10 +594,10 @@ public class MvcNamespaceTests { request = new MockHttpServletRequest("GET", "/foo"); chain = mapping2.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(4); - assertThat(chain.getInterceptors()[1] instanceof ConversionServiceExposingInterceptor).isTrue(); - assertThat(chain.getInterceptors()[2] instanceof LocaleChangeInterceptor).isTrue(); - assertThat(chain.getInterceptors()[3] instanceof ThemeChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().size()).isEqualTo(4); + assertThat(chain.getInterceptorList().get(1) instanceof ConversionServiceExposingInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(2) instanceof LocaleChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(3) instanceof ThemeChangeInterceptor).isTrue(); ModelAndView mv = adapter.handle(request, new MockHttpServletResponse(), chain.getHandler()); assertThat((Object) mv.getViewName()).isNull(); @@ -605,10 +605,10 @@ public class MvcNamespaceTests { request.setContextPath("/myapp"); request.setServletPath("/app"); chain = mapping2.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(4); - assertThat(chain.getInterceptors()[1] instanceof ConversionServiceExposingInterceptor).isTrue(); - assertThat(chain.getInterceptors()[2] instanceof LocaleChangeInterceptor).isTrue(); - assertThat(chain.getInterceptors()[3] instanceof ThemeChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().size()).isEqualTo(4); + assertThat(chain.getInterceptorList().get(1) instanceof ConversionServiceExposingInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(2) instanceof LocaleChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(3) instanceof ThemeChangeInterceptor).isTrue(); mv = adapter.handle(request, new MockHttpServletResponse(), chain.getHandler()); assertThat(mv.getViewName()).isEqualTo("baz"); @@ -616,10 +616,10 @@ public class MvcNamespaceTests { request.setContextPath("/myapp"); request.setServletPath("/app"); chain = mapping2.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(4); - assertThat(chain.getInterceptors()[1] instanceof ConversionServiceExposingInterceptor).isTrue(); - assertThat(chain.getInterceptors()[2] instanceof LocaleChangeInterceptor).isTrue(); - assertThat(chain.getInterceptors()[3] instanceof ThemeChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().size()).isEqualTo(4); + assertThat(chain.getInterceptorList().get(1) instanceof ConversionServiceExposingInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(2) instanceof LocaleChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(3) instanceof ThemeChangeInterceptor).isTrue(); mv = adapter.handle(request, new MockHttpServletResponse(), chain.getHandler()); assertThat(mv.getViewName()).isEqualTo("root"); @@ -660,10 +660,10 @@ public class MvcNamespaceTests { request.setServletPath("/app/"); request.setAttribute("com.ibm.websphere.servlet.uri_non_decoded", "/myapp/app/bar"); HandlerExecutionChain chain = mapping2.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(4); - assertThat(chain.getInterceptors()[1] instanceof ConversionServiceExposingInterceptor).isTrue(); - assertThat(chain.getInterceptors()[2] instanceof LocaleChangeInterceptor).isTrue(); - assertThat(chain.getInterceptors()[3] instanceof ThemeChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().size()).isEqualTo(4); + assertThat(chain.getInterceptorList().get(1) instanceof ConversionServiceExposingInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(2) instanceof LocaleChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(3) instanceof ThemeChangeInterceptor).isTrue(); ModelAndView mv2 = adapter.handle(request, new MockHttpServletResponse(), chain.getHandler()); assertThat(mv2.getViewName()).isEqualTo("baz"); @@ -671,10 +671,10 @@ public class MvcNamespaceTests { request.setContextPath("/myapp"); request.setServletPath("/app/"); chain = mapping2.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(4); - assertThat(chain.getInterceptors()[1] instanceof ConversionServiceExposingInterceptor).isTrue(); - assertThat(chain.getInterceptors()[2] instanceof LocaleChangeInterceptor).isTrue(); - assertThat(chain.getInterceptors()[3] instanceof ThemeChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().size()).isEqualTo(4); + assertThat(chain.getInterceptorList().get(1) instanceof ConversionServiceExposingInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(2) instanceof LocaleChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(3) instanceof ThemeChangeInterceptor).isTrue(); ModelAndView mv3 = adapter.handle(request, new MockHttpServletResponse(), chain.getHandler()); assertThat(mv3.getViewName()).isEqualTo("root"); @@ -682,10 +682,10 @@ public class MvcNamespaceTests { request.setContextPath("/myapp"); request.setServletPath("/"); chain = mapping2.getHandler(request); - assertThat(chain.getInterceptors().length).isEqualTo(4); - assertThat(chain.getInterceptors()[1] instanceof ConversionServiceExposingInterceptor).isTrue(); - assertThat(chain.getInterceptors()[2] instanceof LocaleChangeInterceptor).isTrue(); - assertThat(chain.getInterceptors()[3] instanceof ThemeChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().size()).isEqualTo(4); + assertThat(chain.getInterceptorList().get(1) instanceof ConversionServiceExposingInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(2) instanceof LocaleChangeInterceptor).isTrue(); + assertThat(chain.getInterceptorList().get(3) instanceof ThemeChangeInterceptor).isTrue(); mv3 = adapter.handle(request, new MockHttpServletResponse(), chain.getHandler()); assertThat(mv3.getViewName()).isEqualTo("root"); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportExtensionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportExtensionTests.java index f4055a93da..8b69c9c887 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportExtensionTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportExtensionTests.java @@ -64,6 +64,7 @@ import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.method.support.HandlerMethodReturnValueHandler; import org.springframework.web.servlet.HandlerExceptionResolver; import org.springframework.web.servlet.HandlerExecutionChain; +import org.springframework.web.servlet.HandlerInterceptor; import org.springframework.web.servlet.View; import org.springframework.web.servlet.ViewResolver; import org.springframework.web.servlet.handler.AbstractHandlerMapping; @@ -136,12 +137,13 @@ public class WebMvcConfigurationSupportExtensionTests { assertThat(rmHandlerMapping.getPathMatcher().getClass()).isEqualTo(TestPathMatcher.class); HandlerExecutionChain chain = rmHandlerMapping.getHandler(new MockHttpServletRequest("GET", "/")); assertThat(chain).isNotNull(); - assertThat(chain.getInterceptors()).isNotNull(); - assertThat(chain.getInterceptors().length).isEqualTo(4); - assertThat(chain.getInterceptors()[0].getClass().getSimpleName()).isEqualTo("CorsInterceptor"); - assertThat(chain.getInterceptors()[1].getClass()).isEqualTo(LocaleChangeInterceptor.class); - assertThat(chain.getInterceptors()[2].getClass()).isEqualTo(ConversionServiceExposingInterceptor.class); - assertThat(chain.getInterceptors()[3].getClass()).isEqualTo(ResourceUrlProviderExposingInterceptor.class); + HandlerInterceptor[] interceptors = chain.getInterceptors(); + assertThat(interceptors).isNotNull(); + assertThat(interceptors.length).isEqualTo(4); + assertThat(interceptors[0].getClass().getSimpleName()).isEqualTo("CorsInterceptor"); + assertThat(interceptors[1].getClass()).isEqualTo(LocaleChangeInterceptor.class); + assertThat(interceptors[2].getClass()).isEqualTo(ConversionServiceExposingInterceptor.class); + assertThat(interceptors[3].getClass()).isEqualTo(ResourceUrlProviderExposingInterceptor.class); Map map = rmHandlerMapping.getHandlerMethods(); assertThat(map.size()).isEqualTo(2); @@ -180,12 +182,13 @@ public class WebMvcConfigurationSupportExtensionTests { chain = handlerMapping.getHandler(new MockHttpServletRequest("GET", "/resources/foo.gif")); assertThat(chain).isNotNull(); assertThat(chain.getHandler()).isNotNull(); - assertThat(chain.getInterceptors().length).as(Arrays.toString(chain.getInterceptors())).isEqualTo(5); - assertThat(chain.getInterceptors()[0].getClass().getSimpleName()).isEqualTo("CorsInterceptor"); - // PathExposingHandlerInterceptor at chain.getInterceptors()[1] - assertThat(chain.getInterceptors()[2].getClass()).isEqualTo(LocaleChangeInterceptor.class); - assertThat(chain.getInterceptors()[3].getClass()).isEqualTo(ConversionServiceExposingInterceptor.class); - assertThat(chain.getInterceptors()[4].getClass()).isEqualTo(ResourceUrlProviderExposingInterceptor.class); + interceptors = chain.getInterceptors(); + assertThat(interceptors.length).as(Arrays.toString(interceptors)).isEqualTo(5); + assertThat(interceptors[0].getClass().getSimpleName()).isEqualTo("CorsInterceptor"); + // PathExposingHandlerInterceptor at interceptors[1] + assertThat(interceptors[2].getClass()).isEqualTo(LocaleChangeInterceptor.class); + assertThat(interceptors[3].getClass()).isEqualTo(ConversionServiceExposingInterceptor.class); + assertThat(interceptors[4].getClass()).isEqualTo(ResourceUrlProviderExposingInterceptor.class); handlerMapping = (AbstractHandlerMapping) this.config.defaultServletHandlerMapping(); handlerMapping.setApplicationContext(this.context); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportTests.java index f74629287e..dad6504f9a 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportTests.java @@ -66,6 +66,7 @@ import org.springframework.web.method.support.ModelAndViewContainer; import org.springframework.web.servlet.FlashMapManager; import org.springframework.web.servlet.HandlerExceptionResolver; import org.springframework.web.servlet.HandlerExecutionChain; +import org.springframework.web.servlet.HandlerInterceptor; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.LocaleResolver; import org.springframework.web.servlet.RequestToViewNameTranslator; @@ -123,8 +124,9 @@ public class WebMvcConfigurationSupportTests { HandlerExecutionChain chain = handlerMapping.getHandler(new MockHttpServletRequest("GET", "/")); assertThat(chain).isNotNull(); - assertThat(chain.getInterceptors()).isNotNull(); - assertThat(chain.getInterceptors()[0].getClass()).isEqualTo(ConversionServiceExposingInterceptor.class); + HandlerInterceptor[] interceptors = chain.getInterceptors(); + assertThat(interceptors).isNotNull(); + assertThat(interceptors[0].getClass()).isEqualTo(ConversionServiceExposingInterceptor.class); chain = handlerMapping.getHandler(new MockHttpServletRequest("GET", "/scoped")); assertThat(chain).as("HandlerExecutionChain for '/scoped' mapping should not be null.").isNotNull(); @@ -162,10 +164,11 @@ public class WebMvcConfigurationSupportTests { HandlerExecutionChain chain = handlerMapping.getHandler(request); assertThat(chain).isNotNull(); - assertThat(chain.getInterceptors()).isNotNull(); - assertThat(chain.getInterceptors().length).isEqualTo(3); - assertThat(chain.getInterceptors()[1].getClass()).isEqualTo(ConversionServiceExposingInterceptor.class); - assertThat(chain.getInterceptors()[2].getClass()).isEqualTo(ResourceUrlProviderExposingInterceptor.class); + HandlerInterceptor[] interceptors = chain.getInterceptors(); + assertThat(interceptors).isNotNull(); + assertThat(interceptors.length).isEqualTo(3); + assertThat(interceptors[1].getClass()).isEqualTo(ConversionServiceExposingInterceptor.class); + assertThat(interceptors[2].getClass()).isEqualTo(ResourceUrlProviderExposingInterceptor.class); } @Test diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMappingTests.java index ae204b38b3..48315e7df4 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMappingTests.java @@ -35,6 +35,7 @@ import static org.mockito.Mockito.mock; /** * Unit tests for {@link org.springframework.web.servlet.HandlerMapping}. + * * @author Brian Clozel * @author Rossen Stoyanchev */ @@ -65,17 +66,13 @@ class HandlerMappingTests { mapping.setApplicationContext(new StaticWebApplicationContext()); HandlerExecutionChain chain = mapping.getHandler(requestFactory.apply("/")); - assertThat(chain).isNotNull(); - assertThat(chain.getInterceptors()).contains(i1.getInterceptor(), i2, i3.getInterceptor(), i4); + assertThat(chain.getInterceptorList()).contains(i1.getInterceptor(), i2, i3.getInterceptor(), i4); } private static class TestHandlerMapping extends AbstractHandlerMapping { - TestHandlerMapping() { - } - @Override protected Object getHandlerInternal(HttpServletRequest request) { initLookupPath(request); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/PathMatchingUrlHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/PathMatchingUrlHandlerMappingTests.java index 81d9f658ba..5e4e148c85 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/PathMatchingUrlHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/PathMatchingUrlHandlerMappingTests.java @@ -273,14 +273,11 @@ public class PathMatchingUrlHandlerMappingTests { ServletRequestPathUtils.parseAndCache(request); } - HandlerExecutionChain executionChain = mapping.getHandler(request); - HandlerInterceptor[] interceptors = executionChain.getInterceptors(); - if (interceptors != null) { - for (HandlerInterceptor interceptor : interceptors) { - interceptor.preHandle(request, null, executionChain.getHandler()); - } + HandlerExecutionChain chain = mapping.getHandler(request); + for (HandlerInterceptor interceptor : chain.getInterceptorList()) { + interceptor.preHandle(request, null, chain.getHandler()); } - return executionChain; + return chain; } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMappingTests.java index 48522f10f5..408ee13517 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMappingTests.java @@ -158,11 +158,8 @@ public class SimpleUrlHandlerMappingTests { private HandlerExecutionChain getHandler(HandlerMapping mapping, MockHttpServletRequest request) throws Exception { HandlerExecutionChain chain = mapping.getHandler(request); - HandlerInterceptor[] interceptors = chain.getInterceptors(); - if (interceptors != null) { - for (HandlerInterceptor interceptor : interceptors) { - interceptor.preHandle(request, null, chain.getHandler()); - } + for (HandlerInterceptor interceptor : chain.getInterceptorList()) { + interceptor.preHandle(request, null, chain.getHandler()); } return chain; } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java index 6742d39ade..8b8afa81c9 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java @@ -239,8 +239,7 @@ class RequestMappingInfoHandlerMappingTests { HandlerExecutionChain chain = mapping.getHandler(new MockHttpServletRequest("GET", path)); assertThat(chain).isNotNull(); - assertThat(chain.getInterceptors()).isNotNull(); - assertThat(chain.getInterceptors()[0]).isSameAs(interceptor); + assertThat(chain.getInterceptorList().get(0)).isSameAs(interceptor); chain = mapping.getHandler(new MockHttpServletRequest("GET", "/invalid")); assertThat(chain).isNull(); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java index 584e12a4de..25bb6c5061 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java @@ -373,13 +373,10 @@ class CrossOriginTests { return (CorsConfiguration)accessor.getPropertyValue("config"); } else { - HandlerInterceptor[] interceptors = chain.getInterceptors(); - if (interceptors != null) { - for (HandlerInterceptor interceptor : interceptors) { - if (interceptor.getClass().getSimpleName().equals("CorsInterceptor")) { - DirectFieldAccessor accessor = new DirectFieldAccessor(interceptor); - return (CorsConfiguration) accessor.getPropertyValue("config"); - } + for (HandlerInterceptor interceptor : chain.getInterceptorList()) { + if (interceptor.getClass().getSimpleName().equals("CorsInterceptor")) { + DirectFieldAccessor accessor = new DirectFieldAccessor(interceptor); + return (CorsConfiguration) accessor.getPropertyValue("config"); } } }