Binding DTOs: improve equals() implementation 36/89736/22
authorVladyslav Marchenko <vladyslav.marchenko@pantheon.tech>
Wed, 13 May 2020 10:09:03 +0000 (13:09 +0300)
committerRobert Varga <robert.varga@pantheon.tech>
Mon, 20 Jul 2020 18:02:48 +0000 (20:02 +0200)
DTO generator code recognize the types being compared and perform the following ordering:
1) numeric types (boolean, byte, short, int, long, biginteger, bigdecimal), identityrefs
2) string, binary, bits
3) instance identifier
4) all other (e.g. composite) types

JIRA: MDSAL-88
Change-Id: I8f64ad3cf234e4f0f16d608bd397b7f3fa96d785
Signed-off-by: Vladyslav Marchenko <vladyslav.marchenko@pantheon.tech>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/BindingTypes.java
binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/Types.java
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGenerator.java
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderImplTemplate.xtend
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ByTypeMemberComparator.java [new file with mode: 0644]
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/InterfaceTemplate.xtend
binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGeneratorTest.java
binding/mdsal-binding-java-api-generator/src/test/resources/test-types.yang [new file with mode: 0644]

index 591f4f35fc2980800f063687a205e0ed2227c8b5..92663dd30418f3a84cad22a57f683f3d1c61ae62 100644 (file)
@@ -58,6 +58,8 @@ public final class BindingTypes {
     public static final ConcreteType RPC_OUTPUT = typeForClass(RpcOutput.class);
     public static final ConcreteType RPC_SERVICE = typeForClass(RpcService.class);
     public static final ConcreteType SCALAR_TYPE_OBJECT = typeForClass(ScalarTypeObject.class);
+    public static final ConcreteType INSTANCE_IDENTIFIER = typeForClass(InstanceIdentifier.class);
+    public static final ConcreteType KEYED_INSTANCE_IDENTIFIER = typeForClass(KeyedInstanceIdentifier.class);
 
     // This is an annotation, we are current just referencing the type
     public static final JavaTypeName ROUTING_CONTEXT = JavaTypeName.create(RoutingContext.class);
@@ -70,14 +72,11 @@ public final class BindingTypes {
     static final ConcreteType IDENTIFIABLE = typeForClass(Identifiable.class);
     @VisibleForTesting
     static final ConcreteType IDENTIFIER = typeForClass(Identifier.class);
-    @VisibleForTesting
-    static final ConcreteType INSTANCE_IDENTIFIER = typeForClass(InstanceIdentifier.class);
 
     private static final ConcreteType ACTION = typeForClass(Action.class);
     private static final ConcreteType CHILD_OF = typeForClass(ChildOf.class);
     private static final ConcreteType CHOICE_IN = typeForClass(ChoiceIn.class);
     private static final ConcreteType INSTANCE_NOTIFICATION = typeForClass(InstanceNotification.class);
-    private static final ConcreteType KEYED_INSTANCE_IDENTIFIER = typeForClass(KeyedInstanceIdentifier.class);
     private static final ConcreteType KEYED_LIST_ACTION = typeForClass(KeyedListAction.class);
     private static final ConcreteType KEYED_LIST_NOTIFICATION = typeForClass(KeyedListNotification.class);
     private static final ConcreteType OPAQUE_OBJECT = typeForClass(OpaqueObject.class);
index 288fca8a3ab69c21c0c5fa7015429361a4854ac7..deae83c0e82b30f16e8ef13d1834d683713e8966 100644 (file)
@@ -54,9 +54,9 @@ public final class Types {
     public static final @NonNull ConcreteType STRING = typeForClass(String.class);
     public static final @NonNull ConcreteType VOID = typeForClass(Void.class);
     public static final @NonNull ConcreteType BYTE_ARRAY = typeForClass(byte[].class);
+    public static final @NonNull ConcreteType CLASS = typeForClass(Class.class);
 
     private static final @NonNull ConcreteType BUILDER = typeForClass(Builder.class);
-    private static final @NonNull ConcreteType CLASS = typeForClass(Class.class);
     private static final @NonNull ConcreteType LIST_TYPE = typeForClass(List.class);
     private static final @NonNull ConcreteType LISTENABLE_FUTURE = typeForClass(ListenableFuture.class);
     private static final @NonNull ConcreteType MAP_TYPE = typeForClass(Map.class);
index 392ff4af1d7728b39e3e0416900b329b231476b0..e8399a9672e83f143a33ced3fabc7148afd3bbe4 100644 (file)
@@ -68,17 +68,16 @@ public final class BuilderGenerator implements CodeGenerator {
 
     @VisibleForTesting
     static BuilderTemplate templateForType(final GeneratedType type) {
-        final GeneratedType genType = type;
-        final JavaTypeName origName = genType.getIdentifier();
+        final JavaTypeName origName = type.getIdentifier();
 
         final GeneratedTypeBuilder builderTypeBuilder = new CodegenGeneratedTypeBuilder(
             origName.createSibling(origName.simpleName() + BuilderTemplate.BUILDER_STR));
 
         final GeneratedTOBuilder implTypeBuilder = builderTypeBuilder.addEnclosingTransferObject(
             origName.simpleName() + "Impl");
-        implTypeBuilder.addImplementsType(genType);
+        implTypeBuilder.addImplementsType(type);
 
-        return new BuilderTemplate(builderTypeBuilder.build(), genType, getKey(genType));
+        return new BuilderTemplate(builderTypeBuilder.build(), type, getKey(type));
     }
 
     private static Type getKey(final GeneratedType type) {
index be38f4995cfe822447921eafe873e8684cd81ad4..040e392cc5a8019d935869301d2f30aa9bff6459 100644 (file)
@@ -18,11 +18,11 @@ import java.util.List
 import org.opendaylight.mdsal.binding.model.api.AnnotationType
 import org.opendaylight.mdsal.binding.model.api.GeneratedProperty
 import org.opendaylight.mdsal.binding.model.api.GeneratedType
+import org.opendaylight.mdsal.binding.model.api.MethodSignature.ValueMechanics
 import org.opendaylight.mdsal.binding.model.api.Type
 import org.opendaylight.mdsal.binding.model.util.Types
 import org.opendaylight.mdsal.binding.spec.naming.BindingMapping
 import org.opendaylight.yangtools.yang.binding.AbstractAugmentable
-import org.opendaylight.mdsal.binding.model.api.MethodSignature.ValueMechanics
 
 class BuilderImplTemplate extends AbstractBuilderTemplate {
     val Type builderType;
diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ByTypeMemberComparator.java b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/ByTypeMemberComparator.java
new file mode 100644 (file)
index 0000000..9dfcc8e
--- /dev/null
@@ -0,0 +1,166 @@
+/*
+ * Copyright (c) 2020 PANTHEON.tech, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.mdsal.binding.java.api.generator;
+
+import com.google.common.annotations.Beta;
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Set;
+import org.eclipse.jdt.annotation.NonNull;
+import org.opendaylight.mdsal.binding.model.api.ConcreteType;
+import org.opendaylight.mdsal.binding.model.api.GeneratedProperty;
+import org.opendaylight.mdsal.binding.model.api.GeneratedTransferObject;
+import org.opendaylight.mdsal.binding.model.api.ParameterizedType;
+import org.opendaylight.mdsal.binding.model.api.Type;
+import org.opendaylight.mdsal.binding.model.api.TypeMember;
+import org.opendaylight.mdsal.binding.model.util.BaseYangTypes;
+import org.opendaylight.mdsal.binding.model.util.BindingTypes;
+import org.opendaylight.mdsal.binding.model.util.TypeConstants;
+import org.opendaylight.mdsal.binding.model.util.Types;
+import org.opendaylight.yangtools.yang.model.api.TypeDefinition;
+import org.opendaylight.yangtools.yang.model.api.type.BitsTypeDefinition;
+
+/**
+ * By type member {@link Comparator} which provides sorting by type for members (variables)
+ * in a generated class.
+ *
+ * @param <T> TypeMember type
+ */
+@Beta
+final class ByTypeMemberComparator<T extends TypeMember> implements Comparator<T>, Serializable {
+    private static final long serialVersionUID = 1L;
+
+    /**
+     * Fixed-size comparison. These are all numeric types, boolean, empty, identityref.
+     */
+    private static final int RANK_FIXED_SIZE          = 0;
+    /**
+     * Variable-sized comparison across simple components. These are string, binary and bits type.
+     */
+    private static final int RANK_VARIABLE_ARRAY      = 1;
+    /**
+     * Variable-size comparison across complex components.
+     */
+    private static final int RANK_INSTANCE_IDENTIFIER = 2;
+    /**
+     * Composite structure. DataObject, OpaqueObject and similar.
+     */
+    private static final int RANK_COMPOSITE           = 3;
+
+    private static final Set<Type> FIXED_TYPES = Set.of(
+        BaseYangTypes.INT8_TYPE,
+        BaseYangTypes.INT16_TYPE,
+        BaseYangTypes.INT32_TYPE,
+        BaseYangTypes.INT64_TYPE,
+        BaseYangTypes.DECIMAL64_TYPE,
+        BaseYangTypes.UINT8_TYPE,
+        BaseYangTypes.UINT16_TYPE,
+        BaseYangTypes.UINT32_TYPE,
+        BaseYangTypes.UINT64_TYPE,
+        BaseYangTypes.BOOLEAN_TYPE,
+        BaseYangTypes.EMPTY_TYPE,
+        Types.CLASS);
+
+    /**
+     * Singleton instance.
+     */
+    private static final @NonNull ByTypeMemberComparator<?> INSTANCE = new ByTypeMemberComparator<>();
+
+    private ByTypeMemberComparator() {
+        // Hidden on purpose
+    }
+
+    /**
+     * Returns the one and only instance of this class.
+     *
+     * @return this comparator
+     */
+    @SuppressWarnings("unchecked")
+    public static <T extends TypeMember> ByTypeMemberComparator<T> getInstance() {
+        return (ByTypeMemberComparator<T>) INSTANCE;
+    }
+
+    public static <T extends TypeMember> Collection<T> sort(final Collection<T> input) {
+        if (input.size() < 2) {
+            return input;
+        }
+
+        final List<T> ret = new ArrayList<>(input);
+        ret.sort(getInstance());
+        return ret;
+    }
+
+    @Override
+    public int compare(final T member1, final T member2) {
+        final Type type1 = getConcreteType(member1.getReturnType());
+        final Type type2 = getConcreteType(member2.getReturnType());
+        if (!type1.getIdentifier().equals(type2.getIdentifier())) {
+            final int cmp = rankOf(type1) - rankOf(type2);
+            if (cmp != 0) {
+                return cmp;
+            }
+        }
+        return member1.getName().compareTo(member2.getName());
+    }
+
+    @SuppressWarnings("static-method")
+    private Object readResolve() {
+        return INSTANCE;
+    }
+
+    private static Type getConcreteType(final Type type) {
+        if (type instanceof ConcreteType) {
+            return type;
+        } else if (type instanceof ParameterizedType) {
+            return ((ParameterizedType) type).getRawType();
+        } else if (type instanceof GeneratedTransferObject) {
+            GeneratedTransferObject rootGto = (GeneratedTransferObject) type;
+            while (rootGto.getSuperType() != null) {
+                rootGto = rootGto.getSuperType();
+            }
+            for (GeneratedProperty s : rootGto.getProperties()) {
+                if (TypeConstants.VALUE_PROP.equals(s.getName())) {
+                    return s.getReturnType();
+                }
+            }
+        }
+        return type;
+    }
+
+    private static int rankOf(final Type type) {
+        if (FIXED_TYPES.contains(type)) {
+            return RANK_FIXED_SIZE;
+        }
+        if (type.equals(BaseYangTypes.STRING_TYPE) || type.equals(Types.BYTE_ARRAY)) {
+            return RANK_VARIABLE_ARRAY;
+        }
+        if (type.equals(BindingTypes.INSTANCE_IDENTIFIER) || type.equals(BindingTypes.KEYED_INSTANCE_IDENTIFIER)) {
+            return RANK_INSTANCE_IDENTIFIER;
+        }
+        if (type instanceof GeneratedTransferObject) {
+            final TypeDefinition<?> typedef = topParentTransportObject((GeneratedTransferObject) type).getBaseType();
+            if (typedef instanceof BitsTypeDefinition) {
+                return RANK_VARIABLE_ARRAY;
+            }
+        }
+        return RANK_COMPOSITE;
+    }
+
+    private static GeneratedTransferObject topParentTransportObject(final GeneratedTransferObject type) {
+        GeneratedTransferObject ret = type;
+        GeneratedTransferObject parent = ret.getSuperType();
+        while (parent != null) {
+            ret = parent;
+            parent = ret.getSuperType();
+        }
+        return ret;
+    }
+}
index 8c3ae346644b7820304f39dc7278f48a9c374f63..61dbcf89a4809d91825cf09caf31c7b433a3c0cb 100644 (file)
@@ -281,7 +281,7 @@ class InterfaceTemplate extends BaseTemplate {
                 if (other == null) {
                     return false;
                 }
-                «FOR property : typeAnalysis.value»
+                «FOR property : ByTypeMemberComparator.sort(typeAnalysis.value)»
                     if (!«property.importedUtilClass».equals(thisObj.«property.getterName»(), other.«property.getterName»())) {
                         return false;
                     }
index 8415c74cddec270e11d857cdc745b65d795f9c42..b2317e15f6db8132225dde649ea632b15e8d5b52 100644 (file)
@@ -14,12 +14,16 @@ import static org.mockito.Mockito.spy;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.stream.Collectors;
 import org.junit.Test;
+import org.opendaylight.mdsal.binding.generator.impl.DefaultBindingGenerator;
 import org.opendaylight.mdsal.binding.model.api.GeneratedType;
 import org.opendaylight.mdsal.binding.model.api.JavaTypeName;
 import org.opendaylight.mdsal.binding.model.api.MethodSignature;
 import org.opendaylight.mdsal.binding.model.api.MethodSignature.ValueMechanics;
 import org.opendaylight.mdsal.binding.model.api.Type;
+import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext;
+import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils;
 
 public class BuilderGeneratorTest {
     private static final String TEST = "test";
@@ -152,6 +156,34 @@ public class BuilderGeneratorTest {
                 + "}\n", genToString(mockAugment(mockGenTypeMoreMeth("get" + TEST))).toString());
     }
 
+    @Test
+    public void builderTemplateGenerateToEqualsComparingOrderTest() {
+        final EffectiveModelContext context = YangParserTestUtils.parseYangResource(
+                "/test-types.yang");
+        final List<Type> types = new DefaultBindingGenerator().generateTypes(context);
+        final BuilderTemplate bt = BuilderGenerator.templateForType((GeneratedType) types.get(19));
+
+        final List<String> sortedProperties = bt.properties.stream()
+                .sorted(ByTypeMemberComparator.getInstance())
+                .map(BuilderGeneratedProperty::getName)
+                .collect(Collectors.toList());
+
+        assertEquals(List.of(
+                // numeric types (boolean, byte, short, int, long, biginteger, bigdecimal), identityrefs, empty
+                "id16", "id16Def", "id32", "id32Def", "id64", "id64Def", "id8", "id8Def", "idBoolean", "idBooleanDef",
+                "idDecimal64", "idDecimal64Def","idEmpty", "idEmptyDef", "idIdentityref", "idIdentityrefDef",
+                "idLeafref", "idLeafrefDef", "idU16", "idU16Def", "idU32", "idU32Def", "idU64", "idU64Def", "idU8",
+                "idU8Def",
+                // string, binary, bits
+                "idBinary", "idBinaryDef", "idBits", "idBitsDef", "idGroupLeafString", "idLeafrefContainer1",
+                "idLeafrefContainer1Def", "idString", "idStringDef",
+                // instance identifier
+                "idInstanceIdentifier", "idInstanceIdentifierDef",
+                // other types
+                "idContainer1", "idContainer2", "idEnumeration", "idEnumerationDef",
+                "idGroupContainer", "idList", "idUnion", "idUnionDef"), sortedProperties);
+    }
+
     private static GeneratedType mockAugment(final GeneratedType genType) {
         final List<Type> impls = new ArrayList<>();
         final Type impl = mock(Type.class);
diff --git a/binding/mdsal-binding-java-api-generator/src/test/resources/test-types.yang b/binding/mdsal-binding-java-api-generator/src/test/resources/test-types.yang
new file mode 100644 (file)
index 0000000..7e905c8
--- /dev/null
@@ -0,0 +1,298 @@
+/*
+ * Copyright (c) 2020 PANTHEON.tech, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+module test-types {
+    yang-version 1;
+    namespace "urn:opendaylight:test-types";
+    prefix "t";
+
+    revision "2020-05-13" {
+    }
+
+    typedef boolean-def {
+        type boolean;
+    }
+
+    typedef string-def {
+        type string;
+    }
+    typedef int8-def {
+        type int8;
+    }
+    typedef int16-def {
+        type int16;
+    }
+    typedef int32-def {
+        type int32;
+    }
+    typedef int64-def {
+        type int64;
+    }
+    typedef u8-def {
+        type uint8;
+    }
+    typedef u16-def {
+        type uint16;
+    }
+    typedef u32-def {
+        type uint32;
+    }
+    typedef u64-def {
+        type uint64;
+    }
+    typedef union-def {
+        type union {
+            type string;
+            type binary;
+        }
+    }
+    typedef identityref-def {
+        type identityref {
+            base alg;
+        }
+    }
+    typedef binary-def {
+        type binary {
+            length 1..10;
+        }
+    }
+    typedef bits-def {
+        type bits {
+            bit ctrl;
+            bit alt {
+                position 5;
+            }
+            bit delete;
+        }
+    }
+    typedef instance-identifier-def {
+        type instance-identifier;
+    }
+    typedef decimal64-def {
+        type decimal64 {
+            fraction-digits 4;
+            range "1.5..5.5";
+        }
+    }
+    typedef empty-def {
+        type empty;
+    }
+    typedef enumeration-def {
+        type enumeration {
+            enum zero;
+            enum one;
+            enum seven {
+                value 7;
+            }
+        }
+    }
+    typedef leafref-def {
+        type leafref {
+            path "/nodes/id-64";
+        }
+    }
+    typedef leafref-container1-def {
+        type leafref {
+            path "/nodes/id-container1/node-id-string";
+        }
+    }
+    container container1-def {
+        leaf node-id-string {
+            type string;
+        }
+    }
+
+    grouping id-group {
+        container id-group-container {
+            leaf id-group-leaf-16 {
+                type int16;
+            }
+        }
+        leaf id-group-leaf-string {
+            type string;
+        }
+    }
+
+
+    container nodes {
+        leaf id-boolean {
+            type boolean;
+        }
+        leaf id-boolean-def {
+            type boolean-def;
+        }
+        leaf id-string {
+            type string;
+        }
+        leaf id-string-def {
+            type string-def;
+        }
+        leaf id-8 {
+            type int8;
+        }
+        leaf id-8-def {
+            type int8-def;
+        }
+        leaf id-16 {
+            type int16;
+        }
+        leaf id-16-def {
+            type int16-def;
+        }
+        leaf id-32 {
+            type int32;
+        }
+        leaf id-32-def {
+            type int32-def;
+        }
+        leaf id-64 {
+            type int64;
+        }
+        leaf id-64-def {
+            type int64-def;
+        }
+        leaf id-u8 {
+            type uint8;
+        }
+        leaf id-u16 {
+            type uint16;
+        }
+        leaf id-u32 {
+            type uint32;
+        }
+        leaf id-u64 {
+            type uint64;
+        }
+        leaf id-union {
+            type union {
+                type string;
+                type binary;
+            }
+        }
+        leaf id-u8-def {
+            type u8-def;
+        }
+        leaf id-u16-def {
+            type u16-def;
+        }
+        leaf id-u32-def {
+            type u32-def;
+        }
+        leaf id-u64-def {
+            type u64-def;
+        }
+        leaf id-union-def {
+            type union-def;
+        }
+        leaf id-decimal64 {
+            type decimal64 {
+                fraction-digits 4;
+                range "1.5..5.5";
+            }
+        }
+        leaf id-decimal64-def {
+            type decimal64-def;
+        }
+
+        leaf id-identityref {
+            type identityref {
+                base alg;
+            }
+        }
+        leaf id-identityref-def {
+            type identityref-def;
+        }
+        leaf id-binary {
+            type binary {
+                length 1..10;
+            }
+        }
+        leaf id-binary-def {
+            type binary-def;
+        }
+        leaf id-bits {
+            type bits {
+                bit ctrl;
+                bit alt {
+                    position 5;
+                }
+                bit delete;
+            }
+        }
+        leaf id-bits-def {
+            type bits-def;
+        }
+        leaf id-instance-identifier {
+            type instance-identifier;
+        }
+        leaf id-instance-identifier-def {
+            type instance-identifier-def;
+        }
+        leaf id-empty {
+            type empty;
+        }
+        leaf id-empty-def {
+            type empty-def;
+        }
+        leaf id-enumeration {
+            type enumeration {
+                enum zero;
+                enum one;
+                enum seven {
+                    value 7;
+                }
+            }
+        }
+        leaf id-enumeration-def {
+            type enumeration-def;
+        }
+        leaf id-leafref {
+            type leafref {
+                path "/nodes/id-64";
+            }
+        }
+        leaf id-leafref-def {
+            type leafref-def;
+        }
+        leaf id-leafref-container1 {
+            type leafref {
+                path "/nodes/id-container1/node-id-string";
+            }
+        }
+        leaf id-leafref-container1-def {
+            type leafref-container1-def;
+        }
+        container id-container1 {
+            leaf node-id-string {
+                type string;
+            }
+        }
+        container id-container2 {
+            leaf node-id-string {
+                type string;
+            }
+            leaf node-id-int16 {
+                type int16;
+            }
+        }
+        list id-list {
+            key "key1";
+
+            leaf key1 {
+                type int8;
+            }
+
+            leaf key2 {
+                type string;
+            }
+        }
+        uses id-group;
+    }
+
+    identity alg {
+    }
+}