From ca87835d1ec6e3393a98dcad72b6c96d43331e39 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Wed, 3 Jul 2013 11:48:01 +0200 Subject: [PATCH] [2977] Separated DNSClient interface from implementation. --- src/bin/d2/dns_client.cc | 106 ++++++++++++++++++++--- src/bin/d2/dns_client.h | 41 ++++----- src/bin/d2/tests/dns_client_unittests.cc | 16 ++-- 3 files changed, 117 insertions(+), 46 deletions(-) diff --git a/src/bin/d2/dns_client.cc b/src/bin/d2/dns_client.cc index 38d80c34cd..3ab8a91a67 100644 --- a/src/bin/d2/dns_client.cc +++ b/src/bin/d2/dns_client.cc @@ -31,8 +31,44 @@ using namespace isc::util; using namespace isc::asiolink; using namespace isc::asiodns; -DNSClient::DNSClient(D2UpdateMessagePtr& response_placeholder, - Callback* callback) +// This class provides the implementation for the DNSClient. This allows to +// the separation of the DNSClient interface from the implementation details. +// Currently, implementation uses IOFetch object to handle asynchronous +// communication with the DNS. This design may be revisited in the future. If +// implementation is changed, the DNSClient API will remain unchanged thanks +// to this separation. +class DNSClientImpl : public asiodns::IOFetch::Callback { +public: + // A buffer holding response from a DNS. + util::OutputBufferPtr in_buf_; + // A caller-supplied object holding a parsed response from DNS. + D2UpdateMessagePtr response_; + // A caller-supplied external callback which is invoked when DNS message + // exchange is complete or interrupted. + DNSClient::Callback* callback_; + + DNSClientImpl(D2UpdateMessagePtr& response_placeholder, + DNSClient::Callback* callback); + virtual ~DNSClientImpl(); + + // This internal callback is called when the DNS update message exchange is + // complete. It further invokes the external callback provided by a caller. + // Before external callback is invoked, an object of the D2UpdateMessage + // type, representing a response from the server is set. + virtual void operator()(asiodns::IOFetch::Result result); + + void doUpdate(asiolink::IOService& io_service, + const asiolink::IOAddress& ns_addr, + const uint16_t ns_port, + D2UpdateMessage& update, + const int wait = -1); + + // This function maps the IO error to the DNSClient error. + DNSClient::Status getStatus(const asiodns::IOFetch::Result); +}; + +DNSClientImpl::DNSClientImpl(D2UpdateMessagePtr& response_placeholder, + DNSClient::Callback* callback) : in_buf_(new OutputBuffer(DEFAULT_BUFFER_SIZE)), response_(response_placeholder), callback_(callback) { if (!response_) { @@ -41,29 +77,56 @@ DNSClient::DNSClient(D2UpdateMessagePtr& response_placeholder, } } +DNSClientImpl::~DNSClientImpl() { +} + void -DNSClient::operator()(IOFetch::Result result) { +DNSClientImpl::operator()(asiodns::IOFetch::Result result) { // @todo More sanity checks here. Also, there is a question, what happens if // the exception is thrown here. - if (result == IOFetch::SUCCESS) { + DNSClient::Status status = getStatus(result); + + if (status == DNSClient::SUCCESS) { InputBuffer response_buf(in_buf_->getData(), in_buf_->getLength()); - response_->fromWire(response_buf); + try { + response_->fromWire(response_buf); + } catch (const Exception& ex) { + status = DNSClient::INVALID_RESPONSE; + } } // Once we are done with internal business, let's call a callback supplied // by a caller. if (callback_ != NULL) { - (*callback_)(result); + (*callback_)(status); } } +DNSClient::Status +DNSClientImpl::getStatus(const asiodns::IOFetch::Result result) { + switch (result) { + case IOFetch::SUCCESS: + return (DNSClient::SUCCESS); + + case IOFetch::TIME_OUT: + return (DNSClient::TIMEOUT); + + case IOFetch::STOPPED: + return (DNSClient::IO_STOPPED); + + default: + ; + } + return (DNSClient::OTHER); +} + void -DNSClient::doUpdate(IOService& io_service, - const IOAddress& ns_addr, - const uint16_t ns_port, - D2UpdateMessage& update, - const int wait) { +DNSClientImpl::doUpdate(IOService& io_service, + const IOAddress& ns_addr, + const uint16_t ns_port, + D2UpdateMessage& update, + const int wait) { // A renderer is used by the toWire function which creates the on-wire data // from the DNS Update message. A renderer has its internal buffer where it // renders data by default. However, this buffer can't be directly accessed. @@ -81,7 +144,7 @@ DNSClient::doUpdate(IOService& io_service, // IOFetch has all the mechanisms that we need to perform asynchronous // communication with the DNS server. The last but one argument points to // this object as a completion callback for the message exchange. As a - // result operator()(IOFetch::Result) will be called. + // result operator()(Status) will be called. IOFetch io_fetch(IOFetch::UDP, io_service, msg_buf, ns_addr, ns_port, in_buf_, this, wait); // Post the task to the task queue in the IO service. Caller will actually @@ -90,6 +153,25 @@ DNSClient::doUpdate(IOService& io_service, } +DNSClient::DNSClient(D2UpdateMessagePtr& response_placeholder, + Callback* callback) + : impl_(new DNSClientImpl(response_placeholder, callback)) { +} + +DNSClient::~DNSClient() { + delete (impl_); +} + +void +DNSClient::doUpdate(IOService& io_service, + const IOAddress& ns_addr, + const uint16_t ns_port, + D2UpdateMessage& update, + const int wait) { + impl_->doUpdate(io_service, ns_addr, ns_port, update, wait); +} + + } // namespace d2 } // namespace isc diff --git a/src/bin/d2/dns_client.h b/src/bin/d2/dns_client.h index 56da629a71..d94c9f784c 100644 --- a/src/bin/d2/dns_client.h +++ b/src/bin/d2/dns_client.h @@ -25,6 +25,8 @@ namespace isc { namespace d2 { +class DNSClientImpl; + /// @brief The @c DNSClient class handles communication with the DNS server. /// /// Communication with the DNS server is asynchronous. Caller must provide a @@ -39,13 +41,18 @@ namespace d2 { /// Caller must supply a pointer to the @c D2UpdateMessage object, which will /// encapsulate DNS response, through class constructor. An exception will be /// thrown if the pointer is not initialized by the caller. -/// -/// @todo Currently, only the stub implementation is available for this class. -/// The major missing piece is to create @c D2UpdateMessage object which will -/// encapsulate the response from the DNS server. -class DNSClient : public asiodns::IOFetch::Callback { +class DNSClient { public: + /// @brief A status code of the DNSClient. + enum Status { + SUCCESS, ///< Response received and is ok. + TIMEOUT, ///< No response, timeout. + IO_STOPPED, ///< IO was stopped. + INVALID_RESPONSE, ///< Response received but invalid. + OTHER ///< Other, unclassified error. + }; + /// @brief Callback for the @c DNSClient class. /// /// This is is abstract class which represents the external callback for the @@ -61,7 +68,7 @@ public: /// /// @param result an @c asiodns::IOFetch::Result object representing /// IO status code. - virtual void operator()(asiodns::IOFetch::Result result) = 0; + virtual void operator()(DNSClient::Status status) = 0; }; /// @brief Constructor. @@ -74,7 +81,7 @@ public: DNSClient(D2UpdateMessagePtr& response_placeholder, Callback* callback); /// @brief Virtual destructor, does nothing. - virtual ~DNSClient() { } + ~DNSClient(); /// /// @name Copy constructor and assignment operator @@ -88,18 +95,6 @@ private: //@} public: - - /// @brief Function operator, implementing an internal callback. - /// - /// This internal callback is called when the DNS update message exchange is - /// complete. It further invokes the external callback provided by a caller. - /// Before external callback is invoked, an object of the @c D2UpdateMessage - /// type, representing a response from the server is set. - /// - /// @param result An @c asiodns::IOFetch::Result object representing status - /// code returned by the IO. - virtual void operator()(asiodns::IOFetch::Result result); - /// @brief Start asynchronous DNS Update. /// /// This function starts asynchronous DNS Update and returns. The DNS Update @@ -125,13 +120,7 @@ public: const int wait = -1); private: - /// A buffer holding server's response in the wire format. - util::OutputBufferPtr in_buf_; - /// A pointer to the caller-supplied object, encapsuating a response - /// from DNS. - D2UpdateMessagePtr response_; - /// A pointer to the external callback. - Callback* callback_; + DNSClientImpl* impl_; ///< Pointer to DNSClient implementation. }; } // namespace d2 diff --git a/src/bin/d2/tests/dns_client_unittests.cc b/src/bin/d2/tests/dns_client_unittests.cc index ec73bf1e77..aedced5d06 100644 --- a/src/bin/d2/tests/dns_client_unittests.cc +++ b/src/bin/d2/tests/dns_client_unittests.cc @@ -54,7 +54,7 @@ class DNSClientTest : public virtual ::testing::Test, DNSClient::Callback { public: IOService service_; D2UpdateMessagePtr response_; - IOFetch::Result result_; + DNSClient::Status status_; uint8_t receive_buffer_[MAX_SIZE]; // @brief Constructor. @@ -67,7 +67,7 @@ public: // become messy if such errors were logged. DNSClientTest() : service_(), - result_(IOFetch::SUCCESS) { + status_(DNSClient::SUCCESS) { asiodns::logger.setSeverity(log::INFO); response_.reset(new D2UpdateMessage(D2UpdateMessage::INBOUND)); } @@ -84,9 +84,9 @@ public: // This callback is called when the exchange with the DNS server is // complete or an error occured. This includes the occurence of a timeout. // - // @param result An error code returned by an IO. - virtual void operator()(IOFetch::Result result) { - result_ = result; + // @param status A status code returned by DNSClient. + virtual void operator()(DNSClient::Status status) { + status_ = status; service_.stop(); } @@ -166,8 +166,8 @@ public: service_.run(); // If callback function was called it should have modified the default - // value of result_ with the TIME_OUT error code. - EXPECT_EQ(IOFetch::TIME_OUT, result_); + // value of status_ with the TIMEOUT error code. + EXPECT_EQ(DNSClient::TIMEOUT, status_); } // This test verifies that DNSClient can send DNS Update and receive a @@ -228,7 +228,7 @@ public: udp_socket.close(); // We should have received a response. - EXPECT_EQ(IOFetch::SUCCESS, result_); + EXPECT_EQ(DNSClient::SUCCESS, status_); ASSERT_TRUE(response_); EXPECT_EQ(D2UpdateMessage::RESPONSE, response_->getQRFlag());