Fix contract violation of GroupNode's node comparator.

2 files changed, 94 insertions, 1 deletions

diff --git a/src/net/java/sip/communicator/impl/gui/main/contactlist/GroupNode.java b/src/net/java/sip/communicator/impl/gui/main/contactlist/GroupNode.java
index 777f5b9..058d393 100644
--- a/src/net/java/sip/communicator/impl/gui/main/contactlist/GroupNode.java
+++ b/src/net/java/sip/communicator/impl/gui/main/contactlist/GroupNode.java
@@ -421,7 +421,7 @@ public class GroupNode
      * Note: this comparator imposes orderings that are inconsistent with
      * equals.
      */
-    private static class NodeComparator
+    static class NodeComparator
         implements Comparator<ContactListNode>
     {
         /**
@@ -439,6 +439,10 @@ public class GroupNode
             int index1 = node1.getSourceIndex();
             int index2 = node2.getSourceIndex();
 
+            // If both indexes are unknown, consider them equal. We need this
+            // case to ensure the property of symmetry in the node comparator.
+            if (index1 < 0 && index2 < 0)
+                return 0;
             // If the first index is unknown then we position it at the end.
             if (index1 < 0)
                 return 1;
diff --git a/test/net/java/sip/communicator/impl/gui/main/contactlist/GroupNodeTest.java b/test/net/java/sip/communicator/impl/gui/main/contactlist/GroupNodeTest.java
new file mode 100644
index 0000000..d03d101
--- /dev/null
+++ b/test/net/java/sip/communicator/impl/gui/main/contactlist/GroupNodeTest.java
@@ -0,0 +1,89 @@
+/*
+ * Jitsi, the OpenSource Java VoIP and Instant Messaging client.
+ *
+ * Distributable under LGPL license.
+ * See terms of license at gnu.org.
+ */
+package net.java.sip.communicator.impl.gui.main.contactlist;
+
+import junit.framework.*;
+import net.java.sip.communicator.impl.gui.main.contactlist.GroupNode.NodeComparator;
+import net.java.sip.communicator.service.gui.*;
+
+public class GroupNodeTest
+    extends TestCase
+{
+
+    public void testNodeComparatorUnknownsAtTheEnd()
+    {
+        ContactListNode unknown = new ContactListNode()
+        {
+            @Override
+            public int getSourceIndex()
+            {
+                return -1;
+            }
+        };
+        ContactListNode node = new ContactListNode()
+        {
+            @Override
+            public int getSourceIndex()
+            {
+                return 1;
+            }
+        };
+        NodeComparator comparator = new GroupNode.NodeComparator();
+        Assert.assertEquals(comparator.compare(unknown, node),
+            -1 * comparator.compare(node, unknown));
+        Assert.assertEquals(1, comparator.compare(unknown, node));
+        Assert.assertEquals(-1, comparator.compare(node, unknown));
+    }
+
+    public void testNodeComparatorNormalNodes()
+    {
+        ContactListNode node1 = new ContactListNode()
+        {
+            @Override
+            public int getSourceIndex()
+            {
+                return 4;
+            }
+        };
+        ContactListNode node2 = new ContactListNode()
+        {
+            @Override
+            public int getSourceIndex()
+            {
+                return 7;
+            }
+        };
+        NodeComparator comparator = new GroupNode.NodeComparator();
+        Assert.assertEquals(comparator.compare(node1, node2),
+            -1 * comparator.compare(node2, node1));
+        Assert.assertEquals(-1, comparator.compare(node1, node2));
+        Assert.assertEquals(1, comparator.compare(node2, node1));
+    }
+
+    public void testNodeComparatorSymmetryForUnknownNodes()
+    {
+        ContactListNode unknown1 = new ContactListNode()
+        {
+            @Override
+            public int getSourceIndex()
+            {
+                return -1;
+            }
+        };
+        ContactListNode unknown2 = new ContactListNode()
+        {
+            @Override
+            public int getSourceIndex()
+            {
+                return -1;
+            }
+        };
+        NodeComparator comparator = new GroupNode.NodeComparator();
+        Assert.assertEquals(comparator.compare(unknown1, unknown2),
+            -1 * comparator.compare(unknown2, unknown1));
+    }
+}