Browse Source

[1457] Address (most) review comments

replaced rrsets_as_rrs() by a much simpler generator
style fixes such as private methods
added additional tests
Jelte Jansen 13 years ago
parent
commit
bc9715b080

+ 2 - 3
src/lib/python/isc/ddns/libddns_messages.mes

@@ -109,8 +109,7 @@ A FORMERR response is sent back to the client.
 % LIBDDNS_UPDATE_DATASRC_ERROR error in datasource during DDNS update: %1
 % LIBDDNS_UPDATE_DATASRC_ERROR error in datasource during DDNS update: %1
 An error occured while committing the DDNS update changes to the
 An error occured while committing the DDNS update changes to the
 datasource. The specific error is printed. A SERVFAIL response is sent
 datasource. The specific error is printed. A SERVFAIL response is sent
-back to the client. In theory, if the datasource is implemented
-correctly, no changes should have been made to the zone contents.
+back to the client.
 
 
 % LIBDDNS_UPDATE_DELETE_BAD_TYPE update client %1 for zone %2: update deletion RR bad type: %3
 % LIBDDNS_UPDATE_DELETE_BAD_TYPE update client %1 for zone %2: update deletion RR bad type: %3
 The Update section of a DDNS update message contains a statement
 The Update section of a DDNS update message contains a statement
@@ -126,7 +125,7 @@ A FORMERR response is sent back to the client.
 
 
 % LIBDDNS_UPDATE_DELETE_RRSET_NOT_EMPTY update client %1 for zone %2: update deletion RR contains data %3
 % LIBDDNS_UPDATE_DELETE_RRSET_NOT_EMPTY update client %1 for zone %2: update deletion RR contains data %3
 The Update section of a DDNS update message contains a 'delete rrset'
 The Update section of a DDNS update message contains a 'delete rrset'
-statement with a non-empty Rrset. This is not allowed by the protocol.
+statement with a non-empty RRset. This is not allowed by the protocol.
 A FORMERR response is sent back to the client.
 A FORMERR response is sent back to the client.
 
 
 % LIBDDNS_UPDATE_DELETE_RR_BAD_TYPE update client %1 for zone %2: update deletion RR bad type: %3
 % LIBDDNS_UPDATE_DELETE_RR_BAD_TYPE update client %1 for zone %2: update deletion RR bad type: %3

+ 34 - 45
src/lib/python/isc/ddns/session.py

@@ -59,40 +59,23 @@ class UpdateError(Exception):
         self.rcode = rcode
         self.rcode = rcode
         self.nolog = nolog
         self.nolog = nolog
 
 
-def foreach_rr_in_rrset(rrset, method, *kwargs):
-    '''Helper function. For DDNS, in a number of cases, we need to
-       treat the various RRs in a single RRset separately.
-       Our libdns++ has no concept of RRs, so in that case,
-       what we do is create a temporary 1-RR RRset for each Rdata
-       in the RRset object.
-       This method then calls the given method with the given args
-       for each of the temporary rrsets (the rrset in *wargs is
-       replaced by the temporary one)
-       Note: if this method is useful in more places, we may want
-       to move it out of ddns.
-       Example:
-       Say you have a method that prints a prexif string and an
-       rrset, def my_print(prefix, rrset)
-       Given an rrset my_rrset, you'd print the entire rrset
-       with my_print("foo", rrset)
-       And with this helper function, to print each rr invidually,
-       you'd call
-       foreach_rr_in_rrsetet(rrset, my_print, "foo", rrset)
-       Note the rrset is needed twice, the first to identify it,
-       the second as the 'real' argument to my_print (which is replaced
-       by this function.
+def foreach_rr(rrset):
+    '''
+    Generator that creates a new RRset with one RR from
+    the given RRset upon each iteration, usable in calls that
+    need to loop over an RRset and perform an action with each
+    of the individual RRs in it.
+    Example:
+    for rr in foreach_rr(rrset):
+        print(str(rr))
     '''
     '''
-    result = None
     for rdata in rrset.get_rdata():
     for rdata in rrset.get_rdata():
-        tmp_rrset = isc.dns.RRset(rrset.get_name(),
-                                  rrset.get_class(),
-                                  rrset.get_type(),
-                                  rrset.get_ttl())
-        tmp_rrset.add_rdata(rdata)
-        # Replace the rrset in the original arguments by our rrset
-        args = [arg if arg != rrset else tmp_rrset for arg in kwargs]
-        result = method(*args)
-    return result
+        rr = isc.dns.RRset(rrset.get_name(),
+                           rrset.get_class(),
+                           rrset.get_type(),
+                           rrset.get_ttl())
+        rr.add_rdata(rdata)
+        yield rr
 
 
 def convert_rrset_class(rrset, rrclass):
 def convert_rrset_class(rrset, rrclass):
     '''Returns a (new) rrset with the data from the given rrset,
     '''Returns a (new) rrset with the data from the given rrset,
@@ -228,10 +211,10 @@ class UpdateSession:
         zclass = zrecord.get_class()
         zclass = zrecord.get_class()
         zone_type, datasrc_client = self.__zone_config.find_zone(zname, zclass)
         zone_type, datasrc_client = self.__zone_config.find_zone(zname, zclass)
         if zone_type == isc.ddns.zone_config.ZONE_PRIMARY:
         if zone_type == isc.ddns.zone_config.ZONE_PRIMARY:
+            _, self.__finder = datasrc_client.find_zone(zname)
             self.__zname = zname
             self.__zname = zname
             self.__zclass = zclass
             self.__zclass = zclass
             self.__datasrc_client = datasrc_client
             self.__datasrc_client = datasrc_client
-            _, self.__finder = datasrc_client.find_zone(zname)
             return
             return
         elif zone_type == isc.ddns.zone_config.ZONE_SECONDARY:
         elif zone_type == isc.ddns.zone_config.ZONE_SECONDARY:
             # We are a secondary server; since we don't yet support update
             # We are a secondary server; since we don't yet support update
@@ -468,7 +451,8 @@ class UpdateSession:
                 if rrset.get_type() == RRType.SOA():
                 if rrset.get_type() == RRType.SOA():
                     # In case there's multiple soa records in the update
                     # In case there's multiple soa records in the update
                     # somehow, just take the last
                     # somehow, just take the last
-                    foreach_rr_in_rrset(rrset, self.__set_soa_rrset, rrset)
+                    for rr in foreach_rr(rrset):
+                        self.__set_soa_rrset(rr)
             elif rrset.get_class() == RRClass.ANY():
             elif rrset.get_class() == RRClass.ANY():
                 if rrset.get_ttl().get_value() != 0:
                 if rrset.get_ttl().get_value() != 0:
                     logger.info(LIBDDNS_UPDATE_DELETE_NONZERO_TTL,
                     logger.info(LIBDDNS_UPDATE_DELETE_NONZERO_TTL,
@@ -538,7 +522,7 @@ class UpdateSession:
         '''
         '''
         # For a number of cases, we may need to remove data in the zone
         # For a number of cases, we may need to remove data in the zone
         # (note; SOA is handled separately by __do_update, so that one
         # (note; SOA is handled separately by __do_update, so that one
-        # is not explicitely ignored here)
+        # is explicitely ignored here)
         if rrset.get_type() == RRType.SOA():
         if rrset.get_type() == RRType.SOA():
             return
             return
         result, orig_rrset, _ = self.__finder.find(rrset.get_name(),
         result, orig_rrset, _ = self.__finder.find(rrset.get_name(),
@@ -565,7 +549,8 @@ class UpdateSession:
             # If this type is CNAME, ignore the update
             # If this type is CNAME, ignore the update
             if rrset.get_type() == RRType.CNAME():
             if rrset.get_type() == RRType.CNAME():
                 return
                 return
-        foreach_rr_in_rrset(rrset, self.__do_update_add_single_rr, diff, rrset, orig_rrset)
+        for rr in foreach_rr(rrset):
+            self.__do_update_add_single_rr(diff, rr, orig_rrset)
 
 
     def __do_update_delete_rrset(self, diff, rrset):
     def __do_update_delete_rrset(self, diff, rrset):
         '''Deletes the rrset with the name and type of the given
         '''Deletes the rrset with the name and type of the given
@@ -578,12 +563,14 @@ class UpdateSession:
                                                   rrset.get_type(),
                                                   rrset.get_type(),
                                                   ZoneFinder.NO_WILDCARD |
                                                   ZoneFinder.NO_WILDCARD |
                                                   ZoneFinder.FIND_GLUE_OK)
                                                   ZoneFinder.FIND_GLUE_OK)
-        if to_delete.get_name() == self.__zname and\
-           (to_delete.get_type() == RRType.SOA() or\
-            to_delete.get_type() == RRType.NS()):
-            # ignore
-            return
-        foreach_rr_in_rrset(to_delete, diff.delete_data, to_delete)
+        if result == ZoneFinder.SUCCESS:
+            if to_delete.get_name() == self.__zname and\
+               (to_delete.get_type() == RRType.SOA() or\
+                to_delete.get_type() == RRType.NS()):
+                # ignore
+                return
+            for rr in foreach_rr(to_delete):
+                diff.delete_data(rr)
 
 
     def __ns_deleter_helper(self, diff, rrset):
     def __ns_deleter_helper(self, diff, rrset):
         '''Special case helper for deleting NS resource records
         '''Special case helper for deleting NS resource records
@@ -636,7 +623,8 @@ class UpdateSession:
                     to_delete.get_type() == RRType.NS()):
                     to_delete.get_type() == RRType.NS()):
                     continue
                     continue
                 else:
                 else:
-                    foreach_rr_in_rrset(to_delete, diff.delete_data, to_delete)
+                    for rr in foreach_rr(to_delete):
+                        diff.delete_data(rr)
 
 
     def __do_update_delete_rrs_from_rrset(self, diff, rrset):
     def __do_update_delete_rrs_from_rrset(self, diff, rrset):
         '''Deletes all resource records in the given rrset from the
         '''Deletes all resource records in the given rrset from the
@@ -661,7 +649,8 @@ class UpdateSession:
                 # delegate to helper method
                 # delegate to helper method
                 self.__ns_deleter_helper(diff, to_delete)
                 self.__ns_deleter_helper(diff, to_delete)
                 return
                 return
-        foreach_rr_in_rrset(to_delete, diff.delete_data, to_delete)
+        for rr in foreach_rr(to_delete):
+            diff.delete_data(rr)
 
 
     def __update_soa(self, diff):
     def __update_soa(self, diff):
         '''Checks the member value __added_soa, and depending on
         '''Checks the member value __added_soa, and depending on
@@ -716,7 +705,7 @@ class UpdateSession:
             # of the Diff class, this is not the case, and therefore it
             # of the Diff class, this is not the case, and therefore it
             # is easier to work with full rrsets for the most parts
             # is easier to work with full rrsets for the most parts
             # (less lookups needed; conversion to individual rrs is
             # (less lookups needed; conversion to individual rrs is
-            # the same offort whether it is done here or in the several
+            # the same effort whether it is done here or in the several
             # do_update statements)
             # do_update statements)
             for rrset in self.__message.get_section(SECTION_UPDATE):
             for rrset in self.__message.get_section(SECTION_UPDATE):
                 if rrset.get_class() == self.__zclass:
                 if rrset.get_class() == self.__zclass:

+ 207 - 142
src/lib/python/isc/ddns/tests/session_tests.py

@@ -58,6 +58,16 @@ def create_update_msg(zones=[TEST_ZONE_RECORD], prerequisites=[],
 
 
     return renderer.get_data(), msg
     return renderer.get_data(), msg
 
 
+def add_rdata(rrset, rdata):
+    '''
+    Helper function for easily adding Rdata fields to RRsets.
+    This function assumes the given rdata is of type string or bytes,
+    and corresponds to the given rrset
+    '''
+    rrset.add_rdata(isc.dns.Rdata(rrset.get_type(),
+                                  rrset.get_class(),
+                                  rdata))
+
 def create_rrset(name, rrclass, rrtype, ttl, rdatas = []):
 def create_rrset(name, rrclass, rrtype, ttl, rdatas = []):
     '''
     '''
     Helper method to easily create RRsets, auto-converts
     Helper method to easily create RRsets, auto-converts
@@ -79,16 +89,6 @@ def create_rrset(name, rrclass, rrtype, ttl, rdatas = []):
         add_rdata(rrset, rdata)
         add_rdata(rrset, rdata)
     return rrset
     return rrset
 
 
-def add_rdata(rrset, rdata):
-    '''
-    Helper function for easily adding Rdata fields to RRsets.
-    This function assumes the given rdata is of type string or bytes,
-    and corresponds to the given rrset
-    '''
-    rrset.add_rdata(isc.dns.Rdata(rrset.get_type(),
-                                  rrset.get_class(),
-                                  rdata))
-
 class SessionTest(unittest.TestCase):
 class SessionTest(unittest.TestCase):
     '''Session tests'''
     '''Session tests'''
     def setUp(self):
     def setUp(self):
@@ -185,33 +185,22 @@ class SessionTest(unittest.TestCase):
         # zone class doesn't match
         # zone class doesn't match
         self.check_notauth(Name('example.org'), RRClass.CH())
         self.check_notauth(Name('example.org'), RRClass.CH())
 
 
-    def foreach_rr_in_rrset_helper(self, rr, l):
-        l.append(rr.to_text())
-
     def test_foreach_rr_in_rrset(self):
     def test_foreach_rr_in_rrset(self):
         rrset = create_rrset("www.example.org", TEST_RRCLASS,
         rrset = create_rrset("www.example.org", TEST_RRCLASS,
                              RRType.A(), 3600, [ "192.0.2.1" ])
                              RRType.A(), 3600, [ "192.0.2.1" ])
 
 
         l = []
         l = []
-        foreach_rr_in_rrset(rrset, self.foreach_rr_in_rrset_helper, rrset, l)
+        for rr in foreach_rr(rrset):
+            l.append(str(rr))
         self.assertEqual(["www.example.org. 3600 IN A 192.0.2.1\n"], l)
         self.assertEqual(["www.example.org. 3600 IN A 192.0.2.1\n"], l)
 
 
         add_rdata(rrset, "192.0.2.2")
         add_rdata(rrset, "192.0.2.2")
         add_rdata(rrset, "192.0.2.3")
         add_rdata(rrset, "192.0.2.3")
 
 
-        # if the helper is called directly, the list should have
-        # one entry, with a multiline string
-        # but through the helper, there should be several 1-line entries
-        l = []
-        self.foreach_rr_in_rrset_helper(rrset, l)
-        self.assertEqual(["www.example.org. 3600 IN A 192.0.2.1\n" +
-                          "www.example.org. 3600 IN A 192.0.2.2\n" +
-                          "www.example.org. 3600 IN A 192.0.2.3\n"
-                         ], l)
-
-        # but through the helper, there should be several 1-line entries
+        # but through the generator, there should be several 1-line entries
         l = []
         l = []
-        foreach_rr_in_rrset(rrset, self.foreach_rr_in_rrset_helper, rrset, l)
+        for rr in foreach_rr(rrset):
+            l.append(str(rr))
         self.assertEqual(["www.example.org. 3600 IN A 192.0.2.1\n",
         self.assertEqual(["www.example.org. 3600 IN A 192.0.2.1\n",
                           "www.example.org. 3600 IN A 192.0.2.2\n",
                           "www.example.org. 3600 IN A 192.0.2.2\n",
                           "www.example.org. 3600 IN A 192.0.2.3\n",
                           "www.example.org. 3600 IN A 192.0.2.3\n",
@@ -609,7 +598,7 @@ class SessionTest(unittest.TestCase):
            Function does not do much but makes the code look nicer'''
            Function does not do much but makes the code look nicer'''
         self.assertEqual(expected, method(rrset))
         self.assertEqual(expected, method(rrset))
 
 
-    def initialize_update_rrsets(self):
+    def __initialize_update_rrsets(self):
         '''Prepare a number of RRsets to be used in several update tests
         '''Prepare a number of RRsets to be used in several update tests
            The rrsets are stored in self'''
            The rrsets are stored in self'''
         orig_a_rrset = create_rrset("www.example.org", TEST_RRCLASS,
         orig_a_rrset = create_rrset("www.example.org", TEST_RRCLASS,
@@ -690,7 +679,7 @@ class SessionTest(unittest.TestCase):
         '''Test whether the prescan succeeds on data that is ok, and whether
         '''Test whether the prescan succeeds on data that is ok, and whether
            if notices the SOA if present'''
            if notices the SOA if present'''
         # prepare a set of correct update statements
         # prepare a set of correct update statements
-        self.initialize_update_rrsets()
+        self.__initialize_update_rrsets()
 
 
         self.check_prescan_result(Rcode.NOERROR(), [ self.rrset_update_a ])
         self.check_prescan_result(Rcode.NOERROR(), [ self.rrset_update_a ])
 
 
@@ -785,8 +774,8 @@ class SessionTest(unittest.TestCase):
                              [ b'\x00' ])
                              [ b'\x00' ])
         self.check_prescan_result(Rcode.FORMERR(), [ rrset ])
         self.check_prescan_result(Rcode.FORMERR(), [ rrset ])
 
 
-    def check_inzone_data(self, expected_result, name, rrtype,
-                          expected_rrset = None):
+    def __check_inzone_data(self, expected_result, name, rrtype,
+                            expected_rrset = None):
         '''Does a find on TEST_ZONE for the given rrset's name and type,
         '''Does a find on TEST_ZONE for the given rrset's name and type,
            then checks if the result matches the expected result.
            then checks if the result matches the expected result.
            If so, and if expected_rrset is given, they are compared as
            If so, and if expected_rrset is given, they are compared as
@@ -817,7 +806,7 @@ class SessionTest(unittest.TestCase):
         Tests a sequence of related add and delete updates. Some other
         Tests a sequence of related add and delete updates. Some other
         cases are tested by later tests.
         cases are tested by later tests.
         '''
         '''
-        self.initialize_update_rrsets()
+        self.__initialize_update_rrsets()
 
 
         # initially, the www should only contain one rr
         # initially, the www should only contain one rr
         # (set to self.orig_a_rrset)
         # (set to self.orig_a_rrset)
@@ -830,115 +819,191 @@ class SessionTest(unittest.TestCase):
                                           "192.0.2.3" ])
                                           "192.0.2.3" ])
 
 
         # Sanity check, make sure original data is really there before updates
         # Sanity check, make sure original data is really there before updates
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("www.example.org"),
-                               RRType.A(),
-                               self.orig_a_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A(),
+                                 self.orig_a_rrset)
 
 
         # Add two rrs
         # Add two rrs
         self.check_full_handle_result(Rcode.NOERROR(), [ self.rrset_update_a ])
         self.check_full_handle_result(Rcode.NOERROR(), [ self.rrset_update_a ])
 
 
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("www.example.org"),
-                               RRType.A(),
-                               extended_a_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A(),
+                                 extended_a_rrset)
 
 
         # Adding the same RRsets should not make a difference.
         # Adding the same RRsets should not make a difference.
         self.check_full_handle_result(Rcode.NOERROR(), [ self.rrset_update_a ])
         self.check_full_handle_result(Rcode.NOERROR(), [ self.rrset_update_a ])
 
 
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("www.example.org"),
-                               RRType.A(),
-                               extended_a_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A(),
+                                 extended_a_rrset)
 
 
         # Now delete those two, and we should end up with the original RRset
         # Now delete those two, and we should end up with the original RRset
         self.check_full_handle_result(Rcode.NOERROR(),
         self.check_full_handle_result(Rcode.NOERROR(),
                                       [ self.rrset_update_del_rrset_part ])
                                       [ self.rrset_update_del_rrset_part ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("www.example.org"),
-                               RRType.A(),
-                               self.orig_a_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A(),
+                                 self.orig_a_rrset)
 
 
         # 'Deleting' them again should make no difference
         # 'Deleting' them again should make no difference
         self.check_full_handle_result(Rcode.NOERROR(),
         self.check_full_handle_result(Rcode.NOERROR(),
                                       [ self.rrset_update_del_rrset_part ])
                                       [ self.rrset_update_del_rrset_part ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("www.example.org"),
-                               RRType.A(),
-                               self.orig_a_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A(),
+                                 self.orig_a_rrset)
 
 
         # But deleting the entire rrset, independent of its contents, should
         # But deleting the entire rrset, independent of its contents, should
         # work
         # work
         self.check_full_handle_result(Rcode.NOERROR(),
         self.check_full_handle_result(Rcode.NOERROR(),
                                       [ self.rrset_update_del_rrset ])
                                       [ self.rrset_update_del_rrset ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
-                               isc.dns.Name("www.example.org"),
-                               RRType.A())
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
 
 
         # Check that if we update the SOA, it is updated to our value
         # Check that if we update the SOA, it is updated to our value
         self.check_full_handle_result(Rcode.NOERROR(),
         self.check_full_handle_result(Rcode.NOERROR(),
                                       [ self.rrset_update_soa2 ])
                                       [ self.rrset_update_soa2 ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("example.org"),
-                               RRType.SOA(),
-                               self.rrset_update_soa2)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("example.org"),
+                                 RRType.SOA(),
+                                 self.rrset_update_soa2)
+
+    def test_glue_deletions(self):
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("sub.example.org."),
+                                 RRType.NS())
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("ns.sub.example.org."),
+                                 RRType.A())
+
+        # See that we can delete glue
+        rrset_delete_glue = create_rrset("ns.sub.example.org.",
+                                         RRClass.ANY(),
+                                         RRType.A(),
+                                         0)
+        self.check_full_handle_result(Rcode.NOERROR(),
+                                      [ rrset_delete_glue ])
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("sub.example.org."),
+                                 RRType.NS())
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
+                                 isc.dns.Name("ns.sub.example.org."),
+                                 RRType.A())
+
+        # Check that we don't accidentally delete a delegation if we
+        # try to delete non-existent glue
+        rrset_delete_nonexistent_glue = create_rrset("foo.sub.example.org.",
+                                                     RRClass.ANY(),
+                                                     RRType.A(),
+                                                     0)
+        self.check_full_handle_result(Rcode.NOERROR(),
+                                      [ rrset_delete_nonexistent_glue ])
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("sub.example.org."),
+                                 RRType.NS())
 
 
     def test_update_add_new_data(self):
     def test_update_add_new_data(self):
         '''
         '''
         This tests adds data where none is present
         This tests adds data where none is present
         '''
         '''
         # Add data at a completely new name
         # Add data at a completely new name
-        self.check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
-                               isc.dns.Name("new.example.org"),
-                               RRType.A())
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
+                                 isc.dns.Name("new.example.org"),
+                                 RRType.A())
         rrset = create_rrset("new.example.org", TEST_RRCLASS, RRType.A(),
         rrset = create_rrset("new.example.org", TEST_RRCLASS, RRType.A(),
                              3600, [ "192.0.2.1", "192.0.2.2" ])
                              3600, [ "192.0.2.1", "192.0.2.2" ])
         self.check_full_handle_result(Rcode.NOERROR(), [ rrset ])
         self.check_full_handle_result(Rcode.NOERROR(), [ rrset ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("new.example.org"),
-                               RRType.A(),
-                               rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("new.example.org"),
+                                 RRType.A(),
+                                 rrset)
 
 
         # Also try a name where data is present, but none of this
         # Also try a name where data is present, but none of this
         # specific type
         # specific type
-        self.check_inzone_data(isc.datasrc.ZoneFinder.NXRRSET,
-                               isc.dns.Name("new.example.org"),
-                               RRType.TXT())
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXRRSET,
+                                 isc.dns.Name("new.example.org"),
+                                 RRType.TXT())
         rrset = create_rrset("new.example.org", TEST_RRCLASS, RRType.TXT(),
         rrset = create_rrset("new.example.org", TEST_RRCLASS, RRType.TXT(),
                              3600, [ "foo" ])
                              3600, [ "foo" ])
         self.check_full_handle_result(Rcode.NOERROR(), [ rrset ])
         self.check_full_handle_result(Rcode.NOERROR(), [ rrset ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("new.example.org"),
-                               RRType.TXT(),
-                               rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("new.example.org"),
+                                 RRType.TXT(),
+                                 rrset)
+
+    def test_update_add_new_data_interspersed(self):
+        '''
+        This tests adds data where none is present, similar to
+        test_update_add_new_data, but this time the second RRset
+        is put into the record between the two RRs of the first
+        RRset.
+        '''
+        # Add data at a completely new name
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
+                                 isc.dns.Name("new_a.example.org"),
+                                 RRType.A())
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
+                                 isc.dns.Name("new_txt.example.org"),
+                                 RRType.TXT())
+
+        rrset1 = create_rrset("new_a.example.org", TEST_RRCLASS, RRType.A(),
+                              3600, [ "192.0.2.1" ])
+
+        rrset2 = create_rrset("new_txt.example.org", TEST_RRCLASS, RRType.TXT(),
+                              3600, [ "foo" ])
+
+        rrset3 = create_rrset("new_a.example.org", TEST_RRCLASS, RRType.A(),
+                              3600, [ "192.0.2.2" ])
+
+        self.check_full_handle_result(Rcode.NOERROR(),
+                                      [ rrset1, rrset2, rrset3 ])
+
+        # The update should have merged rrset1 and rrset3
+        rrset_merged = create_rrset("new_a.example.org", TEST_RRCLASS,
+                                    RRType.A(), 3600,
+                                    [ "192.0.2.1", "192.0.2.2" ])
+
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("new_a.example.org"),
+                                 RRType.A(),
+                                 rrset_merged)
+
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("new_txt.example.org"),
+                                 RRType.TXT(),
+                                 rrset2)
 
 
     def test_update_delete_name(self):
     def test_update_delete_name(self):
-        self.initialize_update_rrsets()
+        self.__initialize_update_rrsets()
 
 
         # First check it is there
         # First check it is there
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("www.example.org"),
-                               RRType.A())
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
 
 
         # Delete the entire name
         # Delete the entire name
         self.check_full_handle_result(Rcode.NOERROR(),
         self.check_full_handle_result(Rcode.NOERROR(),
                                       [ self.rrset_update_del_name ])
                                       [ self.rrset_update_del_name ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
-                               isc.dns.Name("www.example.org"),
-                               RRType.A())
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
 
 
         # Should still be gone after pointless second delete
         # Should still be gone after pointless second delete
         self.check_full_handle_result(Rcode.NOERROR(),
         self.check_full_handle_result(Rcode.NOERROR(),
                                       [ self.rrset_update_del_name ])
                                       [ self.rrset_update_del_name ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
-                               isc.dns.Name("www.example.org"),
-                               RRType.A())
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
 
 
     def test_update_apex_special_cases(self):
     def test_update_apex_special_cases(self):
         '''
         '''
         Tests a few special cases when deleting data from the apex
         Tests a few special cases when deleting data from the apex
         '''
         '''
-        self.initialize_update_rrsets()
+        self.__initialize_update_rrsets()
 
 
         # the original SOA
         # the original SOA
         orig_soa_rrset = create_rrset("example.org", TEST_RRCLASS,
         orig_soa_rrset = create_rrset("example.org", TEST_RRCLASS,
@@ -960,60 +1025,60 @@ class SessionTest(unittest.TestCase):
                                       [ "ns3.example.org." ])
                                       [ "ns3.example.org." ])
 
 
         # Sanity check, make sure original data is really there before updates
         # Sanity check, make sure original data is really there before updates
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("example.org"),
-                               RRType.NS(),
-                               orig_ns_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("example.org"),
+                                 RRType.NS(),
+                                 orig_ns_rrset)
         # We will delete the MX record later in this test, so let's make
         # We will delete the MX record later in this test, so let's make
         # sure that it exists (we do not care about its value)
         # sure that it exists (we do not care about its value)
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("example.org"),
-                               RRType.MX())
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 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 direction deletion
         # both by name+type and by full rrset
         # both by name+type and by full rrset
         self.check_full_handle_result(Rcode.NOERROR(),
         self.check_full_handle_result(Rcode.NOERROR(),
                                       [ self.rrset_update_del_soa_apex,
                                       [ self.rrset_update_del_soa_apex,
                                         self.rrset_update_soa_del ])
                                         self.rrset_update_soa_del ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("example.org"),
-                               RRType.SOA(),
-                               orig_soa_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("example.org"),
+                                 RRType.SOA(),
+                                 orig_soa_rrset)
 
 
         # If we delete everything at the apex, the SOA and NS rrsets should be
         # If we delete everything at the apex, the SOA and NS rrsets should be
         # untouched
         # untouched
         self.check_full_handle_result(Rcode.NOERROR(),
         self.check_full_handle_result(Rcode.NOERROR(),
                                       [ self.rrset_update_del_name_apex ])
                                       [ self.rrset_update_del_name_apex ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("example.org"),
-                               RRType.SOA(),
-                               orig_soa_rrset)
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("example.org"),
-                               RRType.NS(),
-                               orig_ns_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("example.org"),
+                                 RRType.SOA(),
+                                 orig_soa_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("example.org"),
+                                 RRType.NS(),
+                                 orig_ns_rrset)
         # but the MX should be gone
         # but the MX should be gone
-        self.check_inzone_data(isc.datasrc.ZoneFinder.NXRRSET,
-                               isc.dns.Name("example.org"),
-                               RRType.MX())
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXRRSET,
+                                 isc.dns.Name("example.org"),
+                                 RRType.MX())
 
 
         # Deleting the NS rrset by name and type only, it should also be left
         # Deleting the NS rrset by name and type only, it should also be left
         # untouched
         # untouched
         self.check_full_handle_result(Rcode.NOERROR(),
         self.check_full_handle_result(Rcode.NOERROR(),
                                       [ self.rrset_update_del_ns_apex ])
                                       [ self.rrset_update_del_ns_apex ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("example.org"),
-                               RRType.NS(),
-                               orig_ns_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("example.org"),
+                                 RRType.NS(),
+                                 orig_ns_rrset)
 
 
         # If we delete the NS at the apex specifically, it should still
         # If we delete the NS at the apex specifically, it should still
         # keep one record
         # keep one record
         self.check_full_handle_result(Rcode.NOERROR(),
         self.check_full_handle_result(Rcode.NOERROR(),
                                       [ self.rrset_update_del_rrset_ns ])
                                       [ self.rrset_update_del_rrset_ns ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("example.org"),
-                               RRType.NS(),
-                               short_ns_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("example.org"),
+                                 RRType.NS(),
+                                 short_ns_rrset)
 
 
     def test_update_delete_normal_rrset_at_apex(self):
     def test_update_delete_normal_rrset_at_apex(self):
         '''
         '''
@@ -1021,64 +1086,64 @@ class SessionTest(unittest.TestCase):
         '''
         '''
 
 
         # MX should simply be deleted
         # MX should simply be deleted
-        self.initialize_update_rrsets()
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("example.org"),
-                               RRType.MX())
+        self.__initialize_update_rrsets()
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("example.org"),
+                                 RRType.MX())
         self.check_full_handle_result(Rcode.NOERROR(),
         self.check_full_handle_result(Rcode.NOERROR(),
                                       [ self.rrset_update_del_rrset_mx ])
                                       [ self.rrset_update_del_rrset_mx ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.NXRRSET,
-                               isc.dns.Name("example.org"),
-                               RRType.MX())
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXRRSET,
+                                 isc.dns.Name("example.org"),
+                                 RRType.MX())
 
 
     def test_update_cname_special_cases(self):
     def test_update_cname_special_cases(self):
-        self.initialize_update_rrsets()
+        self.__initialize_update_rrsets()
 
 
         # Sanity check
         # Sanity check
         orig_cname_rrset = create_rrset("cname.example.org", TEST_RRCLASS,
         orig_cname_rrset = create_rrset("cname.example.org", TEST_RRCLASS,
                                         RRType.CNAME(), 3600,
                                         RRType.CNAME(), 3600,
                                         [ "www.example.org." ])
                                         [ "www.example.org." ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.CNAME,
-                               isc.dns.Name("cname.example.org"),
-                               RRType.A(),
-                               orig_cname_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.CNAME,
+                                 isc.dns.Name("cname.example.org"),
+                                 RRType.A(),
+                                 orig_cname_rrset)
 
 
         # If we try to add data where a cname is preset
         # If we try to add data where a cname is preset
         rrset = create_rrset("cname.example.org", TEST_RRCLASS, RRType.A(),
         rrset = create_rrset("cname.example.org", TEST_RRCLASS, RRType.A(),
                              3600, [ "192.0.2.1" ])
                              3600, [ "192.0.2.1" ])
 
 
         self.check_full_handle_result(Rcode.NOERROR(), [ rrset ])
         self.check_full_handle_result(Rcode.NOERROR(), [ rrset ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.CNAME,
-                               isc.dns.Name("cname.example.org"),
-                               RRType.A(),
-                               orig_cname_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.CNAME,
+                                 isc.dns.Name("cname.example.org"),
+                                 RRType.A(),
+                                 orig_cname_rrset)
 
 
         # But updating the cname itself should work
         # But updating the cname itself should work
         new_cname_rrset = create_rrset("cname.example.org", TEST_RRCLASS,
         new_cname_rrset = create_rrset("cname.example.org", TEST_RRCLASS,
                                        RRType.CNAME(), 3600,
                                        RRType.CNAME(), 3600,
                                        [ "mail.example.org." ])
                                        [ "mail.example.org." ])
         self.check_full_handle_result(Rcode.NOERROR(), [ new_cname_rrset ])
         self.check_full_handle_result(Rcode.NOERROR(), [ new_cname_rrset ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.CNAME,
-                               isc.dns.Name("cname.example.org"),
-                               RRType.A(),
-                               new_cname_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.CNAME,
+                                 isc.dns.Name("cname.example.org"),
+                                 RRType.A(),
+                                 new_cname_rrset)
 
 
-        self.initialize_update_rrsets()
+        self.__initialize_update_rrsets()
 
 
         # Likewise, adding a cname where other data is
         # Likewise, adding a cname where other data is
         # present should do nothing either
         # present should do nothing either
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("www.example.org"),
-                               RRType.A(),
-                               self.orig_a_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A(),
+                                 self.orig_a_rrset)
         new_cname_rrset = create_rrset("www.example.org", TEST_RRCLASS,
         new_cname_rrset = create_rrset("www.example.org", TEST_RRCLASS,
                                        RRType.CNAME(), 3600,
                                        RRType.CNAME(), 3600,
                                        [ "mail.example.org." ])
                                        [ "mail.example.org." ])
         self.check_full_handle_result(Rcode.NOERROR(), [ new_cname_rrset ])
         self.check_full_handle_result(Rcode.NOERROR(), [ new_cname_rrset ])
-        self.check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
-                               isc.dns.Name("www.example.org"),
-                               RRType.A(),
-                               self.orig_a_rrset)
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A(),
+                                 self.orig_a_rrset)
 
 
     def test_update_bad_class(self):
     def test_update_bad_class(self):
         rrset = create_rrset("example.org.", RRClass.CH(), RRType.TXT(), 0,
         rrset = create_rrset("example.org.", RRClass.CH(), RRType.TXT(), 0,