From ecbeb0841e8627fa74a34346e0c380061588366a Mon Sep 17 00:00:00 2001
From: Jason Brown <jasedbrown@gmail.com>
Date: Fri, 26 Feb 2016 11:53:03 -0800
Subject: [PATCH] Range.compareTo() violates the contract of Comparable

patch by jasobrown and blambov for CASSANDRA-11216
---
 CHANGES.txt                                   |  1 +
 src/java/org/apache/cassandra/dht/Range.java  | 20 ++++++++++---------
 .../org/apache/cassandra/dht/RangeTest.java   | 15 ++++++++++++++
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 91621653ef..aa3adf5656 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 2.2.6
+ * Range.compareTo() violates the contract of Comparable (CASSANDRA-11216)
  * Avoid NPE when serializing ErrorMessage with null message (CASSANDRA-11167)
  * Replacing an aggregate with a new version doesn't reset INITCOND (CASSANDRA-10840)
  * (cqlsh) cqlsh cannot be called through symlink (CASSANDRA-11037)
diff --git a/src/java/org/apache/cassandra/dht/Range.java b/src/java/org/apache/cassandra/dht/Range.java
index f2c5996978..34e91eae60 100644
--- a/src/java/org/apache/cassandra/dht/Range.java
+++ b/src/java/org/apache/cassandra/dht/Range.java
@@ -32,6 +32,8 @@ import org.apache.cassandra.utils.Pair;
  * A Range is responsible for the tokens between (left, right].
  *
  * Used by the partitioner and by map/reduce by-token range scans.
+ *
+ * Note: this class has a natural ordering that is inconsistent with equals
  */
 public class Range<T extends RingPosition<T>> extends AbstractBounds<T> implements Comparable<Range<T>>, Serializable
 {
@@ -254,18 +256,18 @@ public class Range<T extends RingPosition<T>> extends AbstractBounds<T> implemen
        return left.compareTo(right) >= 0;
     }
 
+    /**
+     * Note: this class has a natural ordering that is inconsistent with equals
+     */
     public int compareTo(Range<T> rhs)
     {
-        /*
-         * If the range represented by the "this" pointer
-         * is a wrap around then it is the smaller one.
-         */
-        if ( isWrapAround(left, right) )
-            return -1;
-
-        if ( isWrapAround(rhs.left, rhs.right) )
-            return 1;
+        boolean lhsWrap = isWrapAround(left, right);
+        boolean rhsWrap = isWrapAround(rhs.left, rhs.right);
 
+        // if one of the two wraps, that's the smaller one.
+        if (lhsWrap != rhsWrap)
+            return Boolean.compare(!lhsWrap, !rhsWrap);
+        // otherwise compare by right.
         return right.compareTo(rhs.right);
     }
 
diff --git a/test/unit/org/apache/cassandra/dht/RangeTest.java b/test/unit/org/apache/cassandra/dht/RangeTest.java
index 4255487493..9fb49cf2db 100644
--- a/test/unit/org/apache/cassandra/dht/RangeTest.java
+++ b/test/unit/org/apache/cassandra/dht/RangeTest.java
@@ -661,4 +661,19 @@ public class RangeTest
     {
         return new Murmur3Partitioner.LongToken(t);
     }
+
+    @Test
+    public void testCompareTo_SameObject_WrapAround()
+    {
+        Range<Token> range = r(10, -10);
+        assertEquals(0, range.compareTo(range));
+    }
+
+    @Test
+    public void testCompareTo_BothWrapAround()
+    {
+        Range<Token> r0 = r(10, -10);
+        Range<Token> r1 = r(20, -5);
+        assertNotSame(r0.compareTo(r1), r1.compareTo(r0));
+    }
 }