kafka-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From maniku...@apache.org
Subject [kafka] branch trunk updated: KAFKA-8060: The Kafka protocol generator should allow null defaults
Date Fri, 08 Mar 2019 03:55:48 GMT
This is an automated email from the ASF dual-hosted git repository.

manikumar 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 027cbba  KAFKA-8060: The Kafka protocol generator should allow null defaults
027cbba is described below

commit 027cbbaec521542f53274183daccc2073e91cfe9
Author: Colin P. Mccabe <cmccabe@confluent.io>
AuthorDate: Fri Mar 8 09:25:28 2019 +0530

    KAFKA-8060: The Kafka protocol generator should allow null defaults
    
    Author: Colin P. Mccabe <cmccabe@confluent.io>
    
    Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
    
    Closes #6387 from cmccabe/KAFKA-8060
---
 .../src/main/resources/common/message/MetadataResponse.json   |  2 +-
 clients/src/main/resources/common/message/README.md           |  4 +++-
 .../java/org/apache/kafka/message/MessageDataGenerator.java   | 11 ++++++++++-
 .../src/main/java/org/apache/kafka/message/Versions.java      |  7 +++++++
 .../src/test/java/org/apache/kafka/message/VersionsTest.java  |  5 +++++
 5 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/clients/src/main/resources/common/message/MetadataResponse.json b/clients/src/main/resources/common/message/MetadataResponse.json
index 1e199df..e58a720 100644
--- a/clients/src/main/resources/common/message/MetadataResponse.json
+++ b/clients/src/main/resources/common/message/MetadataResponse.json
@@ -44,7 +44,7 @@
         "about": "The broker hostname." },
       { "name": "Port", "type": "int32", "versions": "0+",
         "about": "The broker port." },
-      { "name": "Rack", "type": "string", "versions": "1+", "nullableVersions": "1+", "ignorable":
true,
+      { "name": "Rack", "type": "string", "versions": "1+", "nullableVersions": "1+", "ignorable":
true, "default": "null",
         "about": "The rack of the broker, or null if it has not been assigned to a rack."
}
     ]},
     { "name": "ClusterId", "type": "string", "nullableVersions": "2+", "versions": "2+",
"ignorable": true,
diff --git a/clients/src/main/resources/common/message/README.md b/clients/src/main/resources/common/message/README.md
index 482b1dd..2de09bc 100644
--- a/clients/src/main/resources/common/message/README.md
+++ b/clients/src/main/resources/common/message/README.md
@@ -144,7 +144,9 @@ been set:
 
 * Array fields default to empty.
 
-Null is never used as a default for any field.
+You can specify "null" as a default value for a string field by specifing the
+literal string "null".  Note that you can only specify null as a default if all
+versions of the field are nullable.
 
 Custom Default Values
 ---------------------
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 1af0ce28..76029f4 100644
--- a/generator/src/main/java/org/apache/kafka/message/MessageDataGenerator.java
+++ b/generator/src/main/java/org/apache/kafka/message/MessageDataGenerator.java
@@ -1198,7 +1198,16 @@ public final class MessageDataGenerator {
                 return field.defaultString() + "L";
             }
         } else if (field.type() instanceof FieldType.StringFieldType) {
-            return "\"" + field.defaultString() + "\"";
+            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.");
+                }
+                return "null";
+            } else {
+                return "\"" + field.defaultString() + "\"";
+            }
         } else if (field.type().isBytes()) {
             if (!field.defaultString().isEmpty()) {
                 throw new RuntimeException("Invalid default for bytes field " +
diff --git a/generator/src/main/java/org/apache/kafka/message/Versions.java b/generator/src/main/java/org/apache/kafka/message/Versions.java
index 196540b..3903680 100644
--- a/generator/src/main/java/org/apache/kafka/message/Versions.java
+++ b/generator/src/main/java/org/apache/kafka/message/Versions.java
@@ -122,6 +122,13 @@ public final class Versions {
         return version >= lowest && version <= highest;
     }
 
+    public boolean contains(Versions other) {
+        if (other.empty()) {
+            return true;
+        }
+        return !((lowest > other.lowest) || (highest < other.highest));
+    }
+
     @Override
     public int hashCode() {
         return Objects.hash(lowest, highest);
diff --git a/generator/src/test/java/org/apache/kafka/message/VersionsTest.java b/generator/src/test/java/org/apache/kafka/message/VersionsTest.java
index 4384618..051202e 100644
--- a/generator/src/test/java/org/apache/kafka/message/VersionsTest.java
+++ b/generator/src/test/java/org/apache/kafka/message/VersionsTest.java
@@ -81,5 +81,10 @@ public class VersionsTest {
         assertFalse(newVersions(0, 1).contains((short) 2));
         assertTrue(newVersions(0, Short.MAX_VALUE).contains((short) 100));
         assertFalse(newVersions(2, Short.MAX_VALUE).contains((short) 0));
+        assertTrue(newVersions(2, 3).contains(newVersions(2, 3)));
+        assertTrue(newVersions(2, 3).contains(newVersions(2, 2)));
+        assertFalse(newVersions(2, 3).contains(newVersions(2, 4)));
+        assertTrue(newVersions(2, 3).contains(Versions.NONE));
+        assertTrue(Versions.ALL.contains(newVersions(1, 2)));
     }
 }


Mime
View raw message