Merge branch 'master' of ssh://git.bind10.isc.org/var/bind10/git/bind10

ChangeLog

@@ -1,3 +1,48 @@
+325.	[func]		jinmei
+	Python isc.datasrc: added interfaces for difference management:
+	DataSourceClient.get_updater() now has the 'journaling' parameter
+	to enable storing diffs to the data source, and a new class
+	ZoneJournalReader was introduced to retrieve them, which can be
+	created by the new DataSourceClient.get_journal_reader() method.
+	(Trac #1333, git 3e19362bc1ba7dc67a87768e2b172c48b32417f5,
+	git 39def1d39c9543fc485eceaa5d390062edb97676)
+
+324.	[bug]		jinmei
+	Fixed reference leak in the isc.log Python module.  Most of all
+	BIND 10 Python programs had memory leak (even though the pace of
+	leak may be slow) due to this bug.
+	(Trac #1359, git 164d651a0e4c1059c71f56b52ea87ac72b7f6c77)
+
+323.	[bug]		jinmei
+	b10-xfrout incorrectly skipped adding TSIG RRs to some
+	intermediate responses (when TSIG is to be used for the
+	responses).  While RFC2845 optionally allows to skip intermediate
+	TSIGs (as long as the digest for the skipped part was included
+	in a later TSIG), the underlying TSIG API doesn't support this
+	mode of signing.
+	(Trac #1370, git 76fb414ea5257b639ba58ee336fae9a68998b30d)
+
+322.	[func]		jinmei
+	datasrc: Added C++ API for retrieving difference of two versions
+	of a zone.  A new ZoneJournalReader class was introduced for this
+	purpose, and a corresponding factory method was added to
+	DataSourceClient.
+	(Trac #1332, git c1138d13b2692fa3a4f2ae1454052c866d24e654)
+
+321.	[func]*		jinmei
+	b10-xfrin now installs IXFR differences into the underlying data
+	source (if it supports journaling) so that the stored differences
+	can be used for subsequent IXFR-out transactions.
+	Note: this is a backward incompatibility change for older sqlite3
+	database files.  They need to be upgraded to have a "diffs" table.
+	(Trac #1376, git 1219d81b49e51adece77dc57b5902fa1c6be1407)
+
+320.	[func]*		vorner
+	The --brittle switch was removed from the bind10 executable.
+	It didn't work after change #316 (Trac #213) and the same
+	effect can be accomplished by declaring all components as core.
+	(Trac #1340, git f9224368908dd7ba16875b0d36329cf1161193f0)
+
 319.	[func]		naokikambe
 	b10-stats-httpd was updated. In addition of the access to all
 	statistics items of all modules, the specified item or the items of the
@@ -9,12 +54,12 @@
 	only for the XML documents but also is for the XSD and XSL documents.
 	(Trac #917, git b34bf286c064d44746ec0b79e38a6177d01e6956)
 
-318.    [func]      stephen
+318.    [func]		stephen
 	Add C++ API for accessing zone difference information in database-based
 	data sources.
 	(Trac #1330, git 78770f52c7f1e7268d99e8bfa8c61e889813bb33)
 
-317.    [func]      vorner
+317.    [func]		vorner
 	datasrc: the getUpdater method of DataSourceClient supports an optional
 	'journaling' parameter to indicate the generated updater to store diffs.
 	The database based derived class implements this extension.

src/bin/bind10/bind10.xml

@@ -51,7 +51,6 @@
       <arg><option>-u <replaceable>user</replaceable></option></arg>
       <arg><option>-v</option></arg>
       <arg><option>-w <replaceable>wait_time</replaceable></option></arg>
-      <arg><option>--brittle</option></arg>
       <arg><option>--cmdctl-port</option> <replaceable>port</replaceable></arg>
       <arg><option>--config-file</option> <replaceable>config-filename</replaceable></arg>
       <arg><option>--data-path</option> <replaceable>directory</replaceable></arg>
@@ -92,20 +91,6 @@
 
       <varlistentry>
         <term>
-          <option>--brittle</option>
-        </term>
-        <listitem>
-          <para>
-	    Shutdown if any of the child processes of
-	    <command>bind10</command> exit.  This is intended to
-	    help developers debug the server, and should not be
-	    used in production.
-          </para>
-        </listitem>
-      </varlistentry>
-
-      <varlistentry>
-        <term>
           <option>-c</option> <replaceable>config-filename</replaceable>,
           <option>--config-file</option> <replaceable>config-filename</replaceable>
         </term>

src/bin/bind10/bind10_src.py.in

@@ -219,7 +219,7 @@ class BoB:
     
     def __init__(self, msgq_socket_file=None, data_path=None,
     config_filename=None, nocache=False, verbose=False, setuid=None,
-    username=None, cmdctl_port=None, brittle=False, wait_time=10):
+    username=None, cmdctl_port=None, wait_time=10):
         """
             Initialize the Boss of BIND. This is a singleton (only one can run).
         
@@ -233,19 +233,12 @@ class BoB:
             The cmdctl_port is passed to cmdctl and specify on which port it
             should listen.
 
-            brittle is a debug option that controls whether the Boss shuts down
-            after any process dies.
-
             wait_time controls the amount of time (in seconds) that Boss waits
             for selected processes to initialize before continuing with the
             initialization.  Currently this is only the configuration manager.
         """
         self.cc_session = None
         self.ccs = None
-        self.cfg_start_auth = True
-        self.cfg_start_resolver = False
-        self.cfg_start_dhcp6 = False
-        self.cfg_start_dhcp4 = False
         self.curproc = None
         # XXX: Not used now, waits for reintroduction of restarts.
         self.dead_processes = {}
@@ -264,7 +257,6 @@ class BoB:
         self.data_path = data_path
         self.config_filename = config_filename
         self.cmdctl_port = cmdctl_port
-        self.brittle = brittle
         self.wait_time = wait_time
         self._component_configurator = isc.bind10.component.Configurator(self,
             isc.bind10.special_component.get_specials())
@@ -940,8 +932,6 @@ def parse_args(args=sys.argv[1:], Parser=OptionParser):
     parser.add_option("--pid-file", dest="pid_file", type="string",
                       default=None,
                       help="file to dump the PID of the BIND 10 process")
-    parser.add_option("--brittle", dest="brittle", action="store_true",
-                      help="debugging flag: exit if any component dies")
     parser.add_option("-w", "--wait", dest="wait_time", type="int",
                       default=10, help="Time (in seconds) to wait for config manager to start up")
 
@@ -1046,7 +1036,7 @@ def main():
     # Go bob!
     boss_of_bind = BoB(options.msgq_socket_file, options.data_path,
                        options.config_file, options.nocache, options.verbose,
-                       setuid, username, options.cmdctl_port, options.brittle,
+                       setuid, username, options.cmdctl_port,
                        options.wait_time)
     startup_result = boss_of_bind.startup()
     if startup_result:

src/bin/bind10/tests/bind10_test.py.in

@@ -110,11 +110,6 @@ class TestBoB(unittest.TestCase):
         self.assertEqual(bob.uid, None)
         self.assertEqual(bob.username, None)
         self.assertEqual(bob.nocache, False)
-        self.assertEqual(bob.cfg_start_auth, True)
-        self.assertEqual(bob.cfg_start_resolver, False)
-
-        self.assertEqual(bob.cfg_start_dhcp4, False)
-        self.assertEqual(bob.cfg_start_dhcp6, False)
 
     def test_init_alternate_socket(self):
         bob = BoB("alt_socket_file")
@@ -128,10 +123,6 @@ class TestBoB(unittest.TestCase):
         self.assertEqual(bob.uid, None)
         self.assertEqual(bob.username, None)
         self.assertEqual(bob.nocache, False)
-        self.assertEqual(bob.cfg_start_auth, True)
-        self.assertEqual(bob.cfg_start_resolver, False)
-        self.assertEqual(bob.cfg_start_dhcp4, False)
-        self.assertEqual(bob.cfg_start_dhcp6, False)
 
     def test_command_handler(self):
         class DummySession():
@@ -740,15 +731,6 @@ class TestParseArgs(unittest.TestCase):
         options = parse_args(['--cmdctl-port=1234'], TestOptParser)
         self.assertEqual(1234, options.cmdctl_port)
 
-    def test_brittle(self):
-        """
-        Test we can use the "brittle" flag.
-        """
-        options = parse_args([], TestOptParser)
-        self.assertFalse(options.brittle)
-        options = parse_args(['--brittle'], TestOptParser)
-        self.assertTrue(options.brittle)
-
 class TestPIDFile(unittest.TestCase):
     def setUp(self):
         self.pid_file = '@builddir@' + os.sep + 'bind10.pid'
@@ -796,37 +778,6 @@ class TestPIDFile(unittest.TestCase):
         self.assertRaises(IOError, dump_pid,
                           'nonexistent_dir' + os.sep + 'bind10.pid')
 
-# TODO: Do we want brittle mode? Probably yes. So we need to re-enable to after that.
-@unittest.skip("Brittle mode temporarily broken")
-class TestBrittle(unittest.TestCase):
-    def test_brittle_disabled(self):
-        bob = MockBob()
-        bob.start_all_components()
-        bob.runnable = True
-
-        bob.reap_children()
-        self.assertTrue(bob.runnable)
-
-    def simulated_exit(self):
-        ret_val = self.exit_info
-        self.exit_info = (0, 0)
-        return ret_val
-
-    def test_brittle_enabled(self):
-        bob = MockBob()
-        bob.start_all_components()
-        bob.runnable = True
-
-        bob.brittle = True
-        self.exit_info = (5, 0)
-        bob._get_process_exit_status = self.simulated_exit
-
-        old_stdout = sys.stdout
-        sys.stdout = open("/dev/null", "w")
-        bob.reap_children()
-        sys.stdout = old_stdout
-        self.assertFalse(bob.runnable)
-
 class TestBossComponents(unittest.TestCase):
     """
     Test the boss propagates component configuration properly to the

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/bin/xfrout/tests/xfrout_test.py.in

@@ -470,34 +470,28 @@ class TestXfroutSession(TestXfroutSessionBase):
         msg = self.getmsg()
         msg.make_response()
 
-        # packet number less than TSIG_SIGN_EVERY_NTH
-        packet_neet_not_sign = xfrout.TSIG_SIGN_EVERY_NTH - 1
         self.xfrsess._send_message_with_last_soa(msg, self.sock,
-                                                 self.soa_rrset, 0,
-                                                 packet_neet_not_sign)
+                                                 self.soa_rrset, 0)
         get_msg = self.sock.read_msg()
-        # tsig context is not exist
+        # tsig context does not exist
         self.assertFalse(self.message_has_tsig(get_msg))
 
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_QUESTION), 1)
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_ANSWER), 1)
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_AUTHORITY), 0)
 
-        #answer_rrset_iter = section_iter(get_msg, section.ANSWER())
-        answer = get_msg.get_section(Message.SECTION_ANSWER)[0]#answer_rrset_iter.get_rrset()
+        answer = get_msg.get_section(Message.SECTION_ANSWER)[0]
         self.assertEqual(answer.get_name().to_text(), "example.com.")
         self.assertEqual(answer.get_class(), RRClass("IN"))
         self.assertEqual(answer.get_type().to_text(), "SOA")
         rdata = answer.get_rdata()
         self.assertEqual(rdata[0], self.soa_rrset.get_rdata()[0])
 
-        # msg is the TSIG_SIGN_EVERY_NTH one
-        # sending the message with last soa together
+        # Sending the message with last soa together
         self.xfrsess._send_message_with_last_soa(msg, self.sock,
-                                                 self.soa_rrset, 0,
-                                                 TSIG_SIGN_EVERY_NTH)
+                                                 self.soa_rrset, 0)
         get_msg = self.sock.read_msg()
-        # tsig context is not exist
+        # tsig context does not exist
         self.assertFalse(self.message_has_tsig(get_msg))
 
     def test_send_message_with_last_soa_with_tsig(self):
@@ -507,13 +501,9 @@ class TestXfroutSession(TestXfroutSessionBase):
         msg = self.getmsg()
         msg.make_response()
 
-        # packet number less than TSIG_SIGN_EVERY_NTH
-        packet_neet_not_sign = xfrout.TSIG_SIGN_EVERY_NTH - 1
-        # msg is not the TSIG_SIGN_EVERY_NTH one
-        # sending the message with last soa together
+        # Sending the message with last soa together
         self.xfrsess._send_message_with_last_soa(msg, self.sock,
-                                                 self.soa_rrset, 0,
-                                                 packet_neet_not_sign)
+                                                 self.soa_rrset, 0)
         get_msg = self.sock.read_msg()
         self.assertTrue(self.message_has_tsig(get_msg))
 
@@ -521,14 +511,6 @@ class TestXfroutSession(TestXfroutSessionBase):
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_ANSWER), 1)
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_AUTHORITY), 0)
 
-        # msg is the TSIG_SIGN_EVERY_NTH one
-        # sending the message with last soa together
-        self.xfrsess._send_message_with_last_soa(msg, self.sock,
-                                                 self.soa_rrset, 0,
-                                                 TSIG_SIGN_EVERY_NTH)
-        get_msg = self.sock.read_msg()
-        self.assertTrue(self.message_has_tsig(get_msg))
-
     def test_trigger_send_message_with_last_soa(self):
         rrset_a = RRset(Name("example.com"), RRClass.IN(), RRType.A(), RRTTL(3600))
         rrset_a.add_rdata(Rdata(RRType.A(), RRClass.IN(), "192.0.2.1"))
@@ -540,8 +522,6 @@ class TestXfroutSession(TestXfroutSessionBase):
         # length larger than MAX-len(rrset)
         length_need_split = xfrout.XFROUT_MAX_MESSAGE_SIZE - \
             get_rrset_len(self.soa_rrset) + 1
-        # packet number less than TSIG_SIGN_EVERY_NTH
-        packet_neet_not_sign = xfrout.TSIG_SIGN_EVERY_NTH - 1
 
         # give the function a value that is larger than MAX-len(rrset)
         # this should have triggered the sending of two messages
@@ -549,8 +529,7 @@ class TestXfroutSession(TestXfroutSessionBase):
         # the sending in _with_last_soa)
         self.xfrsess._send_message_with_last_soa(msg, self.sock,
                                                  self.soa_rrset,
-                                                 length_need_split,
-                                                 packet_neet_not_sign)
+                                                 length_need_split)
         get_msg = self.sock.read_msg()
         self.assertFalse(self.message_has_tsig(get_msg))
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_QUESTION), 1)
@@ -570,7 +549,6 @@ class TestXfroutSession(TestXfroutSessionBase):
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_ANSWER), 1)
         self.assertEqual(get_msg.get_rr_count(Message.SECTION_AUTHORITY), 0)
 
-        #answer_rrset_iter = section_iter(get_msg, Message.SECTION_ANSWER)
         answer = get_msg.get_section(Message.SECTION_ANSWER)[0]
         self.assertEqual(answer.get_name().to_text(), "example.com.")
         self.assertEqual(answer.get_class(), RRClass("IN"))
@@ -590,8 +568,6 @@ class TestXfroutSession(TestXfroutSessionBase):
         # length larger than MAX-len(rrset)
         length_need_split = xfrout.XFROUT_MAX_MESSAGE_SIZE - \
             get_rrset_len(self.soa_rrset) + 1
-        # packet number less than TSIG_SIGN_EVERY_NTH
-        packet_neet_not_sign = xfrout.TSIG_SIGN_EVERY_NTH - 1
 
         # give the function a value that is larger than MAX-len(rrset)
         # this should have triggered the sending of two messages
@@ -599,26 +575,10 @@ class TestXfroutSession(TestXfroutSessionBase):
         # the sending in _with_last_soa)
         self.xfrsess._send_message_with_last_soa(msg, self.sock,
                                                  self.soa_rrset,
-                                                 length_need_split,
-                                                 packet_neet_not_sign)
-        get_msg = self.sock.read_msg()
-        # msg is not the TSIG_SIGN_EVERY_NTH one, it shouldn't be tsig signed
-        self.assertFalse(self.message_has_tsig(get_msg))
-        # the last packet should be tsig signed
+                                                 length_need_split)
+        # Both messages should have TSIG RRs
         get_msg = self.sock.read_msg()
         self.assertTrue(self.message_has_tsig(get_msg))
-        # and it should not have sent anything else
-        self.assertEqual(0, len(self.sock.sendqueue))
-
-
-        # msg is the TSIG_SIGN_EVERY_NTH one, it should be tsig signed
-        self.xfrsess._send_message_with_last_soa(msg, self.sock,
-                                                 self.soa_rrset,
-                                                 length_need_split,
-                                                 xfrout.TSIG_SIGN_EVERY_NTH)
-        get_msg = self.sock.read_msg()
-        self.assertTrue(self.message_has_tsig(get_msg))
-        # the last packet should be tsig signed
         get_msg = self.sock.read_msg()
         self.assertTrue(self.message_has_tsig(get_msg))
         # and it should not have sent anything else
@@ -697,29 +657,18 @@ class TestXfroutSession(TestXfroutSessionBase):
         self.xfrsess._tsig_ctx = self.create_mock_tsig_ctx(TSIGError.NOERROR)
         self.xfrsess._reply_xfrout_query(self.getmsg(), self.sock)
 
-        # tsig signed first package
-        reply_msg = self.sock.read_msg()
-        self.assertEqual(reply_msg.get_rr_count(Message.SECTION_ANSWER), 1)
-        self.assertTrue(self.message_has_tsig(reply_msg))
-        # (TSIG_SIGN_EVERY_NTH - 1) packets have no tsig
-        for i in range(0, xfrout.TSIG_SIGN_EVERY_NTH - 1):
-            reply_msg = self.sock.read_msg()
-            self.assertFalse(self.message_has_tsig(reply_msg))
-        # TSIG_SIGN_EVERY_NTH packet has tsig
-        reply_msg = self.sock.read_msg()
-        self.assertTrue(self.message_has_tsig(reply_msg))
-
-        for i in range(0, 100 - TSIG_SIGN_EVERY_NTH):
+        # All messages must have TSIG as we don't support the feature of
+        # skipping intermediate TSIG records (with bulk signing).
+        for i in range(0, 102): # 102 = all 100 RRs from iterator and 2 SOAs
             reply_msg = self.sock.read_msg()
-            self.assertFalse(self.message_has_tsig(reply_msg))
-        # tsig signed last package
-        reply_msg = self.sock.read_msg()
-        self.assertTrue(self.message_has_tsig(reply_msg))
+            # With the hack of get_rrset_len() above, every message must have
+            # exactly one RR in the answer section.
+            self.assertEqual(reply_msg.get_rr_count(Message.SECTION_ANSWER), 1)
+            self.assertTrue(self.message_has_tsig(reply_msg))
 
         # and it should not have sent anything else
         self.assertEqual(0, len(self.sock.sendqueue))
 
-
 class TestXfroutSessionWithSQLite3(TestXfroutSessionBase):
     '''Tests for XFR-out sessions using an SQLite3 DB.
 

src/bin/xfrout/xfrout.py.in

@@ -92,9 +92,6 @@ init_paths()
 SPECFILE_LOCATION = SPECFILE_PATH + "/xfrout.spec"
 AUTH_SPECFILE_LOCATION = AUTH_SPECFILE_PATH + os.sep + "auth.spec"
 VERBOSE_MODE = False
-# tsig sign every N axfr packets.
-TSIG_SIGN_EVERY_NTH = 96
-
 XFROUT_MAX_MESSAGE_SIZE = 65535
 
 # borrowed from xfrin.py @ #1298.  We should eventually unify it.
@@ -316,11 +313,11 @@ class XfroutSession():
             self._server.get_db_file() + '"}'
         self._datasrc_client = self.ClientClass('sqlite3', datasrc_config)
         try:
-            # Note that we disable 'adjust_ttl'.  In xfr-out we need to
+            # Note that we enable 'separate_rrs'.  In xfr-out we need to
             # preserve as many things as possible (even if it's half broken)
             # stored in the zone.
             self._iterator = self._datasrc_client.get_iterator(zone_name,
-                                                               False)
+                                                               True)
         except isc.datasrc.Error:
             # If the current name server does not have authority for the
             # zone, xfrout can't serve for it, return rcode NOTAUTH.
@@ -398,22 +395,15 @@ class XfroutSession():
         msg.set_header_flag(Message.HEADERFLAG_QR)
         return msg
 
-    def _send_message_with_last_soa(self, msg, sock_fd, rrset_soa, message_upper_len,
-                                    count_since_last_tsig_sign):
+    def _send_message_with_last_soa(self, msg, sock_fd, rrset_soa,
+                                    message_upper_len):
         '''Add the SOA record to the end of message. If it can't be
         added, a new message should be created to send out the last soa .
         '''
-        rrset_len = get_rrset_len(rrset_soa)
-
-        if (count_since_last_tsig_sign == TSIG_SIGN_EVERY_NTH and
-            message_upper_len + rrset_len >= XFROUT_MAX_MESSAGE_SIZE):
-            # If tsig context exist, sign the packet with serial number TSIG_SIGN_EVERY_NTH
+        if (message_upper_len + self._tsig_len + get_rrset_len(rrset_soa) >=
+            XFROUT_MAX_MESSAGE_SIZE):
             self._send_message(sock_fd, msg, self._tsig_ctx)
             msg = self._clear_message(msg)
-        elif (count_since_last_tsig_sign != TSIG_SIGN_EVERY_NTH and
-              message_upper_len + rrset_len + self._tsig_len >= XFROUT_MAX_MESSAGE_SIZE):
-            self._send_message(sock_fd, msg)
-            msg = self._clear_message(msg)
 
         # If tsig context exist, sign the last packet
         msg.add_rrset(Message.SECTION_ANSWER, rrset_soa)
@@ -422,7 +412,6 @@ class XfroutSession():
 
     def _reply_xfrout_query(self, msg, sock_fd):
         #TODO, there should be a better way to insert rrset.
-        count_since_last_tsig_sign = TSIG_SIGN_EVERY_NTH
         msg.make_response()
         msg.set_header_flag(Message.HEADERFLAG_AA)
         msg.add_rrset(Message.SECTION_ANSWER, self._soa)
@@ -447,27 +436,17 @@ class XfroutSession():
                 message_upper_len += rrset_len
                 continue
 
-            # If tsig context exist, sign every N packets
-            if count_since_last_tsig_sign == TSIG_SIGN_EVERY_NTH:
-                count_since_last_tsig_sign = 0
-                self._send_message(sock_fd, msg, self._tsig_ctx)
-            else:
-                self._send_message(sock_fd, msg)
+            self._send_message(sock_fd, msg, self._tsig_ctx)
 
-            count_since_last_tsig_sign += 1
             msg = self._clear_message(msg)
             # Add the RRset to the new message
             msg.add_rrset(Message.SECTION_ANSWER, rrset)
 
             # Reserve tsig space for signed packet
-            if count_since_last_tsig_sign == TSIG_SIGN_EVERY_NTH:
-                message_upper_len = rrset_len + self._tsig_len
-            else:
-                message_upper_len = rrset_len
+            message_upper_len = rrset_len + self._tsig_len
 
         self._send_message_with_last_soa(msg, sock_fd, self._soa,
-                                         message_upper_len,
-                                         count_since_last_tsig_sign)
+                                         message_upper_len)
 
 class UnixSockServer(socketserver_mixin.NoPollMixIn,
                      ThreadingUnixStreamServer):

src/lib/datasrc/client.h

@@ -15,6 +15,8 @@
 #ifndef __DATA_SOURCE_CLIENT_H
 #define __DATA_SOURCE_CLIENT_H 1
 
+#include <utility>
+
 #include <boost/noncopyable.hpp>
 #include <boost/shared_ptr.hpp>
 
@@ -215,18 +217,19 @@ public:
     ///
     /// \param name The name of zone apex to be traversed. It doesn't do
     ///     nearest match as findZone.
-    /// \param adjust_ttl If true, the iterator will treat RRs with the same
-    ///                   name and type but different TTL values to be of the
-    ///                   same RRset, and will adjust the TTL to the lowest
-    ///                   value found. If false, it will consider the RR to
-    ///                   belong to a different RRset.
+    /// \param separate_rrs If true, the iterator will return each RR as a
+    ///                     new RRset object. If false, the iterator will
+    ///                     combine consecutive RRs with the name and type
+    ///                     into 1 RRset. The capitalization of the RRset will
+    ///                     be that of the first RR read, and TTLs will be
+    ///                     adjusted to the lowest one found.
     /// \return Pointer to the iterator.
     virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name,
-                                        bool adjust_ttl = true) const {
+                                        bool separate_rrs = false) const {
         // This is here to both document the parameter in doxygen (therefore it
         // needs a name) and avoid unused parameter warning.
         static_cast<void>(name);
-        static_cast<void>(adjust_ttl);
+        static_cast<void>(separate_rrs);
 
         isc_throw(isc::NotImplemented,
                   "Data source doesn't support iteration");
@@ -310,6 +313,55 @@ public:
     virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name& name,
                                       bool replace, bool journaling = false)
         const = 0;
+
+    /// Return a journal reader to retrieve differences of a zone.
+    ///
+    /// A derived version of this method creates a concrete
+    /// \c ZoneJournalReader object specific to the underlying data source
+    /// for the specified name of zone and differences between the versions
+    /// specified by the beginning and ending serials of the corresponding
+    /// SOA RRs.
+    /// The RR class of the zone is the one that the client is expected to
+    /// handle (see the detailed description of this class).
+    ///
+    /// Note that the SOA serials are compared by the semantics of the serial
+    /// number arithmetic.  So, for example, \c begin_serial can be larger than
+    /// \c end_serial as bare unsigned integers.  The underlying data source
+    /// implementation is assumed to keep track of sufficient history to
+    /// identify (if exist) the corresponding difference between the specified
+    /// versions.
+    ///
+    /// This method returns the result as a pair of a result code and
+    /// a pointer to a \c ZoneJournalReader object.  On success, the result
+    /// code is \c SUCCESS and the pointer must be non NULL; otherwise
+    /// the result code is something other than \c SUCCESS and the pinter
+    /// must be NULL.
+    ///
+    /// If the specified zone is not found in the data source, the result
+    /// code is \c NO_SUCH_ZONE.
+    /// Otherwise, if specified range of difference for the zone is not found
+    /// in the data source, the result code is \c NO_SUCH_VERSION.
+    ///
+    /// Handling differences is an optional feature of data source.
+    /// If the underlying data source does not support difference handling,
+    /// this method for that type of data source can throw an exception of
+    /// class \c NotImplemented.
+    ///
+    /// \exception NotImplemented The data source does not support differences.
+    /// \exception DataSourceError Other operational errors at the data source
+    /// level.
+    ///
+    /// \param zone The name of the zone for which the difference should be
+    /// retrieved.
+    /// \param begin_serial The SOA serial of the beginning version of the
+    /// differences.
+    /// \param end_serial The SOA serial of the ending version of the
+    /// differences.
+    ///
+    /// \return A pair of result code and a pointer to \c ZoneJournalReader.
+    virtual std::pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
+    getJournalReader(const isc::dns::Name& zone, uint32_t begin_serial,
+                     uint32_t end_serial) const = 0;
 };
 }
 }

src/lib/datasrc/database.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <string>
+#include <utility>
 #include <vector>
 
 #include <datasrc/database.h>
@@ -707,11 +708,11 @@ public:
     DatabaseIterator(shared_ptr<DatabaseAccessor> accessor,
                      const Name& zone_name,
                      const RRClass& rrclass,
-                     bool adjust_ttl) :
+                     bool separate_rrs) :
         accessor_(accessor),
         class_(rrclass),
         ready_(true),
-        adjust_ttl_(adjust_ttl)
+        separate_rrs_(separate_rrs)
     {
         // Get the zone
         const pair<bool, int> zone(accessor_->getZone(zone_name.toText()));
@@ -769,20 +770,19 @@ public:
         const RRType rtype(rtype_str);
         RRsetPtr rrset(new RRset(name, class_, rtype, RRTTL(ttl)));
         while (data_ready_ && name_ == name_str && rtype_str == rtype_) {
-            if (adjust_ttl_) {
-                if (ttl_ != ttl) {
-                    if (ttl < ttl_) {
-                        ttl_ = ttl;
-                        rrset->setTTL(RRTTL(ttl));
-                    }
-                    LOG_WARN(logger, DATASRC_DATABASE_ITERATE_TTL_MISMATCH).
-                        arg(name_).arg(class_).arg(rtype_).arg(rrset->getTTL());
+            if (ttl_ != ttl) {
+                if (ttl < ttl_) {
+                    ttl_ = ttl;
+                    rrset->setTTL(RRTTL(ttl));
                 }
-            } else if (ttl_ != ttl) {
-                break;
+                LOG_WARN(logger, DATASRC_DATABASE_ITERATE_TTL_MISMATCH).
+                    arg(name_).arg(class_).arg(rtype_).arg(rrset->getTTL());
             }
             rrset->addRdata(rdata::createRdata(rtype, class_, rdata_));
             getData();
+            if (separate_rrs_) {
+                break;
+            }
         }
         LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ITERATE_NEXT).
             arg(rrset->getName()).arg(rrset->getType());
@@ -814,18 +814,18 @@ private:
     string name_, rtype_, rdata_, ttl_;
     // Whether to modify differing TTL values, or treat a different TTL as
     // a different RRset
-    bool adjust_ttl_;
+    bool separate_rrs_;
 };
 
 }
 
 ZoneIteratorPtr
 DatabaseClient::getIterator(const isc::dns::Name& name,
-                            bool adjust_ttl) const
+                            bool separate_rrs) const
 {
     ZoneIteratorPtr iterator = ZoneIteratorPtr(new DatabaseIterator(
                                                    accessor_->clone(), name,
-                                                   rrclass_, adjust_ttl));
+                                                   rrclass_, separate_rrs));
     LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ITERATE).
         arg(name);
 
@@ -1070,5 +1070,105 @@ DatabaseClient::getUpdater(const isc::dns::Name& name, bool replace,
     return (ZoneUpdaterPtr(new DatabaseUpdater(update_accessor, zone.second,
                                                name, rrclass_, journaling)));
 }
+
+//
+// Zone journal reader using some database system as the underlying data
+//  source.
+//
+class DatabaseJournalReader : public ZoneJournalReader {
+private:
+    // A shortcut typedef to keep the code concise.
+    typedef DatabaseAccessor Accessor;
+public:
+    DatabaseJournalReader(shared_ptr<Accessor> accessor, const Name& zone,
+                          int zone_id, const RRClass& rrclass, uint32_t begin,
+                          uint32_t end) :
+        accessor_(accessor), zone_(zone), rrclass_(rrclass),
+        begin_(begin), end_(end), finished_(false)
+    {
+        context_ = accessor_->getDiffs(zone_id, begin, end);
+    }
+    virtual ~DatabaseJournalReader() {}
+    virtual ConstRRsetPtr getNextDiff() {
+        if (finished_) {
+            isc_throw(InvalidOperation,
+                      "Diff read attempt past the end of sequence on "
+                      << accessor_->getDBName());
+        }
+
+        string data[Accessor::COLUMN_COUNT];
+        if (!context_->getNext(data)) {
+            finished_ = true;
+            LOG_DEBUG(logger, DBG_TRACE_BASIC,
+                      DATASRC_DATABASE_JOURNALREADER_END).
+                arg(zone_).arg(rrclass_).arg(accessor_->getDBName()).
+                arg(begin_).arg(end_);
+            return (ConstRRsetPtr());
+        }
+
+        try {
+            RRsetPtr rrset(new RRset(Name(data[Accessor::NAME_COLUMN]),
+                                     rrclass_,
+                                     RRType(data[Accessor::TYPE_COLUMN]),
+                                     RRTTL(data[Accessor::TTL_COLUMN])));
+            rrset->addRdata(rdata::createRdata(rrset->getType(), rrclass_,
+                                               data[Accessor::RDATA_COLUMN]));
+            LOG_DEBUG(logger, DBG_TRACE_DETAILED,
+                      DATASRC_DATABASE_JOURNALREADER_NEXT).
+                arg(rrset->getName()).arg(rrset->getType()).
+                arg(zone_).arg(rrclass_).arg(accessor_->getDBName());
+            return (rrset);
+        } catch (const Exception& ex) {
+            LOG_ERROR(logger, DATASRC_DATABASE_JOURNALREADR_BADDATA).
+                arg(zone_).arg(rrclass_).arg(accessor_->getDBName()).
+                arg(begin_).arg(end_).arg(ex.what());
+            isc_throw(DataSourceError, "Failed to create RRset from diff on "
+                      << accessor_->getDBName());
+        }
+    }
+
+private:
+    shared_ptr<Accessor> accessor_;
+    const Name zone_;
+    const RRClass rrclass_;
+    Accessor::IteratorContextPtr context_;
+    const uint32_t begin_;
+    const uint32_t end_;
+    bool finished_;
+};
+
+// The JournalReader factory
+pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
+DatabaseClient::getJournalReader(const isc::dns::Name& zone,
+                                 uint32_t begin_serial,
+                                 uint32_t end_serial) const
+{
+    shared_ptr<DatabaseAccessor> jnl_accessor(accessor_->clone());
+    const pair<bool, int> zoneinfo(jnl_accessor->getZone(zone.toText()));
+    if (!zoneinfo.first) {
+        return (pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>(
+                    ZoneJournalReader::NO_SUCH_ZONE,
+                    ZoneJournalReaderPtr()));
+    }
+
+    try {
+        const pair<ZoneJournalReader::Result, ZoneJournalReaderPtr> ret(
+            ZoneJournalReader::SUCCESS,
+            ZoneJournalReaderPtr(new DatabaseJournalReader(jnl_accessor,
+                                                           zone,
+                                                           zoneinfo.second,
+                                                           rrclass_,
+                                                           begin_serial,
+                                                           end_serial)));
+        LOG_DEBUG(logger, DBG_TRACE_BASIC,
+                  DATASRC_DATABASE_JOURNALREADER_START).arg(zone).arg(rrclass_).
+            arg(jnl_accessor->getDBName()).arg(begin_serial).arg(end_serial);
+        return (ret);
+    } catch (const NoSuchSerial&) {
+        return (pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>(
+                    ZoneJournalReader::NO_SUCH_VERSION,
+                    ZoneJournalReaderPtr()));
+    }
+}
 }
 }

src/lib/datasrc/database.h

@@ -23,6 +23,8 @@
 #include <dns/rrclass.h>
 #include <dns/rrset.h>
 
+#include <datasrc/data_source.h>
+#include <datasrc/client.h>
 #include <datasrc/client.h>
 
 #include <dns/name.h>
@@ -544,12 +546,10 @@ public:
     /// is not for the SOA RR; it passes TTL for a diff that deletes an RR
     /// while in \c deleteRecordInZone() it's omitted.  This is because
     /// the stored diffs are expected to be retrieved in the form that
-    /// \c getRecordDiffs() is expected to meet.  This means if the caller
+    /// \c getDiffs() is expected to meet.  This means if the caller
     /// wants to use this method with other update operations, it must
     /// ensure the additional information is ready when this method is called.
     ///
-    /// \note \c getRecordDiffs() is not yet implemented.
-    ///
     /// The caller of this method must ensure that the added diffs via
     /// this method in a single transaction form an IXFR-style difference
     /// sequences: Each difference sequence is a sequence of RRs:
@@ -562,7 +562,7 @@ public:
     /// an SOA RR, \c serial must be identical to the serial of that SOA).
     /// The underlying derived class implementation may or may not check
     /// this condition, but if the caller doesn't meet the condition
-    /// a subsequent call to \c getRecordDiffs() will not work as expected.
+    /// a subsequent call to \c getDiffs() will not work as expected.
     ///
     /// Any call to this method must be in a transaction, and, for now,
     /// it must be a transaction triggered by \c startUpdateZone() (that is,
@@ -913,15 +913,16 @@ public:
      * \exception Anything else the underlying DatabaseConnection might
      *     want to throw.
      * \param name The origin of the zone to iterate.
-     * \param adjust_ttl If true, the iterator will treat RRs with the same
-     *                   name and type but different TTL values to be of the
-     *                   same RRset, and will adjust the TTL to the lowest
-     *                   value found. If false, it will consider the RR to
-     *                   belong to a different RRset.
+     * \param separate_rrs If true, the iterator will return each RR as a
+     *                     new RRset object. If false, the iterator will
+     *                     combine consecutive RRs with the name and type
+     *                     into 1 RRset. The capitalization of the RRset will
+     *                     be that of the first RR read, and TTLs will be
+     *                     adjusted to the lowest one found.
      * \return Shared pointer to the iterator (it will never be NULL)
      */
     virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name,
-                                        bool adjust_ttl = true) const;
+                                        bool separate_rrs = false) const;
 
     /// This implementation internally clones the accessor from the one
     /// used in the client and starts a separate transaction using the cloned
@@ -931,6 +932,15 @@ public:
                                       bool replace,
                                       bool journaling = false) const;
 
+
+    /// This implementation internally clones the accessor from the one
+    /// used in the client for retrieving diffs and iterating over them.
+    /// The returned reader object will be able to work separately from
+    /// the original client.
+    virtual std::pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
+    getJournalReader(const isc::dns::Name& zone, uint32_t begin_serial,
+                     uint32_t end_serial) const;
+
 private:
     /// \brief The RR class that this client handles.
     const isc::dns::RRClass rrclass_;

src/lib/datasrc/datasrc_messages.mes

@@ -630,3 +630,31 @@ database module are shown in the log message.
 Debug information.  A set of updates to a zone has been successfully
 committed to the corresponding database backend.  The zone name,
 its class and the database name are printed.
+
+% DATASRC_DATABASE_JOURNALREADER_START %1/%2 on %3 from %4 to %5
+This is a debug message indicating that the program starts reading
+a zone's difference sequences from a database-based data source.  The
+zone's name and class, database name, and the start and end serials
+are shown in the message.
+
+% DATASRC_DATABASE_JOURNALREADER_NEXT %1/%2 in %3/%4 on %5
+This is a debug message indicating that the program retrieves one
+difference in difference sequences of a zone and successfully converts
+it to an RRset.  The zone's name and class, database name, and the
+name and RR type of the retrieved diff are shown in the message.
+
+% DATASRC_DATABASE_JOURNALREADER_END %1/%2 on %3 from %4 to %5
+This is a debug message indicating that the program (successfully)
+reaches the end of sequences of a zone's differences.  The zone's name
+and class, database name, and the start and end serials are shown in
+the message.
+
+% DATASRC_DATABASE_JOURNALREADR_BADDATA failed to convert a diff to RRset in %1/%2 on %3 between %4 and %5: %6
+This is an error message indicating that a zone's diff is broken and
+the data source library failed to convert it to a valid RRset.  The
+most likely cause of this is that someone has manually modified the
+zone's diff in the database and inserted invalid data as a result.
+The zone's name and class, database name, and the start and end
+serials, and an additional detail of the error are shown in the
+message.  The administrator should examine the diff in the database
+to find any invalid data and fix it.

src/lib/datasrc/memory_datasrc.cc

@@ -729,10 +729,14 @@ private:
     Domain::const_iterator dom_iterator_;
     const DomainTree& tree_;
     const DomainNode* node_;
+    // Only used when separate_rrs_ is true
+    RdataIteratorPtr rdata_iterator_;
+    bool separate_rrs_;
     bool ready_;
 public:
-    MemoryIterator(const DomainTree& tree, const Name& origin) :
+    MemoryIterator(const DomainTree& tree, const Name& origin, bool separate_rrs) :
         tree_(tree),
+        separate_rrs_(separate_rrs),
         ready_(true)
     {
         // Find the first node (origin) and preserve the node chain for future
@@ -747,6 +751,9 @@ public:
         // Initialize the iterator if there's somewhere to point to
         if (node_ != NULL && node_->getData() != DomainPtr()) {
             dom_iterator_ = node_->getData()->begin();
+            if (separate_rrs_ && dom_iterator_ != node_->getData()->end()) {
+                rdata_iterator_ = dom_iterator_->second->getRdataIterator();
+            }
         }
     }
 
@@ -766,6 +773,10 @@ public:
             // if the map is empty or not
             if (node_ != NULL && node_->getData() != NULL) {
                 dom_iterator_ = node_->getData()->begin();
+                // New RRset, so get a new rdata iterator
+                if (separate_rrs_) {
+                    rdata_iterator_ = dom_iterator_->second->getRdataIterator();
+                }
             }
         }
         if (node_ == NULL) {
@@ -773,12 +784,35 @@ public:
             ready_ = false;
             return (ConstRRsetPtr());
         }
-        // The iterator points to the next yet unused RRset now
-        ConstRRsetPtr result(dom_iterator_->second);
-        // This one is used, move it to the next time for next call
-        ++dom_iterator_;
 
-        return (result);
+        if (separate_rrs_) {
+            // For separate rrs, reconstruct a new RRset with just the
+            // 'current' rdata
+            RRsetPtr result(new RRset(dom_iterator_->second->getName(),
+                                      dom_iterator_->second->getClass(),
+                                      dom_iterator_->second->getType(),
+                                      dom_iterator_->second->getTTL()));
+            result->addRdata(rdata_iterator_->getCurrent());
+            rdata_iterator_->next();
+            if (rdata_iterator_->isLast()) {
+                // all used up, next.
+                ++dom_iterator_;
+                // New RRset, so get a new rdata iterator, but only if this
+                // was not the final RRset in the chain
+                if (dom_iterator_ != node_->getData()->end()) {
+                    rdata_iterator_ = dom_iterator_->second->getRdataIterator();
+                }
+            }
+            return (result);
+        } else {
+            // The iterator points to the next yet unused RRset now
+            ConstRRsetPtr result(dom_iterator_->second);
+
+            // This one is used, move it to the next time for next call
+            ++dom_iterator_;
+
+            return (result);
+        }
     }
 
     virtual ConstRRsetPtr getSOA() const {
@@ -789,11 +823,7 @@ public:
 } // End of anonymous namespace
 
 ZoneIteratorPtr
-InMemoryClient::getIterator(const Name& name, bool) const {
-    // note: adjust_ttl argument is ignored, as the RRsets are already
-    // individually stored, and hence cannot have different TTLs anymore at
-    // this point
-
+InMemoryClient::getIterator(const Name& name, bool separate_rrs) const {
     ZoneTable::FindResult result(impl_->zone_table.findZone(name));
     if (result.code != result::SUCCESS) {
         isc_throw(DataSourceError, "No such zone: " + name.toText());
@@ -811,7 +841,8 @@ InMemoryClient::getIterator(const Name& name, bool) const {
         isc_throw(Unexpected, "The zone at " + name.toText() +
                   " is not InMemoryZoneFinder");
     }
-    return (ZoneIteratorPtr(new MemoryIterator(zone->impl_->domains_, name)));
+    return (ZoneIteratorPtr(new MemoryIterator(zone->impl_->domains_, name,
+                                               separate_rrs)));
 }
 
 ZoneUpdaterPtr
@@ -819,6 +850,13 @@ InMemoryClient::getUpdater(const isc::dns::Name&, bool, bool) const {
     isc_throw(isc::NotImplemented, "Update attempt on in memory data source");
 }
 
+pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
+InMemoryClient::getJournalReader(const isc::dns::Name&, uint32_t,
+                                 uint32_t) const
+{
+    isc_throw(isc::NotImplemented, "Journaling isn't supported for "
+              "in memory data source");
+}
 
 namespace {
 // convencience function to add an error message to a list of those

src/lib/datasrc/memory_datasrc.h

@@ -273,7 +273,7 @@ public:
 
     /// \brief Implementation of the getIterator method
     virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name,
-                                        bool adjust_ttl = true) const;
+                                        bool separate_rrs = false) const;
 
     /// In-memory data source is read-only, so this derived method will
     /// result in a NotImplemented exception.
@@ -287,6 +287,10 @@ public:
                                       bool replace, bool journaling = false)
         const;
 
+    virtual std::pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
+    getJournalReader(const isc::dns::Name& zone, uint32_t begin_serial,
+                     uint32_t end_serial) const;
+
 private:
     // TODO: Do we still need the PImpl if nobody should manipulate this class
     // directly any more (it should be handled through DataSourceClient)?

src/lib/datasrc/sqlite3_accessor.h

@@ -71,7 +71,6 @@ public:
         DataSourceError(file, line, what) {}
 };
 
-
 struct SQLite3Parameters;
 
 /**

src/lib/datasrc/tests/client_unittest.cc

@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <utility>
+
 #include <datasrc/client.h>
 
 #include <dns/name.h>
@@ -37,6 +39,11 @@ public:
     {
         return (ZoneUpdaterPtr());
     }
+    virtual std::pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
+    getJournalReader(const isc::dns::Name&, uint32_t, uint32_t) const {
+        isc_throw(isc::NotImplemented, "Journaling isn't supported "
+                  "in Nop data source");
+    }
 };
 
 class ClientTest : public ::testing::Test {

src/lib/datasrc/tests/database_unittest.cc

@@ -12,11 +12,15 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <stdlib.h>
+
 #include <boost/shared_ptr.hpp>
 #include <boost/lexical_cast.hpp>
 
 #include <gtest/gtest.h>
 
+#include <exceptions/exceptions.h>
+
 #include <dns/name.h>
 #include <dns/rrttl.h>
 #include <dns/rrset.h>
@@ -31,6 +35,7 @@
 #include <testutils/dnsmessage_test.h>
 
 #include <map>
+#include <vector>
 
 using namespace isc::datasrc;
 using namespace std;
@@ -259,7 +264,7 @@ public:
 
     virtual IteratorContextPtr getDiffs(int, uint32_t, uint32_t) const {
         isc_throw(isc::NotImplemented,
-                  "This database datasource can't be iterated");
+                  "This database datasource doesn't support diffs");
     }
 
     virtual std::string findPreviousName(int, const std::string&) const {
@@ -550,6 +555,29 @@ private:
             }
         }
     };
+    class MockDiffIteratorContext : public IteratorContext {
+        const vector<JournalEntry> diffs_;
+        vector<JournalEntry>::const_iterator it_;
+    public:
+        MockDiffIteratorContext(const vector<JournalEntry>& diffs) :
+            diffs_(diffs), it_(diffs_.begin())
+        {}
+        virtual bool getNext(string (&data)[COLUMN_COUNT]) {
+            if (it_ == diffs_.end()) {
+                return (false);
+            }
+            data[DatabaseAccessor::NAME_COLUMN] =
+                (*it_).data_[DatabaseAccessor::DIFF_NAME];
+            data[DatabaseAccessor::TYPE_COLUMN] =
+                (*it_).data_[DatabaseAccessor::DIFF_TYPE];
+            data[DatabaseAccessor::TTL_COLUMN] =
+                (*it_).data_[DatabaseAccessor::DIFF_TTL];
+            data[DatabaseAccessor::RDATA_COLUMN] =
+                (*it_).data_[DatabaseAccessor::DIFF_RDATA];
+            ++it_;
+            return (true);
+        }
+    };
 public:
     virtual IteratorContextPtr getAllRecords(int id) const {
         if (id == READONLY_ZONE_ID) {
@@ -729,12 +757,52 @@ public:
             isc_throw(DataSourceError, "Test error");
         } else {
             journal_entries_->push_back(JournalEntry(id, serial, operation,
-                                                    data));
+                                                     data));
+        }
+    }
+
+    virtual IteratorContextPtr getDiffs(int id, uint32_t start,
+                                        uint32_t end) const
+    {
+        vector<JournalEntry> selected_jnl;
+
+        for (vector<JournalEntry>::const_iterator it =
+                 journal_entries_->begin();
+             it != journal_entries_->end(); ++it)
+        {
+            // For simplicity we assume this method is called for the
+            // "readonly" zone possibly after making updates on the "writable"
+            // copy and committing them.
+            if (id != READONLY_ZONE_ID) {
+                continue;
+            }
+
+            // Note: the following logic is not 100% accurate in terms of
+            // serial number arithmetic; we prefer brevity for testing.
+            // Skip until we see the starting serial.  Once we started
+            // recording this condition is ignored (to support wrap-around
+            // case).  Also, it ignores the RR type; it only checks the
+            // versions.
+            if ((*it).serial_ < start && selected_jnl.empty()) {
+                continue;
+            }
+            if ((*it).serial_ > end) { // gone over the end serial. we're done.
+                break;
+            }
+            selected_jnl.push_back(*it);
         }
+
+        // Check if we've found the requested range.  If not, throw.
+        if (selected_jnl.empty() || selected_jnl.front().serial_ != start ||
+            selected_jnl.back().serial_ != end) {
+            isc_throw(NoSuchSerial, "requested diff range is not found");
+        }
+
+        return (IteratorContextPtr(new MockDiffIteratorContext(selected_jnl)));
     }
 
     // Check the journal is as expected and clear the journal
-    void checkJournal(const std::vector<JournalEntry> &expected) {
+    void checkJournal(const std::vector<JournalEntry> &expected) const {
         std::vector<JournalEntry> journal;
         // Clean the journal, but keep local copy to check
         journal.swap(*journal_entries_);
@@ -903,6 +971,24 @@ public:
      * times per test.
      */
     void createClient() {
+        // To make sure we always have empty diffs table at the beginning of
+        // each test, we re-install the writable data source here.
+        // Note: this is SQLite3 specific and a waste (though otherwise
+        // harmless) for other types of data sources.  If and when we support
+        // more types of data sources in this test framework, we should
+        // probably move this to some specialized templated method specific
+        // to SQLite3 (or for even a longer term we should add an API to
+        // purge the diffs table).
+        const char* const install_cmd = INSTALL_PROG " " TEST_DATA_DIR
+            "/rwtest.sqlite3 " TEST_DATA_BUILDDIR
+            "/rwtest.sqlite3.copied";
+        if (system(install_cmd) != 0) {
+            // any exception will do, this is failure in test setup, but nice
+            // to show the command that fails, and shouldn't be caught
+            isc_throw(isc::Exception,
+                      "Error setting up; command failed: " << install_cmd);
+        }
+
         current_accessor_ = new ACCESSOR_TYPE();
         is_mock_ = (dynamic_cast<MockAccessor*>(current_accessor_) != NULL);
         client_.reset(new DatabaseClient(qclass_,
@@ -968,6 +1054,48 @@ public:
         }
     }
 
+    void checkJournal(const vector<JournalEntry>& expected) {
+        if (is_mock_) {
+            const MockAccessor* mock_accessor =
+                dynamic_cast<const MockAccessor*>(current_accessor_);
+            mock_accessor->checkJournal(expected);
+        } else {
+            // For other generic databases, retrieve the diff using the
+            // reader class and compare the resulting sequence of RRset.
+            // For simplicity we only consider the case where the expected
+            // sequence is not empty.
+            ASSERT_FALSE(expected.empty());
+            const Name zone_name(expected.front().
+                                 data_[DatabaseAccessor::DIFF_NAME]);
+            ZoneJournalReaderPtr jnl_reader =
+                client_->getJournalReader(zone_name,
+                                          expected.front().serial_,
+                                          expected.back().serial_).second;
+            ASSERT_TRUE(jnl_reader);
+            ConstRRsetPtr rrset;
+            vector<JournalEntry>::const_iterator it = expected.begin();
+            for (rrset = jnl_reader->getNextDiff();
+                 rrset && it != expected.end();
+                 rrset = jnl_reader->getNextDiff(), ++it) {
+                typedef DatabaseAccessor Accessor;
+                RRsetPtr expected_rrset(
+                    new RRset(Name((*it).data_[Accessor::DIFF_NAME]),
+                              qclass_,
+                              RRType((*it).data_[Accessor::DIFF_TYPE]),
+                              RRTTL((*it).data_[Accessor::DIFF_TTL])));
+                expected_rrset->addRdata(
+                    rdata::createRdata(expected_rrset->getType(),
+                                       expected_rrset->getClass(),
+                                       (*it).data_[Accessor::DIFF_RDATA]));
+                isc::testutils::rrsetCheck(expected_rrset, rrset);
+            }
+            // We should have examined all entries of both expected and
+            // actual data.
+            EXPECT_TRUE(it == expected.end());
+            ASSERT_FALSE(rrset);
+        }
+    }
+
     // Some tests only work for MockAccessor.  We remember whether our accessor
     // is of that type.
     bool is_mock_;
@@ -1340,8 +1468,8 @@ TEST_F(MockDatabaseClientTest, ttldiff) {
 
 // Unless we ask for individual RRs in our iterator request. In that case
 // every RR should go into its own 'rrset'
-TEST_F(MockDatabaseClientTest, ttldiff_no_adjust_ttl) {
-    ZoneIteratorPtr it(this->client_->getIterator(Name("example.org"), false));
+TEST_F(MockDatabaseClientTest, ttldiff_separate_rrs) {
+    ZoneIteratorPtr it(this->client_->getIterator(Name("example.org"), true));
 
     // Walk through the full iterator, we should see 1 rrset with name
     // ttldiff1.example.org., and two rdatas. Same for ttldiff2
@@ -2800,17 +2928,20 @@ TEST_F(MockDatabaseClientTest, badName) {
 /*
  * Test correct use of the updater with a journal.
  */
-TEST_F(MockDatabaseClientTest, journal) {
-    updater_ = client_->getUpdater(zname_, false, true);
-    updater_->deleteRRset(*soa_);
-    updater_->deleteRRset(*rrset_);
-    soa_.reset(new RRset(zname_, qclass_, RRType::SOA(), rrttl_));
-    soa_->addRdata(rdata::createRdata(soa_->getType(), soa_->getClass(),
-                                      "ns1.example.org. admin.example.org. "
-                                      "1235 3600 1800 2419200 7200"));
-    updater_->addRRset(*soa_);
-    updater_->addRRset(*rrset_);
-    ASSERT_NO_THROW(updater_->commit());
+TYPED_TEST(DatabaseClientTest, journal) {
+    this->updater_ = this->client_->getUpdater(this->zname_, false, true);
+    this->updater_->deleteRRset(*this->soa_);
+    this->updater_->deleteRRset(*this->rrset_);
+    this->soa_.reset(new RRset(this->zname_, this->qclass_, RRType::SOA(),
+                               this->rrttl_));
+    this->soa_->addRdata(rdata::createRdata(this->soa_->getType(),
+                                            this->soa_->getClass(),
+                                            "ns1.example.org. "
+                                            "admin.example.org. "
+                                            "1235 3600 1800 2419200 7200"));
+    this->updater_->addRRset(*this->soa_);
+    this->updater_->addRRset(*this->rrset_);
+    ASSERT_NO_THROW(this->updater_->commit());
     std::vector<JournalEntry> expected;
     expected.push_back(JournalEntry(WRITABLE_ZONE_ID, 1234,
                                     DatabaseAccessor::DIFF_DELETE,
@@ -2830,21 +2961,21 @@ TEST_F(MockDatabaseClientTest, journal) {
                                     DatabaseAccessor::DIFF_ADD,
                                     "www.example.org.", "A", "3600",
                                     "192.0.2.2"));
-    current_accessor_->checkJournal(expected);
+    this->checkJournal(expected);
 }
 
 /*
  * Push multiple delete-add sequences. Checks it is allowed and all is
  * saved.
  */
-TEST_F(MockDatabaseClientTest, journalMultiple) {
+TYPED_TEST(DatabaseClientTest, journalMultiple) {
     std::vector<JournalEntry> expected;
-    updater_ = client_->getUpdater(zname_, false, true);
+    this->updater_ = this->client_->getUpdater(this->zname_, false, true);
     std::string soa_rdata = "ns1.example.org. admin.example.org. "
         "1234 3600 1800 2419200 7200";
     for (size_t i(1); i < 100; ++ i) {
         // Remove the old SOA
-        updater_->deleteRRset(*soa_);
+        this->updater_->deleteRRset(*this->soa_);
         expected.push_back(JournalEntry(WRITABLE_ZONE_ID, 1234 + i - 1,
                                         DatabaseAccessor::DIFF_DELETE,
                                         "example.org.", "SOA", "3600",
@@ -2852,19 +2983,21 @@ TEST_F(MockDatabaseClientTest, journalMultiple) {
         // Create a new SOA
         soa_rdata = "ns1.example.org. admin.example.org. " +
             lexical_cast<std::string>(1234 + i) + " 3600 1800 2419200 7200";
-        soa_.reset(new RRset(zname_, qclass_, RRType::SOA(), rrttl_));
-        soa_->addRdata(rdata::createRdata(soa_->getType(), soa_->getClass(),
-                                          soa_rdata));
+        this->soa_.reset(new RRset(this->zname_, this->qclass_, RRType::SOA(),
+                                   this->rrttl_));
+        this->soa_->addRdata(rdata::createRdata(this->soa_->getType(),
+                                                this->soa_->getClass(),
+                                                soa_rdata));
         // Add the new SOA
-        updater_->addRRset(*soa_);
+        this->updater_->addRRset(*this->soa_);
         expected.push_back(JournalEntry(WRITABLE_ZONE_ID, 1234 + i,
                                         DatabaseAccessor::DIFF_ADD,
                                         "example.org.", "SOA", "3600",
                                         soa_rdata));
     }
-    ASSERT_NO_THROW(updater_->commit());
+    ASSERT_NO_THROW(this->updater_->commit());
     // Check the journal contains everything.
-    current_accessor_->checkJournal(expected);
+    this->checkJournal(expected);
 }
 
 /*
@@ -2872,46 +3005,50 @@ TEST_F(MockDatabaseClientTest, journalMultiple) {
  *
  * Note that we implicitly test in different testcases (these for add and
  * delete) that if the journaling is false, it doesn't expect the order.
+ *
+ * In this test we don't check with the real databases as this case shouldn't
+ * contain backend specific behavior.
  */
 TEST_F(MockDatabaseClientTest, journalBadSequence) {
     std::vector<JournalEntry> expected;
     {
         SCOPED_TRACE("Delete A before SOA");
-        updater_ = client_->getUpdater(zname_, false, true);
-        EXPECT_THROW(updater_->deleteRRset(*rrset_), isc::BadValue);
+        this->updater_ = this->client_->getUpdater(this->zname_, false, true);
+        EXPECT_THROW(this->updater_->deleteRRset(*this->rrset_),
+                     isc::BadValue);
         // Make sure the journal is empty now
-        current_accessor_->checkJournal(expected);
+        this->checkJournal(expected);
     }
 
     {
         SCOPED_TRACE("Add before delete");
-        updater_ = client_->getUpdater(zname_, false, true);
-        EXPECT_THROW(updater_->addRRset(*soa_), isc::BadValue);
+        this->updater_ = this->client_->getUpdater(this->zname_, false, true);
+        EXPECT_THROW(this->updater_->addRRset(*this->soa_), isc::BadValue);
         // Make sure the journal is empty now
-        current_accessor_->checkJournal(expected);
+        this->checkJournal(expected);
     }
 
     {
         SCOPED_TRACE("Add A before SOA");
-        updater_ = client_->getUpdater(zname_, false, true);
+        this->updater_ = this->client_->getUpdater(this->zname_, false, true);
         // So far OK
-        EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
+        EXPECT_NO_THROW(this->updater_->deleteRRset(*this->soa_));
         // But we miss the add SOA here
-        EXPECT_THROW(updater_->addRRset(*rrset_), isc::BadValue);
+        EXPECT_THROW(this->updater_->addRRset(*this->rrset_), isc::BadValue);
         // Make sure the journal contains only the first one
         expected.push_back(JournalEntry(WRITABLE_ZONE_ID, 1234,
                                         DatabaseAccessor::DIFF_DELETE,
                                         "example.org.", "SOA", "3600",
                                         "ns1.example.org. admin.example.org. "
                                         "1234 3600 1800 2419200 7200"));
-        current_accessor_->checkJournal(expected);
+        this->checkJournal(expected);
     }
 
     {
         SCOPED_TRACE("Commit before add");
-        updater_ = client_->getUpdater(zname_, false, true);
+        this->updater_ = this->client_->getUpdater(this->zname_, false, true);
         // So far OK
-        EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
+        EXPECT_NO_THROW(this->updater_->deleteRRset(*this->soa_));
         // Commit at the wrong time
         EXPECT_THROW(updater_->commit(), isc::BadValue);
         current_accessor_->checkJournal(expected);
@@ -2919,29 +3056,29 @@ TEST_F(MockDatabaseClientTest, journalBadSequence) {
 
     {
         SCOPED_TRACE("Delete two SOAs");
-        updater_ = client_->getUpdater(zname_, false, true);
+        this->updater_ = this->client_->getUpdater(this->zname_, false, true);
         // So far OK
-        EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
+        EXPECT_NO_THROW(this->updater_->deleteRRset(*this->soa_));
         // Delete the SOA again
-        EXPECT_THROW(updater_->deleteRRset(*soa_), isc::BadValue);
-        current_accessor_->checkJournal(expected);
+        EXPECT_THROW(this->updater_->deleteRRset(*this->soa_), isc::BadValue);
+        this->checkJournal(expected);
     }
 
     {
         SCOPED_TRACE("Add two SOAs");
-        updater_ = client_->getUpdater(zname_, false, true);
+        this->updater_ = this->client_->getUpdater(this->zname_, false, true);
         // So far OK
-        EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
+        EXPECT_NO_THROW(this->updater_->deleteRRset(*this->soa_));
         // Still OK
-        EXPECT_NO_THROW(updater_->addRRset(*soa_));
+        EXPECT_NO_THROW(this->updater_->addRRset(*this->soa_));
         // But this one is added again
-        EXPECT_THROW(updater_->addRRset(*soa_), isc::BadValue);
+        EXPECT_THROW(this->updater_->addRRset(*this->soa_), isc::BadValue);
         expected.push_back(JournalEntry(WRITABLE_ZONE_ID, 1234,
                                         DatabaseAccessor::DIFF_ADD,
                                         "example.org.", "SOA", "3600",
                                         "ns1.example.org. admin.example.org. "
                                         "1234 3600 1800 2419200 7200"));
-        current_accessor_->checkJournal(expected);
+        this->checkJournal(expected);
     }
 }
 
@@ -2949,8 +3086,9 @@ TEST_F(MockDatabaseClientTest, journalBadSequence) {
  * Test it rejects to store journals when we request it together with
  * erasing the whole zone.
  */
-TEST_F(MockDatabaseClientTest, journalOnErase) {
-    EXPECT_THROW(client_->getUpdater(zname_, true, true), isc::BadValue);
+TYPED_TEST(DatabaseClientTest, journalOnErase) {
+    EXPECT_THROW(this->client_->getUpdater(this->zname_, true, true),
+                 isc::BadValue);
 }
 
 /*
@@ -2974,4 +3112,149 @@ TEST_F(MockDatabaseClientTest, journalException) {
     EXPECT_THROW(updater_->deleteRRset(*soa_), DataSourceError);
 }
 
+//
+// Tests for the ZoneJournalReader
+//
+
+// Install a simple, commonly used diff sequence: making an update from one
+// SOA to another.  Return the end SOA RRset for the convenience of the caller.
+ConstRRsetPtr
+makeSimpleDiff(DataSourceClient& client, const Name& zname,
+               const RRClass& rrclass, ConstRRsetPtr begin_soa)
+{
+    ZoneUpdaterPtr updater = client.getUpdater(zname, false, true);
+    updater->deleteRRset(*begin_soa);
+    RRsetPtr soa_end(new RRset(zname, rrclass, RRType::SOA(), RRTTL(3600)));
+    soa_end->addRdata(rdata::createRdata(RRType::SOA(), rrclass,
+                                         "ns1.example.org. admin.example.org. "
+                                         "1235 3600 1800 2419200 7200"));
+    updater->addRRset(*soa_end);
+    updater->commit();
+
+    return (soa_end);
+}
+
+TYPED_TEST(DatabaseClientTest, journalReader) {
+    // Check the simple case made by makeSimpleDiff.
+    ConstRRsetPtr soa_end = makeSimpleDiff(*this->client_, this->zname_,
+                                           this->qclass_, this->soa_);
+    pair<ZoneJournalReader::Result, ZoneJournalReaderPtr> result =
+        this->client_->getJournalReader(this->zname_, 1234, 1235);
+    EXPECT_EQ(ZoneJournalReader::SUCCESS, result.first);
+    ZoneJournalReaderPtr jnl_reader = result.second;
+    ASSERT_TRUE(jnl_reader);
+    ConstRRsetPtr rrset = jnl_reader->getNextDiff();
+    ASSERT_TRUE(rrset);
+    isc::testutils::rrsetCheck(this->soa_, rrset);
+    rrset = jnl_reader->getNextDiff();
+    ASSERT_TRUE(rrset);
+    isc::testutils::rrsetCheck(soa_end, rrset);
+    rrset = jnl_reader->getNextDiff();
+    ASSERT_FALSE(rrset);
+
+    // Once it reaches the end of the sequence, further read attempt will
+    // result in exception.
+    EXPECT_THROW(jnl_reader->getNextDiff(), isc::InvalidOperation);
+}
+
+TYPED_TEST(DatabaseClientTest, readLargeJournal) {
+    // Similar to journalMultiple, but check that at a higher level.
+
+    this->updater_ = this->client_->getUpdater(this->zname_, false, true);
+
+    vector<ConstRRsetPtr> expected;
+    for (size_t i = 0; i < 100; ++i) {
+        // Create the old SOA and remove it, and record it in the expected list
+        RRsetPtr rrset1(new RRset(this->zname_, this->qclass_, RRType::SOA(),
+                                  this->rrttl_));
+        string soa_rdata = "ns1.example.org. admin.example.org. " +
+            lexical_cast<std::string>(1234 + i) + " 3600 1800 2419200 7200";
+        rrset1->addRdata(rdata::createRdata(RRType::SOA(), this->qclass_,
+                                            soa_rdata));
+        this->updater_->deleteRRset(*rrset1);
+        expected.push_back(rrset1);
+
+        // Create a new SOA, add it, and record it.
+        RRsetPtr rrset2(new RRset(this->zname_, this->qclass_, RRType::SOA(),
+                                  this->rrttl_));
+        soa_rdata = "ns1.example.org. admin.example.org. " +
+            lexical_cast<std::string>(1234 + i + 1) +
+            " 3600 1800 2419200 7200";
+        rrset2->addRdata(rdata::createRdata(RRType::SOA(), this->qclass_,
+                                            soa_rdata));
+        this->updater_->addRRset(*rrset2);
+        expected.push_back(rrset2);
+    }
+    this->updater_->commit();
+
+    ZoneJournalReaderPtr jnl_reader(this->client_->getJournalReader(
+                                        this->zname_, 1234, 1334).second);
+    ConstRRsetPtr actual;
+    int i = 0;
+    while ((actual = jnl_reader->getNextDiff()) != NULL) {
+        isc::testutils::rrsetCheck(expected.at(i++), actual);
+    }
+    EXPECT_EQ(expected.size(), i); // we should have eaten all expected data
+}
+
+TYPED_TEST(DatabaseClientTest, readJournalForNoRange) {
+    makeSimpleDiff(*this->client_, this->zname_, this->qclass_, this->soa_);
+
+    // The specified range does not exist in the diff storage.  The factory
+    // method should result in NO_SUCH_VERSION
+    pair<ZoneJournalReader::Result, ZoneJournalReaderPtr> result =
+        this->client_->getJournalReader(this->zname_, 1200, 1235);
+    EXPECT_EQ(ZoneJournalReader::NO_SUCH_VERSION, result.first);
+    EXPECT_FALSE(result.second);
+}
+
+TYPED_TEST(DatabaseClientTest, journalReaderForNXZone) {
+    pair<ZoneJournalReader::Result, ZoneJournalReaderPtr> result =
+        this->client_->getJournalReader(Name("nosuchzone"), 0, 1);
+    EXPECT_EQ(ZoneJournalReader::NO_SUCH_ZONE, result.first);
+    EXPECT_FALSE(result.second);
+}
+
+// A helper function for journalWithBadData.  It installs a simple diff
+// from one serial (of 'begin') to another ('begin' + 1), tweaking a specified
+// field of data with some invalid value.
+void
+installBadDiff(MockAccessor& accessor, uint32_t begin,
+               DatabaseAccessor::DiffRecordParams modify_param,
+               const char* const data)
+{
+    string data1[] = {"example.org.", "SOA", "3600", "ns. root. 1 1 1 1 1"};
+    string data2[] = {"example.org.", "SOA", "3600", "ns. root. 2 1 1 1 1"};
+    data1[modify_param] = data;
+    accessor.addRecordDiff(READONLY_ZONE_ID, begin,
+                           DatabaseAccessor::DIFF_DELETE, data1);
+    accessor.addRecordDiff(READONLY_ZONE_ID, begin + 1,
+                           DatabaseAccessor::DIFF_ADD, data2);
+}
+
+TEST_F(MockDatabaseClientTest, journalWithBadData) {
+    MockAccessor& mock_accessor =
+        dynamic_cast<MockAccessor&>(*current_accessor_);
+
+    // One of the fields from the data source is broken as an RR parameter.
+    // The journal reader should still be constructed, but getNextDiff()
+    // should result in exception.
+    installBadDiff(mock_accessor, 1, DatabaseAccessor::DIFF_NAME,
+                   "example..org");
+    installBadDiff(mock_accessor, 3, DatabaseAccessor::DIFF_TYPE,
+                   "bad-rrtype");
+    installBadDiff(mock_accessor, 5, DatabaseAccessor::DIFF_TTL,
+                   "bad-ttl");
+    installBadDiff(mock_accessor, 7, DatabaseAccessor::DIFF_RDATA,
+                   "bad rdata");
+    EXPECT_THROW(this->client_->getJournalReader(this->zname_, 1, 2).
+                 second->getNextDiff(), DataSourceError);
+    EXPECT_THROW(this->client_->getJournalReader(this->zname_, 3, 4).
+                 second->getNextDiff(), DataSourceError);
+    EXPECT_THROW(this->client_->getJournalReader(this->zname_, 5, 6).
+                 second->getNextDiff(), DataSourceError);
+    EXPECT_THROW(this->client_->getJournalReader(this->zname_, 7, 8).
+                 second->getNextDiff(), DataSourceError);
+}
+
 }

src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -177,6 +177,54 @@ TEST_F(InMemoryClientTest, iterator) {
     EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
 }
 
+TEST_F(InMemoryClientTest, iterator_separate_rrs) {
+    // Exactly the same tests as for iterator, but now with separate_rrs = true
+    // For the one that returns actual data, the AAAA should now be split up
+    boost::shared_ptr<InMemoryZoneFinder>
+        zone(new InMemoryZoneFinder(RRClass::IN(), Name("a")));
+    RRsetPtr aRRsetA(new RRset(Name("a"), RRClass::IN(), RRType::A(),
+                                  RRTTL(300)));
+    aRRsetA->addRdata(rdata::in::A("192.0.2.1"));
+    RRsetPtr aRRsetAAAA(new RRset(Name("a"), RRClass::IN(), RRType::AAAA(),
+                                  RRTTL(300)));
+    aRRsetAAAA->addRdata(rdata::in::AAAA("2001:db8::1"));
+    aRRsetAAAA->addRdata(rdata::in::AAAA("2001:db8::2"));
+    RRsetPtr aRRsetAAAA_r1(new RRset(Name("a"), RRClass::IN(), RRType::AAAA(),
+                                  RRTTL(300)));
+    aRRsetAAAA_r1->addRdata(rdata::in::AAAA("2001:db8::1"));
+    RRsetPtr aRRsetAAAA_r2(new RRset(Name("a"), RRClass::IN(), RRType::AAAA(),
+                                  RRTTL(300)));
+    aRRsetAAAA_r2->addRdata(rdata::in::AAAA("2001:db8::2"));
+
+    RRsetPtr subRRsetA(new RRset(Name("sub.x.a"), RRClass::IN(), RRType::A(),
+                                  RRTTL(300)));
+    subRRsetA->addRdata(rdata::in::A("192.0.2.2"));
+    EXPECT_EQ(result::SUCCESS, memory_client.addZone(zone));
+
+    // First, the zone is not there, so it should throw
+    EXPECT_THROW(memory_client.getIterator(Name("b"), true), DataSourceError);
+    // This zone is not there either, even when there's a zone containing this
+    EXPECT_THROW(memory_client.getIterator(Name("x.a")), DataSourceError);
+    // Now, an empty zone
+    ZoneIteratorPtr iterator(memory_client.getIterator(Name("a"), true));
+    EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
+    // It throws Unexpected when we are past the end
+    EXPECT_THROW(iterator->getNextRRset(), isc::Unexpected);
+
+    ASSERT_EQ(result::SUCCESS, zone->add(aRRsetA));
+    ASSERT_EQ(result::SUCCESS, zone->add(aRRsetAAAA));
+    ASSERT_EQ(result::SUCCESS, zone->add(subRRsetA));
+    // Check it with full zone, one by one.
+    // It should be in ascending order in case of InMemory data source
+    // (isn't guaranteed in general)
+    iterator = memory_client.getIterator(Name("a"), true);
+    EXPECT_EQ(aRRsetA->toText(), iterator->getNextRRset()->toText());
+    EXPECT_EQ(aRRsetAAAA_r1->toText(), iterator->getNextRRset()->toText());
+    EXPECT_EQ(aRRsetAAAA_r2->toText(), iterator->getNextRRset()->toText());
+    EXPECT_EQ(subRRsetA->toText(), iterator->getNextRRset()->toText());
+    EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
+}
+
 TEST_F(InMemoryClientTest, getZoneCount) {
     EXPECT_EQ(0, memory_client.getZoneCount());
     memory_client.addZone(

src/lib/datasrc/tests/testdata/Makefile.am

@@ -1,6 +1 @@
 CLEANFILES = *.copied
-BUILT_SOURCES = rwtest.sqlite3.copied
-
-# We use install-sh with the -m option to make sure it's writable
-rwtest.sqlite3.copied: $(srcdir)/rwtest.sqlite3
-	$(top_srcdir)/install-sh -m 644 $(srcdir)/rwtest.sqlite3 $@

src/lib/datasrc/zone.h

@@ -560,6 +560,98 @@ public:
 /// \brief A pointer-like type pointing to a \c ZoneUpdater object.
 typedef boost::shared_ptr<ZoneUpdater> ZoneUpdaterPtr;
 
+/// The base class for retrieving differences between two versions of a zone.
+///
+/// On construction, each derived class object will internally set up
+/// retrieving sequences of differences between two specific version of
+/// a specific zone managed in a particular data source.  So the constructor
+/// of a derived class would normally take parameters to identify the zone
+/// and the two versions for which the differences should be retrieved.
+/// See \c DataSourceClient::getJournalReader for more concrete details
+/// used in this API.
+///
+/// Once constructed, an object of this class will act like an iterator
+/// over the sequences.  Every time the \c getNextDiff() method is called
+/// it returns one element of the differences in the form of an \c RRset
+/// until it reaches the end of the entire sequences.
+class ZoneJournalReader {
+public:
+    /// Result codes used by a factory method for \c ZoneJournalReader
+    enum Result {
+        SUCCESS, ///< A \c ZoneJournalReader object successfully created
+        NO_SUCH_ZONE, ///< Specified zone does not exist in the data source
+        NO_SUCH_VERSION ///< Specified versions do not exist in the diff storage
+    };
+
+protected:
+    /// The default constructor.
+    ///
+    /// This is intentionally defined as protected to ensure that this base
+    /// class is never instantiated directly.
+    ZoneJournalReader() {}
+
+public:
+    /// The destructor
+    virtual ~ZoneJournalReader() {}
+
+    /// Return the next difference RR of difference sequences.
+    ///
+    /// In this API, the difference between two versions of a zone is
+    /// conceptually represented as IXFR-style difference sequences:
+    /// Each difference sequence is a sequence of RRs: an older version of
+    /// SOA (to be deleted), zero or more other deleted RRs, the
+    /// post-transaction SOA (to be added), and zero or more other
+    /// added RRs.  (Note, however, that the underlying data source
+    /// implementation may or may not represent the difference in
+    /// straightforward realization of this concept.  The mapping between
+    /// the conceptual difference and the actual implementation is hidden
+    /// in each derived class).
+    ///
+    /// This method provides an application with a higher level interface
+    /// to retrieve the difference along with the conceptual model: the
+    /// \c ZoneJournalReader object iterates over the entire sequences
+    /// from the beginning SOA (which is to be deleted) to one of the
+    /// added RR of with the ending SOA, and each call to this method returns
+    /// one RR in the form of an \c RRset that contains exactly one RDATA
+    /// in the order of the sequences.
+    ///
+    /// Note that the ordering of the sequences specifies the semantics of
+    /// each difference: add or delete.  For example, the first RR is to
+    /// be deleted, and the last RR is to be added.  So the return value
+    /// of this method does not explicitly indicate whether the RR is to be
+    /// added or deleted.
+    ///
+    /// This method ensures the returned \c RRset represents an RR, that is,
+    /// it contains exactly one RDATA.  However, it does not necessarily
+    /// ensure that the resulting sequences are in the form of IXFR-style.
+    /// For example, the first RR is supposed to be an SOA, and it should
+    /// normally be the case, but this interface does not necessarily require
+    /// the derived class implementation ensure this.  Normally the
+    /// differences are expected to be stored using this API (via a
+    /// \c ZoneUpdater object), and as long as that is the case and the
+    /// underlying implementation follows the requirement of the API, the
+    /// result of this method should be a valid IXFR-style sequences.
+    /// So this API does not mandate the almost redundant check as part of
+    /// the interface.  If the application needs to make it sure 100%, it
+    /// must check the resulting sequence itself.
+    ///
+    /// Once the object reaches the end of the sequences, this method returns
+    /// \c Null.  Any subsequent call will result in an exception of
+    /// class \c InvalidOperation.
+    ///
+    /// \exception InvalidOperation The method is called beyond the end of
+    /// the difference sequences.
+    /// \exception DataSourceError Underlying data is broken and the RR
+    /// cannot be created or other low level data source error.
+    ///
+    /// \return An \c RRset that contains one RDATA corresponding to the
+    /// next difference in the sequences.
+    virtual isc::dns::ConstRRsetPtr getNextDiff() = 0;
+};
+
+/// \brief A pointer-like type pointing to a \c ZoneUpdater object.
+typedef boost::shared_ptr<ZoneJournalReader> ZoneJournalReaderPtr;
+
 } // end of datasrc
 } // end of isc
 

src/lib/exceptions/exceptions.h

@@ -126,6 +126,17 @@ public:
         isc::Exception(file, line, what) {}
 };
 
+/// \brief A generic exception that is thrown if a function is called
+/// in a prohibited way.
+///
+/// For example, this can happen if a class method is called when the object's
+/// state does not allow that particular method.
+class InvalidOperation : public Exception {
+public:
+    InvalidOperation(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
 ///
 /// \brief A generic exception that is thrown when an unexpected
 /// error condition occurs.

src/lib/python/isc/datasrc/Makefile.am

@@ -17,6 +17,7 @@ datasrc_la_SOURCES += client_python.cc client_python.h
 datasrc_la_SOURCES += iterator_python.cc iterator_python.h
 datasrc_la_SOURCES += finder_python.cc finder_python.h
 datasrc_la_SOURCES += updater_python.cc updater_python.h
+datasrc_la_SOURCES += journal_reader_python.cc journal_reader_python.h
 
 datasrc_la_CPPFLAGS = $(AM_CPPFLAGS) $(PYTHON_INCLUDES)
 datasrc_la_CXXFLAGS = $(AM_CXXFLAGS) $(PYTHON_CXXFLAGS)
@@ -30,6 +31,7 @@ EXTRA_DIST = client_inc.cc
 EXTRA_DIST += finder_inc.cc
 EXTRA_DIST += iterator_inc.cc
 EXTRA_DIST += updater_inc.cc
+EXTRA_DIST += journal_reader_inc.cc
 
 CLEANDIRS = __pycache__
 

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

@@ -89,7 +89,7 @@ None\n\
 ";
 
 const char* const DataSourceClient_getIterator_doc = "\
-get_iterator(name, adjust_ttl=True) -> ZoneIterator\n\
+get_iterator(name, separate_rrs=False) -> ZoneIterator\n\
 \n\
 Returns an iterator to the given zone.\n\
 \n\
@@ -111,17 +111,18 @@ anything else.\n\
 Parameters:\n\
   isc.dns.Name The name of zone apex to be traversed. It doesn't do\n\
                nearest match as find_zone.\n\
-  adjust_ttl   If True, the iterator will treat RRs with the same\n\
-               name and type but different TTL values to be of the\n\
-               same RRset, and will adjust the TTL to the lowest\n\
-               value found. If false, it will consider the RR to\n\
-               belong to a different RRset.\n\
+  separate_rrs If true, the iterator will return each RR as a\n\
+               new RRset object. If false, the iterator will\n\
+               combine consecutive RRs with the name and type\n\
+               into 1 RRset. The capitalization of the RRset will\n\
+               be that of the first RR read, and TTLs will be\n\
+               adjusted to the lowest one found.\n\
 \n\
 Return Value(s): Pointer to the iterator.\n\
 ";
 
 const char* const DataSourceClient_getUpdater_doc = "\
-get_updater(name, replace) -> ZoneUpdater\n\
+get_updater(name, replace, journaling=False) -> ZoneUpdater\n\
 \n\
 Return an updater to make updates to a specific zone.\n\
 \n\
@@ -162,6 +163,22 @@ A data source can be \"read only\" or can prohibit partial updates. In\n\
 such cases this method will result in an isc.datasrc.NotImplemented exception\n\
 unconditionally or when replace is false).\n\
 \n\
+If journaling is True, the data source should store a journal of\n\
+changes. These can be used later on by, for example, IXFR-out.\n\
+However, the parameter is a hint only. It might be unable to store\n\
+them and they would be silently discarded. Or it might need to store\n\
+them no matter what (for example a git-based data source would store\n\
+journal implicitly). When the journaling is True, it requires that the\n\
+following update be formatted as IXFR transfer (SOA to be removed,\n\
+bunch of RRs to be removed, SOA to be added, bunch of RRs to be added,\n\
+and possibly repeated). However, it is not required that the updater\n\
+checks that. If it is False, it must not require so and must accept\n\
+any order of changes.\n\
+\n\
+We don't support erasing the whole zone (by replace being True) and\n\
+saving a journal at the same time. In such situation, isc.datasrc.Error\n\
+is thrown.\n\
+\n\
 Exceptions:\n\
   isc.datasrc. NotImplemented The underlying data source does not support\n\
                updates.\n\
@@ -170,6 +187,61 @@ Exceptions:\n\
 Parameters:\n\
   name       The zone name to be updated\n\
   replace    Whether to delete existing RRs before making updates\n\
+  journaling The zone updater should store a journal of the changes.\n\
+\n\
+";
+
+// Modifications from C++ doc:
+//   pointer -> (removed)
+//   Null -> None
+//   exception types
+const char* const DataSourceClient_getJournalReader_doc = "\
+get_journal_reader(zone, begin_serial, end_serial) ->\n\
+   (int, ZoneJournalReader)\n\
+\n\
+Return a journal reader to retrieve differences of a zone.\n\
+\n\
+A derived version of this method creates a concrete ZoneJournalReader\n\
+object specific to the underlying data source for the specified name\n\
+of zone and differences between the versions specified by the\n\
+beginning and ending serials of the corresponding SOA RRs. The RR\n\
+class of the zone is the one that the client is expected to handle\n\
+(see the detailed description of this class).\n\
+\n\
+Note that the SOA serials are compared by the semantics of the serial\n\
+number arithmetic. So, for example, begin_serial can be larger than\n\
+end_serial as bare unsigned integers. The underlying data source\n\
+implementation is assumed to keep track of sufficient history to\n\
+identify (if exist) the corresponding difference between the specified\n\
+versions.\n\
+\n\
+This method returns the result as a pair of a result code and a\n\
+ZoneJournalReader object. On success, the result code is\n\
+SUCCESS and the object must not be None; otherwise the result code is\n\
+something other than SUCCESS and the object must be None.\n\
+\n\
+If the specified zone is not found in the data source, the result code\n\
+is NO_SUCH_ZONE. Otherwise, if specified range of difference for the\n\
+zone is not found in the data source, the result code is\n\
+NO_SUCH_VERSION.\n\
+\n\
+Handling differences is an optional feature of data source. If the\n\
+underlying data source does not support difference handling, this\n\
+method for that type of data source can throw an exception of class\n\
+isc.datasrc.NotImplemented.\n\
 \n\
+Exceptions:\n\
+  isc.datasrc.NotImplemented The data source does not support differences.\n\
+  isc.datasrc.Error Other operational errors at the data source level.\n\
+\n\
+Parameters:\n\
+  zone       The name of the zone for which the difference should be\n\
+             retrieved.\n\
+  begin_serial The SOA serial of the beginning version of the\n\
+             differences.\n\
+  end_serial The SOA serial of the ending version of the differences.\n\
+\n\
+Return Value(s): A pair of result code and a ZoneJournalReader object\n\
+(which can be None)\n                                                  \
 ";
 } // unnamed namespace

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

@@ -38,6 +38,7 @@
 #include "finder_python.h"
 #include "iterator_python.h"
 #include "updater_python.h"
+#include "journal_reader_python.h"
 #include "client_inc.cc"
 
 using namespace std;
@@ -84,26 +85,26 @@ PyObject*
 DataSourceClient_getIterator(PyObject* po_self, PyObject* args) {
     s_DataSourceClient* const self = static_cast<s_DataSourceClient*>(po_self);
     PyObject* name_obj;
-    PyObject* adjust_ttl_obj = NULL;
+    PyObject* separate_rrs_obj = NULL;
     if (PyArg_ParseTuple(args, "O!|O", &name_type, &name_obj,
-                         &adjust_ttl_obj)) {
+                         &separate_rrs_obj)) {
         try {
-            bool adjust_ttl = true;
-            if (adjust_ttl_obj != NULL) {
+            bool separate_rrs = false;
+            if (separate_rrs_obj != NULL) {
                 // store result in local var so we can explicitely check for
                 // -1 error return value
-                int adjust_ttl_no = PyObject_Not(adjust_ttl_obj);
-                if (adjust_ttl_no == 1) {
-                    adjust_ttl = false;
-                } else if (adjust_ttl_no == -1) {
+                int separate_rrs_true = PyObject_IsTrue(separate_rrs_obj);
+                if (separate_rrs_true == 1) {
+                    separate_rrs = true;
+                } else if (separate_rrs_true == -1) {
                     PyErr_SetString(getDataSourceException("Error"),
-                                    "Error getting value of adjust_ttl");
+                                    "Error getting value of separate_rrs");
                     return (NULL);
                 }
             }
             return (createZoneIteratorObject(
                 self->cppobj->getInstance().getIterator(PyName_ToName(name_obj),
-                                                        adjust_ttl),
+                                                        separate_rrs),
                 po_self));
         } catch (const isc::NotImplemented& ne) {
             PyErr_SetString(getDataSourceException("NotImplemented"),
@@ -129,14 +130,17 @@ PyObject*
 DataSourceClient_getUpdater(PyObject* po_self, PyObject* args) {
     s_DataSourceClient* const self = static_cast<s_DataSourceClient*>(po_self);
     PyObject *name_obj;
-    PyObject *replace_obj;
-    if (PyArg_ParseTuple(args, "O!O", &name_type, &name_obj, &replace_obj) &&
-        PyBool_Check(replace_obj)) {
-        bool replace = (replace_obj != Py_False);
+    PyObject *replace_obj = NULL;
+    PyObject *journaling_obj = Py_False;
+    if (PyArg_ParseTuple(args, "O!O|O", &name_type, &name_obj,
+                         &replace_obj, &journaling_obj) &&
+        PyBool_Check(replace_obj) && PyBool_Check(journaling_obj)) {
+        const bool replace = (replace_obj != Py_False);
+        const bool journaling = (journaling_obj == Py_True);
         try {
             ZoneUpdaterPtr updater =
                 self->cppobj->getInstance().getUpdater(PyName_ToName(name_obj),
-                                                       replace);
+                                                       replace, journaling);
             if (!updater) {
                 return (Py_None);
             }
@@ -157,10 +161,44 @@ DataSourceClient_getUpdater(PyObject* po_self, PyObject* args) {
             return (NULL);
         }
     } else {
+        // PyBool_Check doesn't set the error, so we have to set it ourselves.
+        if (replace_obj != NULL && !PyBool_Check(replace_obj)) {
+            PyErr_SetString(PyExc_TypeError, "'replace' for "
+                            "DataSourceClient.get_updater must be boolean");
+        }
+        if (!PyBool_Check(journaling_obj)) {
+            PyErr_SetString(PyExc_TypeError, "'journaling' for "
+                            "DataSourceClient.get_updater must be boolean");
+        }
         return (NULL);
     }
 }
 
+PyObject*
+DataSourceClient_getJournalReader(PyObject* po_self, PyObject* args) {
+    s_DataSourceClient* const self = static_cast<s_DataSourceClient*>(po_self);
+    PyObject *name_obj;
+    unsigned long begin_obj, end_obj;
+
+    if (PyArg_ParseTuple(args, "O!kk", &name_type, &name_obj,
+                         &begin_obj, &end_obj)) {
+        pair<ZoneJournalReader::Result, ZoneJournalReaderPtr> result =
+            self->cppobj->getInstance().getJournalReader(
+                PyName_ToName(name_obj), static_cast<uint32_t>(begin_obj),
+                static_cast<uint32_t>(end_obj));
+        PyObject* po_reader;
+        if (result.first == ZoneJournalReader::SUCCESS) {
+            po_reader = createZoneJournalReaderObject(result.second, po_self);
+        } else {
+            po_reader = Py_None;
+            Py_INCREF(po_reader); // this will soon be released
+        }
+        PyObjectContainer container(po_reader);
+        return (Py_BuildValue("(iO)", result.first, container.get()));
+    }
+    return (NULL);
+}
+
 // This list contains the actual set of functions we have in
 // python. Each entry has
 // 1. Python method name
@@ -168,18 +206,21 @@ DataSourceClient_getUpdater(PyObject* po_self, PyObject* args) {
 // 3. Argument type
 // 4. Documentation
 PyMethodDef DataSourceClient_methods[] = {
-    { "find_zone", reinterpret_cast<PyCFunction>(DataSourceClient_findZone),
-      METH_VARARGS, DataSourceClient_findZone_doc },
+    { "find_zone", DataSourceClient_findZone, METH_VARARGS,
+      DataSourceClient_findZone_doc },
     { "get_iterator",
-      reinterpret_cast<PyCFunction>(DataSourceClient_getIterator), METH_VARARGS,
+      DataSourceClient_getIterator, METH_VARARGS,
       DataSourceClient_getIterator_doc },
-    { "get_updater", reinterpret_cast<PyCFunction>(DataSourceClient_getUpdater),
+    { "get_updater", DataSourceClient_getUpdater,
       METH_VARARGS, DataSourceClient_getUpdater_doc },
+    { "get_journal_reader", DataSourceClient_getJournalReader,
+      METH_VARARGS, DataSourceClient_getJournalReader_doc },
     { NULL, NULL, 0, NULL }
 };
 
 int
-DataSourceClient_init(s_DataSourceClient* self, PyObject* args) {
+DataSourceClient_init(PyObject* po_self, PyObject* args, PyObject*) {
+    s_DataSourceClient* self = static_cast<s_DataSourceClient*>(po_self);
     char* ds_type_str;
     char* ds_config_str;
     try {
@@ -224,7 +265,8 @@ DataSourceClient_init(s_DataSourceClient* self, PyObject* args) {
 }
 
 void
-DataSourceClient_destroy(s_DataSourceClient* const self) {
+DataSourceClient_destroy(PyObject* po_self) {
+    s_DataSourceClient* const self = static_cast<s_DataSourceClient*>(po_self);
     delete self->cppobj;
     self->cppobj = NULL;
     Py_TYPE(self)->tp_free(self);
@@ -243,7 +285,7 @@ PyTypeObject datasourceclient_type = {
     "datasrc.DataSourceClient",
     sizeof(s_DataSourceClient),         // tp_basicsize
     0,                                  // tp_itemsize
-    reinterpret_cast<destructor>(DataSourceClient_destroy),// tp_dealloc
+    DataSourceClient_destroy,           // tp_dealloc
     NULL,                               // tp_print
     NULL,                               // tp_getattr
     NULL,                               // tp_setattr
@@ -274,7 +316,7 @@ PyTypeObject datasourceclient_type = {
     NULL,                               // tp_descr_get
     NULL,                               // tp_descr_set
     0,                                  // tp_dictoffset
-    reinterpret_cast<initproc>(DataSourceClient_init),// tp_init
+    DataSourceClient_init,              // tp_init
     NULL,                               // tp_alloc
     PyType_GenericNew,                  // tp_new
     NULL,                               // tp_free

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

@@ -27,6 +27,7 @@
 #include "finder_python.h"
 #include "iterator_python.h"
 #include "updater_python.h"
+#include "journal_reader_python.h"
 
 #include <util/python/pycppwrapper_util.h>
 #include <dns/python/pydnspp_common.h>
@@ -192,6 +193,41 @@ initModulePart_ZoneUpdater(PyObject* mod) {
     return (true);
 }
 
+bool
+initModulePart_ZoneJournalReader(PyObject* mod) {
+    if (PyType_Ready(&journal_reader_type) < 0) {
+        return (false);
+    }
+    void* p = &journal_reader_type;
+    if (PyModule_AddObject(mod, "ZoneJournalReader",
+                           static_cast<PyObject*>(p)) < 0) {
+        return (false);
+    }
+    Py_INCREF(&journal_reader_type);
+
+    try {
+        installClassVariable(journal_reader_type, "SUCCESS",
+                             Py_BuildValue("I", ZoneJournalReader::SUCCESS));
+        installClassVariable(journal_reader_type, "NO_SUCH_ZONE",
+                             Py_BuildValue("I",
+                                           ZoneJournalReader::NO_SUCH_ZONE));
+        installClassVariable(journal_reader_type, "NO_SUCH_VERSION",
+                             Py_BuildValue("I",
+                                           ZoneJournalReader::NO_SUCH_VERSION));
+    } catch (const std::exception& ex) {
+        const std::string ex_what =
+            "Unexpected failure in ZoneJournalReader initialization: " +
+            std::string(ex.what());
+        PyErr_SetString(po_IscException, ex_what.c_str());
+        return (false);
+    } catch (...) {
+        PyErr_SetString(PyExc_SystemError,
+            "Unexpected failure in ZoneJournalReader initialization");
+        return (false);
+    }
+
+    return (true);
+}
 
 PyObject* po_DataSourceError;
 PyObject* po_NotImplemented;
@@ -239,6 +275,11 @@ PyInit_datasrc(void) {
         return (NULL);
     }
 
+    if (!initModulePart_ZoneJournalReader(mod)) {
+        Py_DECREF(mod);
+        return (NULL);
+    }
+
     try {
         po_DataSourceError = PyErr_NewException("isc.datasrc.Error", NULL,
                                                 NULL);

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

@@ -0,0 +1,80 @@
+namespace {
+const char* const ZoneJournalReader_doc = "\
+The base class for retrieving differences between two versions of a\n\
+zone.\n\
+\n\
+On construction, each derived class object will internally set up\n\
+retrieving sequences of differences between two specific version of a\n\
+specific zone managed in a particular data source. So the constructor\n\
+of a derived class would normally take parameters to identify the zone\n\
+and the two versions for which the differences should be retrieved.\n\
+See DataSourceClient.get_journal_reader for more concrete details used\n\
+in this API.\n\
+\n\
+Once constructed, an object of this class will act like an iterator\n\
+over the sequences. Every time the get_next_diff() method is called it\n\
+returns one element of the differences in the form of an RRset until\n\
+it reaches the end of the entire sequences.\n\
+\n\
+";
+
+// Modifications from C++ doc:
+//   ConstRRsetPtr -> RRset
+//   Null -> None
+//   InvalidOperation -> ValueError
+const char* const ZoneJournalReader_getNextDiff_doc = "\
+get_next_diff() -> isc.dns.RRset\n\
+\n\
+Return the next difference RR of difference sequences.\n\
+\n\
+In this API, the difference between two versions of a zone is\n\
+conceptually represented as IXFR-style difference sequences: Each\n\
+difference sequence is a sequence of RRs: an older version of SOA (to\n\
+be deleted), zero or more other deleted RRs, the post-transaction SOA\n\
+(to be added), and zero or more other added RRs. (Note, however, that\n\
+the underlying data source implementation may or may not represent the\n\
+difference in straightforward realization of this concept. The mapping\n\
+between the conceptual difference and the actual implementation is\n\
+hidden in each derived class).\n\
+\n\
+This method provides an application with a higher level interface to\n\
+retrieve the difference along with the conceptual model: the\n\
+ZoneJournalReader object iterates over the entire sequences from the\n\
+beginning SOA (which is to be deleted) to one of the added RR of with\n\
+the ending SOA, and each call to this method returns one RR in the\n\
+form of an RRset that contains exactly one RDATA in the order of the\n\
+sequences.\n\
+\n\
+Note that the ordering of the sequences specifies the semantics of\n\
+each difference: add or delete. For example, the first RR is to be\n\
+deleted, and the last RR is to be added. So the return value of this\n\
+method does not explicitly indicate whether the RR is to be added or\n\
+deleted.\n\
+\n\
+This method ensures the returned RRset represents an RR, that is, it\n\
+contains exactly one RDATA. However, it does not necessarily ensure\n\
+that the resulting sequences are in the form of IXFR-style. For\n\
+example, the first RR is supposed to be an SOA, and it should normally\n\
+be the case, but this interface does not necessarily require the\n\
+derived class implementation ensure this. Normally the differences are\n\
+expected to be stored using this API (via a ZoneUpdater object), and\n\
+as long as that is the case and the underlying implementation follows\n\
+the requirement of the API, the result of this method should be a\n\
+valid IXFR-style sequences. So this API does not mandate the almost\n\
+redundant check as part of the interface. If the application needs to\n\
+make it sure 100%, it must check the resulting sequence itself.\n\
+\n\
+Once the object reaches the end of the sequences, this method returns\n\
+None. Any subsequent call will result in an exception of class\n\
+ValueError.\n\
+\n\
+Exceptions:\n\
+  ValueError The method is called beyond the end of the\n\
+             difference sequences.\n\
+  isc.datasrc.Error Underlying data is broken and the RR cannot be\n\
+             created or other low level data source error.\n\
+\n\
+Return Value(s): An RRset that contains one RDATA corresponding to the\n\
+next difference in the sequences.\n\
+";
+} // unnamed namespace

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

@@ -0,0 +1,200 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// Python.h needs to be placed at the head of the program file, see:
+// http://docs.python.org/py3k/extending/extending.html#a-simple-example
+#include <Python.h>
+
+#include <util/python/pycppwrapper_util.h>
+
+#include <datasrc/client.h>
+#include <datasrc/database.h>
+
+#include <dns/python/rrset_python.h>
+
+#include "datasrc.h"
+#include "journal_reader_python.h"
+
+#include "journal_reader_inc.cc"
+
+using namespace isc::util::python;
+using namespace isc::dns::python;
+using namespace isc::datasrc;
+using namespace isc::datasrc::python;
+
+namespace {
+// The s_* Class simply covers one instantiation of the object
+class s_ZoneJournalReader : public PyObject {
+public:
+    s_ZoneJournalReader() : cppobj(ZoneJournalReaderPtr()), base_obj(NULL) {};
+    ZoneJournalReaderPtr cppobj;
+    // This is a reference to a base object; if the object of this class
+    // depends on another object to be in scope during its lifetime,
+    // we use INCREF the base object upon creation, and DECREF it at
+    // the end of the destructor
+    // This is an optional argument to createXXX(). If NULL, it is ignored.
+    PyObject* base_obj;
+};
+
+// General creation and destruction
+int
+ZoneJournalReader_init(PyObject*, PyObject*, PyObject*) {
+    // can't be called directly
+    PyErr_SetString(PyExc_TypeError,
+                    "ZoneJournalReader cannot be constructed directly");
+
+    return (-1);
+}
+
+void
+ZoneJournalReader_destroy(PyObject* po_self) {
+    s_ZoneJournalReader* const self =
+        static_cast<s_ZoneJournalReader*>(po_self) ;
+    // cppobj is a shared ptr, but to make sure things are not destroyed in
+    // the wrong order, we reset it here.
+    self->cppobj.reset();
+    if (self->base_obj != NULL) {
+        Py_DECREF(self->base_obj);
+    }
+    Py_TYPE(self)->tp_free(self);
+}
+
+//
+// We declare the functions here, the definitions are below
+// the type definition of the object, since both can use the other
+//
+PyObject*
+ZoneJournalReader_getNextDiff(PyObject* po_self, PyObject*) {
+    s_ZoneJournalReader* self = static_cast<s_ZoneJournalReader*>(po_self);
+    try {
+        isc::dns::ConstRRsetPtr rrset = self->cppobj->getNextDiff();
+        if (!rrset) {
+            Py_RETURN_NONE;
+        }
+        return (createRRsetObject(*rrset));
+    } catch (const isc::InvalidOperation& ex) {
+        PyErr_SetString(PyExc_ValueError, ex.what());
+        return (NULL);
+    } catch (const isc::Exception& isce) {
+        PyErr_SetString(getDataSourceException("Error"), isce.what());
+        return (NULL);
+    } catch (const std::exception& exc) {
+        PyErr_SetString(getDataSourceException("Error"), exc.what());
+        return (NULL);
+    } catch (...) {
+        PyErr_SetString(getDataSourceException("Error"),
+                        "Unexpected exception");
+        return (NULL);
+    }
+}
+
+PyObject*
+ZoneJournalReader_iter(PyObject *self) {
+    Py_INCREF(self);
+    return (self);
+}
+
+PyObject*
+ZoneJournalReader_next(PyObject* self) {
+    PyObject* result = ZoneJournalReader_getNextDiff(self, NULL);
+    // iter_next must return NULL without error instead of Py_None
+    if (result == Py_None) {
+        Py_DECREF(result);
+        return (NULL);
+    } else {
+        return (result);
+    }
+}
+
+PyMethodDef ZoneJournalReader_methods[] = {
+    { "get_next_diff", ZoneJournalReader_getNextDiff, METH_NOARGS,
+      ZoneJournalReader_getNextDiff_doc },
+    { NULL, NULL, 0, NULL }
+};
+
+
+} // end of unnamed namespace
+
+namespace isc {
+namespace datasrc {
+namespace python {
+PyTypeObject journal_reader_type = {
+    PyVarObject_HEAD_INIT(NULL, 0)
+    "datasrc.ZoneJournalReader",
+    sizeof(s_ZoneJournalReader),             // tp_basicsize
+    0,                                  // tp_itemsize
+    ZoneJournalReader_destroy,          // tp_dealloc
+    NULL,                               // tp_print
+    NULL,                               // tp_getattr
+    NULL,                               // tp_setattr
+    NULL,                               // tp_reserved
+    NULL,                               // tp_repr
+    NULL,                               // tp_as_number
+    NULL,                               // tp_as_sequence
+    NULL,                               // tp_as_mapping
+    NULL,                               // tp_hash
+    NULL,                               // tp_call
+    NULL,                               // tp_str
+    NULL,                               // tp_getattro
+    NULL,                               // tp_setattro
+    NULL,                               // tp_as_buffer
+    Py_TPFLAGS_DEFAULT,                 // tp_flags
+    ZoneJournalReader_doc,
+    NULL,                               // tp_traverse
+    NULL,                               // tp_clear
+    NULL,                               // tp_richcompare
+    0,                                  // tp_weaklistoffset
+    ZoneJournalReader_iter,                  // tp_iter
+    ZoneJournalReader_next,                  // tp_iternext
+    ZoneJournalReader_methods,               // tp_methods
+    NULL,                               // tp_members
+    NULL,                               // tp_getset
+    NULL,                               // tp_base
+    NULL,                               // tp_dict
+    NULL,                               // tp_descr_get
+    NULL,                               // tp_descr_set
+    0,                                  // tp_dictoffset
+    ZoneJournalReader_init,             // tp_init
+    NULL,                               // tp_alloc
+    PyType_GenericNew,                  // tp_new
+    NULL,                               // tp_free
+    NULL,                               // tp_is_gc
+    NULL,                               // tp_bases
+    NULL,                               // tp_mro
+    NULL,                               // tp_cache
+    NULL,                               // tp_subclasses
+    NULL,                               // tp_weaklist
+    NULL,                               // tp_del
+    0                                   // tp_version_tag
+};
+
+PyObject*
+createZoneJournalReaderObject(ZoneJournalReaderPtr source,
+                              PyObject* base_obj)
+{
+    s_ZoneJournalReader* po = static_cast<s_ZoneJournalReader*>(
+        journal_reader_type.tp_alloc(&journal_reader_type, 0));
+    if (po != NULL) {
+        po->cppobj = source;
+        po->base_obj = base_obj;
+        if (base_obj != NULL) {
+            Py_INCREF(base_obj);
+        }
+    }
+    return (po);
+}
+
+} // namespace python
+} // namespace datasrc
+} // namespace isc

src/lib/python/isc/datasrc/journal_reader_python.h

@@ -0,0 +1,47 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __PYTHON_DATASRC_JOURNAL_READER_H
+#define __PYTHON_DATASRC_JOURNAL_READER_H 1
+
+#include <Python.h>
+
+#include <datasrc/zone.h>
+
+namespace isc {
+namespace datasrc {
+namespace python {
+
+extern PyTypeObject journal_reader_type;
+
+/// \brief Create a ZoneJournalReader python object
+///
+/// \param source The zone journal reader pointer to wrap
+/// \param base_obj An optional PyObject that this ZoneJournalReader depends on
+///                 Its refcount is increased, and will be decreased when
+///                 this reader is destroyed, making sure that the
+///                 base object is never destroyed before this reader.
+PyObject* createZoneJournalReaderObject(
+    isc::datasrc::ZoneJournalReaderPtr source,
+    PyObject* base_obj = NULL);
+
+
+} // namespace python
+} // namespace datasrc
+} // namespace isc
+#endif // __PYTHON_DATASRC_JOURNAL_READER_H
+
+// Local Variables:
+// mode: c++
+// End:

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

@@ -15,9 +15,10 @@
 
 import isc.log
 import isc.datasrc
-from isc.datasrc import ZoneFinder
-import isc.dns
+from isc.datasrc import ZoneFinder, ZoneJournalReader
+from isc.dns import *
 import unittest
+import sqlite3
 import os
 import shutil
 import sys
@@ -61,6 +62,13 @@ def check_for_rrset(expected_rrsets, rrset):
             return True
     return False
 
+def create_soa(serial):
+    soa = RRset(Name('example.org'), RRClass.IN(), RRType.SOA(), RRTTL(3600))
+    soa.add_rdata(Rdata(RRType.SOA(), RRClass.IN(),
+                        'ns1.example.org. admin.example.org. ' +
+                        str(serial) + ' 3600 1800 2419200 7200'))
+    return soa
+
 class DataSrcClient(unittest.TestCase):
 
     def test_(self):
@@ -82,13 +90,12 @@ class DataSrcClient(unittest.TestCase):
                           isc.datasrc.DataSourceClient, "memory",
                           "{ \"foo\": 1 }")
 
-    @unittest.skip("This test may fail depending on sqlite3 library behavior")
     def test_iterate(self):
         dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
 
         # for RRSIGS, the TTL's are currently modified. This test should
         # start failing when we fix that.
-        rrs = dsc.get_iterator(isc.dns.Name("sql1.example.com."), False)
+        rrs = dsc.get_iterator(isc.dns.Name("sql1.example.com."), True)
 
         # we do not know the order in which they are returned by the iterator
         # but we do want to check them, so we put all records into one list
@@ -115,7 +122,11 @@ class DataSrcClient(unittest.TestCase):
                      "256 3 5 AwEAAdYdRhBAEY67R/8G1N5AjGF6asIiNh/pNGeQ8xDQP13J"+
                      "N2lo+sNqWcmpYNhuVqRbLB+mamsU1XcCICSBvAlSmfz/ZUdafX23knAr"+
                      "TlALxMmspcfdpqun3Yr3YYnztuj06rV7RqmveYckWvAUXVYMSMQZfJ30"+
-                     "5fs0dE/xLztL/CzZ",
+                     "5fs0dE/xLztL/CzZ"
+                  ])
+        add_rrset(expected_rrset_list, name, rrclass,
+                  isc.dns.RRType.DNSKEY(), isc.dns.RRTTL(3600),
+                  [
                      "257 3 5 AwEAAbaKDSa9XEFTsjSYpUTHRotTS9Tz3krfDucugW5UokGQ"+
                      "KC26QlyHXlPTZkC+aRFUs/dicJX2kopndLcnlNAPWiKnKtrsFSCnIJDB"+
                      "ZIyvcKq+9RXmV3HK3bUdHnQZ88IZWBRmWKfZ6wnzHo53kdYKAemTErkz"+
@@ -127,8 +138,16 @@ class DataSrcClient(unittest.TestCase):
         add_rrset(expected_rrset_list, name, rrclass,
                   isc.dns.RRType.NS(), isc.dns.RRTTL(3600),
                   [
-                    "dns01.example.com.",
-                    "dns02.example.com.",
+                    "dns01.example.com."
+                  ])
+        add_rrset(expected_rrset_list, name, rrclass,
+                  isc.dns.RRType.NS(), isc.dns.RRTTL(3600),
+                  [
+                    "dns02.example.com."
+                  ])
+        add_rrset(expected_rrset_list, name, rrclass,
+                  isc.dns.RRType.NS(), isc.dns.RRTTL(3600),
+                  [
                     "dns03.example.com."
                   ])
         add_rrset(expected_rrset_list, name, rrclass,
@@ -139,15 +158,19 @@ class DataSrcClient(unittest.TestCase):
         # For RRSIGS, we can't add the fake data through the API, so we
         # simply pass no rdata at all (which is skipped by the check later)
         
-        # Since we passed adjust_ttl = False to get_iterator, we get several
+        # Since we passed separate_rrs = True to get_iterator, we get several
         # sets of RRSIGs, one for each TTL
         add_rrset(expected_rrset_list, name, rrclass,
                   isc.dns.RRType.RRSIG(), isc.dns.RRTTL(3600), None)
         add_rrset(expected_rrset_list, name, rrclass,
-                  isc.dns.RRType.RRSIG(), isc.dns.RRTTL(7200), None)
+                  isc.dns.RRType.RRSIG(), isc.dns.RRTTL(3600), None)
+        add_rrset(expected_rrset_list, name, rrclass,
+                  isc.dns.RRType.RRSIG(), isc.dns.RRTTL(3600), None)
         add_rrset(expected_rrset_list, name, rrclass,
                   isc.dns.RRType.RRSIG(), isc.dns.RRTTL(3600), None)
         add_rrset(expected_rrset_list, name, rrclass,
+                  isc.dns.RRType.RRSIG(), isc.dns.RRTTL(7200), None)
+        add_rrset(expected_rrset_list, name, rrclass,
                   isc.dns.RRType.SOA(), isc.dns.RRTTL(3600),
                   [
                      "master.example.com. admin.example.com. 678 3600 1800 2419200 7200"
@@ -191,26 +214,26 @@ class DataSrcClient(unittest.TestCase):
         # instead of failing?
         self.assertRaises(isc.datasrc.Error, rrs.get_next_rrset)
 
-        # Without the adjust_ttl argument, it should return 55 RRsets
+        # Without the separate_rrs argument, it should return 55 RRsets
         dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
         rrets = dsc.get_iterator(isc.dns.Name("example.com"))
         # there are more than 80 RRs in this zone... let's just count them
         # (already did a full check of the smaller zone above)
         self.assertEqual(55, len(list(rrets)))
 
-        # same test, but now with explicit True argument for adjust_ttl
+        # same test, but now with explicit False argument for separate_rrs
         dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
-        rrets = dsc.get_iterator(isc.dns.Name("example.com"), True)
+        rrets = dsc.get_iterator(isc.dns.Name("example.com"), False)
         # there are more than 80 RRs in this zone... let's just count them
         # (already did a full check of the smaller zone above)
         self.assertEqual(55, len(list(rrets)))
 
         # Count should be 71 if we request individual rrsets for differing ttls
         dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
-        rrets = dsc.get_iterator(isc.dns.Name("example.com"), False)
+        rrets = dsc.get_iterator(isc.dns.Name("example.com"), True)
         # there are more than 80 RRs in this zone... let's just count them
         # (already did a full check of the smaller zone above)
-        self.assertEqual(71, len(list(rrets)))
+        self.assertEqual(84, len(list(rrets)))
         # TODO should we catch this (iterating past end) and just return None
         # instead of failing?
         self.assertRaises(isc.datasrc.Error, rrs.get_next_rrset)
@@ -565,6 +588,221 @@ class DataSrcUpdater(unittest.TestCase):
         self.assertEqual(None, iterator.get_soa())
         self.assertEqual(None, iterator.get_next_rrset())
 
+class JournalWrite(unittest.TestCase):
+    def setUp(self):
+        # Make a fresh copy of the writable database with all original content
+        shutil.copyfile(READ_ZONE_DB_FILE, WRITE_ZONE_DB_FILE)
+        self.dsc = isc.datasrc.DataSourceClient("sqlite3",
+                                                WRITE_ZONE_DB_CONFIG)
+        self.updater = self.dsc.get_updater(Name("example.com"), False, True)
+
+    def tearDown(self):
+        self.dsc = None
+        self.updater = None
+
+    def check_journal(self, expected_list):
+        # This assumes sqlite3 DB and directly fetches stored data from
+        # the DB file.  It should be generalized using ZoneJournalReader
+        # once it's supported.
+        conn = sqlite3.connect(WRITE_ZONE_DB_FILE)
+        cur = conn.cursor()
+        cur.execute('SELECT name, rrtype, ttl, rdata FROM diffs ORDER BY id')
+        actual_list = cur.fetchall()
+        self.assertEqual(len(expected_list), len(actual_list))
+        for (expected, actual) in zip(expected_list, actual_list):
+            self.assertEqual(expected, actual)
+        conn.close()
+
+    def create_a(self, address):
+        a_rr = RRset(Name('www.example.org'), RRClass.IN(), RRType.A(),
+                     RRTTL(3600))
+        a_rr.add_rdata(Rdata(RRType.A(), RRClass.IN(), address))
+        return (a_rr)
+
+    def test_journal_write(self):
+        # This is a straightforward port of the C++ 'journal' test
+        # Note: we add/delete 'out of zone' data (example.org in the
+        # example.com zone for convenience.
+        self.updater.delete_rrset(create_soa(1234))
+        self.updater.delete_rrset(self.create_a('192.0.2.2'))
+        self.updater.add_rrset(create_soa(1235))
+        self.updater.add_rrset(self.create_a('192.0.2.2'))
+        self.updater.commit()
+
+        expected = []
+        expected.append(("example.org.", "SOA", 3600,
+                         "ns1.example.org. admin.example.org. " +
+                         "1234 3600 1800 2419200 7200"))
+        expected.append(("www.example.org.", "A", 3600, "192.0.2.2"))
+        expected.append(("example.org.", "SOA", 3600,
+                         "ns1.example.org. admin.example.org. " +
+                         "1235 3600 1800 2419200 7200"))
+        expected.append(("www.example.org.", "A", 3600, "192.0.2.2"))
+        self.check_journal(expected)
+
+    def test_journal_write_multiple(self):
+        # This is a straightforward port of the C++ 'journalMultiple' test
+        expected = []
+        for i in range(1, 100):
+            self.updater.delete_rrset(create_soa(1234 + i - 1))
+            expected.append(("example.org.", "SOA", 3600,
+                             "ns1.example.org. admin.example.org. " +
+                             str(1234 + i - 1) + " 3600 1800 2419200 7200"))
+            self.updater.add_rrset(create_soa(1234 + i))
+            expected.append(("example.org.", "SOA", 3600,
+                             "ns1.example.org. admin.example.org. " +
+                             str(1234 + i) + " 3600 1800 2419200 7200"))
+        self.updater.commit()
+        self.check_journal(expected)
+
+    def test_journal_write_bad_sequence(self):
+        # This is a straightforward port of the C++ 'journalBadSequence' test
+
+        # Delete A before SOA
+        self.assertRaises(isc.datasrc.Error, self.updater.delete_rrset,
+                          self.create_a('192.0.2.1'))
+        # Add before delete
+        self.updater = self.dsc.get_updater(Name("example.com"), False, True)
+        self.assertRaises(isc.datasrc.Error, self.updater.add_rrset,
+                          create_soa(1234))
+        # Add A before SOA
+        self.updater = self.dsc.get_updater(Name("example.com"), False, True)
+        self.updater.delete_rrset(create_soa(1234))
+        self.assertRaises(isc.datasrc.Error, self.updater.add_rrset,
+                          self.create_a('192.0.2.1'))
+        # Commit before add
+        self.updater = self.dsc.get_updater(Name("example.com"), False, True)
+        self.updater.delete_rrset(create_soa(1234))
+        self.assertRaises(isc.datasrc.Error, self.updater.commit)
+        # Delete two SOAs
+        self.updater = self.dsc.get_updater(Name("example.com"), False, True)
+        self.updater.delete_rrset(create_soa(1234))
+        self.assertRaises(isc.datasrc.Error, self.updater.delete_rrset,
+                          create_soa(1235))
+        # Add two SOAs
+        self.updater = self.dsc.get_updater(Name("example.com"), False, True)
+        self.updater.delete_rrset(create_soa(1234))
+        self.updater.add_rrset(create_soa(1235))
+        self.assertRaises(isc.datasrc.Error, self.updater.add_rrset,
+                          create_soa(1236))
+
+    def test_journal_write_onerase(self):
+        self.updater = None
+        self.assertRaises(isc.datasrc.Error, self.dsc.get_updater,
+                          Name("example.com"), True, True)
+
+    def test_journal_write_badparam(self):
+        dsc = isc.datasrc.DataSourceClient("sqlite3", WRITE_ZONE_DB_CONFIG)
+        self.assertRaises(TypeError, dsc.get_updater, 0, False, True)
+        self.assertRaises(TypeError, dsc.get_updater, Name('example.com'),
+                          False, 0)
+        self.assertRaises(TypeError, dsc.get_updater, Name("example.com"),
+                          1, True)
+
+class JournalRead(unittest.TestCase):
+    def setUp(self):
+        # Make a fresh copy of the writable database with all original content
+        self.zname = Name('example.com')
+        shutil.copyfile(READ_ZONE_DB_FILE, WRITE_ZONE_DB_FILE)
+        self.dsc = isc.datasrc.DataSourceClient("sqlite3",
+                                                WRITE_ZONE_DB_CONFIG)
+        self.reader = None
+
+    def tearDown(self):
+        # Some tests leave the reader in the middle of sequence, holding
+        # the lock.  Since the unittest framework keeps each test object
+        # until the end of the entire tests, we need to make sure the reader
+        # is released at the end of each test.  The client shouldn't do harm
+        # but we clean it up, too, just in case.
+        self.dsc = None
+        self.reader = None
+
+    def make_simple_diff(self, begin_soa):
+        updater = self.dsc.get_updater(self.zname, False, True)
+        updater.delete_rrset(begin_soa)
+        updater.add_rrset(create_soa(1235))
+        updater.commit()
+
+    def test_journal_reader(self):
+        # This is a straightforward port of the C++ 'journalReader' test
+        self.make_simple_diff(create_soa(1234))
+        result, self.reader = self.dsc.get_journal_reader(self.zname, 1234,
+                                                          1235)
+        self.assertEqual(ZoneJournalReader.SUCCESS, result)
+        self.assertNotEqual(None, self.reader)
+        rrsets_equal(create_soa(1234), self.reader.get_next_diff())
+        rrsets_equal(create_soa(1235), self.reader.get_next_diff())
+        self.assertEqual(None, self.reader.get_next_diff())
+        self.assertRaises(ValueError, self.reader.get_next_diff)
+
+    def test_journal_reader_with_large_serial(self):
+        # similar to the previous one, but use a very large serial to check
+        # if the python wrapper code has unexpected integer overflow
+        self.make_simple_diff(create_soa(4294967295))
+        result, self.reader = self.dsc.get_journal_reader(self.zname,
+                                                          4294967295, 1235)
+        self.assertNotEqual(None, self.reader)
+        # dump to text and compare them in case create_soa happens to have
+        # an overflow bug
+        self.assertEqual('example.org. 3600 IN SOA ns1.example.org. ' + \
+                         'admin.example.org. 4294967295 3600 1800 ' + \
+                             '2419200 7200\n',
+                         self.reader.get_next_diff().to_text())
+
+    def test_journal_reader_large_journal(self):
+        # This is a straightforward port of the C++ 'readLargeJournal' test.
+        # In this test we use the ZoneJournalReader object as a Python
+        # iterator.
+        updater = self.dsc.get_updater(self.zname, False, True)
+        expected = []
+        for i in range(0, 100):
+            rrset = create_soa(1234 + i)
+            updater.delete_rrset(rrset)
+            expected.append(rrset)
+
+            rrset = create_soa(1234 + i + 1)
+            updater.add_rrset(rrset)
+            expected.append(rrset)
+
+        updater.commit()
+        _, self.reader = self.dsc.get_journal_reader(self.zname, 1234, 1334)
+        self.assertNotEqual(None, self.reader)
+        i = 0
+        for rr in self.reader:
+            self.assertNotEqual(len(expected), i)
+            rrsets_equal(expected[i], rr)
+            i += 1
+        self.assertEqual(len(expected), i)
+
+    def test_journal_reader_no_range(self):
+        # This is a straightforward port of the C++ 'readJournalForNoRange'
+        # test
+        self.make_simple_diff(create_soa(1234))
+        result, self.reader = self.dsc.get_journal_reader(self.zname, 1200,
+                                                          1235)
+        self.assertEqual(ZoneJournalReader.NO_SUCH_VERSION, result)
+        self.assertEqual(None, self.reader)
+
+    def test_journal_reader_no_zone(self):
+        # This is a straightforward port of the C++ 'journalReaderForNXZone'
+        # test
+        result, self.reader = self.dsc.get_journal_reader(Name('nosuchzone'),
+                                                          0, 1)
+        self.assertEqual(ZoneJournalReader.NO_SUCH_ZONE, result)
+        self.assertEqual(None, self.reader)
+
+    def test_journal_reader_bad_params(self):
+        self.assertRaises(TypeError, self.dsc.get_journal_reader,
+                          'example.com.', 0, 1)
+        self.assertRaises(TypeError, self.dsc.get_journal_reader,
+                          self.zname, 'must be int', 1)
+        self.assertRaises(TypeError, self.dsc.get_journal_reader,
+                          self.zname, 0, 'must be int')
+
+    def test_journal_reader_direct_construct(self):
+        # ZoneJournalReader can only be constructed via a factory
+        self.assertRaises(TypeError, ZoneJournalReader)
+
 if __name__ == "__main__":
     isc.log.init("bind10")
     isc.log.resetUnitTestRootLogger()

src/lib/python/isc/log/log.cc

@@ -303,7 +303,8 @@ public:
 extern PyTypeObject logger_type;
 
 int
-Logger_init(LoggerWrapper* self, PyObject* args) {
+Logger_init(PyObject* po_self, PyObject* args, PyObject*) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     const char* name;
     if (!PyArg_ParseTuple(args, "s", &name)) {
         return (-1);
@@ -323,7 +324,9 @@ Logger_init(LoggerWrapper* self, PyObject* args) {
 }
 
 void
-Logger_destroy(LoggerWrapper* const self) {
+//Logger_destroy(LoggerWrapper* const self) {
+Logger_destroy(PyObject* po_self) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     delete self->logger_;
     self->logger_ = NULL;
     Py_TYPE(self)->tp_free(self);
@@ -351,7 +354,8 @@ severityToText(const Severity& severity) {
 }
 
 PyObject*
-Logger_getEffectiveSeverity(LoggerWrapper* self, PyObject*) {
+Logger_getEffectiveSeverity(PyObject* po_self, PyObject*) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     try {
         return (Py_BuildValue("s",
                               severityToText(
@@ -368,7 +372,8 @@ Logger_getEffectiveSeverity(LoggerWrapper* self, PyObject*) {
 }
 
 PyObject*
-Logger_getEffectiveDebugLevel(LoggerWrapper* self, PyObject*) {
+Logger_getEffectiveDebugLevel(PyObject* po_self, PyObject*) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     try {
         return (Py_BuildValue("i", self->logger_->getEffectiveDebugLevel()));
     }
@@ -383,7 +388,8 @@ Logger_getEffectiveDebugLevel(LoggerWrapper* self, PyObject*) {
 }
 
 PyObject*
-Logger_setSeverity(LoggerWrapper* self, PyObject* args) {
+Logger_setSeverity(PyObject* po_self, PyObject* args) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     const char* severity;
     int dbgLevel = 0;
     if (!PyArg_ParseTuple(args, "z|i", &severity, &dbgLevel)) {
@@ -425,27 +431,32 @@ Logger_isLevelEnabled(LoggerWrapper* self, FPtr function) {
 }
 
 PyObject*
-Logger_isInfoEnabled(LoggerWrapper* self, PyObject*) {
+Logger_isInfoEnabled(PyObject* po_self, PyObject*) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     return (Logger_isLevelEnabled(self, &Logger::isInfoEnabled));
 }
 
 PyObject*
-Logger_isWarnEnabled(LoggerWrapper* self, PyObject*) {
+Logger_isWarnEnabled(PyObject* po_self, PyObject*) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     return (Logger_isLevelEnabled(self, &Logger::isWarnEnabled));
 }
 
 PyObject*
-Logger_isErrorEnabled(LoggerWrapper* self, PyObject*) {
+Logger_isErrorEnabled(PyObject* po_self, PyObject*) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     return (Logger_isLevelEnabled(self, &Logger::isErrorEnabled));
 }
 
 PyObject*
-Logger_isFatalEnabled(LoggerWrapper* self, PyObject*) {
+Logger_isFatalEnabled(PyObject* po_self, PyObject*) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     return (Logger_isLevelEnabled(self, &Logger::isFatalEnabled));
 }
 
 PyObject*
-Logger_isDebugEnabled(LoggerWrapper* self, PyObject* args) {
+Logger_isDebugEnabled(PyObject* po_self, PyObject* args) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     int level = MIN_DEBUG_LEVEL;
     if (!PyArg_ParseTuple(args, "|i", &level)) {
         return (NULL);
@@ -470,53 +481,39 @@ Logger_isDebugEnabled(LoggerWrapper* self, PyObject* args) {
 
 string
 objectToStr(PyObject* object, bool convert) {
-    PyObject* cleanup(NULL);
+    PyObjectContainer objstr_container;
     if (convert) {
-        object = cleanup = PyObject_Str(object);
-        if (object == NULL) {
+        PyObject* text_obj = PyObject_Str(object);
+        if (text_obj == NULL) {
+            // PyObject_Str could fail for various reasons, including because
+            // the object cannot be converted to a string.  We exit with
+            // InternalError to preserve the PyErr set in PyObject_Str.
             throw InternalError();
         }
-    }
-    const char* value;
-    PyObject* tuple(Py_BuildValue("(O)", object));
-    if (tuple == NULL) {
-        if (cleanup != NULL) {
-            Py_DECREF(cleanup);
-        }
-        throw InternalError();
+        objstr_container.reset(text_obj);
+        object = objstr_container.get();
     }
 
-    if (!PyArg_ParseTuple(tuple, "s", &value)) {
-        Py_DECREF(tuple);
-        if (cleanup != NULL) {
-            Py_DECREF(cleanup);
-        }
+    PyObjectContainer tuple_container(Py_BuildValue("(O)", object));
+    const char* value;
+    if (!PyArg_ParseTuple(tuple_container.get(), "s", &value)) {
         throw InternalError();
     }
-    string result(value);
-    Py_DECREF(tuple);
-    if (cleanup != NULL) {
-        Py_DECREF(cleanup);
-    }
-    return (result);
+    return (string(value));
 }
 
 // Generic function to output the logging message. Called by the real functions.
-template<class Function>
+template <class Function>
 PyObject*
 Logger_performOutput(Function function, PyObject* args, bool dbgLevel) {
     try {
-        Py_ssize_t number(PyObject_Length(args));
+        const Py_ssize_t number(PyObject_Length(args));
         if (number < 0) {
             return (NULL);
         }
 
         // Which argument is the first to format?
-        size_t start(1);
-        if (dbgLevel) {
-            start ++;
-        }
-
+        const size_t start = dbgLevel ? 2 : 1;
         if (number < start) {
             return (PyErr_Format(PyExc_TypeError, "Too few arguments to "
                                  "logging call, at least %zu needed and %zd "
@@ -524,18 +521,10 @@ Logger_performOutput(Function function, PyObject* args, bool dbgLevel) {
         }
 
         // Extract the fixed arguments
-        PyObject *midO(PySequence_GetItem(args, start - 1));
-        if (midO == NULL) {
-            return (NULL);
-        }
-        string mid(objectToStr(midO, false));
         long dbg(0);
         if (dbgLevel) {
-            PyObject *dbgO(PySequence_GetItem(args, 0));
-            if (dbgO == NULL) {
-                return (NULL);
-            }
-            dbg = PyLong_AsLong(dbgO);
+            PyObjectContainer dbg_container(PySequence_GetItem(args, 0));
+            dbg = PyLong_AsLong(dbg_container.get());
             if (PyErr_Occurred()) {
                 return (NULL);
             }
@@ -544,16 +533,16 @@ Logger_performOutput(Function function, PyObject* args, bool dbgLevel) {
         // We create the logging message right now. If we fail to convert a
         // parameter to string, at least the part that we already did will
         // be output
+        PyObjectContainer msgid_container(PySequence_GetItem(args, start - 1));
+        const string mid(objectToStr(msgid_container.get(), false));
         Logger::Formatter formatter(function(dbg, mid.c_str()));
 
         // Now process the rest of parameters, convert each to string and put
         // into the formatter. It will print itself in the end.
         for (size_t i(start); i < number; ++ i) {
-            PyObject* param(PySequence_GetItem(args, i));
-            if (param == NULL) {
-                return (NULL);
-            }
-            formatter = formatter.arg(objectToStr(param, true));
+            PyObjectContainer param_container(PySequence_GetItem(args, i));
+            formatter = formatter.arg(objectToStr(param_container.get(),
+                                                  true));
         }
         Py_RETURN_NONE;
     }
@@ -573,72 +562,74 @@ Logger_performOutput(Function function, PyObject* args, bool dbgLevel) {
 // Now map the functions into the performOutput. I wish C++ could do
 // functional programming.
 PyObject*
-Logger_debug(LoggerWrapper* self, PyObject* args) {
+Logger_debug(PyObject* po_self, PyObject* args) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     return (Logger_performOutput(bind(&Logger::debug, self->logger_, _1, _2),
                                  args, true));
 }
 
 PyObject*
-Logger_info(LoggerWrapper* self, PyObject* args) {
+Logger_info(PyObject* po_self, PyObject* args) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     return (Logger_performOutput(bind(&Logger::info, self->logger_, _2),
                                  args, false));
 }
 
 PyObject*
-Logger_warn(LoggerWrapper* self, PyObject* args) {
+Logger_warn(PyObject* po_self, PyObject* args) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     return (Logger_performOutput(bind(&Logger::warn, self->logger_, _2),
                                  args, false));
 }
 
 PyObject*
-Logger_error(LoggerWrapper* self, PyObject* args) {
+Logger_error(PyObject* po_self, PyObject* args) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     return (Logger_performOutput(bind(&Logger::error, self->logger_, _2),
                                  args, false));
 }
 
 PyObject*
-Logger_fatal(LoggerWrapper* self, PyObject* args) {
+Logger_fatal(PyObject* po_self, PyObject* args) {
+    LoggerWrapper* self = static_cast<LoggerWrapper*>(po_self);
     return (Logger_performOutput(bind(&Logger::fatal, self->logger_, _2),
                                  args, false));
 }
 
 PyMethodDef loggerMethods[] = {
-    { "get_effective_severity",
-        reinterpret_cast<PyCFunction>(Logger_getEffectiveSeverity),
-        METH_NOARGS, "Returns the effective logging severity as string" },
-    { "get_effective_debug_level",
-        reinterpret_cast<PyCFunction>(Logger_getEffectiveDebugLevel),
-        METH_NOARGS, "Returns the current debug level." },
-    { "set_severity",
-        reinterpret_cast<PyCFunction>(Logger_setSeverity), METH_VARARGS,
+    { "get_effective_severity", Logger_getEffectiveSeverity, METH_NOARGS,
+        "Returns the effective logging severity as string" },
+    { "get_effective_debug_level", Logger_getEffectiveDebugLevel, METH_NOARGS,
+        "Returns the current debug level." },
+    { "set_severity", Logger_setSeverity, METH_VARARGS,
         "Sets the severity of a logger. The parameters are severity as a "
         "string and, optionally, a debug level (integer in range 0-99). "
         "The severity may be NULL, in which case an inherited value is taken."
     },
-    { "is_debug_enabled", reinterpret_cast<PyCFunction>(Logger_isDebugEnabled),
-        METH_VARARGS, "Returns if the logger would log debug message now. "
+    { "is_debug_enabled", Logger_isDebugEnabled, METH_VARARGS,
+      "Returns if the logger would log debug message now. "
             "You can provide a desired debug level." },
-    { "is_info_enabled", reinterpret_cast<PyCFunction>(Logger_isInfoEnabled),
-        METH_NOARGS, "Returns if the logger would log info message now." },
-    { "is_warn_enabled", reinterpret_cast<PyCFunction>(Logger_isWarnEnabled),
-        METH_NOARGS, "Returns if the logger would log warn message now." },
-    { "is_error_enabled", reinterpret_cast<PyCFunction>(Logger_isErrorEnabled),
-        METH_NOARGS, "Returns if the logger would log error message now." },
-    { "is_fatal_enabled", reinterpret_cast<PyCFunction>(Logger_isFatalEnabled),
-        METH_NOARGS, "Returns if the logger would log fatal message now." },
-    { "debug", reinterpret_cast<PyCFunction>(Logger_debug), METH_VARARGS,
+    { "is_info_enabled", Logger_isInfoEnabled, METH_NOARGS,
+      "Returns if the logger would log info message now." },
+    { "is_warn_enabled", Logger_isWarnEnabled, METH_NOARGS,
+      "Returns if the logger would log warn message now." },
+    { "is_error_enabled", Logger_isErrorEnabled, METH_NOARGS,
+      "Returns if the logger would log error message now." },
+    { "is_fatal_enabled", Logger_isFatalEnabled, METH_NOARGS,
+      "Returns if the logger would log fatal message now." },
+    { "debug", Logger_debug, METH_VARARGS,
         "Logs a debug-severity message. It takes the debug level, message ID "
         "and any number of stringifiable arguments to the message." },
-    { "info", reinterpret_cast<PyCFunction>(Logger_info), METH_VARARGS,
+    { "info", Logger_info, METH_VARARGS,
         "Logs a info-severity message. It taskes the message ID and any "
         "number of stringifiable arguments to the message." },
-    { "warn", reinterpret_cast<PyCFunction>(Logger_warn), METH_VARARGS,
+    { "warn", Logger_warn, METH_VARARGS,
         "Logs a warn-severity message. It taskes the message ID and any "
         "number of stringifiable arguments to the message." },
-    { "error", reinterpret_cast<PyCFunction>(Logger_error), METH_VARARGS,
+    { "error", Logger_error, METH_VARARGS,
         "Logs a error-severity message. It taskes the message ID and any "
         "number of stringifiable arguments to the message." },
-    { "fatal", reinterpret_cast<PyCFunction>(Logger_fatal), METH_VARARGS,
+    { "fatal", Logger_fatal, METH_VARARGS,
         "Logs a fatal-severity message. It taskes the message ID and any "
         "number of stringifiable arguments to the message." },
     { NULL, NULL, 0, NULL }
@@ -649,7 +640,7 @@ PyTypeObject logger_type = {
     "isc.log.Logger",
     sizeof(LoggerWrapper),                 // tp_basicsize
     0,                                  // tp_itemsize
-    reinterpret_cast<destructor>(Logger_destroy),       // tp_dealloc
+    Logger_destroy,                     // tp_dealloc
     NULL,                               // tp_print
     NULL,                               // tp_getattr
     NULL,                               // tp_setattr
@@ -681,7 +672,7 @@ PyTypeObject logger_type = {
     NULL,                               // tp_descr_get
     NULL,                               // tp_descr_set
     0,                                  // tp_dictoffset
-    reinterpret_cast<initproc>(Logger_init),            // tp_init
+    Logger_init,                        // tp_init
     NULL,                               // tp_alloc
     PyType_GenericNew,                  // tp_new
     NULL,                               // tp_free
@@ -718,21 +709,21 @@ PyInit_log(void) {
         return (NULL);
     }
 
-    if (PyType_Ready(&logger_type) < 0) {
-        return (NULL);
-    }
-
-    if (PyModule_AddObject(mod, "Logger",
-                           static_cast<PyObject*>(static_cast<void*>(
-                               &logger_type))) < 0) {
-        return (NULL);
-    }
-
-    // Add in the definitions of the standard debug levels.  These can then
-    // be referred to in Python through the constants log.DBGLVL_XXX.
+    // Finalize logger class and add in the definitions of the standard debug
+    // levels.  These can then be referred to in Python through the constants
+    // log.DBGLVL_XXX.
     // N.B. These should be kept in sync with the constants defined in
     // log_dbglevels.h.
     try {
+        if (PyType_Ready(&logger_type) < 0) {
+            throw InternalError();
+        }
+        void* p = &logger_type;
+        if (PyModule_AddObject(mod, "Logger",
+                               static_cast<PyObject*>(p)) < 0) {
+            throw InternalError();
+        }
+
         installClassVariable(logger_type, "DBGLVL_START_SHUT",
                              Py_BuildValue("I", DBGLVL_START_SHUT));
         installClassVariable(logger_type, "DBGLVL_COMMAND",
@@ -747,15 +738,20 @@ PyInit_log(void) {
                              Py_BuildValue("I", DBGLVL_TRACE_DETAIL));
         installClassVariable(logger_type, "DBGLVL_TRACE_DETAIL_DATA",
                              Py_BuildValue("I", DBGLVL_TRACE_DETAIL_DATA));
+    } catch (const InternalError&) {
+        Py_DECREF(mod);
+        return (NULL);
     } catch (const std::exception& ex) {
         const std::string ex_what =
             "Unexpected failure in Log initialization: " +
             std::string(ex.what());
         PyErr_SetString(PyExc_SystemError, ex_what.c_str());
+        Py_DECREF(mod);
         return (NULL);
     } catch (...) {
         PyErr_SetString(PyExc_SystemError,
                         "Unexpected failure in Log initialization");
+        Py_DECREF(mod);
         return (NULL);
     }
 

src/lib/python/isc/log/tests/log_test.py

@@ -17,6 +17,7 @@
 import isc.log
 import unittest
 import json
+import sys
 import bind10_config
 from isc.config.ccsession import path_search
 
@@ -89,6 +90,7 @@ class Logger(unittest.TestCase):
     def setUp(self):
         isc.log.init("root", "DEBUG", 50)
         self.sevs = ['INFO', 'WARN', 'ERROR', 'FATAL']
+        self.TEST_MSG = isc.log.create_message('TEST_MESSAGE', '%1')
 
     # Checks defaults of the logger
     def defaults(self, logger):
@@ -169,5 +171,34 @@ class Logger(unittest.TestCase):
         logger = isc.log.Logger("child")
         self.assertEqual(logger.DBGLVL_COMMAND, 10)
 
+    def test_param_reference(self):
+        """
+        Check whether passing a parameter to a logger causes a reference leak.
+        """
+        class LogParam:
+            def __str__(self):
+                return 'LogParam'
+        logger = isc.log.Logger("child")
+        param = LogParam()
+        orig_msgrefcnt = sys.getrefcount(param)
+        orig_idrefcnt = sys.getrefcount(self.TEST_MSG)
+        logger.info(self.TEST_MSG, param);
+        self.assertEqual(sys.getrefcount(self.TEST_MSG), orig_idrefcnt)
+        self.assertEqual(sys.getrefcount(param), orig_msgrefcnt)
+
+        # intentionally pass an invalid type for debug level.  It will
+        # result in TypeError.  The passed object still shouldn't leak a
+        # reference.
+        self.assertRaises(TypeError, logger.debug, param, self.TEST_MSG, param)
+        self.assertEqual(sys.getrefcount(param), orig_msgrefcnt)
+
+    def test_bad_parameter(self):
+        # a log parameter cannot be converted to a string object.
+        class LogParam:
+            def __str__(self):
+                raise ValueError("LogParam can't be converted to string")
+        logger = isc.log.Logger("child")
+        self.assertRaises(ValueError, logger.info, self.TEST_MSG, LogParam())
+
 if __name__ == '__main__':
     unittest.main()

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.