From bc3cf0eeb8b96a75a7a8a6351665975ed8b5081d Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 6 Jul 2018 14:29:19 -0400 Subject: [PATCH] Expose request id at the ServerHttpRequest level Hiding it (at AbstractServerHttpRequest) complicates matters since requests are often mutated and decorated, plus it's also possible to implement the interface directly (we've one, albeit corner case). Issue: SPR-16966 --- .../reactive/AbstractServerHttpRequest.java | 48 +++++++++---------- .../DefaultServerHttpRequestBuilder.java | 10 +--- .../reactive/ReactorServerHttpRequest.java | 2 +- .../server/reactive/ServerHttpRequest.java | 8 ++++ .../reactive/ServerHttpRequestDecorator.java | 5 ++ .../reactive/UndertowServerHttpRequest.java | 2 +- .../server/adapter/HttpWebHandlerAdapter.java | 9 +--- .../server/DefaultServerRequestBuilder.java | 12 ++++- 8 files changed, 50 insertions(+), 46 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java index 099c082bf6f..525a12fc660 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java @@ -64,7 +64,7 @@ public abstract class AbstractServerHttpRequest implements ServerHttpRequest { private SslInfo sslInfo; @Nullable - private String connectionId; + private String id; @Nullable private String logPrefix; @@ -83,6 +83,26 @@ public abstract class AbstractServerHttpRequest implements ServerHttpRequest { } + public String getId() { + if (this.id == null) { + this.id = initId(); + if (this.id == null) { + this.id = ObjectUtils.getIdentityHexString(this); + } + } + return this.id; + } + + /** + * Obtain the request id to use, or {@code null} in which case the Object + * identity of this request instance is used. + * @since 5.1 + */ + @Nullable + protected String initId() { + return null; + } + @Override public URI getURI() { return this.uri; @@ -186,37 +206,13 @@ public abstract class AbstractServerHttpRequest implements ServerHttpRequest { */ public abstract T getNativeRequest(); - /** - * Return an id representing the underlying connection, if available, or - * otherwise the identify of the request object. - * @since 5.1 - */ - public String getConnectionId() { - if (this.connectionId == null) { - this.connectionId = initConnectionId(); - if (this.connectionId == null) { - this.connectionId = ObjectUtils.getIdentityHexString(this); - } - } - return this.connectionId; - } - - /** - * Obtain the connection id, if available. - * @since 5.1 - */ - @Nullable - protected String initConnectionId() { - return null; - } - /** * For internal use in logging at the HTTP adapter layer. * @since 5.1 */ String getLogPrefix() { if (this.logPrefix == null) { - this.logPrefix = "[" + getConnectionId() + "] "; + this.logPrefix = "[" + getId() + "] "; } return this.logPrefix; } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java index 35aa7bf037b..66c38439db3 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java @@ -32,7 +32,6 @@ import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; /** @@ -192,8 +191,6 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder { private final ServerHttpRequest originalRequest; - private final String requestId; - public MutatedServerHttpRequest(URI uri, @Nullable String contextPath, HttpHeaders headers, String methodValue, MultiValueMap cookies, @@ -206,9 +203,6 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder { this.sslInfo = sslInfo != null ? sslInfo : originalRequest.getSslInfo(); this.body = body; this.originalRequest = originalRequest; - this.requestId = originalRequest instanceof AbstractServerHttpRequest ? - ((AbstractServerHttpRequest) originalRequest).getConnectionId() : - ObjectUtils.getIdentityHexString(originalRequest); } @Override @@ -245,8 +239,8 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder { } @Override - public String getConnectionId() { - return this.requestId; + public String getId() { + return this.originalRequest.getId(); } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java index 13ef0a4cc45..a0c343da625 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java @@ -176,7 +176,7 @@ class ReactorServerHttpRequest extends AbstractServerHttpRequest { @Override @Nullable - protected String initConnectionId() { + protected String initId() { return this.request instanceof Connection ? ((Connection) this.request).channel().id().asShortText() : null; } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequest.java index e603bff8c34..4129192da96 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequest.java @@ -38,6 +38,14 @@ import org.springframework.util.MultiValueMap; */ public interface ServerHttpRequest extends HttpRequest, ReactiveHttpInputMessage { + /** + * Return an id that represents the underlying connection, if available, or + * the request, for the purpose of correlating log messages. + * @since 5.1 + * @see org.springframework.web.server.ServerWebExchange#getLogPrefix() + */ + String getId(); + /** * Returns a structured representation of the request path including the * context path + path within application portions, path segments with diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequestDecorator.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequestDecorator.java index f1d09f59083..62418cbcc25 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequestDecorator.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequestDecorator.java @@ -55,6 +55,11 @@ public class ServerHttpRequestDecorator implements ServerHttpRequest { // ServerHttpRequest delegation methods... + @Override + public String getId() { + return getDelegate().getId(); + } + @Override @Nullable public HttpMethod getMethod() { diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java index 481c4b8c974..72be9f80a8b 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java @@ -129,7 +129,7 @@ class UndertowServerHttpRequest extends AbstractServerHttpRequest { } @Override - protected String initConnectionId() { + protected String initId() { return ObjectUtils.getIdentityHexString(this.exchange.getConnection()); } diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java index 1ab589e4fd1..f8d96b51214 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java @@ -29,13 +29,11 @@ import org.springframework.core.NestedExceptionUtils; import org.springframework.http.HttpStatus; import org.springframework.http.codec.LoggingCodecSupport; import org.springframework.http.codec.ServerCodecConfigurer; -import org.springframework.http.server.reactive.AbstractServerHttpRequest; import org.springframework.http.server.reactive.HttpHandler; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebHandler; @@ -216,7 +214,7 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa public Mono handle(ServerHttpRequest request, ServerHttpResponse response) { ServerWebExchange exchange = createExchange(request, response); - exchange.getAttributes().put(ServerWebExchange.LOG_ID_ATTRIBUTE, initLogId(request)); + exchange.getAttributes().put(ServerWebExchange.LOG_ID_ATTRIBUTE, request.getId()); logExchange(exchange); return getDelegate().handle(exchange) @@ -230,11 +228,6 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa getCodecConfigurer(), getLocaleContextResolver(), this.applicationContext); } - private String initLogId(ServerHttpRequest request) { - return request instanceof AbstractServerHttpRequest ? - ((AbstractServerHttpRequest) request).getConnectionId() : ObjectUtils.getIdentityHexString(request); - } - private void logExchange(ServerWebExchange exchange) { if (logger.isDebugEnabled()) { String logPrefix = exchange.getLogPrefix(); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilder.java index aebd82fd77c..f9454c5c78a 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilder.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilder.java @@ -178,7 +178,7 @@ class DefaultServerRequestBuilder implements ServerRequest.Builder { @Override public ServerRequest build() { - ServerHttpRequest serverHttpRequest = new BuiltServerHttpRequest( + ServerHttpRequest serverHttpRequest = new BuiltServerHttpRequest(this.exchange.getRequest().getId(), this.methodName, this.uri, this.headers, this.cookies, this.body); ServerWebExchange exchange = new DelegatingServerWebExchange( serverHttpRequest, this.exchange, this.messageReaders); @@ -190,6 +190,8 @@ class DefaultServerRequestBuilder implements ServerRequest.Builder { private static final Pattern QUERY_PATTERN = Pattern.compile("([^&=]+)(=?)([^&]+)?"); + private final String id; + private final String method; private final URI uri; @@ -204,9 +206,10 @@ class DefaultServerRequestBuilder implements ServerRequest.Builder { private final Flux body; - public BuiltServerHttpRequest(String method, URI uri, HttpHeaders headers, + public BuiltServerHttpRequest(String id, String method, URI uri, HttpHeaders headers, MultiValueMap cookies, Flux body) { + this.id = id; this.method = method; this.uri = uri; this.path = RequestPath.parse(uri, null); @@ -241,6 +244,11 @@ class DefaultServerRequestBuilder implements ServerRequest.Builder { return queryParams; } + @Override + public String getId() { + return this.id; + } + @Override public String getMethodValue() { return this.method;