BIND 10 #1262: AXFR-style IXFR-in protocol handling

[BIND 10 Development]

#1262: AXFR-style IXFR-in protocol handling
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20111011
  blocker                            |            Resolution:
                  Component:  xfrin  |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 I have some points to discuss that may be debatable.  To save time,
 however, I made suggested changes on the branch.  If you agree with
 the sense of the change and the changes themselves, I have no further
 issue.  As soon as #1261 is merged, this branch can be merged too.

 If you don't agree or do want to make more changes, please respond
 (the committed changes were just a suggestion, not a requiremnet).

 Postponing other things is okay for me.

 Specific comments and suggestions below:

 '''XfrinAXFR class'''

 I'd use conn._diff instead of an attribute of XfrinAXFR class for the
 Diff object of AXFR.  One reason is that XfrinState classes are
 expected to be "stateless" (so they wouldn't have a mutable member
 attribute).  More important reason is that I'd delay the commit until
 XfrinAXFREnd.finish_message() (which is the topic of the next
 paragraph), and to keep the diff over multiple states it would be
 easier to hold it in XfrinConn (as indicated in your comment).

 Now, as for delaying commit.  In case of AXFR, I believe we should
 perform some more stricter checks on the received zone, e.g., whether
 the zone has an NS RR before actually publishing the zone (BIND 9
 behaves that way).  We could do this at the end of the XfrinAXFR
 state, but if we do this check I'd also include other checks such
 as whether the message doesn't contain a bogus trailing RR.

 (Note also that to perform such check we need to ensure all buffered
 changes in Diff are really pushed to the data source before actually
 committing the transaction.  So we'll need some extension to the
 Diff object, or we may simply want to use an updater directly for
 AXFR.   But that should be beyond the scope of this task)

 I made my proposed change based on this.  It's commit

 '''xfrin_test'''

 I would add some higher level tests, i.e., AXFR-style versions of
 TestIXFRSession and TestIXFRSessionWithSQLite3.  I added some
 suggested tests to the branch.

-- 
Ticket URL: <http://bind10.isc.org/ticket/1262#comment:6>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development