fix #1505 Expose the response headers via AccessLogArgProvider (#1506)

reactor · Feb 10, 2021 · b26b42c · b26b42c
1 parent 4dace6d
commit b26b42c
Show file tree

Hide file tree

Showing 9 changed files with 132 additions and 40 deletions.
diff --git a/...ty-http/src/main/java/reactor/netty/http/server/logging/AbstractAccessLogArgProvider.java b/...ty-http/src/main/java/reactor/netty/http/server/logging/AbstractAccessLogArgProvider.java
@@ -22,7 +22,6 @@
 import java.net.SocketAddress;
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
-import java.util.Objects;
 import java.util.function.Supplier;
 
 /**
@@ -41,7 +40,6 @@ abstract class AbstractAccessLogArgProvider<SELF extends AbstractAccessLogArgPro
 	CharSequence method;
 	CharSequence uri;
 	String protocol;
-	CharSequence status;
 	boolean chunked;
 	long contentLength = -1;
 	long startTime;
@@ -86,12 +84,6 @@ public String user() {
 		return user;
 	}
 
-	@Override
-	@Nullable
-	public CharSequence status() {
-		return status;
-	}
-
 	@Override
 	public long contentLength() {
 		return contentLength;
@@ -119,17 +111,11 @@ void clear() {
 		this.method = null;
 		this.uri = null;
 		this.protocol = null;
-		this.status = null;
 		this.chunked = false;
 		this.contentLength = -1;
 		this.startTime = 0;
 	}
 
-	SELF status(CharSequence status) {
-		this.status = Objects.requireNonNull(status, "status");
-		return get();
-	}
-
 	SELF chunked(boolean chunked) {
 		this.chunked = chunked;
 		return get();

diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogArgProvider.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogArgProvider.java
@@ -98,12 +98,24 @@ public interface AccessLogArgProvider {
 	long duration();
 
 	/**
-	 * Returns the value of a request header with the specified name.
+	 * Returns the value of a request header with the specified name
+	 * or {@code null} is case such request header does not exist.
 	 *
 	 * @param name the request header name
 	 * @return the value of the request header
 	 */
 	@Nullable
 	CharSequence requestHeader(CharSequence name);
 
+	/**
+	 * Returns the value of a response header with the specified name
+	 * or {@code null} is case such response header does not exist.
+	 *
+	 * @param name the response header name
+	 * @return the value of the response header
+	 * @since 1.0.4
+	 */
+	@Nullable
+	CharSequence responseHeader(CharSequence name);
+
 }
diff --git a/...or-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogArgProviderH1.java b/...or-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogArgProviderH1.java
@@ -16,6 +16,7 @@
 package reactor.netty.http.server.logging;
 
 import io.netty.handler.codec.http.HttpRequest;
+import io.netty.handler.codec.http.HttpResponse;
 import reactor.util.annotation.Nullable;
 
 import java.net.SocketAddress;
@@ -27,6 +28,7 @@
 final class AccessLogArgProviderH1 extends AbstractAccessLogArgProvider<AccessLogArgProviderH1> {
 
 	HttpRequest request;
+	HttpResponse response;
 
 	AccessLogArgProviderH1(@Nullable SocketAddress remoteAddress) {
 		super(remoteAddress);
@@ -38,13 +40,31 @@ AccessLogArgProviderH1 request(HttpRequest request) {
 		return get();
 	}
 
+	AccessLogArgProviderH1 response(HttpResponse response) {
+		this.response = Objects.requireNonNull(response, "response");
+		return get();
+	}
+
+	@Override
+	@Nullable
+	public CharSequence status() {
+		return response == null ? null : response.status().codeAsText();
+	}
+
 	@Override
 	@Nullable
 	public CharSequence requestHeader(CharSequence name) {
 		Objects.requireNonNull(name, "name");
 		return request == null ? null : request.headers().get(name);
 	}
 
+	@Override
+	@Nullable
+	public CharSequence responseHeader(CharSequence name) {
+		Objects.requireNonNull(name, "name");
+		return response == null ? null : response.headers().get(name);
+	}
+
 	@Override
 	void onRequest() {
 		super.onRequest();
@@ -59,6 +79,7 @@ void onRequest() {
 	void clear() {
 		super.clear();
 		this.request = null;
+		this.response = null;
 	}
 
 	AccessLogArgProviderH1 contentLength(long contentLength) {

diff --git a/...or-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogArgProviderH2.java b/...or-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogArgProviderH2.java
@@ -29,6 +29,7 @@ final class AccessLogArgProviderH2 extends AbstractAccessLogArgProvider<AccessLo
 	static final String H2_PROTOCOL_NAME = "HTTP/2.0";
 
 	Http2HeadersFrame requestHeaders;
+	Http2HeadersFrame responseHeaders;
 
 	AccessLogArgProviderH2(@Nullable SocketAddress remoteAddress) {
 		super(remoteAddress);
@@ -40,13 +41,31 @@ AccessLogArgProviderH2 requestHeaders(Http2HeadersFrame requestHeaders) {
 		return get();
 	}
 
+	AccessLogArgProviderH2 responseHeaders(Http2HeadersFrame responseHeaders) {
+		this.responseHeaders = Objects.requireNonNull(responseHeaders, "responseHeaders");
+		return get();
+	}
+
+	@Override
+	@Nullable
+	public CharSequence status() {
+		return responseHeaders == null ? null : responseHeaders.headers().status();
+	}
+
 	@Override
 	@Nullable
 	public CharSequence requestHeader(CharSequence name) {
 		Objects.requireNonNull(name, "name");
 		return requestHeaders == null ? null : requestHeaders.headers().get(name);
 	}
 
+	@Override
+	@Nullable
+	public CharSequence responseHeader(CharSequence name) {
+		Objects.requireNonNull(name, "name");
+		return responseHeaders == null ? null : responseHeaders.headers().get(name);
+	}
+
 	@Override
 	void onRequest() {
 		super.onRequest();
@@ -61,6 +80,7 @@ void onRequest() {
 	void clear() {
 		super.clear();
 		this.requestHeaders = null;
+		this.responseHeaders = null;
 	}
 
 	@Override

diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogHandlerH1.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogHandlerH1.java
@@ -71,7 +71,7 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
 			}
 
 			final boolean chunked = HttpUtil.isTransferEncodingChunked(response);
-			accessLogArgProvider.status(status.codeAsText())
+			accessLogArgProvider.response(response)
 					.chunked(chunked);
 			if (!chunked) {
 				accessLogArgProvider.contentLength(HttpUtil.getContentLength(response, -1));

diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogHandlerH2.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogHandlerH2.java
@@ -19,7 +19,6 @@
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelPromise;
 import io.netty.handler.codec.http2.Http2DataFrame;
-import io.netty.handler.codec.http2.Http2Headers;
 import io.netty.handler.codec.http2.Http2HeadersFrame;
 import reactor.util.annotation.Nullable;
 
@@ -58,10 +57,9 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
 		boolean lastContent = false;
 		if (msg instanceof Http2HeadersFrame) {
 			final Http2HeadersFrame responseHeaders = (Http2HeadersFrame) msg;
-			final Http2Headers headers = responseHeaders.headers();
 			lastContent = responseHeaders.isEndStream();
 
-			accessLogArgProvider.status(headers.status())
+			accessLogArgProvider.responseHeaders(responseHeaders)
 					.chunked(true);
 		}
 		if (msg instanceof Http2DataFrame) {

diff --git a/...tty-http/src/test/java/reactor/netty/http/server/logging/AccessLogArgProviderH1Tests.java b/...tty-http/src/test/java/reactor/netty/http/server/logging/AccessLogArgProviderH1Tests.java
@@ -17,9 +17,14 @@
 
 import io.netty.handler.codec.http.DefaultHttpHeaders;
 import io.netty.handler.codec.http.DefaultHttpRequest;
+import io.netty.handler.codec.http.DefaultHttpResponse;
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpHeaderValues;
 import io.netty.handler.codec.http.HttpHeaders;
 import io.netty.handler.codec.http.HttpMethod;
 import io.netty.handler.codec.http.HttpRequest;
+import io.netty.handler.codec.http.HttpResponse;
+import io.netty.handler.codec.http.HttpResponseStatus;
 import io.netty.handler.codec.http.HttpVersion;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -39,11 +44,16 @@
 class AccessLogArgProviderH1Tests {
 
 	private static final HttpRequest request;
+	private static final HttpResponse response;
 
 	static {
-		HttpHeaders httpHeaders = new DefaultHttpHeaders();
-		httpHeaders.add(HEADER_CONNECTION_NAME, HEADER_CONNECTION_VALUE);
-		request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, URI, httpHeaders);
+		HttpHeaders requestHttpHeaders = new DefaultHttpHeaders();
+		requestHttpHeaders.add(HEADER_CONNECTION_NAME, HEADER_CONNECTION_VALUE);
+		request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, URI, requestHttpHeaders);
+
+		HttpHeaders responseHttpHeaders = new DefaultHttpHeaders();
+		responseHttpHeaders.add(HttpHeaderNames.CONTENT_TYPE, HttpHeaderValues.APPLICATION_JSON);
+		response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK, responseHttpHeaders);
 	}
 
 	private AccessLogArgProviderH1 accessLogArgProvider;
@@ -94,10 +104,14 @@ void requestHeader() {
 	@Test
 	void clear() {
 		assertThat(accessLogArgProvider.request).isNull();
+		assertThat(accessLogArgProvider.response).isNull();
 		accessLogArgProvider.request(request);
+		accessLogArgProvider.response(response);
 		assertThat(accessLogArgProvider.request).isEqualTo(request);
+		assertThat(accessLogArgProvider.response).isEqualTo(response);
 		accessLogArgProvider.clear();
 		assertThat(accessLogArgProvider.request).isNull();
+		assertThat(accessLogArgProvider.response).isNull();
 	}
 
 	@Test
@@ -112,4 +126,19 @@ void contentLength() {
 		assertThat(accessLogArgProvider.contentLength()).isEqualTo(100);
 	}
 
+	@Test
+	void status() {
+		assertThat(accessLogArgProvider.status()).isNull();
+		accessLogArgProvider.response(response);
+		assertThat(accessLogArgProvider.status()).isEqualTo(HttpResponseStatus.OK.codeAsText());
+	}
+
+	@Test
+	void responseHeader() {
+		assertThat(accessLogArgProvider.responseHeader(HttpHeaderNames.CONTENT_TYPE)).isNull();
+		accessLogArgProvider.response(response);
+		assertThat(accessLogArgProvider.responseHeader(HttpHeaderNames.CONTENT_TYPE))
+				.isEqualTo(HttpHeaderValues.APPLICATION_JSON.toString());
+	}
+
 }
diff --git a/...tty-http/src/test/java/reactor/netty/http/server/logging/AccessLogArgProviderH2Tests.java b/...tty-http/src/test/java/reactor/netty/http/server/logging/AccessLogArgProviderH2Tests.java
@@ -15,7 +15,10 @@
  */
 package reactor.netty.http.server.logging;
 
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpHeaderValues;
 import io.netty.handler.codec.http.HttpMethod;
+import io.netty.handler.codec.http.HttpResponseStatus;
 import io.netty.handler.codec.http2.DefaultHttp2Headers;
 import io.netty.handler.codec.http2.DefaultHttp2HeadersFrame;
 import io.netty.handler.codec.http2.Http2Headers;
@@ -38,13 +41,19 @@
 class AccessLogArgProviderH2Tests {
 
 	private static final Http2HeadersFrame requestHeaders;
+	private static final Http2HeadersFrame responseHeaders;
 
 	static {
-		Http2Headers httpHeaders = new DefaultHttp2Headers();
-		httpHeaders.add(HEADER_CONNECTION_NAME, HEADER_CONNECTION_VALUE);
-		httpHeaders.method(HttpMethod.GET.name());
-		httpHeaders.path(URI);
-		requestHeaders = new DefaultHttp2HeadersFrame(httpHeaders);
+		Http2Headers requestHttpHeaders = new DefaultHttp2Headers();
+		requestHttpHeaders.add(HEADER_CONNECTION_NAME, HEADER_CONNECTION_VALUE);
+		requestHttpHeaders.method(HttpMethod.GET.name());
+		requestHttpHeaders.path(URI);
+		requestHeaders = new DefaultHttp2HeadersFrame(requestHttpHeaders);
+
+		Http2Headers responseHttpHeaders = new DefaultHttp2Headers();
+		responseHttpHeaders.add(HttpHeaderNames.CONTENT_TYPE, HttpHeaderValues.APPLICATION_JSON);
+		responseHttpHeaders.status(HttpResponseStatus.OK.codeAsText());
+		responseHeaders = new DefaultHttp2HeadersFrame(responseHttpHeaders);
 	}
 
 	private AccessLogArgProviderH2 accessLogArgProvider;
@@ -94,15 +103,34 @@ void requestHeader() {
 	@Test
 	void clear() {
 		assertThat(accessLogArgProvider.requestHeaders).isNull();
+		assertThat(accessLogArgProvider.responseHeaders).isNull();
 		accessLogArgProvider.requestHeaders(requestHeaders);
+		accessLogArgProvider.responseHeaders(responseHeaders);
 		assertThat(accessLogArgProvider.requestHeaders).isEqualTo(requestHeaders);
+		assertThat(accessLogArgProvider.responseHeaders).isEqualTo(responseHeaders);
 		accessLogArgProvider.clear();
 		assertThat(accessLogArgProvider.requestHeaders).isNull();
+		assertThat(accessLogArgProvider.responseHeaders).isNull();
 	}
 
 	@Test
 	void get() {
 		assertThat(accessLogArgProvider.get()).isEqualTo(accessLogArgProvider);
 	}
 
+	@Test
+	void status() {
+		assertThat(accessLogArgProvider.status()).isNull();
+		accessLogArgProvider.responseHeaders(responseHeaders);
+		assertThat(accessLogArgProvider.status()).isEqualTo(HttpResponseStatus.OK.codeAsText());
+	}
+
+	@Test
+	void responseHeader() {
+		assertThat(accessLogArgProvider.responseHeader(HttpHeaderNames.CONTENT_TYPE)).isNull();
+		accessLogArgProvider.responseHeaders(responseHeaders);
+		assertThat(accessLogArgProvider.responseHeader(HttpHeaderNames.CONTENT_TYPE))
+				.isEqualTo(HttpHeaderValues.APPLICATION_JSON);
+	}
+
 }