Skip to content

Commit

Permalink
Close xcontent parsers (partial) (#31513)
Browse files Browse the repository at this point in the history
Partial pass at closing XContentParsers in server.  This mostly involved adding try-with-resources statements around the usage of XContentParsers.
  • Loading branch information
tomcallahan committed Jun 24, 2018
1 parent 9efb0fe commit 629c376
Show file tree
Hide file tree
Showing 53 changed files with 1,197 additions and 1,036 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -558,9 +558,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.startObject("mappings");
for (Map.Entry<String, String> entry : mappings.entrySet()) {
builder.field(entry.getKey());
XContentParser parser = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY,
DeprecationHandler.THROW_UNSUPPORTED_OPERATION, entry.getValue());
builder.copyCurrentStructure(parser);
try (XContentParser parser = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY,
DeprecationHandler.THROW_UNSUPPORTED_OPERATION, entry.getValue())) {
builder.copyCurrentStructure(parser);
}
}
builder.endObject();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,13 @@ private void doFromXContentTestWithRandomFields(boolean addRandomFields) throws
assertThat(iae.getMessage(),
containsString("[cluster_update_settings_request] unknown field [" + unsupportedField + "], parser not found"));
} else {
XContentParser parser = createParser(xContentType.xContent(), originalBytes);
ClusterUpdateSettingsRequest parsedRequest = ClusterUpdateSettingsRequest.fromXContent(parser);
try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) {
ClusterUpdateSettingsRequest parsedRequest = ClusterUpdateSettingsRequest.fromXContent(parser);

assertNull(parser.nextToken());
assertThat(parsedRequest.transientSettings(), equalTo(request.transientSettings()));
assertThat(parsedRequest.persistentSettings(), equalTo(request.persistentSettings()));
assertNull(parser.nextToken());
assertThat(parsedRequest.transientSettings(), equalTo(request.transientSettings()));
assertThat(parsedRequest.persistentSettings(), equalTo(request.persistentSettings()));
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,12 @@ public static void assertMappingsEqual(Map<String, String> expected, Map<String,
for (Map.Entry<String, String> expectedEntry : expected.entrySet()) {
String expectedValue = expectedEntry.getValue();
String actualValue = actual.get(expectedEntry.getKey());
XContentParser expectedJson = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY,
try (XContentParser expectedJson = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY,
LoggingDeprecationHandler.INSTANCE, expectedValue);
XContentParser actualJson = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY,
LoggingDeprecationHandler.INSTANCE, actualValue);
assertEquals(expectedJson.map(), actualJson.map());
XContentParser actualJson = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY,
LoggingDeprecationHandler.INSTANCE, actualValue)){
assertEquals(expectedJson.map(), actualJson.map());
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,10 @@ public void testToAndFromXContent() throws IOException {

private void assertMappingsEqual(String expected, String actual) throws IOException {

XContentParser expectedJson = createParser(XContentType.JSON.xContent(), expected);
XContentParser actualJson = createParser(XContentType.JSON.xContent(), actual);
assertEquals(expectedJson.mapOrdered(), actualJson.mapOrdered());
try (XContentParser expectedJson = createParser(XContentType.JSON.xContent(), expected);
XContentParser actualJson = createParser(XContentType.JSON.xContent(), actual)) {
assertEquals(expectedJson.mapOrdered(), actualJson.mapOrdered());
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.RandomCreateIndexGenerator;
import org.elasticsearch.test.ESTestCase;
Expand Down Expand Up @@ -93,7 +94,9 @@ public void testToAndFromXContent() throws IOException {

ResizeRequest parsedResizeRequest = new ResizeRequest(resizeRequest.getTargetIndexRequest().index(),
resizeRequest.getSourceIndex());
parsedResizeRequest.fromXContent(createParser(xContentType.xContent(), originalBytes));
try (XContentParser xParser = createParser(xContentType.xContent(), originalBytes)) {
parsedResizeRequest.fromXContent(xParser);
}

assertEquals(resizeRequest.getSourceIndex(), parsedResizeRequest.getSourceIndex());
assertEquals(resizeRequest.getTargetIndexRequest().index(), parsedResizeRequest.getTargetIndexRequest().index());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,20 @@ public void testAddWithInvalidKey() throws IOException {
builder.endArray();
}
builder.endObject();
final XContentParser parser = createParser(builder);
final MultiGetRequest mgr = new MultiGetRequest();
final ParsingException e = expectThrows(
try (XContentParser parser = createParser(builder)) {
final MultiGetRequest mgr = new MultiGetRequest();
final ParsingException e = expectThrows(
ParsingException.class,
() -> {
final String defaultIndex = randomAlphaOfLength(5);
final String defaultType = randomAlphaOfLength(3);
final FetchSourceContext fetchSource = FetchSourceContext.FETCH_SOURCE;
mgr.add(defaultIndex, defaultType, null, fetchSource, null, parser, true);
});
assertThat(
assertThat(
e.toString(),
containsString("unknown key [doc] for a START_ARRAY, expected [docs] or [ids]"));
}
}

public void testUnexpectedField() throws IOException {
Expand Down Expand Up @@ -141,16 +142,17 @@ public void testXContentSerialization() throws IOException {
MultiGetRequest expected = createTestInstance();
XContentType xContentType = randomFrom(XContentType.values());
BytesReference shuffled = toShuffledXContent(expected, xContentType, ToXContent.EMPTY_PARAMS, false);
XContentParser parser = createParser(XContentFactory.xContent(xContentType), shuffled);
MultiGetRequest actual = new MultiGetRequest();
actual.add(null, null, null, null, null, parser, true);
assertThat(parser.nextToken(), nullValue());

assertThat(actual.items.size(), equalTo(expected.items.size()));
for (int i = 0; i < expected.items.size(); i++) {
MultiGetRequest.Item expectedItem = expected.items.get(i);
MultiGetRequest.Item actualItem = actual.items.get(i);
assertThat(actualItem, equalTo(expectedItem));
try (XContentParser parser = createParser(XContentFactory.xContent(xContentType), shuffled)) {
MultiGetRequest actual = new MultiGetRequest();
actual.add(null, null, null, null, null, parser, true);
assertThat(parser.nextToken(), nullValue());

assertThat(actual.items.size(), equalTo(expected.items.size()));
for (int i = 0; i < expected.items.size(); i++) {
MultiGetRequest.Item expectedItem = expected.items.get(i);
MultiGetRequest.Item actualItem = actual.items.get(i);
assertThat(actualItem, equalTo(expectedItem));
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,11 @@ public void testFromXContent() throws IOException {
MultiGetResponse expected = createTestInstance();
XContentType xContentType = randomFrom(XContentType.values());
BytesReference shuffled = toShuffledXContent(expected, xContentType, ToXContent.EMPTY_PARAMS, false);

XContentParser parser = createParser(XContentFactory.xContent(xContentType), shuffled);
MultiGetResponse parsed = MultiGetResponse.fromXContent(parser);
assertNull(parser.nextToken());
MultiGetResponse parsed;
try (XContentParser parser = createParser(XContentFactory.xContent(xContentType), shuffled)) {
parsed = MultiGetResponse.fromXContent(parser);
assertNull(parser.nextToken());
}
assertNotSame(expected, parsed);

assertThat(parsed.getResponses().length, equalTo(expected.getResponses().length));
Expand All @@ -60,6 +61,7 @@ public void testFromXContent() throws IOException {
assertThat(actualItem.getResponse(), equalTo(expectedItem.getResponse()));
}
}

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ public void testFromXContent() throws IOException {
MultiSearchResponse expected = createTestInstance();
XContentType xContentType = randomFrom(XContentType.values());
BytesReference shuffled = toShuffledXContent(expected, xContentType, ToXContent.EMPTY_PARAMS, false);
XContentParser parser = createParser(XContentFactory.xContent(xContentType), shuffled);
MultiSearchResponse actual = MultiSearchResponse.fromXContext(parser);
assertThat(parser.nextToken(), nullValue());
MultiSearchResponse actual;
try (XContentParser parser = createParser(XContentFactory.xContent(xContentType), shuffled)) {
actual = MultiSearchResponse.fromXContext(parser);
assertThat(parser.nextToken(), nullValue());
}

assertThat(actual.getTook(), equalTo(expected.getTook()));
assertThat(actual.getResponses().length, equalTo(expected.getResponses().length));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,7 @@ public void testUnknownFieldClusterMetaData() throws IOException {
.field("random", "value")
.endObject()
.endObject());
XContentParser parser = createParser(JsonXContent.jsonXContent, metadata);
try {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, metadata)) {
MetaData.Builder.fromXContent(parser);
fail();
} catch (IllegalArgumentException e) {
Expand All @@ -197,8 +196,7 @@ public void testUnknownFieldIndexMetaData() throws IOException {
.field("random", "value")
.endObject()
.endObject());
XContentParser parser = createParser(JsonXContent.jsonXContent, metadata);
try {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, metadata)) {
IndexMetaData.Builder.fromXContent(parser);
fail();
} catch (IllegalArgumentException e) {
Expand All @@ -225,9 +223,10 @@ public void testXContentWithIndexGraveyard() throws IOException {
builder.startObject();
originalMeta.toXContent(builder, ToXContent.EMPTY_PARAMS);
builder.endObject();
XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder));
final MetaData fromXContentMeta = MetaData.fromXContent(parser);
assertThat(fromXContentMeta.indexGraveyard(), equalTo(originalMeta.indexGraveyard()));
try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) {
final MetaData fromXContentMeta = MetaData.fromXContent(parser);
assertThat(fromXContentMeta.indexGraveyard(), equalTo(originalMeta.indexGraveyard()));
}
}

public void testSerializationWithIndexGraveyard() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,17 @@ abstract class BaseGeoParsingTestCase extends ESTestCase {
public abstract void testParseGeometryCollection() throws IOException;

protected void assertValidException(XContentBuilder builder, Class expectedException) throws IOException {
XContentParser parser = createParser(builder);
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, expectedException);
try (XContentParser parser = createParser(builder)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, expectedException);
}
}

protected void assertGeometryEquals(Shape expected, XContentBuilder geoJson) throws IOException {
XContentParser parser = createParser(geoJson);
parser.nextToken();
ElasticsearchGeoAssertions.assertEquals(expected, ShapeParser.parse(parser).build());
try (XContentParser parser = createParser(geoJson)) {
parser.nextToken();
ElasticsearchGeoAssertions.assertEquals(expected, ShapeParser.parse(parser).build());
}
}

protected ShapeCollection<Shape> shapeCollection(Shape... shapes) {
Expand Down

0 comments on commit 629c376

Please sign in to comment.