From: Michal 'vorner' Vaner Date: Wed, 6 Jun 2012 15:30:33 +0000 (+0200) Subject: [1975] Implement the loading of configuration X-Git-Tag: trac2351_base~97^2~9^2~4^2~10 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=65ffa0d23485dd6101cbf41d4cc1bd2fd89afee6;p=thirdparty%2Fkea.git [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 2bde4a19cf..9455c0d1bf 100644 --- 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 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 bba5e75f4b..b8492e0ac3 100644 --- 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 1fa27185a2..8ff1317336 100644 --- 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 ds(new TestDS(ds_zones[i])); @@ -229,6 +230,7 @@ public: const Container::SearchResult negativeResult_; vector > ds_; vector 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", "{}"); } }