Skip to content

Commit 908ed4e

Browse files
bernardodemarcobernardodemarco
committed
make boolean configs case insensitive
1 parent bc5728a commit 908ed4e

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
@@ -694,6 +694,10 @@ public boolean stop() {
694694
@Override
695695
@DB
696696
public String updateConfiguration(final long userId, final String name, final String category, String value, final String scope, final Long resourceId) {
697+
if (getConfigurationTypeWrapperClass(name) == Boolean.class) {
698+
value = value.toLowerCase();
699+
}
700+
697701
final String validationMsg = validateConfigurationValue(name, value, scope);
698702
if (validationMsg != null) {
699703
logger.error("Invalid value [{}] for configuration [{}] due to [{}].", value, name, validationMsg);
@@ -1231,18 +1235,9 @@ protected String validateConfigurationValue(String name, String value, String sc
12311235
return "Invalid scope id provided for the parameter " + name;
12321236
}
12331237
}
1234-
Class<?> type;
1235-
Config configuration = Config.getConfig(name);
1236-
if (configuration == null) {
1237-
logger.warn("Did not find configuration " + name + " in Config.java. Perhaps moved to ConfigDepot");
1238-
ConfigKey<?> configKey = _configDepot.get(name);
1239-
if(configKey == null) {
1240-
logger.warn("Did not find configuration " + name + " in ConfigDepot too.");
1241-
return null;
1242-
}
1243-
type = configKey.type();
1244-
} else {
1245-
type = configuration.getType();
1238+
Class<?> type = getConfigurationTypeWrapperClass(name);
1239+
if (type == null) {
1240+
return null;
12461241
}
12471242

12481243
validateSpecificConfigurationValues(name, value, type);
@@ -1252,7 +1247,28 @@ protected String validateConfigurationValue(String name, String value, String sc
12521247
return String.format("Value [%s] is not a valid [%s].", value, type);
12531248
}
12541249

1255-
return validateValueRange(name, value, type, configuration);
1250+
return validateValueRange(name, value, type, Config.getConfig(name));
1251+
}
1252+
1253+
/**
1254+
* Returns the configuration type's wrapper class.
1255+
* @param name name of the configuration.
1256+
* @return if the configuration exists, returns its type's wrapper class; if not, returns null.
1257+
*/
1258+
protected Class<?> getConfigurationTypeWrapperClass(String name) {
1259+
Config configuration = Config.getConfig(name);
1260+
if (configuration != null) {
1261+
return configuration.getType();
1262+
}
1263+
1264+
logger.warn("Did not find configuration [{}] in Config.java. Perhaps moved to ConfigDepot.", name);
1265+
ConfigKey<?> configKey = _configDepot.get(name);
1266+
if (configKey == null) {
1267+
logger.warn("Did not find configuration [{}] in ConfigDepot too.", name);
1268+
return null;
1269+
}
1270+
1271+
return configKey.type();
12561272
}
12571273

12581274
/**
@@ -1262,7 +1278,7 @@ protected String validateConfigurationValue(String name, String value, String sc
12621278
* <ul>
12631279
* <li>String: any value, including null;</li>
12641280
* <li>Character: any value, including null;</li>
1265-
* <li>Boolean: strings that equal "true" or "false" (case-sensitive);</li>
1281+
* <li>Boolean: strings that equal "true" or "false" (case-insensitive);</li>
12661282
* <li>Integer, Short, Long: strings that contain a valid int/short/long;</li>
12671283
* <li>Float, Double: strings that contain a valid float/double, except infinity.</li>
12681284
* </ul>
@@ -8105,36 +8121,32 @@ public ConfigKey<?>[] getConfigKeys() {
81058121
};
81068122
}
81078123

8124+
8125+
/**
8126+
* Returns a string representing the specified configuration's type.
8127+
* @param configName name of the configuration.
8128+
* @return if the configuration exists, returns its type; if not, returns {@link Configuration.ValueType#String}.
8129+
*/
81088130
@Override
81098131
public String getConfigurationType(final String configName) {
81108132
final ConfigurationVO cfg = _configDao.findByName(configName);
81118133
if (cfg == null) {
8112-
logger.warn("Configuration " + configName + " not found");
8134+
logger.warn("Configuration [{}] not found", configName);
81138135
return Configuration.ValueType.String.name();
81148136
}
81158137

81168138
if (weightBasedParametersForValidation.contains(configName)) {
81178139
return Configuration.ValueType.Range.name();
81188140
}
81198141

8120-
Class<?> type = null;
8121-
final Config c = Config.getConfig(configName);
8122-
if (c == null) {
8123-
logger.warn("Configuration " + configName + " no found. Perhaps moved to ConfigDepot");
8124-
final ConfigKey<?> configKey = _configDepot.get(configName);
8125-
if (configKey == null) {
8126-
logger.warn("Couldn't find configuration " + configName + " in ConfigDepot too.");
8127-
return Configuration.ValueType.String.name();
8128-
}
8129-
type = configKey.type();
8130-
} else {
8131-
type = c.getType();
8132-
}
8133-
8134-
return getInputType(type, cfg);
8142+
Class<?> type = getConfigurationTypeWrapperClass(configName);
8143+
return parseConfigurationTypeIntoString(type, cfg);
81358144
}
81368145

8137-
private String getInputType(Class<?> type, ConfigurationVO cfg) {
8146+
/**
8147+
* Parses a configuration type's wrapper class into its string representation.
8148+
*/
8149+
protected String parseConfigurationTypeIntoString(Class<?> type, ConfigurationVO cfg) {
81388150
if (type == null) {
81398151
return Configuration.ValueType.String.name();
81408152
}

‎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;
@@ -50,6 +51,7 @@
5051
import com.cloud.utils.db.SearchCriteria;
5152
import com.cloud.utils.net.NetUtils;
5253
import com.cloud.vm.dao.VMInstanceDao;
54+
import org.apache.cloudstack.acl.RoleService;
5355
import org.apache.cloudstack.annotation.dao.AnnotationDao;
5456
import org.apache.cloudstack.api.command.admin.network.CreateNetworkOfferingCmd;
5557
import org.apache.cloudstack.api.command.admin.offering.UpdateDiskOfferingCmd;
@@ -62,6 +64,7 @@
6264
import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
6365
import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO;
6466
import org.apache.cloudstack.vm.UnmanagedVMsManager;
67+
import org.apache.cloudstack.config.Configuration;
6568
import org.junit.Assert;
6669
import org.junit.Before;
6770
import org.junit.Test;
@@ -851,4 +854,113 @@ public void shouldValidateConfigRangeTestValueIsNotNullAndConfigHasRangeReturnTr
851854
boolean result = configurationManagerImplSpy.shouldValidateConfigRange(Config.ConsoleProxySessionMax.name(), "test", Config.ConsoleProxyUrlDomain);
852855
Assert.assertTrue(result);
853856
}
857+
858+
859+
@Test
860+
public void getConfigurationTypeWrapperClassTestReturnsConfigType() {
861+
Config configuration = Config.AlertEmailAddresses;
862+
863+
Assert.assertEquals(configuration.getType(), configurationManagerImplSpy.getConfigurationTypeWrapperClass(configuration.key()));
864+
}
865+
866+
@Test
867+
public void getConfigurationTypeWrapperClassTestReturnsConfigKeyType() {
868+
String configurationName = "configuration.name";
869+
870+
Mockito.when(configDepot.get(configurationName)).thenReturn(configKeyMock);
871+
Mockito.when(configKeyMock.type()).thenReturn(Integer.class);
872+
873+
Assert.assertEquals(Integer.class, configurationManagerImplSpy.getConfigurationTypeWrapperClass(configurationName));
874+
}
875+
876+
@Test
877+
public void getConfigurationTypeWrapperClassTestReturnsNullWhenConfigurationDoesNotExist() {
878+
String configurationName = "configuration.name";
879+
880+
Mockito.when(configDepot.get(configurationName)).thenReturn(null);
881+
Assert.assertNull(configurationManagerImplSpy.getConfigurationTypeWrapperClass(configurationName));
882+
}
883+
884+
@Test
885+
public void parseConfigurationTypeIntoStringTestReturnsStringWhenTypeIsNull() {
886+
Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(null, null));
887+
}
888+
889+
@Test
890+
public void parseConfigurationTypeIntoStringTestReturnsStringWhenTypeIsStringAndConfigurationKindIsNull() {
891+
Mockito.when(configurationVOMock.getKind()).thenReturn(null);
892+
Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(String.class, configurationVOMock));
893+
}
894+
895+
@Test
896+
public void parseConfigurationTypeIntoStringTestReturnsKindWhenTypeIsStringAndKindIsNotNull() {
897+
Mockito.when(configurationVOMock.getKind()).thenReturn(ConfigKey.Kind.CSV.name());
898+
Assert.assertEquals(ConfigKey.Kind.CSV.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(String.class, configurationVOMock));
899+
}
900+
901+
@Test
902+
public void parseConfigurationTypeIntoStringTestReturnsKindWhenTypeIsCharacterAndKindIsNotNull() {
903+
Mockito.when(configurationVOMock.getKind()).thenReturn(ConfigKey.Kind.CSV.name());
904+
Assert.assertEquals(ConfigKey.Kind.CSV.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Character.class, configurationVOMock));
905+
}
906+
907+
@Test
908+
public void parseConfigurationTypeIntoStringTestReturnsNumberWhenTypeIsInteger() {
909+
Assert.assertEquals(Configuration.ValueType.Number.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Integer.class, configurationVOMock));
910+
}
911+
912+
@Test
913+
public void parseConfigurationTypeIntoStringTestReturnsNumberWhenTypeIsLong() {
914+
Assert.assertEquals(Configuration.ValueType.Number.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Long.class, configurationVOMock));
915+
}
916+
917+
@Test
918+
public void parseConfigurationTypeIntoStringTestReturnsNumberWhenTypeIsShort() {
919+
Assert.assertEquals(Configuration.ValueType.Number.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Short.class, configurationVOMock));
920+
}
921+
922+
@Test
923+
public void parseConfigurationTypeIntoStringTestReturnsDecimalWhenTypeIsFloat() {
924+
Assert.assertEquals(Configuration.ValueType.Decimal.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Float.class, configurationVOMock));
925+
}
926+
927+
@Test
928+
public void parseConfigurationTypeIntoStringTestReturnsDecimalWhenTypeIsDouble() {
929+
Assert.assertEquals(Configuration.ValueType.Decimal.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Double.class, configurationVOMock));
930+
}
931+
932+
@Test
933+
public void parseConfigurationTypeIntoStringTestReturnsBooleanWhenTypeIsBoolean() {
934+
Assert.assertEquals(Configuration.ValueType.Boolean.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Boolean.class, configurationVOMock));
935+
}
936+
937+
@Test
938+
public void parseConfigurationTypeIntoStringTestReturnsStringWhenTypeDoesNotMatchAnyAvailableType() {
939+
Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.parseConfigurationTypeIntoString(Object.class, configurationVOMock));
940+
}
941+
942+
@Test
943+
public void getConfigurationTypeTestReturnsStringWhenConfigurationDoesNotExist() {
944+
Mockito.when(configDao.findByName(Mockito.anyString())).thenReturn(null);
945+
Assert.assertEquals(Configuration.ValueType.String.name(), configurationManagerImplSpy.getConfigurationType(Mockito.anyString()));
946+
}
947+
948+
@Test
949+
public void getConfigurationTypeTestReturnsRangeForConfigurationsThatAcceptIntervals() {
950+
String configurationName = AlertManager.CPUCapacityThreshold.key();
951+
952+
Mockito.when(configDao.findByName(configurationName)).thenReturn(configurationVOMock);
953+
Assert.assertEquals(Configuration.ValueType.Range.name(), configurationManagerImplSpy.getConfigurationType(configurationName));
954+
}
955+
956+
@Test
957+
public void getConfigurationTypeTestReturnsStringRepresentingConfigurationType() {
958+
ConfigKey<Boolean> configuration = RoleService.EnableDynamicApiChecker;
959+
960+
Mockito.when(configDao.findByName(configuration.key())).thenReturn(configurationVOMock);
961+
Mockito.doReturn(configuration.type()).when(configurationManagerImplSpy).getConfigurationTypeWrapperClass(configuration.key());
962+
963+
configurationManagerImplSpy.getConfigurationType(configuration.key());
964+
Mockito.verify(configurationManagerImplSpy).parseConfigurationTypeIntoString(configuration.type(), configurationVOMock);
965+
}
854966
}

0 commit comments

Comments
 (0)