Skip to content

Do not allow version in Rest Update API #43516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 16, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
22 changes: 1 addition & 21 deletions docs/reference/docs/update.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -83,6 +84,12 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
}

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()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -47,4 +60,32 @@ public void testTypeInPath() {
.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"));
}
}