BIND 10 #439: Implement normal query processing

[BIND 10 Development]

#439: Implement normal query processing
-------------------------------+--------------------------------------------
      Reporter:  zzchen_pku    |        Owner:  zzchen_pku
          Type:  task          |       Status:  reviewing 
      Priority:  major         |    Milestone:            
     Component:  Unclassified  |   Resolution:            
      Keywords:                |    Sensitive:  0         
Estimatedhours:  0.0           |        Hours:  0         
      Billable:  1             |   Totalhours:  0         
      Internal:  0             |  
-------------------------------+--------------------------------------------
Changes (by jinmei):

  * owner:  jinmei => zzchen_pku


Comment:

 A few initial things:
  - the important part (successful zone->find()) isn't tested.  even if we
 don't yet have a complete find() implementation of the real MemoryZone
 class, we can test the case using a mock Zone class for testing (and we
 should do so).  Such a mock class would simply hardcode the results, e.g,
 returning NXDOMAIN for nxdomain.example.com; returning CNAME for
 cname.example.com, etc.  In fact, even if we complete the
 MemoryZone::find() work, it would still make sense to test the Query
 behavior using a mock because we can then omit loading a zone file and
 won't be bothered with possible bugs in the MemoryZone implementation.
  - as a related note, we should primarily write tests for the Query class
 in query_unittest.cc, not in auth_srv.cc.  Then we don't even have to
 create a server object for the testing.  In general, a class should be
 tested with minimal dependencies.  we'll still need a test for the AuthSrv
 to check if the new query logic is used when the memory data source is
 configured, but that test can (should) be much simpler in terms of query
 processing.
  - regarding pimpl: while I'm generally a fan (or advocate) of pimpl, in
 this particular case we might want to avoid that.  The Query class would
 be instantiated for every query in a performance sensitive path, and the
 construction/destruction overhead due to new/delete with the pimpl may be
 substantial.  This concern may still be a matter of premature
 optimization, so if there's a reason that can make the code (e.g.) more
 robust, I'm okay with keeping the pimpl approach.  In that case please
 leave comments about performance consideration.
  - this code is NO-OP:
 {{{
                 isc_throw(Unexpected,
                           "Zone::find return unexpected result.");
 }}}
   Note: we should have been able to avoid this type of trivial error with
 TDD.

 BTW: I may not be able to take on further review once these points are
 addressed as I've already committed to several other things (related to
 BIND 10 development).  Maybe you want to contact Michal if he's available.

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