Skip to content

Commit

Permalink
Cache ServerHttpRequest::getMethod in AbstractServerHttpRequest
Browse files Browse the repository at this point in the history
This commit ensures that the HttpMethod, exposed through
ServerHttpRequest::getMethod, is cached in AbstractServerHttpRequest so
that potentially expensive HTTP method lookups are only done once.

Closes gh-30139
  • Loading branch information
poutsma committed Mar 21, 2023
1 parent c27a568 commit 37a4e84
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 62 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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 @@ -54,8 +54,6 @@
*/
public final class MockServerHttpRequest extends AbstractServerHttpRequest {

private final HttpMethod httpMethod;

private final MultiValueMap<String, HttpCookie> cookies;

@Nullable
Expand All @@ -74,8 +72,7 @@ private MockServerHttpRequest(HttpMethod httpMethod,
@Nullable InetSocketAddress localAddress, @Nullable InetSocketAddress remoteAddress,
@Nullable SslInfo sslInfo, Publisher<? extends DataBuffer> body) {

super(uri, contextPath, headers);
this.httpMethod = httpMethod;
super(httpMethod, uri, contextPath, headers);
this.cookies = cookies;
this.localAddress = localAddress;
this.remoteAddress = remoteAddress;
Expand All @@ -84,11 +81,6 @@ private MockServerHttpRequest(HttpMethod httpMethod,
}


@Override
public HttpMethod getMethod() {
return this.httpMethod;
}

@Override
@Nullable
public InetSocketAddress getLocalAddress() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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 @@ -24,8 +24,10 @@

import org.springframework.http.HttpCookie;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.server.RequestPath;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.CollectionUtils;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
Expand All @@ -49,6 +51,10 @@ public abstract class AbstractServerHttpRequest implements ServerHttpRequest {

private final HttpHeaders headers;

// TODO: remove @Nullable once deprecated constructors have been removed
@Nullable
private final HttpMethod method;

@Nullable
private MultiValueMap<String, String> queryParams;

Expand All @@ -71,23 +77,50 @@ public abstract class AbstractServerHttpRequest implements ServerHttpRequest {
* @param contextPath the context path for the request
* @param headers the headers for the request (as {@link MultiValueMap})
* @since 5.3
* @deprecated since 6.0.8, in favor of {@link #AbstractServerHttpRequest(HttpMethod, URI, String, MultiValueMap)}
*/
@Deprecated(since = "6.0.8", forRemoval = true)
public AbstractServerHttpRequest(URI uri, @Nullable String contextPath, MultiValueMap<String, String> headers) {
this.uri = uri;
this.path = RequestPath.parse(uri, contextPath);
this.headers = HttpHeaders.readOnlyHttpHeaders(headers);
this.method = null;
}

/**
* Constructor with the method, URI and headers for the request.
* @param method the HTTP method for the request
* @param uri the URI for the request
* @param contextPath the context path for the request
* @param headers the headers for the request (as {@link MultiValueMap})
* @since 6.0.8
*/
public AbstractServerHttpRequest(HttpMethod method, URI uri, @Nullable String contextPath,
MultiValueMap<String, String> headers) {

Assert.notNull(method, "Method must not be null");
Assert.notNull(uri, "Uri must not be null");
Assert.notNull(headers, "Headers must not be null");

this.method = method;
this.uri = uri;
this.path = RequestPath.parse(uri, contextPath);
this.headers = HttpHeaders.readOnlyHttpHeaders(headers);
}

/**
* Constructor with the URI and headers for the request.
* @param uri the URI for the request
* @param contextPath the context path for the request
* @param headers the headers for the request (as {@link HttpHeaders})
* @deprecated since 6.0.8, in favor of {@link #AbstractServerHttpRequest(HttpMethod, URI, String, MultiValueMap)}
*/
@Deprecated(since = "6.0.8", forRemoval = true)
public AbstractServerHttpRequest(URI uri, @Nullable String contextPath, HttpHeaders headers) {
this.uri = uri;
this.path = RequestPath.parse(uri, contextPath);
this.headers = HttpHeaders.readOnlyHttpHeaders(headers);
this.method = null;
}


Expand All @@ -112,6 +145,18 @@ protected String initId() {
return null;
}

@Override
public HttpMethod getMethod() {
// TODO: remove null check once deprecated constructors have been removed
if (this.method != null) {
return this.method;
}
else {
throw new IllegalStateException("No HttpMethod provided in constructor, " +
"and AbstractServerHttpRequest::getMethod not overridden");
}
}

@Override
public URI getURI() {
return this.uri;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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 @@ -177,8 +177,6 @@ private URI getUriToUse() {

private static class MutatedServerHttpRequest extends AbstractServerHttpRequest {

private final HttpMethod method;

@Nullable
private final SslInfo sslInfo;

Expand All @@ -194,19 +192,13 @@ public MutatedServerHttpRequest(URI uri, @Nullable String contextPath,
HttpMethod method, @Nullable SslInfo sslInfo, @Nullable InetSocketAddress remoteAddress,
HttpHeaders headers, Flux<DataBuffer> body, ServerHttpRequest originalRequest) {

super(uri, contextPath, headers);
this.method = method;
super(method, uri, contextPath, headers);
this.remoteAddress = (remoteAddress != null ? remoteAddress : originalRequest.getRemoteAddress());
this.sslInfo = (sslInfo != null ? sslInfo : originalRequest.getSslInfo());
this.body = body;
this.originalRequest = originalRequest;
}

@Override
public HttpMethod getMethod() {
return this.method;
}

@Override
protected MultiValueMap<String, HttpCookie> initCookies() {
return this.originalRequest.getCookies();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,15 @@ class ReactorNetty2ServerHttpRequest extends AbstractServerHttpRequest {

private final Netty5DataBufferFactory bufferFactory;

private final HttpMethod method;


public ReactorNetty2ServerHttpRequest(HttpServerRequest request, Netty5DataBufferFactory bufferFactory)
throws URISyntaxException {

super(initUri(request), "", new Netty5HeadersAdapter(request.requestHeaders()));
super(HttpMethod.valueOf(request.method().name()), initUri(request), "",
new Netty5HeadersAdapter(request.requestHeaders()));
Assert.notNull(bufferFactory, "DataBufferFactory must not be null");
this.request = request;
this.bufferFactory = bufferFactory;
this.method = HttpMethod.valueOf(request.method().name());
}

private static URI initUri(HttpServerRequest request) throws URISyntaxException {
Expand Down Expand Up @@ -142,11 +140,6 @@ private static String resolveRequestUri(HttpServerRequest request) {
return uri;
}

@Override
public HttpMethod getMethod() {
return this.method;
}

@Override
protected MultiValueMap<String, HttpCookie> initCookies() {
MultiValueMap<String, HttpCookie> cookies = new LinkedMultiValueMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,15 @@ class ReactorServerHttpRequest extends AbstractServerHttpRequest {

private final NettyDataBufferFactory bufferFactory;

private final HttpMethod method;


public ReactorServerHttpRequest(HttpServerRequest request, NettyDataBufferFactory bufferFactory)
throws URISyntaxException {

super(initUri(request), "", new NettyHeadersAdapter(request.requestHeaders()));
super(HttpMethod.valueOf(request.method().name()), initUri(request), "",
new NettyHeadersAdapter(request.requestHeaders()));
Assert.notNull(bufferFactory, "DataBufferFactory must not be null");
this.request = request;
this.bufferFactory = bufferFactory;
this.method = HttpMethod.valueOf(request.method().name());
}

private static URI initUri(HttpServerRequest request) throws URISyntaxException {
Expand Down Expand Up @@ -112,10 +110,6 @@ private static String resolveRequestUri(HttpServerRequest request) {
return uri;
}

@Override
public HttpMethod getMethod() {
return this.method;
}

@Override
protected MultiValueMap<String, HttpCookie> initCookies() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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 @@ -90,7 +90,8 @@ public ServletServerHttpRequest(MultiValueMap<String, String> headers, HttpServl
AsyncContext asyncContext, String servletPath, DataBufferFactory bufferFactory, int bufferSize)
throws IOException, URISyntaxException {

super(initUri(request), request.getContextPath() + servletPath, initHeaders(headers, request));
super(HttpMethod.valueOf(request.getMethod()), initUri(request), request.getContextPath() + servletPath,
initHeaders(headers, request));

Assert.notNull(bufferFactory, "'bufferFactory' must not be null");
Assert.isTrue(bufferSize > 0, "'bufferSize' must be greater than 0");
Expand Down Expand Up @@ -162,11 +163,6 @@ private static MultiValueMap<String, String> initHeaders(
return (headers != null ? headers : headerValues);
}

@Override
public HttpMethod getMethod() {
return HttpMethod.valueOf(this.request.getMethod());
}

@Override
protected MultiValueMap<String, HttpCookie> initCookies() {
MultiValueMap<String, HttpCookie> httpCookies = new LinkedMultiValueMap<>();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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 @@ -64,7 +64,8 @@ class UndertowServerHttpRequest extends AbstractServerHttpRequest {
public UndertowServerHttpRequest(HttpServerExchange exchange, DataBufferFactory bufferFactory)
throws URISyntaxException {

super(initUri(exchange), "", new UndertowHeadersAdapter(exchange.getRequestHeaders()));
super(HttpMethod.valueOf(exchange.getRequestMethod().toString()), initUri(exchange), "",
new UndertowHeadersAdapter(exchange.getRequestHeaders()));
this.exchange = exchange;
this.body = new RequestBodyPublisher(exchange, bufferFactory);
this.body.registerListeners(exchange);
Expand All @@ -78,11 +79,6 @@ private static URI initUri(HttpServerExchange exchange) throws URISyntaxExceptio
return new URI(requestUriAndQuery);
}

@Override
public HttpMethod getMethod() {
return HttpMethod.valueOf(this.exchange.getRequestMethod().toString());
}

@Override
protected MultiValueMap<String, HttpCookie> initCookies() {
MultiValueMap<String, HttpCookie> cookies = new LinkedMultiValueMap<>();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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 @@ -55,8 +55,6 @@
*/
public final class MockServerHttpRequest extends AbstractServerHttpRequest {

private final HttpMethod httpMethod;

private final MultiValueMap<String, HttpCookie> cookies;

@Nullable
Expand All @@ -75,8 +73,7 @@ private MockServerHttpRequest(HttpMethod httpMethod,
@Nullable InetSocketAddress localAddress, @Nullable InetSocketAddress remoteAddress,
@Nullable SslInfo sslInfo, Publisher<? extends DataBuffer> body) {

super(uri, contextPath, headers);
this.httpMethod = httpMethod;
super(httpMethod, uri, contextPath, headers);
this.cookies = cookies;
this.localAddress = localAddress;
this.remoteAddress = remoteAddress;
Expand All @@ -85,11 +82,6 @@ private MockServerHttpRequest(HttpMethod httpMethod,
}


@Override
public HttpMethod getMethod() {
return this.httpMethod;
}

@Override
@Nullable
public InetSocketAddress getLocalAddress() {
Expand Down

0 comments on commit 37a4e84

Please sign in to comment.