Skip to content

Commit 8f2735a

Browse files
bernardodemarcobernardodemarco
authored
Accept case insensitive values in boolean settings (#10663)
1 parent 41de0b9 commit 8f2735a

File tree

2 files changed

+155
-31
lines changed

2 files changed

+155
-31
lines changed

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

Lines changed: 43 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -703,6 +703,10 @@ public boolean stop() {
703703
@Override
704704
@DB
705705
public String updateConfiguration(final long userId, final String name, final String category, String value, final String scope, final Long resourceId) {
706+
if (Boolean.class == getConfigurationTypeWrapperClass(name)) {
707+
value = value.toLowerCase();
708+
}
709+
706710
final String validationMsg = validateConfigurationValue(name, value, scope);
707711
if (validationMsg != null) {
708712
logger.error("Invalid value [{}] for configuration [{}] due to [{}].", value, name, validationMsg);
@@ -1241,18 +1245,9 @@ protected String validateConfigurationValue(String name, String value, String sc
12411245
return "Invalid scope id provided for the parameter " + name;
12421246
}
12431247
}
1244-
Class<?> type;
1245-
Config configuration = Config.getConfig(name);
1246-
if (configuration == null) {
1247-
logger.warn("Did not find configuration " + name + " in Config.java. Perhaps moved to ConfigDepot");
1248-
ConfigKey<?> configKey = _configDepot.get(name);
1249-
if(configKey == null) {
1250-
logger.warn("Did not find configuration " + name + " in ConfigDepot too.");
1251-
return null;
1252-
}
1253-
type = configKey.type();
1254-
} else {
1255-
type = configuration.getType();
1248+
Class<?> type = getConfigurationTypeWrapperClass(name);
1249+
if (type == null) {
1250+
return null;
12561251
}
12571252

12581253
validateSpecificConfigurationValues(name, value, type);
@@ -1262,7 +1257,28 @@ protected String validateConfigurationValue(String name, String value, String sc
12621257
return String.format("Value [%s] is not a valid [%s].", value, type);
12631258
}
12641259

1265-
return validateValueRange(name, value, type, configuration);
1260+
return validateValueRange(name, value, type, Config.getConfig(name));
1261+
}
1262+
1263+
/**
1264+
* Returns the configuration type's wrapper class.
1265+
* @param name name of the configuration.
1266+
* @return if the configuration exists, returns its type's wrapper class; if not, returns null.
1267+
*/
1268+
protected Class<?> getConfigurationTypeWrapperClass(String name) {
1269+
Config configuration = Config.getConfig(name);
1270+
if (configuration != null) {
1271+
return configuration.getType();
1272+
}
1273+
1274+
logger.warn("Did not find configuration [{}] in Config.java. Perhaps moved to ConfigDepot.", name);
1275+
ConfigKey<?> configKey = _configDepot.get(name);
1276+
if (configKey == null) {
1277+
logger.warn("Did not find configuration [{}] in ConfigDepot too.", name);
1278+
return null;
1279+
}
1280+
1281+
return configKey.type();
12661282
}
12671283

12681284
protected void validateConfigurationAllowedOnlyForDefaultAdmin(String configName, String value) {
@@ -1299,7 +1315,7 @@ protected void validateConfigurationAllowedOnlyForDefaultAdmin(String configName
12991315
* <ul>
13001316
* <li>String: any value, including null;</li>
13011317
* <li>Character: any value, including null;</li>
1302-
* <li>Boolean: strings that equal "true" or "false" (case-sensitive);</li>
1318+
* <li>Boolean: strings that equal "true" or "false" (case-insensitive);</li>
13031319
* <li>Integer, Short, Long: strings that contain a valid int/short/long;</li>
13041320
* <li>Float, Double: strings that contain a valid float/double, except infinity.</li>
13051321
* </ul>
@@ -8176,36 +8192,32 @@ public ConfigKey<?>[] getConfigKeys() {
81768192
};
81778193
}
81788194

8195+
8196+
/**
8197+
* Returns a string representing the specified configuration's type.
8198+
* @param configName name of the configuration.
8199+
* @return if the configuration exists, returns its type; if not, returns {@link Configuration.ValueType#String}.
8200+
*/
81798201
@Override
81808202
public String getConfigurationType(final String configName) {
81818203
final ConfigurationVO cfg = _configDao.findByName(configName);
81828204
if (cfg == null) {
8183-
logger.warn("Configuration " + configName + " not found");
8205+
logger.warn("Configuration [{}] not found", configName);
81848206
return Configuration.ValueType.String.name();
81858207
}
81868208

81878209
if (weightBasedParametersForValidation.contains(configName)) {
81888210
return Configuration.ValueType.Range.name();
81898211
}
81908212

8191-
Class<?> type = null;
8192-
final Config c = Config.getConfig(configName);
8193-
if (c == null) {
8194-
logger.warn("Configuration " + configName + " no found. Perhaps moved to ConfigDepot");
8195-
final ConfigKey<?> configKey = _configDepot.get(configName);
8196-
if (configKey == null) {
8197-
logger.warn("Couldn't find configuration " + configName + " in ConfigDepot too.");
8198-
return Configuration.ValueType.String.name();
8199-
}
8200-
type = configKey.type();
8201-
} else {
8202-
type = c.getType();
8203-
}
8204-
8205-
return getInputType(type, cfg);
8213+
Class<?> type = getConfigurationTypeWrapperClass(configName);
8214+
return parseConfigurationTypeIntoString(type, cfg);
82068215
}
82078216

8208-
private String getInputType(Class<?> type, ConfigurationVO cfg) {
8217+
/**
8218+
* Parses a configuration type's wrapper class into its string representation.
8219+
*/
8220+
protected String parseConfigurationTypeIntoString(Class<?> type, ConfigurationVO cfg) {
82098221
if (type == null) {
82108222
return Configuration.ValueType.String.name();
82118223
}

‎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;
@@ -52,6 +53,7 @@
5253
import com.cloud.utils.exception.CloudRuntimeException;
5354
import com.cloud.utils.net.NetUtils;
5455
import com.cloud.vm.dao.VMInstanceDao;
56+
import org.apache.cloudstack.acl.RoleService;
5557
import org.apache.cloudstack.annotation.dao.AnnotationDao;
5658
import org.apache.cloudstack.api.command.admin.network.CreateNetworkOfferingCmd;
5759
import org.apache.cloudstack.api.command.admin.offering.UpdateDiskOfferingCmd;
@@ -65,6 +67,7 @@
6567
import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
6668
import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO;
6769
import org.apache.cloudstack.vm.UnmanagedVMsManager;
70+
import org.apache.cloudstack.config.Configuration;
6871
import org.junit.Assert;
6972
import org.junit.Before;
7073
import org.junit.Test;
@@ -897,4 +900,113 @@ public void testValidateConfigurationAllowedOnlyForDefaultAdmin_withValidConfigN
897900
configurationManagerImplSpy.validateConfigurationAllowedOnlyForDefaultAdmin(AccountManagerImpl.listOfRoleTypesAllowedForOperationsOfSameRoleType.key(), invalidValue);
898901
}
899902
}
903+
904+
905+
@Test
906+
public void getConfigurationTypeWrapperClassTestReturnsConfigType() {
907+
Config configuration = Config.AlertEmailAddresses;
908+
909+
Assert.assertEquals(configuration.getType(), configurationManagerImplSpy.getConfigurationTypeWrapperClass(configuration.key()));
910+
}
911+
912+
@Test
913+
public void getConfigurationTypeWrapperClassTestReturnsConfigKeyType() {
914+
String configurationName = "configuration.name";
915+
916+
Mockito.when(configDepot.get(configurationName)).thenReturn(configKeyMock);
917+
Mockito.when(configKeyMock.type()).thenReturn(Integer.class);
918+
919+
Assert.assertEquals(Integer.class, configurationManagerImplSpy.getConfigurationTypeWrapperClass(configurationName));
920+
}
921+
922+
@Test
923+
public void getConfigurationTypeWrapperClassTestReturnsNullWhenConfigurationDoesNotExist() {
924+
String configurationName = "configuration.name";
925+
926+
Mockito.when(configDepot.get(configurationName)).thenReturn(null);
927+
Assert.assertNull(configurationManagerImplSpy.getConfigurationTypeWrapperClass(configurationName));
928+
}
929+
930+
@Test
931+
public void parseConfigurationTypeIntoStringTestReturnsStringWhenTypeIsNull() {
932+
Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(null, null));
933+
}
934+
935+
@Test
936+
public void parseConfigurationTypeIntoStringTestReturnsStringWhenTypeIsStringAndConfigurationKindIsNull() {
937+
Mockito.when(configurationVOMock.getKind()).thenReturn(null);
938+
Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(String.class, configurationVOMock));
939+
}
940+
941+
@Test
942+
public void parseConfigurationTypeIntoStringTestReturnsKindWhenTypeIsStringAndKindIsNotNull() {
943+
Mockito.when(configurationVOMock.getKind()).thenReturn(ConfigKey.Kind.CSV.name());
944+
Assert.assertEquals(ConfigKey.Kind.CSV.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(String.class, configurationVOMock));
945+
}
946+
947+
@Test
948+
public void parseConfigurationTypeIntoStringTestReturnsKindWhenTypeIsCharacterAndKindIsNotNull() {
949+
Mockito.when(configurationVOMock.getKind()).thenReturn(ConfigKey.Kind.CSV.name());
950+
Assert.assertEquals(ConfigKey.Kind.CSV.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Character.class, configurationVOMock));
951+
}
952+
953+
@Test
954+
public void parseConfigurationTypeIntoStringTestReturnsNumberWhenTypeIsInteger() {
955+
Assert.assertEquals(Configuration.ValueType.Number.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Integer.class, configurationVOMock));
956+
}
957+
958+
@Test
959+
public void parseConfigurationTypeIntoStringTestReturnsNumberWhenTypeIsLong() {
960+
Assert.assertEquals(Configuration.ValueType.Number.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Long.class, configurationVOMock));
961+
}
962+
963+
@Test
964+
public void parseConfigurationTypeIntoStringTestReturnsNumberWhenTypeIsShort() {
965+
Assert.assertEquals(Configuration.ValueType.Number.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Short.class, configurationVOMock));
966+
}
967+
968+
@Test
969+
public void parseConfigurationTypeIntoStringTestReturnsDecimalWhenTypeIsFloat() {
970+
Assert.assertEquals(Configuration.ValueType.Decimal.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Float.class, configurationVOMock));
971+
}
972+
973+
@Test
974+
public void parseConfigurationTypeIntoStringTestReturnsDecimalWhenTypeIsDouble() {
975+
Assert.assertEquals(Configuration.ValueType.Decimal.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Double.class, configurationVOMock));
976+
}
977+
978+
@Test
979+
public void parseConfigurationTypeIntoStringTestReturnsBooleanWhenTypeIsBoolean() {
980+
Assert.assertEquals(Configuration.ValueType.Boolean.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Boolean.class, configurationVOMock));
981+
}
982+
983+
@Test
984+
public void parseConfigurationTypeIntoStringTestReturnsStringWhenTypeDoesNotMatchAnyAvailableType() {
985+
Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Object.class, configurationVOMock));
986+
}
987+
988+
@Test
989+
public void getConfigurationTypeTestReturnsStringWhenConfigurationDoesNotExist() {
990+
Mockito.when(configDao.findByName(Mockito.anyString())).thenReturn(null);
991+
Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.getConfigurationType(Mockito.anyString()));
992+
}
993+
994+
@Test
995+
public void getConfigurationTypeTestReturnsRangeForConfigurationsThatAcceptIntervals() {
996+
String configurationName = AlertManager.CPUCapacityThreshold.key();
997+
998+
Mockito.when(configDao.findByName(configurationName)).thenReturn(configurationVOMock);
999+
Assert.assertEquals(Configuration.ValueType.Range.name(), configurationManagerImplSpy.getConfigurationType(configurationName));
1000+
}
1001+
1002+
@Test
1003+
public void getConfigurationTypeTestReturnsStringRepresentingConfigurationType() {
1004+
ConfigKey<Boolean> configuration = RoleService.EnableDynamicApiChecker;
1005+
1006+
Mockito.when(configDao.findByName(configuration.key())).thenReturn(configurationVOMock);
1007+
Mockito.doReturn(configuration.type()).when(configurationManagerImplSpy).getConfigurationTypeWrapperClass(configuration.key());
1008+
1009+
configurationManagerImplSpy.getConfigurationType(configuration.key());
1010+
Mockito.verify(configurationManagerImplSpy).parseConfigurationTypeIntoString(configuration.type(), configurationVOMock);
1011+
}
9001012
}

0 commit comments

Comments
 (0)