From: Mukund Sivaraman Date: Thu, 27 Jun 2013 03:01:09 +0000 (+0530) Subject: [2993] Extend ConfigurableClientList::getCachedZoneWriter() to have catch_load_error X-Git-Tag: bind10-1.2.0beta1-release~373^2~4 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b178f2a38b94200e35758f6e8d7e570a0cfc41ec;p=thirdparty%2Fkea.git [2993] Extend ConfigurableClientList::getCachedZoneWriter() to have catch_load_error --- diff --git a/configure.ac b/configure.ac index b01be12851..e5b9daca0f 100644 --- a/configure.ac +++ b/configure.ac @@ -1245,6 +1245,7 @@ AC_CONFIG_FILES([Makefile src/lib/python/isc/util/cio/tests/Makefile src/lib/python/isc/datasrc/Makefile src/lib/python/isc/datasrc/tests/Makefile + src/lib/python/isc/datasrc/tests/testdata/Makefile src/lib/python/isc/dns/Makefile src/lib/python/isc/cc/Makefile src/lib/python/isc/cc/cc_generated/Makefile diff --git a/src/bin/auth/datasrc_clients_mgr.h b/src/bin/auth/datasrc_clients_mgr.h index 7b40e1f160..11475e2fab 100644 --- a/src/bin/auth/datasrc_clients_mgr.h +++ b/src/bin/auth/datasrc_clients_mgr.h @@ -623,7 +623,7 @@ DataSrcClientsBuilderBase::getZoneWriter( datasrc::ConfigurableClientList::ZoneWriterPair writerpair; { typename MutexType::Locker locker(*map_mutex_); - writerpair = client_list.getCachedZoneWriter(origin); + writerpair = client_list.getCachedZoneWriter(origin, false); } switch (writerpair.first) { diff --git a/src/lib/datasrc/client_list.cc b/src/lib/datasrc/client_list.cc index 8a609af934..4fe1eb3741 100644 --- a/src/lib/datasrc/client_list.cc +++ b/src/lib/datasrc/client_list.cc @@ -347,6 +347,7 @@ ConfigurableClientList::resetMemorySegment ConfigurableClientList::ZoneWriterPair ConfigurableClientList::getCachedZoneWriter(const Name& name, + bool catch_load_error, const std::string& datasrc_name) { if (!allow_cache_) { @@ -385,7 +386,8 @@ ConfigurableClientList::getCachedZoneWriter(const Name& name, ZoneWriterPtr( new memory::ZoneWriter( *info.ztable_segment_, - load_action, name, rrclass_, false)))); + load_action, name, rrclass_, + catch_load_error)))); } // We can't find the specified zone. If a specific data source was diff --git a/src/lib/datasrc/client_list.h b/src/lib/datasrc/client_list.h index ad18f20e13..e4df415aec 100644 --- a/src/lib/datasrc/client_list.h +++ b/src/lib/datasrc/client_list.h @@ -432,6 +432,8 @@ public: /// of the pair. /// /// \param zone The origin of the zone to load. + /// \param catch_load_errors Whether to make the zone writer catch + /// load errors (see \c ZoneWriter constructor documentation). /// \param datasrc_name If not empty, the name of the data source /// to be used for loading the zone (see above). /// \return The result has two parts. The first one is a status indicating @@ -442,6 +444,7 @@ public: /// \throw DataSourceError or anything else that the data source /// containing the zone might throw is propagated. ZoneWriterPair getCachedZoneWriter(const dns::Name& zone, + bool catch_load_error, const std::string& datasrc_name = ""); /// \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 320833ccaa..04c3069f63 100644 --- a/src/lib/datasrc/tests/client_list_unittest.cc +++ b/src/lib/datasrc/tests/client_list_unittest.cc @@ -210,7 +210,7 @@ public: config_ztable_segment); const ConfigurableClientList::ZoneWriterPair result = - list_->getCachedZoneWriter(zone, dsrc_info.name_); + list_->getCachedZoneWriter(zone, false, dsrc_info.name_); ASSERT_EQ(ConfigurableClientList::ZONE_SUCCESS, result.first); result.second->load(); @@ -1023,7 +1023,7 @@ TEST_P(ListTest, BadMasterFile) { ConfigurableClientList::CacheStatus ListTest::doReload(const Name& origin, const string& datasrc_name) { ConfigurableClientList::ZoneWriterPair - result(list_->getCachedZoneWriter(origin, datasrc_name)); + result(list_->getCachedZoneWriter(origin, false, datasrc_name)); if (result.first == ConfigurableClientList::ZONE_SUCCESS) { // Can't use ASSERT_NE here, it would want to return(), which // it can't in non-void function. diff --git a/src/lib/python/isc/datasrc/configurableclientlist_inc.cc b/src/lib/python/isc/datasrc/configurableclientlist_inc.cc index 9f26d13519..b16a6bb90a 100644 --- a/src/lib/python/isc/datasrc/configurableclientlist_inc.cc +++ b/src/lib/python/isc/datasrc/configurableclientlist_inc.cc @@ -65,7 +65,7 @@ Parameters:\n\ "; const char* const ConfigurableClientList_get_cached_zone_writer_doc = "\ -get_cached_zone_writer(zone, datasrc_name) -> status, zone_writer\n\ +get_cached_zone_writer(zone, catch_load_error, datasrc_name) -> status, zone_writer\n\ \n\ This method returns a ZoneWriter that can be used to (re)load a zone.\n\ \n\ @@ -89,6 +89,7 @@ the status is anything else, the second element is None.\n\ \n\ Parameters:\n\ zone The origin of the zone to (re)load.\n\ + catch_load_error Whether to catch load errors, or to raise them as exceptions.\n\ datasrc_name The name of the data source where the zone is to be loaded (optional).\n\ "; diff --git a/src/lib/python/isc/datasrc/configurableclientlist_python.cc b/src/lib/python/isc/datasrc/configurableclientlist_python.cc index ba020eb559..5e56b18c94 100644 --- a/src/lib/python/isc/datasrc/configurableclientlist_python.cc +++ b/src/lib/python/isc/datasrc/configurableclientlist_python.cc @@ -163,15 +163,17 @@ ConfigurableClientList_getCachedZoneWriter(PyObject* po_self, PyObject* args) { static_cast(po_self); try { PyObject* name_obj; + int catch_load_error; const char* datasrc_name_p = ""; - if (PyArg_ParseTuple(args, "O!|s", &isc::dns::python::name_type, - &name_obj, &datasrc_name_p)) { + if (PyArg_ParseTuple(args, "O!p|s", &isc::dns::python::name_type, + &name_obj, &catch_load_error, &datasrc_name_p)) { const isc::dns::Name& name(isc::dns::python::PyName_ToName(name_obj)); const std::string datasrc_name(datasrc_name_p); const ConfigurableClientList::ZoneWriterPair result = - self->cppobj->getCachedZoneWriter(name, datasrc_name); + self->cppobj->getCachedZoneWriter(name, catch_load_error, + datasrc_name); PyObjectContainer writer; if (!result.second) { diff --git a/src/lib/python/isc/datasrc/tests/Makefile.am b/src/lib/python/isc/datasrc/tests/Makefile.am index 97fc2b6ff0..256ca6259d 100644 --- a/src/lib/python/isc/datasrc/tests/Makefile.am +++ b/src/lib/python/isc/datasrc/tests/Makefile.am @@ -1,17 +1,10 @@ +SUBDIRS = testdata + PYCOVERAGE_RUN = @PYCOVERAGE_RUN@ PYTESTS = datasrc_test.py sqlite3_ds_test.py PYTESTS += clientlist_test.py zone_loader_test.py EXTRA_DIST = $(PYTESTS) -EXTRA_DIST += testdata/brokendb.sqlite3 -EXTRA_DIST += testdata/example.com.sqlite3 -EXTRA_DIST += testdata/example.com.source.sqlite3 -EXTRA_DIST += testdata/newschema.sqlite3 -EXTRA_DIST += testdata/oldschema.sqlite3 -EXTRA_DIST += testdata/new_minor_schema.sqlite3 -EXTRA_DIST += testdata/example.com -EXTRA_DIST += testdata/example.com.ch -EXTRA_DIST += testdata/static.zone CLEANFILES = $(abs_builddir)/rwtest.sqlite3.copied CLEANFILES += $(abs_builddir)/zoneloadertest.sqlite3 diff --git a/src/lib/python/isc/datasrc/tests/clientlist_test.py b/src/lib/python/isc/datasrc/tests/clientlist_test.py index 774e5924aa..e8056a5180 100644 --- a/src/lib/python/isc/datasrc/tests/clientlist_test.py +++ b/src/lib/python/isc/datasrc/tests/clientlist_test.py @@ -253,7 +253,9 @@ class ClientListTest(unittest.TestCase): self.clist.reset_memory_segment("MasterFiles", isc.datasrc.ConfigurableClientList.CREATE, map_params) - result, self.__zone_writer = self.clist.get_cached_zone_writer(isc.dns.Name("example.com")) + result, self.__zone_writer = \ + self.clist.get_cached_zone_writer(isc.dns.Name("example.com"), + False) self.assertEqual(isc.datasrc.ConfigurableClientList.CACHE_STATUS_ZONE_SUCCESS, result) err_msg = self.__zone_writer.load() @@ -264,7 +266,9 @@ class ClientListTest(unittest.TestCase): self.clist.reset_memory_segment("MasterFiles", isc.datasrc.ConfigurableClientList.READ_ONLY, map_params) - result, self.__zone_writer = self.clist.get_cached_zone_writer(isc.dns.Name("example.com")) + result, self.__zone_writer = \ + self.clist.get_cached_zone_writer(isc.dns.Name("example.com"), + False) self.assertEqual(isc.datasrc.ConfigurableClientList.CACHE_STATUS_CACHE_NOT_WRITABLE, result) @@ -280,7 +284,9 @@ class ClientListTest(unittest.TestCase): self.clist = isc.datasrc.ConfigurableClientList(isc.dns.RRClass.IN) self.configure_helper() - result, self.__zone_writer = self.clist.get_cached_zone_writer(isc.dns.Name("example.com")) + result, self.__zone_writer = \ + self.clist.get_cached_zone_writer(isc.dns.Name("example.com"), + False) self.assertEqual(isc.datasrc.ConfigurableClientList.CACHE_STATUS_ZONE_SUCCESS, result) err_msg = self.__zone_writer.load() @@ -288,6 +294,30 @@ class ClientListTest(unittest.TestCase): self.assertRaises(isc.datasrc.Error, self.__zone_writer.load) self.__zone_writer.cleanup() + def test_zone_writer_load_without_raise(self): + """ + Test that the zone writer does not throw when asked not to. + """ + + self.clist = isc.datasrc.ConfigurableClientList(isc.dns.RRClass.IN) + self.clist.configure('''[{ + "type": "MasterFiles", + "params": { + "example.com": "''' + TESTDATA_PATH + '''example.com-broken.zone" + }, + "cache-enable": true + }]''', True) + + result, self.__zone_writer = \ + self.clist.get_cached_zone_writer(isc.dns.Name("example.com"), + True) + self.assertEqual(isc.datasrc.ConfigurableClientList.CACHE_STATUS_ZONE_SUCCESS, + result) + err_msg = self.__zone_writer.load() + self.assertIsNotNone(err_msg) + self.assertTrue('Errors found when validating zone' in err_msg) + self.__zone_writer.cleanup() + def test_zone_writer_install_without_load(self): """ Test that the zone writer throws when install() is called @@ -297,7 +327,9 @@ class ClientListTest(unittest.TestCase): self.clist = isc.datasrc.ConfigurableClientList(isc.dns.RRClass.IN) self.configure_helper() - result, self.__zone_writer = self.clist.get_cached_zone_writer(isc.dns.Name("example.com")) + result, self.__zone_writer = \ + self.clist.get_cached_zone_writer(isc.dns.Name("example.com"), + False) self.assertEqual(isc.datasrc.ConfigurableClientList.CACHE_STATUS_ZONE_SUCCESS, result) self.assertRaises(isc.datasrc.Error, self.__zone_writer.install) diff --git a/src/lib/python/isc/datasrc/tests/testdata/Makefile.am b/src/lib/python/isc/datasrc/tests/testdata/Makefile.am new file mode 100644 index 0000000000..8365a24f17 --- /dev/null +++ b/src/lib/python/isc/datasrc/tests/testdata/Makefile.am @@ -0,0 +1,12 @@ +EXTRA_DIST = \ + brokendb.sqlite3 \ + example.com \ + example.com-broken.zone \ + example.com.ch \ + example.com.source.sqlite3 \ + example.com.sqlite3 \ + Makefile.am \ + new_minor_schema.sqlite3 \ + newschema.sqlite3 \ + oldschema.sqlite3 \ + static.zone diff --git a/src/lib/python/isc/datasrc/tests/testdata/example.com-broken.zone b/src/lib/python/isc/datasrc/tests/testdata/example.com-broken.zone new file mode 100644 index 0000000000..079b400dda --- /dev/null +++ b/src/lib/python/isc/datasrc/tests/testdata/example.com-broken.zone @@ -0,0 +1,6 @@ +; This zone is broken (contains no NS records). +example.com. 1000 IN SOA a.dns.example.com. mail.example.com. 2 1 1 1 1 +a.dns.example.com. 1000 IN A 1.1.1.1 +b.dns.example.com. 1000 IN A 3.3.3.3 +b.dns.example.com. 1000 IN AAAA 4:4::4:4 +b.dns.example.com. 1000 IN AAAA 5:5::5:5 diff --git a/src/lib/python/isc/datasrc/zonewriter_inc.cc b/src/lib/python/isc/datasrc/zonewriter_inc.cc index efe1144ccc..5d862b3601 100644 --- a/src/lib/python/isc/datasrc/zonewriter_inc.cc +++ b/src/lib/python/isc/datasrc/zonewriter_inc.cc @@ -46,8 +46,9 @@ the data actually served, it only prepares them for future use.\n\ This is the first method you should call on the object. Never call it\n\ multiple times.\n\ \n\ -Depending on how the ZoneWriter was constructed, in case a load error\n\ -happens, a string with the error message may be returned. When\n\ +Depending on how the ZoneWriter was constructed (see catch_load_error\n\ +argument to ConfigurableClientList.get_cached_zone_writer()), in case a\n\ +load error happens, a string with the error message may be returned. When\n\ ZoneWriter is not constructed to do that, in case of a load error, a\n\ DataSourceError exception is raised. In all other cases, this method\n\ returns None.\n\