Ignore BaseRestHandler unconsumed content check as it's always consumed

Signed-off-by: Daniel Widdis <[email protected]>
dbwiddis · Apr 18, 2024 · aa5219a · aa5219a
1 parent f5c3ef9
commit aa5219a
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 84 deletions.
diff --git a/server/src/main/java/org/opensearch/rest/BaseRestHandler.java b/server/src/main/java/org/opensearch/rest/BaseRestHandler.java
@@ -122,9 +122,8 @@ 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");
-        }
+        // ignore whether content is consumed
+        // https://github.com/opensearch-project/OpenSearch/issues/13011
 
         usageCount.increment();
         // execute the action

diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestForceMergeAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestForceMergeAction.java
@@ -71,7 +71,10 @@ public String getName() {
 
     @Override
     public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
-        final ForceMergeRequest mergeRequest = new ForceMergeRequest(Strings.splitStringByCommaToArray(request.param("index")));
+        if (request.hasContent()) {
+            throw new IllegalArgumentException("forcemerge takes arguments in query parameters, not in the request body");
+        }
+        ForceMergeRequest mergeRequest = new ForceMergeRequest(Strings.splitStringByCommaToArray(request.param("index")));
         mergeRequest.indicesOptions(IndicesOptions.fromRequest(request, mergeRequest.indicesOptions()));
         mergeRequest.maxNumSegments(request.paramAsInt("max_num_segments", mergeRequest.maxNumSegments()));
         mergeRequest.onlyExpungeDeletes(request.paramAsBoolean("only_expunge_deletes", mergeRequest.onlyExpungeDeletes()));

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
@@ -68,7 +68,7 @@ public void testBodyRejection() throws Exception {
             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"));
+        assertThat(e.getMessage(), equalTo("forcemerge takes arguments in query parameters, not in the request body"));
     }
 
     public void testDeprecationMessage() {

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");