From c6180cf605f83bd12548b0dc7504705fcc4f21f6 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 9 Sep 2021 17:54:59 -0400 Subject: [PATCH] improve FetchSourceContext and add unit tests (#77346) (#77521) Co-authored-by: Elastic Machine Co-authored-by: weizijun Co-authored-by: Elastic Machine --- .../fetch/subphase/FetchSourceContext.java | 27 ++++++++-- .../subphase/FetchSourceContextTests.java | 54 +++++++++++++++++++ .../test/AbstractXContentTestCase.java | 4 +- 3 files changed, 78 insertions(+), 7 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourceContextTests.java diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourceContext.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourceContext.java index 7b9917a68c0dc..4ffa77bce732f 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourceContext.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchSourceContext.java @@ -8,17 +8,17 @@ package org.elasticsearch.search.fetch.subphase; -import org.elasticsearch.core.Booleans; -import org.elasticsearch.common.xcontent.ParseField; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ParseField; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.support.XContentMapValues; +import org.elasticsearch.core.Booleans; import org.elasticsearch.rest.RestRequest; import java.io.IOException; @@ -111,6 +111,10 @@ public static FetchSourceContext parseFromRestRequest(RestRequest request) { } public static FetchSourceContext fromXContent(XContentParser parser) throws IOException { + if (parser.currentToken() == null) { + parser.nextToken(); + } + XContentParser.Token token = parser.currentToken(); boolean fetchSource = true; String[] includes = Strings.EMPTY_ARRAY; @@ -121,7 +125,7 @@ public static FetchSourceContext fromXContent(XContentParser parser) throws IOEx includes = new String[]{parser.text()}; } else if (token == XContentParser.Token.START_ARRAY) { ArrayList list = new ArrayList<>(); - while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { + while ((parser.nextToken()) != XContentParser.Token.END_ARRAY) { list.add(parser.text()); } includes = list.toArray(new String[list.size()]); @@ -172,8 +176,21 @@ public static FetchSourceContext fromXContent(XContentParser parser) throws IOEx } } } else { - throw new ParsingException(parser.getTokenLocation(), "Expected one of [" + XContentParser.Token.VALUE_BOOLEAN + ", " - + XContentParser.Token.START_OBJECT + "] but found [" + token + "]", parser.getTokenLocation()); + throw new ParsingException( + parser.getTokenLocation(), + "Expected one of [" + + XContentParser.Token.VALUE_BOOLEAN + + ", " + + XContentParser.Token.VALUE_STRING + + ", " + + XContentParser.Token.START_ARRAY + + ", " + + XContentParser.Token.START_OBJECT + + "] but found [" + + token + + "]", + parser.getTokenLocation() + ); } return new FetchSourceContext(fetchSource, includes, excludes); } diff --git a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourceContextTests.java b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourceContextTests.java new file mode 100644 index 0000000000000..091466b421bb1 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchSourceContextTests.java @@ -0,0 +1,54 @@ +/* + * 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 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 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.search.fetch.subphase; + +import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.test.AbstractSerializingTestCase; + +import java.io.IOException; + +import static org.hamcrest.Matchers.containsString; + +public class FetchSourceContextTests extends AbstractSerializingTestCase { + @Override + protected FetchSourceContext doParseInstance(XContentParser parser) throws IOException { + return FetchSourceContext.fromXContent(parser); + } + + @Override + protected Writeable.Reader instanceReader() { + return FetchSourceContext::new; + } + + @Override + protected FetchSourceContext createTestInstance() { + return new FetchSourceContext( + true, + randomArray(0, 5, String[]::new, () -> randomAlphaOfLength(5)), + randomArray(0, 5, String[]::new, () -> randomAlphaOfLength(5)) + ); + } + + public void testFromXContentException() throws IOException { + XContentBuilder builder = XContentFactory.jsonBuilder(); + int value = randomInt(); + builder.value(value); + XContentParser parser = createParser(builder); + ParsingException exception = expectThrows(ParsingException.class, () -> FetchSourceContext.fromXContent(parser)); + assertThat( + exception.getMessage(), + containsString("Expected one of [VALUE_BOOLEAN, VALUE_STRING, START_ARRAY, START_OBJECT] but found [VALUE_NUMBER]") + ); + + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractXContentTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractXContentTestCase.java index 5a630aa8bee4b..71caa65a989d1 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractXContentTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractXContentTestCase.java @@ -260,8 +260,8 @@ static BytesReference insertRandomFieldsAndShuffle(BytesReference xContent, XCon } else { withRandomFields = xContent; } - XContentParser parserWithRandonFields = createParserFunction.apply(XContentFactory.xContent(xContentType), withRandomFields); - return BytesReference.bytes(ESTestCase.shuffleXContent(parserWithRandonFields, false, shuffleFieldsExceptions)); + XContentParser parserWithRandomFields = createParserFunction.apply(XContentFactory.xContent(xContentType), withRandomFields); + return BytesReference.bytes(ESTestCase.shuffleXContent(parserWithRandomFields, false, shuffleFieldsExceptions)); } }