Fix immutable NormalizedNode equality

Equality has always been iffy in our immutable implementations, but we
are now making a decent attempt to implement it. There are three gaps in
that effort, where we only consider known subclasses for comparison of
value part of NormalizedNode.

Address this by providing fallback child comparison and add an explicit
test suite.

JIRA: YANGTOOLS-1417
Change-Id: I36e5bcfc6da06c789595b00b7ad86d973cb34703
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableLeafSetNodeBuilder.java b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableLeafSetNodeBuilder.java
index d034b7f942659476d60aeeb64e7e7221b7740911..057fcc0187eedd059ff9d7dc48aa510c05395d6d 100644 (file)
--- a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableLeafSetNodeBuilder.java
+++ b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableLeafSetNodeBuilder.java
@@ -161,7 +161,18 @@ public class ImmutableLeafSetNodeBuilder<T> implements ListNodeBuilder<T, System
 
         @Override
         protected boolean valueEquals(final SystemLeafSetNode<?> other) {
-            return children.equals(((ImmutableLeafSetNode<?>) other).children);
+            if (other instanceof ImmutableLeafSetNode) {
+                return children.equals(((ImmutableLeafSetNode<?>) other).children);
+            }
+            if (size() != other.size()) {
+                return false;
+            }
+            for (var child : children.values()) {
+                if (!child.equals(other.childByArg(child.getIdentifier()))) {
+                    return false;
+                }
+            }
+            return true;
         }
     }
 }

diff --git a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableUserLeafSetNodeBuilder.java b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableUserLeafSetNodeBuilder.java
index 403722c41965740290197f3f3c4c42f82e23697a..017d822d6ede401c73011cf4226155f246d92d6c 100644 (file)
--- a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableUserLeafSetNodeBuilder.java
+++ b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableUserLeafSetNodeBuilder.java
@@ -159,7 +159,11 @@ public class ImmutableUserLeafSetNodeBuilder<T> implements ListNodeBuilder<T, Us
 
         @Override
         protected boolean valueEquals(final UserLeafSetNode<?> other) {
-            return children.equals(((ImmutableUserLeafSetNode<?>) other).children);
+            if (other instanceof ImmutableUserLeafSetNode) {
+                return children.equals(((ImmutableUserLeafSetNode<?>) other).children);
+            }
+            // Note: performs a size() check first
+            return Iterables.elementsEqual(children.values(), other.body());
         }
 
         private Map<NodeWithValue, LeafSetEntryNode<T>> getChildren() {

diff --git a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java
index 3d3556e1dcdaf287a3ab041feff157a56177ce9e..8490d827d79b775f47dc66573edd280a25212ef4 100644 (file)
--- a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java
+++ b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java
@@ -59,7 +59,17 @@ public abstract class AbstractImmutableDataContainerNode<K extends PathArgument,
 
     @Override
     protected boolean valueEquals(final N other) {
-        return other instanceof AbstractImmutableDataContainerNode<?, ?> && children.equals(
-                ((AbstractImmutableDataContainerNode<?, ?>) other).children);
+        if (other instanceof AbstractImmutableDataContainerNode) {
+            return children.equals(((AbstractImmutableDataContainerNode<?, ?>) other).children);
+        }
+        if (size() != other.size()) {
+            return false;
+        }
+        for (var child : body()) {
+            if (!child.equals(other.childByArg(child.getIdentifier()))) {
+                return false;
+            }
+        }
+        return true;
     }
 }

diff --git a/data/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/YT1417Test.java b/data/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/YT1417Test.java
new file mode 100644 (file)
index 0000000..797942e
--- /dev/null
+++ b/data/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/YT1417Test.java
@@ -0,0 +1,167 @@
+/*
+ * Copyright (c) 2022 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.yangtools.yang.data.impl.schema.nodes;
+
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue;
+import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
+import org.opendaylight.yangtools.yang.data.api.schema.SystemLeafSetNode;
+import org.opendaylight.yangtools.yang.data.api.schema.UserLeafSetNode;
+import org.opendaylight.yangtools.yang.data.impl.schema.Builders;
+import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes;
+
+@RunWith(MockitoJUnitRunner.StrictStubs.class)
+public class YT1417Test {
+    public static final QName FOO = QName.create("foo", "foo");
+    public static final QName BAR = QName.create("foo", "bar");
+
+    @Test
+    public void testContainerNodeEquality() {
+        final var mock = mock(ContainerNode.class);
+        doReturn(new NodeIdentifier(FOO)).when(mock).getIdentifier();
+        doReturn(1).when(mock).size();
+        doReturn(ImmutableNodes.leafNode(BAR, "abc")).when(mock).childByArg(new NodeIdentifier(BAR));
+
+        assertEquals(Builders.containerBuilder()
+            .withNodeIdentifier(new NodeIdentifier(FOO))
+            .withChild(ImmutableNodes.leafNode(BAR, "abc"))
+            .build(), mock);
+
+        // Mismatched identifier
+        assertNotEquals(Builders.containerBuilder()
+            .withNodeIdentifier(new NodeIdentifier(BAR))
+            .withChild(ImmutableNodes.leafNode(BAR, "abc"))
+            .build(), mock);
+
+        // Mismatched child size
+        assertNotEquals(Builders.containerBuilder()
+            .withNodeIdentifier(new NodeIdentifier(FOO))
+            .withChild(ImmutableNodes.leafNode(FOO, "abc"))
+            .withChild(ImmutableNodes.leafNode(BAR, "abc"))
+            .build(), mock);
+
+        // Mismatched child
+        assertNotEquals(Builders.containerBuilder()
+            .withNodeIdentifier(new NodeIdentifier(FOO))
+            .withChild(ImmutableNodes.leafNode(FOO, "abc"))
+            .build(), mock);
+    }
+
+    @Test
+    public void testSystemLeafSetNodeEquality() {
+        final var mock = mock(SystemLeafSetNode.class);
+        doReturn(new NodeIdentifier(FOO)).when(mock).getIdentifier();
+        doReturn(1).when(mock).size();
+        doReturn(Builders.leafSetEntryBuilder()
+            .withNodeIdentifier(new NodeWithValue<>(FOO, "abc"))
+            .withValue("abc")
+            .build()).when(mock).childByArg(new NodeWithValue<>(FOO, "abc"));
+
+        assertEquals(Builders.leafSetBuilder()
+            .withNodeIdentifier(new NodeIdentifier(FOO))
+            .withChild(Builders.leafSetEntryBuilder()
+                .withNodeIdentifier(new NodeWithValue<>(FOO, "abc"))
+                .withValue("abc")
+                .build())
+            .build(), mock);
+
+        // Mismatched identifier
+        assertNotEquals(Builders.leafSetBuilder()
+            .withNodeIdentifier(new NodeIdentifier(BAR))
+            .withChild(Builders.leafSetEntryBuilder()
+                .withNodeIdentifier(new NodeWithValue<>(BAR, "abc"))
+                .withValue("abc")
+                .build())
+            .build(), mock);
+
+        // Mismatched child size
+        assertNotEquals(Builders.leafSetBuilder()
+            .withNodeIdentifier(new NodeIdentifier(FOO))
+            .withChild(Builders.leafSetEntryBuilder()
+                .withNodeIdentifier(new NodeWithValue<>(FOO, "abc"))
+                .withValue("abc")
+                .build())
+            .withChild(Builders.leafSetEntryBuilder()
+                .withNodeIdentifier(new NodeWithValue<>(FOO, "def"))
+                .withValue("def")
+                .build())
+            .build(), mock);
+
+        // Mismatched child
+        assertNotEquals(Builders.leafSetBuilder()
+            .withNodeIdentifier(new NodeIdentifier(FOO))
+            .withChild(Builders.leafSetEntryBuilder()
+                .withNodeIdentifier(new NodeWithValue<>(FOO, "def"))
+                .withValue("def")
+                .build())
+            .build(), mock);
+    }
+
+    @Test
+    public void testUserLeafSetNodeEquality() {
+        final var mock = mock(UserLeafSetNode.class);
+        doReturn(new NodeIdentifier(FOO)).when(mock).getIdentifier();
+        doReturn(List.of(
+            Builders.leafSetEntryBuilder()
+                .withNodeIdentifier(new NodeWithValue<>(FOO, "abc"))
+                .withValue("abc")
+                .build(),
+            Builders.leafSetEntryBuilder()
+                .withNodeIdentifier(new NodeWithValue<>(FOO, "def"))
+                .withValue("def")
+                .build())).when(mock).body();
+
+        assertEquals(Builders.orderedLeafSetBuilder()
+            .withNodeIdentifier(new NodeIdentifier(FOO))
+            .withChild(Builders.leafSetEntryBuilder()
+                .withNodeIdentifier(new NodeWithValue<>(FOO, "abc"))
+                .withValue("abc")
+                .build())
+            .withChild(Builders.leafSetEntryBuilder()
+                .withNodeIdentifier(new NodeWithValue<>(FOO, "def"))
+                .withValue("def")
+                .build())
+            .build(), mock);
+
+        // Mismatched identifier
+        assertNotEquals(Builders.orderedLeafSetBuilder()
+            .withNodeIdentifier(new NodeIdentifier(BAR))
+            .withChild(Builders.leafSetEntryBuilder()
+                .withNodeIdentifier(new NodeWithValue<>(BAR, "abc"))
+                .withValue("abc")
+                .build())
+            .withChild(Builders.leafSetEntryBuilder()
+                .withNodeIdentifier(new NodeWithValue<>(BAR, "def"))
+                .withValue("def")
+                .build())
+            .build(), mock);
+
+        // Mismatched child order
+        assertNotEquals(Builders.orderedLeafSetBuilder()
+            .withNodeIdentifier(new NodeIdentifier(FOO))
+            .withChild(Builders.leafSetEntryBuilder()
+                .withNodeIdentifier(new NodeWithValue<>(FOO, "def"))
+                .withValue("def")
+                .build())
+            .withChild(Builders.leafSetEntryBuilder()
+                .withNodeIdentifier(new NodeWithValue<>(FOO, "abc"))
+                .withValue("abc")
+                .build())
+            .build(), mock);
+    }
+}