Skip to content

Commit

Permalink
Deprecate types in update requests. (#36181)
Browse files Browse the repository at this point in the history 
The following updates were made:
* Add deprecation warnings to `RestUpdateAction`, plus a test in `RestUpdateActionTests`.
* Deprecate relevant methods on the Java HLRC requests/ responses.
* Add HLRC integration tests for the typed APIs.
* Update documentation (for both the REST API and Java HLRC).
* Fix failing integration tests.

Because of an earlier PR, the REST yml tests were already updated (one version without types, and another legacy version that retains types).
  • Loading branch information
@jtibshirani
jtibshirani committed Dec 14, 2018
1 parent 68a674e commit ccd1beb
Show file tree
Hide file tree
Showing 14 changed files with 232 additions and 44 deletions.
5 changes: 4 additions & 1 deletion client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.VersionType;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.rankeval.RankEvalRequest;
import org.elasticsearch.index.reindex.AbstractBulkByScrollRequest;
import org.elasticsearch.index.reindex.DeleteByQueryRequest;
Expand Down Expand Up @@ -316,7 +317,9 @@ static Request ping() {
}

static Request update(UpdateRequest updateRequest) throws IOException {
String endpoint = endpoint(updateRequest.index(), updateRequest.type(), updateRequest.id(), "_update");
String endpoint = updateRequest.type().equals(MapperService.SINGLE_MAPPING_NAME)
? endpoint(updateRequest.index(), "_update", updateRequest.id())
: endpoint(updateRequest.index(), updateRequest.type(), updateRequest.id(), "_update");
Request request = new Request(HttpPost.METHOD_NAME, endpoint);

Params parameters = new Params(request);
Expand Down
114 changes: 102 additions & 12 deletions client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@
import org.elasticsearch.index.reindex.UpdateByQueryAction;
import org.elasticsearch.index.reindex.UpdateByQueryRequest;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.rest.action.document.RestDeleteAction;
import org.elasticsearch.rest.action.document.RestGetAction;
import org.elasticsearch.rest.action.document.RestMultiGetAction;
import org.elasticsearch.rest.action.document.RestUpdateAction;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptType;
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
Expand Down Expand Up @@ -173,6 +177,23 @@ public void testDelete() throws IOException {
}
}

public void testDeleteWithTypes() throws IOException {
String docId = "id";
highLevelClient().index(new IndexRequest("index", "type", docId)
.source(Collections.singletonMap("foo", "bar")), RequestOptions.DEFAULT);

DeleteRequest deleteRequest = new DeleteRequest("index", "type", docId);
DeleteResponse deleteResponse = execute(deleteRequest,
highLevelClient()::delete,
highLevelClient()::deleteAsync,
expectWarnings(RestDeleteAction.TYPES_DEPRECATION_MESSAGE));

assertEquals("index", deleteResponse.getIndex());
assertEquals("type", deleteResponse.getType());
assertEquals(docId, deleteResponse.getId());
assertEquals(DocWriteResponse.Result.DELETED, deleteResponse.getResult());
}

public void testExists() throws IOException {
{
GetRequest getRequest = new GetRequest("index", "id");
Expand Down Expand Up @@ -331,6 +352,29 @@ public void testGet() throws IOException {
}
}

public void testGetWithTypes() throws IOException {
String document = "{\"field\":\"value\"}";
IndexRequest index = new IndexRequest("index", "type", "id");
index.source(document, XContentType.JSON);
index.setRefreshPolicy(RefreshPolicy.IMMEDIATE);
highLevelClient().index(index, RequestOptions.DEFAULT);

GetRequest getRequest = new GetRequest("index", "type", "id");
GetResponse getResponse = execute(getRequest,
highLevelClient()::get,
highLevelClient()::getAsync,
expectWarnings(RestGetAction.TYPES_DEPRECATION_MESSAGE));

assertEquals("index", getResponse.getIndex());
assertEquals("type", getResponse.getType());
assertEquals("id", getResponse.getId());

assertTrue(getResponse.isExists());
assertFalse(getResponse.isSourceEmpty());
assertEquals(1L, getResponse.getVersion());
assertEquals(document, getResponse.getSourceAsString());
}

public void testMultiGet() throws IOException {
{
MultiGetRequest multiGetRequest = new MultiGetRequest();
Expand Down Expand Up @@ -387,6 +431,36 @@ public void testMultiGet() throws IOException {
}
}

public void testMultiGetWithTypes() throws IOException {
BulkRequest bulk = new BulkRequest();
bulk.setRefreshPolicy(RefreshPolicy.IMMEDIATE);
bulk.add(new IndexRequest("index", "type", "id1")
.source("{\"field\":\"value1\"}", XContentType.JSON));
bulk.add(new IndexRequest("index", "type", "id2")
.source("{\"field\":\"value2\"}", XContentType.JSON));

highLevelClient().bulk(bulk, RequestOptions.DEFAULT);
MultiGetRequest multiGetRequest = new MultiGetRequest();
multiGetRequest.add("index", "id1");
multiGetRequest.add("index", "type", "id2");

MultiGetResponse response = execute(multiGetRequest,
highLevelClient()::mget,
highLevelClient()::mgetAsync,
expectWarnings(RestMultiGetAction.TYPES_DEPRECATION_MESSAGE));
assertEquals(2, response.getResponses().length);

GetResponse firstResponse = response.getResponses()[0].getResponse();
assertEquals("index", firstResponse.getIndex());
assertEquals("type", firstResponse.getType());
assertEquals("id1", firstResponse.getId());

GetResponse secondResponse = response.getResponses()[1].getResponse();
assertEquals("index", secondResponse.getIndex());
assertEquals("type", secondResponse.getType());
assertEquals("id2", secondResponse.getId());
}

public void testIndex() throws IOException {
final XContentType xContentType = randomFrom(XContentType.values());
{
Expand Down Expand Up @@ -492,7 +566,7 @@ public void testIndex() throws IOException {

public void testUpdate() throws IOException {
{
UpdateRequest updateRequest = new UpdateRequest("index", "_doc", "does_not_exist");
UpdateRequest updateRequest = new UpdateRequest("index", "does_not_exist");
updateRequest.doc(singletonMap("field", "value"), randomFrom(XContentType.values()));

ElasticsearchStatusException exception = expectThrows(ElasticsearchStatusException.class, () ->
Expand All @@ -507,14 +581,14 @@ public void testUpdate() throws IOException {
IndexResponse indexResponse = highLevelClient().index(indexRequest, RequestOptions.DEFAULT);
assertEquals(RestStatus.CREATED, indexResponse.status());

UpdateRequest updateRequest = new UpdateRequest("index", "_doc", "id");
UpdateRequest updateRequest = new UpdateRequest("index", "id");
updateRequest.doc(singletonMap("field", "updated"), randomFrom(XContentType.values()));

UpdateResponse updateResponse = execute(updateRequest, highLevelClient()::update, highLevelClient()::updateAsync);
assertEquals(RestStatus.OK, updateResponse.status());
assertEquals(indexResponse.getVersion() + 1, updateResponse.getVersion());

UpdateRequest updateRequestConflict = new UpdateRequest("index", "_doc", "id");
UpdateRequest updateRequestConflict = new UpdateRequest("index", "id");
updateRequestConflict.doc(singletonMap("field", "with_version_conflict"), randomFrom(XContentType.values()));
updateRequestConflict.version(indexResponse.getVersion());

Expand All @@ -530,7 +604,7 @@ public void testUpdate() throws IOException {
IndexResponse indexResponse = highLevelClient().index(indexRequest, RequestOptions.DEFAULT);
assertEquals(RestStatus.CREATED, indexResponse.status());

UpdateRequest updateRequest = new UpdateRequest("index", "_doc", "with_script");
UpdateRequest updateRequest = new UpdateRequest("index", "with_script");
Script script = new Script(ScriptType.INLINE, "painless", "ctx._source.counter += params.count", singletonMap("count", 8));
updateRequest.script(script);
updateRequest.fetchSource(true);
Expand All @@ -551,7 +625,7 @@ public void testUpdate() throws IOException {
assertEquals(RestStatus.CREATED, indexResponse.status());
assertEquals(12L, indexResponse.getVersion());

UpdateRequest updateRequest = new UpdateRequest("index", "_doc", "with_doc");
UpdateRequest updateRequest = new UpdateRequest("index", "with_doc");
updateRequest.doc(singletonMap("field_2", "two"), randomFrom(XContentType.values()));
updateRequest.fetchSource("field_*", "field_3");

Expand All @@ -573,7 +647,7 @@ public void testUpdate() throws IOException {
assertEquals(RestStatus.CREATED, indexResponse.status());
assertEquals(1L, indexResponse.getVersion());

UpdateRequest updateRequest = new UpdateRequest("index", "_doc", "noop");
UpdateRequest updateRequest = new UpdateRequest("index", "noop");
updateRequest.doc(singletonMap("field", "value"), randomFrom(XContentType.values()));

UpdateResponse updateResponse = execute(updateRequest, highLevelClient()::update, highLevelClient()::updateAsync);
Expand All @@ -589,7 +663,7 @@ public void testUpdate() throws IOException {
assertEquals(2L, updateResponse.getVersion());
}
{
UpdateRequest updateRequest = new UpdateRequest("index", "_doc", "with_upsert");
UpdateRequest updateRequest = new UpdateRequest("index", "with_upsert");
updateRequest.upsert(singletonMap("doc_status", "created"));
updateRequest.doc(singletonMap("doc_status", "updated"));
updateRequest.fetchSource(true);
Expand All @@ -604,7 +678,7 @@ public void testUpdate() throws IOException {
assertEquals("created", getResult.sourceAsMap().get("doc_status"));
}
{
UpdateRequest updateRequest = new UpdateRequest("index", "_doc", "with_doc_as_upsert");
UpdateRequest updateRequest = new UpdateRequest("index", "with_doc_as_upsert");
updateRequest.doc(singletonMap("field", "initialized"));
updateRequest.fetchSource(true);
updateRequest.docAsUpsert(true);
Expand All @@ -619,7 +693,7 @@ public void testUpdate() throws IOException {
assertEquals("initialized", getResult.sourceAsMap().get("field"));
}
{
UpdateRequest updateRequest = new UpdateRequest("index", "_doc", "with_scripted_upsert");
UpdateRequest updateRequest = new UpdateRequest("index", "with_scripted_upsert");
updateRequest.fetchSource(true);
updateRequest.script(new Script(ScriptType.INLINE, "painless", "ctx._source.level = params.test", singletonMap("test", "C")));
updateRequest.scriptedUpsert(true);
Expand All @@ -637,7 +711,7 @@ public void testUpdate() throws IOException {
}
{
IllegalStateException exception = expectThrows(IllegalStateException.class, () -> {
UpdateRequest updateRequest = new UpdateRequest("index", "_doc", "id");
UpdateRequest updateRequest = new UpdateRequest("index", "id");
updateRequest.doc(new IndexRequest().source(Collections.singletonMap("field", "doc"), XContentType.JSON));
updateRequest.upsert(new IndexRequest().source(Collections.singletonMap("field", "upsert"), XContentType.YAML));
execute(updateRequest, highLevelClient()::update, highLevelClient()::updateAsync);
Expand All @@ -647,6 +721,22 @@ public void testUpdate() throws IOException {
}
}

public void testUpdateWithTypes() throws IOException {
IndexRequest indexRequest = new IndexRequest("index", "type", "id");
indexRequest.source(singletonMap("field", "value"));
IndexResponse indexResponse = highLevelClient().index(indexRequest, RequestOptions.DEFAULT);

UpdateRequest updateRequest = new UpdateRequest("index", "type", "id");
updateRequest.doc(singletonMap("field", "updated"), randomFrom(XContentType.values()));
UpdateResponse updateResponse = execute(updateRequest,
highLevelClient()::update,
highLevelClient()::updateAsync,
expectWarnings(RestUpdateAction.TYPES_DEPRECATION_MESSAGE));

assertEquals(RestStatus.OK, updateResponse.status());
assertEquals(indexResponse.getVersion() + 1, updateResponse.getVersion());
}

public void testBulk() throws IOException {
int nbItems = randomIntBetween(10, 100);
boolean[] errors = new boolean[nbItems];
Expand Down Expand Up @@ -687,7 +777,7 @@ public void testBulk() throws IOException {
bulkRequest.add(createRequest);

} else if (opType == DocWriteRequest.OpType.UPDATE) {
UpdateRequest updateRequest = new UpdateRequest("index", "_doc", id)
UpdateRequest updateRequest = new UpdateRequest("index", id)
.doc(new IndexRequest().source(source, xContentType));
if (erroneous == false) {
assertEquals(RestStatus.CREATED,
Expand Down Expand Up @@ -996,7 +1086,7 @@ public void afterBulk(long executionId, BulkRequest request, Throwable failure)
processor.add(createRequest);

} else if (opType == DocWriteRequest.OpType.UPDATE) {
UpdateRequest updateRequest = new UpdateRequest("index", "_doc", id)
UpdateRequest updateRequest = new UpdateRequest("index", id)
.doc(new IndexRequest().source(xContentType, "id", i));
if (erroneous == false) {
assertEquals(RestStatus.CREATED,
Expand Down
22 changes: 19 additions & 3 deletions client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -630,10 +630,9 @@ public void testUpdate() throws IOException {

Map<String, String> expectedParams = new HashMap<>();
String index = randomAlphaOfLengthBetween(3, 10);
String type = randomAlphaOfLengthBetween(3, 10);
String id = randomAlphaOfLengthBetween(3, 10);

UpdateRequest updateRequest = new UpdateRequest(index, type, id);
UpdateRequest updateRequest = new UpdateRequest(index, id);
updateRequest.detectNoop(randomBoolean());

if (randomBoolean()) {
Expand Down Expand Up @@ -687,7 +686,7 @@ public void testUpdate() throws IOException {
}

Request request = RequestConverters.update(updateRequest);
assertEquals("/" + index + "/" + type + "/" + id + "/_update", request.getEndpoint());
assertEquals("/" + index + "/_update/" + id, request.getEndpoint());
assertEquals(expectedParams, request.getParameters());
assertEquals(HttpPost.METHOD_NAME, request.getMethod());

Expand Down Expand Up @@ -718,6 +717,23 @@ public void testUpdate() throws IOException {
}
}

public void testUpdateWithType() throws IOException {
String index = randomAlphaOfLengthBetween(3, 10);
String type = randomAlphaOfLengthBetween(3, 10);
String id = randomAlphaOfLengthBetween(3, 10);

UpdateRequest updateRequest = new UpdateRequest(index, type, id);

XContentType xContentType = XContentType.JSON;
BytesReference source = RandomObjects.randomSource(random(), xContentType);
updateRequest.doc(new IndexRequest().source(source, xContentType));

Request request = RequestConverters.update(updateRequest);
assertEquals("/" + index + "/" + type + "/" + id + "/_update", request.getEndpoint());
assertEquals(HttpPost.METHOD_NAME, request.getMethod());
assertToXContentBody(updateRequest, request.getEntity());
}

public void testUpdateWithDifferentContentTypes() {
IllegalStateException exception = expectThrows(IllegalStateException.class, () -> {
UpdateRequest updateRequest = new UpdateRequest();
Expand Down
Loading

0 comments on commit ccd1beb

Please sign in to comment.