[2044] Error handling on loading to cache

Tests and actual error handling of various scenarios when loading zone
data to caches.

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<InMemoryClient>
                     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<InMemoryZoneFinder>
                         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);
                 }
             }

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.

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);
+}
+
 }