Skip to content

Commit 34e8881

Browse files
mhl-bmhl-b
committed
add hasContent for stream
1 parent 86b8e28 commit 34e8881

File tree

10 files changed

+158
-26
lines changed

10 files changed

+158
-26
lines changed

‎modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpRequest.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import io.netty.handler.codec.http.HttpHeaderNames;
1414
import io.netty.handler.codec.http.HttpHeaders;
1515
import io.netty.handler.codec.http.HttpMethod;
16+
import io.netty.handler.codec.http.HttpUtil;
1617
import io.netty.handler.codec.http.QueryStringDecoder;
1718
import io.netty.handler.codec.http.cookie.Cookie;
1819
import io.netty.handler.codec.http.cookie.ServerCookieDecoder;
@@ -39,6 +40,7 @@ public class Netty4HttpRequest implements HttpRequest {
3940

4041
private final int sequence;
4142
private final io.netty.handler.codec.http.HttpRequest nettyRequest;
43+
private boolean hasContent;
4244
private HttpBody content;
4345
private final Map<String, List<String>> headers;
4446
private final AtomicBoolean released;
@@ -62,13 +64,23 @@ public Netty4HttpRequest(
6264
) {
6365
this.sequence = sequence;
6466
this.nettyRequest = nettyRequest;
67+
this.hasContent = hasContentHeader(nettyRequest);
6568
this.content = content;
6669
this.headers = getHttpHeadersAsMap(nettyRequest.headers());
6770
this.released = released;
6871
this.inboundException = inboundException;
6972
this.queryStringDecoder = new QueryStringDecoder(nettyRequest.uri());
7073
}
7174

75+
private static boolean hasContentHeader(io.netty.handler.codec.http.HttpRequest nettyRequest) {
76+
return HttpUtil.isTransferEncodingChunked(nettyRequest) || HttpUtil.getContentLength(nettyRequest, 0L) > 0;
77+
}
78+
79+
@Override
80+
public boolean hasContent() {
81+
return hasContent;
82+
}
83+
7284
@Override
7385
public RestRequest.Method method() {
7486
return translateRequestMethod(nettyRequest.method());
@@ -93,6 +105,7 @@ public HttpBody body() {
93105
@Override
94106
public void setBody(HttpBody body) {
95107
this.content = body.asFull();
108+
this.hasContent = body.isEmpty() == false;
96109
}
97110

98111
@Override

‎modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpRequestTests.java‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.http.netty4;
11+
12+
import io.netty.handler.codec.http.DefaultHttpHeaders;
13+
import io.netty.handler.codec.http.DefaultHttpRequest;
14+
import io.netty.handler.codec.http.HttpHeaderNames;
15+
import io.netty.handler.codec.http.HttpMethod;
16+
import io.netty.handler.codec.http.HttpUtil;
17+
import io.netty.handler.codec.http.HttpVersion;
18+
19+
import org.elasticsearch.common.bytes.BytesArray;
20+
import org.elasticsearch.http.HttpBody;
21+
import org.elasticsearch.test.ESTestCase;
22+
import org.elasticsearch.test.rest.FakeHttpBodyStream;
23+
24+
public class Netty4HttpRequestTests extends ESTestCase {
25+
26+
public void testEmptyFullContent() {
27+
final var request = new Netty4HttpRequest(0, new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/"), HttpBody.empty());
28+
assertFalse(request.hasContent());
29+
}
30+
31+
public void testEmptyStreamContent() {
32+
final var request = new Netty4HttpRequest(
33+
0,
34+
new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/"),
35+
new FakeHttpBodyStream()
36+
);
37+
assertFalse(request.hasContent());
38+
}
39+
40+
public void testNonEmptyFullContent() {
41+
final var len = between(1, 1024);
42+
final var request = new Netty4HttpRequest(
43+
0,
44+
new DefaultHttpRequest(
45+
HttpVersion.HTTP_1_1,
46+
HttpMethod.GET,
47+
"/",
48+
new DefaultHttpHeaders().add(HttpHeaderNames.CONTENT_LENGTH, len)
49+
),
50+
HttpBody.fromBytesReference(new BytesArray(new byte[len]))
51+
);
52+
assertTrue(request.hasContent());
53+
}
54+
55+
public void testNonEmptyStreamContent() {
56+
final var len = between(1, 1024);
57+
final var nettyRequestWithLen = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/");
58+
HttpUtil.setContentLength(nettyRequestWithLen, len);
59+
final var requestWithLen = new Netty4HttpRequest(0, nettyRequestWithLen, new FakeHttpBodyStream());
60+
assertTrue(requestWithLen.hasContent());
61+
62+
final var nettyChunkedRequest = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/", new DefaultHttpHeaders());
63+
HttpUtil.setTransferEncodingChunked(nettyChunkedRequest, true);
64+
final var chunkedRequest = new Netty4HttpRequest(0, nettyChunkedRequest, new FakeHttpBodyStream());
65+
assertTrue(chunkedRequest.hasContent());
66+
}
67+
68+
public void testReplaceContent() {
69+
final var len = between(1, 1024);
70+
final var nettyRequestWithLen = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/");
71+
HttpUtil.setContentLength(nettyRequestWithLen, len);
72+
final var streamRequest = new Netty4HttpRequest(0, nettyRequestWithLen, new FakeHttpBodyStream());
73+
74+
streamRequest.setBody(HttpBody.fromBytesReference(randomBytesReference(len)));
75+
assertTrue(streamRequest.hasContent());
76+
}
77+
}

‎server/src/main/java/org/elasticsearch/http/HttpBody.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ default boolean isFull() {
3131
return this instanceof Full;
3232
}
3333

34+
default boolean isEmpty() {
35+
if (isFull()) {
36+
return asFull().bytes().length() == 0;
37+
}
38+
return false;
39+
}
40+
3441
default boolean isStream() {
3542
return this instanceof Stream;
3643
}

‎server/src/main/java/org/elasticsearch/http/HttpRequest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ enum HttpVersion {
3838

3939
HttpRequest removeHeader(String header);
4040

41+
boolean hasContent();
42+
4143
/**
4244
* Create an http response from this request and the supplied status and content.
4345
*/

‎server/src/main/java/org/elasticsearch/rest/RestRequest.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -291,13 +291,9 @@ public final String path() {
291291
}
292292

293293
public boolean hasContent() {
294-
return isStreamedContent() || contentLength() > 0;
294+
return httpRequest.hasContent();
295295
}
296296

297-
/**
298-
* Returns content length in bytes. If length is unknown, for example,
299-
* streamed content with chunked transfer-encoding then returns -1.
300-
*/
301297
public int contentLength() {
302298
return switch (httpRequest.body()) {
303299
case HttpBody.Full content -> content.bytes().length();

‎server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -959,13 +959,12 @@ public void testStopLogsProgress() throws Exception {
959959
public void testStopWorksWithNoOpenRequests() {
960960
var grace = SHORT_GRACE_PERIOD_MS;
961961
try (var noWait = LogExpectation.unexpectedTimeout(grace); var transport = new TestHttpServerTransport(gracePeriod(grace))) {
962-
final TestHttpRequest httpRequest = new TestHttpRequest(HttpRequest.HttpVersion.HTTP_1_1, RestRequest.Method.GET, "/") {
963-
@Override
964-
public Map<String, List<String>> getHeaders() {
965-
// close connection before shutting down
966-
return Map.of(CONNECTION, List.of(CLOSE));
967-
}
968-
};
962+
final TestHttpRequest httpRequest = new TestHttpRequest(
963+
HttpRequest.HttpVersion.HTTP_1_1,
964+
RestRequest.Method.GET,
965+
"/",
966+
Map.of(CONNECTION, List.of(CLOSE))
967+
);
969968
TestHttpChannel httpChannel = new TestHttpChannel();
970969
transport.serverAcceptedChannel(httpChannel);
971970
transport.incomingRequest(httpRequest, httpChannel);

‎server/src/test/java/org/elasticsearch/http/TestHttpRequest.java‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,48 @@
2020
import java.util.Map;
2121
import java.util.function.Supplier;
2222

23-
class TestHttpRequest implements HttpRequest {
23+
public class TestHttpRequest implements HttpRequest {
2424

2525
private final Supplier<HttpVersion> version;
2626
private final RestRequest.Method method;
2727
private final String uri;
28-
private final HashMap<String, List<String>> headers = new HashMap<>();
29-
30-
TestHttpRequest(Supplier<HttpVersion> versionSupplier, RestRequest.Method method, String uri) {
28+
private final Map<String, List<String>> headers;
29+
private final HttpBody body;
30+
31+
public TestHttpRequest(
32+
Supplier<HttpVersion> versionSupplier,
33+
RestRequest.Method method,
34+
String uri,
35+
Map<String, List<String>> headers,
36+
HttpBody body
37+
) {
3138
this.version = versionSupplier;
3239
this.method = method;
3340
this.uri = uri;
41+
this.headers = headers;
42+
this.body = body;
43+
}
44+
45+
public TestHttpRequest(RestRequest.Method method, String uri, Map<String, List<String>> headers, HttpBody body) {
46+
this(() -> HttpVersion.HTTP_1_1, method, uri, headers, body);
47+
}
48+
49+
public TestHttpRequest(RestRequest.Method method, String uri, Map<String, List<String>> headers, BytesReference body) {
50+
this(() -> HttpVersion.HTTP_1_1, method, uri, headers, HttpBody.fromBytesReference(body));
51+
}
52+
53+
public TestHttpRequest(Supplier<HttpVersion> versionSupplier, RestRequest.Method method, String uri) {
54+
this(versionSupplier, method, uri, new HashMap<>(), HttpBody.empty());
3455
}
3556

36-
TestHttpRequest(HttpVersion version, RestRequest.Method method, String uri) {
57+
public TestHttpRequest(HttpVersion version, RestRequest.Method method, String uri) {
3758
this(() -> version, method, uri);
3859
}
3960

61+
public TestHttpRequest(HttpVersion version, RestRequest.Method method, String uri, Map<String, List<String>> headers) {
62+
this(() -> version, method, uri, headers, HttpBody.empty());
63+
}
64+
4065
@Override
4166
public RestRequest.Method method() {
4267
return method;
@@ -49,7 +74,7 @@ public String uri() {
4974

5075
@Override
5176
public HttpBody body() {
52-
return HttpBody.empty();
77+
return body;
5378
}
5479

5580
@Override
@@ -77,6 +102,11 @@ public HttpRequest removeHeader(String header) {
77102
throw new UnsupportedOperationException("Do not support removing header on test request.");
78103
}
79104

105+
@Override
106+
public boolean hasContent() {
107+
return body.isEmpty() == false;
108+
}
109+
80110
@Override
81111
public HttpResponse createResponse(RestStatus status, BytesReference content) {
82112
return new TestHttpResponse(status, content);

‎server/src/test/java/org/elasticsearch/rest/RestControllerTests.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,11 @@ public HttpRequest removeHeader(String header) {
908908
return this;
909909
}
910910

911+
@Override
912+
public boolean hasContent() {
913+
return hasContent;
914+
}
915+
911916
@Override
912917
public HttpResponse createResponse(RestStatus status, BytesReference content) {
913918
return null;

‎server/src/test/java/org/elasticsearch/rest/RestRequestTests.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,8 @@
1414
import org.elasticsearch.common.bytes.BytesArray;
1515
import org.elasticsearch.common.bytes.ReleasableBytesReference;
1616
import org.elasticsearch.core.CheckedConsumer;
17-
import org.elasticsearch.http.HttpBody;
1817
import org.elasticsearch.http.HttpChannel;
19-
import org.elasticsearch.http.HttpRequest;
18+
import org.elasticsearch.http.TestHttpRequest;
2019
import org.elasticsearch.test.ESTestCase;
2120
import org.elasticsearch.test.rest.FakeRestRequest;
2221
import org.elasticsearch.xcontent.NamedXContentRegistry;
@@ -41,7 +40,6 @@
4140
import static org.hamcrest.Matchers.instanceOf;
4241
import static org.hamcrest.Matchers.is;
4342
import static org.mockito.Mockito.mock;
44-
import static org.mockito.Mockito.when;
4543

4644
public class RestRequestTests extends ESTestCase {
4745

@@ -86,11 +84,11 @@ public void testContentLengthDoesNotConsumesContent() {
8684
}
8785

8886
private <T extends Exception> void runConsumesContentTest(final CheckedConsumer<RestRequest, T> consumer, final boolean expected) {
89-
final HttpRequest httpRequest = mock(HttpRequest.class);
90-
when(httpRequest.uri()).thenReturn("");
91-
when(httpRequest.body()).thenReturn(HttpBody.fromBytesReference(new BytesArray(new byte[1])));
92-
when(httpRequest.getHeaders()).thenReturn(
93-
Collections.singletonMap("Content-Type", Collections.singletonList(randomFrom("application/json", "application/x-ndjson")))
87+
final var httpRequest = new TestHttpRequest(
88+
RestRequest.Method.GET,
89+
"/",
90+
Map.of("Content-Type", List.of(randomFrom("application/json", "application/x-ndjson"))),
91+
new BytesArray(new byte[1])
9492
);
9593
final RestRequest request = RestRequest.request(XContentParserConfiguration.EMPTY, httpRequest, mock(HttpChannel.class));
9694
assertFalse(request.isContentConsumed());

‎test/framework/src/main/java/org/elasticsearch/test/rest/FakeRestRequest.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,11 @@ public HttpRequest removeHeader(String header) {
117117
return new FakeHttpRequest(method, uri, body, filteredHeaders, inboundException);
118118
}
119119

120+
@Override
121+
public boolean hasContent() {
122+
return body.isEmpty() == false;
123+
}
124+
120125
@Override
121126
public HttpResponse createResponse(RestStatus status, BytesReference unused) {
122127
Map<String, String> responseHeaders = new HashMap<>();

0 commit comments

Comments
 (0)