[master] Merge branch 'trac2016'

src/lib/python/isc/ddns/session.py

@@ -688,8 +688,9 @@ class UpdateSession:
            Special cases: if the delete statement is for the
            zone's apex, and the type is either SOA or NS, it
            is ignored.'''
-        result, to_delete, _ = self.__diff.find(rrset.get_name(),
-                                                rrset.get_type())
+        # find the rrset with local updates
+        result, to_delete, _ = self.__diff.find_updated(rrset.get_name(),
+                                                        rrset.get_type())
         if result == ZoneFinder.SUCCESS:
             if to_delete.get_name() == self.__zname and\
                (to_delete.get_type() == RRType.SOA() or\
@@ -705,14 +706,10 @@ class UpdateSession:
            may never be removed (and any action that would do so
            should be ignored).
         '''
-        # NOTE: This method is currently bad: it WILL delete all
-        # NS rrsets in a number of cases.
-        # We need an extension to our diff.py to handle this correctly
-        # (see ticket #2016)
-        # The related test is currently disabled. When this is fixed,
-        # enable that test again.
-        result, orig_rrset, _ = self.__diff.find(rrset.get_name(),
-                                                 rrset.get_type())
+        # Find the current NS rrset, including local additions and deletions
+        result, orig_rrset, _ = self.__diff.find_updated(rrset.get_name(),
+                                                         rrset.get_type())
+
         # Even a real rrset comparison wouldn't help here...
         # The goal is to make sure that after deletion of the
         # given rrset, at least 1 NS record is left (at the apex).
@@ -742,7 +739,8 @@ class UpdateSession:
            Special case: if the name is the zone's apex, SOA and
            NS records are kept.
         '''
-        result, rrsets, flags = self.__diff.find_all(rrset.get_name())
+        # Find everything with the name, including local additions
+        result, rrsets, flags = self.__diff.find_all_updated(rrset.get_name())
         if result == ZoneFinder.SUCCESS and\
            (flags & ZoneFinder.RESULT_WILDCARD == 0):
             for to_delete in rrsets:

src/lib/python/isc/ddns/tests/session_tests.py

@@ -18,7 +18,7 @@ import shutil
 import isc.log
 import unittest
 from isc.dns import *
-from isc.datasrc import DataSourceClient
+from isc.datasrc import DataSourceClient, ZoneFinder
 from isc.ddns.session import *
 from isc.ddns.zone_config import *
 
@@ -677,7 +677,6 @@ class SessionTest(SessionTestBase):
         else:
             self.assertEqual(UPDATE_ERROR, result)
 
-
     def test_check_prerequisites(self):
         # This test checks if the actual prerequisite-type-specific
         # methods are called.
@@ -853,7 +852,6 @@ class SessionTest(SessionTestBase):
                                               "1233 3600 1800 2419200 7200" ])
         self.rrset_update_soa_del = rrset_update_soa_del
 
-
         rrset_update_soa2 = create_rrset("example.org", TEST_RRCLASS,
                                          RRType.SOA(), 3600,
                                          [ "ns1.example.org. " +
@@ -974,7 +972,6 @@ class SessionTest(SessionTestBase):
         rrset = create_rrset("different.zone", RRClass.ANY(), RRType.TXT(), 0)
         self.check_prescan_result(Rcode.NOTZONE(), [ rrset ])
 
-
         # forbidden type, zone class
         rrset = create_rrset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.ANY(), 0,
                              [ b'\x00' ])
@@ -1207,6 +1204,9 @@ class SessionTest(SessionTestBase):
                                  rrset2)
 
     def test_update_delete_name(self):
+        '''
+        Tests whether deletion of every RR for a name works
+        '''
         self.__initialize_update_rrsets()
 
         # First check it is there
@@ -1268,7 +1268,7 @@ class SessionTest(SessionTestBase):
                                  isc.dns.Name("example.org"),
                                  RRType.MX())
 
-        # Check that we cannot delete the SOA record by direction deletion
+        # Check that we cannot delete the SOA record by direct deletion
         # both by name+type and by full rrset
         self.check_full_handle_result(Rcode.NOERROR(),
                                       [ self.rrset_update_del_soa_apex,
@@ -1304,7 +1304,7 @@ class SessionTest(SessionTestBase):
                                  RRType.NS(),
                                  orig_ns_rrset)
 
-    def DISABLED_test_update_apex_special_case_ns_rrset(self):
+    def test_update_apex_special_case_ns_rrset(self):
         # If we delete the NS at the apex specifically, it should still
         # keep one record
         self.__initialize_update_rrsets()
@@ -1319,6 +1319,21 @@ class SessionTest(SessionTestBase):
                                  RRType.NS(),
                                  short_ns_rrset)
 
+    def test_update_apex_special_case_ns_rrset2(self):
+        # If we add new NS records, then delete all existing ones, it
+        # should not keep any
+        self.__initialize_update_rrsets()
+        new_ns = create_rrset("example.org", TEST_RRCLASS, RRType.NS(), 3600,
+                              [ "newns1.example.org", "newns2.example.org" ])
+
+        self.check_full_handle_result(Rcode.NOERROR(),
+                                      [ new_ns,
+                                        self.rrset_update_del_rrset_ns ])
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("example.org"),
+                                 RRType.NS(),
+                                 new_ns)
+
     def test_update_delete_normal_rrset_at_apex(self):
         '''
         Tests a number of 'normal rrset' deletes at the apex
@@ -1335,6 +1350,64 @@ class SessionTest(SessionTestBase):
                                  isc.dns.Name("example.org"),
                                  RRType.MX())
 
+    def test_update_add_then_delete_rrset(self):
+        # If we add data, then delete the whole rrset, added data should
+        # be gone as well
+        self.__initialize_update_rrsets()
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
+        self.check_full_handle_result(Rcode.NOERROR(),
+                                      [ self.rrset_update_a,
+                                        self.rrset_update_del_rrset ])
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
+
+    def test_update_add_then_delete_name(self):
+        # If we add data, then delete the entire name, added data should
+        # be gone as well
+        self.__initialize_update_rrsets()
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
+        self.check_full_handle_result(Rcode.NOERROR(),
+                                      [ self.rrset_update_a,
+                                        self.rrset_update_del_name ])
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
+
+    def test_update_delete_then_add_rrset(self):
+        # If we delete an entire rrset, then add something there again,
+        # the addition should be done
+        self.__initialize_update_rrsets()
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
+        self.check_full_handle_result(Rcode.NOERROR(),
+                                      [ self.rrset_update_del_rrset,
+                                        self.rrset_update_a ])
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A(),
+                                 self.rrset_update_a)
+
+    def test_update_delete_then_add_rrset(self):
+        # If we delete an entire name, then add something there again,
+        # the addition should be done
+        self.__initialize_update_rrsets()
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
+        self.check_full_handle_result(Rcode.NOERROR(),
+                                      [ self.rrset_update_del_name,
+                                        self.rrset_update_a ])
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A(),
+                                 self.rrset_update_a)
+
     def test_update_cname_special_cases(self):
         self.__initialize_update_rrsets()
 

src/lib/python/isc/xfrin/diff.py

@@ -24,6 +24,7 @@ But for now, it lives here.
 """
 
 import isc.dns
+from isc.datasrc import ZoneFinder
 import isc.log
 from isc.datasrc import ZoneFinder
 from isc.log_messages.libxfrin_messages import *
@@ -180,9 +181,13 @@ class Diff:
                              str(self.__updater.get_class()))
         if self.__single_update_mode:
             if operation == 'add':
-                self.__append_with_soa_check(self.__additions, operation, rr)
+                if not self._remove_rr_from_deletions(rr):
+                    self.__append_with_soa_check(self.__additions, operation,
+                                                 rr)
             elif operation == 'delete':
-                self.__append_with_soa_check(self.__deletions, operation, rr)
+                if not self._remove_rr_from_additions(rr):
+                    self.__append_with_soa_check(self.__deletions, operation,
+                                                 rr)
         else:
             self.__buffer.append((operation, rr))
             if len(self.__buffer) >= DIFF_APPLY_TRESHOLD:
@@ -407,3 +412,175 @@ class Diff:
         """
         self.__check_committed()
         return self.__updater.find_all(name, options)
+
+    def __remove_rr_from_buffer(self, buf, rr):
+        '''Helper for common code in remove_rr_from_deletions() and
+           remove_rr_from_additions();
+           returns the result of the removal operation on the given buffer
+        '''
+        def same_rr(a, b):
+            # Consider two rr's the same if name, type, and rdata match
+            # Note that at this point it should have been checked that
+            # the rr in the buffer and the given rr have exactly one rdata
+            return a.get_name() == b.get_name() and\
+                   a.get_type() == b.get_type() and\
+                   a.get_rdata()[0] == b.get_rdata()[0]
+        if rr.get_type() == isc.dns.RRType.SOA():
+            return buf
+        else:
+            return [ op for op in buf if not same_rr(op[1], rr)]
+
+    def _remove_rr_from_deletions(self, rr):
+        '''
+        Removes the given rr from the currently buffered deletions;
+        returns True if anything is removed, False if the RR was not present.
+        This method is protected; it is not meant to be called from anywhere
+        but the add_data() method. It is not private for easier testing.
+        '''
+        orig_size = len(self.__deletions)
+        self.__deletions = self.__remove_rr_from_buffer(self.__deletions, rr)
+        return len(self.__deletions) != orig_size
+
+    def _remove_rr_from_additions(self, rr):
+        '''
+        Removes the given rr from the currently buffered additions;
+        returns True if anything is removed, False if the RR was not present.
+        This method is protected; it is not meant to be called from anywhere
+        but the delete_data() method. It is not private for easier testing.
+        '''
+        orig_size = len(self.__additions)
+        self.__additions = self.__remove_rr_from_buffer(self.__additions, rr)
+        return len(self.__additions) != orig_size
+
+    def __get_name_from_additions(self, name):
+        '''
+        Returns a list of all rrs in the additions queue that have the given
+        Name.
+        This method is protected; it is not meant to be called from anywhere
+        but the find_all_updated() method. It is not private for easier
+        testing.
+        '''
+        return [ rr for (_, rr) in self.__additions if rr.get_name() == name ]
+
+    def __get_name_from_deletions(self, name):
+        '''
+        Returns a list of all rrs in the deletions queue that have the given
+        Name
+        This method is protected; it is not meant to be called from anywhere
+        but the find_all_updated() method. It is not private for easier
+        testing.
+        '''
+        return [ rr for (_, rr) in self.__deletions if rr.get_name() == name ]
+
+    def __get_name_type_from_additions(self, name, rrtype):
+        '''
+        Returns a list of the rdatas of the rrs in the additions queue that
+        have the given name and type
+        This method is protected; it is not meant to be called from anywhere
+        but the find_updated() method. It is not private for easier testing.
+        '''
+        return [ rr for (_, rr) in self.__additions\
+                    if rr.get_name() == name and rr.get_type() == rrtype ]
+
+    def __get_name_type_from_deletions(self, name, rrtype):
+        '''
+        Returns a list of the rdatas of the rrs in the deletions queue that
+        have the given name and type
+        This method is protected; it is not meant to be called from anywhere
+        but the find_updated() method. It is not private for easier testing.
+        '''
+        return [ rr.get_rdata()[0] for (_, rr) in self.__deletions\
+                    if rr.get_name() == name and rr.get_type() == rrtype ]
+
+    def find_updated(self, name, rrtype):
+        '''
+        Returns the result of find(), but with current updates applied, i.e.
+        as if this diff has been committed. Only performs additional
+        processing in the case find() returns SUCCESS, NXDOMAIN, or NXRRSET;
+        in all other cases, the results are returned directly.
+        Any RRs in the current deletions buffer are removed from the result,
+        and RRs in the current additions buffer are added to the result.
+        If the result was SUCCESS, but every RR in it is removed due to
+        deletions, and there is nothing in the additions, the rcode is changed
+        to NXRRSET.
+        If the result was NXDOMAIN or NXRRSET, and there are rrs in the
+        additions buffer, the result is changed to SUCCESS.
+        '''
+        if not self.__single_update_mode:
+            raise ValueError("find_updated() can only be used in " +
+                             "single-update mode")
+        result, rrset, flags = self.find(name, rrtype)
+
+        added_rrs = self.__get_name_type_from_additions(name, rrtype)
+        deleted_rrs = self.__get_name_type_from_deletions(name, rrtype)
+
+        if result == ZoneFinder.SUCCESS:
+            new_rrset = isc.dns.RRset(name, self.__updater.get_class(),
+                                      rrtype, rrset.get_ttl())
+            for rdata in rrset.get_rdata():
+                if rdata not in deleted_rrs:
+                    new_rrset.add_rdata(rdata)
+            # If all data has been deleted, and there is nothing to add
+            # we cannot really know whether it is NXDOMAIN or NXRRSET,
+            # NXRRSET seems safest (we could find out, but it would require
+            # another search on the name which is probably not worth the
+            # trouble
+            if new_rrset.get_rdata_count() == 0 and len(added_rrs) == 0:
+                result = ZoneFinder.NXRRSET
+                new_rrset = None
+        elif (result == ZoneFinder.NXDOMAIN or result == ZoneFinder.NXRRSET)\
+             and len(added_rrs) > 0:
+            new_rrset = isc.dns.RRset(name, self.__updater.get_class(),
+                                      rrtype, added_rrs[0].get_ttl())
+            # There was no data in the zone, but there is data now
+            result = ZoneFinder.SUCCESS
+        else:
+            # Can't reliably handle other cases, just return the original
+            # data
+            return result, rrset, flags
+
+        for rr in added_rrs:
+            # Can only be 1-rr RRsets at this point
+            new_rrset.add_rdata(rr.get_rdata()[0])
+
+        return result, new_rrset, flags
+
+    def find_all_updated(self, name):
+        '''
+        Returns the result of find_all(), but with current updates applied,
+        i.e. as if this diff has been committed. Only performs additional
+        processing in the case find() returns SUCCESS or NXDOMAIN;
+        in all other cases, the results are returned directly.
+        Any RRs in the current deletions buffer are removed from the result,
+        and RRs in the current additions buffer are added to the result.
+        If the result was SUCCESS, but every RR in it is removed due to
+        deletions, and there is nothing in the additions, the rcode is changed
+        to NXDOMAIN.
+        If the result was NXDOMAIN, and there are rrs in the additions buffer,
+        the result is changed to SUCCESS.
+        '''
+        if not self.__single_update_mode:
+            raise ValueError("find_all_updated can only be used in " +
+                             "single-update mode")
+        result, rrsets, flags = self.find_all(name)
+        new_rrsets = []
+        added_rrs = self.__get_name_from_additions(name)
+        if result == ZoneFinder.SUCCESS and\
+           (flags & ZoneFinder.RESULT_WILDCARD == 0):
+            deleted_rrs = self.__get_name_from_deletions(name)
+            for rr in rrsets:
+                if rr not in deleted_rrs:
+                    new_rrsets.append(rr)
+            if len(new_rrsets) == 0 and len(added_rrs) == 0:
+                result = ZoneFinder.NXDOMAIN
+        elif result == ZoneFinder.NXDOMAIN and\
+            len(added_rrs) > 0:
+            result = ZoneFinder.SUCCESS
+        else:
+            # Can't reliably handle other cases, just return the original
+            # data
+            return result, rrsets, flags
+        for rr in added_rrs:
+            if rr.get_name() == name:
+                new_rrsets.append(rr)
+        return result, new_rrsets, flags

src/lib/python/isc/xfrin/tests/diff_tests.py

@@ -77,6 +77,31 @@ class DiffTest(unittest.TestCase):
         self.__rrset_multi.add_rdata(Rdata(self.__type, self.__rrclass,
                                            '192.0.2.2'))
 
+        # Also create a few other (valid) rrsets
+        # A SOA record
+        self.__rrset_soa = RRset(Name('example.org.'), self.__rrclass,
+                                 RRType.SOA(), RRTTL(3600))
+        self.__rrset_soa.add_rdata(Rdata(RRType.SOA(), self.__rrclass,
+                                         "ns1.example.org. " +
+                                         "admin.example.org. " +
+                                         "1233 3600 1800 2419200 7200"))
+        # A few single-rr rrsets that together would for a multi-rr rrset
+        self.__rrset3 = RRset(Name('c.example.org.'), self.__rrclass,
+                              RRType.TXT(), self.__ttl)
+        self.__rrset3.add_rdata(Rdata(RRType.TXT(), self.__rrclass, "one"))
+        self.__rrset4 = RRset(Name('c.example.org.'), self.__rrclass,
+                              RRType.TXT(), self.__ttl)
+        self.__rrset4.add_rdata(Rdata(RRType.TXT(), self.__rrclass, "two"))
+        self.__rrset5 = RRset(Name('c.example.org.'), self.__rrclass,
+                              RRType.TXT(), self.__ttl)
+        self.__rrset5.add_rdata(Rdata(RRType.TXT(), self.__rrclass, "three"))
+        self.__rrset6 = RRset(Name('d.example.org.'), self.__rrclass,
+                              RRType.A(), self.__ttl)
+        self.__rrset6.add_rdata(Rdata(RRType.A(), self.__rrclass, "192.0.2.1"))
+        self.__rrset7 = RRset(Name('d.example.org.'), self.__rrclass,
+                              RRType.A(), self.__ttl)
+        self.__rrset7.add_rdata(Rdata(RRType.A(), self.__rrclass, "192.0.2.2"))
+
     def __mock_compact(self):
         """
         This can be put into the diff to hook into its compact method and see
@@ -525,6 +550,17 @@ class DiffTest(unittest.TestCase):
         self.assertRaises(ValueError, diff_single.get_buffer)
         self.assertEqual(([], []), diff_single.get_single_update_buffers())
 
+    def test_finds_single(self):
+        '''
+        Test that find_updated() and find_all_updated() can only be used
+        in single-update-mode.
+        '''
+        diff_multi = Diff(self, Name('example.org.'), single_update_mode=False)
+        self.assertRaises(ValueError, diff_multi.find_updated,
+                          Name('example.org.'), RRType.A())
+        self.assertRaises(ValueError, diff_multi.find_all_updated,
+                          Name('example.org.'))
+
     def test_single_update_mode(self):
         '''
         Test single-update mode. In this mode, updates and deletes can
@@ -533,55 +569,47 @@ class DiffTest(unittest.TestCase):
         and it must be the first change.
         '''
 
-        # First create some RRsets to play with
-        soa = RRset(Name('example.org.'), self.__rrclass, RRType.SOA(),
-                    RRTTL(3600))
-        soa.add_rdata(Rdata(soa.get_type(), soa.get_class(),
-                      "ns.example.org. foo.example.org. 1234 28800 "+
-                      "7200 604800 3600"))
-
-        a = RRset(Name('www.example.org.'), self.__rrclass, RRType.A(),
-                      RRTTL(3600))
-        a.add_rdata(Rdata(a.get_type(), a.get_class(),
-                        "192.0.2.1"))
-
-        a2 = RRset(Name('www.example.org.'), self.__rrclass, RRType.A(),
-                      RRTTL(3600))
-        a2.add_rdata(Rdata(a2.get_type(), a2.get_class(),
-                        "192.0.2.2"))
-
         # full rrset for A (to check compact())
-        a_1_2 = RRset(Name('www.example.org.'), self.__rrclass, RRType.A(),
-                      RRTTL(3600))
-        a_1_2.add_rdata(Rdata(a_1_2.get_type(), a_1_2.get_class(),
-                        "192.0.2.1"))
-        a_1_2.add_rdata(Rdata(a_1_2.get_type(), a_1_2.get_class(),
-                        "192.0.2.2"))
+        txt = RRset(Name('c.example.org.'), self.__rrclass, RRType.TXT(),
+                    RRTTL(3600))
+        txt.add_rdata(Rdata(txt.get_type(), txt.get_class(), "one"))
+        txt.add_rdata(Rdata(txt.get_type(), txt.get_class(), "two"))
+        txt.add_rdata(Rdata(txt.get_type(), txt.get_class(), "three"))
+        a = RRset(Name('d.example.org.'), self.__rrclass, RRType.A(),
+                  RRTTL(3600))
+        a.add_rdata(Rdata(a.get_type(), a.get_class(), "192.0.2.1"))
+        a.add_rdata(Rdata(a.get_type(), a.get_class(), "192.0.2.2"))
 
         diff = Diff(self, Name('example.org.'), single_update_mode=True)
 
         # adding a first should fail
         self.assertRaises(ValueError, diff.add_data, a)
         # But soa should work
-        diff.add_data(soa)
+        diff.add_data(self.__rrset_soa)
         # And then A should as well
-        diff.add_data(a)
-        diff.add_data(a2)
+        diff.add_data(self.__rrset3)
+        diff.add_data(self.__rrset4)
+        diff.add_data(self.__rrset5)
         # But another SOA should fail again
-        self.assertRaises(ValueError, diff.add_data, soa)
+        self.assertRaises(ValueError, diff.add_data, self.__rrset_soa)
 
         # Same for delete
-        self.assertRaises(ValueError, diff.delete_data, a)
-        diff.delete_data(soa)
-        diff.delete_data(a)
-        diff.delete_data(a2)
-        self.assertRaises(ValueError, diff.delete_data, soa)
+        self.assertRaises(ValueError, diff.delete_data, self.__rrset6)
+        diff.delete_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset6)
+        diff.delete_data(self.__rrset7)
+        self.assertRaises(ValueError, diff.delete_data, self.__rrset_soa)
 
         # Not compacted yet, so the buffers should be as we
         # filled them
         (delbuf, addbuf) = diff.get_single_update_buffers()
-        self.assertEqual([('delete', soa), ('delete', a), ('delete', a2)], delbuf)
-        self.assertEqual([('add', soa), ('add', a), ('add', a2)], addbuf)
+        self.assertEqual([('delete', self.__rrset_soa),
+                          ('delete', self.__rrset6),
+                          ('delete', self.__rrset7)], delbuf)
+        self.assertEqual([('add', self.__rrset_soa),
+                          ('add', self.__rrset3),
+                          ('add', self.__rrset4),
+                          ('add', self.__rrset5)], addbuf)
 
         # Compact should compact the A records in both buffers
         diff.compact()
@@ -590,18 +618,18 @@ class DiffTest(unittest.TestCase):
         self.assertEqual(2, len(delbuf))
         self.assertEqual(2, len(delbuf[0]))
         self.assertEqual('delete', delbuf[0][0])
-        self.assertEqual(soa.to_text(), delbuf[0][1].to_text())
+        self.assertEqual(self.__rrset_soa.to_text(), delbuf[0][1].to_text())
         self.assertEqual(2, len(delbuf[1]))
         self.assertEqual('delete', delbuf[1][0])
-        self.assertEqual(a_1_2.to_text(), delbuf[1][1].to_text())
+        self.assertEqual(a.to_text(), delbuf[1][1].to_text())
 
         self.assertEqual(2, len(addbuf))
         self.assertEqual(2, len(addbuf[0]))
         self.assertEqual('add', addbuf[0][0])
-        self.assertEqual(soa.to_text(), addbuf[0][1].to_text())
+        self.assertEqual(self.__rrset_soa.to_text(), addbuf[0][1].to_text())
         self.assertEqual(2, len(addbuf[1]))
         self.assertEqual('add', addbuf[1][0])
-        self.assertEqual(a_1_2.to_text(), addbuf[1][1].to_text())
+        self.assertEqual(txt.to_text(), addbuf[1][1].to_text())
 
         # Apply should reset the buffers
         diff.apply()
@@ -614,6 +642,40 @@ class DiffTest(unittest.TestCase):
         self.assertRaises(ValueError, diff.add_data, a)
         self.assertRaises(ValueError, diff.delete_data, a)
 
+    def test_add_delete_same(self):
+        '''
+        Test that if a record is added, then deleted, it is not added to
+        both buffers, but remove from the addition, and vice versa
+        '''
+        diff = Diff(self, Name('example.org.'), single_update_mode=True)
+        # Need SOA records first
+        diff.delete_data(self.__rrset_soa)
+        diff.add_data(self.__rrset_soa)
+
+        deletions, additions = diff.get_single_update_buffers()
+        self.assertEqual(1, len(deletions))
+        self.assertEqual(1, len(additions))
+
+        diff.add_data(self.__rrset1)
+        deletions, additions = diff.get_single_update_buffers()
+        self.assertEqual(1, len(deletions))
+        self.assertEqual(2, len(additions))
+
+        diff.delete_data(self.__rrset1)
+        deletions, additions = diff.get_single_update_buffers()
+        self.assertEqual(1, len(deletions))
+        self.assertEqual(1, len(additions))
+
+        diff.delete_data(self.__rrset2)
+        deletions, additions = diff.get_single_update_buffers()
+        self.assertEqual(2, len(deletions))
+        self.assertEqual(1, len(additions))
+
+        diff.add_data(self.__rrset2)
+        deletions, additions = diff.get_single_update_buffers()
+        self.assertEqual(1, len(deletions))
+        self.assertEqual(1, len(additions))
+
     def test_find(self):
         diff = Diff(self, Name('example.org.'))
         name = Name('www.example.org.')
@@ -675,6 +737,356 @@ class DiffTest(unittest.TestCase):
         self.assertEqual(name, self.__find_all_name)
         self.assertEqual(options, self.__find_all_options)
 
+    def __common_remove_rr_from_buffer(self, diff, add_method, remove_method,
+                                       op_str, buf_nr):
+        add_method(self.__rrset_soa)
+        add_method(self.__rrset2)
+        add_method(self.__rrset3)
+        add_method(self.__rrset4)
+
+        # sanity check
+        buf = diff.get_single_update_buffers()[buf_nr]
+        expected = [ (op_str, str(rr)) for rr in [ self.__rrset_soa,
+                                                   self.__rrset2,
+                                                   self.__rrset3,
+                                                   self.__rrset4 ] ]
+        result = [ (op, str(rr)) for (op, rr) in buf ]
+        self.assertEqual(expected, result)
+
+        # remove one
+        self.assertTrue(remove_method(self.__rrset2))
+        buf = diff.get_single_update_buffers()[buf_nr]
+        expected = [ (op_str, str(rr)) for rr in [ self.__rrset_soa,
+                                                   self.__rrset3,
+                                                   self.__rrset4 ] ]
+        result = [ (op, str(rr)) for (op, rr) in buf ]
+        self.assertEqual(expected, result)
+
+        # SOA should not be removed
+        self.assertFalse(remove_method(self.__rrset_soa))
+        buf = diff.get_single_update_buffers()[buf_nr]
+        expected = [ (op_str, str(rr)) for rr in [ self.__rrset_soa,
+                                                   self.__rrset3,
+                                                   self.__rrset4 ] ]
+        result = [ (op, str(rr)) for (op, rr) in buf ]
+        self.assertEqual(expected, result)
+
+        # remove another
+        self.assertTrue(remove_method(self.__rrset4))
+        buf = diff.get_single_update_buffers()[buf_nr]
+        expected = [ (op_str, str(rr)) for rr in [ self.__rrset_soa,
+                                                   self.__rrset3 ] ]
+        result = [ (op, str(rr)) for (op, rr) in buf ]
+        self.assertEqual(expected, result)
+
+        # remove nonexistent should return False
+        self.assertFalse(remove_method(self.__rrset4))
+        buf = diff.get_single_update_buffers()[buf_nr]
+        expected = [ (op_str, str(rr)) for rr in [ self.__rrset_soa,
+                                                   self.__rrset3 ] ]
+        result = [ (op, str(rr)) for (op, rr) in buf ]
+        self.assertEqual(expected, result)
+
+    def test_remove_rr_from_additions(self):
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+        self.__common_remove_rr_from_buffer(diff, diff.add_data,
+                                               diff._remove_rr_from_additions,
+                                               'add', 1)
+
+    def test_remove_rr_from_deletions(self):
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+        self.__common_remove_rr_from_buffer(diff, diff.delete_data,
+                                            diff._remove_rr_from_deletions,
+                                            'delete', 0)
+
+    def __create_find(self, result, rrset, flags):
+        '''
+        Overwrites the local find() method with a method that returns
+        the tuple (result, rrset, flags)
+        '''
+        def new_find(name, rrtype, fflags):
+            return (result, rrset, flags)
+        self.find = new_find
+
+    def __create_find_all(self, result, rrsets, flags):
+        '''
+        Overwrites the local find() method with a method that returns
+        the tuple (result, rrsets, flags)
+        '''
+        def new_find_all(name, fflags):
+            return (result, rrsets, flags)
+        self.find_all = new_find_all
+
+    def __check_find_call(self, method, query_rrset, expected_rcode,
+                          expected_rdatas=None):
+        '''
+        Helper for find tests; calls the given method with the name and
+        type of the given rrset. Checks for the given rcode.
+        If expected_rdatas is not none, the result name, and type are
+        checked to match the given rrset ones, and the rdatas are checked
+        to be equal.
+        The given method must have the same arguments and return type
+        as find()
+        '''
+        result, rrset, _ = method(query_rrset.get_name(),
+                                  query_rrset.get_type())
+        self.assertEqual(expected_rcode, result)
+        if expected_rdatas is not None:
+            self.assertEqual(query_rrset.get_name(), rrset.get_name())
+            self.assertEqual(query_rrset.get_type(), rrset.get_type())
+            if expected_rdatas is not None:
+                self.assertEqual(expected_rdatas, rrset.get_rdata())
+        else:
+            self.assertEqual(None, rrset)
+
+    def __check_find_all_call(self, method, query_rrset, expected_rcode,
+                              expected_rrs=[]):
+        '''
+        Helper for find tests; calls the given method with the name and
+        type of the given rrset. Checks for the given rcode.
+        If expected_rdatas is not none, the result name, and type are
+        checked to match the given rrset ones, and the rdatas are checked
+        to be equal.
+        The given method must have the same arguments and return type
+        as find()
+        '''
+        result, rrsets, _ = method(query_rrset.get_name())
+        self.assertEqual(expected_rcode, result)
+        # We have no real equality function for rrsets, but since
+        # the rrsets in question are themselves returns, pointer equality
+        # works as well
+        self.assertEqual(expected_rrs, rrsets)
+
+    def test_find_updated_existing_data(self):
+        '''
+        Tests whether existent data is updated with the additions and
+        deletions from the Diff
+        '''
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+        diff.add_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset_soa)
+
+        # override the actual find method
+        self.__create_find(ZoneFinder.SUCCESS, self.__rrset3, 0)
+
+        # sanity check
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+
+        # check that normal find also returns the original data
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+
+        # Adding another should have it returned in the find_updated
+        diff.add_data(self.__rrset4)
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata() +
+                               self.__rrset4.get_rdata())
+
+        # check that normal find still returns the original data
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+
+        # Adding a different type should have no effect
+        diff.add_data(self.__rrset2)
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata() +
+                               self.__rrset4.get_rdata())
+
+        # check that normal find still returns the original data
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+
+        # Deleting 3 now should result in only 4 being updated
+        diff.delete_data(self.__rrset3)
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset4.get_rdata())
+
+        # check that normal find still returns the original data
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+
+        # Deleting 4 now should result in empty rrset
+        diff.delete_data(self.__rrset4)
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.NXRRSET)
+
+        # check that normal find still returns the original data
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+
+    def test_find_updated_nonexistent_data(self):
+        '''
+        Test whether added data for a query that would originally result
+        in NXDOMAIN works
+        '''
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+        diff.add_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset_soa)
+
+        # override the actual find method
+        self.__create_find(ZoneFinder.NXDOMAIN, None, 0)
+
+        # Sanity check
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.NXDOMAIN)
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.NXDOMAIN)
+
+        # Add data and see it is returned
+        diff.add_data(self.__rrset3)
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.NXDOMAIN)
+
+        # Add unrelated data, result should be the same
+        diff.add_data(self.__rrset2)
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.NXDOMAIN)
+
+        # Remove, result should now be NXDOMAIN again
+        diff.delete_data(self.__rrset3)
+        result, rrset, _ = diff.find_updated(self.__rrset3.get_name(),
+                                             self.__rrset3.get_type())
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.NXDOMAIN)
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.NXDOMAIN)
+
+    def test_find_updated_other(self):
+        '''
+        Test that any other ZoneFinder.result code is directly
+        passed on.
+        '''
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+
+        # Add and delete some data to make sure it's not used
+        diff.add_data(self.__rrset_soa)
+        diff.add_data(self.__rrset3)
+        diff.delete_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset2)
+
+        for rcode in [ ZoneFinder.DELEGATION,
+                       ZoneFinder.CNAME,
+                       ZoneFinder.DNAME ]:
+            # override the actual find method
+            self.__create_find(rcode, None, 0)
+            self.__check_find_call(diff.find, self.__rrset3, rcode)
+            self.__check_find_call(diff.find_updated, self.__rrset3, rcode)
+
+    def test_find_all_existing_data(self):
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+        diff.add_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset_soa)
+
+        # override the actual find method
+        self.__create_find_all(ZoneFinder.SUCCESS, [ self.__rrset3 ], 0)
+
+        # Sanity check
+        result, rrsets, _ = diff.find_all_updated(self.__rrset3.get_name())
+        self.assertEqual(ZoneFinder.SUCCESS, result)
+        self.assertEqual([self.__rrset3], rrsets)
+
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset3,
+                                   ZoneFinder.SUCCESS, [self.__rrset3])
+        self.__check_find_all_call(diff.find_all, self.__rrset3,
+                                   ZoneFinder.SUCCESS, [self.__rrset3])
+
+        # Add a second rr with different type at same name
+        add_rrset = RRset(self.__rrset3.get_name(), self.__rrclass,
+                          RRType.A(), self.__ttl)
+        add_rrset.add_rdata(Rdata(RRType.A(), self.__rrclass, "192.0.2.2"))
+        diff.add_data(add_rrset)
+
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset3,
+                                   ZoneFinder.SUCCESS,
+                                   [self.__rrset3, add_rrset])
+        self.__check_find_all_call(diff.find_all, self.__rrset3,
+                                   ZoneFinder.SUCCESS, [self.__rrset3])
+
+        # Remove original one
+        diff.delete_data(self.__rrset3)
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset3,
+                                   ZoneFinder.SUCCESS, [add_rrset])
+        self.__check_find_all_call(diff.find_all, self.__rrset3,
+                                   ZoneFinder.SUCCESS, [self.__rrset3])
+
+        # And remove new one, result should then become NXDOMAIN
+        diff.delete_data(add_rrset)
+        result, rrsets, _ = diff.find_all_updated(self.__rrset3.get_name())
+
+        self.assertEqual(ZoneFinder.NXDOMAIN, result)
+        self.assertEqual([ ], rrsets)
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset3,
+                                   ZoneFinder.NXDOMAIN)
+        self.__check_find_all_call(diff.find_all, self.__rrset3,
+                                   ZoneFinder.SUCCESS, [self.__rrset3])
+
+    def test_find_all_nonexistent_data(self):
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+        diff.add_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset_soa)
+
+        self.__create_find_all(ZoneFinder.NXDOMAIN, [], 0)
+
+        # Sanity check
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset2,
+                                   ZoneFinder.NXDOMAIN)
+        self.__check_find_all_call(diff.find_all, self.__rrset2,
+                                   ZoneFinder.NXDOMAIN)
+
+        # Adding data should change the result
+        diff.add_data(self.__rrset2)
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset2,
+                                   ZoneFinder.SUCCESS, [ self.__rrset2 ])
+        self.__check_find_all_call(diff.find_all, self.__rrset2,
+                                   ZoneFinder.NXDOMAIN)
+
+        # Adding data at other name should not
+        diff.add_data(self.__rrset3)
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset2,
+                                   ZoneFinder.SUCCESS, [ self.__rrset2 ])
+        self.__check_find_all_call(diff.find_all, self.__rrset2,
+                                   ZoneFinder.NXDOMAIN)
+
+        # Deleting it should revert to original
+        diff.delete_data(self.__rrset2)
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset2,
+                                   ZoneFinder.NXDOMAIN)
+        self.__check_find_all_call(diff.find_all, self.__rrset2,
+                                   ZoneFinder.NXDOMAIN)
+
+    def test_find_all_other_results(self):
+        '''
+        Any result code other than SUCCESS and NXDOMAIN should cause
+        the results to be passed on directly
+        '''
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+
+        # Add and delete some data to make sure it's not used
+        diff.add_data(self.__rrset_soa)
+        diff.add_data(self.__rrset3)
+        diff.delete_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset2)
+
+        for rcode in [ ZoneFinder.NXRRSET,
+                       ZoneFinder.DELEGATION,
+                       ZoneFinder.CNAME,
+                       ZoneFinder.DNAME ]:
+            # override the actual find method
+            self.__create_find_all(rcode, [], 0)
+            self.__check_find_all_call(diff.find_all_updated, self.__rrset2,
+                                       rcode)
+            self.__check_find_all_call(diff.find_all_updated, self.__rrset3,
+                                       rcode)
+            self.__check_find_all_call(diff.find_all, self.__rrset2,
+                                       rcode)
+            self.__check_find_all_call(diff.find_all, self.__rrset3,
+                                       rcode)
+
 if __name__ == "__main__":
     isc.log.init("bind10")
     isc.log.resetUnitTestRootLogger()