From 4a1a67527dd60c06e64f5b373621af66812f3ed8 Mon Sep 17 00:00:00 2001 From: Grant Henke Date: Wed, 27 Apr 2016 18:49:04 -0500 Subject: [PATCH 1/3] KAFKA-3631: Fix Struct.toString for nullable arrayOf --- .../apache/kafka/common/protocol/types/Struct.java | 14 ++++++++------ .../protocol/types/ProtocolSerializationTest.java | 11 +++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java b/clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java index 7eee09f0de425..9b796107af19a 100644 --- a/clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java +++ b/clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java @@ -292,13 +292,15 @@ public String toString() { b.append('='); if (f.type() instanceof ArrayOf) { Object[] arrayValue = (Object[]) this.values[i]; - b.append('['); - for (int j = 0; j < arrayValue.length; j++) { - b.append(arrayValue[j]); - if (j < arrayValue.length - 1) - b.append(','); + if (arrayValue != null) { + b.append('['); + for (int j = 0; j < arrayValue.length; j++) { + b.append(arrayValue[j]); + if (j < arrayValue.length - 1) + b.append(','); + } + b.append(']'); } - b.append(']'); } else b.append(this.values[i]); if (i < this.values.length - 1) diff --git a/clients/src/test/java/org/apache/kafka/common/protocol/types/ProtocolSerializationTest.java b/clients/src/test/java/org/apache/kafka/common/protocol/types/ProtocolSerializationTest.java index e91b2fb591d4e..c077777aafd3a 100644 --- a/clients/src/test/java/org/apache/kafka/common/protocol/types/ProtocolSerializationTest.java +++ b/clients/src/test/java/org/apache/kafka/common/protocol/types/ProtocolSerializationTest.java @@ -231,6 +231,17 @@ public void testReadNegativeBytesSize() { } } + @Test + public void testToString() { + try { + String stuctStr = this.struct.toString(); + assertFalse("Stuct string should not be null.", stuctStr == null); + assertFalse("Stuct string should not be empty.", stuctStr.isEmpty()); + } catch (Throwable e) { + fail("Struct.toString throws an exception"); + } + } + private Object roundtrip(Type type, Object obj) { ByteBuffer buffer = ByteBuffer.allocate(type.sizeOf(obj)); type.write(buffer, obj); From 07be5840564f9b4cb18f326c0f5c1c4d96817c2e Mon Sep 17 00:00:00 2001 From: Grant Henke Date: Wed, 27 Apr 2016 18:55:01 -0500 Subject: [PATCH 2/3] Remove exception check --- .../protocol/types/ProtocolSerializationTest.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/clients/src/test/java/org/apache/kafka/common/protocol/types/ProtocolSerializationTest.java b/clients/src/test/java/org/apache/kafka/common/protocol/types/ProtocolSerializationTest.java index c077777aafd3a..503facf2101fe 100644 --- a/clients/src/test/java/org/apache/kafka/common/protocol/types/ProtocolSerializationTest.java +++ b/clients/src/test/java/org/apache/kafka/common/protocol/types/ProtocolSerializationTest.java @@ -233,13 +233,9 @@ public void testReadNegativeBytesSize() { @Test public void testToString() { - try { - String stuctStr = this.struct.toString(); - assertFalse("Stuct string should not be null.", stuctStr == null); - assertFalse("Stuct string should not be empty.", stuctStr.isEmpty()); - } catch (Throwable e) { - fail("Struct.toString throws an exception"); - } + String stuctStr = this.struct.toString(); + assertFalse("Stuct string should not be null.", stuctStr == null); + assertFalse("Stuct string should not be empty.", stuctStr.isEmpty()); } private Object roundtrip(Type type, Object obj) { From cfad93d9aa6c5a092932e10187f4cc2b68f16fae Mon Sep 17 00:00:00 2001 From: Grant Henke Date: Wed, 27 Apr 2016 19:06:06 -0500 Subject: [PATCH 3/3] Address feedback --- .../kafka/common/protocol/types/Struct.java | 16 +++++++--------- .../types/ProtocolSerializationTest.java | 7 ++++--- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java b/clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java index 9b796107af19a..212d701aaaa84 100644 --- a/clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java +++ b/clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java @@ -290,17 +290,15 @@ public String toString() { Field f = this.schema.get(i); b.append(f.name); b.append('='); - if (f.type() instanceof ArrayOf) { + if (f.type() instanceof ArrayOf && this.values[i] != null) { Object[] arrayValue = (Object[]) this.values[i]; - if (arrayValue != null) { - b.append('['); - for (int j = 0; j < arrayValue.length; j++) { - b.append(arrayValue[j]); - if (j < arrayValue.length - 1) - b.append(','); - } - b.append(']'); + b.append('['); + for (int j = 0; j < arrayValue.length; j++) { + b.append(arrayValue[j]); + if (j < arrayValue.length - 1) + b.append(','); } + b.append(']'); } else b.append(this.values[i]); if (i < this.values.length - 1) diff --git a/clients/src/test/java/org/apache/kafka/common/protocol/types/ProtocolSerializationTest.java b/clients/src/test/java/org/apache/kafka/common/protocol/types/ProtocolSerializationTest.java index 503facf2101fe..1633e89acd564 100644 --- a/clients/src/test/java/org/apache/kafka/common/protocol/types/ProtocolSerializationTest.java +++ b/clients/src/test/java/org/apache/kafka/common/protocol/types/ProtocolSerializationTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; import java.nio.ByteBuffer; @@ -233,9 +234,9 @@ public void testReadNegativeBytesSize() { @Test public void testToString() { - String stuctStr = this.struct.toString(); - assertFalse("Stuct string should not be null.", stuctStr == null); - assertFalse("Stuct string should not be empty.", stuctStr.isEmpty()); + String structStr = this.struct.toString(); + assertNotNull("Struct string should not be null.", structStr); + assertFalse("Struct string should not be empty.", structStr.isEmpty()); } private Object roundtrip(Type type, Object obj) {