From 35fed4fb3caf6a7462f680de806389701d78bd6c Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Wed, 7 Jul 2021 11:05:38 +0200 Subject: [PATCH] Revert "Fix mandatory enforcer failure on augmented nodes" This reverts commit 6bcc983bf8875a85e329991f60127b6a6d29cb57. The commit was reverted on master pending rework. Change-Id: I5764983d72f4a9d1d461c0066cfa129d1ee14eb2 Signed-off-by: Robert Varga --- .../impl/schema/tree/MandatoryDescendant.java | 92 ---------- .../schema/tree/MandatoryLeafEnforcer.java | 40 +++-- .../data/impl/schema/tree/YT1276Test.java | 162 ------------------ .../src/test/resources/yt1276.yang | 36 ---- 4 files changed, 22 insertions(+), 308 deletions(-) delete mode 100644 yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MandatoryDescendant.java delete mode 100644 yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/YT1276Test.java delete mode 100644 yang/yang-data-impl/src/test/resources/yt1276.yang diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MandatoryDescendant.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MandatoryDescendant.java deleted file mode 100644 index e5ff14af89..0000000000 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MandatoryDescendant.java +++ /dev/null @@ -1,92 +0,0 @@ -/* - * Copyright (c) 2021 PANTHEON.tech, s.r.o. 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 com.google.common.base.Verify.verify; -import static java.util.Objects.requireNonNull; - -import org.eclipse.jdt.annotation.NonNull; -import org.eclipse.jdt.annotation.Nullable; -import org.opendaylight.yangtools.concepts.Immutable; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; -import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodes; -import org.opendaylight.yangtools.yang.data.util.DataSchemaContextNode; -import org.opendaylight.yangtools.yang.model.api.AugmentationSchemaNode; -import org.opendaylight.yangtools.yang.model.api.AugmentationTarget; -import org.opendaylight.yangtools.yang.model.api.DataNodeContainer; -import org.opendaylight.yangtools.yang.model.api.DataSchemaNode; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * A path to a descendant which must exist. This really should be equivalent to YangInstanceIdentifier, but for now we - * need to deal with up to two possible paths -- one with AugmentationIdentifiers and one without. - */ -// FIXME: 8.0.0: remove this structure and just keep the 'path' YangInstanceIdentifier -final class MandatoryDescendant implements Immutable { - private static final Logger LOG = LoggerFactory.getLogger(MandatoryDescendant.class); - - // Correctly-nested path, mirroring Augmentation and choice/case structure - private final @NonNull YangInstanceIdentifier path; - // Legacy trivial path, - private final @Nullable YangInstanceIdentifier legacyPath; - - private MandatoryDescendant(final YangInstanceIdentifier path, final YangInstanceIdentifier legacyPath) { - this.path = requireNonNull(path); - this.legacyPath = legacyPath; - } - - static @NonNull MandatoryDescendant create(final YangInstanceIdentifier parentId, - final DataNodeContainer parentSchema, final DataSchemaNode childSchema) { - final NodeIdentifier childId = NodeIdentifier.create(childSchema.getQName()); - - if (childSchema.isAugmenting()) { - verify(parentSchema instanceof AugmentationTarget, "Unexpected augmenting child %s in non-target %s at %s", - childSchema, parentSchema, parentId); - - final AugmentationSchemaNode aug = ((AugmentationTarget) parentSchema).getAvailableAugmentations().stream() - .filter(augment -> augment.findDataChildByName(childSchema.getQName()).isPresent()) - .findFirst() - .orElseThrow(() -> new IllegalArgumentException(String.format( - "Node %s is marked as augmenting but is not present in the schema of %s", childSchema.getQName(), - parentSchema))); - - return new MandatoryDescendant( - parentId.node(DataSchemaContextNode.augmentationIdentifierFrom(aug)).node(childId).toOptimized(), - parentId.node(childId).toOptimized()); - } - - return new MandatoryDescendant(parentId.node(childId).toOptimized(), null); - } - - void enforceOnData(final NormalizedNode data) { - // Find try primary path first ... - if (NormalizedNodes.findNode(data, path).isPresent()) { - return; - } - // ... if we have a legacy path, try that as well ... - if (legacyPath != null) { - if (NormalizedNodes.findNode(data, legacyPath).isPresent()) { - // .. this should not really be happening ... - LOG.debug("Found {} at alternate path {}", path, legacyPath); - return; - } - } - - // ... not found, report the error - throw new IllegalArgumentException(String.format("Node %s is missing mandatory descendant %s", - data.getIdentifier(), path)); - } - - @Override - public String toString() { - return legacyPath == null ? path.toString() : "(" + path + " || " + legacyPath + ")"; - } -} diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MandatoryLeafEnforcer.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MandatoryLeafEnforcer.java index 6c99d3bb58..90ae47c986 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MandatoryLeafEnforcer.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MandatoryLeafEnforcer.java @@ -7,6 +7,7 @@ */ package org.opendaylight.yangtools.yang.data.impl.schema.tree; +import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; import com.google.common.collect.ImmutableList; @@ -16,6 +17,7 @@ import org.opendaylight.yangtools.concepts.Immutable; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodes; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeConfiguration; import org.opendaylight.yangtools.yang.data.api.schema.tree.TreeType; import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.TreeNode; @@ -31,56 +33,58 @@ import org.slf4j.LoggerFactory; final class MandatoryLeafEnforcer implements Immutable { private static final Logger LOG = LoggerFactory.getLogger(MandatoryLeafEnforcer.class); - private final ImmutableList mandatoryNodes; + private final ImmutableList mandatoryNodes; - private MandatoryLeafEnforcer(final ImmutableList mandatoryNodes) { + private MandatoryLeafEnforcer(final ImmutableList mandatoryNodes) { this.mandatoryNodes = requireNonNull(mandatoryNodes); } + static Optional forContainer(final DataNodeContainer schema, final DataTreeConfiguration treeConfig) { if (!treeConfig.isMandatoryNodesValidationEnabled()) { return Optional.empty(); } - final Builder builder = ImmutableList.builder(); + final Builder builder = ImmutableList.builder(); findMandatoryNodes(builder, YangInstanceIdentifier.empty(), schema, treeConfig.getTreeType()); - final ImmutableList mandatoryNodes = builder.build(); + final ImmutableList mandatoryNodes = builder.build(); return mandatoryNodes.isEmpty() ? Optional.empty() : Optional.of(new MandatoryLeafEnforcer(mandatoryNodes)); } void enforceOnData(final NormalizedNode data) { - mandatoryNodes.forEach(node -> node.enforceOnData(data)); + for (final YangInstanceIdentifier id : mandatoryNodes) { + checkArgument(NormalizedNodes.findNode(data, id).isPresent(), + "Node %s is missing mandatory descendant %s", data.getIdentifier(), id); + } } void enforceOnTreeNode(final TreeNode tree) { enforceOnData(tree.getData()); } - private static void findMandatoryNodes(final Builder builder, - final YangInstanceIdentifier id, final DataNodeContainer schema, final TreeType type) { + private static void findMandatoryNodes(final Builder builder, + final YangInstanceIdentifier id, final DataNodeContainer schema, final TreeType type) { for (final DataSchemaNode child : schema.getChildNodes()) { if (SchemaAwareApplyOperation.belongsToTree(type, child)) { if (child instanceof ContainerSchemaNode) { final ContainerSchemaNode container = (ContainerSchemaNode) child; if (!container.isPresenceContainer()) { - findMandatoryNodes(builder, - id.node(NodeIdentifier.create(container.getQName())), container, type); + findMandatoryNodes(builder, id.node(NodeIdentifier.create(child.getQName())), container, type); } } else { boolean needEnforce = child instanceof MandatoryAware && ((MandatoryAware) child).isMandatory(); if (!needEnforce && child instanceof ElementCountConstraintAware) { - needEnforce = ((ElementCountConstraintAware) child).getElementCountConstraint() - .map(constraint -> { - final Integer min = constraint.getMinElements(); - return min != null && min > 0; - }) - .orElse(Boolean.FALSE); + needEnforce = ((ElementCountConstraintAware) child) + .getElementCountConstraint().map(constraint -> { + final Integer min = constraint.getMinElements(); + return min != null && min > 0; + }).orElse(Boolean.FALSE).booleanValue(); } if (needEnforce) { - final MandatoryDescendant desc = MandatoryDescendant.create(id, schema, child); - LOG.debug("Adding mandatory child {}", desc); - builder.add(desc); + final YangInstanceIdentifier childId = id.node(NodeIdentifier.create(child.getQName())); + LOG.debug("Adding mandatory child {}", childId); + builder.add(childId.toOptimized()); } } } diff --git a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/YT1276Test.java b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/YT1276Test.java deleted file mode 100644 index 7e138cc929..0000000000 --- a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/YT1276Test.java +++ /dev/null @@ -1,162 +0,0 @@ -/* - * Copyright (c) 2021 PANTHEON.tech, s.r.o. 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.assertThrows; - -import java.util.Set; -import java.util.function.Consumer; -import org.junit.BeforeClass; -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.AugmentationIdentifier; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeConfiguration; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException; -import org.opendaylight.yangtools.yang.data.impl.schema.Builders; -import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; -import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext; -import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils; - -public class YT1276Test { - private static final QName FOO = QName.create("foo", "foo"); - private static final QName BAR = QName.create(FOO, "bar"); - private static final QName BAZ = QName.create(FOO, "baz"); - private static final QName XYZZY_LEAF = QName.create(FOO, "xyzzy-leaf"); - private static final QName XYZZY_AUGMENT = QName.create(FOO, "xyzzy-augment"); - - private static EffectiveModelContext MODEL; - - private final DataTree tree = new InMemoryDataTreeFactory() - .create(DataTreeConfiguration.DEFAULT_CONFIGURATION, MODEL); - - @BeforeClass - public static void beforeClass() { - MODEL = YangParserTestUtils.parseYangResource("/yt1276.yang"); - } - - @Test - public void testFooWithBar() throws DataValidationFailedException { - applyOperation(mod -> { - mod.write(YangInstanceIdentifier.of(FOO), Builders.containerBuilder() - .withNodeIdentifier(new NodeIdentifier(FOO)) - .withChild(Builders.augmentationBuilder() - .withNodeIdentifier(new AugmentationIdentifier(Set.of(BAR))) - .withChild(ImmutableNodes.leafNode(BAR, "xyzzy")) - .build()) - .build()); - }); - } - - @Test - @Deprecated - public void testFooWithBarLegacy() throws DataValidationFailedException { - applyOperation(mod -> { - mod.write(YangInstanceIdentifier.of(FOO), Builders.containerBuilder() - .withNodeIdentifier(new NodeIdentifier(FOO)) - .withChild(ImmutableNodes.leafNode(BAR, "xyzzy")) - .build()); - }); - } - - @Test - public void testFooWithoutBar() { - final IllegalArgumentException ex = assertFailsReady(mod -> { - mod.write(YangInstanceIdentifier.of(FOO), Builders.containerBuilder() - .withNodeIdentifier(new NodeIdentifier(FOO)) - .build()); - }); - assertEquals( - "Node (foo)foo is missing mandatory descendant /AugmentationIdentifier{childNames=[(foo)bar]}/(foo)bar", - ex.getMessage()); - } - - @Test - public void testBarWithXyzzy() throws DataValidationFailedException { - applyOperation(mod -> { - mod.write(YangInstanceIdentifier.of(BAR), Builders.containerBuilder() - .withNodeIdentifier(new NodeIdentifier(BAR)) - .withChild(Builders.choiceBuilder() - .withNodeIdentifier(new NodeIdentifier(BAZ)) - .withChild(ImmutableNodes.leafNode(XYZZY_LEAF, "xyzzy")) - .withChild(Builders.augmentationBuilder() - .withNodeIdentifier(new AugmentationIdentifier(Set.of(XYZZY_AUGMENT))) - .withChild(ImmutableNodes.leafNode(XYZZY_AUGMENT, "xyzzy")) - .build()) - .build()) - .build()); - }); - } - - @Test - @Deprecated - public void testBarWithXyzzyLegacy() throws DataValidationFailedException { - applyOperation(mod -> { - mod.write(YangInstanceIdentifier.of(BAR), Builders.containerBuilder() - .withNodeIdentifier(new NodeIdentifier(BAR)) - .withChild(Builders.choiceBuilder() - .withNodeIdentifier(new NodeIdentifier(BAZ)) - .withChild(ImmutableNodes.leafNode(XYZZY_LEAF, "xyzzy")) - .withChild(ImmutableNodes.leafNode(XYZZY_AUGMENT, "xyzzy")) - .build()) - .build()); - }); - } - - @Test - public void testBarWithoutXyzzyLeaf() { - final IllegalArgumentException ex = assertFailsReady(mod -> { - mod.write(YangInstanceIdentifier.of(BAR), Builders.containerBuilder() - .withNodeIdentifier(new NodeIdentifier(BAR)) - .withChild(Builders.choiceBuilder() - .withNodeIdentifier(new NodeIdentifier(BAZ)) - .withChild(Builders.augmentationBuilder() - .withNodeIdentifier(new AugmentationIdentifier(Set.of(XYZZY_AUGMENT))) - .withChild(ImmutableNodes.leafNode(XYZZY_AUGMENT, "xyzzy")) - .build()) - .build()) - .build()); - }); - assertEquals( - "Node (foo)baz is missing mandatory descendant /(foo)xyzzy-leaf", - ex.getMessage()); - } - - @Test - public void testBarWithoutXyzzyAugment() { - final IllegalArgumentException ex = assertFailsReady(mod -> { - mod.write(YangInstanceIdentifier.of(BAR), Builders.containerBuilder() - .withNodeIdentifier(new NodeIdentifier(BAR)) - .withChild(Builders.choiceBuilder() - .withNodeIdentifier(new NodeIdentifier(BAZ)) - .withChild(ImmutableNodes.leafNode(XYZZY_LEAF, "xyzzy")) - .build()) - .build()); - }); - assertEquals("Node (foo)baz is missing mandatory descendant " - + "/AugmentationIdentifier{childNames=[(foo)xyzzy-augment]}/(foo)xyzzy-augment", - ex.getMessage()); - - } - - private IllegalArgumentException assertFailsReady(final Consumer operation) { - return assertThrows(IllegalArgumentException.class, () -> applyOperation(operation)); - } - - private void applyOperation(final Consumer operation) - throws DataValidationFailedException { - final DataTreeModification mod = tree.takeSnapshot().newModification(); - operation.accept(mod); - mod.ready(); - tree.commit(tree.prepare(mod)); - } -} diff --git a/yang/yang-data-impl/src/test/resources/yt1276.yang b/yang/yang-data-impl/src/test/resources/yt1276.yang deleted file mode 100644 index 33c2979b1c..0000000000 --- a/yang/yang-data-impl/src/test/resources/yt1276.yang +++ /dev/null @@ -1,36 +0,0 @@ -module foo { - namespace foo; - prefix foo; - - container foo { - presence blah; - } - - augment /foo { - leaf bar { - type string; - mandatory true; - } - } - - container bar { - presence blah; - - choice baz { - case xyzzy-case { - leaf xyzzy-leaf { - type string; - mandatory true; - } - } - } - } - - augment /bar/baz/xyzzy-case { - leaf xyzzy-augment { - type string; - mandatory true; - } - } -} - -- 2.36.6