Deprecate types in update_by_query and delete_by_query (#36365)

Relates to #35190
elastic · Dec 11, 2018 · 2f18325 · 2f18325
1 parent 8e988f6
commit 2f18325
Show file tree

Hide file tree

Showing 34 changed files with 251 additions and 181 deletions.
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
@@ -21,20 +21,43 @@
 
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
-import org.elasticsearch.rest.RestController;
-import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.rest.RestRequest;
+import org.elasticsearch.rest.action.search.RestSearchAction;
 import org.elasticsearch.test.rest.FakeRestRequest;
+import org.elasticsearch.test.rest.RestActionTestCase;
 
+import org.junit.Before;
 import java.io.IOException;
 
 import static java.util.Collections.emptyList;
-import static org.mockito.Mockito.mock;
 
-public class RestDeleteByQueryActionTests extends ESTestCase {
+public class RestDeleteByQueryActionTests extends RestActionTestCase {
+    private RestDeleteByQueryAction action;
+
+    @Before
+    public void setUpAction() {
+        action = new RestDeleteByQueryAction(Settings.EMPTY, controller());
+    }
+
+    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
+        // 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);
+    }
+
     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());
     }

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
@@ -21,20 +21,44 @@
 
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
-import org.elasticsearch.rest.RestController;
-import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.rest.RestRequest;
+import org.elasticsearch.rest.action.search.RestSearchAction;
 import org.elasticsearch.test.rest.FakeRestRequest;
+import org.elasticsearch.test.rest.RestActionTestCase;
 
+import org.junit.Before;
 import java.io.IOException;
 
 import static java.util.Collections.emptyList;
-import static org.mockito.Mockito.mock;
 
-public class RestUpdateByQueryActionTests extends ESTestCase {
+public class RestUpdateByQueryActionTests extends RestActionTestCase {
+
+    private RestUpdateByQueryAction action;
+
+    @Before
+    public void setUpAction() {
+        action = new RestUpdateByQueryAction(Settings.EMPTY, controller());
+    }
+
+    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
+        // 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);
+    }
+
     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());
     }

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,12 +124,12 @@
   - 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}
   # 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 }
@@ -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:

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
@@ -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: {}