Skip to content

Commit 3bc5248

Browse files
author
Christoph Büscher
committed
Change ObjectParser exception (#31030)
ObjectParser should throw XContentParseExceptions, not IAE. A dedicated parsing exception can includes the place where the error occurred. Closes #30605
1 parent 11b001d commit 3bc5248

File tree

17 files changed

+105
-78
lines changed

17 files changed

+105
-78
lines changed

‎libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ public Value parse(XContentParser parser, Value value, Context context) throws I
155155
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
156156
if (token == XContentParser.Token.FIELD_NAME) {
157157
currentFieldName = parser.currentName();
158-
fieldParser = getParser(currentFieldName);
158+
fieldParser = getParser(currentFieldName, parser);
159159
} else {
160160
if (currentFieldName == null) {
161161
throw new XContentParseException(parser.getTokenLocation(), "[" + name + "] no field found");
@@ -341,10 +341,11 @@ private void parseSub(XContentParser parser, FieldParser fieldParser, String cur
341341
}
342342
}
343343

344-
private FieldParser getParser(String fieldName) {
344+
private FieldParser getParser(String fieldName, XContentParser xContentParser) {
345345
FieldParser parser = fieldParserMap.get(fieldName);
346346
if (parser == null && false == ignoreUnknownFields) {
347-
throw new IllegalArgumentException("[" + name + "] unknown field [" + fieldName + "], parser not found");
347+
throw new XContentParseException(xContentParser.getTokenLocation(),
348+
"[" + name + "] unknown field [" + fieldName + "], parser not found");
348349
}
349350
return parser;
350351
}

‎libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import java.util.Collections;
3636
import java.util.List;
3737

38-
import static org.hamcrest.CoreMatchers.startsWith;
3938
import static org.hamcrest.Matchers.containsString;
4039
import static org.hamcrest.Matchers.hasSize;
4140

@@ -186,7 +185,6 @@ public URI parseURI(XContentParser parser) {
186185
}
187186

188187
public void testExceptions() throws IOException {
189-
XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"test\" : \"foo\"}");
190188
class TestStruct {
191189
public void setTest(int test) {
192190
}
@@ -195,20 +193,16 @@ public void setTest(int test) {
195193
TestStruct s = new TestStruct();
196194
objectParser.declareInt(TestStruct::setTest, new ParseField("test"));
197195

198-
try {
199-
objectParser.parse(parser, s, null);
200-
fail("numeric value expected");
201-
} catch (XContentParseException ex) {
196+
{
197+
XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"test\" : \"foo\"}");
198+
XContentParseException ex = expectThrows(XContentParseException.class, () -> objectParser.parse(parser, s, null));
202199
assertThat(ex.getMessage(), containsString("[the_parser] failed to parse field [test]"));
203200
assertTrue(ex.getCause() instanceof NumberFormatException);
204201
}
205-
206-
parser = createParser(JsonXContent.jsonXContent, "{\"not_supported_field\" : \"foo\"}");
207-
try {
208-
objectParser.parse(parser, s, null);
209-
fail("field not supported");
210-
} catch (IllegalArgumentException ex) {
211-
assertEquals(ex.getMessage(), "[the_parser] unknown field [not_supported_field], parser not found");
202+
{
203+
XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"not_supported_field\" : \"foo\"}");
204+
XContentParseException ex = expectThrows(XContentParseException.class, () -> objectParser.parse(parser, s, null));
205+
assertEquals(ex.getMessage(), "[1:2] [the_parser] unknown field [not_supported_field], parser not found");
212206
}
213207
}
214208

‎modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainTests.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.elasticsearch.common.xcontent.ToXContent;
2626
import org.elasticsearch.common.xcontent.XContentBuilder;
2727
import org.elasticsearch.common.xcontent.XContentFactory;
28+
import org.elasticsearch.common.xcontent.XContentParseException;
2829
import org.elasticsearch.common.xcontent.XContentParser;
2930
import org.elasticsearch.common.xcontent.XContentType;
3031
import org.elasticsearch.common.xcontent.json.JsonXContent;
@@ -41,7 +42,7 @@
4142
import static org.elasticsearch.index.rankeval.EvaluationMetric.filterUnknownDocuments;
4243
import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
4344
import static org.elasticsearch.test.XContentTestUtils.insertRandomFields;
44-
import static org.hamcrest.Matchers.startsWith;
45+
import static org.hamcrest.CoreMatchers.containsString;
4546

4647
public class DiscountedCumulativeGainTests extends ESTestCase {
4748

@@ -280,9 +281,9 @@ public void testXContentParsingIsNotLenient() throws IOException {
280281
try (XContentParser parser = createParser(xContentType.xContent(), withRandomFields)) {
281282
parser.nextToken();
282283
parser.nextToken();
283-
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class,
284+
XContentParseException exception = expectThrows(XContentParseException.class,
284285
() -> DiscountedCumulativeGain.fromXContent(parser));
285-
assertThat(exception.getMessage(), startsWith("[dcg_at] unknown field"));
286+
assertThat(exception.getMessage(), containsString("[dcg_at] unknown field"));
286287
}
287288
}
288289

‎modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/MeanReciprocalRankTests.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.elasticsearch.common.xcontent.ToXContent;
2626
import org.elasticsearch.common.xcontent.XContentBuilder;
2727
import org.elasticsearch.common.xcontent.XContentFactory;
28+
import org.elasticsearch.common.xcontent.XContentParseException;
2829
import org.elasticsearch.common.xcontent.XContentParser;
2930
import org.elasticsearch.common.xcontent.XContentType;
3031
import org.elasticsearch.common.xcontent.json.JsonXContent;
@@ -41,7 +42,7 @@
4142

4243
import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
4344
import static org.elasticsearch.test.XContentTestUtils.insertRandomFields;
44-
import static org.hamcrest.Matchers.startsWith;
45+
import static org.hamcrest.CoreMatchers.containsString;
4546

4647
public class MeanReciprocalRankTests extends ESTestCase {
4748

@@ -189,9 +190,9 @@ public void testXContentParsingIsNotLenient() throws IOException {
189190
try (XContentParser parser = createParser(xContentType.xContent(), withRandomFields)) {
190191
parser.nextToken();
191192
parser.nextToken();
192-
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class,
193+
XContentParseException exception = expectThrows(XContentParseException.class,
193194
() -> MeanReciprocalRank.fromXContent(parser));
194-
assertThat(exception.getMessage(), startsWith("[reciprocal_rank] unknown field"));
195+
assertThat(exception.getMessage(), containsString("[reciprocal_rank] unknown field"));
195196
}
196197
}
197198

‎modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtKTests.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.elasticsearch.common.xcontent.ToXContent;
2626
import org.elasticsearch.common.xcontent.XContentBuilder;
2727
import org.elasticsearch.common.xcontent.XContentFactory;
28+
import org.elasticsearch.common.xcontent.XContentParseException;
2829
import org.elasticsearch.common.xcontent.XContentParser;
2930
import org.elasticsearch.common.xcontent.XContentType;
3031
import org.elasticsearch.common.xcontent.json.JsonXContent;
@@ -41,7 +42,7 @@
4142

4243
import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
4344
import static org.elasticsearch.test.XContentTestUtils.insertRandomFields;
44-
import static org.hamcrest.Matchers.startsWith;
45+
import static org.hamcrest.CoreMatchers.containsString;
4546

4647
public class PrecisionAtKTests extends ESTestCase {
4748

@@ -203,8 +204,8 @@ public void testXContentParsingIsNotLenient() throws IOException {
203204
try (XContentParser parser = createParser(xContentType.xContent(), withRandomFields)) {
204205
parser.nextToken();
205206
parser.nextToken();
206-
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> PrecisionAtK.fromXContent(parser));
207-
assertThat(exception.getMessage(), startsWith("[precision] unknown field"));
207+
XContentParseException exception = expectThrows(XContentParseException.class, () -> PrecisionAtK.fromXContent(parser));
208+
assertThat(exception.getMessage(), containsString("[precision] unknown field"));
208209
}
209210
}
210211

‎modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentTests.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.common.xcontent.ToXContent;
2525
import org.elasticsearch.common.xcontent.XContentBuilder;
2626
import org.elasticsearch.common.xcontent.XContentFactory;
27+
import org.elasticsearch.common.xcontent.XContentParseException;
2728
import org.elasticsearch.common.xcontent.XContentParser;
2829
import org.elasticsearch.common.xcontent.XContentType;
2930
import org.elasticsearch.test.ESTestCase;
@@ -33,7 +34,7 @@
3334

3435
import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
3536
import static org.elasticsearch.test.XContentTestUtils.insertRandomFields;
36-
import static org.hamcrest.Matchers.startsWith;
37+
import static org.hamcrest.CoreMatchers.containsString;
3738

3839
public class RatedDocumentTests extends ESTestCase {
3940

@@ -59,8 +60,8 @@ public void testXContentParsingIsNotLenient() throws IOException {
5960
BytesReference originalBytes = toShuffledXContent(testItem, xContentType, ToXContent.EMPTY_PARAMS, randomBoolean());
6061
BytesReference withRandomFields = insertRandomFields(xContentType, originalBytes, null, random());
6162
try (XContentParser parser = createParser(xContentType.xContent(), withRandomFields)) {
62-
Exception exception = expectThrows(IllegalArgumentException.class, () -> RatedDocument.fromXContent(parser));
63-
assertThat(exception.getMessage(), startsWith("[rated_document] unknown field"));
63+
XContentParseException exception = expectThrows(XContentParseException.class, () -> RatedDocument.fromXContent(parser));
64+
assertThat(exception.getMessage(), containsString("[rated_document] unknown field"));
6465
}
6566
}
6667

‎server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequestTests.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import org.elasticsearch.common.bytes.BytesReference;
2323
import org.elasticsearch.common.xcontent.ToXContent;
24+
import org.elasticsearch.common.xcontent.XContentParseException;
2425
import org.elasticsearch.common.xcontent.XContentParser;
2526
import org.elasticsearch.common.xcontent.XContentType;
2627
import org.elasticsearch.test.ESTestCase;
@@ -29,7 +30,8 @@
2930
import java.io.IOException;
3031
import java.util.Collections;
3132

32-
import static org.hamcrest.CoreMatchers.equalTo;
33+
import static org.hamcrest.CoreMatchers.containsString;
34+
import static org.hamcrest.CoreMatchers.equalTo;;
3335

3436
public class ClusterUpdateSettingsRequestTests extends ESTestCase {
3537

@@ -51,10 +53,10 @@ private void doFromXContentTestWithRandomFields(boolean addRandomFields) throws
5153
String unsupportedField = "unsupported_field";
5254
BytesReference mutated = BytesReference.bytes(XContentTestUtils.insertIntoXContent(xContentType.xContent(), originalBytes,
5355
Collections.singletonList(""), () -> unsupportedField, () -> randomAlphaOfLengthBetween(3, 10)));
54-
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
56+
XContentParseException iae = expectThrows(XContentParseException.class,
5557
() -> ClusterUpdateSettingsRequest.fromXContent(createParser(xContentType.xContent(), mutated)));
5658
assertThat(iae.getMessage(),
57-
equalTo("[cluster_update_settings_request] unknown field [" + unsupportedField + "], parser not found"));
59+
containsString("[cluster_update_settings_request] unknown field [" + unsupportedField + "], parser not found"));
5860
} else {
5961
XContentParser parser = createParser(xContentType.xContent(), originalBytes);
6062
ClusterUpdateSettingsRequest parsedRequest = ClusterUpdateSettingsRequest.fromXContent(parser);

‎server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.elasticsearch.common.xcontent.XContentBuilder;
3434
import org.elasticsearch.common.xcontent.XContentFactory;
3535
import org.elasticsearch.common.xcontent.XContentHelper;
36+
import org.elasticsearch.common.xcontent.XContentParseException;
3637
import org.elasticsearch.common.xcontent.XContentParser;
3738
import org.elasticsearch.common.xcontent.XContentType;
3839
import org.elasticsearch.common.xcontent.json.JsonXContent;
@@ -71,6 +72,7 @@ public class UpdateRequestTests extends ESTestCase {
7172

7273
private UpdateHelper updateHelper;
7374

75+
@Override
7476
@Before
7577
public void setUp() throws Exception {
7678
super.setUp();
@@ -290,6 +292,28 @@ public void testFieldsParsing() throws Exception {
290292
assertThat(request.fields(), arrayContaining("field1", "field2"));
291293
}
292294

295+
public void testUnknownFieldParsing() throws Exception {
296+
UpdateRequest request = new UpdateRequest("test", "type", "1");
297+
XContentParser contentParser = createParser(XContentFactory.jsonBuilder()
298+
.startObject()
299+
.field("unknown_field", "test")
300+
.endObject());
301+
302+
XContentParseException ex = expectThrows(XContentParseException.class, () -> request.fromXContent(contentParser));
303+
assertEquals("[1:2] [UpdateRequest] unknown field [unknown_field], parser not found", ex.getMessage());
304+
305+
UpdateRequest request2 = new UpdateRequest("test", "type", "1");
306+
XContentParser unknownObject = createParser(XContentFactory.jsonBuilder()
307+
.startObject()
308+
.field("script", "ctx.op = ctx._source.views == params.count ? 'delete' : 'none'")
309+
.startObject("params")
310+
.field("count", 1)
311+
.endObject()
312+
.endObject());
313+
ex = expectThrows(XContentParseException.class, () -> request2.fromXContent(unknownObject));
314+
assertEquals("[1:76] [UpdateRequest] unknown field [params], parser not found", ex.getMessage());
315+
}
316+
293317
public void testFetchSourceParsing() throws Exception {
294318
UpdateRequest request = new UpdateRequest("test", "type1", "1");
295319
request.fromXContent(createParser(XContentFactory.jsonBuilder()

‎server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -158,10 +158,10 @@ public void testFromXContent() throws IOException {
158158
*/
159159
public void testUnknownArrayNameExpection() throws IOException {
160160
{
161-
IllegalArgumentException e = expectParseThrows(IllegalArgumentException.class, "{\n" +
161+
XContentParseException e = expectParseThrows(XContentParseException.class, "{\n" +
162162
" \"bad_fieldname\" : [ \"field1\" 1 \"field2\" ]\n" +
163163
"}\n");
164-
assertEquals("[highlight] unknown field [bad_fieldname], parser not found", e.getMessage());
164+
assertEquals("[2:5] [highlight] unknown field [bad_fieldname], parser not found", e.getMessage());
165165
}
166166

167167
{
@@ -174,7 +174,7 @@ public void testUnknownArrayNameExpection() throws IOException {
174174
"}\n");
175175
assertThat(e.getMessage(), containsString("[highlight] failed to parse field [fields]"));
176176
assertThat(e.getCause().getMessage(), containsString("[fields] failed to parse field [body]"));
177-
assertEquals("[highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage());
177+
assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage());
178178
}
179179
}
180180

@@ -188,10 +188,10 @@ private <T extends Throwable> T expectParseThrows(Class<T> exceptionClass, Strin
188188
*/
189189
public void testUnknownFieldnameExpection() throws IOException {
190190
{
191-
IllegalArgumentException e = expectParseThrows(IllegalArgumentException.class, "{\n" +
191+
XContentParseException e = expectParseThrows(XContentParseException.class, "{\n" +
192192
" \"bad_fieldname\" : \"value\"\n" +
193193
"}\n");
194-
assertEquals("[highlight] unknown field [bad_fieldname], parser not found", e.getMessage());
194+
assertEquals("[2:5] [highlight] unknown field [bad_fieldname], parser not found", e.getMessage());
195195
}
196196

197197
{
@@ -204,7 +204,7 @@ public void testUnknownFieldnameExpection() throws IOException {
204204
"}\n");
205205
assertThat(e.getMessage(), containsString("[highlight] failed to parse field [fields]"));
206206
assertThat(e.getCause().getMessage(), containsString("[fields] failed to parse field [body]"));
207-
assertEquals("[highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage());
207+
assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage());
208208
}
209209
}
210210

@@ -213,10 +213,10 @@ public void testUnknownFieldnameExpection() throws IOException {
213213
*/
214214
public void testUnknownObjectFieldnameExpection() throws IOException {
215215
{
216-
IllegalArgumentException e = expectParseThrows(IllegalArgumentException.class, "{\n" +
216+
XContentParseException e = expectParseThrows(XContentParseException.class, "{\n" +
217217
" \"bad_fieldname\" : { \"field\" : \"value\" }\n \n" +
218218
"}\n");
219-
assertEquals("[highlight] unknown field [bad_fieldname], parser not found", e.getMessage());
219+
assertEquals("[2:5] [highlight] unknown field [bad_fieldname], parser not found", e.getMessage());
220220
}
221221

222222
{
@@ -229,7 +229,7 @@ public void testUnknownObjectFieldnameExpection() throws IOException {
229229
"}\n");
230230
assertThat(e.getMessage(), containsString("[highlight] failed to parse field [fields]"));
231231
assertThat(e.getCause().getMessage(), containsString("[fields] failed to parse field [body]"));
232-
assertEquals("[highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage());
232+
assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage());
233233
}
234234
}
235235

‎server/src/test/java/org/elasticsearch/search/rescore/QueryRescorerBuilderTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ public void testRescoreQueryNull() throws IOException {
170170

171171
class AlwaysRewriteQueryBuilder extends MatchAllQueryBuilder {
172172

173+
@Override
173174
protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) throws IOException {
174175
return new MatchAllQueryBuilder();
175176
}
@@ -254,8 +255,8 @@ public void testUnknownFieldsExpection() throws IOException {
254255
"}\n";
255256
{
256257
XContentParser parser = createParser(rescoreElement);
257-
Exception e = expectThrows(IllegalArgumentException.class, () -> RescorerBuilder.parseFromXContent(parser));
258-
assertEquals("[query] unknown field [bad_fieldname], parser not found", e.getMessage());
258+
XContentParseException e = expectThrows(XContentParseException.class, () -> RescorerBuilder.parseFromXContent(parser));
259+
assertEquals("[3:17] [query] unknown field [bad_fieldname], parser not found", e.getMessage());
259260
}
260261

261262
rescoreElement = "{\n" +

0 commit comments

Comments
 (0)