[1179] some added tests, and force ptr cleanup

src/lib/python/isc/datasrc/finder_python.cc

@@ -84,7 +84,9 @@ ZoneFinder_init(s_ZoneFinder* self, PyObject* args) {
 
 void
 ZoneFinder_destroy(s_ZoneFinder* const self) {
-    // cppobj is shared ptr, so no need for explicit delete
+    // cppobj is a shared ptr, but to make sure things are not destroyed in
+    // the wrong order, we reset it here.
+    self->cppobj.reset();
     Py_TYPE(self)->tp_free(self);
 }
 

src/lib/python/isc/datasrc/iterator_python.cc

@@ -78,7 +78,9 @@ ZoneIterator_init(s_ZoneIterator* self, PyObject* args) {
 // In many cases you can use it without modification, but check that carefully.
 void
 ZoneIterator_destroy(s_ZoneIterator* const self) {
-    // cppobj is a shared ptr so no need to delete that
+    // cppobj is a shared ptr, but to make sure things are not destroyed in
+    // the wrong order, we reset it here.
+    self->cppobj.reset();
     Py_TYPE(self)->tp_free(self);
 }
 

src/lib/python/isc/datasrc/tests/datasrc_test.py

@@ -152,7 +152,7 @@ class DataSrcUpdater(unittest.TestCase):
         shutil.copyfile(READ_ZONE_DB_FILE, WRITE_ZONE_DB_FILE)
 
 
-    def test_update(self):
+    def test_update_delete_commit(self):
         dsc = isc.datasrc.DataSourceClient(WRITE_ZONE_DB_FILE)
 
         # first make sure, through a separate finder, that some record exists
@@ -170,13 +170,6 @@ class DataSrcUpdater(unittest.TestCase):
 
         rrset_to_delete = rrset;
 
-        result, rrset = finder.find(isc.dns.Name("doesnotexist.example.com"),
-                                    isc.dns.RRType.TXT(),
-                                    None,
-                                    finder.FIND_DEFAULT)
-        self.assertEqual(finder.NXDOMAIN, result)
-        self.assertEqual(None, rrset)
-
         # can't delete rrset with associated sig. Abuse that to force an
         # exception first, then remove the sig, then delete the record
         updater = dsc.get_updater(isc.dns.Name("example.com"), True)
@@ -185,8 +178,26 @@ class DataSrcUpdater(unittest.TestCase):
         rrset_to_delete.remove_rrsig()
 
         updater.delete_rrset(rrset_to_delete)
+
+        # The record should be gone in the updater, but not in the original
+        # finder (since we have not committed)
+        result, rrset = updater.find(isc.dns.Name("www.example.com"),
+                                     isc.dns.RRType.A(),
+                                     None,
+                                     finder.FIND_DEFAULT)
+        self.assertEqual(finder.NXDOMAIN, result)
+        self.assertEqual(None, rrset)
+
+        result, rrset = finder.find(isc.dns.Name("www.example.com"),
+                                    isc.dns.RRType.A(),
+                                    None,
+                                    finder.FIND_DEFAULT)
+        self.assertEqual(finder.SUCCESS, result)
+        self.assertEqual("www.example.com. 3600 IN A 192.0.2.1\n", rrset.to_text())
+
         updater.commit()
 
+        # the record should be gone now in the 'real' finder as well
         result, rrset = finder.find(isc.dns.Name("www.example.com"),
                                     isc.dns.RRType.A(),
                                     None,
@@ -209,6 +220,53 @@ class DataSrcUpdater(unittest.TestCase):
         self.assertEqual(finder.SUCCESS, result)
         self.assertEqual("www.example.com. 3600 IN A 192.0.2.1\n", rrset.to_text())
 
+    def test_update_delete_abort(self):
+        dsc = isc.datasrc.DataSourceClient(WRITE_ZONE_DB_FILE)
+
+        # first make sure, through a separate finder, that some record exists
+        result, finder = dsc.find_zone(isc.dns.Name("example.com"))
+        self.assertEqual(finder.SUCCESS, result)
+        self.assertEqual(isc.dns.RRClass.IN(), finder.get_class())
+        self.assertEqual("example.com.", finder.get_origin().to_text())
+
+        result, rrset = finder.find(isc.dns.Name("www.example.com"),
+                                    isc.dns.RRType.A(),
+                                    None,
+                                    finder.FIND_DEFAULT)
+        self.assertEqual(finder.SUCCESS, result)
+        self.assertEqual("www.example.com. 3600 IN A 192.0.2.1\n", rrset.to_text())
+
+        rrset_to_delete = rrset;
+
+        # can't delete rrset with associated sig. Abuse that to force an
+        # exception first, then remove the sig, then delete the record
+        updater = dsc.get_updater(isc.dns.Name("example.com"), True)
+        self.assertRaises(isc.datasrc.Error, updater.delete_rrset, rrset_to_delete)
+
+        rrset_to_delete.remove_rrsig()
+
+        updater.delete_rrset(rrset_to_delete)
+
+        # The record should be gone in the updater, but not in the original
+        # finder (since we have not committed)
+        result, rrset = updater.find(isc.dns.Name("www.example.com"),
+                                     isc.dns.RRType.A(),
+                                     None,
+                                     finder.FIND_DEFAULT)
+        self.assertEqual(finder.NXDOMAIN, result)
+        self.assertEqual(None, rrset)
+
+        # destroy the updater, which should make it roll back
+        updater = None
+
+        # the record should still be available in the 'real' finder as well
+        result, rrset = finder.find(isc.dns.Name("www.example.com"),
+                                    isc.dns.RRType.A(),
+                                    None,
+                                    finder.FIND_DEFAULT)
+        self.assertEqual(finder.SUCCESS, result)
+        self.assertEqual("www.example.com. 3600 IN A 192.0.2.1\n", rrset.to_text())
+
 
 if __name__ == "__main__":
     isc.log.init("bind10")

src/lib/python/isc/datasrc/updater_python.cc

@@ -86,14 +86,15 @@ ZoneUpdater_init(s_ZoneUpdater* self, PyObject* args) {
 // In many cases you can use it without modification, but check that carefully.
 void
 ZoneUpdater_destroy(s_ZoneUpdater* const self) {
-    // cppobj is a shared ptr so should take care of itself
+    // cppobj is a shared ptr, but to make sure things are not destroyed in
+    // the wrong order, we reset it here.
+    self->cppobj.reset();
     Py_TYPE(self)->tp_free(self);
 }
 
 // These are the functions we export
 //
 PyObject* ZoneUpdater_addRRset(PyObject* po_self, PyObject* args) {
-    // TODO err handling
     s_ZoneUpdater* const self = static_cast<s_ZoneUpdater*>(po_self);
     PyObject* rrset_obj;
     if (PyArg_ParseTuple(args, "O!", &isc::dns::python::rrset_type, &rrset_obj)) {