Skip to content

Commit

Permalink
Avoided repeated creation of ReadOnlyHttpHeaders wrapper
Browse files Browse the repository at this point in the history
  • Loading branch information
rstoyanchev authored and kenny5he committed Jun 21, 2020
1 parent 86acc94 commit 1b26b42
Show file tree
Hide file tree
Showing 10 changed files with 116 additions and 77 deletions.
18 changes: 8 additions & 10 deletions spring-web/src/main/java/org/springframework/http/HttpHeaders.java
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
* An empty {@code HttpHeaders} instance (immutable).
* @since 5.0
*/
public static final HttpHeaders EMPTY = new ReadOnlyHttpHeaders(new LinkedMultiValueMap<>());
public static final HttpHeaders EMPTY = new ReadOnlyHttpHeaders(new HttpHeaders(new LinkedMultiValueMap<>(0)));

/**
* Pattern matching ETag multiple field values in headers such as "If-Match", "If-None-Match".
Expand Down Expand Up @@ -1769,21 +1769,19 @@ public String toString() {


/**
* Apply a read-only {@code HttpHeaders} wrapper around the given headers,
* if necessary.
* @param headers the headers to expose
* @return a read-only variant of the headers, or the original headers as-is
* Apply a read-only {@code HttpHeaders} wrapper around the given headers
* that also caches the parsed representations of the "Accept" and
* "Content-Type" headers.
*/
public static HttpHeaders readOnlyHttpHeaders(HttpHeaders headers) {
public static HttpHeaders readOnlyHttpHeaders(MultiValueMap<String, String> headers) {
Assert.notNull(headers, "HttpHeaders must not be null");
return (headers instanceof ReadOnlyHttpHeaders ? headers : new ReadOnlyHttpHeaders(headers.headers));
return (headers instanceof ReadOnlyHttpHeaders ?
(HttpHeaders) headers : new ReadOnlyHttpHeaders(headers));
}

/**
* Remove any read-only wrapper that may have been previously applied around
* the given headers via {@link #readOnlyHttpHeaders(HttpHeaders)}.
* @param headers the headers to expose
* @return a writable variant of the headers, or the original headers as-is
* the given headers via {@link #readOnlyHttpHeaders(MultiValueMap)}.
* @since 5.1.1
*/
public static HttpHeaders writableHttpHeaders(HttpHeaders headers) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2014 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.
Expand All @@ -20,6 +20,7 @@
import java.io.OutputStream;

import org.springframework.http.HttpHeaders;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

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

private boolean executed = false;

@Nullable
private HttpHeaders readOnlyHeaders;


@Override
public final HttpHeaders getHeaders() {
return (this.executed ? HttpHeaders.readOnlyHttpHeaders(this.headers) : this.headers);
if (this.readOnlyHeaders != null) {
return this.readOnlyHeaders;
}
else if (this.executed) {
this.readOnlyHeaders = HttpHeaders.readOnlyHttpHeaders(this.headers);
return this.readOnlyHeaders;
}
else {
return this.headers;
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -60,6 +60,9 @@ private enum State {NEW, COMMITTING, COMMITTED}

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

@Nullable
private HttpHeaders readOnlyHeaders;


public AbstractClientHttpRequest() {
this(new HttpHeaders());
Expand All @@ -74,10 +77,16 @@ public AbstractClientHttpRequest(HttpHeaders headers) {

@Override
public HttpHeaders getHeaders() {
if (State.COMMITTED.equals(this.state.get())) {
return HttpHeaders.readOnlyHttpHeaders(this.headers);
if (this.readOnlyHeaders != null) {
return this.readOnlyHeaders;
}
else if (State.COMMITTED.equals(this.state.get())) {
this.readOnlyHeaders = HttpHeaders.readOnlyHttpHeaders(this.headers);
return this.readOnlyHeaders;
}
else {
return this.headers;
}
return this.headers;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -47,6 +47,9 @@ public class ServletServerHttpResponse implements ServerHttpResponse {

private boolean bodyUsed = false;

@Nullable
private HttpHeaders readOnlyHeaders;


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

@Override
public HttpHeaders getHeaders() {
return (this.headersWritten ? HttpHeaders.readOnlyHttpHeaders(this.headers) : this.headers);
if (this.readOnlyHeaders != null) {
return this.readOnlyHeaders;
}
else if (this.headersWritten) {
this.readOnlyHeaders = HttpHeaders.readOnlyHttpHeaders(this.headers);
return this.readOnlyHeaders;
}
else {
return this.headers;
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ private enum State {NEW, COMMITTING, COMMITTED}

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

@Nullable
private HttpHeaders readOnlyHeaders;


public AbstractServerHttpResponse(DataBufferFactory dataBufferFactory) {
this(dataBufferFactory, new HttpHeaders());
Expand Down Expand Up @@ -155,8 +158,16 @@ public Integer getStatusCodeValue() {

@Override
public HttpHeaders getHeaders() {
return (this.state.get() == State.COMMITTED ?
HttpHeaders.readOnlyHttpHeaders(this.headers) : this.headers);
if (this.readOnlyHeaders != null) {
return this.readOnlyHeaders;
}
else if (this.state.get() == State.COMMITTED) {
this.readOnlyHeaders = HttpHeaders.readOnlyHttpHeaders(this.headers);
return this.readOnlyHeaders;
}
else {
return this.headers;
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,29 +234,28 @@ HttpRequest request() {

private class DefaultHeaders implements Headers {

private HttpHeaders delegate() {
return response.getHeaders();
}
private final HttpHeaders httpHeaders =
HttpHeaders.readOnlyHttpHeaders(response.getHeaders());

@Override
public OptionalLong contentLength() {
return toOptionalLong(delegate().getContentLength());
return toOptionalLong(this.httpHeaders.getContentLength());
}

@Override
public Optional<MediaType> contentType() {
return Optional.ofNullable(delegate().getContentType());
return Optional.ofNullable(this.httpHeaders.getContentType());
}

@Override
public List<String> header(String headerName) {
List<String> headerValues = delegate().get(headerName);
List<String> headerValues = this.httpHeaders.get(headerName);
return (headerValues != null ? headerValues : Collections.emptyList());
}

@Override
public HttpHeaders asHttpHeaders() {
return HttpHeaders.readOnlyHttpHeaders(delegate());
return this.httpHeaders;
}

private OptionalLong toOptionalLong(long value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,24 +106,20 @@ public DefaultWebClientBuilder(DefaultWebClientBuilder other) {
this.defaultUriVariables = (other.defaultUriVariables != null ?
new LinkedHashMap<>(other.defaultUriVariables) : null);
this.uriBuilderFactory = other.uriBuilderFactory;

if (other.defaultHeaders != null) {
this.defaultHeaders = new HttpHeaders();
this.defaultHeaders.putAll(other.defaultHeaders);
}
else {
this.defaultHeaders = null;
}

this.defaultCookies = (other.defaultCookies != null ?
new LinkedMultiValueMap<>(other.defaultCookies) : null);
this.defaultRequest = other.defaultRequest;
this.filters = (other.filters != null ? new ArrayList<>(other.filters) : null);

this.filters = other.filters != null ? new ArrayList<>(other.filters) : null;
this.connector = other.connector;
this.strategies = other.strategies;
this.strategiesConfigurers = (other.strategiesConfigurers != null ?
new ArrayList<>(other.strategiesConfigurers) : null);
this.strategiesConfigurers = other.strategiesConfigurers != null ? new ArrayList<>(other.strategiesConfigurers) : null;
this.exchangeFunction = other.exchangeFunction;
}

Expand Down Expand Up @@ -232,8 +228,8 @@ public WebClient.Builder exchangeStrategies(ExchangeStrategies strategies) {
return this;
}

@SuppressWarnings("deprecation")
@Override
@Deprecated
public WebClient.Builder exchangeStrategies(Consumer<ExchangeStrategies.Builder> configurer) {
if (this.strategiesConfigurers == null) {
this.strategiesConfigurers = new ArrayList<>(4);
Expand Down Expand Up @@ -270,7 +266,7 @@ public WebClient build() {
.orElse(exchange) : exchange);
return new DefaultWebClient(filteredExchange, initUriBuilderFactory(),
this.defaultHeaders != null ? HttpHeaders.readOnlyHttpHeaders(this.defaultHeaders) : null,
this.defaultCookies != null ? CollectionUtils.unmodifiableMultiValueMap(this.defaultCookies) : null,
this.defaultCookies != null ? HttpHeaders.readOnlyHttpHeaders(this.defaultCookies) : null,
this.defaultRequest, new DefaultWebClientBuilder(this));
}

Expand All @@ -292,10 +288,12 @@ else if (httpComponentsClientPresent) {

private ExchangeStrategies initExchangeStrategies() {
if (CollectionUtils.isEmpty(this.strategiesConfigurers)) {
return (this.strategies != null ? this.strategies : ExchangeStrategies.withDefaults());
return this.strategies != null ? this.strategies : ExchangeStrategies.withDefaults();
}

ExchangeStrategies.Builder builder =
(this.strategies != null ? this.strategies.mutate() : ExchangeStrategies.builder());
this.strategies != null ? this.strategies.mutate() : ExchangeStrategies.builder();

this.strategiesConfigurers.forEach(configurer -> configurer.accept(builder));
return builder.build();
}
Expand All @@ -304,10 +302,14 @@ private UriBuilderFactory initUriBuilderFactory() {
if (this.uriBuilderFactory != null) {
return this.uriBuilderFactory;
}
DefaultUriBuilderFactory factory = (this.baseUrl != null ?
new DefaultUriBuilderFactory(this.baseUrl) : new DefaultUriBuilderFactory());
DefaultUriBuilderFactory factory = this.baseUrl != null ?
new DefaultUriBuilderFactory(this.baseUrl) : new DefaultUriBuilderFactory();
factory.setDefaultUriVariables(this.defaultUriVariables);
return factory;
}

private static <K, V> MultiValueMap<K, V> unmodifiableCopy(MultiValueMap<K, V> map) {
return CollectionUtils.unmodifiableMultiValueMap(new LinkedMultiValueMap<>(map));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -260,61 +260,60 @@ public String toString() {


private class DefaultHeaders implements Headers {

private HttpHeaders delegate() {
return request().getHeaders();
}

private final HttpHeaders httpHeaders =
HttpHeaders.readOnlyHttpHeaders(request().getHeaders());

@Override
public List<MediaType> accept() {
return delegate().getAccept();
return this.httpHeaders.getAccept();
}

@Override
public List<Charset> acceptCharset() {
return delegate().getAcceptCharset();
return this.httpHeaders.getAcceptCharset();
}

@Override
public List<Locale.LanguageRange> acceptLanguage() {
return delegate().getAcceptLanguage();
return this.httpHeaders.getAcceptLanguage();
}

@Override
public OptionalLong contentLength() {
long value = delegate().getContentLength();
long value = this.httpHeaders.getContentLength();
return (value != -1 ? OptionalLong.of(value) : OptionalLong.empty());
}

@Override
public Optional<MediaType> contentType() {
return Optional.ofNullable(delegate().getContentType());
return Optional.ofNullable(this.httpHeaders.getContentType());
}

@Override
public InetSocketAddress host() {
return delegate().getHost();
return this.httpHeaders.getHost();
}

@Override
public List<HttpRange> range() {
return delegate().getRange();
return this.httpHeaders.getRange();
}

@Override
public List<String> header(String headerName) {
List<String> headerValues = delegate().get(headerName);
List<String> headerValues = this.httpHeaders.get(headerName);
return (headerValues != null ? headerValues : Collections.emptyList());
}

@Override
public HttpHeaders asHttpHeaders() {
return HttpHeaders.readOnlyHttpHeaders(delegate());
return this.httpHeaders;
}

@Override
public String toString() {
return delegate().toString();
return this.httpHeaders.toString();
}
}

Expand Down
Loading

0 comments on commit 1b26b42

Please sign in to comment.