Browse Source

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
Lee Hinman 7 years ago
parent
commit
eebda6974d

+ 2 - 2
server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java

@@ -43,7 +43,7 @@ import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.xcontent.UnknownNamedObjectException;
+import org.elasticsearch.common.xcontent.NamedObjectNotFoundException;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -1173,7 +1173,7 @@ public class MetaData implements Iterable<IndexMetaData>, Diffable<MetaData>, To
                         try {
                             Custom custom = parser.namedObject(Custom.class, currentFieldName, null);
                             builder.putCustom(custom.getWriteableName(), custom);
-                        } catch (UnknownNamedObjectException ex) {
+                        } catch (NamedObjectNotFoundException ex) {
                             logger.warn("Skipping unknown custom object with type {}", currentFieldName);
                             parser.skipChildren();
                         }

+ 35 - 0
server/src/main/java/org/elasticsearch/common/xcontent/NamedObjectNotFoundException.java

@@ -0,0 +1,35 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.common.xcontent;
+
+/**
+ * Thrown when {@link NamedXContentRegistry} cannot locate a named object to
+ * parse for a particular name
+ */
+public class NamedObjectNotFoundException extends XContentParseException {
+
+    public NamedObjectNotFoundException(String message) {
+        this(null, message);
+    }
+
+    public NamedObjectNotFoundException(XContentLocation location, String message) {
+        super(location, message);
+    }
+}

+ 10 - 9
server/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java

@@ -19,10 +19,8 @@
 
 package org.elasticsearch.common.xcontent;
 
-import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.common.CheckedFunction;
 import org.elasticsearch.common.ParseField;
-import org.elasticsearch.common.ParsingException;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -114,28 +112,31 @@ public class NamedXContentRegistry {
     }
 
     /**
-     * Parse a named object, throwing an exception if the parser isn't found. Throws an {@link ElasticsearchException} if the
-     * {@code categoryClass} isn't registered because this is almost always a bug. Throws a {@link UnknownNamedObjectException} if the
+     * Parse a named object, throwing an exception if the parser isn't found. Throws an {@link NamedObjectNotFoundException} if the
+     * {@code categoryClass} isn't registered because this is almost always a bug. Throws an {@link NamedObjectNotFoundException} if the
      * {@code categoryClass} is registered but the {@code name} isn't.
+     *
+     * @throws NamedObjectNotFoundException if the categoryClass or name is not registered
      */
     public <T, C> T parseNamedObject(Class<T> categoryClass, String name, XContentParser parser, C context) throws IOException {
         Map<String, Entry> parsers = registry.get(categoryClass);
         if (parsers == null) {
             if (registry.isEmpty()) {
                 // The "empty" registry will never work so we throw a better exception as a hint.
-                throw new ElasticsearchException("namedObject is not supported for this parser");
+                throw new NamedObjectNotFoundException("named objects are not supported for this parser");
             }
-            throw new ElasticsearchException("Unknown namedObject category [" + categoryClass.getName() + "]");
+            throw new NamedObjectNotFoundException("unknown named object category [" + categoryClass.getName() + "]");
         }
         Entry entry = parsers.get(name);
         if (entry == null) {
-            throw new UnknownNamedObjectException(parser.getTokenLocation(), categoryClass, name);
+            throw new NamedObjectNotFoundException(parser.getTokenLocation(), "unable to parse " + categoryClass.getSimpleName() +
+                " with name [" + name + "]: parser not found");
         }
         if (false == entry.name.match(name, parser.getDeprecationHandler())) {
             /* Note that this shouldn't happen because we already looked up the entry using the names but we need to call `match` anyway
              * because it is responsible for logging deprecation warnings. */
-            throw new ParsingException(parser.getTokenLocation(),
-                    "Unknown " + categoryClass.getSimpleName() + " [" + name + "]: Parser didn't match");
+            throw new NamedObjectNotFoundException(parser.getTokenLocation(),
+                    "unable to parse " + categoryClass.getSimpleName() + " with name [" + name + "]: parser didn't match");
         }
         return categoryClass.cast(entry.parser.parse(parser, context));
     }

+ 52 - 0
server/src/main/java/org/elasticsearch/common/xcontent/XContentParseException.java

@@ -0,0 +1,52 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.common.xcontent;
+
+import java.util.Optional;
+
+/**
+ * Thrown when one of the XContent parsers cannot parse something.
+ */
+public class XContentParseException extends IllegalArgumentException {
+
+    private final Optional<XContentLocation> location;
+
+    public XContentParseException(String message) {
+        this(null, message);
+    }
+
+    public XContentParseException(XContentLocation location, String message) {
+        super(message);
+        this.location = Optional.ofNullable(location);
+    }
+
+    public int getLineNumber() {
+        return location.map(l -> l.lineNumber).orElse(-1);
+    }
+
+    public int getColumnNumber() {
+        return location.map(l -> l.columnNumber).orElse(-1);
+    }
+
+    @Override
+    public String getMessage() {
+        return location.map(l -> "[" + l.toString() + "] ").orElse("") + super.getMessage();
+    }
+}

+ 3 - 3
server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java

@@ -31,7 +31,7 @@ import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.lucene.BytesRefs;
 import org.elasticsearch.common.xcontent.AbstractObjectParser;
-import org.elasticsearch.common.xcontent.UnknownNamedObjectException;
+import org.elasticsearch.common.xcontent.NamedObjectNotFoundException;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentLocation;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -316,11 +316,11 @@ public abstract class AbstractQueryBuilder<QB extends AbstractQueryBuilder<QB>>
         QueryBuilder result;
         try {
             result = parser.namedObject(QueryBuilder.class, queryName, null);
-        } catch (UnknownNamedObjectException e) {
+        } catch (NamedObjectNotFoundException e) {
             // Preserve the error message from 5.0 until we have a compellingly better message so we don't break BWC.
             // This intentionally doesn't include the causing exception because that'd change the "root_cause" of any unknown query errors
             throw new ParsingException(new XContentLocation(e.getLineNumber(), e.getColumnNumber()),
-                    "no [query] registered for [" + e.getName() + "]");
+                    "no [query] registered for [" + queryName + "]");
         }
         //end_object of the specific query (e.g. match, multi_match etc.) element
         if (parser.currentToken() != XContentParser.Token.END_OBJECT) {

+ 7 - 8
server/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java

@@ -67,6 +67,7 @@ import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonMap;
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.notNullValue;
@@ -1007,22 +1008,20 @@ public abstract class BaseXContentTestCase extends ESTestCase {
         {
             p.nextToken();
             assertEquals("test", p.namedObject(Object.class, "str", null));
-            UnknownNamedObjectException e = expectThrows(UnknownNamedObjectException.class,
+            NamedObjectNotFoundException e = expectThrows(NamedObjectNotFoundException.class,
                     () -> p.namedObject(Object.class, "unknown", null));
-            assertEquals("Unknown Object [unknown]", e.getMessage());
-            assertEquals("java.lang.Object", e.getCategoryClass());
-            assertEquals("unknown", e.getName());
+            assertThat(e.getMessage(), endsWith("unable to parse Object with name [unknown]: parser not found"));
         }
         {
-            Exception e = expectThrows(ElasticsearchException.class, () -> p.namedObject(String.class, "doesn't matter", null));
-            assertEquals("Unknown namedObject category [java.lang.String]", e.getMessage());
+            Exception e = expectThrows(NamedObjectNotFoundException.class, () -> p.namedObject(String.class, "doesn't matter", null));
+            assertEquals("unknown named object category [java.lang.String]", e.getMessage());
         }
         {
             XContentParser emptyRegistryParser = xcontentType().xContent()
                 .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, new byte[] {});
-            Exception e = expectThrows(ElasticsearchException.class,
+            Exception e = expectThrows(NamedObjectNotFoundException.class,
                     () -> emptyRegistryParser.namedObject(String.class, "doesn't matter", null));
-            assertEquals("namedObject is not supported for this parser", e.getMessage());
+            assertEquals("named objects are not supported for this parser", e.getMessage());
         }
     }
 

+ 3 - 4
server/src/test/java/org/elasticsearch/common/xcontent/XContentParserUtilsTests.java

@@ -40,6 +40,7 @@ import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpect
 import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureFieldName;
 import static org.elasticsearch.common.xcontent.XContentParserUtils.parseTypedKeysObject;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.instanceOf;
 
 public class XContentParserUtilsTests extends ESTestCase {
@@ -187,11 +188,9 @@ public class XContentParserUtilsTests extends ESTestCase {
             ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
             ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.nextToken(), parser::getTokenLocation);
             ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
-            UnknownNamedObjectException e = expectThrows(UnknownNamedObjectException.class,
+            NamedObjectNotFoundException e = expectThrows(NamedObjectNotFoundException.class,
                     () -> parseTypedKeysObject(parser, delimiter, Boolean.class, a -> {}));
-            assertEquals("Unknown Boolean [type]", e.getMessage());
-            assertEquals("type", e.getName());
-            assertEquals("java.lang.Boolean", e.getCategoryClass());
+            assertThat(e.getMessage(), endsWith("unable to parse Boolean with name [type]: parser not found"));
         }
 
         final long longValue = randomLong();

+ 3 - 7
server/src/test/java/org/elasticsearch/search/rescore/QueryRescorerBuilderTests.java

@@ -19,15 +19,14 @@
 
 package org.elasticsearch.search.rescore;
 
-import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
-import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.NamedObjectNotFoundException;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -40,9 +39,7 @@ import org.elasticsearch.index.mapper.ContentPath;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.Mapper;
 import org.elasticsearch.index.mapper.TextFieldMapper;
-import org.elasticsearch.index.query.BoolQueryBuilder;
 import org.elasticsearch.index.query.MatchAllQueryBuilder;
-import org.elasticsearch.index.query.MatchQueryBuilder;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryRewriteContext;
 import org.elasticsearch.index.query.QueryShardContext;
@@ -58,7 +55,6 @@ import java.io.IOException;
 
 import static java.util.Collections.emptyList;
 import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
-import static org.hamcrest.Matchers.containsString;
 
 public class QueryRescorerBuilderTests extends ESTestCase {
 
@@ -220,8 +216,8 @@ public class QueryRescorerBuilderTests extends ESTestCase {
                 "}\n";
         {
             XContentParser parser = createParser(rescoreElement);
-            Exception e = expectThrows(ParsingException.class, () -> RescorerBuilder.parseFromXContent(parser));
-            assertEquals("Unknown RescorerBuilder [bad_rescorer_name]", e.getMessage());
+            Exception e = expectThrows(NamedObjectNotFoundException.class, () -> RescorerBuilder.parseFromXContent(parser));
+            assertEquals("[3:27] unable to parse RescorerBuilder with name [bad_rescorer_name]: parser not found", e.getMessage());
         }
 
         rescoreElement = "{\n" +

+ 3 - 3
server/src/test/java/org/elasticsearch/search/suggest/SuggestionTests.java

@@ -19,10 +19,10 @@
 
 package org.elasticsearch.search.suggest;
 
-import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.text.Text;
 import org.elasticsearch.common.xcontent.DeprecationHandler;
+import org.elasticsearch.common.xcontent.NamedObjectNotFoundException;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContent;
@@ -180,8 +180,8 @@ public class SuggestionTests extends ESTestCase {
             ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
             ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.nextToken(), parser::getTokenLocation);
             ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.nextToken(), parser::getTokenLocation);
-            ParsingException e = expectThrows(ParsingException.class, () -> Suggestion.fromXContent(parser));
-            assertEquals("Unknown Suggestion [unknownType]", e.getMessage());
+            NamedObjectNotFoundException e = expectThrows(NamedObjectNotFoundException.class, () -> Suggestion.fromXContent(parser));
+            assertEquals("[1:31] unable to parse Suggestion with name [unknownType]: parser not found", e.getMessage());
         }
     }