MINOR: Fix PartitionRegistration.hashCode (apache#12774)

`PartitionRegistration.hashCode` passes raw arrays to `Objects.hash` in the `hashCode` implementation. This doesn't work since the `equals` implementation uses `Arrays.equals`. We should use `Arrays.hashCode` instead. Reviewers: David Arthur <mumrah@gmail.com>
rutvijmehta-harness · Feb 9, 2024 · 072cd8c · 072cd8c
1 parent 1224593
commit 072cd8c
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 2 deletions.
diff --git a/metadata/src/main/java/org/apache/kafka/metadata/PartitionRegistration.java b/metadata/src/main/java/org/apache/kafka/metadata/PartitionRegistration.java
@@ -210,8 +210,8 @@ public boolean isReassigning() {
 
     @Override
     public int hashCode() {
-        return Objects.hash(replicas, isr, removingReplicas, addingReplicas, leader, leaderRecoveryState,
-            leaderEpoch, partitionEpoch);
+        return Objects.hash(Arrays.hashCode(replicas), Arrays.hashCode(isr), Arrays.hashCode(removingReplicas),
+            Arrays.hashCode(addingReplicas), leader, leaderRecoveryState, leaderEpoch, partitionEpoch);
     }
 
     @Override

diff --git a/metadata/src/test/java/org/apache/kafka/metadata/PartitionRegistrationTest.java b/metadata/src/test/java/org/apache/kafka/metadata/PartitionRegistrationTest.java
@@ -17,6 +17,11 @@
 
 package org.apache.kafka.metadata;
 
+import net.jqwik.api.Arbitraries;
+import net.jqwik.api.Arbitrary;
+import net.jqwik.api.ForAll;
+import net.jqwik.api.Property;
+import net.jqwik.api.Provide;
 import org.apache.kafka.common.TopicPartition;
 import org.apache.kafka.common.Uuid;
 import org.apache.kafka.common.message.LeaderAndIsrRequestData.LeaderAndIsrPartitionState;
@@ -31,6 +36,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 
@@ -125,4 +131,43 @@ public void testMergePartitionChangeRecordWithReassignmentData() {
             new int[] {1, 2, 4}, Replicas.NONE, Replicas.NONE, 1, LeaderRecoveryState.RECOVERED, 100, 202), partition2);
         assertFalse(partition2.isReassigning());
     }
+
+    @Property
+    public void testConsistentEqualsAndHashCode(
+        @ForAll("uniqueSamples") PartitionRegistration a,
+        @ForAll("uniqueSamples") PartitionRegistration b
+    ) {
+        if (a.equals(b)) {
+            assertEquals(a.hashCode(), b.hashCode());
+        }
+
+        if (a.hashCode() != b.hashCode()) {
+            assertNotEquals(a, b);
+        }
+    }
+
+    @Provide
+    Arbitrary<PartitionRegistration> uniqueSamples() {
+        return Arbitraries.of(
+            new PartitionRegistration(new int[] {1, 2, 3}, new int[] {1, 2, 3}, Replicas.NONE, Replicas.NONE,
+                1, LeaderRecoveryState.RECOVERED, 100, 200),
+            new PartitionRegistration(new int[] {1, 2, 3}, new int[] {1, 2, 3}, Replicas.NONE, Replicas.NONE,
+                1, LeaderRecoveryState.RECOVERED, 101, 200),
+            new PartitionRegistration(new int[] {1, 2, 3}, new int[] {1, 2, 3}, Replicas.NONE, Replicas.NONE,
+                1, LeaderRecoveryState.RECOVERED, 100, 201),
+            new PartitionRegistration(new int[] {1, 2, 3}, new int[] {1, 2, 3}, Replicas.NONE, Replicas.NONE,
+                2, LeaderRecoveryState.RECOVERED, 100, 200),
+            new PartitionRegistration(new int[] {1, 2, 3}, new int[] {1}, Replicas.NONE, Replicas.NONE,
+                1, LeaderRecoveryState.RECOVERING, 100, 200),
+            new PartitionRegistration(new int[] {1, 2, 3, 4, 5, 6}, new int[] {1, 2, 3}, new int[] {4, 5, 6}, new int[] {1, 2, 3},
+                1, LeaderRecoveryState.RECOVERED, 100, 200),
+            new PartitionRegistration(new int[] {1, 2, 3, 4, 5, 6}, new int[] {1, 2, 3}, new int[] {1, 2, 3}, new int[] {4, 5, 6},
+                1, LeaderRecoveryState.RECOVERED, 100, 200),
+            new PartitionRegistration(new int[] {1, 2, 3, 4, 5, 6}, new int[] {1, 2, 3}, new int[] {1, 2, 3}, Replicas.NONE,
+                1, LeaderRecoveryState.RECOVERED, 100, 200),
+            new PartitionRegistration(new int[] {1, 2, 3, 4, 5, 6}, new int[] {1, 2, 3}, Replicas.NONE, new int[] {4, 5, 6},
+                1, LeaderRecoveryState.RECOVERED, 100, 200)
+        );
+    }
+
 }