diff --git a/server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java b/server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java index 2dcd35dfb36b9..744eae211be36 100644 --- a/server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java +++ b/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 implements DocWriteRequest, WriteRequest, 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 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) { diff --git a/server/src/main/java/org/elasticsearch/action/update/UpdateRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/update/UpdateRequestBuilder.java index 5ba187013e79f..c2b4222d86ce2 100644 --- a/server/src/main/java/org/elasticsearch/action/update/UpdateRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/update/UpdateRequestBuilder.java @@ -31,7 +31,6 @@ 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; @@ -39,7 +38,7 @@ public class UpdateRequestBuilder extends InstanceShardOperationRequestBuilder implements WriteRequestBuilder { 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()); diff --git a/server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java b/server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java index 1dca992c24d15..b65ffbe070684 100644 --- a/server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java +++ b/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() {