BUG-5970: do not add value to union hashCode/equals/toString

The value of a union is derived from its members and should not
be part of general Object methods, as the members are already
part of these methods.

Also override definition for getValue() and generate alternate
code to lazily fill _value. The output could use some visual
improvement, but that is something for another day.

It also fixes the copy constructor so it does not clone the
_value field unnecessarily.

Change-Id: Ibbbce99e1e84a0dfa55f70872fedad78e57e8f1e
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/yangtools/sal/binding/yang/types/TypeProviderImpl.java b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/yangtools/sal/binding/yang/types/TypeProviderImpl.java
index 50f04381b44ebdb659b844d114a8756738905cb9..e0771573a601f6c28b365a05a9dcfc953199e4c0 100644 (file)
--- a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/yangtools/sal/binding/yang/types/TypeProviderImpl.java
+++ b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/yangtools/sal/binding/yang/types/TypeProviderImpl.java
@@ -11,7 +11,6 @@ import static org.opendaylight.yangtools.binding.generator.util.BindingGenerator
 import static org.opendaylight.yangtools.yang.model.util.SchemaContextUtil.findDataSchemaNode;
 import static org.opendaylight.yangtools.yang.model.util.SchemaContextUtil.findDataSchemaNodeForRelativeXPath;
 import static org.opendaylight.yangtools.yang.model.util.SchemaContextUtil.findParentModule;
-
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
@@ -848,25 +847,18 @@ public final class TypeProviderImpl implements TypeProvider {
      * @return generated TO builder with the list of enclosed generated TO
      *         builders
      */
-    public GeneratedTOBuilder provideGeneratedTOBuilderForUnionTypeDef(final String basePackageName, final UnionTypeDefinition typedef, final String typeDefName, final SchemaNode parentNode) {
-        final List<GeneratedTOBuilder> genTOBuilders = provideGeneratedTOBuildersForUnionTypeDef(basePackageName,
+    public GeneratedTOBuilder provideGeneratedTOBuilderForUnionTypeDef(final String basePackageName,
+            final UnionTypeDefinition typedef, final String typeDefName, final SchemaNode parentNode) {
+        final List<GeneratedTOBuilder> builders = provideGeneratedTOBuildersForUnionTypeDef(basePackageName,
                 typedef, typeDefName, parentNode);
-        GeneratedTOBuilder resultTOBuilder = null;
-        if (genTOBuilders.isEmpty()) {
-            throw new IllegalStateException("No GeneratedTOBuilder objects generated from union " + typedef);
-        }
+        Preconditions.checkState(!builders.isEmpty(), "No GeneratedTOBuilder objects generated from union %s", typedef);
 
-        resultTOBuilder = genTOBuilders.remove(0);
-        for (GeneratedTOBuilder genTOBuilder : genTOBuilders) {
+        final GeneratedTOBuilder resultTOBuilder = builders.remove(0);
+        for (GeneratedTOBuilder genTOBuilder : builders) {
             resultTOBuilder.addEnclosingTransferObject(genTOBuilder);
         }
 
-        final GeneratedPropertyBuilder genPropBuilder = resultTOBuilder.addProperty("value");
-        genPropBuilder.setReturnType(Types.CHAR_ARRAY);
-        resultTOBuilder.addEqualsIdentity(genPropBuilder);
-        resultTOBuilder.addHashIdentity(genPropBuilder);
-        resultTOBuilder.addToStringProperty(genPropBuilder);
-
+        resultTOBuilder.addProperty("value").setReturnType(Types.CHAR_ARRAY);
         return resultTOBuilder;
     }
 

diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/yangtools/sal/java/api/generator/BaseTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/yangtools/sal/java/api/generator/BaseTemplate.xtend
index f39f7a3d1d573bb65d4ffa203e1e3a3ef6d194e3..43cfaae1c4424259ec924635aed5abb232ba7762 100644 (file)
--- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/yangtools/sal/java/api/generator/BaseTemplate.xtend
+++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/yangtools/sal/java/api/generator/BaseTemplate.xtend
@@ -103,7 +103,7 @@ abstract class BaseTemplate {
      * generated property with data about field which is generated as the getter method
      * @return string with the getter method source code in JAVA format
      */
-    final protected def getterMethod(GeneratedProperty field) {
+    protected def getterMethod(GeneratedProperty field) {
         '''
             public «field.returnType.importedName» «field.getterMethodName»() {
                 «IF field.returnType.importedName.contains("[]")»

diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/yangtools/sal/java/api/generator/ClassTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/yangtools/sal/java/api/generator/ClassTemplate.xtend
index 6e1875a632e0665f2349bb5c5c35232511524459..e42b8b53bff14145bca4b31db9733b586d1506c8 100644 (file)
--- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/yangtools/sal/java/api/generator/ClassTemplate.xtend
+++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/yangtools/sal/java/api/generator/ClassTemplate.xtend
@@ -479,11 +479,15 @@ class ClassTemplate extends BaseTemplate {
     def protected generateFields() '''
         «IF !properties.empty»
             «FOR f : properties»
-                private«IF f.readOnly» final«ENDIF» «f.returnType.importedName» «f.fieldName»;
+                private«IF isReadOnly(f)» final«ENDIF» «f.returnType.importedName» «f.fieldName»;
             «ENDFOR»
         «ENDIF»
     '''
 
+    protected def isReadOnly(GeneratedProperty field) {
+        return field.readOnly
+    }
+
     /**
      * Template method which generates the method <code>hashCode()</code>.
      *

diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/yangtools/sal/java/api/generator/UnionTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/yangtools/sal/java/api/generator/UnionTemplate.xtend
index d356c28ee02457ba30b803b02911665e3c628226..b2c5d5a0bbdfdb83edfb6b7002cf2532f8afdb23 100644 (file)
--- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/yangtools/sal/java/api/generator/UnionTemplate.xtend
+++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/yangtools/sal/java/api/generator/UnionTemplate.xtend
@@ -7,10 +7,12 @@
  */
 package org.opendaylight.yangtools.sal.java.api.generator
 
-import org.opendaylight.yangtools.sal.binding.model.api.GeneratedTransferObject
+import static org.opendaylight.yangtools.binding.generator.util.Types.*
+import com.google.common.base.Preconditions;
 import java.beans.ConstructorProperties
+import org.opendaylight.yangtools.sal.binding.model.api.GeneratedProperty
+import org.opendaylight.yangtools.sal.binding.model.api.GeneratedTransferObject
 import org.opendaylight.yangtools.sal.binding.model.api.Enumeration
-import static org.opendaylight.yangtools.binding.generator.util.Types.*
 
 /**
  * Template for generating JAVA class.
@@ -57,8 +59,7 @@ class UnionTemplate extends ClassTemplate {
     private def unionConstructors() '''
         «FOR property : finalProperties SEPARATOR "\n"»
             «val propRet = property.returnType»
-            «val isCharArray = "char[]".equals(propRet.name)»
-            «IF isCharArray»
+            «IF "char[]".equals(propRet.name)»
                 /**
                  * Constructor provided only for using in JMX. Don't use it for
                  * construction new object of this union type.
@@ -85,54 +86,15 @@ class UnionTemplate extends ClassTemplate {
                     super(«parentProperties.asArguments»);
                     this.«property.fieldName» = «property.fieldName»;
                     «FOR other : finalProperties»
-                        «IF property != other»
-                            «IF "value".equals(other.name)»
-                                «IF "java.lang.String".equals(propRet.fullyQualifiedName)»
-                                    ««« type string
-                                    this.«other.fieldName» = «property.fieldName».toCharArray();
-                                «ELSEIF "byte[]".equals(propRet.name)»
-                                    ««« type binary
-                                    this.«other.fieldName» = new «String.importedName»(«property.fieldName»).toCharArray();
-                                «ELSEIF propRet.fullyQualifiedName.startsWith("java.lang")
-                                    || propRet instanceof Enumeration
-                                    || propRet.fullyQualifiedName.startsWith("java.math")»
-                                    ««« type int*, uint, decimal64 or enumeration*
-                                    this.«other.fieldName» = «property.fieldName».toString().toCharArray();
-                                «ELSEIF propRet instanceof GeneratedTransferObject && (propRet as GeneratedTransferObject).unionType»
-                                    ««« union type
-                                    this.«other.fieldName» = «property.fieldName».getValue();
-                                «ELSEIF propRet instanceof GeneratedTransferObject // Is it a GeneratedTransferObject
-                                        && (propRet as GeneratedTransferObject).typedef  // Is it a typedef
-                                        && (propRet as GeneratedTransferObject).properties != null
-                                        && !(propRet as GeneratedTransferObject).properties.empty
-                                        && ((propRet as GeneratedTransferObject).properties.size == 1)
-                                        && (propRet as GeneratedTransferObject).properties.get(0).name.equals("value")
-                                        && BOOLEAN.equals((propRet as GeneratedTransferObject).properties.get(0).returnType)» // And the property value is of type boolean
-                                    ««« generated boolean typedef
-                                    this.«other.fieldName» = «property.fieldName».isValue().toString().toCharArray();
-                                «ELSEIF propRet instanceof GeneratedTransferObject // Is it a GeneratedTransferObject
-                                        && (propRet as GeneratedTransferObject).typedef  // Is it a typedef
-                                        && (propRet as GeneratedTransferObject).properties != null
-                                        && !(propRet as GeneratedTransferObject).properties.empty
-                                        && ((propRet as GeneratedTransferObject).properties.size == 1)
-                                        && (propRet as GeneratedTransferObject).properties.get(0).name.equals("value")
-                                        && "byte[]".equals((propRet as GeneratedTransferObject).properties.get(0).returnType.name)»
-                                    ««« generated byte[] typedef
-                                    this.«other.fieldName» = BaseEncoding.base64().encode(«property.fieldName».getValue()).toCharArray();
-                                «ELSE»
-                                    ««« generated type
-                                    this.«other.fieldName» = «property.fieldName».getValue().toString().toCharArray();
-                                «ENDIF»
-                            «ELSE»
-                                this.«other.fieldName» = null;
-                            «ENDIF»
+                        «IF property != other && !"value".equals(other.name)»
+                             this.«other.fieldName» = null;
                         «ENDIF»
                     «ENDFOR»
                 }
             «ENDIF»
         «ENDFOR»
     '''
-    
+
     def typeBuilder() {
         val outerCls = getOuterClassName(type);
         if(outerCls !== null) {
@@ -149,6 +111,67 @@ class UnionTemplate extends ClassTemplate {
         «ENDFOR»
     '''
 
+    override protected getterMethod(GeneratedProperty field) {
+        if (!"value".equals(field.name)) {
+            return super.getterMethod(field)
+        }
+
+        Preconditions.checkArgument("char[]".equals(field.returnType.importedName))
+
+        '''
+            public char[] «field.getterMethodName»() {
+                if («field.fieldName» == null) {
+                    «FOR property : finalProperties.filter([ p | !"value".equals(p.name)]) SEPARATOR " else"»
+                        if («property.fieldName» != null) {
+                            «val propRet = property.returnType»
+                            «IF "java.lang.String".equals(propRet.fullyQualifiedName)»
+                                ««« type string
+                                «field.fieldName» = «property.fieldName».toCharArray();
+                            «ELSEIF "byte[]".equals(propRet.name)»
+                                ««« type binary
+                                «field.fieldName» = new «String.importedName»(«property.fieldName»).toCharArray();
+                            «ELSEIF propRet.fullyQualifiedName.startsWith("java.lang")
+                                || propRet instanceof Enumeration
+                                || propRet.fullyQualifiedName.startsWith("java.math")»
+                                ««« type int*, uint, decimal64 or enumeration*
+                                «field.fieldName» = «property.fieldName».toString().toCharArray();
+                            «ELSEIF propRet instanceof GeneratedTransferObject && (propRet as GeneratedTransferObject).unionType»
+                                ««« union type
+                                «field.fieldName» = «property.fieldName».getValue();
+                            «ELSEIF propRet instanceof GeneratedTransferObject // Is it a GeneratedTransferObject
+                                    && (propRet as GeneratedTransferObject).typedef  // Is it a typedef
+                                    && (propRet as GeneratedTransferObject).properties != null
+                                    && !(propRet as GeneratedTransferObject).properties.empty
+                                    && ((propRet as GeneratedTransferObject).properties.size == 1)
+                                    && (propRet as GeneratedTransferObject).properties.get(0).name.equals("value")
+                                    && BOOLEAN.equals((propRet as GeneratedTransferObject).properties.get(0).returnType)» // And the property value is of type boolean
+                                ««« generated boolean typedef
+                                «field.fieldName» = «property.fieldName».isValue().toString().toCharArray();
+                            «ELSEIF propRet instanceof GeneratedTransferObject // Is it a GeneratedTransferObject
+                                    && (propRet as GeneratedTransferObject).typedef  // Is it a typedef
+                                    && (propRet as GeneratedTransferObject).properties != null
+                                    && !(propRet as GeneratedTransferObject).properties.empty
+                                    && ((propRet as GeneratedTransferObject).properties.size == 1)
+                                    && (propRet as GeneratedTransferObject).properties.get(0).name.equals("value")
+                                    && "byte[]".equals((propRet as GeneratedTransferObject).properties.get(0).returnType.name)»
+                                ««« generated byte[] typedef
+                                «field.fieldName» = BaseEncoding.base64().encode(«property.fieldName».getValue()).toCharArray();
+                            «ELSE»
+                                ««« generated type
+                                «field.fieldName» = «property.fieldName».getValue().toString().toCharArray();
+                            «ENDIF»
+                        }
+                    «ENDFOR»
+                }
+                return «field.fieldName» == null ? null : «field.fieldName».clone();
+            }
+        '''
+    }
+
+    override def isReadOnly(GeneratedProperty field) {
+        return !"value".equals(field.name) && super.isReadOnly(field)
+    }
+
     override protected copyConstructor() '''
         /**
          * Creates a copy from Source Object.
@@ -161,7 +184,7 @@ class UnionTemplate extends ClassTemplate {
             «ENDIF»
             «IF !properties.empty»
                 «FOR p : properties»
-                    «IF p.returnType.importedName.contains("[]")»
+                    «IF !"value".equals(p.name) && p.returnType.importedName.contains("[]")»
                     this.«p.fieldName» = source.«p.fieldName» == null ? null : source.«p.fieldName».clone();
                     «ELSE»
                     this.«p.fieldName» = source.«p.fieldName»;