Ignore BaseRestHandler unconsumed content check as it's always consum…

…ed (opensearch-project#13290) * Ignore BaseRestHandler unconsumed content check as it's always consumed Signed-off-by: Daniel Widdis <[email protected]> * Remove comment, continue to ignore content on Force Merge Signed-off-by: Daniel Widdis <[email protected]> * Remove no-body test from RestDeletePitActionTests Signed-off-by: Daniel Widdis <[email protected]> --------- Signed-off-by: Daniel Widdis <[email protected]>
kkewwei · Apr 19, 2024 · 0282e64 · 0282e64
1 parent f5c3ef9
commit 0282e64
Show file tree

Hide file tree

Showing 5 changed files with 1 addition and 130 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -59,6 +59,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Enabled mockTelemetryPlugin for IT and fixed OOM issues ([#13054](https://github.com/opensearch-project/OpenSearch/pull/13054))
 - Fix implement mark() and markSupported() in class FilterStreamInput ([#13098](https://github.com/opensearch-project/OpenSearch/pull/13098))
 - Fix snapshot _status API to return correct status for partial snapshots ([#12812](https://github.com/opensearch-project/OpenSearch/pull/12812))
+- Ignore BaseRestHandler unconsumed content check as it's always consumed. ([#13290](https://github.com/opensearch-project/OpenSearch/pull/13290))
 
 ### Security
 

diff --git a/server/src/main/java/org/opensearch/rest/BaseRestHandler.java b/server/src/main/java/org/opensearch/rest/BaseRestHandler.java
@@ -122,10 +122,6 @@ public final void handleRequest(RestRequest request, RestChannel channel, NodeCl
             throw new IllegalArgumentException(unrecognized(request, unconsumedParams, candidateParams, "parameter"));
         }
 
-        if (request.hasContent() && request.isContentConsumed() == false) {
-            throw new IllegalArgumentException("request [" + request.method() + " " + request.path() + "] does not support having a body");
-        }
-
         usageCount.increment();
         // execute the action
         action.accept(channel);

diff --git a/...c/test/java/org/opensearch/action/admin/indices/forcemerge/RestForceMergeActionTests.java b/...c/test/java/org/opensearch/action/admin/indices/forcemerge/RestForceMergeActionTests.java
@@ -32,45 +32,23 @@
 
 package org.opensearch.action.admin.indices.forcemerge;
 
-import org.opensearch.client.node.NodeClient;
-import org.opensearch.common.xcontent.json.JsonXContent;
-import org.opensearch.core.common.bytes.BytesArray;
-import org.opensearch.core.xcontent.MediaTypeRegistry;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.rest.RestRequest;
 import org.opensearch.rest.action.admin.indices.RestForceMergeAction;
-import org.opensearch.test.rest.FakeRestChannel;
 import org.opensearch.test.rest.FakeRestRequest;
 import org.opensearch.test.rest.RestActionTestCase;
 import org.junit.Before;
 
 import java.util.HashMap;
 import java.util.Map;
 
-import static org.hamcrest.Matchers.equalTo;
-import static org.mockito.Mockito.mock;
-
 public class RestForceMergeActionTests extends RestActionTestCase {
 
     @Before
     public void setUpAction() {
         controller().registerHandler(new RestForceMergeAction());
     }
 
-    public void testBodyRejection() throws Exception {
-        final RestForceMergeAction handler = new RestForceMergeAction();
-        String json = JsonXContent.contentBuilder().startObject().field("max_num_segments", 1).endObject().toString();
-        final FakeRestRequest request = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withContent(
-            new BytesArray(json),
-            MediaTypeRegistry.JSON
-        ).withPath("/_forcemerge").build();
-        IllegalArgumentException e = expectThrows(
-            IllegalArgumentException.class,
-            () -> handler.handleRequest(request, new FakeRestChannel(request, randomBoolean(), 1), mock(NodeClient.class))
-        );
-        assertThat(e.getMessage(), equalTo("request [GET /_forcemerge] does not support having a body"));
-    }
-
     public void testDeprecationMessage() {
         final Map<String, String> params = new HashMap<>();
         params.put("only_expunge_deletes", Boolean.TRUE.toString());

diff --git a/server/src/test/java/org/opensearch/rest/BaseRestHandlerTests.java b/server/src/test/java/org/opensearch/rest/BaseRestHandlerTests.java
@@ -35,10 +35,6 @@
 import org.opensearch.client.node.NodeClient;
 import org.opensearch.common.Table;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.xcontent.json.JsonXContent;
-import org.opensearch.core.common.bytes.BytesArray;
-import org.opensearch.core.xcontent.MediaTypeRegistry;
-import org.opensearch.core.xcontent.XContentBuilder;
 import org.opensearch.rest.RestHandler.ReplacedRoute;
 import org.opensearch.rest.RestHandler.Route;
 import org.opensearch.rest.RestRequest.Method;
@@ -281,81 +277,6 @@ public String getName() {
         assertTrue(executed.get());
     }
 
-    public void testConsumedBody() throws Exception {
-        final AtomicBoolean executed = new AtomicBoolean();
-        final BaseRestHandler handler = new BaseRestHandler() {
-            @Override
-            protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
-                request.content();
-                return channel -> executed.set(true);
-            }
-
-            @Override
-            public String getName() {
-                return "test_consumed_body";
-            }
-        };
-
-        try (XContentBuilder builder = JsonXContent.contentBuilder().startObject().endObject()) {
-            final RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(
-                new BytesArray(builder.toString()),
-                MediaTypeRegistry.JSON
-            ).build();
-            final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
-            handler.handleRequest(request, channel, mockClient);
-            assertTrue(executed.get());
-        }
-    }
-
-    public void testUnconsumedNoBody() throws Exception {
-        final AtomicBoolean executed = new AtomicBoolean();
-        final BaseRestHandler handler = new BaseRestHandler() {
-            @Override
-            protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
-                return channel -> executed.set(true);
-            }
-
-            @Override
-            public String getName() {
-                return "test_unconsumed_body";
-            }
-        };
-
-        final RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).build();
-        final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
-        handler.handleRequest(request, channel, mockClient);
-        assertTrue(executed.get());
-    }
-
-    public void testUnconsumedBody() throws IOException {
-        final AtomicBoolean executed = new AtomicBoolean();
-        final BaseRestHandler handler = new BaseRestHandler() {
-            @Override
-            protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
-                return channel -> executed.set(true);
-            }
-
-            @Override
-            public String getName() {
-                return "test_unconsumed_body";
-            }
-        };
-
-        try (XContentBuilder builder = JsonXContent.contentBuilder().startObject().endObject()) {
-            final RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(
-                new BytesArray(builder.toString()),
-                MediaTypeRegistry.JSON
-            ).build();
-            final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
-            final IllegalArgumentException e = expectThrows(
-                IllegalArgumentException.class,
-                () -> handler.handleRequest(request, channel, mockClient)
-            );
-            assertThat(e, hasToString(containsString("request [GET /] does not support having a body")));
-            assertFalse(executed.get());
-        }
-    }
-
     public void testReplaceRoutesMethod() throws Exception {
         List<Route> routes = Arrays.asList(new Route(Method.GET, "/path/test"), new Route(Method.PUT, "/path2/test"));
         List<ReplacedRoute> replacedRoutes = RestHandler.replaceRoutes(routes, "/prefix", "/deprecatedPrefix");

diff --git a/server/src/test/java/org/opensearch/search/pit/RestDeletePitActionTests.java b/server/src/test/java/org/opensearch/search/pit/RestDeletePitActionTests.java
@@ -82,31 +82,6 @@ public void deletePits(DeletePitRequest request, ActionListener<DeletePitRespons
         }
     }
 
-    public void testDeleteAllPitWithBody() {
-        SetOnce<Boolean> pitCalled = new SetOnce<>();
-        try (NodeClient nodeClient = new NoOpNodeClient(this.getTestName()) {
-            @Override
-            public void deletePits(DeletePitRequest request, ActionListener<DeletePitResponse> listener) {
-                pitCalled.set(true);
-                assertThat(request.getPitIds(), hasSize(1));
-                assertThat(request.getPitIds().get(0), equalTo("_all"));
-            }
-        }) {
-            RestDeletePitAction action = new RestDeletePitAction();
-            RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(
-                new BytesArray("{\"pit_id\": [\"BODY\"]}"),
-                MediaTypeRegistry.JSON
-            ).withPath("/_all").build();
-            FakeRestChannel channel = new FakeRestChannel(request, false, 0);
-
-            IllegalArgumentException ex = expectThrows(
-                IllegalArgumentException.class,
-                () -> action.handleRequest(request, channel, nodeClient)
-            );
-            assertTrue(ex.getMessage().contains("request [GET /_all] does not support having a body"));
-        }
-    }
-
     public void testDeletePitQueryStringParamsShouldThrowException() {
         SetOnce<Boolean> pitCalled = new SetOnce<>();
         try (NodeClient nodeClient = new NoOpNodeClient(this.getTestName()) {