Skip to content

Commit 74a00c4

Browse files
sudo87sudo87
committed
Following changes are done in this commit:
1. Change type for scope paramter for method updateConfiguration and validateConfigurationValue 2. Standardize scope variable to be of ConfigKey type and scopeVal as string
1 parent 7280d5b commit 74a00c4

File tree

4 files changed

+33
-32
lines changed

4 files changed

+33
-32
lines changed

‎engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,14 @@ public interface ConfigurationManager {
7777

7878
/**
7979
* Updates a configuration entry with a new value
80-
*
8180
* @param userId
8281
* @param name
82+
* @param category
8383
* @param value
84+
* @param scope
85+
* @param id
8486
*/
85-
String updateConfiguration(long userId, String name, String category, String value, String scope, Long id);
87+
String updateConfiguration(long userId, String name, String category, String value, ConfigKey.Scope scope, Long id);
8688

8789
// /**
8890
// * Creates a new service offering

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

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -693,7 +693,7 @@ public boolean stop() {
693693

694694
@Override
695695
@DB
696-
public String updateConfiguration(final long userId, final String name, final String category, String value, final String scope, final Long resourceId) {
696+
public String updateConfiguration(final long userId, final String name, final String category, String value, ConfigKey.Scope scope, final Long resourceId) {
697697
final String validationMsg = validateConfigurationValue(name, value, scope);
698698
if (validationMsg != null) {
699699
logger.error("Invalid value [{}] for configuration [{}] due to [{}].", value, name, validationMsg);
@@ -704,15 +704,14 @@ public String updateConfiguration(final long userId, final String name, final St
704704
// corresponding details table,
705705
// if scope is mentioned as global or not mentioned then it is normal
706706
// global parameter updation
707-
if (scope != null && !scope.isEmpty() && !ConfigKey.Scope.Global.toString().equalsIgnoreCase(scope)) {
707+
if (scope != null && !ConfigKey.Scope.Global.equals(scope)) {
708708
boolean valueEncrypted = shouldEncryptValue(category);
709709
if (valueEncrypted) {
710710
value = DBEncryptionUtil.encrypt(value);
711711
}
712712

713713
ApiCommandResourceType resourceType;
714-
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
715-
switch (scopeVal) {
714+
switch (scope) {
716715
case Zone:
717716
final DataCenterVO zone = _zoneDao.findById(resourceId);
718717
if (zone == null) {
@@ -811,9 +810,9 @@ public String updateConfiguration(final long userId, final String name, final St
811810

812811
CallContext.current().setEventResourceType(resourceType);
813812
CallContext.current().setEventResourceId(resourceId);
814-
CallContext.current().setEventDetails(String.format(" Name: %s, New Value: %s, Scope: %s", name, value, scope));
813+
CallContext.current().setEventDetails(String.format(" Name: %s, New Value: %s, Scope: %s", name, value, scope.name()));
815814

816-
_configDepot.invalidateConfigCache(name, scopeVal, resourceId);
815+
_configDepot.invalidateConfigCache(name, scope, resourceId);
817816
return valueEncrypted ? DBEncryptionUtil.decrypt(value) : value;
818817
}
819818

@@ -1028,7 +1027,7 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
10281027
}
10291028

10301029
String currentValueInScope = getConfigurationValueInScope(config, name, scope, id);
1031-
final String updatedValue = updateConfiguration(userId, name, category, value, scope.name(), id);
1030+
final String updatedValue = updateConfiguration(userId, name, category, value, scope, id);
10321031
if (value == null && updatedValue == null || updatedValue.equalsIgnoreCase(value)) {
10331032
logger.debug("Config: {} value is updated from: {} to {} for scope: {}", name,
10341033
encryptEventValueIfConfigIsEncrypted(config, currentValueInScope),
@@ -1096,7 +1095,7 @@ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) thr
10961095
configScope = config.getScopes();
10971096
}
10981097

1099-
String scope = "";
1098+
String scopeVal = "";
11001099
Map<String, Long> scopeMap = new LinkedHashMap<>();
11011100

11021101
Long id = null;
@@ -1112,23 +1111,23 @@ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) thr
11121111
ParamCountPair paramCountPair = getParamCount(scopeMap);
11131112
id = paramCountPair.getId();
11141113
paramCountCheck = paramCountPair.getParamCount();
1115-
scope = paramCountPair.getScope();
1114+
scopeVal = paramCountPair.getScope();
11161115

11171116
if (paramCountCheck > 1) {
11181117
throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");
11191118
}
11201119

1121-
if (scope != null) {
1122-
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
1123-
if (!scope.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scopeVal)) {
1120+
if (scopeVal != null) {
1121+
ConfigKey.Scope scope = ConfigKey.Scope.valueOf(scopeVal);
1122+
if (!scopeVal.equals(ConfigKey.Scope.Global.toString()) && !configScope.contains(scope)) {
11241123
throw new InvalidParameterValueException("Invalid scope id provided for the parameter " + name);
11251124
}
11261125
}
11271126

11281127
String newValue = null;
1129-
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
1130-
String currentValueInScope = getConfigurationValueInScope(config, name, scopeVal, id);
1131-
switch (scopeVal) {
1128+
ConfigKey.Scope scope = ConfigKey.Scope.valueOf(scopeVal);
1129+
String currentValueInScope = getConfigurationValueInScope(config, name, scope, id);
1130+
switch (scope) {
11321131
case Zone:
11331132
final DataCenterVO zone = _zoneDao.findById(id);
11341133
if (zone == null) {
@@ -1215,9 +1214,9 @@ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd cmd) thr
12151214

12161215
logger.debug("Config: {} value is updated from: {} to {} for scope: {}", name,
12171216
encryptEventValueIfConfigIsEncrypted(config, currentValueInScope),
1218-
encryptEventValueIfConfigIsEncrypted(config, newValue), scopeVal);
1217+
encryptEventValueIfConfigIsEncrypted(config, newValue), scope);
12191218

1220-
_configDepot.invalidateConfigCache(name, scopeVal, id);
1219+
_configDepot.invalidateConfigCache(name, scope, id);
12211220

12221221
CallContext.current().setEventDetails(" Name: " + name + " New Value: " + (name.toLowerCase().contains("password") ? "*****" : defaultValue == null ? "" : defaultValue));
12231222
return new Pair<Configuration, String>(_configDao.findByName(name), newValue);
@@ -1242,7 +1241,7 @@ private String getConfigurationValueInScope(ConfigurationVO config, String name,
12421241
* @param scope scope of the configuration.
12431242
* @return null if the value is valid; otherwise, returns an error message.
12441243
*/
1245-
protected String validateConfigurationValue(String name, String value, String scope) {
1244+
protected String validateConfigurationValue(String name, String value, ConfigKey.Scope scope) {
12461245
final ConfigurationVO cfg = _configDao.findByName(name);
12471246
if (cfg == null) {
12481247
logger.error("Missing configuration variable " + name + " in configuration table");
@@ -1251,10 +1250,9 @@ protected String validateConfigurationValue(String name, String value, String sc
12511250

12521251
List<ConfigKey.Scope> configScope = cfg.getScopes();
12531252
if (scope != null) {
1254-
ConfigKey.Scope scopeVal = ConfigKey.Scope.valueOf(scope);
1255-
if (!configScope.contains(scopeVal) &&
1253+
if (!configScope.contains(scope) &&
12561254
!(ENABLE_ACCOUNT_SETTINGS_FOR_DOMAIN.value() && configScope.contains(ConfigKey.Scope.Account) &&
1257-
scope.equals(ConfigKey.Scope.Domain.toString()))) {
1255+
ConfigKey.Scope.Domain.equals(scope))) {
12581256
logger.error("Invalid scope id provided for the parameter " + name);
12591257
return "Invalid scope id provided for the parameter " + name;
12601258
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@ public void testCreateNetworkOfferingForNsx() {
455455
@Test
456456
public void testValidateInvalidConfiguration() {
457457
Mockito.doReturn(null).when(configDao).findByName(Mockito.anyString());
458-
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Global.toString());
458+
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Global);
459459
Assert.assertEquals("Invalid configuration variable.", msg);
460460
}
461461

@@ -464,7 +464,7 @@ public void testValidateInvalidScopeForConfiguration() {
464464
ConfigurationVO cfg = mock(ConfigurationVO.class);
465465
when(cfg.getScopes()).thenReturn(List.of(ConfigKey.Scope.Account));
466466
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
467-
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Domain.toString());
467+
String msg = configurationManagerImplSpy.validateConfigurationValue("test.config.name", "testvalue", ConfigKey.Scope.Domain);
468468
Assert.assertEquals("Invalid scope id provided for the parameter test.config.name", msg);
469469
}
470470

@@ -476,7 +476,7 @@ public void testValidateConfig_ThreadsOnKVMHostToTransferVMwareVMFiles_Failure()
476476
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
477477
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
478478

479-
String result = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "11", configKey.getScopes().get(0).name());
479+
String result = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "11", configKey.getScopes().get(0));
480480

481481
Assert.assertNotNull(result);
482482
}
@@ -488,7 +488,7 @@ public void testValidateConfig_ThreadsOnKVMHostToTransferVMwareVMFiles_Success()
488488
ConfigKey<Integer> configKey = UnmanagedVMsManager.ThreadsOnKVMHostToImportVMwareVMFiles;
489489
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
490490
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
491-
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "10", configKey.getScopes().get(0).name());
491+
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "10", configKey.getScopes().get(0));
492492
Assert.assertNull(msg);
493493
}
494494

@@ -501,7 +501,7 @@ public void testValidateConfig_ConvertVmwareInstanceToKvmTimeout_Failure() {
501501
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
502502
configurationManagerImplSpy.populateConfigValuesForValidationSet();
503503

504-
String result = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "0", configKey.getScopes().get(0).name());
504+
String result = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "0", configKey.getScopes().get(0));
505505

506506
Assert.assertNotNull(result);
507507
}
@@ -514,7 +514,7 @@ public void testValidateConfig_ConvertVmwareInstanceToKvmTimeout_Success() {
514514
Mockito.doReturn(cfg).when(configDao).findByName(Mockito.anyString());
515515
Mockito.doReturn(configKey).when(configurationManagerImplSpy._configDepot).get(configKey.key());
516516
configurationManagerImplSpy.populateConfigValuesForValidationSet();
517-
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "9", configKey.getScopes().get(0).name());
517+
String msg = configurationManagerImplSpy.validateConfigurationValue(configKey.key(), "9", configKey.getScopes().get(0));
518518
Assert.assertNull(msg);
519519
}
520520

@@ -629,14 +629,14 @@ public void checkIfDomainIsChildDomainTestNonChildDomainThrowException() {
629629
@Test
630630
public void validateConfigurationValueTestValidatesValueType() {
631631
Mockito.when(configKeyMock.type()).thenReturn(Integer.class);
632-
configurationManagerImplSpy.validateConfigurationValue("validate.type", "100", ConfigKey.Scope.Global.name());
632+
configurationManagerImplSpy.validateConfigurationValue("validate.type", "100", ConfigKey.Scope.Global);
633633
Mockito.verify(configurationManagerImplSpy).validateValueType("100", Integer.class);
634634
}
635635

636636
@Test
637637
public void validateConfigurationValueTestValidatesValueRange() {
638638
Mockito.when(configKeyMock.type()).thenReturn(Integer.class);
639-
configurationManagerImplSpy.validateConfigurationValue("validate.range", "100", ConfigKey.Scope.Global.name());
639+
configurationManagerImplSpy.validateConfigurationValue("validate.range", "100", ConfigKey.Scope.Global);
640640
Mockito.verify(configurationManagerImplSpy).validateValueRange("validate.range", "100", Integer.class, null);
641641
}
642642

‎server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
import org.apache.cloudstack.api.command.admin.zone.UpdateZoneCmd;
8383
import org.apache.cloudstack.api.command.user.network.ListNetworkOfferingsCmd;
8484
import org.apache.cloudstack.config.Configuration;
85+
import org.apache.cloudstack.framework.config.ConfigKey;
8586
import org.apache.cloudstack.framework.config.impl.ConfigurationSubGroupVO;
8687
import org.apache.cloudstack.region.PortableIp;
8788
import org.apache.cloudstack.region.PortableIpRange;
@@ -497,7 +498,7 @@ public String getName() {
497498
* @see com.cloud.configuration.ConfigurationManager#updateConfiguration(long, java.lang.String, java.lang.String, java.lang.String)
498499
*/
499500
@Override
500-
public String updateConfiguration(long userId, String name, String category, String value, String scope, Long resourceId) {
501+
public String updateConfiguration(long userId, String name, String category, String value, ConfigKey.Scope scope, Long resourceId) {
501502
// TODO Auto-generated method stub
502503
return null;
503504
}

0 commit comments

Comments
 (0)