From 8bb72bff26c4f1d2da94af6463ba8ae212e2223a Mon Sep 17 00:00:00 2001 From: Tony Tkacik Date: Wed, 13 May 2015 11:23:27 +0200 Subject: [PATCH] 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 --- .../tree/InMemoryDataTreeModification.java | 29 ++++++++++++------- .../tree/ListConstraintsValidationTest.java | 6 ++-- 2 files changed, 21 insertions(+), 14 deletions(-) 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 817110a8f1..37818b96ed 100644 --- 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 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 potential = operation.getChild(pathArg); + for(final PathArgument pathArg : path.getPathArguments()) { + final Optional 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 tempRoot = strategyTree.apply(rootNode, Optional.of(originalSnapshotRoot), version); + final TreeNode originalSnapshotRoot = snapshot.getRootNode(); + final Optional 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 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 e866abb0d8..3f6bca552d 100644 --- 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 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> masterContainer = snapshotAfterCommit.readNode(MASTER_CONTAINER_PATH); assertTrue(masterContainer.isPresent()); final Optional 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 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); -- 2.36.6