Skip to content

Commit af1ecd3

Browse files
fdesufdesu
authored
Cleanup HttpServerTransport.Dispatcher in Netty tests (opensearch-project#20160)
* Cleanup HttpServerTransport.Dispatcher in Netty tests Signed-off-by: Sergei Ustimenko <fdesu@proton.me> * Update CHANGELOG Signed-off-by: Sergei Ustimenko <fdesu@proton.me> * Fix potential issue with builder reuse Signed-off-by: Sergei Ustimenko <fdesu@proton.me> * Apply suggestions Signed-off-by: Sergei Ustimenko <fdesu@proton.me> * Move TestDispatcherBuilder to test/framework Signed-off-by: Sergei Ustimenko <fdesu@proton.me> * Fix javadocs Signed-off-by: Sergei Ustimenko <fdesu@proton.me> * Run CI Signed-off-by: Sergei Ustimenko <fdesu@proton.me> * Rerun CI because of a flaky test Signed-off-by: Sergei Ustimenko <fdesu@proton.me> * Rerun CI as Jenkins didn't start the build job Signed-off-by: Sergei Ustimenko <fdesu@proton.me> * Rerun CI as the InternalDistributionBwcSetupPluginFuncTest failed Signed-off-by: Sergei Ustimenko <fdesu@proton.me> --------- Signed-off-by: Sergei Ustimenko <fdesu@proton.me>
1 parent 799fb9b commit af1ecd3

File tree

10 files changed

+291
-522
lines changed

10 files changed

+291
-522
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
1212
- Handle custom metadata files in subdirectory-store ([#20157](https://github.com/opensearch-project/OpenSearch/pull/20157))
1313
- Add support for missing proto fields in GRPC FunctionScore and Highlight ([#20169](https://github.com/opensearch-project/OpenSearch/pull/20169))
1414
- Ensure all modules are included in INTEG_TEST testcluster distribution ([#20241](https://github.com/opensearch-project/OpenSearch/pull/20241))
15+
- Cleanup HttpServerTransport.Dispatcher in Netty tests ([#20160](https://github.com/opensearch-project/OpenSearch/pull/20160))
1516

1617
### Fixed
1718
- Fix bug of warm index: FullFileCachedIndexInput was closed error ([#20055](https://github.com/opensearch-project/OpenSearch/pull/20055))

‎modules/transport-netty4/src/test/java/org/opensearch/http/netty4/Netty4BadRequestTests.java‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,12 @@
3838
import org.opensearch.common.settings.Settings;
3939
import org.opensearch.common.util.MockBigArrays;
4040
import org.opensearch.common.util.MockPageCacheRecycler;
41-
import org.opensearch.common.util.concurrent.ThreadContext;
4241
import org.opensearch.core.common.transport.TransportAddress;
4342
import org.opensearch.core.indices.breaker.NoneCircuitBreakerService;
4443
import org.opensearch.core.rest.RestStatus;
4544
import org.opensearch.http.HttpServerTransport;
4645
import org.opensearch.http.HttpTransportSettings;
4746
import org.opensearch.rest.BytesRestResponse;
48-
import org.opensearch.rest.RestChannel;
49-
import org.opensearch.rest.RestRequest;
5047
import org.opensearch.telemetry.tracing.noop.NoopTracer;
5148
import org.opensearch.test.OpenSearchTestCase;
5249
import org.opensearch.threadpool.TestThreadPool;
@@ -63,6 +60,7 @@
6360
import io.netty.handler.codec.http.FullHttpResponse;
6461
import io.netty.util.ReferenceCounted;
6562

63+
import static org.opensearch.http.TestDispatcherBuilder.dispatcherBuilderWithDefaults;
6664
import static org.hamcrest.Matchers.containsString;
6765
import static org.hamcrest.Matchers.equalTo;
6866
import static org.hamcrest.Matchers.hasSize;
@@ -74,35 +72,28 @@ public class Netty4BadRequestTests extends OpenSearchTestCase {
7472
private ThreadPool threadPool;
7573

7674
@Before
77-
public void setup() throws Exception {
75+
public void setup() {
7876
networkService = new NetworkService(Collections.emptyList());
7977
bigArrays = new MockBigArrays(new MockPageCacheRecycler(Settings.EMPTY), new NoneCircuitBreakerService());
8078
threadPool = new TestThreadPool("test");
8179
}
8280

8381
@After
84-
public void shutdown() throws Exception {
82+
public void shutdown() {
8583
terminate(threadPool);
8684
}
8785

8886
public void testBadParameterEncoding() throws Exception {
89-
final HttpServerTransport.Dispatcher dispatcher = new HttpServerTransport.Dispatcher() {
90-
@Override
91-
public void dispatchRequest(RestRequest request, RestChannel channel, ThreadContext threadContext) {
92-
fail();
87+
HttpServerTransport.Dispatcher dispatcher = dispatcherBuilderWithDefaults().withDispatchRequest(
88+
(request, channel, threadContext) -> fail()
89+
).withDispatchBadRequest((channel, threadContext, cause) -> {
90+
try {
91+
final Exception e = cause instanceof Exception ? (Exception) cause : new OpenSearchException(cause);
92+
channel.sendResponse(new BytesRestResponse(channel, RestStatus.BAD_REQUEST, e));
93+
} catch (final IOException e) {
94+
throw new UncheckedIOException(e);
9395
}
94-
95-
@Override
96-
public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, Throwable cause) {
97-
try {
98-
final Exception e = cause instanceof Exception ? (Exception) cause : new OpenSearchException(cause);
99-
channel.sendResponse(new BytesRestResponse(channel, RestStatus.BAD_REQUEST, e));
100-
} catch (final IOException e) {
101-
throw new UncheckedIOException(e);
102-
}
103-
}
104-
};
105-
96+
}).build();
10697
Settings settings = Settings.builder().put(HttpTransportSettings.SETTING_HTTP_PORT.getKey(), getPortRange()).build();
10798
try (
10899
HttpServerTransport httpServerTransport = new Netty4HttpServerTransport(

‎modules/transport-netty4/src/test/java/org/opensearch/http/netty4/Netty4HttpServerTransportTests.java‎

Lines changed: 16 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232

3333
package org.opensearch.http.netty4;
3434

35-
import org.apache.logging.log4j.message.ParameterizedMessage;
3635
import org.opensearch.OpenSearchException;
3736
import org.opensearch.common.network.NetworkAddress;
3837
import org.opensearch.common.network.NetworkService;
@@ -42,7 +41,6 @@
4241
import org.opensearch.common.unit.TimeValue;
4342
import org.opensearch.common.util.MockBigArrays;
4443
import org.opensearch.common.util.MockPageCacheRecycler;
45-
import org.opensearch.common.util.concurrent.ThreadContext;
4644
import org.opensearch.core.common.bytes.BytesArray;
4745
import org.opensearch.core.common.transport.TransportAddress;
4846
import org.opensearch.core.common.unit.ByteSizeValue;
@@ -53,11 +51,8 @@
5351
import org.opensearch.http.HttpTransportSettings;
5452
import org.opensearch.http.NullDispatcher;
5553
import org.opensearch.rest.BytesRestResponse;
56-
import org.opensearch.rest.RestChannel;
57-
import org.opensearch.rest.RestRequest;
5854
import org.opensearch.telemetry.tracing.noop.NoopTracer;
5955
import org.opensearch.test.OpenSearchTestCase;
60-
import org.opensearch.test.rest.FakeRestRequest;
6156
import org.opensearch.threadpool.TestThreadPool;
6257
import org.opensearch.threadpool.ThreadPool;
6358
import org.opensearch.transport.NettyAllocator;
@@ -102,6 +97,7 @@
10297
import static org.opensearch.core.rest.RestStatus.OK;
10398
import static org.opensearch.http.HttpTransportSettings.SETTING_CORS_ALLOW_ORIGIN;
10499
import static org.opensearch.http.HttpTransportSettings.SETTING_CORS_ENABLED;
100+
import static org.opensearch.http.TestDispatcherBuilder.dispatcherBuilderWithDefaults;
105101
import static org.hamcrest.Matchers.containsString;
106102
import static org.hamcrest.Matchers.equalTo;
107103
import static org.hamcrest.Matchers.instanceOf;
@@ -118,15 +114,15 @@ public class Netty4HttpServerTransportTests extends OpenSearchTestCase {
118114
private ClusterSettings clusterSettings;
119115

120116
@Before
121-
public void setup() throws Exception {
117+
public void setup() {
122118
networkService = new NetworkService(Collections.emptyList());
123119
threadPool = new TestThreadPool("test");
124120
bigArrays = new MockBigArrays(new MockPageCacheRecycler(Settings.EMPTY), new NoneCircuitBreakerService());
125121
clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
126122
}
127123

128124
@After
129-
public void shutdown() throws Exception {
125+
public void shutdown() {
130126
if (threadPool != null) {
131127
threadPool.shutdownNow();
132128
}
@@ -175,21 +171,11 @@ private void runExpectHeaderTest(
175171
final int contentLength,
176172
final HttpResponseStatus expectedStatus
177173
) throws InterruptedException {
178-
final HttpServerTransport.Dispatcher dispatcher = new HttpServerTransport.Dispatcher() {
179-
@Override
180-
public void dispatchRequest(RestRequest request, RestChannel channel, ThreadContext threadContext) {
181-
channel.sendResponse(new BytesRestResponse(OK, BytesRestResponse.TEXT_CONTENT_TYPE, new BytesArray("done")));
182-
}
183-
184-
@Override
185-
public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, Throwable cause) {
186-
logger.error(
187-
new ParameterizedMessage("--> Unexpected bad request [{}]", FakeRestRequest.requestToString(channel.request())),
188-
cause
189-
);
190-
throw new AssertionError();
191-
}
192-
};
174+
HttpServerTransport.Dispatcher dispatcher = dispatcherBuilderWithDefaults().withDispatchRequest(
175+
(request, channel, threadContext) -> channel.sendResponse(
176+
new BytesRestResponse(OK, BytesRestResponse.TEXT_CONTENT_TYPE, new BytesArray("done"))
177+
)
178+
).build();
193179
try (
194180
Netty4HttpServerTransport transport = new Netty4HttpServerTransport(
195181
settings,
@@ -280,16 +266,8 @@ public void testBindUnavailableAddress() {
280266

281267
public void testBadRequest() throws InterruptedException {
282268
final AtomicReference<Throwable> causeReference = new AtomicReference<>();
283-
final HttpServerTransport.Dispatcher dispatcher = new HttpServerTransport.Dispatcher() {
284-
285-
@Override
286-
public void dispatchRequest(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) {
287-
logger.error("--> Unexpected successful request [{}]", FakeRestRequest.requestToString(request));
288-
throw new AssertionError();
289-
}
290-
291-
@Override
292-
public void dispatchBadRequest(final RestChannel channel, final ThreadContext threadContext, final Throwable cause) {
269+
final HttpServerTransport.Dispatcher dispatcher = dispatcherBuilderWithDefaults().withDispatchBadRequest(
270+
(channel, threadContext, cause) -> {
293271
causeReference.set(cause);
294272
try {
295273
final OpenSearchException e = new OpenSearchException("you sent a bad request and you should feel bad");
@@ -298,9 +276,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th
298276
throw new AssertionError(e);
299277
}
300278
}
301-
302-
};
303-
279+
).build();
304280
final Settings settings;
305281
final int maxInitialLineLength;
306282
final Setting<ByteSizeValue> httpMaxInitialLineLengthSetting = HttpTransportSettings.SETTING_HTTP_MAX_INITIAL_LINE_LENGTH;
@@ -352,29 +328,16 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th
352328
public void testLargeCompressedResponse() throws InterruptedException {
353329
final String responseString = randomAlphaOfLength(4 * 1024 * 1024);
354330
final String url = "/thing";
355-
final HttpServerTransport.Dispatcher dispatcher = new HttpServerTransport.Dispatcher() {
356-
357-
@Override
358-
public void dispatchRequest(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) {
331+
final HttpServerTransport.Dispatcher dispatcher = dispatcherBuilderWithDefaults().withDispatchRequest(
332+
(request, channel, threadContext) -> {
359333
if (url.equals(request.uri())) {
360334
channel.sendResponse(new BytesRestResponse(OK, responseString));
361335
} else {
362336
logger.error("--> Unexpected successful uri [{}]", request.uri());
363337
throw new AssertionError();
364338
}
365339
}
366-
367-
@Override
368-
public void dispatchBadRequest(final RestChannel channel, final ThreadContext threadContext, final Throwable cause) {
369-
logger.error(
370-
new ParameterizedMessage("--> Unexpected bad request [{}]", FakeRestRequest.requestToString(channel.request())),
371-
cause
372-
);
373-
throw new AssertionError();
374-
}
375-
376-
};
377-
340+
).build();
378341
try (
379342
Netty4HttpServerTransport transport = new Netty4HttpServerTransport(
380343
Settings.EMPTY,
@@ -425,25 +388,7 @@ private long getHugeAllocationCount() {
425388
}
426389

427390
public void testCorsRequest() throws InterruptedException {
428-
final HttpServerTransport.Dispatcher dispatcher = new HttpServerTransport.Dispatcher() {
429-
430-
@Override
431-
public void dispatchRequest(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) {
432-
logger.error("--> Unexpected successful request [{}]", FakeRestRequest.requestToString(request));
433-
throw new AssertionError();
434-
}
435-
436-
@Override
437-
public void dispatchBadRequest(final RestChannel channel, final ThreadContext threadContext, final Throwable cause) {
438-
logger.error(
439-
new ParameterizedMessage("--> Unexpected bad request [{}]", FakeRestRequest.requestToString(channel.request())),
440-
cause
441-
);
442-
throw new AssertionError();
443-
}
444-
445-
};
446-
391+
final HttpServerTransport.Dispatcher dispatcher = dispatcherBuilderWithDefaults().build();
447392
final Settings settings = createBuilderWithPort().put(SETTING_CORS_ENABLED.getKey(), true)
448393
.put(SETTING_CORS_ALLOW_ORIGIN.getKey(), "test-cors.org")
449394
.build();
@@ -497,25 +442,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th
497442
}
498443

499444
public void testReadTimeout() throws Exception {
500-
final HttpServerTransport.Dispatcher dispatcher = new HttpServerTransport.Dispatcher() {
501-
502-
@Override
503-
public void dispatchRequest(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) {
504-
logger.error("--> Unexpected successful request [{}]", FakeRestRequest.requestToString(request));
505-
throw new AssertionError("Should not have received a dispatched request");
506-
}
507-
508-
@Override
509-
public void dispatchBadRequest(final RestChannel channel, final ThreadContext threadContext, final Throwable cause) {
510-
logger.error(
511-
new ParameterizedMessage("--> Unexpected bad request [{}]", FakeRestRequest.requestToString(channel.request())),
512-
cause
513-
);
514-
throw new AssertionError("Should not have received a dispatched request");
515-
}
516-
517-
};
518-
445+
HttpServerTransport.Dispatcher dispatcher = dispatcherBuilderWithDefaults().build();
519446
Settings settings = createBuilderWithPort().put(
520447
HttpTransportSettings.SETTING_HTTP_READ_TIMEOUT.getKey(),
521448
new TimeValue(randomIntBetween(100, 300))

0 commit comments

Comments
 (0)