Improve SchemaInferenceStack error reporting

Include a description of parent's identity when we fail to locate
its child. This makes it easier to understand what is going on in most
failure situations.

The message now ends with something like
- not present in schema parent (namespace)parentName
- not present in grouping (namespace)groupingName
- not present in module (namespace)moduleName
or similar.

Also expand the test suite to cover negative scenarios around
enterChoice().

JIRA: YANGTOOLS-1336
Change-Id: I8a642dcd0fe5705f4c33c1a3e9d779c6db91ef24
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit b13eeb7d98bd4cc1374604a6047e78ed70efd04b)

diff --git a/model/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/SchemaInferenceStack.java b/model/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/SchemaInferenceStack.java
index 21813d349fa4a8ad4676d49bde43fdad63a71a36..c14e2db81dd07a4ddb4e9ad528585ac83a9f81fd 100644 (file)
--- a/model/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/SchemaInferenceStack.java
+++ b/model/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/SchemaInferenceStack.java
@@ -381,22 +381,27 @@ public final class SchemaInferenceStack implements Mutable, EffectiveModelContex
      * @throws IllegalArgumentException if the corresponding choice cannot be found
      */
     public @NonNull ChoiceEffectiveStatement enterChoice(final QName nodeIdentifier) {
+        final QName nodeId = requireNonNull(nodeIdentifier);
         final EffectiveStatement<?, ?> parent = deque.peek();
         if (parent instanceof ChoiceEffectiveStatement) {
-            return enterChoice((ChoiceEffectiveStatement) parent, nodeIdentifier);
+            return enterChoice((ChoiceEffectiveStatement) parent, nodeId);
         }
 
         // Fall back to schema tree lookup. Note if it results in non-choice, we rewind before reporting an error
-        final SchemaTreeEffectiveStatement<?> result = enterSchemaTree(nodeIdentifier);
+        final SchemaTreeEffectiveStatement<?> result = enterSchemaTree(nodeId);
         if (result instanceof ChoiceEffectiveStatement) {
             return (ChoiceEffectiveStatement) result;
         }
         exit();
-        throw new IllegalArgumentException("Choice " + nodeIdentifier + " not present");
+
+        if (parent != null) {
+            throw notPresent(parent, "Choice", nodeId);
+        }
+        throw new IllegalArgumentException("Choice " + nodeId + " not present");
     }
 
     // choice -> choice transition, we have to deal with intermediate case nodes
-    private @NonNull ChoiceEffectiveStatement enterChoice(final ChoiceEffectiveStatement parent,
+    private @NonNull ChoiceEffectiveStatement enterChoice(final @NonNull ChoiceEffectiveStatement parent,
             final QName nodeIdentifier) {
         for (EffectiveStatement<?, ?> stmt : parent.effectiveSubstatements()) {
             if (stmt instanceof CaseEffectiveStatement) {
@@ -412,7 +417,7 @@ public final class SchemaInferenceStack implements Mutable, EffectiveModelContex
                 }
             }
         }
-        throw new IllegalArgumentException("Choice " + nodeIdentifier + " not present");
+        throw notPresent(parent, "Choice", nodeIdentifier);
     }
 
     /**
@@ -743,7 +748,7 @@ public final class SchemaInferenceStack implements Mutable, EffectiveModelContex
         final GroupingEffectiveStatement ret = parent.streamEffectiveSubstatements(GroupingEffectiveStatement.class)
             .filter(stmt -> nodeIdentifier.equals(stmt.argument()))
             .findFirst()
-            .orElseThrow(() -> new IllegalArgumentException("Grouping " + nodeIdentifier + " not present"));
+            .orElseThrow(() -> notPresent(parent, "Grouping", nodeIdentifier));
         deque.push(ret);
         ++groupingDepth;
         return ret;
@@ -769,8 +774,8 @@ public final class SchemaInferenceStack implements Mutable, EffectiveModelContex
 
     private @NonNull SchemaTreeEffectiveStatement<?> pushSchema(
             final @NonNull SchemaTreeAwareEffectiveStatement<?, ?> parent, final @NonNull QName nodeIdentifier) {
-        final SchemaTreeEffectiveStatement<?> ret = parent.findSchemaTreeNode(nodeIdentifier).orElseThrow(
-            () -> new IllegalArgumentException("Schema tree child " + nodeIdentifier + " not present"));
+        final SchemaTreeEffectiveStatement<?> ret = parent.findSchemaTreeNode(nodeIdentifier)
+            .orElseThrow(() -> notPresent(parent, "Schema tree child ", nodeIdentifier));
         deque.push(ret);
         return ret;
     }
@@ -795,8 +800,8 @@ public final class SchemaInferenceStack implements Mutable, EffectiveModelContex
 
     private @NonNull DataTreeEffectiveStatement<?> pushData(final @NonNull DataTreeAwareEffectiveStatement<?, ?> parent,
             final @NonNull QName nodeIdentifier) {
-        final DataTreeEffectiveStatement<?> ret = parent.findDataTreeNode(nodeIdentifier).orElseThrow(
-            () -> new IllegalArgumentException("Data tree child " + nodeIdentifier + " not present"));
+        final DataTreeEffectiveStatement<?> ret = parent.findDataTreeNode(nodeIdentifier)
+            .orElseThrow(() -> notPresent(parent, "Data tree child", nodeIdentifier));
         deque.push(ret);
         clean = false;
         return ret;
@@ -820,7 +825,7 @@ public final class SchemaInferenceStack implements Mutable, EffectiveModelContex
         final TypedefEffectiveStatement ret = parent.streamEffectiveSubstatements(TypedefEffectiveStatement.class)
             .filter(stmt -> nodeIdentifier.equals(stmt.argument()))
             .findFirst()
-            .orElseThrow(() -> new IllegalArgumentException("Typedef " + nodeIdentifier + " not present"));
+            .orElseThrow(() -> notPresent(parent, "Typedef", nodeIdentifier));
         deque.push(ret);
         return ret;
     }
@@ -986,4 +991,26 @@ public final class SchemaInferenceStack implements Mutable, EffectiveModelContex
         }
         return obj;
     }
+
+    private static @NonNull IllegalArgumentException notPresent(final @NonNull EffectiveStatement<?, ?> parent,
+            final @NonNull String name, final QName nodeIdentifier) {
+        return new IllegalArgumentException(name + " " + nodeIdentifier + " not present in " + describeParent(parent));
+    }
+
+    private static @NonNull String describeParent(final @NonNull EffectiveStatement<?, ?> parent) {
+        // Add just enough information to be useful without being overly-verbose. Note we want to expose namespace
+        // information, so that we understand what revisions we are dealing with
+        if (parent instanceof SchemaTreeEffectiveStatement) {
+            return "schema parent " + parent.argument();
+        } else if (parent instanceof GroupingEffectiveStatement) {
+            return "grouping " + parent.argument();
+        } else if (parent instanceof ModuleEffectiveStatement) {
+            final var module = (ModuleEffectiveStatement) parent;
+            return "module " + module.argument().bindTo(module.localQNameModule());
+        } else {
+            // Shorthand for QNames, should provide enough context
+            final Object arg = parent.argument();
+            return "parent " + (arg instanceof QName ? arg : parent);
+        }
+    }
 }

diff --git a/model/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/LeafrefStaticAnalysisTest.java b/model/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/LeafrefStaticAnalysisTest.java
index 65da97f894c78b24121367e26200d2265c7d9572..5bd25d677aaec99a6323fff6ab3ac8721dd4155e 100644 (file)
--- a/model/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/LeafrefStaticAnalysisTest.java
+++ b/model/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/LeafrefStaticAnalysisTest.java
@@ -124,7 +124,7 @@ public class LeafrefStaticAnalysisTest {
         final SchemaInferenceStack stack = SchemaInferenceStack.of(context);
         stack.enterGrouping(grp.getQName());
         stack.enterSchemaTree(QName.create(FOO, "deref-non-existent"));
-        assertThrowsMissingXyzzy(stack, leaf);
+        assertThrowsMissingXyzzy(stack, leaf, "grouping (leafrefs)grp");
     }
 
     @Test
@@ -133,7 +133,7 @@ public class LeafrefStaticAnalysisTest {
             QName.create(FOO, "deref-non-existent")).get();
         final SchemaInferenceStack stack = SchemaInferenceStack.ofDataTreePath(context, foo.getQName(), bar.getQName());
         stack.enterSchemaTree(QName.create(FOO, "deref-non-existent"));
-        assertThrowsMissingXyzzy(stack, leaf);
+        assertThrowsMissingXyzzy(stack, leaf, "schema parent (leafrefs)bar");
     }
 
     @Test
@@ -143,7 +143,7 @@ public class LeafrefStaticAnalysisTest {
         final SchemaInferenceStack stack = SchemaInferenceStack.of(context);
         stack.enterGrouping(grp.getQName());
         stack.enterSchemaTree(QName.create(FOO, "non-existent-deref"));
-        assertThrowsMissingXyzzy(stack, leaf);
+        assertThrowsMissingXyzzy(stack, leaf, "schema parent (leafrefs)foo");
     }
 
     @Test
@@ -152,7 +152,7 @@ public class LeafrefStaticAnalysisTest {
             QName.create(FOO, "non-existent-deref")).get();
         final SchemaInferenceStack stack = SchemaInferenceStack.ofDataTreePath(context, foo.getQName(), bar.getQName());
         stack.enterSchemaTree(QName.create(FOO, "non-existent-deref"));
-        assertThrowsMissingXyzzy(stack, leaf);
+        assertThrowsMissingXyzzy(stack, leaf, "schema parent (leafrefs)foo");
     }
 
     @Test
@@ -161,7 +161,7 @@ public class LeafrefStaticAnalysisTest {
                 QName.create(FOO, "indirect-with-current")).get();
         final SchemaInferenceStack stack = SchemaInferenceStack.ofDataTreePath(context,
             foo.getQName(), bar.getQName(), QName.create(FOO, "indirect-with-current"));
-        assertThrowsMissingChild(stack, leaf, "(leafrefs)n");
+        assertThrowsMissingChild(stack, leaf, "(leafrefs)n", "module (leafrefs)leafrefs");
     }
 
     private static void assertThrowsInvalidPath(final SchemaInferenceStack stack, final LeafSchemaNode leaf) {
@@ -172,13 +172,15 @@ public class LeafrefStaticAnalysisTest {
         assertEquals("Unexpected current GroupingEffectiveStatementImpl[qname=(leafrefs)grp]", cause.getMessage());
     }
 
-    private static void assertThrowsMissingXyzzy(final SchemaInferenceStack stack, final LeafSchemaNode leaf) {
-        assertThrowsMissingChild(stack, leaf, "(leafrefs)xyzzy");
+    private static void assertThrowsMissingXyzzy(final SchemaInferenceStack stack, final LeafSchemaNode leaf,
+            final String parentDesc) {
+        assertThrowsMissingChild(stack, leaf, "(leafrefs)xyzzy", parentDesc);
     }
 
     private static void assertThrowsMissingChild(final SchemaInferenceStack stack, final LeafSchemaNode leaf,
-            final String childName) {
-        assertEquals("Data tree child " + childName + " not present", assertThrowsIAE(stack, leaf).getMessage());
+            final String childName, final String parentDesc) {
+        assertEquals("Data tree child " + childName + " not present in " + parentDesc,
+            assertThrowsIAE(stack, leaf).getMessage());
     }
 
     private static IllegalArgumentException assertThrowsIAE(final SchemaInferenceStack stack,

diff --git a/model/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/SchemaInferenceStackTest.java b/model/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/SchemaInferenceStackTest.java
index 48422d237293969bce185eb85e973169c28ae91a..19c31be4602ccb48d22c8ef32c7f4ffad3f5a18f 100644 (file)
--- a/model/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/SchemaInferenceStackTest.java
+++ b/model/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/SchemaInferenceStackTest.java
@@ -79,9 +79,9 @@ public class SchemaInferenceStackTest {
     @Test
     public void enterGroupingNegativeTest() {
         final SchemaInferenceStack stack = SchemaInferenceStack.of(context);
-        assertNotExistentGrouping(stack);
+        assertNotExistentGrouping(stack, "module (uri:my-module?revision=2014-10-07)my-module");
         stack.enterDataTree(QName.create(myModule.getQNameModule(), "my-container"));
-        assertNotExistentGrouping(stack);
+        assertNotExistentGrouping(stack, "schema parent (uri:my-module?revision=2014-10-07)my-container");
     }
 
     @Test
@@ -94,23 +94,21 @@ public class SchemaInferenceStackTest {
     @Test
     public void enterTypedefNegativeTest() {
         final SchemaInferenceStack stack = SchemaInferenceStack.of(context);
-        assertNotExistentTypedef(stack);
+        assertNotExistentTypedef(stack, "module (uri:my-module?revision=2014-10-07)my-module");
         stack.enterDataTree(QName.create(myModule.getQNameModule(), "my-container"));
-        assertNotExistentTypedef(stack);
+        assertNotExistentTypedef(stack, "schema parent (uri:my-module?revision=2014-10-07)my-container");
     }
 
-    private static void assertNotExistentGrouping(final SchemaInferenceStack stack) {
+    private static void assertNotExistentGrouping(final SchemaInferenceStack stack, final String parentDesc) {
         final QName nonExistent = QName.create(myModule.getQNameModule(), "non-existent");
-        final IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
-            () -> stack.enterGrouping(nonExistent));
-        assertEquals("Grouping (uri:my-module?revision=2014-10-07)non-existent not present", ex.getMessage());
+        assertEquals("Grouping (uri:my-module?revision=2014-10-07)non-existent not present in " + parentDesc,
+            assertThrows(IllegalArgumentException.class, () -> stack.enterGrouping(nonExistent)).getMessage());
     }
 
-    private static void assertNotExistentTypedef(final SchemaInferenceStack stack) {
+    private static void assertNotExistentTypedef(final SchemaInferenceStack stack, final String parentDesc) {
         final QName nonExistent = QName.create(myModule.getQNameModule(), "non-existent");
-        final IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
-            () -> stack.enterTypedef(nonExistent));
-        assertEquals("Typedef (uri:my-module?revision=2014-10-07)non-existent not present", ex.getMessage());
+        assertEquals("Typedef (uri:my-module?revision=2014-10-07)non-existent not present in " + parentDesc,
+            assertThrows(IllegalArgumentException.class, () -> stack.enterTypedef(nonExistent)).getMessage());
     }
 
     private static GroupingDefinition getGroupingByName(final DataNodeContainer dataNodeContainer, final String name) {

diff --git a/model/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/YT1231Test.java b/model/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/YT1231Test.java
index d9d7c4942a45892c7bf9cea67fc315068029a89d..4ea01d376dcc6b33fe4a2969567434b2534d11fb 100644 (file)
--- a/model/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/YT1231Test.java
+++ b/model/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/YT1231Test.java
@@ -11,7 +11,9 @@ import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThrows;
 
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext;
@@ -27,14 +29,20 @@ public class YT1231Test {
     private static final QName BAZ = QName.create("foo", "baz");
     private static final QName XYZZY = QName.create("foo", "xyzzy");
 
+    private static EffectiveModelContext CONTEXT;
+
+    private final SchemaInferenceStack stack = SchemaInferenceStack.of(CONTEXT);
+
+    @BeforeClass
+    public static void beforeClass() {
+        CONTEXT = YangParserTestUtils.parseYangResource("/yt1231.yang");
+    }
+
     @Test
     public void testEnterDataTree() {
-        final EffectiveModelContext context = YangParserTestUtils.parseYangResource("/yt1231.yang");
-        final SchemaInferenceStack stack = SchemaInferenceStack.of(context);
-
         // Trivial
         assertThat(stack.enterDataTree(FOO), instanceOf(ContainerEffectiveStatement.class));
-        assertSame(context.getModuleStatement(FOO.getModule()), stack.currentModule());
+        assertSame(CONTEXT.getModuleStatement(FOO.getModule()), stack.currentModule());
         assertEquals(Absolute.of(FOO), stack.toSchemaNodeIdentifier());
         assertThat(stack.enterDataTree(FOO), instanceOf(ContainerEffectiveStatement.class));
         assertEquals(Absolute.of(FOO, FOO), stack.toSchemaNodeIdentifier());
@@ -52,9 +60,6 @@ public class YT1231Test {
 
     @Test
     public void testEnterChoice() {
-        final EffectiveModelContext context = YangParserTestUtils.parseYangResource("/yt1231.yang");
-        final SchemaInferenceStack stack = SchemaInferenceStack.of(context);
-
         // Simple
         assertThat(stack.enterDataTree(FOO), instanceOf(ContainerEffectiveStatement.class));
         assertEquals(Absolute.of(FOO), stack.toSchemaNodeIdentifier());
@@ -71,4 +76,30 @@ public class YT1231Test {
         assertThat(stack.enterChoice(BAZ), instanceOf(ChoiceEffectiveStatement.class));
         assertEquals(Absolute.of(FOO, BAR, BAR, BAZ), stack.toSchemaNodeIdentifier());
     }
+
+    @Test
+    public void testEnterChoiceToRootContainer() {
+        assertEquals("Choice (foo)foo not present", assertEnterChoiceThrows(FOO));
+    }
+
+    @Test
+    public void testEnterChoiceToNestedContainer() {
+        assertThat(stack.enterDataTree(FOO), instanceOf(ContainerEffectiveStatement.class));
+        assertEquals(Absolute.of(FOO), stack.toSchemaNodeIdentifier());
+        assertEquals("Choice (foo)foo not present in schema parent (foo)foo", assertEnterChoiceThrows(FOO));
+    }
+
+    @Test
+    public void testEnterChoiceNonExistent() {
+        assertThat(stack.enterDataTree(FOO), instanceOf(ContainerEffectiveStatement.class));
+        assertEquals(Absolute.of(FOO), stack.toSchemaNodeIdentifier());
+        assertThat(stack.enterSchemaTree(BAR), instanceOf(ChoiceEffectiveStatement.class));
+
+        assertEquals("Choice (foo)foo not present in schema parent (foo)bar", assertEnterChoiceThrows(FOO));
+        assertEquals("Choice (foo)bar not present in schema parent (foo)bar", assertEnterChoiceThrows(BAR));
+    }
+
+    private String assertEnterChoiceThrows(final QName nodeIdentifier) {
+        return assertThrows(IllegalArgumentException.class, () -> stack.enterChoice(nodeIdentifier)).getMessage();
+    }
 }