BUG-3189: Check consistency of Instance Identifier and Data

DataTree allows for a NormalizedNode to be stored at a position which is
inconsistent with its identifier. This can lead to weird errors, as
child metadata nodes are created. Add an explicit check to ensure this
cannot occur.

Change-Id: I25918f1f866beadb1ba7c9687ba1a880942a0d9e
Signed-off-by: Tony Tkacik <ttkacik@cisco.com>

diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java
index 817110a8f126fc8f7dc69ff2465e7a71e9fa2082..37818b96ed6ebb7a619a3987fbbe363cf325dc44 100644 (file)
--- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java
+++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java
@@ -65,14 +65,14 @@ final class InMemoryDataTreeModification implements DataTreeModification {
     @Override
     public void write(final YangInstanceIdentifier path, final NormalizedNode<?, ?> data) {
         checkSealed();
-
+        checkIdentifierReferencesData(path, data);
         resolveModificationFor(path).write(data);
     }
 
     @Override
     public void merge(final YangInstanceIdentifier path, final NormalizedNode<?, ?> data) {
         checkSealed();
-
+        checkIdentifierReferencesData(path, data);
         resolveModificationFor(path).merge(data);
     }
 
@@ -96,7 +96,7 @@ final class InMemoryDataTreeModification implements DataTreeModification {
 
         final Optional<TreeNode> result = resolveSnapshot(key, mod);
         if (result.isPresent()) {
-            NormalizedNode<?, ?> data = result.get().getData();
+            final NormalizedNode<?, ?> data = result.get().getData();
             return NormalizedNodes.findNode(key, data, path);
         } else {
             return Optional.absent();
@@ -112,7 +112,7 @@ final class InMemoryDataTreeModification implements DataTreeModification {
         try {
             return resolveModificationStrategy(path).apply(modification, modification.getOriginal(),
                     version);
-        } catch (Exception e) {
+        } catch (final Exception e) {
             LOG.error("Could not create snapshot for {}:{}", path, modification, e);
             throw e;
         }
@@ -148,8 +148,8 @@ final class InMemoryDataTreeModification implements DataTreeModification {
         ModifiedNode modification = rootNode;
 
         int i = 1;
-        for(PathArgument pathArg : path.getPathArguments()) {
-            Optional<ModificationApplyOperation> potential = operation.getChild(pathArg);
+        for(final PathArgument pathArg : path.getPathArguments()) {
+            final Optional<ModificationApplyOperation> potential = operation.getChild(pathArg);
             if (!potential.isPresent()) {
                 throw new IllegalArgumentException(String.format("Child %s is not present in schema tree.",
                         Iterables.toString(Iterables.limit(path.getPathArguments(), i))));
@@ -193,11 +193,11 @@ final class InMemoryDataTreeModification implements DataTreeModification {
          * We will use preallocated version, this means returned snapshot will
          * have same version each time this method is called.
          */
-        TreeNode originalSnapshotRoot = snapshot.getRootNode();
-        Optional<TreeNode> tempRoot = strategyTree.apply(rootNode, Optional.of(originalSnapshotRoot), version);
+        final TreeNode originalSnapshotRoot = snapshot.getRootNode();
+        final Optional<TreeNode> tempRoot = strategyTree.apply(rootNode, Optional.of(originalSnapshotRoot), version);
         Preconditions.checkState(tempRoot.isPresent(), "Data tree root is not present, possibly removed by previous modification");
 
-        InMemoryDataTreeSnapshot tempTree = new InMemoryDataTreeSnapshot(snapshot.getSchemaContext(), tempRoot.get(), strategyTree);
+        final InMemoryDataTreeSnapshot tempTree = new InMemoryDataTreeSnapshot(snapshot.getSchemaContext(), tempRoot.get(), strategyTree);
         return tempTree.newModification();
     }
 
@@ -209,7 +209,7 @@ final class InMemoryDataTreeModification implements DataTreeModification {
         final Collection<ModifiedNode> children = node.getChildren();
         if (!children.isEmpty()) {
             cursor.enter(node.getIdentifier());
-            for (ModifiedNode child : children) {
+            for (final ModifiedNode child : children) {
                 applyNode(cursor, child);
             }
             cursor.exit();
@@ -245,8 +245,15 @@ final class InMemoryDataTreeModification implements DataTreeModification {
 
     @Override
     public void applyToCursor(final DataTreeModificationCursor cursor) {
-        for (ModifiedNode child : rootNode.getChildren()) {
+        for (final ModifiedNode child : rootNode.getChildren()) {
             applyNode(cursor, child);
         }
     }
+
+    private static void checkIdentifierReferencesData(final YangInstanceIdentifier path,
+            final NormalizedNode<?, ?> data) {
+        final PathArgument lastArg = path.getLastPathArgument();
+        Preconditions.checkArgument(data.getIdentifier().equals(lastArg),
+                "Instance identifier references %s but data identifier is %s", lastArg, data.getIdentifier());
+    }
 }

diff --git a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListConstraintsValidationTest.java b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListConstraintsValidationTest.java
index e866abb0d89d09102c3fe0e0ac20175b25ba638a..3f6bca552d5e1b4f8e6a507b84c8547edebf0327 100644 (file)
--- a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListConstraintsValidationTest.java
+++ b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListConstraintsValidationTest.java
@@ -195,7 +195,7 @@ public class ListConstraintsValidationTest {
                 .withValue("goo").build();
 
         final LeafSetNode<Object> fooLeafSetNode = ImmutableLeafSetNodeBuilder.create()
-                .withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(MIN_MAX_LIST_QNAME))
+                .withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(MIN_MAX_LEAF_LIST_QNAME))
                 .withChildValue("foo").build();
 
         modificationTree.write(MIN_MAX_LEAF_LIST_PATH, fooLeafSetNode);
@@ -211,7 +211,7 @@ public class ListConstraintsValidationTest {
         final Optional<NormalizedNode<?, ?>> masterContainer = snapshotAfterCommit.readNode(MASTER_CONTAINER_PATH);
         assertTrue(masterContainer.isPresent());
         final Optional<NormalizedNodeContainer> leafList = ((NormalizedNodeContainer) masterContainer.get()).getChild(
-                new YangInstanceIdentifier.NodeIdentifier(MIN_MAX_LIST_QNAME));
+                new YangInstanceIdentifier.NodeIdentifier(MIN_MAX_LEAF_LIST_QNAME));
         assertTrue(leafList.isPresent());
         assertTrue(leafList.get().getValue().size() == 2);
     }
@@ -237,7 +237,7 @@ public class ListConstraintsValidationTest {
                 .withValue("fuu").build();
 
         final LeafSetNode<Object> fooLeafSetNode = ImmutableLeafSetNodeBuilder.create()
-                .withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(MIN_MAX_LIST_QNAME))
+                .withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(MIN_MAX_LEAF_LIST_QNAME))
                 .withChildValue("foo").build();
 
         modificationTree.write(MIN_MAX_LEAF_LIST_PATH, fooLeafSetNode);