Skip to content

Commit 046f86f

Browse files
author
Christoph Büscher
authored
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.
1 parent e21054d commit 046f86f

File tree

5 files changed

+83
-27
lines changed

5 files changed

+83
-27
lines changed

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ public void testReindex() throws IOException {
4848
createIndex(sourceIndex, settings);
4949
createIndex(destinationIndex, settings);
5050
BulkRequest bulkRequest = new BulkRequest()
51-
.add(new IndexRequest(sourceIndex, "type", "1").source(Collections.singletonMap("foo", "bar"), XContentType.JSON))
52-
.add(new IndexRequest(sourceIndex, "type", "2").source(Collections.singletonMap("foo2", "bar2"), XContentType.JSON))
51+
.add(new IndexRequest(sourceIndex).id("1").source(Collections.singletonMap("foo", "bar"), XContentType.JSON))
52+
.add(new IndexRequest(sourceIndex).id("2").source(Collections.singletonMap("foo2", "bar2"), XContentType.JSON))
5353
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE);
5454
assertEquals(
5555
RestStatus.OK,
@@ -64,7 +64,7 @@ public void testReindex() throws IOException {
6464
ReindexRequest reindexRequest = new ReindexRequest();
6565
reindexRequest.setSourceIndices(sourceIndex);
6666
reindexRequest.setDestIndex(destinationIndex);
67-
reindexRequest.setSourceQuery(new IdsQueryBuilder().addIds("1").types("type"));
67+
reindexRequest.setSourceQuery(new IdsQueryBuilder().addIds("1"));
6868
reindexRequest.setRefresh(true);
6969

7070
BulkByScrollResponse bulkResponse = execute(reindexRequest, highLevelClient()::reindex, highLevelClient()::reindexAsync);
@@ -94,8 +94,8 @@ public void testReindexTask() throws IOException, InterruptedException {
9494
createIndex(sourceIndex, settings);
9595
createIndex(destinationIndex, settings);
9696
BulkRequest bulkRequest = new BulkRequest()
97-
.add(new IndexRequest(sourceIndex, "type", "1").source(Collections.singletonMap("foo", "bar"), XContentType.JSON))
98-
.add(new IndexRequest(sourceIndex, "type", "2").source(Collections.singletonMap("foo2", "bar2"), XContentType.JSON))
97+
.add(new IndexRequest(sourceIndex).id("1").source(Collections.singletonMap("foo", "bar"), XContentType.JSON))
98+
.add(new IndexRequest(sourceIndex).id("2").source(Collections.singletonMap("foo2", "bar2"), XContentType.JSON))
9999
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE);
100100
assertEquals(
101101
RestStatus.OK,

docs/reference/docs/reindex.asciidoc

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -138,16 +138,15 @@ POST _reindex
138138
// CONSOLE
139139
// TEST[setup:twitter]
140140

141-
You can limit the documents by adding a type to the `source` or by adding a
142-
query. This will only copy tweets made by `kimchy` into `new_twitter`:
141+
You can limit the documents by adding a query to the `source`.
142+
This will only copy tweets made by `kimchy` into `new_twitter`:
143143

144144
[source,js]
145145
--------------------------------------------------
146146
POST _reindex
147147
{
148148
"source": {
149149
"index": "twitter",
150-
"type": "_doc",
151150
"query": {
152151
"term": {
153152
"user": "kimchy"
@@ -162,21 +161,19 @@ POST _reindex
162161
// CONSOLE
163162
// TEST[setup:twitter]
164163

165-
`index` and `type` in `source` can both be lists, allowing you to copy from
166-
lots of sources in one request. This will copy documents from the `_doc` and
167-
`post` types in the `twitter` and `blog` indices.
164+
`index` in `source` can be a list, allowing you to copy from lots
165+
of sources in one request. This will copy documents from the
166+
`twitter` and `blog` indices:
168167

169168
[source,js]
170169
--------------------------------------------------
171170
POST _reindex
172171
{
173172
"source": {
174-
"index": ["twitter", "blog"],
175-
"type": ["_doc", "post"]
173+
"index": ["twitter", "blog"]
176174
},
177175
"dest": {
178-
"index": "all_together",
179-
"type": "_doc"
176+
"index": "all_together"
180177
}
181178
}
182179
--------------------------------------------------
@@ -299,7 +296,6 @@ Think of the possibilities! Just be careful; you are able to
299296
change:
300297

301298
* `_id`
302-
* `_type`
303299
* `_index`
304300
* `_version`
305301
* `_routing`

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@
1919

2020
package org.elasticsearch.index.reindex;
2121

22+
import org.apache.logging.log4j.LogManager;
2223
import org.elasticsearch.action.index.IndexRequest;
2324
import org.elasticsearch.client.node.NodeClient;
2425
import org.elasticsearch.common.ParseField;
2526
import org.elasticsearch.common.Strings;
2627
import org.elasticsearch.common.bytes.BytesReference;
28+
import org.elasticsearch.common.logging.DeprecationLogger;
2729
import org.elasticsearch.common.settings.Settings;
2830
import org.elasticsearch.common.unit.TimeValue;
2931
import org.elasticsearch.common.xcontent.ObjectParser;
@@ -56,6 +58,8 @@
5658
*/
5759
public class RestReindexAction extends AbstractBaseReindexRestHandler<ReindexRequest, ReindexAction> {
5860
static final ObjectParser<ReindexRequest, Void> PARSER = new ObjectParser<>("reindex");
61+
static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Specifying types in reindex requests is deprecated.";
62+
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestReindexAction.class));
5963

6064
static {
6165
ObjectParser.Parser<ReindexRequest, Void> sourceParser = (parser, request, context) -> {
@@ -67,6 +71,7 @@ public class RestReindexAction extends AbstractBaseReindexRestHandler<ReindexReq
6771
}
6872
String[] types = extractStringArray(source, "type");
6973
if (types != null) {
74+
deprecationLogger.deprecatedAndMaybeLog("reindex_with_types", TYPES_DEPRECATION_MESSAGE);
7075
request.getSearchRequest().types(types);
7176
}
7277
request.setRemoteInfo(buildRemoteInfo(source));
@@ -81,7 +86,10 @@ public class RestReindexAction extends AbstractBaseReindexRestHandler<ReindexReq
8186

8287
ObjectParser<IndexRequest, Void> destParser = new ObjectParser<>("dest");
8388
destParser.declareString(IndexRequest::index, new ParseField("index"));
84-
destParser.declareString(IndexRequest::type, new ParseField("type"));
89+
destParser.declareString((request, type) -> {
90+
deprecationLogger.deprecatedAndMaybeLog("reindex_with_types", TYPES_DEPRECATION_MESSAGE);
91+
request.type(type);
92+
}, new ParseField("type"));
8593
destParser.declareString(IndexRequest::routing, new ParseField("routing"));
8694
destParser.declareString(IndexRequest::opType, new ParseField("op_type"));
8795
destParser.declareString(IndexRequest::setPipeline, new ParseField("pipeline"));

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

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,28 @@
2626
import org.elasticsearch.common.xcontent.XContentParser;
2727
import org.elasticsearch.common.xcontent.XContentType;
2828
import org.elasticsearch.common.xcontent.json.JsonXContent;
29-
import org.elasticsearch.rest.RestController;
30-
import org.elasticsearch.test.ESTestCase;
29+
import org.elasticsearch.rest.RestRequest.Method;
3130
import org.elasticsearch.test.rest.FakeRestRequest;
31+
import org.elasticsearch.test.rest.RestActionTestCase;
32+
import org.junit.Before;
3233

3334
import java.io.IOException;
35+
import java.util.Arrays;
3436
import java.util.HashMap;
3537
import java.util.Map;
3638

3739
import static java.util.Collections.singletonMap;
3840
import static org.elasticsearch.common.unit.TimeValue.timeValueSeconds;
39-
import static org.mockito.Mockito.mock;
4041

41-
public class RestReindexActionTests extends ESTestCase {
42+
public class RestReindexActionTests extends RestActionTestCase {
43+
44+
private RestReindexAction action;
45+
46+
@Before
47+
public void setUpAction() {
48+
action = new RestReindexAction(Settings.EMPTY, controller());
49+
}
50+
4251
public void testBuildRemoteInfoNoRemote() throws IOException {
4352
assertNull(RestReindexAction.buildRemoteInfo(new HashMap<>()));
4453
}
@@ -160,8 +169,6 @@ public void testReindexFromRemoteRequestParsing() throws IOException {
160169
}
161170

162171
public void testPipelineQueryParameterIsError() throws IOException {
163-
RestReindexAction action = new RestReindexAction(Settings.EMPTY, mock(RestController.class));
164-
165172
FakeRestRequest.Builder request = new FakeRestRequest.Builder(xContentRegistry());
166173
try (XContentBuilder body = JsonXContent.contentBuilder().prettyPrint()) {
167174
body.startObject(); {
@@ -185,14 +192,12 @@ public void testPipelineQueryParameterIsError() throws IOException {
185192

186193
public void testSetScrollTimeout() throws IOException {
187194
{
188-
RestReindexAction action = new RestReindexAction(Settings.EMPTY, mock(RestController.class));
189195
FakeRestRequest.Builder requestBuilder = new FakeRestRequest.Builder(xContentRegistry());
190196
requestBuilder.withContent(new BytesArray("{}"), XContentType.JSON);
191197
ReindexRequest request = action.buildRequest(requestBuilder.build());
192198
assertEquals(AbstractBulkByScrollRequest.DEFAULT_SCROLL_TIMEOUT, request.getScrollTime());
193199
}
194200
{
195-
RestReindexAction action = new RestReindexAction(Settings.EMPTY, mock(RestController.class));
196201
FakeRestRequest.Builder requestBuilder = new FakeRestRequest.Builder(xContentRegistry());
197202
requestBuilder.withParams(singletonMap("scroll", "10m"));
198203
requestBuilder.withContent(new BytesArray("{}"), XContentType.JSON);
@@ -210,4 +215,46 @@ private RemoteInfo buildRemoteInfoHostTestCase(String hostInRest) throws IOExcep
210215

211216
return RestReindexAction.buildRemoteInfo(source);
212217
}
218+
219+
/**
220+
* test deprecation is logged if one or more types are used in source search request inside reindex
221+
*/
222+
public void testTypeInSource() throws IOException {
223+
FakeRestRequest.Builder requestBuilder = new FakeRestRequest.Builder(xContentRegistry())
224+
.withMethod(Method.POST)
225+
.withPath("/_reindex");
226+
XContentBuilder b = JsonXContent.contentBuilder().startObject();
227+
{
228+
b.startObject("source");
229+
{
230+
b.field("type", randomFrom(Arrays.asList("\"t1\"", "[\"t1\", \"t2\"]", "\"_doc\"")));
231+
}
232+
b.endObject();
233+
}
234+
b.endObject();
235+
requestBuilder.withContent(new BytesArray(BytesReference.bytes(b).toBytesRef()), XContentType.JSON);
236+
dispatchRequest(requestBuilder.build());
237+
assertWarnings(RestReindexAction.TYPES_DEPRECATION_MESSAGE);
238+
}
239+
240+
/**
241+
* test deprecation is logged if a type is used in the destination index request inside reindex
242+
*/
243+
public void testTypeInDestination() throws IOException {
244+
FakeRestRequest.Builder requestBuilder = new FakeRestRequest.Builder(xContentRegistry())
245+
.withMethod(Method.POST)
246+
.withPath("/_reindex");
247+
XContentBuilder b = JsonXContent.contentBuilder().startObject();
248+
{
249+
b.startObject("dest");
250+
{
251+
b.field("type", (randomBoolean() ? "_doc" : randomAlphaOfLength(4)));
252+
}
253+
b.endObject();
254+
}
255+
b.endObject();
256+
requestBuilder.withContent(new BytesArray(BytesReference.bytes(b).toBytesRef()), XContentType.JSON);
257+
dispatchRequest(requestBuilder.build());
258+
assertWarnings(RestReindexAction.TYPES_DEPRECATION_MESSAGE);
259+
}
213260
}

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.elasticsearch.common.xcontent.ToXContentObject;
3030
import org.elasticsearch.common.xcontent.XContentBuilder;
3131
import org.elasticsearch.index.VersionType;
32+
import org.elasticsearch.index.mapper.MapperService;
3233
import org.elasticsearch.index.query.QueryBuilder;
3334
import org.elasticsearch.search.sort.SortOrder;
3435
import org.elasticsearch.tasks.TaskId;
@@ -294,15 +295,19 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
294295
builder.field("remote", remoteInfo);
295296
}
296297
builder.array("index", getSearchRequest().indices());
297-
builder.array("type", getSearchRequest().types());
298+
String[] types = getSearchRequest().types();
299+
if (types.length > 0) {
300+
builder.array("type", types);
301+
}
298302
getSearchRequest().source().innerToXContent(builder, params);
299303
builder.endObject();
300304
}
301305
{
302306
// build destination
303307
builder.startObject("dest");
304308
builder.field("index", getDestination().index());
305-
if (getDestination().type() != null) {
309+
String type = getDestination().type();
310+
if (type != null && type.equals(MapperService.SINGLE_MAPPING_NAME) == false) {
306311
builder.field("type", getDestination().type());
307312
}
308313
if (getDestination().routing() != null) {

0 commit comments

Comments
 (0)