Skip to content

Commit 0bfb59d

Browse files
Ke Linik9000
Ke Li
authored and
nik9000
committed
Using ObjectParser in UpdateRequest (#29293)
CRUD: Parsing changes for UpdateRequest (#29293) Use `ObjectParser` to parse `UpdateRequest` so we reject unknown fields and drop support for the `_fields` parameter because it was deprecated in 5.x.
1 parent a004a33 commit 0bfb59d

File tree

18 files changed

+145
-275
lines changed

18 files changed

+145
-275
lines changed

‎client/client-benchmark-noop-api-plugin/src/main/java/org/elasticsearch/plugin/noop/action/bulk/RestNoopBulkAction.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.elasticsearch.action.update.UpdateResponse;
2828
import org.elasticsearch.client.Requests;
2929
import org.elasticsearch.client.node.NodeClient;
30-
import org.elasticsearch.common.Strings;
3130
import org.elasticsearch.common.settings.Settings;
3231
import org.elasticsearch.common.xcontent.XContentBuilder;
3332
import org.elasticsearch.index.shard.ShardId;
@@ -68,17 +67,15 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
6867
String defaultIndex = request.param("index");
6968
String defaultType = request.param("type");
7069
String defaultRouting = request.param("routing");
71-
String fieldsParam = request.param("fields");
7270
String defaultPipeline = request.param("pipeline");
73-
String[] defaultFields = fieldsParam != null ? Strings.commaDelimitedListToStringArray(fieldsParam) : null;
7471

7572
String waitForActiveShards = request.param("wait_for_active_shards");
7673
if (waitForActiveShards != null) {
7774
bulkRequest.waitForActiveShards(ActiveShardCount.parseString(waitForActiveShards));
7875
}
7976
bulkRequest.timeout(request.paramAsTime("timeout", BulkShardRequest.DEFAULT_TIMEOUT));
8077
bulkRequest.setRefreshPolicy(request.param("refresh"));
81-
bulkRequest.add(request.requiredContent(), defaultIndex, defaultType, defaultRouting, defaultFields,
78+
bulkRequest.add(request.requiredContent(), defaultIndex, defaultType, defaultRouting,
8279
null, defaultPipeline, null, true, request.getXContentType());
8380

8481
// short circuit the call to the transport layer

‎docs/reference/migration/migrate_7_0/api.asciidoc‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
=== Breaking API changes in 7.0
33

44
==== Camel case and underscore parameters deprecated in 6.x have been removed
5-
A number of duplicate parameters deprecated in 6.x have been removed from
5+
A number of duplicate parameters deprecated in 6.x have been removed from
66
Bulk request, Multi Get request, Term Vectors request, and More Like This Query
77
requests.
88

@@ -22,3 +22,7 @@ The following parameters starting with underscore have been removed:
2222
Instead of these removed parameters, use their non camel case equivalents without
2323
starting underscore, e.g. use `version_type` instead of `_version_type` or `versionType`.
2424

25+
26+
==== The parameter `fields` deprecated in 6.x has been removed from Bulk request
27+
and Update request. The Update API returns `400 - Bad request` if request contains
28+
unknown parameters (instead of ignored in the previous version).

‎rest-api-spec/src/main/resources/rest-api-spec/api/bulk.json‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,6 @@
3737
"type" : "string",
3838
"description" : "Default document type for items which don't provide one"
3939
},
40-
"fields": {
41-
"type": "list",
42-
"description" : "Default comma-separated list of fields to return in the response for updates, can be overridden on each sub-request"
43-
},
4440
"_source": {
4541
"type" : "list",
4642
"description" : "True or false to return the _source field or not, or default list of fields to return, can be overridden on each sub-request"

‎rest-api-spec/src/main/resources/rest-api-spec/api/update.json‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,6 @@
2727
"type": "string",
2828
"description": "Sets the number of shard copies that must be active before proceeding with the update operation. Defaults to 1, meaning the primary shard only. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)"
2929
},
30-
"fields": {
31-
"type": "list",
32-
"description": "A comma-separated list of fields to return in the response"
33-
},
3430
"_source": {
3531
"type" : "list",
3632
"description" : "True or false to return the _source field or not, or a list of fields to return"

‎server/src/main/java/org/elasticsearch/action/bulk/BulkProcessor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ public BulkProcessor add(BytesReference data, @Nullable String defaultIndex, @Nu
299299
*/
300300
public synchronized BulkProcessor add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType,
301301
@Nullable String defaultPipeline, @Nullable Object payload, XContentType xContentType) throws Exception {
302-
bulkRequest.add(data, defaultIndex, defaultType, null, null, null, defaultPipeline, payload, true, xContentType);
302+
bulkRequest.add(data, defaultIndex, defaultType, null, null, defaultPipeline, payload, true, xContentType);
303303
executeIfNeeded();
304304
return this;
305305
}

‎server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java‎

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@
3636
import org.elasticsearch.common.bytes.BytesReference;
3737
import org.elasticsearch.common.io.stream.StreamInput;
3838
import org.elasticsearch.common.io.stream.StreamOutput;
39-
import org.elasticsearch.common.logging.DeprecationLogger;
40-
import org.elasticsearch.common.logging.Loggers;
4139
import org.elasticsearch.common.lucene.uid.Versions;
4240
import org.elasticsearch.common.unit.TimeValue;
4341
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
@@ -66,8 +64,6 @@
6664
* @see org.elasticsearch.client.Client#bulk(BulkRequest)
6765
*/
6866
public class BulkRequest extends ActionRequest implements CompositeIndicesRequest, WriteRequest<BulkRequest> {
69-
private static final DeprecationLogger DEPRECATION_LOGGER =
70-
new DeprecationLogger(Loggers.getLogger(BulkRequest.class));
7167

7268
private static final int REQUEST_OVERHEAD = 50;
7369

@@ -80,7 +76,6 @@ public class BulkRequest extends ActionRequest implements CompositeIndicesReques
8076
private static final ParseField VERSION_TYPE = new ParseField("version_type");
8177
private static final ParseField RETRY_ON_CONFLICT = new ParseField("retry_on_conflict");
8278
private static final ParseField PIPELINE = new ParseField("pipeline");
83-
private static final ParseField FIELDS = new ParseField("fields");
8479
private static final ParseField SOURCE = new ParseField("_source");
8580

8681
/**
@@ -277,20 +272,21 @@ public BulkRequest add(byte[] data, int from, int length, @Nullable String defau
277272
*/
278273
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType,
279274
XContentType xContentType) throws IOException {
280-
return add(data, defaultIndex, defaultType, null, null, null, null, null, true, xContentType);
275+
return add(data, defaultIndex, defaultType, null, null, null, null, true, xContentType);
281276
}
282277

283278
/**
284279
* Adds a framed data in binary format
285280
*/
286281
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, boolean allowExplicitIndex,
287282
XContentType xContentType) throws IOException {
288-
return add(data, defaultIndex, defaultType, null, null, null, null, null, allowExplicitIndex, xContentType);
283+
return add(data, defaultIndex, defaultType, null, null, null, null, allowExplicitIndex, xContentType);
289284
}
290285

291-
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, @Nullable String
292-
defaultRouting, @Nullable String[] defaultFields, @Nullable FetchSourceContext defaultFetchSourceContext, @Nullable String
293-
defaultPipeline, @Nullable Object payload, boolean allowExplicitIndex, XContentType xContentType) throws IOException {
286+
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType,
287+
@Nullable String defaultRouting, @Nullable FetchSourceContext defaultFetchSourceContext,
288+
@Nullable String defaultPipeline, @Nullable Object payload, boolean allowExplicitIndex,
289+
XContentType xContentType) throws IOException {
294290
XContent xContent = xContentType.xContent();
295291
int line = 0;
296292
int from = 0;
@@ -333,7 +329,6 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
333329
String id = null;
334330
String routing = defaultRouting;
335331
FetchSourceContext fetchSourceContext = defaultFetchSourceContext;
336-
String[] fields = defaultFields;
337332
String opType = null;
338333
long version = Versions.MATCH_ANY;
339334
VersionType versionType = VersionType.INTERNAL;
@@ -371,21 +366,14 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
371366
retryOnConflict = parser.intValue();
372367
} else if (PIPELINE.match(currentFieldName, parser.getDeprecationHandler())) {
373368
pipeline = parser.text();
374-
} else if (FIELDS.match(currentFieldName, parser.getDeprecationHandler())) {
375-
throw new IllegalArgumentException("Action/metadata line [" + line + "] contains a simple value for parameter [fields] while a list is expected");
376369
} else if (SOURCE.match(currentFieldName, parser.getDeprecationHandler())) {
377370
fetchSourceContext = FetchSourceContext.fromXContent(parser);
378371
} else {
379372
throw new IllegalArgumentException("Action/metadata line [" + line + "] contains an unknown parameter [" + currentFieldName + "]");
380373
}
381374
} else if (token == XContentParser.Token.START_ARRAY) {
382-
if (FIELDS.match(currentFieldName, parser.getDeprecationHandler())) {
383-
DEPRECATION_LOGGER.deprecated("Deprecated field [fields] used, expected [_source] instead");
384-
List<Object> values = parser.list();
385-
fields = values.toArray(new String[values.size()]);
386-
} else {
387-
throw new IllegalArgumentException("Malformed action/metadata line [" + line + "], expected a simple value for field [" + currentFieldName + "] but found [" + token + "]");
388-
}
375+
throw new IllegalArgumentException("Malformed action/metadata line [" + line +
376+
"], expected a simple value for field [" + currentFieldName + "] but found [" + token + "]");
389377
} else if (token == XContentParser.Token.START_OBJECT && SOURCE.match(currentFieldName, parser.getDeprecationHandler())) {
390378
fetchSourceContext = FetchSourceContext.fromXContent(parser);
391379
} else if (token != XContentParser.Token.VALUE_NULL) {
@@ -435,10 +423,6 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
435423
if (fetchSourceContext != null) {
436424
updateRequest.fetchSource(fetchSourceContext);
437425
}
438-
if (fields != null) {
439-
updateRequest.fields(fields);
440-
}
441-
442426
IndexRequest upsertRequest = updateRequest.upsertRequest();
443427
if (upsertRequest != null) {
444428
upsertRequest.version(version);

‎server/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,8 +291,7 @@ static BulkItemResultHolder processUpdateResponse(final UpdateRequest updateRequ
291291
indexResponse.getId(), indexResponse.getSeqNo(), indexResponse.getPrimaryTerm(), indexResponse.getVersion(),
292292
indexResponse.getResult());
293293

294-
if ((updateRequest.fetchSource() != null && updateRequest.fetchSource().fetchSource()) ||
295-
(updateRequest.fields() != null && updateRequest.fields().length > 0)) {
294+
if (updateRequest.fetchSource() != null && updateRequest.fetchSource().fetchSource()) {
296295
final BytesReference indexSourceAsBytes = updateIndexRequest.source();
297296
final Tuple<XContentType, Map<String, Object>> sourceAndContent =
298297
XContentHelper.convertToMap(indexSourceAsBytes, true, updateIndexRequest.getContentType());

‎server/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,7 @@ protected void shardOperation(final UpdateRequest request, final ActionListener<
180180
bulkAction.execute(toSingleItemBulkRequest(upsertRequest), wrapBulkResponse(
181181
ActionListener.<IndexResponse>wrap(response -> {
182182
UpdateResponse update = new UpdateResponse(response.getShardInfo(), response.getShardId(), response.getType(), response.getId(), response.getSeqNo(), response.getPrimaryTerm(), response.getVersion(), response.getResult());
183-
if ((request.fetchSource() != null && request.fetchSource().fetchSource()) ||
184-
(request.fields() != null && request.fields().length > 0)) {
183+
if (request.fetchSource() != null && request.fetchSource().fetchSource()) {
185184
Tuple<XContentType, Map<String, Object>> sourceAndContent =
186185
XContentHelper.convertToMap(upsertSourceBytes, true, upsertRequest.getContentType());
187186
update.setGetResult(UpdateHelper.extractGetResult(request, request.concreteIndex(), response.getVersion(), sourceAndContent.v2(), sourceAndContent.v1(), upsertSourceBytes));

‎server/src/main/java/org/elasticsearch/action/update/UpdateHelper.java‎

Lines changed: 15 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.elasticsearch.common.bytes.BytesReference;
3030
import org.elasticsearch.common.collect.Tuple;
3131
import org.elasticsearch.common.component.AbstractComponent;
32-
import org.elasticsearch.common.document.DocumentField;
3332
import org.elasticsearch.common.io.stream.BytesStreamOutput;
3433
import org.elasticsearch.common.io.stream.Streamable;
3534
import org.elasticsearch.common.settings.Settings;
@@ -49,7 +48,7 @@
4948
import org.elasticsearch.search.lookup.SourceLookup;
5049

5150
import java.io.IOException;
52-
import java.util.ArrayList;
51+
import java.util.Collections;
5352
import java.util.HashMap;
5453
import java.util.Map;
5554
import java.util.function.LongSupplier;
@@ -292,61 +291,33 @@ private Map<String, Object> executeScript(Script script, Map<String, Object> ctx
292291

293292
/**
294293
* Applies {@link UpdateRequest#fetchSource()} to the _source of the updated document to be returned in a update response.
295-
* For BWC this function also extracts the {@link UpdateRequest#fields()} from the updated document to be returned in a update response
296294
*/
297295
public static GetResult extractGetResult(final UpdateRequest request, String concreteIndex, long version,
298296
final Map<String, Object> source, XContentType sourceContentType,
299297
@Nullable final BytesReference sourceAsBytes) {
300-
if ((request.fields() == null || request.fields().length == 0) &&
301-
(request.fetchSource() == null || request.fetchSource().fetchSource() == false)) {
298+
if (request.fetchSource() == null || request.fetchSource().fetchSource() == false) {
302299
return null;
303300
}
304-
SourceLookup sourceLookup = new SourceLookup();
305-
sourceLookup.setSource(source);
306-
boolean sourceRequested = false;
307-
Map<String, DocumentField> fields = null;
308-
if (request.fields() != null && request.fields().length > 0) {
309-
for (String field : request.fields()) {
310-
if (field.equals("_source")) {
311-
sourceRequested = true;
312-
continue;
313-
}
314-
Object value = sourceLookup.extractValue(field);
315-
if (value != null) {
316-
if (fields == null) {
317-
fields = new HashMap<>(2);
318-
}
319-
DocumentField documentField = fields.get(field);
320-
if (documentField == null) {
321-
documentField = new DocumentField(field, new ArrayList<>(2));
322-
fields.put(field, documentField);
323-
}
324-
documentField.getValues().add(value);
325-
}
326-
}
327-
}
328301

329302
BytesReference sourceFilteredAsBytes = sourceAsBytes;
330-
if (request.fetchSource() != null && request.fetchSource().fetchSource()) {
331-
sourceRequested = true;
332-
if (request.fetchSource().includes().length > 0 || request.fetchSource().excludes().length > 0) {
333-
Object value = sourceLookup.filter(request.fetchSource());
334-
try {
335-
final int initialCapacity = Math.min(1024, sourceAsBytes.length());
336-
BytesStreamOutput streamOutput = new BytesStreamOutput(initialCapacity);
337-
try (XContentBuilder builder = new XContentBuilder(sourceContentType.xContent(), streamOutput)) {
338-
builder.value(value);
339-
sourceFilteredAsBytes = BytesReference.bytes(builder);
340-
}
341-
} catch (IOException e) {
342-
throw new ElasticsearchException("Error filtering source", e);
303+
if (request.fetchSource().includes().length > 0 || request.fetchSource().excludes().length > 0) {
304+
SourceLookup sourceLookup = new SourceLookup();
305+
sourceLookup.setSource(source);
306+
Object value = sourceLookup.filter(request.fetchSource());
307+
try {
308+
final int initialCapacity = Math.min(1024, sourceAsBytes.length());
309+
BytesStreamOutput streamOutput = new BytesStreamOutput(initialCapacity);
310+
try (XContentBuilder builder = new XContentBuilder(sourceContentType.xContent(), streamOutput)) {
311+
builder.value(value);
312+
sourceFilteredAsBytes = BytesReference.bytes(builder);
343313
}
314+
} catch (IOException e) {
315+
throw new ElasticsearchException("Error filtering source", e);
344316
}
345317
}
346318

347319
// TODO when using delete/none, we can still return the source as bytes by generating it (using the sourceContentType)
348-
return new GetResult(concreteIndex, request.type(), request.id(), version, true,
349-
sourceRequested ? sourceFilteredAsBytes : null, fields);
320+
return new GetResult(concreteIndex, request.type(), request.id(), version, true, sourceFilteredAsBytes, Collections.emptyMap());
350321
}
351322

352323
public static class Result {

0 commit comments

Comments
 (0)