[GitHub] merlimat closed pull request #1285: BatchMessageIdImpl can be compared to MessageIdImpl

[GitBox <gi...@apache.org>]

merlimat closed pull request #1285: BatchMessageIdImpl can be compared to MessageIdImpl
URL: https://github.com/apache/incubator-pulsar/pull/1285
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageIdImpl.java b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageIdImpl.java
index 9fe50a507..f7c5bddb5 100644
--- a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageIdImpl.java
+++ b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageIdImpl.java
@@ -24,6 +24,7 @@
 /**
  */
 public class BatchMessageIdImpl extends MessageIdImpl {
+    private final static int NO_BATCH = -1;
     private final int batchIndex;
 
     public BatchMessageIdImpl(long ledgerId, long entryId, int partitionIndex, int batchIndex) {
@@ -36,7 +37,7 @@ public BatchMessageIdImpl(MessageIdImpl other) {
         if (other instanceof BatchMessageIdImpl) {
             this.batchIndex = ((BatchMessageIdImpl) other).batchIndex;
         } else {
-            this.batchIndex = -1;
+            this.batchIndex = NO_BATCH;
         }
     }
 
@@ -46,18 +47,25 @@ public int getBatchIndex() {
 
     @Override
     public int compareTo(MessageId o) {
-        if (!(o instanceof BatchMessageIdImpl)) {
+        if (o instanceof BatchMessageIdImpl) {
+            BatchMessageIdImpl other = (BatchMessageIdImpl) o;
+            return ComparisonChain.start()
+                .compare(this.ledgerId, other.ledgerId)
+                .compare(this.entryId, other.entryId)
+                .compare(this.batchIndex, other.batchIndex)
+                .compare(this.getPartitionIndex(), other.getPartitionIndex())
+                .result();
+        } else if (o instanceof MessageIdImpl) {
+            int res = super.compareTo(o);
+            if (res == 0 && batchIndex > NO_BATCH) {
+                return 1;
+            } else {
+                return res;
+            }
+        } else {
             throw new IllegalArgumentException(
                     "expected BatchMessageIdImpl object. Got instance of " + o.getClass().getName());
         }
-
-        BatchMessageIdImpl other = (BatchMessageIdImpl) o;
-        return ComparisonChain.start()
-            .compare(this.ledgerId, other.ledgerId)
-            .compare(this.entryId, other.entryId)
-            .compare(this.batchIndex, other.batchIndex)
-            .compare(this.getPartitionIndex(), other.getPartitionIndex())
-            .result();
     }
 
     @Override
@@ -71,6 +79,8 @@ public boolean equals(Object obj) {
             BatchMessageIdImpl other = (BatchMessageIdImpl) obj;
             return ledgerId == other.ledgerId && entryId == other.entryId && partitionIndex == other.partitionIndex
                     && batchIndex == other.batchIndex;
+        } else if (obj instanceof MessageIdImpl) {
+            return batchIndex == NO_BATCH && obj.equals(this);
         }
         return false;
     }
@@ -81,7 +91,6 @@ public String toString() {
     }
 
     // Serialization
-
     @Override
     public byte[] toByteArray() {
         return toByteArray(batchIndex);
diff --git a/pulsar-client/src/test/java/org/apache/pulsar/client/impl/MessageIdCompareToTest.java b/pulsar-client/src/test/java/org/apache/pulsar/client/impl/MessageIdCompareToTest.java
index b66217dc4..f6b066889 100644
--- a/pulsar-client/src/test/java/org/apache/pulsar/client/impl/MessageIdCompareToTest.java
+++ b/pulsar-client/src/test/java/org/apache/pulsar/client/impl/MessageIdCompareToTest.java
@@ -106,18 +106,16 @@ public void testLessThan() {
 
     @Test
     public void testCompareDifferentType() {
-        // Expected throw IllegalArgumentException
         MessageIdImpl messageIdImpl = new MessageIdImpl(123L, 345L, 567);
-        BatchMessageIdImpl batchMessageId = new BatchMessageIdImpl(123L, 345L, 567, 789);
-
-        assertTrue( messageIdImpl.compareTo(batchMessageId) == 0, "Expected to be equal");
-
-        try {
-            batchMessageId.compareTo(messageIdImpl);
-            fail("Should throw IllegalArgumentException when compare different type");
-        } catch (IllegalArgumentException e) {
-            // expected
-        }
+        BatchMessageIdImpl batchMessageId1 = new BatchMessageIdImpl(123L, 345L, 566, 789);
+        BatchMessageIdImpl batchMessageId2 = new BatchMessageIdImpl(123L, 345L, 567, 789);
+        BatchMessageIdImpl batchMessageId3 = new BatchMessageIdImpl(messageIdImpl);
+        assertTrue(messageIdImpl.compareTo(batchMessageId1) > 0, "Expected to be greater than");
+        assertTrue(messageIdImpl.compareTo(batchMessageId2) == 0, "Expected to be equal");
+        assertTrue(messageIdImpl.compareTo(batchMessageId3) == 0, "Expected to be equal");
+        assertTrue(batchMessageId1.compareTo(messageIdImpl) < 0, "Expected to be less than");
+        assertTrue(batchMessageId2.compareTo(messageIdImpl) > 0, "Expected to be greater than");
+        assertTrue(batchMessageId3.compareTo(messageIdImpl) == 0, "Expected to be equal");
     }
 
 }


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services