Don't emit deprecation warnings on calls to the monitoring bulk API.

[jpountz]

The monitoring bulk API accepts the same format as the bulk API, yet its concept
of types is different from "mapping types" and the deprecation warning is only
emitted as a side-effect of this API reusing the parsing logic of bulk requests.

This commit extracts the parsing logic from _bulk into its own class with a
new flag that allows to configure whether usage of _type should emit a warning
or not. Support for payloads has been removed for simplicity since they were
unused.

@jakelandis has a separate change that removes this notion of type from the
monitoring bulk API that we are considering bringing to 8.0: #39336.

[elasticmachine]

Pinging @elastic/es-core-features

[jakelandis]

LGTM, just a couple minor comments.

Also, just for my own education, what were payloads ?

[jakelandis]

Should this method as-is carry a @deprecate and then add in a non-deprecrated variant that passes in the defaultType for type for you. I found that deprecation (in the BulkRequest) helpful to know I shouldn't pass in a defaultType (with out that pattern I would be unsure to explicitly send _doc or null).

For example BulkRequest#add helps the developer out to know not to pass in a defaultType https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java#L343

[jakelandis]

nit: should the variables get renamed ? defaultRouting -> routing and defaultPipeline -> pipeline (since here there is no delineation between default or global)

[jpountz]

I think the defaultRouting and defaultPipeline names still make sense since they can be overridden on a per-document basis?

[jakelandis]

Does this (and tests below) need to conditionally assert assertWarnings(RestBulkAction.TYPES_DEPRECATION_MESSAGE); based on the randomBoolean() (line 34) ?

[jpountz]

They don't since the document doesn't include a _type.

[jakelandis]

has a separate change that removes this notion of type from the monitoring bulk API that we are considering bringing to 8.0: #39336.

The statement is correct, but it won't be that PR #.

[jakelandis]

The CI failure is can be fixed with

diff --git a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java
index 6d3e4c04c13..ebd6590a80c 100644
--- a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java
+++ b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java
@@ -352,7 +352,7 @@ public class BulkRequestTests extends ESTestCase {
         String bulkAction = copyToStringFromClasspath("/org/elasticsearch/action/bulk/simple-bulk11.json");
         IllegalArgumentException expectThrows = expectThrows(IllegalArgumentException.class, () -> new BulkRequest()
                 .add(bulkAction.getBytes(StandardCharsets.UTF_8), 0, bulkAction.length(), null, XContentType.JSON));
-        assertEquals("The bulk request must be terminated by a newline [\n]", expectThrows.getMessage());
+        assertEquals("The bulk request must be terminated by a newline [\\n]", expectThrows.getMessage());

         String bulkActionWithNewLine = bulkAction + "\n";
         BulkRequest bulkRequestWithNewLine = new BulkRequest();

(I assume that was an intentional change)

[jpountz]

Thanks, I wanted to open this PR before logging off and I had not had time to run all tests yet. The change to the error is intentional indeed. Thanks for reviewing!

I am not entirely sure about payloads. I suspect it was a way to associate metadata with every index request that would be included in the response as well, but the git log suggests that we started adding this feature and never finished.