From 571ca42c4490ff5ed9be024a6d6086e45962cf05 Mon Sep 17 00:00:00 2001 From: alena Date: Tue, 2 Nov 2010 15:07:48 -0700 Subject: [PATCH] 1) Return embedded object for addCfg/updateCfg/listCfg. 2) Fixed NPE in updateCfg command (used to happen when no value was specified) 3) Fixed addCfg command to call correct getName method while setting config name (used to call the method returning the command name) --- .../configuration/dao/ConfigurationDao.java | 4 +++- .../dao/ConfigurationDaoImpl.java | 7 ++++++ .../src/com/cloud/api/ApiResponseHelper.java | 12 ++++++++++ .../com/cloud/api/commands/AddConfigCmd.java | 17 +++++--------- .../com/cloud/api/commands/ListCfgsByCmd.java | 8 ++----- .../com/cloud/api/commands/UpdateCfgCmd.java | 19 ++++++++-------- .../configuration/ConfigurationManager.java | 13 +++++------ .../ConfigurationManagerImpl.java | 22 +++++++++++++------ 8 files changed, 60 insertions(+), 42 deletions(-) diff --git a/core/src/com/cloud/configuration/dao/ConfigurationDao.java b/core/src/com/cloud/configuration/dao/ConfigurationDao.java index 807cb09cda7..b44343ffe52 100644 --- a/core/src/com/cloud/configuration/dao/ConfigurationDao.java +++ b/core/src/com/cloud/configuration/dao/ConfigurationDao.java @@ -63,5 +63,7 @@ public interface ConfigurationDao extends GenericDao { * returns whether or not this is a premium configuration * @return true if premium configuration, false otherwise */ - boolean isPremium(); + boolean isPremium(); + + ConfigurationVO findByName(String name); } diff --git a/core/src/com/cloud/configuration/dao/ConfigurationDaoImpl.java b/core/src/com/cloud/configuration/dao/ConfigurationDaoImpl.java index 366c19850b2..5a5e684ee8a 100644 --- a/core/src/com/cloud/configuration/dao/ConfigurationDaoImpl.java +++ b/core/src/com/cloud/configuration/dao/ConfigurationDaoImpl.java @@ -145,5 +145,12 @@ public class ConfigurationDaoImpl extends GenericDaoBase sc = NameSearch.create(); + sc.setParameters("name", name); + return findOneIncludingRemovedBy(sc); } } diff --git a/server/src/com/cloud/api/ApiResponseHelper.java b/server/src/com/cloud/api/ApiResponseHelper.java index 612a078ecf2..89d702e5e73 100644 --- a/server/src/com/cloud/api/ApiResponseHelper.java +++ b/server/src/com/cloud/api/ApiResponseHelper.java @@ -21,11 +21,13 @@ import java.util.Iterator; import java.util.List; import com.cloud.api.response.AccountResponse; +import com.cloud.api.response.ConfigurationResponse; import com.cloud.api.response.DiskOfferingResponse; import com.cloud.api.response.DomainResponse; import com.cloud.api.response.ResourceLimitResponse; import com.cloud.api.response.ServiceOfferingResponse; import com.cloud.api.response.UserResponse; +import com.cloud.configuration.ConfigurationVO; import com.cloud.configuration.ResourceCount.ResourceType; import com.cloud.configuration.ResourceLimitVO; import com.cloud.domain.DomainVO; @@ -227,5 +229,15 @@ public class ApiResponseHelper { return offeringResponse; } + public static ConfigurationResponse createConfigurationResponse (ConfigurationVO cfg) { + ConfigurationResponse cfgResponse = new ConfigurationResponse(); + cfgResponse.setCategory(cfg.getCategory()); + cfgResponse.setDescription(cfg.getDescription()); + cfgResponse.setName(cfg.getName()); + cfgResponse.setValue(cfg.getValue()); + + return cfgResponse; + } + } diff --git a/server/src/com/cloud/api/commands/AddConfigCmd.java b/server/src/com/cloud/api/commands/AddConfigCmd.java index 9697520f499..33dd3ac8237 100644 --- a/server/src/com/cloud/api/commands/AddConfigCmd.java +++ b/server/src/com/cloud/api/commands/AddConfigCmd.java @@ -20,6 +20,7 @@ package com.cloud.api.commands; import org.apache.log4j.Logger; import com.cloud.api.ApiConstants; +import com.cloud.api.ApiResponseHelper; import com.cloud.api.BaseCmd; import com.cloud.api.Implementation; import com.cloud.api.Parameter; @@ -98,19 +99,13 @@ public class AddConfigCmd extends BaseCmd { @Override @SuppressWarnings("unchecked") public ConfigurationResponse getResponse() { - ConfigurationResponse response = new ConfigurationResponse(); - ConfigurationVO responseObject = (ConfigurationVO)getResponseObject(); - if (responseObject != null) { - response.setName(responseObject.getName()); - response.setValue(responseObject.getValue()); - //TODO - return description and category if needed (didn't return in 2.1 release) - + ConfigurationVO cfg = (ConfigurationVO)getResponseObject(); + if (cfg != null) { + ConfigurationResponse response = ApiResponseHelper.createConfigurationResponse(cfg); + response.setResponseName(getName()); + return response; } else { throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to add config"); } - - response.setResponseName(getName()); - return response; - //return ApiResponseSerializer.toSerializedString(response); } } diff --git a/server/src/com/cloud/api/commands/ListCfgsByCmd.java b/server/src/com/cloud/api/commands/ListCfgsByCmd.java index 282f35e7882..1949a5d237e 100644 --- a/server/src/com/cloud/api/commands/ListCfgsByCmd.java +++ b/server/src/com/cloud/api/commands/ListCfgsByCmd.java @@ -24,6 +24,7 @@ import java.util.List; import org.apache.log4j.Logger; import com.cloud.api.ApiConstants; +import com.cloud.api.ApiResponseHelper; import com.cloud.api.BaseListCmd; import com.cloud.api.Implementation; import com.cloud.api.Parameter; @@ -77,12 +78,7 @@ public class ListCfgsByCmd extends BaseListCmd { ListResponse response = new ListResponse(); List configResponses = new ArrayList(); for (ConfigurationVO cfg : configurations) { - ConfigurationResponse cfgResponse = new ConfigurationResponse(); - cfgResponse.setCategory(cfg.getCategory()); - cfgResponse.setDescription(cfg.getDescription()); - cfgResponse.setName(cfg.getName()); - cfgResponse.setValue(cfg.getValue()); - + ConfigurationResponse cfgResponse = ApiResponseHelper.createConfigurationResponse(cfg); cfgResponse.setResponseName("configuration"); configResponses.add(cfgResponse); } diff --git a/server/src/com/cloud/api/commands/UpdateCfgCmd.java b/server/src/com/cloud/api/commands/UpdateCfgCmd.java index 4b5c10a9ff4..2c6dfb13469 100644 --- a/server/src/com/cloud/api/commands/UpdateCfgCmd.java +++ b/server/src/com/cloud/api/commands/UpdateCfgCmd.java @@ -21,12 +21,14 @@ package com.cloud.api.commands; import org.apache.log4j.Logger; import com.cloud.api.ApiConstants; +import com.cloud.api.ApiResponseHelper; import com.cloud.api.BaseCmd; import com.cloud.api.Implementation; import com.cloud.api.Parameter; import com.cloud.api.ServerApiException; -import com.cloud.api.response.SuccessResponse; +import com.cloud.api.response.ConfigurationResponse; import com.cloud.configuration.ConfigurationManager; +import com.cloud.configuration.ConfigurationVO; @Implementation(method="updateConfiguration", manager=ConfigurationManager.class, description="Updates a configuration.") public class UpdateCfgCmd extends BaseCmd { @@ -64,18 +66,15 @@ public class UpdateCfgCmd extends BaseCmd { } @Override @SuppressWarnings("unchecked") - public SuccessResponse getResponse() { - SuccessResponse response = new SuccessResponse(); - Boolean responseObject = (Boolean)getResponseObject(); + public ConfigurationResponse getResponse() { + ConfigurationVO cfg = (ConfigurationVO)getResponseObject(); - if (responseObject != null) { - response.setSuccess(responseObject); - response.setDisplayText("Successfully updated configuration value."); + if (cfg != null) { + ConfigurationResponse response = ApiResponseHelper.createConfigurationResponse(cfg); + response.setResponseName(getName()); + return response; } else { throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to update config"); } - - response.setResponseName(getName()); - return response; } } diff --git a/server/src/com/cloud/configuration/ConfigurationManager.java b/server/src/com/cloud/configuration/ConfigurationManager.java index d09b7c9df20..c95e1d30f5d 100644 --- a/server/src/com/cloud/configuration/ConfigurationManager.java +++ b/server/src/com/cloud/configuration/ConfigurationManager.java @@ -41,6 +41,7 @@ import com.cloud.dc.VlanVO; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.InsufficientAddressCapacityException; import com.cloud.exception.InsufficientCapacityException; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.service.ServiceOfferingVO; import com.cloud.storage.DiskOfferingVO; import com.cloud.utils.component.Manager; @@ -63,10 +64,10 @@ public interface ConfigurationManager extends Manager { /** * Updates a configuration entry with a new value * @param cmd - the command wrapping name and value parameters - * @return true or false - * @throws , + * @return updated configuration object if successful + * @throws InvalidParameterValueException */ - boolean updateConfiguration(UpdateCfgCmd cmd); + ConfigurationVO updateConfiguration(UpdateCfgCmd cmd) throws InvalidParameterValueException; /** * Creates a new service offering @@ -298,9 +299,7 @@ public interface ConfigurationManager extends Manager { /** * Persists a config value via the API call - * @param cmd - the command that wraps instance, component, category, name, value, description parameters - * @throws , - * @return true or false + * @return newly created Config object */ - boolean addConfig(AddConfigCmd cmd); + ConfigurationVO addConfig(AddConfigCmd cmd); } diff --git a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java index 60eecf21d82..daa6fee8182 100644 --- a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java @@ -184,15 +184,23 @@ public class ConfigurationManagerImpl implements ConfigurationManager { } @Override - public boolean updateConfiguration(UpdateCfgCmd cmd) { + public ConfigurationVO updateConfiguration(UpdateCfgCmd cmd) throws InvalidParameterValueException{ Long userId = UserContext.current().getUserId(); String name = cmd.getCfgName(); String value = cmd.getValue(); + + //check if config value exists + if (_configDao.findByName(name) == null) + throw new InvalidParameterValueException("Config parameter with name " + name + " doesn't exist"); + + if (value == null) + return _configDao.findByName(name); + updateConfiguration (userId, name, value); if (_configDao.getValue(name).equalsIgnoreCase(value)) - return true; + return _configDao.findByName(name); else - return false; + throw new CloudRuntimeException("Unable to update configuration parameter" + name); } @@ -2139,11 +2147,11 @@ public class ConfigurationManagerImpl implements ConfigurationManager { } @Override - public boolean addConfig(AddConfigCmd cmd){ + public ConfigurationVO addConfig(AddConfigCmd cmd){ String category = cmd.getCategory(); String instance = cmd.getInstance(); String component = cmd.getComponent(); - String name = cmd.getName(); + String name = cmd.getConfigPropName(); String value = cmd.getValue(); String description = cmd.getDescription(); try @@ -2151,12 +2159,12 @@ public class ConfigurationManagerImpl implements ConfigurationManager { ConfigurationVO entity = new ConfigurationVO(category, instance, component, name, value, description); _configDao.persist(entity); s_logger.info("Successfully added configuration value into db: category:"+category+" instance:"+instance+" component:"+component+" name:"+name+" value:"+value); - return true; + return _configDao.findByName(name); } catch(Exception ex) { s_logger.error("Unable to add the new config entry:",ex); - return false; + throw new CloudRuntimeException("Unable to add configuration parameter" + name); } }