fix ConfigListOption values is empty

Change-Id: Ifff6198d18488864ad65f33209a3187056675b7b
apache · Dec 29, 2019 · 00f08c8 · 00f08c8
1 parent a082d4b
commit 00f08c8
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 21 deletions.
diff --git a/src/main/java/com/baidu/hugegraph/config/ConfigListConvOption.java b/src/main/java/com/baidu/hugegraph/config/ConfigListConvOption.java
@@ -36,8 +36,7 @@ public class ConfigListConvOption<T, R> extends TypedOption<List<T>, List<R>> {
     public ConfigListConvOption(String name, String desc,
                                 Predicate<List<T>> pred, Function<T, R> convert,
                                 T... values) {
-        this(name, false, desc, pred, convert,
-             (Class<T>) values[0].getClass(), Arrays.asList(values));
+        this(name, false, desc, pred, convert, null, Arrays.asList(values));
     }
 
     @SuppressWarnings("unchecked")
@@ -46,8 +45,11 @@ public ConfigListConvOption(String name, boolean required, String desc,
                                 Class<T> clazz, List<T> values) {
         super(name, required, desc, pred,
               (Class<List<T>>) values.getClass(), values);
-        E.checkNotNull(clazz, "element class");
         E.checkNotNull(convert, "convert");
+        if (clazz == null && values.size() > 0) {
+            clazz = (Class<T>) values.get(0).getClass();
+        }
+        E.checkArgumentNotNull(clazz, "Element class can't be null");
         this.elemClass = clazz;
         this.converter = convert;
     }

diff --git a/src/main/java/com/baidu/hugegraph/config/ConfigListOption.java b/src/main/java/com/baidu/hugegraph/config/ConfigListOption.java
@@ -34,8 +34,7 @@ public class ConfigListOption<T> extends ConfigOption<List<T>> {
     @SuppressWarnings("unchecked")
     public ConfigListOption(String name, String desc,
                             Predicate<List<T>> pred, T... values) {
-        this(name, false, desc, pred,
-             (Class<T>) values[0].getClass(), Arrays.asList(values));
+        this(name, false, desc, pred, null, Arrays.asList(values));
     }
 
     @SuppressWarnings("unchecked")
@@ -44,6 +43,9 @@ public ConfigListOption(String name, boolean required, String desc,
                             List<T> values) {
         super(name, required, desc, pred,
               (Class<List<T>>) values.getClass(), values);
+        if (clazz == null && values.size() > 0) {
+            clazz = (Class<T>) values.get(0).getClass();
+        }
         E.checkArgumentNotNull(clazz, "Element class can't be null");
         this.elemClass = clazz;
     }

diff --git a/src/main/java/com/baidu/hugegraph/config/OptionSpace.java b/src/main/java/com/baidu/hugegraph/config/OptionSpace.java
@@ -21,6 +21,7 @@
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.util.Collections;
 import java.util.Map;
 import java.util.Set;
@@ -64,6 +65,9 @@ public static void register(String module, String holder) {
         Exception exception = null;
         try {
             Method method = clazz.getMethod(INSTANCE_METHOD);
+            if (!Modifier.isStatic(method.getModifiers())) {
+                throw new NoSuchMethodException(INSTANCE_METHOD);
+            }
             instance = (OptionHolder) method.invoke(null);
             if (instance == null) {
                 exception = new ConfigException(

diff --git a/src/test/java/com/baidu/hugegraph/unit/config/HugeConfigTest.java b/src/test/java/com/baidu/hugegraph/unit/config/HugeConfigTest.java
@@ -202,6 +202,40 @@ public void testOptionWithError() {
                     11D
             );
         });
+
+        Assert.assertThrows(ConfigException.class, () -> {
+            new ConfigListOption<>(
+                    "group1.list",
+                    "description of list with invalid default values",
+                    disallowEmpty()
+            );
+        });
+
+        Assert.assertThrows(IllegalArgumentException.class, () -> {
+            new ConfigListOption<>(
+                    "group1.list",
+                    "description of list with invalid default values",
+                    null
+            );
+        });
+
+        Assert.assertThrows(ConfigException.class, () -> {
+            new ConfigListConvOption<String, WeekDay>(
+                    "group1.list_conv",
+                    "description of list_conv with invalid default values",
+                    disallowEmpty(),
+                    s -> WeekDay.valueOf(s)
+            );
+        });
+
+        Assert.assertThrows(IllegalArgumentException.class, () -> {
+            new ConfigListConvOption<String, WeekDay>(
+                    "group1.list_conv",
+                    "description of list_conv with invalid default values",
+                    null,
+                    s -> WeekDay.valueOf(s)
+            );
+        });
     }
 
     @Test

diff --git a/src/test/java/com/baidu/hugegraph/unit/config/OptionSpaceTest.java b/src/test/java/com/baidu/hugegraph/unit/config/OptionSpaceTest.java
@@ -39,13 +39,13 @@ public void tesRegister() {
 
         OptionSpace.register("testgroup1", OptionHolder1.class.getName());
         Assert.assertEquals(oldSize + 2, OptionSpace.keys().size());
-        Assert.assertTrue(OptionSpace.containKey("group1.text1"));
-        Assert.assertTrue(OptionSpace.containKey("group1.text2"));
+        Assert.assertTrue(OptionSpace.containKey("testgroup1.text1"));
+        Assert.assertTrue(OptionSpace.containKey("testgroup1.text2"));
 
         OptionSpace.register("testgroup1", new OptionHolder1());
         Assert.assertEquals(oldSize + 2, OptionSpace.keys().size());
-        Assert.assertTrue(OptionSpace.containKey("group1.text1"));
-        Assert.assertTrue(OptionSpace.containKey("group1.text2"));
+        Assert.assertTrue(OptionSpace.containKey("testgroup1.text1"));
+        Assert.assertTrue(OptionSpace.containKey("testgroup1.text2"));
 
         OptionSpace.register("testgroup2", OptionHolder2.class.getName());
         Assert.assertEquals(oldSize + 4, OptionSpace.keys().size());
@@ -75,22 +75,11 @@ public void testRegisterWithError() {
             OptionSpace.register("test-error", Exception.class.getName());
         });
 
-        class OptionHolderWithoutInstance extends OptionHolder {
-            // no instance()
-        }
         Assert.assertThrows(ConfigException.class, () -> {
             OptionSpace.register("test-error",
                                  OptionHolderWithoutInstance.class.getName());
         });
 
-        class OptionHolderWithNonStaticInstance extends OptionHolder {
-
-            // not static instance()
-            @SuppressWarnings("unused")
-            public OptionHolderWithNonStaticInstance instance() {
-                return new OptionHolderWithNonStaticInstance();
-            }
-        }
         Assert.assertThrows(ConfigException.class, () -> {
             OptionSpace.register("test-error", OptionHolderWithNonStaticInstance
                                                .class.getName());
@@ -112,6 +101,18 @@ public OptionHolderWithNonStaticInstance instance() {
         });
     }
 
+    public static class OptionHolderWithoutInstance extends OptionHolder {
+        // no instance()
+    }
+
+    public static class OptionHolderWithNonStaticInstance extends OptionHolder {
+
+        // not static instance()
+        public OptionHolderWithNonStaticInstance instance() {
+            return new OptionHolderWithNonStaticInstance();
+        }
+    }
+
     public static class OptionHolderWithInstanceNull extends OptionHolder {
 
         public static OptionHolderWithInstanceNull instance() {
@@ -132,7 +133,7 @@ public static OptionHolderWithInvalidOption instance() {
             return new OptionHolderWithInvalidOption();
         }
 
-        OptionHolderWithInvalidOption() {
+        private OptionHolderWithInvalidOption() {
             this.registerOptions();
         }