[2851] another cleanup: introduce NoSuchZone exception based on DataSourceError

getIterator now throws NoSuchZone if the given zone doesn't exist.
this is another step of detailing the too-generic DataSourceError, and
while not absolutely necessary, it's still related to the subject of
this branch.

diff --git a/src/lib/datasrc/cache_config.cc b/src/lib/datasrc/cache_config.cc
index 38964147309f308ad73167b1bb679bd6480bbedd..ec3cfeba827412477200658f8271b936f47f7c29 100644 (file)
--- a/src/lib/datasrc/cache_config.cc
+++ b/src/lib/datasrc/cache_config.cc
@@ -177,7 +177,7 @@ CacheConfig::getLoadAction(const dns::RRClass& rrclass,
     assert(datasrc_client_);
 
     // If the specified zone name does not exist in our client of the source,
-    // DataSourceError is thrown, which is exactly the result what we
+    // NoSuchZone is thrown, which is exactly the result what we
     // want, so no need to handle it.
     ZoneIteratorPtr iterator(datasrc_client_->getIterator(zone_name));
     if (!iterator) {

diff --git a/src/lib/datasrc/cache_config.h b/src/lib/datasrc/cache_config.h
index 7781f4960277767cfdcde5ccd76dd775112481a0..35b5ead3540619f92ab0991378b33bbae7eb6fe0 100644 (file)
--- a/src/lib/datasrc/cache_config.h
+++ b/src/lib/datasrc/cache_config.h
@@ -157,12 +157,13 @@ public:
     /// It doesn't throw an exception in this case because the expected caller
     /// of this method would handle such a case internally.
     ///
-    /// \throw DataSourceError error happens in the underlying data source
-    /// storing the cache data.  Most commonly it's because the specified zone
-    /// doesn't exist there.
+    /// \throw NoSuchZone The specified zone doesn't exist in the
+    /// underlying data source storing the original data to be cached.
+    /// \throw DataSourceError Other, unexpected but possible error happens
+    /// in the underlying data source.
     /// \throw Unexpected Unexpected error happens in the underlying data
-    /// source storing the cache data.  This shouldn't happen as long as the
-    /// data source implementation meets the public API requirement.
+    /// source.  This shouldn't happen as long as the data source
+    /// implementation meets the public API requirement.
     ///
     /// \param rrclass The RR class of the zone
     /// \param zone_name The origin name of the zone

diff --git a/src/lib/datasrc/client.h b/src/lib/datasrc/client.h
index 69299463e03f812801b86f5dafc064d6c6aefad2..169e4616459c030f10ef8e35d6465ae0e1c7dc48 100644 (file)
--- a/src/lib/datasrc/client.h
+++ b/src/lib/datasrc/client.h
@@ -201,9 +201,6 @@ public:
     /// This allows for traversing the whole zone. The returned object can
     /// provide the RRsets one by one.
     ///
-    /// This throws DataSourceError when the zone does not exist in the
-    /// datasource.
-    ///
     /// The default implementation throws isc::NotImplemented. This allows
     /// for easy and fast deployment of minimal custom data sources, where
     /// the user/implementer doesn't have to care about anything else but
@@ -214,6 +211,11 @@ public:
     /// It is not fixed if a concrete implementation of this method can throw
     /// anything else.
     ///
+    /// \throw NoSuchZone the zone does not exist in the datasource.
+    /// \throw Others Possibly implementation specific exceptions (it is
+    /// not fixed if a concrete implementation of this method can throw
+    /// anything else.)
+    ///
     /// \param name The name of zone apex to be traversed. It doesn't do
     ///     nearest match as findZone.
     /// \param separate_rrs If true, the iterator will return each RR as a

diff --git a/src/lib/datasrc/client_list.cc b/src/lib/datasrc/client_list.cc
index 4c55cd558c7b096225b696c246c9e7a54b359f4e..b6683698060fb78432bf524bbaa221614a9cc802 100644 (file)
--- a/src/lib/datasrc/client_list.cc
+++ b/src/lib/datasrc/client_list.cc
@@ -147,9 +147,9 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                 memory::LoadAction load_action;
                 try {
                     load_action = cache_conf->getLoadAction(rrclass_, zname);
-                } catch (const DataSourceError&) {
-                    isc_throw(ConfigurationError, "Data source error for "
-                              "loading a zone (possibly non-existent) "
+                } catch (const NoSuchZone&) {
+                    isc_throw(ConfigurationError, "Unable to cache "
+                              "non-existent zone: "
                               << zname << "/" << rrclass_);
                 }
                 assert(load_action); // in this loop this should be always true

diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index 70f4df0b423f10d1ec05ec683a58f08b004b65c9..9c8505434113bf7b81d108ecfda698588a109b0e 100644 (file)
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -1233,7 +1233,7 @@ public:
         const pair<bool, int> zone(accessor_->getZone(zone_name.toText()));
         if (!zone.first) {
             // No such zone, can't continue
-            isc_throw(DataSourceError, "Zone " + zone_name.toText() +
+            isc_throw(NoSuchZone, "Zone " + zone_name.toText() +
                       " can not be iterated, because it doesn't exist "
                       "in this data source");
         }

diff --git a/src/lib/datasrc/exceptions.h b/src/lib/datasrc/exceptions.h
index f9c56553d7ba411935b20077e9e30f146d8fa913..d3b25da95dc7240b4f01a13c53bd1074e47f0c20 100644 (file)
--- a/src/lib/datasrc/exceptions.h
+++ b/src/lib/datasrc/exceptions.h
@@ -20,8 +20,14 @@
 namespace isc {
 namespace datasrc {
 
-/// This exception represents Backend-independent errors relating to
-/// data source operations.
+/// \brief Top level exception related to data source.
+///
+/// This exception is the most generic form of exception for errors or
+/// unexpected events that can happen in the data source module.  In general,
+/// if an application needs to catch these conditions explicitly, it should
+/// catch more specific exceptions derived from this class; the severity
+/// of the conditions will vary very much, and such an application would
+/// normally like to behave differently depending on the severity.
 class DataSourceError : public Exception {
 public:
     DataSourceError(const char* file, size_t line, const char* what) :
@@ -40,6 +46,16 @@ public:
         DataSourceError(file, line, what) {}
 };
 
+/// \brief A specified zone does not exist in the specified data source.
+///
+/// This exception is thrown from methods that take a zone name and perform
+/// some action regarding that zone on the corresponding data source.
+class NoSuchZone : public DataSourceError {
+public:
+    NoSuchZone(const char* file, size_t line, const char* what) :
+        DataSourceError(file, line, what) {}
+};
+
 /// Base class for a number of exceptions that are thrown while working
 /// with zones.
 struct ZoneException : public Exception {

diff --git a/src/lib/datasrc/memory/memory_client.cc b/src/lib/datasrc/memory/memory_client.cc
index d49359a945a363b2ab3b33231b528f3a3404e81c..a6342ded5e1025b83cb9a96fb69fd24a09adf11e 100644 (file)
--- a/src/lib/datasrc/memory/memory_client.cc
+++ b/src/lib/datasrc/memory/memory_client.cc
@@ -242,7 +242,7 @@ InMemoryClient::getIterator(const Name& name, bool separate_rrs) const {
     const ZoneTable* zone_table = ztable_segment_->getHeader().getTable();
     const ZoneTable::FindResult result(zone_table->findZone(name));
     if (result.code != result::SUCCESS) {
-        isc_throw(DataSourceError, "No such zone: " + name.toText());
+        isc_throw(NoSuchZone, "No such zone: " + name.toText());
     }
 
     return (ZoneIteratorPtr(new MemoryIterator(

diff --git a/src/lib/datasrc/tests/cache_config_unittest.cc b/src/lib/datasrc/tests/cache_config_unittest.cc
index e73b06db9acb3c6c6947dcedb1f762a11543a719..34dd5d1ec4cdf8925c1a3ee0f53faaf59ceee13e 100644 (file)
--- a/src/lib/datasrc/tests/cache_config_unittest.cc
+++ b/src/lib/datasrc/tests/cache_config_unittest.cc
@@ -281,7 +281,7 @@ TEST_F(CacheConfigTest, getLoadActionWithMock) {
     // Zone configured for the cache but doesn't exist in the underling data
     // source.
     EXPECT_THROW(cache_conf.getLoadAction(RRClass::IN(), Name("example.net")),
-                 DataSourceError);
+                 NoSuchZone);
 
     // buggy data source client: it returns a null pointer from getIterator.
     EXPECT_THROW(cache_conf.getLoadAction(RRClass::IN(), Name("null.org")),

diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index 16b06279286e49a9bf430923ccecb51d652dbc97..79cc2c3fa1149237f7bd3e3c96af02cb301af018 100644 (file)
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -1417,7 +1417,7 @@ TEST(GenericDatabaseClientTest, noAccessorException) {
 
 // If the zone doesn't exist, exception is thrown
 TEST_P(DatabaseClientTest, noZoneIterator) {
-    EXPECT_THROW(client_->getIterator(Name("example.com")), DataSourceError);
+    EXPECT_THROW(client_->getIterator(Name("example.com")), NoSuchZone);
 }
 
 // If the zone doesn't exist and iteration is not implemented, it still throws
@@ -1427,7 +1427,7 @@ TEST(GenericDatabaseClientTest, noZoneNotImplementedIterator) {
                                 boost::shared_ptr<DatabaseAccessor>(
                                     new NopAccessor())).getIterator(
                                         Name("example.com")),
-                 DataSourceError);
+                 NoSuchZone);
 }
 
 TEST(GenericDatabaseClientTest, notImplementedIterator) {

diff --git a/src/lib/datasrc/tests/memory/memory_client_unittest.cc b/src/lib/datasrc/tests/memory/memory_client_unittest.cc
index f5b72a0dff5329ad80ba8a97f6412d34ada51992..5aed1dd42aa7c478dee26c1f84608472b9f84992 100644 (file)
--- a/src/lib/datasrc/tests/memory/memory_client_unittest.cc
+++ b/src/lib/datasrc/tests/memory/memory_client_unittest.cc
@@ -669,7 +669,7 @@ TEST_F(MemoryClientTest, getZoneCount) {
 
 TEST_F(MemoryClientTest, getIteratorForNonExistentZone) {
     // Zone "." doesn't exist
-    EXPECT_THROW(client_->getIterator(Name(".")), DataSourceError);
+    EXPECT_THROW(client_->getIterator(Name(".")), NoSuchZone);
 }
 
 TEST_F(MemoryClientTest, getIterator) {

diff --git a/src/lib/datasrc/tests/mock_client.cc b/src/lib/datasrc/tests/mock_client.cc
index fd3916d255e9440ea50464e5790474b83e18c1c6..8cdc05dacbbe13af7653901b030739cd5b17c14c 100644 (file)
--- a/src/lib/datasrc/tests/mock_client.cc
+++ b/src/lib/datasrc/tests/mock_client.cc
@@ -187,7 +187,7 @@ MockDataSourceClient::getIterator(const Name& name, bool) const {
         if (result.code == isc::datasrc::result::SUCCESS) {
             return (ZoneIteratorPtr(new Iterator(name, have_a_)));
         } else {
-            isc_throw(DataSourceError, "No such zone");
+            isc_throw(NoSuchZone, "No such zone");
         }
     }
 }