This is an automated email from the ASF dual-hosted git repository.
gwenshap pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/trunk by this push:
new d70fe5e KAFKA-8644; The Kafka protocol generator should allow null
defaults for bytes and array fields
d70fe5e is described below
commit d70fe5ed2d25af432ec6ce02ccfeaa40bcd2f295
Author: Colin P. Mccabe <cmcc...@confluent.io>
AuthorDate: Thu Jul 11 09:52:01 2019 -0700
KAFKA-8644; The Kafka protocol generator should allow null defaults for
bytes and array fields
Author: Colin P. Mccabe <cmcc...@confluent.io>
Reviewers: Stanislav Kozlovski, Gwen Shapira
Closes #7059 from cmccabe/KAFKA-8644
---
.../apache/kafka/message/MessageDataGenerator.java | 42 +++++++---
.../kafka/message/MessageDataGeneratorTest.java | 93 ++++++++++++++++++++++
2 files changed, 124 insertions(+), 11 deletions(-)
diff --git
a/generator/src/main/java/org/apache/kafka/message/MessageDataGenerator.java
b/generator/src/main/java/org/apache/kafka/message/MessageDataGenerator.java
index ee0c3df..d0e5044 100644
--- a/generator/src/main/java/org/apache/kafka/message/MessageDataGenerator.java
+++ b/generator/src/main/java/org/apache/kafka/message/MessageDataGenerator.java
@@ -937,9 +937,17 @@ public final class MessageDataGenerator {
buffer.incrementIndent();
headerGenerator.addImport(MessageGenerator.UNSUPPORTED_VERSION_EXCEPTION_CLASS);
if (field.type().isArray()) {
- buffer.printf("if (!%s.isEmpty()) {%n", field.camelCaseName());
+ if (fieldDefault(field).equals("null")) {
+ buffer.printf("if (%s != null) {%n", field.camelCaseName());
+ } else {
+ buffer.printf("if (!%s.isEmpty()) {%n", field.camelCaseName());
+ }
} else if (field.type().isBytes()) {
- buffer.printf("if (%s.length != 0) {%n", field.camelCaseName());
+ if (fieldDefault(field).equals("null")) {
+ buffer.printf("if (%s != null) {%n", field.camelCaseName());
+ } else {
+ buffer.printf("if (%s.length != 0) {%n",
field.camelCaseName());
+ }
} else if (field.type().isString()) {
if (fieldDefault(field).equals("null")) {
buffer.printf("if (%s != null) {%n", field.camelCaseName());
@@ -1221,19 +1229,19 @@ public final class MessageDataGenerator {
}
} else if (field.type() instanceof FieldType.StringFieldType) {
if (field.defaultString().equals("null")) {
- if (!(field.nullableVersions().contains(field.versions()))) {
- throw new RuntimeException("null cannot be the default for
field " +
- field.name() + ", because not all versions of this
field are " +
- "nullable.");
- }
+ validateNullDefault(field);
return "null";
} else {
return "\"" + field.defaultString() + "\"";
}
} else if (field.type().isBytes()) {
- if (!field.defaultString().isEmpty()) {
+ if (field.defaultString().equals("null")) {
+ validateNullDefault(field);
+ return "null";
+ } else if (!field.defaultString().isEmpty()) {
throw new RuntimeException("Invalid default for bytes field " +
- field.name() + ": custom defaults are not supported for
bytes fields.");
+ field.name() + ". The only valid default for a bytes
field " +
+ "is empty or null.");
}
headerGenerator.addImport(MessageGenerator.BYTES_CLASS);
return "Bytes.EMPTY";
@@ -1244,9 +1252,13 @@ public final class MessageDataGenerator {
}
return "new " + field.type().toString() + "()";
} else if (field.type().isArray()) {
- if (!field.defaultString().isEmpty()) {
+ if (field.defaultString().equals("null")) {
+ validateNullDefault(field);
+ return "null";
+ } else if (!field.defaultString().isEmpty()) {
throw new RuntimeException("Invalid default for array field " +
- field.name() + ": custom defaults are not supported for
array fields.");
+ field.name() + ". The only valid default for an array
field " +
+ "is the empty array or null.");
}
FieldType.ArrayType arrayType = (FieldType.ArrayType) field.type();
if (structRegistry.isStructArrayWithKeys(field)) {
@@ -1260,6 +1272,14 @@ public final class MessageDataGenerator {
}
}
+ private void validateNullDefault(FieldSpec field) {
+ if (!(field.nullableVersions().contains(field.versions()))) {
+ throw new RuntimeException("null cannot be the default for field "
+
+ field.name() + ", because not all versions of this field
are " +
+ "nullable.");
+ }
+ }
+
private void generateFieldAccessor(FieldSpec field) {
buffer.printf("%n");
generateAccessor(fieldAbstractJavaType(field), field.camelCaseName(),
diff --git
a/generator/src/test/java/org/apache/kafka/message/MessageDataGeneratorTest.java
b/generator/src/test/java/org/apache/kafka/message/MessageDataGeneratorTest.java
new file mode 100644
index 0000000..e8fcaf2
--- /dev/null
+++
b/generator/src/test/java/org/apache/kafka/message/MessageDataGeneratorTest.java
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kafka.message;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.Timeout;
+
+import java.util.Arrays;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class MessageDataGeneratorTest {
+ @Rule
+ final public Timeout globalTimeout = Timeout.millis(120000);
+
+ @Test
+ public void testNullDefaults() throws Exception {
+ MessageSpec testMessageSpec =
MessageGenerator.JSON_SERDE.readValue(String.join("", Arrays.asList(
+ "{",
+ " \"type\": \"request\",",
+ " \"name\": \"FooBar\",",
+ " \"validVersions\": \"0-2\",",
+ " \"fields\": [",
+ " { \"name\": \"field1\", \"type\": \"int32\",
\"versions\": \"0+\" },",
+ " { \"name\": \"field2\", \"type\": \"[]TestStruct\",
\"versions\": \"1+\", ",
+ " \"nullableVersions\": \"1+\", \"default\": \"null\",
\"fields\": [",
+ " { \"name\": \"field1\", \"type\": \"int32\",
\"versions\": \"0+\" }",
+ " ]},",
+ " { \"name\": \"field3\", \"type\": \"bytes\",
\"versions\": \"2+\", ",
+ " \"nullableVersions\": \"2+\", \"default\": \"null\" }",
+ " ]",
+ "}")), MessageSpec.class);
+ new MessageDataGenerator().generate(testMessageSpec);
+ }
+
+ @Test
+ public void testInvalidNullDefaultForInt() throws Exception {
+ MessageSpec testMessageSpec =
MessageGenerator.JSON_SERDE.readValue(String.join("", Arrays.asList(
+ "{",
+ " \"type\": \"request\",",
+ " \"name\": \"FooBar\",",
+ " \"validVersions\": \"0-2\",",
+ " \"fields\": [",
+ " { \"name\": \"field1\", \"type\": \"int32\", \"versions\":
\"0+\", \"default\": \"null\" }",
+ " ]",
+ "}")), MessageSpec.class);
+ try {
+ new MessageDataGenerator().generate(testMessageSpec);
+ fail("Expected MessageDataGenerator#generate to fail");
+ } catch (Throwable e) {
+ assertTrue("Invalid error message: " + e.getMessage(),
+ e.getMessage().contains("Invalid default for int32"));
+ }
+ }
+
+ @Test
+ public void testInvalidNullDefaultForPotentiallyNonNullableArray() throws
Exception {
+ MessageSpec testMessageSpec =
MessageGenerator.JSON_SERDE.readValue(String.join("", Arrays.asList(
+ "{",
+ " \"type\": \"request\",",
+ " \"name\": \"FooBar\",",
+ " \"validVersions\": \"0-2\",",
+ " \"fields\": [",
+ " { \"name\": \"field1\", \"type\": \"[]int32\",
\"versions\": \"0+\", \"nullableVersions\": \"1+\", ",
+ " \"default\": \"null\" }",
+ " ]",
+ "}")), MessageSpec.class);
+ try {
+ new MessageDataGenerator().generate(testMessageSpec);
+ fail("Expected MessageDataGenerator#generate to fail");
+ } catch (RuntimeException e) {
+ assertTrue("Invalid error message: " + e.getMessage(),
+ e.getMessage().contains("not all versions of this field
are nullable"));
+ }
+ }
+}
