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

Author: PnPie

docs/reference/docs/update.asciidoc

@@ -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

server/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java

@@ -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 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()));

server/src/test/java/org/elasticsearch/rest/action/document/RestUpdateActionTests.java

@@ -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 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"));
+    }
 }