Remove unnecessary RestHandler#supportsBulkContent (#135900)

DaveCTurner · web-flow · commit 502416971ecb · 2025-10-06T10:53:22.000+02:00
This method only exists to add some additional `Content-type`
validation, but we can do all the validation needed using
`RestHandler#mediaTypesValid` instead.
diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/cbor/CborXContentImpl.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/cbor/CborXContentImpl.java
@@ -63,6 +63,11 @@ public XContentType type() {
         return XContentType.CBOR;
     }
 
+    @Override
+    public boolean hasBulkSeparator() {
+        return false;
+    }
+
     @Override
     public byte bulkSeparator() {
         throw new XContentParseException("cbor does not support bulk parsing...");
diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java
@@ -68,6 +68,11 @@ public XContentType type() {
         return XContentType.JSON;
     }
 
+    @Override
+    public boolean hasBulkSeparator() {
+        return true;
+    }
+
     @Override
     public byte bulkSeparator() {
         return '\n';
diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/smile/SmileXContentImpl.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/smile/SmileXContentImpl.java
@@ -70,6 +70,11 @@ public byte bulkSeparator() {
         return (byte) 0xFF;
     }
 
+    @Override
+    public boolean hasBulkSeparator() {
+        return true;
+    }
+
     @Override
     public boolean detectContent(byte[] bytes, int offset, int length) {
         return length > 2
diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/yaml/YamlXContentImpl.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/yaml/YamlXContentImpl.java
@@ -61,6 +61,11 @@ public XContentType type() {
         return XContentType.YAML;
     }
 
+    @Override
+    public boolean hasBulkSeparator() {
+        return false;
+    }
+
     @Override
     public byte bulkSeparator() {
         throw new UnsupportedOperationException("yaml does not support bulk parsing...");
diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContent.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContent.java
@@ -25,6 +25,20 @@ public interface XContent {
      */
     XContentType type();
 
+    /**
+     * @return {@code true} if this {@link XContent} can be sent in bulk, delimited by the byte returned by {@link #bulkSeparator()}, or
+     * {@code false} if this {@link XContent} does not support a delimited bulk format (in which case {@link #bulkSeparator()} throws an
+     * exception.
+     * <p>
+     * In practice, this is {@code true} for content with canonical type {@link XContentType#JSON} or {@link XContentType#SMILE} and
+     * {@code false} for content with canonical type {@link XContentType#CBOR} or {@link XContentType#YAML}.
+     */
+    boolean hasBulkSeparator();
+
+    /**
+     * @return a {@link byte} that separates items in a bulk request that uses this {@link XContent}.
+     * @throws RuntimeException if this {@link XContent} does not support a delimited bulk format. See {@link #hasBulkSeparator()}.
+     */
     byte bulkSeparator();
 
     @Deprecated
diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentType.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentType.java
@@ -9,6 +9,7 @@
 
 package org.elasticsearch.xcontent;
 
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.xcontent.cbor.CborXContent;
 import org.elasticsearch.xcontent.json.JsonXContent;
 import org.elasticsearch.xcontent.smile.SmileXContent;
@@ -287,4 +288,15 @@ public XContentType canonical() {
     public static XContentType ofOrdinal(int ordinal) {
         return values[ordinal];
     }
+
+    /**
+     * Indicates if the {@link XContentType} (from the {@code Content-Type} header of a REST request) is compatible with delimited bulk
+     * content. A bulk request contains multiple objects each terminated with {@link XContent#bulkSeparator()}.
+     * <p>
+     * In practice, this returns {@code true} if the argument has canonical type {@link XContentType#JSON} or {@link XContentType#SMILE}
+     * and {@code false} if the argument is {@code null} or has canonical type {@link XContentType#CBOR} or {@link XContentType#YAML}.
+     */
+    public static boolean supportsDelimitedBulkRequests(@Nullable XContentType xContentType) {
+        return xContentType != null && xContentType.xContent.hasBulkSeparator();
+    }
 }
diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestMultiSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestMultiSearchTemplateAction.java
@@ -18,6 +18,7 @@
 import org.elasticsearch.rest.action.RestToXContentListener;
 import org.elasticsearch.rest.action.search.RestMultiSearchAction;
 import org.elasticsearch.rest.action.search.RestSearchAction;
+import org.elasticsearch.xcontent.XContentType;
 
 import java.io.IOException;
 import java.util.List;
@@ -95,8 +96,8 @@ public MultiSearchTemplateRequest parseRequest(RestRequest restRequest, boolean
     }
 
     @Override
-    public boolean supportsBulkContent() {
-        return true;
+    public boolean mediaTypesValid(RestRequest request) {
+        return super.mediaTypesValid(request) && XContentType.supportsDelimitedBulkRequests(request.getXContentType());
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/rest/FilterRestHandler.java b/server/src/main/java/org/elasticsearch/rest/FilterRestHandler.java
@@ -43,11 +43,6 @@ public boolean canTripCircuitBreaker() {
         return delegate.canTripCircuitBreaker();
     }
 
-    @Override
-    public boolean supportsBulkContent() {
-        return delegate.supportsBulkContent();
-    }
-
     @Override
     public boolean mediaTypesValid(RestRequest request) {
         return delegate.mediaTypesValid(request);
diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java
@@ -421,20 +421,6 @@ private void dispatchRequest(
                 sendContentTypeErrorMessage(request.getAllHeaderValues("Content-Type"), channel);
                 return;
             }
-            final XContentType xContentType = request.getXContentType();
-            // TODO consider refactoring to handler.supportsContentStream(xContentType). It is only used with JSON and SMILE
-            if (handler.supportsBulkContent()
-                && XContentType.JSON != xContentType.canonical()
-                && XContentType.SMILE != xContentType.canonical()) {
-                channel.sendResponse(
-                    RestResponse.createSimpleErrorResponse(
-                        channel,
-                        RestStatus.NOT_ACCEPTABLE,
-                        "Content-Type [" + xContentType + "] does not support stream parsing. Use JSON or SMILE instead"
-                    )
-                );
-                return;
-            }
         }
         RestChannel responseChannel = channel;
         if (apiProtections.isEnabled()) {
diff --git a/server/src/main/java/org/elasticsearch/rest/RestHandler.java b/server/src/main/java/org/elasticsearch/rest/RestHandler.java
@@ -15,7 +15,6 @@
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.RestApiVersion;
 import org.elasticsearch.rest.RestRequest.Method;
-import org.elasticsearch.xcontent.XContent;
 
 import java.util.Collections;
 import java.util.List;
@@ -44,15 +43,6 @@ default boolean supportsContentStream() {
         return false;
     }
 
-    /**
-     * Indicates if the RestHandler supports bulk content. A bulk request contains multiple objects
-     * delineated by {@link XContent#bulkSeparator()}. If a handler returns true this will affect
-     * the types of content that can be sent to this endpoint.
-     */
-    default boolean supportsBulkContent() {
-        return false;
-    }
-
     /**
      * Returns the concrete RestHandler for this RestHandler. That is, if this is a delegating RestHandler it returns the delegate.
      * Otherwise it returns itself.
diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java
@@ -36,6 +36,7 @@
 import org.elasticsearch.rest.action.RestToXContentListener;
 import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
 import org.elasticsearch.transport.Transports;
+import org.elasticsearch.xcontent.XContentType;
 
 import java.io.IOException;
 import java.util.ArrayDeque;
@@ -303,12 +304,12 @@ private ArrayList<Releasable> accountParsing(int bytesConsumed) {
     }
 
     @Override
-    public boolean supportsBulkContent() {
-        return true;
+    public Set<String> supportedCapabilities() {
+        return capabilities;
     }
 
     @Override
-    public Set<String> supportedCapabilities() {
-        return capabilities;
+    public boolean mediaTypesValid(RestRequest request) {
+        return super.mediaTypesValid(request) && XContentType.supportsDelimitedBulkRequests(request.getXContentType());
     }
 }
diff --git a/server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java b/server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java
@@ -209,8 +209,8 @@ public static void parseMultiLineRequest(
     }
 
     @Override
-    public boolean supportsBulkContent() {
-        return true;
+    public boolean mediaTypesValid(RestRequest request) {
+        return super.mediaTypesValid(request) && XContentType.supportsDelimitedBulkRequests(request.getXContentType());
     }
 
     @Override
diff --git a/server/src/test/java/org/elasticsearch/rest/DeprecationRestHandlerTests.java b/server/src/test/java/org/elasticsearch/rest/DeprecationRestHandlerTests.java
@@ -22,7 +22,6 @@
 
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
 
 /**
  * Tests {@link DeprecationRestHandler}.
@@ -156,22 +155,6 @@ public void testInvalidHeaderValueEmpty() {
         expectThrows(IllegalArgumentException.class, () -> DeprecationRestHandler.requireValidHeader(blank));
     }
 
-    public void testSupportsBulkContentTrue() {
-        when(handler.supportsBulkContent()).thenReturn(true);
-        assertTrue(
-            new DeprecationRestHandler(handler, METHOD, PATH, Level.WARN, deprecationMessage, deprecationLogger, false)
-                .supportsBulkContent()
-        );
-    }
-
-    public void testSupportsBulkContentFalse() {
-        when(handler.supportsBulkContent()).thenReturn(false);
-        assertFalse(
-            new DeprecationRestHandler(handler, METHOD, PATH, Level.WARN, deprecationMessage, deprecationLogger, false)
-                .supportsBulkContent()
-        );
-    }
-
     public void testDeprecationLevel() {
         DeprecationRestHandler handler = new DeprecationRestHandler(
             this.handler,
diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java
@@ -640,8 +640,8 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
             }
 
             @Override
-            public boolean supportsBulkContent() {
-                return true;
+            public boolean mediaTypesValid(RestRequest request) {
+                return RestHandler.super.mediaTypesValid(request) && XContentType.supportsDelimitedBulkRequests(request.getXContentType());
             }
         });
 
@@ -679,8 +679,8 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
             }
 
             @Override
-            public boolean supportsBulkContent() {
-                return true;
+            public boolean mediaTypesValid(RestRequest request) {
+                return RestHandler.super.mediaTypesValid(request) && XContentType.supportsDelimitedBulkRequests(request.getXContentType());
             }
         });
 
@@ -704,8 +704,8 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
             }
 
             @Override
-            public boolean supportsBulkContent() {
-                return true;
+            public boolean mediaTypesValid(RestRequest request) {
+                return RestHandler.super.mediaTypesValid(request) && XContentType.supportsDelimitedBulkRequests(request.getXContentType());
             }
         });
 
@@ -730,8 +730,8 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
             }
 
             @Override
-            public boolean supportsBulkContent() {
-                return true;
+            public boolean mediaTypesValid(RestRequest request) {
+                return RestHandler.super.mediaTypesValid(request) && XContentType.supportsDelimitedBulkRequests(request.getXContentType());
             }
         });
         assertFalse(channel.getSendResponseCalled());
@@ -755,8 +755,8 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
             }
 
             @Override
-            public boolean supportsBulkContent() {
-                return true;
+            public boolean mediaTypesValid(RestRequest request) {
+                return RestHandler.super.mediaTypesValid(request) && XContentType.supportsDelimitedBulkRequests(request.getXContentType());
             }
         });
         assertFalse(channel.getSendResponseCalled());
diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestPostDataAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestPostDataAction.java
@@ -12,6 +12,7 @@
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.rest.action.RestToXContentListener;
+import org.elasticsearch.xcontent.XContentType;
 import org.elasticsearch.xpack.core.ml.action.PostDataAction;
 import org.elasticsearch.xpack.core.ml.job.config.Job;
 
@@ -54,7 +55,7 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient
     }
 
     @Override
-    public boolean supportsBulkContent() {
-        return true;
+    public boolean mediaTypesValid(RestRequest request) {
+        return super.mediaTypesValid(request) && XContentType.supportsDelimitedBulkRequests(request.getXContentType());
     }
 }
diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkAction.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkAction.java
@@ -16,6 +16,7 @@
 import org.elasticsearch.rest.RestResponse;
 import org.elasticsearch.rest.action.RestBuilderListener;
 import org.elasticsearch.xcontent.XContentBuilder;
+import org.elasticsearch.xcontent.XContentType;
 import org.elasticsearch.xpack.core.monitoring.MonitoredSystem;
 import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkRequestBuilder;
 import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkResponse;
@@ -100,8 +101,8 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
     }
 
     @Override
-    public boolean supportsBulkContent() {
-        return true;
+    public boolean mediaTypesValid(RestRequest request) {
+        return super.mediaTypesValid(request) && XContentType.supportsDelimitedBulkRequests(request.getXContentType());
     }
 
     /**
diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkActionTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkActionTests.java
@@ -28,6 +28,7 @@
 import org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils;
 
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import static org.elasticsearch.xpack.core.monitoring.exporter.MonitoringTemplateUtils.TEMPLATE_VERSION;
@@ -47,7 +48,18 @@ public void testGetName() {
 
     public void testSupportsBulkContent() {
         // if you change this, it's a very breaking change for Monitoring
-        assertThat(action.supportsBulkContent(), is(true));
+        final var route = action.routes().get(0);
+        for (var xContentType : XContentType.values()) {
+            final var request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(route.getMethod())
+                .withPath(route.getPath())
+                .withHeaders(Map.of("Content-Type", List.of(xContentType.mediaType())))
+                .build();
+            assertEquals(
+                xContentType.toString(),
+                XContentType.supportsDelimitedBulkRequests(request.getXContentType()),
+                action.mediaTypesValid(request)
+            );
+        }
     }
 
     public void testMissingSystemId() {
diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringMigrateAlertsActionTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringMigrateAlertsActionTests.java
@@ -11,6 +11,8 @@
 import org.elasticsearch.rest.RestResponse;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.rest.FakeRestRequest;
+import org.elasticsearch.xcontent.XContentType;
 import org.elasticsearch.xcontent.json.JsonXContent;
 import org.elasticsearch.xpack.core.monitoring.action.MonitoringMigrateAlertsResponse;
 import org.elasticsearch.xpack.core.monitoring.action.MonitoringMigrateAlertsResponse.ExporterMigrationResult;
@@ -20,6 +22,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.startsWith;
@@ -34,8 +37,15 @@ public void testGetName() {
         assertThat(action.getName(), is("monitoring_migrate_alerts"));
     }
 
-    public void testSupportsContentStream() {
-        assertThat(action.supportsBulkContent(), is(false));
+    public void testSupportsAllContentTypes() {
+        final var route = action.routes().get(0);
+        for (var xContentType : XContentType.values()) {
+            final var request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(route.getMethod())
+                .withPath(route.getPath())
+                .withHeaders(Map.of("Content-Type", List.of(xContentType.mediaType())))
+                .build();
+            assertTrue(xContentType.toString(), action.mediaTypesValid(request));
+        }
     }
 
     public void testRestActionCompletion() throws Exception {
