Early validation of index requests for bulk request

[jprante]

In the Java API for adding index requests to a bulk request, the index requests are not validated immediately. This can lead to unexpected NullPointerExceptions when the source is examined.

This pull request adds early validation. It helps to detect bogus index requests before they are sent to the cluster in bulk.

Also, some bulk request test cases are included to check successful validation.

[bleskes]

Thx @jprante . It feels more inline with the rest of the code to do these things in BulkRequest#validateRequest . Maybe something there this missing (ala a null pointer check)?

[jprante]

@bleskes it seems that BulkRequest#validate() is never used in the current code.

[bleskes]

It should be called by TransportAction#execute(Request, org.elasticsearch.action.ActionListener) . I wonder how you tested?

[jprante]

The tests are very simple. NPEs can happen before TransportAction#execute.

ES 1.6 examples how to trigger NPE:

BulkRequestBuilder builder = new BulkRequestBuilder(client)
                .add(Requests.indexRequest());

BulkRequestBuilder builder = new BulkRequestBuilder(client)
                .add((IndexRequest)null);

IndexRequestBuilder r = new IndexRequestBuilder(client);
        BulkRequestBuilder builder = new BulkRequestBuilder(client)
                .add(r.request());

client.prepareBulk()
                .add(Requests.indexRequest().index(null).type(null).id(null))

That is why early validation is needed to reject invalid IndexRequests from being added to a BulkRequest.

[danielmitterdorfer]

Hi @jprante,

sorry that the feedback took a while. I've checked your examples against the latest master and the only case that is causing trouble (i.e. NPEs) now is: BulkRequestBuilder builder = new BulkRequestBuilder(client, BulkAction.INSTANCE).add((IndexRequest)null);

I agree that it is a programmer error if a null value is passed here. We should fail fast in this case and add a precondition check in BulkRequest#internalAdd() and all other spots where we add a request to the internal collection. As we can rely on Java 7, I'd use `Objects#requireNonNull() from the JDK for the precondition check.

Do want to give it a shot?

[dakrone]

Hi @jprante is there still interest in working on this PR?

[danielmitterdorfer]

I have now created a new PR #18347 that implements the necessary not-null precondition checks and I am closing this PR in favor of the new one.