BIND 10 #2378: define and implement datasrc::ZoneLoader class

[BIND 10 Development]

#2378: define and implement datasrc::ZoneLoader class
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  vorner
            Priority:  medium        |                       Status:
           Component:  data source   |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20121218
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  4             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  loadzone-ng
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => vorner


Comment:

 I have no problems with the doxygen changes, but we may want to swing it
 past Jeremy (and/or merge it separately).

 Also I'm assuming 2377 is ok for the purposes of this review (i noticed
 make cppcheck complained about one thing in 2377).

 I pushed a few small fixes and changes to comments, please check :)

 zone_loader.h:

 We may want to add a small note that loadIncremental does not return
 'complete' yet if you read the exact number of rrs that are in the source
 (because end-of-iterator or end-of-file is only triggered upon the next
 read).

 May be a matter of taste, but from the looks of it, loadIncremental could
 simply put 'return (complete_);' at the end, instead of separate returns
 for each branch. And following that, it could also pull out the commit,
 making it
 {{{
     if (iterator_ == ZoneIteratorPtr()) {
         assert(loader_.get() != NULL);
         complete_ = loader_->loadIncremental(limit);
         if (complete_ && !loaded_ok_) {
             isc_throw(MasterFileError, "Error while loading master file");
         }
     } else {
         complete_ = copyRRsets(updater_, iterator_, limit);
     }
     if (complete_) {
         updater_->commit();
     }
     return (complete_);
 }}}

 (again, just a matter of taste, saves 8 lines :p) Though it would set
 complete_ if updater_->commit() throws (which may or may not be
 intentional, right now that depends on the type of loader, in which case
 my suggestion is to put them both before or after the commit :))

 Oh and I found one other corner case (though this should probably belong
 to 2377): loading an empty file succeeds, and I think it should fail
 ('empty as in no rrsets, and mainly, no SOA). Not sure if this would be
 considered something validate() would do, just noting that currently it is
 accepted :)

 For the rest it looks good

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