[Fix] Fix ReadonlyConfig lose key error (apache#5565)

hailin0 · Sep 28, 2023 · 36754cc · 36754cc
1 parent 512d982
commit 36754cc
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 2 deletions.
diff --git a/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/ConfigUtil.java b/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/ConfigUtil.java
@@ -113,7 +113,7 @@ private static void putKeyValueToMapCheck(
                 } else if (propertiesMap.get(tempPrefix) instanceof String) {
                     loadPropertiesStyleMap(temp).put("", propertiesMap.get(tempPrefix));
                 } else {
-                    ((Map) propertiesMap.get(tempPrefix)).putAll(loadPropertiesStyleMap(temp));
+                    mergeTwoMap((Map) propertiesMap.get(tempPrefix), loadPropertiesStyleMap(temp));
                 }
             } else {
                 propertiesMap.put(tempPrefix, loadPropertiesStyleObject(temp));
@@ -122,6 +122,30 @@ private static void putKeyValueToMapCheck(
         }
     }
 
+    private static void mergeTwoMap(Map<String, Object> base, Map<String, Object> merged) {
+        for (Map.Entry<String, Object> entry : merged.entrySet()) {
+            if (base.containsKey(entry.getKey())) {
+                if (base.get(entry.getKey()) instanceof Map && entry.getValue() instanceof Map) {
+                    mergeTwoMap((Map) base.get(entry.getKey()), (Map) entry.getValue());
+                } else if (base.get(entry.getKey()) instanceof Map) {
+                    ((Map) base.get(entry.getKey())).put("", entry.getValue());
+                } else if (entry.getValue() instanceof Map) {
+                    Map<String, Object> child = new LinkedHashMap<>();
+                    child.put("", base.get(entry.getKey()));
+                    child.putAll((Map) entry.getValue());
+                    base.put(entry.getKey(), child);
+                } else {
+                    throw new IllegalArgumentException(
+                            String.format(
+                                    "Duplicate key '%s' in config file, value '%s' and value '%s'",
+                                    entry.getKey(), base.get(entry.getKey()), entry.getValue()));
+                }
+            } else {
+                base.put(entry.getKey(), entry.getValue());
+            }
+        }
+    }
+
     private static List<Object> loadPropertiesStyleList(Map<List<String>, String> properties) {
         List<Object> propertiesList = new ArrayList<>();
         Map<List<String>, String> temp = new LinkedHashMap<>();
@@ -148,8 +172,14 @@ private static List<Object> loadPropertiesStyleList(Map<List<String>, String> pr
     }
 
     private static Object loadPropertiesStyleObject(Map<List<String>, String> properties) {
-        if (properties.containsKey(null)) {
+        if (properties.containsKey(null) && properties.size() == 1) {
             return StringEscapeUtils.unescapeJava(properties.get(null));
+        } else if (properties.containsKey(null)) {
+            if (properties.containsKey(null)) {
+                properties.put(Collections.singletonList(""), properties.get(null));
+                properties.remove(null);
+            }
+            return loadPropertiesStyleMap(properties);
         } else if (properties.entrySet().stream().anyMatch(kv -> kv.getKey().get(0).equals("1"))) {
             return loadPropertiesStyleList(properties);
         } else {

diff --git a/seatunnel-api/src/test/java/org/apache/seatunnel/api/configuration/util/ConfigUtilTest.java b/seatunnel-api/src/test/java/org/apache/seatunnel/api/configuration/util/ConfigUtilTest.java
@@ -38,6 +38,7 @@
 import java.nio.file.Paths;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -139,4 +140,55 @@ public void treeMapFunctionTest() throws IOException, URISyntaxException {
         Assertions.assertEquals(
                 ((Map) ((List) value.get("source")).get(0)).get("start_mode"), expect);
     }
+
+    @Test
+    public void testSamePrefixDifferentSuffixKey() {
+        Map<String, Object> config = new LinkedHashMap<>();
+        config.put(
+                "fs.s3a.aws.credentials.provider",
+                "org.apache.hadoop.fs.s3a.SimpleAWSCredentialsProvider");
+        config.put("other", "value");
+        config.put("fs.s3a.endpoint", "s3.cn-northwest-1.amazonaws.com.cn");
+        Map<String, Object> result = ConfigUtil.treeMap(config);
+        Map<String, Object> s3aMap =
+                (Map<String, Object>) ((Map<String, Object>) result.get("fs")).get("s3a");
+        Assertions.assertTrue(s3aMap.containsKey("aws"));
+        Assertions.assertTrue(s3aMap.containsKey("endpoint"));
+        Assertions.assertEquals(2, s3aMap.size());
+    }
+
+    @Test
+    public void testSamePrefixDifferentValueType() {
+        Map<String, Object> config = new LinkedHashMap<>();
+        config.put("start.mode", "CONSUME_FROM_TIMESTAMP");
+        config.put("other", "value");
+        config.put("start.mode.timestamp", "1667179890315");
+        Map<String, Object> result = ConfigUtil.treeMap(config);
+        Map<String, Object> s3aMap =
+                (Map<String, Object>) ((Map<String, Object>) result.get("start")).get("mode");
+        Assertions.assertTrue(s3aMap.containsKey(""));
+        Assertions.assertTrue(s3aMap.containsKey("timestamp"));
+        Assertions.assertEquals(2, s3aMap.size());
+
+        config.clear();
+        config.put("start.mode", "CONSUME_FROM_TIMESTAMP");
+        config.put("start.mode.timestamp", "1667179890315");
+        Map<String, Object> result2 = ConfigUtil.treeMap(config);
+        Map<String, Object> s3aMap2 =
+                (Map<String, Object>) ((Map<String, Object>) result2.get("start")).get("mode");
+        Assertions.assertTrue(s3aMap2.containsKey(""));
+        Assertions.assertTrue(s3aMap2.containsKey("timestamp"));
+        Assertions.assertEquals(2, s3aMap2.size());
+
+        config.clear();
+        config.put("start.mode", "CONSUME_FROM_TIMESTAMP");
+        config.put("start.mode.timestamp.test1", "1667179890315");
+        config.put("start.mode.timestamp.test2", "1667179890315");
+        Map<String, Object> result3 = ConfigUtil.treeMap(config);
+        Map<String, Object> s3aMap3 =
+                (Map<String, Object>) ((Map<String, Object>) result3.get("start")).get("mode");
+        Assertions.assertTrue(s3aMap3.containsKey(""));
+        Assertions.assertTrue(s3aMap3.containsKey("timestamp"));
+        Assertions.assertEquals(2, s3aMap3.size());
+    }
 }