Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Deprecate types in update requests. #36181

Merged
merged 5 commits into from
Dec 14, 2018
+232 −44
Merged

Deprecate types in update requests. #36181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f735cf1
Deprecate types in update requests.
jtibshirani Dec 3, 2018
c333648
Address review comments.
jtibshirani Dec 12, 2018
36dca3c
Add HLRC integration tests for the typed APIs.
jtibshirani Dec 12, 2018
23aeeff
Test conversion for both typed and typeless HLRC requests.
jtibshirani Dec 12, 2018
574b085
Fix failing integration tests.
jtibshirani Dec 12, 2018
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
5 changes: 4 additions & 1 deletion client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java
View file
Open in desktop
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
Open in desktop
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");
Copy link
Contributor

@markharwood markharwood Dec 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Is it worth keeping a deprecated example usage in this test?
I think there's some work to be done to make ESRestTestCase support assertWarnings checks first (currently have to use getStrictDeprecationMode=false to avoid test failures when making deprecated calls)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do, this shouldn't be hard now that we are able to check warnings in REST integration tests.

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
Open in desktop
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