Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.elasticsearch.action.update.UpdateResponse;
import org.elasticsearch.client.Requests;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.shard.ShardId;
Expand Down Expand Up @@ -68,17 +67,15 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
String defaultIndex = request.param("index");
String defaultType = request.param("type");
String defaultRouting = request.param("routing");
String fieldsParam = request.param("fields");
String defaultPipeline = request.param("pipeline");
String[] defaultFields = fieldsParam != null ? Strings.commaDelimitedListToStringArray(fieldsParam) : null;

String waitForActiveShards = request.param("wait_for_active_shards");
if (waitForActiveShards != null) {
bulkRequest.waitForActiveShards(ActiveShardCount.parseString(waitForActiveShards));
}
bulkRequest.timeout(request.paramAsTime("timeout", BulkShardRequest.DEFAULT_TIMEOUT));
bulkRequest.setRefreshPolicy(request.param("refresh"));
bulkRequest.add(request.requiredContent(), defaultIndex, defaultType, defaultRouting, defaultFields,
bulkRequest.add(request.requiredContent(), defaultIndex, defaultType, defaultRouting,
null, defaultPipeline, null, true, request.getXContentType());

// short circuit the call to the transport layer
Expand Down
3 changes: 3 additions & 0 deletions docs/reference/migration/migrate_7_0/api.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,6 @@ The following parameters starting with underscore have been removed:
Instead of these removed parameters, use their non camel case equivalents without
starting underscore, e.g. use `version_type` instead of `_version_type` or `versionType`.


==== The parameter `fields` deprecated in 6.x has been removed from Bulk request
and Update request.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add a note about the change you update request?

I'm not sure this is the right file for them but you may as well put them here for now and we'll move them if we decide there is a better spot.

Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,9 @@ public void testBulkUpdateUpsertWithParent() throws Exception {
"\n" +
"{" +
" \"script\" : {" +
" \"inline\" : \"ctx._source.field2 = 'value2'\"" +
" \"inline\" : \"ctx._source.field2 = 'value2'\"," +
" \"lang\" : \"" + InnerHitsIT.CustomScriptPlugin.NAME + "\"" +
" }," +
" \"lang\" : \"" + InnerHitsIT.CustomScriptPlugin.NAME + "\"," +
" \"upsert\" : {" +
" \"field1\" : \"value1'\"" +
" }" +
Expand Down
4 changes: 0 additions & 4 deletions rest-api-spec/src/main/resources/rest-api-spec/api/bulk.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,6 @@
"type" : "string",
"description" : "Default document type for items which don't provide one"
},
"fields": {
"type": "list",
"description" : "Default comma-separated list of fields to return in the response for updates, can be overridden on each sub-request"
},
"_source": {
"type" : "list",
"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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@
"type": "string",
"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)"
},
"fields": {
"type": "list",
"description": "A comma-separated list of fields to return in the response"
},
"_source": {
"type" : "list",
"description" : "True or false to return the _source field or not, or a list of fields to return"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
---
"Source filtering":
- skip:
version: " - 6.99.99"
reason: fields dropped in 7.0

- do:
index:
refresh: true
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
---
"Update check shard header":
- skip:
version: " - 6.99.99"
reason: fields dropped in 7.0

- do:
indices.create:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
---
"Update result field":
- skip:
version: " - 6.99.99"
reason: fields dropped in 7.0

- do:
update:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
---
"Doc upsert":
- skip:
version: " - 6.99.99"
reason: fields dropped in 7.0

- do:
update:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
---
"Doc as upsert":
- skip:
version: " - 6.99.99"
reason: fields dropped in 7.0

- do:
update:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
---
"Internal version":
- skip:
version: " - 6.99.99"
reason: fields dropped in 7.0

- do:
- do:
catch: missing
update:
index: test_1
Expand All @@ -11,15 +14,15 @@
body:
doc: { foo: baz }

- do:
- do:
index:
index: test_1
type: test
id: 1
body:
doc: { foo: baz }

- do:
- do:
catch: conflict
update:
index: test_1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
---
"Routing":
- skip:
version: " - 6.99.99"
reason: fields dropped in 7.0

- do:
indices.create:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ setup:
_parent: { type: "foo" }
---
"Parent":
- skip:
version: " - 6.99.99"
reason: fields dropped in 7.0

- do:
- do:
catch: /routing_missing_exception/
update:
index: test_1
Expand All @@ -20,7 +23,7 @@ setup:
doc: { foo: baz }
upsert: { foo: bar }

- do:
- do:
update:
index: test_1
type: test
Expand All @@ -30,18 +33,18 @@ setup:
doc: { foo: baz }
upsert: { foo: bar }

- do:
- do:
get:
index: test_1
type: test
id: 1
parent: 5
stored_fields: [_parent, _routing]

- match: { _parent: "5"}
- match: { _routing: "5"}
- match: { _parent: "5"}
- match: { _routing: "5"}

- do:
- do:
update:
index: test_1
type: test
Expand All @@ -51,7 +54,7 @@ setup:
body:
doc: { foo: baz }

- match: { get._source.foo: baz }
- match: { get._source.foo: baz }

---
"Parent omitted":
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
---
"Parent with routing":
- skip:
version: " - 6.99.99"
reason: fields dropped in 7.0

- do:
indices.create:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,37 +79,41 @@

---
"refresh=wait_for waits until changes are visible in search":
- do:
- skip:
version: " - 6.99.99"
reason: fields dropped in 7.0

- do:
index:
index: update_60_refresh_1
type: test
id: update_60_refresh_id1
body: { foo: bar }
refresh: true
- is_true: forced_refresh
- is_true: forced_refresh

- do:
- do:
search:
index: update_60_refresh_1
type: test
body:
query: { term: { _id: update_60_refresh_id1 }}
- match: { hits.total: 1 }
- match: { hits.total: 1 }

- do:
- do:
update:
index: update_60_refresh_1
type: test
id: update_60_refresh_id1
refresh: wait_for
body:
doc: { test: asdf }
- is_false: forced_refresh
- is_false: forced_refresh

- do:
- do:
search:
index: update_60_refresh_1
type: test
body:
query: { match: { test: asdf } }
- match: { hits.total: 1 }
- match: { hits.total: 1 }
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ public BulkProcessor add(BytesReference data, @Nullable String defaultIndex, @Nu
*/
public synchronized BulkProcessor add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType,
@Nullable String defaultPipeline, @Nullable Object payload, XContentType xContentType) throws Exception {
bulkRequest.add(data, defaultIndex, defaultType, null, null, null, defaultPipeline, payload, true, xContentType);
bulkRequest.add(data, defaultIndex, defaultType, null, null, defaultPipeline, payload, true, xContentType);
executeIfNeeded();
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.lucene.uid.Versions;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
Expand Down Expand Up @@ -66,8 +64,6 @@
* @see org.elasticsearch.client.Client#bulk(BulkRequest)
*/
public class BulkRequest extends ActionRequest implements CompositeIndicesRequest, WriteRequest<BulkRequest> {
private static final DeprecationLogger DEPRECATION_LOGGER =
new DeprecationLogger(Loggers.getLogger(BulkRequest.class));

private static final int REQUEST_OVERHEAD = 50;

Expand All @@ -81,7 +77,6 @@ public class BulkRequest extends ActionRequest implements CompositeIndicesReques
private static final ParseField VERSION_TYPE = new ParseField("version_type");
private static final ParseField RETRY_ON_CONFLICT = new ParseField("retry_on_conflict");
private static final ParseField PIPELINE = new ParseField("pipeline");
private static final ParseField FIELDS = new ParseField("fields");
private static final ParseField SOURCE = new ParseField("_source");

/**
Expand Down Expand Up @@ -278,20 +273,21 @@ public BulkRequest add(byte[] data, int from, int length, @Nullable String defau
*/
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType,
XContentType xContentType) throws IOException {
return add(data, defaultIndex, defaultType, null, null, null, null, null, true, xContentType);
return add(data, defaultIndex, defaultType, null, null, null, null, true, xContentType);
}

/**
* Adds a framed data in binary format
*/
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, boolean allowExplicitIndex,
XContentType xContentType) throws IOException {
return add(data, defaultIndex, defaultType, null, null, null, null, null, allowExplicitIndex, xContentType);
return add(data, defaultIndex, defaultType, null, null, null, null, allowExplicitIndex, xContentType);
}

public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, @Nullable String
defaultRouting, @Nullable String[] defaultFields, @Nullable FetchSourceContext defaultFetchSourceContext, @Nullable String
defaultPipeline, @Nullable Object payload, boolean allowExplicitIndex, XContentType xContentType) throws IOException {
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType,
@Nullable String defaultRouting, @Nullable FetchSourceContext defaultFetchSourceContext,
@Nullable String defaultPipeline, @Nullable Object payload, boolean allowExplicitIndex,
XContentType xContentType) throws IOException {
XContent xContent = xContentType.xContent();
int line = 0;
int from = 0;
Expand Down Expand Up @@ -335,7 +331,6 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
String routing = defaultRouting;
String parent = null;
FetchSourceContext fetchSourceContext = defaultFetchSourceContext;
String[] fields = defaultFields;
String opType = null;
long version = Versions.MATCH_ANY;
VersionType versionType = VersionType.INTERNAL;
Expand Down Expand Up @@ -375,21 +370,14 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
retryOnConflict = parser.intValue();
} else if (PIPELINE.match(currentFieldName, parser.getDeprecationHandler())) {
pipeline = parser.text();
} else if (FIELDS.match(currentFieldName, parser.getDeprecationHandler())) {
throw new IllegalArgumentException("Action/metadata line [" + line + "] contains a simple value for parameter [fields] while a list is expected");
} else if (SOURCE.match(currentFieldName, parser.getDeprecationHandler())) {
fetchSourceContext = FetchSourceContext.fromXContent(parser);
} else {
throw new IllegalArgumentException("Action/metadata line [" + line + "] contains an unknown parameter [" + currentFieldName + "]");
}
} else if (token == XContentParser.Token.START_ARRAY) {
if (FIELDS.match(currentFieldName, parser.getDeprecationHandler())) {
DEPRECATION_LOGGER.deprecated("Deprecated field [fields] used, expected [_source] instead");
List<Object> values = parser.list();
fields = values.toArray(new String[values.size()]);
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the stuff under the else actually makes us start parsing requests with arrays very strangely. I think it is worth keeping the throw new IllegalArgumentException("Malformed action/metadata line [" + line + "], expected a simple value for field [" + currentFieldName + "] but found [" + token + "]"); part in an if statement that checks for START_ARRAY. And probably adding a test with an array.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I put the throw back and add a test.

throw new IllegalArgumentException("Malformed action/metadata line [" + line + "], expected a simple value for field [" + currentFieldName + "] but found [" + token + "]");
}
throw new IllegalArgumentException("Malformed action/metadata line [" + line +
"], expected a simple value for field [" + currentFieldName + "] but found [" + token + "]");
} else if (token == XContentParser.Token.START_OBJECT && SOURCE.match(currentFieldName, parser.getDeprecationHandler())) {
fetchSourceContext = FetchSourceContext.fromXContent(parser);
} else if (token != XContentParser.Token.VALUE_NULL) {
Expand Down Expand Up @@ -440,10 +428,6 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
if (fetchSourceContext != null) {
updateRequest.fetchSource(fetchSourceContext);
}
if (fields != null) {
updateRequest.fields(fields);
}

IndexRequest upsertRequest = updateRequest.upsertRequest();
if (upsertRequest != null) {
upsertRequest.version(version);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,7 @@ static BulkItemResultHolder processUpdateResponse(final UpdateRequest updateRequ
indexResponse.getId(), indexResponse.getSeqNo(), indexResponse.getPrimaryTerm(), indexResponse.getVersion(),
indexResponse.getResult());

if ((updateRequest.fetchSource() != null && updateRequest.fetchSource().fetchSource()) ||
(updateRequest.fields() != null && updateRequest.fields().length > 0)) {
if (updateRequest.fetchSource() != null && updateRequest.fetchSource().fetchSource()) {
final BytesReference indexSourceAsBytes = updateIndexRequest.source();
final Tuple<XContentType, Map<String, Object>> sourceAndContent =
XContentHelper.convertToMap(indexSourceAsBytes, true, updateIndexRequest.getContentType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,7 @@ protected void shardOperation(final UpdateRequest request, final ActionListener<
bulkAction.execute(toSingleItemBulkRequest(upsertRequest), wrapBulkResponse(
ActionListener.<IndexResponse>wrap(response -> {
UpdateResponse update = new UpdateResponse(response.getShardInfo(), response.getShardId(), response.getType(), response.getId(), response.getSeqNo(), response.getPrimaryTerm(), response.getVersion(), response.getResult());
if ((request.fetchSource() != null && request.fetchSource().fetchSource()) ||
(request.fields() != null && request.fields().length > 0)) {
if (request.fetchSource() != null && request.fetchSource().fetchSource()) {
Tuple<XContentType, Map<String, Object>> sourceAndContent =
XContentHelper.convertToMap(upsertSourceBytes, true, upsertRequest.getContentType());
update.setGetResult(UpdateHelper.extractGetResult(request, request.concreteIndex(), response.getVersion(), sourceAndContent.v2(), sourceAndContent.v1(), upsertSourceBytes));
Expand Down
Loading