Fix leafref require-instance implementation 01/70201/1
authorRobert Varga <robert.varga@pantheon.tech>
Thu, 29 Mar 2018 21:59:34 +0000 (23:59 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 29 Mar 2018 22:47:01 +0000 (00:47 +0200)
Both identityref and leafref types default to require-instace=true,
hence we should be starting with that and also properly inherit
the property from the parent type.

Add RequireInstanceRestrictedTypeDefinition to capture the common
method and adjust RequireInstanceRestrictedTypeBuilder to check
for it and initialize the require-instance value appropriately.

JIRA: YANGTOOLS-872
Change-Id: I125d5687950af21694c4510ce64adb00f92ae4e6
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/type/InstanceIdentifierTypeDefinition.java
yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/type/LeafrefTypeDefinition.java
yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/type/RequireInstanceRestrictedTypeDefinition.java [new file with mode: 0644]
yang/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/type/RequireInstanceRestrictedTypeBuilder.java
yang/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/LeafrefTest.java
yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/parser/stmt/rfc7950/LeafrefStatementTest.java

index 147b4e35ef0e5bed3c6d73b73634869c850a5c81..eecdc7139c8c0bdc90eead090de8fc33ccd14baf 100644 (file)
@@ -8,21 +8,12 @@
 package org.opendaylight.yangtools.yang.model.api.type;
 
 import java.util.Objects;
-import org.opendaylight.yangtools.yang.model.api.TypeDefinition;
 
 /**
- * Contains methods for getting data from the <code>instance-identifier</code>
- * YANG built-in type.
+ * Contains methods for getting data from the <code>instance-identifier</code> YANG built-in type.
  */
-public interface InstanceIdentifierTypeDefinition extends TypeDefinition<InstanceIdentifierTypeDefinition> {
-    /**
-     * Returns true|false which represents argument of <code>require-instance</code> statement. This statement is the
-     * substatement of the <code>type</code> statement.
-     *
-     * @return boolean value which is true if the <code>require-instance</code> statement is true and vice versa
-     */
-    boolean requireInstance();
-
+public interface InstanceIdentifierTypeDefinition
+        extends RequireInstanceRestrictedTypeDefinition<InstanceIdentifierTypeDefinition> {
     static int hashCode(final InstanceIdentifierTypeDefinition type) {
         return Objects.hash(type.getPath(), type.getUnknownSchemaNodes(), type.getBaseType(),
             type.getUnits().orElse(null), type.getDefaultValue().orElse(null), type.requireInstance());
index c827bd5a9b47d8064279951583083fc3e93b8ec8..80d7f40edfab065b2285a67a88ae36d458865b26 100644 (file)
@@ -9,19 +9,19 @@ package org.opendaylight.yangtools.yang.model.api.type;
 
 import java.util.Objects;
 import org.opendaylight.yangtools.yang.model.api.RevisionAwareXPath;
-import org.opendaylight.yangtools.yang.model.api.TypeDefinition;
 
-public interface LeafrefTypeDefinition extends TypeDefinition<LeafrefTypeDefinition> {
+public interface LeafrefTypeDefinition extends RequireInstanceRestrictedTypeDefinition<LeafrefTypeDefinition> {
 
     // FIXME: this is not the same syntax as when statement. See https://tools.ietf.org/html/rfc7950#section-9.9.2
     RevisionAwareXPath getPathStatement();
 
     /**
-     * Require instance pointed to by {@link #getPathStatement()} to be present or not. For YANG version (RFC6020), this
-     * should always return true.
+     * {@inheritDoc}
      *
-     * @return True if the instance pointed to by {@link #getPathStatement()} must be present in the data tree.
+     * <p>
+     * For YANG version 1 (RFC6020), this should always return true.
      */
+    @Override
     boolean requireInstance();
 
     static int hashCode(final LeafrefTypeDefinition type) {
diff --git a/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/type/RequireInstanceRestrictedTypeDefinition.java b/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/type/RequireInstanceRestrictedTypeDefinition.java
new file mode 100644 (file)
index 0000000..b04fd8e
--- /dev/null
@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) 2013 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.model.api.type;
+
+import org.opendaylight.yangtools.yang.model.api.TypeDefinition;
+
+/**
+ * Interface for {@link TypeDefinition}s which can be restricted through a require-instance statement.
+ *
+ * @param <T> Concrete {@link TypeDefinition} subinterface
+ */
+public interface RequireInstanceRestrictedTypeDefinition<T extends TypeDefinition<T>> extends TypeDefinition<T> {
+    /**
+     * Returns true or false which represents argument of <code>require-instance</code> statement. This statement is
+     * the substatement of the <code>type</code> statement.
+     *
+     * @return boolean value which is true if the <code>require-instance</code> statement is true and vice versa
+     */
+    boolean requireInstance();
+}
index 6d4b988b12365e3cc8d16a7d114aab5ab75f16b9..c3c238e3cf6ef4004a7d0a9d50f741c2501dac47 100644 (file)
@@ -5,15 +5,15 @@
  * 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.model.util.type;
 
 import com.google.common.annotations.Beta;
-import com.google.common.base.Preconditions;
 import org.opendaylight.yangtools.yang.model.api.SchemaPath;
 import org.opendaylight.yangtools.yang.model.api.TypeDefinition;
+import org.opendaylight.yangtools.yang.model.api.type.RequireInstanceRestrictedTypeDefinition;
 
 @Beta
+//FIXME: 3.0.0: this should require T to be a RequireInstanceRestrictedTypeDefinition
 public abstract class RequireInstanceRestrictedTypeBuilder<T extends TypeDefinition<T>>
         extends AbstractRestrictedTypeBuilder<T> {
 
@@ -21,13 +21,11 @@ public abstract class RequireInstanceRestrictedTypeBuilder<T extends TypeDefinit
 
     RequireInstanceRestrictedTypeBuilder(final T baseType, final SchemaPath path) {
         super(baseType, path);
+        requireInstance = baseType instanceof RequireInstanceRestrictedTypeDefinition
+                ? ((RequireInstanceRestrictedTypeDefinition<?>)baseType).requireInstance() : true;
     }
 
     public final void setRequireInstance(final boolean requireInstance) {
-        if (this.requireInstance) {
-            Preconditions.checkArgument(requireInstance, "Cannot switch off require-instance in type %s", getPath());
-        }
-
         this.requireInstance = requireInstance;
         touch();
     }
index 19d4c00f42487c59257db80577e1e7c7418d68cd..313ad5ace13394a1fc61c17856a0dd73dd0f6167 100644 (file)
@@ -13,7 +13,6 @@ import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 import java.util.Optional;
 import org.junit.Test;
@@ -23,6 +22,8 @@ import org.opendaylight.yangtools.yang.model.api.Status;
 import org.opendaylight.yangtools.yang.model.api.type.LeafrefTypeDefinition;
 import org.opendaylight.yangtools.yang.model.util.type.BaseTypes;
 import org.opendaylight.yangtools.yang.model.util.type.LeafrefTypeBuilder;
+import org.opendaylight.yangtools.yang.model.util.type.RequireInstanceRestrictedTypeBuilder;
+import org.opendaylight.yangtools.yang.model.util.type.RestrictedTypes;
 
 public class LeafrefTest {
 
@@ -69,29 +70,24 @@ public class LeafrefTest {
         final SchemaPath schemaPath = SchemaPath.create(true, QName.create("test", "my-cont"),
             QName.create("test", "my-leafref"));
         final RevisionAwareXPathImpl path = new RevisionAwareXPathImpl("../my-leaf", false);
+        final LeafrefTypeBuilder leafrefTypeBuilder = BaseTypes.leafrefTypeBuilder(schemaPath).setPathStatement(path);
 
-        LeafrefTypeBuilder leafrefTypeBuilder = BaseTypes.leafrefTypeBuilder(schemaPath).setPathStatement(path);
+        assertTrue(leafrefTypeBuilder.build().requireInstance());
 
         leafrefTypeBuilder.setRequireInstance(false);
-        LeafrefTypeDefinition leafref = leafrefTypeBuilder.build();
-        assertFalse(leafref.requireInstance());
+        final LeafrefTypeDefinition falseLeafref = leafrefTypeBuilder.build();
+        assertFalse(falseLeafref.requireInstance());
 
         leafrefTypeBuilder.setRequireInstance(true);
-        leafref = leafrefTypeBuilder.build();
-        assertTrue(leafref.requireInstance());
+        final LeafrefTypeDefinition trueLeafref = leafrefTypeBuilder.build();
+        assertTrue(trueLeafref.requireInstance());
 
-        leafrefTypeBuilder.setRequireInstance(true);
-        leafref = leafrefTypeBuilder.build();
-        assertTrue(leafref.requireInstance());
-
-        try {
-            leafrefTypeBuilder.setRequireInstance(false);
-            fail("An IllegalArgumentException should have been thrown.");
-        } catch (IllegalArgumentException ex) {
-            assertEquals(
-                "Cannot switch off require-instance in type AbsoluteSchemaPath{path=[(test)my-cont, (test)my-leafref]}",
-                ex.getMessage());
-        }
+        final RequireInstanceRestrictedTypeBuilder<LeafrefTypeDefinition> falseBuilder =
+                RestrictedTypes.newLeafrefBuilder(falseLeafref, schemaPath);
+        assertFalse(falseBuilder.build().requireInstance());
 
+        final RequireInstanceRestrictedTypeBuilder<LeafrefTypeDefinition> trueBuilder =
+                RestrictedTypes.newLeafrefBuilder(trueLeafref, schemaPath);
+        assertTrue(trueBuilder.build().requireInstance());
     }
 }
index 8b6c58d90c8308879f3054ba93606c02bad579cc..da05fe5b30843c12d00cc64d1c74bd7b8bef9e88 100644 (file)
@@ -53,7 +53,7 @@ public class LeafrefStatementTest {
         final LeafSchemaNode leafrefC = (LeafSchemaNode) foo.getDataChildByName(QName.create(foo.getQNameModule(),
                 "leafref-c"));
         assertNotNull(leafrefC);
-        assertRequireInstanceInLeafref(leafrefC, false);
+        assertRequireInstanceInLeafref(leafrefC, true);
     }
 
     private static void assertRequireInstanceInLeafref(final LeafSchemaNode leaf, final boolean requireInstance) {