From 4ad4e83aa9150c6c32af1e04fc879e844edae5cd Mon Sep 17 00:00:00 2001 From: Sangwook Ha Date: Sat, 3 Dec 2022 22:00:47 -0800 Subject: [PATCH] Do not suppress 'uses' node effects Parser tries to process an 'augment' statement with 'if-feature' and fails when the feature is not supported and the 'augment' statement targets a node defined with a nested grouping and dependent on the feature. Add a test case (4th one) to reproduce the issue. The other test cases show that there is no parsing issue if the augmentation target node is defined with a single grouping or feature is supported. The problem here is not AugmentStatementSupport, but rather UsesStatementSupport, which fails to propagate children when if-feature is in effect -- hence those children never make it to the schema tree, and hence fail to be looked up. Use 'uses' statement's if-feature to mark any children as unsupporte, hence they are reflected in namespaces, but they will not be built. JIRA: YANGTOOLS-1471 Change-Id: Ibd86530388a99ed3c8ade7bf9af109c6a0cfb80a Signed-off-by: Sangwook Ha Signed-off-by: Robert Varga --- .../stmt/uses/UsesStatementSupport.java | 6 +- .../yangtools/yang/stmt/AbstractYangTest.java | 12 +++- .../yangtools/yang/stmt/TestUtils.java | 22 +++++- .../yangtools/yang/stmt/YT1471Test.java | 69 +++++++++++++++++++ .../resources/bugs/YT1471/nested/foo.yang | 28 ++++++++ .../resources/bugs/YT1471/single/foo.yang | 24 +++++++ 6 files changed, 152 insertions(+), 9 deletions(-) create mode 100644 parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1471Test.java create mode 100644 parser/yang-parser-rfc7950/src/test/resources/bugs/YT1471/nested/foo.yang create mode 100644 parser/yang-parser-rfc7950/src/test/resources/bugs/YT1471/single/foo.yang diff --git a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/uses/UsesStatementSupport.java b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/uses/UsesStatementSupport.java index 63d9c2c395..3110ce8a80 100644 --- a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/uses/UsesStatementSupport.java +++ b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/uses/UsesStatementSupport.java @@ -87,9 +87,6 @@ public final class UsesStatementSupport @Override public void onFullDefinitionDeclared(final Mutable usesNode) { - if (!usesNode.isSupportedByFeatures()) { - return; - } super.onFullDefinitionDeclared(usesNode); final var usesAction = usesNode.newInferenceAction(ModelProcessingPhase.EFFECTIVE_MODEL); @@ -195,11 +192,12 @@ public final class UsesStatementSupport final var effective = sourceGrpStmtCtx.effectiveSubstatements(); final var buffer = new ArrayList>(declared.size() + effective.size()); final var newQNameModule = getNewQNameModule(targetCtx, sourceGrpStmtCtx); + final var unsupported = !usesNode.isSupportedByFeatures(); for (var original : declared) { if (shouldCopy(original)) { original.copyAsChildOf(targetCtx, CopyType.ADDED_BY_USES, newQNameModule).ifPresent(copy -> { - if (!original.isSupportedByFeatures() || !original.isSupportedToBuildEffective()) { + if (unsupported || !original.isSupportedByFeatures() || !original.isSupportedToBuildEffective()) { copy.setUnsupported(); } buffer.add(copy); diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/AbstractYangTest.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/AbstractYangTest.java index 4f389f691d..e2068e0d15 100644 --- a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/AbstractYangTest.java +++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/AbstractYangTest.java @@ -14,8 +14,11 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThrows; import com.google.common.base.Throwables; +import java.util.Set; import org.eclipse.jdt.annotation.NonNull; +import org.eclipse.jdt.annotation.Nullable; import org.hamcrest.Matcher; +import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext; import org.opendaylight.yangtools.yang.model.ri.type.InvalidBitDefinitionException; import org.opendaylight.yangtools.yang.model.ri.type.InvalidEnumDefinitionException; @@ -41,11 +44,16 @@ public abstract class AbstractYangTest { return ret; } - @SuppressWarnings("checkstyle:illegalCatch") public static @NonNull EffectiveModelContext assertEffectiveModelDir(final String resourceDirName) { + return assertEffectiveModelDir(resourceDirName, null); + } + + @SuppressWarnings("checkstyle:illegalCatch") + public static @NonNull EffectiveModelContext assertEffectiveModelDir(final String resourceDirName, + final @Nullable Set supportedFeatures) { final EffectiveModelContext ret; try { - ret = TestUtils.loadModules(resourceDirName); + ret = TestUtils.loadModules(resourceDirName, supportedFeatures); } catch (Exception e) { Throwables.throwIfUnchecked(e); throw new AssertionError("Failed to assemble effective model of " + resourceDirName, e); diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/TestUtils.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/TestUtils.java index 0d07b374dd..ccd2213953 100644 --- a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/TestUtils.java +++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/TestUtils.java @@ -14,7 +14,10 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Set; import org.eclipse.jdt.annotation.NonNull; +import org.eclipse.jdt.annotation.Nullable; +import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext; import org.opendaylight.yangtools.yang.model.api.Module; import org.opendaylight.yangtools.yang.model.api.ModuleImport; @@ -55,11 +58,24 @@ public final class TestUtils { return loadModules(TestUtils.class, resourceDirectory); } + public static EffectiveModelContext loadModules(final String resourceDirectory, + final @Nullable Set supportedFeatures) throws Exception { + return loadModules(TestUtils.class, resourceDirectory, supportedFeatures); + } + public static EffectiveModelContext loadModules(final Class cls, final String resourceDirectory) throws Exception { - return RFC7950Reactors.defaultReactor().newBuild() - .addSources(loadSources(cls, resourceDirectory)) - .buildEffective(); + return loadModules(cls, resourceDirectory, null); + } + + public static EffectiveModelContext loadModules(final Class cls, final String resourceDirectory, + final @Nullable Set supportedFeatures) throws Exception { + final var action = RFC7950Reactors.defaultReactor().newBuild() + .addSources(loadSources(cls, resourceDirectory)); + if (supportedFeatures != null) { + action.setSupportedFeatures(supportedFeatures); + } + return action.buildEffective(); } public static EffectiveModelContext parseYangSource(final String... yangSourceFilePath) throws Exception { diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1471Test.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1471Test.java new file mode 100644 index 0000000000..bda40b9e6b --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1471Test.java @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2022 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.CoreMatchers.instanceOf; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.junit.Assert.assertEquals; + +import java.util.List; +import java.util.Set; +import org.junit.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; +import org.opendaylight.yangtools.yang.model.api.stmt.UsesEffectiveStatement; + +public class YT1471Test extends AbstractYangTest { + private static final QName FOO = QName.create("urn:foo", "foo"); + private static final QName BAR = QName.create("urn:foo", "bar"); + private static final QName BAZ = QName.create("urn:foo", "baz"); + private static final QName BAZ_LEAF = QName.create("urn:foo", "baz-leaf"); + + @Test + public void testAugmentSingleGroupingWithFeatureSupported() { + assertSupportedFoo("single"); + } + + @Test + public void testAugmentSingleGroupingWithFeatureNotSupported() { + assertEquals(List.of(), assertFoo("single", Set.of()).effectiveSubstatements()); + } + + @Test + public void testAugmentNestedGroupingWithFeatureSupported() { + assertSupportedFoo("nested"); + } + + @Test + public void testAugmentNestedGroupingWithFeatureNotSupported() { + assertThat(assertFoo("nested", Set.of()).effectiveSubstatements(), + contains(instanceOf(UsesEffectiveStatement.class))); + } + + private static ContainerEffectiveStatement assertFoo(final String dirName, final Set supportedFeatures) { + final var foo = assertEffectiveModelDir("/bugs/YT1471/" + dirName, supportedFeatures) + .findModuleStatement(FOO).orElseThrow() + .findFirstEffectiveSubstatement(ContainerEffectiveStatement.class).orElseThrow(); + assertEquals(FOO, foo.argument()); + return foo; + } + + private static void assertSupportedFoo(final String dirName) { + final var bar = assertFoo(dirName, null) + .findFirstEffectiveSubstatement(ContainerEffectiveStatement.class).orElseThrow(); + assertEquals(BAR, bar.argument()); + + final var baz = bar.findFirstEffectiveSubstatement(ContainerEffectiveStatement.class).orElseThrow(); + assertEquals(BAZ, baz.argument()); + + final var bazLeaf = baz.findFirstEffectiveSubstatement(LeafEffectiveStatement.class).orElseThrow(); + assertEquals(BAZ_LEAF, bazLeaf.argument()); + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1471/nested/foo.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1471/nested/foo.yang new file mode 100644 index 0000000000..d59c6464b1 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1471/nested/foo.yang @@ -0,0 +1,28 @@ +module foo { + namespace "urn:foo"; + prefix "foo"; + + feature bar-feature; + + grouping baz-group { + container baz; + } + + grouping bar-group { + container bar { + if-feature bar-feature; + uses baz-group; + } + } + + container foo { + uses bar-group; + } + + augment "/foo/bar/baz" { + if-feature bar-feature; + leaf baz-leaf { + type string; + } + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1471/single/foo.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1471/single/foo.yang new file mode 100644 index 0000000000..faa7a2a248 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1471/single/foo.yang @@ -0,0 +1,24 @@ +module foo { + namespace "urn:foo"; + prefix "foo"; + + feature bar-feature; + + grouping baz-group { + container baz; + } + + container foo { + container bar { + if-feature bar-feature; + uses baz-group; + } + } + + augment "/foo/bar/baz" { + if-feature bar-feature; + leaf baz-leaf { + type string; + } + } +} -- 2.36.6