From 15c936ec023b7f8f67a0a5098b799a5b02540c2e Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Thu, 2 Mar 2017 12:37:07 +0100 Subject: [PATCH] Correctly parse BulkItemResponse.Failure's status (#23432) Today the status is lost when parsing back a BulkItemResponse.Failure. This commit changes the BulkItemResponse.Failure parsing method so that it correctly instantiates a failure with the parsed status instead of realying on the parsed ElasticsearchException (that always return an internal server error status). --- .../java/org/elasticsearch/client/CrudIT.java | 4 ++-- .../action/bulk/BulkItemResponse.java | 17 +++++++++++++---- .../action/bulk/BulkItemResponseTests.java | 8 ++++++-- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java index 346d7d7c756..2a7dfebd284 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java @@ -588,10 +588,10 @@ public class CrudIT extends ESRestHighLevelClientTestCase { DocWriteRequest.OpType requestOpType = bulkRequest.requests().get(i).opType(); if (requestOpType == DocWriteRequest.OpType.INDEX || requestOpType == DocWriteRequest.OpType.CREATE) { assertEquals(errors[i], bulkItemResponse.isFailed()); - assertEquals(errors[i] ? RestStatus.INTERNAL_SERVER_ERROR : RestStatus.CREATED, bulkItemResponse.status()); + assertEquals(errors[i] ? RestStatus.CONFLICT : RestStatus.CREATED, bulkItemResponse.status()); } else if (requestOpType == DocWriteRequest.OpType.UPDATE) { assertEquals(errors[i], bulkItemResponse.isFailed()); - assertEquals(errors[i] ? RestStatus.INTERNAL_SERVER_ERROR : RestStatus.OK, bulkItemResponse.status()); + assertEquals(errors[i] ? RestStatus.NOT_FOUND : RestStatus.OK, bulkItemResponse.status()); } else if (requestOpType == DocWriteRequest.OpType.DELETE) { assertFalse(bulkItemResponse.isFailed()); assertEquals(errors[i] ? RestStatus.NOT_FOUND : RestStatus.OK, bulkItemResponse.status()); diff --git a/core/src/main/java/org/elasticsearch/action/bulk/BulkItemResponse.java b/core/src/main/java/org/elasticsearch/action/bulk/BulkItemResponse.java index 31511e6b94f..2e2a7f15401 100644 --- a/core/src/main/java/org/elasticsearch/action/bulk/BulkItemResponse.java +++ b/core/src/main/java/org/elasticsearch/action/bulk/BulkItemResponse.java @@ -122,6 +122,7 @@ public class BulkItemResponse implements Streamable, StatusToXContentObject { throwUnknownField(currentFieldName, parser.getTokenLocation()); } + RestStatus status = null; ElasticsearchException exception = null; while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { @@ -132,7 +133,11 @@ public class BulkItemResponse implements Streamable, StatusToXContentObject { if (token == XContentParser.Token.START_OBJECT) { exception = ElasticsearchException.fromXContent(parser); } - } else if (STATUS.equals(currentFieldName) == false) { + } else if (STATUS.equals(currentFieldName)) { + if (token == XContentParser.Token.VALUE_NUMBER) { + status = RestStatus.fromCode(parser.intValue()); + } + } else { itemParser.accept(parser); } } @@ -143,7 +148,7 @@ public class BulkItemResponse implements Streamable, StatusToXContentObject { BulkItemResponse bulkItemResponse; if (exception != null) { - Failure failure = new Failure(builder.getShardId().getIndexName(), builder.getType(), builder.getId(), exception); + Failure failure = new Failure(builder.getShardId().getIndexName(), builder.getType(), builder.getId(), exception, status); bulkItemResponse = new BulkItemResponse(id, opType, failure); } else { bulkItemResponse = new BulkItemResponse(id, opType, builder.build()); @@ -167,12 +172,16 @@ public class BulkItemResponse implements Streamable, StatusToXContentObject { private final Exception cause; private final RestStatus status; - public Failure(String index, String type, String id, Exception cause) { + Failure(String index, String type, String id, Exception cause, RestStatus status) { this.index = index; this.type = type; this.id = id; this.cause = cause; - this.status = ExceptionsHelper.status(cause); + this.status = status; + } + + public Failure(String index, String type, String id, Exception cause) { + this(index, type, id, cause, ExceptionsHelper.status(cause)); } /** diff --git a/core/src/test/java/org/elasticsearch/action/bulk/BulkItemResponseTests.java b/core/src/test/java/org/elasticsearch/action/bulk/BulkItemResponseTests.java index 580c7f4435f..e92f6e40ab3 100644 --- a/core/src/test/java/org/elasticsearch/action/bulk/BulkItemResponseTests.java +++ b/core/src/test/java/org/elasticsearch/action/bulk/BulkItemResponseTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.action.bulk; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.DocWriteResponse; import org.elasticsearch.action.bulk.BulkItemResponse.Failure; @@ -99,8 +100,11 @@ public class BulkItemResponseTests extends ESTestCase { final Tuple exceptions = randomExceptions(); - BulkItemResponse bulkItemResponse = new BulkItemResponse(itemId, opType, new Failure(index, type, id, (Exception) exceptions.v1())); - BulkItemResponse expectedBulkItemResponse = new BulkItemResponse(itemId, opType, new Failure(index, type, id, exceptions.v2())); + Exception bulkItemCause = (Exception) exceptions.v1(); + Failure bulkItemFailure = new Failure(index, type, id, bulkItemCause); + BulkItemResponse bulkItemResponse = new BulkItemResponse(itemId, opType, bulkItemFailure); + Failure expectedBulkItemFailure = new Failure(index, type, id, exceptions.v2(), ExceptionsHelper.status(bulkItemCause)); + BulkItemResponse expectedBulkItemResponse = new BulkItemResponse(itemId, opType, expectedBulkItemFailure); BytesReference originalBytes = toXContent(bulkItemResponse, xContentType, randomBoolean()); // Shuffle the XContent fields