Separate out builder/impl copy generators

Builders are taking an immutable implementation of target type,
hence for list entries they can rely on the key to be present and
do not have to check it. Implementations need to account
for the builder key being set to null, hence they need to
instantiate the key themselves.

This patch separates the two code paths, so both cases are properly
expressed in the object hierarchy. This allows us to simplify
the abstract template, removing weird boolean-based decisions.

We also promote implementation constructor to package-visible, which
allows javac to skip generation of synthetic accessor.

We further optimize the template output by capturing importedNames
when they are reused -- leading to xtend generating cleaner Java code.

JIRA: MDSAL-374
Change-Id: I3274e35780b8e6a56d470caf80459e9e115a5374
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 3a2c2590fe64953e59953c9a7dde5e97ecfe34ec)

diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/AbstractBuilderTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/AbstractBuilderTemplate.xtend
index 6622ee2d4bed465a525eb52af6deee413469c4c8..60e6983beeb8bfab5bebcaea8dfcf0aba6b87548 100644 (file)
--- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/AbstractBuilderTemplate.xtend
+++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/AbstractBuilderTemplate.xtend
@@ -11,22 +11,20 @@ import static org.opendaylight.mdsal.binding.spec.naming.BindingMapping.AUGMENTA
 import static org.opendaylight.mdsal.binding.spec.naming.BindingMapping.AUGMENTATION_FIELD
 
 import com.google.common.base.MoreObjects
+import java.util.ArrayList
 import java.util.Collection
 import java.util.Collections
+import java.util.List
 import java.util.Map
 import java.util.Set
 import org.opendaylight.mdsal.binding.model.api.GeneratedProperty
+import org.opendaylight.mdsal.binding.model.api.GeneratedTransferObject
 import org.opendaylight.mdsal.binding.model.api.GeneratedType
 import org.opendaylight.mdsal.binding.model.api.Type
-import org.opendaylight.yangtools.yang.binding.CodeHelpers
-import java.util.ArrayList
 import org.opendaylight.mdsal.binding.model.util.Types
-import org.opendaylight.yangtools.yang.binding.Identifiable
-import org.opendaylight.mdsal.binding.model.api.GeneratedTransferObject
 import org.opendaylight.mdsal.binding.spec.naming.BindingMapping
-import com.google.common.collect.ImmutableMap
-import org.opendaylight.yangtools.yang.binding.AugmentationHolder
-import java.util.HashMap
+import org.opendaylight.yangtools.yang.binding.CodeHelpers
+import org.opendaylight.yangtools.yang.binding.Identifiable
 
 abstract class AbstractBuilderTemplate extends BaseTemplate {
     /**
@@ -70,7 +68,7 @@ abstract class AbstractBuilderTemplate extends BaseTemplate {
      * @param makeFinal value which specify whether field is|isn't final
      * @return string with class attributes and their types
      */
-    def generateFields(boolean makeFinal) '''
+    def protected final generateFields(boolean makeFinal) '''
         «IF properties !== null»
             «FOR f : properties»
                 private«IF makeFinal» final«ENDIF» «f.returnType.importedName» «f.fieldName»;
@@ -81,7 +79,7 @@ abstract class AbstractBuilderTemplate extends BaseTemplate {
         «ENDIF»
     '''
 
-    def generateAugmentField(boolean isPrivate) '''
+    def protected final generateAugmentField(boolean isPrivate) '''
         «IF augmentType !== null»
             «IF isPrivate»private «ENDIF»«Map.importedName»<«Class.importedName»<? extends «augmentType.importedName»>, «augmentType.importedName»> «AUGMENTATION_FIELD» = «Collections.importedName».emptyMap();
         «ENDIF»
@@ -108,7 +106,7 @@ abstract class AbstractBuilderTemplate extends BaseTemplate {
      *
      * @return string with getter methods
      */
-    def generateGetters(boolean addOverride) '''
+    def final generateGetters(boolean addOverride) '''
         «IF keyType !== null»
             «IF addOverride»@«Override.importedName»«ENDIF»
             public «keyType.importedName» «BindingMapping.IDENTIFIABLE_KEY_NAME»() {
@@ -132,8 +130,8 @@ abstract class AbstractBuilderTemplate extends BaseTemplate {
         «ENDIF»
     '''
 
-    def CharSequence generateCopyConstructor(boolean impl, Type fromType, Type implType) '''
-        «IF impl»private«ELSE»public«ENDIF» «type.name»(«fromType.importedName» base) {
+    def protected final CharSequence generateCopyConstructor(Type fromType, Type implType) '''
+        «type.name»(«fromType.importedName» base) {
             «val allProps = new ArrayList(properties)»
             «val isList = implementsIfc(targetType, Types.parameterizedTypeFor(Types.typeForClass(Identifiable), targetType))»
             «IF isList && keyType !== null»
@@ -142,46 +140,21 @@ abstract class AbstractBuilderTemplate extends BaseTemplate {
                 «FOR field : keyProps»
                     «removeProperty(allProps, field.name)»
                 «ENDFOR»
-                if (base.«BindingMapping.IDENTIFIABLE_KEY_NAME»() == null) {
-                    this.key = new «keyType.importedName»(
-                        «FOR keyProp : keyProps SEPARATOR ", "»
-                            base.«keyProp.getterMethodName»()
-                        «ENDFOR»
-                    );
-                    «FOR field : keyProps»
-                        this.«field.fieldName» = base.«field.getterMethodName»();
-                    «ENDFOR»
-                } else {
-                    this.key = base.«BindingMapping.IDENTIFIABLE_KEY_NAME»();
-                    «FOR field : keyProps»
-                           this.«field.fieldName» = key.«field.getterMethodName»();
-                    «ENDFOR»
-                }
+                «generateCopyKeys(keyProps)»
             «ENDIF»
             «FOR field : allProps»
                 this.«field.fieldName» = base.«field.getterMethodName»();
             «ENDFOR»
             «IF augmentType !== null»
-                «IF impl»
-                    this.«AUGMENTATION_FIELD» = «ImmutableMap.importedName».copyOf(base.«AUGMENTATION_FIELD»);
-                «ELSE»
-                    if (base instanceof «implType.importedName») {
-                        «implType.importedName» impl = («implType.importedName») base;
-                        if (!impl.«AUGMENTATION_FIELD».isEmpty()) {
-                            this.«AUGMENTATION_FIELD» = new «HashMap.importedName»<>(impl.«AUGMENTATION_FIELD»);
-                        }
-                    } else if (base instanceof «AugmentationHolder.importedName») {
-                        @SuppressWarnings("unchecked")
-                        «AugmentationHolder.importedName»<«fromType.importedName»> casted =(«AugmentationHolder.importedName»<«fromType.importedName»>) base;
-                        if (!casted.augmentations().isEmpty()) {
-                            this.«AUGMENTATION_FIELD» = new «HashMap.importedName»<>(casted.augmentations());
-                        }
-                    }
-                «ENDIF»
+                «generateCopyAugmentation(implType)»
             «ENDIF»
         }
     '''
 
+    def protected abstract CharSequence generateCopyKeys(List<GeneratedProperty> keyProps)
+
+    def protected abstract CharSequence generateCopyAugmentation(Type implType);
+
     private def boolean implementsIfc(GeneratedType type, Type impl) {
         for (Type ifc : type.implements) {
             if (ifc.equals(impl)) {
@@ -202,4 +175,4 @@ abstract class AbstractBuilderTemplate extends BaseTemplate {
             props.remove(toRemove);
         }
     }
-}
\ No newline at end of file
+}

diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderImplTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderImplTemplate.xtend
index 3eb6e7989482b9098c1d3d8f19d174d4326c9a0d..0e2e2085eb3f72a3da438f98253cce282016d3f1 100644 (file)
--- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderImplTemplate.xtend
+++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderImplTemplate.xtend
@@ -10,11 +10,15 @@ package org.opendaylight.mdsal.binding.java.api.generator
 import static org.opendaylight.mdsal.binding.spec.naming.BindingMapping.AUGMENTATION_FIELD
 import static org.opendaylight.mdsal.binding.spec.naming.BindingMapping.AUGMENTABLE_AUGMENTATION_NAME
 
+import com.google.common.collect.ImmutableMap
 import java.util.Arrays
-import java.util.Objects
+import java.util.List
 import java.util.Map
+import java.util.Objects
+import org.opendaylight.mdsal.binding.model.api.GeneratedProperty
 import org.opendaylight.mdsal.binding.model.api.GeneratedType
 import org.opendaylight.mdsal.binding.model.api.Type
+import org.opendaylight.mdsal.binding.spec.naming.BindingMapping
 import org.opendaylight.yangtools.yang.binding.DataObject
 
 class BuilderImplTemplate extends AbstractBuilderTemplate {
@@ -33,7 +37,7 @@ class BuilderImplTemplate extends AbstractBuilderTemplate {
 
             «generateAugmentField(true)»
 
-            «generateCopyConstructor(true, builderType, type)»
+            «generateCopyConstructor(builderType, type)»
 
             @«Override.importedName»
             public «Class.importedName»<«targetType.importedName»> getImplementedInterface() {
@@ -139,4 +143,19 @@ class BuilderImplTemplate extends AbstractBuilderTemplate {
             }
         «ENDIF»
     '''
+
+    override protected generateCopyKeys(List<GeneratedProperty> keyProps) '''
+        if (base.«BindingMapping.IDENTIFIABLE_KEY_NAME»() != null) {
+            this.key = base.«BindingMapping.IDENTIFIABLE_KEY_NAME»();
+        } else {
+            this.key = new «keyType.importedName»(«FOR keyProp : keyProps SEPARATOR ", "»base.«keyProp.getterMethodName»()«ENDFOR»);
+        }
+        «FOR field : keyProps»
+            this.«field.fieldName» = key.«field.getterMethodName»();
+        «ENDFOR»
+    '''
+
+    override protected generateCopyAugmentation(Type implType) '''
+        this.«AUGMENTATION_FIELD» = «ImmutableMap.importedName».copyOf(base.«AUGMENTATION_FIELD»);
+    '''
 }
\ No newline at end of file

diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderTemplate.xtend
index d2be1ba763ddd6244458c0a13194efec61b92048..8c595ba6416989eefefa827d68e0b2cdf6c0db7c 100644 (file)
--- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderTemplate.xtend
+++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderTemplate.xtend
@@ -27,7 +27,9 @@ import org.opendaylight.mdsal.binding.model.api.ParameterizedType
 import org.opendaylight.mdsal.binding.model.api.Type
 import org.opendaylight.mdsal.binding.model.util.TypeConstants
 import org.opendaylight.mdsal.binding.model.util.Types
+import org.opendaylight.mdsal.binding.spec.naming.BindingMapping
 import org.opendaylight.yangtools.concepts.Builder
+import org.opendaylight.yangtools.yang.binding.AugmentationHolder
 import org.opendaylight.yangtools.yang.binding.CodeHelpers
 import org.opendaylight.yangtools.yang.binding.DataObject
 
@@ -71,7 +73,7 @@ class BuilderTemplate extends AbstractBuilderTemplate {
 
             «generateConstructorsFromIfcs()»
 
-            «generateCopyConstructor(false, targetType, type.enclosedTypes.get(0))»
+            public «generateCopyConstructor(targetType, type.enclosedTypes.get(0))»
 
             «generateMethodFieldsFrom()»
 
@@ -346,5 +348,33 @@ class BuilderTemplate extends AbstractBuilderTemplate {
             «ENDIF»
         '''.toString
     }
+
+    override protected generateCopyKeys(List<GeneratedProperty> keyProps) '''
+        this.key = base.«BindingMapping.IDENTIFIABLE_KEY_NAME»();
+        «FOR field : keyProps»
+            this.«field.fieldName» = base.«field.getterMethodName»();
+        «ENDFOR»
+    '''
+
+    override protected generateCopyAugmentation(Type implType) {
+        val implTypeRef = implType.importedName
+        val augmentationHolderRef = AugmentationHolder.importedName
+        val typeRef = targetType.importedName
+        val hashMapRef = HashMap.importedName
+        return '''
+            if (base instanceof «implTypeRef») {
+                «implTypeRef» impl = («implTypeRef») base;
+                if (!impl.«AUGMENTATION_FIELD».isEmpty()) {
+                    this.«AUGMENTATION_FIELD» = new «hashMapRef»<>(impl.«AUGMENTATION_FIELD»);
+                }
+            } else if (base instanceof «augmentationHolderRef») {
+                @SuppressWarnings("unchecked")
+                «augmentationHolderRef»<«typeRef»> casted =(«augmentationHolderRef»<«typeRef»>) base;
+                if (!casted.augmentations().isEmpty()) {
+                    this.«AUGMENTATION_FIELD» = new «hashMapRef»<>(casted.augmentations());
+                }
+            }
+        '''
+    }
 }