Deprecate types in update_by_query and delete_by_query

Relates to elastic#35190
mayya-sharipova · Dec 7, 2018 · c323868 · c323868
1 parent c32e4fb
commit c323868
Show file tree

Hide file tree

Showing 26 changed files with 267 additions and 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
@@ -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
@@ -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/.../reindex/src/test/java/org/elasticsearch/client/documentation/ReindexDocumentationIT.java b/.../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/...s/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java b/...s/reindex/src/test/java/org/elasticsearch/index/reindex/RestDeleteByQueryActionTests.java
@@ -19,23 +19,61 @@
 
 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()))
                 .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/...s/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java b/...s/reindex/src/test/java/org/elasticsearch/index/reindex/RestUpdateByQueryActionTests.java
@@ -19,23 +19,62 @@
 
 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()))
                 .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
@@ -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
@@ -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
@@ -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/...index/src/test/resources/rest-api-spec/test/delete_by_query/50_wait_for_active_shards.yml b/...index/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: