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 <sangwook.ha@verizon.com>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 4ad4e83aa9150c6c32af1e04fc879e844edae5cd)
(cherry picked from commit 3d3cd8719491b8bdbdcfc18b66e390cc46d01abd)

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 d4d37aa1724e6c28a3974ea763c1508a42df853d..cfd3e54ac5a65c007afb554337c3303d9986b752 100644 (file)
--- 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
@@ -89,9 +89,6 @@ public final class UsesStatementSupport
 
     @Override
     public void onFullDefinitionDeclared(final Mutable<QName, UsesStatement, UsesEffectiveStatement> usesNode) {
-        if (!usesNode.isSupportedByFeatures()) {
-            return;
-        }
         super.onFullDefinitionDeclared(usesNode);
 
         final var usesAction = usesNode.newInferenceAction(ModelProcessingPhase.EFFECTIVE_MODEL);
@@ -198,11 +195,12 @@ public final class UsesStatementSupport
         final var effective = sourceGrpStmtCtx.effectiveSubstatements();
         final var buffer = new ArrayList<Mutable<?, ?, ?>>(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 4f389f691dd0fd723344138991c822bf371c910f..e2068e0d1533ded2f91f32e745285603de4c2d98 100644 (file)
--- 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<QName> 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 0d07b374dd6d01d307eeb15377f27b2a69b8b148..ccd2213953828ffd9969979be43cb9c1fc75b43d 100644 (file)
--- 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<QName> 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<QName> 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 (file)
index 0000000..bda40b9
--- /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<QName> 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 (file)
index 0000000..d59c646
--- /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 (file)
index 0000000..faa7a2a
--- /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;
+    }
+  }
+}