[2977] Separated DNSClient interface from implementation.

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
 

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

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());