From 046f86f2748e5536690540bf33618f2d64927cb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 3 Jan 2019 10:29:14 +0100 Subject: [PATCH] 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. --- .../org/elasticsearch/client/ReindexIT.java | 10 +-- docs/reference/docs/reindex.asciidoc | 18 +++--- .../index/reindex/RestReindexAction.java | 10 ++- .../index/reindex/RestReindexActionTests.java | 63 ++++++++++++++++--- .../index/reindex/ReindexRequest.java | 9 ++- 5 files changed, 83 insertions(+), 27 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ReindexIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ReindexIT.java index f94cc41432c..73594d8a878 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ReindexIT.java +++ b/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, diff --git a/docs/reference/docs/reindex.asciidoc b/docs/reference/docs/reindex.asciidoc index 642bcb20518..b12a27d2e60 100644 --- a/docs/reference/docs/reindex.asciidoc +++ b/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` diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java index 150853b3170..ae9ca0be7ca 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java +++ b/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 { static final ObjectParser 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 sourceParser = (parser, request, context) -> { @@ -67,6 +71,7 @@ public class RestReindexAction extends AbstractBaseReindexRestHandler 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")); diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestReindexActionTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestReindexActionTests.java index 1c6b60b705a..f0aca38545b 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RestReindexActionTests.java +++ b/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); + } } diff --git a/server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java b/server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java index 52a1c89d4b3..7e5e4ea51ab 100644 --- a/server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java +++ b/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 0) { + builder.array("type", types); + } getSearchRequest().source().innerToXContent(builder, params); builder.endObject(); } @@ -302,7 +306,8 @@ public class ReindexRequest extends AbstractBulkIndexByScrollRequest