Bug 8922 - Evaluation of if-features is done regardless of ancestors

Evaluation of if-features for a statement is done regardless of its
ancestors. This is not correct and if an ancestor of a statement is
not supported by features, then current statement should be
unsupported too. In other words, if a statement is not supported by
features, then all its children should be unsupported too.

Change-Id: Ieb8a3c32849808a4492f518f4481aaef45c11cae
Signed-off-by: Peter Kajsa <pkajsa@cisco.com>

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/RootStatementContext.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/RootStatementContext.java
index 5832c5e7df4d083476b3ba8b668b8795a355b521..60db37533505a59a4884015a9cf1e8249f85354e 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/RootStatementContext.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/RootStatementContext.java
@@ -268,4 +268,9 @@ public class RootStatementContext<A, D extends DeclaredStatement<A>, E extends E
     protected boolean isIgnoringConfig() {
         return false;
     }
+
+    @Override
+    protected boolean isParentSupportedByFeatures() {
+        return true;
+    }
 }

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java
index 0fd0460571f58e8366d4e0ab999cdfb42fc770a2..c59c81552922bdb33385de73e9832e346fb2b246 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java
@@ -147,24 +147,42 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
 
     @Override
     public boolean isSupportedByFeatures() {
+        if (OptionalBoolean.isPresent(supportedByFeatures)) {
+            return OptionalBoolean.get(supportedByFeatures);
+        }
+
         if (isIgnoringIfFeatures()) {
+            supportedByFeatures = OptionalBoolean.of(true);
             return true;
         }
-        if (OptionalBoolean.isPresent(supportedByFeatures)) {
-            return OptionalBoolean.get(supportedByFeatures);
+
+        final boolean isParentSupported = isParentSupportedByFeatures();
+        /*
+         * If parent is not supported, then this context is also not supported.
+         * So we do not need to check if-features statements of this context and
+         * we can return false immediately.
+         */
+        if (!isParentSupported) {
+            supportedByFeatures = OptionalBoolean.of(false);
+            return false;
         }
 
+        /*
+         * If parent is supported, we need to check if-features statements of
+         * this context.
+         */
         // If the set of supported features has not been provided, all features are supported by default.
         final Set<QName> supportedFeatures = getFromNamespace(SupportedFeaturesNamespace.class,
-            SupportedFeatures.SUPPORTED_FEATURES);
+                SupportedFeatures.SUPPORTED_FEATURES);
         final boolean ret = supportedFeatures == null ? true
-            : StmtContextUtils.checkFeatureSupport(this, supportedFeatures);
+                : StmtContextUtils.checkFeatureSupport(this, supportedFeatures);
 
         supportedByFeatures = OptionalBoolean.of(ret);
         return ret;
-
     }
 
+    protected abstract boolean isParentSupportedByFeatures();
+
     protected abstract boolean isIgnoringIfFeatures();
 
     protected abstract boolean isIgnoringConfig();

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SubstatementContext.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SubstatementContext.java
index 0a2420519b5abc68020f92f0f7d8471106fda345..83215464fa3d9a84df12353af5478b4bc3ad5116 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SubstatementContext.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SubstatementContext.java
@@ -376,4 +376,9 @@ final class SubstatementContext<A, D extends DeclaredStatement<A>, E extends Eff
 
         return ret;
     }
+
+    @Override
+    protected boolean isParentSupportedByFeatures() {
+        return parent.isSupportedByFeatures();
+    }
 }

diff --git a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug8922Test.java b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug8922Test.java
new file mode 100644 (file)
index 0000000..adf893c
--- /dev/null
+++ b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug8922Test.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2017 Cisco Systems, Inc. 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.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import com.google.common.collect.ImmutableSet;
+import org.junit.Test;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.model.api.ContainerSchemaNode;
+import org.opendaylight.yangtools.yang.model.api.SchemaContext;
+import org.opendaylight.yangtools.yang.model.api.SchemaNode;
+import org.opendaylight.yangtools.yang.model.api.SchemaPath;
+import org.opendaylight.yangtools.yang.model.util.SchemaContextUtil;
+
+public class Bug8922Test {
+    private static final String NS = "foo";
+    private static final String REV = "1970-01-01";
+
+    @Test
+    public void testAllFeaturesSupported() throws Exception {
+        final SchemaContext context = StmtTestUtils.parseYangSource("/bugs/bug8922/foo.yang");
+        assertNotNull(context);
+        final SchemaNode findNode = findNode(context, qN("target"), qN("my-con"));
+        assertTrue(findNode instanceof ContainerSchemaNode);
+        assertEquals("New description", ((ContainerSchemaNode) findNode).getDescription());
+    }
+
+    @Test
+    public void testNoFeatureSupported() throws Exception {
+        final SchemaContext context = StmtTestUtils.parseYangSource("/bugs/bug8922/foo.yang", ImmutableSet.of());
+        assertNotNull(context);
+        final SchemaNode findNode = findNode(context, qN("target"), qN("my-con"));
+        assertNull(findNode);
+        assertTrue(context.getAvailableAugmentations().isEmpty());
+    }
+
+    private static SchemaNode findNode(final SchemaContext context, final QName... qNames) {
+        return SchemaContextUtil.findDataSchemaNode(context, SchemaPath.create(true, qNames));
+    }
+
+    private static QName qN(final String localName) {
+        return QName.create(NS, REV, localName);
+    }
+}

diff --git a/yang/yang-parser-impl/src/test/resources/bugs/bug8922/foo.yang b/yang/yang-parser-impl/src/test/resources/bugs/bug8922/foo.yang
new file mode 100644 (file)
index 0000000..891d6bb
--- /dev/null
+++ b/yang/yang-parser-impl/src/test/resources/bugs/bug8922/foo.yang
@@ -0,0 +1,25 @@
+module foo {
+    namespace foo;
+    prefix f;
+
+    feature my-feature;
+
+    container target {
+    }
+
+    augment "/target" {
+        if-feature my-feature;
+        uses my-grp {
+            refine "my-con" {
+                description
+                                "New description";
+            }
+        }
+    }
+
+    grouping my-grp {
+        container my-con {
+            if-feature my-feature;
+        }
+    }
+}