diff --git a/src/main/java/com/baidu/hugegraph/config/OptionHolder.java b/src/main/java/com/baidu/hugegraph/config/OptionHolder.java index 6b5c398d9..e5c5be6a6 100644 --- a/src/main/java/com/baidu/hugegraph/config/OptionHolder.java +++ b/src/main/java/com/baidu/hugegraph/config/OptionHolder.java @@ -40,6 +40,10 @@ public OptionHolder() { protected void registerOptions() { for (Field field : this.getClass().getFields()) { + if (!TypedOption.class.isAssignableFrom(field.getType())) { + // Skip if not option + continue; + } try { TypedOption option = (TypedOption) field.get(this); // Fields of subclass first, don't overwrite by superclass diff --git a/src/main/java/com/baidu/hugegraph/config/OptionSpace.java b/src/main/java/com/baidu/hugegraph/config/OptionSpace.java index 485fb81d2..9b2aef85f 100644 --- a/src/main/java/com/baidu/hugegraph/config/OptionSpace.java +++ b/src/main/java/com/baidu/hugegraph/config/OptionSpace.java @@ -21,12 +21,14 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.Collections; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import org.slf4j.Logger; +import com.baidu.hugegraph.util.E; import com.baidu.hugegraph.util.Log; public final class OptionSpace { @@ -63,6 +65,11 @@ public static void register(String module, String holder) { try { Method method = clazz.getMethod(INSTANCE_METHOD); instance = (OptionHolder) method.invoke(null); + if (instance == null) { + exception = new ConfigException( + "Returned null from %s() method", + INSTANCE_METHOD); + } } catch (NoSuchMethodException e) { LOG.warn("Class {} does not has static method {}.", holder, INSTANCE_METHOD); @@ -91,6 +98,7 @@ public static void register(String module, OptionHolder holder) { LOG.warn("Already registered option holder: {} ({})", module, holders.get(module)); } + E.checkArgumentNotNull(holder, "OptionHolder can't be null"); holders.put(module, holder.getClass()); options.putAll(holder.options()); LOG.debug("Registered options for OptionHolder: {}", @@ -98,10 +106,10 @@ public static void register(String module, OptionHolder holder) { } public static Set keys() { - return options.keySet(); + return Collections.unmodifiableSet(options.keySet()); } - public static Boolean containKey(String key) { + public static boolean containKey(String key) { return options.containsKey(key); } diff --git a/src/main/java/com/baidu/hugegraph/config/TypedOption.java b/src/main/java/com/baidu/hugegraph/config/TypedOption.java index 182bcaf2b..57d77c6fa 100644 --- a/src/main/java/com/baidu/hugegraph/config/TypedOption.java +++ b/src/main/java/com/baidu/hugegraph/config/TypedOption.java @@ -150,17 +150,21 @@ protected Object parse(Object value, Class dataType) { protected void check(Object value) { E.checkNotNull(value, "value", this.name); - E.checkArgument(this.dataType.isInstance(value), - "Invalid type of value '%s' for option '%s', " + - "expect type %s but got %s", value, this.name, - this.dataType.getSimpleName(), - value.getClass().getSimpleName()); + if (!this.dataType.isInstance(value)) { + throw new ConfigException( + "Invalid type of value '%s' for option '%s', " + + "expect type %s but got %s", value, this.name, + this.dataType.getSimpleName(), + value.getClass().getSimpleName()); + } + if (this.checkFunc != null) { @SuppressWarnings("unchecked") T result = (T) value; - E.checkArgument(this.checkFunc.apply(result), - "Invalid option value for '%s': %s", - this.name, value); + if (!this.checkFunc.apply(result)) { + throw new ConfigException("Invalid option value for '%s': %s", + this.name, value); + } } } diff --git a/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java b/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java index a8d597a6e..4d292d626 100644 --- a/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java +++ b/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java @@ -24,9 +24,10 @@ import com.baidu.hugegraph.testutil.AssertTest; import com.baidu.hugegraph.testutil.WhiteboxTest; -import com.baidu.hugegraph.unit.concurrent.RowLockTest; import com.baidu.hugegraph.unit.concurrent.LockGroupTest; +import com.baidu.hugegraph.unit.concurrent.RowLockTest; import com.baidu.hugegraph.unit.config.HugeConfigTest; +import com.baidu.hugegraph.unit.config.OptionSpaceTest; import com.baidu.hugegraph.unit.date.SafeDateFormatTest; import com.baidu.hugegraph.unit.event.EventHubTest; import com.baidu.hugegraph.unit.iterator.ExtendableIteratorTest; @@ -60,6 +61,7 @@ LockGroupTest.class, RowLockTest.class, HugeConfigTest.class, + OptionSpaceTest.class, SafeDateFormatTest.class, EventHubTest.class, PerfUtilTest.class, diff --git a/src/test/java/com/baidu/hugegraph/unit/config/HugeConfigTest.java b/src/test/java/com/baidu/hugegraph/unit/config/HugeConfigTest.java index 7e2242aaa..b3dbcaade 100644 --- a/src/test/java/com/baidu/hugegraph/unit/config/HugeConfigTest.java +++ b/src/test/java/com/baidu/hugegraph/unit/config/HugeConfigTest.java @@ -28,6 +28,7 @@ import static com.baidu.hugegraph.config.OptionChecker.rangeInt; import java.util.Arrays; +import java.util.List; import org.apache.commons.configuration.Configuration; import org.apache.commons.configuration.PropertiesConfiguration; @@ -35,6 +36,7 @@ import org.junit.Test; import com.baidu.hugegraph.config.ConfigConvOption; +import com.baidu.hugegraph.config.ConfigException; import com.baidu.hugegraph.config.ConfigListConvOption; import com.baidu.hugegraph.config.ConfigListOption; import com.baidu.hugegraph.config.ConfigOption; @@ -44,12 +46,15 @@ import com.baidu.hugegraph.testutil.Assert; import com.baidu.hugegraph.testutil.Whitebox; import com.baidu.hugegraph.unit.BaseUnitTest; +import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; public class HugeConfigTest extends BaseUnitTest { - private static final String CONF = - "src/test/java/com/baidu/hugegraph/unit/config/test.conf"; + private static final String PATH = + "src/test/java/com/baidu/hugegraph/unit/config/"; + private static final String CONF = PATH + "test.conf"; @BeforeClass public static void init() { @@ -83,6 +88,113 @@ public void testOptionsToString() { TestSubOptions.textsub.toString()); } + @Test + public void testOptionDataType() { + Assert.assertEquals(String.class, TestOptions.text1.dataType()); + Assert.assertEquals(Integer.class, TestOptions.int1.dataType()); + Assert.assertEquals(Long.class, TestOptions.long1.dataType()); + Assert.assertEquals(Float.class, TestOptions.float1.dataType()); + Assert.assertEquals(Double.class, TestOptions.double1.dataType()); + Assert.assertEquals(Boolean.class, TestOptions.bool.dataType()); + + Assert.assertEquals(List.class, TestOptions.list.dataType()); + Assert.assertEquals(List.class, TestOptions.map.dataType()); + + Assert.assertEquals(String.class, TestOptions.weekday.dataType()); + Assert.assertEquals(List.class, TestOptions.weekdays.dataType()); + } + + @Test + public void testOptionDesc() { + Assert.assertEquals("description of group1.text1", + TestOptions.text1.desc()); + Assert.assertEquals("description of group1.text2 sub", + TestSubOptions.text2.desc()); + } + + @Test + public void testOptionRequired() { + Assert.assertEquals(false, TestOptions.text1.required()); + Assert.assertEquals(true, TestSubOptions.text2.required()); + } + + @Test + public void testOptionWithError() { + Assert.assertThrows(ConfigException.class, () -> { + new ConfigOption<>( + "group1.text", + "description of group1.text", + disallowEmpty(), + "" + ); + }); + + Assert.assertThrows(ConfigException.class, () -> { + new ConfigListOption<>( + "group1.list", + true, + "description of group1.list", + disallowEmpty(), + String.class, + ImmutableList.of() + ); + }); + + Assert.assertThrows(ConfigException.class, () -> { + new ConfigOption<>( + "group1.int", + "description of group1.int", + positiveInt(), + 0 + ); + }); + + Assert.assertThrows(ConfigException.class, () -> { + new ConfigOption<>( + "group1.int", + "description of group1.int", + nonNegativeInt(), + -1 + ); + }); + + Assert.assertThrows(ConfigException.class, () -> { + new ConfigOption<>( + "group1.long", + "description of group1.long", + rangeInt(1L, 100L), + 0L + ); + }); + + Assert.assertThrows(ConfigException.class, () -> { + new ConfigOption<>( + "group1.long", + "description of group1.long", + rangeInt(1L, 100L), + 101L + ); + }); + + Assert.assertThrows(ConfigException.class, () -> { + new ConfigOption<>( + "group1.double", + "description of group1.double", + rangeDouble(1D, 10D), + 0D + ); + }); + + Assert.assertThrows(ConfigException.class, () -> { + new ConfigOption<>( + "group1.double", + "description of group1.double", + rangeDouble(1D, 10D), + 11D + ); + }); + } + @Test public void testHugeConfig() throws Exception { Configuration conf = new PropertiesConfiguration(); @@ -170,6 +282,36 @@ public void testHugeConfigWithOverride() throws Exception { config.get(TestSubOptions.textsub)); } + @Test + public void testHugeConfigWithTypeError() { + OptionSpace.register("test-type-error", + TestOptionsWithTypeError.class.getName()); + + Assert.assertThrows(ConfigException.class, () -> { + new HugeConfig(PATH + "test-type-error.conf"); + }); + } + + @Test + public void testHugeConfigWithCheckError() throws Exception { + OptionSpace.register("test-check-error", + TestOptionsWithCheckError.class.getName()); + + Assert.assertThrows(ConfigException.class, () -> { + new HugeConfig(PATH + "test-check-error.conf"); + }); + } + + @Test + public void testHugeConfigWithListOptionError() throws Exception { + OptionSpace.register("test-list-error", + TestOptionsWithListError.class.getName()); + + Assert.assertThrows(IllegalStateException.class, () -> { + new HugeConfig(PATH + "test-list-error.conf"); + }); + } + public static class TestOptions extends OptionHolder { private static volatile TestOptions instance; @@ -304,8 +446,10 @@ public static class TestSubOptions extends TestOptions { public static final ConfigOption text2 = new ConfigOption<>( "group1.text2", - "description of group1.text2", + true, + "description of group1.text2 sub", disallowEmpty(), + String.class, "text2-value-override" ); @@ -318,6 +462,87 @@ public static class TestSubOptions extends TestOptions { ); } + public static class TestOptionsWithTypeError extends OptionHolder { + + private static volatile TestOptionsWithTypeError instance; + + public static synchronized TestOptionsWithTypeError instance() { + if (instance == null) { + instance = new TestOptionsWithTypeError(); + instance.registerOptions(); + } + return instance; + } + + public static final ConfigOption intError = + new ConfigOption<>( + "group1.int_type_error", + "description of group1.int_type_error", + rangeInt(1, 100), + 1 + ); + } + + public static class TestOptionsWithCheckError extends OptionHolder { + + private static volatile TestOptionsWithCheckError instance; + + public static synchronized TestOptionsWithCheckError instance() { + if (instance == null) { + instance = new TestOptionsWithCheckError(); + instance.registerOptions(); + } + return instance; + } + + public static final ConfigOption intError = + new ConfigOption<>( + "group1.int_check_error", + "description of group1.int_check_error", + rangeInt(1, 100), + 1 + ); + } + + public static class TestOptionsWithListError extends OptionHolder { + + private static volatile TestOptionsWithListError instance; + + public static synchronized TestOptionsWithListError instance() { + if (instance == null) { + instance = new TestOptionsWithListError(); + instance.registerOptions(); + } + return instance; + } + + public static final InvalidConfigListOption listError = + new InvalidConfigListOption<>( + "group1.list_for_list_error", + "description of group1.list_for_list_error", + disallowEmpty(), + 1 + ); + + static class InvalidConfigListOption extends ConfigOption> { + + @SuppressWarnings("unchecked") + public InvalidConfigListOption(String name, String desc, + Predicate> pred, + T... values) { + super(name, false, desc, pred, + (Class>) Arrays.asList(values).getClass(), + Arrays.asList(values)); + } + + @Override + protected boolean forList() { + return false; + } + } + } + + public enum WeekDay { SUNDAY, MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY, SATURDAY; diff --git a/src/test/java/com/baidu/hugegraph/unit/config/OptionSpaceTest.java b/src/test/java/com/baidu/hugegraph/unit/config/OptionSpaceTest.java new file mode 100644 index 000000000..f0cf8457f --- /dev/null +++ b/src/test/java/com/baidu/hugegraph/unit/config/OptionSpaceTest.java @@ -0,0 +1,214 @@ +/* + * Copyright 2017 HugeGraph Authors + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package com.baidu.hugegraph.unit.config; + +import static com.baidu.hugegraph.config.OptionChecker.disallowEmpty; + +import org.junit.Test; + +import com.baidu.hugegraph.config.ConfigException; +import com.baidu.hugegraph.config.ConfigOption; +import com.baidu.hugegraph.config.OptionHolder; +import com.baidu.hugegraph.config.OptionSpace; +import com.baidu.hugegraph.testutil.Assert; +import com.baidu.hugegraph.unit.BaseUnitTest; +import com.google.common.base.Predicate; + +public class OptionSpaceTest extends BaseUnitTest { + + @Test + public void tesRegister() { + OptionSpace.register("test-group1", OptionHolder1.class.getName()); + Assert.assertEquals(2, OptionSpace.keys().size()); + Assert.assertTrue(OptionSpace.containKey("group1.text1")); + Assert.assertTrue(OptionSpace.containKey("group1.text2")); + + OptionSpace.register("test-group1", new OptionHolder1()); + Assert.assertEquals(2, OptionSpace.keys().size()); + Assert.assertTrue(OptionSpace.containKey("group1.text1")); + Assert.assertTrue(OptionSpace.containKey("group1.text2")); + + OptionSpace.register("test-group2", OptionHolder2.class.getName()); + Assert.assertEquals(4, OptionSpace.keys().size()); + + Assert.assertTrue(OptionSpace.containKey("group1.text1")); + Assert.assertTrue(OptionSpace.containKey("group1.text2")); + Assert.assertTrue(OptionSpace.containKey("group2.text1")); + Assert.assertTrue(OptionSpace.containKey("group2.text2")); + + Assert.assertEquals("text1 value", + OptionSpace.get("group1.text1").defaultValue()); + Assert.assertEquals("text2 value", + OptionSpace.get("group1.text2").defaultValue()); + Assert.assertEquals("text1 value", + OptionSpace.get("group2.text1").defaultValue()); + Assert.assertEquals("text2 value", + OptionSpace.get("group2.text2").defaultValue()); + } + + @Test + public void testRegisterWithError() { + Assert.assertThrows(ConfigException.class, () -> { + OptionSpace.register("test-error", "fake"); + }); + + Assert.assertThrows(ConfigException.class, () -> { + 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()); + }); + + Assert.assertThrows(ConfigException.class, () -> { + OptionSpace.register("test-error", + OptionHolderWithInstanceNull.class.getName()); + }); + + Assert.assertThrows(ConfigException.class, () -> { + OptionSpace.register("test-error", + OptionHolderWithInstanceThrow.class.getName()); + }); + + Assert.assertThrows(ConfigException.class, () -> { + OptionSpace.register("test-error", + OptionHolderWithInvalidOption.class.getName()); + }); + } + + public static class OptionHolderWithInstanceNull extends OptionHolder { + + public static OptionHolderWithInstanceNull instance() { + return null; + } + } + + public static class OptionHolderWithInstanceThrow extends OptionHolder { + + public static OptionHolderWithInstanceNull instance() { + throw new RuntimeException("test error"); + } + } + + public static class OptionHolderWithInvalidOption extends OptionHolder { + + public static OptionHolderWithInvalidOption instance() { + return new OptionHolderWithInvalidOption(); + } + + OptionHolderWithInvalidOption() { + this.registerOptions(); + } + + public static final String fake = "fake"; + + public static final ConfigOption invalid = + new InvalidOption<>( + "group1.text1", + "description of group1.text1", + disallowEmpty(), + "value" + ); + + public static class InvalidOption extends ConfigOption { + + public InvalidOption(String name, String desc, + Predicate pred, T value) { + super(name, desc, pred, value); + } + + @Override + public String name() { + throw new RuntimeException("fake"); + } + } + } + + public static class OptionHolder1 extends OptionHolder { + + public static OptionHolder1 instance() { + return new OptionHolder1(); + } + + OptionHolder1() { + this.registerOptions(); + } + + public static final ConfigOption text1 = + new ConfigOption<>( + "group1.text1", + "description of group1.text1", + disallowEmpty(), + "text1 value" + ); + + public static final ConfigOption text2 = + new ConfigOption<>( + "group1.text2", + "description of group1.text2", + disallowEmpty(), + "text2 value" + ); + } + + public static class OptionHolder2 extends OptionHolder { + + public static OptionHolder2 instance() { + return new OptionHolder2(); + } + + OptionHolder2() { + this.registerOptions(); + } + + public static final ConfigOption text1 = + new ConfigOption<>( + "group2.text1", + "description of group2.text1", + disallowEmpty(), + "text1 value" + ); + + public static final ConfigOption text2 = + new ConfigOption<>( + "group2.text2", + "description of group2.text2", + disallowEmpty(), + "text2 value" + ); + } +} diff --git a/src/test/java/com/baidu/hugegraph/unit/config/test-check-error.conf b/src/test/java/com/baidu/hugegraph/unit/config/test-check-error.conf new file mode 100644 index 000000000..53aec1846 --- /dev/null +++ b/src/test/java/com/baidu/hugegraph/unit/config/test-check-error.conf @@ -0,0 +1 @@ +group1.int_check_error=101 diff --git a/src/test/java/com/baidu/hugegraph/unit/config/test-list-error.conf b/src/test/java/com/baidu/hugegraph/unit/config/test-list-error.conf new file mode 100644 index 000000000..aa57b3bf0 --- /dev/null +++ b/src/test/java/com/baidu/hugegraph/unit/config/test-list-error.conf @@ -0,0 +1 @@ +group1.list_for_list_error=[1,2,3] \ No newline at end of file diff --git a/src/test/java/com/baidu/hugegraph/unit/config/test-type-error.conf b/src/test/java/com/baidu/hugegraph/unit/config/test-type-error.conf new file mode 100644 index 000000000..ac70d4891 --- /dev/null +++ b/src/test/java/com/baidu/hugegraph/unit/config/test-type-error.conf @@ -0,0 +1 @@ +group1.int_type_error=string