Do not allow version in Rest Update API (#43516)

The versioning of Update API doesn't rely on version number anymore (and
rather on sequence number). But in rest api level we ignored the
"version" and "version_type" parameter, so that the server cannot raise
the exception when whey were set.

This PR restores "version" and "version_type" parsing in Update Rest API
so that we can get the appropriate errors.

Relates to #42497
This commit is contained in:
Yu 2019-07-17 00:53:47 +08:00 committed by Nhat Nguyen
parent 5ea89a4e7b
commit 563a78829f
3 changed files with 50 additions and 22 deletions

View File

@ -5,8 +5,7 @@ The update API allows to update a document based on a script provided.
The operation gets the document (collocated with the shard) from the
index, runs the script (with optional script language and parameters),
and indexes back the result (also allows to delete, or ignore the
operation). It uses versioning to make sure no updates have happened
during the "get" and "reindex".
operation).
Note, this operation still means full reindex of the document, it just
removes some network roundtrips and reduces chances of version conflicts
@ -333,25 +332,6 @@ Allows to control if and how the updated source should be returned in the respon
By default the updated source is not returned.
See <<search-request-source-filtering, Source filtering>> for details.
`version`::
The update API uses the Elasticsearch versioning support internally to make
sure the document doesn't change during the update. You can use the `version`
parameter to specify that the document should only be updated if its version
matches the one specified.
[NOTE]
.The update API does not support versioning other than internal
=====================================================
External (version types `external` and `external_gte`) or forced (version type `force`)
versioning is not supported by the update API as it would result in Elasticsearch
version numbers being out of sync with the external system. Use the
<<docs-index_,`index` API>> instead.
=====================================================
`if_seq_no` and `if_primary_term`::
Update operations can be made conditional and only be performed if the last

View File

@ -20,6 +20,7 @@
package org.elasticsearch.rest.action.document;
import org.apache.logging.log4j.LogManager;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.action.update.UpdateRequest;
@ -83,6 +84,12 @@ public class RestUpdateAction extends BaseRestHandler {
}
updateRequest.retryOnConflict(request.paramAsInt("retry_on_conflict", updateRequest.retryOnConflict()));
if (request.hasParam("version") || request.hasParam("version_type")) {
final ActionRequestValidationException versioningError = new ActionRequestValidationException();
versioningError.addValidationError("internal versioning can not be used for optimistic concurrency control. " +
"Please use `if_seq_no` and `if_primary_term` instead");
throw versioningError;
}
updateRequest.setIfSeqNo(request.paramAsLong("if_seq_no", updateRequest.ifSeqNo()));
updateRequest.setIfPrimaryTerm(request.paramAsLong("if_primary_term", updateRequest.ifPrimaryTerm()));

View File

@ -19,18 +19,31 @@
package org.elasticsearch.rest.action.document;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.VersionType;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.RestRequest.Method;
import org.elasticsearch.test.rest.FakeRestRequest;
import org.elasticsearch.test.rest.RestActionTestCase;
import org.junit.Before;
import java.util.HashMap;
import java.util.Map;
import static org.hamcrest.CoreMatchers.containsString;
import static org.mockito.Mockito.mock;
public class RestUpdateActionTests extends RestActionTestCase {
private RestUpdateAction action;
@Before
public void setUpAction() {
new RestUpdateAction(Settings.EMPTY, controller());
action = new RestUpdateAction(Settings.EMPTY, controller());
}
public void testTypeInPath() {
@ -47,4 +60,32 @@ public class RestUpdateActionTests extends RestActionTestCase {
.build();
dispatchRequest(validRequest);
}
public void testUpdateDocVersion() {
Map<String, String> params = new HashMap<>();
if (randomBoolean()) {
params.put("version", Long.toString(randomNonNegativeLong()));
params.put("version_type", randomFrom(VersionType.values()).name());
} else if (randomBoolean()) {
params.put("version", Long.toString(randomNonNegativeLong()));
} else {
params.put("version_type", randomFrom(VersionType.values()).name());
}
String content =
"{\n" +
" \"doc\" : {\n" +
" \"name\" : \"new_name\"\n" +
" }\n" +
"}";
FakeRestRequest updateRequest = new FakeRestRequest.Builder(xContentRegistry())
.withMethod(RestRequest.Method.POST)
.withPath("test/_update/1")
.withParams(params)
.withContent(new BytesArray(content), XContentType.JSON)
.build();
ActionRequestValidationException e = expectThrows(ActionRequestValidationException.class,
() -> action.prepareRequest(updateRequest, mock(NodeClient.class)));
assertThat(e.getMessage(), containsString("internal versioning can not be used for optimistic concurrency control. " +
"Please use `if_seq_no` and `if_primary_term` instead"));
}
}