Parcourir la source

[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.
JINMEI Tatuya il y a 12 ans
Parent
commit
3e678f3bda

+ 1 - 1
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) {

+ 6 - 5
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

+ 5 - 3
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

+ 3 - 3
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

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

+ 18 - 2
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 {

+ 1 - 1
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(

+ 1 - 1
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")),

+ 2 - 2
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) {

+ 1 - 1
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) {

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