Skip to content

Commit adafb7e

Browse files
dakronedakrone
committed
Decouple NamedXContentRegistry from ElasticsearchException (#29253)
* Decouple NamedXContentRegistry from ElasticsearchException This commit decouples `NamedXContentRegistry` from using either `ElasticsearchException`, `ParsingException`, or `UnknownNamedObjectException`. This will allow us to move NamedXContentRegistry to its own lib as part of the xcontent extraction work. Relates to #28504
1 parent 89cd269 commit adafb7e

File tree

9 files changed

+118
-36
lines changed

9 files changed

+118
-36
lines changed

‎server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
import org.elasticsearch.common.settings.Setting;
4444
import org.elasticsearch.common.settings.Setting.Property;
4545
import org.elasticsearch.common.settings.Settings;
46-
import org.elasticsearch.common.xcontent.UnknownNamedObjectException;
46+
import org.elasticsearch.common.xcontent.NamedObjectNotFoundException;
4747
import org.elasticsearch.common.xcontent.ToXContent;
4848
import org.elasticsearch.common.xcontent.ToXContentFragment;
4949
import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -1173,7 +1173,7 @@ public static MetaData fromXContent(XContentParser parser) throws IOException {
11731173
try {
11741174
Custom custom = parser.namedObject(Custom.class, currentFieldName, null);
11751175
builder.putCustom(custom.getWriteableName(), custom);
1176-
} catch (UnknownNamedObjectException ex) {
1176+
} catch (NamedObjectNotFoundException ex) {
11771177
logger.warn("Skipping unknown custom object with type {}", currentFieldName);
11781178
parser.skipChildren();
11791179
}

‎server/src/main/java/org/elasticsearch/common/xcontent/NamedObjectNotFoundException.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.common.xcontent;
21+
22+
/**
23+
* Thrown when {@link NamedXContentRegistry} cannot locate a named object to
24+
* parse for a particular name
25+
*/
26+
public class NamedObjectNotFoundException extends XContentParseException {
27+
28+
public NamedObjectNotFoundException(String message) {
29+
this(null, message);
30+
}
31+
32+
public NamedObjectNotFoundException(XContentLocation location, String message) {
33+
super(location, message);
34+
}
35+
}

‎server/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,8 @@
1919

2020
package org.elasticsearch.common.xcontent;
2121

22-
import org.elasticsearch.ElasticsearchException;
2322
import org.elasticsearch.common.CheckedFunction;
2423
import org.elasticsearch.common.ParseField;
25-
import org.elasticsearch.common.ParsingException;
2624

2725
import java.io.IOException;
2826
import java.util.ArrayList;
@@ -114,28 +112,31 @@ public NamedXContentRegistry(List<Entry> entries) {
114112
}
115113

116114
/**
117-
* Parse a named object, throwing an exception if the parser isn't found. Throws an {@link ElasticsearchException} if the
118-
* {@code categoryClass} isn't registered because this is almost always a bug. Throws a {@link UnknownNamedObjectException} if the
115+
* Parse a named object, throwing an exception if the parser isn't found. Throws an {@link NamedObjectNotFoundException} if the
116+
* {@code categoryClass} isn't registered because this is almost always a bug. Throws an {@link NamedObjectNotFoundException} if the
119117
* {@code categoryClass} is registered but the {@code name} isn't.
118+
*
119+
* @throws NamedObjectNotFoundException if the categoryClass or name is not registered
120120
*/
121121
public <T, C> T parseNamedObject(Class<T> categoryClass, String name, XContentParser parser, C context) throws IOException {
122122
Map<String, Entry> parsers = registry.get(categoryClass);
123123
if (parsers == null) {
124124
if (registry.isEmpty()) {
125125
// The "empty" registry will never work so we throw a better exception as a hint.
126-
throw new ElasticsearchException("namedObject is not supported for this parser");
126+
throw new NamedObjectNotFoundException("named objects are not supported for this parser");
127127
}
128-
throw new ElasticsearchException("Unknown namedObject category [" + categoryClass.getName() + "]");
128+
throw new NamedObjectNotFoundException("unknown named object category [" + categoryClass.getName() + "]");
129129
}
130130
Entry entry = parsers.get(name);
131131
if (entry == null) {
132-
throw new UnknownNamedObjectException(parser.getTokenLocation(), categoryClass, name);
132+
throw new NamedObjectNotFoundException(parser.getTokenLocation(), "unable to parse " + categoryClass.getSimpleName() +
133+
" with name [" + name + "]: parser not found");
133134
}
134135
if (false == entry.name.match(name, parser.getDeprecationHandler())) {
135136
/* Note that this shouldn't happen because we already looked up the entry using the names but we need to call `match` anyway
136137
* because it is responsible for logging deprecation warnings. */
137-
throw new ParsingException(parser.getTokenLocation(),
138-
"Unknown " + categoryClass.getSimpleName() + " [" + name + "]: Parser didn't match");
138+
throw new NamedObjectNotFoundException(parser.getTokenLocation(),
139+
"unable to parse " + categoryClass.getSimpleName() + " with name [" + name + "]: parser didn't match");
139140
}
140141
return categoryClass.cast(entry.parser.parse(parser, context));
141142
}

‎server/src/main/java/org/elasticsearch/common/xcontent/XContentParseException.java

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.common.xcontent;
21+
22+
import java.util.Optional;
23+
24+
/**
25+
* Thrown when one of the XContent parsers cannot parse something.
26+
*/
27+
public class XContentParseException extends IllegalArgumentException {
28+
29+
private final Optional<XContentLocation> location;
30+
31+
public XContentParseException(String message) {
32+
this(null, message);
33+
}
34+
35+
public XContentParseException(XContentLocation location, String message) {
36+
super(message);
37+
this.location = Optional.ofNullable(location);
38+
}
39+
40+
public int getLineNumber() {
41+
return location.map(l -> l.lineNumber).orElse(-1);
42+
}
43+
44+
public int getColumnNumber() {
45+
return location.map(l -> l.columnNumber).orElse(-1);
46+
}
47+
48+
@Override
49+
public String getMessage() {
50+
return location.map(l -> "[" + l.toString() + "] ").orElse("") + super.getMessage();
51+
}
52+
}

‎server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
import org.elasticsearch.common.io.stream.StreamOutput;
3232
import org.elasticsearch.common.lucene.BytesRefs;
3333
import org.elasticsearch.common.xcontent.AbstractObjectParser;
34-
import org.elasticsearch.common.xcontent.UnknownNamedObjectException;
34+
import org.elasticsearch.common.xcontent.NamedObjectNotFoundException;
3535
import org.elasticsearch.common.xcontent.XContentBuilder;
3636
import org.elasticsearch.common.xcontent.XContentLocation;
3737
import org.elasticsearch.common.xcontent.XContentParser;
@@ -316,11 +316,11 @@ public static QueryBuilder parseInnerQueryBuilder(XContentParser parser) throws
316316
QueryBuilder result;
317317
try {
318318
result = parser.namedObject(QueryBuilder.class, queryName, null);
319-
} catch (UnknownNamedObjectException e) {
319+
} catch (NamedObjectNotFoundException e) {
320320
// Preserve the error message from 5.0 until we have a compellingly better message so we don't break BWC.
321321
// This intentionally doesn't include the causing exception because that'd change the "root_cause" of any unknown query errors
322322
throw new ParsingException(new XContentLocation(e.getLineNumber(), e.getColumnNumber()),
323-
"no [query] registered for [" + e.getName() + "]");
323+
"no [query] registered for [" + queryName + "]");
324324
}
325325
//end_object of the specific query (e.g. match, multi_match etc.) element
326326
if (parser.currentToken() != XContentParser.Token.END_OBJECT) {

‎server/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
import static java.util.Collections.singletonMap;
6868
import static org.hamcrest.Matchers.allOf;
6969
import static org.hamcrest.Matchers.containsString;
70+
import static org.hamcrest.Matchers.endsWith;
7071
import static org.hamcrest.Matchers.equalTo;
7172
import static org.hamcrest.Matchers.instanceOf;
7273
import static org.hamcrest.Matchers.notNullValue;
@@ -1007,22 +1008,20 @@ public void testNamedObject() throws IOException {
10071008
{
10081009
p.nextToken();
10091010
assertEquals("test", p.namedObject(Object.class, "str", null));
1010-
UnknownNamedObjectException e = expectThrows(UnknownNamedObjectException.class,
1011+
NamedObjectNotFoundException e = expectThrows(NamedObjectNotFoundException.class,
10111012
() -> p.namedObject(Object.class, "unknown", null));
1012-
assertEquals("Unknown Object [unknown]", e.getMessage());
1013-
assertEquals("java.lang.Object", e.getCategoryClass());
1014-
assertEquals("unknown", e.getName());
1013+
assertThat(e.getMessage(), endsWith("unable to parse Object with name [unknown]: parser not found"));
10151014
}
10161015
{
1017-
Exception e = expectThrows(ElasticsearchException.class, () -> p.namedObject(String.class, "doesn't matter", null));
1018-
assertEquals("Unknown namedObject category [java.lang.String]", e.getMessage());
1016+
Exception e = expectThrows(NamedObjectNotFoundException.class, () -> p.namedObject(String.class, "doesn't matter", null));
1017+
assertEquals("unknown named object category [java.lang.String]", e.getMessage());
10191018
}
10201019
{
10211020
XContentParser emptyRegistryParser = xcontentType().xContent()
10221021
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, new byte[] {});
1023-
Exception e = expectThrows(ElasticsearchException.class,
1022+
Exception e = expectThrows(NamedObjectNotFoundException.class,
10241023
() -> emptyRegistryParser.namedObject(String.class, "doesn't matter", null));
1025-
assertEquals("namedObject is not supported for this parser", e.getMessage());
1024+
assertEquals("named objects are not supported for this parser", e.getMessage());
10261025
}
10271026
}
10281027

‎server/src/test/java/org/elasticsearch/common/xcontent/XContentParserUtilsTests.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureFieldName;
4141
import static org.elasticsearch.common.xcontent.XContentParserUtils.parseTypedKeysObject;
4242
import static org.hamcrest.Matchers.containsString;
43+
import static org.hamcrest.Matchers.endsWith;
4344
import static org.hamcrest.Matchers.instanceOf;
4445

4546
public class XContentParserUtilsTests extends ESTestCase {
@@ -187,11 +188,9 @@ public void testParseTypedKeysObject() throws IOException {
187188
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
188189
ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.nextToken(), parser::getTokenLocation);
189190
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
190-
UnknownNamedObjectException e = expectThrows(UnknownNamedObjectException.class,
191+
NamedObjectNotFoundException e = expectThrows(NamedObjectNotFoundException.class,
191192
() -> parseTypedKeysObject(parser, delimiter, Boolean.class, a -> {}));
192-
assertEquals("Unknown Boolean [type]", e.getMessage());
193-
assertEquals("type", e.getName());
194-
assertEquals("java.lang.Boolean", e.getCategoryClass());
193+
assertThat(e.getMessage(), endsWith("unable to parse Boolean with name [type]: parser not found"));
195194
}
196195

197196
final long longValue = randomLong();

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,14 @@
1919

2020
package org.elasticsearch.search.rescore;
2121

22-
import org.apache.lucene.search.MatchAllDocsQuery;
2322
import org.apache.lucene.search.Query;
2423
import org.elasticsearch.ElasticsearchParseException;
2524
import org.elasticsearch.Version;
2625
import org.elasticsearch.cluster.metadata.IndexMetaData;
2726
import org.elasticsearch.common.ParsingException;
2827
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
29-
import org.elasticsearch.common.io.stream.StreamOutput;
3028
import org.elasticsearch.common.settings.Settings;
29+
import org.elasticsearch.common.xcontent.NamedObjectNotFoundException;
3130
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
3231
import org.elasticsearch.common.xcontent.ToXContent;
3332
import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -40,9 +39,7 @@
4039
import org.elasticsearch.index.mapper.MappedFieldType;
4140
import org.elasticsearch.index.mapper.Mapper;
4241
import org.elasticsearch.index.mapper.TextFieldMapper;
43-
import org.elasticsearch.index.query.BoolQueryBuilder;
4442
import org.elasticsearch.index.query.MatchAllQueryBuilder;
45-
import org.elasticsearch.index.query.MatchQueryBuilder;
4643
import org.elasticsearch.index.query.QueryBuilder;
4744
import org.elasticsearch.index.query.QueryRewriteContext;
4845
import org.elasticsearch.index.query.QueryShardContext;
@@ -58,7 +55,6 @@
5855

5956
import static java.util.Collections.emptyList;
6057
import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
61-
import static org.hamcrest.Matchers.containsString;
6258

6359
public class QueryRescorerBuilderTests extends ESTestCase {
6460

@@ -220,8 +216,8 @@ public void testUnknownFieldsExpection() throws IOException {
220216
"}\n";
221217
{
222218
XContentParser parser = createParser(rescoreElement);
223-
Exception e = expectThrows(ParsingException.class, () -> RescorerBuilder.parseFromXContent(parser));
224-
assertEquals("Unknown RescorerBuilder [bad_rescorer_name]", e.getMessage());
219+
Exception e = expectThrows(NamedObjectNotFoundException.class, () -> RescorerBuilder.parseFromXContent(parser));
220+
assertEquals("[3:27] unable to parse RescorerBuilder with name [bad_rescorer_name]: parser not found", e.getMessage());
225221
}
226222

227223
rescoreElement = "{\n" +

‎server/src/test/java/org/elasticsearch/search/suggest/SuggestionTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@
1919

2020
package org.elasticsearch.search.suggest;
2121

22-
import org.elasticsearch.common.ParsingException;
2322
import org.elasticsearch.common.bytes.BytesReference;
2423
import org.elasticsearch.common.text.Text;
2524
import org.elasticsearch.common.xcontent.DeprecationHandler;
25+
import org.elasticsearch.common.xcontent.NamedObjectNotFoundException;
2626
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
2727
import org.elasticsearch.common.xcontent.ToXContent;
2828
import org.elasticsearch.common.xcontent.XContent;
@@ -180,8 +180,8 @@ public void testUnknownSuggestionTypeThrows() throws IOException {
180180
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
181181
ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.nextToken(), parser::getTokenLocation);
182182
ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.nextToken(), parser::getTokenLocation);
183-
ParsingException e = expectThrows(ParsingException.class, () -> Suggestion.fromXContent(parser));
184-
assertEquals("Unknown Suggestion [unknownType]", e.getMessage());
183+
NamedObjectNotFoundException e = expectThrows(NamedObjectNotFoundException.class, () -> Suggestion.fromXContent(parser));
184+
assertEquals("[1:31] unable to parse Suggestion with name [unknownType]: parser not found", e.getMessage());
185185
}
186186
}
187187

0 commit comments

Comments
 (0)