From 396022e7439379eec4c26458407b3860306589b4 Mon Sep 17 00:00:00 2001
From: "voonhou.su" <voonhou.su@shopee.com>
Date: Mon, 4 Jul 2022 11:58:43 +0800
Subject: [PATCH] Added UT for ConfigUtils.toMap and handled more edge cases

---
 .../org/apache/hudi/hive/HiveSyncConfig.java  |  2 -
 .../hudi/sync/common/util/ConfigUtils.java    |  4 ++
 .../sync/common/util/TestConfigUtils.java     | 50 +++++++++++++++++++
 3 files changed, 54 insertions(+), 2 deletions(-)
 create mode 100644 hudi-sync/hudi-sync-common/src/test/java/org/apache/hudi/sync/common/util/TestConfigUtils.java

diff --git a/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncConfig.java b/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncConfig.java
index 6f2cc50a0af6..19710b1685bb 100644
--- a/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncConfig.java
+++ b/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncConfig.java
@@ -129,8 +129,6 @@ public static class HiveSyncConfigParams {
     public String bucketSpec;
     @Parameter(names = {"--sync-comment"}, description = "synchronize table comments to hive")
     public Boolean syncComment;
-    @Parameter(names = {"--with-operation-field"}, description = "Whether to include the '_hoodie_operation' field in the metadata fields")
-    public Boolean withOperationField; // TODO remove this as it's not used
 
     public boolean isHelp() {
       return hoodieSyncConfigParams.isHelp();
diff --git a/hudi-sync/hudi-sync-common/src/main/java/org/apache/hudi/sync/common/util/ConfigUtils.java b/hudi-sync/hudi-sync-common/src/main/java/org/apache/hudi/sync/common/util/ConfigUtils.java
index 9c9979d5539b..62cd4c1748fc 100644
--- a/hudi-sync/hudi-sync-common/src/main/java/org/apache/hudi/sync/common/util/ConfigUtils.java
+++ b/hudi-sync/hudi-sync-common/src/main/java/org/apache/hudi/sync/common/util/ConfigUtils.java
@@ -47,6 +47,10 @@ public static Map<String, String> toMap(String keyValueConfig) {
     String[] keyvalues = keyValueConfig.split("\n");
     Map<String, String> tableProperties = new HashMap<>();
     for (String keyValue : keyvalues) {
+      // Handle multiple new lines and lines that contain only spaces after splitting
+      if (keyValue.trim().isEmpty()) {
+        continue;
+      }
       String[] keyValueArray = keyValue.split("=");
       if (keyValueArray.length == 1 || keyValueArray.length == 2) {
         String key = keyValueArray[0].trim();
diff --git a/hudi-sync/hudi-sync-common/src/test/java/org/apache/hudi/sync/common/util/TestConfigUtils.java b/hudi-sync/hudi-sync-common/src/test/java/org/apache/hudi/sync/common/util/TestConfigUtils.java
new file mode 100644
index 000000000000..f6e48543ff0d
--- /dev/null
+++ b/hudi-sync/hudi-sync-common/src/test/java/org/apache/hudi/sync/common/util/TestConfigUtils.java
@@ -0,0 +1,50 @@
+package org.apache.hudi.sync.common.util;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.HashMap;
+import java.util.Map;
+import org.junit.jupiter.api.Test;
+
+public class TestConfigUtils {
+
+  @Test
+  public void testToMapSucceeds() {
+    Map<String, String> expectedMap = new HashMap<>();
+    expectedMap.put("k.1.1.2", "v1");
+    expectedMap.put("k.2.1.2", "v2");
+    expectedMap.put("k.3.1.2", "v3");
+
+    // Test base case
+    String srcKv = "k.1.1.2=v1\nk.2.1.2=v2\nk.3.1.2=v3";
+    Map<String, String> outMap = ConfigUtils.toMap(srcKv);
+    assertEquals(expectedMap, outMap);
+
+    // Test ends with new line
+    srcKv = "k.1.1.2=v1\nk.2.1.2=v2\nk.3.1.2=v3\n";
+    outMap = ConfigUtils.toMap(srcKv);
+    assertEquals(expectedMap, outMap);
+
+    // Test delimited by multiple new lines
+    srcKv = "k.1.1.2=v1\nk.2.1.2=v2\n\nk.3.1.2=v3";
+    outMap = ConfigUtils.toMap(srcKv);
+    assertEquals(expectedMap, outMap);
+
+    // Test delimited by multiple new lines with spaces in between
+    srcKv = "k.1.1.2=v1\n  \nk.2.1.2=v2\n\nk.3.1.2=v3";
+    outMap = ConfigUtils.toMap(srcKv);
+    assertEquals(expectedMap, outMap);
+
+    // Test with random spaces if trim works properly
+    srcKv = " k.1.1.2 =   v1\n k.2.1.2 = v2 \nk.3.1.2 = v3";
+    outMap = ConfigUtils.toMap(srcKv);
+    assertEquals(expectedMap, outMap);
+  }
+
+  @Test
+  public void testToMapThrowError() {
+    String srcKv = "k.1.1.2=v1=v1.1\nk.2.1.2=v2\nk.3.1.2=v3";
+    assertThrows(IllegalArgumentException.class, () -> ConfigUtils.toMap(srcKv));
+  }
+}
\ No newline at end of file