You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@samza.apache.org by xi...@apache.org on 2017/11/29 19:36:45 UTC

[6/8] samza git commit: SAMZA-1514: Prevent changelog stream names with empty strings.

SAMZA-1514: Prevent changelog stream names with empty strings.

This fix prevents changelog stream names with empty or whitespace-only strings.

Author: Daniel Nishimura <dn...@gmail.com>

Reviewers: Jacob Maes <jm...@linkedin.com>

Closes #371 from dnishimura/samza-1514-empty-string-changelog


Project: http://git-wip-us.apache.org/repos/asf/samza/repo
Commit: http://git-wip-us.apache.org/repos/asf/samza/commit/3b2a1fa4
Tree: http://git-wip-us.apache.org/repos/asf/samza/tree/3b2a1fa4
Diff: http://git-wip-us.apache.org/repos/asf/samza/diff/3b2a1fa4

Branch: refs/heads/0.14.0
Commit: 3b2a1fa4398dc26f9270c405c01edaac59ba9fc1
Parents: d262f66
Author: Daniel Nishimura <dn...@gmail.com>
Authored: Tue Nov 28 16:19:45 2017 -0800
Committer: Jacob Maes <jm...@linkedin.com>
Committed: Tue Nov 28 16:19:45 2017 -0800

----------------------------------------------------------------------
 .../org/apache/samza/config/JavaStorageConfig.java     |  5 +++--
 .../org/apache/samza/config/TestJavaStorageConfig.java | 13 +++++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/samza/blob/3b2a1fa4/samza-core/src/main/java/org/apache/samza/config/JavaStorageConfig.java
----------------------------------------------------------------------
diff --git a/samza-core/src/main/java/org/apache/samza/config/JavaStorageConfig.java b/samza-core/src/main/java/org/apache/samza/config/JavaStorageConfig.java
index 34e5683..48beec9 100644
--- a/samza-core/src/main/java/org/apache/samza/config/JavaStorageConfig.java
+++ b/samza-core/src/main/java/org/apache/samza/config/JavaStorageConfig.java
@@ -21,6 +21,7 @@ package org.apache.samza.config;
 
 import java.util.ArrayList;
 import java.util.List;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.samza.SamzaException;
 import org.apache.samza.execution.StreamManager;
 
@@ -58,11 +59,11 @@ public class JavaStorageConfig extends MapConfig {
     // If the config specifies 'stores.<storename>.changelog' as '<system>.<stream>' combination - it will take precedence.
     // If this config only specifies <astream> and there is a value in job.changelog.system=<asystem> -
     // these values will be combined into <asystem>.<astream>
-    String systemStream = get(String.format(CHANGELOG_STREAM, storeName), null);
-    String changelogSystem = getChangelogSystem();
+    String systemStream = StringUtils.trimToNull(get(String.format(CHANGELOG_STREAM, storeName), null));
 
     String systemStreamRes;
     if (systemStream != null  && !systemStream.contains(".")) {
+      String changelogSystem = getChangelogSystem();
       // contains only stream name
       if (changelogSystem != null) {
         systemStreamRes = changelogSystem + "." + systemStream;

http://git-wip-us.apache.org/repos/asf/samza/blob/3b2a1fa4/samza-core/src/test/java/org/apache/samza/config/TestJavaStorageConfig.java
----------------------------------------------------------------------
diff --git a/samza-core/src/test/java/org/apache/samza/config/TestJavaStorageConfig.java b/samza-core/src/test/java/org/apache/samza/config/TestJavaStorageConfig.java
index cc80430..c04d14f 100644
--- a/samza-core/src/test/java/org/apache/samza/config/TestJavaStorageConfig.java
+++ b/samza-core/src/test/java/org/apache/samza/config/TestJavaStorageConfig.java
@@ -68,4 +68,17 @@ public class TestJavaStorageConfig {
        // do nothing, it is expected
     }
   }
+
+  @Test
+  public void testEmptyStringOrNullChangelogStream() {
+    Map<String, String> configMap = new HashMap<>();
+    configMap.put("stores.store1.changelog", "");
+    configMap.put("stores.store2.changelog", " ");
+
+    JavaStorageConfig config = new JavaStorageConfig(new MapConfig(configMap));
+
+    assertNull(config.getChangelogStream("store1"));
+    assertNull(config.getChangelogStream("store2"));
+    assertNull(config.getChangelogStream("store-changelog-none"));
+  }
 }