Skip to content

Commit bb9fcad

Browse files
rstoyanchevrstoyanchev
committed
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
1 parent 8f96712 commit bb9fcad

12 files changed

+175
-83
lines changed

‎spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ConsumesRequestCondition.java

+22-8
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525

2626
import org.springframework.http.InvalidMediaTypeException;
2727
import org.springframework.http.MediaType;
28+
import org.springframework.lang.Nullable;
29+
import org.springframework.util.CollectionUtils;
2830
import org.springframework.web.bind.annotation.RequestMapping;
2931
import org.springframework.web.cors.reactive.CorsUtils;
3032
import org.springframework.web.server.ServerWebExchange;
@@ -68,15 +70,16 @@ public ConsumesRequestCondition(String... consumes) {
6870
* @param headers as described in {@link RequestMapping#headers()}
6971
*/
7072
public ConsumesRequestCondition(String[] consumes, String[] headers) {
71-
this(parseExpressions(consumes, headers));
73+
this.expressions = new ArrayList<>(parseExpressions(consumes, headers));
74+
Collections.sort(this.expressions);
7275
}
7376

7477
/**
75-
* Private constructor accepting parsed media type expressions.
78+
* Private constructor for internal when creating matching conditions.
79+
* Note the expressions List is neither sorted nor deep copied.
7680
*/
77-
private ConsumesRequestCondition(Collection<ConsumeMediaTypeExpression> expressions) {
78-
this.expressions = new ArrayList<>(expressions);
79-
Collections.sort(this.expressions);
81+
private ConsumesRequestCondition(List<ConsumeMediaTypeExpression> expressions) {
82+
this.expressions = expressions;
8083
}
8184

8285

@@ -166,9 +169,20 @@ public ConsumesRequestCondition getMatchingCondition(ServerWebExchange exchange)
166169
if (isEmpty()) {
167170
return this;
168171
}
169-
Set<ConsumeMediaTypeExpression> result = new LinkedHashSet<>(this.expressions);
170-
result.removeIf(expression -> !expression.match(exchange));
171-
return (!result.isEmpty() ? new ConsumesRequestCondition(result) : null);
172+
List<ConsumeMediaTypeExpression> result = getMatchingExpressions(exchange);
173+
return !CollectionUtils.isEmpty(result) ? new ConsumesRequestCondition(result) : null;
174+
}
175+
176+
@Nullable
177+
private List<ConsumeMediaTypeExpression> getMatchingExpressions(ServerWebExchange exchange) {
178+
List<ConsumeMediaTypeExpression> result = null;
179+
for (ConsumeMediaTypeExpression expression : this.expressions) {
180+
if (expression.match(exchange)) {
181+
result = result != null ? result : new ArrayList<>();
182+
result.add(expression);
183+
}
184+
}
185+
return result;
172186
}
173187

174188
/**

‎spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/HeadersRequestCondition.java

+4-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,7 +17,6 @@
1717
package org.springframework.web.reactive.result.condition;
1818

1919
import java.util.Collection;
20-
import java.util.Collections;
2120
import java.util.LinkedHashSet;
2221
import java.util.Set;
2322

@@ -56,12 +55,12 @@ public HeadersRequestCondition(String... headers) {
5655
this(parseExpressions(headers));
5756
}
5857

59-
private HeadersRequestCondition(Collection<HeaderExpression> conditions) {
60-
this.expressions = Collections.unmodifiableSet(new LinkedHashSet<>(conditions));
58+
private HeadersRequestCondition(Set<HeaderExpression> conditions) {
59+
this.expressions = conditions;
6160
}
6261

6362

64-
private static Collection<HeaderExpression> parseExpressions(String... headers) {
63+
private static Set<HeaderExpression> parseExpressions(String... headers) {
6564
Set<HeaderExpression> expressions = new LinkedHashSet<>();
6665
if (headers != null) {
6766
for (String header : headers) {

‎spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.

‎spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java

+25-14
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,18 @@
1717
package org.springframework.web.reactive.result.condition;
1818

1919
import java.util.ArrayList;
20-
import java.util.Collection;
2120
import java.util.Collections;
2221
import java.util.LinkedHashSet;
2322
import java.util.List;
2423
import java.util.Set;
2524

2625
import org.springframework.http.MediaType;
2726
import org.springframework.lang.Nullable;
27+
import org.springframework.util.CollectionUtils;
2828
import org.springframework.util.StringUtils;
2929
import org.springframework.web.accept.ContentNegotiationManager;
3030
import org.springframework.web.bind.annotation.RequestMapping;
3131
import org.springframework.web.cors.reactive.CorsUtils;
32-
import org.springframework.web.reactive.accept.HeaderContentTypeResolver;
3332
import org.springframework.web.reactive.accept.RequestedContentTypeResolver;
3433
import org.springframework.web.reactive.accept.RequestedContentTypeResolverBuilder;
3534
import org.springframework.web.server.NotAcceptableStatusException;
@@ -48,6 +47,9 @@
4847
*/
4948
public final class ProducesRequestCondition extends AbstractRequestCondition<ProducesRequestCondition> {
5049

50+
private static final RequestedContentTypeResolver DEFAULT_CONTENT_TYPE_RESOLVER =
51+
new RequestedContentTypeResolverBuilder().build();
52+
5153
private static final ProducesRequestCondition EMPTY_CONDITION = new ProducesRequestCondition();
5254

5355

@@ -90,18 +92,16 @@ public ProducesRequestCondition(String[] produces, String[] headers) {
9092
public ProducesRequestCondition(String[] produces, String[] headers, RequestedContentTypeResolver resolver) {
9193
this.expressions = new ArrayList<>(parseExpressions(produces, headers));
9294
Collections.sort(this.expressions);
93-
this.contentTypeResolver = (resolver != null ? resolver : new HeaderContentTypeResolver());
95+
this.contentTypeResolver = resolver != null ? resolver : DEFAULT_CONTENT_TYPE_RESOLVER;
9496
}
9597

9698
/**
97-
* Private constructor with already parsed media type expressions.
99+
* Private constructor for internal use to create matching conditions.
100+
* Note the expressions List is neither sorted, nor deep copied.
98101
*/
99-
private ProducesRequestCondition(Collection<ProduceMediaTypeExpression> expressions,
100-
RequestedContentTypeResolver resolver) {
101-
102-
this.expressions = new ArrayList<>(expressions);
103-
Collections.sort(this.expressions);
104-
this.contentTypeResolver = (resolver != null ? resolver : new RequestedContentTypeResolverBuilder().build());
102+
private ProducesRequestCondition(List<ProduceMediaTypeExpression> expressions, ProducesRequestCondition other) {
103+
this.expressions = expressions;
104+
this.contentTypeResolver = other.contentTypeResolver;
105105
}
106106

107107

@@ -189,10 +189,9 @@ public ProducesRequestCondition getMatchingCondition(ServerWebExchange exchange)
189189
if (isEmpty() || CorsUtils.isPreFlightRequest(exchange.getRequest())) {
190190
return EMPTY_CONDITION;
191191
}
192-
Set<ProduceMediaTypeExpression> result = new LinkedHashSet<>(this.expressions);
193-
result.removeIf(expression -> !expression.match(exchange));
194-
if (!result.isEmpty()) {
195-
return new ProducesRequestCondition(result, this.contentTypeResolver);
192+
List<ProduceMediaTypeExpression> result = getMatchingExpressions(exchange);
193+
if (!CollectionUtils.isEmpty(result)) {
194+
return new ProducesRequestCondition(result, this);
196195
}
197196
else {
198197
try {
@@ -207,6 +206,18 @@ public ProducesRequestCondition getMatchingCondition(ServerWebExchange exchange)
207206
return null;
208207
}
209208

209+
@Nullable
210+
private List<ProduceMediaTypeExpression> getMatchingExpressions(ServerWebExchange exchange) {
211+
List<ProduceMediaTypeExpression> result = null;
212+
for (ProduceMediaTypeExpression expression : this.expressions) {
213+
if (expression.match(exchange)) {
214+
result = result != null ? result : new ArrayList<>();
215+
result.add(expression);
216+
}
217+
}
218+
return result;
219+
}
220+
210221
/**
211222
* Compares this and another "produces" condition as follows:
212223
* <ol>

‎spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/RequestMethodsRequestCondition.java

+19-9
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919
import java.util.Arrays;
2020
import java.util.Collection;
2121
import java.util.Collections;
22+
import java.util.HashMap;
2223
import java.util.LinkedHashSet;
2324
import java.util.List;
25+
import java.util.Map;
2426
import java.util.Set;
2527

2628
import org.springframework.http.HttpMethod;
@@ -39,8 +41,15 @@
3941
*/
4042
public final class RequestMethodsRequestCondition extends AbstractRequestCondition<RequestMethodsRequestCondition> {
4143

42-
private static final RequestMethodsRequestCondition GET_CONDITION =
43-
new RequestMethodsRequestCondition(RequestMethod.GET);
44+
/** Per HTTP method cache to return ready instances from getMatchingCondition. */
45+
private static final Map<String, RequestMethodsRequestCondition> requestMethodConditionCache;
46+
47+
static {
48+
requestMethodConditionCache = new HashMap<>(RequestMethod.values().length);
49+
for (RequestMethod method : RequestMethod.values()) {
50+
requestMethodConditionCache.put(method.name(), new RequestMethodsRequestCondition(method));
51+
}
52+
}
4453

4554

4655
private final Set<RequestMethod> methods;
@@ -110,36 +119,37 @@ public RequestMethodsRequestCondition getMatchingCondition(ServerWebExchange exc
110119
}
111120
if (getMethods().isEmpty()) {
112121
if (RequestMethod.OPTIONS.name().equals(exchange.getRequest().getMethodValue())) {
113-
return null; // No implicit match for OPTIONS (we handle it)
122+
return null; // We handle OPTIONS transparently, so don't match if no explicit declarations
114123
}
115124
return this;
116125
}
117-
return matchRequestMethod(exchange.getRequest().getMethod());
126+
return matchRequestMethod(exchange.getRequest().getMethodValue());
118127
}
119128

120129
/**
121130
* On a pre-flight request match to the would-be, actual request.
122131
* Hence empty conditions is a match, otherwise try to match to the HTTP
123132
* method in the "Access-Control-Request-Method" header.
124133
*/
134+
@Nullable
125135
private RequestMethodsRequestCondition matchPreFlight(ServerHttpRequest request) {
126136
if (getMethods().isEmpty()) {
127137
return this;
128138
}
129139
HttpMethod expectedMethod = request.getHeaders().getAccessControlRequestMethod();
130-
return matchRequestMethod(expectedMethod);
140+
return expectedMethod != null ? matchRequestMethod(expectedMethod.name()) : null;
131141
}
132142

133143
@Nullable
134-
private RequestMethodsRequestCondition matchRequestMethod(@Nullable HttpMethod httpMethod) {
144+
private RequestMethodsRequestCondition matchRequestMethod(@Nullable String httpMethod) {
135145
if (httpMethod != null) {
136146
for (RequestMethod method : getMethods()) {
137147
if (httpMethod.matches(method.name())) {
138-
return new RequestMethodsRequestCondition(method);
148+
return requestMethodConditionCache.get(method.name());
139149
}
140150
}
141-
if (httpMethod == HttpMethod.HEAD && getMethods().contains(RequestMethod.GET)) {
142-
return GET_CONDITION;
151+
if (HttpMethod.HEAD.matches(httpMethod) && getMethods().contains(RequestMethod.GET)) {
152+
return requestMethodConditionCache.get(HttpMethod.GET.name());
143153
}
144154
}
145155
return null;

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/AbstractNameValueExpression.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public final boolean match(HttpServletRequest request) {
8080
else {
8181
isMatch = matchName(request);
8282
}
83-
return (this.isNegated ? !isMatch : isMatch);
83+
return this.isNegated != isMatch;
8484
}
8585

8686

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java

+21-8
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.springframework.http.InvalidMediaTypeException;
2828
import org.springframework.http.MediaType;
2929
import org.springframework.lang.Nullable;
30+
import org.springframework.util.CollectionUtils;
3031
import org.springframework.util.StringUtils;
3132
import org.springframework.web.bind.annotation.RequestMapping;
3233
import org.springframework.web.cors.CorsUtils;
@@ -70,15 +71,16 @@ public ConsumesRequestCondition(String... consumes) {
7071
* @param headers as described in {@link RequestMapping#headers()}
7172
*/
7273
public ConsumesRequestCondition(String[] consumes, @Nullable String[] headers) {
73-
this(parseExpressions(consumes, headers));
74+
this.expressions = new ArrayList<>(parseExpressions(consumes, headers));
75+
Collections.sort(this.expressions);
7476
}
7577

7678
/**
77-
* Private constructor accepting parsed media type expressions.
79+
* Private constructor for internal when creating matching conditions.
80+
* Note the expressions List is neither sorted nor deep copied.
7881
*/
79-
private ConsumesRequestCondition(Collection<ConsumeMediaTypeExpression> expressions) {
80-
this.expressions = new ArrayList<>(expressions);
81-
Collections.sort(this.expressions);
82+
private ConsumesRequestCondition(List<ConsumeMediaTypeExpression> expressions) {
83+
this.expressions = expressions;
8284
}
8385

8486

@@ -179,9 +181,20 @@ public ConsumesRequestCondition getMatchingCondition(HttpServletRequest request)
179181
return null;
180182
}
181183

182-
Set<ConsumeMediaTypeExpression> result = new LinkedHashSet<>(this.expressions);
183-
result.removeIf(expression -> !expression.match(contentType));
184-
return (!result.isEmpty() ? new ConsumesRequestCondition(result) : null);
184+
List<ConsumeMediaTypeExpression> result = getMatchingExpressions(contentType);
185+
return !CollectionUtils.isEmpty(result) ? new ConsumesRequestCondition(result) : null;
186+
}
187+
188+
@Nullable
189+
private List<ConsumeMediaTypeExpression> getMatchingExpressions(MediaType contentType) {
190+
List<ConsumeMediaTypeExpression> result = null;
191+
for (ConsumeMediaTypeExpression expression : this.expressions) {
192+
if (expression.match(contentType)) {
193+
result = result != null ? result : new ArrayList<>();
194+
result.add(expression);
195+
}
196+
}
197+
return result;
185198
}
186199

187200
/**

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/HeadersRequestCondition.java

+4-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,7 +17,6 @@
1717
package org.springframework.web.servlet.mvc.condition;
1818

1919
import java.util.Collection;
20-
import java.util.Collections;
2120
import java.util.LinkedHashSet;
2221
import java.util.Set;
2322
import javax.servlet.http.HttpServletRequest;
@@ -58,12 +57,12 @@ public HeadersRequestCondition(String... headers) {
5857
this(parseExpressions(headers));
5958
}
6059

61-
private HeadersRequestCondition(Collection<HeaderExpression> conditions) {
62-
this.expressions = Collections.unmodifiableSet(new LinkedHashSet<>(conditions));
60+
private HeadersRequestCondition(Set<HeaderExpression> conditions) {
61+
this.expressions = conditions;
6362
}
6463

6564

66-
private static Collection<HeaderExpression> parseExpressions(String... headers) {
65+
private static Set<HeaderExpression> parseExpressions(String... headers) {
6766
Set<HeaderExpression> expressions = new LinkedHashSet<>();
6867
for (String header : headers) {
6968
HeaderExpression expr = new HeaderExpression(header);

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ParamsRequestCondition.java

+15-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
1818

1919
import java.util.Collection;
2020
import java.util.Collections;
21+
import java.util.HashSet;
2122
import java.util.LinkedHashSet;
2223
import java.util.Set;
2324
import javax.servlet.http.HttpServletRequest;
@@ -142,8 +143,15 @@ private long getValueMatchCount(Set<ParamExpression> expressions) {
142143
*/
143144
static class ParamExpression extends AbstractNameValueExpression<String> {
144145

146+
private final Set<String> namesToMatch = new HashSet<>(WebUtils.SUBMIT_IMAGE_SUFFIXES.length + 1);
147+
148+
145149
ParamExpression(String expression) {
146150
super(expression);
151+
this.namesToMatch.add(getName());
152+
for (String suffix : WebUtils.SUBMIT_IMAGE_SUFFIXES) {
153+
this.namesToMatch.add(getName() + suffix);
154+
}
147155
}
148156

149157
@Override
@@ -158,8 +166,12 @@ protected String parseValue(String valueExpression) {
158166

159167
@Override
160168
protected boolean matchName(HttpServletRequest request) {
161-
return (WebUtils.hasSubmitParameter(request, this.name) ||
162-
request.getParameterMap().containsKey(this.name));
169+
for (String current : this.namesToMatch) {
170+
if (request.getParameterMap().get(current) != null) {
171+
return true;
172+
}
173+
}
174+
return request.getParameterMap().containsKey(this.name);
163175
}
164176

165177
@Override

0 commit comments

Comments
 (0)