added back mvc:interceptors element and added interceptor matching based on path pattern; needs review

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/config/AnnotationDrivenBeanDefinitionParser.java

@@ -64,7 +64,6 @@ public class AnnotationDrivenBeanDefinitionParser implements BeanDefinitionParse
 		RootBeanDefinition annMappingDef = new RootBeanDefinition(DefaultAnnotationHandlerMapping.class);
 		annMappingDef.setSource(source);
 		annMappingDef.getPropertyValues().add("order", 0);
-		annMappingDef.getPropertyValues().add("detectInterceptors", true);
 		String annMappingName = parserContext.getReaderContext().registerWithGeneratedName(annMappingDef);
 
 		RootBeanDefinition bindingDef = new RootBeanDefinition(ConfigurableWebBindingInitializer.class);

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/config/InterceptorsBeanDefinitionParser.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2002-2009 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.web.servlet.config;
+
+import java.util.List;
+
+import org.springframework.beans.factory.config.BeanDefinition;
+import org.springframework.beans.factory.config.BeanDefinitionHolder;
+import org.springframework.beans.factory.support.RootBeanDefinition;
+import org.springframework.beans.factory.xml.BeanDefinitionParser;
+import org.springframework.beans.factory.xml.ParserContext;
+import org.springframework.util.xml.DomUtils;
+import org.springframework.web.servlet.handler.MappedInterceptor;
+import org.w3c.dom.Element;
+
+/**
+ * {@link org.springframework.beans.factory.xml.BeanDefinitionParser} that parses a {@code interceptors} element to register
+ * a set of {@link MappedInterceptor} definitions.
+ * 
+ * @author Keith Donald
+ * @since 3.0
+ */
+class InterceptorsBeanDefinitionParser implements BeanDefinitionParser {
+
+	public BeanDefinition parse(Element element, ParserContext parserContext) {
+		List<Element> interceptors = DomUtils.getChildElementsByTagName(element, "interceptor");
+		for (Element interceptor : interceptors) {
+			RootBeanDefinition mappedInterceptorDef = new RootBeanDefinition(MappedInterceptor.class);
+			mappedInterceptorDef.getConstructorArgumentValues().addIndexedArgumentValue(0, interceptor.getAttribute("path"));
+			RootBeanDefinition interceptorDef = new RootBeanDefinition(interceptor.getAttribute("class"));
+			BeanDefinitionHolder holder = new BeanDefinitionHolder(interceptorDef, parserContext.getReaderContext().generateBeanName(interceptorDef));
+			holder = parserContext.getDelegate().decorateBeanDefinitionIfRequired(interceptor, holder);
+			parserContext.getDelegate().parseConstructorArgElements(interceptor, interceptorDef);
+			parserContext.getDelegate().parsePropertyElements(interceptor, interceptorDef);
+			mappedInterceptorDef.getConstructorArgumentValues().addIndexedArgumentValue(1, holder);
+			parserContext.getReaderContext().registerWithGeneratedName(mappedInterceptorDef);
+		}
+		return null;
+	}
+	
+}

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/config/MvcNamespaceHandler.java

@@ -29,6 +29,7 @@ public class MvcNamespaceHandler extends NamespaceHandlerSupport {
 
 	public void init() {
 		registerBeanDefinitionParser("annotation-driven", new AnnotationDrivenBeanDefinitionParser());
+		registerBeanDefinitionParser("interceptors", new InterceptorsBeanDefinitionParser());		
 		registerBeanDefinitionParser("view-controller", new ViewControllerBeanDefinitionParser());
 	}
 

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/config/ViewControllerBeanDefinitionParser.java

@@ -53,7 +53,6 @@ class ViewControllerBeanDefinitionParser implements BeanDefinitionParser {
 			handlerMappingDef = new RootBeanDefinition(SimpleUrlHandlerMapping.class);
 			handlerMappingDef.setSource(source);
 			handlerMappingDef.getPropertyValues().add("order", "1");
-			handlerMappingDef.getPropertyValues().add("detectInterceptors", true);			
 			this.handlerMappingBeanName = parserContext.getReaderContext().registerWithGeneratedName(handlerMappingDef);			
 		} else {
 			handlerMappingDef = (RootBeanDefinition) parserContext.getReaderContext().getRegistry().getBeanDefinition(this.handlerMappingBeanName);

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java

@@ -60,8 +60,6 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport
 
 	private HandlerInterceptor[] adaptedInterceptors;
 
-	private boolean detectInterceptors;
-
 	/**
 	 * Specify the order value for this HandlerMapping bean.
 	 * <p>Default value is <code>Integer.MAX_VALUE</code>, meaning that it's non-ordered.
@@ -104,16 +102,6 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport
 		this.interceptors.addAll(Arrays.asList(interceptors));
 	}
 
-	/**
-	 * Configure whether this handler mapping should detect interceptors registered in the WebApplicationContext.
-	 * If true, {@link HandlerInterceptor} and {@link WebRequestInterceptor} beans will be detected by type and added to the interceptors list.
-	 * Default is false.
-	 * @param detectInterceptors the detect interceptors flag
-	 */
-	public void setDetectInterceptors(boolean detectInterceptors) {
-		this.detectInterceptors = detectInterceptors;
-	}
-
 	/**
 	 * Initializes the interceptors.
 	 * @see #extendInterceptors(java.util.List)
@@ -144,21 +132,7 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport
 	 * @see #adaptInterceptor
 	 */
 	protected void initInterceptors() {
-		if (this.detectInterceptors) {
-			Map<String, HandlerInterceptor> handlerInterceptors = BeanFactoryUtils.beansOfTypeIncludingAncestors(getApplicationContext(), HandlerInterceptor.class, true, false);
-			if (!handlerInterceptors.isEmpty()) {
-				this.interceptors.addAll(handlerInterceptors.values());
-			}
-			Map<String, WebRequestInterceptor> webInterceptors = BeanFactoryUtils.beansOfTypeIncludingAncestors(getApplicationContext(), WebRequestInterceptor.class, true, false);
-			if (!webInterceptors.isEmpty()) {
-				for (WebRequestInterceptor interceptor : webInterceptors.values()) {
-					this.interceptors.add(new WebRequestHandlerInterceptorAdapter(interceptor));			
-				}
-			}			
-		}
-		
 		if (!this.interceptors.isEmpty()) {
-			OrderComparator.sort(this.interceptors);
 			this.adaptedInterceptors = new HandlerInterceptor[this.interceptors.size()];
 			for (int i = 0; i < this.interceptors.size(); i++) {
 				Object interceptor = this.interceptors.get(i);

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java

@@ -16,20 +16,24 @@
 
 package org.springframework.web.servlet.handler;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.LinkedHashMap;
-import java.util.Map;
 import java.util.List;
-import java.util.ArrayList;
+import java.util.Map;
+import java.util.Set;
+
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.BeanFactoryUtils;
 import org.springframework.util.AntPathMatcher;
 import org.springframework.util.Assert;
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.PathMatcher;
 import org.springframework.web.servlet.HandlerExecutionChain;
+import org.springframework.web.servlet.HandlerInterceptor;
 import org.springframework.web.servlet.HandlerMapping;
 import org.springframework.web.util.UrlPathHelper;
 
@@ -67,7 +71,8 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping {
 
 	private final Map<String, Object> handlerMap = new LinkedHashMap<String, Object>();
 
-
+	private MappedInterceptors mappedInterceptors;
+	
 	/**
 	 * Set if URL lookup should always use the full path within the current servlet
 	 * context. Else, the path within the current servlet mapping is used if applicable
@@ -151,6 +156,21 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping {
 		this.lazyInitHandlers = lazyInitHandlers;
 	}
 
+	public void setMappedInterceptors(MappedInterceptor[] mappedInterceptors) {
+		this.mappedInterceptors = new MappedInterceptors(mappedInterceptors);
+	}
+
+	
+	@Override
+	protected void initInterceptors() {
+		super.initInterceptors();
+		
+		Map<String, MappedInterceptor> mappedInterceptors = BeanFactoryUtils.beansOfTypeIncludingAncestors(getApplicationContext(), MappedInterceptor.class, true, false);
+		if (!mappedInterceptors.isEmpty()) {
+			this.mappedInterceptors = new MappedInterceptors(mappedInterceptors.values().toArray(new MappedInterceptor[mappedInterceptors.size()]));
+		}
+
+	}
 
 	/**
 	 * Look up a handler for the URL path of the given request.
@@ -181,6 +201,18 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping {
 				handler = buildPathExposingHandler(rawHandler, lookupPath, lookupPath, null);
 			}
 		}
+		if (handler != null & this.mappedInterceptors != null) {
+			Set<HandlerInterceptor> mappedInterceptors = this.mappedInterceptors.getInterceptors(lookupPath, this.pathMatcher);
+			if (!mappedInterceptors.isEmpty()) {
+				HandlerExecutionChain chain;
+				if (handler instanceof HandlerExecutionChain) {
+					chain = (HandlerExecutionChain) handler;
+				} else {
+					chain = new HandlerExecutionChain(handler);
+				}
+				chain.addInterceptors(mappedInterceptors.toArray(new HandlerInterceptor[mappedInterceptors.size()]));
+			}
+		}
 		if (handler != null && logger.isDebugEnabled()) {
 			logger.debug("Mapping [" + lookupPath + "] to handler '" + handler + "'");
 		}

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/handler/MappedInterceptor.java

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2002-2009 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.web.servlet.handler;
+
+import org.springframework.web.context.request.WebRequestInterceptor;
+import org.springframework.web.servlet.HandlerInterceptor;
+
+/**
+ * Holds information about a HandlerInterceptor mapped to a path into the application.
+ * @author Keith Donald
+ * @since 3.0
+ */
+public final class MappedInterceptor {
+	
+	private final String pathPattern;
+	
+	private final HandlerInterceptor interceptor;
+
+	/**
+	 * Creates a new mapped interceptor.
+	 * @param pathPattern the path pattern
+	 * @param interceptor the interceptor
+	 */
+	public MappedInterceptor(String pathPattern, HandlerInterceptor interceptor) {
+		this.pathPattern = pathPattern;
+		this.interceptor = interceptor;
+	}
+
+	/**
+	 * Creates a new mapped interceptor.
+	 * @param pathPattern the path pattern
+	 * @param interceptor the interceptor
+	 */
+	public MappedInterceptor(String pathPattern, WebRequestInterceptor interceptor) {
+		this.pathPattern = pathPattern;
+		this.interceptor = new WebRequestHandlerInterceptorAdapter(interceptor);
+	}
+
+	/**
+	 * The path into the application the interceptor is mapped to.
+	 */
+	public String getPathPattern() {
+		return pathPattern;
+	}
+
+	/**
+	 * The actual Interceptor reference.
+	 */
+	public HandlerInterceptor getInterceptor() {
+		return interceptor;
+	}
+		
+}

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/handler/MappedInterceptors.java

@@ -0,0 +1,37 @@
+package org.springframework.web.servlet.handler;
+
+import java.util.LinkedHashSet;
+import java.util.Set;
+
+import org.springframework.util.PathMatcher;
+import org.springframework.util.StringUtils;
+import org.springframework.web.servlet.HandlerInterceptor;
+
+class MappedInterceptors {
+	
+	private MappedInterceptor[] mappedInterceptors;
+
+	public MappedInterceptors(MappedInterceptor[] mappedInterceptors) {
+		this.mappedInterceptors = mappedInterceptors;
+	}
+	
+	public Set<HandlerInterceptor> getInterceptors(String lookupPath, PathMatcher pathMatcher) {
+		Set<HandlerInterceptor> interceptors = new LinkedHashSet<HandlerInterceptor>();
+		for (MappedInterceptor interceptor : this.mappedInterceptors) {
+			if (matches(interceptor, lookupPath, pathMatcher)) {
+				interceptors.add(interceptor.getInterceptor());				
+			}
+		}
+		return interceptors;
+	}
+	
+	private boolean matches(MappedInterceptor interceptor, String lookupPath, PathMatcher pathMatcher) {
+		String pathPattern = interceptor.getPathPattern();
+		if (StringUtils.hasText(pathPattern)) {
+			return pathMatcher.match(pathPattern, lookupPath);
+		} else {
+			return true;
+		}
+	}
+	
+}

org.springframework.web.servlet/src/main/resources/org/springframework/web/servlet/config/spring-mvc-3.0.xsd

@@ -59,7 +59,42 @@
 		</xsd:annotation>			
 		<xsd:complexType>
 			<xsd:sequence>
-				<xsd:element ref="beans:bean" maxOccurs="unbounded" />
+				<xsd:element name="interceptor" maxOccurs="unbounded">
+					<xsd:annotation>
+						<xsd:documentation source="java:org.springframework.web.servlet.handler.MappedInterceptor"><![CDATA[
+	Registers a interceptor definition.
+						]]></xsd:documentation>
+					</xsd:annotation>
+					<xsd:complexType>
+						<xsd:choice minOccurs="0" maxOccurs="unbounded">
+							<xsd:element ref="beans:constructor-arg"/>
+							<xsd:element ref="beans:property"/>
+						</xsd:choice>
+						<xsd:attribute name="path" type="xsd:string">
+							<xsd:annotation>
+								<xsd:documentation><![CDATA[
+	The path into the application intercepted by this interceptor.
+	Exact path mapping URIs (such as "/myPath") are supported as well as Ant-stype path patterns (such as /myPath/**).
+	If not specified, the interceptor intercepts all paths ("/**").
+								]]></xsd:documentation>
+							</xsd:annotation>
+						</xsd:attribute>
+						<xsd:attribute name="class" type="xsd:string" use="required">
+							<xsd:annotation>
+								<xsd:documentation source="java:java.lang.Class"><![CDATA[
+	The interceptor class.
+	Must implement org.springframework.web.servlet.HandlerInterceptor or org.springframework.web.context.request.WebRequestInterceptor.
+								]]></xsd:documentation>
+							<xsd:appinfo>
+								<tool:annotation kind="direct">
+									<tool:expected-type type="java.lang.Class"/>
+								</tool:annotation>
+							</xsd:appinfo>								
+							</xsd:annotation>
+						</xsd:attribute>
+						<xsd:anyAttribute namespace="##other" processContents="lax"/>		
+					</xsd:complexType>			
+				</xsd:element>
 			</xsd:sequence>
 		</xsd:complexType>
 	</xsd:element>

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java

@@ -35,6 +35,7 @@ import org.springframework.context.i18n.LocaleContextHolder;
 import org.springframework.core.convert.ConversionFailedException;
 import org.springframework.core.convert.ConversionService;
 import org.springframework.core.io.ClassPathResource;
+import org.springframework.core.style.StylerUtils;
 import org.springframework.format.annotation.DateTimeFormat;
 import org.springframework.format.annotation.DateTimeFormat.ISO;
 import org.springframework.format.support.FormattingConversionServiceFactoryBean;
@@ -155,21 +156,27 @@ public class MvcNamespaceTests {
 		mapping.setDefaultHandler(new TestController());
 		
 		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.setRequestURI("/accounts/12345");
 		request.addParameter("locale", "en");
 		request.addParameter("theme", "green");
 
 		HandlerExecutionChain chain = mapping.getHandler(request);
-		assertEquals(4, chain.getInterceptors().length);
+		assertEquals(3, chain.getInterceptors().length);
 		assertTrue(chain.getInterceptors()[1] instanceof LocaleChangeInterceptor);
 		assertTrue(chain.getInterceptors()[2] instanceof ThemeChangeInterceptor);
+
+		request.setRequestURI("/logged/accounts/12345");
+		chain = mapping.getHandler(request);
+		assertEquals(4, chain.getInterceptors().length);
 		assertTrue(chain.getInterceptors()[3] instanceof WebRequestHandlerInterceptorAdapter);
+
 	}
 
 	@Test
 	public void testBeanDecoration() throws Exception {
 		XmlBeanDefinitionReader reader = new XmlBeanDefinitionReader(container);
 		reader.loadBeanDefinitions(new ClassPathResource("mvc-config-bean-decoration.xml", getClass()));
-		assertEquals(5, container.getBeanDefinitionCount());
+		assertEquals(6, container.getBeanDefinitionCount());
 		container.refresh();
 
 		DefaultAnnotationHandlerMapping mapping = container.getBean(DefaultAnnotationHandlerMapping.class);
@@ -179,10 +186,12 @@ public class MvcNamespaceTests {
 		MockHttpServletRequest request = new MockHttpServletRequest();
 
 		HandlerExecutionChain chain = mapping.getHandler(request);
-		assertEquals(2, chain.getInterceptors().length);
+		assertEquals(3, chain.getInterceptors().length);
 		assertTrue(chain.getInterceptors()[1] instanceof LocaleChangeInterceptor);
 		LocaleChangeInterceptor interceptor = (LocaleChangeInterceptor) chain.getInterceptors()[1];
 		assertEquals("lang", interceptor.getParamName());
+		ThemeChangeInterceptor interceptor2 = (ThemeChangeInterceptor) chain.getInterceptors()[2];
+		assertEquals("style", interceptor2.getParamName());
 	}
 
 	@Test

org.springframework.web.servlet/src/test/resources/org/springframework/web/servlet/config/mvc-config-bean-decoration.xml

@@ -8,6 +8,11 @@
 
 	<mvc:annotation-driven />
 
-	<bean class="org.springframework.web.servlet.i18n.LocaleChangeInterceptor" p:paramName="lang" />
+	<mvc:interceptors>
+		<mvc:interceptor class="org.springframework.web.servlet.i18n.LocaleChangeInterceptor" p:paramName="lang" />
+		<mvc:interceptor class="org.springframework.web.servlet.theme.ThemeChangeInterceptor">
+			<property name="paramName" value="style" />
+		</mvc:interceptor>	
+	</mvc:interceptors>
 
 </beans>

org.springframework.web.servlet/src/test/resources/org/springframework/web/servlet/config/mvc-config-interceptors.xml

@@ -7,8 +7,10 @@
 
 	<mvc:annotation-driven />
 
-	<bean class="org.springframework.web.servlet.i18n.LocaleChangeInterceptor" />
+	<mvc:interceptors>
-	<bean class="org.springframework.web.servlet.theme.ThemeChangeInterceptor" />
+		<mvc:interceptor class="org.springframework.web.servlet.i18n.LocaleChangeInterceptor" />
-	<bean class="org.springframework.web.context.request.Log4jNestedDiagnosticContextInterceptor" />
+		<mvc:interceptor path="/**" class="org.springframework.web.servlet.theme.ThemeChangeInterceptor" />
-
+		<mvc:interceptor path="/logged/**" class="org.springframework.web.context.request.Log4jNestedDiagnosticContextInterceptor" />		
+	</mvc:interceptors>
+	
 </beans>

org.springframework.web.servlet/src/test/resources/org/springframework/web/servlet/config/mvc-config-view-controllers.xml

@@ -10,8 +10,10 @@
 	<mvc:view-controller path="/foo" />
 	
 	<mvc:view-controller path="/bar" view-name="baz" />
-	
-	<bean class="org.springframework.web.servlet.i18n.LocaleChangeInterceptor" />
-	<bean class="org.springframework.web.servlet.theme.ThemeChangeInterceptor" />
 
+	<mvc:interceptors>
+		<mvc:interceptor class="org.springframework.web.servlet.i18n.LocaleChangeInterceptor" />
+		<mvc:interceptor class="org.springframework.web.servlet.theme.ThemeChangeInterceptor" />
+	</mvc:interceptors>
+	
 </beans>