Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #1505 Expose the response headers via AccessLogArgProvider #1506

Merged
merged 3 commits into from
Feb 10, 2021
Merged

fix #1505 Expose the response headers via AccessLogArgProvider #1506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4244446
Merge pull request #1 from reactor/master
Feb 10, 2021
77ea0ce
Expose the response headers via `AccessLogArgProvider`
Feb 10, 2021
febab23
Fix comments
Feb 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions ...ty-http/src/main/java/reactor/netty/http/server/logging/AbstractAccessLogArgProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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;
Expand Down Expand Up @@ -86,12 +84,6 @@ public String user() {
return user;
}

@Override
@Nullable
public CharSequence status() {
return status;
}

@Override
public long contentLength() {
return contentLength;
Expand Down Expand Up @@ -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();
Expand Down
14 changes: 13 additions & 1 deletion reactor-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogArgProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
violetagg marked this conversation as resolved.
Show resolved Hide resolved
* @param name the response header name
* @return the value of the response header
violetagg marked this conversation as resolved.
Show resolved Hide resolved
* @since 1.0.4
*/
@Nullable
CharSequence responseHeader(CharSequence name);

}
21 changes: 21 additions & 0 deletions ...or-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogArgProviderH1.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,6 +28,7 @@
final class AccessLogArgProviderH1 extends AbstractAccessLogArgProvider<AccessLogArgProviderH1> {

HttpRequest request;
HttpResponse response;

AccessLogArgProviderH1(@Nullable SocketAddress remoteAddress) {
super(remoteAddress);
Expand All @@ -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();
Expand All @@ -59,6 +79,7 @@ void onRequest() {
void clear() {
super.clear();
this.request = null;
this.response = null;
}

AccessLogArgProviderH1 contentLength(long contentLength) {
Expand Down
20 changes: 20 additions & 0 deletions ...or-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogArgProviderH2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();
Expand All @@ -61,6 +80,7 @@ void onRequest() {
void clear() {
super.clear();
this.requestHeaders = null;
this.responseHeaders = null;
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion reactor-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogHandlerH1.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
4 changes: 1 addition & 3 deletions reactor-netty-http/src/main/java/reactor/netty/http/server/logging/AccessLogHandlerH2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down
35 changes: 32 additions & 3 deletions ...tty-http/src/test/java/reactor/netty/http/server/logging/AccessLogArgProviderH1Tests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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());
}

}
38 changes: 33 additions & 5 deletions ...tty-http/src/test/java/reactor/netty/http/server/logging/AccessLogArgProviderH2Tests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

}
Loading