Fix UpdateRequest.fromXContent

server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java

@@ -30,6 +30,8 @@
 import org.elasticsearch.common.Nullable;
 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.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
@@ -56,6 +58,9 @@
 public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
         implements DocWriteRequest<UpdateRequest>, WriteRequest<UpdateRequest>, ToXContentObject {
 
+    private static final DeprecationLogger DEPRECATION_LOGGER =
+        new DeprecationLogger(Loggers.getLogger(UpdateRequest.class));
+
     private String type;
     private String id;
     @Nullable
@@ -726,7 +731,22 @@ public UpdateRequest fromXContent(XContentParser parser) throws IOException {
             return this;
         }
         String currentFieldName = null;
-        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
+        int depth = 0;
+        while (token != null) {
+            token = parser.nextToken();
+            // have to track depth to avoid premature end of parsing due to nested objects as
+            // update request could have item ( script / scripted_upsert / upsert / doc etc ) at any nested level
+            if (token == XContentParser.Token.START_OBJECT) {
+                depth++;
+            } else if (token == XContentParser.Token.END_OBJECT) {
+                currentFieldName = null;
+                if (depth == 0) {
+                    break;
+                }
+                depth--;
+                continue;
+            }
+
             if (token == XContentParser.Token.FIELD_NAME) {
                 currentFieldName = parser.currentName();
             } else if ("script".equals(currentFieldName)) {
@@ -746,6 +766,7 @@ public UpdateRequest fromXContent(XContentParser parser) throws IOException {
             } else if ("detect_noop".equals(currentFieldName)) {
                 detectNoop(parser.booleanValue());
             } else if ("fields".equals(currentFieldName)) {
+                DEPRECATION_LOGGER.deprecated("Deprecated field [fields] used, expected [_source] instead");
                 List<Object> fields = null;
                 if (token == XContentParser.Token.START_ARRAY) {
                     fields = (List) parser.list();
@@ -757,6 +778,14 @@ public UpdateRequest fromXContent(XContentParser parser) throws IOException {
                 }
             } else if ("_source".equals(currentFieldName)) {
                 fetchSourceContext = FetchSourceContext.fromXContent(parser);
+            } else {
+                DEPRECATION_LOGGER.deprecated("Unknown field [{}] used in {} which has no value and will not be accepted in future",
+                    currentFieldName, UpdateRequest.class.getSimpleName());
+            }
+
+            // copyCurrentStructure / SomeObject.fromXContent moves current token to END_OBJECT
+            if (parser.currentToken() == XContentParser.Token.END_OBJECT) {
+                depth--;
             }
         }
         if (script != null) {

server/src/main/java/org/elasticsearch/action/update/UpdateRequestBuilder.java

@@ -31,15 +31,14 @@
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.VersionType;
-import org.elasticsearch.rest.action.document.RestUpdateAction;
 import org.elasticsearch.script.Script;
 
 import java.util.Map;
 
 public class UpdateRequestBuilder extends InstanceShardOperationRequestBuilder<UpdateRequest, UpdateResponse, UpdateRequestBuilder>
         implements WriteRequestBuilder<UpdateRequestBuilder> {
     private static final DeprecationLogger DEPRECATION_LOGGER =
-        new DeprecationLogger(Loggers.getLogger(RestUpdateAction.class));
+        new DeprecationLogger(Loggers.getLogger(UpdateRequestBuilder.class));
 
     public UpdateRequestBuilder(ElasticsearchClient client, UpdateAction action) {
         super(client, action, new UpdateRequest());

server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.action.update;
 
+import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.DocWriteResponse;
 import org.elasticsearch.action.delete.DeleteRequest;
 import org.elasticsearch.action.index.IndexRequest;
@@ -62,8 +63,10 @@
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
 import static org.hamcrest.Matchers.arrayContaining;
 import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 
 public class UpdateRequestTests extends ESTestCase {
@@ -275,17 +278,62 @@ public void testFromXContent() throws Exception {
         assertThat(((Map) doc.get("compound")).get("field2").toString(), equalTo("value2"));
     }
 
+    public void testFromXContentBothScriptAndNestedDocs() throws Exception {
+        // related to https://github.com/elastic/elasticsearch/issues/34069
+        UpdateRequest request = new UpdateRequest("test", "type1", "1")
+            .fromXContent(
+                createParser(JsonXContent.jsonXContent,
+                    new BytesArray("{\"any_odd_name_for_update\":{\"doc\":{\"message\":\"set by update:doc\"}},"
+                        + "\"script\":{\"source\":\"ctx._source.message = 'set by script'\"}}")));
+
+        assertWarnings("Unknown field [any_odd_name_for_update] used in UpdateRequest"
+            + " which has no value and will not be accepted in future");
+
+        assertThat(request.doc(), notNullValue());
+        assertThat(request.script(), notNullValue());
+
+        ActionRequestValidationException validate = request.validate();
+        assertThat(validate, notNullValue());
+        assertThat(validate.validationErrors(), not(empty()));
+        assertThat(String.valueOf(validate.validationErrors()),
+            validate.validationErrors(), contains("can't provide both script and doc"));
+
+        request = new UpdateRequest("test", "type1", "1")
+            .fromXContent(
+                createParser(JsonXContent.jsonXContent,
+                    new BytesArray("{\"whatever\": {\"script\":{\"source\":\"ctx._source.message = 'set by script'\"}},"
+                        + "\"nested_update1\":{\"nested_update2\":{\"doc\":{\"message\":\"set by update:doc\"}}}}")));
+
+        assertWarnings("Unknown field [whatever] used in UpdateRequest"
+            + " which has no value and will not be accepted in future",
+            "Unknown field [nested_update1] used in UpdateRequest"
+                + " which has no value and will not be accepted in future",
+            "Unknown field [nested_update2] used in UpdateRequest"
+                + " which has no value and will not be accepted in future");
+
+        assertThat(request.doc(), notNullValue());
+        assertThat(request.script(), notNullValue());
+
+        validate = request.validate();
+        assertThat(validate, notNullValue());
+        assertThat(validate.validationErrors(), not(empty()));
+        assertThat(String.valueOf(validate.validationErrors()),
+            validate.validationErrors(), contains("can't provide both script and doc"));
+    }
+
     // Related to issue 15338
     public void testFieldsParsing() throws Exception {
         UpdateRequest request = new UpdateRequest("test", "type1", "1").fromXContent(
                 createParser(JsonXContent.jsonXContent, new BytesArray("{\"doc\": {\"field1\": \"value1\"}, \"fields\": \"_source\"}")));
         assertThat(request.doc().sourceAsMap().get("field1").toString(), equalTo("value1"));
         assertThat(request.fields(), arrayContaining("_source"));
+        assertWarnings("Deprecated field [fields] used, expected [_source] instead");
 
         request = new UpdateRequest("test", "type2", "2").fromXContent(createParser(JsonXContent.jsonXContent,
                 new BytesArray("{\"doc\": {\"field2\": \"value2\"}, \"fields\": [\"field1\", \"field2\"]}")));
         assertThat(request.doc().sourceAsMap().get("field2").toString(), equalTo("value2"));
         assertThat(request.fields(), arrayContaining("field1", "field2"));
+        assertWarnings("Deprecated field [fields] used, expected [_source] instead");
     }
 
     public void testFetchSourceParsing() throws Exception {
@@ -442,7 +490,9 @@ public void testToAndFromXContent() throws IOException {
             BytesReference source = RandomObjects.randomSource(random(), xContentType);
             updateRequest.upsert(new IndexRequest().source(source, xContentType));
         }
-        if (randomBoolean()) {
+
+        final boolean fieldsAreUsed = randomBoolean();
+        if (fieldsAreUsed) {
             String[] fields = new String[randomIntBetween(0, 5)];
             for (int i = 0; i < fields.length; i++) {
                 fields[i] = randomAlphaOfLength(5);
@@ -493,6 +543,10 @@ public void testToAndFromXContent() throws IOException {
 
         BytesReference finalBytes = toXContent(parsedUpdateRequest, xContentType, humanReadable);
         assertToXContentEquivalent(originalBytes, finalBytes, xContentType);
+
+        if (fieldsAreUsed) {
+            assertWarnings("Deprecated field [fields] used, expected [_source] instead");
+        }
     }
 
     public void testToValidateUpsertRequestAndVersion() {