From 4870d4af9337078cf8c14a8cd57a20b5c7880341 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 12 Oct 2018 23:11:22 +0200 Subject: [PATCH] Fixup MinMaxElementsValidation with disappearances When a list disappears, even when it has a certain number of minimum elements, MinMaxElementsValidation should not take any action. The resposibility for enforcing the list being present when it has min-elements > 0 falls under perview of MandatoryNodeValidator, which covers the non-existence of the list at the proper enforcement point -- something MinMaxElementsValidation cannot do. JIRA: YANGTOOLS-909 Change-Id: I3612a1c1461456b2bd41651decab1032581f85d1 Signed-off-by: Robert Varga --- .../schema/tree/MinMaxElementsValidation.java | 34 ++++++---- .../data/impl/schema/tree/Bug4454Test.java | 68 +++++++++++++++---- .../yang/data/impl/schema/tree/YT776Test.java | 33 +++++++++ .../src/test/resources/bug-4454-test.yang | 15 ++++ .../src/test/resources/yt776/yt776.yang | 14 ++++ 5 files changed, 136 insertions(+), 28 deletions(-) diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MinMaxElementsValidation.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MinMaxElementsValidation.java index eab6d9c55e..db32cf1627 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MinMaxElementsValidation.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MinMaxElementsValidation.java @@ -60,8 +60,11 @@ final class MinMaxElementsValidation extends ModificationApplyOperation { if (ret == null) { // Deal with the result moving on us ret = delegate.apply(modification, storeMeta, version); - checkChildren(modification.getIdentifier(), numOfChildrenFromTreeNode(ret)); + if (ret.isPresent()) { + checkChildren(ret.get().getData()); + } } + return ret; } @@ -77,9 +80,12 @@ final class MinMaxElementsValidation extends ModificationApplyOperation { final ModifiedNode modified = (ModifiedNode) modification; // We need to actually perform the operation to deal with merge in a sane manner. We know the modification - // is immutable, so the result of validation will probably not change. + // is immutable, so the result of validation will probably not change. Note we should not be checking number final Optional maybeApplied = delegate.apply(modified, current, version); - validateMinMaxElements(path, modified.getIdentifier(), numOfChildrenFromTreeNode(maybeApplied)); + if (maybeApplied.isPresent()) { + // We only enforce min/max on present data and rely on MandatoryLeafEnforcer to take care of the empty case + validateMinMaxElements(path, maybeApplied.get().getData()); + } // Everything passed. We now have a snapshot of the result node, it would be too bad if we just threw it out. // We know what the result of an apply operation is going to be *if* the following are kept unchanged: @@ -94,7 +100,7 @@ final class MinMaxElementsValidation extends ModificationApplyOperation { void verifyStructure(final NormalizedNode modification, final boolean verifyChildren) { delegate.verifyStructure(modification, verifyChildren); if (verifyChildren) { - checkChildren(modification.getIdentifier(), numOfChildrenFromValue(modification)); + checkChildren(modification); } } @@ -118,29 +124,29 @@ final class MinMaxElementsValidation extends ModificationApplyOperation { delegate.recursivelyVerifyStructure(value); } - private void validateMinMaxElements(final ModificationPath path, final PathArgument id, final int children) + private void validateMinMaxElements(final ModificationPath path, final NormalizedNode value) throws DataValidationFailedException { + final PathArgument id = value.getIdentifier(); + final int children = numOfChildrenFromValue(value); if (minElements > children) { - throw new RequiredElementCountException(path.toInstanceIdentifier(), minElements, maxElements, - children, "%s does not have enough elements (%s), needs at least %s", id, children, minElements); + throw new RequiredElementCountException(path.toInstanceIdentifier(), minElements, maxElements, children, + "%s does not have enough elements (%s), needs at least %s", id, children, minElements); } if (maxElements < children) { - throw new RequiredElementCountException(path.toInstanceIdentifier(), minElements, maxElements, - children, "%s has too many elements (%s), can have at most %s", id, children, maxElements); + throw new RequiredElementCountException(path.toInstanceIdentifier(), minElements, maxElements, children, + "%s has too many elements (%s), can have at most %s", id, children, maxElements); } } - private void checkChildren(final PathArgument id, final int children) { + private void checkChildren(final NormalizedNode value) { + final PathArgument id = value.getIdentifier(); + final int children = numOfChildrenFromValue(value); checkArgument(minElements <= children, "Node %s does not have enough elements (%s), needs at least %s", id, children, minElements); checkArgument(maxElements >= children, "Node %s has too many elements (%s), can have at most %s", id, children, maxElements); } - private static int numOfChildrenFromTreeNode(final Optional node) { - return node.isPresent() ? numOfChildrenFromValue(node.get().getData()) : 0; - } - private static int numOfChildrenFromValue(final NormalizedNode value) { if (value instanceof NormalizedNodeContainer) { return ((NormalizedNodeContainer) value).getValue().size(); diff --git a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug4454Test.java b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug4454Test.java index 5ba941861a..03760d4788 100644 --- a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug4454Test.java +++ b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug4454Test.java @@ -26,6 +26,7 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue; +import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; import org.opendaylight.yangtools.yang.data.api.schema.LeafNode; import org.opendaylight.yangtools.yang.data.api.schema.LeafSetEntryNode; import org.opendaylight.yangtools.yang.data.api.schema.LeafSetNode; @@ -57,12 +58,15 @@ public class Bug4454Test { .create(MASTER_CONTAINER_QNAME, "min-max-list-no-minmax"); private static final QName MIN_MAX_KEY_LEAF_QNAME = QName.create(MASTER_CONTAINER_QNAME, "min-max-key-leaf"); private static final QName MIN_MAX_VALUE_LEAF_QNAME = QName.create(MASTER_CONTAINER_QNAME, "min-max-value-leaf"); + private static final QName PRESENCE_QNAME = QName.create(MASTER_CONTAINER_QNAME, "presence"); private static final YangInstanceIdentifier MASTER_CONTAINER_PATH = YangInstanceIdentifier .of(MASTER_CONTAINER_QNAME); private static final YangInstanceIdentifier MIN_MAX_LIST_PATH = YangInstanceIdentifier .builder(MASTER_CONTAINER_PATH) .node(MIN_MAX_LIST_QNAME).build(); + private static final YangInstanceIdentifier PRESENCE_PATH = YangInstanceIdentifier.of(PRESENCE_QNAME); + private static final YangInstanceIdentifier PRESENCE_MIN_MAX_LIST_PATH = PRESENCE_PATH.node(MIN_MAX_LIST_QNAME); private static final YangInstanceIdentifier MIN_MAX_LIST_NO_MINMAX_PATH = YangInstanceIdentifier .builder(MASTER_CONTAINER_PATH) .node(MIN_MAX_LIST_QNAME_NO_MINMAX).build(); @@ -303,29 +307,24 @@ public class Bug4454Test { } @Test - public void minMaxListDeleteExceptionTest() { + public void minMaxListDeleteTest() throws DataValidationFailedException { final DataTreeModification modificationTree = inMemoryDataTree.takeSnapshot().newModification(); - Map key = new HashMap<>(); - key.put(MIN_MAX_KEY_LEAF_QNAME, "foo"); - NodeIdentifierWithPredicates mapEntryPath2 = new NodeIdentifierWithPredicates(MIN_MAX_LIST_QNAME, key); + NodeIdentifierWithPredicates mapEntryPath2 = new NodeIdentifierWithPredicates(MIN_MAX_LIST_QNAME, + ImmutableMap.of(MIN_MAX_KEY_LEAF_QNAME, "foo")); final YangInstanceIdentifier minMaxLeafFoo = MASTER_CONTAINER_PATH .node(MIN_MAX_LIST_QNAME).node(mapEntryPath2); - key.clear(); - key.put(MIN_MAX_KEY_LEAF_QNAME, "bar"); - - mapEntryPath2 = new NodeIdentifierWithPredicates(MIN_MAX_LIST_QNAME, key); + mapEntryPath2 = new NodeIdentifierWithPredicates(MIN_MAX_LIST_QNAME, + ImmutableMap.of(MIN_MAX_KEY_LEAF_QNAME, "bar")); final YangInstanceIdentifier minMaxLeafBar = MASTER_CONTAINER_PATH .node(MIN_MAX_LIST_QNAME).node(mapEntryPath2); - key.clear(); - key.put(MIN_MAX_KEY_LEAF_QNAME, "baz"); - - mapEntryPath2 = new NodeIdentifierWithPredicates(MIN_MAX_LIST_QNAME, key); + mapEntryPath2 = new NodeIdentifierWithPredicates(MIN_MAX_LIST_QNAME, + ImmutableMap.of(MIN_MAX_KEY_LEAF_QNAME, "baz")); final YangInstanceIdentifier minMaxLeafBaz = MASTER_CONTAINER_PATH .node(MIN_MAX_LIST_QNAME).node(mapEntryPath2); @@ -337,13 +336,54 @@ public class Bug4454Test { modificationTree.delete(minMaxLeafBar); modificationTree.delete(minMaxLeafBaz); + modificationTree.ready(); + + inMemoryDataTree.validate(modificationTree); + final DataTreeCandidate prepare = inMemoryDataTree.prepare(modificationTree); + inMemoryDataTree.commit(prepare); + + // Empty list should have disappeared, along with the container, as we are not enforcing root + final NormalizedNode data = inMemoryDataTree.takeSnapshot().readNode(YangInstanceIdentifier.EMPTY).get(); + assertTrue(data instanceof ContainerNode); + assertEquals(0, ((ContainerNode) data).getValue().size()); + } + + @Test + public void minMaxListDeleteExceptionTest() throws DataValidationFailedException { + final DataTreeModification modificationTree = inMemoryDataTree.takeSnapshot().newModification(); + + NodeIdentifierWithPredicates mapEntryPath2 = new NodeIdentifierWithPredicates(MIN_MAX_LIST_QNAME, + ImmutableMap.of(MIN_MAX_KEY_LEAF_QNAME, "foo")); + + final YangInstanceIdentifier minMaxLeafFoo = PRESENCE_PATH.node(MIN_MAX_LIST_QNAME).node(mapEntryPath2); + + mapEntryPath2 = new NodeIdentifierWithPredicates(MIN_MAX_LIST_QNAME, + ImmutableMap.of(MIN_MAX_KEY_LEAF_QNAME, "bar")); + + final YangInstanceIdentifier minMaxLeafBar = PRESENCE_PATH.node(MIN_MAX_LIST_QNAME).node(mapEntryPath2); + + mapEntryPath2 = new NodeIdentifierWithPredicates(MIN_MAX_LIST_QNAME, + ImmutableMap.of(MIN_MAX_KEY_LEAF_QNAME, "baz")); + + final YangInstanceIdentifier minMaxLeafBaz = PRESENCE_PATH.node(MIN_MAX_LIST_QNAME).node(mapEntryPath2); + + modificationTree.write(PRESENCE_PATH, ImmutableNodes.containerNode(PRESENCE_QNAME)); + modificationTree.write(PRESENCE_MIN_MAX_LIST_PATH, mapNodeFooWithNodes); + modificationTree.merge(PRESENCE_MIN_MAX_LIST_PATH, mapNodeBar); + modificationTree.merge(PRESENCE_MIN_MAX_LIST_PATH, mapNodeBaz); + modificationTree.delete(minMaxLeafFoo); + modificationTree.delete(minMaxLeafBar); + modificationTree.delete(minMaxLeafBaz); + try { + // Unlike minMaxListDeleteTest(), presence container enforces the list to be present modificationTree.ready(); fail("Should have failed with IAE"); } catch (IllegalArgumentException e) { assertEquals("Node (urn:opendaylight:params:xml:ns:yang:list-constraints-validation-test-model?" - + "revision=2015-02-02)min-max-list does not have enough elements (0), needs at least 1", - e.getMessage()); + + "revision=2015-02-02)presence is missing mandatory descendant " + + "/(urn:opendaylight:params:xml:ns:yang:list-constraints-validation-test-model?" + + "revision=2015-02-02)min-max-list", e.getMessage()); } } diff --git a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/YT776Test.java b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/YT776Test.java index 322e6837a9..83c202ef85 100644 --- a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/YT776Test.java +++ b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/YT776Test.java @@ -9,6 +9,7 @@ package org.opendaylight.yangtools.yang.data.impl.schema.tree; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import static org.opendaylight.yangtools.yang.data.impl.schema.Builders.choiceBuilder; import static org.opendaylight.yangtools.yang.data.impl.schema.Builders.containerBuilder; import static org.opendaylight.yangtools.yang.data.impl.schema.Builders.leafBuilder; import static org.opendaylight.yangtools.yang.data.impl.schema.Builders.leafSetBuilder; @@ -50,6 +51,13 @@ public class YT776Test { private static final NodeIdentifierWithPredicates NESTED_ITEM = new NodeIdentifierWithPredicates(NESTED, ImmutableMap.of(NESTED_ATTRIBUTE, "foo")); + private static final NodeIdentifier ANY_OF = new NodeIdentifier(QName.create(MODULE, "any-of")); + private static final QName SOME_LEAF = QName.create(MODULE, "some-leaf"); + private static final NodeIdentifier SOME_LEAF_ID = new NodeIdentifier(SOME_LEAF); + private static final QName SOME_LIST = QName.create(MODULE, "some-list"); + private static final NodeIdentifier SOME_LIST_ID = new NodeIdentifier(SOME_LIST); + private static final NodeIdentifierWithPredicates SOME_LIST_ITEM = new NodeIdentifierWithPredicates(SOME_LIST, + ImmutableMap.of(SOME_LEAF, "foo")); private static SchemaContext SCHEMA_CONTEXT; private DataTree dataTree; @@ -212,6 +220,31 @@ public class YT776Test { commit(mod); } + @Test + public void testDisappearInChoice() throws DataValidationFailedException { + DataTreeModification mod = dataTree.takeSnapshot().newModification(); + // Initialize choice with list + mod.write(YangInstanceIdentifier.create(BOX), containerBuilder().withNodeIdentifier(BOX) + .withChild(choiceBuilder().withNodeIdentifier(ANY_OF) + .withChild(mapBuilder().withNodeIdentifier(SOME_LIST_ID) + .withChild(mapEntryBuilder() + .withNodeIdentifier(SOME_LIST_ITEM) + .withChild(leafBuilder().withNodeIdentifier(SOME_LEAF_ID).withValue("foo").build()) + .build()) + .build()) + .build()) + .build()); + commit(mod); + + // Now delete the single item, causing the list to fizzle, while creating the alterinative case + mod = dataTree.takeSnapshot().newModification(); + mod.delete(YangInstanceIdentifier.create(BOX, ANY_OF, SOME_LIST_ID, SOME_LIST_ITEM)); + mod.write(YangInstanceIdentifier.create(BOX, ANY_OF, SOME_LEAF_ID), + leafBuilder().withNodeIdentifier(SOME_LEAF_ID).withValue("foo").build()); + + commit(mod); + } + private DataTreeModification write(final ContainerNode data) throws DataValidationFailedException { final DataTreeModification mod = dataTree.takeSnapshot().newModification(); mod.write(YangInstanceIdentifier.create(BOX), data); diff --git a/yang/yang-data-impl/src/test/resources/bug-4454-test.yang b/yang/yang-data-impl/src/test/resources/bug-4454-test.yang index 04edcf786b..8db5e73bd1 100644 --- a/yang/yang-data-impl/src/test/resources/bug-4454-test.yang +++ b/yang/yang-data-impl/src/test/resources/bug-4454-test.yang @@ -36,4 +36,19 @@ module Bug4454Test { type string; } } + + container presence { + presence "anchor point"; + + list min-max-list { + min-elements 2; + max-elements 3; + + key "min-max-key-leaf"; + + leaf min-max-key-leaf { + type string; + } + } + } } diff --git a/yang/yang-data-impl/src/test/resources/yt776/yt776.yang b/yang/yang-data-impl/src/test/resources/yt776/yt776.yang index 3db9b84de0..5a4217761f 100644 --- a/yang/yang-data-impl/src/test/resources/yt776/yt776.yang +++ b/yang/yang-data-impl/src/test/resources/yt776/yt776.yang @@ -25,6 +25,20 @@ module yt776 { } } } + + choice any-of { + leaf some-leaf { + type string; + } + list some-list { + key some-leaf; + min-elements 1; + + leaf some-leaf { + type string; + } + } + } } } -- 2.36.6