Bug 8831 - Yang 1.1 default values are not checked correctly

When a leaf has a union type, and a default that does not correspond
to the first member of that union type, the default is not processed
correctly and IllegalStateException is thrown during the check of if-feature
statements. The same bug occurs also in case, when the default is a
number, which is in the range of the first member of the union etc..

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

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/TypeUtils.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/TypeUtils.java
index 4e8241307e59e4ed19aa6cbb7dd640eeb9f07f7f..279cd10cdfbce168e1f2e4fe81864a103b421ca2 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/TypeUtils.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/TypeUtils.java
@@ -266,9 +266,6 @@ public final class TypeUtils {
      *
      * @return true if any of specified default values is marked with an
      *         if-feature, otherwise false
-     *
-     * @throws IllegalStateException
-     *             if any of specified default values has not been found
      */
     public static boolean hasDefaultValueMarkedWithIfFeature(final YangVersion yangVersion,
             final TypeEffectiveStatement<?> typeStmt, final Set<String> defaultValues) {
@@ -290,9 +287,6 @@ public final class TypeUtils {
      *
      * @return true if specified default value is marked with an if-feature,
      *         otherwise false
-     *
-     * @throws IllegalStateException
-     *             if specified default value has not been found
      */
     public static boolean hasDefaultValueMarkedWithIfFeature(final YangVersion yangVersion,
             final TypeEffectiveStatement<?> typeStmt, final String defaultValue) {
@@ -315,7 +309,9 @@ public final class TypeUtils {
 
     private static boolean isAnyDefaultValueMarkedWithIfFeature(final TypeEffectiveStatement<?> typeStmt,
             final Set<String> defaultValues) {
-        for (final EffectiveStatement<?, ?> effectiveSubstatement : typeStmt.effectiveSubstatements()) {
+        final Iterator<? extends EffectiveStatement<?, ?>> iter = typeStmt.effectiveSubstatements().iterator();
+        while (iter.hasNext() && !defaultValues.isEmpty()) {
+            final EffectiveStatement<?, ?> effectiveSubstatement = iter.next();
             if (YangStmtMapping.BIT.equals(effectiveSubstatement.statementDefinition())) {
                 final QName bitQName = (QName) effectiveSubstatement.argument();
                 if (defaultValues.remove(bitQName.getLocalName()) && containsIfFeature(effectiveSubstatement)) {
@@ -325,13 +321,12 @@ public final class TypeUtils {
                     && defaultValues.remove(effectiveSubstatement.argument())
                     && containsIfFeature(effectiveSubstatement)) {
                 return true;
-            } else if (effectiveSubstatement instanceof TypeEffectiveStatement) {
-                return isAnyDefaultValueMarkedWithIfFeature((TypeEffectiveStatement<?>) effectiveSubstatement,
-                        defaultValues);
+            } else if (effectiveSubstatement instanceof TypeEffectiveStatement && isAnyDefaultValueMarkedWithIfFeature(
+                    (TypeEffectiveStatement<?>) effectiveSubstatement, defaultValues)) {
+                return true;
             }
         }
 
-        Preconditions.checkState(defaultValues.isEmpty(), "Unable to find following default values %s", defaultValues);
         return false;
     }
 

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/LeafEffectiveStatementImpl.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/LeafEffectiveStatementImpl.java
index 93d2cfc87ca294bf54e1c6c9a47ab14cdf6d5533..605d6782cab38637a196ffcbc992a18e9be27a08 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/LeafEffectiveStatementImpl.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/LeafEffectiveStatementImpl.java
@@ -57,15 +57,10 @@ public final class LeafEffectiveStatementImpl extends AbstractEffectiveDataSchem
             }
         }
 
-        try {
-            SourceException.throwIf(TypeUtils.hasDefaultValueMarkedWithIfFeature(ctx.getRootVersion(), typeStmt, dflt),
-                    ctx.getStatementSourceReference(),
-                    "Leaf '%s' has default value '%s' marked with an if-feature statement.",
-                    ctx.getStatementArgument(), dflt);
-        } catch (final IllegalStateException e) {
-            throw new SourceException(ctx.getStatementSourceReference(), e,
-                    "Unable to find a default value for leaf '%s'", ctx.getStatementArgument());
-        }
+        SourceException.throwIf(TypeUtils.hasDefaultValueMarkedWithIfFeature(ctx.getRootVersion(), typeStmt, dflt),
+                ctx.getStatementSourceReference(),
+                "Leaf '%s' has default value '%s' marked with an if-feature statement.", ctx.getStatementArgument(),
+                dflt);
 
         defaultStr = dflt;
         unitsStr = units;

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/LeafListEffectiveStatementImpl.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/LeafListEffectiveStatementImpl.java
index fc779aef34cf5df68fa91826010f3e1cbc0fd1b5..4b47a7f387cfb6837cf335601c3e78197b475b83 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/LeafListEffectiveStatementImpl.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/LeafListEffectiveStatementImpl.java
@@ -66,16 +66,11 @@ public final class LeafListEffectiveStatementImpl extends AbstractEffectiveDataS
         }
 
         defaultValues = defaultValuesBuilder.build();
-        try {
-            SourceException.throwIf(
-                    TypeUtils.hasDefaultValueMarkedWithIfFeature(ctx.getRootVersion(), typeStmt, defaultValues),
-                    ctx.getStatementSourceReference(),
-                    "Leaf-list '%s' has one of its default values '%s' marked with an if-feature statement.",
-                    ctx.getStatementArgument(), defaultValues);
-        } catch (final IllegalStateException e) {
-            throw new SourceException(ctx.getStatementSourceReference(), e,
-                    "Unable to find a default value for leaf-list '%s'", ctx.getStatementArgument());
-        }
+        SourceException.throwIf(
+                TypeUtils.hasDefaultValueMarkedWithIfFeature(ctx.getRootVersion(), typeStmt, defaultValues),
+                ctx.getStatementSourceReference(),
+                "Leaf-list '%s' has one of its default values '%s' marked with an if-feature statement.",
+                ctx.getStatementArgument(), defaultValues);
 
         type = builder.build();
         userOrdered = isUserOrdered;

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/TypeDefEffectiveStatementImpl.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/TypeDefEffectiveStatementImpl.java
index b2eaea3e6105782e8156409aaf87c2335a39c455..01db20da00173f877453a9ee61815f8a42f8a262 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/TypeDefEffectiveStatementImpl.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/TypeDefEffectiveStatementImpl.java
@@ -64,16 +64,11 @@ public final class TypeDefEffectiveStatementImpl extends AbstractEffectiveSchema
             }
         }
 
-        try {
-            SourceException
-                    .throwIf(TypeUtils.hasDefaultValueMarkedWithIfFeature(ctx.getRootVersion(), typeEffectiveStmt,
-                            defaultValue), ctx.getStatementSourceReference(),
-                            "Typedef '%s' has default value '%s' marked with an if-feature statement.", ctx
-                                    .getStatementArgument(), defaultValue);
-        } catch (final IllegalStateException e) {
-            throw new SourceException(ctx.getStatementSourceReference(), e,
-                    "Unable to find a default value for typedef '%s'", ctx.getStatementArgument());
-        }
+        SourceException.throwIf(
+                TypeUtils.hasDefaultValueMarkedWithIfFeature(ctx.getRootVersion(), typeEffectiveStmt, defaultValue),
+                ctx.getStatementSourceReference(),
+                "Typedef '%s' has default value '%s' marked with an if-feature statement.", ctx.getStatementArgument(),
+                defaultValue);
 
         typeDefinition = builder.build();
     }

diff --git a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/stmt/rfc7950/Bug6901Test.java b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/stmt/rfc7950/Bug6901Test.java
index 52c85e867f0c398f67ddf7e9b97472da8904d407..6e480dfae409a8ab32092a25376898dd16aafa6a 100644 (file)
--- a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/stmt/rfc7950/Bug6901Test.java
+++ b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/stmt/rfc7950/Bug6901Test.java
@@ -12,7 +12,6 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
-import com.google.common.collect.ImmutableSet;
 import org.junit.Test;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 import org.opendaylight.yangtools.yang.parser.spi.meta.SomeModifiersUnresolvedException;
@@ -84,13 +83,11 @@ public class Bug6901Test {
     @Test
     public void unsupportedFeatureTest() throws Exception {
         try {
-            StmtTestUtils.parseYangSource("/rfc7950/bug6901/invalid-foo-enum.yang", ImmutableSet.of());
+            StmtTestUtils.parseYangSource("/rfc7950/bug6901/invalid-foo-enum.yang");
             fail("Test should fail due to invalid Yang 1.1");
         } catch (final SomeModifiersUnresolvedException e) {
-            assertTrue(e.getCause().getMessage()
-                    .startsWith("Unable to find a default value for leaf '(foo?revision=1970-01-01)enum-leaf'"));
-            assertTrue(e.getCause().getCause().getMessage()
-                    .startsWith("Unable to find following default values [two]"));
+            assertTrue(
+                    e.getCause().getMessage().contains("has default value 'two' marked with an if-feature statement"));
         }
     }
 

diff --git a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug8831Test.java b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug8831Test.java
new file mode 100644 (file)
index 0000000..e770d96
--- /dev/null
+++ b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug8831Test.java
@@ -0,0 +1,46 @@
+/*
+ * 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.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import org.junit.Test;
+import org.opendaylight.yangtools.yang.model.api.SchemaContext;
+import org.opendaylight.yangtools.yang.parser.spi.meta.SomeModifiersUnresolvedException;
+
+public class Bug8831Test {
+    @Test
+    public void test() throws Exception {
+        final SchemaContext context = TestUtils.parseYangSources("/bugs/bug8831/valid");
+        assertNotNull(context);
+    }
+
+    @Test
+    public void invalidModelsTest() throws Exception {
+        try {
+            TestUtils.parseYangSource("/bugs/bug8831/invalid/inv-model.yang");
+            fail("Test should fails due to invalid yang 1.1 model");
+        } catch (final SomeModifiersUnresolvedException e) {
+            assertTrue(
+                    e.getCause().getMessage().contains("has default value 'any' marked with an if-feature statement"));
+        }
+    }
+
+    @Test
+    public void invalidModelsTest2() throws Exception {
+        try {
+            TestUtils.parseYangSource("/bugs/bug8831/invalid/inv-model2.yang");
+            fail("Test should fails due to invalid yang 1.1 model");
+        } catch (final SomeModifiersUnresolvedException e) {
+            assertTrue(
+                    e.getCause().getMessage().contains("has default value 'any' marked with an if-feature statement"));
+        }
+    }
+}

diff --git a/yang/yang-parser-impl/src/test/resources/bugs/bug8831/invalid/inv-model.yang b/yang/yang-parser-impl/src/test/resources/bugs/bug8831/invalid/inv-model.yang
new file mode 100644 (file)
index 0000000..6b8e724
--- /dev/null
+++ b/yang/yang-parser-impl/src/test/resources/bugs/bug8831/invalid/inv-model.yang
@@ -0,0 +1,29 @@
+module inv-model {
+    yang-version 1.1;
+    namespace "http://www.inv-model.com";
+    prefix ex;
+
+    revision 2017-07-10;
+
+    feature my-feature {
+        description "my feature";
+    }
+
+    typedef enum-last {
+        type union {
+            type uint16;
+            type enumeration {
+                enum "any" {
+                    if-feature my-feature;
+                }
+            }
+        }
+    }
+
+    container top {
+        leaf enum-last-leaf {
+            type enum-last;
+            default "any";
+        }
+    }
+}

diff --git a/yang/yang-parser-impl/src/test/resources/bugs/bug8831/invalid/inv-model2.yang b/yang/yang-parser-impl/src/test/resources/bugs/bug8831/invalid/inv-model2.yang
new file mode 100644 (file)
index 0000000..246976c
--- /dev/null
+++ b/yang/yang-parser-impl/src/test/resources/bugs/bug8831/invalid/inv-model2.yang
@@ -0,0 +1,29 @@
+module inv-model2 {
+    yang-version 1.1;
+    namespace "http://www.inv-model2.com";
+    prefix ex;
+
+    revision 2017-07-10;
+
+    feature my-feature {
+        description "my feature";
+    }
+
+    typedef enum-first {
+        type union {
+            type enumeration {
+                enum "any" {
+                    if-feature my-feature;
+                }
+            }
+            type uint16;
+        }
+    }
+
+    container top {
+        leaf enum-first-leaf {
+            type enum-first;
+            default "any";
+        }
+    }
+}

diff --git a/yang/yang-parser-impl/src/test/resources/bugs/bug8831/valid/example-model.yang b/yang/yang-parser-impl/src/test/resources/bugs/bug8831/valid/example-model.yang
new file mode 100644 (file)
index 0000000..715f87b
--- /dev/null
+++ b/yang/yang-parser-impl/src/test/resources/bugs/bug8831/valid/example-model.yang
@@ -0,0 +1,36 @@
+module example-model {
+    yang-version 1.1;
+    namespace "http://www.example.com";
+    prefix ex;
+
+    revision 2017-07-10;
+
+    typedef enum-first {
+        type union {
+            type enumeration {
+                enum "any";
+            }
+            type uint16;
+        }
+    }
+
+    typedef enum-last {
+        type union {
+            type uint16;
+            type enumeration {
+                enum "any";
+            }
+        }
+    }
+
+    container top {
+        leaf enum-first-leaf {
+            type enum-first;
+            default "any";
+        }
+        leaf enum-last-leaf {
+            type enum-last;
+            default "any";
+        }
+    }
+}

diff --git a/yang/yang-parser-impl/src/test/resources/bugs/bug8831/valid/example-model2.yang b/yang/yang-parser-impl/src/test/resources/bugs/bug8831/valid/example-model2.yang
new file mode 100644 (file)
index 0000000..3b39c75
--- /dev/null
+++ b/yang/yang-parser-impl/src/test/resources/bugs/bug8831/valid/example-model2.yang
@@ -0,0 +1,29 @@
+module example-model2 {
+    yang-version 1.1;
+    namespace "http://www.example2.com";
+    prefix ex2;
+
+    revision 2017-07-10;
+
+    feature my-feature {
+        description "my feature";
+    }
+
+    typedef enum-last {
+        type union {
+            type uint16;
+            type enumeration {
+                enum "any" {
+                    if-feature my-feature;
+                }
+            }
+        }
+    }
+
+    container top {
+        leaf enum-last-leaf {
+            type enum-last;
+            default "16";
+        }
+    }
+}