From 46497a560b73b86f51936e3326881657e523185f Mon Sep 17 00:00:00 2001 From: Sangwook Ha Date: Mon, 30 Jan 2023 13:50:33 -0800 Subject: [PATCH] Fix augment/deviate mechanics Parsing of YANG models with a deviation fails if the deviation target node is conditionally augmented based on a feature. The problem is we are reusing are reusing setUnsupported() for two cases: when the feature is not supported and when the target is not available. Separate the second case into a separate boolean, and if the target is available, propagate children to target as unsupported -- which makes namespace resolution work correctly and deviate properly sees the children as unsupported. JIRA: YANGTOOLS-1485 Change-Id: I954185dd0067667faae8073e222f07b65907e675 Signed-off-by: Sangwook Ha Signed-off-by: Robert Varga --- .../stmt/augment/AugmentInferenceAction.java | 9 ++- .../yangtools/yang/stmt/YT1485Test.java | 55 +++++++++++++++++++ .../resources/bugs/YT1485/augment/bar.yang | 12 ++++ .../resources/bugs/YT1485/augment/foo.yang | 17 ++++++ .../test/resources/bugs/YT1485/leaf/bar.yang | 12 ++++ .../test/resources/bugs/YT1485/leaf/foo.yang | 13 +++++ 6 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1485Test.java create mode 100644 parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/augment/bar.yang create mode 100644 parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/augment/foo.yang create mode 100644 parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/leaf/bar.yang create mode 100644 parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/leaf/foo.yang diff --git a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AugmentInferenceAction.java b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AugmentInferenceAction.java index 338afb78b8..54ee72d8ab 100644 --- a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AugmentInferenceAction.java +++ b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AugmentInferenceAction.java @@ -52,6 +52,8 @@ final class AugmentInferenceAction implements InferenceAction { private final Prerequisite>> target; private final AbstractAugmentStatementSupport statementSupport; + private boolean targetUnavailable; + AugmentInferenceAction(final AbstractAugmentStatementSupport statementSupport, final Mutable augmentNode, final Prerequisite>> target) { @@ -62,8 +64,9 @@ final class AugmentInferenceAction implements InferenceAction { @Override public void apply(final InferenceContext ctx) { - if (!augmentNode.isSupportedToBuildEffective()) { - // We are not building effective model, hence we should not be performing any effects + if (targetUnavailable) { + // Target node is not available, no further processing and the augment should not leak into effective world + augmentNode.setUnsupported(); return; } @@ -112,7 +115,7 @@ final class AugmentInferenceAction implements InferenceAction { @Override public void prerequisiteUnavailable(final Prerequisite unavail) { if (target.equals(unavail)) { - augmentNode.setUnsupported(); + targetUnavailable = true; } else { prerequisiteFailed(List.of(unavail)); } diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1485Test.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1485Test.java new file mode 100644 index 0000000000..4c0dbb0a45 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1485Test.java @@ -0,0 +1,55 @@ +/* + * Copyright (c) 2023 Verizon 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.stmt; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.List; +import java.util.Set; +import org.junit.jupiter.api.Test; +import org.opendaylight.yangtools.yang.common.QName; +import org.opendaylight.yangtools.yang.model.api.stmt.ContainerEffectiveStatement; +import org.opendaylight.yangtools.yang.model.api.stmt.LeafEffectiveStatement; + +class YT1485Test extends AbstractYangTest { + private static final QName FOO = QName.create("urn:foo", "foo"); + + @Test + void testLeafFooFeatureSupported() { + assertEquals(List.of(), assertFoo("leaf", null).effectiveSubstatements()); + } + + @Test + void testLeafFooFeatureNotSupported() { + assertEquals(List.of(), assertFoo("leaf", Set.of()).effectiveSubstatements()); + } + + @Test + void testAugmentFooFeatureSupported() { + assertThat(assertFoo("augment", null).effectiveSubstatements(), + not(hasItem(instanceOf(LeafEffectiveStatement.class)))); + } + + @Test + void testAugmentFooFeatureNotSupported() { + assertEquals(List.of(), assertFoo("augment", Set.of()).effectiveSubstatements()); + } + + private static ContainerEffectiveStatement assertFoo(final String dirName, + final Set supportedFeatures) { + final var foo = assertEffectiveModelDir("/bugs/YT1485/" + dirName, supportedFeatures) + .findModuleStatement(FOO).orElseThrow() + .findFirstEffectiveSubstatement(ContainerEffectiveStatement.class).orElseThrow(); + assertEquals(FOO, foo.argument()); + return foo; + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/augment/bar.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/augment/bar.yang new file mode 100644 index 0000000000..e3b49f67f7 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/augment/bar.yang @@ -0,0 +1,12 @@ +module bar { + namespace "urn:bar"; + prefix "bar"; + + import foo { + prefix foo; + } + + deviation /foo:foo/foo:foo-leaf { + deviate not-supported; + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/augment/foo.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/augment/foo.yang new file mode 100644 index 0000000000..7cb42be1d0 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/augment/foo.yang @@ -0,0 +1,17 @@ +module foo { + namespace "urn:foo"; + prefix "foo"; + + feature foo-feature; + + container foo { + } + + augment /foo:foo { + if-feature foo-feature; + + leaf foo-leaf { + type string; + } + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/leaf/bar.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/leaf/bar.yang new file mode 100644 index 0000000000..e3b49f67f7 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/leaf/bar.yang @@ -0,0 +1,12 @@ +module bar { + namespace "urn:bar"; + prefix "bar"; + + import foo { + prefix foo; + } + + deviation /foo:foo/foo:foo-leaf { + deviate not-supported; + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/leaf/foo.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/leaf/foo.yang new file mode 100644 index 0000000000..7f3c026241 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1485/leaf/foo.yang @@ -0,0 +1,13 @@ +module foo { + namespace "urn:foo"; + prefix "foo"; + + feature foo-feature; + + container foo { + leaf foo-leaf { + if-feature foo-feature; + type string; + } + } +} -- 2.36.6