[#2089] check keys stats in unittests

diff --git a/src/lib/d2srv/dns_client.cc b/src/lib/d2srv/dns_client.cc
index 4d407c724cc42d90d63f836e37b2efbe59265fe0..57982076705f00d2922d5dfade7348d495d6e94c 100644 (file)
--- a/src/lib/d2srv/dns_client.cc
+++ b/src/lib/d2srv/dns_client.cc
@@ -38,42 +38,68 @@ using namespace isc::stats;
 // to this separation.
 class DNSClientImpl : public asiodns::IOFetch::Callback {
 public:
-    // A buffer holding response from a DNS.
+    /// @brief A buffer holding response from a DNS.
     util::OutputBufferPtr in_buf_;
-    // A caller-supplied object which will hold the parsed response from DNS.
-    // The response object is (or descends from) isc::dns::Message and is
-    // populated using Message::fromWire().  This method may only be called
-    // once in the lifetime of a Message instance.  Therefore, response_ is a
-    // pointer reference thus allowing this class to replace the object
-    // pointed to with a new Message instance each time a message is
-    // received. This allows a single DNSClientImpl instance to be used for
-    // multiple, sequential IOFetch calls. (@todo Trac# 3286 has been opened
-    // against dns::Message::fromWire.  Should the behavior of fromWire change
-    // the behavior here with could be reexamined).
+
+    /// A caller-supplied object which will hold the parsed response from DNS.
+    /// The response object is (or descends from) isc::dns::Message and is
+    /// populated using Message::fromWire().  This method may only be called
+    /// once in the lifetime of a Message instance.  Therefore, response_ is a
+    /// pointer reference thus allowing this class to replace the object
+    /// pointed to with a new Message instance each time a message is
+    /// received. This allows a single DNSClientImpl instance to be used for
+    /// multiple, sequential IOFetch calls. (@todo Trac# 3286 has been opened
+    /// against dns::Message::fromWire.  Should the behavior of fromWire change
+    /// the behavior here with could be reexamined).
     D2UpdateMessagePtr& response_;
-    // A caller-supplied external callback which is invoked when DNS message
-    // exchange is complete or interrupted.
+
+    /// @brief A caller-supplied external callback which is invoked when DNS
+    /// message exchange is complete or interrupted.
     DNSClient::Callback* callback_;
-    // A Transport Layer protocol used to communicate with a DNS.
+
+    /// @brief A Transport Layer protocol used to communicate with a DNS.
     DNSClient::Protocol proto_;
-    // TSIG context used to sign outbound and verify inbound messages.
+
+    /// @brief TSIG context used to sign outbound and verify inbound messages.
     dns::TSIGContextPtr tsig_context_;
-    // TSIG key name for stats.
+
+    /// @brief TSIG key name for stats.
     std::string tsig_key_name_;
 
-    // Constructor and Destructor
+    /// @brief Constructor.
+    ///
+    /// @param response_placeholder Message object pointer which will be updated
+    /// with dynamically allocated object holding the DNS server's response.
+    /// @param callback Pointer to an object implementing @c DNSClient::Callback
+    /// class. This object will be called when DNS message exchange completes or
+    /// if an error occurs. NULL value disables callback invocation.
+    /// @param proto caller's preference regarding Transport layer protocol to
+    /// be used by DNS Client to communicate with a server.
     DNSClientImpl(D2UpdateMessagePtr& response_placeholder,
                   DNSClient::Callback* callback,
                   const DNSClient::Protocol proto);
+
+    /// @brief Destructor.
     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.
+    /// @brief 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);
 
-    // Starts asynchronous DNS Update using TSIG.
+    /// @brief Starts asynchronous DNS Update using TSIG.
+    ///
+    /// @param io_service IO service to be used to run the message exchange.
+    /// @param ns_addr DNS server address.
+    /// @param ns_port DNS server port.
+    /// @param update A DNS Update message to be sent to the server.
+    /// @param wait A timeout (in milliseconds) for the response. If a response
+    /// is not received within the timeout, exchange is interrupted. This value
+    /// must not exceed maximal value for 'int' data type.
+    /// @param tsig_key A pointer to an @c D2TsigKeyPtr object that will
+    /// (if not null) be used to sign the DNS Update message and verify the
+    /// response.
     void doUpdate(asiolink::IOService& io_service,
                   const asiolink::IOAddress& ns_addr,
                   const uint16_t ns_port,
@@ -81,10 +107,17 @@ public:
                   const unsigned int wait,
                   const D2TsigKeyPtr& tsig_key);
 
-    // This function maps the IO error to the DNSClient error.
-    DNSClient::Status getStatus(const asiodns::IOFetch::Result);
-
-    // This function updates statistics.
+    /// @brief This function maps the IO error to the DNSClient error.
+    ///
+    /// @param result The IOFetch result to be converted to DNSClient status.
+    /// @return The DNSClient status corresponding to the IOFetch result.
+    DNSClient::Status getStatus(const asiodns::IOFetch::Result result);
+
+    /// @brief This function updates statistics.
+    ///
+    /// @param stat The statistic name to be incremented.
+    /// @param update_key The flag indicating if the key statistics should also
+    /// be updated.
     void incrStats(const std::string& stat, bool update_key = true);
 };
 
@@ -269,7 +302,6 @@ DNSClient::DNSClient(D2UpdateMessagePtr& response_placeholder,
 }
 
 DNSClient::~DNSClient() {
-    delete (impl_);
 }
 
 unsigned int

diff --git a/src/lib/d2srv/dns_client.h b/src/lib/d2srv/dns_client.h
index 2cfd8a8cf2b341c8daeb1016c931ad55e58dc937..c63a7a0c0a07000cd940051f357faf7e904647cd 100644 (file)
--- a/src/lib/d2srv/dns_client.h
+++ b/src/lib/d2srv/dns_client.h
@@ -147,7 +147,8 @@ public:
                   const D2TsigKeyPtr& tsig_key = D2TsigKeyPtr());
 
 private:
-    DNSClientImpl* impl_;  ///< Pointer to DNSClient implementation.
+    /// @brief Pointer to DNSClient implementation.
+    std::unique_ptr<DNSClientImpl> impl_;
 };
 
 } // namespace d2

diff --git a/src/lib/d2srv/tests/dns_client_unittests.cc b/src/lib/d2srv/tests/dns_client_unittests.cc
index 36ba16e49a1fb5b5ef4c04a1295e097596828bbe..a560546b0fc6fd523172e83dad353f5fe0ebf1c6 100644 (file)
--- a/src/lib/d2srv/tests/dns_client_unittests.cc
+++ b/src/lib/d2srv/tests/dns_client_unittests.cc
@@ -56,7 +56,8 @@ const long TEST_TIMEOUT = 5 * 1000;
 // properly handled a task may be hanging for a long time. In order to prevent
 // it, the asiolink::IntervalTimer is used to break a running test if test
 // timeout is hit. This will result in test failure.
-class DNSClientTest : public virtual D2StatTest, DNSClient::Callback {
+class DNSClientTest : public ::testing::Test, DNSClient::Callback,
+                      public D2StatTest {
 public:
     /// @brief The IOService which handles IO operations.
     IOService service_;
@@ -79,10 +80,10 @@ public:
     /// @brief The DNS client performing DNS update.
     DNSClientPtr dns_client_;
 
-    /// @brief The flag which indicates if the response should be corrupted.
+    /// @brief The flag which specifies if the response should be corrupted.
     bool corrupt_response_;
 
-    /// @brief The flag which indicates if a response is expected.
+    /// @brief The flag which specifies if a response is expected.
     bool expect_response_;
 
     /// @brief The timeout timer.
@@ -94,8 +95,8 @@ public:
     /// @brief The number of expected DNS updates.
     int expected_;
 
-    /// @brief The flag which indicates if the server should continue
-    /// with receiving DNS updates.
+    /// @brief The flag which specifies if the server should continue with
+    /// receiving DNS updates.
     bool go_on_;
 
     /// @brief Constructor
@@ -183,8 +184,8 @@ public:
     /// @param remote A pointer to an object which specifies the host (address
     /// and port) from which a request has come.
     /// @param receive_length A length (in bytes) of the received data.
-    /// @param corrupt_response A bool value which indicates that the server's
-    /// response should be invalid (true) or valid (false)
+    /// @param corrupt_response A bool value which specifies if the server's
+    /// response should be invalid (true) or valid (false).
     void udpReceiveHandler(udp::socket* socket, udp::endpoint* remote,
                            size_t receive_length, const bool corrupt_response) {
         // The easiest way to create a response message is to copy the entire

diff --git a/src/lib/d2srv/testutils/stats_test_utils.h b/src/lib/d2srv/testutils/stats_test_utils.h
index d7e4ae14621fe1fb4a71a1175cbfee2f5d00a078..4b32b9c09055ff8e2ba174a0a42cf642e2696909 100644 (file)
--- a/src/lib/d2srv/testutils/stats_test_utils.h
+++ b/src/lib/d2srv/testutils/stats_test_utils.h
@@ -21,8 +21,8 @@ namespace test {
 /// @brief Type of name x value for statistics.
 typedef std::map<std::string, int64_t> StatMap;
 
-/// @brief Test fixture class with utility functions to test statistics.
-class D2StatTest : public ::testing::Test {
+/// @brief Test class with utility functions to test statistics.
+class D2StatTest {
 public:
     /// @brief Constructor.
     D2StatTest();