BIND 10 #439: Implement normal query processing
BIND 10 Development
do-not-reply at isc.org
Thu Dec 23 05:22:35 UTC 2010
#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 |
-------------------------------+--------------------------------------------
Comment(by zzchen_pku):
Replying to [comment:2 jinmei]:
> 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.
Updated, add a MockZone class for testing.
> - 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.
Done.
> - 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.
Agree, updated.
> - 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.
Removed.
> 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.
Okay, thank you for your comments.
--
Ticket URL: <http://bind10.isc.org/ticket/439#comment:3>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list