Generate legacy value contructors for all classes

This expands legacy constructor compatibility to also include
Key classes, not only value wrappers. The codegen parts is relatively
straightforward.

IdentitiableItemCodec is coded on the assumption there are only
two constructors -- one copy and one all-value, which is now violated.

Hence we update IdentifiableItemCodec to also ignore any constructors
which are marked as deprecated.

JIRA: MDSAL-330
Change-Id: Ie23ba8b3788b320b2d11e263c9af228b4c2e41a9
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/IdentifiableItemCodec.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/IdentifiableItemCodec.java
index 15f914eb20defcf386c43b397d27d4f63f554f0c..ecad6df385ed59efcfdc141396b2a00668b3e53d 100644 (file)
--- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/IdentifiableItemCodec.java
+++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/IdentifiableItemCodec.java
@@ -31,6 +31,8 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier.IdentifiableIt
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
 import org.opendaylight.yangtools.yang.model.api.ListSchemaNode;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Codec support for extracting the {@link Identifiable#key()} method return from a MapEntryNode.
@@ -49,7 +51,7 @@ abstract class IdentifiableItemCodec
             super(schema, keyClass, identifiable);
             this.keyContext = requireNonNull(keyContext);
             this.keyName = requireNonNull(keyName);
-            ctor = getConstructor(keyClass).asType(CTOR_TYPE);
+            ctor = getConstructor(keyClass, 1).asType(CTOR_TYPE);
         }
 
         @Override
@@ -73,7 +75,7 @@ abstract class IdentifiableItemCodec
                 final Class<?> identifiable, final Map<QName, ValueContext> keyValueContexts) {
             super(schema, keyClass, identifiable);
 
-            final MethodHandle tmpCtor = getConstructor(keyClass);
+            final MethodHandle tmpCtor = getConstructor(keyClass, keyValueContexts.size());
             final MethodHandle inv = MethodHandles.spreadInvoker(tmpCtor.type(), 0);
             this.ctor = inv.asType(inv.type().changeReturnType(Identifier.class)).bindTo(tmpCtor);
 
@@ -120,6 +122,8 @@ abstract class IdentifiableItemCodec
         }
     }
 
+    private static final Logger LOG = LoggerFactory.getLogger(IdentifiableItemCodec.class);
+
     private final Class<?> identifiable;
     private final QName qname;
 
@@ -171,18 +175,37 @@ abstract class IdentifiableItemCodec
 
     abstract @NonNull NodeIdentifierWithPredicates serializeIdentifier(QName qname, Identifier<?> key);
 
-    static MethodHandle getConstructor(final Class<? extends Identifier<?>> clazz) {
-        for (@SuppressWarnings("rawtypes") final Constructor constr : clazz.getConstructors()) {
-            final Class<?>[] parameters = constr.getParameterTypes();
-            if (!clazz.equals(parameters[0])) {
-                // It is not copy constructor...
-                try {
-                    return MethodHandles.publicLookup().unreflectConstructor(constr);
-                } catch (IllegalAccessException e) {
-                    throw new IllegalStateException("Cannot access constructor " + constr + " in class " + clazz, e);
-                }
+    static MethodHandle getConstructor(final Class<? extends Identifier<?>> clazz, final int nrArgs) {
+        for (final Constructor<?> ctor : clazz.getConstructors()) {
+            // Check argument count
+            if (ctor.getParameterCount() != nrArgs) {
+                LOG.debug("Skipping {} due to argument count mismatch", ctor);
+                continue;
+            }
+
+            // Do not consider deprecated constructors
+            if (isDeprecated(ctor)) {
+                LOG.debug("Skipping deprecated constructor {}", ctor);
+                continue;
+            }
+
+            // Do not consider copy constructors
+            if (clazz.equals(ctor.getParameterTypes()[0])) {
+                LOG.debug("Skipping copy constructor {}", ctor);
+                continue;
+            }
+
+            try {
+                return MethodHandles.publicLookup().unreflectConstructor(ctor);
+            } catch (IllegalAccessException e) {
+                throw new IllegalStateException("Cannot access constructor " + ctor + " in class " + clazz, e);
             }
         }
-        throw new IllegalArgumentException("Supplied class " + clazz + "does not have required constructor.");
+        throw new IllegalArgumentException("Supplied class " + clazz + " does not have required constructor.");
+    }
+
+    // This could be inlined, but then it throws off Eclipse analysis, which thinks the return is always non-null
+    private static boolean isDeprecated(final Constructor<?> ctor) {
+        return ctor.getAnnotation(Deprecated.class) != null;
     }
 }

diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ClassTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ClassTemplate.xtend
index 35dc26bd7c9fd91bbc071a9f3810c8adeae76164..6b8dc16b3f2b79294d7dcd017c7a0042d6fc5c8e 100644 (file)
--- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ClassTemplate.xtend
+++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ClassTemplate.xtend
@@ -203,8 +203,10 @@ class ClassTemplate extends BaseTemplate {
             «genUnionConstructor»
         «ELSEIF genTO.typedef && allProperties.size == 1 && allProperties.get(0).name.equals("value")»
             «typedefConstructor»
+            «legacyConstructor»
         «ELSE»
             «allValuesConstructor»
+            «legacyConstructor»
         «ENDIF»
 
         «IF !allProperties.empty»
@@ -261,23 +263,31 @@ class ClassTemplate extends BaseTemplate {
             «ENDIF»
         «ENDFOR»
     }
-    «val propType = allProperties.get(0).returnType»
-    «val uintType = UINT_TYPES.get(propType)»
-    «IF uintType !== null»
-
-        /**
-         * Utility migration constructor.
-         *
-         * @param value Wrapped value in legacy type
-         * @deprecated Use {#link «type.name»(«propType.importedName»)} instead.
-         */
-        @Deprecated(forRemoval = true)
-        public «type.getName»(final «uintType.importedName» value) {
-            this(«CodeHelpers.importedName».compatUint(value));
-        }
-    «ENDIF»
     '''
 
+    def private legacyConstructor() {
+        if (!hasUintProperties) {
+            return ""
+        }
+
+        val compatUint = CodeHelpers.importedName + ".compatUint("
+        return '''
+
+            /**
+             * Utility migration constructor.
+             *
+             «FOR prop : allProperties»
+             * @param «prop.fieldName» «prop.name»«IF prop.isUintType» in legacy Java type«ENDIF»
+             «ENDFOR»
+             * @deprecated Use {#link «type.name»(«FOR prop : allProperties SEPARATOR ", "»«prop.returnType.importedName»«ENDFOR»)} instead.
+             */
+            @Deprecated(forRemoval = true)
+            public «type.getName»(«FOR prop : allProperties SEPARATOR ", "»«prop.legacyType.importedName» «prop.fieldName»«ENDFOR») {
+                this(«FOR prop : allProperties SEPARATOR ", "»«IF prop.isUintType»«compatUint»«prop.fieldName»)«ELSE»«prop.fieldName»«ENDIF»«ENDFOR»);
+            }
+        '''
+    }
+
     def protected genUnionConstructor() '''
     «FOR p : allProperties»
         «val List<GeneratedProperty> other = new ArrayList(properties)»
@@ -576,7 +586,28 @@ class ClassTemplate extends BaseTemplate {
                 return prop;
             }
         }
-        return null;
+        return null
     }
 
+    def private hasUintProperties() {
+        for (GeneratedProperty prop : allProperties) {
+            if (prop.isUintType) {
+                return true
+            }
+        }
+        return false
+    }
+
+    def private static isUintType(GeneratedProperty prop) {
+        UINT_TYPES.containsKey(prop.returnType)
+    }
+
+    def private static legacyType(GeneratedProperty prop) {
+        val type = prop.returnType
+        val uint = UINT_TYPES.get(type)
+        if (uint !== null) {
+            return uint
+        }
+        return type
+    }
 }