Fixing issue in component config subsystem where the locally registered
property value does not get properly updated on startup and thus wrongly
retains its default value.
Also update the Javadocs for the newly added ComponentConfigStore methods
to properly reflect the sparse map semantics.
Change-Id: I4be4fe66d45a6e20bd235c61b06e2ebcb81dfe5b
(cherry picked from commit a3b55a90bb6b09d80ae486bf8a53ae028db2aafa)
diff --git a/core/api/src/main/java/org/onosproject/cfg/ComponentConfigStore.java b/core/api/src/main/java/org/onosproject/cfg/ComponentConfigStore.java
index 00e8eab..c7cb4af 100644
--- a/core/api/src/main/java/org/onosproject/cfg/ComponentConfigStore.java
+++ b/core/api/src/main/java/org/onosproject/cfg/ComponentConfigStore.java
@@ -47,10 +47,11 @@
/**
- * Returns set of component configuration property names.
+ * Returns set of component configuration property names. This includes
+ * only the names of properties whose values depart from their default.
*
* @param component component name
- * @return set of property names
+ * @return set of property names whose values are set to non-default values
*/
default Set<String> getProperties(String component) {
return ImmutableSet.of();
@@ -58,9 +59,11 @@
/**
* Returns the string value of the given component configuration property.
+ * For properties whose values are set to their default this may return null.
*
* @param component component name
- * @param name property name; null if no property found
+ * @param name property name; null if no property found or if value
+ * is default
* @return set of property names
*/
default String getProperty(String component, String name) {
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 60feca0..011733c 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
@@ -335,17 +335,34 @@
String name = p.name();
String globalValue = store.getProperty(componentName, name);
String localValue = localProperties != null ? (String) localProperties.get(name) : null;
+
+ log.debug("Processing {} property {}; global {}; local {}",
+ componentName, name, globalValue, localValue);
try {
- // If the global value is the same as default, reset to
- // default locally.
- if (Objects.equals(globalValue, p.defaultValue())) {
+ // 1) If the global value is the same as default, or
+ // 2) if it is not set explicitly, but the local value is
+ // not the same as the default, reset local value to default.
+ if (Objects.equals(globalValue, p.defaultValue()) ||
+ (globalValue == null &&
+ !Objects.equals(localValue, p.defaultValue()))) {
+ log.debug("Resetting {} property {}; value same as default",
+ componentName, name);
reset(componentName, name);
} else if (!Objects.equals(globalValue, localValue)) {
- // If the local value is different from global value?
+ // If the local value is different from global value
// validate the global value and apply it locally.
+ log.debug("Setting {} property {} to {}",
+ componentName, name, globalValue);
checkValidity(componentName, name, globalValue);
set(componentName, name, globalValue);
+
+ } else {
+ // Otherwise, simply update the registered property
+ // with the global value.
+ log.debug("Syncing {} property {} to {}",
+ componentName, name, globalValue);
+ map.put(name, ConfigProperty.setProperty(p, globalValue));
}
} catch (IllegalArgumentException e) {
log.warn("Value {} is not a valid {}; using default",
@@ -356,18 +373,15 @@
} catch (IOException e) {
log.error("Unable to get configuration for " + componentName, e);
}
-
}
- // FIXME: This should be a slightly deferred execution to allow changing
- // values just once per component when a number of updates arrive shortly
- // after each other.
private void triggerUpdate(String componentName) {
try {
Configuration cfg = cfgAdmin.getConfiguration(componentName, null);
Map<String, ConfigProperty> map = properties.get(componentName);
Dictionary<String, Object> props = new Hashtable<>();
- map.values().stream().filter(p -> p.value() != null).forEach(p -> props.put(p.name(), p.value()));
+ map.values().stream().filter(p -> p.value() != null)
+ .forEach(p -> props.put(p.name(), p.value()));
cfg.update(props);
} catch (IOException e) {
log.warn("Unable to update configuration for " + componentName, e);