[master] Merge branch 'trac1376'

src/bin/xfrin/tests/xfrin_test.py

@@ -14,8 +14,10 @@
 # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 import unittest
+import re
 import shutil
 import socket
+import sqlite3
 import sys
 import io
 from isc.testutils.tsigctx_mock import MockTSIGContext
@@ -170,7 +172,8 @@ class MockDataSourceClient():
             return (ZoneFinder.SUCCESS, dup_soa_rrset)
         raise ValueError('Unexpected input to mock finder: bug in test case?')
 
-    def get_updater(self, zone_name, replace):
+    def get_updater(self, zone_name, replace, journaling=False):
+        self._journaling_enabled = journaling
         return self
 
     def add_rrset(self, rrset):
@@ -1132,6 +1135,7 @@ class TestAXFR(TestXfrinConnection):
     def test_do_xfrin(self):
         self.conn.response_generator = self._create_normal_response_data
         self.assertEqual(self.conn.do_xfrin(False), XFRIN_OK)
+        self.assertFalse(self.conn._datasrc_client._journaling_enabled)
 
     def test_do_xfrin_with_tsig(self):
         # use TSIG with a mock context.  we fake all verify results to
@@ -1283,6 +1287,7 @@ class TestIXFRResponse(TestXfrinConnection):
             answers=[soa_rrset, begin_soa_rrset, soa_rrset, soa_rrset])
         self.conn._handle_xfrin_responses()
         self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate()))
+        self.assertTrue(self.conn._datasrc_client._journaling_enabled)
         self.assertEqual([], self.conn._datasrc_client.diffs)
         check_diffs(self.assertEqual,
                     [[('delete', begin_soa_rrset), ('add', soa_rrset)]],
@@ -1387,6 +1392,8 @@ class TestIXFRResponse(TestXfrinConnection):
             answers=[soa_rrset, ns_rr, a_rr, soa_rrset])
         self.conn._handle_xfrin_responses()
         self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        # In the case AXFR-style IXFR, journaling must have been disabled.
+        self.assertFalse(self.conn._datasrc_client._journaling_enabled)
         self.assertEqual([], self.conn._datasrc_client.diffs)
         # The SOA should be added exactly once, and in our implementation
         # it should be added at the end of the sequence.
@@ -1540,6 +1547,19 @@ class TestXFRSessionWithSQLite3(TestXfrinConnection):
         self.assertEqual(XFRIN_OK, self.conn.do_xfrin(False, RRType.IXFR()))
         self.assertEqual(1234, self.get_zone_serial())
 
+        # Also confirm the corresponding diffs are stored in the diffs table
+        conn = sqlite3.connect(self.sqlite3db_obj)
+        cur = conn.cursor()
+        cur.execute('SELECT name, rrtype, ttl, rdata FROM diffs ORDER BY id')
+        soa_rdata_base = 'master.example.com. admin.example.com. ' + \
+            'SERIAL 3600 1800 2419200 7200'
+        self.assertEqual(cur.fetchall(),
+                         [(TEST_ZONE_NAME_STR, 'SOA', 3600,
+                           re.sub('SERIAL', str(1230), soa_rdata_base)),
+                          (TEST_ZONE_NAME_STR, 'SOA', 3600,
+                           re.sub('SERIAL', str(1234), soa_rdata_base))])
+        conn.close()
+
     def test_do_ixfrin_sqlite3_fail(self):
         '''Similar to the previous test, but xfrin fails due to error.
 

src/bin/xfrin/xfrin.py.in

@@ -367,7 +367,10 @@ class XfrinIXFRDeleteSOA(XfrinState):
                                  ' RR is given in IXFRDeleteSOA state')
         # This is the beginning state of one difference sequence (changes
         # for one SOA update).  We need to create a new Diff object now.
-        conn._diff = Diff(conn._datasrc_client, conn._zone_name)
+        # Note also that we (unconditionally) enable journaling here.  The
+        # Diff constructor may internally disable it, however, if the
+        # underlying data source doesn't support journaling.
+        conn._diff = Diff(conn._datasrc_client, conn._zone_name, False, True)
         conn._diff.delete_data(rr)
         self.set_xfrstate(conn, XfrinIXFRDelete())
         return True

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

@@ -59,7 +59,7 @@ class Diff:
     the changes to underlying data source right away, but keeps them for
     a while.
     """
-    def __init__(self, ds_client, zone, replace=False):
+    def __init__(self, ds_client, zone, replace=False, journaling=False):
         """
         Initializes the diff to a ready state. It checks the zone exists
         in the datasource and if not, NoSuchZone is raised. This also creates
@@ -67,13 +67,25 @@ class Diff:
 
         The ds_client is the datasource client containing the zone. Zone is
         isc.dns.Name object representing the name of the zone (its apex).
-        If replace is true, the content of the whole zone is wiped out before
+        If replace is True, the content of the whole zone is wiped out before
         applying the diff.
 
+        If journaling is True, the history of subsequent updates will be
+        recorded as well as the updates themselves as long as the underlying
+        data source support the journaling.  If the data source allows
+        incoming updates but does not support journaling, the Diff object
+        will still continue applying the diffs with disabling journaling.
+
         You can also expect isc.datasrc.Error or isc.datasrc.NotImplemented
         exceptions.
         """
-        self.__updater = ds_client.get_updater(zone, replace)
+        try:
+            self.__updater = ds_client.get_updater(zone, replace, journaling)
+        except isc.datasrc.NotImplemented as ex:
+            if not journaling:
+                raise ex
+            self.__updater = ds_client.get_updater(zone, replace, False)
+            logger.info(LIBXFRIN_NO_JOURNAL, zone, ds_client)
         if self.__updater is None:
             # The no such zone case
             raise NoSuchZone("Zone " + str(zone) +

src/lib/python/isc/xfrin/libxfrin_messages.mes

@@ -19,3 +19,13 @@
 The xfrin module received an update containing multiple rdata changes for the
 same RRset. But the TTLs of these don't match each other. As we combine them
 together, the later one get's overwritten to the earlier one in the sequence.
+
+% LIBXFRIN_NO_JOURNAL disabled journaling for updates to %1 on %2
+An attempt was made to create a Diff object with journaling enabled, but
+the underlying data source didn't support journaling (while still allowing
+updates) and so the created object has it disabled.  At a higher level this
+means that the updates will be applied to the zone but subsequent IXFR requests
+will result in a full zone transfer (i.e., an AXFR-style IXFR).  Unless the
+overhead of the full transfer is an issue this message can be ignored;
+otherwise you may want to check why the journaling wasn't allowed on the
+data source and either fix the issue or use a different type of data source.

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

@@ -15,6 +15,7 @@
 
 import isc.log
 import unittest
+import isc.datasrc
 from isc.dns import Name, RRset, RRClass, RRType, RRTTL, Rdata
 from isc.xfrin.diff import Diff, NoSuchZone
 
@@ -127,7 +128,7 @@ class DiffTest(unittest.TestCase):
         """
         return self.__rrclass
 
-    def get_updater(self, zone_name, replace):
+    def get_updater(self, zone_name, replace, journaling=False):
         """
         This one pretends this is the data source client and serves
         getting an updater.
@@ -138,11 +139,20 @@ class DiffTest(unittest.TestCase):
         # The diff should not delete the old data.
         self.assertEqual(self.__should_replace, replace)
         self.__updater_requested = True
-        # Pretend this zone doesn't exist
         if zone_name == Name('none.example.org.'):
+            # Pretend this zone doesn't exist
             return None
+
+        # If journaling is enabled, record the fact; for a special zone
+        # pretend that we don't support journaling.
+        if journaling:
+            if zone_name == Name('nodiff.example.org'):
+                raise isc.datasrc.NotImplemented('journaling not supported')
+            self.__journaling_enabled = True
         else:
-            return self
+            self.__journaling_enabled = False
+
+        return self
 
     def test_create(self):
         """
@@ -152,6 +162,8 @@ class DiffTest(unittest.TestCase):
         diff = Diff(self, Name('example.org.'))
         self.assertTrue(self.__updater_requested)
         self.assertEqual([], diff.get_buffer())
+        # By default journaling is disabled
+        self.assertFalse(self.__journaling_enabled)
 
     def test_create_nonexist(self):
         """
@@ -161,6 +173,14 @@ class DiffTest(unittest.TestCase):
         self.assertRaises(NoSuchZone, Diff, self, Name('none.example.org.'))
         self.assertTrue(self.__updater_requested)
 
+    def test_create_withjournal(self):
+        Diff(self, Name('example.org'), False, True)
+        self.assertTrue(self.__journaling_enabled)
+
+    def test_create_nojournal(self):
+        Diff(self, Name('nodiff.example.org'), False, True)
+        self.assertFalse(self.__journaling_enabled)
+
     def __data_common(self, diff, method, operation):
         """
         Common part of test for test_add and test_delte.