From ff38b9cd270582f73d2ee4604820084ffbf9122a Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 27 May 2020 18:41:56 +0100 Subject: [PATCH] Optimal initialization of empty request mapping conditions Closes gh-25143 --- .../condition/ConsumesRequestCondition.java | 31 +++--- .../condition/HeadersRequestCondition.java | 34 ++++-- .../condition/ParamsRequestCondition.java | 36 ++++--- .../condition/PatternsRequestCondition.java | 62 +++++------ .../condition/ProducesRequestCondition.java | 35 +++--- .../RequestMethodsRequestCondition.java | 28 +++-- .../result/method/RequestMappingInfo.java | 100 +++++++++++++----- .../PatternsRequestConditionTests.java | 44 ++++---- .../condition/RequestMappingInfoTests.java | 19 +++- .../condition/ConsumesRequestCondition.java | 36 ++++--- .../condition/HeadersRequestCondition.java | 43 +++++--- .../mvc/condition/ParamsRequestCondition.java | 29 +++-- .../condition/PatternsRequestCondition.java | 92 ++++++++-------- .../condition/ProducesRequestCondition.java | 39 ++++--- .../RequestMethodsRequestCondition.java | 22 +++- .../mvc/method/RequestMappingInfo.java | 65 +++++++++--- .../PatternsRequestConditionTests.java | 27 ++--- .../mvc/method/RequestMappingInfoTests.java | 21 +++- 18 files changed, 479 insertions(+), 284 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 d8474303a05f..5fd28ab5a356 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 @@ -29,6 +29,7 @@ import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.cors.reactive.CorsUtils; @@ -75,39 +76,39 @@ public ConsumesRequestCondition(String... consumes) { * @param headers as described in {@link RequestMapping#headers()} */ public ConsumesRequestCondition(String[] consumes, String[] headers) { - this.expressions = new ArrayList<>(parseExpressions(consumes, headers)); + this.expressions = parseExpressions(consumes, headers); if (this.expressions.size() > 1) { Collections.sort(this.expressions); } } - /** - * Private constructor for internal when creating matching conditions. - * Note the expressions List is neither sorted nor deep copied. - */ - private ConsumesRequestCondition(List expressions) { - this.expressions = expressions; - } - - - private static Set parseExpressions(String[] consumes, String[] headers) { - Set result = new LinkedHashSet<>(); - if (headers != null) { + private static List parseExpressions(String[] consumes, String[] headers) { + Set result = null; + if (!ObjectUtils.isEmpty(headers)) { for (String header : headers) { HeadersRequestCondition.HeaderExpression expr = new HeadersRequestCondition.HeaderExpression(header); if ("Content-Type".equalsIgnoreCase(expr.name)) { + result = (result != null ? result : new LinkedHashSet<>()); for (MediaType mediaType : MediaType.parseMediaTypes(expr.value)) { result.add(new ConsumeMediaTypeExpression(mediaType, expr.isNegated)); } } } } - if (consumes != null) { + if (!ObjectUtils.isEmpty(consumes)) { + result = (result != null ? result : new LinkedHashSet<>()); for (String consume : consumes) { result.add(new ConsumeMediaTypeExpression(consume)); } } - return result; + return (result != null ? new ArrayList<>(result) : Collections.emptyList()); + } + + /** + * Private constructor for internal when creating matching conditions. + */ + private ConsumesRequestCondition(List expressions) { + this.expressions = expressions; } 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 878a70eb907c..229b610b8449 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-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,10 +17,12 @@ package org.springframework.web.reactive.result.condition; import java.util.Collection; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.Set; import org.springframework.lang.Nullable; +import org.springframework.util.ObjectUtils; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.cors.reactive.CorsUtils; import org.springframework.web.server.ServerWebExchange; @@ -52,28 +54,29 @@ public final class HeadersRequestCondition extends AbstractRequestCondition conditions) { - this.expressions = conditions; - } - - private static Set parseExpressions(String... headers) { - Set expressions = new LinkedHashSet<>(); - if (headers != null) { + Set result = null; + if (!ObjectUtils.isEmpty(headers)) { for (String header : headers) { HeaderExpression expr = new HeaderExpression(header); if ("Accept".equalsIgnoreCase(expr.name) || "Content-Type".equalsIgnoreCase(expr.name)) { continue; } - expressions.add(expr); + result = (result != null ? result : new LinkedHashSet<>(headers.length)); + result.add(expr); } } - return expressions; + return (result != null ? result : Collections.emptySet()); } + private HeadersRequestCondition(Set conditions) { + this.expressions = conditions; + } + + /** * Return the contained request header expressions. */ @@ -97,6 +100,15 @@ protected String getToStringInfix() { */ @Override public HeadersRequestCondition combine(HeadersRequestCondition other) { + if (isEmpty() && other.isEmpty()) { + return this; + } + else if (other.isEmpty()) { + return this; + } + else if (isEmpty()) { + return other; + } Set set = new LinkedHashSet<>(this.expressions); set.addAll(other.expressions); return new HeadersRequestCondition(set); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ParamsRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ParamsRequestCondition.java index 544726cafc15..9b1bc9f904ea 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ParamsRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ParamsRequestCondition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 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. @@ -21,6 +21,7 @@ import java.util.LinkedHashSet; import java.util.Set; +import org.springframework.util.ObjectUtils; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.server.ServerWebExchange; @@ -42,22 +43,22 @@ public final class ParamsRequestCondition extends AbstractRequestCondition conditions) { - this.expressions = Collections.unmodifiableSet(new LinkedHashSet<>(conditions)); + private static Set parseExpressions(String... params) { + if (ObjectUtils.isEmpty(params)) { + return Collections.emptySet(); + } + Set result = new LinkedHashSet<>(params.length); + for (String param : params) { + result.add(new ParamExpression(param)); + } + return result; } - - private static Collection parseExpressions(String... params) { - Set expressions = new LinkedHashSet<>(); - if (params != null) { - for (String param : params) { - expressions.add(new ParamExpression(param)); - } - } - return expressions; + private ParamsRequestCondition(Set conditions) { + this.expressions = conditions; } @@ -84,6 +85,15 @@ protected String getToStringInfix() { */ @Override public ParamsRequestCondition combine(ParamsRequestCondition other) { + if (isEmpty() && other.isEmpty()) { + return this; + } + else if (other.isEmpty()) { + return this; + } + else if (isEmpty()) { + return other; + } Set set = new LinkedHashSet<>(this.expressions); set.addAll(other.expressions); return new ParamsRequestCondition(set); 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 f485c64560cd..e0035a724829 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-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. @@ -27,6 +27,7 @@ import org.springframework.http.server.PathContainer; import org.springframework.lang.Nullable; +import org.springframework.util.ObjectUtils; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.util.pattern.PathPattern; import org.springframework.web.util.pattern.PathPatternParser; @@ -41,7 +42,7 @@ */ public final class PatternsRequestCondition extends AbstractRequestCondition { - private static final SortedSet EMPTY_PATTERNS = + private static final SortedSet EMPTY_PATH_PATTERN = new TreeSet<>(Collections.singleton(new PathPatternParser().parse(""))); @@ -53,21 +54,21 @@ public final class PatternsRequestCondition extends AbstractRequestCondition patterns) { - this(patterns.isEmpty() ? EMPTY_PATTERNS : new TreeSet<>(patterns)); + this.patterns = (patterns.isEmpty() ? EMPTY_PATH_PATTERN : new TreeSet<>(patterns)); } - private PatternsRequestCondition(SortedSet patterns) { this.patterns = patterns; } + public Set getPatterns() { return this.patterns; } @@ -94,25 +95,28 @@ protected String getToStringInfix() { */ @Override public PatternsRequestCondition combine(PatternsRequestCondition other) { - SortedSet combined; - if (!this.patterns.isEmpty() && !other.patterns.isEmpty()) { - combined = new TreeSet<>(); + if (isEmptyPathPattern() && other.isEmptyPathPattern()) { + return this; + } + else if (other.isEmptyPathPattern()) { + return this; + } + else if (isEmptyPathPattern()) { + return other; + } + else { + SortedSet combined = new TreeSet<>(); for (PathPattern pattern1 : this.patterns) { for (PathPattern pattern2 : other.patterns) { combined.add(pattern1.combine(pattern2)); } } + return new PatternsRequestCondition(combined); } - else if (!this.patterns.isEmpty()) { - combined = this.patterns; - } - else if (!other.patterns.isEmpty()) { - combined = other.patterns; - } - else { - combined = EMPTY_PATTERNS; - } - return new PatternsRequestCondition(combined); + } + + private boolean isEmptyPathPattern() { + return this.patterns == EMPTY_PATH_PATTERN; } /** @@ -126,31 +130,21 @@ else if (!other.patterns.isEmpty()) { @Override @Nullable public PatternsRequestCondition getMatchingCondition(ServerWebExchange exchange) { - if (this.patterns.isEmpty()) { - return this; - } SortedSet matches = getMatchingPatterns(exchange); - return (!matches.isEmpty() ? new PatternsRequestCondition(matches) : null); + return (matches != null ? new PatternsRequestCondition(matches) : null); } - /** - * Find the patterns matching the given lookup path. Invoking this method should - * yield results equivalent to those of calling - * {@link #getMatchingCondition(ServerWebExchange)}. - * This method is provided as an alternative to be used if no request is available - * (e.g. introspection, tooling, etc). - * @param exchange the current exchange - * @return a sorted set of matching patterns sorted with the closest match first - */ + @Nullable private SortedSet getMatchingPatterns(ServerWebExchange exchange) { PathContainer lookupPath = exchange.getRequest().getPath().pathWithinApplication(); - TreeSet pathPatterns = new TreeSet<>(); + TreeSet result = null; for (PathPattern pattern : this.patterns) { if (pattern.matches(lookupPath)) { - pathPatterns.add(pattern); + result = (result != null ? result : new TreeSet<>()); + result.add(pattern); } } - return pathPatterns; + return result; } /** 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 a6cee7b9906f..6fa70307ad73 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 @@ -25,6 +25,7 @@ import org.springframework.http.MediaType; import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; import org.springframework.web.accept.ContentNegotiationManager; import org.springframework.web.bind.annotation.RequestMapping; @@ -92,43 +93,45 @@ public ProducesRequestCondition(String[] produces, String[] headers) { * @param resolver used to determine requested content type */ public ProducesRequestCondition(String[] produces, String[] headers, RequestedContentTypeResolver resolver) { - this.expressions = new ArrayList<>(parseExpressions(produces, headers)); + this.expressions = parseExpressions(produces, headers); if (this.expressions.size() > 1) { Collections.sort(this.expressions); } this.contentTypeResolver = resolver != null ? resolver : DEFAULT_CONTENT_TYPE_RESOLVER; } - /** - * Private constructor for internal use to create matching conditions. - * Note the expressions List is neither sorted, nor deep copied. - */ - private ProducesRequestCondition(List expressions, ProducesRequestCondition other) { - this.expressions = expressions; - this.contentTypeResolver = other.contentTypeResolver; - } - - - private Set parseExpressions(String[] produces, String[] headers) { - Set result = new LinkedHashSet<>(); - if (headers != null) { + private List parseExpressions(String[] produces, String[] headers) { + Set result = null; + if (!ObjectUtils.isEmpty(headers)) { for (String header : headers) { HeadersRequestCondition.HeaderExpression expr = new HeadersRequestCondition.HeaderExpression(header); if ("Accept".equalsIgnoreCase(expr.name)) { for (MediaType mediaType : MediaType.parseMediaTypes(expr.value)) { + result = (result != null ? result : new LinkedHashSet<>()); result.add(new ProduceMediaTypeExpression(mediaType, expr.isNegated)); } } } } - if (produces != null) { + if (!ObjectUtils.isEmpty(produces)) { for (String produce : produces) { + result = (result != null ? result : new LinkedHashSet<>()); result.add(new ProduceMediaTypeExpression(produce)); } } - return result; + return (result != null ? new ArrayList<>(result) : Collections.emptyList()); } + /** + * Private constructor for internal use to create matching conditions. + * Note the expressions List is neither sorted, nor deep copied. + */ + private ProducesRequestCondition(List expressions, ProducesRequestCondition other) { + this.expressions = expressions; + this.contentTypeResolver = other.contentTypeResolver; + } + + /** * Return the contained "produces" expressions. */ 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 ece8e22477fd..716d6027e0ae 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 @@ -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. @@ -21,13 +21,13 @@ 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; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.lang.Nullable; +import org.springframework.util.ObjectUtils; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.cors.reactive.CorsUtils; import org.springframework.web.server.ServerWebExchange; @@ -62,16 +62,15 @@ public final class RequestMethodsRequestCondition extends AbstractRequestConditi * if, 0 the condition will match to every request */ public RequestMethodsRequestCondition(RequestMethod... requestMethods) { - this(asList(requestMethods)); + this.methods = (ObjectUtils.isEmpty(requestMethods) ? + Collections.emptySet() : new LinkedHashSet<>(Arrays.asList(requestMethods))); } - private RequestMethodsRequestCondition(Collection requestMethods) { - this.methods = Collections.unmodifiableSet(new LinkedHashSet<>(requestMethods)); - } - - - private static List asList(RequestMethod... requestMethods) { - return (requestMethods != null ? Arrays.asList(requestMethods) : Collections.emptyList()); + /** + * Private constructor for internal use when combining conditions. + */ + private RequestMethodsRequestCondition(Set requestMethods) { + this.methods = requestMethods; } @@ -98,6 +97,15 @@ protected String getToStringInfix() { */ @Override public RequestMethodsRequestCondition combine(RequestMethodsRequestCondition other) { + if (isEmpty() && other.isEmpty()) { + return this; + } + else if (other.isEmpty()) { + return this; + } + else if (isEmpty()) { + return other; + } Set set = new LinkedHashSet<>(this.methods); set.addAll(other.methods); return new RequestMethodsRequestCondition(set); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java index f2a55f60a232..9572cfb64160 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.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. @@ -16,12 +16,13 @@ package org.springframework.web.reactive.result.method; -import java.util.Arrays; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Set; -import java.util.stream.Collectors; import org.springframework.lang.Nullable; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; @@ -54,6 +55,19 @@ */ public final class RequestMappingInfo implements RequestCondition { + private static final PatternsRequestCondition EMPTY_PATTERNS = new PatternsRequestCondition(); + + private static final RequestMethodsRequestCondition EMPTY_REQUEST_METHODS = new RequestMethodsRequestCondition(); + + private static final ParamsRequestCondition EMPTY_PARAMS = new ParamsRequestCondition(); + + private static final HeadersRequestCondition EMPTY_HEADERS = new HeadersRequestCondition(); + + private static final ConsumesRequestCondition EMPTY_CONSUMES = new ConsumesRequestCondition(); + + private static final ProducesRequestCondition EMPTY_PRODUCES = new ProducesRequestCondition(); + + @Nullable private final String name; @@ -78,12 +92,12 @@ public RequestMappingInfo(@Nullable String name, @Nullable PatternsRequestCondit @Nullable ProducesRequestCondition produces, @Nullable RequestCondition custom) { this.name = (StringUtils.hasText(name) ? name : null); - this.patternsCondition = (patterns != null ? patterns : new PatternsRequestCondition()); - this.methodsCondition = (methods != null ? methods : new RequestMethodsRequestCondition()); - this.paramsCondition = (params != null ? params : new ParamsRequestCondition()); - this.headersCondition = (headers != null ? headers : new HeadersRequestCondition()); - this.consumesCondition = (consumes != null ? consumes : new ConsumesRequestCondition()); - this.producesCondition = (produces != null ? produces : new ProducesRequestCondition()); + this.patternsCondition = (patterns != null ? patterns : EMPTY_PATTERNS); + this.methodsCondition = (methods != null ? methods : EMPTY_REQUEST_METHODS); + this.paramsCondition = (params != null ? params : EMPTY_PARAMS); + this.headersCondition = (headers != null ? headers : EMPTY_HEADERS); + this.consumesCondition = (consumes != null ? consumes : EMPTY_CONSUMES); + this.producesCondition = (produces != null ? produces : EMPTY_PRODUCES); this.customConditionHolder = new RequestConditionHolder(custom); } @@ -430,6 +444,10 @@ private static class DefaultBuilder implements Builder { @Nullable private String[] produces; + private boolean hasContentType; + + private boolean hasAccept; + @Nullable private String mappingName; @@ -438,6 +456,7 @@ private static class DefaultBuilder implements Builder { private BuilderConfiguration options = new BuilderConfiguration(); + public DefaultBuilder(String... paths) { this.paths = paths; } @@ -462,6 +481,12 @@ public DefaultBuilder params(String... params) { @Override public DefaultBuilder headers(String... headers) { + for (String header : headers) { + this.hasContentType = this.hasContentType || + header.contains("Content-Type") || header.contains("content-type"); + this.hasAccept = this.hasAccept || + header.contains("Accept") || header.contains("accept"); + } this.headers = headers; return this; } @@ -498,31 +523,50 @@ public Builder options(BuilderConfiguration options) { @Override public RequestMappingInfo build() { - RequestedContentTypeResolver contentTypeResolver = this.options.getContentTypeResolver(); PathPatternParser parser = (this.options.getPatternParser() != null ? this.options.getPatternParser() : new PathPatternParser()); - PatternsRequestCondition patternsCondition = new PatternsRequestCondition(parse(this.paths, parser)); - - return new RequestMappingInfo(this.mappingName, patternsCondition, - new RequestMethodsRequestCondition(this.methods), - new ParamsRequestCondition(this.params), - new HeadersRequestCondition(this.headers), - new ConsumesRequestCondition(this.consumes, this.headers), - new ProducesRequestCondition(this.produces, this.headers, contentTypeResolver), + + RequestedContentTypeResolver contentTypeResolver = this.options.getContentTypeResolver(); + + return new RequestMappingInfo(this.mappingName, + isEmpty(this.paths) ? null : new PatternsRequestCondition(parse(this.paths, parser)), + ObjectUtils.isEmpty(this.methods) ? + null : new RequestMethodsRequestCondition(this.methods), + ObjectUtils.isEmpty(this.params) ? + null : new ParamsRequestCondition(this.params), + ObjectUtils.isEmpty(this.headers) ? + null : new HeadersRequestCondition(this.headers), + ObjectUtils.isEmpty(this.consumes) && !this.hasContentType ? + null : new ConsumesRequestCondition(this.consumes, this.headers), + ObjectUtils.isEmpty(this.produces) && !this.hasAccept ? + null : new ProducesRequestCondition(this.produces, this.headers, contentTypeResolver), this.customCondition); } - private static List parse(String[] paths, PathPatternParser parser) { - return Arrays - .stream(paths) - .map(path -> { - if (StringUtils.hasText(path) && !path.startsWith("/")) { - path = "/" + path; - } - return parser.parse(path); - }) - .collect(Collectors.toList()); + private static List parse(String[] patterns, PathPatternParser parser) { + if (isEmpty(patterns)) { + return Collections.emptyList(); + } + List result = new ArrayList<>(patterns.length); + for (String path : patterns) { + if (StringUtils.hasText(path) && !path.startsWith("/")) { + path = "/" + path; + } + result.add(parser.parse(path)); + } + return result; + } + + private static boolean isEmpty(String[] patterns) { + if (!ObjectUtils.isEmpty(patterns)) { + for (String pattern : patterns) { + if (StringUtils.hasText(pattern)) { + return false; + } + } + } + return true; } } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.java index 4439843cdd90..8ccfd31895bc 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.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. @@ -42,15 +42,19 @@ public class PatternsRequestConditionTests { @Test public void prependNonEmptyPatternsOnly() { PatternsRequestCondition c = createPatternsCondition(""); - assertThat(c.getPatterns().iterator().next().getPatternString()).as("Do not prepend empty patterns (SPR-8255)").isEqualTo(""); + assertThat(c.getPatterns().iterator().next().getPatternString()) + .as("Do not prepend empty patterns (SPR-8255)") + .isEqualTo(""); } @Test public void combineEmptySets() { PatternsRequestCondition c1 = new PatternsRequestCondition(); PatternsRequestCondition c2 = new PatternsRequestCondition(); + PatternsRequestCondition c3 = c1.combine(c2); - assertThat(c1.combine(c2)).isEqualTo(createPatternsCondition()); + assertThat(c3).isSameAs(c1); + assertThat(c1.getPatterns()).isSameAs(c2.getPatterns()).containsExactly(this.parser.parse("")); } @Test @@ -75,7 +79,7 @@ public void combineMultiplePatterns() { } @Test - public void matchDirectPath() throws Exception { + public void matchDirectPath() { PatternsRequestCondition condition = createPatternsCondition("/foo"); MockServerWebExchange exchange = MockServerWebExchange.from(get("/foo")); PatternsRequestCondition match = condition.getMatchingCondition(exchange); @@ -84,7 +88,7 @@ public void matchDirectPath() throws Exception { } @Test - public void matchPattern() throws Exception { + public void matchPattern() { PatternsRequestCondition condition = createPatternsCondition("/foo/*"); MockServerWebExchange exchange = MockServerWebExchange.from(get("/foo/bar")); PatternsRequestCondition match = condition.getMatchingCondition(exchange); @@ -93,7 +97,7 @@ public void matchPattern() throws Exception { } @Test - public void matchSortPatterns() throws Exception { + public void matchSortPatterns() { PatternsRequestCondition condition = createPatternsCondition("/*/*", "/foo/bar", "/foo/*"); MockServerWebExchange exchange = MockServerWebExchange.from(get("/foo/bar")); PatternsRequestCondition match = condition.getMatchingCondition(exchange); @@ -103,20 +107,24 @@ public void matchSortPatterns() throws Exception { } @Test - public void matchTrailingSlash() throws Exception { + public void matchTrailingSlash() { MockServerWebExchange exchange = MockServerWebExchange.from(get("/foo/")); PatternsRequestCondition condition = createPatternsCondition("/foo"); PatternsRequestCondition match = condition.getMatchingCondition(exchange); assertThat(match).isNotNull(); - assertThat(match.getPatterns().iterator().next().getPatternString()).as("Should match by default").isEqualTo("/foo"); + assertThat(match.getPatterns().iterator().next().getPatternString()) + .as("Should match by default") + .isEqualTo("/foo"); condition = createPatternsCondition("/foo"); match = condition.getMatchingCondition(exchange); assertThat(match).isNotNull(); - assertThat(match.getPatterns().iterator().next().getPatternString()).as("Trailing slash should be insensitive to useSuffixPatternMatch settings (SPR-6164, SPR-5636)").isEqualTo("/foo"); + assertThat(match.getPatterns().iterator().next().getPatternString()) + .as("Trailing slash should be insensitive to useSuffixPatternMatch settings (SPR-6164, SPR-5636)") + .isEqualTo("/foo"); PathPatternParser parser = new PathPatternParser(); parser.setMatchOptionalTrailingSeparator(false); @@ -127,7 +135,7 @@ public void matchTrailingSlash() throws Exception { } @Test - public void matchPatternContainsExtension() throws Exception { + public void matchPatternContainsExtension() { PatternsRequestCondition condition = createPatternsCondition("/foo.jpg"); MockServerWebExchange exchange = MockServerWebExchange.from(get("/foo.html")); PatternsRequestCondition match = condition.getMatchingCondition(exchange); @@ -138,18 +146,16 @@ public void matchPatternContainsExtension() throws Exception { @Test // gh-22543 public void matchWithEmptyPatterns() { PatternsRequestCondition condition = new PatternsRequestCondition(); - assertThat(condition).isEqualTo(new PatternsRequestCondition(this.parser.parse(""))); assertThat(condition.getMatchingCondition(MockServerWebExchange.from(get("")))).isNotNull(); assertThat(condition.getMatchingCondition(MockServerWebExchange.from(get("/anything")))).isNull(); condition = condition.combine(new PatternsRequestCondition()); - assertThat(condition).isEqualTo(new PatternsRequestCondition(this.parser.parse(""))); assertThat(condition.getMatchingCondition(MockServerWebExchange.from(get("")))).isNotNull(); assertThat(condition.getMatchingCondition(MockServerWebExchange.from(get("/anything")))).isNull(); } @Test - public void compareToConsistentWithEquals() throws Exception { + public void compareToConsistentWithEquals() { PatternsRequestCondition c1 = createPatternsCondition("/foo*"); PatternsRequestCondition c2 = createPatternsCondition("/foo*"); @@ -157,7 +163,7 @@ public void compareToConsistentWithEquals() throws Exception { } @Test - public void equallyMatchingPatternsAreBothPresent() throws Exception { + public void equallyMatchingPatternsAreBothPresent() { PatternsRequestCondition c = createPatternsCondition("/a", "/b"); assertThat(c.getPatterns().size()).isEqualTo(2); Iterator itr = c.getPatterns().iterator(); @@ -166,7 +172,7 @@ public void equallyMatchingPatternsAreBothPresent() throws Exception { } @Test - public void comparePatternSpecificity() throws Exception { + public void comparePatternSpecificity() { ServerWebExchange exchange = MockServerWebExchange.from(get("/foo")); PatternsRequestCondition c1 = createPatternsCondition("/fo*"); @@ -177,11 +183,13 @@ public void comparePatternSpecificity() throws Exception { c1 = createPatternsCondition("/fo*"); c2 = createPatternsCondition("/*oo"); - assertThat(c1.compareTo(c2, exchange)).as("Patterns are equally specific even if not the same").isEqualTo(0); + assertThat(c1.compareTo(c2, exchange)) + .as("Patterns are equally specific even if not the same") + .isEqualTo(0); } @Test - public void compareNumberOfMatchingPatterns() throws Exception { + public void compareNumberOfMatchingPatterns() { ServerWebExchange exchange = MockServerWebExchange.from(get("/foo.html")); PatternsRequestCondition c1 = createPatternsCondition("/foo.*", "/foo.jpeg"); @@ -197,7 +205,7 @@ public void compareNumberOfMatchingPatterns() throws Exception { private PatternsRequestCondition createPatternsCondition(String... patterns) { return new PatternsRequestCondition(Arrays .stream(patterns) - .map(rawPattern -> this.parser.parse(rawPattern)) + .map(this.parser::parse) .collect(Collectors.toList())); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/RequestMappingInfoTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/RequestMappingInfoTests.java index b9ded4a53609..528821c2f757 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/RequestMappingInfoTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/RequestMappingInfoTests.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. @@ -49,7 +49,6 @@ public class RequestMappingInfoTests { // TODO: CORS pre-flight (see @Disabled) - @Test public void createEmpty() { RequestMappingInfo info = paths().build(); @@ -62,6 +61,22 @@ public void createEmpty() { assertThat(info.getParamsCondition()).isNotNull(); assertThat(info.getHeadersCondition()).isNotNull(); assertThat(info.getCustomCondition()).isNull(); + + RequestMappingInfo anotherInfo = paths().build(); + assertThat(info.getPatternsCondition()).isSameAs(anotherInfo.getPatternsCondition()); + assertThat(info.getMethodsCondition()).isSameAs(anotherInfo.getMethodsCondition()); + assertThat(info.getParamsCondition()).isSameAs(anotherInfo.getParamsCondition()); + assertThat(info.getHeadersCondition()).isSameAs(anotherInfo.getHeadersCondition()); + assertThat(info.getConsumesCondition()).isSameAs(anotherInfo.getConsumesCondition()); + assertThat(info.getProducesCondition()).isSameAs(anotherInfo.getProducesCondition()); + + RequestMappingInfo result = info.combine(anotherInfo); + assertThat(info.getPatternsCondition()).isSameAs(result.getPatternsCondition()); + assertThat(info.getMethodsCondition()).isSameAs(result.getMethodsCondition()); + assertThat(info.getParamsCondition()).isSameAs(result.getParamsCondition()); + assertThat(info.getHeadersCondition()).isSameAs(result.getHeadersCondition()); + assertThat(info.getConsumesCondition()).isSameAs(result.getConsumesCondition()); + assertThat(info.getProducesCondition()).isSameAs(result.getProducesCondition()); } @Test 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 66fabb42a1f3..0efe0c849b3d 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 @@ -30,6 +30,7 @@ import org.springframework.http.MediaType; import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.cors.CorsUtils; @@ -76,37 +77,40 @@ public ConsumesRequestCondition(String... consumes) { * @param headers as described in {@link RequestMapping#headers()} */ public ConsumesRequestCondition(String[] consumes, @Nullable String[] headers) { - this.expressions = new ArrayList<>(parseExpressions(consumes, headers)); + this.expressions = parseExpressions(consumes, headers); if (this.expressions.size() > 1) { Collections.sort(this.expressions); } } - /** - * Private constructor for internal when creating matching conditions. - * Note the expressions List is neither sorted nor deep copied. - */ - private ConsumesRequestCondition(List expressions) { - this.expressions = expressions; - } - - - private static Set parseExpressions(String[] consumes, @Nullable String[] headers) { - Set result = new LinkedHashSet<>(); - if (headers != null) { + private static List parseExpressions(String[] consumes, @Nullable String[] headers) { + Set result = null; + if (!ObjectUtils.isEmpty(headers)) { for (String header : headers) { HeaderExpression expr = new HeaderExpression(header); if ("Content-Type".equalsIgnoreCase(expr.name) && expr.value != null) { + result = (result != null ? result : new LinkedHashSet<>()); for (MediaType mediaType : MediaType.parseMediaTypes(expr.value)) { result.add(new ConsumeMediaTypeExpression(mediaType, expr.isNegated)); } } } } - for (String consume : consumes) { - result.add(new ConsumeMediaTypeExpression(consume)); + if (!ObjectUtils.isEmpty(consumes)) { + result = (result != null ? result : new LinkedHashSet<>()); + for (String consume : consumes) { + result.add(new ConsumeMediaTypeExpression(consume)); + } } - return result; + return (result != null ? new ArrayList<>(result) : Collections.emptyList()); + } + + /** + * Private constructor for internal when creating matching conditions. + * Note the expressions List is neither sorted nor deep copied. + */ + private ConsumesRequestCondition(List expressions) { + this.expressions = expressions; } 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 206e010cda00..e3f1773318ee 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-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,7 @@ package org.springframework.web.servlet.mvc.condition; import java.util.Collection; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.Set; @@ -55,26 +56,29 @@ public final class HeadersRequestCondition extends AbstractRequestCondition conditions) { - this.expressions = conditions; - } - - private static Set parseExpressions(String... headers) { - Set expressions = new LinkedHashSet<>(); - for (String header : headers) { - HeaderExpression expr = new HeaderExpression(header); - if ("Accept".equalsIgnoreCase(expr.name) || "Content-Type".equalsIgnoreCase(expr.name)) { - continue; + Set result = null; + if (!ObjectUtils.isEmpty(headers)) { + for (String header : headers) { + HeaderExpression expr = new HeaderExpression(header); + if ("Accept".equalsIgnoreCase(expr.name) || "Content-Type".equalsIgnoreCase(expr.name)) { + continue; + } + result = (result != null ? result : new LinkedHashSet<>(headers.length)); + result.add(expr); } - expressions.add(expr); } - return expressions; + return (result != null ? result : Collections.emptySet()); } + private HeadersRequestCondition(Set conditions) { + this.expressions = conditions; + } + + /** * Return the contained request header expressions. */ @@ -98,6 +102,15 @@ protected String getToStringInfix() { */ @Override public HeadersRequestCondition combine(HeadersRequestCondition other) { + if (isEmpty() && other.isEmpty()) { + return this; + } + else if (other.isEmpty()) { + return this; + } + else if (isEmpty()) { + return other; + } Set set = new LinkedHashSet<>(this.expressions); set.addAll(other.expressions); return new HeadersRequestCondition(set); @@ -157,7 +170,7 @@ private long getValueMatchCount(Set expressions) { */ static class HeaderExpression extends AbstractNameValueExpression { - public HeaderExpression(String expression) { + HeaderExpression(String expression) { super(expression); } 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 fd062a038216..f06f363aa2c3 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-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. @@ -48,22 +48,24 @@ public final class ParamsRequestCondition extends AbstractRequestCondition conditions) { - this.expressions = Collections.unmodifiableSet(new LinkedHashSet<>(conditions)); - } - - - private static Collection parseExpressions(String... params) { - Set expressions = new LinkedHashSet<>(); + private static Set parseExpressions(String... params) { + if (ObjectUtils.isEmpty(params)) { + return Collections.emptySet(); + } + Set expressions = new LinkedHashSet<>(params.length); for (String param : params) { expressions.add(new ParamExpression(param)); } return expressions; } + private ParamsRequestCondition(Set conditions) { + this.expressions = conditions; + } + /** * Return the contained request parameter expressions. @@ -88,6 +90,15 @@ protected String getToStringInfix() { */ @Override public ParamsRequestCondition combine(ParamsRequestCondition other) { + if (isEmpty() && other.isEmpty()) { + return this; + } + else if (other.isEmpty()) { + return this; + } + else if (isEmpty()) { + return other; + } Set set = new LinkedHashSet<>(this.expressions); set.addAll(other.expressions); return new ParamsRequestCondition(set); 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 846e4f646296..3616aa33a9d9 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 @@ -17,7 +17,6 @@ package org.springframework.web.servlet.mvc.condition; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Comparator; @@ -30,6 +29,7 @@ import org.springframework.lang.Nullable; import org.springframework.util.AntPathMatcher; +import org.springframework.util.ObjectUtils; import org.springframework.util.PathMatcher; import org.springframework.util.StringUtils; import org.springframework.web.servlet.HandlerMapping; @@ -42,7 +42,10 @@ * @author Rossen Stoyanchev * @since 3.1 */ -public final class PatternsRequestCondition extends AbstractRequestCondition { +public class PatternsRequestCondition extends AbstractRequestCondition { + + private final static Set EMPTY_PATH_PATTERN = Collections.singleton(""); + private final Set patterns; @@ -64,7 +67,7 @@ public final class PatternsRequestCondition extends AbstractRequestCondition fileExtensions) { - this(Arrays.asList(patterns), urlPathHelper, pathMatcher, useSuffixPatternMatch, - useTrailingSlashMatch, fileExtensions); - } - - /** - * Private constructor accepting a collection of patterns. - */ - private PatternsRequestCondition(Collection patterns, @Nullable UrlPathHelper urlPathHelper, - @Nullable PathMatcher pathMatcher, boolean useSuffixPatternMatch, - boolean useTrailingSlashMatch, @Nullable List fileExtensions) { - - this.patterns = Collections.unmodifiableSet(prependLeadingSlash(patterns)); + this.patterns = initPatterns(patterns); this.pathHelper = urlPathHelper != null ? urlPathHelper : new UrlPathHelper(); this.pathMatcher = pathMatcher != null ? pathMatcher : new AntPathMatcher(); this.useSuffixPatternMatch = useSuffixPatternMatch; @@ -144,6 +136,31 @@ private PatternsRequestCondition(Collection patterns, @Nullable UrlPathH } } + private static Set initPatterns(String[] patterns) { + if (!hasPattern(patterns)) { + return EMPTY_PATH_PATTERN; + } + Set result = new LinkedHashSet<>(patterns.length); + for (String pattern : patterns) { + if (StringUtils.hasLength(pattern) && !pattern.startsWith("/")) { + pattern = "/" + pattern; + } + result.add(pattern); + } + return result; + } + + private static boolean hasPattern(String[] patterns) { + if (!ObjectUtils.isEmpty(patterns)) { + for (String pattern : patterns) { + if (StringUtils.hasText(pattern)) { + return true; + } + } + } + return false; + } + /** * Private constructor for use when combining and matching. */ @@ -157,20 +174,6 @@ private PatternsRequestCondition(Set patterns, PatternsRequestCondition } - private static Set prependLeadingSlash(Collection patterns) { - if (patterns.isEmpty()) { - return Collections.singleton(""); - } - Set result = new LinkedHashSet<>(patterns.size()); - for (String pattern : patterns) { - if (StringUtils.hasLength(pattern) && !pattern.startsWith("/")) { - pattern = "/" + pattern; - } - result.add(pattern); - } - return result; - } - public Set getPatterns() { return this.patterns; } @@ -197,6 +200,15 @@ protected String getToStringInfix() { */ @Override public PatternsRequestCondition combine(PatternsRequestCondition other) { + if (isEmptyPathPattern() && other.isEmptyPathPattern()) { + return this; + } + else if (other.isEmptyPathPattern()) { + return this; + } + else if (isEmptyPathPattern()) { + return other; + } Set result = new LinkedHashSet<>(); if (!this.patterns.isEmpty() && !other.patterns.isEmpty()) { for (String pattern1 : this.patterns) { @@ -205,18 +217,13 @@ public PatternsRequestCondition combine(PatternsRequestCondition other) { } } } - else if (!this.patterns.isEmpty()) { - result.addAll(this.patterns); - } - else if (!other.patterns.isEmpty()) { - result.addAll(other.patterns); - } - else { - result.add(""); - } return new PatternsRequestCondition(result, this); } + private boolean isEmptyPathPattern() { + return this.patterns == EMPTY_PATH_PATTERN; + } + /** * Checks if any of the patterns match the given request and returns an instance * that is guaranteed to contain matching patterns, sorted via @@ -236,9 +243,6 @@ else if (!other.patterns.isEmpty()) { @Override @Nullable public PatternsRequestCondition getMatchingCondition(HttpServletRequest request) { - if (this.patterns.isEmpty()) { - return this; - } String lookupPath = this.pathHelper.getLookupPathForRequest(request, HandlerMapping.LOOKUP_PATH); List matches = getMatchingPatterns(lookupPath); return !matches.isEmpty() ? new PatternsRequestCondition(new LinkedHashSet<>(matches), this) : null; @@ -257,7 +261,7 @@ public List getMatchingPatterns(String lookupPath) { for (String pattern : this.patterns) { String match = getMatchingPattern(pattern, lookupPath); if (match != null) { - matches = matches != null ? matches : new ArrayList<>(); + matches = (matches != null ? matches : new ArrayList<>()); matches.add(match); } } 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 b031c7fc42e3..7a6af2272599 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 @@ -27,6 +27,7 @@ import org.springframework.http.MediaType; import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; import org.springframework.web.HttpMediaTypeException; import org.springframework.web.HttpMediaTypeNotAcceptableException; @@ -96,41 +97,45 @@ public ProducesRequestCondition(String[] produces, @Nullable String[] headers) { public ProducesRequestCondition(String[] produces, @Nullable String[] headers, @Nullable ContentNegotiationManager manager) { - this.expressions = new ArrayList<>(parseExpressions(produces, headers)); + this.expressions = parseExpressions(produces, headers); if (this.expressions.size() > 1) { Collections.sort(this.expressions); } this.contentNegotiationManager = manager != null ? manager : DEFAULT_CONTENT_NEGOTIATION_MANAGER; } - /** - * Private constructor for internal use to create matching conditions. - * Note the expressions List is neither sorted nor deep copied. - */ - private ProducesRequestCondition(List expressions, ProducesRequestCondition other) { - this.expressions = expressions; - this.contentNegotiationManager = other.contentNegotiationManager; - } - - - private Set parseExpressions(String[] produces, @Nullable String[] headers) { - Set result = new LinkedHashSet<>(); - if (headers != null) { + private List parseExpressions(String[] produces, @Nullable String[] headers) { + Set result = null; + if (!ObjectUtils.isEmpty(headers)) { for (String header : headers) { HeaderExpression expr = new HeaderExpression(header); if ("Accept".equalsIgnoreCase(expr.name) && expr.value != null) { for (MediaType mediaType : MediaType.parseMediaTypes(expr.value)) { + result = (result != null ? result : new LinkedHashSet<>()); result.add(new ProduceMediaTypeExpression(mediaType, expr.isNegated)); } } } } - for (String produce : produces) { - result.add(new ProduceMediaTypeExpression(produce)); + if (!ObjectUtils.isEmpty(produces)) { + for (String produce : produces) { + result = (result != null ? result : new LinkedHashSet<>()); + result.add(new ProduceMediaTypeExpression(produce)); + } } - return result; + return (result != null ? new ArrayList<>(result) : Collections.emptyList()); } + /** + * Private constructor for internal use to create matching conditions. + * Note the expressions List is neither sorted nor deep copied. + */ + private ProducesRequestCondition(List expressions, ProducesRequestCondition other) { + this.expressions = expressions; + this.contentNegotiationManager = other.contentNegotiationManager; + } + + /** * Return the contained "produces" expressions. */ 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 c12aa14c12a3..96958c2e4310 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-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. @@ -30,6 +30,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.lang.Nullable; +import org.springframework.util.ObjectUtils; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.cors.CorsUtils; @@ -63,11 +64,15 @@ public final class RequestMethodsRequestCondition extends AbstractRequestConditi * if, 0 the condition will match to every request */ public RequestMethodsRequestCondition(RequestMethod... requestMethods) { - this(Arrays.asList(requestMethods)); + this.methods = (ObjectUtils.isEmpty(requestMethods) ? + Collections.emptySet() : new LinkedHashSet<>(Arrays.asList(requestMethods))); } - private RequestMethodsRequestCondition(Collection requestMethods) { - this.methods = Collections.unmodifiableSet(new LinkedHashSet<>(requestMethods)); + /** + * Private constructor for internal use when combining conditions. + */ + private RequestMethodsRequestCondition(Set methods) { + this.methods = methods; } @@ -94,6 +99,15 @@ protected String getToStringInfix() { */ @Override public RequestMethodsRequestCondition combine(RequestMethodsRequestCondition other) { + if (isEmpty() && other.isEmpty()) { + return this; + } + else if (other.isEmpty()) { + return this; + } + else if (isEmpty()) { + return other; + } Set set = new LinkedHashSet<>(this.methods); set.addAll(other.methods); return new RequestMethodsRequestCondition(set); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java index 7d7283999617..67a2012d4cf2 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java @@ -23,6 +23,7 @@ import org.springframework.http.HttpMethod; import org.springframework.lang.Nullable; +import org.springframework.util.ObjectUtils; import org.springframework.util.PathMatcher; import org.springframework.util.StringUtils; import org.springframework.web.accept.ContentNegotiationManager; @@ -56,6 +57,19 @@ */ public final class RequestMappingInfo implements RequestCondition { + private static final PatternsRequestCondition EMPTY_PATTERNS = new PatternsRequestCondition(); + + private static final RequestMethodsRequestCondition EMPTY_REQUEST_METHODS = new RequestMethodsRequestCondition(); + + private static final ParamsRequestCondition EMPTY_PARAMS = new ParamsRequestCondition(); + + private static final HeadersRequestCondition EMPTY_HEADERS = new HeadersRequestCondition(); + + private static final ConsumesRequestCondition EMPTY_CONSUMES = new ConsumesRequestCondition(); + + private static final ProducesRequestCondition EMPTY_PRODUCES = new ProducesRequestCondition(); + + @Nullable private final String name; @@ -80,12 +94,12 @@ public RequestMappingInfo(@Nullable String name, @Nullable PatternsRequestCondit @Nullable ProducesRequestCondition produces, @Nullable RequestCondition custom) { this.name = (StringUtils.hasText(name) ? name : null); - this.patternsCondition = (patterns != null ? patterns : new PatternsRequestCondition()); - this.methodsCondition = (methods != null ? methods : new RequestMethodsRequestCondition()); - this.paramsCondition = (params != null ? params : new ParamsRequestCondition()); - this.headersCondition = (headers != null ? headers : new HeadersRequestCondition()); - this.consumesCondition = (consumes != null ? consumes : new ConsumesRequestCondition()); - this.producesCondition = (produces != null ? produces : new ProducesRequestCondition()); + this.patternsCondition = (patterns != null ? patterns : EMPTY_PATTERNS); + this.methodsCondition = (methods != null ? methods : EMPTY_REQUEST_METHODS); + this.paramsCondition = (params != null ? params : EMPTY_PARAMS); + this.headersCondition = (headers != null ? headers : EMPTY_HEADERS); + this.consumesCondition = (consumes != null ? consumes : EMPTY_CONSUMES); + this.producesCondition = (produces != null ? produces : EMPTY_PRODUCES); this.customConditionHolder = new RequestConditionHolder(custom); } @@ -427,7 +441,7 @@ public interface Builder { private static class DefaultBuilder implements Builder { - private String[] paths = new String[0]; + private String[] paths; private RequestMethod[] methods = new RequestMethod[0]; @@ -439,6 +453,10 @@ private static class DefaultBuilder implements Builder { private String[] produces = new String[0]; + private boolean hasContentType; + + private boolean hasAccept; + @Nullable private String mappingName; @@ -471,6 +489,12 @@ public DefaultBuilder params(String... params) { @Override public DefaultBuilder headers(String... headers) { + for (String header : headers) { + this.hasContentType = this.hasContentType || + header.contains("Content-Type") || header.contains("content-type"); + this.hasAccept = this.hasAccept || + header.contains("Accept") || header.contains("accept"); + } this.headers = headers; return this; } @@ -508,19 +532,26 @@ public Builder options(BuilderConfiguration options) { @Override @SuppressWarnings("deprecation") public RequestMappingInfo build() { - ContentNegotiationManager manager = this.options.getContentNegotiationManager(); - PatternsRequestCondition patternsCondition = new PatternsRequestCondition( - this.paths, this.options.getUrlPathHelper(), this.options.getPathMatcher(), - this.options.useSuffixPatternMatch(), this.options.useTrailingSlashMatch(), - this.options.getFileExtensions()); + PatternsRequestCondition patternsCondition = ObjectUtils.isEmpty(this.paths) ? null : + new PatternsRequestCondition( + this.paths, this.options.getUrlPathHelper(), this.options.getPathMatcher(), + this.options.useSuffixPatternMatch(), this.options.useTrailingSlashMatch(), + this.options.getFileExtensions()); + + ContentNegotiationManager manager = this.options.getContentNegotiationManager(); return new RequestMappingInfo(this.mappingName, patternsCondition, - new RequestMethodsRequestCondition(this.methods), - new ParamsRequestCondition(this.params), - new HeadersRequestCondition(this.headers), - new ConsumesRequestCondition(this.consumes, this.headers), - new ProducesRequestCondition(this.produces, this.headers, manager), + ObjectUtils.isEmpty(this.methods) ? + null : new RequestMethodsRequestCondition(this.methods), + ObjectUtils.isEmpty(this.params) ? + null : new ParamsRequestCondition(this.params), + ObjectUtils.isEmpty(this.headers) ? + null : new HeadersRequestCondition(this.headers), + ObjectUtils.isEmpty(this.consumes) && !this.hasContentType ? + null : new ConsumesRequestCondition(this.consumes, this.headers), + ObjectUtils.isEmpty(this.produces) && !this.hasAccept ? + null : new ProducesRequestCondition(this.produces, this.headers, manager), this.customCondition); } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.java index d24e4df73ae4..4ae8551ecf4f 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.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,7 +17,7 @@ package org.springframework.web.servlet.mvc.condition; import java.util.Arrays; -import java.util.List; +import java.util.Collections; import javax.servlet.http.HttpServletRequest; @@ -48,8 +48,10 @@ public void prependNonEmptyPatternsOnly() { public void combineEmptySets() { PatternsRequestCondition c1 = new PatternsRequestCondition(); PatternsRequestCondition c2 = new PatternsRequestCondition(); + PatternsRequestCondition c3 = c1.combine(c2); - assertThat(c1.combine(c2)).isEqualTo(new PatternsRequestCondition("")); + assertThat(c3).isSameAs(c1); + assertThat(c1.getPatterns()).isSameAs(c2.getPatterns()).containsExactly(""); } @Test @@ -118,9 +120,8 @@ public void matchSuffixPattern() { @Test // SPR-8410 public void matchSuffixPatternUsingFileExtensions() { - String[] patterns = new String[] {"/jobs/{jobName}"}; - List extensions = Arrays.asList("json"); - PatternsRequestCondition condition = new PatternsRequestCondition(patterns, null, null, true, false, extensions); + PatternsRequestCondition condition = new PatternsRequestCondition( + new String[] {"/jobs/{jobName}"}, null, null, true, false, Collections.singletonList("json")); MockHttpServletRequest request = new MockHttpServletRequest("GET", "/jobs/my.job"); PatternsRequestCondition match = condition.getMatchingCondition(request); @@ -165,9 +166,11 @@ public void matchTrailingSlash() { match = condition.getMatchingCondition(request); assertThat(match).isNotNull(); - assertThat(match.getPatterns().iterator().next()).as("Trailing slash should be insensitive to useSuffixPatternMatch settings (SPR-6164, SPR-5636)").isEqualTo("/foo/"); + assertThat(match.getPatterns().iterator().next()) + .as("Trailing slash should be insensitive to useSuffixPatternMatch settings (SPR-6164, SPR-5636)") + .isEqualTo("/foo/"); - condition = new PatternsRequestCondition(new String[] {"/foo"}, null, null, false, false); + condition = new PatternsRequestCondition(new String[] {"/foo"}, null, null, false); match = condition.getMatchingCondition(request); assertThat(match).isNull(); @@ -175,8 +178,8 @@ public void matchTrailingSlash() { @Test public void matchPatternContainsExtension() { - PatternsRequestCondition condition = new PatternsRequestCondition("/foo.jpg"); - PatternsRequestCondition match = condition.getMatchingCondition(new MockHttpServletRequest("GET", "/foo.html")); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo.html"); + PatternsRequestCondition match = new PatternsRequestCondition("/foo.jpg").getMatchingCondition(request); assertThat(match).isNull(); } @@ -184,12 +187,10 @@ public void matchPatternContainsExtension() { @Test // gh-22543 public void matchWithEmptyPatterns() { PatternsRequestCondition condition = new PatternsRequestCondition(); - assertThat(condition).isEqualTo(new PatternsRequestCondition("")); assertThat(condition.getMatchingCondition(new MockHttpServletRequest("GET", ""))).isNotNull(); assertThat(condition.getMatchingCondition(new MockHttpServletRequest("GET", "/anything"))).isNull(); condition = condition.combine(new PatternsRequestCondition()); - assertThat(condition).isEqualTo(new PatternsRequestCondition("")); assertThat(condition.getMatchingCondition(new MockHttpServletRequest("GET", ""))).isNotNull(); assertThat(condition.getMatchingCondition(new MockHttpServletRequest("GET", "/anything"))).isNull(); } @@ -211,7 +212,7 @@ public void comparePatternSpecificity() { } @Test - public void compareNumberOfMatchingPatterns() throws Exception { + public void compareNumberOfMatchingPatterns() { HttpServletRequest request = new MockHttpServletRequest("GET", "/foo.html"); PatternsRequestCondition c1 = new PatternsRequestCondition("/foo", "*.jpeg"); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java index 35acd292f1cc..2529942d7e92 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java @@ -44,16 +44,33 @@ public class RequestMappingInfoTests { @Test public void createEmpty() { + RequestMappingInfo info = paths().build(); // gh-22543 assertThat(info.getPatternsCondition().getPatterns()).isEqualTo(Collections.singleton("")); assertThat(info.getMethodsCondition().getMethods().size()).isEqualTo(0); - assertThat(info.getConsumesCondition().isEmpty()).isEqualTo(true); - assertThat(info.getProducesCondition().isEmpty()).isEqualTo(true); assertThat(info.getParamsCondition()).isNotNull(); assertThat(info.getHeadersCondition()).isNotNull(); + assertThat(info.getConsumesCondition().isEmpty()).isEqualTo(true); + assertThat(info.getProducesCondition().isEmpty()).isEqualTo(true); assertThat(info.getCustomCondition()).isNull(); + + RequestMappingInfo anotherInfo = paths().build(); + assertThat(info.getPatternsCondition()).isSameAs(anotherInfo.getPatternsCondition()); + assertThat(info.getMethodsCondition()).isSameAs(anotherInfo.getMethodsCondition()); + assertThat(info.getParamsCondition()).isSameAs(anotherInfo.getParamsCondition()); + assertThat(info.getHeadersCondition()).isSameAs(anotherInfo.getHeadersCondition()); + assertThat(info.getConsumesCondition()).isSameAs(anotherInfo.getConsumesCondition()); + assertThat(info.getProducesCondition()).isSameAs(anotherInfo.getProducesCondition()); + + RequestMappingInfo result = info.combine(anotherInfo); + assertThat(info.getPatternsCondition()).isSameAs(result.getPatternsCondition()); + assertThat(info.getMethodsCondition()).isSameAs(result.getMethodsCondition()); + assertThat(info.getParamsCondition()).isSameAs(result.getParamsCondition()); + assertThat(info.getHeadersCondition()).isSameAs(result.getHeadersCondition()); + assertThat(info.getConsumesCondition()).isSameAs(result.getConsumesCondition()); + assertThat(info.getProducesCondition()).isSameAs(result.getProducesCondition()); } @Test