[1975] Implement the loading of configuration

The loading was moved from constructor to separate function, because:
 * It is not possible to call virtual functions from constructor (it is,
   but they are not called in the virtual manner). It is a problem for
   test mock purposes.
 * It is possible to call the function multiple times on the same
   object, to reuse the same object.

Updated tests accordingly.

diff --git a/src/lib/datasrc/container.cc b/src/lib/datasrc/container.cc
index 2bde4a19cfc8f9cc90130e8e6e2866be7905280d..9455c0d1bfad3f32f7a2d08e03dbaa3bc6058b7f 100644 (file)
--- a/src/lib/datasrc/container.cc
+++ b/src/lib/datasrc/container.cc
@@ -25,10 +25,44 @@ using namespace std;
 namespace isc {
 namespace datasrc {
 
-ConfigurableContainer::ConfigurableContainer(const ConstElementPtr&, bool,
-                                             const ConstContainerPtr&)
-{
-    // TODO: Implement
+void
+ConfigurableContainer::configure(const ConstElementPtr& config, bool) {
+    // TODO: Implement the cache
+    // TODO: Implement recyclation from the old configuration.
+    size_t i(0); // Outside of the try to be able to access it in the catch
+    try {
+        vector<DataSourceInfo> new_data_sources;
+        for (; i < config->size(); ++i) {
+            // Extract the parameters
+            const ConstElementPtr dconf(config->get(i));
+            const ConstElementPtr typeElem(dconf->get("type"));
+            if (typeElem == ConstElementPtr()) {
+                isc_throw(ConfigurationError, "Missing the type option in "
+                          "data source no " << i);
+            }
+            const string type(typeElem->stringValue());
+            ConstElementPtr paramConf(dconf->get("params"));
+            if (paramConf == ConstElementPtr()) {
+                paramConf.reset(new NullElement());
+            }
+            // TODO: Special-case the master files type.
+            // Ask the factory to create the data source for us
+            const DataSourcePair ds(this->getDataSource(type, paramConf));
+            // And put it into the vector
+            DataSourceInfo info;
+            info.data_src_ = ds.first;
+            info.container_ = ds.second;
+            new_data_sources.push_back(info);
+        }
+        // If everything is OK up until now, we have the new configuration
+        // ready. So just put it there and let the old one die when we exit
+        // the scope.
+        data_sources_.swap(new_data_sources);
+    }
+    catch (const TypeError& te) {
+        isc_throw(ConfigurationError, "Malformed configuration at data source "
+                  "no. " << i << ": " << te.what());
+    }
 }
 
 Container::SearchResult

diff --git a/src/lib/datasrc/container.h b/src/lib/datasrc/container.h
index bba5e75f4b46d90b7734133dfae53f4b39177929..b8492e0ac3141e807aab7915ac7a83dedaa3f0f8 100644 (file)
--- a/src/lib/datasrc/container.h
+++ b/src/lib/datasrc/container.h
@@ -181,11 +181,14 @@ public:
             Exception(file, line, what)
         { }
     };
-    /// \brief Constructor.
+    /// \brief Sets the configuration.
     ///
-    /// This creates the container and fills it with data sources corresponding
-    /// to the configuration. The data sources are newly created or taken from
-    /// the container passed as old.
+    /// This fills the Container with data sources corresponding to the
+    /// configuration. The data sources are newly created or recycled from
+    /// previous configuration.
+    ///
+    /// If any error is detected, an exception is thrown and the current
+    /// configuration is preserved.
     ///
     /// \param configuration The JSON element describing the configuration to
     ///     use.
@@ -193,17 +196,11 @@ public:
     ///     configuration is used and some zones are cached into an In-Memory
     ///     data source according to it. If it is false, it is ignored and
     ///     no In-Memory data sources are created.
-    /// \param old This can be set to a previous container. It will be used as
-    ///     a source of data sources that were already created in the previous
-    ///     configuration. The designed use is when there's an update to
-    ///     configuration, so not all things would have to be re-created from
-    ///     scratch. Note that the old data source must not be used any more.
     /// \throw DataSourceError if there's a problem creating a data source.
     /// \throw ConfigurationError if the configuration is invalid in some
     ///     sense.
-    ConfigurableContainer(const data::ConstElementPtr& configuration,
-                          bool allow_cache,
-                          const ConstContainerPtr& old = ConstContainerPtr());
+    void configure(const data::ConstElementPtr& configuration,
+                   bool allow_cache);
     /// \brief Implementation of the Container::search.
     virtual SearchResult search(const dns::Name& zone,
                                 bool want_exact_match = false,

diff --git a/src/lib/datasrc/tests/container_unittest.cc b/src/lib/datasrc/tests/container_unittest.cc
index 1fa27185a2c78271d7e5ef0f661e43c067b59de9..8ff13173360541279ed931acdc4781f14d9a6e33 100644 (file)
--- a/src/lib/datasrc/tests/container_unittest.cc
+++ b/src/lib/datasrc/tests/container_unittest.cc
@@ -113,10 +113,6 @@ private:
 // some methods to dig directly in the internals, for the tests.
 class TestedContainer : public ConfigurableContainer {
 public:
-    TestedContainer(const ConstElementPtr& configuration,
-                    bool allow_cache) :
-        ConfigurableContainer(configuration, allow_cache)
-    { }
     DataSources& dataSources() { return (data_sources_); }
     // Overwrite the containers method to get a data source with given type
     // and configuration. We mock the data source and don't create the
@@ -162,8 +158,13 @@ class ContainerTest : public ::testing::Test {
 public:
     ContainerTest() :
         // The empty list corresponds to a container with no elements inside
-        container_(new TestedContainer(ConstElementPtr(new ListElement()),
-                                       true))
+        container_(new TestedContainer()),
+        config_elem_(Element::fromJSON("["
+            "{"
+            "   \"type\": \"test_type\","
+            "   \"cache\": \"off\","
+            "   \"params\": {}"
+            "}]"))
     {
         for (size_t i(0); i < ds_count; ++ i) {
             shared_ptr<TestDS> ds(new TestDS(ds_zones[i]));
@@ -229,6 +230,7 @@ public:
     const Container::SearchResult negativeResult_;
     vector<shared_ptr<TestDS> > ds_;
     vector<ConfigurableContainer::DataSourceInfo> ds_info_;
+    const ConstElementPtr config_elem_;
 };
 
 // Test the test itself
@@ -341,7 +343,7 @@ TEST_F(ContainerTest, multiBestMatch) {
 // Check the configuration is empty when the list is empty
 TEST_F(ContainerTest, configureEmpty) {
     ConstElementPtr elem(new ListElement);
-    container_.reset(new TestedContainer(elem, true));
+    container_->configure(elem, true);
     EXPECT_TRUE(container_->dataSources().empty());
 }
 
@@ -359,7 +361,7 @@ TEST_F(ContainerTest, configureMulti) {
         "   \"params\": {}"
         "}]"
     ));
-    container_.reset(new TestedContainer(elem, true));
+    container_->configure(elem, true);
     EXPECT_EQ(2, container_->dataSources().size());
     checkDS(0, "type1", "{}");
     checkDS(1, "type2", "{}");
@@ -385,7 +387,7 @@ TEST_F(ContainerTest, configureParams) {
             "   \"cache\": \"off\","
             "   \"params\": ") + *param +
             "}]"));
-        container_.reset(new TestedContainer(elem, true));
+        container_->configure(elem, true);
         EXPECT_EQ(1, container_->dataSources().size());
         checkDS(0, "t", *param);
     }
@@ -412,11 +414,16 @@ TEST_F(ContainerTest, wrongConfig) {
         // TODO: Once cache is supported, add some invalid cache values
         NULL
     };
+    // Put something inside to see it survives the exception
+    container_->configure(config_elem_, true);
+    checkDS(0, "test_type", "{}");
     for (const char** config(configs); *config; ++config) {
         SCOPED_TRACE(*config);
         ConstElementPtr elem(Element::fromJSON(*config));
-        EXPECT_THROW(TestedContainer(elem, true),
+        EXPECT_THROW(container_->configure(elem, true),
                      ConfigurableContainer::ConfigurationError);
+        // Still untouched
+        checkDS(0, "test_type", "{}");
     }
 }
 
@@ -426,7 +433,7 @@ TEST_F(ContainerTest, defaults) {
         "{"
         "   \"type\": \"type1\""
         "}]"));
-    container_.reset(new TestedContainer(elem, true));
+    container_->configure(elem, true);
     EXPECT_EQ(1, container_->dataSources().size());
     checkDS(0, "type1", "null");
 }
@@ -437,7 +444,10 @@ TEST_F(ContainerTest, dataSrcError) {
         "{"
         "   \"type\": \"error\""
         "}]"));
-    EXPECT_THROW(TestedContainer(elem, true), DataSourceError);
+    container_->configure(config_elem_, true);
+    checkDS(0, "test_type", "{}");
+    EXPECT_THROW(container_->configure(elem, true), DataSourceError);
+    checkDS(0, "test_type", "{}");
 }
 
 }