BIND 10 trac2044, updated. db1cb6a8bb11f390e7a56bddcf3b8042d60f3191 [2044] Error handling on loading to cache

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Jun 26 14:40:44 UTC 2012


The branch, trac2044 has been updated
       via  db1cb6a8bb11f390e7a56bddcf3b8042d60f3191 (commit)
      from  42b565e0690fc72752047c9aa73f249fae1162a4 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit db1cb6a8bb11f390e7a56bddcf3b8042d60f3191
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Jun 26 16:39:11 2012 +0200

    [2044] Error handling on loading to cache
    
    Tests and actual error handling of various scenarios when loading zone
    data to caches.

-----------------------------------------------------------------------

Summary of changes:
 src/lib/datasrc/client_list.cc                |   19 ++++-
 src/lib/datasrc/client_list.h                 |    5 ++
 src/lib/datasrc/tests/client_list_unittest.cc |   94 ++++++++++++++++++++++---
 3 files changed, 106 insertions(+), 12 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/client_list.cc b/src/lib/datasrc/client_list.cc
index b9d9241..dd0a715 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<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);
                 }
             }
diff --git a/src/lib/datasrc/client_list.h b/src/lib/datasrc/client_list.h
index 1b59b68..5105d08 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 af428b3..e031eea 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);
+}
+
 }



More information about the bind10-changes mailing list