diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index e69454e48cd0..8fe5ed161bb5 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -471,13 +471,6 @@ private static void applySimpleConfig() //InetAddressAndPort and get the right defaults InetAddressAndPort.initializeDefaultPort(getStoragePort()); - // below 2 checks are needed in order to match the pre-CASSANDRA-15234 upper bound for those parameters which were still in megabits per second - setProperty(ConfigFields.STREAM_THROUGHPUT_OUTBOUND, conf.stream_throughput_outbound); - setProperty(ConfigFields.INTER_DC_STREAM_THROUGHPUT_OUTBOUND, conf.inter_dc_stream_throughput_outbound); - setProperty(ConfigFields.ENTIRE_SSTABLE_STREAM_THROUGHPUT_OUTBOUND, conf.entire_sstable_stream_throughput_outbound); - setProperty(ConfigFields.ENTIRE_SSTABLE_INTER_DC_STREAM_THROUGHPUT_OUTBOUND, conf.entire_sstable_inter_dc_stream_throughput_outbound); - setProperty(ConfigFields.COMPACTION_THROUGHPUT, conf.compaction_throughput); - if (conf.auto_snapshot_ttl != null) { try @@ -606,7 +599,6 @@ else if (conf.disk_access_mode == Config.DiskAccessMode.mmap_index_only) conf.repair_session_max_tree_depth = 20; } - setProperty(ConfigFields.REPAIR_SESSION_SPACE, conf.repair_session_space); checkForLowestAcceptedTimeouts(conf); long valueInBytes = conf.native_transport_max_frame_size.toBytes(); diff --git a/src/java/org/apache/cassandra/config/ValidatedBy.java b/src/java/org/apache/cassandra/config/ValidatedBy.java index 18752ba9951e..26756d10e283 100644 --- a/src/java/org/apache/cassandra/config/ValidatedBy.java +++ b/src/java/org/apache/cassandra/config/ValidatedBy.java @@ -19,6 +19,7 @@ package org.apache.cassandra.config; import java.lang.annotation.ElementType; +import java.lang.annotation.Repeatable; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; @@ -29,6 +30,7 @@ */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.FIELD) +@Repeatable(ValidatedByList.class) public @interface ValidatedBy { String useClass(); diff --git a/src/java/org/apache/cassandra/config/YamlConfigurationLoader.java b/src/java/org/apache/cassandra/config/YamlConfigurationLoader.java index a1a720fbef23..bad050f749c1 100644 --- a/src/java/org/apache/cassandra/config/YamlConfigurationLoader.java +++ b/src/java/org/apache/cassandra/config/YamlConfigurationLoader.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import javax.annotation.Nullable; import com.google.common.annotations.VisibleForTesting; @@ -45,6 +46,8 @@ import org.yaml.snakeyaml.TypeDescription; import org.yaml.snakeyaml.Yaml; import org.yaml.snakeyaml.composer.Composer; +import org.yaml.snakeyaml.constructor.BaseConstructor; +import org.yaml.snakeyaml.constructor.Constructor; import org.yaml.snakeyaml.constructor.CustomClassLoaderConstructor; import org.yaml.snakeyaml.constructor.SafeConstructor; import org.yaml.snakeyaml.error.YAMLException; @@ -52,7 +55,11 @@ import org.yaml.snakeyaml.introspector.MissingProperty; import org.yaml.snakeyaml.introspector.Property; import org.yaml.snakeyaml.introspector.PropertyUtils; +import org.yaml.snakeyaml.nodes.MappingNode; import org.yaml.snakeyaml.nodes.Node; +import org.yaml.snakeyaml.nodes.NodeId; +import org.yaml.snakeyaml.nodes.NodeTuple; +import org.yaml.snakeyaml.nodes.ScalarNode; import org.yaml.snakeyaml.nodes.Tag; import org.yaml.snakeyaml.representer.Represent; import org.yaml.snakeyaml.representer.Representer; @@ -137,8 +144,7 @@ public Config loadConfig(URL url) throws ConfigurationException Map, Map> replacements = getNameReplacements(Config.class); verifyReplacements(replacements, configBytes); PropertiesChecker propertiesChecker = new PropertiesChecker(); - Yaml yaml = YamlFactory.instance.newYamlInstance(new CustomConstructor(Config.class, Yaml.class.getClassLoader()), - propertiesChecker); + Yaml yaml = YamlFactory.instance.newYamlInstance(new ConfigWithValidationConstructor(Config.class, Yaml.class.getClassLoader()), propertiesChecker); Config result = loadConfig(yaml, configBytes); propertiesChecker.check(); maybeAddSystemProperties(result); @@ -218,7 +224,7 @@ public static T fromMap(Map map, Class klass) @SuppressWarnings("unchecked") //getSingleData returns Object, not T public static T fromMap(Map map, boolean shouldCheck, Class klass) { - SafeConstructor constructor = new YamlConfigurationLoader.CustomConstructor(klass, klass.getClassLoader()); + SafeConstructor constructor = new ConfigWithValidationConstructor(klass, klass.getClassLoader()); Map, Map> replacements = getNameReplacements(Config.class); verifyReplacements(replacements, map); YamlConfigurationLoader.PropertiesChecker propertiesChecker = new PropertiesChecker(); @@ -242,7 +248,7 @@ public Node getSingleNode() public static T updateFromMap(Map map, boolean shouldCheck, T obj) { Class klass = (Class) obj.getClass(); - SafeConstructor constructor = new YamlConfigurationLoader.CustomConstructor(klass, klass.getClassLoader()) + SafeConstructor constructor = new ConfigWithValidationConstructor(klass, klass.getClassLoader()) { @Override protected Object newInstance(Node node) @@ -255,7 +261,6 @@ protected Object newInstance(Node node) Map, Map> replacements = getNameReplacements(Config.class); verifyReplacements(replacements, map); YamlConfigurationLoader.PropertiesChecker propertiesChecker = new PropertiesChecker(); - constructor.setPropertyUtils(propertiesChecker); Yaml yaml = YamlFactory.instance.newYamlInstance(constructor, propertiesChecker); Node node = yaml.represent(map); constructor.setComposer(new Composer(null, null) @@ -272,20 +277,104 @@ public Node getSingleNode() return value; } + /** + * For the configuration properties that are not mentioned in the configuration yaml file, we need additionally + * trigger the validation methods for the remaining properties to make sure the default values are valid and set. + * These validation methods are configured with {@link ValidatedBy} for each property. This will create + * a {@link MappingNode} with default values obtained from the {@link Config} class intance to finalize the + * configuration instance creation. + */ @VisibleForTesting - static class CustomConstructor extends CustomClassLoaderConstructor + static class ConfigWithValidationConstructor extends CustomClassLoaderConstructor { - CustomConstructor(Class theRoot, ClassLoader classLoader) + private final Class theRoot; + + public ConfigWithValidationConstructor(Class theRoot, ClassLoader classLoader) { super(theRoot, classLoader); + this.theRoot = theRoot; + yamlClassConstructors.put(NodeId.mapping, new CassandraMappingConstructor()); + } + + private class CassandraMappingConstructor extends Constructor.ConstructMapping + { + @Override + protected Object constructJavaBean2ndStep(MappingNode loadedYamlNode, Object object) + { + Object result = super.constructJavaBean2ndStep(loadedYamlNode, object); - TypeDescription seedDesc = new TypeDescription(ParameterizedClass.class); - seedDesc.putMapPropertyType("parameters", String.class, String.class); - addTypeDescription(seedDesc); + // If the node is not the root node, we don't need to handle the validation. + if (!loadedYamlNode.getTag().equals(rootTag)) + return result; - TypeDescription memtableDesc = new TypeDescription(Config.MemtableOptions.class); - memtableDesc.addPropertyParameters("configurations", String.class, InheritingClass.class); - addTypeDescription(memtableDesc); + assert theRoot.isInstance(result); + Representer representer = YamlFactory.representer(new PropertyUtils() + { + private final Loader loader = Properties.defaultLoader(); + + @Override + protected Map getPropertiesMap(Class type, BeanAccess bAccess) + { + Map result = loader.getProperties(type); + // Filter out properties that are not validated by any method. + return result.entrySet() + .stream() + .filter(e -> { + Property property = e.getValue(); + return property.getAnnotation(ValidatedBy.class) != null && + (property.getAnnotation(ValidatedByList.class) == null || + property.getAnnotation(ValidatedByList.class).value().length == 0); + }) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + } + }); + + // Load default values from the Config class instance for the properties that are not mentioned + // in the configuration yaml file. + MappingNode allWithDefauls = (MappingNode) new Yaml(representer).represent(object); + allWithDefauls.setType(theRoot); + removeIfLoadedFromYaml(loadedYamlNode, allWithDefauls); + + // This will trigger the validation and default values set for the remaining properties + // that annotated with @ValidatedBy. + return super.constructJavaBean2ndStep(allWithDefauls, result); + } + + private void removeIfLoadedFromYaml(MappingNode loadedYamlNode, MappingNode all) + { + all.getValue().removeIf(nodeTuple -> { + Node valueNode = nodeTuple.getValueNode(); + String key = mappingKeyHandleReplacements((ScalarNode) nodeTuple.getKeyNode(), all.getType()); + Node mappingNodeOrig = mappingNodeByKey(loadedYamlNode, key); + + if (valueNode instanceof MappingNode && mappingNodeOrig instanceof MappingNode) + removeIfLoadedFromYaml((MappingNode) mappingNodeOrig, (MappingNode) valueNode); + + return mappingNodeOrig != null; + }); + } + + private String mappingKeyHandleReplacements(ScalarNode keyNode, Class rootType) + { + keyNode.setType(String.class); + String key = (String) constructObject(keyNode); + List replacements = Replacements.getReplacements(rootType); + for (Replacement replacement : replacements) + { + if (replacement.oldName.equals(key)) + return replacement.newName; + } + return key; + } + + private Node mappingNodeByKey(MappingNode node, String key) + { + return node.getValue().stream() + .filter(t -> mappingKeyHandleReplacements((ScalarNode) t.getKeyNode(), node.getType()).equals(key)) + .findFirst() + .map(NodeTuple::getValueNode) + .orElse(null); + } } @Override @@ -325,8 +414,6 @@ private static class PropertiesChecker extends PropertyUtils private final Loader loader = Properties.withReplacementsLoader(Properties.validatedPropertyLoader()); private final Set missingProperties = new HashSet<>(); - private final Set nullProperties = new HashSet<>(); - private final Set deprecationWarnings = new HashSet<>(); PropertiesChecker() @@ -360,7 +447,7 @@ public void set(Object object, Object value) throws Exception { // TODO: CASSANDRA-17785, add @Nullable to all nullable Config properties and remove value == null if (value == null && get(object) != null && !allowsNull) - nullProperties.add(getName()); + throw new ConfigurationException("Invalid yaml. The property '" + result.getName() + "' can't be null", false); result.set(object, value); } @@ -405,9 +492,6 @@ private Property getNestedProperty(Class type, String name) public void check() throws ConfigurationException { - if (!nullProperties.isEmpty()) - throw new ConfigurationException("Invalid yaml. Those properties " + nullProperties + " are not valid", false); - if (!missingProperties.isEmpty()) throw new ConfigurationException("Invalid yaml. Please remove properties " + missingProperties + " from your cassandra.yaml", false); @@ -441,15 +525,25 @@ public Yaml newYamlInstance(Class root) PropertyUtils propertyUtils = new PropertyUtils(); propertyUtils.setBeanAccess(BeanAccess.FIELD); propertyUtils.setAllowReadOnlyProperties(true); - return newYamlInstance(new CustomConstructor(root, Config.class.getClassLoader()), propertyUtils); + return newYamlInstance(new ConfigWithValidationConstructor(root, Config.class.getClassLoader()), propertyUtils); } - public Yaml newYamlInstance(SafeConstructor constructor, PropertyUtils propertyUtils) + public Yaml newYamlInstance(BaseConstructor constructor, PropertyUtils propertyUtils) { return create(constructor, propertyUtils); } - private static Yaml create(SafeConstructor constructor, PropertyUtils propertyUtils) + private static Yaml create(BaseConstructor constructor, PropertyUtils propertyUtils) + { + constructor.setPropertyUtils(propertyUtils); + Yaml yaml = new Yaml(constructor, representer(propertyUtils)); + + scalarCassandraTypes.forEach(yaml::addTypeDescription); + javaBeanCassandraTypes.forEach(yaml::addTypeDescription); + return yaml; + } + + private static Representer representer(PropertyUtils propertyUtils) { DumperOptions options = new DumperOptions(); options.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK); @@ -458,12 +552,7 @@ private static Yaml create(SafeConstructor constructor, PropertyUtils propertyUt options.setPrettyFlow(true); // to remove brackets around Representer representer = new CassandraRepresenter(scalarCassandraTypes, options); representer.setPropertyUtils(propertyUtils); - constructor.setPropertyUtils(propertyUtils); - Yaml yaml = new Yaml(constructor, representer); - - scalarCassandraTypes.forEach(yaml::addTypeDescription); - javaBeanCassandraTypes.forEach(yaml::addTypeDescription); - return yaml; + return representer; } private static List loadScalarTypeDescriptions() diff --git a/test/conf/cassandra-validatedby-cases.yaml b/test/conf/cassandra-validatedby-cases.yaml new file mode 100644 index 000000000000..670fc7fa0032 --- /dev/null +++ b/test/conf/cassandra-validatedby-cases.yaml @@ -0,0 +1,15 @@ +# +# Some configuration options are validated by the methods used in the @ValidatedBy annotation. +# +sstable_preemptive_open_interval: +credentials_update_interval: ~ + +# Warning! This is for test the backward compatibility of the configuration stream_throughput_outbound. +stream_throughput_outbound_megabits_per_sec: 200000000 + +# Null value must be handled by the validation method and the corresponding default value must be set correctly. +repair_session_space: null + +# non-default values are validated by the methods used in the @ValidatedBy annotation and loaded. +snapshot_links_per_second: 10 +compaction_throughput: 48MiB/s \ No newline at end of file diff --git a/test/conf/cassandra-validatedby-null-case.yaml b/test/conf/cassandra-validatedby-null-case.yaml new file mode 100644 index 000000000000..ac9a2fb4e6d4 --- /dev/null +++ b/test/conf/cassandra-validatedby-null-case.yaml @@ -0,0 +1,3 @@ +# Null value are not allowed for the configuration property as it is not annotated by @Nullable, +# so the validation method must throw an exception. +entire_sstable_inter_dc_stream_throughput_outbound: \ No newline at end of file diff --git a/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java b/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java index b49aeedfe38b..5228be83e3f0 100644 --- a/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java +++ b/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java @@ -102,6 +102,10 @@ public class DatabaseDescriptorRefTest "org.apache.cassandra.config.Config$PaxosVariant", "org.apache.cassandra.config.Config$RepairCommandPoolFullStrategy", "org.apache.cassandra.config.Config$SSTableConfig", + "org.apache.cassandra.config.Config$SSTableConfigBeanInfo", + "org.apache.cassandra.config.Config$SSTableConfigBeanInfo", + "org.apache.cassandra.config.Config$SSTableConfigCustomizer", + "org.apache.cassandra.config.Config$SSTableConfigCustomizer", "org.apache.cassandra.config.Config$UserFunctionTimeoutPolicy", "org.apache.cassandra.config.ConfigBeanInfo", "org.apache.cassandra.config.ConfigCustomizer", @@ -179,6 +183,10 @@ public class DatabaseDescriptorRefTest "org.apache.cassandra.config.DurationSpec$IntSecondsBoundCustomizer", "org.apache.cassandra.config.DurationSpec$IntSecondsBoundCustomizer", "org.apache.cassandra.config.DurationSpec$LongMicrosecondsBound", + "org.apache.cassandra.config.DurationSpec$LongMicrosecondsBoundBeanInfo", + "org.apache.cassandra.config.DurationSpec$LongMicrosecondsBoundBeanInfo", + "org.apache.cassandra.config.DurationSpec$LongMicrosecondsBoundCustomizer", + "org.apache.cassandra.config.DurationSpec$LongMicrosecondsBoundCustomizer", "org.apache.cassandra.config.DurationSpec$LongMillisecondsBound", "org.apache.cassandra.config.DurationSpec$LongMillisecondsBoundBeanInfo", "org.apache.cassandra.config.DurationSpec$LongMillisecondsBoundBeanInfo", @@ -215,8 +223,8 @@ public class DatabaseDescriptorRefTest "org.apache.cassandra.config.GuardrailsOptions$TableProperties", "org.apache.cassandra.config.InheritingClass", "org.apache.cassandra.config.ListenableProperty", - "org.apache.cassandra.config.ListenableProperty$Listener", "org.apache.cassandra.config.ListenableProperty$Invoker", + "org.apache.cassandra.config.ListenableProperty$Listener", "org.apache.cassandra.config.ListenableProperty$PropertyValidateInvoker", "org.apache.cassandra.config.ListenableProperty$Remover", "org.apache.cassandra.config.Loader", @@ -255,7 +263,10 @@ public class DatabaseDescriptorRefTest "org.apache.cassandra.config.TransparentDataEncryptionOptionsCustomizer", "org.apache.cassandra.config.TransparentDataEncryptionOptionsCustomizer", "org.apache.cassandra.config.ValidatedBy", + "org.apache.cassandra.config.ValidatedByList", "org.apache.cassandra.config.ValidatedPropertyLoader", + "org.apache.cassandra.config.ValidatedPropertyLoader$MethodInvokeAdapter", + "org.apache.cassandra.config.ValidatedPropertyLoader$MethodInvoker", "org.apache.cassandra.config.ValidatedPropertyLoader$ValidationPropertyHandler", "org.apache.cassandra.config.WithReplacementLoader", "org.apache.cassandra.config.YamlConfigurationLoader", @@ -393,7 +404,7 @@ public class DatabaseDescriptorRefTest "org.apache.cassandra.utils.concurrent.RefCounted", "org.apache.cassandra.utils.concurrent.SelfRefCounted", "org.apache.cassandra.utils.concurrent.Transactional", - "org.apache.cassandra.utils.concurrent.UncheckedInterruptedException" + "org.apache.cassandra.utils.concurrent.UncheckedInterruptedException", }; static final Set checkedClasses = new HashSet<>(Arrays.asList(validClasses)); diff --git a/test/unit/org/apache/cassandra/config/LoadOldYAMLBackwardCompatibilityTest.java b/test/unit/org/apache/cassandra/config/LoadOldYAMLBackwardCompatibilityTest.java index 8554ed4b2050..4275c5ea9c41 100644 --- a/test/unit/org/apache/cassandra/config/LoadOldYAMLBackwardCompatibilityTest.java +++ b/test/unit/org/apache/cassandra/config/LoadOldYAMLBackwardCompatibilityTest.java @@ -58,7 +58,7 @@ public void testConfigurationLoaderBackwardCompatibility() assertEquals(new DurationSpec.LongMillisecondsBound(500), config.slow_query_log_timeout); assertNull(config.memtable_heap_space); assertNull(config.memtable_offheap_space); - assertNull( config.repair_session_space); + assertEquals(new DataStorageSpec.IntMebibytesBound("910MiB"), config.repair_session_space); assertEquals(new DataStorageSpec.IntBytesBound(4194304), config.internode_application_send_queue_capacity); assertEquals(new DataStorageSpec.IntBytesBound(134217728), config.internode_application_send_queue_reserve_endpoint_capacity); assertEquals(new DataStorageSpec.IntBytesBound(536870912), config.internode_application_send_queue_reserve_global_capacity); diff --git a/test/unit/org/apache/cassandra/config/ParseAndConvertUnitsTest.java b/test/unit/org/apache/cassandra/config/ParseAndConvertUnitsTest.java index a21f23552455..650fb2e09640 100644 --- a/test/unit/org/apache/cassandra/config/ParseAndConvertUnitsTest.java +++ b/test/unit/org/apache/cassandra/config/ParseAndConvertUnitsTest.java @@ -79,7 +79,7 @@ public void testConfigurationLoaderParser() //Confirm space parameters were successfully parsed with the default values in cassandra.yaml assertNull(config.memtable_heap_space); assertNull(config.memtable_offheap_space); - assertNull(config.repair_session_space); //null everywhere so should be correct, let's check whether it will bomb + assertEquals(new DataStorageSpec.IntMebibytesBound("910MiB"), config.repair_session_space); // The value is set on pre-parse to 910MiB assertEquals(new DataStorageSpec.IntBytesBound(4194304), config.internode_application_send_queue_capacity); assertEquals(new DataStorageSpec.IntBytesBound(134217728), config.internode_application_send_queue_reserve_endpoint_capacity); assertEquals(new DataStorageSpec.IntBytesBound(536870912), config.internode_application_send_queue_reserve_global_capacity); diff --git a/test/unit/org/apache/cassandra/config/ValidatedPropertyLoaderTest.java b/test/unit/org/apache/cassandra/config/ValidatedPropertyLoaderTest.java index e251bbd99a13..0358c854e84d 100644 --- a/test/unit/org/apache/cassandra/config/ValidatedPropertyLoaderTest.java +++ b/test/unit/org/apache/cassandra/config/ValidatedPropertyLoaderTest.java @@ -32,11 +32,19 @@ public class ValidatedPropertyLoaderTest @Test public void testListenablePropertyMethodPattern() throws Exception { - assertThatThrownBy(() -> LOADER.getProperties(TestConfig.class) - .get("test_property") - .set(new TestConfig(), 42)) + Property property = LOADER.getProperties(TestConfig.class).get("test_property"); + + assertThatThrownBy(() -> property.set(new TestConfig(), 42)) .hasRootCauseInstanceOf(ConfigurationException.class) - .hasRootCauseMessage("Value must not be equal to 42"); + .hasRootCauseMessage("Value for the property 'test_property' must not be equal to 42"); + + assertThatThrownBy(() -> property.set(new TestConfig(), 41)) + .hasRootCauseInstanceOf(ConfigurationException.class) + .hasRootCauseMessage("TestConfig instance TestConfig{}, the value for the property 'test_property' must not be equal to 41"); + + assertThatThrownBy(() -> property.set(new TestConfig(), 40)) + .hasRootCauseInstanceOf(ConfigurationException.class) + .hasRootCauseMessage("Value must not be equal to 40"); } @Test @@ -53,11 +61,25 @@ public void testListenablePropertyNotExists() throws Exception } @Test - public void testListenablePropertySet() throws Exception + public void testListenablePropertyMigthtBeSet() throws Exception { TestConfig conf = new TestConfig(); - LOADER.getProperties(TestConfig.class).get("test_property").set(conf, 40); - assertThat(conf.test_property).isEqualTo(40); + LOADER.getProperties(TestConfig.class).get("test_property").set(conf, 39); + assertThat(conf.test_property).isEqualTo(39); + } + + @Test + public void testListenablePropertyHandler() throws Exception + { + TestConfig conf = new TestConfig(); + Property property = LOADER.getProperties(TestConfig.class).get("test_string_property"); + assertThatThrownBy(() -> property.set(conf, "test")) + .hasRootCauseInstanceOf(ConfigurationException.class) + .hasRootCauseMessage("Value must be null"); + + property.set(conf, null); + assertThat(property.get(conf)).isEqualTo("test_string_property_value"); + assertThat(conf.test_string_property).isEqualTo("test_string_property_value"); } @SuppressWarnings("unchecked") @@ -90,13 +112,57 @@ public Integer before(TestConfig source, String name, Integer oldValue, Integer public static class TestConfig { - @ValidatedBy(useClass = "org.apache.cassandra.config.ValidatedPropertyLoaderTest$TestConfig", useClassMethod = "validateTestPropertyInvalid") + @ValidatedBy(useClass = "org.apache.cassandra.config.ValidatedPropertyLoaderTest$TestConfig", useClassMethod = "validateTestProperty1Arguments") + @ValidatedBy(useClass = "org.apache.cassandra.config.ValidatedPropertyLoaderTest$TestConfig", useClassMethod = "validateTestProperty2Arguments") + @ValidatedBy(useClass = "org.apache.cassandra.config.ValidatedPropertyLoaderTest$TestConfig", useClassMethod = "validateTestProperty3Arguments") public Integer test_property = 0; - public static void validateTestPropertyInvalid(TestConfig conf, Integer value) + @ValidatedBy(useClass = "org.apache.cassandra.config.ValidatedPropertyLoaderTest$TestConfig", useClassMethod = "handleTestStringProperty") + public String test_string_property = null; + + public NestedTestConfig nested_test_config = new NestedTestConfig(); + + public static void validateTestProperty1Arguments(Integer value) + { + if (value == 40) + throw new ConfigurationException("Value must not be equal to 40"); + } + + public static void validateTestProperty2Arguments(String name, Integer value) { if (value == 42) - throw new ConfigurationException("Value must not be equal to 42"); + throw new ConfigurationException("Value for the property '" + name + "' must not be equal to 42"); + } + + public static void validateTestProperty3Arguments(TestConfig conf, String name, Integer value) + { + if (value == 41) + throw new ConfigurationException("TestConfig instance " + conf + ", the value for the property '" + name + "' must not be equal to 41"); + } + + public static String handleTestStringProperty(String value) + { + if (value == null) + return "test_string_property_value"; + throw new ConfigurationException("Value must be null"); + } + + @Override + public String toString() + { + return "TestConfig{}"; + } + } + + public static class NestedTestConfig + { + @ValidatedBy(useClass = "org.apache.cassandra.config.ValidatedPropertyLoaderTest$NestedTestConfig", useClassMethod = "validateTestProperty1Arguments") + public String good_string_to_go_with = "good_string_to_go_with"; + + public static void validateTestProperty1Arguments(String value) + { + if (value.length() > 10) + throw new ConfigurationException("String value must be greater than 10 characters"); } } } \ No newline at end of file diff --git a/test/unit/org/apache/cassandra/config/YamlConfigurationLoaderTest.java b/test/unit/org/apache/cassandra/config/YamlConfigurationLoaderTest.java index 6ba76e0f9f15..7fc1759204b4 100644 --- a/test/unit/org/apache/cassandra/config/YamlConfigurationLoaderTest.java +++ b/test/unit/org/apache/cassandra/config/YamlConfigurationLoaderTest.java @@ -41,6 +41,7 @@ import static org.apache.cassandra.config.YamlConfigurationLoader.SYSTEM_PROPERTY_PREFIX; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.catchThrowableOfType; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -124,6 +125,31 @@ public void readConvertersSpecialCasesFromConfig() assertThat(c.cache_load_timeout).isEqualTo(new DurationSpec.IntSecondsBound("0s")); } + @Test + public void testValidationWithConfiguredMethodsConfig() + { + Config c = load("test/conf/cassandra-validatedby-cases.yaml"); + assertThat(c.snapshot_links_per_second).isEqualTo(10); + assertThat(c.compaction_throughput).isEqualTo(new DataRateSpec.LongBytesPerSecondBound("48MiB/s")); + assertThat(c.repair_session_space).isNotNull() + .isEqualTo(new DataStorageSpec.IntMebibytesBound("910MiB")); + assertThat(c.stream_throughput_outbound).isEqualTo(new DataRateSpec.LongBytesPerSecondBound("25000000000000B/s")); + assertThat(c.sstable_preemptive_open_interval).isNull(); + assertThat(c.credentials_update_interval).isNull(); + } + + @Test + public void testValidationIfNullIsSetConfig() throws Exception + { + assertThat(catchThrowableOfType(() -> load("test/conf/cassandra-validatedby-null-case.yaml"), + ConfigurationException.class)) + .extracting(Throwable::getCause) + .extracting(Throwable::getCause) + .isInstanceOf(ConfigurationException.class) + .extracting(Throwable::getMessage) + .isEqualTo("Invalid yaml. The property 'entire_sstable_inter_dc_stream_throughput_outbound' can't be null"); + } + @Test public void readConvertersSpecialCasesFromMap() {