Deprecate use of type in reindex request body (#36823)

Types can be used both in the source and dest section of the body which will
be translated to search and index requests respectively. Adding a deprecation warning
for those cases and removing examples using more than one type in reindex since
support for this is going to be removed.

client/rest-high-level/src/test/java/org/elasticsearch/client/ReindexIT.java

@@ -48,8 +48,8 @@ public class ReindexIT extends ESRestHighLevelClientTestCase {
             createIndex(sourceIndex, settings);
             createIndex(destinationIndex, settings);
             BulkRequest bulkRequest = new BulkRequest()
-                .add(new IndexRequest(sourceIndex, "type", "1").source(Collections.singletonMap("foo", "bar"), XContentType.JSON))
-                .add(new IndexRequest(sourceIndex, "type", "2").source(Collections.singletonMap("foo2", "bar2"), XContentType.JSON))
+                .add(new IndexRequest(sourceIndex).id("1").source(Collections.singletonMap("foo", "bar"), XContentType.JSON))
+                .add(new IndexRequest(sourceIndex).id("2").source(Collections.singletonMap("foo2", "bar2"), XContentType.JSON))
                 .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE);
             assertEquals(
                 RestStatus.OK,
@@ -64,7 +64,7 @@ public class ReindexIT extends ESRestHighLevelClientTestCase {
             ReindexRequest reindexRequest = new ReindexRequest();
             reindexRequest.setSourceIndices(sourceIndex);
             reindexRequest.setDestIndex(destinationIndex);
-            reindexRequest.setSourceQuery(new IdsQueryBuilder().addIds("1").types("type"));
+            reindexRequest.setSourceQuery(new IdsQueryBuilder().addIds("1"));
             reindexRequest.setRefresh(true);
 
             BulkByScrollResponse bulkResponse = execute(reindexRequest, highLevelClient()::reindex, highLevelClient()::reindexAsync);
@@ -94,8 +94,8 @@ public class ReindexIT extends ESRestHighLevelClientTestCase {
             createIndex(sourceIndex, settings);
             createIndex(destinationIndex, settings);
             BulkRequest bulkRequest = new BulkRequest()
-                .add(new IndexRequest(sourceIndex, "type", "1").source(Collections.singletonMap("foo", "bar"), XContentType.JSON))
-                .add(new IndexRequest(sourceIndex, "type", "2").source(Collections.singletonMap("foo2", "bar2"), XContentType.JSON))
+                .add(new IndexRequest(sourceIndex).id("1").source(Collections.singletonMap("foo", "bar"), XContentType.JSON))
+                .add(new IndexRequest(sourceIndex).id("2").source(Collections.singletonMap("foo2", "bar2"), XContentType.JSON))
                 .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE);
             assertEquals(
                 RestStatus.OK,

docs/reference/docs/reindex.asciidoc

@@ -138,8 +138,8 @@ POST _reindex
 // CONSOLE
 // TEST[setup:twitter]
 
-You can limit the documents by adding a type to the `source` or by adding a
-query. This will only copy tweets made by `kimchy` into `new_twitter`:
+You can limit the documents by adding a query to the `source`.
+This will only copy tweets made by `kimchy` into `new_twitter`:
 
 [source,js]
 --------------------------------------------------
@@ -147,7 +147,6 @@ POST _reindex
 {
   "source": {
     "index": "twitter",
-    "type": "_doc",
     "query": {
       "term": {
         "user": "kimchy"
@@ -162,21 +161,19 @@ POST _reindex
 // CONSOLE
 // TEST[setup:twitter]
 
-`index` and `type` in `source` can both be lists, allowing you to copy from
-lots of sources in one request. This will copy documents from the `_doc` and
-`post` types in the `twitter` and `blog` indices.
+`index` in `source` can be a list, allowing you to copy from lots 
+of sources in one request. This will copy documents from the
+`twitter` and `blog` indices:
 
 [source,js]
 --------------------------------------------------
 POST _reindex
 {
   "source": {
-    "index": ["twitter", "blog"],
-    "type": ["_doc", "post"]
+    "index": ["twitter", "blog"]
   },
   "dest": {
-    "index": "all_together",
-    "type": "_doc"
+    "index": "all_together"
   }
 }
 --------------------------------------------------
@@ -299,7 +296,6 @@ Think of the possibilities! Just be careful; you are able to
 change:
 
  * `_id`
- * `_type`
  * `_index`
  * `_version`
  * `_routing`

modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java

@@ -19,11 +19,13 @@
 
 package org.elasticsearch.index.reindex;
 
+import org.apache.logging.log4j.LogManager;
 import org.elasticsearch.action.index.IndexRequest;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.xcontent.ObjectParser;
@@ -56,6 +58,8 @@ import static org.elasticsearch.rest.RestRequest.Method.POST;
  */
 public class RestReindexAction extends AbstractBaseReindexRestHandler<ReindexRequest, ReindexAction> {
     static final ObjectParser<ReindexRequest, Void> PARSER = new ObjectParser<>("reindex");
+    static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in reindex requests is deprecated.";
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestReindexAction.class));
 
     static {
         ObjectParser.Parser<ReindexRequest, Void> sourceParser = (parser, request, context) -> {
@@ -67,6 +71,7 @@ public class RestReindexAction extends AbstractBaseReindexRestHandler<ReindexReq
             }
             String[] types = extractStringArray(source, "type");
             if (types != null) {
+                deprecationLogger.deprecatedAndMaybeLog("reindex_with_types", TYPES_DEPRECATION_MESSAGE);
                 request.getSearchRequest().types(types);
             }
             request.setRemoteInfo(buildRemoteInfo(source));
@@ -81,7 +86,10 @@ public class RestReindexAction extends AbstractBaseReindexRestHandler<ReindexReq
 
         ObjectParser<IndexRequest, Void> destParser = new ObjectParser<>("dest");
         destParser.declareString(IndexRequest::index, new ParseField("index"));
-        destParser.declareString(IndexRequest::type, new ParseField("type"));
+        destParser.declareString((request, type) -> {
+            deprecationLogger.deprecatedAndMaybeLog("reindex_with_types", TYPES_DEPRECATION_MESSAGE);
+            request.type(type);
+        }, new ParseField("type"));
         destParser.declareString(IndexRequest::routing, new ParseField("routing"));
         destParser.declareString(IndexRequest::opType, new ParseField("op_type"));
         destParser.declareString(IndexRequest::setPipeline, new ParseField("pipeline"));

modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestReindexActionTests.java

@@ -26,19 +26,28 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
-import org.elasticsearch.rest.RestController;
-import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.rest.RestRequest.Method;
 import org.elasticsearch.test.rest.FakeRestRequest;
+import org.elasticsearch.test.rest.RestActionTestCase;
+import org.junit.Before;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 
 import static java.util.Collections.singletonMap;
 import static org.elasticsearch.common.unit.TimeValue.timeValueSeconds;
-import static org.mockito.Mockito.mock;
 
-public class RestReindexActionTests extends ESTestCase {
+public class RestReindexActionTests extends RestActionTestCase {
+
+    private RestReindexAction action;
+
+    @Before
+    public void setUpAction() {
+        action = new RestReindexAction(Settings.EMPTY, controller());
+    }
+
     public void testBuildRemoteInfoNoRemote() throws IOException {
         assertNull(RestReindexAction.buildRemoteInfo(new HashMap<>()));
     }
@@ -160,8 +169,6 @@ public class RestReindexActionTests extends ESTestCase {
     }
 
     public void testPipelineQueryParameterIsError() throws IOException {
-        RestReindexAction action = new RestReindexAction(Settings.EMPTY, mock(RestController.class));
-
         FakeRestRequest.Builder request = new FakeRestRequest.Builder(xContentRegistry());
         try (XContentBuilder body = JsonXContent.contentBuilder().prettyPrint()) {
             body.startObject(); {
@@ -185,14 +192,12 @@ public class RestReindexActionTests extends ESTestCase {
 
     public void testSetScrollTimeout() throws IOException {
         {
-            RestReindexAction action = new RestReindexAction(Settings.EMPTY, mock(RestController.class));
             FakeRestRequest.Builder requestBuilder = new FakeRestRequest.Builder(xContentRegistry());
             requestBuilder.withContent(new BytesArray("{}"), XContentType.JSON);
             ReindexRequest request = action.buildRequest(requestBuilder.build());
             assertEquals(AbstractBulkByScrollRequest.DEFAULT_SCROLL_TIMEOUT, request.getScrollTime());
         }
         {
-            RestReindexAction action = new RestReindexAction(Settings.EMPTY, mock(RestController.class));
             FakeRestRequest.Builder requestBuilder = new FakeRestRequest.Builder(xContentRegistry());
             requestBuilder.withParams(singletonMap("scroll", "10m"));
             requestBuilder.withContent(new BytesArray("{}"), XContentType.JSON);
@@ -210,4 +215,46 @@ public class RestReindexActionTests extends ESTestCase {
 
         return RestReindexAction.buildRemoteInfo(source);
     }
+
+    /**
+     * test deprecation is logged if one or more types are used in source search request inside reindex
+     */
+    public void testTypeInSource() throws IOException {
+        FakeRestRequest.Builder requestBuilder = new FakeRestRequest.Builder(xContentRegistry())
+                .withMethod(Method.POST)
+                .withPath("/_reindex");
+        XContentBuilder b = JsonXContent.contentBuilder().startObject();
+        {
+            b.startObject("source");
+            {
+                b.field("type", randomFrom(Arrays.asList("\"t1\"", "[\"t1\", \"t2\"]", "\"_doc\"")));
+            }
+            b.endObject();
+        }
+        b.endObject();
+        requestBuilder.withContent(new BytesArray(BytesReference.bytes(b).toBytesRef()), XContentType.JSON);
+        dispatchRequest(requestBuilder.build());
+        assertWarnings(RestReindexAction.TYPES_DEPRECATION_MESSAGE);
+    }
+
+    /**
+     * test deprecation is logged if a type is used in the destination index request inside reindex
+     */
+    public void testTypeInDestination() throws IOException {
+        FakeRestRequest.Builder requestBuilder = new FakeRestRequest.Builder(xContentRegistry())
+                .withMethod(Method.POST)
+                .withPath("/_reindex");
+        XContentBuilder b = JsonXContent.contentBuilder().startObject();
+        {
+            b.startObject("dest");
+            {
+                b.field("type", (randomBoolean() ? "_doc" : randomAlphaOfLength(4)));
+            }
+            b.endObject();
+        }
+        b.endObject();
+        requestBuilder.withContent(new BytesArray(BytesReference.bytes(b).toBytesRef()), XContentType.JSON);
+        dispatchRequest(requestBuilder.build());
+        assertWarnings(RestReindexAction.TYPES_DEPRECATION_MESSAGE);
+    }
 }

server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java

@@ -29,6 +29,7 @@ import org.elasticsearch.common.lucene.uid.Versions;
 import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.index.VersionType;
+import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.search.sort.SortOrder;
 import org.elasticsearch.tasks.TaskId;
@@ -294,7 +295,10 @@ public class ReindexRequest extends AbstractBulkIndexByScrollRequest<ReindexRequ
                 builder.field("remote", remoteInfo);
             }
             builder.array("index", getSearchRequest().indices());
-            builder.array("type", getSearchRequest().types());
+            String[] types = getSearchRequest().types();
+            if (types.length > 0) {
+                builder.array("type", types);
+            }
             getSearchRequest().source().innerToXContent(builder, params);
             builder.endObject();
         }
@@ -302,7 +306,8 @@ public class ReindexRequest extends AbstractBulkIndexByScrollRequest<ReindexRequ
             // build destination
             builder.startObject("dest");
             builder.field("index", getDestination().index());
-            if (getDestination().type() != null) {
+            String type = getDestination().type();
+            if (type != null && type.equals(MapperService.SINGLE_MAPPING_NAME) == false) {
                 builder.field("type", getDestination().type());
             }
             if (getDestination().routing() != null) {