From bb9fcad58ace1882666c1a703d12bab5a9581b43 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 3 Apr 2019 14:23:33 -0400 Subject: [PATCH 1/3] RequestCondition implementations minor refactoring Reduce object creation by pre-computing instances that can be re-used, and eliminating collection copying and sorting where not needed. See gh-22644 --- .../condition/ConsumesRequestCondition.java | 30 ++++++++++---- .../condition/HeadersRequestCondition.java | 9 ++--- .../condition/PatternsRequestCondition.java | 2 +- .../condition/ProducesRequestCondition.java | 39 ++++++++++++------- .../RequestMethodsRequestCondition.java | 28 ++++++++----- .../AbstractNameValueExpression.java | 2 +- .../condition/ConsumesRequestCondition.java | 29 ++++++++++---- .../condition/HeadersRequestCondition.java | 9 ++--- .../mvc/condition/ParamsRequestCondition.java | 18 +++++++-- .../condition/PatternsRequestCondition.java | 32 ++++++++++----- .../condition/ProducesRequestCondition.java | 38 +++++++++++------- .../RequestMethodsRequestCondition.java | 22 ++++++++--- 12 files changed, 175 insertions(+), 83 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ConsumesRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ConsumesRequestCondition.java index cca903941b..671780e621 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ConsumesRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ConsumesRequestCondition.java @@ -25,6 +25,8 @@ import java.util.Set; import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; +import org.springframework.lang.Nullable; +import org.springframework.util.CollectionUtils; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.cors.reactive.CorsUtils; import org.springframework.web.server.ServerWebExchange; @@ -68,15 +70,16 @@ public final class ConsumesRequestCondition extends AbstractRequestCondition(parseExpressions(consumes, headers)); + Collections.sort(this.expressions); } /** - * Private constructor accepting parsed media type expressions. + * Private constructor for internal when creating matching conditions. + * Note the expressions List is neither sorted nor deep copied. */ - private ConsumesRequestCondition(Collection expressions) { - this.expressions = new ArrayList<>(expressions); - Collections.sort(this.expressions); + private ConsumesRequestCondition(List expressions) { + this.expressions = expressions; } @@ -166,9 +169,20 @@ public final class ConsumesRequestCondition extends AbstractRequestCondition result = new LinkedHashSet<>(this.expressions); - result.removeIf(expression -> !expression.match(exchange)); - return (!result.isEmpty() ? new ConsumesRequestCondition(result) : null); + List result = getMatchingExpressions(exchange); + return !CollectionUtils.isEmpty(result) ? new ConsumesRequestCondition(result) : null; + } + + @Nullable + private List getMatchingExpressions(ServerWebExchange exchange) { + List result = null; + for (ConsumeMediaTypeExpression expression : this.expressions) { + if (expression.match(exchange)) { + result = result != null ? result : new ArrayList<>(); + result.add(expression); + } + } + return result; } /** diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/HeadersRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/HeadersRequestCondition.java index 9136c6d9a1..878a70eb90 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/HeadersRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/HeadersRequestCondition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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,7 +17,6 @@ package org.springframework.web.reactive.result.condition; import java.util.Collection; -import java.util.Collections; import java.util.LinkedHashSet; import java.util.Set; @@ -56,12 +55,12 @@ public final class HeadersRequestCondition extends AbstractRequestCondition conditions) { - this.expressions = Collections.unmodifiableSet(new LinkedHashSet<>(conditions)); + private HeadersRequestCondition(Set conditions) { + this.expressions = conditions; } - private static Collection parseExpressions(String... headers) { + private static Set parseExpressions(String... headers) { Set expressions = new LinkedHashSet<>(); if (headers != null) { for (String header : headers) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java index 75c09038f6..ff87244c8b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java index a90597c372..8ed2942210 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java @@ -17,7 +17,6 @@ package org.springframework.web.reactive.result.condition; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; @@ -25,11 +24,11 @@ import java.util.Set; import org.springframework.http.MediaType; import org.springframework.lang.Nullable; +import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; import org.springframework.web.accept.ContentNegotiationManager; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.cors.reactive.CorsUtils; -import org.springframework.web.reactive.accept.HeaderContentTypeResolver; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; import org.springframework.web.reactive.accept.RequestedContentTypeResolverBuilder; import org.springframework.web.server.NotAcceptableStatusException; @@ -48,6 +47,9 @@ import org.springframework.web.server.UnsupportedMediaTypeStatusException; */ public final class ProducesRequestCondition extends AbstractRequestCondition { + private static final RequestedContentTypeResolver DEFAULT_CONTENT_TYPE_RESOLVER = + new RequestedContentTypeResolverBuilder().build(); + private static final ProducesRequestCondition EMPTY_CONDITION = new ProducesRequestCondition(); @@ -90,18 +92,16 @@ public final class ProducesRequestCondition extends AbstractRequestCondition(parseExpressions(produces, headers)); Collections.sort(this.expressions); - this.contentTypeResolver = (resolver != null ? resolver : new HeaderContentTypeResolver()); + this.contentTypeResolver = resolver != null ? resolver : DEFAULT_CONTENT_TYPE_RESOLVER; } /** - * Private constructor with already parsed media type expressions. + * Private constructor for internal use to create matching conditions. + * Note the expressions List is neither sorted, nor deep copied. */ - private ProducesRequestCondition(Collection expressions, - RequestedContentTypeResolver resolver) { - - this.expressions = new ArrayList<>(expressions); - Collections.sort(this.expressions); - this.contentTypeResolver = (resolver != null ? resolver : new RequestedContentTypeResolverBuilder().build()); + private ProducesRequestCondition(List expressions, ProducesRequestCondition other) { + this.expressions = expressions; + this.contentTypeResolver = other.contentTypeResolver; } @@ -189,10 +189,9 @@ public final class ProducesRequestCondition extends AbstractRequestCondition result = new LinkedHashSet<>(this.expressions); - result.removeIf(expression -> !expression.match(exchange)); - if (!result.isEmpty()) { - return new ProducesRequestCondition(result, this.contentTypeResolver); + List result = getMatchingExpressions(exchange); + if (!CollectionUtils.isEmpty(result)) { + return new ProducesRequestCondition(result, this); } else { try { @@ -207,6 +206,18 @@ public final class ProducesRequestCondition extends AbstractRequestCondition getMatchingExpressions(ServerWebExchange exchange) { + List result = null; + for (ProduceMediaTypeExpression expression : this.expressions) { + if (expression.match(exchange)) { + result = result != null ? result : new ArrayList<>(); + result.add(expression); + } + } + return result; + } + /** * Compares this and another "produces" condition as follows: *
    diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/RequestMethodsRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/RequestMethodsRequestCondition.java index cda3f8bd39..636347061a 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/RequestMethodsRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/RequestMethodsRequestCondition.java @@ -19,8 +19,10 @@ package org.springframework.web.reactive.result.condition; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; import java.util.Set; import org.springframework.http.HttpMethod; @@ -39,8 +41,15 @@ import org.springframework.web.server.ServerWebExchange; */ public final class RequestMethodsRequestCondition extends AbstractRequestCondition { - private static final RequestMethodsRequestCondition GET_CONDITION = - new RequestMethodsRequestCondition(RequestMethod.GET); + /** Per HTTP method cache to return ready instances from getMatchingCondition. */ + private static final Map requestMethodConditionCache; + + static { + requestMethodConditionCache = new HashMap<>(RequestMethod.values().length); + for (RequestMethod method : RequestMethod.values()) { + requestMethodConditionCache.put(method.name(), new RequestMethodsRequestCondition(method)); + } + } private final Set methods; @@ -110,11 +119,11 @@ public final class RequestMethodsRequestCondition extends AbstractRequestConditi } if (getMethods().isEmpty()) { if (RequestMethod.OPTIONS.name().equals(exchange.getRequest().getMethodValue())) { - return null; // No implicit match for OPTIONS (we handle it) + return null; // We handle OPTIONS transparently, so don't match if no explicit declarations } return this; } - return matchRequestMethod(exchange.getRequest().getMethod()); + return matchRequestMethod(exchange.getRequest().getMethodValue()); } /** @@ -122,24 +131,25 @@ public final class RequestMethodsRequestCondition extends AbstractRequestConditi * Hence empty conditions is a match, otherwise try to match to the HTTP * method in the "Access-Control-Request-Method" header. */ + @Nullable private RequestMethodsRequestCondition matchPreFlight(ServerHttpRequest request) { if (getMethods().isEmpty()) { return this; } HttpMethod expectedMethod = request.getHeaders().getAccessControlRequestMethod(); - return matchRequestMethod(expectedMethod); + return expectedMethod != null ? matchRequestMethod(expectedMethod.name()) : null; } @Nullable - private RequestMethodsRequestCondition matchRequestMethod(@Nullable HttpMethod httpMethod) { + private RequestMethodsRequestCondition matchRequestMethod(@Nullable String httpMethod) { if (httpMethod != null) { for (RequestMethod method : getMethods()) { if (httpMethod.matches(method.name())) { - return new RequestMethodsRequestCondition(method); + return requestMethodConditionCache.get(method.name()); } } - if (httpMethod == HttpMethod.HEAD && getMethods().contains(RequestMethod.GET)) { - return GET_CONDITION; + if (HttpMethod.HEAD.matches(httpMethod) && getMethods().contains(RequestMethod.GET)) { + return requestMethodConditionCache.get(HttpMethod.GET.name()); } } return null; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractNameValueExpression.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractNameValueExpression.java index 38e6f42b7d..5acc07c3dc 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractNameValueExpression.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractNameValueExpression.java @@ -80,7 +80,7 @@ abstract class AbstractNameValueExpression implements NameValueExpression else { isMatch = matchName(request); } - return (this.isNegated ? !isMatch : isMatch); + return this.isNegated != isMatch; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java index 76b5ecc3fc..a956612a19 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java @@ -27,6 +27,7 @@ import javax.servlet.http.HttpServletRequest; import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; import org.springframework.lang.Nullable; +import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.cors.CorsUtils; @@ -70,15 +71,16 @@ public final class ConsumesRequestCondition extends AbstractRequestCondition(parseExpressions(consumes, headers)); + Collections.sort(this.expressions); } /** - * Private constructor accepting parsed media type expressions. + * Private constructor for internal when creating matching conditions. + * Note the expressions List is neither sorted nor deep copied. */ - private ConsumesRequestCondition(Collection expressions) { - this.expressions = new ArrayList<>(expressions); - Collections.sort(this.expressions); + private ConsumesRequestCondition(List expressions) { + this.expressions = expressions; } @@ -179,9 +181,20 @@ public final class ConsumesRequestCondition extends AbstractRequestCondition result = new LinkedHashSet<>(this.expressions); - result.removeIf(expression -> !expression.match(contentType)); - return (!result.isEmpty() ? new ConsumesRequestCondition(result) : null); + List result = getMatchingExpressions(contentType); + return !CollectionUtils.isEmpty(result) ? new ConsumesRequestCondition(result) : null; + } + + @Nullable + private List getMatchingExpressions(MediaType contentType) { + List result = null; + for (ConsumeMediaTypeExpression expression : this.expressions) { + if (expression.match(contentType)) { + result = result != null ? result : new ArrayList<>(); + result.add(expression); + } + } + return result; } /** diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/HeadersRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/HeadersRequestCondition.java index 061305dd42..482a5ca818 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/HeadersRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/HeadersRequestCondition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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,7 +17,6 @@ package org.springframework.web.servlet.mvc.condition; import java.util.Collection; -import java.util.Collections; import java.util.LinkedHashSet; import java.util.Set; import javax.servlet.http.HttpServletRequest; @@ -58,12 +57,12 @@ public final class HeadersRequestCondition extends AbstractRequestCondition conditions) { - this.expressions = Collections.unmodifiableSet(new LinkedHashSet<>(conditions)); + private HeadersRequestCondition(Set conditions) { + this.expressions = conditions; } - private static Collection parseExpressions(String... headers) { + private static Set parseExpressions(String... headers) { Set expressions = new LinkedHashSet<>(); for (String header : headers) { HeaderExpression expr = new HeaderExpression(header); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java index d93d6b9922..40987fa4af 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -18,6 +18,7 @@ package org.springframework.web.servlet.mvc.condition; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Set; import javax.servlet.http.HttpServletRequest; @@ -142,8 +143,15 @@ public final class ParamsRequestCondition extends AbstractRequestCondition { + private final Set namesToMatch = new HashSet<>(WebUtils.SUBMIT_IMAGE_SUFFIXES.length + 1); + + ParamExpression(String expression) { super(expression); + this.namesToMatch.add(getName()); + for (String suffix : WebUtils.SUBMIT_IMAGE_SUFFIXES) { + this.namesToMatch.add(getName() + suffix); + } } @Override @@ -158,8 +166,12 @@ public final class ParamsRequestCondition extends AbstractRequestCondition fileExtensions) { this.patterns = Collections.unmodifiableSet(prependLeadingSlash(patterns)); - this.pathHelper = (urlPathHelper != null ? urlPathHelper : new UrlPathHelper()); - this.pathMatcher = (pathMatcher != null ? pathMatcher : new AntPathMatcher()); + this.pathHelper = urlPathHelper != null ? urlPathHelper : new UrlPathHelper(); + this.pathMatcher = pathMatcher != null ? pathMatcher : new AntPathMatcher(); this.useSuffixPatternMatch = useSuffixPatternMatch; this.useTrailingSlashMatch = useTrailingSlashMatch; @@ -120,6 +120,18 @@ public final class PatternsRequestCondition extends AbstractRequestCondition patterns, PatternsRequestCondition other) { + this.patterns = patterns; + this.pathHelper = other.pathHelper; + this.pathMatcher = other.pathMatcher; + this.useSuffixPatternMatch = other.useSuffixPatternMatch; + this.useTrailingSlashMatch = other.useTrailingSlashMatch; + this.fileExtensions.addAll(other.fileExtensions); + } + private static Set prependLeadingSlash(Collection patterns) { Set result = new LinkedHashSet<>(patterns.size()); @@ -175,8 +187,7 @@ public final class PatternsRequestCondition extends AbstractRequestCondition matches = getMatchingPatterns(lookupPath); - return (!matches.isEmpty() ? - new PatternsRequestCondition(matches, this.pathHelper, this.pathMatcher, - this.useSuffixPatternMatch, this.useTrailingSlashMatch, this.fileExtensions) : null); + return !matches.isEmpty() ? new PatternsRequestCondition(new LinkedHashSet<>(matches), this) : null; } /** * Find the patterns matching the given lookup path. Invoking this method should - * yield results equivalent to those of calling - * {@link #getMatchingCondition(javax.servlet.http.HttpServletRequest)}. + * yield results equivalent to those of calling {@link #getMatchingCondition}. * This method is provided as an alternative to be used if no request is available * (e.g. introspection, tooling, etc). * @param lookupPath the lookup path to match to existing patterns * @return a collection of matching patterns sorted with the closest match at the top */ public List getMatchingPatterns(String lookupPath) { - List matches = new ArrayList<>(); + List matches = null; for (String pattern : this.patterns) { String match = getMatchingPattern(pattern, lookupPath); if (match != null) { + matches = matches != null ? matches : new ArrayList<>(); matches.add(match); } } + if (matches == null) { + return Collections.emptyList(); + } if (matches.size() > 1) { matches.sort(this.pathMatcher.getPatternComparator(lookupPath)); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java index 3f41cd2430..986097a0a7 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java @@ -17,7 +17,6 @@ package org.springframework.web.servlet.mvc.condition; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; @@ -26,6 +25,7 @@ import javax.servlet.http.HttpServletRequest; import org.springframework.http.MediaType; import org.springframework.lang.Nullable; +import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; import org.springframework.web.HttpMediaTypeException; import org.springframework.web.HttpMediaTypeNotAcceptableException; @@ -48,6 +48,9 @@ import org.springframework.web.servlet.mvc.condition.HeadersRequestCondition.Hea */ public final class ProducesRequestCondition extends AbstractRequestCondition { + private static final ContentNegotiationManager DEFAULT_CONTENT_NEGOTIATION_MANAGER = + new ContentNegotiationManager(); + private static final ProducesRequestCondition EMPTY_CONDITION = new ProducesRequestCondition(); private static final List MEDIA_TYPE_ALL_LIST = @@ -92,18 +95,16 @@ public final class ProducesRequestCondition extends AbstractRequestCondition(parseExpressions(produces, headers)); Collections.sort(this.expressions); - this.contentNegotiationManager = (manager != null ? manager : new ContentNegotiationManager()); + this.contentNegotiationManager = manager != null ? manager : DEFAULT_CONTENT_NEGOTIATION_MANAGER; } /** - * Private constructor with already parsed media type expressions. + * Private constructor for internal use to create matching conditions. + * Note the expressions List is neither sorted nor deep copied. */ - private ProducesRequestCondition(Collection expressions, - @Nullable ContentNegotiationManager manager) { - - this.expressions = new ArrayList<>(expressions); - Collections.sort(this.expressions); - this.contentNegotiationManager = (manager != null ? manager : new ContentNegotiationManager()); + private ProducesRequestCondition(List expressions, ProducesRequestCondition other) { + this.expressions = expressions; + this.contentNegotiationManager = other.contentNegotiationManager; } @@ -198,10 +199,9 @@ public final class ProducesRequestCondition extends AbstractRequestCondition result = new LinkedHashSet<>(this.expressions); - result.removeIf(expression -> !expression.match(acceptedMediaTypes)); - if (!result.isEmpty()) { - return new ProducesRequestCondition(result, this.contentNegotiationManager); + List result = getMatchingExpressions(acceptedMediaTypes); + if (!CollectionUtils.isEmpty(result)) { + return new ProducesRequestCondition(result, this); } else if (MediaType.ALL.isPresentIn(acceptedMediaTypes)) { return EMPTY_CONDITION; @@ -211,6 +211,18 @@ public final class ProducesRequestCondition extends AbstractRequestCondition getMatchingExpressions(List acceptedMediaTypes) { + List result = null; + for (ProduceMediaTypeExpression expression : this.expressions) { + if (expression.match(acceptedMediaTypes)) { + result = result != null ? result : new ArrayList<>(); + result.add(expression); + } + } + return result; + } + /** * Compares this and another "produces" condition as follows: *
      diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestCondition.java index 49956c5a2a..2ab9721e67 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestCondition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -19,7 +19,9 @@ package org.springframework.web.servlet.mvc.condition; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashSet; +import java.util.Map; import java.util.Set; import javax.servlet.DispatcherType; import javax.servlet.http.HttpServletRequest; @@ -40,8 +42,16 @@ import org.springframework.web.cors.CorsUtils; */ public final class RequestMethodsRequestCondition extends AbstractRequestCondition { - private static final RequestMethodsRequestCondition GET_CONDITION = - new RequestMethodsRequestCondition(RequestMethod.GET); + /** Per HTTP method cache to return ready instances from getMatchingCondition. */ + private static final Map requestMethodConditionCache; + + static { + requestMethodConditionCache = new HashMap<>(RequestMethod.values().length); + for (RequestMethod method : RequestMethod.values()) { + requestMethodConditionCache.put(method.name(), new RequestMethodsRequestCondition(method)); + } + } + private final Set methods; @@ -108,7 +118,7 @@ public final class RequestMethodsRequestCondition extends AbstractRequestConditi if (RequestMethod.OPTIONS.name().equals(request.getMethod()) && !DispatcherType.ERROR.equals(request.getDispatcherType())) { - return null; // No implicit match for OPTIONS (we handle it) + return null; // We handle OPTIONS transparently, so don't match if no explicit declarations } return this; } @@ -136,11 +146,11 @@ public final class RequestMethodsRequestCondition extends AbstractRequestConditi if (httpMethod != null) { for (RequestMethod method : getMethods()) { if (httpMethod.matches(method.name())) { - return new RequestMethodsRequestCondition(method); + return requestMethodConditionCache.get(method.name()); } } if (httpMethod == HttpMethod.HEAD && getMethods().contains(RequestMethod.GET)) { - return GET_CONDITION; + return requestMethodConditionCache.get(HttpMethod.GET.name()); } } return null; From 254f06e1a1615fb7692005c392389c69a14a7e94 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 3 Apr 2019 12:03:33 -0400 Subject: [PATCH 2/3] Request attribute cache for resolved lookupPath See gh-22644 --- ...thExtensionContentNegotiationStrategy.java | 3 ++- .../cors/UrlBasedCorsConfigurationSource.java | 18 +++++++++++++-- .../web/util/UrlPathHelper.java | 22 ++++++++++++++++++- .../web/servlet/HandlerMapping.java | 9 ++++++++ .../handler/AbstractHandlerMapping.java | 3 ++- .../handler/AbstractHandlerMethodMapping.java | 1 + .../handler/AbstractUrlHandlerMapping.java | 3 ++- .../mvc/UrlFilenameViewController.java | 4 ++-- .../servlet/mvc/WebContentInterceptor.java | 5 +++-- .../condition/PatternsRequestCondition.java | 5 +++-- .../RequestMappingHandlerMapping.java | 4 ++-- .../servlet/resource/ResourceUrlProvider.java | 5 +++-- .../DefaultRequestToViewNameTranslator.java | 5 +++-- 13 files changed, 69 insertions(+), 18 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java b/spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java index 46a6711e5d..4b1c998547 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java +++ b/spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -91,6 +91,7 @@ public class PathExtensionContentNegotiationStrategy extends AbstractMappingCont if (request == null) { return null; } + // Ignore LOOKUP_PATH attribute, use our own "fixed" UrlPathHelper with decoding off String path = this.urlPathHelper.getLookupPathForRequest(request); String extension = UriUtils.extractFileExtension(path); return (StringUtils.hasText(extension) ? extension.toLowerCase(Locale.ENGLISH) : null); diff --git a/spring-web/src/main/java/org/springframework/web/cors/UrlBasedCorsConfigurationSource.java b/spring-web/src/main/java/org/springframework/web/cors/UrlBasedCorsConfigurationSource.java index 60755e7e20..b74018961f 100644 --- a/spring-web/src/main/java/org/springframework/web/cors/UrlBasedCorsConfigurationSource.java +++ b/spring-web/src/main/java/org/springframework/web/cors/UrlBasedCorsConfigurationSource.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2019 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. @@ -45,6 +45,9 @@ public class UrlBasedCorsConfigurationSource implements CorsConfigurationSource private UrlPathHelper urlPathHelper = new UrlPathHelper(); + @Nullable + private String lookupPathAttributeName; + /** * Set the PathMatcher implementation to use for matching URL paths @@ -72,6 +75,17 @@ public class UrlBasedCorsConfigurationSource implements CorsConfigurationSource this.urlPathHelper.setUrlDecode(urlDecode); } + /** + * Optionally configure the name of the attribute that caches the lookupPath. + * This is used to make the call to + * {@link UrlPathHelper#getLookupPathForRequest(HttpServletRequest, String)} + * @param lookupPathAttributeName the request attribute to check + * @since 5.2 + */ + public void setLookupPathAttributeName(@Nullable String lookupPathAttributeName) { + this.lookupPathAttributeName = lookupPathAttributeName; + } + /** * Shortcut to same property on underlying {@link #setUrlPathHelper UrlPathHelper}. * @see org.springframework.web.util.UrlPathHelper#setRemoveSemicolonContent(boolean) @@ -117,7 +131,7 @@ public class UrlBasedCorsConfigurationSource implements CorsConfigurationSource @Override @Nullable public CorsConfiguration getCorsConfiguration(HttpServletRequest request) { - String lookupPath = this.urlPathHelper.getLookupPathForRequest(request); + String lookupPath = this.urlPathHelper.getLookupPathForRequest(request, this.lookupPathAttributeName); for (Map.Entry entry : this.corsConfigurations.entrySet()) { if (this.pathMatcher.match(entry.getKey(), lookupPath)) { return entry.getValue(); diff --git a/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java b/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java index 1494c133fe..b371aca795 100644 --- a/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java +++ b/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -175,6 +175,26 @@ public class UrlPathHelper { } } + /** + * Variant of {@link #getLookupPathForRequest(HttpServletRequest)} that + * automates checking for a previously computed lookupPath saved as a + * request attribute. The attribute is only used for lookup purposes. + * @param request current HTTP request + * @param lookupPathAttributeName the request attribute to check + * @return the lookup path + * @since 5.2 + * @see org.springframework.web.servlet.HandlerMapping#LOOKUP_PATH + */ + public String getLookupPathForRequest(HttpServletRequest request, @Nullable String lookupPathAttributeName) { + if (lookupPathAttributeName != null) { + String result = (String) request.getAttribute(lookupPathAttributeName); + if (result != null) { + return result; + } + } + return getLookupPathForRequest(request); + } + /** * Return the path within the servlet mapping for the given request, * i.e. the part of the request's URL beyond the part that called the servlet, diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerMapping.java index c10eb23999..18377f0cd9 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerMapping.java @@ -62,6 +62,15 @@ public interface HandlerMapping { */ String BEST_MATCHING_HANDLER_ATTRIBUTE = HandlerMapping.class.getName() + ".bestMatchingHandler"; + /** + * Name of the {@link HttpServletRequest} attribute that contains the path + * used to look up the matching handler, which depending on the configured + * {@link org.springframework.web.util.UrlPathHelper} could be the full path + * or without the context path, decoded or not, etc. + * @since 5.2 + */ + String LOOKUP_PATH = HandlerMapping.class.getName() + ".lookupPath"; + /** * Name of the {@link HttpServletRequest} attribute that contains the path * within the handler mapping, in case of a pattern match, or the full 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 bea11de046..bdb0600650 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 @@ -212,6 +212,7 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport source.setCorsConfigurations(corsConfigurations); source.setPathMatcher(this.pathMatcher); source.setUrlPathHelper(this.urlPathHelper); + source.setLookupPathAttributeName(LOOKUP_PATH); this.corsConfigurationSource = source; } else { @@ -463,7 +464,7 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport HandlerExecutionChain chain = (handler instanceof HandlerExecutionChain ? (HandlerExecutionChain) handler : new HandlerExecutionChain(handler)); - String lookupPath = this.urlPathHelper.getLookupPathForRequest(request); + String lookupPath = this.urlPathHelper.getLookupPathForRequest(request, LOOKUP_PATH); for (HandlerInterceptor interceptor : this.adaptedInterceptors) { if (interceptor instanceof MappedInterceptor) { MappedInterceptor mappedInterceptor = (MappedInterceptor) interceptor; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java index cf4f430ca6..f34f457c7a 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java @@ -355,6 +355,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap @Override protected HandlerMethod getHandlerInternal(HttpServletRequest request) throws Exception { String lookupPath = getUrlPathHelper().getLookupPathForRequest(request); + request.setAttribute(LOOKUP_PATH, lookupPath); this.mappingRegistry.acquireReadLock(); try { HandlerMethod handlerMethod = lookupHandlerMethod(lookupPath, request); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java index 746d0a3497..7b10d0881b 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java @@ -120,6 +120,7 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping i @Nullable protected Object getHandlerInternal(HttpServletRequest request) throws Exception { String lookupPath = getUrlPathHelper().getLookupPathForRequest(request); + request.setAttribute(LOOKUP_PATH, lookupPath); Object handler = lookupHandler(lookupPath, request); if (handler == null) { // We need to care for the default handler directly, since we need to @@ -291,7 +292,7 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping i @Override @Nullable public RequestMatchResult match(HttpServletRequest request, String pattern) { - String lookupPath = getUrlPathHelper().getLookupPathForRequest(request); + String lookupPath = getUrlPathHelper().getLookupPathForRequest(request, LOOKUP_PATH); if (getPathMatcher().match(pattern, lookupPath)) { return new RequestMatchResult(pattern, lookupPath, getPathMatcher()); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/UrlFilenameViewController.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/UrlFilenameViewController.java index b000dbbe21..9782e5a92e 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/UrlFilenameViewController.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/UrlFilenameViewController.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -110,7 +110,7 @@ public class UrlFilenameViewController extends AbstractUrlViewController { protected String extractOperableUrl(HttpServletRequest request) { String urlPath = (String) request.getAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE); if (!StringUtils.hasText(urlPath)) { - urlPath = getUrlPathHelper().getLookupPathForRequest(request); + urlPath = getUrlPathHelper().getLookupPathForRequest(request, HandlerMapping.LOOKUP_PATH); } return urlPath; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/WebContentInterceptor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/WebContentInterceptor.java index 9350956fb7..90b94aec99 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/WebContentInterceptor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/WebContentInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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,6 +30,7 @@ import org.springframework.util.AntPathMatcher; import org.springframework.util.Assert; import org.springframework.util.PathMatcher; import org.springframework.web.servlet.HandlerInterceptor; +import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.support.WebContentGenerator; import org.springframework.web.util.UrlPathHelper; @@ -169,7 +170,7 @@ public class WebContentInterceptor extends WebContentGenerator implements Handle checkRequest(request); - String lookupPath = this.urlPathHelper.getLookupPathForRequest(request); + String lookupPath = this.urlPathHelper.getLookupPathForRequest(request, HandlerMapping.LOOKUP_PATH); CacheControl cacheControl = lookupCacheControl(lookupPath); Integer cacheSeconds = lookupCacheSeconds(lookupPath); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java index ca3691b808..00d2eff8f4 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java @@ -31,6 +31,7 @@ import org.springframework.lang.Nullable; import org.springframework.util.AntPathMatcher; import org.springframework.util.PathMatcher; import org.springframework.util.StringUtils; +import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.util.UrlPathHelper; /** @@ -212,7 +213,7 @@ public final class PatternsRequestCondition extends AbstractRequestCondition matches = getMatchingPatterns(lookupPath); return !matches.isEmpty() ? new PatternsRequestCondition(new LinkedHashSet<>(matches), this) : null; } @@ -287,7 +288,7 @@ public final class PatternsRequestCondition extends AbstractRequestCondition patternComparator = this.pathMatcher.getPatternComparator(lookupPath); Iterator iterator = this.patterns.iterator(); Iterator iteratorOther = other.patterns.iterator(); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java index 1e9a421a9d..3e1a772a8e 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -340,7 +340,7 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi return null; } Set patterns = matchingInfo.getPatternsCondition().getPatterns(); - String lookupPath = getUrlPathHelper().getLookupPathForRequest(request); + String lookupPath = getUrlPathHelper().getLookupPathForRequest(request, LOOKUP_PATH); return new RequestMatchResult(patterns.iterator().next(), lookupPath, getPathMatcher()); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java index f3110cba7d..6806938854 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -34,6 +34,7 @@ import org.springframework.lang.Nullable; import org.springframework.util.AntPathMatcher; import org.springframework.util.PathMatcher; import org.springframework.util.StringUtils; +import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.handler.SimpleUrlHandlerMapping; import org.springframework.web.util.UrlPathHelper; @@ -180,7 +181,7 @@ public class ResourceUrlProvider implements ApplicationListener Date: Wed, 3 Apr 2019 14:55:51 -0400 Subject: [PATCH 3/3] ProducesRequestCondition caches accepted media types Closes gh-22644 --- .../org/springframework/util/MimeTypeUtils.java | 2 +- .../result/condition/ProducesRequestCondition.java | 9 ++++++++- .../mvc/condition/ConsumesRequestCondition.java | 2 ++ .../mvc/condition/ProducesRequestCondition.java | 14 ++++++++++++-- 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java b/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java index c169aa22b3..428f08c33a 100644 --- a/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java +++ b/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java @@ -161,7 +161,7 @@ public abstract class MimeTypeUtils { private static final ConcurrentLruCache cachedMimeTypes = - new ConcurrentLruCache<>(32, MimeTypeUtils::parseMimeTypeInternal); + new ConcurrentLruCache<>(64, MimeTypeUtils::parseMimeTypeInternal); @Nullable private static volatile Random random; diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java index 8ed2942210..6683372e80 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java @@ -52,6 +52,8 @@ public final class ProducesRequestCondition extends AbstractRequestCondition mediaTypeAllList = Collections.singletonList(new ProduceMediaTypeExpression(MediaType.ALL_VALUE)); @@ -262,7 +264,12 @@ public final class ProducesRequestCondition extends AbstractRequestCondition getAcceptedMediaTypes(ServerWebExchange exchange) throws NotAcceptableStatusException { - return this.contentTypeResolver.resolveMediaTypes(exchange); + List result = exchange.getAttribute(MEDIA_TYPES_ATTRIBUTE); + if (result == null) { + result = this.contentTypeResolver.resolveMediaTypes(exchange); + exchange.getAttributes().put(MEDIA_TYPES_ATTRIBUTE, result); + } + return result; } private int indexOfEqualMediaType(MediaType mediaType) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java index a956612a19..396e6bf052 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java @@ -171,6 +171,8 @@ public final class ConsumesRequestCondition extends AbstractRequestCondition MEDIA_TYPE_ALL_LIST = Collections.singletonList(new ProduceMediaTypeExpression(MediaType.ALL_VALUE)); + private static final String MEDIA_TYPES_ATTRIBUTE = ProducesRequestCondition.class.getName() + ".MEDIA_TYPES"; + private final List expressions; @@ -266,8 +268,16 @@ public final class ProducesRequestCondition extends AbstractRequestCondition getAcceptedMediaTypes(HttpServletRequest request) throws HttpMediaTypeNotAcceptableException { - return this.contentNegotiationManager.resolveMediaTypes(new ServletWebRequest(request)); + @SuppressWarnings("unchecked") + private List getAcceptedMediaTypes(HttpServletRequest request) + throws HttpMediaTypeNotAcceptableException { + + List result = (List) request.getAttribute(MEDIA_TYPES_ATTRIBUTE); + if (result == null) { + result = this.contentNegotiationManager.resolveMediaTypes(new ServletWebRequest(request)); + request.setAttribute(MEDIA_TYPES_ATTRIBUTE, result); + } + return result; } private int indexOfEqualMediaType(MediaType mediaType) {