From 202fa5cdb3a3d0cfe6967e85fa167d978244f28a Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Wed, 15 Mar 2023 18:49:29 +0000 Subject: [PATCH] Polishing and minor refactoring in HandlerMappingIntrospector Closes gh-30127 --- .../handler/HandlerMappingIntrospector.java | 153 +++++++++--------- .../PathPatternMatchableHandlerMapping.java | 11 +- 2 files changed, 81 insertions(+), 83 deletions(-) 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 b828491885a4..623c42d8e34c 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 @@ -82,7 +82,7 @@ public class HandlerMappingIntrospector @Nullable private List handlerMappings; - private Map pathPatternHandlerMappings = Collections.emptyMap(); + private Map pathPatternMappings = Collections.emptyMap(); @Override @@ -95,10 +95,55 @@ public void afterPropertiesSet() { if (this.handlerMappings == null) { Assert.notNull(this.applicationContext, "No ApplicationContext"); this.handlerMappings = initHandlerMappings(this.applicationContext); - this.pathPatternHandlerMappings = initPathPatternMatchableHandlerMappings(this.handlerMappings); + + this.pathPatternMappings = this.handlerMappings.stream() + .filter(m -> m instanceof MatchableHandlerMapping hm && hm.getPatternParser() != null) + .map(mapping -> (MatchableHandlerMapping) mapping) + .collect(Collectors.toMap(mapping -> mapping, PathPatternMatchableHandlerMapping::new)); } } + private static List initHandlerMappings(ApplicationContext context) { + + Map beans = + BeanFactoryUtils.beansOfTypeIncludingAncestors(context, HandlerMapping.class, true, false); + + if (!beans.isEmpty()) { + List mappings = new ArrayList<>(beans.values()); + AnnotationAwareOrderComparator.sort(mappings); + return Collections.unmodifiableList(mappings); + } + + return Collections.unmodifiableList(initFallback(context)); + } + + private static List initFallback(ApplicationContext applicationContext) { + Properties properties; + try { + Resource resource = new ClassPathResource("DispatcherServlet.properties", DispatcherServlet.class); + properties = PropertiesLoaderUtils.loadProperties(resource); + } + catch (IOException ex) { + throw new IllegalStateException("Could not load DispatcherServlet.properties: " + ex.getMessage()); + } + + String value = properties.getProperty(HandlerMapping.class.getName()); + String[] names = StringUtils.commaDelimitedListToStringArray(value); + List result = new ArrayList<>(names.length); + for (String name : names) { + try { + Class clazz = ClassUtils.forName(name, DispatcherServlet.class.getClassLoader()); + Object mapping = applicationContext.getAutowireCapableBeanFactory().createBean(clazz); + result.add((HandlerMapping) mapping); + } + catch (ClassNotFoundException ex) { + throw new IllegalStateException("Could not find default HandlerMapping [" + name + "]"); + } + } + return result; + } + + /** * Return the configured or detected {@code HandlerMapping}s. */ @@ -109,27 +154,27 @@ public List getHandlerMappings() { /** * Find the {@link HandlerMapping} that would handle the given request and - * return it as a {@link MatchableHandlerMapping} that can be used to test - * request-matching criteria. - *

If the matching HandlerMapping is not an instance of - * {@link MatchableHandlerMapping}, an IllegalStateException is raised. + * return a {@link MatchableHandlerMapping} to use for path matching. * @param request the current request - * @return the resolved matcher, or {@code null} + * @return the resolved {@code MatchableHandlerMapping}, or {@code null} + * @throws IllegalStateException if the matching HandlerMapping is not an + * instance of {@link MatchableHandlerMapping} * @throws Exception if any of the HandlerMapping's raise an exception */ @Nullable public MatchableHandlerMapping getMatchableHandlerMapping(HttpServletRequest request) throws Exception { HttpServletRequest wrappedRequest = new AttributesPreservingRequest(request); - return doWithMatchingMapping(wrappedRequest, false, (matchedMapping, executionChain) -> { - if (matchedMapping instanceof MatchableHandlerMapping matchableHandlerMapping) { - PathPatternMatchableHandlerMapping mapping = this.pathPatternHandlerMappings.get(matchedMapping); - if (mapping != null) { + + return doWithHandlerMapping(wrappedRequest, false, (mapping, executionChain) -> { + if (mapping instanceof MatchableHandlerMapping) { + PathPatternMatchableHandlerMapping pathPatternMapping = this.pathPatternMappings.get(mapping); + if (pathPatternMapping != null) { RequestPath requestPath = ServletRequestPathUtils.getParsedRequestPath(wrappedRequest); - return new PathSettingHandlerMapping(mapping, requestPath); + return new LookupPathMatchableHandlerMapping(pathPatternMapping, requestPath); } else { String lookupPath = (String) wrappedRequest.getAttribute(UrlPathHelper.PATH_ATTRIBUTE); - return new PathSettingHandlerMapping(matchableHandlerMapping, lookupPath); + return new LookupPathMatchableHandlerMapping((MatchableHandlerMapping) mapping, lookupPath); } } throw new IllegalStateException("HandlerMapping is not a MatchableHandlerMapping"); @@ -140,7 +185,7 @@ public MatchableHandlerMapping getMatchableHandlerMapping(HttpServletRequest req @Nullable public CorsConfiguration getCorsConfiguration(HttpServletRequest request) { AttributesPreservingRequest wrappedRequest = new AttributesPreservingRequest(request); - return doWithMatchingMappingIgnoringException(wrappedRequest, (handlerMapping, executionChain) -> { + return doWithHandlerMappingIgnoringException(wrappedRequest, (handlerMapping, executionChain) -> { for (HandlerInterceptor interceptor : executionChain.getInterceptorList()) { if (interceptor instanceof CorsConfigurationSource ccs) { return ccs.getCorsConfiguration(wrappedRequest); @@ -154,15 +199,15 @@ public CorsConfiguration getCorsConfiguration(HttpServletRequest request) { } @Nullable - private T doWithMatchingMapping( + private T doWithHandlerMapping( HttpServletRequest request, boolean ignoreException, - BiFunction matchHandler) throws Exception { + BiFunction extractor) throws Exception { - Assert.state(this.handlerMappings != null, "Handler mappings not initialized"); + Assert.state(this.handlerMappings != null, "HandlerMapping's not initialized"); - boolean parseRequestPath = !this.pathPatternHandlerMappings.isEmpty(); + boolean parsePath = !this.pathPatternMappings.isEmpty(); RequestPath previousPath = null; - if (parseRequestPath) { + if (parsePath) { previousPath = (RequestPath) request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE); ServletRequestPathUtils.parseAndCache(request); } @@ -180,11 +225,11 @@ private T doWithMatchingMapping( if (chain == null) { continue; } - return matchHandler.apply(handlerMapping, chain); + return extractor.apply(handlerMapping, chain); } } finally { - if (parseRequestPath) { + if (parsePath) { ServletRequestPathUtils.setParsedRequestPath(previousPath, request); } } @@ -192,11 +237,11 @@ private T doWithMatchingMapping( } @Nullable - private T doWithMatchingMappingIgnoringException( + private T doWithHandlerMappingIgnoringException( HttpServletRequest request, BiFunction matchHandler) { try { - return doWithMatchingMapping(request, true, matchHandler); + return doWithHandlerMapping(request, true, matchHandler); } catch (Exception ex) { throw new IllegalStateException("HandlerMapping exception not suppressed", ex); @@ -204,55 +249,6 @@ private T doWithMatchingMappingIgnoringException( } - private static List initHandlerMappings(ApplicationContext applicationContext) { - Map beans = BeanFactoryUtils.beansOfTypeIncludingAncestors( - applicationContext, HandlerMapping.class, true, false); - if (!beans.isEmpty()) { - List mappings = new ArrayList<>(beans.values()); - AnnotationAwareOrderComparator.sort(mappings); - return Collections.unmodifiableList(mappings); - } - return Collections.unmodifiableList(initFallback(applicationContext)); - } - - private static List initFallback(ApplicationContext applicationContext) { - Properties props; - String path = "DispatcherServlet.properties"; - try { - Resource resource = new ClassPathResource(path, DispatcherServlet.class); - props = PropertiesLoaderUtils.loadProperties(resource); - } - catch (IOException ex) { - throw new IllegalStateException("Could not load '" + path + "': " + ex.getMessage()); - } - - String value = props.getProperty(HandlerMapping.class.getName()); - String[] names = StringUtils.commaDelimitedListToStringArray(value); - List result = new ArrayList<>(names.length); - for (String name : names) { - try { - Class clazz = ClassUtils.forName(name, DispatcherServlet.class.getClassLoader()); - Object mapping = applicationContext.getAutowireCapableBeanFactory().createBean(clazz); - result.add((HandlerMapping) mapping); - } - catch (ClassNotFoundException ex) { - throw new IllegalStateException("Could not find default HandlerMapping [" + name + "]"); - } - } - return result; - } - - private static Map initPathPatternMatchableHandlerMappings( - List mappings) { - - return mappings.stream() - .filter(MatchableHandlerMapping.class::isInstance) - .map(MatchableHandlerMapping.class::cast) - .filter(mapping -> mapping.getPatternParser() != null) - .collect(Collectors.toMap(mapping -> mapping, PathPatternMatchableHandlerMapping::new)); - } - - /** * Request wrapper that buffers request attributes in order protect the * underlying request from attribute changes. @@ -298,26 +294,27 @@ public void removeAttribute(String name) { } - private static class PathSettingHandlerMapping implements MatchableHandlerMapping { + private static class LookupPathMatchableHandlerMapping implements MatchableHandlerMapping { private final MatchableHandlerMapping delegate; - private final Object path; + private final Object lookupPath; private final String pathAttributeName; - PathSettingHandlerMapping(MatchableHandlerMapping delegate, Object path) { + LookupPathMatchableHandlerMapping(MatchableHandlerMapping delegate, Object lookupPath) { this.delegate = delegate; - this.path = path; - this.pathAttributeName = (path instanceof RequestPath ? + this.lookupPath = lookupPath; + this.pathAttributeName = (lookupPath instanceof RequestPath ? ServletRequestPathUtils.PATH_ATTRIBUTE : UrlPathHelper.PATH_ATTRIBUTE); } @Nullable @Override public RequestMatchResult match(HttpServletRequest request, String pattern) { + pattern = (StringUtils.hasLength(pattern) && !pattern.startsWith("/") ? "/" + pattern : pattern); Object previousPath = request.getAttribute(this.pathAttributeName); - request.setAttribute(this.pathAttributeName, this.path); + request.setAttribute(this.pathAttributeName, this.lookupPath); try { return this.delegate.match(request, pattern); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/PathPatternMatchableHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/PathPatternMatchableHandlerMapping.java index 3057331dfb2c..5df30cbc553c 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/PathPatternMatchableHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/PathPatternMatchableHandlerMapping.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -30,8 +30,9 @@ import org.springframework.web.util.pattern.PathPatternParser; /** - * Wraps {@link MatchableHandlerMapping}s configured with a {@link PathPatternParser} - * in order to parse patterns lazily and cache them for re-ues. + * Decorate another {@link MatchableHandlerMapping} that's configured with a + * {@link PathPatternParser} in order to parse and cache String patterns passed + * into the {@code match} method. * * @author Rossen Stoyanchev * @since 5.3 @@ -49,8 +50,8 @@ class PathPatternMatchableHandlerMapping implements MatchableHandlerMapping { public PathPatternMatchableHandlerMapping(MatchableHandlerMapping delegate) { - Assert.notNull(delegate, "Delegate MatchableHandlerMapping is required."); - Assert.notNull(delegate.getPatternParser(), "PatternParser is required."); + Assert.notNull(delegate, "HandlerMapping to delegate to is required."); + Assert.notNull(delegate.getPatternParser(), "Expected HandlerMapping configured to use PatternParser."); this.delegate = delegate; this.parser = delegate.getPatternParser(); }