From 9877defab78a0ea397c0acfc8386c2f7bc357d9f Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Thu, 5 Sep 2019 15:11:38 +0200 Subject: [PATCH] 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 --- .../dom/codec/impl/IdentifiableItemCodec.java | 49 +++++++++++---- .../java/api/generator/ClassTemplate.xtend | 63 ++++++++++++++----- 2 files changed, 83 insertions(+), 29 deletions(-) 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 15f914eb20..ecad6df385 100644 --- 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 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> 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> 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 35dc26bd7c..6b8dc16b3f 100644 --- 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 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 + } } -- 2.36.6