Skip to content

Commit

Permalink
Remove isCreated and isFound from the Java API
Browse files Browse the repository at this point in the history 
This is cleanup work from elastic#19566, where @nik9000 suggested trying to nuke the isCreated and isFound methods. I've combined nuking the two methods with removing UpdateHelper.Operation in favor of DocWriteResponse.Operation here.

Closes elastic#19631.
  • Loading branch information
@a2lin @nik9000
a2lin authored and nik9000 committed Jul 29, 2016
1 parent c9790a1 commit 119026b
Show file tree
Hide file tree
Showing 31 changed files with 131 additions and 145 deletions.
10 changes: 5 additions & 5 deletions core/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ private Tuple<Translog.Location, BulkItemRequest> update(IndexMetaData metaData,
location = locationToSync(location, updateResult.writeResult.getLocation());
}
switch (updateResult.result.operation()) {
case UPSERT:
case CREATE:
case INDEX:
@SuppressWarnings("unchecked")
WriteResult<IndexResponse> result = updateResult.writeResult;
Expand All @@ -267,7 +267,7 @@ private Tuple<Translog.Location, BulkItemRequest> update(IndexMetaData metaData,
item = request.items()[requestIndex] = new BulkItemRequest(request.items()[requestIndex].id(), deleteRequest);
setResponse(item, new BulkItemResponse(item.id(), OP_TYPE_UPDATE, updateResponse));
break;
case NONE:
case NOOP:
setResponse(item, new BulkItemResponse(item.id(), OP_TYPE_UPDATE, updateResult.noopResult));
item.setIgnoreOnReplica(); // no need to go to the replica
break;
Expand Down Expand Up @@ -300,7 +300,7 @@ private Tuple<Translog.Location, BulkItemRequest> update(IndexMetaData metaData,
setResponse(item, new BulkItemResponse(item.id(), OP_TYPE_UPDATE, new BulkItemResponse.Failure(request.index(), updateRequest.type(), updateRequest.id(), e)));
} else {
switch (updateResult.result.operation()) {
case UPSERT:
case CREATE:
case INDEX:
IndexRequest indexRequest = updateResult.request();
logFailure(e, "index", request.shardId(), indexRequest);
Expand Down Expand Up @@ -400,7 +400,7 @@ <T extends ActionRequest> T request() {
private UpdateResult shardUpdateOperation(IndexMetaData metaData, BulkShardRequest bulkShardRequest, UpdateRequest updateRequest, IndexShard indexShard) {
UpdateHelper.Result translate = updateHelper.prepare(updateRequest, indexShard);
switch (translate.operation()) {
case UPSERT:
case CREATE:
case INDEX:
IndexRequest indexRequest = translate.action();
try {
Expand All @@ -427,7 +427,7 @@ private UpdateResult shardUpdateOperation(IndexMetaData metaData, BulkShardReque
}
return new UpdateResult(translate, deleteRequest, retry, cause, null);
}
case NONE:
case NOOP:
UpdateResponse updateResponse = translate.action();
indexShard.noopUpdate(updateRequest.type());
return new UpdateResult(translate, updateResponse);
Expand Down
11 changes: 2 additions & 9 deletions core/src/main/java/org/elasticsearch/action/delete/DeleteResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,14 @@ public DeleteResponse(ShardId shardId, String type, String id, long version, boo
super(shardId, type, id, version, found ? Operation.DELETE : Operation.NOOP);
}

/**
* Returns <tt>true</tt> if a doc was found to delete.
*/
public boolean isFound() {
return operation == Operation.DELETE;
}

@Override
public RestStatus status() {
return isFound() ? super.status() : RestStatus.NOT_FOUND;
return operation == Operation.DELETE ? super.status() : RestStatus.NOT_FOUND;
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.field("found", isFound());
builder.field("found", operation == Operation.DELETE);
super.toXContent(builder, params);
return builder;
}
Expand Down
13 changes: 2 additions & 11 deletions core/src/main/java/org/elasticsearch/action/index/IndexResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
package org.elasticsearch.action.index;

import org.elasticsearch.action.DocWriteResponse;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.rest.RestStatus;
Expand All @@ -44,16 +42,9 @@ public IndexResponse(ShardId shardId, String type, String id, long version, bool
super(shardId, type, id, version, created ? Operation.CREATE : Operation.INDEX);
}

/**
* Returns true if the document was created, false if updated.
*/
public boolean isCreated() {
return this.operation == Operation.CREATE;
}

@Override
public RestStatus status() {
return isCreated() ? RestStatus.CREATED : super.status();
return operation == Operation.CREATE ? RestStatus.CREATED : super.status();
}

@Override
Expand All @@ -72,7 +63,7 @@ public String toString() {
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
super.toXContent(builder, params);
builder.field("created", isCreated());
builder.field("created", operation == Operation.CREATE);
return builder;
}
}
4 changes: 2 additions & 2 deletions core/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ protected void shardOperation(final UpdateRequest request, final ActionListener<
final IndexShard indexShard = indexService.getShard(shardId.getId());
final UpdateHelper.Result result = updateHelper.prepare(request, indexShard);
switch (result.operation()) {
case UPSERT:
case CREATE:
IndexRequest upsertRequest = result.action();
// we fetch it from the index request so we don't generate the bytes twice, its already done in the index request
final BytesReference upsertSourceBytes = upsertRequest.source();
Expand Down Expand Up @@ -277,7 +277,7 @@ protected void doRun() {
}
});
break;
case NONE:
case NOOP:
UpdateResponse update = result.action();
IndexService indexServiceOrNull = indicesService.indexService(shardId.getIndex());
if (indexServiceOrNull != null) {
Expand Down
31 changes: 13 additions & 18 deletions core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.action.update;

import org.elasticsearch.action.DocWriteResponse;
import org.elasticsearch.action.delete.DeleteRequest;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.client.Requests;
Expand Down Expand Up @@ -116,9 +117,9 @@ protected Result prepare(ShardId shardId, UpdateRequest request, final GetResult
request.script.getScript());
}
UpdateResponse update = new UpdateResponse(shardId, getResult.getType(), getResult.getId(),
getResult.getVersion(), UpdateResponse.convert(Operation.NONE));
getResult.getVersion(), DocWriteResponse.Operation.NOOP);
update.setGetResult(getResult);
return new Result(update, Operation.NONE, upsertDoc, XContentType.JSON);
return new Result(update, DocWriteResponse.Operation.NOOP, upsertDoc, XContentType.JSON);
}
indexRequest.source((Map) ctx.get("_source"));
}
Expand All @@ -135,7 +136,7 @@ protected Result prepare(ShardId shardId, UpdateRequest request, final GetResult
// in all but the internal versioning mode, we want to create the new document using the given version.
indexRequest.version(request.version()).versionType(request.versionType());
}
return new Result(indexRequest, Operation.UPSERT, null, null);
return new Result(indexRequest, DocWriteResponse.Operation.CREATE, null, null);
}

long updateVersion = getResult.getVersion();
Expand Down Expand Up @@ -226,21 +227,21 @@ protected Result prepare(ShardId shardId, UpdateRequest request, final GetResult
.consistencyLevel(request.consistencyLevel())
.timestamp(timestamp).ttl(ttl)
.setRefreshPolicy(request.getRefreshPolicy());
return new Result(indexRequest, Operation.INDEX, updatedSourceAsMap, updateSourceContentType);
return new Result(indexRequest, DocWriteResponse.Operation.INDEX, updatedSourceAsMap, updateSourceContentType);
} else if ("delete".equals(operation)) {
DeleteRequest deleteRequest = Requests.deleteRequest(request.index()).type(request.type()).id(request.id()).routing(routing).parent(parent)
.version(updateVersion).versionType(request.versionType())
.consistencyLevel(request.consistencyLevel())
.setRefreshPolicy(request.getRefreshPolicy());
return new Result(deleteRequest, Operation.DELETE, updatedSourceAsMap, updateSourceContentType);
return new Result(deleteRequest, DocWriteResponse.Operation.DELETE, updatedSourceAsMap, updateSourceContentType);
} else if ("none".equals(operation)) {
UpdateResponse update = new UpdateResponse(shardId, getResult.getType(), getResult.getId(), getResult.getVersion(), UpdateResponse.convert(Operation.NONE));
UpdateResponse update = new UpdateResponse(shardId, getResult.getType(), getResult.getId(), getResult.getVersion(), DocWriteResponse.Operation.NOOP);
update.setGetResult(extractGetResult(request, request.index(), getResult.getVersion(), updatedSourceAsMap, updateSourceContentType, getResult.internalSourceRef()));
return new Result(update, Operation.NONE, updatedSourceAsMap, updateSourceContentType);
return new Result(update, DocWriteResponse.Operation.NOOP, updatedSourceAsMap, updateSourceContentType);
} else {
logger.warn("Used update operation [{}] for script [{}], doing nothing...", operation, request.script.getScript());
UpdateResponse update = new UpdateResponse(shardId, getResult.getType(), getResult.getId(), getResult.getVersion(), UpdateResponse.convert(Operation.NONE));
return new Result(update, Operation.NONE, updatedSourceAsMap, updateSourceContentType);
UpdateResponse update = new UpdateResponse(shardId, getResult.getType(), getResult.getId(), getResult.getVersion(), DocWriteResponse.Operation.NOOP);
return new Result(update, DocWriteResponse.Operation.NOOP, updatedSourceAsMap, updateSourceContentType);
}
}

Expand Down Expand Up @@ -309,11 +310,11 @@ public GetResult extractGetResult(final UpdateRequest request, String concreteIn
public static class Result {

private final Streamable action;
private final Operation operation;
private final DocWriteResponse.Operation operation;
private final Map<String, Object> updatedSourceAsMap;
private final XContentType updateSourceContentType;

public Result(Streamable action, Operation operation, Map<String, Object> updatedSourceAsMap, XContentType updateSourceContentType) {
public Result(Streamable action, DocWriteResponse.Operation operation, Map<String, Object> updatedSourceAsMap, XContentType updateSourceContentType) {
this.action = action;
this.operation = operation;
this.updatedSourceAsMap = updatedSourceAsMap;
Expand All @@ -325,7 +326,7 @@ public <T extends Streamable> T action() {
return (T) action;
}

public Operation operation() {
public DocWriteResponse.Operation operation() {
return operation;
}

Expand All @@ -338,10 +339,4 @@ public XContentType updateSourceContentType() {
}
}

public enum Operation {
UPSERT,
INDEX,
DELETE,
NONE
}
}
23 changes: 1 addition & 22 deletions core/src/main/java/org/elasticsearch/action/update/UpdateResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,6 @@ public UpdateResponse(ShardInfo shardInfo, ShardId shardId, String type, String
setShardInfo(shardInfo);
}

public static Operation convert(UpdateHelper.Operation op) {
switch(op) {
case UPSERT:
return Operation.CREATE;
case INDEX:
return Operation.INDEX;
case DELETE:
return Operation.DELETE;
case NONE:
return Operation.NOOP;
}
throw new IllegalArgumentException();
}

public void setGetResult(GetResult getResult) {
this.getResult = getResult;
}
Expand All @@ -72,16 +58,9 @@ public GetResult getGetResult() {
return this.getResult;
}

/**
* Returns true if document was created due to an UPSERT operation
*/
public boolean isCreated() {
return this.operation == Operation.CREATE;
}

@Override
public RestStatus status() {
return isCreated() ? RestStatus.CREATED : super.status();
return this.operation == Operation.CREATE ? RestStatus.CREATED : super.status();
}

@Override
Expand Down
6 changes: 3 additions & 3 deletions core/src/test/java/org/elasticsearch/action/IndicesRequestIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ public void testUpdate() {
client().prepareIndex(indexOrAlias, "type", "id").setSource("field", "value").get();
UpdateRequest updateRequest = new UpdateRequest(indexOrAlias, "type", "id").doc("field1", "value1");
UpdateResponse updateResponse = internalCluster().coordOnlyNodeClient().update(updateRequest).actionGet();
assertThat(updateResponse.isCreated(), equalTo(false));
assertEquals(DocWriteResponse.Operation.INDEX, updateResponse.getOperation());

clearInterceptedActions();
assertSameIndices(updateRequest, updateShardActions);
Expand All @@ -248,7 +248,7 @@ public void testUpdateUpsert() {
String indexOrAlias = randomIndexOrAlias();
UpdateRequest updateRequest = new UpdateRequest(indexOrAlias, "type", "id").upsert("field", "value").doc("field1", "value1");
UpdateResponse updateResponse = internalCluster().coordOnlyNodeClient().update(updateRequest).actionGet();
assertThat(updateResponse.isCreated(), equalTo(true));
assertEquals(DocWriteResponse.Operation.CREATE, updateResponse.getOperation());

clearInterceptedActions();
assertSameIndices(updateRequest, updateShardActions);
Expand All @@ -264,7 +264,7 @@ public void testUpdateDelete() {
UpdateRequest updateRequest = new UpdateRequest(indexOrAlias, "type", "id")
.script(new Script("ctx.op='delete'", ScriptService.ScriptType.INLINE, CustomScriptPlugin.NAME, Collections.emptyMap()));
UpdateResponse updateResponse = internalCluster().coordOnlyNodeClient().update(updateRequest).actionGet();
assertThat(updateResponse.isCreated(), equalTo(false));
assertEquals(DocWriteResponse.Operation.DELETE, updateResponse.getOperation());

clearInterceptedActions();
assertSameIndices(updateRequest, updateShardActions);
Expand Down
14 changes: 7 additions & 7 deletions core/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@

package org.elasticsearch.action.bulk;

import org.elasticsearch.action.DocWriteResponse;
import org.elasticsearch.action.admin.indices.alias.Alias;
import org.elasticsearch.action.delete.DeleteRequest;
import org.elasticsearch.action.get.GetResponse;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.index.IndexResponse;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.support.WriteRequest.RefreshPolicy;
import org.elasticsearch.action.update.UpdateRequest;
Expand Down Expand Up @@ -207,11 +207,11 @@ public void testBulkVersioning() throws Exception {
.add(client().prepareIndex("test", "type", "2").setCreate(true).setSource("field", "1"))
.add(client().prepareIndex("test", "type", "1").setSource("field", "2")).get();

assertTrue(((IndexResponse) bulkResponse.getItems()[0].getResponse()).isCreated());
assertEquals(DocWriteResponse.Operation.CREATE, bulkResponse.getItems()[0].getResponse().getOperation());
assertThat(bulkResponse.getItems()[0].getResponse().getVersion(), equalTo(1L));
assertTrue(((IndexResponse) bulkResponse.getItems()[1].getResponse()).isCreated());
assertEquals(DocWriteResponse.Operation.CREATE, bulkResponse.getItems()[1].getResponse().getOperation());
assertThat(bulkResponse.getItems()[1].getResponse().getVersion(), equalTo(1L));
assertFalse(((IndexResponse) bulkResponse.getItems()[2].getResponse()).isCreated());
assertEquals(DocWriteResponse.Operation.INDEX, bulkResponse.getItems()[2].getResponse().getOperation());
assertThat(bulkResponse.getItems()[2].getResponse().getVersion(), equalTo(2L));

bulkResponse = client().prepareBulk()
Expand All @@ -232,11 +232,11 @@ public void testBulkVersioning() throws Exception {
.setSource("field", "2").setVersion(12).setVersionType(VersionType.EXTERNAL))
.get();

assertTrue(((IndexResponse) bulkResponse.getItems()[0].getResponse()).isCreated());
assertEquals(DocWriteResponse.Operation.CREATE, bulkResponse.getItems()[0].getResponse().getOperation());
assertThat(bulkResponse.getItems()[0].getResponse().getVersion(), equalTo(10L));
assertTrue(((IndexResponse) bulkResponse.getItems()[1].getResponse()).isCreated());
assertEquals(DocWriteResponse.Operation.CREATE, bulkResponse.getItems()[1].getResponse().getOperation());
assertThat(bulkResponse.getItems()[1].getResponse().getVersion(), equalTo(10L));
assertFalse(((IndexResponse) bulkResponse.getItems()[2].getResponse()).isCreated());
assertEquals(DocWriteResponse.Operation.INDEX, bulkResponse.getItems()[2].getResponse().getOperation());
assertThat(bulkResponse.getItems()[2].getResponse().getVersion(), equalTo(12L));

bulkResponse = client().prepareBulk()
Expand Down
3 changes: 2 additions & 1 deletion core/src/test/java/org/elasticsearch/action/support/master/IndexingMasterFailoverIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* under the License.
*/

import org.elasticsearch.action.DocWriteResponse;
import org.elasticsearch.action.index.IndexResponse;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.discovery.DiscoverySettings;
Expand Down Expand Up @@ -97,7 +98,7 @@ public void run() {
for (int i = 0; i < 10; i++) {
// index data with mapping changes
IndexResponse response = client(dataNode).prepareIndex("myindex", "mytype").setSource("field_" + i, "val").get();
assertThat(response.isCreated(), equalTo(true));
assertEquals(DocWriteResponse.Operation.CREATE, response.getOperation());
}
}
});
Expand Down
Loading

0 comments on commit 119026b

Please sign in to comment.