kafka-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject kafka git commit: KAFKA-6308; Connect Struct should use deepEquals/deepHashCode
Date Thu, 14 Dec 2017 23:30:00 GMT
Repository: kafka
Updated Branches:
  refs/heads/trunk 616321bcb -> 68712dcde


KAFKA-6308; Connect Struct should use deepEquals/deepHashCode

This changes the Struct's equals and hashCode method to use Arrays#deepEquals and Arrays#deepHashCode,
respectively. This resolves a problem where two structs with values of type byte[] would not
be considered equal even though the byte arrays' contents are equal. By using deepEquals,
the byte arrays' contents are compared instead of ther identity.

Since this changes the behavior of the equals method for byte array values, the behavior of
hashCode must change alongside it to ensure the methods still fulfill the general contract
of "equal objects must have equal hashCodes".

Test rationale:
All existing unit tests for equals were untouched and continue to work. A new test method
was added to verify the behavior of equals and hashCode for Struct instances that contain
a byte array value. I verify the reflixivity and transitivity of equals as well as the fact
that equal Structs have equal hashCodes
and not-equal structs do not have equal hashCodes.

Author: Tobias Gies <tobias.gies@trivago.com>
Author: Tobias Gies <tobias@tobiasgies.de>

Reviewers: Randall Hauch <rhauch@gmail.com>, Jason Gustafson <jason@confluent.io>

Closes #4293 from tobiasgies/feature/kafka-6308-deepequals


Project: http://git-wip-us.apache.org/repos/asf/kafka/repo
Commit: http://git-wip-us.apache.org/repos/asf/kafka/commit/68712dcd
Tree: http://git-wip-us.apache.org/repos/asf/kafka/tree/68712dcd
Diff: http://git-wip-us.apache.org/repos/asf/kafka/diff/68712dcd

Branch: refs/heads/trunk
Commit: 68712dcdec6e15f24f720f094ccc504ae7829b30
Parents: 616321b
Author: Tobias Gies <tobias.gies@trivago.com>
Authored: Thu Dec 14 15:26:20 2017 -0800
Committer: Jason Gustafson <jason@confluent.io>
Committed: Thu Dec 14 15:26:20 2017 -0800

----------------------------------------------------------------------
 .../org/apache/kafka/connect/data/Struct.java   |  4 +-
 .../apache/kafka/connect/data/StructTest.java   | 49 ++++++++++++++++++++
 2 files changed, 51 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kafka/blob/68712dcd/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java
----------------------------------------------------------------------
diff --git a/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java b/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java
index 6e7b5d2..1650fa2 100644
--- a/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java
+++ b/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java
@@ -238,12 +238,12 @@ public class Struct {
         if (o == null || getClass() != o.getClass()) return false;
         Struct struct = (Struct) o;
         return Objects.equals(schema, struct.schema) &&
-                Arrays.equals(values, struct.values);
+                Arrays.deepEquals(values, struct.values);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(schema, Arrays.hashCode(values));
+        return Objects.hash(schema, Arrays.deepHashCode(values));
     }
 
     private Field lookupField(String fieldName) {

http://git-wip-us.apache.org/repos/asf/kafka/blob/68712dcd/connect/api/src/test/java/org/apache/kafka/connect/data/StructTest.java
----------------------------------------------------------------------
diff --git a/connect/api/src/test/java/org/apache/kafka/connect/data/StructTest.java b/connect/api/src/test/java/org/apache/kafka/connect/data/StructTest.java
index 42345b1..45d2509 100644
--- a/connect/api/src/test/java/org/apache/kafka/connect/data/StructTest.java
+++ b/connect/api/src/test/java/org/apache/kafka/connect/data/StructTest.java
@@ -31,6 +31,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNull;
 
+
 public class StructTest {
 
     private static final Schema FLAT_STRUCT_SCHEMA = SchemaBuilder.struct()
@@ -236,6 +237,54 @@ public class StructTest {
         assertNotEquals(struct1, struct3);
     }
 
+    @Test
+    public void testEqualsAndHashCodeWithByteArrayValue() {
+        Struct struct1 = new Struct(FLAT_STRUCT_SCHEMA)
+                .put("int8", (byte) 12)
+                .put("int16", (short) 12)
+                .put("int32", 12)
+                .put("int64", (long) 12)
+                .put("float32", 12.f)
+                .put("float64", 12.)
+                .put("boolean", true)
+                .put("string", "foobar")
+                .put("bytes", "foobar".getBytes());
+
+        Struct struct2 = new Struct(FLAT_STRUCT_SCHEMA)
+                .put("int8", (byte) 12)
+                .put("int16", (short) 12)
+                .put("int32", 12)
+                .put("int64", (long) 12)
+                .put("float32", 12.f)
+                .put("float64", 12.)
+                .put("boolean", true)
+                .put("string", "foobar")
+                .put("bytes", "foobar".getBytes());
+
+        Struct struct3 = new Struct(FLAT_STRUCT_SCHEMA)
+                .put("int8", (byte) 12)
+                .put("int16", (short) 12)
+                .put("int32", 12)
+                .put("int64", (long) 12)
+                .put("float32", 12.f)
+                .put("float64", 12.)
+                .put("boolean", true)
+                .put("string", "foobar")
+                .put("bytes", "mismatching_string".getBytes());
+
+        // Verify contract for equals: method must be reflexive and transitive
+        assertEquals(struct1, struct2);
+        assertEquals(struct2, struct1);
+        assertNotEquals(struct1, struct3);
+        assertNotEquals(struct2, struct3);
+        // Testing hashCode against a hardcoded value here would be incorrect: hashCode values
need not be equal for any
+        // two distinct executions. However, based on the general contract for hashCode,
if two objects are equal, their
+        // hashCodes must be equal. If they are not equal, their hashCodes should not be
equal for performance reasons.
+        assertEquals(struct1.hashCode(), struct2.hashCode());
+        assertNotEquals(struct1.hashCode(), struct3.hashCode());
+        assertNotEquals(struct2.hashCode(), struct3.hashCode());
+    }
+
     @Rule
     public ExpectedException thrown = ExpectedException.none();
 


Mime
View raw message