You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kafka.apache.org by ew...@apache.org on 2018/06/17 19:12:56 UTC

[kafka] branch 2.0 updated: KAFKA-7068: Handle null config values during transform (KIP-297)

This is an automated email from the ASF dual-hosted git repository.

ewencp pushed a commit to branch 2.0
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/2.0 by this push:
     new 6041220  KAFKA-7068: Handle null config values during transform (KIP-297)
6041220 is described below

commit 6041220e900f5a9e427a749ceb204c3717a97668
Author: Robert Yokota <ra...@gmail.com>
AuthorDate: Sun Jun 17 12:12:11 2018 -0700

    KAFKA-7068: Handle null config values during transform (KIP-297)
    
    Fix NPE when processing null config values during transform.
    
    Author: Robert Yokota <ra...@gmail.com>
    
    Reviewers: Magesh Nandakumar <ma...@gmail.com>, Ewen Cheslack-Postava <ew...@confluent.io>
    
    Closes #5241 from rayokota/KIP-297-null-config-values
    
    (cherry picked from commit d06da1b7f424ebad16ea5eca11b58b7c2ca3fa34)
    Signed-off-by: Ewen Cheslack-Postava <me...@ewencp.org>
---
 .../kafka/common/config/ConfigTransformer.java     | 15 ++++++++-----
 .../kafka/common/config/ConfigTransformerTest.java | 26 +++++++++++++++++++++-
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/clients/src/main/java/org/apache/kafka/common/config/ConfigTransformer.java b/clients/src/main/java/org/apache/kafka/common/config/ConfigTransformer.java
index 7e21a32..f5a3737 100644
--- a/clients/src/main/java/org/apache/kafka/common/config/ConfigTransformer.java
+++ b/clients/src/main/java/org/apache/kafka/common/config/ConfigTransformer.java
@@ -80,11 +80,13 @@ public class ConfigTransformer {
 
         // Collect the variables from the given configs that need transformation
         for (Map.Entry<String, String> config : configs.entrySet()) {
-            List<ConfigVariable> vars = getVars(config.getKey(), config.getValue(), DEFAULT_PATTERN);
-            for (ConfigVariable var : vars) {
-                Map<String, Set<String>> keysByPath = keysByProvider.computeIfAbsent(var.providerName, k -> new HashMap<>());
-                Set<String> keys = keysByPath.computeIfAbsent(var.path, k -> new HashSet<>());
-                keys.add(var.variable);
+            if (config.getValue() != null) {
+                List<ConfigVariable> vars = getVars(config.getKey(), config.getValue(), DEFAULT_PATTERN);
+                for (ConfigVariable var : vars) {
+                    Map<String, Set<String>> keysByPath = keysByProvider.computeIfAbsent(var.providerName, k -> new HashMap<>());
+                    Set<String> keys = keysByPath.computeIfAbsent(var.path, k -> new HashSet<>());
+                    keys.add(var.variable);
+                }
             }
         }
 
@@ -131,6 +133,9 @@ public class ConfigTransformer {
     private static String replace(Map<String, Map<String, Map<String, String>>> lookupsByProvider,
                                   String value,
                                   Pattern pattern) {
+        if (value == null) {
+            return null;
+        }
         Matcher matcher = pattern.matcher(value);
         StringBuilder builder = new StringBuilder();
         int i = 0;
diff --git a/clients/src/test/java/org/apache/kafka/common/config/ConfigTransformerTest.java b/clients/src/test/java/org/apache/kafka/common/config/ConfigTransformerTest.java
index d6bd3dc..e2b9f6b 100644
--- a/clients/src/test/java/org/apache/kafka/common/config/ConfigTransformerTest.java
+++ b/clients/src/test/java/org/apache/kafka/common/config/ConfigTransformerTest.java
@@ -26,6 +26,7 @@ import java.util.Map;
 import java.util.Set;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 public class ConfigTransformerTest {
@@ -37,6 +38,7 @@ public class ConfigTransformerTest {
     public static final String TEST_PATH = "testPath";
     public static final String TEST_RESULT = "testResult";
     public static final String TEST_RESULT_WITH_TTL = "testResultWithTTL";
+    public static final String TEST_RESULT_NO_PATH = "testResultNoPath";
 
     private ConfigTransformer configTransformer;
 
@@ -84,6 +86,24 @@ public class ConfigTransformerTest {
         assertEquals("${test:testPath:testResult}", data.get(MY_KEY));
     }
 
+    @Test
+    public void testReplaceVariableNoPath() throws Exception {
+        ConfigTransformerResult result = configTransformer.transform(Collections.singletonMap(MY_KEY, "${test:testKey}"));
+        Map<String, String> data = result.data();
+        Map<String, Long> ttls = result.ttls();
+        assertEquals(TEST_RESULT_NO_PATH, data.get(MY_KEY));
+        assertTrue(ttls.isEmpty());
+    }
+
+    @Test
+    public void testNullConfigValue() throws Exception {
+        ConfigTransformerResult result = configTransformer.transform(Collections.singletonMap(MY_KEY, null));
+        Map<String, String> data = result.data();
+        Map<String, Long> ttls = result.ttls();
+        assertNull(data.get(MY_KEY));
+        assertTrue(ttls.isEmpty());
+    }
+
     public static class TestConfigProvider implements ConfigProvider {
 
         public void configure(Map<String, ?> configs) {
@@ -96,7 +116,7 @@ public class ConfigTransformerTest {
         public ConfigData get(String path, Set<String> keys) {
             Map<String, String> data = new HashMap<>();
             Long ttl = null;
-            if (path.equals(TEST_PATH)) {
+            if (TEST_PATH.equals(path)) {
                 if (keys.contains(TEST_KEY)) {
                     data.put(TEST_KEY, TEST_RESULT);
                 }
@@ -107,6 +127,10 @@ public class ConfigTransformerTest {
                 if (keys.contains(TEST_INDIRECTION)) {
                     data.put(TEST_INDIRECTION, "${test:testPath:testResult}");
                 }
+            } else {
+                if (keys.contains(TEST_KEY)) {
+                    data.put(TEST_KEY, TEST_RESULT_NO_PATH);
+                }
             }
             return new ConfigData(data, ttl);
         }

-- 
To stop receiving notification emails like this one, please contact
ewencp@apache.org.