From e0a42d9a2458f04ebd29ab821c38132075aba644 Mon Sep 17 00:00:00 2001 From: Peter Kajsa Date: Fri, 20 May 2016 09:46:37 +0200 Subject: [PATCH] Bug 5830: Mandatory leaf enforcement is not correct with presence container Some operations (e.g. write of MapEnryNode) don't invoke any method on PresenceContainerModificationStrategy in order to enforce presence of mandatory nodes. This fix ensures invoking of MandatoryLeafEnforcer of PresenceContainerModificationStrategy when verifyStructure method is performed. Change-Id: I6f5900f555e8f94916a84b747fbd895592606f38 Signed-off-by: Peter Kajsa Signed-off-by: Robert Varga (cherry picked from commit dfb9d8c251fc77e434abf46083c3d13dd8c20917) --- ...PresenceContainerModificationStrategy.java | 9 + .../data/impl/schema/tree/Bug5830Test.java | 258 ++++++++++++++++++ .../test/resources/bug5830/foo-multiple.yang | 56 ++++ .../resources/bug5830/foo-non-presence.yang | 33 +++ .../test/resources/bug5830/foo-presence.yang | 34 +++ 5 files changed, 390 insertions(+) create mode 100644 yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug5830Test.java create mode 100644 yang/yang-data-impl/src/test/resources/bug5830/foo-multiple.yang create mode 100644 yang/yang-data-impl/src/test/resources/bug5830/foo-non-presence.yang create mode 100644 yang/yang-data-impl/src/test/resources/bug5830/foo-presence.yang diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/PresenceContainerModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/PresenceContainerModificationStrategy.java index 4becd4f709..25246eb521 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/PresenceContainerModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/PresenceContainerModificationStrategy.java @@ -9,6 +9,7 @@ package org.opendaylight.yangtools.yang.data.impl.schema.tree; import com.google.common.base.Optional; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.TreeType; import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.TreeNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.Version; @@ -26,6 +27,14 @@ final class PresenceContainerModificationStrategy extends ContainerModificationS enforcer = MandatoryLeafEnforcer.forContainer(schemaNode, treeType); } + @Override + void verifyStructure(final NormalizedNode writtenValue, final boolean verifyChildren) { + if(verifyChildrenStructure() && verifyChildren) { + enforcer.enforceOnTreeNode(writtenValue); + } + super.verifyStructure(writtenValue, verifyChildren); + } + @Override protected TreeNode applyMerge(final ModifiedNode modification, final TreeNode currentMeta, final Version version) { final TreeNode ret = super.applyMerge(modification, currentMeta, version); diff --git a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug5830Test.java b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug5830Test.java new file mode 100644 index 0000000000..0be49c3094 --- /dev/null +++ b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug5830Test.java @@ -0,0 +1,258 @@ +/* + * Copyright (c) 2016 Cisco Systems, Inc. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.yangtools.yang.data.impl.schema.tree; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import org.junit.Test; +import org.opendaylight.yangtools.yang.common.QName; +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.schema.ContainerNode; +import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild; +import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode; +import org.opendaylight.yangtools.yang.data.api.schema.MapNode; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException; +import org.opendaylight.yangtools.yang.data.api.schema.tree.TreeType; +import org.opendaylight.yangtools.yang.data.impl.schema.Builders; +import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; +import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContainerNodeAttrBuilder; +import org.opendaylight.yangtools.yang.model.api.SchemaContext; +import org.opendaylight.yangtools.yang.parser.spi.meta.ReactorException; + +public class Bug5830Test { + private static final String NS = "foo"; + private static final String REV = "2016-05-17"; + private static final QName TASK_CONTAINER = QName.create(NS, REV, "task-container"); + private static final QName TASK = QName.create(NS, REV, "task"); + private static final QName TASK_ID = QName.create(NS, REV, "task-id"); + private static final QName TASK_DATA = QName.create(NS, REV, "task-data"); + private static final QName OTHER_DATA = QName.create(NS, REV, "other-data"); + private static final QName MANDATORY_DATA = QName.create(NS, REV, "mandatory-data"); + private static final QName TASK_MANDATORY_LEAF = QName.create(NS, REV, "task-mandatory-leaf"); + private static final QName NON_PRESENCE_CONTAINER = QName.create(NS, REV, "non-presence-container"); + private static final QName NON_PRESENCE_CONTAINER_2 = QName.create(NS, REV, "non-presence-container-2"); + private static final QName PRESENCE_CONTAINER_2 = QName.create(NS, REV, "presence-container-2"); + private static final QName MANDATORY_LEAF_2 = QName.create(NS, REV, "mandatory-leaf-2"); + + private static InMemoryDataTree initDataTree(final SchemaContext schemaContext) + throws DataValidationFailedException { + final InMemoryDataTree inMemoryDataTree = (InMemoryDataTree) InMemoryDataTreeFactory.getInstance().create( + TreeType.CONFIGURATION); + inMemoryDataTree.setSchemaContext(schemaContext); + + final MapNode taskNode = Builders.mapBuilder().withNodeIdentifier(new NodeIdentifier(TASK)).build(); + final InMemoryDataTreeModification modificationTree = inMemoryDataTree.takeSnapshot().newModification(); + modificationTree.write(YangInstanceIdentifier.of(TASK_CONTAINER).node(TASK), taskNode); + modificationTree.ready(); + + inMemoryDataTree.validate(modificationTree); + final DataTreeCandidate prepare = inMemoryDataTree.prepare(modificationTree); + inMemoryDataTree.commit(prepare); + return inMemoryDataTree; + } + + @Test + public void testMandatoryNodes() throws ReactorException, DataValidationFailedException { + testPresenceContainer(); + testNonPresenceContainer(); + testMultipleContainers(); + } + + private static void testPresenceContainer() throws ReactorException, DataValidationFailedException { + final SchemaContext schemaContext = TestModel.createTestContext("/bug5830/foo-presence.yang"); + assertNotNull("Schema context must not be null.", schemaContext); + + testContainerIsNotPresent(schemaContext); + try { + testContainerIsPresent(schemaContext); + fail("Should fail due to missing mandatory node under present presence container."); + } catch (final IllegalArgumentException e) { + assertEquals( + "Node (foo?revision=2016-05-17)task-data is missing mandatory descendant /(foo?revision=2016-05-17)" + + "mandatory-data", e.getMessage()); + } + testMandatoryDataLeafIsPresent(schemaContext); + } + + private static void testNonPresenceContainer() throws ReactorException, DataValidationFailedException { + final SchemaContext schemaContext = TestModel.createTestContext("/bug5830/foo-non-presence.yang"); + assertNotNull("Schema context must not be null.", schemaContext); + + try { + testContainerIsNotPresent(schemaContext); + fail("Should fail due to missing mandatory node."); + } catch (final IllegalArgumentException e) { + assertEquals( + "Node (foo?revision=2016-05-17)task[{(foo?revision=2016-05-17)task-id=123}] is missing mandatory " + + "descendant /(foo?revision=2016-05-17)task-data/mandatory-data", e.getMessage()); + } + + try { + testContainerIsPresent(schemaContext); + fail("Should fail due to missing mandatory node."); + } catch (final IllegalArgumentException e) { + assertEquals( + "Node (foo?revision=2016-05-17)task[{(foo?revision=2016-05-17)task-id=123}] is missing mandatory " + + "descendant /(foo?revision=2016-05-17)task-data/mandatory-data", e.getMessage()); + } + testMandatoryDataLeafIsPresent(schemaContext); + } + + private static void testMultipleContainers() throws ReactorException, DataValidationFailedException { + final SchemaContext schemaContext = TestModel.createTestContext("/bug5830/foo-multiple.yang"); + assertNotNull("Schema context must not be null.", schemaContext); + + testContainerIsNotPresent(schemaContext); + + try { + testContainerIsPresent(schemaContext); + fail("Should fail due to missing mandatory node under present presence container."); + } catch (final IllegalArgumentException e) { + assertTrue(e.getMessage().startsWith( + "Node (foo?revision=2016-05-17)task-data is missing mandatory descendant")); + } + + try { + testMandatoryDataLeafIsPresent(schemaContext); + fail("Should fail due to missing mandatory node under present presence container."); + } catch (final IllegalArgumentException e) { + assertEquals("Node (foo?revision=2016-05-17)task-data " + + "is missing mandatory descendant /(foo?revision=2016-05-17)non-presence-container/" + + "non-presence-container-2/mandatory-leaf-2", e.getMessage()); + } + + testMandatoryLeaf2IsPresent(schemaContext, false); + + try { + testMandatoryLeaf2IsPresent(schemaContext, true); + fail("Should fail due to missing mandatory node under present presence container."); + } catch (final IllegalArgumentException e) { + assertEquals("Node (foo?revision=2016-05-17)presence-container-2 is missing mandatory " + + "descendant /(foo?revision=2016-05-17)mandatory-leaf-3", e.getMessage()); + } + } + + private static void testContainerIsNotPresent(final SchemaContext schemaContext) + throws DataValidationFailedException { + final InMemoryDataTree inMemoryDataTree = initDataTree(schemaContext); + final MapEntryNode taskEntryNode = Builders.mapEntryBuilder() + .withNodeIdentifier(new NodeIdentifierWithPredicates(TASK, TASK_ID, "123")) + .withChild(ImmutableNodes.leafNode(TASK_ID, "123")) + .withChild(ImmutableNodes.leafNode(TASK_MANDATORY_LEAF, "mandatory data")).build(); + + final InMemoryDataTreeModification modificationTree = inMemoryDataTree.takeSnapshot().newModification(); + modificationTree.write( + YangInstanceIdentifier.of(TASK_CONTAINER).node(TASK) + .node(new NodeIdentifierWithPredicates(TASK, TASK_ID, "123")), taskEntryNode); + modificationTree.ready(); + + inMemoryDataTree.validate(modificationTree); + final DataTreeCandidate prepare = inMemoryDataTree.prepare(modificationTree); + inMemoryDataTree.commit(prepare); + } + + private static void testContainerIsPresent(final SchemaContext schemaContext) throws DataValidationFailedException { + final InMemoryDataTree inMemoryDataTree = initDataTree(schemaContext); + + final MapEntryNode taskEntryNode = Builders.mapEntryBuilder() + .withNodeIdentifier(new NodeIdentifierWithPredicates(TASK, TASK_ID, "123")) + .withChild(ImmutableNodes.leafNode(TASK_ID, "123")) + .withChild(ImmutableNodes.leafNode(TASK_MANDATORY_LEAF, "mandatory data")) + .withChild(createTaskDataContainer(false)).build(); + + final InMemoryDataTreeModification modificationTree = inMemoryDataTree.takeSnapshot().newModification(); + modificationTree.write( + YangInstanceIdentifier.of(TASK_CONTAINER).node(TASK) + .node(new NodeIdentifierWithPredicates(TASK, TASK_ID, "123")), taskEntryNode); + modificationTree.ready(); + + inMemoryDataTree.validate(modificationTree); + final DataTreeCandidate prepare = inMemoryDataTree.prepare(modificationTree); + inMemoryDataTree.commit(prepare); + } + + private static void testMandatoryDataLeafIsPresent(final SchemaContext schemaContext) + throws DataValidationFailedException { + final InMemoryDataTree inMemoryDataTree = initDataTree(schemaContext); + + final MapEntryNode taskEntryNode = Builders.mapEntryBuilder() + .withNodeIdentifier(new NodeIdentifierWithPredicates(TASK, TASK_ID, "123")) + .withChild(ImmutableNodes.leafNode(TASK_ID, "123")) + .withChild(ImmutableNodes.leafNode(TASK_MANDATORY_LEAF, "mandatory data")) + .withChild(createTaskDataContainer(true)).build(); + + final InMemoryDataTreeModification modificationTree = inMemoryDataTree.takeSnapshot().newModification(); + modificationTree.write( + YangInstanceIdentifier.of(TASK_CONTAINER).node(TASK) + .node(new NodeIdentifierWithPredicates(TASK, TASK_ID, "123")), taskEntryNode); + modificationTree.ready(); + + inMemoryDataTree.validate(modificationTree); + final DataTreeCandidate prepare = inMemoryDataTree.prepare(modificationTree); + inMemoryDataTree.commit(prepare); + } + + private static void testMandatoryLeaf2IsPresent(final SchemaContext schemaContext, final boolean withPresenceContianer) + throws DataValidationFailedException { + final InMemoryDataTree inMemoryDataTree = initDataTree(schemaContext); + + final MapEntryNode taskEntryNode = Builders.mapEntryBuilder() + .withNodeIdentifier(new NodeIdentifierWithPredicates(TASK, TASK_ID, "123")) + .withChild(ImmutableNodes.leafNode(TASK_ID, "123")) + .withChild(ImmutableNodes.leafNode(TASK_MANDATORY_LEAF, "mandatory data")) + .withChild(createTaskDataMultipleContainer(withPresenceContianer)).build(); + + final InMemoryDataTreeModification modificationTree = inMemoryDataTree.takeSnapshot().newModification(); + modificationTree.write( + YangInstanceIdentifier.of(TASK_CONTAINER).node(TASK) + .node(new NodeIdentifierWithPredicates(TASK, TASK_ID, "123")), taskEntryNode); + modificationTree.ready(); + + inMemoryDataTree.validate(modificationTree); + final DataTreeCandidate prepare = inMemoryDataTree.prepare(modificationTree); + inMemoryDataTree.commit(prepare); + } + + private static DataContainerChild createTaskDataContainer(final boolean withMandatoryNode) { + final DataContainerNodeAttrBuilder taskDataBuilder = Builders.containerBuilder() + .withNodeIdentifier(new NodeIdentifier(TASK_DATA)) + .withChild(ImmutableNodes.leafNode(OTHER_DATA, "foo")); + if (withMandatoryNode) { + taskDataBuilder.withChild(ImmutableNodes.leafNode(MANDATORY_DATA, "mandatory-data-value")); + } + return taskDataBuilder.build(); + } + + private static DataContainerChild createTaskDataMultipleContainer(final boolean withPresenceContianer) { + final DataContainerNodeAttrBuilder nonPresenceContainerBuilder = Builders + .containerBuilder() + .withNodeIdentifier(new NodeIdentifier(NON_PRESENCE_CONTAINER)) + .withChild( + Builders.containerBuilder().withNodeIdentifier(new NodeIdentifier(NON_PRESENCE_CONTAINER_2)) + .withChild(ImmutableNodes.leafNode(MANDATORY_LEAF_2, "mandatory leaf data 2")).build()); + + if (withPresenceContianer) { + nonPresenceContainerBuilder.withChild(Builders.containerBuilder() + .withNodeIdentifier(new NodeIdentifier(PRESENCE_CONTAINER_2)).build()); + } + + final DataContainerNodeAttrBuilder taskDataBuilder = Builders.containerBuilder() + .withNodeIdentifier(new NodeIdentifier(TASK_DATA)) + .withChild(ImmutableNodes.leafNode(OTHER_DATA, "foo")); + taskDataBuilder.withChild(ImmutableNodes.leafNode(MANDATORY_DATA, "mandatory-data-value")); + taskDataBuilder.withChild(nonPresenceContainerBuilder.build()); + + return taskDataBuilder.build(); + } +} diff --git a/yang/yang-data-impl/src/test/resources/bug5830/foo-multiple.yang b/yang/yang-data-impl/src/test/resources/bug5830/foo-multiple.yang new file mode 100644 index 0000000000..7947fae560 --- /dev/null +++ b/yang/yang-data-impl/src/test/resources/bug5830/foo-multiple.yang @@ -0,0 +1,56 @@ +module foo-multiple { + yang-version 1; + namespace "foo"; + prefix foo; + + revision 2016-05-17 { + description "test"; + } + + container task-container { + list task { + key "task-id"; + + leaf task-id { + type string; + } + leaf task-mandatory-leaf { + type string; + mandatory true; + } + + container task-data { + presence "Task data"; + leaf mandatory-data { + type string; + mandatory true; + } + leaf other-data { + type string; + } + container non-presence-container { + container presence-container { + presence "presence container"; + leaf mandatory-leaf { + mandatory true; + type string; + } + } + container non-presence-container-2 { + leaf mandatory-leaf-2 { + mandatory true; + type string; + } + } + container presence-container-2 { + presence "presence container"; + leaf mandatory-leaf-3 { + mandatory true; + type string; + } + } + } + } + } + } +} diff --git a/yang/yang-data-impl/src/test/resources/bug5830/foo-non-presence.yang b/yang/yang-data-impl/src/test/resources/bug5830/foo-non-presence.yang new file mode 100644 index 0000000000..3fc1a72848 --- /dev/null +++ b/yang/yang-data-impl/src/test/resources/bug5830/foo-non-presence.yang @@ -0,0 +1,33 @@ +module foo-non-presence { + yang-version 1; + namespace "foo"; + prefix foo; + + revision 2016-05-17 { + description "test"; + } + + container task-container { + list task { + key "task-id"; + + leaf task-id { + type string; + } + leaf task-mandatory-leaf { + type string; + mandatory true; + } + + container task-data { + leaf mandatory-data { + type string; + mandatory true; + } + leaf other-data { + type string; + } + } + } + } +} diff --git a/yang/yang-data-impl/src/test/resources/bug5830/foo-presence.yang b/yang/yang-data-impl/src/test/resources/bug5830/foo-presence.yang new file mode 100644 index 0000000000..af038278ac --- /dev/null +++ b/yang/yang-data-impl/src/test/resources/bug5830/foo-presence.yang @@ -0,0 +1,34 @@ +module foo-presence { + yang-version 1; + namespace "foo"; + prefix foo; + + revision 2016-05-17 { + description "test"; + } + + container task-container { + list task { + key "task-id"; + + leaf task-id { + type string; + } + leaf task-mandatory-leaf { + type string; + mandatory true; + } + + container task-data { + presence "Task data"; + leaf mandatory-data { + type string; + mandatory true; + } + leaf other-data { + type string; + } + } + } + } +} -- 2.36.6