Fix failures in BulkProcessorIT#testGlobalParametersAndBulkProcessor.

[jtibshirani]

This PR fixes a couple test issues:

• It narrows an assertWarnings call that was too broad, and wasn't always
  applicable with certain random sequences.
• Previously, we could send a typeless bulk request containing '_type: 'null'.
  Now we omit the _type key altogether for typeless requests.

[elasticmachine]

Pinging @elastic/es-search

[jtibshirani]

These issues originally came to light because @atorok saw this test fail an intake build: https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+intake/1702/testReport/junit/org.elasticsearch.client/BulkProcessorIT/testGlobalParametersAndBulkProcessor/

[markharwood]

These issues originally came to light because @atorok saw this test fail

I can't see the original failure now. Do you have the reproduce line?

It narrows an assertWarnings call that was too broad

I don't see that broader previous assertWarnings in the diff? I can see you've added a new one.
The code that is here in the PR makes sense I'm just trying to reconcile with the stated intent.

[jtibshirani]

Oops, I didn't realize that link would expire. Here was the reproduce line it contained:

./gradlew :client:rest-high-level:integTestRunner \
    -Dtests.seed=A1A91C16AA652749 \
    -Dtests.class=org.elasticsearch.client.BulkProcessorIT \
    -Dtests.method="testGlobalParametersAndBulkProcessor" \
    -Dtests.security.manager=true \
    -Dtests.locale=es-ES \
    -Dtests.timezone=Australia/NSW \
    -Dcompiler.java=11 \
    -Druntime.java=8

The assertWarnings that turned out to be too broad was here: https://github.com/elastic/elasticsearch/pull/38129/files#diff-52c3c5ac9fc17246e52a27ec10e5bfbfL430

[markharwood]

LGTM.
There was clearly an issue with serializing null.
I added a comment signalling my unease about the warnings checks we have here - we're not really cleanly testing an API's contract here. We're just squashing some awkward side effects of reusing server-side POJOs on the client - they're setting up warnings in thread locals that have no business appearing in client-side processes but we're having to acknowledge them anyway just to make tests pass. It's ugly (no discredit to you intended) but I guess we have to live with it until we fully divorce HLRC from server-side POJOs.

[markharwood]

This line troubled me. Bulk requests can pass a type on each request line (localType var here) or pass a default fallback choice which is inherited by any request lines that don't declare a localType (the fallback is called globalType).
So here, I assumed the test's logic should ideally be:

 if (localType !=null || globalType != null) {

These are the conditions under which a user has sent a choice of custom type to the server.
Sadly this doesn't work. It looks like in the server-side BulkRequest that only types expressed at the line level are flagged. It assumes any "global" references to type are already flagged by the main RestBulkAction which we are not exercising in this test. What we are squashing with these assertWarnings checks in BulkProcessorIT are the ThreadContext values normally set up on the server side by the server-side objects like BulkRequest which we are (questionably) re-using in HLRC.
These warning checks are not really the same as simply checking that the response from a _bulk REST interaction has the expected warning if you use happen to use global or local type references in the request. That contract is exercised in BulkRequestWithGlobalParametersIT so I think we're good there.

[jtibshirani]

Thanks @markharwood for the review. Off the top of my head, it seems like if we had an allowWarnings option, it would be more appropriate here than assertWarnings. This is because we're not trying to test an API contract around warnings, but just allow for the possibility of warnings while testing other functionality.