Expose property name when checking key components 80/93180/3
authorRobert Varga <robert.varga@pantheon.tech>
Mon, 19 Oct 2020 13:39:51 +0000 (15:39 +0200)
committerRobert Varga <nite@hq.sk>
Sat, 24 Oct 2020 17:20:30 +0000 (17:20 +0000)
Experience with null enforcement of key components indicates we should
carry the name of the property in the exception message -- otherwise
the string is not really helpful.

This patch re-organizes the checking so that we do that, via
newly-introduced CodeHelpers.requireKeyProp() method.

Since we are in the area, also codegen methods' handling of arrays by
creating an explicit 'cloneCall()' utility method.

JIRA: MDSAL-599
Change-Id: If9a87b4976ecdcd09e1fdd83ddfaf03ab4d09a85
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BaseTemplate.xtend
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ClassTemplate.xtend
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/JavaFileTemplate.java
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ListKeyTemplate.xtend
binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java

index e6a0a9be3010d5dae6eddd02e1ef865fa8ff59ab..ed6bcea2c14bac35a071b2499c7bb3a61e6b6ecc 100644 (file)
@@ -124,18 +124,16 @@ abstract class BaseTemplate extends JavaFileTemplate {
      * generated property with data about field which is generated as the getter method
      * @return string with the getter method source code in JAVA format
      */
-    protected def getterMethod(GeneratedProperty field) {
-        '''
-            public «field.returnType.importedName» «field.getterMethodName»() {
-                «val fieldName = field.fieldName»
-                «IF field.returnType.name.endsWith("[]")»
-                return «fieldName» == null ? null : «fieldName».clone();
-                «ELSE»
-                return «fieldName»;
-                «ENDIF»
-            }
-        '''
-    }
+    protected def getterMethod(GeneratedProperty field) '''
+        public «field.returnType.importedName» «field.getterMethodName»() {
+            «val fieldName = field.fieldName»
+            «IF field.returnType.name.endsWith("[]")»
+            return «fieldName» == null ? null : «fieldName».clone();
+            «ELSE»
+            return «fieldName»;
+            «ENDIF»
+        }
+    '''
 
     final protected def getterMethodName(GeneratedProperty field) {
         val prefix = if(field.returnType.equals(Types.BOOLEAN)) "is" else "get"
index 4ee8d603f97633278a209de2c745a4bae831b451..71368811f26851ab50ba97e77c683558aa90cb4c 100644 (file)
@@ -214,7 +214,7 @@ class ClassTemplate extends BaseTemplate {
     def private scalarTypeObjectValue(GeneratedProperty field) '''
         @«OVERRIDE.importedName»
         public «field.returnType.importedName» «BindingMapping.SCALAR_TYPE_OBJECT_GET_VALUE_NAME»() {
-            return «field.fieldName»«IF field.returnType.name.endsWith("[]")».clone()«ENDIF»;
+            return «field.fieldName»«field.cloneCall»;
         }
     '''
 
@@ -306,11 +306,7 @@ class ClassTemplate extends BaseTemplate {
 
         «FOR p : properties»
             «val fieldName = p.fieldName»
-            «IF p.returnType.name.endsWith("[]")»
-                this.«fieldName» = «fieldName».clone();
-            «ELSE»
-                this.«fieldName» = «fieldName»;
-            «ENDIF»
+            this.«fieldName» = «fieldName»«p.cloneCall»;
         «ENDFOR»
     }
     '''
index cec30795866d613d06c439294d4d44a609d42d0e..ec891f26250447d2dce4056f7a72587fcbb2a5b6 100644 (file)
@@ -241,6 +241,16 @@ class JavaFileTemplate {
         return null;
     }
 
+    /**
+     * Generate a call to {@link Object#clone()} if target field represents an array. Returns an empty string otherwise.
+     *
+     * @param property Generated property
+     * @return The string used to clone the property, or an empty string
+     */
+    static final String cloneCall(final GeneratedProperty property) {
+        return property.getReturnType().getName().endsWith("[]") ? ".clone()" : "";
+    }
+
     /**
      * Returns set of method signature instances which contains all the methods of the <code>genType</code>
      * and all the methods of the implemented interfaces.
index 55f099e95d5718074cb523c01ae7114a978e99bd..0087e95aeb4e031ce0e91c33b8b7b850d4c226a0 100644 (file)
@@ -13,8 +13,7 @@ import org.opendaylight.mdsal.binding.model.api.GeneratedTransferObject
 /**
  * Template for generating JAVA class.
  */
-class ListKeyTemplate extends ClassTemplate {
-
+final class ListKeyTemplate extends ClassTemplate {
     /**
      * Creates instance of this class with concrete <code>genType</code>.
      *
@@ -24,37 +23,21 @@ class ListKeyTemplate extends ClassTemplate {
         super(genType)
     }
 
-
-    override final allValuesConstructor() '''
+    override allValuesConstructor() '''
         public «type.name»(«allProperties.asNonNullArgumentsDeclaration») {
             «FOR p : allProperties»
-                «CODEHELPERS.importedName».requireValue(«p.fieldName»);
+                «val fieldName = p.fieldName»
+                this.«fieldName» = «CODEHELPERS.importedName».requireKeyProp(«fieldName», "«p.name»")«p.cloneCall»;
             «ENDFOR»
             «FOR p : properties»
                 «generateRestrictions(type, p.fieldName, p.returnType)»
             «ENDFOR»
-
-            «FOR p : allProperties»
-                «val fieldName = p.fieldName»
-                «IF p.returnType.name.endsWith("[]")»
-                    this.«fieldName» = «fieldName».clone();
-                «ELSE»
-                    this.«fieldName» = «fieldName»;
-                «ENDIF»
-            «ENDFOR»
         }
     '''
 
-    override final getterMethod(GeneratedProperty field) {
-        '''
-            public «field.returnType.importedNonNull» «field.getterMethodName»() {
-                «val fieldName = field.fieldName»
-                «IF field.returnType.name.endsWith("[]")»
-                return «fieldName».clone();
-                «ELSE»
-                return «fieldName»;
-                «ENDIF»
-            }
-        '''
-    }
+    override getterMethod(GeneratedProperty field) '''
+        public «field.returnType.importedNonNull» «field.getterMethodName»() {
+            return «field.fieldName»«field.cloneCall»;
+        }
+    '''
 }
index 5d224d300a55f41f627bd58220a356eb1de20b48..1952e10cf536202d01721bf47df4aec894856825 100644 (file)
@@ -53,13 +53,28 @@ public final class CodeHelpers {
         checkArgument(expression, "expected one of: %s \n%but was: %s", options, value);
     }
 
+    /**
+     * A shortcut for {@code Preconditions.checkNotNull(value, "Key component \"%s\" must not be null", name)}.
+     *
+     * @param value Value itself
+     * @param name Name of the value
+     * @return Non-null value
+     * @throws NullPointerException if value is null
+     */
+    public static <T> @NonNull T requireKeyProp(final @Nullable T value, final @NonNull String name) {
+        if (value == null) {
+            throw new NullPointerException("Key component \"" + name + "\" may not be null");
+        }
+        return value;
+    }
+
     /**
      * A shortcut for {@code Objects.requireNonNull(value, "Supplied value may not be null")}.
      *
      * @param value Value itself
      * @throws NullPointerException if value is null
      */
-    public static void requireValue(@Nullable final Object value) {
+    public static void requireValue(final @Nullable Object value) {
         requireNonNull(value, "Supplied value may not be null");
     }