Fix transitive if-feature augments

We cannot just ignore augments, as they drive namespace population.
Instead of that, let's populate target nodes, but make sure they
are marked as unsupported.

This allows ChildSchemaNodeNamespace-driven lookups to find the resulting
node, but does not allow it to materialize as an EffectiveStatement.

JIRA: YANGTOOLS-859
Change-Id: Ib817e6e20563005427471098bf6ff2991fb60e01
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/yang/yang-parser-rfc7950/src/main/java/module-info.java b/yang/yang-parser-rfc7950/src/main/java/module-info.java
index 3bcdf030158e7134c9c62cff723ead78087009c8..18fc8de10e5dd6bbdc5785abaa5bf20205e9fa3d 100644 (file)
--- a/yang/yang-parser-rfc7950/src/main/java/module-info.java
+++ b/yang/yang-parser-rfc7950/src/main/java/module-info.java
@@ -27,6 +27,7 @@ module org.opendaylight.yangtools.yang.parser.rfc7950 {
     requires org.opendaylight.yangtools.yang.parser.reactor;
     requires org.opendaylight.yangtools.util;
     requires org.slf4j;
+    requires org.opendaylight.yangtools.yang.model.api;
 
     // FIXME: hide these
     exports org.opendaylight.yangtools.yang.parser.rfc7950.stmt.action;

diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java
index a432c6507bbe6b49453e875b64ec9a233aead6bc..abbd44738770cd42d137a034ae3e80c52a610b73 100644 (file)
--- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java
+++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java
@@ -78,7 +78,8 @@ abstract class AbstractAugmentStatementSupport
     public final void onFullDefinitionDeclared(
             final Mutable<SchemaNodeIdentifier, AugmentStatement, AugmentEffectiveStatement> augmentNode) {
         if (!augmentNode.isSupportedByFeatures()) {
-            return;
+            // We need this augment node to be present, but it should not escape to effective world
+            augmentNode.setIsSupportedToBuildEffective(false);
         }
 
         super.onFullDefinitionDeclared(augmentNode);
@@ -210,18 +211,18 @@ abstract class AbstractAugmentStatementSupport
          */
         final boolean skipCheckOfMandatoryNodes = YangVersion.VERSION_1_1.equals(sourceCtx.getRootVersion())
                 && isConditionalAugmentStmt(sourceCtx);
+        final boolean unsupported = !sourceCtx.isSupportedByFeatures();
 
         final Collection<? extends Mutable<?, ?, ?>> declared = sourceCtx.mutableDeclaredSubstatements();
         final Collection<? extends Mutable<?, ?, ?>> effective = sourceCtx.mutableEffectiveSubstatements();
         final Collection<Mutable<?, ?, ?>> buffer = new ArrayList<>(declared.size() + effective.size());
 
         for (final Mutable<?, ?, ?> originalStmtCtx : declared) {
-            if (originalStmtCtx.isSupportedByFeatures()) {
-                copyStatement(originalStmtCtx, targetCtx, typeOfCopy, buffer, skipCheckOfMandatoryNodes);
-            }
+            copyStatement(originalStmtCtx, targetCtx, typeOfCopy, buffer, skipCheckOfMandatoryNodes,
+                unsupported || !originalStmtCtx.isSupportedByFeatures());
         }
         for (final Mutable<?, ?, ?> originalStmtCtx : effective) {
-            copyStatement(originalStmtCtx, targetCtx, typeOfCopy, buffer, skipCheckOfMandatoryNodes);
+            copyStatement(originalStmtCtx, targetCtx, typeOfCopy, buffer, skipCheckOfMandatoryNodes, unsupported);
         }
 
         targetCtx.addEffectiveSubstatements(buffer);
@@ -247,12 +248,18 @@ abstract class AbstractAugmentStatementSupport
 
     private static void copyStatement(final Mutable<?, ?, ?> original, final StatementContextBase<?, ?, ?> target,
             final CopyType typeOfCopy, final Collection<Mutable<?, ?, ?>> buffer,
-            final boolean skipCheckOfMandatoryNodes) {
+            final boolean skipCheckOfMandatoryNodes, final boolean unsupported) {
+        // We always copy statements, but if either the source statement or the augmentation which causes it are not
+        // supported to build we also mark the target as such.
         if (needToCopyByAugment(original)) {
             validateNodeCanBeCopiedByAugment(original, target, typeOfCopy, skipCheckOfMandatoryNodes);
 
-            buffer.add(target.childCopyOf(original, typeOfCopy));
-        } else if (isReusedByAugment(original)) {
+            final Mutable<?, ?, ?> copy = target.childCopyOf(original, typeOfCopy);
+            if (unsupported) {
+                copy.setIsSupportedToBuildEffective(false);
+            }
+            buffer.add(copy);
+        } else if (isReusedByAugment(original) && !unsupported) {
             buffer.add(original);
         }
     }

diff --git a/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT859Test.java b/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT859Test.java
new file mode 100644 (file)
index 0000000..4898c38
--- /dev/null
+++ b/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT859Test.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2020 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.stmt;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import java.util.Optional;
+import org.junit.Test;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
+import org.opendaylight.yangtools.yang.model.api.ListSchemaNode;
+import org.opendaylight.yangtools.yang.model.api.SchemaContext;
+import org.opendaylight.yangtools.yang.model.repo.api.StatementParserMode;
+
+public class YT859Test {
+    @Test
+    public void testAugmentUnsupported() throws Exception {
+        final SchemaContext context = StmtTestUtils.parseYangSources("/bugs/YT859/", ImmutableSet.of(),
+            StatementParserMode.DEFAULT_MODE);
+        assertEquals(4, context.getModules().size());
+
+        final DataSchemaNode named = Iterables.getOnlyElement(context.findModules("xyzzy"))
+            .findDataChildByName(QName.create("xyzzy", "xyzzy"), QName.create("xyzzy", "named")).orElseThrow();
+        assertThat(named, instanceOf(ListSchemaNode.class));
+        assertEquals(Optional.empty(), ((ListSchemaNode) named).findDataChildByName(QName.create("foo", "foo")));
+    }
+}

diff --git a/yang/yang-parser-rfc7950/src/test/resources/bugs/YT859/bar.yang b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT859/bar.yang
new file mode 100644 (file)
index 0000000..a0d1ef3
--- /dev/null
+++ b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT859/bar.yang
@@ -0,0 +1,18 @@
+module bar {
+  yang-version 1.1;
+  namespace bar;
+  prefix bar;
+
+  import xyzzy { prefix xyzzy; }
+  import foo { prefix foo; }
+
+  augment /xyzzy:xyzzy/xyzzy:named/foo:foo {
+    container bar;
+  }
+
+  deviation /xyzzy:xyzzy/xyzzy:named/foo:foo {
+    deviate add {
+      config false;
+    } 
+  }
+}

diff --git a/yang/yang-parser-rfc7950/src/test/resources/bugs/YT859/baz.yang b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT859/baz.yang
new file mode 100644 (file)
index 0000000..502329d
--- /dev/null
+++ b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT859/baz.yang
@@ -0,0 +1,21 @@
+module baz {
+  yang-version 1.1;
+  namespace baz;
+  prefix baz;
+
+  import xyzzy { prefix xyzzy; }
+  import foo { prefix foo; }
+  import bar { prefix bar; }
+
+  augment /xyzzy:xyzzy/xyzzy:named/foo:foo/bar:bar {
+    leaf baz {
+      type string;
+    }
+  }
+
+  deviation /xyzzy:xyzzy/xyzzy:named/foo:foo/bar:bar {
+    deviate add {
+      config false;
+    }
+  }
+}

diff --git a/yang/yang-parser-rfc7950/src/test/resources/bugs/YT859/foo.yang b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT859/foo.yang
new file mode 100644 (file)
index 0000000..95d7a6c
--- /dev/null
+++ b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT859/foo.yang
@@ -0,0 +1,19 @@
+module foo {
+  yang-version 1.1;
+  namespace foo;
+  prefix foo;
+
+  import xyzzy { prefix xyzzy; }
+
+  feature foo;
+
+  container foo {
+    if-feature foo;
+  }
+
+  augment /xyzzy:xyzzy/xyzzy:named {
+    container foo {
+      if-feature foo;
+    }
+  }
+}

diff --git a/yang/yang-parser-rfc7950/src/test/resources/bugs/YT859/xyzzy.yang b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT859/xyzzy.yang
new file mode 100644 (file)
index 0000000..4320e71
--- /dev/null
+++ b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT859/xyzzy.yang
@@ -0,0 +1,15 @@
+module xyzzy {
+  namespace xyzzy;
+  prefix xyzzy;
+
+  container xyzzy {
+
+    list named {
+      key name;
+
+      leaf name {
+        type string;
+      }
+    }
+  }
+}