Skip to content

Commit b0a21fd

Browse files
bernardodemarcodhslove
bernardodemarco
authored and
dhslove
committed
Accept case insensitive values in boolean settings (apache#10663)
1 parent 05e6a04 commit b0a21fd

File tree

2 files changed

+156
-32
lines changed

2 files changed

+156
-32
lines changed

‎server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java‎

Lines changed: 44 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,11 @@ public boolean stop() {
739739

740740
@Override
741741
@DB
742-
public String updateConfiguration(final long userId, final String name, final String category, String value, ConfigKey.Scope scope, final Long resourceId) {
742+
public String updateConfiguration(final long userId, final String name, final String category, String value, final String scope, final Long resourceId) {
743+
if (Boolean.class == getConfigurationTypeWrapperClass(name)) {
744+
value = value.toLowerCase();
745+
}
746+
743747
final String validationMsg = validateConfigurationValue(name, value, scope);
744748
if (validationMsg != null) {
745749
logger.error("Invalid value [{}] for configuration [{}] due to [{}].", value, name, validationMsg);
@@ -1307,18 +1311,9 @@ protected String validateConfigurationValue(String name, String value, ConfigKey
13071311
return "Invalid scope id provided for the parameter " + name;
13081312
}
13091313
}
1310-
Class<?> type;
1311-
Config configuration = Config.getConfig(name);
1312-
if (configuration == null) {
1313-
logger.warn("Did not find configuration " + name + " in Config.java. Perhaps moved to ConfigDepot");
1314-
ConfigKey<?> configKey = _configDepot.get(name);
1315-
if(configKey == null) {
1316-
logger.warn("Did not find configuration " + name + " in ConfigDepot too.");
1317-
return null;
1318-
}
1319-
type = configKey.type();
1320-
} else {
1321-
type = configuration.getType();
1314+
Class<?> type = getConfigurationTypeWrapperClass(name);
1315+
if (type == null) {
1316+
return null;
13221317
}
13231318

13241319
validateSpecificConfigurationValues(name, value, type);
@@ -1330,7 +1325,28 @@ protected String validateConfigurationValue(String name, String value, ConfigKey
13301325
return String.format("Value [%s] is not a valid [%s].", value, type);
13311326
}
13321327

1333-
return validateValueRange(name, value, type, configuration);
1328+
return validateValueRange(name, value, type, Config.getConfig(name));
1329+
}
1330+
1331+
/**
1332+
* Returns the configuration type's wrapper class.
1333+
* @param name name of the configuration.
1334+
* @return if the configuration exists, returns its type's wrapper class; if not, returns null.
1335+
*/
1336+
protected Class<?> getConfigurationTypeWrapperClass(String name) {
1337+
Config configuration = Config.getConfig(name);
1338+
if (configuration != null) {
1339+
return configuration.getType();
1340+
}
1341+
1342+
logger.warn("Did not find configuration [{}] in Config.java. Perhaps moved to ConfigDepot.", name);
1343+
ConfigKey<?> configKey = _configDepot.get(name);
1344+
if (configKey == null) {
1345+
logger.warn("Did not find configuration [{}] in ConfigDepot too.", name);
1346+
return null;
1347+
}
1348+
1349+
return configKey.type();
13341350
}
13351351

13361352
/**
@@ -1340,7 +1356,7 @@ protected String validateConfigurationValue(String name, String value, ConfigKey
13401356
* <ul>
13411357
* <li>String: any value, including null;</li>
13421358
* <li>Character: any value, including null;</li>
1343-
* <li>Boolean: strings that equal "true" or "false" (case-sensitive);</li>
1359+
* <li>Boolean: strings that equal "true" or "false" (case-insensitive);</li>
13441360
* <li>Integer, Short, Long: strings that contain a valid int/short/long;</li>
13451361
* <li>Float, Double: strings that contain a valid float/double, except infinity.</li>
13461362
* </ul>
@@ -8287,36 +8303,32 @@ public ConfigKey<?>[] getConfigKeys() {
82878303
};
82888304
}
82898305

8306+
8307+
/**
8308+
* Returns a string representing the specified configuration's type.
8309+
* @param configName name of the configuration.
8310+
* @return if the configuration exists, returns its type; if not, returns {@link Configuration.ValueType#String}.
8311+
*/
82908312
@Override
82918313
public String getConfigurationType(final String configName) {
82928314
final ConfigurationVO cfg = _configDao.findByName(configName);
82938315
if (cfg == null) {
8294-
logger.warn("Configuration " + configName + " not found");
8316+
logger.warn("Configuration [{}] not found", configName);
82958317
return Configuration.ValueType.String.name();
82968318
}
82978319

82988320
if (weightBasedParametersForValidation.contains(configName)) {
82998321
return Configuration.ValueType.Range.name();
83008322
}
83018323

8302-
Class<?> type = null;
8303-
final Config c = Config.getConfig(configName);
8304-
if (c == null) {
8305-
logger.warn("Configuration " + configName + " no found. Perhaps moved to ConfigDepot");
8306-
final ConfigKey<?> configKey = _configDepot.get(configName);
8307-
if (configKey == null) {
8308-
logger.warn("Couldn't find configuration " + configName + " in ConfigDepot too.");
8309-
return Configuration.ValueType.String.name();
8310-
}
8311-
type = configKey.type();
8312-
} else {
8313-
type = c.getType();
8314-
}
8315-
8316-
return getInputType(type, cfg);
8324+
Class<?> type = getConfigurationTypeWrapperClass(configName);
8325+
return parseConfigurationTypeIntoString(type, cfg);
83178326
}
83188327

8319-
private String getInputType(Class<?> type, ConfigurationVO cfg) {
8328+
/**
8329+
* Parses a configuration type's wrapper class into its string representation.
8330+
*/
8331+
protected String parseConfigurationTypeIntoString(Class<?> type, ConfigurationVO cfg) {
83208332
if (type == null) {
83218333
return Configuration.ValueType.String.name();
83228334
}

‎server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
// under the License.
1717
package com.cloud.configuration;
1818

19+
import com.cloud.alert.AlertManager;
1920
import com.cloud.capacity.dao.CapacityDao;
2021
import com.cloud.dc.DataCenterVO;
2122
import com.cloud.dc.VlanVO;
@@ -63,6 +64,7 @@
6364
import com.cloud.utils.db.SearchCriteria;
6465
import com.cloud.utils.net.NetUtils;
6566
import com.cloud.vm.dao.VMInstanceDao;
67+
import org.apache.cloudstack.acl.RoleService;
6668
import org.apache.cloudstack.annotation.dao.AnnotationDao;
6769
import org.apache.cloudstack.api.command.admin.config.ResetCfgCmd;
6870
import org.apache.cloudstack.api.command.admin.network.CreateNetworkOfferingCmd;
@@ -80,6 +82,7 @@
8082
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
8183
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
8284
import org.apache.cloudstack.vm.UnmanagedVMsManager;
85+
import org.apache.cloudstack.config.Configuration;
8386
import org.junit.Assert;
8487
import org.junit.Before;
8588
import org.junit.Test;
@@ -935,4 +938,113 @@ public void testValidateConfigurationAllowedOnlyForDefaultAdmin_withValidConfigN
935938
configurationManagerImplSpy.validateConfigurationAllowedOnlyForDefaultAdmin(AccountManagerImpl.listOfRoleTypesAllowedForOperationsOfSameRoleType.key(), invalidValue);
936939
}
937940
}
941+
942+
943+
@Test
944+
public void getConfigurationTypeWrapperClassTestReturnsConfigType() {
945+
Config configuration = Config.AlertEmailAddresses;
946+
947+
Assert.assertEquals(configuration.getType(), configurationManagerImplSpy.getConfigurationTypeWrapperClass(configuration.key()));
948+
}
949+
950+
@Test
951+
public void getConfigurationTypeWrapperClassTestReturnsConfigKeyType() {
952+
String configurationName = "configuration.name";
953+
954+
Mockito.when(configDepot.get(configurationName)).thenReturn(configKeyMock);
955+
Mockito.when(configKeyMock.type()).thenReturn(Integer.class);
956+
957+
Assert.assertEquals(Integer.class, configurationManagerImplSpy.getConfigurationTypeWrapperClass(configurationName));
958+
}
959+
960+
@Test
961+
public void getConfigurationTypeWrapperClassTestReturnsNullWhenConfigurationDoesNotExist() {
962+
String configurationName = "configuration.name";
963+
964+
Mockito.when(configDepot.get(configurationName)).thenReturn(null);
965+
Assert.assertNull(configurationManagerImplSpy.getConfigurationTypeWrapperClass(configurationName));
966+
}
967+
968+
@Test
969+
public void parseConfigurationTypeIntoStringTestReturnsStringWhenTypeIsNull() {
970+
Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(null, null));
971+
}
972+
973+
@Test
974+
public void parseConfigurationTypeIntoStringTestReturnsStringWhenTypeIsStringAndConfigurationKindIsNull() {
975+
Mockito.when(configurationVOMock.getKind()).thenReturn(null);
976+
Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(String.class, configurationVOMock));
977+
}
978+
979+
@Test
980+
public void parseConfigurationTypeIntoStringTestReturnsKindWhenTypeIsStringAndKindIsNotNull() {
981+
Mockito.when(configurationVOMock.getKind()).thenReturn(ConfigKey.Kind.CSV.name());
982+
Assert.assertEquals(ConfigKey.Kind.CSV.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(String.class, configurationVOMock));
983+
}
984+
985+
@Test
986+
public void parseConfigurationTypeIntoStringTestReturnsKindWhenTypeIsCharacterAndKindIsNotNull() {
987+
Mockito.when(configurationVOMock.getKind()).thenReturn(ConfigKey.Kind.CSV.name());
988+
Assert.assertEquals(ConfigKey.Kind.CSV.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Character.class, configurationVOMock));
989+
}
990+
991+
@Test
992+
public void parseConfigurationTypeIntoStringTestReturnsNumberWhenTypeIsInteger() {
993+
Assert.assertEquals(Configuration.ValueType.Number.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Integer.class, configurationVOMock));
994+
}
995+
996+
@Test
997+
public void parseConfigurationTypeIntoStringTestReturnsNumberWhenTypeIsLong() {
998+
Assert.assertEquals(Configuration.ValueType.Number.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Long.class, configurationVOMock));
999+
}
1000+
1001+
@Test
1002+
public void parseConfigurationTypeIntoStringTestReturnsNumberWhenTypeIsShort() {
1003+
Assert.assertEquals(Configuration.ValueType.Number.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Short.class, configurationVOMock));
1004+
}
1005+
1006+
@Test
1007+
public void parseConfigurationTypeIntoStringTestReturnsDecimalWhenTypeIsFloat() {
1008+
Assert.assertEquals(Configuration.ValueType.Decimal.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Float.class, configurationVOMock));
1009+
}
1010+
1011+
@Test
1012+
public void parseConfigurationTypeIntoStringTestReturnsDecimalWhenTypeIsDouble() {
1013+
Assert.assertEquals(Configuration.ValueType.Decimal.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Double.class, configurationVOMock));
1014+
}
1015+
1016+
@Test
1017+
public void parseConfigurationTypeIntoStringTestReturnsBooleanWhenTypeIsBoolean() {
1018+
Assert.assertEquals(Configuration.ValueType.Boolean.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Boolean.class, configurationVOMock));
1019+
}
1020+
1021+
@Test
1022+
public void parseConfigurationTypeIntoStringTestReturnsStringWhenTypeDoesNotMatchAnyAvailableType() {
1023+
Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Object.class, configurationVOMock));
1024+
}
1025+
1026+
@Test
1027+
public void getConfigurationTypeTestReturnsStringWhenConfigurationDoesNotExist() {
1028+
Mockito.when(configDao.findByName(Mockito.anyString())).thenReturn(null);
1029+
Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.getConfigurationType(Mockito.anyString()));
1030+
}
1031+
1032+
@Test
1033+
public void getConfigurationTypeTestReturnsRangeForConfigurationsThatAcceptIntervals() {
1034+
String configurationName = AlertManager.CPUCapacityThreshold.key();
1035+
1036+
Mockito.when(configDao.findByName(configurationName)).thenReturn(configurationVOMock);
1037+
Assert.assertEquals(Configuration.ValueType.Range.name(), configurationManagerImplSpy.getConfigurationType(configurationName));
1038+
}
1039+
1040+
@Test
1041+
public void getConfigurationTypeTestReturnsStringRepresentingConfigurationType() {
1042+
ConfigKey<Boolean> configuration = RoleService.EnableDynamicApiChecker;
1043+
1044+
Mockito.when(configDao.findByName(configuration.key())).thenReturn(configurationVOMock);
1045+
Mockito.doReturn(configuration.type()).when(configurationManagerImplSpy).getConfigurationTypeWrapperClass(configuration.key());
1046+
1047+
configurationManagerImplSpy.getConfigurationType(configuration.key());
1048+
Mockito.verify(configurationManagerImplSpy).parseConfigurationTypeIntoString(configuration.type(), configurationVOMock);
1049+
}
9381050
}

0 commit comments

Comments
 (0)