Use XMLStreamException for reporting parsing errors 95/81495/3
authorRobert Varga <robert.varga@pantheon.tech>
Tue, 9 Apr 2019 17:16:37 +0000 (19:16 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Tue, 9 Apr 2019 18:56:13 +0000 (20:56 +0200)
Using IllegalStateException is pure evil, as users do not expect
a RuntimeException to be thrown from contexts which can clearly
report a checked exception.

Use a properly-formatted XMLStreamException instead.

Change-Id: Iecee17e19b3ac380bcb2b7fc1c46f27e789c20bd
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
yang/yang-data-codec-xml/src/main/java/org/opendaylight/yangtools/yang/data/codec/xml/XmlParserStream.java
yang/yang-data-codec-xml/src/test/java/org/opendaylight/yangtools/yang/data/codec/xml/StrictParsingModeTest.java
yang/yang-data-codec-xml/src/test/java/org/opendaylight/yangtools/yang/data/codec/xml/XmlToNormalizedNodesTest.java

index 471f29ba300c4dc4dc6b3f5940c3d68c6a23f851..c613ce7e9c8c4ae77c838fa600a351334059f536 100644 (file)
@@ -30,7 +30,6 @@ import java.util.Optional;
 import java.util.Set;
 import javax.xml.XMLConstants;
 import javax.xml.namespace.NamespaceContext;
-import javax.xml.stream.Location;
 import javax.xml.stream.XMLStreamConstants;
 import javax.xml.stream.XMLStreamException;
 import javax.xml.stream.XMLStreamReader;
@@ -413,10 +412,9 @@ public final class XmlParserStream implements Closeable, Flushable {
 
                     final String xmlElementNamespace = in.getNamespaceURI();
                     if (!namesakes.add(new SimpleImmutableEntry<>(xmlElementNamespace, xmlElementName))) {
-                        final Location loc = in.getLocation();
-                        throw new IllegalStateException(String.format(
-                                "Duplicate namespace \"%s\" element \"%s\" in XML input at: line %s column %s",
-                                xmlElementNamespace, xmlElementName, loc.getLineNumber(), loc.getColumnNumber()));
+                        throw new XMLStreamException(String.format(
+                            "Duplicate namespace \"%s\" element \"%s\" in XML input", xmlElementNamespace,
+                            xmlElementName), in.getLocation());
                     }
 
                     final Deque<DataSchemaNode> childDataSchemaNodes =
@@ -424,10 +422,16 @@ public final class XmlParserStream implements Closeable, Flushable {
                                     new URI(xmlElementNamespace));
 
                     if (childDataSchemaNodes.isEmpty()) {
-                        checkState(!strictParsing, "Schema for node with name %s and namespace %s does not exist at %s",
-                            xmlElementName, xmlElementNamespace, parentSchema.getPath());
-                        skipUnknownNode(in);
-                        continue;
+                        if (!strictParsing) {
+                            LOG.debug("Skipping unknown node ns=\"{}\" localName=\"{}\" at path {}",
+                                xmlElementNamespace, xmlElementName, parentSchema.getPath());
+                            skipUnknownNode(in);
+                            continue;
+                        }
+
+                        throw new XMLStreamException(String.format(
+                            "Schema for node with name %s and namespace %s does not exist at %s",
+                            xmlElementName, xmlElementNamespace, parentSchema.getPath(), in.getLocation()));
                     }
 
                     read(in, ((CompositeNodeDataWithSchema<?>) parent).addChild(childDataSchemaNodes), rootElement);
index e49aad3aad982bba67b015d4db09d105462ce96d..3d425eb4605663e845bf4dc1a325a191aa464748 100644 (file)
@@ -13,6 +13,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.fail;
 
 import java.io.InputStream;
+import javax.xml.stream.XMLStreamException;
 import javax.xml.stream.XMLStreamReader;
 import org.junit.Test;
 import org.opendaylight.yangtools.util.xml.UntrustedXML;
@@ -73,8 +74,8 @@ public class StrictParsingModeTest {
         final XmlParserStream xmlParser = XmlParserStream.create(streamWriter, schemaContext, topLevelContainer, true);
         try {
             xmlParser.parse(reader);
-            fail("IllegalStateException should have been thrown because of an unknown child node.");
-        } catch (IllegalStateException ex) {
+            fail("XMLStreamException should have been thrown because of an unknown child node.");
+        } catch (XMLStreamException ex) {
             assertEquals("Schema for node with name unknown-container-a and namespace foo does not exist at "
                     + "AbsoluteSchemaPath{path=[(foo)top-level-container]}", ex.getMessage());
         }
index 9947379fa9875af74f1bb4b3d8c3115a323c8ad4..9915ba7152414dca170878050f9fceb33a0cf71a 100644 (file)
@@ -9,7 +9,6 @@
 package org.opendaylight.yangtools.yang.data.codec.xml;
 
 import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.CoreMatchers.startsWith;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
@@ -157,10 +156,10 @@ public class XmlToNormalizedNodesTest {
         final XmlParserStream xmlParser = XmlParserStream.create(streamWriter, schemaContext, parentContainerSchema);
         try {
             xmlParser.parse(reader);
-            fail("IllegalStateException should have been thrown because of duplicate leaf.");
-        } catch (IllegalStateException ex) {
-            assertThat(ex.getMessage(), startsWith("Duplicate namespace \"foo-namespace\" element \"decimal64-leaf\" "
-                    + "in XML input at: line 7 column "));
+            fail("XMLStreamException should have been thrown because of duplicate leaf.");
+        } catch (XMLStreamException ex) {
+            assertThat(ex.getMessage(), containsString("Duplicate namespace \"foo-namespace\" element "
+                    + "\"decimal64-leaf\" in XML input"));
         }
     }
 
@@ -177,10 +176,9 @@ public class XmlToNormalizedNodesTest {
         final XmlParserStream xmlParser = XmlParserStream.create(streamWriter, schemaContext, parentContainerSchema);
         try {
             xmlParser.parse(reader);
-            fail("IllegalStateException should have been thrown because of duplicate anyxml");
-        } catch (IllegalStateException ex) {
-            assertThat(ex.getMessage(), startsWith("Duplicate namespace \"foo-namespace\" element \"my-anyxml\" in XML "
-                    + "input at: line 19 column "));
+            fail("XMLStreamException should have been thrown because of duplicate anyxml");
+        } catch (XMLStreamException ex) {
+            assertThat(ex.getMessage(), containsString("Duplicate namespace \"foo-namespace\" element \"my-anyxml\""));
         }
     }
 
@@ -197,10 +195,10 @@ public class XmlToNormalizedNodesTest {
         final XmlParserStream xmlParser = XmlParserStream.create(streamWriter, schemaContext, parentContainerSchema);
         try {
             xmlParser.parse(reader);
-            fail("IllegalStateException should have been thrown because of duplicate container");
-        } catch (IllegalStateException ex) {
-            assertThat(ex.getMessage(), startsWith("Duplicate namespace \"foo-namespace\" element \"leaf-container\" "
-                + "in XML input at: line 13 column "));
+            fail("XMLStreamException should have been thrown because of duplicate container");
+        } catch (XMLStreamException ex) {
+            assertThat(ex.getMessage(), containsString("Duplicate namespace \"foo-namespace\" element "
+                    + "\"leaf-container\" in XML input"));
         }
     }
 
@@ -274,8 +272,8 @@ public class XmlToNormalizedNodesTest {
         final XmlParserStream xmlParser = XmlParserStream.create(streamWriter, schemaContext, outerContainerSchema);
         try {
             xmlParser.parse(reader);
-            fail("IllegalStateException should have been thrown because of an unknown child node.");
-        } catch (IllegalStateException ex) {
+            fail("XMLStreamException should have been thrown because of an unknown child node.");
+        } catch (XMLStreamException ex) {
             assertEquals("Schema for node with name my-container-1 and namespace baz-namespace does not exist at "
                     + "AbsoluteSchemaPath{path=[(baz-namespace)outer-container, (baz-namespace)my-container-1]}",
                     ex.getMessage());