ONOS-3725 Adding ability to retain pending configs.

This allows up-load of configurations before their backing classes are registered by apps/subsystems.

 Validation and delegation of network config change events is deferred until the class registration.

Change-Id: Ifc9c97fbc86e764cb03cecb1f73f7191de3e7754

@@ -135,6 +135,28 @@ public interface NetworkConfigService
                                            JsonNode json);
 
     /**
+     * Applies configuration for the specified subject and configuration
+     * key using the raw JSON object. If configuration already exists, it
+     * will be updated. If the specified configuration key does not yet have
+     * a registered class associated with it, the configuration will be pending
+     * and null value will be returned. Once the backing configuration class is
+     * registered, the configuration will be validated and accepted.
+     *
+     * @param subjectClassKey subject class key
+     * @param subject         configuration subject
+     * @param configKey       configuration class key
+     * @param json            raw JSON node containing the configuration data
+     * @param <S>             type of subject
+     * @param <C>             type of configuration
+     * @return configuration object or null if configuration key does not have
+     *                                  a registered class yet
+     * @throws IllegalArgumentException if the supplied JSON node contains
+     *                                  invalid data
+     */
+    <S, C extends Config<S>> C applyConfig(String subjectClassKey, S subject,
+                                           String configKey, JsonNode json);
+
+    /**
      * Clears any configuration for the specified subject and configuration
      * class. If one does not exist, this call has no effect.
      *
@@ -145,4 +167,13 @@ public interface NetworkConfigService
      */
     <S, C extends Config<S>> void removeConfig(S subject, Class<C> configClass);
 
+    /**
+     * Clears any configuration for the specified subject and configuration
+     * key. If one does not exist, this call has no effect.
+     *
+     * @param subject     configuration subject
+     * @param configKey   configuration key
+     * @param <S>         type of subject
+     */
+    <S> void removeConfig(String subjectClassKey, S subject, String configKey);
 }

@@ -129,4 +129,26 @@ public interface NetworkConfigStore extends Store<NetworkConfigEvent, NetworkCon
      */
     <S, C extends Config<S>> void clearConfig(S subject, Class<C> configClass);
 
+    /**
+     * Queues pending configuration for the specified subject and configuration
+     * class using the raw JSON object.
+     *
+     * @param subject   configuration subject
+     * @param configKey configuration key
+     * @param json      raw JSON node containing the configuration data
+     * @param <S>       type of subject
+     * @throws IllegalArgumentException if the supplied JSON node contains
+     *                                  invalid data
+     */
+    <S> void queueConfig(S subject, String configKey, JsonNode json);
+
+    /**
+     * Clears the configuration of the given class for the specified subject.
+     *
+     * @param subject   configuration subject
+     * @param configKey configuration key
+     * @param <S>       type of subject
+     */
+    <S> void clearQueuedConfig(S subject, String configKey);
+
 }

@@ -74,17 +74,24 @@ public class NetworkConfigServiceAdapter implements NetworkConfigService {
     }
 
     @Override
+    public <S, C extends Config<S>> C applyConfig(String subjectClassKey, S subject, String configKey, JsonNode json) {
+        return null;
+    }
+
+    @Override
     public <S, C extends Config<S>> void removeConfig(S subject, Class<C> configClass) {
 
     }
 
     @Override
-    public void addListener(NetworkConfigListener listener) {
+    public <S> void removeConfig(String subjectClassKey, S subject, String configKey) {
+    }
 
+    @Override
+    public void addListener(NetworkConfigListener listener) {
     }
 
     @Override
     public void removeListener(NetworkConfigListener listener) {
-
     }
 }

@@ -56,8 +56,11 @@ public class NetworkConfigManager
 
     private static final String NULL_FACTORY_MSG = "Factory cannot be null";
     private static final String NULL_SCLASS_MSG = "Subject class cannot be null";
+    private static final String NULL_SKEY_MSG = "Subject key cannot be null";
     private static final String NULL_CCLASS_MSG = "Config class cannot be null";
+    private static final String NULL_CKEY_MSG = "Config key cannot be null";
     private static final String NULL_SUBJECT_MSG = "Subject cannot be null";
+    private static final String NULL_JSON_MSG = "JSON cannot be null";
 
     // Inventory of configuration factories
     private final Map<ConfigKey, ConfigFactory> factories = Maps.newConcurrentMap();
@@ -156,6 +159,8 @@ public class NetworkConfigManager
 
     @Override
     public Class<? extends Config> getConfigClass(String subjectClassKey, String configKey) {
+        checkNotNull(subjectClassKey, NULL_SKEY_MSG);
+        checkNotNull(configKey, NULL_CKEY_MSG);
         return configClasses.get(new ConfigIdentifier(subjectClassKey, configKey));
     }
 
@@ -182,7 +187,7 @@ public class NetworkConfigManager
     }
 
     @Override
-    public <S, T extends Config<S>> T getConfig(S subject, Class<T> configClass) {
+    public <S, C extends Config<S>> C getConfig(S subject, Class<C> configClass) {
         checkNotNull(subject, NULL_SUBJECT_MSG);
         checkNotNull(configClass, NULL_CCLASS_MSG);
         return store.getConfig(subject, configClass);
@@ -200,16 +205,47 @@ public class NetworkConfigManager
     public <S, C extends Config<S>> C applyConfig(S subject, Class<C> configClass, JsonNode json) {
         checkNotNull(subject, NULL_SUBJECT_MSG);
         checkNotNull(configClass, NULL_CCLASS_MSG);
+        checkNotNull(subject, NULL_JSON_MSG);
         return store.applyConfig(subject, configClass, json);
     }
 
     @Override
+    @SuppressWarnings("unchecked")
+    public <S, C extends Config<S>> C applyConfig(String subjectClassKey, S subject,
+                                                  String configKey, JsonNode json) {
+        checkNotNull(subjectClassKey, NULL_SKEY_MSG);
+        checkNotNull(subject, NULL_SUBJECT_MSG);
+        checkNotNull(configKey, NULL_CKEY_MSG);
+        checkNotNull(subject, NULL_JSON_MSG);
+        Class<? extends Config> configClass = configClasses.get(new ConfigIdentifier(subjectClassKey, configKey));
+        if (configClass != null) {
+            return store.applyConfig(subject, (Class<C>) configClass, json);
+        } else {
+            store.queueConfig(subject, configKey, json);
+            return null;
+        }
+    }
+
+    @Override
     public <S, C extends Config<S>> void removeConfig(S subject, Class<C> configClass) {
         checkNotNull(subject, NULL_SUBJECT_MSG);
         checkNotNull(configClass, NULL_CCLASS_MSG);
         store.clearConfig(subject, configClass);
     }
 
+    @Override
+    public <S> void removeConfig(String subjectClassKey, S subject, String configKey) {
+        checkNotNull(subjectClassKey, NULL_SKEY_MSG);
+        checkNotNull(subject, NULL_SUBJECT_MSG);
+        checkNotNull(configKey, NULL_CCLASS_MSG);
+        Class<? extends Config> configClass = configClasses.get(new ConfigIdentifier(subjectClassKey, configKey));
+        if (configClass != null) {
+            store.clearConfig(subject, configClass);
+        } else {
+            store.clearQueuedConfig(subject, configKey);
+         }
+    }
+
     // Auxiliary store delegate to receive notification about changes in
     // the network configuration store state - by the store itself.
     private class InternalStoreDelegate implements NetworkConfigStoreDelegate {
@@ -259,17 +295,17 @@ public class NetworkConfigManager
     }
 
     static final class ConfigIdentifier {
-        final String subjectKey;
+        final String subjectClassKey;
         final String configKey;
 
-        protected ConfigIdentifier(String subjectKey, String configKey) {
-            this.subjectKey = subjectKey;
+        protected ConfigIdentifier(String subjectClassKey, String configKey) {
+            this.subjectClassKey = subjectClassKey;
             this.configKey = configKey;
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(subjectKey, configKey);
+            return Objects.hash(subjectClassKey, configKey);
         }
 
         @Override
@@ -279,10 +315,11 @@ public class NetworkConfigManager
             }
             if (obj instanceof ConfigIdentifier) {
                 final ConfigIdentifier other = (ConfigIdentifier) obj;
-                return Objects.equals(this.subjectKey, other.subjectKey)
+                return Objects.equals(this.subjectClassKey, other.subjectClassKey)
                         && Objects.equals(this.configKey, other.configKey);
             }
             return false;
         }
     }
+
 }

@@ -29,7 +29,7 @@ import com.fasterxml.jackson.databind.node.ShortNode;
 import com.fasterxml.jackson.databind.node.TextNode;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
-
+import com.google.common.collect.Sets;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
@@ -117,10 +117,35 @@ public class DistributedNetworkConfigStore
     @Override
     public void addConfigFactory(ConfigFactory configFactory) {
         factoriesByConfig.put(configFactory.configClass().getName(), configFactory);
+        processPendingConfigs(configFactory);
         notifyDelegate(new NetworkConfigEvent(CONFIG_REGISTERED, configFactory.configKey(),
                                               configFactory.configClass()));
     }
 
+    // Sweep through any pending configurations, validate them and then prune them.
+    private void processPendingConfigs(ConfigFactory configFactory) {
+        Set<ConfigKey> toBePruned = Sets.newHashSet();
+        configs.keySet().forEach(k -> {
+            if (Objects.equals(k.configKey, configFactory.configKey())) {
+                validateConfig(k, configFactory, configs.get(k).value());
+                toBePruned.add(k); // Prune whether valid or not
+            }
+        });
+        toBePruned.forEach(configs::remove);
+    }
+
+    @SuppressWarnings("unchecked")
+    private void validateConfig(ConfigKey key, ConfigFactory configFactory, JsonNode json) {
+        Config config = createConfig(key.subject, configFactory.configClass(), json);
+        try {
+            checkArgument(config.isValid(), INVALID_CONFIG_JSON);
+            configs.putAndGet(key(key.subject, configFactory.configClass()), json);
+        } catch (Exception e) {
+            log.warn("Failed to validate pending {} configuration for {}: {}",
+                     key.configKey, configFactory.subjectFactory().subjectKey(key.subject), json);
+        }
+    }
+
     @Override
     public void removeConfigFactory(ConfigFactory configFactory) {
         factoriesByConfig.remove(configFactory.configClass().getName());
@@ -152,7 +177,7 @@ public class DistributedNetworkConfigStore
         ImmutableSet.Builder<S> builder = ImmutableSet.builder();
         String cName = configClass.getName();
         configs.keySet().forEach(k -> {
-            if (subjectClass.isInstance(k.subject) && cName.equals(k.configClass)) {
+            if (subjectClass.isInstance(k.subject) && Objects.equals(cName, k.configClass)) {
                 builder.add((S) k.subject);
             }
         });
@@ -164,7 +189,7 @@ public class DistributedNetworkConfigStore
     public <S> Set<Class<? extends Config<S>>> getConfigClasses(S subject) {
         ImmutableSet.Builder<Class<? extends Config<S>>> builder = ImmutableSet.builder();
         configs.keySet().forEach(k -> {
-            if (Objects.equals(subject, k.subject) && delegate != null) {
+            if (Objects.equals(subject, k.subject) && k.configClass != null && delegate != null) {
                 builder.add(factoriesByConfig.get(k.configClass).configClass());
             }
         });
@@ -201,8 +226,12 @@ public class DistributedNetworkConfigStore
 
         // Re-create the config if for some reason what we attempted to put
         // was supplanted by someone else already.
-        return versioned.value() == json ? config :
-                createConfig(subject, configClass, versioned.value());
+        return versioned.value() == json ? config : createConfig(subject, configClass, versioned.value());
+    }
+
+    @Override
+    public <S> void queueConfig(S subject, String configKey, JsonNode json) {
+        configs.put(key(subject, configKey), json);
     }
 
     @Override
@@ -210,6 +239,11 @@ public class DistributedNetworkConfigStore
         configs.remove(key(subject, configClass));
     }
 
+    @Override
+    public <S> void clearQueuedConfig(S subject, String configKey) {
+        configs.remove(key(subject, configKey));
+    }
+
     /**
      * Produces a config from the specified subject, config class and raw JSON.
      *
@@ -248,19 +282,35 @@ public class DistributedNetworkConfigStore
         return new ConfigKey(subject, configClass);
     }
 
+    // Produces a key for uniquely tracking a subject config.
+    private static ConfigKey key(Object subject, String configKey) {
+        return new ConfigKey(subject, configKey);
+    }
+
     // Auxiliary key to track subject configurations.
+    // Keys with non-null configKey are pending configurations.
     private static final class ConfigKey {
         final Object subject;
+        final String configKey;
         final String configClass;
 
+        // Create a key for pending configuration class
+        private ConfigKey(Object subject, String configKey) {
+            this.subject = subject;
+            this.configKey = configKey;
+            this.configClass = null;
+        }
+
+        // Create a key for registered class configuration
         private ConfigKey(Object subject, Class<?> configClass) {
             this.subject = subject;
+            this.configKey = null;
             this.configClass = configClass.getName();
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(subject, configClass);
+            return Objects.hash(subject, configKey, configClass);
         }
 
         @Override
@@ -271,6 +321,7 @@ public class DistributedNetworkConfigStore
             if (obj instanceof ConfigKey) {
                 final ConfigKey other = (ConfigKey) obj;
                 return Objects.equals(this.subject, other.subject)
+                        && Objects.equals(this.configKey, other.configKey)
                         && Objects.equals(this.configClass, other.configClass);
             }
             return false;
@@ -280,6 +331,11 @@ public class DistributedNetworkConfigStore
     private class InternalMapListener implements MapEventListener<ConfigKey, JsonNode> {
         @Override
         public void event(MapEvent<ConfigKey, JsonNode> event) {
+            // Do not delegate pending configs.
+            if (event.key().configClass == null) {
+                return;
+            }
+
             NetworkConfigEvent.Type type;
             switch (event.type()) {
                 case INSERT:

@@ -62,9 +62,8 @@ public class DistributedNetworkConfigStoreTest {
      * Config factory class for testing.
      */
     public class MockConfigFactory extends ConfigFactory<String, BasicConfig> {
-        protected MockConfigFactory(SubjectFactory<String> subjectFactory,
-                                Class<BasicConfig> configClass, String configKey) {
-            super(subjectFactory, configClass, configKey);
+        protected MockConfigFactory(Class<BasicConfig> configClass, String configKey) {
+            super(new MockSubjectFactory("strings"), configClass, configKey);
         }
         @Override
         public BasicConfig createConfig() {
@@ -73,11 +72,25 @@ public class DistributedNetworkConfigStoreTest {
     }
 
     /**
+     * Subject factory class for testing.
+     */
+    public class MockSubjectFactory extends SubjectFactory<String> {
+        protected MockSubjectFactory(String subjectClassKey) {
+            super(String.class, subjectClassKey);
+        }
+
+        @Override
+        public String createSubject(String subjectKey) {
+            return subjectKey;
+        }
+    }
+
+    /**
      * Tests creation, query and removal of a config.
      */
     @Test
     public void testCreateConfig() {
-        configStore.addConfigFactory(new MockConfigFactory(null, BasicConfig.class, "config1"));
+        configStore.addConfigFactory(new MockConfigFactory(BasicConfig.class, "config1"));
 
         configStore.createConfig("config1", BasicConfig.class);
         assertThat(configStore.getConfigClasses("config1"), hasSize(1));
@@ -101,7 +114,7 @@ public class DistributedNetworkConfigStoreTest {
      */
     @Test
     public void testCreateFactory() {
-        MockConfigFactory mockFactory = new MockConfigFactory(null, BasicConfig.class, "config1");
+        MockConfigFactory mockFactory = new MockConfigFactory(BasicConfig.class, "config1");
 
         assertThat(configStore.getConfigFactory(BasicConfig.class), nullValue());
 
@@ -117,7 +130,7 @@ public class DistributedNetworkConfigStoreTest {
      */
     @Test
     public void testApplyConfig() {
-        configStore.addConfigFactory(new MockConfigFactory(null, BasicConfig.class, "config1"));
+        configStore.addConfigFactory(new MockConfigFactory(BasicConfig.class, "config1"));
 
         configStore.applyConfig("config1", BasicConfig.class, new ObjectMapper().createObjectNode());
         assertThat(configStore.getConfigClasses("config1"), hasSize(1));

@@ -264,8 +264,9 @@ public class NetworkConfigWebResource extends AbstractWebResource {
                            InputStream request) throws IOException {
         NetworkConfigService service = get(NetworkConfigService.class);
         ObjectNode root = (ObjectNode) mapper().readTree(request);
-        service.applyConfig(service.getSubjectFactory(subjectClassKey).createSubject(subjectKey),
-                            service.getConfigClass(subjectClassKey, configKey), root);
+        service.applyConfig(subjectClassKey,
+                            service.getSubjectFactory(subjectClassKey).createSubject(subjectKey),
+                            configKey, root);
         return Response.ok().build();
     }
 
@@ -279,13 +280,14 @@ public class NetworkConfigWebResource extends AbstractWebResource {
 
     private void consumeSubjectJson(NetworkConfigService service,
                                     ObjectNode subjectNode, Object subject,
-                                    String subjectKey) {
-        subjectNode.fieldNames().forEachRemaining(c ->
-            service.applyConfig(subject, service.getConfigClass(subjectKey, c),
-                                subjectNode.path(c)));
+                                    String subjectClassKey) {
+        subjectNode.fieldNames().forEachRemaining(configKey ->
+            service.applyConfig(subjectClassKey, subject, configKey, subjectNode.path(configKey)));
     }
 
 
+    // FIXME: Refactor to allow queued configs to be removed
+
     /**
      * Clear entire network configuration base.
      *

@@ -56,6 +56,7 @@ import static org.junit.Assert.fail;
  */
 public class NetworkConfigWebResourceTest extends ResourceTest {
 
+
     MockNetworkConfigService mockNetworkConfigService;
 
     public class MockDeviceConfig extends Config<Device> {
@@ -201,7 +202,7 @@ public class NetworkConfigWebResourceTest extends ResourceTest {
         mockNetworkConfigService = new MockNetworkConfigService();
         ServiceDirectory testDirectory =
                 new TestServiceDirectory()
-                .add(NetworkConfigService.class, mockNetworkConfigService);
+                        .add(NetworkConfigService.class, mockNetworkConfigService);
         BaseResource.setServiceDirectory(testDirectory);
     }