Make sure we compare key members via their property name

As it turns out, simple comparison ignoring cases does not correctly
work if the schema is using CamelCase. We need to do the hard work of
deriving Binding property name and sort on that to be consistent with
what the codegen does.

JIRA: MDSAL-355
Change-Id: I34cdb032fbdeb093e2973b91c5011e302e6280bc
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit f1353f855b99a1f1fd2e5c32b0f58ecf83398600)

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 57b45a2d7199e1efec7f590dbb64cfa710165eaa..7913f4ecd84ada75391592d30fc033c46a9d52e7 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
@@ -14,12 +14,13 @@ import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.reflect.Constructor;
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.Comparator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import org.opendaylight.yangtools.concepts.Codec;
+import org.opendaylight.yangtools.yang.binding.BindingMapping;
 import org.opendaylight.yangtools.yang.binding.Identifier;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier.IdentifiableItem;
 import org.opendaylight.yangtools.yang.common.QName;
@@ -64,7 +65,7 @@ final class IdentifiableItemCodec implements Codec<NodeIdentifierWithPredicates,
          *
          * We do not have to perform a sort if the source collection has less than two
          * elements.
-
+         *
          * We always perform an ImmutableList.copyOf(), as that will turn into a no-op
          * if the source is already immutable. It will also produce optimized implementations
          * for empty and singleton collections.
@@ -75,7 +76,8 @@ final class IdentifiableItemCodec implements Codec<NodeIdentifierWithPredicates,
         final List<QName> sortedKeys;
         if (unsortedKeys.size() > 1) {
             final List<QName> tmp = new ArrayList<>(unsortedKeys);
-            Collections.sort(tmp, (q1, q2) -> q1.getLocalName().compareToIgnoreCase(q2.getLocalName()));
+            // This is not terribly efficient but gets the job done
+            tmp.sort(Comparator.comparing(qname -> BindingMapping.getPropertyName(qname.getLocalName())));
             sortedKeys = tmp;
         } else {
             sortedKeys = unsortedKeys;

diff --git a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/test/InstanceIdentifierTest.java b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/test/InstanceIdentifierTest.java
index 1a9577799ee4183877722df9c97f5cf16dee7a1a..f945da05cf37dbe6ca056fdf68a5323612925b61 100644 (file)
--- a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/test/InstanceIdentifierTest.java
+++ b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/test/InstanceIdentifierTest.java
@@ -10,20 +10,27 @@ package org.opendaylight.mdsal.binding.dom.codec.test;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+import com.google.common.collect.ImmutableMap;
 import javassist.ClassPool;
 import org.junit.Test;
 import org.opendaylight.mdsal.binding.dom.codec.gen.impl.StreamWriterGenerator;
 import org.opendaylight.mdsal.binding.dom.codec.impl.BindingNormalizedNodeCodecRegistry;
 import org.opendaylight.mdsal.binding.generator.util.JavassistUtils;
+import org.opendaylight.yang.gen.v1.mdsal._355.norev.OspfStatLsdbBrief;
+import org.opendaylight.yang.gen.v1.mdsal._355.norev.OspfStatLsdbBriefKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.augment.rev140709.TreeComplexUsesAugment;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.augment.rev140709.TreeLeafOnlyAugment;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.Top;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.two.level.list.TopLevelList;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.two.level.list.TopLevelListKey;
+import org.opendaylight.yangtools.yang.binding.Identifier;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+import org.opendaylight.yangtools.yang.binding.KeyedInstanceIdentifier;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.AugmentationIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 
 public class InstanceIdentifierTest extends AbstractBindingRuntimeTest {
@@ -63,4 +70,17 @@ public class InstanceIdentifierTest extends AbstractBindingRuntimeTest {
         assertTrue(((AugmentationIdentifier) leafOnlyLastArg).getPossibleChildNames().contains(SIMPLE_VALUE_QNAME));
     }
 
+    @Test
+    public void testCamelCaseKeys() {
+        final InstanceIdentifier<?> result = registry.fromYangInstanceIdentifier(YangInstanceIdentifier.create(
+            NodeIdentifier.create(OspfStatLsdbBrief.QNAME),
+            new NodeIdentifierWithPredicates(OspfStatLsdbBrief.QNAME, ImmutableMap.of(
+                QName.create(OspfStatLsdbBrief.QNAME, "AreaIndex"), 1,
+                QName.create(OspfStatLsdbBrief.QNAME, "LsaType"), (short) 2,
+                QName.create(OspfStatLsdbBrief.QNAME, "LsId"), 3,
+                QName.create(OspfStatLsdbBrief.QNAME, "AdvRtr"), "foo"))));
+        assertTrue(result instanceof KeyedInstanceIdentifier);
+        final Identifier<?> key = ((KeyedInstanceIdentifier<?, ?>) result).getKey();
+        assertEquals(new OspfStatLsdbBriefKey("foo", 1, 3, (short)2), key);
+    }
 }

diff --git a/binding/mdsal-binding-test-model/src/main/yang/mdsal-355.yang b/binding/mdsal-binding-test-model/src/main/yang/mdsal-355.yang
new file mode 100644 (file)
index 0000000..a1effc5
--- /dev/null
+++ b/binding/mdsal-binding-test-model/src/main/yang/mdsal-355.yang
@@ -0,0 +1,13 @@
+module mdsal-355 {
+       namespace "mdsal-355";
+       prefix "mdsal355";
+
+    list OspfStatLsdbBrief {
+        key "AreaIndex LsaType LsId AdvRtr";
+        leaf AreaIndex { type int32; description "Area Index"; }
+        leaf LsaType { type uint8; description "LSA Type"; }
+        leaf LsId { type int32; description "LSA ID"; }
+        leaf AdvRtr { type string; description "Advertised Router"; }
+    }
+}
+