BIND 10 trac494, updated. 430e0e89ec5ab05bdbbd43b5fba9c93bdcf8f6c9 [trac494] addressed review comments

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Feb 2 10:38:11 UTC 2011


The branch, trac494 has been updated
       via  430e0e89ec5ab05bdbbd43b5fba9c93bdcf8f6c9 (commit)
      from  640640363d2a29cc0f4ab8151ca5c02dc9f7c361 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 430e0e89ec5ab05bdbbd43b5fba9c93bdcf8f6c9
Author: Jelte Jansen <jelte at isc.org>
Date:   Wed Feb 2 11:36:14 2011 +0100

    [trac494] addressed review comments
    
    Removed some dead code, added two unit tests, changed sendQuery() to the
    more descriptive resolve(), and removed a superfluous call to dlog()

-----------------------------------------------------------------------

Summary of changes:
 src/bin/resolver/resolver.cc                       |    5 +-
 src/lib/asiolink/asiolink.cc                       |   18 +---
 src/lib/asiolink/asiolink.h                        |   20 ++---
 src/lib/asiolink/tests/asiolink_unittest.cc        |    8 +-
 src/lib/nsas/nameserver_entry.h                    |    1 -
 src/lib/resolve/tests/Makefile.am                  |    1 +
 .../resolve/tests/resolver_callback_unittest.cc    |   90 ++++++++++++++++++++
 7 files changed, 111 insertions(+), 32 deletions(-)
 create mode 100644 src/lib/resolve/tests/resolver_callback_unittest.cc

-----------------------------------------------------------------------
diff --git a/src/bin/resolver/resolver.cc b/src/bin/resolver/resolver.cc
index dc3a773..d237e03 100644
--- a/src/bin/resolver/resolver.cc
+++ b/src/bin/resolver/resolver.cc
@@ -326,7 +326,6 @@ Resolver::~Resolver() {
     delete checkin_;
     delete dns_lookup_;
     delete dns_answer_;
-    dlog("Deleting the Resolver",true);
 }
 
 void
@@ -439,7 +438,7 @@ void
 ResolverImpl::resolve(const QuestionPtr& question,
     const isc::resolve::ResolverInterface::CallbackPtr& callback)
 {
-    rec_query_->sendQuery(question, callback);
+    rec_query_->resolve(question, callback);
 }
 
 void
@@ -449,7 +448,7 @@ ResolverImpl::processNormalQuery(const Question& question,
                                  DNSServer* server)
 {
     dlog("Processing normal query");
-    rec_query_->sendQuery(question, answer_message, buffer, server);
+    rec_query_->resolve(question, answer_message, buffer, server);
 }
 
 namespace {
diff --git a/src/lib/asiolink/asiolink.cc b/src/lib/asiolink/asiolink.cc
index fbbcc86..222bec8 100644
--- a/src/lib/asiolink/asiolink.cc
+++ b/src/lib/asiolink/asiolink.cc
@@ -479,7 +479,6 @@ public:
         MessagePtr answer_message, shared_ptr<AddressVector> upstream,
         shared_ptr<AddressVector> upstream_root,
         OutputBufferPtr buffer,
-        //DNSServer* server,
         isc::resolve::ResolverInterface::CallbackPtr cb,
         int timeout,
         unsigned retries) :
@@ -489,7 +488,6 @@ public:
         upstream_(upstream),
         upstream_root_(upstream_root),
         buffer_(buffer),
-        //server_(server->clone()),
         resolvercallback_(cb),
         timeout_(timeout),
         retries_(retries),
@@ -560,19 +558,13 @@ public:
 }
 
 void
-RecursiveQuery::sendQuery(const isc::dns::QuestionPtr& question,
+RecursiveQuery::resolve(const isc::dns::QuestionPtr& question,
     const isc::resolve::ResolverInterface::CallbackPtr callback)
 {
     asio::io_service& io = dns_service_.get_io_service();
 
     MessagePtr answer_message(new Message(Message::RENDER));
     OutputBufferPtr buffer(new OutputBuffer(0));
-    /*
-    answer_message->setOpcode(isc::dns::Opcode::QUERY());
-    isc::resolve::ResolverCallbackDirect* rcd =
-        new isc::resolve::ResolverCallbackDirect(callback,
-                                                 answer_message);
-    */
     
     // It will delete itself when it is done
     new RunningQuery(io, *question, answer_message, upstream_,
@@ -580,10 +572,10 @@ RecursiveQuery::sendQuery(const isc::dns::QuestionPtr& question,
 }
 
 void
-RecursiveQuery::sendQuery(const Question& question,
-                          MessagePtr answer_message,
-                          OutputBufferPtr buffer,
-                          DNSServer* server)
+RecursiveQuery::resolve(const Question& question,
+                        MessagePtr answer_message,
+                        OutputBufferPtr buffer,
+                        DNSServer* server)
 {
     // XXX: eventually we will need to be able to determine whether
     // the message should be sent via TCP or UDP, or sent initially via
diff --git a/src/lib/asiolink/asiolink.h b/src/lib/asiolink/asiolink.h
index cc97417..a8b4083 100644
--- a/src/lib/asiolink/asiolink.h
+++ b/src/lib/asiolink/asiolink.h
@@ -575,12 +575,10 @@ public:
     /// failure(). See ResolverInterface::Callback for more information.
     ///
     /// \param question The question being answered <qname/qclass/qtype>
-    /// \param callback Callback object
-    /// \param buffer An output Message into which the final response will be copied
-    /// \param buffer An output buffer into which the intermediate responses will be copied
-    /// \param server A pointer to the \c DNSServer object handling the client
-    void sendQuery(const isc::dns::QuestionPtr& question,
-                   const isc::resolve::ResolverInterface::CallbackPtr callback);
+    /// \param callback Callback object. See
+    ///        \c ResolverInterface::Callback for more information
+    void resolve(const isc::dns::QuestionPtr& question,
+                 const isc::resolve::ResolverInterface::CallbackPtr callback);
 
 
     /// \brief Initiates resolving for the given question.
@@ -590,13 +588,13 @@ public:
     /// object.
     ///
     /// \param question The question being answered <qname/qclass/qtype>
-    /// \param buffer An output Message into which the final response will be copied
+    /// \param answer_message An output Message into which the final response will be copied
     /// \param buffer An output buffer into which the intermediate responses will be copied
     /// \param server A pointer to the \c DNSServer object handling the client
-    void sendQuery(const isc::dns::Question& question,
-                   isc::dns::MessagePtr answer_message,
-                   isc::dns::OutputBufferPtr buffer,
-                   DNSServer* server);
+    void resolve(const isc::dns::Question& question,
+                 isc::dns::MessagePtr answer_message,
+                 isc::dns::OutputBufferPtr buffer,
+                 DNSServer* server);
 private:
     DNSService& dns_service_;
     boost::shared_ptr<std::vector<std::pair<std::string, uint16_t> > >
diff --git a/src/lib/asiolink/tests/asiolink_unittest.cc b/src/lib/asiolink/tests/asiolink_unittest.cc
index 4d79b0f..cdce363 100644
--- a/src/lib/asiolink/tests/asiolink_unittest.cc
+++ b/src/lib/asiolink/tests/asiolink_unittest.cc
@@ -678,7 +678,7 @@ TEST_F(ASIOLinkTest, forwarderSend) {
     Question q(Name("example.com"), RRClass::IN(), RRType::TXT());
     OutputBufferPtr buffer(new OutputBuffer(0));
     MessagePtr answer(new Message(Message::RENDER));
-    rq.sendQuery(q, answer, buffer, &server);
+    rq.resolve(q, answer, buffer, &server);
 
     char data[4096];
     size_t size = sizeof(data);
@@ -726,7 +726,7 @@ TEST_F(ASIOLinkTest, recursiveTimeout) {
     Question question(Name("example.net"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
     MessagePtr answer(new Message(Message::RENDER));
-    query.sendQuery(question, answer, buffer, &server);
+    query.resolve(question, answer, buffer, &server);
 
     // Run the test
     io_service_->run();
@@ -773,7 +773,7 @@ TEST_F(ASIOLinkTest, DISABLED_recursiveSendOk) {
     Question q(Name("www.isc.org"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
     MessagePtr answer(new Message(Message::RENDER));
-    rq.sendQuery(q, answer, buffer, &server);
+    rq.resolve(q, answer, buffer, &server);
     io_service_->run();
 
     // Check that the answer we got matches the one we wanted
@@ -798,7 +798,7 @@ TEST_F(ASIOLinkTest, DISABLED_recursiveSendNXDOMAIN) {
     Question q(Name("wwwdoesnotexist.isc.org"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
     MessagePtr answer(new Message(Message::RENDER));
-    rq.sendQuery(q, answer, buffer, &server);
+    rq.resolve(q, answer, buffer, &server);
     io_service_->run();
 
     // Check that the answer we got matches the one we wanted
diff --git a/src/lib/nsas/nameserver_entry.h b/src/lib/nsas/nameserver_entry.h
index 256a256..9a8e542 100644
--- a/src/lib/nsas/nameserver_entry.h
+++ b/src/lib/nsas/nameserver_entry.h
@@ -85,7 +85,6 @@ public:
 };
 
 class ZoneEntry;
-//class ResolverInterface;
 
 /// \brief Nameserver Entry
 ///
diff --git a/src/lib/resolve/tests/Makefile.am b/src/lib/resolve/tests/Makefile.am
index aae85d6..e064cc0 100644
--- a/src/lib/resolve/tests/Makefile.am
+++ b/src/lib/resolve/tests/Makefile.am
@@ -14,6 +14,7 @@ TESTS += run_unittests
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 run_unittests_SOURCES = run_unittests.cc
+run_unittests_SOURCES += resolver_callback_unittest.cc
 
 run_unittests_LDADD = $(GTEST_LDADD)
 run_unittests_LDADD +=  $(top_builddir)/src/lib/resolve/libresolve.la
diff --git a/src/lib/resolve/tests/resolver_callback_unittest.cc b/src/lib/resolve/tests/resolver_callback_unittest.cc
new file mode 100644
index 0000000..6370e22
--- /dev/null
+++ b/src/lib/resolve/tests/resolver_callback_unittest.cc
@@ -0,0 +1,90 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <gtest/gtest.h>
+#include <resolve/resolver_callback.h>
+#include <asiolink/asiolink.h>
+
+using namespace isc::resolve;
+
+// Dummy subclass for DNSServer*
+// We want to check if resume is called
+// Since the server will get cloned(), we want the clones to share
+// our bools for whether resume got called and with what value
+class DummyServer : public asiolink::DNSServer {
+public:
+    DummyServer(DummyServer* orig) {
+        resume_called_ = orig->getResumeCalled();
+        resume_value_ = orig->getResumeValue();
+    }
+    DummyServer(bool* resume_called, bool* resume_value) :
+        resume_called_(resume_called), resume_value_(resume_value)
+    {}
+    
+    bool* getResumeCalled() { return resume_called_; }
+    bool* getResumeValue() { return resume_value_; }
+    
+    DNSServer* clone() {
+        DummyServer* n = new DummyServer(this);
+        return n;
+    }
+
+    void resume(bool value) {
+        *resume_called_ = true;
+        *resume_value_ = value;
+    }
+
+private:
+    bool* resume_called_;
+    bool* resume_value_;
+};
+
+class ResolverCallbackServerTest : public ::testing::Test {
+public:
+    ResolverCallbackServerTest() : resume_called_(false),
+                                   resume_value_(false) {
+        server_ = new DummyServer(&resume_called_, &resume_value_);
+        callback_ = new ResolverCallbackServer(server_);
+    };
+
+    ~ResolverCallbackServerTest() {
+        delete callback_;
+        delete server_;
+    }
+
+    DummyServer* getServer() { return server_; }
+    ResolverCallbackServer* getCallback() { return callback_; }
+    bool getResumeCalled() { return resume_called_; }
+    bool getResumeValue() { return resume_value_; }
+
+private:
+    DummyServer* server_;
+    ResolverCallbackServer* callback_;
+    bool resume_called_;
+    bool resume_value_;
+};
+
+TEST_F(ResolverCallbackServerTest, testSuccess) {
+    EXPECT_FALSE(getResumeCalled());
+    getCallback()->success(isc::dns::MessagePtr());
+    EXPECT_TRUE(getResumeCalled());
+    EXPECT_TRUE(getResumeValue());
+}
+
+TEST_F(ResolverCallbackServerTest, testFailure) {
+    EXPECT_FALSE(getResumeCalled());
+    getCallback()->failure();
+    EXPECT_TRUE(getResumeCalled());
+    EXPECT_FALSE(getResumeValue());
+}




More information about the bind10-changes mailing list