Skip to content
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.

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

ObjectParser : replace IllegalStateException with ParsingException #27302

Merged
merged 1 commit into from
Nov 8, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public Value parse(XContentParser parser, Value value, Context context) throws I
} else {
token = parser.nextToken();
if (token != XContentParser.Token.START_OBJECT) {
throw new IllegalStateException("[" + name + "] Expected START_OBJECT but was: " + token);
throw new ParsingException(parser.getTokenLocation(), "[" + name + "] Expected START_OBJECT but was: " + token);
}
}

Expand All @@ -159,13 +159,13 @@ public Value parse(XContentParser parser, Value value, Context context) throws I
fieldParser = getParser(currentFieldName);
} else {
if (currentFieldName == null) {
throw new IllegalStateException("[" + name + "] no field found");
throw new ParsingException(parser.getTokenLocation(), "[" + name + "] no field found");
}
if (fieldParser == null) {
assert ignoreUnknownFields : "this should only be possible if configured to ignore known fields";
parser.skipChildren(); // noop if parser points to a value, skips children if parser is start object or start array
} else {
fieldParser.assertSupports(name, token, currentFieldName);
fieldParser.assertSupports(name, token, currentFieldName, parser.getTokenLocation());
parseSub(parser, fieldParser, currentFieldName, value, context);
}
fieldParser = null;
Expand Down Expand Up @@ -330,7 +330,7 @@ private void parseSub(XContentParser parser, FieldParser fieldParser, String cur
case END_OBJECT:
case END_ARRAY:
case FIELD_NAME:
throw new IllegalStateException("[" + name + "]" + token + " is unexpected");
throw new ParsingException(parser.getTokenLocation(), "[" + name + "]" + token + " is unexpected");
case VALUE_STRING:
case VALUE_NUMBER:
case VALUE_BOOLEAN:
Expand Down Expand Up @@ -361,12 +361,12 @@ private class FieldParser {
this.type = type;
}

void assertSupports(String parserName, XContentParser.Token token, String currentFieldName) {
void assertSupports(String parserName, XContentParser.Token token, String currentFieldName, XContentLocation location) {
if (parseField.match(currentFieldName) == false) {
throw new IllegalStateException("[" + parserName + "] parsefield doesn't accept: " + currentFieldName);
throw new ParsingException(location, "[" + parserName + "] parsefield doesn't accept: " + currentFieldName);
}
if (supportedTokens.contains(token) == false) {
throw new IllegalArgumentException(
throw new ParsingException(location,
"[" + parserName + "] " + currentFieldName + " doesn't support values of type: " + token);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.util.Collections;
import java.util.List;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasSize;

public class ObjectParserTests extends ESTestCase {
Expand Down Expand Up @@ -231,12 +232,8 @@ class TestStruct {
TestStruct s = new TestStruct();

objectParser.declareField((i, c, x) -> c.test = i.text(), new ParseField("numeric_value"), ObjectParser.ValueType.FLOAT);
try {
objectParser.parse(parser, s, null);
fail("wrong type - must be number");
} catch (IllegalArgumentException ex) {
assertEquals(ex.getMessage(), "[foo] numeric_value doesn't support values of type: VALUE_BOOLEAN");
}
Exception e = expectThrows(ParsingException.class, () -> objectParser.parse(parser, s, null));
assertThat(e.getMessage(), containsString("[foo] numeric_value doesn't support values of type: VALUE_BOOLEAN"));
}

public void testParseNested() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.test.ESTestCase;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
Expand Down Expand Up @@ -93,12 +94,8 @@ public void testParseErrorOnBooleanPrecision() throws Exception {
XContentParser stParser = createParser(JsonXContent.jsonXContent, "{\"field\":\"my_loc\", \"precision\":false}");
XContentParser.Token token = stParser.nextToken();
assertSame(XContentParser.Token.START_OBJECT, token);
try {
GeoGridAggregationBuilder.parse("geohash_grid", stParser);
fail();
} catch (IllegalArgumentException ex) {
assertEquals("[geohash_grid] precision doesn't support values of type: VALUE_BOOLEAN", ex.getMessage());
}
Exception e = expectThrows(ParsingException.class, () -> GeoGridAggregationBuilder.parse("geohash_grid", stParser));
assertThat(e.getMessage(), containsString("[geohash_grid] precision doesn't support values of type: VALUE_BOOLEAN"));
}

public void testParseErrorOnPrecisionOutOfRange() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.SortField;
import org.apache.lucene.search.TermQuery;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource;
Expand Down Expand Up @@ -245,7 +246,7 @@ public void testParseUnexpectedToken() throws IOException {
parser.nextToken();
parser.nextToken();

Exception e = expectThrows(IllegalArgumentException.class, () -> ScriptSortBuilder.fromXContent(parser, null));
Exception e = expectThrows(ParsingException.class, () -> ScriptSortBuilder.fromXContent(parser, null));
assertEquals("[_script] script doesn't support values of type: START_ARRAY", e.getMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public void testIllegalXContent() throws IOException {
logger.info("Skipping test as it uses a custom duplicate check that is obsolete when strict duplicate checks are enabled.");
} else {
directGenerator = "{ \"field\" : \"f1\", \"field\" : \"f2\" }";
assertIllegalXContent(directGenerator, ParsingException.class,
assertIllegalXContent(directGenerator, IllegalArgumentException.class,
"[direct_generator] failed to parse field [field]");
}

Expand All @@ -162,7 +162,7 @@ public void testIllegalXContent() throws IOException {

// test unexpected token
directGenerator = "{ \"size\" : [ \"xxl\" ] }";
assertIllegalXContent(directGenerator, IllegalArgumentException.class,
assertIllegalXContent(directGenerator, ParsingException.class,
"[direct_generator] size doesn't support values of type: START_ARRAY");
}

Expand Down