[ONOS-6354] CFG values and applied values are mismatching while trying to set a property with an invalid value
Change-Id: Id2e343c61419441d0b9554df3291b35829e850ab
(cherry picked from commit c86d95f969864664beb3948bc5ff2c455e372393)
diff --git a/cli/src/main/java/org/onosproject/cli/cfg/ComponentConfigCommand.java b/cli/src/main/java/org/onosproject/cli/cfg/ComponentConfigCommand.java
index 64f6b50..0a85ae0 100644
--- a/cli/src/main/java/org/onosproject/cli/cfg/ComponentConfigCommand.java
+++ b/cli/src/main/java/org/onosproject/cli/cfg/ComponentConfigCommand.java
@@ -72,20 +72,24 @@
@Override
protected void execute() {
service = get(ComponentConfigService.class);
- if (isNullOrEmpty(command)) {
- listComponents();
- } else if (command.equals(GET) && isNullOrEmpty(component)) {
- listAllComponentsProperties();
- } else if (command.equals(GET) && isNullOrEmpty(name)) {
- listComponentProperties(component);
- } else if (command.equals(GET)) {
- listComponentProperty(component, name);
- } else if (command.equals(SET) && isNullOrEmpty(value)) {
- service.unsetProperty(component, name);
- } else if (command.equals(SET)) {
- service.setProperty(component, name, value);
- } else {
- error("Illegal usage");
+ try {
+ if (isNullOrEmpty(command)) {
+ listComponents();
+ } else if (command.equals(GET) && isNullOrEmpty(component)) {
+ listAllComponentsProperties();
+ } else if (command.equals(GET) && isNullOrEmpty(name)) {
+ listComponentProperties(component);
+ } else if (command.equals(GET)) {
+ listComponentProperty(component, name);
+ } else if (command.equals(SET) && isNullOrEmpty(value)) {
+ service.unsetProperty(component, name);
+ } else if (command.equals(SET)) {
+ service.setProperty(component, name, value);
+ } else {
+ error("Illegal usage");
+ }
+ } catch (IllegalArgumentException e) {
+ error(e.getMessage());
}
}
diff --git a/core/net/src/main/java/org/onosproject/cfg/impl/ComponentConfigManager.java b/core/net/src/main/java/org/onosproject/cfg/impl/ComponentConfigManager.java
index 144c0e7..9fd99f8 100644
--- a/core/net/src/main/java/org/onosproject/cfg/impl/ComponentConfigManager.java
+++ b/core/net/src/main/java/org/onosproject/cfg/impl/ComponentConfigManager.java
@@ -168,6 +168,7 @@
COMPONENT_MISSING, componentName);
checkArgument(properties.get(componentName).containsKey(name),
PROPERTY_MISSING, name, componentName);
+ checkValidity(componentName, name, value);
store.setProperty(componentName, name, value);
}
@@ -178,7 +179,7 @@
checkNotNull(componentName, COMPONENT_NULL);
checkNotNull(name, PROPERTY_NULL);
-
+ checkValidity(componentName, name, value);
store.setProperty(componentName, name, value);
}
@@ -281,6 +282,48 @@
}
}
+ // Checks whether the value of the specified configuration property is a valid one or not.
+ private void checkValidity(String componentName, String name, String newValue) {
+ Map<String, ConfigProperty> map = properties.get(componentName);
+ if (map == null) {
+ return;
+ }
+ ConfigProperty prop = map.get(name);
+ ConfigProperty.Type type = prop.type();
+ try {
+ switch (type) {
+ case INTEGER:
+ Integer.parseInt(newValue);
+ break;
+ case LONG:
+ Long.parseLong(newValue);
+ break;
+ case FLOAT:
+ Float.parseFloat(newValue);
+ break;
+ case DOUBLE:
+ Double.parseDouble(newValue);
+ break;
+ case BOOLEAN:
+ if (!((newValue != null) && (newValue.equalsIgnoreCase("true") ||
+ newValue.equalsIgnoreCase("false")))) {
+ throw new IllegalArgumentException("Invalid " + type + " value");
+ }
+ break;
+ case STRING:
+ case BYTE:
+ //do nothing
+ break;
+ default:
+ log.warn("Not a valid config property type");
+ break;
+
+ }
+ } catch (NumberFormatException e) {
+ throw new NumberFormatException("Invalid " + type + " value");
+ }
+ }
+
// Loads existing property values that may have been set.
private void loadExistingValues(String componentName) {
try {
@@ -292,8 +335,14 @@
while (it.hasMoreElements()) {
String name = it.nextElement();
ConfigProperty p = map.get(name);
- if (p != null) {
- map.put(name, ConfigProperty.setProperty(p, (String) props.get(name)));
+ try {
+ if (p != null) {
+ checkValidity(componentName, name, (String) props.get(name));
+ map.put(name, ConfigProperty.setProperty(p, (String) props.get(name)));
+ }
+ } catch (IllegalArgumentException e) {
+ log.warn("Default values will be applied " +
+ "since presetted value is not a valid " + p.type());
}
}
}
diff --git a/web/api/src/main/java/org/onosproject/rest/resources/ComponentConfigWebResource.java b/web/api/src/main/java/org/onosproject/rest/resources/ComponentConfigWebResource.java
index 5751e35..ad45e50 100644
--- a/web/api/src/main/java/org/onosproject/rest/resources/ComponentConfigWebResource.java
+++ b/web/api/src/main/java/org/onosproject/rest/resources/ComponentConfigWebResource.java
@@ -15,6 +15,7 @@
*/
package org.onosproject.rest.resources;
+import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.onosproject.cfg.ComponentConfigService;
import org.onosproject.cfg.ConfigProperty;
@@ -33,6 +34,8 @@
import javax.ws.rs.core.Response;
import java.io.IOException;
import java.io.InputStream;
+import java.util.List;
+import java.util.ArrayList;
import java.util.Set;
import static org.onlab.util.Tools.nullIsNotFound;
@@ -42,6 +45,7 @@
*/
@Path("configuration")
public class ComponentConfigWebResource extends AbstractWebResource {
+ private static final int MULTI_STATUS_RESPONE = 207;
/**
* Gets all component configurations.
@@ -102,16 +106,36 @@
InputStream request) throws IOException {
ComponentConfigService service = get(ComponentConfigService.class);
ObjectNode props = (ObjectNode) mapper().readTree(request);
+ List<String> errorMsgs = new ArrayList<String>();
if (preset) {
- props.fieldNames().forEachRemaining(k ->
- service.preSetProperty(component, k, props.path(k).asText()));
+ props.fieldNames().forEachRemaining(k -> {
+ try {
+ service.preSetProperty(component, k, props.path(k).asText());
+ } catch (IllegalArgumentException e) {
+ errorMsgs.add(e.getMessage());
+ }
+ });
} else {
- props.fieldNames().forEachRemaining(k ->
- service.setProperty(component, k, props.path(k).asText()));
+ props.fieldNames().forEachRemaining(k -> {
+ try {
+ service.setProperty(component, k, props.path(k).asText());
+ } catch (IllegalArgumentException e) {
+ errorMsgs.add(e.getMessage());
+ }
+ });
+ }
+ if (!errorMsgs.isEmpty()) {
+ return Response.status(MULTI_STATUS_RESPONE).entity(produceErrorJson(errorMsgs)).build();
}
return Response.ok().build();
}
+ private ObjectNode produceErrorJson(List<String> errorMsgs) {
+ ObjectMapper mapper = new ObjectMapper();
+ ObjectNode result = mapper.createObjectNode().put("code", 207).putPOJO("message", errorMsgs);
+ return result;
+ }
+
/**
* Selectively clears configuration properties.
* Clears only the properties present in the JSON request.