Skip to content

Pass InputStream when creating XContent parser #28754

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 21, 2018
Merged

Pass InputStream when creating XContent parser #28754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
559bfdd
Pass InputStream when creating XContent parser
dakrone Feb 20, 2018
56cd6ed
Undo boolean removal
dakrone Feb 21, 2018
535490a
Enhance deprecation javadoc
dakrone Feb 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public void execute(IngestDocument document) throws Exception {
Object fieldValue = document.getFieldValue(field, Object.class);
BytesReference bytesRef = (fieldValue == null) ? new BytesArray("null") : new BytesArray(fieldValue.toString());
try (XContentParser parser = JsonXContent.jsonXContent
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytesRef)) {
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytesRef.streamInput())) {
Copy link
Member

@jasontedor jasontedor Feb 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These input streams need to be closed.

Copy link
Member

@rjernst rjernst Feb 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe closing the XContentParser closes the internal JsonParser, which closes the stream (similar to other wrapping chains with streams/readers).

Copy link
Member

@jasontedor jasontedor Feb 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s correct but it’s not enough. An exception could be thrown constructing the parser in which case the stream will not be closed.

XContentParser.Token token = parser.nextToken();
Object value = null;
if (token == XContentParser.Token.VALUE_NULL) {
Expand Down
2 changes: 1 addition & 1 deletion ...reindex/src/main/java/org/elasticsearch/index/reindex/AbstractBulkByQueryRestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ private XContentParser extractRequestSpecificFields(RestRequest restRequest,
}
}
return parser.contentType().xContent().createParser(parser.getXContentRegistry(),
parser.getDeprecationHandler(), builder.map(body).bytes());
parser.getDeprecationHandler(), builder.map(body).bytes().streamInput());
}
}
}
2 changes: 1 addition & 1 deletion modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestReindexAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public class RestReindexAction extends AbstractBaseReindexRestHandler<ReindexReq
XContentBuilder builder = XContentFactory.contentBuilder(parser.contentType());
builder.map(source);
try (XContentParser innerParser = parser.contentType().xContent()
.createParser(parser.getXContentRegistry(), parser.getDeprecationHandler(), builder.bytes())) {
.createParser(parser.getXContentRegistry(), parser.getDeprecationHandler(), builder.bytes().streamInput())) {
request.getSearchRequest().source().parseXContent(innerParser);
}
};
Expand Down
3 changes: 2 additions & 1 deletion modules/reindex/src/main/java/org/elasticsearch/index/reindex/TransportReindexAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,8 @@ protected RequestWrapper<IndexRequest> buildRequest(ScrollableHitSource.Hit doc)
if (mainRequestXContentType != null && doc.getXContentType() != mainRequestXContentType) {
// we need to convert
try (XContentParser parser = sourceXContentType.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, doc.getSource());
.createParser(NamedXContentRegistry.EMPTY,
DeprecationHandler.THROW_UNSUPPORTED_OPERATION, doc.getSource().streamInput());
XContentBuilder builder = XContentBuilder.builder(mainRequestXContentType.xContent())) {
parser.nextToken();
builder.copyCurrentStructure(parser);
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
// EMPTY is safe here because we never call namedObject
try (XContentParser parser = xContent
.createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE,
data.slice(from, nextMarker - from))) {
data.slice(from, nextMarker - from).streamInput())) {
// move pointers
from = nextMarker + 1;

Expand Down Expand Up @@ -432,7 +432,7 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null
.parent(parent);
// EMPTY is safe here because we never call namedObject
try (XContentParser sliceParser = xContent.createParser(NamedXContentRegistry.EMPTY,
LoggingDeprecationHandler.INSTANCE, sliceTrimmingCarriageReturn(data, from, nextMarker, xContentType))) {
LoggingDeprecationHandler.INSTANCE, sliceTrimmingCarriageReturn(data, from, nextMarker, xContentType).streamInput())) {
updateRequest.fromXContent(sliceParser);
}
if (fetchSourceContext != null) {
Expand Down
5 changes: 3 additions & 2 deletions server/src/main/java/org/elasticsearch/action/search/MultiSearchRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ public static void readMultiLineFormat(BytesReference data,
// now parse the action
if (nextMarker - from > 0) {
try (XContentParser parser = xContent
.createParser(registry, LoggingDeprecationHandler.INSTANCE, data.slice(from, nextMarker - from))) {
.createParser(registry, LoggingDeprecationHandler.INSTANCE,
data.slice(from, nextMarker - from).streamInput())) {
Map<String, Object> source = parser.map();
for (Map.Entry<String, Object> entry : source.entrySet()) {
Object value = entry.getValue();
Expand Down Expand Up @@ -244,7 +245,7 @@ public static void readMultiLineFormat(BytesReference data,
break;
}
BytesReference bytes = data.slice(from, nextMarker - from);
try (XContentParser parser = xContent.createParser(registry, LoggingDeprecationHandler.INSTANCE, bytes)) {
try (XContentParser parser = xContent.createParser(registry, LoggingDeprecationHandler.INSTANCE, bytes.streamInput())) {
consumer.accept(searchRequest, parser);
}
// move pointers
Expand Down
3 changes: 3 additions & 0 deletions server/src/main/java/org/elasticsearch/common/xcontent/XContent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,10 @@ XContentParser createParser(NamedXContentRegistry xContentRegistry,

/**
* Creates a parser over the provided bytes.
* @deprecated use {@link #createParser(NamedXContentRegistry, DeprecationHandler, InputStream)} instead,
* the BytesReference coupling in this class will be removed in a future commit
*/
@Deprecated
XContentParser createParser(NamedXContentRegistry xContentRegistry,
DeprecationHandler deprecationHandler, BytesReference bytes) throws IOException;

Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ protected ScoreFunction doToFunction(QueryShardContext context) throws IOExcepti
AbstractDistanceScoreFunction scoreFunction;
// EMPTY is safe because parseVariable doesn't use namedObject
try (XContentParser parser = XContentFactory.xContent(functionBytes)
.createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, functionBytes)) {
.createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, functionBytes.streamInput())) {
scoreFunction = parseVariable(fieldName, parser, context, multiValueMode);
}
return scoreFunction;
Expand Down
6 changes: 3 additions & 3 deletions server/src/main/java/org/elasticsearch/rest/RestRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ public NamedXContentRegistry getXContentRegistry() {
*/
public final XContentParser contentParser() throws IOException {
BytesReference content = requiredContent(); // will throw exception if body or content type missing
return xContentType.get().xContent().createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, content);
return xContentType.get().xContent().createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, content.streamInput());
}

/**
Expand Down Expand Up @@ -372,7 +372,7 @@ public final boolean hasContentOrSourceParam() {
*/
public final XContentParser contentOrSourceParamParser() throws IOException {
Tuple<XContentType, BytesReference> tuple = contentOrSourceParam();
return tuple.v1().xContent().createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, tuple.v2());
return tuple.v1().xContent().createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, tuple.v2().streamInput());
}

/**
Expand All @@ -386,7 +386,7 @@ public final void withContentOrSourceParamParserOrNull(CheckedConsumer<XContentP
BytesReference content = tuple.v2();
XContentType xContentType = tuple.v1();
try (XContentParser parser = xContentType.xContent()
.createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, content)) {
.createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, content.streamInput())) {
withParser.accept(parser);
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/script/Script.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ public static Script parse(Settings settings) {
settings.toXContent(builder, ToXContent.EMPTY_PARAMS);
builder.endObject();
return parse(JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY,
LoggingDeprecationHandler.INSTANCE, builder.bytes()));
LoggingDeprecationHandler.INSTANCE, builder.bytes().streamInput()));
} catch (IOException e) {
// it should not happen since we are not actually reading from a stream but an in-memory byte[]
throw new IllegalStateException(e);
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/script/StoredScriptSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ private StoredScriptSource build() {
*/
public static StoredScriptSource parse(BytesReference content, XContentType xContentType) {
try (XContentParser parser = xContentType.xContent()
.createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, content)) {
.createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, content.streamInput())) {
Token token = parser.nextToken();

if (token != Token.START_OBJECT) {
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,7 @@ public void testNamedObject() throws IOException {
new NamedXContentRegistry.Entry(Object.class, new ParseField("str"), p -> p.text())));
XContentBuilder b = XContentBuilder.builder(xcontentType().xContent());
b.value("test");
XContentParser p = xcontentType().xContent().createParser(registry, LoggingDeprecationHandler.INSTANCE, b.bytes());
XContentParser p = xcontentType().xContent().createParser(registry, LoggingDeprecationHandler.INSTANCE, b.bytes().streamInput());
assertEquals(test1, p.namedObject(Object.class, "test1", null));
assertEquals(test2, p.namedObject(Object.class, "test2", null));
assertEquals(test2, p.namedObject(Object.class, "deprecated", null));
Expand Down
10 changes: 5 additions & 5 deletions server/src/test/java/org/elasticsearch/common/xcontent/XContentParserUtilsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public void testParseTypedKeysObject() throws IOException {
BytesReference bytes = toXContent((builder, params) -> builder.startObject("name").field("field", 0).endObject(), xContentType,
randomBoolean());
try (XContentParser parser = xContentType.xContent()
.createParser(namedXContentRegistry, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes)) {
.createParser(namedXContentRegistry, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes.streamInput())) {
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.nextToken(), parser::getTokenLocation);
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
Expand All @@ -183,7 +183,7 @@ public void testParseTypedKeysObject() throws IOException {
bytes = toXContent((builder, params) -> builder.startObject("type" + delimiter + "name").field("bool", true).endObject(),
xContentType, randomBoolean());
try (XContentParser parser = xContentType.xContent()
.createParser(namedXContentRegistry, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes)) {
.createParser(namedXContentRegistry, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes.streamInput())) {
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.nextToken(), parser::getTokenLocation);
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
Expand All @@ -203,7 +203,7 @@ public void testParseTypedKeysObject() throws IOException {
}, xContentType, randomBoolean());

try (XContentParser parser = xContentType.xContent()
.createParser(namedXContentRegistry, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes)) {
.createParser(namedXContentRegistry, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes.streamInput())) {
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);

ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.nextToken(), parser::getTokenLocation);
Expand All @@ -230,7 +230,7 @@ public void testParseTypedKeysObjectErrors() throws IOException {
BytesReference bytes = toXContent((builder, params) -> builder.startObject("name").field("field", 0).endObject(), xContentType,
randomBoolean());
try (XContentParser parser = xContentType.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes)) {
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes.streamInput())) {
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.nextToken(), parser::getTokenLocation);
ParsingException exception = expectThrows(ParsingException.class,
Expand All @@ -243,7 +243,7 @@ public void testParseTypedKeysObjectErrors() throws IOException {
BytesReference bytes = toXContent((builder, params) -> builder.startObject("").field("field", 0).endObject(), xContentType,
randomBoolean());
try (XContentParser parser = xContentType.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes)) {
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes.streamInput())) {
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.nextToken(), parser::getTokenLocation);
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/common/xcontent/support/XContentHelperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public void testToXContent() throws IOException {
} else {
BytesReference bytes = XContentHelper.toXContent(toXContent, xContentType, randomBoolean());
try (XContentParser parser = xContentType.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes)) {
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes.streamInput())) {
assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken());
assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken());
assertTrue(parser.nextToken().isValue());
Expand Down
6 changes: 4 additions & 2 deletions ...rg/elasticsearch/common/xcontent/support/filtering/AbstractXContentFilteringTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,11 @@ static void assertXContentBuilderAsBytes(final XContentBuilder expected, final X
try {
XContent xContent = XContentFactory.xContent(actual.contentType());
XContentParser jsonParser =
xContent.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, expected.bytes());
xContent.createParser(NamedXContentRegistry.EMPTY,
DeprecationHandler.THROW_UNSUPPORTED_OPERATION, expected.bytes().streamInput());
XContentParser testParser =
xContent.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, actual.bytes());
xContent.createParser(NamedXContentRegistry.EMPTY,
DeprecationHandler.THROW_UNSUPPORTED_OPERATION, actual.bytes().streamInput());

while (true) {
XContentParser.Token token1 = jsonParser.nextToken();
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/ingest/PipelineConfigurationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void testParser() throws IOException {
}

XContentParser xContentParser = xContentType.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes);
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes.streamInput());
PipelineConfiguration parsed = parser.parse(xContentParser, null);
assertEquals(xContentType, parsed.getXContentType());
assertEquals("{}", XContentHelper.convertToJson(parsed.getConfig(), false, parsed.getXContentType()));
Expand Down
8 changes: 4 additions & 4 deletions server/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ public void testFromXContentLoading() throws Exception {
XContentBuilder builder = XContentFactory.jsonBuilder();
builder.startObject().field("lang0#id0", "script0").field("lang1#id0", "script1").endObject();
XContentParser parser0 = XContentType.JSON.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, builder.bytes());
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, builder.bytes().streamInput());
expectThrows(IllegalArgumentException.class, () -> ScriptMetaData.fromXContent(parser0));

// failure to load a new namespace script and old namespace script with the same id but different langs
builder = XContentFactory.jsonBuilder();
builder.startObject().field("lang0#id0", "script0")
.startObject("id0").field("lang", "lang1").field("source", "script1").endObject().endObject();
XContentParser parser1 = XContentType.JSON.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, builder.bytes());
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, builder.bytes().streamInput());
expectThrows(IllegalArgumentException.class, () -> ScriptMetaData.fromXContent(parser1));

// failure to load a new namespace script and old namespace script with the same id but different langs with additional scripts
Expand All @@ -56,15 +56,15 @@ public void testFromXContentLoading() throws Exception {
.startObject("id1").field("lang", "lang0").field("source", "script0").endObject()
.startObject("id0").field("lang", "lang1").field("source", "script1").endObject().endObject();
XContentParser parser2 = XContentType.JSON.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, builder.bytes());
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, builder.bytes().streamInput());
expectThrows(IllegalArgumentException.class, () -> ScriptMetaData.fromXContent(parser2));

// okay to load the same script from the new and old namespace if the lang is the same
builder = XContentFactory.jsonBuilder();
builder.startObject().field("lang0#id0", "script0")
.startObject("id0").field("lang", "lang0").field("source", "script1").endObject().endObject();
XContentParser parser3 = XContentType.JSON.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, builder.bytes());
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, builder.bytes().streamInput());
ScriptMetaData.fromXContent(parser3);
}

Expand Down
3 changes: 2 additions & 1 deletion test/framework/src/main/java/org/elasticsearch/search/RandomSearchRequestGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,8 @@ public static SearchSourceBuilder randomSearchSourceBuilder(
jsonBuilder.endArray();
jsonBuilder.endObject();
XContentParser parser = XContentFactory.xContent(XContentType.JSON)
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, jsonBuilder.bytes());
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
jsonBuilder.bytes().streamInput());
parser.nextToken();
parser.nextToken();
parser.nextToken();
Expand Down
Loading