From: Michal 'vorner' Vaner Date: Tue, 26 Jun 2012 14:39:11 +0000 (+0200) Subject: [2044] Error handling on loading to cache X-Git-Tag: trac2351_base~196^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=db1cb6a8bb11f390e7a56bddcf3b8042d60f3191;p=thirdparty%2Fkea.git [2044] Error handling on loading to cache Tests and actual error handling of various scenarios when loading zone data to caches. --- diff --git a/src/lib/datasrc/client_list.cc b/src/lib/datasrc/client_list.cc index b9d92412c3..dd0a715064 100644 --- a/src/lib/datasrc/client_list.cc +++ b/src/lib/datasrc/client_list.cc @@ -69,6 +69,11 @@ ConfigurableClientList::configure(const Element& config, bool allowCache) { new_data_sources.push_back(DataSourceInfo(ds.first, ds.second, wantCache)); if (wantCache) { + if (!dconf->contains("cache-zones")) { + isc_throw(isc::NotImplemented, "Auto-detection of zones " + "to cache is not yet implemented, supply " + "cache-zones parameter"); + } const ConstElementPtr zones(dconf->get("cache-zones")); const shared_ptr cache(new_data_sources.back().cache_); @@ -78,12 +83,22 @@ ConfigurableClientList::configure(const Element& config, bool allowCache) { const Name origin(zones->get(i)->stringValue()); const DataSourceClient::FindResult zone(client->findZone(origin)); - // TODO check it is SUCCESS + if (zone.code != result::SUCCESS) { + // The data source does not contain the zone, it can't + // be cached. + isc_throw(ConfigurationError, "Unable to cache " + "non-existent zone " << origin); + } shared_ptr finder(new InMemoryZoneFinder(zone.zone_finder->getClass(), origin)); - finder->load(*client->getIterator(origin)); + ZoneIteratorPtr iterator(client->getIterator(origin)); + if (!iterator) { + isc_throw(isc::Unexpected, "Got NULL iterator for " + "zone " << origin); + } + finder->load(*iterator); cache->addZone(finder); } } diff --git a/src/lib/datasrc/client_list.h b/src/lib/datasrc/client_list.h index 1b59b684ea..5105d08af2 100644 --- a/src/lib/datasrc/client_list.h +++ b/src/lib/datasrc/client_list.h @@ -213,6 +213,11 @@ public: /// client. /// \throw ConfigurationError if the configuration is invalid in some /// sense. + /// \throw Unexpected if something misbehaves (like the data source + /// returning NULL iterator). + /// \throw NotImplemented if the auto-detection of list of zones is + /// needed. + /// \throw Whatever is propagated from within the data source. void configure(const data::Element& configuration, bool allow_cache); /// \brief Implementation of the ClientList::find. diff --git a/src/lib/datasrc/tests/client_list_unittest.cc b/src/lib/datasrc/tests/client_list_unittest.cc index af428b3127..e031eea669 100644 --- a/src/lib/datasrc/tests/client_list_unittest.cc +++ b/src/lib/datasrc/tests/client_list_unittest.cc @@ -143,7 +143,13 @@ public: isc_throw(isc::NotImplemented, "Not implemented"); } virtual ZoneIteratorPtr getIterator(const Name& name, bool) const { - return (ZoneIteratorPtr(new Iterator(name))); + if (name == Name("noiter.org")) { + isc_throw(isc::NotImplemented, "Asked not to be implemented"); + } else if (name == Name("null.org")) { + return (ZoneIteratorPtr()); + } else { + return (ZoneIteratorPtr(new Iterator(name))); + } } const string type_; const ConstElementPtr configuration_; @@ -393,14 +399,14 @@ TEST_F(ListTest, multiBestMatch) { // Check the configuration is empty when the list is empty TEST_F(ListTest, configureEmpty) { - ConstElementPtr elem(new ListElement); + const ConstElementPtr elem(new ListElement); list_->configure(*elem, true); EXPECT_TRUE(list_->getDataSources().empty()); } // Check we can get multiple data sources and they are in the right order. TEST_F(ListTest, configureMulti) { - ConstElementPtr elem(Element::fromJSON("[" + const ConstElementPtr elem(Element::fromJSON("[" "{" " \"type\": \"type1\"," " \"cache\": \"off\"," @@ -462,7 +468,28 @@ TEST_F(ListTest, wrongConfig) { "[{\"type\": \"test_type\", \"params\": 13}, {\"type\": null}]", "[{\"type\": \"test_type\", \"params\": 13}, {\"type\": []}]", "[{\"type\": \"test_type\", \"params\": 13}, {\"type\": {}}]", - // TODO: Once cache is supported, add some invalid cache values + // Bad type of cache-enable + "[{\"type\": \"test_type\", \"params\": 13}, " + "{\"type\": \"x\", \"cache-enable\": 13, \"cache-zones\": []}]", + "[{\"type\": \"test_type\", \"params\": 13}, " + "{\"type\": \"x\", \"cache-enable\": \"xx\", \"cache-zones\": []}]", + "[{\"type\": \"test_type\", \"params\": 13}, " + "{\"type\": \"x\", \"cache-enable\": [], \"cache-zones\": []}]", + "[{\"type\": \"test_type\", \"params\": 13}, " + "{\"type\": \"x\", \"cache-enable\": {}, \"cache-zones\": []}]", + "[{\"type\": \"test_type\", \"params\": 13}, " + "{\"type\": \"x\", \"cache-enable\": null, \"cache-zones\": []}]", + // Bad type of cache-zones + "[{\"type\": \"test_type\", \"params\": 13}, " + "{\"type\": \"x\", \"cache-enable\": true, \"cache-zones\": \"x\"}]", + "[{\"type\": \"test_type\", \"params\": 13}, " + "{\"type\": \"x\", \"cache-enable\": true, \"cache-zones\": true}]", + "[{\"type\": \"test_type\", \"params\": 13}, " + "{\"type\": \"x\", \"cache-enable\": true, \"cache-zones\": null}]", + "[{\"type\": \"test_type\", \"params\": 13}, " + "{\"type\": \"x\", \"cache-enable\": true, \"cache-zones\": 13}]", + "[{\"type\": \"test_type\", \"params\": 13}, " + "{\"type\": \"x\", \"cache-enable\": true, \"cache-zones\": {}}]", NULL }; // Put something inside to see it survives the exception @@ -481,7 +508,7 @@ TEST_F(ListTest, wrongConfig) { // The param thing defaults to null. Cache is not used yet. TEST_F(ListTest, defaults) { - ConstElementPtr elem(Element::fromJSON("[" + const ConstElementPtr elem(Element::fromJSON("[" "{" " \"type\": \"type1\"" "}]")); @@ -492,7 +519,7 @@ TEST_F(ListTest, defaults) { // Check we can call the configure multiple times, to change the configuration TEST_F(ListTest, reconfigure) { - ConstElementPtr empty(new ListElement); + const ConstElementPtr empty(new ListElement); list_->configure(*config_elem_, true); checkDS(0, "test_type", "{}", false); list_->configure(*empty, true); @@ -503,7 +530,7 @@ TEST_F(ListTest, reconfigure) { // Make sure the data source error exception from the factory is propagated TEST_F(ListTest, dataSrcError) { - ConstElementPtr elem(Element::fromJSON("[" + const ConstElementPtr elem(Element::fromJSON("[" "{" " \"type\": \"error\"" "}]")); @@ -515,7 +542,7 @@ TEST_F(ListTest, dataSrcError) { // Check we can get the cache TEST_F(ListTest, configureCacheEmpty) { - ConstElementPtr elem(Element::fromJSON("[" + const ConstElementPtr elem(Element::fromJSON("[" "{" " \"type\": \"type1\"," " \"cache-enable\": true," @@ -537,7 +564,7 @@ TEST_F(ListTest, configureCacheEmpty) { // But no cache if we disallow it globally TEST_F(ListTest, configureCacheDisabled) { - ConstElementPtr elem(Element::fromJSON("[" + const ConstElementPtr elem(Element::fromJSON("[" "{" " \"type\": \"type1\"," " \"cache-enable\": true," @@ -559,7 +586,7 @@ TEST_F(ListTest, configureCacheDisabled) { // Put some zones into the cache TEST_F(ListTest, cacheZones) { - ConstElementPtr elem(Element::fromJSON("[" + const ConstElementPtr elem(Element::fromJSON("[" "{" " \"type\": \"type1\"," " \"cache-enable\": true," @@ -578,4 +605,51 @@ TEST_F(ListTest, cacheZones) { EXPECT_EQ(result::NOTFOUND, cache->findZone(Name("example.cz")).code); } +// Check the caching handles misbehaviour from the data source and +// misconfiguration gracefully +TEST_F(ListTest, badCache) { + list_->configure(*config_elem_, true); + checkDS(0, "test_type", "{}", false); + // First, the zone is not in the data source + const ConstElementPtr elem1(Element::fromJSON("[" + "{" + " \"type\": \"type1\"," + " \"cache-enable\": true," + " \"cache-zones\": [\"example.org\"]," + " \"params\": []" + "}]")); + EXPECT_THROW(list_->configure(*elem1, true), + ConfigurableClientList::ConfigurationError); + checkDS(0, "test_type", "{}", false); + // Now, the zone doesn't give an iterator + const ConstElementPtr elem2(Element::fromJSON("[" + "{" + " \"type\": \"type1\"," + " \"cache-enable\": true," + " \"cache-zones\": [\"noiter.org\"]," + " \"params\": [\"noiter.org\"]" + "}]")); + EXPECT_THROW(list_->configure(*elem2, true), isc::NotImplemented); + checkDS(0, "test_type", "{}", false); + // Now, the zone returns NULL iterator + const ConstElementPtr elem3(Element::fromJSON("[" + "{" + " \"type\": \"type1\"," + " \"cache-enable\": true," + " \"cache-zones\": [\"null.org\"]," + " \"params\": [\"null.org\"]" + "}]")); + EXPECT_THROW(list_->configure(*elem3, true), isc::Unexpected); + checkDS(0, "test_type", "{}", false); + // The autodetection of zones is not enabled + const ConstElementPtr elem4(Element::fromJSON("[" + "{" + " \"type\": \"type1\"," + " \"cache-enable\": true," + " \"params\": [\"example.org\"]" + "}]")); + EXPECT_THROW(list_->configure(*elem4, true), isc::NotImplemented); + checkDS(0, "test_type", "{}", false); +} + }