From: JINMEI Tatuya Date: Thu, 2 May 2013 22:14:32 +0000 (-0700) Subject: [2851] update getCachedZoneWriter so it only checks config and real datasrc. X-Git-Tag: bind10-1.2.0beta1-release~466^2~12 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=74c2ed970512562b5ea53004d3e152e2d8519329;p=thirdparty%2Fkea.git [2851] update getCachedZoneWriter so it only checks config and real datasrc. - it now still create the zone writer even if the zone is currently not loaded (due to initial failure). the original behavior is mostly preserved, but in some minor cases the resulting error code (or whether to throw) differ. test cases were adjusted accordingly. - some description of CacheStatus now does not perfectly match the behavior, so are updated. --- diff --git a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc index 01ac47e76b..671037c814 100644 --- a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc +++ b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc @@ -406,6 +406,22 @@ TEST_F(DataSrcClientsBuilderTest, EXPECT_EQ(orig_lock_count + 1, map_mutex.lock_count); EXPECT_EQ(orig_unlock_count + 1, map_mutex.unlock_count); + // zone doesn't exist in the data source + const ConstElementPtr config_nozone(Element::fromJSON("{" + "\"IN\": [{" + " \"type\": \"sqlite3\"," + " \"params\": {\"database_file\": \"" + test_db + "\"}," + " \"cache-enable\": true," + " \"cache-zones\": [\"nosuchzone.example\"]" + "}]}")); + clients_map = configureDataSource(config_nozone); + EXPECT_THROW( + builder.handleCommand( + Command(LOADZONE, Element::fromJSON( + "{\"class\": \"IN\"," + " \"origin\": \"nosuchzone.example\"}"))), + TestDataSrcClientsBuilder::InternalCommandError); + // basically impossible case: in-memory cache is completely disabled. // In this implementation of manager-builder, this should never happen, // but it catches it like other configuration error and keeps going. @@ -503,14 +519,6 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) { }, ""); } - // zone doesn't exist in the data source - EXPECT_THROW( - builder.handleCommand( - Command(LOADZONE, - Element::fromJSON( - "{\"class\": \"IN\", \"origin\": \"xx\"}"))), - TestDataSrcClientsBuilder::InternalCommandError); - // origin is bogus EXPECT_THROW(builder.handleCommand( Command(LOADZONE, diff --git a/src/lib/datasrc/client_list.cc b/src/lib/datasrc/client_list.cc index 2405587f46..e9a8bc6dff 100644 --- a/src/lib/datasrc/client_list.cc +++ b/src/lib/datasrc/client_list.cc @@ -314,26 +314,31 @@ ConfigurableClientList::getCachedZoneWriter(const Name& name) { if (!allow_cache_) { return (ZoneWriterPair(CACHE_DISABLED, ZoneWriterPtr())); } - // Try to find the correct zone. - MutableResult result; - findInternal(result, name, true, true); - if (!result.finder) { - return (ZoneWriterPair(ZONE_NOT_FOUND, ZoneWriterPtr())); - } + // Find the data source from which the zone to be loaded into memory. // Then get the appropriate load action and create a zone writer. - // Note that getCacheConfig() must return non NULL in this module (only - // tests could set it to a bogus value). - const memory::LoadAction load_action = - result.info->getCacheConfig()->getLoadAction(rrclass_, name); - if (!load_action) { - return (ZoneWriterPair(ZONE_NOT_CACHED, ZoneWriterPtr())); + BOOST_FOREACH(const DataSourceInfo& info, data_sources_) { + // If there's a underlying "real" data source and it doesn't contain + // the given name, obviously we cannot load it. + if (info.data_src_client_ && + info.data_src_client_->findZone(name).code != result::SUCCESS) { + continue; + } + + // Note that getCacheConfig() must return non NULL in this module + // (only tests could set it to a bogus value). + const memory::LoadAction load_action = + info.getCacheConfig()->getLoadAction(rrclass_, name); + if (!load_action) { + return (ZoneWriterPair(ZONE_NOT_CACHED, ZoneWriterPtr())); + } + return (ZoneWriterPair(ZONE_SUCCESS, + ZoneWriterPtr( + new memory::ZoneWriter( + *info.ztable_segment_, + load_action, name, rrclass_)))); } - return (ZoneWriterPair(ZONE_SUCCESS, - ZoneWriterPtr( - new memory::ZoneWriter( - *result.info->ztable_segment_, - load_action, name, rrclass_)))); + return (ZoneWriterPair(ZONE_NOT_FOUND, ZoneWriterPtr())); } // NOTE: This function is not tested, it would be complicated. However, the diff --git a/src/lib/datasrc/client_list.h b/src/lib/datasrc/client_list.h index 912bed3871..5af30cfd39 100644 --- a/src/lib/datasrc/client_list.h +++ b/src/lib/datasrc/client_list.h @@ -348,12 +348,12 @@ public: /// This is used as a result of the getCachedZoneWriter() method. enum CacheStatus { CACHE_DISABLED, ///< The cache is not enabled in this list. - ZONE_NOT_CACHED, ///< Zone is served directly, not from cache - /// (including the case cache is disabled for - /// the specific data source). + ZONE_NOT_CACHED, ///< Zone is not to be cached (including the case + /// where caching is disabled for the specific + /// data source). ZONE_NOT_FOUND, ///< Zone does not exist in this list. - ZONE_SUCCESS ///< The zone was successfully reloaded or - /// the writer provided. + ZONE_SUCCESS ///< Zone to be cached is successfully found and + /// is ready to be loaded }; /// \brief Return value of getCachedZoneWriter() @@ -408,12 +408,12 @@ public: boost::shared_ptr ztable_segment_; std::string name_; + // cache_conf_ can be accessed only from this read-only getter, + // to protect its integrity as much as possible. const internal::CacheConfig* getCacheConfig() const { return (cache_conf_.get()); } private: - // this is kept private for now. When it needs to be accessed, - // we'll add a read-only getter method. boost::shared_ptr cache_conf_; }; diff --git a/src/lib/datasrc/tests/client_list_unittest.cc b/src/lib/datasrc/tests/client_list_unittest.cc index 252e032b23..2fb4019b7c 100644 --- a/src/lib/datasrc/tests/client_list_unittest.cc +++ b/src/lib/datasrc/tests/client_list_unittest.cc @@ -859,7 +859,7 @@ ListTest::doReload(const Name& origin) { TEST_F(ListTest, reloadSuccess) { list_->configure(config_elem_zones_, true); const Name name("example.org"); - this->prepareCache(0, name); + prepareCache(0, name); // The cache currently contains a tweaked version of zone, which // doesn't have "tstzonedata" A record. So the lookup should result // in NXDOMAIN. @@ -917,8 +917,8 @@ TEST_F(ListTest, reloadNoSuchZone) { // Not in the data sources EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND, doReload(Name("exmaple.cz"))); - // Not cached - EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND, doReload(name)); + // If it's not configured to be cached, it won't be reloaded. + EXPECT_EQ(ConfigurableClientList::ZONE_NOT_CACHED, doReload(name)); // Partial match EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND, doReload(Name("sub.example.com"))); @@ -952,8 +952,8 @@ TEST_F(ListTest, reloadZoneGone) { static_cast( list_->getDataSources()[0].data_src_client_)->eraseZone(name); - // The zone is not there, so abort the reload. - EXPECT_THROW(doReload(name), DataSourceError); + // The zone is not there, so reload doesn't take place. + EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND, doReload(name)); // The (cached) zone is not hurt. EXPECT_EQ(ZoneFinder::SUCCESS, list_->find(name).finder_->find(name, RRType::SOA())->code);