Better null handling for lists in YAML (#612)

smallrye · Jul 5, 2021 · f0af955 · f0af955
1 parent caf0dd6
commit f0af955
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 11 deletions.
diff --git a/sources/yaml/src/main/java/io/smallrye/config/source/yaml/YamlConfigSource.java b/sources/yaml/src/main/java/io/smallrye/config/source/yaml/YamlConfigSource.java
@@ -6,6 +6,7 @@
 import java.io.InputStream;
 import java.io.UncheckedIOException;
 import java.net.URL;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -149,10 +150,7 @@ private static void flattenYaml(String path, Map<Object, Object> source, Map<Str
             }
 
             if (value instanceof String) {
-                // We don't add the indexed form for List of String and keep in the MP form, comma-separated.
-                if (!indexed) {
-                    target.put(key, (String) value);
-                }
+                target.put(key, (String) value);
             } else if (value instanceof Map) {
                 flattenYaml(key, (Map<Object, Object>) value, target, false);
             } else if (value instanceof List) {
@@ -162,16 +160,29 @@ private static void flattenYaml(String path, Map<Object, Object> source, Map<Str
                     flattenYaml(key, Collections.singletonMap("[" + i + "]", list.get(i)), target, true);
                 }
             } else {
-                target.put(key, (value != null ? value.toString() : ""));
+                if (value != null) {
+                    target.put(key, value.toString());
+                }
             }
         });
     }
 
     private static void flattenList(String key, List<Object> source, Map<String, String> target) {
-        if (source.stream().allMatch(o -> o instanceof String)) {
-            target.put(key, source.stream().map(o -> {
+        boolean mixed = false;
+        List<String> flatten = new ArrayList<>();
+        for (Object value : source) {
+            if (value instanceof String || value instanceof Boolean) {
+                flatten.add(value.toString());
+            } else if (value != null) {
+                mixed = true;
+                break;
+            }
+        }
+
+        if (!mixed) {
+            target.put(key, flatten.stream().map(value -> {
                 StringBuilder sb = new StringBuilder();
-                escapeCommas(sb, o.toString(), 1);
+                escapeCommas(sb, value, 1);
                 return sb.toString();
             }).collect(Collectors.joining(",")));
         } else {
@@ -216,6 +227,8 @@ private static Set<String> filterPropertyNames(Map<String, String> source) {
      */
     private static class StringConstructor extends Constructor {
         public StringConstructor() {
+            this.yamlConstructors.put(Tag.INT, new ConstructYamlStr());
+            this.yamlConstructors.put(Tag.FLOAT, new ConstructYamlStr());
             this.yamlConstructors.put(Tag.TIMESTAMP, new ConstructYamlStr());
         }
     }

diff --git a/sources/yaml/src/test/java/io/smallrye/config/source/yaml/ArrayTest.java b/sources/yaml/src/test/java/io/smallrye/config/source/yaml/ArrayTest.java
@@ -1,10 +1,17 @@
 package io.smallrye.config.source.yaml;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import java.util.Set;
 
 import org.eclipse.microprofile.config.spi.ConfigSource;
 import org.junit.jupiter.api.Test;
 
+import io.smallrye.config.SmallRyeConfig;
+import io.smallrye.config.SmallRyeConfigBuilder;
+
 class ArrayTest {
     @Test
     void array() {
@@ -25,4 +32,22 @@ void array() {
         assertEquals("Google", src.getValue("de.javahippie.mpadmin.instances[1].name"));
         assertEquals("https://www.google.com", src.getValue("de.javahippie.mpadmin.instances[1].uri"));
     }
+
+    @Test
+    void nullValue() {
+        SmallRyeConfig config = new SmallRyeConfigBuilder()
+                .withSources(new YamlConfigSource("Yaml", "foo:\n"
+                        + "    - something\n"
+                        + "    - 1\n"
+                        + "    - true\n"
+                        + "    - ~\n"))
+                .build();
+
+        assertEquals("something,1,true", config.getRawValue("foo"));
+        assertEquals("something", config.getRawValue("foo[0]"));
+        assertEquals("1", config.getRawValue("foo[1]"));
+        assertEquals("true", config.getRawValue("foo[2]"));
+        assertNull(config.getRawValue("foo[3]"));
+        assertFalse(((Set<String>) config.getPropertyNames()).contains("foo[3]"));
+    }
 }
diff --git a/sources/yaml/src/test/java/io/smallrye/config/source/yaml/YamlConfigSourceTest.java b/sources/yaml/src/test/java/io/smallrye/config/source/yaml/YamlConfigSourceTest.java
@@ -4,7 +4,6 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -147,8 +146,8 @@ void propertyNames() throws Exception {
         assertTrue(propertyNames.contains("quarkus.http.port"));
         assertTrue(propertyNames.contains("quarkus.http.ssl-port"));
         assertTrue(propertyNames.contains("quarkus.http.ssl.protocols"));
-        assertFalse(propertyNames.contains("quarkus.http.ssl.protocols[0]"));
-        assertNull(config.getRawValue("quarkus.http.ssl.protocols[0]"));
+        assertTrue(propertyNames.contains("quarkus.http.ssl.protocols[0]"));
+        assertNotNull(config.getRawValue("quarkus.http.ssl.protocols[0]"));
     }
 
     @Test