This is an automated email from the ASF dual-hosted git repository.
tison pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/master by this push:
new 0a2d9273cc4 [improve][client] Implement equals by compareTo to avoid
logic unaligned (#18843)
0a2d9273cc4 is described below
commit 0a2d9273cc44eade712f93a611ad8e84c81a2f7b
Author: tison <[email protected]>
AuthorDate: Tue Dec 13 15:56:27 2022 +0800
[improve][client] Implement equals by compareTo to avoid logic unaligned
(#18843)
Signed-off-by: tison <[email protected]>
---
.../pulsar/client/impl/BatchMessageIdImpl.java | 25 +++-----------
.../apache/pulsar/client/impl/MessageIdImpl.java | 38 ++++------------------
.../pulsar/client/impl/MultiMessageIdImpl.java | 18 +++++-----
.../apache/pulsar/client/api/MessageIdTest.java | 1 -
.../pulsar/client/impl/BatchMessageIdImplTest.java | 29 +++++++++++++++--
5 files changed, 44 insertions(+), 67 deletions(-)
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 fab0ceeca33..7e3a143dff8 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
@@ -18,6 +18,7 @@
*/
package org.apache.pulsar.client.impl;
+import javax.annotation.Nonnull;
import org.apache.pulsar.client.api.MessageId;
/**
@@ -68,7 +69,7 @@ public class BatchMessageIdImpl extends MessageIdImpl {
}
@Override
- public int compareTo(MessageId o) {
+ public int compareTo(@Nonnull MessageId o) {
if (o instanceof MessageIdImpl) {
MessageIdImpl other = (MessageIdImpl) o;
int batchIndex = (o instanceof BatchMessageIdImpl) ?
((BatchMessageIdImpl) o).batchIndex : NO_BATCH;
@@ -90,30 +91,12 @@ public class BatchMessageIdImpl extends MessageIdImpl {
@Override
public boolean equals(Object o) {
- if (o instanceof MessageIdImpl) {
- MessageIdImpl other = (MessageIdImpl) o;
- int batchIndex = (o instanceof BatchMessageIdImpl) ?
((BatchMessageIdImpl) o).batchIndex : NO_BATCH;
- return messageIdEquals(
- this.ledgerId, this.entryId, this.partitionIndex,
this.batchIndex,
- other.ledgerId, other.entryId, other.partitionIndex, batchIndex
- );
- } else if (o instanceof TopicMessageIdImpl) {
- return equals(((TopicMessageIdImpl) o).getInnerMessageId());
- }
- return false;
+ return super.equals(o);
}
@Override
public String toString() {
- return new StringBuilder()
- .append(ledgerId)
- .append(':')
- .append(entryId)
- .append(':')
- .append(partitionIndex)
- .append(':')
- .append(batchIndex)
- .toString();
+ return ledgerId + ":" + entryId + ":" + partitionIndex + ":" +
batchIndex;
}
// Serialization
diff --git
a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MessageIdImpl.java
b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MessageIdImpl.java
index 26f236cdfb1..02298e0f9d6 100644
---
a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MessageIdImpl.java
+++
b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MessageIdImpl.java
@@ -25,6 +25,7 @@ import io.netty.buffer.Unpooled;
import io.netty.util.concurrent.FastThreadLocal;
import java.io.IOException;
import java.util.Objects;
+import javax.annotation.Nonnull;
import org.apache.pulsar.client.api.MessageId;
import org.apache.pulsar.common.api.proto.MessageIdData;
import org.apache.pulsar.common.naming.TopicName;
@@ -65,28 +66,14 @@ public class MessageIdImpl implements MessageId {
@Override
public boolean equals(Object o) {
- if (o instanceof MessageIdImpl) {
- MessageIdImpl other = (MessageIdImpl) o;
- int batchIndex = (o instanceof BatchMessageIdImpl) ?
((BatchMessageIdImpl) o).getBatchIndex() : NO_BATCH;
- return messageIdEquals(
- this.ledgerId, this.entryId, this.partitionIndex, NO_BATCH,
- other.ledgerId, other.entryId, other.partitionIndex, batchIndex
- );
- } else if (o instanceof TopicMessageIdImpl) {
- return equals(((TopicMessageIdImpl) o).getInnerMessageId());
- }
- return false;
+ return (o instanceof MessageId)
+ && !(o instanceof MultiMessageIdImpl)
+ && (compareTo((MessageId) o) == 0);
}
@Override
public String toString() {
- return new StringBuilder()
- .append(ledgerId)
- .append(':')
- .append(entryId)
- .append(':')
- .append(partitionIndex)
- .toString();
+ return ledgerId + ":" + entryId + ":" + partitionIndex;
}
// / Serialization
@@ -215,10 +202,7 @@ public class MessageIdImpl implements MessageId {
}
@Override
- public int compareTo(MessageId o) {
- if (o == null) {
- throw new UnsupportedOperationException("MessageId is null");
- }
+ public int compareTo(@Nonnull MessageId o) {
if (o instanceof MessageIdImpl) {
MessageIdImpl other = (MessageIdImpl) o;
int batchIndex = (o instanceof BatchMessageIdImpl) ?
((BatchMessageIdImpl) o).getBatchIndex() : NO_BATCH;
@@ -237,16 +221,6 @@ public class MessageIdImpl implements MessageId {
return (int) (31 * (ledgerId + 31 * entryId) + (31 * (long)
partitionIndex) + batchIndex);
}
- static boolean messageIdEquals(
- long ledgerId1, long entryId1, int partitionIndex1, int batchIndex1,
- long ledgerId2, long entryId2, int partitionIndex2, int batchIndex2
- ) {
- return ledgerId1 == ledgerId2
- && entryId1 == entryId2
- && partitionIndex1 == partitionIndex2
- && batchIndex1 == batchIndex2;
- }
-
static int messageIdCompare(
long ledgerId1, long entryId1, int partitionIndex1, int batchIndex1,
long ledgerId2, long entryId2, int partitionIndex2, int batchIndex2
diff --git
a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiMessageIdImpl.java
b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiMessageIdImpl.java
index b6accaf4cf4..6e60239ffe5 100644
---
a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiMessageIdImpl.java
+++
b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiMessageIdImpl.java
@@ -94,17 +94,15 @@ public class MultiMessageIdImpl implements MessageId {
@Override
public boolean equals(Object obj) {
- if (!(obj instanceof MultiMessageIdImpl)) {
- throw new IllegalArgumentException(
- "expected MultiMessageIdImpl object. Got instance of " +
obj.getClass().getName());
- }
+ if (obj instanceof MultiMessageIdImpl) {
+ MultiMessageIdImpl other = (MultiMessageIdImpl) obj;
- MultiMessageIdImpl other = (MultiMessageIdImpl) obj;
-
- try {
- return compareTo(other) == 0;
- } catch (IllegalArgumentException e) {
- return false;
+ try {
+ return compareTo(other) == 0;
+ } catch (IllegalArgumentException e) {
+ return false;
+ }
}
+ return false;
}
}
diff --git
a/pulsar-client/src/test/java/org/apache/pulsar/client/api/MessageIdTest.java
b/pulsar-client/src/test/java/org/apache/pulsar/client/api/MessageIdTest.java
index e5377ad9848..257342ef320 100644
---
a/pulsar-client/src/test/java/org/apache/pulsar/client/api/MessageIdTest.java
+++
b/pulsar-client/src/test/java/org/apache/pulsar/client/api/MessageIdTest.java
@@ -25,7 +25,6 @@ import org.apache.pulsar.client.impl.MessageIdImpl;
import org.testng.annotations.Test;
public class MessageIdTest {
-
@Test
public void messageIdTest() {
MessageId mId = new MessageIdImpl(1, 2, 3);
diff --git
a/pulsar-client/src/test/java/org/apache/pulsar/client/impl/BatchMessageIdImplTest.java
b/pulsar-client/src/test/java/org/apache/pulsar/client/impl/BatchMessageIdImplTest.java
index d10b3da81ff..6bf9cd94348 100644
---
a/pulsar-client/src/test/java/org/apache/pulsar/client/impl/BatchMessageIdImplTest.java
+++
b/pulsar-client/src/test/java/org/apache/pulsar/client/impl/BatchMessageIdImplTest.java
@@ -22,14 +22,13 @@ import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotEquals;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;
-
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectWriter;
+import java.io.IOException;
+import java.util.Collections;
import org.apache.pulsar.common.util.ObjectMapperFactory;
import org.testng.annotations.Test;
-import java.io.IOException;
-
public class BatchMessageIdImplTest {
@Test
@@ -74,6 +73,30 @@ public class BatchMessageIdImplTest {
assertEquals(msgId, batchMsgId4);
}
+ @Test
+ public void notEqualsMultiTest() {
+ BatchMessageIdImpl batchMsgId = new BatchMessageIdImpl(0, 0, 0, 0);
+ MessageIdImpl msgId = new MessageIdImpl(0, 0, 0);
+ MultiMessageIdImpl multiMsgId = new
MultiMessageIdImpl(Collections.singletonMap("topic", msgId));
+
+ assertNotEquals(msgId, multiMsgId);
+ assertNotEquals(multiMsgId, msgId);
+ assertNotEquals(batchMsgId, multiMsgId);
+ assertNotEquals(multiMsgId, batchMsgId);
+ }
+
+ @Test
+ public void compareToUnbatchedTest() {
+ MessageIdImpl msgId = new MessageIdImpl(1, 2, 3);
+ BatchMessageIdImpl batchMsgId = new BatchMessageIdImpl(1, 2, 3, 0);
+ assertEquals(msgId.compareTo(batchMsgId), -1);
+ assertEquals(batchMsgId.compareTo(msgId), 1);
+
+ batchMsgId = new BatchMessageIdImpl(1, 2, 3, -1);
+ assertEquals(msgId.compareTo(batchMsgId), 0);
+ assertEquals(batchMsgId.compareTo(msgId), 0);
+ }
+
@Test
public void equalsUnbatchedTest() {
BatchMessageIdImpl batchMsgId1 = new BatchMessageIdImpl(0, 0, 0, -1);
