Skip to content

Commit df99889

Browse files
rstoyanchevrstoyanchev
committed
Avoided repeated creation of ReadOnlyHttpHeaders wrapper
See gh-24680
1 parent 3276f81 commit df99889

File tree

10 files changed

+99
-62
lines changed

10 files changed

+99
-62
lines changed

‎spring-web/src/main/java/org/springframework/http/HttpHeaders.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1769,7 +1769,9 @@ public String toString() {
17691769

17701770

17711771
/**
1772-
* Apply a read-only {@code HttpHeaders} wrapper around the given headers.
1772+
* Apply a read-only {@code HttpHeaders} wrapper around the given headers
1773+
* that also caches the parsed representations of the "Accept" and
1774+
* "Content-Type" headers.
17731775
*/
17741776
public static HttpHeaders readOnlyHttpHeaders(MultiValueMap<String, String> headers) {
17751777
Assert.notNull(headers, "HttpHeaders must not be null");

‎spring-web/src/main/java/org/springframework/http/client/AbstractClientHttpRequest.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2020 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.
@@ -20,6 +20,7 @@
2020
import java.io.OutputStream;
2121

2222
import org.springframework.http.HttpHeaders;
23+
import org.springframework.lang.Nullable;
2324
import org.springframework.util.Assert;
2425

2526
/**
@@ -35,10 +36,22 @@ public abstract class AbstractClientHttpRequest implements ClientHttpRequest {
3536

3637
private boolean executed = false;
3738

39+
@Nullable
40+
private HttpHeaders readOnlyHeaders;
41+
3842

3943
@Override
4044
public final HttpHeaders getHeaders() {
41-
return (this.executed ? HttpHeaders.readOnlyHttpHeaders(this.headers) : this.headers);
45+
if (this.readOnlyHeaders != null) {
46+
return this.readOnlyHeaders;
47+
}
48+
else if (this.executed) {
49+
this.readOnlyHeaders = HttpHeaders.readOnlyHttpHeaders(this.headers);
50+
return this.readOnlyHeaders;
51+
}
52+
else {
53+
return this.headers;
54+
}
4255
}
4356

4457
@Override

‎spring-web/src/main/java/org/springframework/http/client/reactive/AbstractClientHttpRequest.java

Lines changed: 13 additions & 4 deletions
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-2020 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.
@@ -60,6 +60,9 @@ private enum State {NEW, COMMITTING, COMMITTED}
6060

6161
private final List<Supplier<? extends Publisher<Void>>> commitActions = new ArrayList<>(4);
6262

63+
@Nullable
64+
private HttpHeaders readOnlyHeaders;
65+
6366

6467
public AbstractClientHttpRequest() {
6568
this(new HttpHeaders());
@@ -74,10 +77,16 @@ public AbstractClientHttpRequest(HttpHeaders headers) {
7477

7578
@Override
7679
public HttpHeaders getHeaders() {
77-
if (State.COMMITTED.equals(this.state.get())) {
78-
return HttpHeaders.readOnlyHttpHeaders(this.headers);
80+
if (this.readOnlyHeaders != null) {
81+
return this.readOnlyHeaders;
82+
}
83+
else if (State.COMMITTED.equals(this.state.get())) {
84+
this.readOnlyHeaders = HttpHeaders.readOnlyHttpHeaders(this.headers);
85+
return this.readOnlyHeaders;
86+
}
87+
else {
88+
return this.headers;
7989
}
80-
return this.headers;
8190
}
8291

8392
@Override

‎spring-web/src/main/java/org/springframework/http/server/ServletServerHttpResponse.java

Lines changed: 14 additions & 2 deletions
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-2020 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.
@@ -47,6 +47,9 @@ public class ServletServerHttpResponse implements ServerHttpResponse {
4747

4848
private boolean bodyUsed = false;
4949

50+
@Nullable
51+
private HttpHeaders readOnlyHeaders;
52+
5053

5154
/**
5255
* Construct a new instance of the ServletServerHttpResponse based on the given {@link HttpServletResponse}.
@@ -74,7 +77,16 @@ public void setStatusCode(HttpStatus status) {
7477

7578
@Override
7679
public HttpHeaders getHeaders() {
77-
return (this.headersWritten ? HttpHeaders.readOnlyHttpHeaders(this.headers) : this.headers);
80+
if (this.readOnlyHeaders != null) {
81+
return this.readOnlyHeaders;
82+
}
83+
else if (this.headersWritten) {
84+
this.readOnlyHeaders = HttpHeaders.readOnlyHttpHeaders(this.headers);
85+
return this.readOnlyHeaders;
86+
}
87+
else {
88+
return this.headers;
89+
}
7890
}
7991

8092
@Override

‎spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ private enum State {NEW, COMMITTING, COMMITTED}
7575

7676
private final List<Supplier<? extends Mono<Void>>> commitActions = new ArrayList<>(4);
7777

78+
@Nullable
79+
private HttpHeaders readOnlyHeaders;
80+
7881

7982
public AbstractServerHttpResponse(DataBufferFactory dataBufferFactory) {
8083
this(dataBufferFactory, new HttpHeaders());
@@ -155,8 +158,16 @@ public Integer getStatusCodeValue() {
155158

156159
@Override
157160
public HttpHeaders getHeaders() {
158-
return (this.state.get() == State.COMMITTED ?
159-
HttpHeaders.readOnlyHttpHeaders(this.headers) : this.headers);
161+
if (this.readOnlyHeaders != null) {
162+
return this.readOnlyHeaders;
163+
}
164+
else if (this.state.get() == State.COMMITTED) {
165+
this.readOnlyHeaders = HttpHeaders.readOnlyHttpHeaders(this.headers);
166+
return this.readOnlyHeaders;
167+
}
168+
else {
169+
return this.headers;
170+
}
160171
}
161172

162173
@Override

‎spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponse.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -234,29 +234,28 @@ HttpRequest request() {
234234

235235
private class DefaultHeaders implements Headers {
236236

237-
private HttpHeaders delegate() {
238-
return response.getHeaders();
239-
}
237+
private final HttpHeaders httpHeaders =
238+
HttpHeaders.readOnlyHttpHeaders(response.getHeaders());
240239

241240
@Override
242241
public OptionalLong contentLength() {
243-
return toOptionalLong(delegate().getContentLength());
242+
return toOptionalLong(this.httpHeaders.getContentLength());
244243
}
245244

246245
@Override
247246
public Optional<MediaType> contentType() {
248-
return Optional.ofNullable(delegate().getContentType());
247+
return Optional.ofNullable(this.httpHeaders.getContentType());
249248
}
250249

251250
@Override
252251
public List<String> header(String headerName) {
253-
List<String> headerValues = delegate().get(headerName);
252+
List<String> headerValues = this.httpHeaders.get(headerName);
254253
return (headerValues != null ? headerValues : Collections.emptyList());
255254
}
256255

257256
@Override
258257
public HttpHeaders asHttpHeaders() {
259-
return HttpHeaders.readOnlyHttpHeaders(delegate());
258+
return this.httpHeaders;
260259
}
261260

262261
private OptionalLong toOptionalLong(long value) {

‎spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClientBuilder.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -265,8 +265,8 @@ public WebClient build() {
265265
.map(filter -> filter.apply(exchange))
266266
.orElse(exchange) : exchange);
267267
return new DefaultWebClient(filteredExchange, initUriBuilderFactory(),
268-
this.defaultHeaders != null ? unmodifiableCopy(this.defaultHeaders) : null,
269-
this.defaultCookies != null ? unmodifiableCopy(this.defaultCookies) : null,
268+
this.defaultHeaders != null ? HttpHeaders.readOnlyHttpHeaders(this.defaultHeaders) : null,
269+
this.defaultCookies != null ? HttpHeaders.readOnlyHttpHeaders(this.defaultCookies) : null,
270270
this.defaultRequest, new DefaultWebClientBuilder(this));
271271
}
272272

@@ -308,10 +308,6 @@ private UriBuilderFactory initUriBuilderFactory() {
308308
return factory;
309309
}
310310

311-
private static HttpHeaders unmodifiableCopy(HttpHeaders headers) {
312-
return HttpHeaders.readOnlyHttpHeaders(headers);
313-
}
314-
315311
private static <K, V> MultiValueMap<K, V> unmodifiableCopy(MultiValueMap<K, V> map) {
316312
return CollectionUtils.unmodifiableMultiValueMap(new LinkedMultiValueMap<>(map));
317313
}

‎spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequest.java

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -260,61 +260,60 @@ public String toString() {
260260

261261

262262
private class DefaultHeaders implements Headers {
263-
264-
private HttpHeaders delegate() {
265-
return request().getHeaders();
266-
}
263+
264+
private final HttpHeaders httpHeaders =
265+
HttpHeaders.readOnlyHttpHeaders(request().getHeaders());
267266

268267
@Override
269268
public List<MediaType> accept() {
270-
return delegate().getAccept();
269+
return this.httpHeaders.getAccept();
271270
}
272271

273272
@Override
274273
public List<Charset> acceptCharset() {
275-
return delegate().getAcceptCharset();
274+
return this.httpHeaders.getAcceptCharset();
276275
}
277276

278277
@Override
279278
public List<Locale.LanguageRange> acceptLanguage() {
280-
return delegate().getAcceptLanguage();
279+
return this.httpHeaders.getAcceptLanguage();
281280
}
282281

283282
@Override
284283
public OptionalLong contentLength() {
285-
long value = delegate().getContentLength();
284+
long value = this.httpHeaders.getContentLength();
286285
return (value != -1 ? OptionalLong.of(value) : OptionalLong.empty());
287286
}
288287

289288
@Override
290289
public Optional<MediaType> contentType() {
291-
return Optional.ofNullable(delegate().getContentType());
290+
return Optional.ofNullable(this.httpHeaders.getContentType());
292291
}
293292

294293
@Override
295294
public InetSocketAddress host() {
296-
return delegate().getHost();
295+
return this.httpHeaders.getHost();
297296
}
298297

299298
@Override
300299
public List<HttpRange> range() {
301-
return delegate().getRange();
300+
return this.httpHeaders.getRange();
302301
}
303302

304303
@Override
305304
public List<String> header(String headerName) {
306-
List<String> headerValues = delegate().get(headerName);
305+
List<String> headerValues = this.httpHeaders.get(headerName);
307306
return (headerValues != null ? headerValues : Collections.emptyList());
308307
}
309308

310309
@Override
311310
public HttpHeaders asHttpHeaders() {
312-
return HttpHeaders.readOnlyHttpHeaders(delegate());
311+
return this.httpHeaders;
313312
}
314313

315314
@Override
316315
public String toString() {
317-
return delegate().toString();
316+
return this.httpHeaders.toString();
318317
}
319318
}
320319

‎spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseTests.java

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2020 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.
@@ -60,6 +60,8 @@ public class DefaultClientResponseTests {
6060

6161
private ClientHttpResponse mockResponse;
6262

63+
private final HttpHeaders httpHeaders = new HttpHeaders();
64+
6365
private ExchangeStrategies mockExchangeStrategies;
6466

6567
private DefaultClientResponse defaultClientResponse;
@@ -68,6 +70,7 @@ public class DefaultClientResponseTests {
6870
@BeforeEach
6971
public void createMocks() {
7072
mockResponse = mock(ClientHttpResponse.class);
73+
given(mockResponse.getHeaders()).willReturn(this.httpHeaders);
7174
mockExchangeStrategies = mock(ExchangeStrategies.class);
7275
defaultClientResponse = new DefaultClientResponse(mockResponse, mockExchangeStrategies, "", "", () -> null);
7376
}
@@ -91,7 +94,6 @@ public void rawStatusCode() {
9194

9295
@Test
9396
public void header() {
94-
HttpHeaders httpHeaders = new HttpHeaders();
9597
long contentLength = 42L;
9698
httpHeaders.setContentLength(contentLength);
9799
MediaType contentType = MediaType.TEXT_PLAIN;
@@ -233,7 +235,6 @@ public void toEntityWithUnknownStatusCode() throws Exception {
233235
= factory.wrap(ByteBuffer.wrap("foo".getBytes(StandardCharsets.UTF_8)));
234236
Flux<DataBuffer> body = Flux.just(dataBuffer);
235237

236-
HttpHeaders httpHeaders = new HttpHeaders();
237238
httpHeaders.setContentType(MediaType.TEXT_PLAIN);
238239
given(mockResponse.getHeaders()).willReturn(httpHeaders);
239240
given(mockResponse.getStatusCode()).willThrow(new IllegalArgumentException("999"));
@@ -293,13 +294,12 @@ public void toEntityList() {
293294
}
294295

295296
@Test
296-
public void toEntityListWithUnknownStatusCode() throws Exception {
297+
public void toEntityListWithUnknownStatusCode() {
297298
DefaultDataBufferFactory factory = new DefaultDataBufferFactory();
298299
DefaultDataBuffer dataBuffer =
299300
factory.wrap(ByteBuffer.wrap("foo".getBytes(StandardCharsets.UTF_8)));
300301
Flux<DataBuffer> body = Flux.just(dataBuffer);
301302

302-
HttpHeaders httpHeaders = new HttpHeaders();
303303
httpHeaders.setContentType(MediaType.TEXT_PLAIN);
304304
given(mockResponse.getHeaders()).willReturn(httpHeaders);
305305
given(mockResponse.getStatusCode()).willThrow(new IllegalArgumentException("999"));
@@ -321,8 +321,7 @@ public void toEntityListWithUnknownStatusCode() throws Exception {
321321
@Test
322322
public void toEntityListTypeReference() {
323323
DefaultDataBufferFactory factory = new DefaultDataBufferFactory();
324-
DefaultDataBuffer dataBuffer =
325-
factory.wrap(ByteBuffer.wrap("foo".getBytes(StandardCharsets.UTF_8)));
324+
DefaultDataBuffer dataBuffer = factory.wrap(ByteBuffer.wrap("foo".getBytes(StandardCharsets.UTF_8)));
326325
Flux<DataBuffer> body = Flux.just(dataBuffer);
327326

328327
mockTextPlainResponse(body);
@@ -332,8 +331,7 @@ public void toEntityListTypeReference() {
332331
given(mockExchangeStrategies.messageReaders()).willReturn(messageReaders);
333332

334333
ResponseEntity<List<String>> result = defaultClientResponse.toEntityList(
335-
new ParameterizedTypeReference<String>() {
336-
}).block();
334+
new ParameterizedTypeReference<String>() {}).block();
337335
assertThat(result.getBody()).isEqualTo(Collections.singletonList("foo"));
338336
assertThat(result.getStatusCode()).isEqualTo(HttpStatus.OK);
339337
assertThat(result.getStatusCodeValue()).isEqualTo(HttpStatus.OK.value());
@@ -342,9 +340,7 @@ public void toEntityListTypeReference() {
342340

343341

344342
private void mockTextPlainResponse(Flux<DataBuffer> body) {
345-
HttpHeaders httpHeaders = new HttpHeaders();
346343
httpHeaders.setContentType(MediaType.TEXT_PLAIN);
347-
given(mockResponse.getHeaders()).willReturn(httpHeaders);
348344
given(mockResponse.getStatusCode()).willReturn(HttpStatus.OK);
349345
given(mockResponse.getRawStatusCode()).willReturn(HttpStatus.OK.value());
350346
given(mockResponse.getBody()).willReturn(body);

0 commit comments

Comments
 (0)