From c3238686b45ebd85184601e87d1569dfcd185143 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Fri, 7 Dec 2018 08:43:35 -0500 Subject: [PATCH 1/4] Deprecate types in update_by_query and delete_by_query Relates to #35190 --- .../client/RequestConverters.java | 10 ++-- docs/java-api/docs/update-by-query.asciidoc | 2 +- .../documentation/ReindexDocumentationIT.java | 2 +- .../reindex/RestDeleteByQueryActionTests.java | 38 +++++++++++++++ .../reindex/RestUpdateByQueryActionTests.java | 39 ++++++++++++++++ .../test/delete_by_query/10_basic.yml | 28 +++++------ .../test/delete_by_query/20_validation.yml | 6 +-- .../test/delete_by_query/40_versioning.yml | 4 +- .../50_wait_for_active_shards.yml | 2 +- .../test/delete_by_query/70_throttle.yml | 24 +++++----- .../test/delete_by_query/80_slices.yml | 36 +++++++-------- .../test/update_by_query/10_basic.yml | 32 ++++++------- .../test/update_by_query/20_validation.yml | 8 ++-- .../test/update_by_query/30_new_fields.yml | 4 +- .../update_by_query/35_search_failure.yml | 2 +- .../test/update_by_query/40_versioning.yml | 8 ++-- .../test/update_by_query/50_consistency.yml | 4 +- .../test/update_by_query/60_throttle.yml | 24 +++++----- .../test/update_by_query/70_slices.yml | 36 +++++++-------- .../test/update_by_query/80_scripting.yml | 46 +++++++++---------- .../ingest/30_update_by_query_with_ingest.yml | 2 +- .../action/search/SearchRequest.java | 4 ++ .../index/reindex/DeleteByQueryRequest.java | 19 ++++++++ .../index/reindex/UpdateByQueryRequest.java | 9 ++++ .../rest-api-spec/test/20_update_by_query.yml | 18 ++++---- .../rest-api-spec/test/30_delete_by_query.yml | 16 +++---- 26 files changed, 267 insertions(+), 156 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java index 6583cd35a793a..374af77c28edf 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java @@ -512,8 +512,9 @@ private static Request prepareReindexRequest(ReindexRequest reindexRequest, bool } static Request updateByQuery(UpdateByQueryRequest updateByQueryRequest) throws IOException { - String endpoint = - endpoint(updateByQueryRequest.indices(), updateByQueryRequest.getDocTypes(), "_update_by_query"); + String endpoint = updateByQueryRequest.isNoTypeRequest() + ? endpoint(updateByQueryRequest.indices(), "_update_by_query") + : endpoint(updateByQueryRequest.indices(), updateByQueryRequest.getDocTypes(), "_update_by_query"); Request request = new Request(HttpPost.METHOD_NAME, endpoint); Params params = new Params(request) .withRouting(updateByQueryRequest.getRouting()) @@ -540,8 +541,9 @@ static Request updateByQuery(UpdateByQueryRequest updateByQueryRequest) throws I } static Request deleteByQuery(DeleteByQueryRequest deleteByQueryRequest) throws IOException { - String endpoint = - endpoint(deleteByQueryRequest.indices(), deleteByQueryRequest.getDocTypes(), "_delete_by_query"); + String endpoint = deleteByQueryRequest.isNoTypeRequest() + ? endpoint(deleteByQueryRequest.indices(), "_delete_by_query") + : endpoint(deleteByQueryRequest.indices(), deleteByQueryRequest.getDocTypes(), "_delete_by_query"); Request request = new Request(HttpPost.METHOD_NAME, endpoint); Params params = new Params(request) .withRouting(deleteByQueryRequest.getRouting()) diff --git a/docs/java-api/docs/update-by-query.asciidoc b/docs/java-api/docs/update-by-query.asciidoc index ae4f8d72ee1d9..d4fe7c1c0419f 100644 --- a/docs/java-api/docs/update-by-query.asciidoc +++ b/docs/java-api/docs/update-by-query.asciidoc @@ -88,7 +88,7 @@ This API doesn't allow you to move the documents it touches, just modify their source. This is intentional! We've made no provisions for removing the document from its original location. -You can also perform these operations on multiple indices and types at once, similar to the search API: +You can also perform these operations on multiple indices at once, similar to the search API: ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- diff --git a/modules/reindex/src/test/java/org/elasticsearch/client/documentation/ReindexDocumentationIT.java b/modules/reindex/src/test/java/org/elasticsearch/client/documentation/ReindexDocumentationIT.java index 4ba814e4238df..8def6dbb40316 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/client/documentation/ReindexDocumentationIT.java +++ b/modules/reindex/src/test/java/org/elasticsearch/client/documentation/ReindexDocumentationIT.java @@ -172,7 +172,7 @@ public void testUpdateByQuery() { // tag::update-by-query-multi-index UpdateByQueryRequestBuilder updateByQuery = new UpdateByQueryRequestBuilder(client, UpdateByQueryAction.INSTANCE); - updateByQuery.source("foo", "bar").source().setTypes("a", "b"); + updateByQuery.source("foo", "bar"); BulkByScrollResponse response = updateByQuery.get(); // end::update-by-query-multi-index } diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java index 1f972cd282425..5724e314bee99 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java @@ -19,18 +19,50 @@ package org.elasticsearch.index.reindex; +import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; +import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestController; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.action.search.RestSearchAction; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.rest.FakeRestChannel; import org.elasticsearch.test.rest.FakeRestRequest; +import org.elasticsearch.usage.UsageService; import java.io.IOException; +import java.util.Collections; import static java.util.Collections.emptyList; import static org.mockito.Mockito.mock; public class RestDeleteByQueryActionTests extends ESTestCase { + private RestController controller; + + public void setUp() throws Exception { + super.setUp(); + controller = new RestController(Collections.emptySet(), null, + mock(NodeClient.class), + new NoneCircuitBreakerService(), + new UsageService()); + new RestDeleteByQueryAction(Settings.EMPTY, controller); + } + + public void testTypeInPath() { + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) + .withMethod(RestRequest.Method.POST) + .withPath("/some_index/some_type/_delete_by_query") + .build(); + + performRequest(request); + // RestDeleteByQueryAction itself doesn't check for a deprecated type usage + // checking here for a deprecation from its internal search request + assertWarnings(RestSearchAction.TYPES_DEPRECATION_MESSAGE); + } + public void testParseEmpty() throws IOException { RestDeleteByQueryAction action = new RestDeleteByQueryAction(Settings.EMPTY, mock(RestController.class)); DeleteByQueryRequest request = action.buildRequest(new FakeRestRequest.Builder(new NamedXContentRegistry(emptyList())) @@ -38,4 +70,10 @@ public void testParseEmpty() throws IOException { assertEquals(AbstractBulkByScrollRequest.SIZE_ALL_MATCHES, request.getSize()); assertEquals(AbstractBulkByScrollRequest.DEFAULT_SCROLL_SIZE, request.getSearchRequest().source().size()); } + + private void performRequest(RestRequest request) { + RestChannel channel = new FakeRestChannel(request, false, 1); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + controller.dispatchRequest(request, channel, threadContext); + } } diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java index efb6e20a20089..2f05bdc5472df 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java @@ -19,18 +19,51 @@ package org.elasticsearch.index.reindex; +import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; +import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestController; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.action.search.RestSearchAction; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.rest.FakeRestChannel; import org.elasticsearch.test.rest.FakeRestRequest; +import org.elasticsearch.usage.UsageService; import java.io.IOException; +import java.util.Collections; import static java.util.Collections.emptyList; import static org.mockito.Mockito.mock; public class RestUpdateByQueryActionTests extends ESTestCase { + + private RestController controller; + + public void setUp() throws Exception { + super.setUp(); + controller = new RestController(Collections.emptySet(), null, + mock(NodeClient.class), + new NoneCircuitBreakerService(), + new UsageService()); + new RestUpdateByQueryAction(Settings.EMPTY, controller); + } + + public void testTypeInPath() { + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) + .withMethod(RestRequest.Method.POST) + .withPath("/some_index/some_type/_update_by_query") + .build(); + + performRequest(request); + // RestUpdateByQueryAction itself doesn't check for a deprecated type usage + // checking here for a deprecation from its internal search request + assertWarnings(RestSearchAction.TYPES_DEPRECATION_MESSAGE); + } + public void testParseEmpty() throws IOException { RestUpdateByQueryAction action = new RestUpdateByQueryAction(Settings.EMPTY, mock(RestController.class)); UpdateByQueryRequest request = action.buildRequest(new FakeRestRequest.Builder(new NamedXContentRegistry(emptyList())) @@ -38,4 +71,10 @@ public void testParseEmpty() throws IOException { assertEquals(AbstractBulkByScrollRequest.SIZE_ALL_MATCHES, request.getSize()); assertEquals(AbstractBulkByScrollRequest.DEFAULT_SCROLL_SIZE, request.getSearchRequest().source().size()); } + + private void performRequest(RestRequest request) { + RestChannel channel = new FakeRestChannel(request, false, 1); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + controller.dispatchRequest(request, channel, threadContext); + } } diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yml b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yml index e5bf6368eaba8..35ca1c11c58eb 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yml @@ -3,7 +3,7 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -42,7 +42,7 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -99,7 +99,7 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -108,7 +108,7 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test2" } @@ -124,7 +124,7 @@ - match: {version_conflicts: 1} - match: {batches: 1} - match: {failures.0.index: test} - - match: {failures.0.type: foo} + - match: {failures.0.type: _doc} - match: {failures.0.id: "1"} - match: {failures.0.status: 409} - match: {failures.0.cause.type: version_conflict_engine_exception} @@ -154,7 +154,7 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -163,7 +163,7 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test2" } @@ -197,13 +197,13 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "user": "kimchy" } - do: index: index: twitter - type: tweet + type: _doc id: 2 body: { "user": "junk" } - do: @@ -234,13 +234,13 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "user": "kimchy" } - do: index: index: twitter - type: tweet + type: _doc id: 2 body: { "user": "kimchy" } - do: @@ -281,17 +281,17 @@ - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: indices.refresh: {} diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/20_validation.yml b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/20_validation.yml index 89ab990bf9b43..7be77132b5910 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/20_validation.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/20_validation.yml @@ -19,7 +19,7 @@ - do: index: index: test - type: test + type: _doc id: 1 body: { "text": "test" } - do: @@ -36,7 +36,7 @@ - do: index: index: test - type: test + type: _doc id: 1 body: { "text": "test" } - do: @@ -53,7 +53,7 @@ - do: index: index: test - type: test + type: _doc id: 1 body: { "text": "test" } - do: diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/40_versioning.yml b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/40_versioning.yml index c81305e282431..1e2afacca148e 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/40_versioning.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/40_versioning.yml @@ -3,7 +3,7 @@ - do: index: index: index1 - type: type1 + type: _doc id: 1 version: 0 # Starting version is zero version_type: external @@ -24,6 +24,6 @@ - do: get: index: index1 - type: type1 + type: _doc id: 1 - match: {_version: 0} diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/50_wait_for_active_shards.yml b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/50_wait_for_active_shards.yml index ab21e6802f91d..9e7d9c4298a2f 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/50_wait_for_active_shards.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/50_wait_for_active_shards.yml @@ -9,7 +9,7 @@ - do: index: index: test - type: test + type: _doc id: 1 body: {"text": "test"} - do: diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/70_throttle.yml b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/70_throttle.yml index fc6081a3de846..d02985e13bc40 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/70_throttle.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/70_throttle.yml @@ -10,17 +10,17 @@ - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: indices.refresh: {} @@ -50,17 +50,17 @@ - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: indices.refresh: {} @@ -91,17 +91,17 @@ - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: indices.refresh: {} @@ -151,17 +151,17 @@ - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: indices.refresh: {} diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/80_slices.yml b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/80_slices.yml index 2f40d6bfd3e57..d92910af00c66 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/80_slices.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/80_slices.yml @@ -3,25 +3,25 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 2 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 3 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 4 body: { "text": "test" } - do: @@ -69,25 +69,25 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 2 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 3 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 4 body: { "text": "test" } - do: @@ -176,37 +176,37 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 2 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 3 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 4 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 5 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 6 body: { "text": "test" } - do: @@ -297,25 +297,25 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 2 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 3 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 4 body: { "text": "test" } - do: diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/10_basic.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/10_basic.yml index 784623f714ca6..785ff39f1493f 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/10_basic.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/10_basic.yml @@ -3,7 +3,7 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -30,7 +30,7 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -84,7 +84,7 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -93,7 +93,7 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test2" } @@ -105,7 +105,7 @@ - match: {version_conflicts: 1} - match: {batches: 1} - match: {failures.0.index: test} - - match: {failures.0.type: foo} + - match: {failures.0.type: _doc} - match: {failures.0.id: "1"} - match: {failures.0.status: 409} - match: {failures.0.cause.type: version_conflict_engine_exception} @@ -126,7 +126,7 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -135,7 +135,7 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test2" } @@ -156,13 +156,13 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "user": "kimchy" } - do: index: index: twitter - type: tweet + type: _doc id: 2 body: { "user": "junk" } - do: @@ -186,13 +186,13 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "user": "kimchy" } - do: index: index: twitter - type: tweet + type: _doc id: 2 body: { "user": "kimchy" } - do: @@ -220,17 +220,17 @@ - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: indices.refresh: {} @@ -246,7 +246,7 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: {} - do: @@ -260,7 +260,7 @@ - do: get: index: test - type: foo + type: _doc id: 1 - match: { _source: {} } - match: { _version: 2 } diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/20_validation.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/20_validation.yml index b7499180cdaa8..72cec657d937c 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/20_validation.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/20_validation.yml @@ -3,7 +3,7 @@ - do: index: index: test - type: test + type: _doc id: 1 body: { "text": "test" } - do: @@ -17,7 +17,7 @@ - do: index: index: test - type: test + type: _doc id: 1 body: { "text": "test" } - do: @@ -31,7 +31,7 @@ - do: index: index: test - type: test + type: _doc id: 1 body: { "text": "test" } - do: @@ -53,7 +53,7 @@ - do: index: index: test - type: test + type: _doc id: 1 body: { age: 23 } - do: diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/30_new_fields.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/30_new_fields.yml index 201410d68227f..ae6e7559e2dfd 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/30_new_fields.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/30_new_fields.yml @@ -12,7 +12,7 @@ - do: index: index: test - type: place + type: _doc id: 1 refresh: true body: { "name": "bob! house" } @@ -20,7 +20,7 @@ - do: indices.put_mapping: index: test - type: place + type: _doc body: properties: name: diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/35_search_failure.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/35_search_failure.yml index e7f3a146480ff..631f06a78476e 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/35_search_failure.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/35_search_failure.yml @@ -10,7 +10,7 @@ - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test" } - do: diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/40_versioning.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/40_versioning.yml index 1718714defd4e..7d2083f925b99 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/40_versioning.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/40_versioning.yml @@ -3,7 +3,7 @@ - do: index: index: test - type: test + type: _doc id: 1 body: {"text": "test"} - do: @@ -18,7 +18,7 @@ - do: get: index: test - type: test + type: _doc id: 1 - match: {_version: 2} @@ -27,7 +27,7 @@ - do: index: index: index1 - type: type1 + type: _doc id: 1 version: 0 # Starting version is zero version_type: external @@ -45,6 +45,6 @@ - do: get: index: index1 - type: type1 + type: _doc id: 1 - match: {_version: 0} diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/50_consistency.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/50_consistency.yml index 6e9c40146c56f..bb6d5abf2a7af 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/50_consistency.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/50_consistency.yml @@ -9,7 +9,7 @@ - do: index: index: test - type: test + type: _doc id: 1 body: {"text": "test"} - do: @@ -35,5 +35,5 @@ - do: get: index: test - type: test + type: _doc id: 1 diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/60_throttle.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/60_throttle.yml index eb64bd8d3820c..2153508e88d32 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/60_throttle.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/60_throttle.yml @@ -10,17 +10,17 @@ - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: indices.refresh: {} @@ -46,17 +46,17 @@ - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: indices.refresh: {} @@ -83,17 +83,17 @@ - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: indices.refresh: {} @@ -130,17 +130,17 @@ - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: index: index: test - type: foo + type: _doc body: { "text": "test" } - do: indices.refresh: {} diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/70_slices.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/70_slices.yml index 709e08854b25e..4474eaece9727 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/70_slices.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/70_slices.yml @@ -3,25 +3,25 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 2 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 3 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 4 body: { "text": "test" } - do: @@ -61,25 +61,25 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 2 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 3 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 4 body: { "text": "test" } - do: @@ -163,37 +163,37 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 2 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 3 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 4 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 5 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 6 body: { "text": "test" } - do: @@ -280,25 +280,25 @@ - do: index: index: test - type: foo + type: _doc id: 1 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 2 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 3 body: { "text": "test" } - do: index: index: test - type: foo + type: _doc id: 4 body: { "text": "test" } - do: diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/80_scripting.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/80_scripting.yml index b30f639591eee..6ce30c6036287 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/80_scripting.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/80_scripting.yml @@ -3,7 +3,7 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "user": "kimchy" } - do: @@ -35,7 +35,7 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "user": "kimchy" } - do: @@ -64,13 +64,13 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "user": "kimchy" } - do: index: index: twitter - type: tweet + type: _doc id: 2 body: { "user": "foo" } - do: @@ -112,13 +112,13 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "user": "kimchy" } - do: index: index: twitter - type: tweet + type: _doc id: 2 body: { "user": "foo" } - do: @@ -141,7 +141,7 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "user": "kimchy" } - do: @@ -161,7 +161,7 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "user": "kimchy" } - do: @@ -181,25 +181,25 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "level": 9, "last_updated": "2016-01-01T12:10:30Z" } - do: index: index: twitter - type: tweet + type: _doc id: 2 body: { "level": 10, "last_updated": "2016-01-01T12:10:30Z" } - do: index: index: twitter - type: tweet + type: _doc id: 3 body: { "level": 11, "last_updated": "2016-01-01T12:10:30Z" } - do: index: index: twitter - type: tweet + type: _doc id: 4 body: { "level": 12, "last_updated": "2016-01-01T12:10:30Z" } - do: @@ -247,25 +247,25 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "level": 9, "last_updated": "2016-01-01T12:10:30Z" } - do: index: index: twitter - type: tweet + type: _doc id: 2 body: { "level": 10, "last_updated": "2016-01-01T12:10:30Z" } - do: index: index: twitter - type: tweet + type: _doc id: 3 body: { "level": 11, "last_updated": "2016-01-01T12:10:30Z" } - do: index: index: twitter - type: tweet + type: _doc id: 4 body: { "level": 12, "last_updated": "2016-01-01T12:10:30Z" } - do: @@ -326,13 +326,13 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "user": "kimchy" } - do: index: index: twitter - type: tweet + type: _doc id: 2 body: { "user": "foo" } - do: @@ -355,25 +355,25 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "level": 9, "last_updated": "2016-01-01T12:10:30Z" } - do: index: index: twitter - type: tweet + type: _doc id: 2 body: { "level": 10, "last_updated": "2016-01-01T12:10:30Z" } - do: index: index: twitter - type: tweet + type: _doc id: 3 body: { "level": 11, "last_updated": "2016-01-01T12:10:30Z" } - do: index: index: twitter - type: tweet + type: _doc id: 4 body: { "level": 12, "last_updated": "2016-01-01T12:10:30Z" } - do: @@ -439,7 +439,7 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "user": "kimchy" } - do: diff --git a/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/30_update_by_query_with_ingest.yml b/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/30_update_by_query_with_ingest.yml index a8f1a3fea6604..0f187cc439016 100644 --- a/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/30_update_by_query_with_ingest.yml +++ b/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/30_update_by_query_with_ingest.yml @@ -18,7 +18,7 @@ - do: index: index: twitter - type: tweet + type: _doc id: 1 body: { "user": "kimchy" } - do: diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java index d5ab34fe582f4..f5af282cad851 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java @@ -538,4 +538,8 @@ public String toString() { ", allowPartialSearchResults=" + allowPartialSearchResults + ", source=" + source + '}'; } + + public boolean isNoTypeRequest() { + return types.length == 0; + } } diff --git a/server/src/main/java/org/elasticsearch/index/reindex/DeleteByQueryRequest.java b/server/src/main/java/org/elasticsearch/index/reindex/DeleteByQueryRequest.java index 18307e0a56812..280ae831146cd 100644 --- a/server/src/main/java/org/elasticsearch/index/reindex/DeleteByQueryRequest.java +++ b/server/src/main/java/org/elasticsearch/index/reindex/DeleteByQueryRequest.java @@ -89,7 +89,9 @@ public DeleteByQueryRequest setQuery(QueryBuilder query) { /** * Set the document types for the delete + * @deprecated Types are in the process of being removed. */ + @Deprecated public DeleteByQueryRequest setDocTypes(String... types) { if (types != null) { getSearchRequest().types(types); @@ -140,7 +142,9 @@ public String getRouting() { /** * Gets the document types on which this request would be executed. Returns an empty array if all * types are to be processed. + * @deprecated Types are in the process of being removed. */ + @Deprecated public String[] getDocTypes() { if (getSearchRequest().types() != null) { return getSearchRequest().types(); @@ -202,11 +206,21 @@ public IndicesOptions indicesOptions() { return getSearchRequest().indicesOptions(); } + /** + * Gets the document types on which this request would be executed. + * @deprecated Types are in the process of being removed. + */ + @Deprecated public String[] types() { assert getSearchRequest() != null; return getSearchRequest().types(); } + /** + * Set the document types for the delete + * @deprecated Types are in the process of being removed. + */ + @Deprecated public DeleteByQueryRequest types(String... types) { assert getSearchRequest() != null; getSearchRequest().types(types); @@ -220,4 +234,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endObject(); return builder; } + + // Remove in 8.0 when all requests are typeless + public boolean isNoTypeRequest() { + return getSearchRequest().isNoTypeRequest(); + } } diff --git a/server/src/main/java/org/elasticsearch/index/reindex/UpdateByQueryRequest.java b/server/src/main/java/org/elasticsearch/index/reindex/UpdateByQueryRequest.java index 71ffadc930327..cfc8fc7ccdb5e 100644 --- a/server/src/main/java/org/elasticsearch/index/reindex/UpdateByQueryRequest.java +++ b/server/src/main/java/org/elasticsearch/index/reindex/UpdateByQueryRequest.java @@ -85,7 +85,9 @@ public UpdateByQueryRequest setQuery(QueryBuilder query) { /** * Set the document types for the update + * @deprecated Types are in the process of being removed. */ + @Deprecated public UpdateByQueryRequest setDocTypes(String... types) { if (types != null) { getSearchRequest().types(types); @@ -136,7 +138,9 @@ public String getRouting() { /** * Gets the document types on which this request would be executed. Returns an empty array if all * types are to be processed. + * @deprecated Types are in the process of being removed. */ + @Deprecated public String[] getDocTypes() { if (getSearchRequest().types() != null) { return getSearchRequest().types(); @@ -215,4 +219,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endObject(); return builder; } + + // Remove in 8.0 when all requests are typeless + public boolean isNoTypeRequest() { + return getSearchRequest().isNoTypeRequest(); + } } diff --git a/x-pack/qa/reindex-tests-with-security/src/test/resources/rest-api-spec/test/20_update_by_query.yml b/x-pack/qa/reindex-tests-with-security/src/test/resources/rest-api-spec/test/20_update_by_query.yml index 805d9973bfde2..b54925701bbe8 100644 --- a/x-pack/qa/reindex-tests-with-security/src/test/resources/rest-api-spec/test/20_update_by_query.yml +++ b/x-pack/qa/reindex-tests-with-security/src/test/resources/rest-api-spec/test/20_update_by_query.yml @@ -7,7 +7,7 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -38,7 +38,7 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -70,7 +70,7 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -102,7 +102,7 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -120,7 +120,7 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -138,13 +138,13 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test" } - do: index: index: source - type: foo + type: _doc id: 2 body: { "text": "test", "hidden": true } - do: @@ -192,7 +192,7 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test", "foo": "z", "bar": "z" } - do: @@ -210,7 +210,7 @@ setup: - do: get: index: source - type: foo + type: _doc id: 1 # These were visible to the user running the update_by_query so they stayed. - match: { _source.foo: z } diff --git a/x-pack/qa/reindex-tests-with-security/src/test/resources/rest-api-spec/test/30_delete_by_query.yml b/x-pack/qa/reindex-tests-with-security/src/test/resources/rest-api-spec/test/30_delete_by_query.yml index 22bd593cfb5b1..677a843ec53aa 100644 --- a/x-pack/qa/reindex-tests-with-security/src/test/resources/rest-api-spec/test/30_delete_by_query.yml +++ b/x-pack/qa/reindex-tests-with-security/src/test/resources/rest-api-spec/test/30_delete_by_query.yml @@ -7,7 +7,7 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -33,7 +33,7 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -60,7 +60,7 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -87,7 +87,7 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -114,7 +114,7 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test" } - do: @@ -141,13 +141,13 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test", "hidden": false } - do: index: index: source - type: foo + type: _doc id: 2 body: { "text": "test", "hidden": true } - do: @@ -211,7 +211,7 @@ setup: - do: index: index: source - type: foo + type: _doc id: 1 body: { "text": "test", "foo": "z", "bar": "z" } - do: From f0a5ecf7ab89fefa8bbb82b77d49b42d20f80eab Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Mon, 10 Dec 2018 14:38:53 -0500 Subject: [PATCH 2/4] Address Julie's comments and fix texts --- .../client/RequestConverters.java | 10 ++--- .../reindex/RestDeleteByQueryActionTests.java | 41 +++++-------------- .../reindex/RestUpdateByQueryActionTests.java | 41 +++++-------------- .../test/delete_by_query/10_basic.yml | 2 +- .../test/update_by_query/10_basic.yml | 2 +- .../test/update_by_query/20_validation.yml | 4 +- .../action/search/SearchRequest.java | 4 -- .../index/reindex/DeleteByQueryRequest.java | 17 ++++---- .../index/reindex/UpdateByQueryRequest.java | 11 ++--- .../document/RestDeleteActionTests.java | 2 +- .../RestMultiTermVectorsActionTests.java | 2 +- .../document/RestTermVectorsActionTests.java | 2 +- .../action/search/RestCountActionTests.java | 2 +- .../search/RestMultiSearchActionTests.java | 2 +- .../action/search/RestSearchActionTests.java | 2 +- .../test/rest}/RestActionTestCase.java | 3 +- 16 files changed, 49 insertions(+), 98 deletions(-) rename {server/src/test/java/org/elasticsearch/rest/action => test/framework/src/main/java/org/elasticsearch/test/rest}/RestActionTestCase.java (96%) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java index 693648ec528a8..85d92573221a7 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java @@ -519,9 +519,8 @@ private static Request prepareReindexRequest(ReindexRequest reindexRequest, bool } static Request updateByQuery(UpdateByQueryRequest updateByQueryRequest) throws IOException { - String endpoint = updateByQueryRequest.isNoTypeRequest() - ? endpoint(updateByQueryRequest.indices(), "_update_by_query") - : endpoint(updateByQueryRequest.indices(), updateByQueryRequest.getDocTypes(), "_update_by_query"); + String endpoint = + endpoint(updateByQueryRequest.indices(), updateByQueryRequest.getDocTypes(), "_update_by_query"); Request request = new Request(HttpPost.METHOD_NAME, endpoint); Params params = new Params(request) .withRouting(updateByQueryRequest.getRouting()) @@ -548,9 +547,8 @@ static Request updateByQuery(UpdateByQueryRequest updateByQueryRequest) throws I } static Request deleteByQuery(DeleteByQueryRequest deleteByQueryRequest) throws IOException { - String endpoint = deleteByQueryRequest.isNoTypeRequest() - ? endpoint(deleteByQueryRequest.indices(), "_delete_by_query") - : endpoint(deleteByQueryRequest.indices(), deleteByQueryRequest.getDocTypes(), "_delete_by_query"); + String endpoint = + endpoint(deleteByQueryRequest.indices(), deleteByQueryRequest.getDocTypes(), "_delete_by_query"); Request request = new Request(HttpPost.METHOD_NAME, endpoint); Params params = new Params(request) .withRouting(deleteByQueryRequest.getRouting()) diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java index 5724e314bee99..b7ffc16e0df3c 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java @@ -19,36 +19,24 @@ package org.elasticsearch.index.reindex; -import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; -import org.elasticsearch.rest.RestChannel; -import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.action.search.RestSearchAction; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.rest.FakeRestChannel; import org.elasticsearch.test.rest.FakeRestRequest; -import org.elasticsearch.usage.UsageService; +import org.elasticsearch.test.rest.RestActionTestCase; +import org.junit.Before; import java.io.IOException; -import java.util.Collections; import static java.util.Collections.emptyList; -import static org.mockito.Mockito.mock; -public class RestDeleteByQueryActionTests extends ESTestCase { - private RestController controller; +public class RestDeleteByQueryActionTests extends RestActionTestCase { + private RestDeleteByQueryAction action; - public void setUp() throws Exception { - super.setUp(); - controller = new RestController(Collections.emptySet(), null, - mock(NodeClient.class), - new NoneCircuitBreakerService(), - new UsageService()); - new RestDeleteByQueryAction(Settings.EMPTY, controller); + @Before + public void setUpAction() { + action = new RestDeleteByQueryAction(Settings.EMPTY, controller()); } public void testTypeInPath() { @@ -56,24 +44,17 @@ public void testTypeInPath() { .withMethod(RestRequest.Method.POST) .withPath("/some_index/some_type/_delete_by_query") .build(); - - performRequest(request); + dispatchRequest(request); + // checks the type in the URL is propagated correctly to the request object + assertEquals("some_type", request.param("type")); // RestDeleteByQueryAction itself doesn't check for a deprecated type usage // checking here for a deprecation from its internal search request assertWarnings(RestSearchAction.TYPES_DEPRECATION_MESSAGE); } public void testParseEmpty() throws IOException { - RestDeleteByQueryAction action = new RestDeleteByQueryAction(Settings.EMPTY, mock(RestController.class)); - DeleteByQueryRequest request = action.buildRequest(new FakeRestRequest.Builder(new NamedXContentRegistry(emptyList())) - .build()); + DeleteByQueryRequest request = action.buildRequest(new FakeRestRequest.Builder(new NamedXContentRegistry(emptyList())).build()); assertEquals(AbstractBulkByScrollRequest.SIZE_ALL_MATCHES, request.getSize()); assertEquals(AbstractBulkByScrollRequest.DEFAULT_SCROLL_SIZE, request.getSearchRequest().source().size()); } - - private void performRequest(RestRequest request) { - RestChannel channel = new FakeRestChannel(request, false, 1); - ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - controller.dispatchRequest(request, channel, threadContext); - } } diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java index 2f05bdc5472df..2af1da0239aaa 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java @@ -19,37 +19,25 @@ package org.elasticsearch.index.reindex; -import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; -import org.elasticsearch.rest.RestChannel; -import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.action.search.RestSearchAction; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.rest.FakeRestChannel; import org.elasticsearch.test.rest.FakeRestRequest; -import org.elasticsearch.usage.UsageService; +import org.elasticsearch.test.rest.RestActionTestCase; +import org.junit.Before; import java.io.IOException; -import java.util.Collections; import static java.util.Collections.emptyList; -import static org.mockito.Mockito.mock; -public class RestUpdateByQueryActionTests extends ESTestCase { +public class RestUpdateByQueryActionTests extends RestActionTestCase { - private RestController controller; + private RestUpdateByQueryAction action; - public void setUp() throws Exception { - super.setUp(); - controller = new RestController(Collections.emptySet(), null, - mock(NodeClient.class), - new NoneCircuitBreakerService(), - new UsageService()); - new RestUpdateByQueryAction(Settings.EMPTY, controller); + @Before + public void setUpAction() { + action = new RestUpdateByQueryAction(Settings.EMPTY, controller()); } public void testTypeInPath() { @@ -57,24 +45,17 @@ public void testTypeInPath() { .withMethod(RestRequest.Method.POST) .withPath("/some_index/some_type/_update_by_query") .build(); - - performRequest(request); + dispatchRequest(request); + // checks the type in the URL is propagated correctly to the request object + assertEquals("some_type", request.param("type")); // RestUpdateByQueryAction itself doesn't check for a deprecated type usage // checking here for a deprecation from its internal search request assertWarnings(RestSearchAction.TYPES_DEPRECATION_MESSAGE); } public void testParseEmpty() throws IOException { - RestUpdateByQueryAction action = new RestUpdateByQueryAction(Settings.EMPTY, mock(RestController.class)); - UpdateByQueryRequest request = action.buildRequest(new FakeRestRequest.Builder(new NamedXContentRegistry(emptyList())) - .build()); + UpdateByQueryRequest request = action.buildRequest(new FakeRestRequest.Builder(new NamedXContentRegistry(emptyList())).build()); assertEquals(AbstractBulkByScrollRequest.SIZE_ALL_MATCHES, request.getSize()); assertEquals(AbstractBulkByScrollRequest.DEFAULT_SCROLL_SIZE, request.getSearchRequest().source().size()); } - - private void performRequest(RestRequest request) { - RestChannel channel = new FakeRestChannel(request, false, 1); - ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - controller.dispatchRequest(request, channel, threadContext); - } } diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yml b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yml index 35ca1c11c58eb..9f9a99f0f5c2e 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yml @@ -129,7 +129,7 @@ - match: {failures.0.status: 409} - match: {failures.0.cause.type: version_conflict_engine_exception} # Use a regex so we don't mind if the current version isn't always 1. Sometimes it comes out 2. - - match: {failures.0.cause.reason: "/\\[foo\\]\\[1\\]:.version.conflict,.current.version.\\[\\d+\\].is.different.than.the.one.provided.\\[\\d+\\]/"} + - match: {failures.0.cause.reason: "/\\[_doc\\]\\[1\\]:.version.conflict,.current.version.\\[\\d+\\].is.different.than.the.one.provided.\\[\\d+\\]/"} - match: {failures.0.cause.shard: /\d+/} - match: {failures.0.cause.index: test} - gte: { took: 0 } diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/10_basic.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/10_basic.yml index 785ff39f1493f..d3881035d924f 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/10_basic.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/10_basic.yml @@ -110,7 +110,7 @@ - match: {failures.0.status: 409} - match: {failures.0.cause.type: version_conflict_engine_exception} # Use a regex so we don't mind if the current version isn't always 1. Sometimes it comes out 2. - - match: {failures.0.cause.reason: "/\\[foo\\]\\[1\\]:.version.conflict,.current.version.\\[\\d+\\].is.different.than.the.one.provided.\\[\\d+\\]/"} + - match: {failures.0.cause.reason: "/\\[_doc\\]\\[1\\]:.version.conflict,.current.version.\\[\\d+\\].is.different.than.the.one.provided.\\[\\d+\\]/"} - match: {failures.0.cause.shard: /\d+/} - match: {failures.0.cause.index: test} - gte: { took: 0 } diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/20_validation.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/20_validation.yml index 72cec657d937c..8fdce4bd9db99 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/20_validation.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/20_validation.yml @@ -47,7 +47,7 @@ index: test body: mappings: - test: + _doc: _source: enabled: false - do: @@ -60,7 +60,7 @@ indices.refresh: {} - do: - catch: /\[test\]\[test\]\[1\] didn't store _source/ + catch: /\[test\]\[_doc\]\[1\] didn't store _source/ update_by_query: index: test diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java index f5af282cad851..d5ab34fe582f4 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java @@ -538,8 +538,4 @@ public String toString() { ", allowPartialSearchResults=" + allowPartialSearchResults + ", source=" + source + '}'; } - - public boolean isNoTypeRequest() { - return types.length == 0; - } } diff --git a/server/src/main/java/org/elasticsearch/index/reindex/DeleteByQueryRequest.java b/server/src/main/java/org/elasticsearch/index/reindex/DeleteByQueryRequest.java index 280ae831146cd..227ddd489779c 100644 --- a/server/src/main/java/org/elasticsearch/index/reindex/DeleteByQueryRequest.java +++ b/server/src/main/java/org/elasticsearch/index/reindex/DeleteByQueryRequest.java @@ -89,7 +89,8 @@ public DeleteByQueryRequest setQuery(QueryBuilder query) { /** * Set the document types for the delete - * @deprecated Types are in the process of being removed. + * @deprecated Types are in the process of being removed. Instead of + * using a type, prefer to filter on a field of the document. */ @Deprecated public DeleteByQueryRequest setDocTypes(String... types) { @@ -142,7 +143,8 @@ public String getRouting() { /** * Gets the document types on which this request would be executed. Returns an empty array if all * types are to be processed. - * @deprecated Types are in the process of being removed. + * @deprecated Types are in the process of being removed. Instead of + * using a type, prefer to filter on a field of the document. */ @Deprecated public String[] getDocTypes() { @@ -208,7 +210,8 @@ public IndicesOptions indicesOptions() { /** * Gets the document types on which this request would be executed. - * @deprecated Types are in the process of being removed. + * @deprecated Types are in the process of being removed. Instead of + * using a type, prefer to filter on a field of the document. */ @Deprecated public String[] types() { @@ -218,7 +221,8 @@ public String[] types() { /** * Set the document types for the delete - * @deprecated Types are in the process of being removed. + * @deprecated Types are in the process of being removed. Instead of + * using a type, prefer to filter on a field of the document. */ @Deprecated public DeleteByQueryRequest types(String... types) { @@ -234,9 +238,4 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endObject(); return builder; } - - // Remove in 8.0 when all requests are typeless - public boolean isNoTypeRequest() { - return getSearchRequest().isNoTypeRequest(); - } } diff --git a/server/src/main/java/org/elasticsearch/index/reindex/UpdateByQueryRequest.java b/server/src/main/java/org/elasticsearch/index/reindex/UpdateByQueryRequest.java index cfc8fc7ccdb5e..03922ccc79bd4 100644 --- a/server/src/main/java/org/elasticsearch/index/reindex/UpdateByQueryRequest.java +++ b/server/src/main/java/org/elasticsearch/index/reindex/UpdateByQueryRequest.java @@ -85,7 +85,8 @@ public UpdateByQueryRequest setQuery(QueryBuilder query) { /** * Set the document types for the update - * @deprecated Types are in the process of being removed. + * @deprecated Types are in the process of being removed. Instead of + * using a type, prefer to filter on a field of the document. */ @Deprecated public UpdateByQueryRequest setDocTypes(String... types) { @@ -138,7 +139,8 @@ public String getRouting() { /** * Gets the document types on which this request would be executed. Returns an empty array if all * types are to be processed. - * @deprecated Types are in the process of being removed. + * @deprecated Types are in the process of being removed. Instead of + * using a type, prefer to filter on a field of the document. */ @Deprecated public String[] getDocTypes() { @@ -219,9 +221,4 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endObject(); return builder; } - - // Remove in 8.0 when all requests are typeless - public boolean isNoTypeRequest() { - return getSearchRequest().isNoTypeRequest(); - } } diff --git a/server/src/test/java/org/elasticsearch/rest/action/document/RestDeleteActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/document/RestDeleteActionTests.java index cd41edfc00bb5..0b485af7df27d 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/document/RestDeleteActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/document/RestDeleteActionTests.java @@ -22,7 +22,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestRequest.Method; -import org.elasticsearch.rest.action.RestActionTestCase; +import org.elasticsearch.test.rest.RestActionTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import org.junit.Before; diff --git a/server/src/test/java/org/elasticsearch/rest/action/document/RestMultiTermVectorsActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/document/RestMultiTermVectorsActionTests.java index 3027b728a9a2b..dd98089246b51 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/document/RestMultiTermVectorsActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/document/RestMultiTermVectorsActionTests.java @@ -26,7 +26,7 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestRequest.Method; -import org.elasticsearch.rest.action.RestActionTestCase; +import org.elasticsearch.test.rest.RestActionTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import org.junit.Before; diff --git a/server/src/test/java/org/elasticsearch/rest/action/document/RestTermVectorsActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/document/RestTermVectorsActionTests.java index ea2c272648bc7..d93f7749f63e3 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/document/RestTermVectorsActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/document/RestTermVectorsActionTests.java @@ -26,7 +26,7 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestRequest.Method; -import org.elasticsearch.rest.action.RestActionTestCase; +import org.elasticsearch.test.rest.RestActionTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import org.junit.Before; diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/RestCountActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/RestCountActionTests.java index e0228598e1e34..c6af3d12e2913 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/search/RestCountActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/search/RestCountActionTests.java @@ -22,7 +22,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestRequest.Method; -import org.elasticsearch.rest.action.RestActionTestCase; +import org.elasticsearch.test.rest.RestActionTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import org.junit.Before; diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/RestMultiSearchActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/RestMultiSearchActionTests.java index cd36cf43f69cd..3b80d2002c5c5 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/search/RestMultiSearchActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/search/RestMultiSearchActionTests.java @@ -23,7 +23,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.action.RestActionTestCase; +import org.elasticsearch.test.rest.RestActionTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import org.junit.Before; diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/RestSearchActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/RestSearchActionTests.java index 54b53d7e87a4f..522d04b37c663 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/search/RestSearchActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/search/RestSearchActionTests.java @@ -21,7 +21,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.action.RestActionTestCase; +import org.elasticsearch.test.rest.RestActionTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import org.junit.Before; diff --git a/server/src/test/java/org/elasticsearch/rest/action/RestActionTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java similarity index 96% rename from server/src/test/java/org/elasticsearch/rest/action/RestActionTestCase.java rename to test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java index 1f480e1bd7fc5..f5ab14971b890 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/RestActionTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java @@ -17,7 +17,7 @@ * under the License. */ -package org.elasticsearch.rest.action; +package org.elasticsearch.test.rest; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.settings.Settings; @@ -26,7 +26,6 @@ import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.rest.FakeRestChannel; import org.elasticsearch.usage.UsageService; import org.junit.Before; From 1e69aebf7efa8b76e25f495a5783ca287b7bdbb0 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Tue, 11 Dec 2018 08:26:21 -0500 Subject: [PATCH 3/4] Correct tests --- .../index/reindex/RestDeleteByQueryActionTests.java | 8 ++++++-- .../index/reindex/RestUpdateByQueryActionTests.java | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java index b7ffc16e0df3c..788fad3210037 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java @@ -39,14 +39,18 @@ public void setUpAction() { action = new RestDeleteByQueryAction(Settings.EMPTY, controller()); } - public void testTypeInPath() { + public void testTypeInPath() throws IOException { RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) .withMethod(RestRequest.Method.POST) .withPath("/some_index/some_type/_delete_by_query") .build(); dispatchRequest(request); + // checks the type in the URL is propagated correctly to the request object - assertEquals("some_type", request.param("type")); + // only works after the request is dispatched, so its params are filled from url. + DeleteByQueryRequest dbqRequest = action.buildRequest(request); + assertArrayEquals(new String[]{"some_type"}, dbqRequest.getDocTypes()); + // RestDeleteByQueryAction itself doesn't check for a deprecated type usage // checking here for a deprecation from its internal search request assertWarnings(RestSearchAction.TYPES_DEPRECATION_MESSAGE); diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java index 2af1da0239aaa..b5333226bb92e 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java @@ -40,14 +40,18 @@ public void setUpAction() { action = new RestUpdateByQueryAction(Settings.EMPTY, controller()); } - public void testTypeInPath() { + public void testTypeInPath() throws IOException { RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) .withMethod(RestRequest.Method.POST) .withPath("/some_index/some_type/_update_by_query") .build(); dispatchRequest(request); + // checks the type in the URL is propagated correctly to the request object - assertEquals("some_type", request.param("type")); + // only works after the request is dispatched, so its params are filled from url. + UpdateByQueryRequest ubqRequest = action.buildRequest(request); + assertArrayEquals(new String[]{"some_type"}, ubqRequest.getDocTypes()); + // RestUpdateByQueryAction itself doesn't check for a deprecated type usage // checking here for a deprecation from its internal search request assertWarnings(RestSearchAction.TYPES_DEPRECATION_MESSAGE); From 1cf1058132d0546035a6fdab8ce94b22aa438f5e Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Tue, 11 Dec 2018 12:54:17 -0500 Subject: [PATCH 4/4] Correct tests --- .../elasticsearch/rest/action/document/RestGetActionTests.java | 2 +- .../rest/action/document/RestMultiGetActionTests.java | 2 +- .../rest/action/search/RestExplainActionTests.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/rest/action/document/RestGetActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/document/RestGetActionTests.java index f68288fa26199..44781443ec452 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/document/RestGetActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/document/RestGetActionTests.java @@ -21,8 +21,8 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.RestRequest.Method; -import org.elasticsearch.rest.action.RestActionTestCase; import org.elasticsearch.test.rest.FakeRestRequest; +import org.elasticsearch.test.rest.RestActionTestCase; import org.junit.Before; public class RestGetActionTests extends RestActionTestCase { diff --git a/server/src/test/java/org/elasticsearch/rest/action/document/RestMultiGetActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/document/RestMultiGetActionTests.java index db25a6b5739fd..da5c7e1fdfd6d 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/document/RestMultiGetActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/document/RestMultiGetActionTests.java @@ -26,8 +26,8 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestRequest.Method; -import org.elasticsearch.rest.action.RestActionTestCase; import org.elasticsearch.test.rest.FakeRestRequest; +import org.elasticsearch.test.rest.RestActionTestCase; import org.junit.Before; public class RestMultiGetActionTests extends RestActionTestCase { diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/RestExplainActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/RestExplainActionTests.java index 121df83ef81e7..1c7f266fe9948 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/search/RestExplainActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/search/RestExplainActionTests.java @@ -21,8 +21,8 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.action.RestActionTestCase; import org.elasticsearch.test.rest.FakeRestRequest; +import org.elasticsearch.test.rest.RestActionTestCase; import org.junit.Before; public class RestExplainActionTests extends RestActionTestCase {