From 8e1f6ba37614a228e8d05a375ae692166d139983 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Thu, 23 Jan 2025 15:14:20 +0100 Subject: [PATCH 01/12] Adding query param include_source_on_error to optionally disable returning the source in case of parsing errors (JSON). --- .../XContentParserConfigurationImpl.java | 41 ++++++++-- .../provider/json/JsonXContentImpl.java | 15 +++- .../xcontent/XContentParserConfiguration.java | 7 ++ .../xcontent/XContentParserTests.java | 27 +++++++ .../action/document/RestBulkActionIT.java | 75 +++++++++++++++++++ .../action/document/RestIndexActionIT.java | 52 +++++++++++++ .../action/document/RestUpdateActionIT.java | 52 +++++++++++++ .../org/elasticsearch/TransportVersions.java | 1 + .../action/bulk/BulkRequest.java | 18 ++++- .../action/bulk/BulkRequestParser.java | 19 ++++- .../action/bulk/TransportShardBulkAction.java | 1 + .../bulk/TransportSimulateBulkAction.java | 1 + .../action/index/IndexRequest.java | 18 +++++ .../index/mapper/DocumentParser.java | 6 +- .../index/mapper/SourceToParse.java | 13 +++- .../org/elasticsearch/rest/RestRequest.java | 12 ++- .../rest/action/document/RestBulkAction.java | 30 ++++---- .../rest/action/document/RestIndexAction.java | 1 + .../index/mapper/DynamicTemplatesTests.java | 1 + .../index/mapper/MapperServiceTestCase.java | 1 + .../ml/process/IndexingStateProcessor.java | 2 +- 21 files changed, 357 insertions(+), 36 deletions(-) create mode 100644 server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestBulkActionIT.java create mode 100644 server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestIndexActionIT.java create mode 100644 server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestUpdateActionIT.java diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/XContentParserConfigurationImpl.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/XContentParserConfigurationImpl.java index 70adc59b9c6a9..e04c640ad7461 100644 --- a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/XContentParserConfigurationImpl.java +++ b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/XContentParserConfigurationImpl.java @@ -31,7 +31,8 @@ public class XContentParserConfigurationImpl implements XContentParserConfigurat RestApiVersion.current(), null, null, - false + false, + true ); final NamedXContentRegistry registry; @@ -40,6 +41,7 @@ public class XContentParserConfigurationImpl implements XContentParserConfigurat final FilterPath[] includes; final FilterPath[] excludes; final boolean filtersMatchFieldNamesWithDots; + final boolean includeSourceOnError; private XContentParserConfigurationImpl( NamedXContentRegistry registry, @@ -47,7 +49,8 @@ private XContentParserConfigurationImpl( RestApiVersion restApiVersion, FilterPath[] includes, FilterPath[] excludes, - boolean filtersMatchFieldNamesWithDots + boolean filtersMatchFieldNamesWithDots, + boolean includeSourceOnError ) { this.registry = registry; this.deprecationHandler = deprecationHandler; @@ -55,6 +58,28 @@ private XContentParserConfigurationImpl( this.includes = includes; this.excludes = excludes; this.filtersMatchFieldNamesWithDots = filtersMatchFieldNamesWithDots; + this.includeSourceOnError = includeSourceOnError; + } + + @Override + public boolean includeSourceOnError() { + return includeSourceOnError; + } + + @Override + public XContentParserConfiguration withIncludeSourceOnError(boolean includeSourceOnError) { + if (includeSourceOnError == this.includeSourceOnError) { + return this; + } + return new XContentParserConfigurationImpl( + registry, + deprecationHandler, + restApiVersion, + includes, + excludes, + filtersMatchFieldNamesWithDots, + includeSourceOnError + ); } @Override @@ -65,7 +90,8 @@ public XContentParserConfigurationImpl withRegistry(NamedXContentRegistry regist restApiVersion, includes, excludes, - filtersMatchFieldNamesWithDots + filtersMatchFieldNamesWithDots, + includeSourceOnError ); } @@ -80,7 +106,8 @@ public XContentParserConfiguration withDeprecationHandler(DeprecationHandler dep restApiVersion, includes, excludes, - filtersMatchFieldNamesWithDots + filtersMatchFieldNamesWithDots, + includeSourceOnError ); } @@ -95,7 +122,8 @@ public XContentParserConfiguration withRestApiVersion(RestApiVersion restApiVers restApiVersion, includes, excludes, - filtersMatchFieldNamesWithDots + filtersMatchFieldNamesWithDots, + includeSourceOnError ); } @@ -143,7 +171,8 @@ public XContentParserConfiguration withFiltering( restApiVersion, includePaths, excludePaths, - filtersMatchFieldNamesWithDots + filtersMatchFieldNamesWithDots, + includeSourceOnError ); } diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java index c842e3bbc50f4..7f52467caf49b 100644 --- a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java +++ b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java @@ -87,23 +87,30 @@ public XContentGenerator createGenerator(OutputStream os, Set includes, return new JsonXContentGenerator(jsonFactory.createGenerator(os, JsonEncoding.UTF8), os, includes, excludes); } + private XContentParser createParser(XContentParserConfiguration config, JsonParser parser) { + if (config.includeSourceOnError() == false) { + parser.disable(JsonParser.Feature.INCLUDE_SOURCE_IN_LOCATION); // enabled by default, disable if requested + } + return new JsonXContentParser(config, parser); + } + @Override public XContentParser createParser(XContentParserConfiguration config, String content) throws IOException { - return new JsonXContentParser(config, jsonFactory.createParser(content)); + return createParser(config, jsonFactory.createParser(content)); } @Override public XContentParser createParser(XContentParserConfiguration config, InputStream is) throws IOException { - return new JsonXContentParser(config, jsonFactory.createParser(is)); + return createParser(config, jsonFactory.createParser(is)); } @Override public XContentParser createParser(XContentParserConfiguration config, byte[] data, int offset, int length) throws IOException { - return new JsonXContentParser(config, jsonFactory.createParser(data, offset, length)); + return createParser(config, jsonFactory.createParser(data, offset, length)); } @Override public XContentParser createParser(XContentParserConfiguration config, Reader reader) throws IOException { - return new JsonXContentParser(config, jsonFactory.createParser(reader)); + return createParser(config, jsonFactory.createParser(reader)); } } diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java index 59e5cd5d6485c..73ebdfce222ad 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java +++ b/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java @@ -26,6 +26,13 @@ public interface XContentParserConfiguration { */ XContentParserConfiguration EMPTY = XContentProvider.provider().empty(); + /** + * Disable to not include the source in case of parsing errors (defaults to true). + */ + XContentParserConfiguration withIncludeSourceOnError(boolean includeSourceOnError); + + boolean includeSourceOnError(); + /** * Replace the registry backing {@link XContentParser#namedObject}. */ diff --git a/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java index 5aff60b1a4c75..d978944a90aaa 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java @@ -15,7 +15,10 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.json.JsonXContent; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.StringReader; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -33,6 +36,7 @@ import static org.hamcrest.Matchers.in; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; import static org.junit.internal.matchers.ThrowableMessageMatcher.hasMessage; @@ -655,6 +659,29 @@ public void testCreateRootSubParser() throws IOException { } + public void testJsonIncludeSourceOnParserError() throws IOException { + var xContent = XContentFactory.xContent(XContentType.JSON); + var source = "{\"field\": invalid}"; // causes parse exception + var sourceEnabled = XContentParserConfiguration.EMPTY; + var sourceDisabled = XContentParserConfiguration.EMPTY.withIncludeSourceOnError(false); + + var parseException = assertThrows(XContentParseException.class, () -> createParser(xContent, sourceEnabled, source).map()); + assertThat(parseException.getMessage(), containsString(source)); + + parseException = assertThrows(XContentParseException.class, () -> createParser(xContent, sourceDisabled, source).map()); + assertThat(parseException.getMessage(), not(containsString(source))); + } + + private XContentParser createParser(XContent xContent, XContentParserConfiguration config, String content) throws IOException { + return switch (randomInt(3)) { + case 0 -> xContent.createParser(config, content); + case 1 -> xContent.createParser(config, content.getBytes(StandardCharsets.UTF_8)); + case 2 -> xContent.createParser(config, new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8))); + case 3 -> xContent.createParser(config, new StringReader(content)); + default -> throw new IllegalArgumentException(); + }; + } + /** * Generates a random object {"first_field": "foo", "marked_field": {...random...}, "last_field": "bar} * diff --git a/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestBulkActionIT.java b/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestBulkActionIT.java new file mode 100644 index 0000000000000..f73870e807182 --- /dev/null +++ b/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestBulkActionIT.java @@ -0,0 +1,75 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.rest.action.document; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.io.Streams; +import org.elasticsearch.test.ESIntegTestCase; + +import java.io.InputStreamReader; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.hamcrest.Matchers.both; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; + +public class RestBulkActionIT extends ESIntegTestCase { + @Override + protected boolean addMockHttpTransport() { + return false; + } + + public void testBulkIndexWithSourceOnErrorDisabled() throws Exception { + var source = "{\"field\": \"index\",}"; + var sourceEscaped = "{\\\"field\\\": \\\"index\\\",}"; + + var request = new Request("PUT", "/test_index/_bulk"); + request.setJsonEntity(Strings.format("{\"index\":{\"_id\":\"1\"}}\n%s\n", source)); + + Response response = getRestClient().performRequest(request); + String responseContent = Streams.copyToString(new InputStreamReader(response.getEntity().getContent(), UTF_8)); + assertThat(responseContent, containsString(sourceEscaped)); + + request.addParameter("include_source_on_error", "false"); + + response = getRestClient().performRequest(request); + responseContent = Streams.copyToString(new InputStreamReader(response.getEntity().getContent(), UTF_8)); + assertThat( + responseContent, + both(not(containsString(sourceEscaped))).and( + containsString("REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled)") + ) + ); + } + + public void testBulkUpdateWithSourceOnErrorDisabled() throws Exception { + var source = "{\"field\": \"index\",}"; + var sourceEscaped = "{\\\"field\\\": \\\"index\\\",}"; + + var request = new Request("PUT", "/test_index/_bulk"); + request.addParameter("include_source_on_error", "false"); + request.setJsonEntity(Strings.format("{\"update\":{\"_id\":\"1\"}}\n{\"doc\":%s}}\n", source)); + + // note: this behavior is not consistent with bulk index actions + // In case of updates by doc, the source is eagerly parsed and will fail the entire request if it cannot be parsed + var exception = assertThrows(ResponseException.class, () -> getRestClient().performRequest(request)); + String response = Streams.copyToString(new InputStreamReader(exception.getResponse().getEntity().getContent(), UTF_8)); + + assertThat( + response, + both(not(containsString(sourceEscaped))).and( + containsString("REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled)") + ) + ); + } +} diff --git a/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestIndexActionIT.java b/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestIndexActionIT.java new file mode 100644 index 0000000000000..5a98ba7dd41bc --- /dev/null +++ b/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestIndexActionIT.java @@ -0,0 +1,52 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.rest.action.document; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.common.io.Streams; +import org.elasticsearch.test.ESIntegTestCase; + +import java.io.InputStreamReader; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.hamcrest.Matchers.both; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; + +public class RestIndexActionIT extends ESIntegTestCase { + @Override + protected boolean addMockHttpTransport() { + return false; + } + + public void testIndexWithSourceOnErrorDisabled() throws Exception { + var source = "{\"field\": \"value}"; + var sourceEscaped = "{\\\"field\\\": \\\"value}"; + + var request = new Request("POST", "/test_index/_doc/1"); + request.setJsonEntity(source); + + var exception = assertThrows(ResponseException.class, () -> getRestClient().performRequest(request)); + String response = Streams.copyToString(new InputStreamReader(exception.getResponse().getEntity().getContent(), UTF_8)); + assertThat(response, containsString(sourceEscaped)); + + // disable source on error + request.addParameter("include_source_on_error", "false"); + exception = assertThrows(ResponseException.class, () -> getRestClient().performRequest(request)); + response = Streams.copyToString(new InputStreamReader(exception.getResponse().getEntity().getContent(), UTF_8)); + assertThat( + response, + both(not(containsString(sourceEscaped))).and( + containsString("REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled)") + ) + ); + } +} diff --git a/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestUpdateActionIT.java b/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestUpdateActionIT.java new file mode 100644 index 0000000000000..4fa3958f49732 --- /dev/null +++ b/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestUpdateActionIT.java @@ -0,0 +1,52 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.rest.action.document; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.common.io.Streams; +import org.elasticsearch.test.ESIntegTestCase; + +import java.io.InputStreamReader; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.hamcrest.Matchers.both; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; + +public class RestUpdateActionIT extends ESIntegTestCase { + @Override + protected boolean addMockHttpTransport() { + return false; + } + + public void testUpdateByDocWithSourceOnErrorDisabled() throws Exception { + var updateRequest = "{\"doc\":{\"field\": \"value}}"; + var sourceEscaped = "{\\\"field\\\": \\\"value}"; + + var request = new Request("POST", "/test_index/_update/1"); + request.setJsonEntity(updateRequest); + + var exception = assertThrows(ResponseException.class, () -> getRestClient().performRequest(request)); + String response = Streams.copyToString(new InputStreamReader(exception.getResponse().getEntity().getContent(), UTF_8)); + assertThat(response, containsString(sourceEscaped)); + + // disable source on error + request.addParameter("include_source_on_error", "false"); + exception = assertThrows(ResponseException.class, () -> getRestClient().performRequest(request)); + response = Streams.copyToString(new InputStreamReader(exception.getResponse().getEntity().getContent(), UTF_8)); + assertThat( + response, + both(not(containsString(sourceEscaped))).and( + containsString("REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled)") + ) + ); + } +} diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 750b23caf2151..b2bf87e7cc63d 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -161,6 +161,7 @@ static TransportVersion def(int id) { public static final TransportVersion ESQL_SKIP_ES_INDEX_SERIALIZATION = def(8_827_00_0); public static final TransportVersion ADD_INDEX_BLOCK_TWO_PHASE = def(8_828_00_0); public static final TransportVersion RESOLVE_CLUSTER_NO_INDEX_EXPRESSION = def(8_829_00_0); + public static final TransportVersion INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR = def(8_830_00_0); /* * STOP! READ THIS FIRST! No, really, diff --git a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java index 91caebc420ffb..c491caaf66f83 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java @@ -84,6 +84,7 @@ public class BulkRequest extends ActionRequest private String globalIndex; private Boolean globalRequireAlias; private Boolean globalRequireDatsStream; + private boolean includeSourceOnError = true; private long sizeInBytes = 0; @@ -103,6 +104,9 @@ public BulkRequest(StreamInput in) throws IOException { } else { incrementalState = BulkRequest.IncrementalState.EMPTY; } + if (in.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { + includeSourceOnError = in.readBoolean(); + } } public BulkRequest(@Nullable String globalIndex) { @@ -278,7 +282,7 @@ public BulkRequest add( String pipeline = valueOrDefault(defaultPipeline, globalPipeline); Boolean requireAlias = valueOrDefault(defaultRequireAlias, globalRequireAlias); Boolean requireDataStream = valueOrDefault(defaultRequireDataStream, globalRequireDatsStream); - new BulkRequestParser(true, restApiVersion).parse( + new BulkRequestParser(true, includeSourceOnError, restApiVersion).parse( data, defaultIndex, routing, @@ -341,6 +345,11 @@ public void incrementalState(IncrementalState incrementalState) { this.incrementalState = incrementalState; } + public final BulkRequest includeSourceOnError(boolean includeSourceOnError) { + this.includeSourceOnError = includeSourceOnError; + return this; + } + /** * Note for internal callers (NOT high level rest client), * the global parameter setting is ignored when used with: @@ -399,6 +408,10 @@ public Boolean requireDataStream() { return globalRequireDatsStream; } + public boolean includeSourceOnError() { + return includeSourceOnError; + } + /** * Note for internal callers (NOT high level rest client), * the global parameter setting is ignored when used with: @@ -457,6 +470,9 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_16_0)) { incrementalState.writeTo(out); } + if (out.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { + out.writeBoolean(includeSourceOnError); + } } @Override diff --git a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java index 9be1feae5ccfe..709a29fe66142 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java @@ -81,12 +81,24 @@ public final class BulkRequestParser { * Create a new parser. * * @param deprecateOrErrorOnType whether to allow _type information in the index line; used by BulkMonitoring + * @param includeSourceOnError if to include the source in parser error messages * @param restApiVersion */ - public BulkRequestParser(boolean deprecateOrErrorOnType, RestApiVersion restApiVersion) { + public BulkRequestParser(boolean deprecateOrErrorOnType, boolean includeSourceOnError, RestApiVersion restApiVersion) { this.deprecateOrErrorOnType = deprecateOrErrorOnType; this.config = XContentParserConfiguration.EMPTY.withDeprecationHandler(LoggingDeprecationHandler.INSTANCE) - .withRestApiVersion(restApiVersion); + .withRestApiVersion(restApiVersion) + .withIncludeSourceOnError(includeSourceOnError); + } + + /** + * Create a new parser. + * + * @param deprecateOrErrorOnType whether to allow _type information in the index line; used by BulkMonitoring + * @param restApiVersion + */ + public BulkRequestParser(boolean deprecateOrErrorOnType, RestApiVersion restApiVersion) { + this(deprecateOrErrorOnType, true, restApiVersion); } private static int findNextMarker(byte marker, int from, BytesReference data, boolean lastData) { @@ -480,7 +492,8 @@ private boolean parseActionLine(BytesReference data, int from, int to) throws IO .setDynamicTemplates(dynamicTemplates) .setRequireAlias(requireAlias) .setRequireDataStream(requireDataStream) - .setListExecutedPipelines(currentListExecutedPipelines); + .setListExecutedPipelines(currentListExecutedPipelines) + .setIncludeSourceOnError(config.includeSourceOnError()); if ("create".equals(action)) { indexRequest = indexRequest.create(true); } else if (opType != null) { diff --git a/server/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java b/server/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java index 89cee714a9ff2..33c73898c0394 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java @@ -376,6 +376,7 @@ static boolean executeBulkItemRequest( request.getContentType(), request.routing(), request.getDynamicTemplates(), + request.getIncludeSourceOnError(), meteringParserDecorator ); result = primary.applyIndexOperationOnPrimary( diff --git a/server/src/main/java/org/elasticsearch/action/bulk/TransportSimulateBulkAction.java b/server/src/main/java/org/elasticsearch/action/bulk/TransportSimulateBulkAction.java index 18c420d99f525..106e40771ced5 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportSimulateBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportSimulateBulkAction.java @@ -187,6 +187,7 @@ private Tuple, Exception> validateMappings( request.getContentType(), request.routing(), request.getDynamicTemplates(), + request.getIncludeSourceOnError(), XContentMeteringParserDecorator.NOOP ); diff --git a/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java b/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java index bcb8a7fb78bf3..2bfc229464b58 100644 --- a/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java +++ b/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java @@ -116,6 +116,8 @@ public class IndexRequest extends ReplicatedWriteRequest implement private boolean requireDataStream; + private boolean includeSourceOnError = true; + /** * Transient flag denoting that the local request should be routed to a failure store. Not persisted across the wire. */ @@ -210,6 +212,10 @@ public IndexRequest(@Nullable ShardId shardId, StreamInput in) throws IOExceptio in.readBoolean(); // obsolete originatesFromUpdateByDoc } } + + if (in.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { + includeSourceOnError = in.readBoolean(); + } } public IndexRequest() { @@ -806,6 +812,9 @@ private void writeBody(StreamOutput out) throws IOException { out.writeBoolean(false); // obsolete originatesFromUpdateByDoc } } + if (out.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { + out.writeBoolean(includeSourceOnError); + } } @Override @@ -874,6 +883,15 @@ public IndexRequest setRequireDataStream(boolean requireDataStream) { return this; } + public boolean getIncludeSourceOnError() { + return includeSourceOnError; + } + + public IndexRequest setIncludeSourceOnError(boolean includeSourceOnError) { + this.includeSourceOnError = includeSourceOnError; + return this; + } + @Override public Index getConcreteWriteIndex(IndexAbstraction ia, Metadata metadata) { if (DataStream.isFailureStoreFeatureFlagEnabled() && writeToFailureStore) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index 9ddb6f0d496a0..fd7d1a60f45ff 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -84,7 +84,11 @@ public ParsedDocument parseDocument(SourceToParse source, MappingLookup mappingL XContentMeteringParserDecorator meteringParserDecorator = source.getMeteringParserDecorator(); try ( XContentParser parser = meteringParserDecorator.decorate( - XContentHelper.createParser(parserConfiguration, source.source(), xContentType) + XContentHelper.createParser( + parserConfiguration.withIncludeSourceOnError(source.getIncludeSourceOnError()), + source.source(), + xContentType + ) ) ) { context = new RootDocumentParserContext(mappingLookup, mappingParserContext, source, parser); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java index 879e0fe785df2..41464babebf69 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java @@ -29,6 +29,9 @@ public class SourceToParse { private final XContentType xContentType; private final Map dynamicTemplates; + + private final boolean includeSourceOnError; + private final XContentMeteringParserDecorator meteringParserDecorator; public SourceToParse( @@ -37,6 +40,7 @@ public SourceToParse( XContentType xContentType, @Nullable String routing, Map dynamicTemplates, + boolean includeSourceOnError, XContentMeteringParserDecorator meteringParserDecorator ) { this.id = id; @@ -46,15 +50,16 @@ public SourceToParse( this.xContentType = Objects.requireNonNull(xContentType); this.routing = routing; this.dynamicTemplates = Objects.requireNonNull(dynamicTemplates); + this.includeSourceOnError = includeSourceOnError; this.meteringParserDecorator = meteringParserDecorator; } public SourceToParse(String id, BytesReference source, XContentType xContentType) { - this(id, source, xContentType, null, Map.of(), XContentMeteringParserDecorator.NOOP); + this(id, source, xContentType, null, Map.of(), true, XContentMeteringParserDecorator.NOOP); } public SourceToParse(String id, BytesReference source, XContentType xContentType, String routing) { - this(id, source, xContentType, routing, Map.of(), XContentMeteringParserDecorator.NOOP); + this(id, source, xContentType, routing, Map.of(), true, XContentMeteringParserDecorator.NOOP); } public BytesReference source() { @@ -94,4 +99,8 @@ public XContentType getXContentType() { public XContentMeteringParserDecorator getMeteringParserDecorator() { return meteringParserDecorator; } + + boolean getIncludeSourceOnError() { + return includeSourceOnError; + } } diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index a04bdcb32f2b4..eb675f0230bd0 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -537,6 +537,11 @@ public XContentParserConfiguration contentParserConfig() { return parserConfig; } + private XContentParserConfiguration internalParserConfig() { + // consume query param lazily so we can report if consumed or not + return parserConfig.withIncludeSourceOnError(paramAsBoolean("include_source_on_error", true)); + } + /** * A parser for the contents of this request if there is a body, otherwise throws an {@link ElasticsearchParseException}. Use * {@link #applyContentParser(CheckedConsumer)} if you want to gracefully handle when the request doesn't have any contents. Use @@ -544,8 +549,7 @@ public XContentParserConfiguration contentParserConfig() { */ public final XContentParser contentParser() throws IOException { BytesReference content = requiredContent(); // will throw exception if body or content type missing - return XContentHelper.createParserNotCompressed(parserConfig, content, xContentType.get()); - + return XContentHelper.createParserNotCompressed(internalParserConfig(), content, xContentType.get()); } /** @@ -574,7 +578,7 @@ public final boolean hasContentOrSourceParam() { */ public final XContentParser contentOrSourceParamParser() throws IOException { Tuple tuple = contentOrSourceParam(); - return XContentHelper.createParserNotCompressed(parserConfig, tuple.v2(), tuple.v1().xContent().type()); + return XContentHelper.createParserNotCompressed(internalParserConfig(), tuple.v2(), tuple.v1().xContent().type()); } /** @@ -585,7 +589,7 @@ public final XContentParser contentOrSourceParamParser() throws IOException { public final void withContentOrSourceParamParserOrNull(CheckedConsumer withParser) throws IOException { if (hasContentOrSourceParam()) { Tuple tuple = contentOrSourceParam(); - try (XContentParser parser = XContentHelper.createParserNotCompressed(parserConfig, tuple.v2(), tuple.v1())) { + try (XContentParser parser = XContentHelper.createParserNotCompressed(internalParserConfig(), tuple.v2(), tuple.v1())) { withParser.accept(parser); } } else { diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java index dea7b7138d0d0..8c9961ac8aea8 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java @@ -103,6 +103,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC boolean defaultRequireDataStream = request.paramAsBoolean(DocWriteRequest.REQUIRE_DATA_STREAM, false); bulkRequest.timeout(request.paramAsTime("timeout", BulkShardRequest.DEFAULT_TIMEOUT)); bulkRequest.setRefreshPolicy(request.param("refresh")); + bulkRequest.includeSourceOnError(request.paramAsBoolean("include_source_on_error", true)); ReleasableBytesReference content = request.requiredContent(); try { @@ -160,20 +161,21 @@ static class ChunkHandler implements BaseRestHandler.RequestBodyChunkConsumer { ChunkHandler(boolean allowExplicitIndex, RestRequest request, Supplier handlerSupplier) { this.request = request; this.handlerSupplier = handlerSupplier; - this.parser = new BulkRequestParser(true, request.getRestApiVersion()).incrementalParser( - request.param("index"), - request.param("routing"), - FetchSourceContext.parseFromRestRequest(request), - request.param("pipeline"), - request.paramAsBoolean(DocWriteRequest.REQUIRE_ALIAS, false), - request.paramAsBoolean(DocWriteRequest.REQUIRE_DATA_STREAM, false), - request.paramAsBoolean("list_executed_pipelines", false), - allowExplicitIndex, - request.getXContentType(), - (indexRequest, type) -> items.add(indexRequest), - items::add, - items::add - ); + this.parser = new BulkRequestParser(true, request.paramAsBoolean("include_source_on_error", true), request.getRestApiVersion()) + .incrementalParser( + request.param("index"), + request.param("routing"), + FetchSourceContext.parseFromRestRequest(request), + request.param("pipeline"), + request.paramAsBoolean(DocWriteRequest.REQUIRE_ALIAS, false), + request.paramAsBoolean(DocWriteRequest.REQUIRE_DATA_STREAM, false), + request.paramAsBoolean("list_executed_pipelines", false), + allowExplicitIndex, + request.getXContentType(), + (indexRequest, type) -> items.add(indexRequest), + items::add, + items::add + ); } @Override diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java index d40c6225cc7b4..f0b2db1dcd8fe 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java @@ -120,6 +120,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC indexRequest.setIfPrimaryTerm(request.paramAsLong("if_primary_term", indexRequest.ifPrimaryTerm())); indexRequest.setRequireAlias(request.paramAsBoolean(DocWriteRequest.REQUIRE_ALIAS, indexRequest.isRequireAlias())); indexRequest.setRequireDataStream(request.paramAsBoolean(DocWriteRequest.REQUIRE_DATA_STREAM, indexRequest.isRequireDataStream())); + indexRequest.setIncludeSourceOnError(request.paramAsBoolean("include_source_on_error", true)); String sOpType = request.param("op_type"); String waitForActiveShards = request.param("wait_for_active_shards"); if (waitForActiveShards != null) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java index 78643a2d581cc..25c1062a375b8 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java @@ -675,6 +675,7 @@ public void testTemplateWithoutMatchPredicates() throws Exception { XContentType.JSON, null, Map.of("foo", "geo_point"), + true, XContentMeteringParserDecorator.NOOP ) ); diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java index 8c44b49f36357..e8187bbf964da 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java @@ -395,6 +395,7 @@ protected static SourceToParse source( XContentType.JSON, routing, dynamicTemplates, + true, XContentMeteringParserDecorator.NOOP ); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/process/IndexingStateProcessor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/process/IndexingStateProcessor.java index 56b0483e07c78..85e05460a9107 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/process/IndexingStateProcessor.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/process/IndexingStateProcessor.java @@ -142,7 +142,7 @@ void findAppropriateIndexOrAliasAndPersist(BytesReference bytes) throws IOExcept void persist(String indexOrAlias, BytesReference bytes) throws IOException { BulkRequest bulkRequest = new BulkRequest().setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) .requireAlias(AnomalyDetectorsIndex.jobStateIndexWriteAlias().equals(indexOrAlias)); - bulkRequest.add(bytes, indexOrAlias, XContentType.JSON); + bulkRequest.add(bytes, indexOrAlias, false, XContentType.JSON); if (bulkRequest.numberOfActions() > 0) { LOGGER.trace("[{}] Persisting job state document: index [{}], length [{}]", jobId, indexOrAlias, bytes.length()); try { From b347b12a0b2a351be0d9533ee992bf49d2550d62 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Thu, 23 Jan 2025 15:41:38 +0100 Subject: [PATCH 02/12] cleanup --- .../rest/action/document/RestBulkActionIT.java | 5 +++-- .../rest/action/document/RestIndexActionIT.java | 3 ++- .../action/document/RestUpdateActionIT.java | 3 ++- .../index/mapper/SourceToParse.java | 10 ++++++++++ .../org/elasticsearch/rest/RestRequest.java | 2 +- .../java/org/elasticsearch/rest/RestUtils.java | 17 +++++++++++++++++ .../rest/action/document/RestBulkAction.java | 5 +++-- .../rest/action/document/RestIndexAction.java | 3 ++- .../index/mapper/DynamicTemplatesTests.java | 13 +------------ .../index/mapper/MapperServiceTestCase.java | 11 +---------- .../ml/process/IndexingStateProcessor.java | 2 +- 11 files changed, 43 insertions(+), 31 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestBulkActionIT.java b/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestBulkActionIT.java index f73870e807182..d0b5ec4562903 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestBulkActionIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestBulkActionIT.java @@ -14,6 +14,7 @@ import org.elasticsearch.client.ResponseException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.Streams; +import org.elasticsearch.rest.RestUtils; import org.elasticsearch.test.ESIntegTestCase; import java.io.InputStreamReader; @@ -40,7 +41,7 @@ public void testBulkIndexWithSourceOnErrorDisabled() throws Exception { String responseContent = Streams.copyToString(new InputStreamReader(response.getEntity().getContent(), UTF_8)); assertThat(responseContent, containsString(sourceEscaped)); - request.addParameter("include_source_on_error", "false"); + request.addParameter(RestUtils.INCLUDE_SOURCE_ON_ERROR_PARAMETER, "false"); response = getRestClient().performRequest(request); responseContent = Streams.copyToString(new InputStreamReader(response.getEntity().getContent(), UTF_8)); @@ -57,7 +58,7 @@ public void testBulkUpdateWithSourceOnErrorDisabled() throws Exception { var sourceEscaped = "{\\\"field\\\": \\\"index\\\",}"; var request = new Request("PUT", "/test_index/_bulk"); - request.addParameter("include_source_on_error", "false"); + request.addParameter(RestUtils.INCLUDE_SOURCE_ON_ERROR_PARAMETER, "false"); request.setJsonEntity(Strings.format("{\"update\":{\"_id\":\"1\"}}\n{\"doc\":%s}}\n", source)); // note: this behavior is not consistent with bulk index actions diff --git a/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestIndexActionIT.java b/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestIndexActionIT.java index 5a98ba7dd41bc..1a27e704ad497 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestIndexActionIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestIndexActionIT.java @@ -12,6 +12,7 @@ import org.elasticsearch.client.Request; import org.elasticsearch.client.ResponseException; import org.elasticsearch.common.io.Streams; +import org.elasticsearch.rest.RestUtils; import org.elasticsearch.test.ESIntegTestCase; import java.io.InputStreamReader; @@ -39,7 +40,7 @@ public void testIndexWithSourceOnErrorDisabled() throws Exception { assertThat(response, containsString(sourceEscaped)); // disable source on error - request.addParameter("include_source_on_error", "false"); + request.addParameter(RestUtils.INCLUDE_SOURCE_ON_ERROR_PARAMETER, "false"); exception = assertThrows(ResponseException.class, () -> getRestClient().performRequest(request)); response = Streams.copyToString(new InputStreamReader(exception.getResponse().getEntity().getContent(), UTF_8)); assertThat( diff --git a/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestUpdateActionIT.java b/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestUpdateActionIT.java index 4fa3958f49732..f25a2b8855c06 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestUpdateActionIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/rest/action/document/RestUpdateActionIT.java @@ -12,6 +12,7 @@ import org.elasticsearch.client.Request; import org.elasticsearch.client.ResponseException; import org.elasticsearch.common.io.Streams; +import org.elasticsearch.rest.RestUtils; import org.elasticsearch.test.ESIntegTestCase; import java.io.InputStreamReader; @@ -39,7 +40,7 @@ public void testUpdateByDocWithSourceOnErrorDisabled() throws Exception { assertThat(response, containsString(sourceEscaped)); // disable source on error - request.addParameter("include_source_on_error", "false"); + request.addParameter(RestUtils.INCLUDE_SOURCE_ON_ERROR_PARAMETER, "false"); exception = assertThrows(ResponseException.class, () -> getRestClient().performRequest(request)); response = Streams.copyToString(new InputStreamReader(exception.getResponse().getEntity().getContent(), UTF_8)); assertThat( diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java index 41464babebf69..474defe77c257 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java @@ -62,6 +62,16 @@ public SourceToParse(String id, BytesReference source, XContentType xContentType this(id, source, xContentType, routing, Map.of(), true, XContentMeteringParserDecorator.NOOP); } + public SourceToParse( + String id, + BytesReference source, + XContentType xContentType, + String routing, + Map dynamicTemplates + ) { + this(id, source, xContentType, routing, dynamicTemplates, true, XContentMeteringParserDecorator.NOOP); + } + public BytesReference source() { return this.source; } diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index eb675f0230bd0..eff73f33cf546 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -539,7 +539,7 @@ public XContentParserConfiguration contentParserConfig() { private XContentParserConfiguration internalParserConfig() { // consume query param lazily so we can report if consumed or not - return parserConfig.withIncludeSourceOnError(paramAsBoolean("include_source_on_error", true)); + return parserConfig.withIncludeSourceOnError(RestUtils.getIncludeSourceOnError(this)); } /** diff --git a/server/src/main/java/org/elasticsearch/rest/RestUtils.java b/server/src/main/java/org/elasticsearch/rest/RestUtils.java index 10e72035cf1f5..68ec7a758e73c 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestUtils.java +++ b/server/src/main/java/org/elasticsearch/rest/RestUtils.java @@ -291,6 +291,12 @@ public static Optional extractTraceId(String traceparent) { */ public static final String REST_TIMEOUT_PARAM = "timeout"; + /** + * The name of the common {@code ?include_source_on_error} query parameter. + * By default, the document source is included in the error response in case of parsing errors. This parameter allows to disable this. + */ + public static final String INCLUDE_SOURCE_ON_ERROR_PARAMETER = "include_source_on_error"; + /** * Extract the {@code ?master_timeout} parameter from the request, imposing the common default of {@code 30s} in case the parameter is * missing. @@ -329,6 +335,17 @@ public static TimeValue getTimeout(RestRequest restRequest) { return restRequest.paramAsTime(REST_TIMEOUT_PARAM, null); } + /** + * Extract the {@code ?include_source_on_error} parameter from the request, returning {@code true} in case the parameter is missing. + * + * @param restRequest The request from which to extract the {@code ?include_source_on_error} parameter + * @return the value of the {@code ?include_source_on_error} parameter from the request, with a default of {@code true} if the request + */ + public static boolean getIncludeSourceOnError(RestRequest restRequest) { + assert restRequest != null; + return restRequest.paramAsBoolean(INCLUDE_SOURCE_ON_ERROR_PARAMETER, true); + } + // Remove the BWC support for the deprecated ?local parameter. // NOTE: ensure each usage of this method has been deprecated for long enough to remove it. @UpdateForV10(owner = UpdateForV10.Owner.DISTRIBUTED_COORDINATION) diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java index 8c9961ac8aea8..944edc2e5e1f8 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/document/RestBulkAction.java @@ -29,6 +29,7 @@ import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.RestUtils; import org.elasticsearch.rest.Scope; import org.elasticsearch.rest.ServerlessScope; import org.elasticsearch.rest.action.RestRefCountedChunkedToXContentListener; @@ -103,7 +104,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC boolean defaultRequireDataStream = request.paramAsBoolean(DocWriteRequest.REQUIRE_DATA_STREAM, false); bulkRequest.timeout(request.paramAsTime("timeout", BulkShardRequest.DEFAULT_TIMEOUT)); bulkRequest.setRefreshPolicy(request.param("refresh")); - bulkRequest.includeSourceOnError(request.paramAsBoolean("include_source_on_error", true)); + bulkRequest.includeSourceOnError(RestUtils.getIncludeSourceOnError(request)); ReleasableBytesReference content = request.requiredContent(); try { @@ -161,7 +162,7 @@ static class ChunkHandler implements BaseRestHandler.RequestBodyChunkConsumer { ChunkHandler(boolean allowExplicitIndex, RestRequest request, Supplier handlerSupplier) { this.request = request; this.handlerSupplier = handlerSupplier; - this.parser = new BulkRequestParser(true, request.paramAsBoolean("include_source_on_error", true), request.getRestApiVersion()) + this.parser = new BulkRequestParser(true, RestUtils.getIncludeSourceOnError(request), request.getRestApiVersion()) .incrementalParser( request.param("index"), request.param("routing"), diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java index f0b2db1dcd8fe..14c2428d29081 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java @@ -20,6 +20,7 @@ import org.elasticsearch.index.VersionType; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.RestUtils; import org.elasticsearch.rest.Scope; import org.elasticsearch.rest.ServerlessScope; import org.elasticsearch.rest.action.RestActions; @@ -120,7 +121,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC indexRequest.setIfPrimaryTerm(request.paramAsLong("if_primary_term", indexRequest.ifPrimaryTerm())); indexRequest.setRequireAlias(request.paramAsBoolean(DocWriteRequest.REQUIRE_ALIAS, indexRequest.isRequireAlias())); indexRequest.setRequireDataStream(request.paramAsBoolean(DocWriteRequest.REQUIRE_DATA_STREAM, indexRequest.isRequireDataStream())); - indexRequest.setIncludeSourceOnError(request.paramAsBoolean("include_source_on_error", true)); + indexRequest.setIncludeSourceOnError(RestUtils.getIncludeSourceOnError(request)); String sOpType = request.param("op_type"); String waitForActiveShards = request.param("wait_for_active_shards"); if (waitForActiveShards != null) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java index 25c1062a375b8..0c68ba5ca0837 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java @@ -22,7 +22,6 @@ import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.IndexVersions; -import org.elasticsearch.plugins.internal.XContentMeteringParserDecorator; import org.elasticsearch.test.XContentTestUtils; import org.elasticsearch.test.index.IndexVersionUtils; import org.elasticsearch.xcontent.XContentBuilder; @@ -668,17 +667,7 @@ public void testTemplateWithoutMatchPredicates() throws Exception { {"foo": "41.12,-71.34", "bar": "41.12,-71.34"} """; ParsedDocument doc = mapperService.documentMapper() - .parse( - new SourceToParse( - "1", - new BytesArray(json), - XContentType.JSON, - null, - Map.of("foo", "geo_point"), - true, - XContentMeteringParserDecorator.NOOP - ) - ); + .parse(new SourceToParse("1", new BytesArray(json), XContentType.JSON, null, Map.of("foo", "geo_point"))); assertThat(doc.rootDoc().getFields("foo"), hasSize(1)); assertThat(doc.rootDoc().getFields("bar"), hasSize(1)); } diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java index e8187bbf964da..459480d1d7316 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java @@ -61,7 +61,6 @@ import org.elasticsearch.plugins.MapperPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.TelemetryPlugin; -import org.elasticsearch.plugins.internal.XContentMeteringParserDecorator; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptCompiler; import org.elasticsearch.script.ScriptContext; @@ -389,15 +388,7 @@ protected static SourceToParse source( XContentBuilder builder = JsonXContent.contentBuilder().startObject(); build.accept(builder); builder.endObject(); - return new SourceToParse( - id, - BytesReference.bytes(builder), - XContentType.JSON, - routing, - dynamicTemplates, - true, - XContentMeteringParserDecorator.NOOP - ); + return new SourceToParse(id, BytesReference.bytes(builder), XContentType.JSON, routing, dynamicTemplates); } /** diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/process/IndexingStateProcessor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/process/IndexingStateProcessor.java index 85e05460a9107..56b0483e07c78 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/process/IndexingStateProcessor.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/process/IndexingStateProcessor.java @@ -142,7 +142,7 @@ void findAppropriateIndexOrAliasAndPersist(BytesReference bytes) throws IOExcept void persist(String indexOrAlias, BytesReference bytes) throws IOException { BulkRequest bulkRequest = new BulkRequest().setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) .requireAlias(AnomalyDetectorsIndex.jobStateIndexWriteAlias().equals(indexOrAlias)); - bulkRequest.add(bytes, indexOrAlias, false, XContentType.JSON); + bulkRequest.add(bytes, indexOrAlias, XContentType.JSON); if (bulkRequest.numberOfActions() > 0) { LOGGER.trace("[{}] Persisting job state document: index [{}], length [{}]", jobId, indexOrAlias, bytes.length()); try { From b05a7439183b46756b7cd7526fda523043660205 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Thu, 23 Jan 2025 18:41:34 +0100 Subject: [PATCH 03/12] fix --- .../elasticsearch/xcontent/XContentParserTests.java | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java index d978944a90aaa..84c9fcfeaf81e 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java @@ -15,9 +15,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.json.JsonXContent; -import java.io.ByteArrayInputStream; import java.io.IOException; -import java.io.StringReader; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; @@ -673,13 +671,9 @@ public void testJsonIncludeSourceOnParserError() throws IOException { } private XContentParser createParser(XContent xContent, XContentParserConfiguration config, String content) throws IOException { - return switch (randomInt(3)) { - case 0 -> xContent.createParser(config, content); - case 1 -> xContent.createParser(config, content.getBytes(StandardCharsets.UTF_8)); - case 2 -> xContent.createParser(config, new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8))); - case 3 -> xContent.createParser(config, new StringReader(content)); - default -> throw new IllegalArgumentException(); - }; + return randomBoolean() + ? xContent.createParser(config, content) + : xContent.createParser(config, content.getBytes(StandardCharsets.UTF_8)); } /** From 1bcbac20b884f857a148ceb1c050ca9af77fc044 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Thu, 23 Jan 2025 18:43:45 +0100 Subject: [PATCH 04/12] Update docs/changelog/120725.yaml --- docs/changelog/120725.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/120725.yaml diff --git a/docs/changelog/120725.yaml b/docs/changelog/120725.yaml new file mode 100644 index 0000000000000..1cdd511685809 --- /dev/null +++ b/docs/changelog/120725.yaml @@ -0,0 +1,5 @@ +pr: 120725 +summary: Add common query param `include_source_on_error` +area: Infra/REST API +type: enhancement +issues: [] From a01b45b4b877445a2d091828ed71357ff2aa688d Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Thu, 23 Jan 2025 17:50:48 +0000 Subject: [PATCH 05/12] [CI] Auto commit changes from spotless --- server/src/main/java/org/elasticsearch/TransportVersions.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 9d5bc2bcadf09..1ed9c607675f5 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -166,7 +166,6 @@ static TransportVersion def(int id) { public static final TransportVersion ESQL_RESPONSE_PARTIAL = def(8_832_00_0); public static final TransportVersion INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR = def(8_833_00_0); - /* * STOP! READ THIS FIRST! No, really, * ____ _____ ___ ____ _ ____ _____ _ ____ _____ _ _ ___ ____ _____ ___ ____ ____ _____ _ From 77f41388ffdb5870c211ec95a84870bab14dfa9e Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Mon, 27 Jan 2025 10:10:24 +0100 Subject: [PATCH 06/12] review --- .../xcontent/XContentParserTests.java | 4 +-- .../action/bulk/BulkRequestParser.java | 10 ------- .../index/mapper/SourceToParse.java | 2 +- .../action/bulk/BulkRequestParserTests.java | 28 +++++++++---------- .../action/MonitoringBulkRequest.java | 2 +- .../test/CoreTestTranslater.java | 2 +- 6 files changed, 19 insertions(+), 29 deletions(-) diff --git a/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java index 84c9fcfeaf81e..5aa3b1e140074 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java @@ -663,10 +663,10 @@ public void testJsonIncludeSourceOnParserError() throws IOException { var sourceEnabled = XContentParserConfiguration.EMPTY; var sourceDisabled = XContentParserConfiguration.EMPTY.withIncludeSourceOnError(false); - var parseException = assertThrows(XContentParseException.class, () -> createParser(xContent, sourceEnabled, source).map()); + var parseException = expectThrows(XContentParseException.class, () -> createParser(xContent, sourceEnabled, source).map()); assertThat(parseException.getMessage(), containsString(source)); - parseException = assertThrows(XContentParseException.class, () -> createParser(xContent, sourceDisabled, source).map()); + parseException = expectThrows(XContentParseException.class, () -> createParser(xContent, sourceDisabled, source).map()); assertThat(parseException.getMessage(), not(containsString(source))); } diff --git a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java index 709a29fe66142..2f336566953ba 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java @@ -91,16 +91,6 @@ public BulkRequestParser(boolean deprecateOrErrorOnType, boolean includeSourceOn .withIncludeSourceOnError(includeSourceOnError); } - /** - * Create a new parser. - * - * @param deprecateOrErrorOnType whether to allow _type information in the index line; used by BulkMonitoring - * @param restApiVersion - */ - public BulkRequestParser(boolean deprecateOrErrorOnType, RestApiVersion restApiVersion) { - this(deprecateOrErrorOnType, true, restApiVersion); - } - private static int findNextMarker(byte marker, int from, BytesReference data, boolean lastData) { final int res = data.indexOf(marker, from); if (res != -1) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java index 474defe77c257..5396fdef0f041 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java @@ -110,7 +110,7 @@ public XContentMeteringParserDecorator getMeteringParserDecorator() { return meteringParserDecorator; } - boolean getIncludeSourceOnError() { + public boolean getIncludeSourceOnError() { return includeSourceOnError; } } diff --git a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java index 9d944d43f4c36..de2dc8f9ea0b5 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestParserTests.java @@ -39,7 +39,7 @@ public void testParserCannotBeReusedAfterFailure() { {} """); - BulkRequestParser parser = new BulkRequestParser(randomBoolean(), RestApiVersion.current()); + BulkRequestParser parser = new BulkRequestParser(randomBoolean(), true, RestApiVersion.current()); BulkRequestParser.IncrementalParser incrementalParser = parser.incrementalParser( null, null, @@ -70,7 +70,7 @@ public void testIncrementalParsing() throws IOException { ArrayList> updateRequests = new ArrayList<>(); ArrayList> deleteRequests = new ArrayList<>(); - BulkRequestParser parser = new BulkRequestParser(randomBoolean(), RestApiVersion.current()); + BulkRequestParser parser = new BulkRequestParser(randomBoolean(), true, RestApiVersion.current()); BulkRequestParser.IncrementalParser incrementalParser = parser.incrementalParser( null, null, @@ -116,7 +116,7 @@ public void testIndexRequest() throws IOException { { "index":{ "_id": "bar" } } {} """); - BulkRequestParser parser = new BulkRequestParser(randomBoolean(), RestApiVersion.current()); + BulkRequestParser parser = new BulkRequestParser(randomBoolean(), true, RestApiVersion.current()); final AtomicBoolean parsed = new AtomicBoolean(); parser.parse(request, "foo", null, null, null, null, null, null, false, XContentType.JSON, (indexRequest, type) -> { assertFalse(parsed.get()); @@ -152,7 +152,7 @@ public void testDeleteRequest() throws IOException { BytesArray request = new BytesArray(""" { "delete":{ "_id": "bar" } } """); - BulkRequestParser parser = new BulkRequestParser(randomBoolean(), RestApiVersion.current()); + BulkRequestParser parser = new BulkRequestParser(randomBoolean(), true, RestApiVersion.current()); final AtomicBoolean parsed = new AtomicBoolean(); parser.parse( request, @@ -182,7 +182,7 @@ public void testUpdateRequest() throws IOException { { "update":{ "_id": "bar" } } {} """); - BulkRequestParser parser = new BulkRequestParser(randomBoolean(), RestApiVersion.current()); + BulkRequestParser parser = new BulkRequestParser(randomBoolean(), true, RestApiVersion.current()); final AtomicBoolean parsed = new AtomicBoolean(); parser.parse(request, "foo", null, null, null, null, null, null, false, XContentType.JSON, (req, type) -> fail(), updateRequest -> { assertFalse(parsed.get()); @@ -218,7 +218,7 @@ public void testBarfOnLackOfTrailingNewline() throws IOException { BytesArray request = new BytesArray(""" { "index":{ "_id": "bar" } } {}"""); - BulkRequestParser parser = new BulkRequestParser(randomBoolean(), RestApiVersion.current()); + BulkRequestParser parser = new BulkRequestParser(randomBoolean(), true, RestApiVersion.current()); IllegalArgumentException e = expectThrows( IllegalArgumentException.class, () -> parser.parse( @@ -266,7 +266,7 @@ public void testFailOnExplicitIndex() { { "index":{ "_index": "foo", "_id": "bar" } } {} """); - BulkRequestParser parser = new BulkRequestParser(randomBoolean(), RestApiVersion.current()); + BulkRequestParser parser = new BulkRequestParser(randomBoolean(), true, RestApiVersion.current()); IllegalArgumentException ex = expectThrows( IllegalArgumentException.class, @@ -294,7 +294,7 @@ public void testTypesStillParsedForBulkMonitoring() throws IOException { { "index":{ "_type": "quux", "_id": "bar" } } {} """); - BulkRequestParser parser = new BulkRequestParser(false, RestApiVersion.current()); + BulkRequestParser parser = new BulkRequestParser(false, true, RestApiVersion.current()); final AtomicBoolean parsed = new AtomicBoolean(); parser.parse(request, "foo", null, null, null, null, null, null, false, XContentType.JSON, (indexRequest, type) -> { assertFalse(parsed.get()); @@ -313,7 +313,7 @@ public void testParseDeduplicatesParameterStrings() throws IOException { { "index":{ "_index": "bar", "pipeline": "foo", "routing": "blub" } } {} """); - BulkRequestParser parser = new BulkRequestParser(randomBoolean(), RestApiVersion.current()); + BulkRequestParser parser = new BulkRequestParser(randomBoolean(), true, RestApiVersion.current()); final List indexRequests = new ArrayList<>(); parser.parse( request, @@ -343,7 +343,7 @@ public void testFailOnInvalidAction() { { "invalidaction":{ } } {} """); - BulkRequestParser parser = new BulkRequestParser(randomBoolean(), randomFrom(RestApiVersion.values())); + BulkRequestParser parser = new BulkRequestParser(randomBoolean(), true, randomFrom(RestApiVersion.values())); IllegalArgumentException ex = expectThrows( IllegalArgumentException.class, @@ -374,7 +374,7 @@ public void testFailMissingCloseBrace() { { "index":{ } {} """); - BulkRequestParser parser = new BulkRequestParser(randomBoolean(), randomFrom(REST_API_VERSIONS_POST_V8)); + BulkRequestParser parser = new BulkRequestParser(randomBoolean(), true, randomFrom(REST_API_VERSIONS_POST_V8)); IllegalArgumentException ex = expectThrows( IllegalArgumentException.class, @@ -402,7 +402,7 @@ public void testFailExtraKeys() { { "index":{ }, "something": "unexpected" } {} """); - BulkRequestParser parser = new BulkRequestParser(randomBoolean(), randomFrom(REST_API_VERSIONS_POST_V8)); + BulkRequestParser parser = new BulkRequestParser(randomBoolean(), true, randomFrom(REST_API_VERSIONS_POST_V8)); IllegalArgumentException ex = expectThrows( IllegalArgumentException.class, @@ -430,7 +430,7 @@ public void testFailContentAfterClosingBrace() { { "index":{ } } { "something": "unexpected" } {} """); - BulkRequestParser parser = new BulkRequestParser(randomBoolean(), randomFrom(REST_API_VERSIONS_POST_V8)); + BulkRequestParser parser = new BulkRequestParser(randomBoolean(), true, randomFrom(REST_API_VERSIONS_POST_V8)); IllegalArgumentException ex = expectThrows( IllegalArgumentException.class, @@ -458,7 +458,7 @@ public void testListExecutedPipelines() throws IOException { { "index":{ "_id": "bar" } } {} """); - BulkRequestParser parser = new BulkRequestParser(randomBoolean(), RestApiVersion.current()); + BulkRequestParser parser = new BulkRequestParser(randomBoolean(), true, RestApiVersion.current()); parser.parse(request, "foo", null, null, null, null, null, null, false, XContentType.JSON, (indexRequest, type) -> { assertFalse(indexRequest.getListExecutedPipelines()); }, req -> fail(), req -> fail()); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/monitoring/action/MonitoringBulkRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/monitoring/action/MonitoringBulkRequest.java index 638e57207fbeb..36bc2db95932d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/monitoring/action/MonitoringBulkRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/monitoring/action/MonitoringBulkRequest.java @@ -83,7 +83,7 @@ public MonitoringBulkRequest add( ) throws IOException { // MonitoringBulkRequest accepts a body request that has the same format as the BulkRequest - new BulkRequestParser(false, RestApiVersion.current()).parse( + new BulkRequestParser(false, true, RestApiVersion.current()).parse( content, null, null, diff --git a/x-pack/qa/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/test/CoreTestTranslater.java b/x-pack/qa/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/test/CoreTestTranslater.java index d34303ea803d6..51c9e35c95a3d 100644 --- a/x-pack/qa/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/test/CoreTestTranslater.java +++ b/x-pack/qa/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/test/CoreTestTranslater.java @@ -366,7 +366,7 @@ private boolean handleBulk(ApiCallSection bulk) { bos.write(JsonXContent.jsonXContent.bulkSeparator()); } List indexRequests = new ArrayList<>(); - new BulkRequestParser(false, RestApiVersion.current()).parse( + new BulkRequestParser(false, true, RestApiVersion.current()).parse( bos.bytes(), defaultIndex, defaultRouting, From 3eae9e1003edfe41f318b7525c1f14ad31f44b27 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Mon, 27 Jan 2025 11:40:11 +0100 Subject: [PATCH 07/12] update api specs --- rest-api-spec/src/main/resources/rest-api-spec/api/bulk.json | 4 ++++ .../src/main/resources/rest-api-spec/api/create.json | 4 ++++ rest-api-spec/src/main/resources/rest-api-spec/api/index.json | 4 ++++ .../src/main/resources/rest-api-spec/api/update.json | 4 ++++ 4 files changed, 16 insertions(+) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/bulk.json b/rest-api-spec/src/main/resources/rest-api-spec/api/bulk.json index f9c8041d7221f..490bb6fd73bbe 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/bulk.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/bulk.json @@ -83,6 +83,10 @@ "list_executed_pipelines": { "type": "boolean", "description": "Sets list_executed_pipelines for all incoming documents. Defaults to unset (false)" + }, + "include_source_on_error": { + "type": "boolean", + "description": "True or false if to include the document source in the error message in case of parsing errors. Defaults to true." } }, "body":{ diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/create.json b/rest-api-spec/src/main/resources/rest-api-spec/api/create.json index 8ed4c04917d3a..65cb0da4753cc 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/create.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/create.json @@ -69,6 +69,10 @@ "pipeline":{ "type":"string", "description":"The pipeline id to preprocess incoming documents with" + }, + "include_source_on_error": { + "type": "boolean", + "description": "True or false if to include the document source in the error message in case of parsing errors. Defaults to true." } }, "body":{ diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/index.json b/rest-api-spec/src/main/resources/rest-api-spec/api/index.json index 102ca4e012e85..79ecbd794024a 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/index.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/index.json @@ -105,6 +105,10 @@ "require_data_stream": { "type": "boolean", "description": "When true, requires the destination to be a data stream (existing or to-be-created). Default is false" + }, + "include_source_on_error": { + "type": "boolean", + "description": "True or false if to include the document source in the error message in case of parsing errors. Defaults to true." } }, "body":{ diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/update.json b/rest-api-spec/src/main/resources/rest-api-spec/api/update.json index e588777e990ec..9e47e80547e88 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/update.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/update.json @@ -83,6 +83,10 @@ "require_alias": { "type": "boolean", "description": "When true, requires destination is an alias. Default is false" + }, + "include_source_on_error": { + "type": "boolean", + "description": "True or false if to include the document source in the error message in case of parsing errors. Defaults to true." } }, "body":{ From 6a3e74a53bd4cf01cf75c7f59cc5f8ce1be29aef Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Mon, 27 Jan 2025 12:09:57 +0100 Subject: [PATCH 08/12] Make includeSourceOnError nullable to control default in a single place --- .../provider/XContentParserConfigurationImpl.java | 10 +++++----- .../xcontent/provider/json/JsonXContentImpl.java | 2 +- .../xcontent/XContentParserConfiguration.java | 4 ++-- .../org/elasticsearch/action/bulk/BulkRequest.java | 10 +++++----- .../elasticsearch/action/bulk/BulkRequestParser.java | 2 +- .../org/elasticsearch/action/index/IndexRequest.java | 10 +++++----- .../elasticsearch/index/mapper/SourceToParse.java | 12 ++++++------ .../main/java/org/elasticsearch/rest/RestUtils.java | 9 +++++---- 8 files changed, 30 insertions(+), 29 deletions(-) diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/XContentParserConfigurationImpl.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/XContentParserConfigurationImpl.java index e04c640ad7461..755eda4f1be22 100644 --- a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/XContentParserConfigurationImpl.java +++ b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/XContentParserConfigurationImpl.java @@ -32,7 +32,7 @@ public class XContentParserConfigurationImpl implements XContentParserConfigurat null, null, false, - true + null ); final NamedXContentRegistry registry; @@ -41,7 +41,7 @@ public class XContentParserConfigurationImpl implements XContentParserConfigurat final FilterPath[] includes; final FilterPath[] excludes; final boolean filtersMatchFieldNamesWithDots; - final boolean includeSourceOnError; + final Boolean includeSourceOnError; private XContentParserConfigurationImpl( NamedXContentRegistry registry, @@ -50,7 +50,7 @@ private XContentParserConfigurationImpl( FilterPath[] includes, FilterPath[] excludes, boolean filtersMatchFieldNamesWithDots, - boolean includeSourceOnError + Boolean includeSourceOnError ) { this.registry = registry; this.deprecationHandler = deprecationHandler; @@ -62,12 +62,12 @@ private XContentParserConfigurationImpl( } @Override - public boolean includeSourceOnError() { + public Boolean includeSourceOnError() { return includeSourceOnError; } @Override - public XContentParserConfiguration withIncludeSourceOnError(boolean includeSourceOnError) { + public XContentParserConfiguration withIncludeSourceOnError(Boolean includeSourceOnError) { if (includeSourceOnError == this.includeSourceOnError) { return this; } diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java index 7f52467caf49b..bc82ecb944262 100644 --- a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java +++ b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java @@ -88,7 +88,7 @@ public XContentGenerator createGenerator(OutputStream os, Set includes, } private XContentParser createParser(XContentParserConfiguration config, JsonParser parser) { - if (config.includeSourceOnError() == false) { + if (config.includeSourceOnError() == Boolean.FALSE) { parser.disable(JsonParser.Feature.INCLUDE_SOURCE_IN_LOCATION); // enabled by default, disable if requested } return new JsonXContentParser(config, parser); diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java index 73ebdfce222ad..6d239b55ba943 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java +++ b/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java @@ -29,9 +29,9 @@ public interface XContentParserConfiguration { /** * Disable to not include the source in case of parsing errors (defaults to true). */ - XContentParserConfiguration withIncludeSourceOnError(boolean includeSourceOnError); + XContentParserConfiguration withIncludeSourceOnError(Boolean includeSourceOnError); - boolean includeSourceOnError(); + Boolean includeSourceOnError(); /** * Replace the registry backing {@link XContentParser#namedObject}. diff --git a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java index c491caaf66f83..4aed34f1c9cc3 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java @@ -84,7 +84,7 @@ public class BulkRequest extends ActionRequest private String globalIndex; private Boolean globalRequireAlias; private Boolean globalRequireDatsStream; - private boolean includeSourceOnError = true; + private Boolean includeSourceOnError; private long sizeInBytes = 0; @@ -105,7 +105,7 @@ public BulkRequest(StreamInput in) throws IOException { incrementalState = BulkRequest.IncrementalState.EMPTY; } if (in.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { - includeSourceOnError = in.readBoolean(); + includeSourceOnError = in.readOptionalBoolean(); } } @@ -345,7 +345,7 @@ public void incrementalState(IncrementalState incrementalState) { this.incrementalState = incrementalState; } - public final BulkRequest includeSourceOnError(boolean includeSourceOnError) { + public final BulkRequest includeSourceOnError(Boolean includeSourceOnError) { this.includeSourceOnError = includeSourceOnError; return this; } @@ -408,7 +408,7 @@ public Boolean requireDataStream() { return globalRequireDatsStream; } - public boolean includeSourceOnError() { + public Boolean includeSourceOnError() { return includeSourceOnError; } @@ -471,7 +471,7 @@ public void writeTo(StreamOutput out) throws IOException { incrementalState.writeTo(out); } if (out.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { - out.writeBoolean(includeSourceOnError); + out.writeOptionalBoolean(includeSourceOnError); } } diff --git a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java index 2f336566953ba..f35a5f8cf5b69 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java @@ -84,7 +84,7 @@ public final class BulkRequestParser { * @param includeSourceOnError if to include the source in parser error messages * @param restApiVersion */ - public BulkRequestParser(boolean deprecateOrErrorOnType, boolean includeSourceOnError, RestApiVersion restApiVersion) { + public BulkRequestParser(boolean deprecateOrErrorOnType, Boolean includeSourceOnError, RestApiVersion restApiVersion) { this.deprecateOrErrorOnType = deprecateOrErrorOnType; this.config = XContentParserConfiguration.EMPTY.withDeprecationHandler(LoggingDeprecationHandler.INSTANCE) .withRestApiVersion(restApiVersion) diff --git a/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java b/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java index 2bfc229464b58..f6d488ee0e809 100644 --- a/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java +++ b/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java @@ -116,7 +116,7 @@ public class IndexRequest extends ReplicatedWriteRequest implement private boolean requireDataStream; - private boolean includeSourceOnError = true; + private Boolean includeSourceOnError; /** * Transient flag denoting that the local request should be routed to a failure store. Not persisted across the wire. @@ -214,7 +214,7 @@ public IndexRequest(@Nullable ShardId shardId, StreamInput in) throws IOExceptio } if (in.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { - includeSourceOnError = in.readBoolean(); + includeSourceOnError = in.readOptionalBoolean(); } } @@ -813,7 +813,7 @@ private void writeBody(StreamOutput out) throws IOException { } } if (out.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { - out.writeBoolean(includeSourceOnError); + out.writeOptionalBoolean(includeSourceOnError); } } @@ -883,11 +883,11 @@ public IndexRequest setRequireDataStream(boolean requireDataStream) { return this; } - public boolean getIncludeSourceOnError() { + public Boolean getIncludeSourceOnError() { return includeSourceOnError; } - public IndexRequest setIncludeSourceOnError(boolean includeSourceOnError) { + public IndexRequest setIncludeSourceOnError(Boolean includeSourceOnError) { this.includeSourceOnError = includeSourceOnError; return this; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java index 5396fdef0f041..a860117e6ea03 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java @@ -30,7 +30,7 @@ public class SourceToParse { private final Map dynamicTemplates; - private final boolean includeSourceOnError; + private final Boolean includeSourceOnError; private final XContentMeteringParserDecorator meteringParserDecorator; @@ -40,7 +40,7 @@ public SourceToParse( XContentType xContentType, @Nullable String routing, Map dynamicTemplates, - boolean includeSourceOnError, + Boolean includeSourceOnError, XContentMeteringParserDecorator meteringParserDecorator ) { this.id = id; @@ -55,11 +55,11 @@ public SourceToParse( } public SourceToParse(String id, BytesReference source, XContentType xContentType) { - this(id, source, xContentType, null, Map.of(), true, XContentMeteringParserDecorator.NOOP); + this(id, source, xContentType, null, Map.of(), null, XContentMeteringParserDecorator.NOOP); } public SourceToParse(String id, BytesReference source, XContentType xContentType, String routing) { - this(id, source, xContentType, routing, Map.of(), true, XContentMeteringParserDecorator.NOOP); + this(id, source, xContentType, routing, Map.of(), null, XContentMeteringParserDecorator.NOOP); } public SourceToParse( @@ -69,7 +69,7 @@ public SourceToParse( String routing, Map dynamicTemplates ) { - this(id, source, xContentType, routing, dynamicTemplates, true, XContentMeteringParserDecorator.NOOP); + this(id, source, xContentType, routing, dynamicTemplates, null, XContentMeteringParserDecorator.NOOP); } public BytesReference source() { @@ -110,7 +110,7 @@ public XContentMeteringParserDecorator getMeteringParserDecorator() { return meteringParserDecorator; } - public boolean getIncludeSourceOnError() { + public Boolean getIncludeSourceOnError() { return includeSourceOnError; } } diff --git a/server/src/main/java/org/elasticsearch/rest/RestUtils.java b/server/src/main/java/org/elasticsearch/rest/RestUtils.java index 68ec7a758e73c..a71b43853ef3e 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestUtils.java +++ b/server/src/main/java/org/elasticsearch/rest/RestUtils.java @@ -336,14 +336,15 @@ public static TimeValue getTimeout(RestRequest restRequest) { } /** - * Extract the {@code ?include_source_on_error} parameter from the request, returning {@code true} in case the parameter is missing. + * Extract the {@code ?include_source_on_error} parameter from the request, or {@code null} if missing. + * By default, the source is included, this parameter allows to disable this. * * @param restRequest The request from which to extract the {@code ?include_source_on_error} parameter - * @return the value of the {@code ?include_source_on_error} parameter from the request, with a default of {@code true} if the request + * @return the value of the {@code ?include_source_on_error} parameter from the request. */ - public static boolean getIncludeSourceOnError(RestRequest restRequest) { + public static Boolean getIncludeSourceOnError(RestRequest restRequest) { assert restRequest != null; - return restRequest.paramAsBoolean(INCLUDE_SOURCE_ON_ERROR_PARAMETER, true); + return restRequest.paramAsBoolean(INCLUDE_SOURCE_ON_ERROR_PARAMETER, null); } // Remove the BWC support for the deprecated ?local parameter. From c380dea9b856690052e00aab8dd929c9e9374ead Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Mon, 27 Jan 2025 12:42:25 +0100 Subject: [PATCH 09/12] do not apply includeSourceOnError silently to all content parsers --- .../org/elasticsearch/rest/RestRequest.java | 29 ++++++++++++------- .../action/document/RestUpdateAction.java | 3 +- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index eff73f33cf546..4fcb294d71453 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -537,32 +537,39 @@ public XContentParserConfiguration contentParserConfig() { return parserConfig; } - private XContentParserConfiguration internalParserConfig() { - // consume query param lazily so we can report if consumed or not - return parserConfig.withIncludeSourceOnError(RestUtils.getIncludeSourceOnError(this)); - } - /** * A parser for the contents of this request if there is a body, otherwise throws an {@link ElasticsearchParseException}. Use * {@link #applyContentParser(CheckedConsumer)} if you want to gracefully handle when the request doesn't have any contents. Use * {@link #contentOrSourceParamParser()} for requests that support specifying the request body in the {@code source} param. */ public final XContentParser contentParser() throws IOException { + return contentParser(parserConfig); + } + + private XContentParser contentParser(XContentParserConfiguration parserConfig) throws IOException { BytesReference content = requiredContent(); // will throw exception if body or content type missing - return XContentHelper.createParserNotCompressed(internalParserConfig(), content, xContentType.get()); + return XContentHelper.createParserNotCompressed(parserConfig, content, xContentType.get()); } /** - * If there is any content then call {@code applyParser} with the parser, otherwise do nothing. + * If there is any content then call {@code applyParser} with the parser modified by {@code includeSourceOnError}, otherwise do nothing. */ - public final void applyContentParser(CheckedConsumer applyParser) throws IOException { + public final void applyContentParser(Boolean includeSourceOnError, CheckedConsumer applyParser) + throws IOException { if (hasContent()) { - try (XContentParser parser = contentParser()) { + try (XContentParser parser = contentParser(parserConfig.withIncludeSourceOnError(includeSourceOnError))) { applyParser.accept(parser); } } } + /** + * If there is any content then call {@code applyParser} with the parser, otherwise do nothing. + */ + public final void applyContentParser(CheckedConsumer applyParser) throws IOException { + applyContentParser(null, applyParser); + } + /** * Does this request have content or a {@code source} parameter? Use this instead of {@link #hasContent()} if this * {@linkplain RestHandler} treats the {@code source} parameter like the body content. @@ -578,7 +585,7 @@ public final boolean hasContentOrSourceParam() { */ public final XContentParser contentOrSourceParamParser() throws IOException { Tuple tuple = contentOrSourceParam(); - return XContentHelper.createParserNotCompressed(internalParserConfig(), tuple.v2(), tuple.v1().xContent().type()); + return XContentHelper.createParserNotCompressed(parserConfig, tuple.v2(), tuple.v1().xContent().type()); } /** @@ -589,7 +596,7 @@ public final XContentParser contentOrSourceParamParser() throws IOException { public final void withContentOrSourceParamParserOrNull(CheckedConsumer withParser) throws IOException { if (hasContentOrSourceParam()) { Tuple tuple = contentOrSourceParam(); - try (XContentParser parser = XContentHelper.createParserNotCompressed(internalParserConfig(), tuple.v2(), tuple.v1())) { + try (XContentParser parser = XContentHelper.createParserNotCompressed(parserConfig, tuple.v2(), tuple.v1())) { withParser.accept(parser); } } else { diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java index 57b3a89b2303b..a61be96b37caa 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java @@ -19,6 +19,7 @@ import org.elasticsearch.index.VersionType; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.RestUtils; import org.elasticsearch.rest.Scope; import org.elasticsearch.rest.ServerlessScope; import org.elasticsearch.rest.action.RestActions; @@ -72,7 +73,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC updateRequest.setIfPrimaryTerm(request.paramAsLong("if_primary_term", updateRequest.ifPrimaryTerm())); updateRequest.setRequireAlias(request.paramAsBoolean(DocWriteRequest.REQUIRE_ALIAS, updateRequest.isRequireAlias())); - request.applyContentParser(parser -> { + request.applyContentParser(RestUtils.getIncludeSourceOnError(request), parser -> { updateRequest.fromXContent(parser); IndexRequest upsertRequest = updateRequest.upsertRequest(); if (upsertRequest != null) { From e54173accb285cfa7209ceeaeabf6de8422a311e Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Mon, 27 Jan 2025 18:38:32 +0100 Subject: [PATCH 10/12] Revert "Make includeSourceOnError nullable to control default in a single place" This reverts commit 6a3e74a53bd4cf01cf75c7f59cc5f8ce1be29aef. --- .../provider/XContentParserConfigurationImpl.java | 10 +++++----- .../xcontent/provider/json/JsonXContentImpl.java | 2 +- .../xcontent/XContentParserConfiguration.java | 4 ++-- .../org/elasticsearch/action/bulk/BulkRequest.java | 10 +++++----- .../elasticsearch/action/bulk/BulkRequestParser.java | 2 +- .../org/elasticsearch/action/index/IndexRequest.java | 10 +++++----- .../elasticsearch/index/mapper/SourceToParse.java | 12 ++++++------ .../main/java/org/elasticsearch/rest/RestUtils.java | 9 ++++----- 8 files changed, 29 insertions(+), 30 deletions(-) diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/XContentParserConfigurationImpl.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/XContentParserConfigurationImpl.java index 755eda4f1be22..e04c640ad7461 100644 --- a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/XContentParserConfigurationImpl.java +++ b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/XContentParserConfigurationImpl.java @@ -32,7 +32,7 @@ public class XContentParserConfigurationImpl implements XContentParserConfigurat null, null, false, - null + true ); final NamedXContentRegistry registry; @@ -41,7 +41,7 @@ public class XContentParserConfigurationImpl implements XContentParserConfigurat final FilterPath[] includes; final FilterPath[] excludes; final boolean filtersMatchFieldNamesWithDots; - final Boolean includeSourceOnError; + final boolean includeSourceOnError; private XContentParserConfigurationImpl( NamedXContentRegistry registry, @@ -50,7 +50,7 @@ private XContentParserConfigurationImpl( FilterPath[] includes, FilterPath[] excludes, boolean filtersMatchFieldNamesWithDots, - Boolean includeSourceOnError + boolean includeSourceOnError ) { this.registry = registry; this.deprecationHandler = deprecationHandler; @@ -62,12 +62,12 @@ private XContentParserConfigurationImpl( } @Override - public Boolean includeSourceOnError() { + public boolean includeSourceOnError() { return includeSourceOnError; } @Override - public XContentParserConfiguration withIncludeSourceOnError(Boolean includeSourceOnError) { + public XContentParserConfiguration withIncludeSourceOnError(boolean includeSourceOnError) { if (includeSourceOnError == this.includeSourceOnError) { return this; } diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java index bc82ecb944262..7f52467caf49b 100644 --- a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java +++ b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/json/JsonXContentImpl.java @@ -88,7 +88,7 @@ public XContentGenerator createGenerator(OutputStream os, Set includes, } private XContentParser createParser(XContentParserConfiguration config, JsonParser parser) { - if (config.includeSourceOnError() == Boolean.FALSE) { + if (config.includeSourceOnError() == false) { parser.disable(JsonParser.Feature.INCLUDE_SOURCE_IN_LOCATION); // enabled by default, disable if requested } return new JsonXContentParser(config, parser); diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java index 6d239b55ba943..73ebdfce222ad 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java +++ b/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java @@ -29,9 +29,9 @@ public interface XContentParserConfiguration { /** * Disable to not include the source in case of parsing errors (defaults to true). */ - XContentParserConfiguration withIncludeSourceOnError(Boolean includeSourceOnError); + XContentParserConfiguration withIncludeSourceOnError(boolean includeSourceOnError); - Boolean includeSourceOnError(); + boolean includeSourceOnError(); /** * Replace the registry backing {@link XContentParser#namedObject}. diff --git a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java index 4aed34f1c9cc3..c491caaf66f83 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java @@ -84,7 +84,7 @@ public class BulkRequest extends ActionRequest private String globalIndex; private Boolean globalRequireAlias; private Boolean globalRequireDatsStream; - private Boolean includeSourceOnError; + private boolean includeSourceOnError = true; private long sizeInBytes = 0; @@ -105,7 +105,7 @@ public BulkRequest(StreamInput in) throws IOException { incrementalState = BulkRequest.IncrementalState.EMPTY; } if (in.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { - includeSourceOnError = in.readOptionalBoolean(); + includeSourceOnError = in.readBoolean(); } } @@ -345,7 +345,7 @@ public void incrementalState(IncrementalState incrementalState) { this.incrementalState = incrementalState; } - public final BulkRequest includeSourceOnError(Boolean includeSourceOnError) { + public final BulkRequest includeSourceOnError(boolean includeSourceOnError) { this.includeSourceOnError = includeSourceOnError; return this; } @@ -408,7 +408,7 @@ public Boolean requireDataStream() { return globalRequireDatsStream; } - public Boolean includeSourceOnError() { + public boolean includeSourceOnError() { return includeSourceOnError; } @@ -471,7 +471,7 @@ public void writeTo(StreamOutput out) throws IOException { incrementalState.writeTo(out); } if (out.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { - out.writeOptionalBoolean(includeSourceOnError); + out.writeBoolean(includeSourceOnError); } } diff --git a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java index f35a5f8cf5b69..2f336566953ba 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java @@ -84,7 +84,7 @@ public final class BulkRequestParser { * @param includeSourceOnError if to include the source in parser error messages * @param restApiVersion */ - public BulkRequestParser(boolean deprecateOrErrorOnType, Boolean includeSourceOnError, RestApiVersion restApiVersion) { + public BulkRequestParser(boolean deprecateOrErrorOnType, boolean includeSourceOnError, RestApiVersion restApiVersion) { this.deprecateOrErrorOnType = deprecateOrErrorOnType; this.config = XContentParserConfiguration.EMPTY.withDeprecationHandler(LoggingDeprecationHandler.INSTANCE) .withRestApiVersion(restApiVersion) diff --git a/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java b/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java index f6d488ee0e809..2bfc229464b58 100644 --- a/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java +++ b/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java @@ -116,7 +116,7 @@ public class IndexRequest extends ReplicatedWriteRequest implement private boolean requireDataStream; - private Boolean includeSourceOnError; + private boolean includeSourceOnError = true; /** * Transient flag denoting that the local request should be routed to a failure store. Not persisted across the wire. @@ -214,7 +214,7 @@ public IndexRequest(@Nullable ShardId shardId, StreamInput in) throws IOExceptio } if (in.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { - includeSourceOnError = in.readOptionalBoolean(); + includeSourceOnError = in.readBoolean(); } } @@ -813,7 +813,7 @@ private void writeBody(StreamOutput out) throws IOException { } } if (out.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { - out.writeOptionalBoolean(includeSourceOnError); + out.writeBoolean(includeSourceOnError); } } @@ -883,11 +883,11 @@ public IndexRequest setRequireDataStream(boolean requireDataStream) { return this; } - public Boolean getIncludeSourceOnError() { + public boolean getIncludeSourceOnError() { return includeSourceOnError; } - public IndexRequest setIncludeSourceOnError(Boolean includeSourceOnError) { + public IndexRequest setIncludeSourceOnError(boolean includeSourceOnError) { this.includeSourceOnError = includeSourceOnError; return this; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java index a860117e6ea03..5396fdef0f041 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceToParse.java @@ -30,7 +30,7 @@ public class SourceToParse { private final Map dynamicTemplates; - private final Boolean includeSourceOnError; + private final boolean includeSourceOnError; private final XContentMeteringParserDecorator meteringParserDecorator; @@ -40,7 +40,7 @@ public SourceToParse( XContentType xContentType, @Nullable String routing, Map dynamicTemplates, - Boolean includeSourceOnError, + boolean includeSourceOnError, XContentMeteringParserDecorator meteringParserDecorator ) { this.id = id; @@ -55,11 +55,11 @@ public SourceToParse( } public SourceToParse(String id, BytesReference source, XContentType xContentType) { - this(id, source, xContentType, null, Map.of(), null, XContentMeteringParserDecorator.NOOP); + this(id, source, xContentType, null, Map.of(), true, XContentMeteringParserDecorator.NOOP); } public SourceToParse(String id, BytesReference source, XContentType xContentType, String routing) { - this(id, source, xContentType, routing, Map.of(), null, XContentMeteringParserDecorator.NOOP); + this(id, source, xContentType, routing, Map.of(), true, XContentMeteringParserDecorator.NOOP); } public SourceToParse( @@ -69,7 +69,7 @@ public SourceToParse( String routing, Map dynamicTemplates ) { - this(id, source, xContentType, routing, dynamicTemplates, null, XContentMeteringParserDecorator.NOOP); + this(id, source, xContentType, routing, dynamicTemplates, true, XContentMeteringParserDecorator.NOOP); } public BytesReference source() { @@ -110,7 +110,7 @@ public XContentMeteringParserDecorator getMeteringParserDecorator() { return meteringParserDecorator; } - public Boolean getIncludeSourceOnError() { + public boolean getIncludeSourceOnError() { return includeSourceOnError; } } diff --git a/server/src/main/java/org/elasticsearch/rest/RestUtils.java b/server/src/main/java/org/elasticsearch/rest/RestUtils.java index a71b43853ef3e..68ec7a758e73c 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestUtils.java +++ b/server/src/main/java/org/elasticsearch/rest/RestUtils.java @@ -336,15 +336,14 @@ public static TimeValue getTimeout(RestRequest restRequest) { } /** - * Extract the {@code ?include_source_on_error} parameter from the request, or {@code null} if missing. - * By default, the source is included, this parameter allows to disable this. + * Extract the {@code ?include_source_on_error} parameter from the request, returning {@code true} in case the parameter is missing. * * @param restRequest The request from which to extract the {@code ?include_source_on_error} parameter - * @return the value of the {@code ?include_source_on_error} parameter from the request. + * @return the value of the {@code ?include_source_on_error} parameter from the request, with a default of {@code true} if the request */ - public static Boolean getIncludeSourceOnError(RestRequest restRequest) { + public static boolean getIncludeSourceOnError(RestRequest restRequest) { assert restRequest != null; - return restRequest.paramAsBoolean(INCLUDE_SOURCE_ON_ERROR_PARAMETER, null); + return restRequest.paramAsBoolean(INCLUDE_SOURCE_ON_ERROR_PARAMETER, true); } // Remove the BWC support for the deprecated ?local parameter. From a911e45ebb7da28dd5809f345a691bed089fcf6b Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Mon, 27 Jan 2025 18:51:56 +0100 Subject: [PATCH 11/12] fix after revert --- .../java/org/elasticsearch/action/bulk/BulkRequest.java | 2 +- .../java/org/elasticsearch/action/index/IndexRequest.java | 2 +- .../src/main/java/org/elasticsearch/rest/RestRequest.java | 8 ++++++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java index c491caaf66f83..cd4602ead42b5 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java @@ -106,7 +106,7 @@ public BulkRequest(StreamInput in) throws IOException { } if (in.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { includeSourceOnError = in.readBoolean(); - } + } // else default value is true } public BulkRequest(@Nullable String globalIndex) { diff --git a/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java b/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java index 2bfc229464b58..e774384f87343 100644 --- a/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java +++ b/server/src/main/java/org/elasticsearch/action/index/IndexRequest.java @@ -215,7 +215,7 @@ public IndexRequest(@Nullable ShardId shardId, StreamInput in) throws IOExceptio if (in.getTransportVersion().onOrAfter(TransportVersions.INGEST_REQUEST_INCLUDE_SOURCE_ON_ERROR)) { includeSourceOnError = in.readBoolean(); - } + } // else default value is true } public IndexRequest() { diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index 4fcb294d71453..fb8a8b44d8ec3 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -554,7 +554,7 @@ private XContentParser contentParser(XContentParserConfiguration parserConfig) t /** * If there is any content then call {@code applyParser} with the parser modified by {@code includeSourceOnError}, otherwise do nothing. */ - public final void applyContentParser(Boolean includeSourceOnError, CheckedConsumer applyParser) + public final void applyContentParser(boolean includeSourceOnError, CheckedConsumer applyParser) throws IOException { if (hasContent()) { try (XContentParser parser = contentParser(parserConfig.withIncludeSourceOnError(includeSourceOnError))) { @@ -567,7 +567,11 @@ public final void applyContentParser(Boolean includeSourceOnError, CheckedConsum * If there is any content then call {@code applyParser} with the parser, otherwise do nothing. */ public final void applyContentParser(CheckedConsumer applyParser) throws IOException { - applyContentParser(null, applyParser); + if (hasContent()) { + try (XContentParser parser = contentParser(parserConfig)) { + applyParser.accept(parser); + } + } } /** From 9f83956fb0725a817a2717b85279a5c27452e034 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Tue, 28 Jan 2025 08:22:51 +0100 Subject: [PATCH 12/12] changelog --- docs/changelog/120725.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/changelog/120725.yaml b/docs/changelog/120725.yaml index 1cdd511685809..71d256a559a7d 100644 --- a/docs/changelog/120725.yaml +++ b/docs/changelog/120725.yaml @@ -1,5 +1,7 @@ pr: 120725 -summary: Add common query param `include_source_on_error` +summary: |- + A new query parameter `?include_source_on_error` was added for create / index, update and bulk REST APIs to control + if to include the document source in the error response in case of parsing errors. The default value is `true`. area: Infra/REST API type: enhancement issues: []