From: JINMEI Tatuya Date: Thu, 2 May 2013 17:41:50 +0000 (-0700) Subject: [2851] another cleanup: introduce NoSuchZone exception based on DataSourceError X-Git-Tag: bind10-1.2.0beta1-release~466^2~14 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3e678f3bda8a502c8a4fe483e12166bf1c9077d3;p=thirdparty%2Fkea.git [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 3896414730..ec3cfeba82 100644 --- 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 7781f49602..35b5ead354 100644 --- 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 69299463e0..169e461645 100644 --- 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 4c55cd558c..b668369806 100644 --- 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 70f4df0b42..9c85054341 100644 --- a/src/lib/datasrc/database.cc +++ b/src/lib/datasrc/database.cc @@ -1233,7 +1233,7 @@ public: const pair 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 f9c56553d7..d3b25da95d 100644 --- 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 d49359a945..a6342ded5e 100644 --- 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 e73b06db9a..34dd5d1ec4 100644 --- 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 16b0627928..79cc2c3fa1 100644 --- 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( 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 f5b72a0dff..5aed1dd42a 100644 --- 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 fd3916d255..8cdc05dacb 100644 --- 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"); } } }