BIND 10 #2975: Create initial DnsClient class

[BIND 10 Development]

#2975: Create initial DnsClient class
-------------------------------------+-------------------------------------
            Reporter:  tmark         |                        Owner:
                Type:  enhancement   |  UnAssigned
            Priority:  medium        |                       Status:
           Component:  dhcp-ddns     |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130717
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  0             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by marcin):

 * owner:  tmark => UnAssigned


Comment:

 Replying to [comment:8 tmark]:
 > Review Comments:
 >
 > Really I only have a few comments.  Good job on this.
 >
 > General:
 >
 > * From a naming standpoint it is somewhat confusing for DNSClient to
 derive from IOFetch::Callback and have an embedded class named Callback
 and member callback_.
 >
 > Perhaps changing the name of the instance member from callback_ to
 client_callback_  or external_callback_.  You could call
 DNSClient::Callback something like !ResponseHandler.

 This has been addressed with #2977. The !DNSClient class uses pimpl idiom
 to separate implementation from the interface. As a result, the
 implementation class derives from IOFetch::Callback and this is not
 visible to a caller.

 >
 > * Question: - " Caller must provide a callback, which will be invoked
 when the response from the DNS server is received, a timeout has occured
 or IO service has been stopped for any
 > reason. "
 >
 > What if the address given is bad, or there is a network error, like a
 disconnect of some kind other than timeout?  I notice the IOFetch::Result
 doesn't provide a value for this.   "Stopped" should only happen if the
 IOService is stopped which an IO error should not cause.

 Well, this is a very good question. I analysed the IOFetch code and all
 socket functions that it calls. It appears to me that the server's address
 is only used to determine the address family for the socket: V4 or V6. So,
 when the socket is being open there should be no exception thrown, even if
 address if wrong. In such case, we will hit the timeout which has a
 corresponding error code. If however, there is an exception in the IOFetch
 it would propagate to the IOService::run(). Therefore, it is up to the
 IOService::run caller to handle it.

 At some point we may find that current error codes may be not sufficient.
 It shouldn't be a real problem to extend the set of error codes and
 slightly modify implementation of !DNSClient.

 >
 > * Am guessing the @todo for DNSClient::operator() is being done under
 2977?

 Yes. it has been done with #2977.

 >
 > * As per our discussion, providing a protocol selection parameter for
 TCP/UDP would be good, and we need to think about TSIG as well.  Meaning
 that, perhaps we need a variant of update which accepts a TSIGKey
 parameter?  Initial release will not implement actual TSIG,but the
 interface could accommodate it.

 I added an extra constructor parameter which specifies user's preferrence
 regarding Transport protocol. Also, I added a variant of !doUpdate which
 takes !TSIGKey.

 >
 > * Minor pick - spell check, Marcin.  You have several misspellings in
 the commentary.  I use GVIM to do my spell checking ;)

 I hate VIM, but fortunately emacs has a support for spell checks using
 ispell :-) I corrected all !''occurs!'' and some typos.

 >
 >
 > * I think we need a !ChangeLog entry for this.

 Here it is:
 {{{
 6XX.    [func]          marcin
         b10-dhcp-ddns: Implemented DNSClient class which implements
         asynchronous DNS updates using UDP. The TCP and TSIG support
         will be implemented at later time. Nevertheless, class API
         accomodates the use of TCP and TSIG.
         (Trac #2975, git abc)
         (Trac #2977, git abc)

 }}}

 >
 >
 > Also I have taken the liberty to attach a class diagram for you:
 >
 > [[Image(dns_client.svg)]]
 >
 >

 Note that the review comments raised here have been addressed on trac2977
 branch (not trac2975).

 Thanks for the useful comments.

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