Fix ConstraintDefinition inconsistency 06/45806/5
authorRobert Varga <rovarga@cisco.com>
Mon, 19 Sep 2016 12:49:42 +0000 (14:49 +0200)
committerRobert Varga <nite@hq.sk>
Mon, 19 Sep 2016 14:52:33 +0000 (14:52 +0000)
The API contract around min/max elements is unclear, but the intent
is to provide a simple 'no effective constraint' result. This is
represented by a null return, which is in fact what the users expect.

Clarify the API contract and fix the two implementations to follow
it instead of using 0/MAX_INT for constraints.

Change-Id: I84b2e2714ddd017f319be4fc6dd0f7bec02bbf0d
Signed-off-by: Robert Varga <rovarga@cisco.com>
yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/ConstraintDefinition.java
yang/yang-model-export/src/test/resources/schema-context-emitter-test/foo.xml
yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EffectiveConstraintDefinitionImpl.java
yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EmptyConstraintDefinition.java
yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/GroupingTest.java
yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/YangParserSimpleTest.java

index bf1694b6d2e0e4444f89c8f9b394edfaa0d36c53..30b690df52bf79ee0b093c2066d0d37b0fd09e29 100644 (file)
@@ -8,6 +8,7 @@
 package org.opendaylight.yangtools.yang.model.api;
 
 import java.util.Set;
+import javax.annotation.Nullable;
 
 /**
  * Contains method which returns various data constraints for some YANG element
@@ -63,9 +64,9 @@ public interface ConstraintDefinition {
      *
      * It is used with YANG statements leaf-list, list, deviate.
      *
-     * @return integer with minimal number of elements
+     * @return integer with minimal number of elements, or null if no minimum is defined
      */
-    Integer getMinElements();
+    @Nullable Integer getMinElements();
 
     /**
      * Returns the maximum admissible number of data elements for node where
@@ -76,7 +77,7 @@ public interface ConstraintDefinition {
      *
      * It is used with YANG statements leaf-list, list, deviate.
      *
-     * @return integer with maximum number of elements
+     * @return integer with maximum number of elements, or null if no maximum is defined
      */
-    Integer getMaxElements();
+    @Nullable Integer getMaxElements();
 }
index 52ffe1a036ba5c7ff51a844f3da253e23d0a6e9f..4b39d76f9eff9d728faee8c5def769de5b61f1c7 100644 (file)
@@ -81,8 +81,6 @@
         <leaf-list name="test-leaf-list">
             <type name="string"></type>
             <config value="false"></config>
-            <min-elements value="0"></min-elements>
-            <max-elements value="2147483647"></max-elements>
             <ordered-by value="user"></ordered-by>
             <status value="current"></status>
         </leaf-list>
@@ -90,7 +88,6 @@
             <key value="key-leaf-1 key-leaf-2"></key>
             <config value="true"></config>
             <min-elements value="5"></min-elements>
-            <max-elements value="2147483647"></max-elements>
             <ordered-by value="system"></ordered-by>
             <status value="current"></status>
             <leaf name="key-leaf-1">
             <refine target-node="test-list">
                 <config value="true"></config>
                 <min-elements value="5"></min-elements>
-                <max-elements value="2147483647"></max-elements>
             </refine>
             <refine target-node="test-leaf-list">
                 <config value="false"></config>
-                <min-elements value="0"></min-elements>
-                <max-elements value="2147483647"></max-elements>
             </refine>
             <refine target-node="test-leaf-1">
                 <default value="def-val"></default>
             </refine>
         </uses>
     </notification>
-</module>
\ No newline at end of file
+</module>
index 72e981d42916760f155e08abc3a87421c7446e69..dad039b9087df82a6d3e54a0e0c253295eca2009 100644 (file)
@@ -15,8 +15,8 @@ import org.opendaylight.yangtools.yang.model.api.MustDefinition;
 import org.opendaylight.yangtools.yang.model.api.RevisionAwareXPath;
 
 final class EffectiveConstraintDefinitionImpl implements ConstraintDefinition {
-    private static final Integer UNBOUNDED_INT = Integer.MAX_VALUE;
     private static final String UNBOUNDED_STR = "unbounded";
+
     private final RevisionAwareXPath whenCondition;
     private final Set<MustDefinition> mustConstraints;
     private final Integer minElements;
@@ -27,8 +27,8 @@ final class EffectiveConstraintDefinitionImpl implements ConstraintDefinition {
             final Integer maxElements, final RevisionAwareXPath whenCondition,
             final Set<MustDefinition> mustConstraints) {
         this.mandatory = mandatory;
-        this.minElements = Preconditions.checkNotNull(minElements);
-        this.maxElements = Preconditions.checkNotNull(maxElements);
+        this.minElements = minElements;
+        this.maxElements = maxElements;
         this.whenCondition = whenCondition;
         this.mustConstraints = Preconditions.checkNotNull(mustConstraints);
     }
@@ -36,27 +36,34 @@ final class EffectiveConstraintDefinitionImpl implements ConstraintDefinition {
     static ConstraintDefinition forParent(final EffectiveStatementBase<?, ?> parent) {
         final MinElementsEffectiveStatementImpl firstMinElementsStmt = parent
                 .firstEffective(MinElementsEffectiveStatementImpl.class);
-        final Integer minElements = (firstMinElementsStmt == null) ? 0 : firstMinElementsStmt.argument();
+        final Integer minElements;
+        if (firstMinElementsStmt != null) {
+            final Integer m = firstMinElementsStmt.argument();
+            minElements = m > 0 ? m : null;
+        } else {
+            minElements = null;
+        }
 
         final MaxElementsEffectiveStatementImpl firstMaxElementsStmt = parent
                 .firstEffective(MaxElementsEffectiveStatementImpl.class);
         final String maxElementsArg = (firstMaxElementsStmt == null) ? UNBOUNDED_STR : firstMaxElementsStmt.argument();
         final Integer maxElements;
-        if (UNBOUNDED_STR.equals(maxElementsArg)) {
-            maxElements = UNBOUNDED_INT;
+        if (!UNBOUNDED_STR.equals(maxElementsArg)) {
+            final Integer m = Integer.valueOf(maxElementsArg);
+            maxElements = m < Integer.MAX_VALUE ? m : null;
         } else {
-            maxElements = Integer.valueOf(maxElementsArg);
+            maxElements = null;
         }
 
         final MandatoryEffectiveStatement firstMandatoryStmt = parent.firstEffective(MandatoryEffectiveStatement.class);
-        final boolean mandatory = (firstMandatoryStmt == null) ? minElements > 0 : firstMandatoryStmt.argument();
+        final boolean mandatory = (firstMandatoryStmt == null) ? minElements != null : firstMandatoryStmt.argument();
 
         final Set<MustDefinition> mustSubstatements = ImmutableSet.copyOf(parent.allSubstatementsOfType(
             MustDefinition.class));
         final WhenEffectiveStatementImpl firstWhenStmt = parent.firstEffective(WhenEffectiveStatementImpl.class);
 
         // Check for singleton instances
-        if (minElements == 0 && maxElements == UNBOUNDED_INT && mustSubstatements.isEmpty() && firstWhenStmt == null) {
+        if (minElements == null && maxElements == null && mustSubstatements.isEmpty() && firstWhenStmt == null) {
             return EmptyConstraintDefinition.create(mandatory);
         }
 
index d0bc8acb27a7d0e7b865280946fcfc5b43ef3e93..194d150b24e14f3aface64e7b162584d26ddafd0 100644 (file)
@@ -50,12 +50,12 @@ abstract class EmptyConstraintDefinition implements ConstraintDefinition {
 
     @Override
     public final Integer getMinElements() {
-        return 0;
+        return null;
     }
 
     @Override
     public final Integer getMaxElements() {
-        return Integer.MAX_VALUE;
+        return null;
     }
 
     @Override
index d4ab088c8c0ab3dad5f33885b1856288948ec081..087ab6ae80cd0281ac7832d0ad0450cc550abff7 100644 (file)
@@ -117,7 +117,7 @@ public class GroupingTest {
         assertEquals("addresses reference added by refine", refineList.getReference());
         assertFalse(refineList.isConfiguration());
         assertEquals(2, (int) refineList.getConstraints().getMinElements());
-        assertEquals(Integer.MAX_VALUE, (int) refineList.getConstraints().getMaxElements());
+        assertNull(refineList.getConstraints().getMaxElements());
 
         // leaf id
         assertNotNull(refineInnerLeaf);
@@ -625,6 +625,9 @@ public class GroupingTest {
             }
         }
 
+        assertNotNull(foo);
+        assertNotNull(imp);
+
         final LeafSchemaNode leaf = (LeafSchemaNode) ((ContainerSchemaNode) foo.getDataChildByName(QName.create(
                 foo.getQNameModule(), "my-container")))
                 .getDataChildByName(QName.create(foo.getQNameModule(), "my-leaf"));
@@ -638,6 +641,7 @@ public class GroupingTest {
             }
         }
 
+        assertNotNull(impType);
         assertEquals(leaf.getType().getQName(), impType.getQName());
     }
 }
index 28272d9bc9ef1ffe13163e16c7864f926b310186..b874fb5de8dd8a5700ab7d26440606b4dc32603e 100644 (file)
@@ -97,12 +97,12 @@ public class YangParserSimpleTest {
         assertTrue(found2);
 
         assertTrue(constraints.isMandatory());
-        assertTrue(0 == constraints.getMinElements());
-        assertTrue(Integer.MAX_VALUE == constraints.getMaxElements());
+        assertNull(constraints.getMinElements());
+        assertNull(constraints.getMaxElements());
     }
 
     @Test
-    public void testParseContainer() {
+    public void testParseContainer() throws ParseException {
         final Module test = TestUtils.findModule(modules, "simple-nodes");
 
         final ContainerSchemaNode nodes = (ContainerSchemaNode) test.getDataChildByName(QName.create(test.getQNameModule(), "nodes"));
@@ -147,8 +147,8 @@ public class YangParserSimpleTest {
         assertTrue(found2);
 
         assertFalse(constraints.isMandatory());
-        assertTrue(0 == constraints.getMinElements());
-        assertTrue(Integer.MAX_VALUE == constraints.getMaxElements());
+        assertNull(constraints.getMinElements());
+        assertNull(constraints.getMaxElements());
         assertTrue(nodes.isPresenceContainer());
 
         // typedef
@@ -189,20 +189,14 @@ public class YangParserSimpleTest {
     }
 
 
-    private final URI ns = URI.create("urn:opendaylight:simple-nodes");
-    private Date rev;
-    private final String prefix = "sn";
+    private static final URI NS = URI.create("urn:opendaylight:simple-nodes");
 
-    private SchemaPath createPath(final String... names) {
-        try {
-            rev = new SimpleDateFormat("yyyy-MM-dd").parse("2013-07-30");
-        } catch (final ParseException e) {
-            e.printStackTrace();
-        }
+    private static SchemaPath createPath(final String... names) throws ParseException {
+        final Date rev = new SimpleDateFormat("yyyy-MM-dd").parse("2013-07-30");
 
         final List<QName> path = new ArrayList<>();
         for (final String name : names) {
-            path.add(QName.create(ns, rev, name));
+            path.add(QName.create(NS, rev, name));
         }
         return SchemaPath.create(path, true);
     }