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.g...@trivago.com> Author: Tobias Gies <tob...@tobiasgies.de> Reviewers: Randall Hauch <rha...@gmail.com>, Jason Gustafson <ja...@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.g...@trivago.com> Authored: Thu Dec 14 15:26:20 2017 -0800 Committer: Jason Gustafson <ja...@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();
