[2993] Extend ConfigurableClientList::getCachedZoneWriter() to have catch_load_error

diff --git a/configure.ac b/configure.ac
index b01be128512b04af49eef14feda0e75f7dd196a2..e5b9daca0f7d921cdc099a286eabf92c61f99939 100644 (file)
--- 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 7b40e1f16071ac32e587aa1df2a8bdbe5f6ebeb3..11475e2faba5c966a46d0e4860de36830a97cea7 100644 (file)
--- a/src/bin/auth/datasrc_clients_mgr.h
+++ b/src/bin/auth/datasrc_clients_mgr.h
@@ -623,7 +623,7 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::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 8a609af934bcb6e6e975a65b95b79af76c855793..4fe1eb3741ccdd4134b21e7b2537c2610f2073d0 100644 (file)
--- 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 ad18f20e13457b16b781d27f6b084d59626853c9..e4df415aecf61607843afdc79cb5ef8eeee929b8 100644 (file)
--- 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 320833ccaa6806ecf5fb600b90795b19c3ce446a..04c3069f637289446c770864c0c6838d5a6eaf04 100644 (file)
--- 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 9f26d135193f1405617c709cd2bfe71c33194ad2..b16a6bb90a64b70bd393c950467913d06332abed 100644 (file)
--- 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 ba020eb559706819ef4c5a23a48bc0af779e4a12..5e56b18c943faa87bc15a9b1e1c31b18d1e9d835 100644 (file)
--- 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<s_ConfigurableClientList*>(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 97fc2b6ff0e2b25c03f7d3a4572bef40cc547406..256ca6259dcdc0c65e41e2058b423e2f53eb91ad 100644 (file)
--- 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 774e5924aa5608bc336262b1cd89d2ff6bc84534..e8056a51800ed965974d0c5a7e33514e18cf941e 100644 (file)
--- 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 (file)
index 0000000..8365a24
--- /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 (file)
index 0000000..079b400
--- /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 efe1144ccc653a270bf414740c99b9678fe8e459..5d862b360140562f1b2df04dabe9efb0b0c5bcff 100644 (file)
--- 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\