From 20123a0c9e449857b68c1af0306b163495b354cc Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sat, 28 May 2016 12:17:22 +0200 Subject: [PATCH] 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 --- .../binding/yang/types/TypeProviderImpl.java | 22 ++-- .../sal/java/api/generator/BaseTemplate.xtend | 2 +- .../java/api/generator/ClassTemplate.xtend | 6 +- .../java/api/generator/UnionTemplate.xtend | 117 +++++++++++------- 4 files changed, 83 insertions(+), 64 deletions(-) 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 50f04381b4..e0771573a6 100644 --- 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 genTOBuilders = provideGeneratedTOBuildersForUnionTypeDef(basePackageName, + public GeneratedTOBuilder provideGeneratedTOBuilderForUnionTypeDef(final String basePackageName, + final UnionTypeDefinition typedef, final String typeDefName, final SchemaNode parentNode) { + final List 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 f39f7a3d1d..43cfaae1c4 100644 --- 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 6e1875a632..e42b8b53bf 100644 --- 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 hashCode(). * 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 d356c28ee0..b2c5d5a0bb 100644 --- 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»; -- 2.36.6