Skip to content

Commit 0282e64

Browse files
dbwiddisdbwiddis
authored
Ignore BaseRestHandler unconsumed content check as it's always consumed (opensearch-project#13290)
* Ignore BaseRestHandler unconsumed content check as it's always consumed Signed-off-by: Daniel Widdis <widdis@gmail.com> * Remove comment, continue to ignore content on Force Merge Signed-off-by: Daniel Widdis <widdis@gmail.com> * Remove no-body test from RestDeletePitActionTests Signed-off-by: Daniel Widdis <widdis@gmail.com> --------- Signed-off-by: Daniel Widdis <widdis@gmail.com>
1 parent f5c3ef9 commit 0282e64

File tree

5 files changed

+1
-130
lines changed

5 files changed

+1
-130
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
5959
- Enabled mockTelemetryPlugin for IT and fixed OOM issues ([#13054](https://github.com/opensearch-project/OpenSearch/pull/13054))
6060
- Fix implement mark() and markSupported() in class FilterStreamInput ([#13098](https://github.com/opensearch-project/OpenSearch/pull/13098))
6161
- Fix snapshot _status API to return correct status for partial snapshots ([#12812](https://github.com/opensearch-project/OpenSearch/pull/12812))
62+
- Ignore BaseRestHandler unconsumed content check as it's always consumed. ([#13290](https://github.com/opensearch-project/OpenSearch/pull/13290))
6263

6364
### Security
6465

‎server/src/main/java/org/opensearch/rest/BaseRestHandler.java

-4
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,6 @@ public final void handleRequest(RestRequest request, RestChannel channel, NodeCl
122122
throw new IllegalArgumentException(unrecognized(request, unconsumedParams, candidateParams, "parameter"));
123123
}
124124

125-
if (request.hasContent() && request.isContentConsumed() == false) {
126-
throw new IllegalArgumentException("request [" + request.method() + " " + request.path() + "] does not support having a body");
127-
}
128-
129125
usageCount.increment();
130126
// execute the action
131127
action.accept(channel);

‎server/src/test/java/org/opensearch/action/admin/indices/forcemerge/RestForceMergeActionTests.java

-22
Original file line numberDiff line numberDiff line change
@@ -32,45 +32,23 @@
3232

3333
package org.opensearch.action.admin.indices.forcemerge;
3434

35-
import org.opensearch.client.node.NodeClient;
36-
import org.opensearch.common.xcontent.json.JsonXContent;
37-
import org.opensearch.core.common.bytes.BytesArray;
38-
import org.opensearch.core.xcontent.MediaTypeRegistry;
3935
import org.opensearch.core.xcontent.NamedXContentRegistry;
4036
import org.opensearch.rest.RestRequest;
4137
import org.opensearch.rest.action.admin.indices.RestForceMergeAction;
42-
import org.opensearch.test.rest.FakeRestChannel;
4338
import org.opensearch.test.rest.FakeRestRequest;
4439
import org.opensearch.test.rest.RestActionTestCase;
4540
import org.junit.Before;
4641

4742
import java.util.HashMap;
4843
import java.util.Map;
4944

50-
import static org.hamcrest.Matchers.equalTo;
51-
import static org.mockito.Mockito.mock;
52-
5345
public class RestForceMergeActionTests extends RestActionTestCase {
5446

5547
@Before
5648
public void setUpAction() {
5749
controller().registerHandler(new RestForceMergeAction());
5850
}
5951

60-
public void testBodyRejection() throws Exception {
61-
final RestForceMergeAction handler = new RestForceMergeAction();
62-
String json = JsonXContent.contentBuilder().startObject().field("max_num_segments", 1).endObject().toString();
63-
final FakeRestRequest request = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withContent(
64-
new BytesArray(json),
65-
MediaTypeRegistry.JSON
66-
).withPath("/_forcemerge").build();
67-
IllegalArgumentException e = expectThrows(
68-
IllegalArgumentException.class,
69-
() -> handler.handleRequest(request, new FakeRestChannel(request, randomBoolean(), 1), mock(NodeClient.class))
70-
);
71-
assertThat(e.getMessage(), equalTo("request [GET /_forcemerge] does not support having a body"));
72-
}
73-
7452
public void testDeprecationMessage() {
7553
final Map<String, String> params = new HashMap<>();
7654
params.put("only_expunge_deletes", Boolean.TRUE.toString());

‎server/src/test/java/org/opensearch/rest/BaseRestHandlerTests.java

-79
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,6 @@
3535
import org.opensearch.client.node.NodeClient;
3636
import org.opensearch.common.Table;
3737
import org.opensearch.common.settings.Settings;
38-
import org.opensearch.common.xcontent.json.JsonXContent;
39-
import org.opensearch.core.common.bytes.BytesArray;
40-
import org.opensearch.core.xcontent.MediaTypeRegistry;
41-
import org.opensearch.core.xcontent.XContentBuilder;
4238
import org.opensearch.rest.RestHandler.ReplacedRoute;
4339
import org.opensearch.rest.RestHandler.Route;
4440
import org.opensearch.rest.RestRequest.Method;
@@ -281,81 +277,6 @@ public String getName() {
281277
assertTrue(executed.get());
282278
}
283279

284-
public void testConsumedBody() throws Exception {
285-
final AtomicBoolean executed = new AtomicBoolean();
286-
final BaseRestHandler handler = new BaseRestHandler() {
287-
@Override
288-
protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
289-
request.content();
290-
return channel -> executed.set(true);
291-
}
292-
293-
@Override
294-
public String getName() {
295-
return "test_consumed_body";
296-
}
297-
};
298-
299-
try (XContentBuilder builder = JsonXContent.contentBuilder().startObject().endObject()) {
300-
final RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(
301-
new BytesArray(builder.toString()),
302-
MediaTypeRegistry.JSON
303-
).build();
304-
final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
305-
handler.handleRequest(request, channel, mockClient);
306-
assertTrue(executed.get());
307-
}
308-
}
309-
310-
public void testUnconsumedNoBody() throws Exception {
311-
final AtomicBoolean executed = new AtomicBoolean();
312-
final BaseRestHandler handler = new BaseRestHandler() {
313-
@Override
314-
protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
315-
return channel -> executed.set(true);
316-
}
317-
318-
@Override
319-
public String getName() {
320-
return "test_unconsumed_body";
321-
}
322-
};
323-
324-
final RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).build();
325-
final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
326-
handler.handleRequest(request, channel, mockClient);
327-
assertTrue(executed.get());
328-
}
329-
330-
public void testUnconsumedBody() throws IOException {
331-
final AtomicBoolean executed = new AtomicBoolean();
332-
final BaseRestHandler handler = new BaseRestHandler() {
333-
@Override
334-
protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
335-
return channel -> executed.set(true);
336-
}
337-
338-
@Override
339-
public String getName() {
340-
return "test_unconsumed_body";
341-
}
342-
};
343-
344-
try (XContentBuilder builder = JsonXContent.contentBuilder().startObject().endObject()) {
345-
final RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(
346-
new BytesArray(builder.toString()),
347-
MediaTypeRegistry.JSON
348-
).build();
349-
final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
350-
final IllegalArgumentException e = expectThrows(
351-
IllegalArgumentException.class,
352-
() -> handler.handleRequest(request, channel, mockClient)
353-
);
354-
assertThat(e, hasToString(containsString("request [GET /] does not support having a body")));
355-
assertFalse(executed.get());
356-
}
357-
}
358-
359280
public void testReplaceRoutesMethod() throws Exception {
360281
List<Route> routes = Arrays.asList(new Route(Method.GET, "/path/test"), new Route(Method.PUT, "/path2/test"));
361282
List<ReplacedRoute> replacedRoutes = RestHandler.replaceRoutes(routes, "/prefix", "/deprecatedPrefix");

‎server/src/test/java/org/opensearch/search/pit/RestDeletePitActionTests.java

-25
Original file line numberDiff line numberDiff line change
@@ -82,31 +82,6 @@ public void deletePits(DeletePitRequest request, ActionListener<DeletePitRespons
8282
}
8383
}
8484

85-
public void testDeleteAllPitWithBody() {
86-
SetOnce<Boolean> pitCalled = new SetOnce<>();
87-
try (NodeClient nodeClient = new NoOpNodeClient(this.getTestName()) {
88-
@Override
89-
public void deletePits(DeletePitRequest request, ActionListener<DeletePitResponse> listener) {
90-
pitCalled.set(true);
91-
assertThat(request.getPitIds(), hasSize(1));
92-
assertThat(request.getPitIds().get(0), equalTo("_all"));
93-
}
94-
}) {
95-
RestDeletePitAction action = new RestDeletePitAction();
96-
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(
97-
new BytesArray("{\"pit_id\": [\"BODY\"]}"),
98-
MediaTypeRegistry.JSON
99-
).withPath("/_all").build();
100-
FakeRestChannel channel = new FakeRestChannel(request, false, 0);
101-
102-
IllegalArgumentException ex = expectThrows(
103-
IllegalArgumentException.class,
104-
() -> action.handleRequest(request, channel, nodeClient)
105-
);
106-
assertTrue(ex.getMessage().contains("request [GET /_all] does not support having a body"));
107-
}
108-
}
109-
11085
public void testDeletePitQueryStringParamsShouldThrowException() {
11186
SetOnce<Boolean> pitCalled = new SetOnce<>();
11287
try (NodeClient nodeClient = new NoOpNodeClient(this.getTestName()) {

0 commit comments

Comments
 (0)