ObjectParser: Replace IllegalStateException with ParsingException (#27302)

Relates to #27147
This commit is contained in:
olcbean 2017-11-08 14:10:11 +01:00 committed by Christoph Büscher
parent 74b1e7db51
commit bd5e7002be
5 changed files with 17 additions and 22 deletions

View File

@ -147,7 +147,7 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
} else { } else {
token = parser.nextToken(); token = parser.nextToken();
if (token != XContentParser.Token.START_OBJECT) { 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);
} }
} }
@ -159,13 +159,13 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
fieldParser = getParser(currentFieldName); fieldParser = getParser(currentFieldName);
} else { } else {
if (currentFieldName == null) { if (currentFieldName == null) {
throw new IllegalStateException("[" + name + "] no field found"); throw new ParsingException(parser.getTokenLocation(), "[" + name + "] no field found");
} }
if (fieldParser == null) { if (fieldParser == null) {
assert ignoreUnknownFields : "this should only be possible if configured to ignore known fields"; 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 parser.skipChildren(); // noop if parser points to a value, skips children if parser is start object or start array
} else { } else {
fieldParser.assertSupports(name, token, currentFieldName); fieldParser.assertSupports(name, token, currentFieldName, parser.getTokenLocation());
parseSub(parser, fieldParser, currentFieldName, value, context); parseSub(parser, fieldParser, currentFieldName, value, context);
} }
fieldParser = null; fieldParser = null;
@ -330,7 +330,7 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
case END_OBJECT: case END_OBJECT:
case END_ARRAY: case END_ARRAY:
case FIELD_NAME: case FIELD_NAME:
throw new IllegalStateException("[" + name + "]" + token + " is unexpected"); throw new ParsingException(parser.getTokenLocation(), "[" + name + "]" + token + " is unexpected");
case VALUE_STRING: case VALUE_STRING:
case VALUE_NUMBER: case VALUE_NUMBER:
case VALUE_BOOLEAN: case VALUE_BOOLEAN:
@ -361,12 +361,12 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
this.type = type; 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) { 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) { if (supportedTokens.contains(token) == false) {
throw new IllegalArgumentException( throw new ParsingException(location,
"[" + parserName + "] " + currentFieldName + " doesn't support values of type: " + token); "[" + parserName + "] " + currentFieldName + " doesn't support values of type: " + token);
} }
} }

View File

@ -34,6 +34,7 @@ import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.hasSize;
public class ObjectParserTests extends ESTestCase { public class ObjectParserTests extends ESTestCase {
@ -231,12 +232,8 @@ public class ObjectParserTests extends ESTestCase {
TestStruct s = new TestStruct(); TestStruct s = new TestStruct();
objectParser.declareField((i, c, x) -> c.test = i.text(), new ParseField("numeric_value"), ObjectParser.ValueType.FLOAT); objectParser.declareField((i, c, x) -> c.test = i.text(), new ParseField("numeric_value"), ObjectParser.ValueType.FLOAT);
try { Exception e = expectThrows(ParsingException.class, () -> objectParser.parse(parser, s, null));
objectParser.parse(parser, s, null); assertThat(e.getMessage(), containsString("[foo] numeric_value doesn't support values of type: VALUE_BOOLEAN"));
fail("wrong type - must be number");
} catch (IllegalArgumentException ex) {
assertEquals(ex.getMessage(), "[foo] numeric_value doesn't support values of type: VALUE_BOOLEAN");
}
} }
public void testParseNested() throws IOException { public void testParseNested() throws IOException {

View File

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

View File

@ -24,6 +24,7 @@ import org.apache.lucene.index.Term;
import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.SortField; import org.apache.lucene.search.SortField;
import org.apache.lucene.search.TermQuery; import org.apache.lucene.search.TermQuery;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource; import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource;
@ -245,7 +246,7 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
parser.nextToken(); parser.nextToken();
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()); assertEquals("[_script] script doesn't support values of type: START_ARRAY", e.getMessage());
} }

View File

@ -146,7 +146,7 @@ public class DirectCandidateGeneratorTests extends ESTestCase {
logger.info("Skipping test as it uses a custom duplicate check that is obsolete when strict duplicate checks are enabled."); logger.info("Skipping test as it uses a custom duplicate check that is obsolete when strict duplicate checks are enabled.");
} else { } else {
directGenerator = "{ \"field\" : \"f1\", \"field\" : \"f2\" }"; directGenerator = "{ \"field\" : \"f1\", \"field\" : \"f2\" }";
assertIllegalXContent(directGenerator, ParsingException.class, assertIllegalXContent(directGenerator, IllegalArgumentException.class,
"[direct_generator] failed to parse field [field]"); "[direct_generator] failed to parse field [field]");
} }
@ -162,7 +162,7 @@ public class DirectCandidateGeneratorTests extends ESTestCase {
// test unexpected token // test unexpected token
directGenerator = "{ \"size\" : [ \"xxl\" ] }"; directGenerator = "{ \"size\" : [ \"xxl\" ] }";
assertIllegalXContent(directGenerator, IllegalArgumentException.class, assertIllegalXContent(directGenerator, ParsingException.class,
"[direct_generator] size doesn't support values of type: START_ARRAY"); "[direct_generator] size doesn't support values of type: START_ARRAY");
} }