BIND 10 #1793: update b10-auth command handler to reload in-memory/sqlite3

[BIND 10 Development]

#1793: update b10-auth command handler to reload in-memory/sqlite3
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120501
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  b10-auth                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  4
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:  xfr    |
  for in-memory                      |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 '''general'''

 I don't know for which tests we need the hack, but I think it's okay
 for now due to the observation that it shouldn't be a long term thing
 (although we have too many such things:-).

 I think multiple class-IN cases can be ignored, too.  We should
 clarify that in the general configuration framework.

 '''command.cc'''

 - I'd suggest separating the part that looks for the zone config from
   validate() into a separate method, say, getZoneConfig() mainly
   because validate() is getting quite long and approaching an
   unreadable level.  Also, according to the assumption that the zone
   configuration should have been already validated and the fact that
   getting the old finder succeeds, this is actually beyond
   "validation".  In that sense, we might even assert() the checks than
   throwing for unexpected results (although it would probably break
   some test cases).

 - minor nit: we don't insert a space between `++` and `i`:
 {{{#!cpp
         for (size_t i(0); i < config->size(); ++ i) {
 }}}
   according to http://bind10.isc.org/wiki/BIND9CodingGuidelines
   (see "Spaces" and look for `++`)

 '''command_unittest'''

 - SPEC_FILE is used only for loadZoneSQLite3 so it doesn't have to be
   defined in the namespace scope.

 - in loadZoneSQLite3, maybe you intended to add comment like
 {{{#!cpp
     // The previous zone is not hurt in any way
 }}}
   for the EXPECT_EQ below?
 {{{#!cpp
     // Some error cases. First, the zone has no configuration.
     dsrc->addZone(ZoneFinderPtr(new InMemoryZoneFinder(RRClass::IN(),
 Name("example.com"))));
     result_ = execAuthServerCommand(server_, "loadzone",
         Element::fromJSON("{\"origin\": \"example.com\"}"));
     checkAnswer(1);

     EXPECT_EQ(ZoneFinder::SUCCESS,
 server_.getInMemoryClient(RRClass::IN())->
 }}}

 - btw (not a comment or request to the branch itself), the fact that
   we need to bother to mention things like below is one of my
   motivations for suggesting the separation of the communication
   module and the config data class:
 {{{#!cpp
     // Then store a config of the zone to the auth server
     // This omits many config options of the auth server, but these are
     // not read now.
     isc::testutils::MockSession session;
     // The session should not take care of anything or start anything, we
     // need it only to hold the config we're going to put into it.
     ModuleCCSession moduleSession(SPEC_FILE, session, NULL, NULL, false,
                                   false);
 }}}

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