Normalize predicate order in ImmutableNodes.fromInstanceId()

The only user of this method is NETCONF, which requires predicates
to be ordered in the order of key definition, as per XML encoding
rules.

Make sure we re-create NodeIdentifierWithPredicates in the schema
definition order if the provided order does not already match it.

JIRA: YANGTOOLS-1037
Change-Id: I57c389eeecb9680062be8b56c834eb569e7d3d3b
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 4f869bbe2f963d9168eb659bc1761bfd174e6ab3)

diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/InstanceIdToCompositeNodes.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/InstanceIdToCompositeNodes.java
index 8c7695517fb5b1d2c90aab37aac2d81ebc735b82..94ed1545060bf52722f39e6b7e15a2100a76e18b 100644 (file)
--- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/InstanceIdToCompositeNodes.java
+++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/InstanceIdToCompositeNodes.java
@@ -8,17 +8,23 @@
 package org.opendaylight.yangtools.yang.data.impl.schema;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Verify.verify;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
 import java.util.Collection;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 import org.eclipse.jdt.annotation.NonNull;
+import org.opendaylight.yangtools.util.ImmutableOffsetMap;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.AugmentationIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
@@ -47,11 +53,15 @@ import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.LeafListSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.ListSchemaNode;
 import org.opendaylight.yangtools.yang.model.util.EffectiveAugmentationSchema;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Base strategy for converting an instance identifier into a normalized node structure for container-like types.
  */
 abstract class InstanceIdToCompositeNodes<T extends PathArgument> extends InstanceIdToNodes<T> {
+    private static final Logger LOG = LoggerFactory.getLogger(InstanceIdToCompositeNodes.class);
+
     InstanceIdToCompositeNodes(final T identifier) {
         super(identifier);
     }
@@ -142,10 +152,20 @@ abstract class InstanceIdToCompositeNodes<T extends PathArgument> extends Instan
             super(NodeIdentifierWithPredicates.of(schema.getQName()), schema);
         }
 
+        @Override
+        boolean isMixin() {
+            return false;
+        }
+
         @Override
         DataContainerNodeBuilder<NodeIdentifierWithPredicates, MapEntryNode> createBuilder(
                 final PathArgument currentArg) {
             final NodeIdentifierWithPredicates arg = (NodeIdentifierWithPredicates) currentArg;
+            return createBuilder(arg.size() < 2 ? arg : reorderPredicates(schema().getKeyDefinition(), arg));
+        }
+
+        private static DataContainerNodeBuilder<NodeIdentifierWithPredicates, MapEntryNode> createBuilder(
+                final NodeIdentifierWithPredicates arg) {
             final DataContainerNodeBuilder<NodeIdentifierWithPredicates, MapEntryNode> builder = Builders
                     .mapEntryBuilder().withNodeIdentifier(arg);
             for (final Entry<QName, Object> keyValue : arg.entrySet()) {
@@ -156,9 +176,34 @@ abstract class InstanceIdToCompositeNodes<T extends PathArgument> extends Instan
             return builder;
         }
 
-        @Override
-        boolean isMixin() {
-            return false;
+        private static NodeIdentifierWithPredicates reorderPredicates(final List<QName> keys,
+                final NodeIdentifierWithPredicates arg) {
+            if (Iterables.elementsEqual(keys, arg.keySet())) {
+                // Iteration order matches key order, reuse the identifier
+                return arg;
+            }
+
+            // We care about iteration order here!
+            final LinkedHashMap<QName, Object> map = Maps.newLinkedHashMapWithExpectedSize(arg.size());
+            for (QName qname : keys) {
+                final Object value = arg.getValue(qname);
+                if (value != null) {
+                    map.put(qname, value);
+                }
+            }
+            if (map.size() < arg.size()) {
+                // Okay, this should not happen, but let's handle that anyway
+                LOG.debug("Extra predicates in {} while expecting {}", arg, keys);
+                for (Entry<QName, Object> entry : arg.entrySet()) {
+                    map.putIfAbsent(entry.getKey(), entry.getValue());
+                }
+            }
+
+            // This copy retains iteration order and since we have more than one argument, it should always be
+            // and ImmutableOffsetMap -- which is guaranteed to be taken as-is
+            final Map<QName, Object> copy = ImmutableOffsetMap.orderedCopyOf(map);
+            verify(copy instanceof ImmutableOffsetMap);
+            return NodeIdentifierWithPredicates.of(arg.getNodeType(), (ImmutableOffsetMap<QName, Object>) copy);
         }
     }
 

diff --git a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/InstanceIdToNodesTest.java b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/InstanceIdToNodesTest.java
index 788b2be3fe84a66770e143f40060bfac7f47bb58..76371a05ba78984d6cb3adc12296f40dce65f543 100644 (file)
--- a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/InstanceIdToNodesTest.java
+++ b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/InstanceIdToNodesTest.java
@@ -7,12 +7,19 @@
  */
 package org.opendaylight.yangtools.yang.data.impl.schema;
 
+import static org.hamcrest.Matchers.isA;
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import java.util.Collection;
+import java.util.Map;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.opendaylight.yangtools.util.ImmutableOffsetMap;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.AugmentationIdentifier;
@@ -32,6 +39,10 @@ public class InstanceIdToNodesTest {
     private static final String NS = "urn:opendaylight:params:xml:ns:yang:controller:md:sal:normalization:test";
     private static final String REVISION = "2014-03-13";
     private static final QName ID = QName.create(NS, REVISION, "id");
+    private static final QName FOO = QName.create(ID, "foo");
+    private static final QName BAR = QName.create(ID, "bar");
+    private static final NodeIdentifier TWO_KEY_LIST = NodeIdentifier.create(QName.create(ID, "two-key-list"));
+
     private static SchemaContext ctx;
 
     private final NodeIdentifier rootContainer = new NodeIdentifier(QName.create(NS, REVISION, "test"));
@@ -204,4 +215,21 @@ public class InstanceIdToNodesTest {
         assertEquals(ImmutableNodes.containerNode(SchemaContext.NAME),
             ImmutableNodes.fromInstanceId(ctx, YangInstanceIdentifier.empty()));
     }
+
+    @Test
+    public void testKeyOrdering() {
+        final Map<QName, Object> misordered = ImmutableOffsetMap.orderedCopyOf(ImmutableMap.of(BAR, "bar", FOO, "foo"));
+        final NodeIdentifierWithPredicates id = NodeIdentifierWithPredicates.of(TWO_KEY_LIST.getNodeType(), misordered);
+        assertArrayEquals(new Object[] { BAR, FOO }, id.keySet().toArray());
+
+        final NormalizedNode<?, ?> filter = ImmutableNodes.fromInstanceId(ctx,
+            YangInstanceIdentifier.create(TWO_KEY_LIST, id));
+        assertThat(filter, isA(MapNode.class));
+        final Collection<MapEntryNode> value = ((MapNode) filter).getValue();
+        assertEquals(1, value.size());
+        final MapEntryNode entry = value.iterator().next();
+
+        // The entry must have a the proper order
+        assertArrayEquals(new Object[] { FOO, BAR }, entry.getIdentifier().keySet().toArray());
+    }
 }

diff --git a/yang/yang-data-impl/src/test/resources/filter-test.yang b/yang/yang-data-impl/src/test/resources/filter-test.yang
index 6df5306850496da5970d01d7c2044c7b7cde9598..00c99405e059711e040e36aa08f05ae15b842e95 100644 (file)
--- a/yang/yang-data-impl/src/test/resources/filter-test.yang
+++ b/yang/yang-data-impl/src/test/resources/filter-test.yang
@@ -65,10 +65,20 @@ module normalization-test {
         anyxml any-xml-data;
     }
 
+    list two-key-list {
+        key "foo bar";
+        leaf bar {
+            type string;
+        }
+        leaf foo {
+            type string;
+        }
+    }
+
     augment /norm-test:test/norm-test:outer-container {
 
         leaf augmented-leaf {
            type string;
         }
     }
-}
\ No newline at end of file
+}