From 160f97d7af4a34c724125da793d83408fbbcedaa Mon Sep 17 00:00:00 2001 From: Ismael Juma Date: Fri, 15 Oct 2021 09:25:47 -0700 Subject: [PATCH 1/2] MINOR: Remove topic null check from `TopicIdPartition` and adjust constructor order `TopicPartition` allows a null `topic` and there are cases where we have a topic id, but no topic name. Even for `TopicIdPartition`, the non null topic name check was only enforced in one constructor. Also adjust the constructor order to move the nullable parameter to the end, update tests and javadoc. --- .../apache/kafka/common/TopicIdPartition.java | 21 +++++++++++---- .../kafka/common/TopicIdPartitionTest.java | 26 ++++++++++++++++--- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/common/TopicIdPartition.java b/clients/src/main/java/org/apache/kafka/common/TopicIdPartition.java index 027648f3edfa8..3f3c8240a1d92 100644 --- a/clients/src/main/java/org/apache/kafka/common/TopicIdPartition.java +++ b/clients/src/main/java/org/apache/kafka/common/TopicIdPartition.java @@ -27,16 +27,27 @@ public class TopicIdPartition { private final Uuid topicId; private final TopicPartition topicPartition; + /** + * Create an instance with the provided parameters. + * + * @param topicId the topic id + * @param topicPartition the topic partition + */ public TopicIdPartition(Uuid topicId, TopicPartition topicPartition) { this.topicId = Objects.requireNonNull(topicId, "topicId can not be null"); this.topicPartition = Objects.requireNonNull(topicPartition, "topicPartition can not be null"); } - public TopicIdPartition(String topic, Uuid topicId, int partition) { + /** + * Create an instance with the provided parameters. + * + * @param topicId the topic id + * @param partition the partition id + * @param topic the topic name or null + */ + public TopicIdPartition(Uuid topicId, int partition, String topic) { this.topicId = Objects.requireNonNull(topicId, "topicId can not be null"); - this.topicPartition = new TopicPartition( - Objects.requireNonNull(topic, "topic can not be null"), - partition); + this.topicPartition = new TopicPartition(topic, partition); } /** @@ -47,7 +58,7 @@ public Uuid topicId() { } /** - * @return the topic name. + * @return the topic name or null. */ public String topic() { return topicPartition.topic(); diff --git a/clients/src/test/java/org/apache/kafka/common/TopicIdPartitionTest.java b/clients/src/test/java/org/apache/kafka/common/TopicIdPartitionTest.java index 08de64f8cb127..d39481c5b37d1 100644 --- a/clients/src/test/java/org/apache/kafka/common/TopicIdPartitionTest.java +++ b/clients/src/test/java/org/apache/kafka/common/TopicIdPartitionTest.java @@ -30,21 +30,31 @@ class TopicIdPartitionTest { private final int partition1 = 1; private final TopicPartition topicPartition0 = new TopicPartition(topicName0, partition1); private final TopicIdPartition topicIdPartition0 = new TopicIdPartition(topicId0, topicPartition0); - private final TopicIdPartition topicIdPartition1 = new TopicIdPartition(topicName0, topicId0, - partition1); + private final TopicIdPartition topicIdPartition1 = new TopicIdPartition(topicId0, + partition1, topicName0); + + private final TopicIdPartition topicIdPartitionWithNullTopic0 = new TopicIdPartition(topicId0, + partition1, null); + private final TopicIdPartition topicIdPartitionWithNullTopic1 = new TopicIdPartition(topicId0, + new TopicPartition(null, partition1)); private final Uuid topicId1 = new Uuid(7759286116672424028L, -5081215629859775948L); private final String topicName1 = "another_topic_name"; - private final TopicIdPartition topicIdPartition2 = new TopicIdPartition(topicName1, topicId1, - partition1); + private final TopicIdPartition topicIdPartition2 = new TopicIdPartition(topicId1, + partition1, topicName1); + private final TopicIdPartition topicIdPartitionWithNullTopic2 = new TopicIdPartition(topicId1, + new TopicPartition(null, partition1)); @Test public void testEquals() { assertEquals(topicIdPartition0, topicIdPartition1); assertEquals(topicIdPartition1, topicIdPartition0); + assertEquals(topicIdPartitionWithNullTopic0, topicIdPartitionWithNullTopic1); assertNotEquals(topicIdPartition0, topicIdPartition2); assertNotEquals(topicIdPartition2, topicIdPartition0); + assertNotEquals(topicIdPartition0, topicIdPartitionWithNullTopic0); + assertNotEquals(topicIdPartitionWithNullTopic0, topicIdPartitionWithNullTopic2); } @Test @@ -52,12 +62,20 @@ public void testHashCode() { assertEquals(Objects.hash(topicIdPartition0.topicId(), topicIdPartition0.topicPartition()), topicIdPartition0.hashCode()); assertEquals(topicIdPartition0.hashCode(), topicIdPartition1.hashCode()); + + assertEquals(Objects.hash(topicIdPartitionWithNullTopic0.topicId(), + new TopicPartition(null, partition1)), topicIdPartitionWithNullTopic0.hashCode()); + assertEquals(topicIdPartitionWithNullTopic0.hashCode(), topicIdPartitionWithNullTopic1.hashCode()); + assertNotEquals(topicIdPartition0.hashCode(), topicIdPartition2.hashCode()); + assertNotEquals(topicIdPartition0.hashCode(), topicIdPartitionWithNullTopic0.hashCode()); + assertNotEquals(topicIdPartitionWithNullTopic0.hashCode(), topicIdPartitionWithNullTopic2.hashCode()); } @Test public void testToString() { assertEquals("vDiRhkpVQgmtSLnsAZx7lA:a_topic_name-1", topicIdPartition0.toString()); + assertEquals("vDiRhkpVQgmtSLnsAZx7lA:null-1", topicIdPartitionWithNullTopic0.toString()); } } From 7ba23533533ea377516b99b89615d54a47def0ee Mon Sep 17 00:00:00 2001 From: Ismael Juma Date: Tue, 9 Nov 2021 08:48:54 -0800 Subject: [PATCH 2/2] Minor comment adjustment --- .../src/main/java/org/apache/kafka/common/TopicIdPartition.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/src/main/java/org/apache/kafka/common/TopicIdPartition.java b/clients/src/main/java/org/apache/kafka/common/TopicIdPartition.java index 3f3c8240a1d92..09d861e69efbe 100644 --- a/clients/src/main/java/org/apache/kafka/common/TopicIdPartition.java +++ b/clients/src/main/java/org/apache/kafka/common/TopicIdPartition.java @@ -58,7 +58,7 @@ public Uuid topicId() { } /** - * @return the topic name or null. + * @return the topic name or null if it is unknown. */ public String topic() { return topicPartition.topic();