Committed by
Gerrit Code Review
Simplified component config loader with preset method that sets the configuratio…
…n regardless of the component state Change-Id: Ia2e987c3b6d15339396737dfa68c6973d714798c
Showing
5 changed files
with
54 additions
and
74 deletions
| ... | @@ -63,6 +63,16 @@ public interface ComponentConfigService { | ... | @@ -63,6 +63,16 @@ public interface ComponentConfigService { |
| 63 | void setProperty(String componentName, String name, String value); | 63 | void setProperty(String componentName, String name, String value); |
| 64 | 64 | ||
| 65 | /** | 65 | /** |
| 66 | + * Presets the value of the specified configuration property, regardless | ||
| 67 | + * of the component's state. | ||
| 68 | + * | ||
| 69 | + * @param componentName component name | ||
| 70 | + * @param name property name | ||
| 71 | + * @param value new property value | ||
| 72 | + */ | ||
| 73 | + void preSetProperty(String componentName, String name, String value); | ||
| 74 | + | ||
| 75 | + /** | ||
| 66 | * Clears the value of the specified configuration property thus making | 76 | * Clears the value of the specified configuration property thus making |
| 67 | * the property take on its default value. | 77 | * the property take on its default value. |
| 68 | * | 78 | * |
| ... | @@ -72,3 +82,4 @@ public interface ComponentConfigService { | ... | @@ -72,3 +82,4 @@ public interface ComponentConfigService { |
| 72 | void unsetProperty(String componentName, String name); | 82 | void unsetProperty(String componentName, String name); |
| 73 | 83 | ||
| 74 | } | 84 | } |
| 85 | + | ... | ... |
| ... | @@ -50,6 +50,10 @@ public class ComponentConfigAdapter implements ComponentConfigService { | ... | @@ -50,6 +50,10 @@ public class ComponentConfigAdapter implements ComponentConfigService { |
| 50 | } | 50 | } |
| 51 | 51 | ||
| 52 | @Override | 52 | @Override |
| 53 | + public void preSetProperty(String componentName, String name, String value) { | ||
| 54 | + } | ||
| 55 | + | ||
| 56 | + @Override | ||
| 53 | public void unsetProperty(String componentName, String name) { | 57 | public void unsetProperty(String componentName, String name) { |
| 54 | 58 | ||
| 55 | } | 59 | } | ... | ... |
| ... | @@ -18,19 +18,14 @@ package org.onosproject.cfg.impl; | ... | @@ -18,19 +18,14 @@ package org.onosproject.cfg.impl; |
| 18 | 18 | ||
| 19 | import com.fasterxml.jackson.databind.ObjectMapper; | 19 | import com.fasterxml.jackson.databind.ObjectMapper; |
| 20 | import com.fasterxml.jackson.databind.node.ObjectNode; | 20 | import com.fasterxml.jackson.databind.node.ObjectNode; |
| 21 | -import com.google.common.collect.ImmutableSet; | ||
| 22 | -import com.google.common.collect.Sets; | ||
| 23 | import org.apache.felix.scr.annotations.Activate; | 21 | import org.apache.felix.scr.annotations.Activate; |
| 24 | import org.apache.felix.scr.annotations.Component; | 22 | import org.apache.felix.scr.annotations.Component; |
| 25 | import org.apache.felix.scr.annotations.Reference; | 23 | import org.apache.felix.scr.annotations.Reference; |
| 26 | import org.apache.felix.scr.annotations.ReferenceCardinality; | 24 | import org.apache.felix.scr.annotations.ReferenceCardinality; |
| 27 | -import org.onlab.util.SharedExecutors; | ||
| 28 | import org.onosproject.cfg.ComponentConfigService; | 25 | import org.onosproject.cfg.ComponentConfigService; |
| 29 | import org.slf4j.Logger; | 26 | import org.slf4j.Logger; |
| 30 | 27 | ||
| 31 | import java.io.File; | 28 | import java.io.File; |
| 32 | -import java.util.Set; | ||
| 33 | -import java.util.TimerTask; | ||
| 34 | 29 | ||
| 35 | import static org.slf4j.LoggerFactory.getLogger; | 30 | import static org.slf4j.LoggerFactory.getLogger; |
| 36 | 31 | ||
| ... | @@ -44,50 +39,31 @@ public class ComponentConfigLoader { | ... | @@ -44,50 +39,31 @@ public class ComponentConfigLoader { |
| 44 | private static final String CFG_JSON = "../config/component-cfg.json"; | 39 | private static final String CFG_JSON = "../config/component-cfg.json"; |
| 45 | static File cfgFile = new File(CFG_JSON); | 40 | static File cfgFile = new File(CFG_JSON); |
| 46 | 41 | ||
| 47 | - protected int retryDelay = 5_000; // millis between retries | ||
| 48 | - protected int stopRetryTime = 60_000; // deadline in millis | ||
| 49 | - | ||
| 50 | private final Logger log = getLogger(getClass()); | 42 | private final Logger log = getLogger(getClass()); |
| 51 | 43 | ||
| 52 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) | 44 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) |
| 53 | protected ComponentConfigService configService; | 45 | protected ComponentConfigService configService; |
| 54 | 46 | ||
| 55 | private ObjectNode root; | 47 | private ObjectNode root; |
| 56 | - private final Set<String> pendingComponents = Sets.newHashSet(); | ||
| 57 | - private long initialTimestamp; | ||
| 58 | - | ||
| 59 | - // TimerTask object that calls the load configuration for each component | ||
| 60 | - // in the pending components set and cancels itself if the set is empty or | ||
| 61 | - // after a set period of time. | ||
| 62 | - protected final TimerTask loader = new TimerTask() { | ||
| 63 | - | ||
| 64 | - @Override | ||
| 65 | - public void run() { | ||
| 66 | - ImmutableSet.copyOf(pendingComponents) | ||
| 67 | - .forEach(k -> loadConfig(k, (ObjectNode) root.path(k))); | ||
| 68 | - if (pendingComponents.isEmpty() | ||
| 69 | - || System.currentTimeMillis() - initialTimestamp >= stopRetryTime) { | ||
| 70 | - this.cancel(); | ||
| 71 | - } | ||
| 72 | - } | ||
| 73 | - }; | ||
| 74 | 48 | ||
| 75 | @Activate | 49 | @Activate |
| 76 | protected void activate() { | 50 | protected void activate() { |
| 77 | - initialTimestamp = System.currentTimeMillis(); | ||
| 78 | this.loadConfigs(); | 51 | this.loadConfigs(); |
| 79 | log.info("Started"); | 52 | log.info("Started"); |
| 80 | } | 53 | } |
| 81 | 54 | ||
| 82 | // Loads the configurations for each component from the file in | 55 | // Loads the configurations for each component from the file in |
| 83 | - // ../config/component-cfg.json, adds them to a set and schedules a task | 56 | + // ../config/component-cfg.json, using the preSetProperty method. |
| 84 | - // to try and load them. | ||
| 85 | private void loadConfigs() { | 57 | private void loadConfigs() { |
| 86 | try { | 58 | try { |
| 87 | if (cfgFile.exists()) { | 59 | if (cfgFile.exists()) { |
| 88 | root = (ObjectNode) new ObjectMapper().readTree(cfgFile); | 60 | root = (ObjectNode) new ObjectMapper().readTree(cfgFile); |
| 89 | - root.fieldNames().forEachRemaining(pendingComponents::add); | 61 | + root.fieldNames(). |
| 90 | - SharedExecutors.getTimer().schedule(loader, 0, retryDelay); | 62 | + forEachRemaining(component -> root.path(component).fieldNames() |
| 63 | + .forEachRemaining(k -> configService | ||
| 64 | + .preSetProperty(component, k, | ||
| 65 | + root.path(component).path(k) | ||
| 66 | + .asText()))); | ||
| 91 | log.info("Loaded initial component configuration from {}", cfgFile); | 67 | log.info("Loaded initial component configuration from {}", cfgFile); |
| 92 | } | 68 | } |
| 93 | } catch (Exception e) { | 69 | } catch (Exception e) { |
| ... | @@ -95,15 +71,4 @@ public class ComponentConfigLoader { | ... | @@ -95,15 +71,4 @@ public class ComponentConfigLoader { |
| 95 | cfgFile, e); | 71 | cfgFile, e); |
| 96 | } | 72 | } |
| 97 | } | 73 | } |
| 98 | - | ||
| 99 | - // Loads a configuration for a single component and removes it from the | ||
| 100 | - // components set. | ||
| 101 | - private void loadConfig(String component, ObjectNode config) { | ||
| 102 | - if (configService.getComponentNames().contains(component)) { | ||
| 103 | - config.fieldNames() | ||
| 104 | - .forEachRemaining(k -> configService.setProperty(component, k, | ||
| 105 | - config.path(k).asText())); | ||
| 106 | - pendingComponents.remove(component); | ||
| 107 | - } | ||
| 108 | - } | ||
| 109 | } | 74 | } | ... | ... |
| ... | @@ -172,6 +172,17 @@ public class ComponentConfigManager implements ComponentConfigService { | ... | @@ -172,6 +172,17 @@ public class ComponentConfigManager implements ComponentConfigService { |
| 172 | } | 172 | } |
| 173 | 173 | ||
| 174 | @Override | 174 | @Override |
| 175 | + public void preSetProperty(String componentName, String name, String value) { | ||
| 176 | + | ||
| 177 | + checkPermission(CONFIG_WRITE); | ||
| 178 | + | ||
| 179 | + checkNotNull(componentName, COMPONENT_NULL); | ||
| 180 | + checkNotNull(name, PROPERTY_NULL); | ||
| 181 | + | ||
| 182 | + store.setProperty(componentName, name, value); | ||
| 183 | + } | ||
| 184 | + | ||
| 185 | + @Override | ||
| 175 | public void unsetProperty(String componentName, String name) { | 186 | public void unsetProperty(String componentName, String name) { |
| 176 | checkPermission(CONFIG_WRITE); | 187 | checkPermission(CONFIG_WRITE); |
| 177 | 188 | ... | ... |
| ... | @@ -21,17 +21,17 @@ import com.google.common.io.Files; | ... | @@ -21,17 +21,17 @@ import com.google.common.io.Files; |
| 21 | import org.junit.Before; | 21 | import org.junit.Before; |
| 22 | import org.junit.Test; | 22 | import org.junit.Test; |
| 23 | import org.onosproject.cfg.ComponentConfigAdapter; | 23 | import org.onosproject.cfg.ComponentConfigAdapter; |
| 24 | +import org.slf4j.Logger; | ||
| 24 | 25 | ||
| 25 | import java.io.File; | 26 | import java.io.File; |
| 26 | import java.io.IOException; | 27 | import java.io.IOException; |
| 27 | -import java.lang.reflect.Field; | ||
| 28 | import java.util.Set; | 28 | import java.util.Set; |
| 29 | -import java.util.TimerTask; | ||
| 30 | 29 | ||
| 31 | import static com.google.common.io.ByteStreams.toByteArray; | 30 | import static com.google.common.io.ByteStreams.toByteArray; |
| 32 | import static com.google.common.io.Files.write; | 31 | import static com.google.common.io.Files.write; |
| 33 | -import static org.junit.Assert.*; | 32 | +import static org.junit.Assert.assertEquals; |
| 34 | -import static org.onlab.junit.TestTools.assertAfter; | 33 | +import static org.junit.Assert.assertNull; |
| 34 | +import static org.slf4j.LoggerFactory.getLogger; | ||
| 35 | 35 | ||
| 36 | /** | 36 | /** |
| 37 | * UnitTest for ComponentLoader. | 37 | * UnitTest for ComponentLoader. |
| ... | @@ -46,6 +46,8 @@ public class ComponentConfigLoaderTest { | ... | @@ -46,6 +46,8 @@ public class ComponentConfigLoaderTest { |
| 46 | 46 | ||
| 47 | private TestConfigService service; | 47 | private TestConfigService service; |
| 48 | 48 | ||
| 49 | + private final Logger log = getLogger(getClass()); | ||
| 50 | + | ||
| 49 | /* | 51 | /* |
| 50 | * Method to SetUp the test environment with test file, a config loader a service, | 52 | * Method to SetUp the test environment with test file, a config loader a service, |
| 51 | * and assign it to the loader.configService for the test. | 53 | * and assign it to the loader.configService for the test. |
| ... | @@ -56,8 +58,6 @@ public class ComponentConfigLoaderTest { | ... | @@ -56,8 +58,6 @@ public class ComponentConfigLoaderTest { |
| 56 | loader = new ComponentConfigLoader(); | 58 | loader = new ComponentConfigLoader(); |
| 57 | service = new TestConfigService(); | 59 | service = new TestConfigService(); |
| 58 | loader.configService = service; | 60 | loader.configService = service; |
| 59 | - loader.retryDelay = 50; | ||
| 60 | - loader.stopRetryTime = 200; | ||
| 61 | } | 61 | } |
| 62 | 62 | ||
| 63 | /* | 63 | /* |
| ... | @@ -67,7 +67,7 @@ public class ComponentConfigLoaderTest { | ... | @@ -67,7 +67,7 @@ public class ComponentConfigLoaderTest { |
| 67 | public void basics() throws IOException { | 67 | public void basics() throws IOException { |
| 68 | stageTestResource("basic.json"); | 68 | stageTestResource("basic.json"); |
| 69 | loader.activate(); | 69 | loader.activate(); |
| 70 | - assertAfter(1_000, () -> assertEquals("incorrect component", FOO_COMPONENT, service.component)); | 70 | + assertEquals("incorrect component", FOO_COMPONENT, service.component); |
| 71 | } | 71 | } |
| 72 | 72 | ||
| 73 | /* | 73 | /* |
| ... | @@ -79,28 +79,7 @@ public class ComponentConfigLoaderTest { | ... | @@ -79,28 +79,7 @@ public class ComponentConfigLoaderTest { |
| 79 | public void badConfig() throws IOException { | 79 | public void badConfig() throws IOException { |
| 80 | stageTestResource("badConfig.json"); | 80 | stageTestResource("badConfig.json"); |
| 81 | loader.activate(); | 81 | loader.activate(); |
| 82 | - assertAfter(1_000, () -> assertNull("incorrect configuration", service.component)); | 82 | + assertNull("incorrect configuration", service.component); |
| 83 | - | ||
| 84 | - } | ||
| 85 | - | ||
| 86 | - /* | ||
| 87 | - * Tests that tasks stops itself after the stopRetryTime if the component was | ||
| 88 | - * not loaded. | ||
| 89 | - */ | ||
| 90 | - @Test | ||
| 91 | - public void noComponentForConfig() throws IOException { | ||
| 92 | - stageTestResource("badComponent.json"); | ||
| 93 | - loader.activate(); | ||
| 94 | - assertAfter(loader.stopRetryTime + loader.retryDelay, () -> { | ||
| 95 | - try { | ||
| 96 | - Field state = TimerTask.class.getDeclaredField("state"); | ||
| 97 | - state.setAccessible(true); | ||
| 98 | - assertEquals("incorrect component", state.getInt(loader.loader), 3); | ||
| 99 | - } catch (Exception e) { | ||
| 100 | - e.printStackTrace(); | ||
| 101 | - fail(); | ||
| 102 | - } | ||
| 103 | - }); | ||
| 104 | 83 | ||
| 105 | } | 84 | } |
| 106 | 85 | ||
| ... | @@ -117,9 +96,9 @@ public class ComponentConfigLoaderTest { | ... | @@ -117,9 +96,9 @@ public class ComponentConfigLoaderTest { |
| 117 | */ | 96 | */ |
| 118 | private class TestConfigService extends ComponentConfigAdapter { | 97 | private class TestConfigService extends ComponentConfigAdapter { |
| 119 | 98 | ||
| 120 | - private String component; | 99 | + protected String component; |
| 121 | - private String name; | 100 | + protected String name; |
| 122 | - private String value; | 101 | + protected String value; |
| 123 | 102 | ||
| 124 | @Override | 103 | @Override |
| 125 | public Set<String> getComponentNames() { | 104 | public Set<String> getComponentNames() { |
| ... | @@ -127,7 +106,17 @@ public class ComponentConfigLoaderTest { | ... | @@ -127,7 +106,17 @@ public class ComponentConfigLoaderTest { |
| 127 | } | 106 | } |
| 128 | 107 | ||
| 129 | @Override | 108 | @Override |
| 109 | + public void preSetProperty(String componentName, String name, String value) { | ||
| 110 | + log.info("preSet"); | ||
| 111 | + this.component = componentName; | ||
| 112 | + this.name = name; | ||
| 113 | + this.value = value; | ||
| 114 | + | ||
| 115 | + } | ||
| 116 | + | ||
| 117 | + @Override | ||
| 130 | public void setProperty(String componentName, String name, String value) { | 118 | public void setProperty(String componentName, String name, String value) { |
| 119 | + log.info("Set"); | ||
| 131 | this.component = componentName; | 120 | this.component = componentName; |
| 132 | this.name = name; | 121 | this.name = name; |
| 133 | this.value = value; | 122 | this.value = value; | ... | ... |
-
Please register or login to post a comment