Skip to content

Remove deprecated created and found from index, delete and bulk #25516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 7, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
package org.elasticsearch.action.delete;

import org.elasticsearch.action.DocWriteResponse;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.rest.RestStatus;
Expand All @@ -37,8 +36,6 @@
*/
public class DeleteResponse extends DocWriteResponse {

private static final String FOUND = "found";

public DeleteResponse() {
}

Expand All @@ -64,13 +61,6 @@ public String toString() {
return builder.append("]").toString();
}

@Override
public XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws IOException {
builder.field(FOUND, result == Result.DELETED);
super.innerToXContent(builder, params);
return builder;
}

public static DeleteResponse fromXContent(XContentParser parser) throws IOException {
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);

Expand All @@ -85,16 +75,7 @@ public static DeleteResponse fromXContent(XContentParser parser) throws IOExcept
* Parse the current token and update the parsing context appropriately.
*/
public static void parseXContentFields(XContentParser parser, Builder context) throws IOException {
XContentParser.Token token = parser.currentToken();
String currentFieldName = parser.currentName();

if (FOUND.equals(currentFieldName)) {
if (token.isValue()) {
context.setFound(parser.booleanValue());
}
} else {
DocWriteResponse.parseInnerToXContent(parser, context);
}
DocWriteResponse.parseInnerToXContent(parser, context);
}

/**
Expand All @@ -104,15 +85,10 @@ public static void parseXContentFields(XContentParser parser, Builder context) t
*/
public static class Builder extends DocWriteResponse.Builder {

private boolean found = false;

public void setFound(boolean found) {
this.found = found;
}

@Override
public DeleteResponse build() {
DeleteResponse deleteResponse = new DeleteResponse(shardId, type, id, seqNo, primaryTerm, version, found);
DeleteResponse deleteResponse = new DeleteResponse(shardId, type, id, seqNo, primaryTerm, version,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should pass the Result here. Maybe also add an assertion in the ctor that the result is either Result.DELETED or Result.NOT_FOUND.

result == Result.DELETED ? true : false);
deleteResponse.setForcedRefresh(forcedRefresh);
if (shardInfo != null) {
deleteResponse.setShardInfo(shardInfo);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import org.elasticsearch.action.DocWriteResponse;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.rest.RestStatus;
Expand All @@ -38,8 +37,6 @@
*/
public class IndexResponse extends DocWriteResponse {

private static final String CREATED = "created";

public IndexResponse() {
}

Expand Down Expand Up @@ -67,13 +64,6 @@ public String toString() {
return builder.append("]").toString();
}

@Override
public XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws IOException {
super.innerToXContent(builder, params);
builder.field(CREATED, result == Result.CREATED);
return builder;
}

public static IndexResponse fromXContent(XContentParser parser) throws IOException {
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);

Expand All @@ -88,16 +78,7 @@ public static IndexResponse fromXContent(XContentParser parser) throws IOExcepti
* Parse the current token and update the parsing context appropriately.
*/
public static void parseXContentFields(XContentParser parser, Builder context) throws IOException {
XContentParser.Token token = parser.currentToken();
String currentFieldName = parser.currentName();

if (CREATED.equals(currentFieldName)) {
if (token.isValue()) {
context.setCreated(parser.booleanValue());
}
} else {
DocWriteResponse.parseInnerToXContent(parser, context);
}
DocWriteResponse.parseInnerToXContent(parser, context);
}

/**
Expand All @@ -107,15 +88,10 @@ public static void parseXContentFields(XContentParser parser, Builder context) t
*/
public static class Builder extends DocWriteResponse.Builder {

private boolean created = false;

public void setCreated(boolean created) {
this.created = created;
}

@Override
public IndexResponse build() {
IndexResponse indexResponse = new IndexResponse(shardId, type, id, seqNo, primaryTerm, version, created);
IndexResponse indexResponse = new IndexResponse(shardId, type, id, seqNo, primaryTerm, version,
result == Result.CREATED ? true : false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same deal here as on Delete. I think you should pass the result into the ctor and assert that it is sane.

indexResponse.setForcedRefresh(forcedRefresh);
if (shardInfo != null) {
indexResponse.setShardInfo(shardInfo);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ public void testToXContent() {
{
DeleteResponse response = new DeleteResponse(new ShardId("index", "index_uuid", 0), "type", "id", 3, 17, 5, true);
String output = Strings.toString(response);
assertEquals("{\"found\":true,\"_index\":\"index\",\"_type\":\"type\",\"_id\":\"id\",\"_version\":5,\"result\":\"deleted\"," +
assertEquals("{\"_index\":\"index\",\"_type\":\"type\",\"_id\":\"id\",\"_version\":5,\"result\":\"deleted\"," +
"\"_shards\":null,\"_seq_no\":3,\"_primary_term\":17}", output);
}
{
DeleteResponse response = new DeleteResponse(new ShardId("index", "index_uuid", 0), "type", "id", -1, 0, 7, true);
response.setForcedRefresh(true);
response.setShardInfo(new ReplicationResponse.ShardInfo(10, 5));
String output = Strings.toString(response);
assertEquals("{\"found\":true,\"_index\":\"index\",\"_type\":\"type\",\"_id\":\"id\",\"_version\":7,\"result\":\"deleted\"," +
assertEquals("{\"_index\":\"index\",\"_type\":\"type\",\"_id\":\"id\",\"_version\":7,\"result\":\"deleted\"," +
"\"forced_refresh\":true,\"_shards\":{\"total\":10,\"successful\":5,\"failed\":0}}", output);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ public void testToXContent() {
IndexResponse indexResponse = new IndexResponse(new ShardId("index", "index_uuid", 0), "type", "id", 3, 17, 5, true);
String output = Strings.toString(indexResponse);
assertEquals("{\"_index\":\"index\",\"_type\":\"type\",\"_id\":\"id\",\"_version\":5,\"result\":\"created\",\"_shards\":null," +
"\"_seq_no\":3,\"_primary_term\":17,\"created\":true}", output);
"\"_seq_no\":3,\"_primary_term\":17}", output);
}
{
IndexResponse indexResponse = new IndexResponse(new ShardId("index", "index_uuid", 0), "type", "id", -1, 17, 7, true);
indexResponse.setForcedRefresh(true);
indexResponse.setShardInfo(new ReplicationResponse.ShardInfo(10, 5));
String output = Strings.toString(indexResponse);
assertEquals("{\"_index\":\"index\",\"_type\":\"type\",\"_id\":\"id\",\"_version\":7,\"result\":\"created\"," +
"\"forced_refresh\":true,\"_shards\":{\"total\":10,\"successful\":5,\"failed\":0},\"created\":true}", output);
"\"forced_refresh\":true,\"_shards\":{\"total\":10,\"successful\":5,\"failed\":0}}", output);
}
}

Expand Down
3 changes: 0 additions & 3 deletions docs/reference/docs/bulk.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,13 @@ The result of this bulk operation is:
"successful": 1,
"failed": 0
},
"created": true,
"status": 201,
"_seq_no" : 0,
"_primary_term": 1
}
},
{
"delete": {
"found": false,
"_index": "test",
"_type": "type1",
"_id": "2",
Expand Down Expand Up @@ -138,7 +136,6 @@ The result of this bulk operation is:
"successful": 1,
"failed": 0
},
"created": true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm so glad we assert that these are right now!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly the kind of thing I'd miss when removing these without the assertions.

"status": 201,
"_seq_no" : 2,
"_primary_term" : 3
Expand Down
1 change: 0 additions & 1 deletion docs/reference/docs/delete.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ The result of the above delete operation is:
"failed" : 0,
"successful" : 2
},
"found" : true,
"_index" : "twitter",
"_type" : "tweet",
"_id" : "1",
Expand Down
2 changes: 0 additions & 2 deletions docs/reference/docs/index_.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ The result of the above index operation is:
"_type" : "tweet",
"_id" : "1",
"_version" : 1,
"created" : true,
"_seq_no" : 0,
"_primary_term" : 1,
"result" : created
Expand Down Expand Up @@ -230,7 +229,6 @@ The result of the above index operation is:
"_type" : "tweet",
"_id" : "6a8ca01c-7896-48e9-81cc-9f70661fcb32",
"_version" : 1,
"created" : true,
"_seq_no" : 0,
"_primary_term" : 1,
"result": "created"
Expand Down
1 change: 0 additions & 1 deletion docs/reference/getting-started.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,6 @@ And the response:
"successful" : 1,
"failed" : 0
},
"created" : true,
"_seq_no" : 0,
"_primary_term" : 1
}
Expand Down
2 changes: 0 additions & 2 deletions docs/reference/ingest/ingest-node.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,6 @@ PUT /myindex/type/1?pipeline=monthlyindex
"successful" : 1,
"failed" : 0
},
"created" : true,
"_seq_no" : 0,
"_primary_term" : 1
}
Expand Down Expand Up @@ -1831,7 +1830,6 @@ The response from the above index request:
},
"_seq_no": 0,
"_primary_term": 1,
"created": true
}
--------------------------------------------------
// TESTRESPONSE
Expand Down
12 changes: 12 additions & 0 deletions docs/reference/migration/migrate_6_0/docs.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,15 @@ Document modification operations may no longer specify the `version_type` of
==== <<upserts,Upserts>> no longer support versions

Adding a `version` to an upsert request is no longer supported.

==== `created` field removed in the Index API

The `created` field has been removed in the Index API as in the `index` and
`create` bulk operations. `operation` field should be used instead.


==== `found` field removed in the Delete API

The `found` field has been removed in the Delete API as in the `delete` bulk
operations. `operation` field should be used instead.

1 change: 0 additions & 1 deletion docs/reference/query-dsl/percolate-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ Index response:
"successful": 1,
"failed": 0
},
"created": true,
"result": "created",
"_seq_no" : 0,
"_primary_term" : 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@
- match: { items.0.index.status: 400 }
- match: { items.0.index.error.type: illegal_argument_exception }
- match: { items.0.index.error.reason: if _id is specified it must not be empty }
- match: { items.1.index.created: true }
- match: { items.2.index.created: true }
- match: { items.1.index.result: created }
- match: { items.2.index.result: created }

- do:
count:
Expand Down