[master] Merge branch trac1261 with fixing conflicts, etc.

- I need to add another mock method in xfrin/test to be compatible with the
  latest version of Diff class.
- I also replaced 'remove' with 'delete' more completely in the Diff class
  (and adjusted the caller side accordingly) for consistency.

configure.ac

@@ -811,6 +811,7 @@ AC_CONFIG_FILES([Makefile
                  src/bin/sockcreator/tests/Makefile
                  src/bin/xfrin/Makefile
                  src/bin/xfrin/tests/Makefile
+                 src/bin/xfrin/tests/testdata/Makefile
                  src/bin/xfrout/Makefile
                  src/bin/xfrout/tests/Makefile
                  src/bin/zonemgr/Makefile

src/bin/xfrin/tests/Makefile.am

@@ -1,3 +1,5 @@
+SUBDIRS = testdata .
+
 PYCOVERAGE_RUN=@PYCOVERAGE_RUN@
 PYTESTS = xfrin_test.py
 EXTRA_DIST = $(PYTESTS)
@@ -20,5 +22,7 @@ endif
 	echo Running test: $$pytest ; \
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	PYTHONPATH=$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/bin/xfrin:$(COMMON_PYTHON_PATH) \
+	TESTDATASRCDIR=$(abs_top_srcdir)/src/bin/xfrin/tests/testdata/ \
+	TESTDATAOBJDIR=$(abs_top_builddir)/src/bin/xfrin/tests/testdata/ \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/bin/xfrin/tests/testdata/Makefile.am

@@ -0,0 +1,2 @@
+EXTRA_DIST = example.com # not necessarily needed, but for reference
+EXTRA_DIST += example.com.sqlite3

src/bin/xfrin/tests/testdata/example.com

@@ -0,0 +1,17 @@
+;; This is a simplest form of zone file for 'example.com', which is the
+;; source of the corresponding sqlite3 DB file.  This file is provided
+;; for reference purposes only; it's not actually used anywhere.
+
+example.com.		3600	IN SOA	master.example.com. admin.example.com. (
+					1230       ; serial
+					3600       ; refresh (1 hour)
+					1800       ; retry (30 minutes)
+					2419200    ; expire (4 weeks)
+					7200       ; minimum (2 hours)
+					)
+			3600	NS	dns01.example.com.
+			3600	NS	dns02.example.com.
+			3600	NS	dns03.example.com.
+dns01.example.com.	3600	IN A	192.0.2.1
+dns02.example.com.	3600	IN A	192.0.2.2
+dns03.example.com.	3600	IN A	192.0.2.3

src/bin/xfrin/tests/xfrin_test.py

@@ -14,10 +14,12 @@
 # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 import unittest
+import shutil
 import socket
 import io
 from isc.testutils.tsigctx_mock import MockTSIGContext
 from xfrin import *
+from isc.xfrin.diff import Diff
 import isc.log
 
 #
@@ -36,22 +38,31 @@ TEST_MASTER_IPV6_ADDRESS = '::1'
 TEST_MASTER_IPV6_ADDRINFO = (socket.AF_INET6, socket.SOCK_STREAM,
                              socket.IPPROTO_TCP, '',
                              (TEST_MASTER_IPV6_ADDRESS, 53))
+
+TESTDATA_SRCDIR = os.getenv("TESTDATASRCDIR")
+TESTDATA_OBJDIR = os.getenv("TESTDATAOBJDIR")
 # XXX: This should be a non priviledge port that is unlikely to be used.
 # If some other process uses this port test will fail.
 TEST_MASTER_PORT = '53535'
 
 TSIG_KEY = TSIGKey("example.com:SFuWd/q99SzF8Yzd1QbB9g==")
 
+# SOA intended to be used for the new SOA as a result of transfer.
 soa_rdata = Rdata(RRType.SOA(), TEST_RRCLASS,
                   'master.example.com. admin.example.com ' +
                   '1234 3600 1800 2419200 7200')
-soa_rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(),
-                  RRTTL(3600))
+soa_rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), RRTTL(3600))
 soa_rrset.add_rdata(soa_rdata)
-example_axfr_question = Question(TEST_ZONE_NAME, TEST_RRCLASS,
-                                 RRType.AXFR())
-example_soa_question = Question(TEST_ZONE_NAME, TEST_RRCLASS,
-                                 RRType.SOA())
+
+# SOA intended to be used for the current SOA at the secondary side.
+# Note that its serial is smaller than that of soa_rdata.
+begin_soa_rdata = Rdata(RRType.SOA(), TEST_RRCLASS,
+                        'master.example.com. admin.example.com ' +
+                        '1230 3600 1800 2419200 7200')
+begin_soa_rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), RRTTL(3600))
+begin_soa_rrset.add_rdata(begin_soa_rdata)
+example_axfr_question = Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.AXFR())
+example_soa_question = Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA())
 default_questions = [example_axfr_question]
 default_answers = [soa_rrset]
 
@@ -65,6 +76,78 @@ class MockCC():
         if identifier == "zones/class":
             return TEST_RRCLASS_STR
 
+class MockDataSourceClient():
+    '''A simple mock data source client.
+
+    This class provides a minimal set of wrappers related the data source
+    API that would be used by Diff objects.  For our testing purposes they
+    only keep truck of the history of the changes.
+
+    '''
+    def __init__(self):
+        self.committed_diffs = []
+        self.diffs = []
+
+    def get_class(self):
+        '''Mock version of get_class().
+
+        We simply return the commonly used constant RR class.  If and when
+        we use this mock for a different RR class we need to adjust it
+        accordingly.
+
+        '''
+        return TEST_RRCLASS
+
+    def find_zone(self, zone_name):
+        '''Mock version of find_zone().
+
+        It returns itself (subsequently acting as a mock ZoneFinder) for
+        some test zone names.  For some others it returns either NOTFOUND
+        or PARTIALMATCH.
+
+        '''
+        if zone_name == TEST_ZONE_NAME or \
+                zone_name == Name('no-soa.example') or \
+                zone_name == Name('dup-soa.example'):
+            return (isc.datasrc.DataSourceClient.SUCCESS, self)
+        elif zone_name == Name('no-such-zone.example'):
+            return (DataSourceClient.NOTFOUND, None)
+        elif zone_name == Name('partial-match-zone.example'):
+            return (DataSourceClient.PARTIALMATCH, self)
+        raise ValueError('Unexpected input to mock client: bug in test case?')
+
+    def find(self, name, rrtype, target, options):
+        '''Mock ZoneFinder.find().
+
+        It returns the predefined SOA RRset to queries for SOA of the common
+        test zone name.  It also emulates some unusual cases for special
+        zone names.
+
+        '''
+        if name == TEST_ZONE_NAME and rrtype == RRType.SOA():
+            return (ZoneFinder.SUCCESS, begin_soa_rrset)
+        if name == Name('no-soa.example'):
+            return (ZoneFinder.NXDOMAIN, None)
+        if name == Name('dup-soa.example'):
+            dup_soa_rrset = RRset(name, TEST_RRCLASS, RRType.SOA(), RRTTL(0))
+            dup_soa_rrset.add_rdata(begin_soa_rdata)
+            dup_soa_rrset.add_rdata(soa_rdata)
+            return (ZoneFinder.SUCCESS, dup_soa_rrset)
+        raise ValueError('Unexpected input to mock finder: bug in test case?')
+
+    def get_updater(self, zone_name, replace):
+        return self
+
+    def add_rrset(self, rrset):
+        self.diffs.append(('add', rrset))
+
+    def delete_rrset(self, rrset):
+        self.diffs.append(('delete', rrset))
+
+    def commit(self):
+        self.committed_diffs.append(self.diffs)
+        self.diffs = []
+
 class MockXfrin(Xfrin):
     # This is a class attribute of a callable object that specifies a non
     # default behavior triggered in _cc_check_command().  Specific test methods
@@ -87,20 +170,21 @@ class MockXfrin(Xfrin):
             MockXfrin.check_command_hook()
 
     def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo,
-                    tsig_key, check_soa=True):
+                    tsig_key, request_type, check_soa=True):
         # store some of the arguments for verification, then call this
         # method in the superclass
         self.xfrin_started_master_addr = master_addrinfo[2][0]
         self.xfrin_started_master_port = master_addrinfo[2][1]
-        return Xfrin.xfrin_start(self, zone_name, rrclass, db_file,
+        self.xfrin_started_request_type = request_type
+        return Xfrin.xfrin_start(self, zone_name, rrclass, None,
                                  master_addrinfo, tsig_key,
-                                 check_soa)
+                                 request_type, check_soa)
 
 class MockXfrinConnection(XfrinConnection):
     def __init__(self, sock_map, zone_name, rrclass, db_file, shutdown_event,
                  master_addr):
-        super().__init__(sock_map, zone_name, rrclass, db_file, shutdown_event,
-                         master_addr)
+        super().__init__(sock_map, zone_name, rrclass, MockDataSourceClient(),
+                         db_file, shutdown_event, master_addr)
         self.query_data = b''
         self.reply_data = b''
         self.force_time_out = False
@@ -122,7 +206,8 @@ class MockXfrinConnection(XfrinConnection):
         data = self.reply_data[:size]
         self.reply_data = self.reply_data[size:]
         if len(data) < size:
-            raise XfrinTestException('cannot get reply data')
+            raise XfrinTestException('cannot get reply data (' + str(size) +
+                                     ' bytes)')
         return data
 
     def send(self, data):
@@ -174,12 +259,241 @@ class MockXfrinConnection(XfrinConnection):
 
         return reply_data
 
+class TestXfrinState(unittest.TestCase):
+    def setUp(self):
+        self.sock_map = {}
+        self.conn = MockXfrinConnection(self.sock_map, TEST_ZONE_NAME,
+                                        TEST_RRCLASS, TEST_DB_FILE,
+                                        threading.Event(),
+                                        TEST_MASTER_IPV4_ADDRINFO)
+        self.begin_soa = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(),
+                               RRTTL(3600))
+        self.begin_soa.add_rdata(Rdata(RRType.SOA(), TEST_RRCLASS,
+                                       'm. r. 1230 0 0 0 0'))
+        self.ns_rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.NS(),
+                              RRTTL(3600))
+        self.ns_rrset.add_rdata(Rdata(RRType.NS(), TEST_RRCLASS,
+                                      'ns.example.com'))
+        self.conn._datasrc_client = MockDataSourceClient()
+        self.conn._diff = Diff(MockDataSourceClient(), TEST_ZONE_NAME)
+
+class TestXfrinStateBase(TestXfrinState):
+    def setUp(self):
+        super().setUp()
+
+    def test_handle_rr_on_base(self):
+        # The base version of handle_rr() isn't supposed to be called
+        # directly (the argument doesn't matter in this test)
+        self.assertRaises(XfrinException, XfrinState().handle_rr, None)
+
+class TestXfrinInitialSOA(TestXfrinState):
+    def setUp(self):
+        super().setUp()
+        self.state = XfrinInitialSOA()
+
+    def test_handle_rr(self):
+        # normal case
+        self.assertTrue(self.state.handle_rr(self.conn, soa_rrset))
+        self.assertEqual(type(XfrinFirstData()),
+                         type(self.conn.get_xfrstate()))
+        self.assertEqual(1234, self.conn._end_serial)
+
+    def test_handle_not_soa(self):
+        # The given RR is not of SOA
+        self.assertRaises(XfrinProtocolError, self.state.handle_rr, self.conn,
+                          self.ns_rrset)
+
+    def test_finish_message(self):
+        self.assertTrue(self.state.finish_message(self.conn))
+
+class TestXfrinFirstData(TestXfrinState):
+    def setUp(self):
+        super().setUp()
+        self.state = XfrinFirstData()
+        self.conn._request_type = RRType.IXFR()
+        self.conn._request_serial = 1230 # arbitrary chosen serial < 1234
+
+    def test_handle_ixfr_begin_soa(self):
+        self.conn._request_type = RRType.IXFR()
+        self.assertFalse(self.state.handle_rr(self.conn, self.begin_soa))
+        self.assertEqual(type(XfrinIXFRDeleteSOA()),
+                         type(self.conn.get_xfrstate()))
+
+    def test_handle_axfr(self):
+        # If the original type is AXFR, other conditions aren't considered,
+        # and AXFR processing will continue
+        self.conn._request_type = RRType.AXFR()
+        self.assertFalse(self.state.handle_rr(self.conn, self.begin_soa))
+        self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate()))
+
+    def test_handle_ixfr_to_axfr(self):
+        # Detecting AXFR-compatible IXFR response by seeing a non SOA RR after
+        # the initial SOA.  Should switch to AXFR.
+        self.assertFalse(self.state.handle_rr(self.conn, self.ns_rrset))
+        self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate()))
+
+    def test_handle_ixfr_to_axfr_by_different_soa(self):
+        # An unusual case: Response contains two consecutive SOA but the
+        # serial of the second does not match the requested one.  See
+        # the documentation for XfrinFirstData.handle_rr().
+        self.assertFalse(self.state.handle_rr(self.conn, soa_rrset))
+        self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate()))
+
+    def test_finish_message(self):
+        self.assertTrue(self.state.finish_message(self.conn))
+
+class TestXfrinIXFRDeleteSOA(TestXfrinState):
+    def setUp(self):
+        super().setUp()
+        self.state = XfrinIXFRDeleteSOA()
+        # In this state a new Diff object is expected to be created.  To
+        # confirm it, we nullify it beforehand.
+        self.conn._diff = None
+
+    def test_handle_rr(self):
+        self.assertTrue(self.state.handle_rr(self.conn, self.begin_soa))
+        self.assertEqual(type(XfrinIXFRDelete()),
+                         type(self.conn.get_xfrstate()))
+        self.assertEqual([('delete', self.begin_soa)],
+                         self.conn._diff.get_buffer())
+
+    def test_handle_non_soa(self):
+        self.assertRaises(XfrinException, self.state.handle_rr, self.conn,
+                          self.ns_rrset)
+
+    def test_finish_message(self):
+        self.assertTrue(self.state.finish_message(self.conn))
+
+class TestXfrinIXFRDelete(TestXfrinState):
+    def setUp(self):
+        super().setUp()
+        # We need record the state in 'conn' to check the case where the
+        # state doesn't change.
+        XfrinIXFRDelete().set_xfrstate(self.conn, XfrinIXFRDelete())
+        self.state = self.conn.get_xfrstate()
+
+    def test_handle_delete_rr(self):
+        # Non SOA RRs are simply (goting to be) deleted in this state
+        self.assertTrue(self.state.handle_rr(self.conn, self.ns_rrset))
+        self.assertEqual([('delete', self.ns_rrset)],
+                         self.conn._diff.get_buffer())
+        # The state shouldn't change
+        self.assertEqual(type(XfrinIXFRDelete()),
+                         type(self.conn.get_xfrstate()))
+
+    def test_handle_soa(self):
+        # SOA in this state means the beginning of added RRs.  This SOA
+        # should also be added in the next state, so handle_rr() should return
+        # false.
+        self.assertFalse(self.state.handle_rr(self.conn, soa_rrset))
+        self.assertEqual([], self.conn._diff.get_buffer())
+        self.assertEqual(1234, self.conn._current_serial)
+        self.assertEqual(type(XfrinIXFRAddSOA()),
+                         type(self.conn.get_xfrstate()))
+
+    def test_finish_message(self):
+        self.assertTrue(self.state.finish_message(self.conn))
+
+class TestXfrinIXFRAddSOA(TestXfrinState):
+    def setUp(self):
+        super().setUp()
+        self.state = XfrinIXFRAddSOA()
+
+    def test_handle_rr(self):
+        self.assertTrue(self.state.handle_rr(self.conn, soa_rrset))
+        self.assertEqual(type(XfrinIXFRAdd()), type(self.conn.get_xfrstate()))
+        self.assertEqual([('add', soa_rrset)],
+                         self.conn._diff.get_buffer())
+
+    def test_handle_non_soa(self):
+        self.assertRaises(XfrinException, self.state.handle_rr, self.conn,
+                          self.ns_rrset)
+
+    def test_finish_message(self):
+        self.assertTrue(self.state.finish_message(self.conn))
+
+class TestXfrinIXFRAdd(TestXfrinState):
+    def setUp(self):
+        super().setUp()
+        # We need record the state in 'conn' to check the case where the
+        # state doesn't change.
+        XfrinIXFRAdd().set_xfrstate(self.conn, XfrinIXFRAdd())
+        self.conn._current_serial = 1230
+        self.state = self.conn.get_xfrstate()
+
+    def test_handle_add_rr(self):
+        # Non SOA RRs are simply (goting to be) added in this state
+        self.assertTrue(self.state.handle_rr(self.conn, self.ns_rrset))
+        self.assertEqual([('add', self.ns_rrset)],
+                         self.conn._diff.get_buffer())
+        # The state shouldn't change
+        self.assertEqual(type(XfrinIXFRAdd()), type(self.conn.get_xfrstate()))
+
+    def test_handle_end_soa(self):
+        self.conn._end_serial = 1234
+        self.conn._diff.add_data(self.ns_rrset) # put some dummy change
+        self.assertTrue(self.state.handle_rr(self.conn, soa_rrset))
+        self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate()))
+        # handle_rr should have caused commit, and the buffer should now be
+        # empty.
+        self.assertEqual([], self.conn._diff.get_buffer())
+
+    def test_handle_new_delete(self):
+        self.conn._end_serial = 1234
+        # SOA RR whose serial is the current one means we are going to a new
+        # difference, starting with removing that SOA.
+        self.conn._diff.add_data(self.ns_rrset) # put some dummy change
+        self.assertFalse(self.state.handle_rr(self.conn, self.begin_soa))
+        self.assertEqual([], self.conn._diff.get_buffer())
+        self.assertEqual(type(XfrinIXFRDeleteSOA()),
+                         type(self.conn.get_xfrstate()))
+
+    def test_handle_out_of_sync(self):
+        # getting SOA with an inconsistent serial.  This is an error.
+        self.conn._end_serial = 1235
+        self.assertRaises(XfrinProtocolError, self.state.handle_rr,
+                          self.conn, soa_rrset)
+
+    def test_finish_message(self):
+        self.assertTrue(self.state.finish_message(self.conn))
+
+class TestXfrinIXFREnd(TestXfrinState):
+    def setUp(self):
+        super().setUp()
+        self.state = XfrinIXFREnd()
+
+    def test_handle_rr(self):
+        self.assertRaises(XfrinProtocolError, self.state.handle_rr, self.conn,
+                          self.ns_rrset)
+
+    def test_finish_message(self):
+        self.assertFalse(self.state.finish_message(self.conn))
+
+class TestXfrinAXFR(TestXfrinState):
+    def setUp(self):
+        super().setUp()
+        self.state = XfrinAXFR()
+
+    def test_handle_rr(self):
+        self.assertRaises(XfrinException, self.state.handle_rr, self.conn,
+                          soa_rrset)
+
+    def test_finish_message(self):
+        self.assertTrue(self.state.finish_message(self.conn))
+
 class TestXfrinConnection(unittest.TestCase):
+    '''Convenient parent class for XFR-protocol tests.
+
+    This class provides common setups and helper methods for protocol related
+    tests on AXFR and IXFR.
+
+    '''
+
     def setUp(self):
         if os.path.exists(TEST_DB_FILE):
             os.remove(TEST_DB_FILE)
         self.sock_map = {}
-        self.conn = MockXfrinConnection(self.sock_map, 'example.com.',
+        self.conn = MockXfrinConnection(self.sock_map, TEST_ZONE_NAME,
                                         TEST_RRCLASS, TEST_DB_FILE,
                                         threading.Event(),
                                         TEST_MASTER_IPV4_ADDRINFO)
@@ -201,6 +515,101 @@ class TestXfrinConnection(unittest.TestCase):
         if os.path.exists(TEST_DB_FILE):
             os.remove(TEST_DB_FILE)
 
+    def _handle_xfrin_response(self):
+        # This helper methods iterates over all RRs (excluding the ending SOA)
+        # transferred, and simply returns the number of RRs.  The return value
+        # may be used an assertion value for test cases.
+        rrs = 0
+        for rr in self.conn._handle_axfrin_response():
+            rrs += 1
+        return rrs
+
+    def _create_normal_response_data(self):
+        # This helper method creates a simple sequence of DNS messages that
+        # forms a valid AXFR transaction.  It consists of two messages, each
+        # containing just a single SOA RR.
+        tsig_1st = self.axfr_response_params['tsig_1st']
+        tsig_2nd = self.axfr_response_params['tsig_2nd']
+        self.conn.reply_data = self.conn.create_response_data(tsig_ctx=tsig_1st)
+        self.conn.reply_data += \
+            self.conn.create_response_data(tsig_ctx=tsig_2nd)
+
+    def _create_soa_response_data(self):
+        # This helper method creates a DNS message that is supposed to be
+        # used a valid response to SOA queries prior to XFR.
+        # If tsig is True, it tries to verify the query with a locally
+        # created TSIG context (which may or may not succeed) so that the
+        # response will include a TSIG.
+        # If axfr_after_soa is True, it resets the response_generator so that
+        # a valid XFR messages will follow.
+
+        verify_ctx = None
+        if self.soa_response_params['tsig']:
+            # xfrin (currently) always uses TCP.  strip off the length field.
+            query_data = self.conn.query_data[2:]
+            query_message = Message(Message.PARSE)
+            query_message.from_wire(query_data)
+            verify_ctx = TSIGContext(TSIG_KEY)
+            verify_ctx.verify(query_message.get_tsig_record(), query_data)
+
+        self.conn.reply_data = self.conn.create_response_data(
+            bad_qid=self.soa_response_params['bad_qid'],
+            response=self.soa_response_params['response'],
+            rcode=self.soa_response_params['rcode'],
+            questions=self.soa_response_params['questions'],
+            tsig_ctx=verify_ctx)
+        if self.soa_response_params['axfr_after_soa'] != None:
+            self.conn.response_generator = \
+                self.soa_response_params['axfr_after_soa']
+
+    def _create_broken_response_data(self):
+        # This helper method creates a bogus "DNS message" that only contains
+        # 4 octets of data.  The DNS message parser will raise an exception.
+        bogus_data = b'xxxx'
+        self.conn.reply_data = struct.pack('H', socket.htons(len(bogus_data)))
+        self.conn.reply_data += bogus_data
+
+    def check_diffs(self, expected, actual):
+        '''A helper method checking the differences made in the IXFR session.
+
+        '''
+        self.assertEqual(len(expected), len(actual))
+        for (diffs_exp, diffs_actual) in zip(expected, actual):
+            self.assertEqual(len(diffs_exp), len(diffs_actual))
+            for (diff_exp, diff_actual) in zip(diffs_exp, diffs_actual):
+                # operation should match
+                self.assertEqual(diff_exp[0], diff_actual[0])
+                # The diff as RRset should be equal (for simplicity we assume
+                # all RRsets contain exactly one RDATA)
+                self.assertEqual(diff_exp[1].get_name(),
+                                 diff_actual[1].get_name())
+                self.assertEqual(diff_exp[1].get_type(),
+                                 diff_actual[1].get_type())
+                self.assertEqual(diff_exp[1].get_class(),
+                                 diff_actual[1].get_class())
+                self.assertEqual(diff_exp[1].get_rdata_count(),
+                                 diff_actual[1].get_rdata_count())
+                self.assertEqual(1, diff_exp[1].get_rdata_count())
+                self.assertEqual(diff_exp[1].get_rdata()[0],
+                                 diff_actual[1].get_rdata()[0])
+
+    def _create_a(self, address):
+        rrset = RRset(Name('a.example.com'), TEST_RRCLASS, RRType.A(),
+                      RRTTL(3600))
+        rrset.add_rdata(Rdata(RRType.A(), TEST_RRCLASS, address))
+        return rrset
+
+    def _create_soa(self, serial):
+        rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(),
+                      RRTTL(3600))
+        rdata_str = 'm. r. ' + serial + ' 3600 1800 2419200 7200'
+        rrset.add_rdata(Rdata(RRType.SOA(), TEST_RRCLASS, rdata_str))
+        return rrset
+
+class TestAXFR(TestXfrinConnection):
+    def setUp(self):
+        super().setUp()
+
     def __create_mock_tsig(self, key, error):
         # This helper function creates a MockTSIGContext for a given key
         # and TSIG error to be used as a result of verify (normally faked
@@ -236,31 +645,82 @@ class TestXfrinConnection(unittest.TestCase):
         # to confirm an AF_INET6 socket has been created.  A naive application
         # tends to assume it's IPv4 only and hardcode AF_INET.  This test
         # uncovers such a bug.
-        c = MockXfrinConnection({}, 'example.com.', TEST_RRCLASS, TEST_DB_FILE,
+        c = MockXfrinConnection({}, TEST_ZONE_NAME, TEST_RRCLASS, TEST_DB_FILE,
                                 threading.Event(),
                                 TEST_MASTER_IPV6_ADDRINFO)
         c.bind(('::', 0))
         c.close()
 
     def test_init_chclass(self):
-        c = XfrinConnection({}, 'example.com.', RRClass.CH(), TEST_DB_FILE,
-                            threading.Event(), TEST_MASTER_IPV4_ADDRINFO)
+        c = MockXfrinConnection({}, TEST_ZONE_NAME, RRClass.CH(), TEST_DB_FILE,
+                                threading.Event(), TEST_MASTER_IPV4_ADDRINFO)
         axfrmsg = c._create_query(RRType.AXFR())
         self.assertEqual(axfrmsg.get_question()[0].get_class(),
                          RRClass.CH())
         c.close()
 
-    def test_send_query(self):
-        def create_msg(query_type):
-            msg = Message(Message.RENDER)
-            query_id = 0x1035
-            msg.set_qid(query_id)
-            msg.set_opcode(Opcode.QUERY())
-            msg.set_rcode(Rcode.NOERROR())
-            query_question = Question(Name("example.com."), RRClass.IN(), query_type)
-            msg.add_question(query_question)
-            return msg
+    def test_create_query(self):
+        def check_query(expected_qtype, expected_auth):
+            '''Helper method to repeat the same pattern of tests'''
+            self.assertEqual(Opcode.QUERY(), msg.get_opcode())
+            self.assertEqual(Rcode.NOERROR(), msg.get_rcode())
+            self.assertEqual(1, msg.get_rr_count(Message.SECTION_QUESTION))
+            self.assertEqual(TEST_ZONE_NAME, msg.get_question()[0].get_name())
+            self.assertEqual(expected_qtype, msg.get_question()[0].get_type())
+            self.assertEqual(0, msg.get_rr_count(Message.SECTION_ANSWER))
+            self.assertEqual(0, msg.get_rr_count(Message.SECTION_ADDITIONAL))
+            if expected_auth is None:
+                self.assertEqual(0,
+                                 msg.get_rr_count(Message.SECTION_AUTHORITY))
+            else:
+                self.assertEqual(1,
+                                 msg.get_rr_count(Message.SECTION_AUTHORITY))
+                auth_rr = msg.get_section(Message.SECTION_AUTHORITY)[0]
+                self.assertEqual(expected_auth.get_name(), auth_rr.get_name())
+                self.assertEqual(expected_auth.get_type(), auth_rr.get_type())
+                self.assertEqual(expected_auth.get_class(),
+                                 auth_rr.get_class())
+                # In our test scenario RDATA must be 1
+                self.assertEqual(1, expected_auth.get_rdata_count())
+                self.assertEqual(1, auth_rr.get_rdata_count())
+                self.assertEqual(expected_auth.get_rdata()[0],
+                                 auth_rr.get_rdata()[0])
+
+        # Actual tests start here
+        # SOA query
+        msg = self.conn._create_query(RRType.SOA())
+        check_query(RRType.SOA(), None)
+
+        # AXFR query
+        msg = self.conn._create_query(RRType.AXFR())
+        check_query(RRType.AXFR(), None)
+
+        # IXFR query
+        msg = self.conn._create_query(RRType.IXFR())
+        check_query(RRType.IXFR(), begin_soa_rrset)
+        self.assertEqual(1230, self.conn._request_serial)
+
+    def test_create_ixfr_query_fail(self):
+        # In these cases _create_query() will fail to find a valid SOA RR to
+        # insert in the IXFR query, and should raise an exception.
+
+        self.conn._zone_name = Name('no-such-zone.example')
+        self.assertRaises(XfrinException, self.conn._create_query,
+                          RRType.IXFR())
+
+        self.conn._zone_name = Name('partial-match-zone.example')
+        self.assertRaises(XfrinException, self.conn._create_query,
+                          RRType.IXFR())
+
+        self.conn._zone_name = Name('no-soa.example')
+        self.assertRaises(XfrinException, self.conn._create_query,
+                          RRType.IXFR())
+
+        self.conn._zone_name = Name('dup-soa.example')
+        self.assertRaises(XfrinException, self.conn._create_query,
+                          RRType.IXFR())
 
+    def test_send_query(self):
         def message_has_tsig(data):
             # a simple check if the actual data contains a TSIG RR.
             # At our level this simple check should suffice; other detailed
@@ -269,14 +729,6 @@ class TestXfrinConnection(unittest.TestCase):
             msg.from_wire(data)
             return msg.get_tsig_record() is not None
 
-        self.conn._create_query = create_msg
-        # soa request
-        self.conn._send_query(RRType.SOA())
-        self.assertEqual(self.conn.query_data, b'\x00\x1d\x105\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x07example\x03com\x00\x00\x06\x00\x01')
-        # axfr request
-        self.conn._send_query(RRType.AXFR())
-        self.assertEqual(self.conn.query_data, b'\x00\x1d\x105\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x07example\x03com\x00\x00\xfc\x00\x01')
-
         # soa request with tsig
         self.conn._tsig_key = TSIG_KEY
         self.conn._send_query(RRType.SOA())
@@ -467,7 +919,7 @@ class TestXfrinConnection(unittest.TestCase):
         self.conn._send_query(RRType.AXFR())
         self.assertRaises(Exception, self._handle_xfrin_response)
 
-    def test_response(self):
+    def test_axfr_response(self):
         # normal case.
         self.conn.response_generator = self._create_normal_response_data
         self.conn._send_query(RRType.AXFR())
@@ -598,10 +1050,7 @@ class TestXfrinConnection(unittest.TestCase):
 
     def test_do_soacheck_broken_response(self):
         self.conn.response_generator = self._create_broken_response_data
-        # XXX: TODO: this test failed here, should xfr not raise an
-        # exception but simply drop and return FAIL?
-        #self.assertEqual(self.conn.do_xfrin(True), XFRIN_FAIL)
-        self.assertRaises(MessageTooShort, self.conn.do_xfrin, True)
+        self.assertEqual(self.conn.do_xfrin(True), XFRIN_FAIL)
 
     def test_do_soacheck_badqid(self):
         # the QID mismatch would internally trigger a XfrinException exception,
@@ -610,59 +1059,226 @@ class TestXfrinConnection(unittest.TestCase):
         self.conn.response_generator = self._create_soa_response_data
         self.assertEqual(self.conn.do_xfrin(True), XFRIN_FAIL)
 
-    def _handle_xfrin_response(self):
-        # This helper methods iterates over all RRs (excluding the ending SOA)
-        # transferred, and simply returns the number of RRs.  The return value
-        # may be used an assertion value for test cases.
-        rrs = 0
-        for rr in self.conn._handle_xfrin_response():
-            rrs += 1
-        return rrs
-
-    def _create_normal_response_data(self):
-        # This helper method creates a simple sequence of DNS messages that
-        # forms a valid XFR transaction.  It consists of two messages, each
-        # containing just a single SOA RR.
-        tsig_1st = self.axfr_response_params['tsig_1st']
-        tsig_2nd = self.axfr_response_params['tsig_2nd']
-        self.conn.reply_data = self.conn.create_response_data(tsig_ctx=tsig_1st)
-        self.conn.reply_data += \
-            self.conn.create_response_data(tsig_ctx=tsig_2nd)
+class TestIXFRResponse(TestXfrinConnection):
+    def setUp(self):
+        super().setUp()
+        self.conn._query_id = self.conn.qid = 1035
+        self.conn._request_serial = 1230
+        self.conn._request_type = RRType.IXFR()
+        self._zone_name = TEST_ZONE_NAME
+        self.conn._datasrc_client = MockDataSourceClient()
+        XfrinInitialSOA().set_xfrstate(self.conn, XfrinInitialSOA())
 
-    def _create_soa_response_data(self):
-        # This helper method creates a DNS message that is supposed to be
-        # used a valid response to SOA queries prior to XFR.
-        # If tsig is True, it tries to verify the query with a locally
-        # created TSIG context (which may or may not succeed) so that the
-        # response will include a TSIG.
-        # If axfr_after_soa is True, it resets the response_generator so that
-        # a valid XFR messages will follow.
+    def test_ixfr_response(self):
+        '''A simplest form of IXFR response.
 
-        verify_ctx = None
-        if self.soa_response_params['tsig']:
-            # xfrin (curreently) always uses TCP.  strip off the length field.
-            query_data = self.conn.query_data[2:]
-            query_message = Message(Message.PARSE)
-            query_message.from_wire(query_data)
-            verify_ctx = TSIGContext(TSIG_KEY)
-            verify_ctx.verify(query_message.get_tsig_record(), query_data)
+        It simply updates the zone's SOA one time.
 
+        '''
         self.conn.reply_data = self.conn.create_response_data(
-            bad_qid=self.soa_response_params['bad_qid'],
-            response=self.soa_response_params['response'],
-            rcode=self.soa_response_params['rcode'],
-            questions=self.soa_response_params['questions'],
-            tsig_ctx=verify_ctx)
-        if self.soa_response_params['axfr_after_soa'] != None:
-            self.conn.response_generator = \
-                self.soa_response_params['axfr_after_soa']
+            questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())],
+            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.assertEqual([], self.conn._datasrc_client.diffs)
+        self.check_diffs([[('delete', begin_soa_rrset), ('add', soa_rrset)]],
+                         self.conn._datasrc_client.committed_diffs)
+
+    def test_ixfr_response_multi_sequences(self):
+        '''Similar to the previous case, but with multiple diff seqs.
+
+        '''
+        self.conn.reply_data = self.conn.create_response_data(
+            questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())],
+            answers=[soa_rrset,
+                     # removing one A in serial 1230
+                     begin_soa_rrset, self._create_a('192.0.2.1'),
+                     # adding one A in serial 1231
+                     self._create_soa('1231'), self._create_a('192.0.2.2'),
+                     # removing one A in serial 1231
+                     self._create_soa('1231'), self._create_a('192.0.2.3'),
+                     # adding one A in serial 1232
+                     self._create_soa('1232'), self._create_a('192.0.2.4'),
+                     # removing one A in serial 1232
+                     self._create_soa('1232'), self._create_a('192.0.2.5'),
+                     # adding one A in serial 1234
+                     soa_rrset, self._create_a('192.0.2.6'),
+                     soa_rrset])
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate()))
+        self.assertEqual([], self.conn._datasrc_client.diffs)
+        self.check_diffs([[('delete', begin_soa_rrset),
+                           ('delete', self._create_a('192.0.2.1')),
+                           ('add', self._create_soa('1231')),
+                           ('add', self._create_a('192.0.2.2'))],
+                          [('delete', self._create_soa('1231')),
+                           ('delete', self._create_a('192.0.2.3')),
+                           ('add', self._create_soa('1232')),
+                           ('add', self._create_a('192.0.2.4'))],
+                          [('delete', self._create_soa('1232')),
+                           ('delete', self._create_a('192.0.2.5')),
+                           ('add', soa_rrset),
+                           ('add', self._create_a('192.0.2.6'))]],
+                         self.conn._datasrc_client.committed_diffs)
+
+    def test_ixfr_response_multi_messages(self):
+        '''Similar to the first case, but RRs span over multiple messages.
+
+        '''
+        self.conn.reply_data = self.conn.create_response_data(
+            questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())],
+            answers=[soa_rrset, begin_soa_rrset, soa_rrset])
+        self.conn.reply_data += self.conn.create_response_data(
+            questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())],
+            answers=[soa_rrset])
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate()))
+        self.check_diffs([[('delete', begin_soa_rrset), ('add', soa_rrset)]],
+                         self.conn._datasrc_client.committed_diffs)
+
+    def test_ixfr_response_broken(self):
+        '''Test with a broken response.
+
+        '''
+        # SOA sequence is out-of-sync
+        self.conn.reply_data = self.conn.create_response_data(
+            questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())],
+            answers=[soa_rrset, begin_soa_rrset, soa_rrset,
+                     self._create_soa('1235')])
+        self.assertRaises(XfrinProtocolError,
+                          self.conn._handle_xfrin_responses)
+        # no diffs should have been committed
+        self.check_diffs([], self.conn._datasrc_client.committed_diffs)
+
+    def test_ixfr_response_extra(self):
+        '''Test with an extra RR after the end of IXFR diff sequences.
+
+        IXFR should be rejected, but complete diff sequences should be
+        committed; it's not clear whether it's compliant to the protocol
+        specification, but it is how BIND 9 works and we do the same.
+        '''
+        self.conn.reply_data = self.conn.create_response_data(
+            questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())],
+            answers=[soa_rrset, begin_soa_rrset, soa_rrset, soa_rrset,
+                     self._create_a('192.0.2.1')])
+        self.assertRaises(XfrinProtocolError,
+                          self.conn._handle_xfrin_responses)
+        self.check_diffs([[('delete', begin_soa_rrset), ('add', soa_rrset)]],
+                         self.conn._datasrc_client.committed_diffs)
+
+class TestIXFRSession(TestXfrinConnection):
+    '''Tests for a full IXFR session (query and response).
+
+    Detailed corner cases should have been covered in test_create_query()
+    and TestIXFRResponse, so we'll only check some typical cases to confirm
+    the general logic flow.
+    '''
+    def setUp(self):
+        super().setUp()
 
-    def _create_broken_response_data(self):
-        # This helper method creates a bogus "DNS message" that only contains
-        # 4 octets of data.  The DNS message parser will raise an exception.
-        bogus_data = b'xxxx'
-        self.conn.reply_data = struct.pack('H', socket.htons(len(bogus_data)))
-        self.conn.reply_data += bogus_data
+    def test_do_xfrin(self):
+        def create_ixfr_response():
+            self.conn.reply_data = self.conn.create_response_data(
+                questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
+                                    RRType.IXFR())],
+                answers=[soa_rrset, begin_soa_rrset, soa_rrset, soa_rrset])
+        self.conn.response_generator = create_ixfr_response
+        self.assertEqual(XFRIN_OK, self.conn.do_xfrin(False, RRType.IXFR()))
+
+        # Check some details of the IXFR protocol processing
+        self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate()))
+        self.check_diffs([[('delete', begin_soa_rrset), ('add', soa_rrset)]],
+                         self.conn._datasrc_client.committed_diffs)
+
+        # Check if the query was IXFR.
+        qdata = self.conn.query_data[2:]
+        qmsg = Message(Message.PARSE)
+        qmsg.from_wire(qdata, len(qdata))
+        self.assertEqual(1, qmsg.get_rr_count(Message.SECTION_QUESTION))
+        self.assertEqual(TEST_ZONE_NAME, qmsg.get_question()[0].get_name())
+        self.assertEqual(RRType.IXFR(), qmsg.get_question()[0].get_type())
+
+    def test_do_xfrin_fail(self):
+        '''IXFR fails due to a protocol error.
+
+        '''
+        def create_ixfr_response():
+            self.conn.reply_data = self.conn.create_response_data(
+                questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
+                                    RRType.IXFR())],
+                answers=[soa_rrset, begin_soa_rrset, soa_rrset,
+                         self._create_soa('1235')])
+        self.conn.response_generator = create_ixfr_response
+        self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, RRType.IXFR()))
+
+    def test_do_xfrin_fail(self):
+        '''IXFR fails due to a bogus DNS message.
+
+        '''
+        self._create_broken_response_data()
+        self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, RRType.IXFR()))
+
+class TestIXFRSessionWithSQLite3(TestXfrinConnection):
+    '''Tests for IXFR sessions using an SQLite3 DB.
+
+    These are provided mainly to confirm the implementation actually works
+    in an environment closer to actual operational environments.  So we
+    only check a few common cases; other details are tested using mock
+    data sources.
+
+    '''
+    def setUp(self):
+        self.sqlite3db_src = TESTDATA_SRCDIR + '/example.com.sqlite3'
+        self.sqlite3db_obj = TESTDATA_OBJDIR + '/example.com.sqlite3.copy'
+        super().setUp()
+        if os.path.exists(self.sqlite3db_obj):
+            os.unlink(self.sqlite3db_obj)
+        shutil.copyfile(self.sqlite3db_src, self.sqlite3db_obj)
+        self.conn._datasrc_client = DataSourceClient(self.sqlite3db_obj)
+
+    def tearDown(self):
+        if os.path.exists(self.sqlite3db_obj):
+            os.unlink(self.sqlite3db_obj)
+
+    def get_zone_serial(self):
+        result, finder = self.conn._datasrc_client.find_zone(TEST_ZONE_NAME)
+        self.assertEqual(DataSourceClient.SUCCESS, result)
+        result, soa = finder.find(TEST_ZONE_NAME, RRType.SOA(),
+                                  None, ZoneFinder.FIND_DEFAULT)
+        self.assertEqual(ZoneFinder.SUCCESS, result)
+        self.assertEqual(1, soa.get_rdata_count())
+        return get_soa_serial(soa.get_rdata()[0])
+
+    def test_do_xfrin_sqlite3(self):
+        def create_ixfr_response():
+            self.conn.reply_data = self.conn.create_response_data(
+                questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
+                                    RRType.IXFR())],
+                answers=[soa_rrset, begin_soa_rrset, soa_rrset, soa_rrset])
+        self.conn.response_generator = create_ixfr_response
+
+        # Confirm xfrin succeeds and SOA is updated
+        self.assertEqual(1230, self.get_zone_serial())
+        self.assertEqual(XFRIN_OK, self.conn.do_xfrin(False, RRType.IXFR()))
+        self.assertEqual(1234, self.get_zone_serial())
+
+    def test_do_xfrin_sqlite3_fail(self):
+        '''Similar to the previous test, but xfrin fails due to error.
+
+        Check the DB is not changed.
+
+        '''
+        def create_ixfr_response():
+            self.conn.reply_data = self.conn.create_response_data(
+                questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
+                                    RRType.IXFR())],
+                answers=[soa_rrset, begin_soa_rrset, soa_rrset,
+                         self._create_soa('1235')])
+        self.conn.response_generator = create_ixfr_response
+
+        self.assertEqual(1230, self.get_zone_serial())
+        self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, RRType.IXFR()))
+        self.assertEqual(1230, self.get_zone_serial())
 
 class TestXfrinRecorder(unittest.TestCase):
     def setUp(self):
@@ -789,6 +1405,8 @@ class TestXfrin(unittest.TestCase):
                                                   self.args)['result'][0], 0)
         self.assertEqual(self.args['master'], self.xfr.xfrin_started_master_addr)
         self.assertEqual(int(self.args['port']), self.xfr.xfrin_started_master_port)
+        # By default we use AXFR (for now)
+        self.assertEqual(RRType.AXFR(), self.xfr.xfrin_started_request_type)
 
     def test_command_handler_retransfer_short_command1(self):
         # try it when only specifying the zone name (of unknown zone)
@@ -901,6 +1519,8 @@ class TestXfrin(unittest.TestCase):
                          self.xfr.xfrin_started_master_addr)
         self.assertEqual(int(TEST_MASTER_PORT),
                          self.xfr.xfrin_started_master_port)
+        # By default we use AXFR (for now)
+        self.assertEqual(RRType.AXFR(), self.xfr.xfrin_started_request_type)
 
     def test_command_handler_notify(self):
         # at this level, refresh is no different than retransfer.
@@ -1090,6 +1710,38 @@ class TestXfrin(unittest.TestCase):
         # since this has failed, we should still have the previous config
         self._check_zones_config(config2)
 
+    def common_ixfr_setup(self, xfr_mode, ixfr_disabled):
+        # This helper method explicitly sets up a zone configuration with
+        # ixfr_disabled, and invokes either retransfer or refresh.
+        # Shared by some of the following test cases.
+        config = {'zones': [
+                {'name': 'example.com.',
+                 'master_addr': '192.0.2.1',
+                 'ixfr_disabled': ixfr_disabled}]}
+        self.assertEqual(self.xfr.config_handler(config)['result'][0], 0)
+        self.assertEqual(self.xfr.command_handler(xfr_mode,
+                                                  self.args)['result'][0], 0)
+
+    def test_command_handler_retransfer_ixfr_enabled(self):
+        # If IXFR is explicitly enabled in config, IXFR will be used
+        self.common_ixfr_setup('retransfer', False)
+        self.assertEqual(RRType.IXFR(), self.xfr.xfrin_started_request_type)
+
+    def test_command_handler_refresh_ixfr_enabled(self):
+        # Same for refresh
+        self.common_ixfr_setup('refresh', False)
+        self.assertEqual(RRType.IXFR(), self.xfr.xfrin_started_request_type)
+
+    def test_command_handler_retransfer_ixfr_disabled(self):
+        # Similar to the previous case, but explicitly disabled.  AXFR should
+        # be used.
+        self.common_ixfr_setup('retransfer', True)
+        self.assertEqual(RRType.AXFR(), self.xfr.xfrin_started_request_type)
+
+    def test_command_handler_refresh_ixfr_disabled(self):
+        # Same for refresh
+        self.common_ixfr_setup('refresh', True)
+        self.assertEqual(RRType.AXFR(), self.xfr.xfrin_started_request_type)
 
 def raise_interrupt():
     raise KeyboardInterrupt()

src/bin/xfrin/xfrin.py.in

@@ -28,7 +28,9 @@ from optparse import OptionParser, OptionValueError
 from isc.config.ccsession import *
 from isc.notify import notify_out
 import isc.util.process
+from isc.datasrc import DataSourceClient, ZoneFinder
 import isc.net.parse
+from isc.xfrin.diff import Diff
 from isc.log_messages.xfrin_messages import *
 
 isc.log.init("b10-xfrin")
@@ -62,6 +64,9 @@ ZONE_MANAGER_MODULE_NAME = 'Zonemgr'
 REFRESH_FROM_ZONEMGR = 'refresh_from_zonemgr'
 ZONE_XFRIN_FAILED = 'zone_xfrin_failed'
 
+# Constants for debug levels, to be removed when we have #1074.
+DBG_XFRIN_TRACE = 3
+
 # These two default are currently hard-coded. For config this isn't
 # necessary, but we need these defaults for optional command arguments
 # (TODO: have similar support to get default values for command
@@ -77,6 +82,11 @@ XFRIN_FAIL = 1
 class XfrinException(Exception):
     pass
 
+class XfrinProtocolError(Exception):
+    '''An exception raised for errors encountered in xfrin protocol handling.
+    '''
+    pass
+
 class XfrinZoneInfoException(Exception):
     """This exception is raised if there is an error in the given
        configuration (part), or when a command does not have a required
@@ -112,24 +122,317 @@ def _check_zone_class(zone_class_str):
     except InvalidRRClass as irce:
         raise XfrinZoneInfoException("bad zone class: " + zone_class_str + " (" + str(irce) + ")")
 
+def get_soa_serial(soa_rdata):
+    '''Extract the serial field of an SOA RDATA and returns it as an intger.
+
+    We don't have to be very efficient here, so we first dump the entire RDATA
+    as a string and convert the first corresponding field.  This should be
+    sufficient in practice, but may not always work when the MNAME or RNAME
+    contains an (escaped) space character in their labels.  Ideally there
+    should be a more direct and convenient way to get access to the SOA
+    fields.
+    '''
+    return int(soa_rdata.to_text().split()[2])
+
+class XfrinState:
+    '''
+    The states of the incomding *XFR state machine.
+
+    We (will) handle both IXFR and AXFR with a single integrated state
+    machine because they cannot be distinguished immediately - an AXFR
+    response to an IXFR request can only be detected when the first two (2)
+    response RRs have already been received.
+    NOTE: the AXFR part of the state machine is incomplete at this point.
+
+    The following diagram summarizes the state transition.  After sending
+    the query, xfrin starts the process with the InitialSOA state (all
+    IXFR/AXFR response begins with an SOA).  When it reaches IXFREnd
+    (or AXFREnd, which is not yet implemented and not shown here), the
+    process successfully completes.
+
+
+            (recv SOA)       (AXFR-style IXFR)
+    InitialSOA------->FirstData------------->AXFR
+                          |                     (non SOA, delete)
+               (pure IXFR,|                           +-------+
+            keep handling)|             (Delete SOA)  V       |
+                          + ->IXFRDeleteSOA------>IXFRDelete--+
+                                   ^                   |
+                (see SOA, not end, |          (see SOA)|
+            commit, keep handling) |                   |
+                                   |                   V
+                      +---------IXFRAdd<----------+IXFRAddSOA
+        (non SOA, add)|         ^  |    (Add SOA)
+                      ----------+  |
+                                   |(see SOA w/ end serial, commit changes)
+                                   V
+                                IXFREnd
+
+    Note that changes are committed for every "difference sequence"
+    (i.e. changes for one SOA update).  This means when an IXFR response
+    contains multiple difference sequences and something goes wrong
+    after several commits, these changes have been published and visible
+    to clients even if the IXFR session is subsequently aborted.
+    It is not clear if this is valid in terms of the protocol specification.
+    Section 4 of RFC 1995 states:
+
+       An IXFR client, should only replace an older version with a newer
+       version after all the differences have been successfully processed.
+
+    If this "replacement" is for the changes of one difference sequence
+    and "all the differences" mean the changes for that sequence, this
+    implementation strictly follows what RFC states.  If this is for
+    the entire IXFR response (that may contain multiple sequences),
+    we should implement it with one big transaction and one final commit
+    at the very end.
+
+    For now, we implement it with multiple smaller commits for two
+    reasons.  First, this is what BIND 9 does, and we generally port
+    the implementation logic here.  BIND 9 has been supporting IXFR
+    for many years, so the fact that it still behaves this way
+    probably means it at least doesn't cause a severe operational
+    problem in practice.  Second, especially because BIND 10 would
+    often uses a database backend, a larger transaction could cause an
+    undesirable effects, e.g. suspending normal lookups for a longer
+    period depending on the characteristics of the database.  Even if
+    we find something wrong in a later sequeunce and abort the
+    session, we can start another incremental update from what has
+    been validated, or we can switch to AXFR to replace the zone
+    completely.
+
+    This implementation uses the state design pattern, where each state
+    is represented as a subclass of the base XfrinState class.  Each concrete
+    subclass of XfrinState is assumed to define two methods: handle_rr() and
+    finish_message().  These methods handle specific part of XFR protocols
+    and (if necessary) perform the state transition.
+
+    Conceptually, XfrinState and its subclasses are a "friend" of
+    XfrinConnection and are assumed to be allowed to access its internal
+    information (even though Python does not have a strict access control
+    between different classes).
+
+    The XfrinState and its subclasses are designed to be stateless, and
+    can be used as singleton objects.  For now, however, we always instantiate
+    a new object for every state transition, partly because the introduction
+    of singleton will make a code bit complicated, and partly because
+    the overhead of object instantiotion wouldn't be significant for xfrin.
+
+    '''
+    def set_xfrstate(self, conn, new_state):
+        '''Set the XfrConnection to a given new state.
+
+        As a "friend" class, this method intentionally gets access to the
+        connection's "private" method.
+
+        '''
+        conn._XfrinConnection__set_xfrstate(new_state)
+
+    def handle_rr(self, conn):
+        '''Handle one RR of an XFR response message.
+
+        Depending on the state, the RR is generally added or deleted in the
+        corresponding data source, or in some special cases indicates
+        a specifi transition, such as starting a new IXFR difference
+        sequence or completing the session.
+
+        All subclass has their specific behaviors for this method, so
+        there is no default definition.  If the base class version
+        is called, it's a bug of the caller, and it's notified via
+        an XfrinException exception.
+
+        This method returns a boolean value: True if the given RR was
+        fully handled and the caller should go to the next RR; False
+        if the caller needs to call this method with the (possibly) new
+        state for the same RR again.
+
+        '''
+        raise XfrinException("Internal bug: " +
+                             "XfrinState.handle_rr() called directly")
+
+    def finish_message(self, conn):
+        '''Perform any final processing after handling all RRs of a response.
+
+        This method then returns a boolean indicating whether to continue
+        receiving the message.  Unless it's in the end of the entire XFR
+        session, we should continue, so this default method simply returns
+        True.
+
+        '''
+        return True
+
+class XfrinInitialSOA(XfrinState):
+    def handle_rr(self, conn, rr):
+        if rr.get_type() != RRType.SOA():
+            raise XfrinProtocolError('First RR in zone transfer must be SOA ('
+                                     + rr.get_type().to_text() + ' received)')
+        conn._end_serial = get_soa_serial(rr.get_rdata()[0])
+
+        # FIXME: we need to check the serial is actually greater than ours.
+        # To do so, however, we need to implement serial number arithmetic.
+        # Although it wouldn't be a big task, we'll leave it for a separate
+        # task for now.  (Always performing xfr could be inefficient, but
+        # shouldn't do any harm otherwise)
+
+        self.set_xfrstate(conn, XfrinFirstData())
+        return True
+
+class XfrinFirstData(XfrinState):
+    def handle_rr(self, conn, rr):
+        '''Handle the first RR after initial SOA in an XFR session.
+
+        This state happens exactly once in an XFR session, where
+        we decide whether it's incremental update ("real" IXFR) or
+        non incremental update (AXFR or AXFR-style IXFR).
+        If we initiated IXFR and the transfer begins with two SOAs
+        (the serial of the second one being equal to our serial),
+        it's incremental; otherwise it's non incremental.
+
+        This method always return False (unlike many other handle_rr()
+        methods) because this first RR must be examined again in the
+        determined update context.
+
+        Note that in the non incremental case the RR should normally be
+        something other SOA, but it's still possible it's an SOA with a
+        different serial than ours.  The only possible interpretation at
+        this point is that it's non incremental update that only consists
+        of the SOA RR.  It will result in broken zone (for example, it
+        wouldn't even contain an apex NS) and should be rejected at post
+        XFR processing, but in terms of the XFR session processing we
+        accept it and move forward.
+
+        Note further that, in the half-broken SOA-only transfer case,
+        these two SOAs are supposed to be the same as stated in Section 2.2
+        of RFC 5936.  We don't check that condition here, either; we'll
+        leave whether and how to deal with that situation to the end of
+        the processing of non incremental update.  See also a related
+        discussion at the IETF dnsext wg:
+        http://www.ietf.org/mail-archive/web/dnsext/current/msg07908.html
+
+        '''
+        if conn._request_type == RRType.IXFR() and \
+                rr.get_type() == RRType.SOA() and \
+                conn._request_serial == get_soa_serial(rr.get_rdata()[0]):
+            logger.debug(DBG_XFRIN_TRACE, XFRIN_GOT_INCREMENTAL_RESP,
+                         conn.zone_str())
+            self.set_xfrstate(conn, XfrinIXFRDeleteSOA())
+        else:
+            logger.debug(DBG_XFRIN_TRACE, XFRIN_GOT_NONINCREMENTAL_RESP,
+                 conn.zone_str())
+            self.set_xfrstate(conn, XfrinAXFR())
+        return False
+
+class XfrinIXFRDeleteSOA(XfrinState):
+    def handle_rr(self, conn, rr):
+        if rr.get_type() != RRType.SOA():
+            # this shouldn't happen; should this occur it means an internal
+            # bug.
+            raise XfrinException(rr.get_type().to_text() +
+                                 ' 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)
+        conn._diff.delete_data(rr)
+        self.set_xfrstate(conn, XfrinIXFRDelete())
+        return True
+
+class XfrinIXFRDelete(XfrinState):
+    def handle_rr(self, conn, rr):
+        if rr.get_type() == RRType.SOA():
+            # This is the only place where current_serial is set
+            conn._current_serial = get_soa_serial(rr.get_rdata()[0])
+            self.set_xfrstate(conn, XfrinIXFRAddSOA())
+            return False
+        conn._diff.delete_data(rr)
+        return True
+
+class XfrinIXFRAddSOA(XfrinState):
+    def handle_rr(self, conn, rr):
+        if rr.get_type() != RRType.SOA():
+            # this shouldn't happen; should this occur it means an internal
+            # bug.
+            raise XfrinException(rr.get_type().to_text() +
+                                 ' RR is given in IXFRAddSOA state')
+        conn._diff.add_data(rr)
+        self.set_xfrstate(conn, XfrinIXFRAdd())
+        return True
+
+class XfrinIXFRAdd(XfrinState):
+    def handle_rr(self, conn, rr):
+        if rr.get_type() == RRType.SOA():
+            soa_serial = get_soa_serial(rr.get_rdata()[0])
+            if soa_serial == conn._end_serial:
+                conn._diff.commit()
+                self.set_xfrstate(conn, XfrinIXFREnd())
+                return True
+            elif soa_serial != conn._current_serial:
+                raise XfrinProtocolError('IXFR out of sync: expected ' +
+                                         'serial ' +
+                                         str(conn._current_serial) +
+                                         ', got ' + str(soa_serial))
+            else:
+                conn._diff.commit()
+                self.set_xfrstate(conn, XfrinIXFRDeleteSOA())
+                return False
+        conn._diff.add_data(rr)
+        return True
+
+class XfrinIXFREnd(XfrinState):
+    def handle_rr(self, conn, rr):
+        raise XfrinProtocolError('Extra data after the end of IXFR diffs: ' +
+                                 rr.to_text())
+
+    def finish_message(self, conn):
+        '''Final processing after processing an entire IXFR session.
+
+        There will be more actions here, but for now we simply return False,
+        indicating there will be no more message to receive.
+
+        '''
+        return False
+
+class XfrinAXFR(XfrinState):
+    def handle_rr(self, conn, rr):
+        raise XfrinException('Falling back from IXFR to AXFR not ' +
+                             'supported yet')
+
 class XfrinConnection(asyncore.dispatcher):
     '''Do xfrin in this class. '''
 
     def __init__(self,
-                 sock_map, zone_name, rrclass, db_file, shutdown_event,
-                 master_addrinfo, tsig_key = None, verbose = False,
-                 idle_timeout = 60):
-        ''' idle_timeout: max idle time for read data from socket.
-            db_file: specify the data source file.
-            check_soa: when it's true, check soa first before sending xfr query
+                 sock_map, zone_name, rrclass, datasrc_client, db_file,
+                 shutdown_event, master_addrinfo, tsig_key = None,
+                 verbose=False, idle_timeout=60):
+        '''Constructor of the XfrinConnection class.
+
+        idle_timeout: max idle time for read data from socket.
+        datasrc_client: the data source client object used for the XFR session.
+                        This will eventually replace db_file completely.
+        db_file: specify the data source file (should soon be deprecated).
+
         '''
 
         asyncore.dispatcher.__init__(self, map=sock_map)
-        self.create_socket(master_addrinfo[0], master_addrinfo[1])
+
+        # The XFR state.  Conceptually this is purely private, so we emphasize
+        # the fact by the double underscore.  Other classes are assumed to
+        # get access to this via get_xfrstate(), and only XfrinState classes
+        # are assumed to be allowed to modify it via __set_xfrstate().
+        self.__state = None
+
+        # Requested transfer type (RRType.AXFR or RRType.IXFR).  The actual
+        # transfer type may differ due to IXFR->AXFR fallback:
+        self._request_type = None
+
+        # Zone parameters
         self._zone_name = zone_name
-        self._sock_map = sock_map
         self._rrclass = rrclass
-        self._db_file = db_file
+
+        # Data source handlers
+        self._db_file = db_file # temporary for sqlite3 specific code
+        self._datasrc_client = datasrc_client
+
+        self.create_socket(master_addrinfo[0], master_addrinfo[1])
+        self._sock_map = sock_map
         self._soa_rr_count = 0
         self._idle_timeout = idle_timeout
         self.setblocking(1)
@@ -145,6 +448,16 @@ class XfrinConnection(asyncore.dispatcher):
     def __create_tsig_ctx(self, key):
         return TSIGContext(key)
 
+    def __set_xfrstate(self, new_state):
+        self.__state = new_state
+
+    def get_xfrstate(self):
+        return self.__state
+
+    def zone_str(self):
+        '''A convenient function for logging to include zone name and class'''
+        return self._zone_name.to_text() + '/' + str(self._rrclass)
+
     def connect_to_master(self):
         '''Connect to master in TCP.'''
 
@@ -156,16 +469,47 @@ class XfrinConnection(asyncore.dispatcher):
             return False
 
     def _create_query(self, query_type):
-        '''Create dns query message. '''
+        '''Create an XFR-related query message.
+
+        query_type is either SOA, AXFR or IXFR.  For type IXFR, it searches
+        the associated data source for the current SOA record to include
+        it in the query.  If the corresponding zone or the SOA record
+        cannot be found, it raises an XfrinException exception.  Note that
+        this may not necessarily a broken configuration; for the first attempt
+        of transfer the secondary may not have any boot-strap zone
+        information, in which case IXFR simply won't work.  The xfrin
+        should then fall back to AXFR.  _request_serial is recorded for
+        later use.
 
+        '''
         msg = Message(Message.RENDER)
         query_id = random.randint(0, 0xFFFF)
         self._query_id = query_id
         msg.set_qid(query_id)
         msg.set_opcode(Opcode.QUERY())
         msg.set_rcode(Rcode.NOERROR())
-        query_question = Question(Name(self._zone_name), self._rrclass, query_type)
-        msg.add_question(query_question)
+        msg.add_question(Question(self._zone_name, self._rrclass, query_type))
+        if query_type == RRType.IXFR():
+            # get the zone finder.  this must be SUCCESS (not even
+            # PARTIALMATCH) because we are specifying the zone origin name.
+            result, finder = self._datasrc_client.find_zone(self._zone_name)
+            if result != DataSourceClient.SUCCESS:
+                raise XfrinException('Zone not found in the given data ' +
+                                     'source: ' + self.zone_str())
+            result, soa_rrset = finder.find(self._zone_name, RRType.SOA(),
+                                            None, ZoneFinder.FIND_DEFAULT)
+            if result != ZoneFinder.SUCCESS:
+                raise XfrinException('SOA RR not found in zone: ' +
+                                     self.zone_str())
+            # Especially for database-based zones, a working zone may be in
+            # a broken state where it has more than one SOA RR.  We proactively
+            # check the condition and abort the xfr attempt if we identify it.
+            if soa_rrset.get_rdata_count() != 1:
+                raise XfrinException('Invalid number of SOA RRs for ' +
+                                     self.zone_str() + ': ' +
+                                     str(soa_rrset.get_rdata_count()))
+            msg.add_rrset(Message.SECTION_AUTHORITY, soa_rrset)
+            self._request_serial = get_soa_serial(soa_rrset.get_rdata()[0])
         return msg
 
     def _send_data(self, data):
@@ -256,39 +600,61 @@ class XfrinConnection(asyncore.dispatcher):
         # now.
         return XFRIN_OK
 
-    def do_xfrin(self, check_soa, ixfr_first = False):
-        '''Do xfr by sending xfr request and parsing response. '''
+    def do_xfrin(self, check_soa, request_type=RRType.AXFR()):
+        '''Do an xfr session by sending xfr request and parsing responses.'''
 
         try:
             ret = XFRIN_OK
+            self._request_type = request_type
+            # Right now RRType.[IA]XFR().to_text() is 'TYPExxx', so we need
+            # to hardcode here.
+            request_str = 'IXFR' if request_type == RRType.IXFR() else 'AXFR'
             if check_soa:
-                logstr = 'SOA check for \'%s\' ' % self._zone_name
                 ret =  self._check_soa_serial()
 
             if ret == XFRIN_OK:
-                logger.info(XFRIN_AXFR_TRANSFER_STARTED, self._zone_name)
-                self._send_query(RRType.AXFR())
-                isc.datasrc.sqlite3_ds.load(self._db_file, self._zone_name,
-                                            self._handle_xfrin_response)
-
-                logger.info(XFRIN_AXFR_TRANSFER_SUCCESS, self._zone_name)
-
-        except XfrinException as e:
-            logger.error(XFRIN_AXFR_TRANSFER_FAILURE, self._zone_name, str(e))
+                logger.info(XFRIN_XFR_TRANSFER_STARTED, request_str,
+                            self.zone_str())
+                if self._request_type == RRType.IXFR():
+                    self._request_type = RRType.IXFR()
+                    self._send_query(self._request_type)
+                    self.__state = XfrinInitialSOA()
+                    self._handle_xfrin_responses()
+                else:
+                    self._send_query(self._request_type)
+                    isc.datasrc.sqlite3_ds.load(self._db_file,
+                                                self._zone_name.to_text(),
+                                                self._handle_axfrin_response)
+                logger.info(XFRIN_XFR_TRANSFER_SUCCESS, request_str,
+                            self.zone_str())
+
+        except (XfrinException, XfrinProtocolError) as e:
+            logger.error(XFRIN_XFR_TRANSFER_FAILURE, request_str,
+                         self.zone_str(), str(e))
             ret = XFRIN_FAIL
-            #TODO, recover data source.
         except isc.datasrc.sqlite3_ds.Sqlite3DSError as e:
-            logger.error(XFRIN_AXFR_DATABASE_FAILURE, self._zone_name, str(e))
+            # Note: this is old code and used only for AXFR.  This will be
+            # soon removed anyway, so we'll leave it.
+            logger.error(XFRIN_AXFR_DATABASE_FAILURE, self.zone_str(), str(e))
             ret = XFRIN_FAIL
-        except UserWarning as e:
-            # XXX: this is an exception from our C++ library via the
-            # Boost.Python binding.  It would be better to have more more
-            # specific exceptions, but at this moment this is the finest
-            # granularity.
-            logger.error(XFRIN_AXFR_INTERNAL_FAILURE, self._zone_name, str(e))
+        except Exception as e:
+            # Catching all possible exceptions like this is generally not a
+            # good practice, but handling an xfr session could result in
+            # so many types of exceptions, including ones from the DNS library
+            # or from the data source library.  Eventually we'd introduce a
+            # hierarchy for exception classes from a base "ISC exception" and
+            # catch it here, but until then we need broadest coverage so that
+            # we won't miss anything.
+
+            logger.error(XFRIN_XFR_OTHER_FAILURE, request_str,
+                         self.zone_str(), str(e))
             ret = XFRIN_FAIL
         finally:
-           self.close()
+            # Make sure any remaining transaction in the diff is closed
+            # (if not yet - possible in case of xfr-level exception) as soon
+            # as possible
+            self._diff = None
+            self.close()
 
         return ret
 
@@ -351,7 +717,32 @@ class XfrinConnection(asyncore.dispatcher):
                 yield (rrset_name, rrset_ttl, rrset_class, rrset_type,
                        rdata_text)
 
-    def _handle_xfrin_response(self):
+    def _handle_xfrin_responses(self):
+        read_next_msg = True
+        while read_next_msg:
+            data_len = self._get_request_response(2)
+            msg_len = socket.htons(struct.unpack('H', data_len)[0])
+            recvdata = self._get_request_response(msg_len)
+            msg = Message(Message.PARSE)
+            msg.from_wire(recvdata, Message.PRESERVE_ORDER)
+
+            # TSIG related checks, including an unexpected signed response
+            self._check_response_tsig(msg, recvdata)
+
+            # Perform response status validation
+            self._check_response_status(msg)
+
+            for rr in msg.get_section(Message.SECTION_ANSWER):
+                rr_handled = False
+                while not rr_handled:
+                    rr_handled = self.__state.handle_rr(self, rr)
+
+            read_next_msg = self.__state.finish_message(self)
+
+            if self._shutdown_event.is_set():
+                raise XfrinException('xfrin is forced to stop')
+
+    def _handle_axfrin_response(self):
         '''Return a generator for the response to a zone transfer. '''
         while True:
             data_len = self._get_request_response(2)
@@ -394,15 +785,27 @@ class XfrinConnection(asyncore.dispatcher):
 
 def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file,
                   shutdown_event, master_addrinfo, check_soa, verbose,
-                  tsig_key):
+                  tsig_key, request_type):
     xfrin_recorder.increment(zone_name)
+
+    # Create a data source client used in this XFR session.  Right now we
+    # still assume an sqlite3-based data source, and use both the old and new
+    # data source APIs.  We also need to use a mock client for tests.
+    # For a temporary workaround to deal with these situations, we skip the
+    # creation when the given file is none (the test case).  Eventually
+    # this code will be much cleaner.
+    datasrc_client = None
+    if db_file is not None:
+        datasrc_client = DataSourceClient(db_file)
+
+    # Create a TCP connection for the XFR session and perform the operation.
     sock_map = {}
-    conn = XfrinConnection(sock_map, zone_name, rrclass, db_file,
-                           shutdown_event, master_addrinfo,
+    conn = XfrinConnection(sock_map, zone_name, rrclass, datasrc_client,
+                           db_file, shutdown_event, master_addrinfo,
                            tsig_key, verbose)
     ret = XFRIN_FAIL
     if conn.connect_to_master():
-        ret = conn.do_xfrin(check_soa)
+        ret = conn.do_xfrin(check_soa, request_type)
 
     # Publish the zone transfer result news, so zonemgr can reset the
     # zone timer, and xfrout can notify the zone's slaves if the result
@@ -646,7 +1049,7 @@ class Xfrin:
                                            rrclass,
                                            self._get_db_file(),
                                            master_addr,
-                                           zone_info.tsig_key,
+                                           zone_info.tsig_key, RRType.AXFR(),
                                            True)
                     answer = create_answer(ret[0], ret[1])
 
@@ -659,14 +1062,17 @@ class Xfrin:
                                                           rrclass)
                 zone_info = self._get_zone_info(zone_name, rrclass)
                 tsig_key = None
+                request_type = RRType.AXFR()
                 if zone_info:
                     tsig_key = zone_info.tsig_key
+                    if not zone_info.ixfr_disabled:
+                        request_type = RRType.IXFR()
                 db_file = args.get('db_file') or self._get_db_file()
                 ret = self.xfrin_start(zone_name,
                                        rrclass,
                                        db_file,
                                        master_addr,
-                                       tsig_key,
+                                       tsig_key, request_type,
                                        (False if command == 'retransfer' else True))
                 answer = create_answer(ret[0], ret[1])
 
@@ -746,7 +1152,8 @@ class Xfrin:
         news(command: zone_new_data_ready) to zone manager and xfrout.
         if xfrin failed, just tell the bad news to zone manager, so that
         it can reset the refresh timer for that zone. '''
-        param = {'zone_name': zone_name, 'zone_class': zone_class.to_text()}
+        param = {'zone_name': zone_name.to_text(),
+                 'zone_class': zone_class.to_text()}
         if xfr_result == XFRIN_OK:
             msg = create_command(notify_out.ZONE_NEW_DATA_READY_CMD, param)
             # catch the exception, in case msgq has been killed.
@@ -783,8 +1190,8 @@ class Xfrin:
         while not self._shutdown_event.is_set():
             self._cc_check_command()
 
-    def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo, tsig_key,
-                    check_soa = True):
+    def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo,
+                    tsig_key, request_type, check_soa=True):
         if "pydnspp" not in sys.modules:
             return (1, "xfrin failed, can't load dns message python library: 'pydnspp'")
 
@@ -798,13 +1205,13 @@ class Xfrin:
         xfrin_thread = threading.Thread(target = process_xfrin,
                                         args = (self,
                                                 self.recorder,
-                                                zone_name.to_text(),
+                                                zone_name,
                                                 rrclass,
                                                 db_file,
                                                 self._shutdown_event,
                                                 master_addrinfo, check_soa,
                                                 self._verbose,
-                                                tsig_key))
+                                                tsig_key, request_type))
 
         xfrin_thread.start()
         return (0, 'zone xfrin is started')

src/bin/xfrin/xfrin_messages.mes

@@ -15,25 +15,26 @@
 # No namespace declaration - these constants go in the global namespace
 # of the xfrin messages python module.
 
-% XFRIN_AXFR_INTERNAL_FAILURE AXFR transfer of zone %1 failed: %2
-The AXFR transfer for the given zone has failed due to an internal
-problem in the bind10 python wrapper library.
-The error is shown in the log message.
+% XFRIN_XFR_OTHER_FAILURE %1 transfer of zone %2 failed: %3
+The XFR transfer for the given zone has failed due to a problem outside
+of the xfrin module.  Possible reasons are a broken DNS message or failure
+in database connection.  The error is shown in the log message.
 
 % XFRIN_AXFR_DATABASE_FAILURE AXFR transfer of zone %1 failed: %2
 The AXFR transfer for the given zone has failed due to a database problem.
-The error is shown in the log message.
+The error is shown in the log message.  Note: due to the code structure
+this can only happen for AXFR.
 
-% XFRIN_AXFR_TRANSFER_FAILURE AXFR transfer of zone %1 failed: %2
-The AXFR transfer for the given zone has failed due to a protocol error.
+% XFRIN_XFR_TRANSFER_FAILURE %1 transfer of zone %2 failed: %3
+The XFR transfer for the given zone has failed due to a protocol error.
 The error is shown in the log message.
 
-% XFRIN_AXFR_TRANSFER_STARTED AXFR transfer of zone %1 started
+% XFRIN_XFR_TRANSFER_STARTED %1 transfer of zone %2 started
 A connection to the master server has been made, the serial value in
 the SOA record has been checked, and a zone transfer has been started.
 
-% XFRIN_AXFR_TRANSFER_SUCCESS AXFR transfer of zone %1 succeeded
-The AXFR transfer of the given zone was successfully completed.
+% XFRIN_XFR_TRANSFER_SUCCESS %1 transfer of zone %2 succeeded
+The XFR transfer of the given zone was successfully completed.
 
 % XFRIN_BAD_MASTER_ADDR_FORMAT bad format for master address: %1
 The given master address is not a valid IP address.
@@ -89,3 +90,12 @@ daemon will now shut down.
 % XFRIN_UNKNOWN_ERROR unknown error: %1
 An uncaught exception was raised while running the xfrin daemon. The
 exception message is printed in the log message.
+
+% XFRIN_GOT_INCREMENTAL_RESP got incremental response for %1
+In an attempt of IXFR processing, the begenning SOA of the first difference
+(following the initial SOA that specified the final SOA for all the
+differences) was found.  This means a connection for xfrin tried IXFR
+and really aot a response for incremental updates.
+
+% XFRIN_GOT_NONINCREMENTAL_RESP got nonincremental response for %1
+TBD

src/lib/datasrc/sqlite3_accessor.cc

@@ -641,8 +641,12 @@ doUpdate(SQLite3Parameters& dbparams, StatementID stmt_id,
     const size_t column_count =
         sizeof(update_params) / sizeof(update_params[0]);
     for (int i = 0; i < column_count; ++i) {
-        if (sqlite3_bind_text(stmt, ++param_id, update_params[i].c_str(), -1,
-                              SQLITE_TRANSIENT) != SQLITE_OK) {
+        // The old sqlite3 data source API assumes NULL for an empty column.
+        // We need to provide compatibility at least for now.
+        if (sqlite3_bind_text(stmt, ++param_id,
+                              update_params[i].empty() ? NULL :
+                              update_params[i].c_str(),
+                              -1, SQLITE_TRANSIENT) != SQLITE_OK) {
             isc_throw(DataSourceError, "failed to bind SQLite3 parameter: " <<
                       sqlite3_errmsg(dbparams.db_));
         }

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

@@ -93,7 +93,7 @@ class Diff:
         """
         Schedules an operation with rr.
 
-        It does all the real work of add_data and remove_data, including
+        It does all the real work of add_data and delete_data, including
         all checks.
         """
         self.__check_commited()
@@ -122,9 +122,9 @@ class Diff:
         """
         self.__data_common(rr, 'add')
 
-    def remove_data(self, rr):
+    def delete_data(self, rr):
         """
-        Schedules removal of an RR from the zone in this diff.
+        Schedules deleting an RR from the zone in this diff.
 
         The rr is of isc.dns.RRset type and it must contain only one RR.
         If this is not the case or if the diff was already commited, this
@@ -133,7 +133,7 @@ class Diff:
         The rr class must match the one of the datasource client. If
         it does not, ValueError is raised.
         """
-        self.__data_common(rr, 'remove')
+        self.__data_common(rr, 'delete')
 
     def compact(self):
         """
@@ -189,8 +189,8 @@ class Diff:
             for (operation, rrset) in self.__buffer:
                 if operation == 'add':
                     self.__updater.add_rrset(rrset)
-                elif operation == 'remove':
-                    self.__updater.remove_rrset(rrset)
+                elif operation == 'delete':
+                    self.__updater.delete_rrset(rrset)
                 else:
                     raise ValueError('Unknown operation ' + operation)
             # As everything is already in, drop the buffer
@@ -219,15 +219,15 @@ class Diff:
             # Remove the updater. That will free some resources for one, but
             # mark this object as already commited, so we can check
 
-            # We remove it even in case the commit failed, as that makes us
+            # We delete it even in case the commit failed, as that makes us
             # unusable.
             self.__updater = None
 
     def get_buffer(self):
         """
         Returns the current buffer of changes not yet passed into the data
-        source. It is in a form like [('add', rrset), ('remove', rrset),
-        ('remove', rrset), ...].
+        source. It is in a form like [('add', rrset), ('delete', rrset),
+        ('delete', rrset), ...].
 
         Probably useful only for testing and introspection purposes. Don't
         modify the list.

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

@@ -112,12 +112,12 @@ class DiffTest(unittest.TestCase):
         """
         self.__data_operations.append(('add', rrset))
 
-    def remove_rrset(self, rrset):
+    def delete_rrset(self, rrset):
         """
         This one is part of pretending to be a zone updater. It writes down
         removal of an rrset was requested.
         """
-        self.__data_operations.append(('remove', rrset))
+        self.__data_operations.append(('delete', rrset))
 
     def get_class(self):
         """
@@ -162,7 +162,7 @@ class DiffTest(unittest.TestCase):
 
     def __data_common(self, diff, method, operation):
         """
-        Common part of test for test_add and test_remove.
+        Common part of test for test_add and test_delte.
         """
         # Try putting there the bad data first
         self.assertRaises(ValueError, method, self.__rrset_empty)
@@ -188,7 +188,7 @@ class DiffTest(unittest.TestCase):
         diff = Diff(self, Name('example.org.'))
         self.__data_common(diff, diff.add_data, 'add')
 
-    def test_remove(self):
+    def test_delete(self):
         """
         Try scheduling removal of few items into the diff and see they are
         stored in there.
@@ -196,7 +196,7 @@ class DiffTest(unittest.TestCase):
         Also try passing an rrset that has different amount of RRs than 1.
         """
         diff = Diff(self, Name('example.org.'))
-        self.__data_common(diff, diff.remove_data, 'remove')
+        self.__data_common(diff, diff.delete_data, 'delete')
 
     def test_apply(self):
         """
@@ -206,8 +206,8 @@ class DiffTest(unittest.TestCase):
         # Prepare the diff
         diff = Diff(self, Name('example.org.'))
         diff.add_data(self.__rrset1)
-        diff.remove_data(self.__rrset2)
-        dlist = [('add', self.__rrset1), ('remove', self.__rrset2)]
+        diff.delete_data(self.__rrset2)
+        dlist = [('add', self.__rrset1), ('delete', self.__rrset2)]
         self.assertEqual(dlist, diff.get_buffer())
         # Do the apply, hook the compact method
         diff.compact = self.__mock_compact
@@ -241,7 +241,7 @@ class DiffTest(unittest.TestCase):
         # Now check all range of other methods raise ValueError
         self.assertRaises(ValueError, diff.commit)
         self.assertRaises(ValueError, diff.add_data, self.__rrset2)
-        self.assertRaises(ValueError, diff.remove_data, self.__rrset1)
+        self.assertRaises(ValueError, diff.delete_data, self.__rrset1)
         diff.apply = orig_apply
         self.assertRaises(ValueError, diff.apply)
         # This one does not state it should raise, so check it doesn't
@@ -278,14 +278,14 @@ class DiffTest(unittest.TestCase):
         # Reset the buffer by calling the original apply.
         orig_apply()
         self.assertEqual([], diff.get_buffer())
-        # Similar with remove
+        # Similar with delete
         self.__apply_called = False
         for i in range(0, 99):
-            diff.remove_data(self.__rrset2)
-        expected = [('remove', self.__rrset2)] * 99
+            diff.delete_data(self.__rrset2)
+        expected = [('delete', self.__rrset2)] * 99
         self.assertEqual(expected, diff.get_buffer())
         self.assertFalse(self.__apply_called)
-        diff.remove_data(self.__rrset2)
+        diff.delete_data(self.__rrset2)
         self.assertTrue(self.__apply_called)
 
     def test_compact(self):
@@ -310,9 +310,9 @@ class DiffTest(unittest.TestCase):
             # Different type.
             ('add', 'a', 'AAAA', ['2001:db8::1', '2001:db8::2']),
             # Different operation
-            ('remove', 'a', 'AAAA', ['2001:db8::3']),
+            ('delete', 'a', 'AAAA', ['2001:db8::3']),
             # Different domain
-            ('remove', 'b', 'AAAA', ['2001:db8::4']),
+            ('delete', 'b', 'AAAA', ['2001:db8::4']),
             # This does not get merged with the first, even if logically
             # possible. We just don't do this.
             ('add', 'a', 'A', ['192.0.2.3'])
@@ -327,7 +327,7 @@ class DiffTest(unittest.TestCase):
                 if op == 'add':
                     diff.add_data(rrset)
                 else:
-                    diff.remove_data(rrset)
+                    diff.delete_data(rrset)
         # Compact it
         diff.compact()
         # Now check they got compacted. They should be in the same order as
@@ -363,7 +363,7 @@ class DiffTest(unittest.TestCase):
                       self.__ttl)
         rrset.add_rdata(Rdata(RRType.NS(), RRClass.CH(), 'ns.example.org.'))
         self.assertRaises(ValueError, diff.add_data, rrset)
-        self.assertRaises(ValueError, diff.remove_data, rrset)
+        self.assertRaises(ValueError, diff.delete_data, rrset)
 
     def __do_raise_test(self):
         """
@@ -372,11 +372,11 @@ class DiffTest(unittest.TestCase):
         """
         diff = Diff(self, Name('example.org.'))
         diff.add_data(self.__rrset1)
-        diff.remove_data(self.__rrset2)
+        diff.delete_data(self.__rrset2)
         self.assertRaises(TestError, diff.commit)
         self.assertTrue(self.__broken_called)
         self.assertRaises(ValueError, diff.add_data, self.__rrset1)
-        self.assertRaises(ValueError, diff.remove_data, self.__rrset2)
+        self.assertRaises(ValueError, diff.delete_data, self.__rrset2)
         self.assertRaises(ValueError, diff.commit)
         self.assertRaises(ValueError, diff.apply)
 
@@ -388,12 +388,12 @@ class DiffTest(unittest.TestCase):
         self.add_rrset = self.__broken_operation
         self.__do_raise_test()
 
-    def test_raise_remove(self):
+    def test_raise_delete(self):
         """
-        Test the exception from remove_rrset is propagated and the diff can't be
+        Test the exception from delete_rrset is propagated and the diff can't be
         used afterwards.
         """
-        self.remove_rrset = self.__broken_operation
+        self.delete_rrset = self.__broken_operation
         self.__do_raise_test()
 
     def test_raise_commit(self):