MessageId#compare is not symmetric (MessageIdImpl, BatchMessageIdImpl)

[racorn]

Describe the bug
Given to MessageIds, of which one happens to be a MessageIdImpl and the other is a BatchMessageIdImpl, the compare method is not symmetric.

To Reproduce

import static org.assertj.core.api.Assertions.assertThat;
@Test
public void shouldBeSymmetric() {

    MessageIdImpl simpleMessageId = new MessageIdImpl(1, 2, 3);
    BatchMessageIdImpl batchMessageId = new BatchMessageIdImpl(1, 2, 3, 0);
    assertThat(simpleMessageId.compareTo(batchMessageId)).isEqualTo(batchMessageId.compareTo(simpleMessageId));

    }

Expected behavior
MessageId#compare should be symmetric

Additional context
Pulsar 2.5.0

[jiazhai]

Thanks @racorn for the reporting. Would you like to provide a fix for this issue?

[racorn]

Yes, I could, but I am not sure how MessageIdImpl and BatchMessageIdImpl should compare?

It seems to me that the asymmetry was introduced by purpose with #1285, the code reads as

} else if (o instanceof MessageIdImpl) {
    int res = super.compareTo(o);
    if (res == 0 && batchIndex > NO_BATCH) {
        return 1;
    } else {
        return res;
    }
}

But as it is now, it breaks the Comparable contract, and that makes it confusing for user of the client API.

[zhanghaou]

I'm interesting in fix this issue.

[codelipenghui]

@zhanghaou Thanks, look forwards to your PR.