ConfigBeanFactory: comprehensively list allowed types

We want to avoid ever getting a ClassCastException, instead we want a more helpful error. So write out all types explicitly. This also will make it easier to ensure that tests are comprehensive, and this commit adds a lot more tests. This commit now supports typesafe List<T>, and allows you to have fields of type Object, ConfigObject, Config, ConfigValue, and Map<String,Object>, so if you want to accept "anything" you can do that by specifying a vague type and then casting it yourself.
2025-01-28 21:20:07 +08:00 · 2015-02-27 20:30:39 -05:00 · 2015-02-27 20:30:39 -05:00 · b692e988a6
commit b692e988a6
parent 26eec7be90
10 changed files with 374 additions and 39 deletions
--- a/config/src/main/java/com/typesafe/config/impl/ConfigBeanImpl.java
+++ b/config/src/main/java/com/typesafe/config/impl/ConfigBeanImpl.java
@ -6,6 +6,8 @@ import java.beans.Introspector;
 import java.beans.PropertyDescriptor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@ -14,6 +16,8 @@ import java.util.concurrent.TimeUnit;
 import java.time.Duration;

 import com.typesafe.config.Config;
+import com.typesafe.config.ConfigObject;
+import com.typesafe.config.ConfigList;
 import com.typesafe.config.ConfigException;
 import com.typesafe.config.ConfigMemorySize;
 import com.typesafe.config.ConfigValue;
@ -68,6 +72,7 @@ public class ConfigBeanImpl {
            for (PropertyDescriptor beanProp : beanProps) {
                Method setter = beanProp.getWriteMethod();
                Class parameterClass = setter.getParameterTypes()[0];
+
                ConfigValueType expectedType = getValueTypeOrNull(parameterClass);
                if (expectedType != null) {
                    String name = originalNames.get(beanProp.getName());
@ -91,17 +96,9 @@ public class ConfigBeanImpl {
            T bean = clazz.newInstance();
            for (PropertyDescriptor beanProp : beanProps) {
                Method setter = beanProp.getWriteMethod();
-                ConfigValue configValue = configProps.get(beanProp.getName());
-                Object unwrapped;
-                if (configValue == null) {
-                    throw new ConfigException.Missing(beanProp.getName());
-                }
-                if (configValue instanceof SimpleConfigObject) {
-                    unwrapped = createInternal(config.getConfig(originalNames.get(beanProp.getDisplayName())), beanProp.getPropertyType());
-                } else {
-                    Class parameterClass = setter.getParameterTypes()[0];
-                    unwrapped = getValueWithAutoConversion(parameterClass, config, originalNames.get(beanProp.getDisplayName()));
-                }
+                Type parameterType = setter.getGenericParameterTypes()[0];
+                Class parameterClass = setter.getParameterTypes()[0];
+                Object unwrapped = getValue(clazz, parameterType, parameterClass, config, originalNames.get(beanProp.getName()));
                setter.invoke(bean, unwrapped);
            }
            return bean;
@ -114,7 +111,15 @@ public class ConfigBeanImpl {
        }
    }

-    private static Object getValueWithAutoConversion(Class parameterClass, Config config, String configPropName) {
+    // we could magically make this work in many cases by doing
+    // getAnyRef() (or getValue().unwrapped()), but anytime we
+    // rely on that, we aren't doing the type conversions Config
+    // usually does, and we will throw ClassCastException instead
+    // of a nicer error message giving the name of the bad
+    // setting. So, instead, we only support a limited number of
+    // types plus you can always use Object, ConfigValue, Config,
+    // ConfigObject, etc.  as an escape hatch.
+    private static Object getValue(Class beanClass, Type parameterType, Class parameterClass, Config config, String configPropName) {
        if (parameterClass == Boolean.class || parameterClass == boolean.class) {
            return config.getBoolean(configPropName);
        } else if (parameterClass == Integer.class || parameterClass == int.class) {
@ -129,12 +134,59 @@ public class ConfigBeanImpl {
            return config.getDuration(configPropName);
        } else if (parameterClass == ConfigMemorySize.class) {
            return config.getMemorySize(configPropName);
-        }
+        } else if (parameterClass == Object.class) {
+            return config.getAnyRef(configPropName);
+        } else if (parameterClass == List.class) {
+            Type elementType = ((ParameterizedType)parameterType).getActualTypeArguments()[0];

-        return config.getAnyRef(configPropName);
+            if (elementType == Boolean.class) {
+                return config.getBooleanList(configPropName);
+            } else if (elementType == Integer.class) {
+                return config.getIntList(configPropName);
+            } else if (elementType == Double.class) {
+                return config.getDoubleList(configPropName);
+            } else if (elementType == Long.class) {
+                return config.getLongList(configPropName);
+            } else if (elementType == String.class) {
+                return config.getStringList(configPropName);
+            } else if (elementType == Duration.class) {
+                return config.getDurationList(configPropName);
+            } else if (elementType == ConfigMemorySize.class) {
+                return config.getMemorySizeList(configPropName);
+            } else if (elementType == Object.class) {
+                return config.getAnyRefList(configPropName);
+            } else if (elementType == Config.class) {
+                return config.getConfigList(configPropName);
+            } else if (elementType == ConfigObject.class) {
+                return config.getObjectList(configPropName);
+            } else if (elementType == ConfigValue.class) {
+                return config.getList(configPropName);
+            } else {
+                throw new ConfigException.BadBean("Bean property '" + configPropName + "' of class " + beanClass.getName() + " has unsupported list element type " + elementType);
+            }
+        } else if (parameterClass == Map.class) {
+            // we could do better here, but right now we don't.
+            Type[] typeArgs = ((ParameterizedType)parameterType).getActualTypeArguments();
+            if (typeArgs[0] != String.class || typeArgs[1] != Object.class) {
+                throw new ConfigException.BadBean("Bean property '" + configPropName + "' of class " + beanClass.getName() + " has unsupported Map<" + typeArgs[0] + "," + typeArgs[1] + ">, only Map<String,Object> is supported right now");
+            }
+            return config.getObject(configPropName).unwrapped();
+        } else if (parameterClass == Config.class) {
+            return config.getConfig(configPropName);
+        } else if (parameterClass == ConfigObject.class) {
+            return config.getObject(configPropName);
+        } else if (parameterClass == ConfigValue.class) {
+            return config.getValue(configPropName);
+        } else if (parameterClass == ConfigList.class) {
+            return config.getList(configPropName);
+        } else if (hasAtLeastOneBeanProperty(parameterClass)) {
+            return createInternal(config.getConfig(configPropName), parameterClass);
+        } else {
+            throw new ConfigException.BadBean("Bean property " + configPropName + " of class " + beanClass.getName() + " has unsupported type " + parameterType);
+        }
    }

-    // null if we can't easily say
+    // null if we can't easily say; this is heuristic/best-effort
    private static ConfigValueType getValueTypeOrNull(Class<?> parameterClass) {
        if (parameterClass == Boolean.class || parameterClass == boolean.class) {
            return ConfigValueType.BOOLEAN;
@ -150,12 +202,35 @@ public class ConfigBeanImpl {
            return null;
        } else if (parameterClass == ConfigMemorySize.class) {
            return null;
-        } else if (parameterClass.isAssignableFrom(List.class)) {
+        } else if (parameterClass == List.class) {
            return ConfigValueType.LIST;
-        } else if (parameterClass.isAssignableFrom(Map.class)) {
+        } else if (parameterClass == Map.class) {
            return ConfigValueType.OBJECT;
+        } else if (parameterClass == Config.class) {
+            return ConfigValueType.OBJECT;
+        } else if (parameterClass == ConfigObject.class) {
+            return ConfigValueType.OBJECT;
+        } else if (parameterClass == ConfigList.class) {
+            return ConfigValueType.LIST;
        } else {
            return null;
        }
    }
+
+    private static boolean hasAtLeastOneBeanProperty(Class<?> clazz) {
+        BeanInfo beanInfo = null;
+        try {
+            beanInfo = Introspector.getBeanInfo(clazz);
+        } catch (IntrospectionException e) {
+            return false;
+        }
+
+        for (PropertyDescriptor beanProp : beanInfo.getPropertyDescriptors()) {
+            if (beanProp.getReadMethod() != null && beanProp.getWriteMethod() != null) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }
--- a/config/src/test/java/beanconfig/ArraysConfig.java
+++ b/config/src/test/java/beanconfig/ArraysConfig.java
@ -1,6 +1,12 @@
 package beanconfig;

 import java.util.List;
+import java.time.Duration;
+
+import com.typesafe.config.Config;
+import com.typesafe.config.ConfigMemorySize;
+import com.typesafe.config.ConfigObject;
+import com.typesafe.config.ConfigValue;

 public class ArraysConfig {

@ -8,11 +14,15 @@ public class ArraysConfig {
    List<Integer> ofInt;
    List<String> ofString;
    List<Double> ofDouble;
+    List<Long> ofLong;
    List<Object> ofNull;
    List<Boolean> ofBoolean;
-    List<List> ofArray;
    List<Object> ofObject;
-
+    List<Config> ofConfig;
+    List<ConfigObject> ofConfigObject;
+    List<ConfigValue> ofConfigValue;
+    List<Duration> ofDuration;
+    List<ConfigMemorySize> ofMemorySize;

    public List<Integer> getEmpty() {
        return empty;
@ -62,14 +72,6 @@ public class ArraysConfig {
        this.ofBoolean = ofBoolean;
    }

-    public List<List> getOfArray() {
-        return ofArray;
-    }
-
-    public void setOfArray(List<List> ofArray) {
-        this.ofArray = ofArray;
-    }
-
    public List<Object> getOfObject() {
        return ofObject;
    }
@ -77,4 +79,52 @@ public class ArraysConfig {
    public void setOfObject(List<Object> ofObject) {
        this.ofObject = ofObject;
    }
+
+    public List<Long> getOfLong() {
+        return ofLong;
+    }
+
+    public void setOfLong(List<Long> ofLong) {
+        this.ofLong = ofLong;
+    }
+
+    public List<Config> getOfConfig() {
+        return ofConfig;
+    }
+
+    public void setOfConfig(List<Config> ofConfig) {
+        this.ofConfig = ofConfig;
+    }
+
+    public List<ConfigObject> getOfConfigObject() {
+        return ofConfigObject;
+    }
+
+    public void setOfConfigObject(List<ConfigObject> ofConfigObject) {
+        this.ofConfigObject = ofConfigObject;
+    }
+
+    public List<ConfigValue> getOfConfigValue() {
+        return ofConfigValue;
+    }
+
+    public void setOfConfigValue(List<ConfigValue> ofConfigValue) {
+        this.ofConfigValue = ofConfigValue;
+    }
+
+    public List<Duration> getOfDuration() {
+        return ofDuration;
+    }
+
+    public void setOfDuration(List<Duration> ofDuration) {
+        this.ofDuration = ofDuration;
+    }
+
+    public List<ConfigMemorySize> getOfMemorySize() {
+        return ofMemorySize;
+    }
+
+    public void setOfMemorySize(List<ConfigMemorySize> ofMemorySize) {
+        this.ofMemorySize = ofMemorySize;
+    }
 }
--- a/config/src/test/java/beanconfig/BooleansConfig.java
+++ b/config/src/test/java/beanconfig/BooleansConfig.java
@ -7,7 +7,7 @@ public class BooleansConfig {
    Boolean falseVal;
    Boolean falseValAgain;
    Boolean on;
-    Boolean off;
+    boolean off;

    public Boolean getTrueVal() {
        return trueVal;
@ -49,11 +49,11 @@ public class BooleansConfig {
        this.on = on;
    }

-    public Boolean getOff() {
+    public boolean getOff() {
        return off;
    }

-    public void setOff(Boolean off) {
+    public void setOff(boolean off) {
        this.off = off;
    }
 }
--- a/config/src/test/java/beanconfig/NotABeanFieldConfig.java
+++ b/config/src/test/java/beanconfig/NotABeanFieldConfig.java
@ -0,0 +1,18 @@
+package beanconfig;
+
+public class NotABeanFieldConfig {
+
+    public static class NotABean {
+        int stuff;
+    }
+
+    private NotABean notBean;
+
+    public NotABean getNotBean() {
+        return notBean;
+    }
+
+    public void setNotBean(NotABean notBean) {
+        this.notBean = notBean;
+    }
+}
--- a/config/src/test/java/beanconfig/UnsupportedListElementConfig.java
+++ b/config/src/test/java/beanconfig/UnsupportedListElementConfig.java
@ -0,0 +1,16 @@
+package beanconfig;
+
+import java.net.URI;
+import java.util.List;
+
+public class UnsupportedListElementConfig {
+    private List<URI> uri;
+
+    public List<URI> getUri() {
+        return uri;
+    }
+
+    public void setUri(List<URI> uri) {
+        this.uri = uri;
+    }
+}
--- a/config/src/test/java/beanconfig/UnsupportedMapKeyConfig.java
+++ b/config/src/test/java/beanconfig/UnsupportedMapKeyConfig.java
@ -0,0 +1,16 @@
+package beanconfig;
+
+import java.util.Map;
+
+public class UnsupportedMapKeyConfig {
+    private Map<Integer, Object> map;
+
+    public Map<Integer, Object> getMap() {
+        return map;
+    }
+
+    public void setMap(Map<Integer, Object> map) {
+        this.map = map;
+    }
+
+}
--- a/config/src/test/java/beanconfig/UnsupportedMapValueConfig.java
+++ b/config/src/test/java/beanconfig/UnsupportedMapValueConfig.java
@ -0,0 +1,16 @@
+package beanconfig;
+
+import java.util.Map;
+
+public class UnsupportedMapValueConfig {
+    private Map<String, Integer> map;
+
+    public Map<String, Integer> getMap() {
+        return map;
+    }
+
+    public void setMap(Map<String, Integer> map) {
+        this.map = map;
+    }
+
+}
--- a/config/src/test/java/beanconfig/ValuesConfig.java
+++ b/config/src/test/java/beanconfig/ValuesConfig.java
@ -0,0 +1,68 @@
+package beanconfig;
+
+import java.util.Map;
+
+import com.typesafe.config.Config;
+import com.typesafe.config.ConfigList;
+import com.typesafe.config.ConfigObject;
+import com.typesafe.config.ConfigValue;
+
+// test bean for various "uncooked" values
+public class ValuesConfig {
+
+    Object obj;
+    Config config;
+    ConfigObject configObj;
+    ConfigValue configValue;
+    ConfigList list;
+    Map<String,Object> unwrappedMap;
+
+    public Object getObj() {
+        return obj;
+    }
+
+    public void setObj(Object obj) {
+        this.obj = obj;
+    }
+
+    public Config getConfig() {
+        return config;
+    }
+
+    public void setConfig(Config config) {
+        this.config = config;
+    }
+
+    public ConfigObject getConfigObj() {
+        return configObj;
+    }
+
+    public void setConfigObj(ConfigObject configObj) {
+        this.configObj = configObj;
+    }
+
+    public ConfigValue getConfigValue() {
+        return configValue;
+    }
+
+    public void setConfigValue(ConfigValue configValue) {
+        this.configValue = configValue;
+    }
+
+    public ConfigList getList() {
+        return list;
+    }
+
+    public void setList(ConfigList list) {
+        this.list = list;
+    }
+
+    public Map<String, Object> getUnwrappedMap() {
+        return unwrappedMap;
+    }
+
+    public void setUnwrappedMap(Map<String, Object> unwrappedMap) {
+        this.unwrappedMap = unwrappedMap;
+    }
+
+}
--- a/config/src/test/resources/beanconfig/beanconfig01.conf
+++ b/config/src/test/resources/beanconfig/beanconfig01.conf
@ -43,10 +43,16 @@
        "ofInt" : [1, 2, 3],
        "ofString" : [ ${strings.a}, ${strings.b}, ${strings.c} ],
        "of-double" : [3.14, 4.14, 5.14],
+        "of-long" : { "1" : 32, "2" : 42, "3" : 52 }, // object-to-list conversion
        "ofNull" : [null, null, null],
        "ofBoolean" : [true, false],
        "ofArray" : [${arrays.ofString}, ${arrays.ofString}, ${arrays.ofString}],
-        "ofObject" : [${numbers}, ${booleans}, ${strings}]
+        "ofObject" : [${numbers}, ${booleans}, ${strings}],
+        "ofConfig" : [${numbers}, ${booleans}, ${strings}],
+        "ofConfigObject" : [${numbers}, ${booleans}, ${strings}],
+        "ofConfigValue" : [1, 2, "a"],
+        "ofDuration" : [1, 2h, 3 days],
+        "ofMemorySize" : [1024, 1M, 1G]
    },
    "bytes" : {
        "kilobyte" : "1kB",
@ -68,5 +74,13 @@
      "fooBar" : "yes",
      "baz-bar" : "no",
      "bazBar" : "yes"
+    },
+    "values" : {
+      "obj" : 42,
+      "config" : ${strings},
+      "configObj" : ${numbers},
+      "configValue" : "hello world",
+      "list" : [1,2,3],
+      "unwrappedMap" : ${validation}
    }
 }
--- a/config/src/test/scala/com/typesafe/config/impl/ConfigBeanFactoryTest.scala
+++ b/config/src/test/scala/com/typesafe/config/impl/ConfigBeanFactoryTest.scala
@ -46,10 +46,10 @@ class ConfigBeanFactoryTest extends TestUtils {
            ConfigBeanFactory.create(config, classOf[ValidationBeanConfig])
        }

-        val expecteds = Seq(Missing("propNotListedInConfig", 61, "string"),
-            WrongType("shouldBeInt", 62, "number", "boolean"),
-            WrongType("should-be-boolean", 63, "boolean", "number"),
-            WrongType("should-be-list", 64, "list", "string"))
+        val expecteds = Seq(Missing("propNotListedInConfig", 67, "string"),
+            WrongType("shouldBeInt", 68, "number", "boolean"),
+            WrongType("should-be-boolean", 69, "boolean", "number"),
+            WrongType("should-be-list", 70, "list", "string"))

        checkValidationException(e, expecteds)
    }
@ -91,13 +91,26 @@ class ConfigBeanFactoryTest extends TestUtils {
        assertNotNull(beanConfig)
        assertEquals(List().asJava, beanConfig.getEmpty)
        assertEquals(List(1, 2, 3).asJava, beanConfig.getOfInt)
+        assertEquals(List(32L, 42L, 52L).asJava, beanConfig.getOfLong)
        assertEquals(List("a", "b", "c").asJava, beanConfig.getOfString)
-        assertEquals(List(List("a", "b", "c").asJava,
-            List("a", "b", "c").asJava,
-            List("a", "b", "c").asJava).asJava,
-            beanConfig.getOfArray)
+        //assertEquals(List(List("a", "b", "c").asJava,
+        //    List("a", "b", "c").asJava,
+        //    List("a", "b", "c").asJava).asJava,
+        //    beanConfig.getOfArray)
        assertEquals(3, beanConfig.getOfObject.size)
        assertEquals(3, beanConfig.getOfDouble.size)
+        assertEquals(3, beanConfig.getOfConfig.size)
+        assertTrue(beanConfig.getOfConfig.get(0).isInstanceOf[Config])
+        assertEquals(3, beanConfig.getOfConfigObject.size)
+        assertTrue(beanConfig.getOfConfigObject.get(0).isInstanceOf[ConfigObject])
+        assertEquals(List(intValue(1), intValue(2), stringValue("a")),
+            beanConfig.getOfConfigValue.asScala)
+        assertEquals(List(Duration.ofMillis(1), Duration.ofHours(2), Duration.ofDays(3)),
+            beanConfig.getOfDuration.asScala)
+        assertEquals(List(ConfigMemorySize.ofBytes(1024),
+            ConfigMemorySize.ofBytes(1048576),
+            ConfigMemorySize.ofBytes(1073741824)),
+            beanConfig.getOfMemorySize.asScala)
    }

    @Test
@ -127,6 +140,55 @@ class ConfigBeanFactoryTest extends TestUtils {
        assertEquals("yes", beanConfig.getBazBar)
    }

+    @Test
+    def testValues() {
+        val beanConfig = ConfigBeanFactory.create(loadConfig().getConfig("values"), classOf[ValuesConfig])
+        assertNotNull(beanConfig)
+        assertEquals(42, beanConfig.getObj())
+        assertEquals("abcd", beanConfig.getConfig.getString("abcd"))
+        assertEquals(3, beanConfig.getConfigObj.toConfig.getInt("intVal"))
+        assertEquals(stringValue("hello world"), beanConfig.getConfigValue)
+        assertEquals(List(1, 2, 3).map(intValue(_)), beanConfig.getList.asScala)
+        assertEquals(true, beanConfig.getUnwrappedMap.get("shouldBeInt"))
+        assertEquals(42, beanConfig.getUnwrappedMap.get("should-be-boolean"))
+    }
+
+    @Test
+    def testNotABeanField() {
+        val e = intercept[ConfigException.BadBean] {
+            ConfigBeanFactory.create(parseConfig("notBean=42"), classOf[NotABeanFieldConfig])
+        }
+        assertTrue("unsupported type error", e.getMessage.contains("unsupported type"))
+        assertTrue("error about the right property", e.getMessage.contains("notBean"))
+    }
+
+    @Test
+    def testUnsupportedListElement() {
+        val e = intercept[ConfigException.BadBean] {
+            ConfigBeanFactory.create(parseConfig("uri=[42]"), classOf[UnsupportedListElementConfig])
+        }
+        assertTrue("unsupported element type error", e.getMessage.contains("unsupported list element type"))
+        assertTrue("error about the right property", e.getMessage.contains("uri"))
+    }
+
+    @Test
+    def testUnsupportedMapKey() {
+        val e = intercept[ConfigException.BadBean] {
+            ConfigBeanFactory.create(parseConfig("map={}"), classOf[UnsupportedMapKeyConfig])
+        }
+        assertTrue("unsupported map type error", e.getMessage.contains("unsupported Map"))
+        assertTrue("error about the right property", e.getMessage.contains("'map'"))
+    }
+
+    @Test
+    def testUnsupportedMapValue() {
+        val e = intercept[ConfigException.BadBean] {
+            ConfigBeanFactory.create(parseConfig("map={}"), classOf[UnsupportedMapValueConfig])
+        }
+        assertTrue("unsupported map type error", e.getMessage.contains("unsupported Map"))
+        assertTrue("error about the right property", e.getMessage.contains("'map'"))
+    }
+
    private def loadConfig(): Config = {
        val configIs: InputStream = this.getClass().getClassLoader().getResourceAsStream("beanconfig/beanconfig01.conf")
        try {