Use map factory in getValues (#1097)

radcortez · radcortez · commit 7d7be30c0800 · 2024-02-06T16:28:14.000Z
diff --git a/cdi/src/main/java/io/smallrye/config/inject/ConfigProducerUtil.java b/cdi/src/main/java/io/smallrye/config/inject/ConfigProducerUtil.java
@@ -172,7 +172,7 @@ private static <T> Converter<T> resolveConverter(final Type type, final SmallRye
                 return (Converter<T>) newCollectionConverter(resolveConverter(typeArgs[0], config), HashSet::new);
             } else if (rawType == Map.class) {
                 return (Converter<T>) newMapConverter(resolveConverter(typeArgs[0], config),
-                        resolveConverter(typeArgs[1], config));
+                        resolveConverter(typeArgs[1], config), HashMap::new);
             } else if (rawType == Optional.class) {
                 return (Converter<T>) newOptionalConverter(resolveConverter(typeArgs[0], config));
             } else if (rawType == Supplier.class) {
diff --git a/implementation/src/main/java/io/smallrye/config/Converters.java b/implementation/src/main/java/io/smallrye/config/Converters.java
@@ -322,9 +322,15 @@ public static <A, T> Converter<A> newArrayConverter(Converter<? extends T> itemC
      * @param <V> the type of the values
      * @return the new converter (not {@code null})
      */
+    @Deprecated
     public static <K, V> Converter<Map<K, V>> newMapConverter(Converter<? extends K> keyConverter,
             Converter<? extends V> valueConverter) {
-        return new MapConverter<>(keyConverter, valueConverter);
+        return newMapConverter(keyConverter, valueConverter, HashMap::new);
+    }
+
+    public static <K, V> Converter<Map<K, V>> newMapConverter(Converter<? extends K> keyConverter,
+            Converter<? extends V> valueConverter, IntFunction<Map<K, V>> mapFactory) {
+        return new MapConverter<>(keyConverter, valueConverter, mapFactory);
     }
 
     /**
@@ -1037,25 +1043,31 @@ static class MapConverter<K, V> extends AbstractConverter<Map<K, V>> {
          * The converter to use the for values.
          */
         private final Converter<? extends V> valueConverter;
+        private final IntFunction<Map<K, V>> mapFactory;
 
         /**
          * Construct a {@code MapConverter} with the given converters.
          *
          * @param keyConverter the converter to use the for keys
          * @param valueConverter the converter to use the for values
+         * @param mapFactory
          */
-        MapConverter(Converter<? extends K> keyConverter, Converter<? extends V> valueConverter) {
+        MapConverter(
+                final Converter<? extends K> keyConverter,
+                final Converter<? extends V> valueConverter,
+                final IntFunction<Map<K, V>> mapFactory) {
             this.keyConverter = keyConverter;
             this.valueConverter = valueConverter;
+            this.mapFactory = mapFactory;
         }
 
         @Override
         public Map<K, V> convert(String value) throws IllegalArgumentException, NullPointerException {
             if (value == null) {
                 return null;
             }
-            final Map<K, V> map = new HashMap<>();
-            final StringBuilder currentLine = new StringBuilder(value.length());
+            Map<K, V> map = mapFactory.apply(0);
+            StringBuilder currentLine = new StringBuilder(value.length());
             int fromIndex = 0;
             for (int idx; (idx = value.indexOf(';', fromIndex)) >= 0; fromIndex = idx + 1) {
                 if (value.charAt(idx - 1) == '\\') {
@@ -1082,7 +1094,7 @@ public Map<K, V> convert(String value) throws IllegalArgumentException, NullPoin
          * @throws NoSuchElementException if the line could not be converted into an entry or doesn't have the expected format.
          */
         private void processLine(Map<K, V> map, String value, String rawLine) {
-            final String line = rawLine.replace("\\;", ";");
+            String line = rawLine.replace("\\;", ";");
             for (int idx, fromIndex = 0; (idx = line.indexOf('=', fromIndex)) >= 0; fromIndex = idx + 1) {
                 if (line.charAt(idx - 1) == '\\') {
                     // The key separator has been escaped
diff --git a/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java b/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java
@@ -215,7 +215,7 @@ public <K, V> Map<K, V> getValues(
             Converter<V> valueConverter,
             IntFunction<Map<K, V>> mapFactory) {
         try {
-            return getValue(name, newMapConverter(keyConverter, valueConverter));
+            return getValue(name, newMapConverter(keyConverter, valueConverter, mapFactory));
         } catch (NoSuchElementException e) {
             Map<String, String> mapKeys = getMapKeys(name);
             if (mapKeys.isEmpty()) {
@@ -246,7 +246,8 @@ public <K, V, C extends Collection<V>> Map<K, C> getValues(
             IntFunction<Map<K, C>> mapFactory,
             IntFunction<C> collectionFactory) {
         try {
-            return getValue(name, newMapConverter(keyConverter, newCollectionConverter(valueConverter, collectionFactory)));
+            return getValue(name,
+                    newMapConverter(keyConverter, newCollectionConverter(valueConverter, collectionFactory), mapFactory));
         } catch (NoSuchElementException e) {
             Map<String, String> mapCollectionKeys = getMapKeys(name);
             if (mapCollectionKeys.isEmpty()) {
@@ -463,7 +464,7 @@ public <K, V> Optional<Map<K, V>> getOptionalValues(String name, Converter<K> ke
 
     public <K, V> Optional<Map<K, V>> getOptionalValues(String name, Converter<K> keyConverter, Converter<V> valueConverter,
             IntFunction<Map<K, V>> mapFactory) {
-        Optional<Map<K, V>> optionalValue = getOptionalValue(name, newMapConverter(keyConverter, valueConverter));
+        Optional<Map<K, V>> optionalValue = getOptionalValue(name, newMapConverter(keyConverter, valueConverter, mapFactory));
         if (optionalValue.isPresent()) {
             return optionalValue;
         }
@@ -491,7 +492,7 @@ public <K, V, C extends Collection<V>> Optional<Map<K, C>> getOptionalValues(
             IntFunction<Map<K, C>> mapFactory,
             IntFunction<C> collectionFactory) {
         Optional<Map<K, C>> optionalValue = getOptionalValue(name,
-                newMapConverter(keyConverter, newCollectionConverter(valueConverter, collectionFactory)));
+                newMapConverter(keyConverter, newCollectionConverter(valueConverter, collectionFactory), mapFactory));
         if (optionalValue.isPresent()) {
             return optionalValue;
         }
diff --git a/implementation/src/test/java/io/smallrye/config/SmallRyeConfigTest.java b/implementation/src/test/java/io/smallrye/config/SmallRyeConfigTest.java
@@ -21,10 +21,12 @@
 import java.util.NoSuchElementException;
 import java.util.Optional;
 import java.util.Set;
+import java.util.TreeMap;
 import java.util.function.IntFunction;
 
 import org.eclipse.microprofile.config.Config;
 import org.eclipse.microprofile.config.spi.ConfigSource;
+import org.eclipse.microprofile.config.spi.Converter;
 import org.junit.jupiter.api.Test;
 
 import io.smallrye.config.common.AbstractConfigSource;
@@ -377,13 +379,22 @@ void getValuesMap() {
         assertEquals("value", map.get("key.nested"));
         assertEquals("value", map.get("key.quoted"));
 
+        Converter<String> stringConverter = config.requireConverter(String.class);
+        Map<String, String> treeMap = config.getValues("my.prop", stringConverter, stringConverter, t -> new TreeMap<>());
+        assertTrue(treeMap instanceof TreeMap);
+
         Optional<Map<String, String>> optionalMap = config.getOptionalValues("my.prop", String.class, String.class);
         assertTrue(optionalMap.isPresent());
         assertEquals(3, optionalMap.get().size());
         assertEquals("value", optionalMap.get().get("key"));
         assertEquals("value", optionalMap.get().get("key.nested"));
         assertEquals("value", optionalMap.get().get("key.quoted"));
 
+        Optional<Map<String, String>> optionalTreeMap = config.getOptionalValues("my.prop", stringConverter, stringConverter,
+                t -> new TreeMap<>());
+        assertTrue(optionalTreeMap.isPresent());
+        assertTrue(optionalTreeMap.get() instanceof TreeMap);
+
         assertTrue(config.getOptionalValues("my.optional", String.class, String.class).isEmpty());
     }
 
@@ -428,6 +439,11 @@ void getValuesMapList() {
         assertEquals("value", map.get("key.quoted").get(0));
         assertEquals("value", map.get("key.quoted").get(1));
 
+        Converter<String> stringConverter = config.requireConverter(String.class);
+        Map<String, List<String>> treeMap = config.getValues("my.prop", stringConverter, stringConverter, t -> new TreeMap<>(),
+                ArrayList::new);
+        assertTrue(treeMap instanceof TreeMap);
+
         Optional<Map<String, List<String>>> optionalMap = config.getOptionalValues("my.prop", String.class, String.class,
                 ArrayList::new);
         assertTrue(optionalMap.isPresent());
@@ -439,6 +455,11 @@ void getValuesMapList() {
         assertEquals("value", optionalMap.get().get("key.quoted").get(0));
         assertEquals("value", optionalMap.get().get("key.quoted").get(1));
 
+        Optional<Map<String, List<String>>> optionalTreeMap = config.getOptionalValues("my.prop", stringConverter,
+                stringConverter, t -> new TreeMap<>(), ArrayList::new);
+        assertTrue(optionalTreeMap.isPresent());
+        assertTrue(optionalTreeMap.get() instanceof TreeMap);
+
         assertTrue(config.getOptionalValues("my.optional", String.class, String.class, ArrayList::new).isEmpty());
     }
 
