diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index c8ab6b4b0691..0f41a7e6b7a2 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -703,6 +703,10 @@ public boolean stop() { @Override @DB public String updateConfiguration(final long userId, final String name, final String category, String value, final String scope, final Long resourceId) { + if (Boolean.class == getConfigurationTypeWrapperClass(name)) { + value = value.toLowerCase(); + } + final String validationMsg = validateConfigurationValue(name, value, scope); if (validationMsg != null) { logger.error("Invalid value [{}] for configuration [{}] due to [{}].", value, name, validationMsg); @@ -1241,18 +1245,9 @@ protected String validateConfigurationValue(String name, String value, String sc return "Invalid scope id provided for the parameter " + name; } } - Class type; - Config configuration = Config.getConfig(name); - if (configuration == null) { - logger.warn("Did not find configuration " + name + " in Config.java. Perhaps moved to ConfigDepot"); - ConfigKey configKey = _configDepot.get(name); - if(configKey == null) { - logger.warn("Did not find configuration " + name + " in ConfigDepot too."); - return null; - } - type = configKey.type(); - } else { - type = configuration.getType(); + Class type = getConfigurationTypeWrapperClass(name); + if (type == null) { + return null; } validateSpecificConfigurationValues(name, value, type); @@ -1262,7 +1257,28 @@ protected String validateConfigurationValue(String name, String value, String sc return String.format("Value [%s] is not a valid [%s].", value, type); } - return validateValueRange(name, value, type, configuration); + return validateValueRange(name, value, type, Config.getConfig(name)); + } + + /** + * Returns the configuration type's wrapper class. + * @param name name of the configuration. + * @return if the configuration exists, returns its type's wrapper class; if not, returns null. + */ + protected Class getConfigurationTypeWrapperClass(String name) { + Config configuration = Config.getConfig(name); + if (configuration != null) { + return configuration.getType(); + } + + logger.warn("Did not find configuration [{}] in Config.java. Perhaps moved to ConfigDepot.", name); + ConfigKey configKey = _configDepot.get(name); + if (configKey == null) { + logger.warn("Did not find configuration [{}] in ConfigDepot too.", name); + return null; + } + + return configKey.type(); } protected void validateConfigurationAllowedOnlyForDefaultAdmin(String configName, String value) { @@ -1299,7 +1315,7 @@ protected void validateConfigurationAllowedOnlyForDefaultAdmin(String configName * @@ -8176,11 +8192,17 @@ public ConfigKey[] getConfigKeys() { }; } + + /** + * Returns a string representing the specified configuration's type. + * @param configName name of the configuration. + * @return if the configuration exists, returns its type; if not, returns {@link Configuration.ValueType#String}. + */ @Override public String getConfigurationType(final String configName) { final ConfigurationVO cfg = _configDao.findByName(configName); if (cfg == null) { - logger.warn("Configuration " + configName + " not found"); + logger.warn("Configuration [{}] not found", configName); return Configuration.ValueType.String.name(); } @@ -8188,24 +8210,14 @@ public String getConfigurationType(final String configName) { return Configuration.ValueType.Range.name(); } - Class type = null; - final Config c = Config.getConfig(configName); - if (c == null) { - logger.warn("Configuration " + configName + " no found. Perhaps moved to ConfigDepot"); - final ConfigKey configKey = _configDepot.get(configName); - if (configKey == null) { - logger.warn("Couldn't find configuration " + configName + " in ConfigDepot too."); - return Configuration.ValueType.String.name(); - } - type = configKey.type(); - } else { - type = c.getType(); - } - - return getInputType(type, cfg); + Class type = getConfigurationTypeWrapperClass(configName); + return parseConfigurationTypeIntoString(type, cfg); } - private String getInputType(Class type, ConfigurationVO cfg) { + /** + * Parses a configuration type's wrapper class into its string representation. + */ + protected String parseConfigurationTypeIntoString(Class type, ConfigurationVO cfg) { if (type == null) { return Configuration.ValueType.String.name(); } diff --git a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java index 1309842b706d..227f66dd72f3 100644 --- a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java +++ b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.configuration; +import com.cloud.alert.AlertManager; import com.cloud.capacity.dao.CapacityDao; import com.cloud.dc.DataCenterVO; import com.cloud.dc.VlanVO; @@ -52,6 +53,7 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.NetUtils; import com.cloud.vm.dao.VMInstanceDao; +import org.apache.cloudstack.acl.RoleService; import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.api.command.admin.network.CreateNetworkOfferingCmd; import org.apache.cloudstack.api.command.admin.offering.UpdateDiskOfferingCmd; @@ -65,6 +67,7 @@ import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO; import org.apache.cloudstack.vm.UnmanagedVMsManager; +import org.apache.cloudstack.config.Configuration; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -897,4 +900,113 @@ public void testValidateConfigurationAllowedOnlyForDefaultAdmin_withValidConfigN configurationManagerImplSpy.validateConfigurationAllowedOnlyForDefaultAdmin(AccountManagerImpl.listOfRoleTypesAllowedForOperationsOfSameRoleType.key(), invalidValue); } } + + + @Test + public void getConfigurationTypeWrapperClassTestReturnsConfigType() { + Config configuration = Config.AlertEmailAddresses; + + Assert.assertEquals(configuration.getType(), configurationManagerImplSpy.getConfigurationTypeWrapperClass(configuration.key())); + } + + @Test + public void getConfigurationTypeWrapperClassTestReturnsConfigKeyType() { + String configurationName = "configuration.name"; + + Mockito.when(configDepot.get(configurationName)).thenReturn(configKeyMock); + Mockito.when(configKeyMock.type()).thenReturn(Integer.class); + + Assert.assertEquals(Integer.class, configurationManagerImplSpy.getConfigurationTypeWrapperClass(configurationName)); + } + + @Test + public void getConfigurationTypeWrapperClassTestReturnsNullWhenConfigurationDoesNotExist() { + String configurationName = "configuration.name"; + + Mockito.when(configDepot.get(configurationName)).thenReturn(null); + Assert.assertNull(configurationManagerImplSpy.getConfigurationTypeWrapperClass(configurationName)); + } + + @Test + public void parseConfigurationTypeIntoStringTestReturnsStringWhenTypeIsNull() { + Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(null, null)); + } + + @Test + public void parseConfigurationTypeIntoStringTestReturnsStringWhenTypeIsStringAndConfigurationKindIsNull() { + Mockito.when(configurationVOMock.getKind()).thenReturn(null); + Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(String.class, configurationVOMock)); + } + + @Test + public void parseConfigurationTypeIntoStringTestReturnsKindWhenTypeIsStringAndKindIsNotNull() { + Mockito.when(configurationVOMock.getKind()).thenReturn(ConfigKey.Kind.CSV.name()); + Assert.assertEquals(ConfigKey.Kind.CSV.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(String.class, configurationVOMock)); + } + + @Test + public void parseConfigurationTypeIntoStringTestReturnsKindWhenTypeIsCharacterAndKindIsNotNull() { + Mockito.when(configurationVOMock.getKind()).thenReturn(ConfigKey.Kind.CSV.name()); + Assert.assertEquals(ConfigKey.Kind.CSV.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Character.class, configurationVOMock)); + } + + @Test + public void parseConfigurationTypeIntoStringTestReturnsNumberWhenTypeIsInteger() { + Assert.assertEquals(Configuration.ValueType.Number.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Integer.class, configurationVOMock)); + } + + @Test + public void parseConfigurationTypeIntoStringTestReturnsNumberWhenTypeIsLong() { + Assert.assertEquals(Configuration.ValueType.Number.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Long.class, configurationVOMock)); + } + + @Test + public void parseConfigurationTypeIntoStringTestReturnsNumberWhenTypeIsShort() { + Assert.assertEquals(Configuration.ValueType.Number.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Short.class, configurationVOMock)); + } + + @Test + public void parseConfigurationTypeIntoStringTestReturnsDecimalWhenTypeIsFloat() { + Assert.assertEquals(Configuration.ValueType.Decimal.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Float.class, configurationVOMock)); + } + + @Test + public void parseConfigurationTypeIntoStringTestReturnsDecimalWhenTypeIsDouble() { + Assert.assertEquals(Configuration.ValueType.Decimal.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Double.class, configurationVOMock)); + } + + @Test + public void parseConfigurationTypeIntoStringTestReturnsBooleanWhenTypeIsBoolean() { + Assert.assertEquals(Configuration.ValueType.Boolean.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Boolean.class, configurationVOMock)); + } + + @Test + public void parseConfigurationTypeIntoStringTestReturnsStringWhenTypeDoesNotMatchAnyAvailableType() { + Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Object.class, configurationVOMock)); + } + + @Test + public void getConfigurationTypeTestReturnsStringWhenConfigurationDoesNotExist() { + Mockito.when(configDao.findByName(Mockito.anyString())).thenReturn(null); + Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.getConfigurationType(Mockito.anyString())); + } + + @Test + public void getConfigurationTypeTestReturnsRangeForConfigurationsThatAcceptIntervals() { + String configurationName = AlertManager.CPUCapacityThreshold.key(); + + Mockito.when(configDao.findByName(configurationName)).thenReturn(configurationVOMock); + Assert.assertEquals(Configuration.ValueType.Range.name(), configurationManagerImplSpy.getConfigurationType(configurationName)); + } + + @Test + public void getConfigurationTypeTestReturnsStringRepresentingConfigurationType() { + ConfigKey configuration = RoleService.EnableDynamicApiChecker; + + Mockito.when(configDao.findByName(configuration.key())).thenReturn(configurationVOMock); + Mockito.doReturn(configuration.type()).when(configurationManagerImplSpy).getConfigurationTypeWrapperClass(configuration.key()); + + configurationManagerImplSpy.getConfigurationType(configuration.key()); + Mockito.verify(configurationManagerImplSpy).parseConfigurationTypeIntoString(configuration.type(), configurationVOMock); + } }