update unittest and code according to review comments

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac439@3974 e5f2f494-b856-4b98-b285-d166d9295462

src/bin/auth/query.cc

@@ -25,48 +25,25 @@ using namespace isc::datasrc;
 namespace isc {
 namespace auth {
 
-struct Query::QueryImpl {
-    QueryImpl(const MemoryDataSrc& memory_datasrc, const Name& qname,
-              const RRType& qtype, Message& response) :
-        memory_datasrc_(memory_datasrc), qname_(qname), qtype_(qtype),
-        response_(response)
-    {}
-
-    const MemoryDataSrc& memory_datasrc_;
-    const Name& qname_;
-    const RRType& qtype_;
-    Message& response_;
-};
-
-Query::Query(const MemoryDataSrc& memory_datasrc, const Name& qname,
-             const RRType& qtype, Message& response) :
-    impl_(new QueryImpl(memory_datasrc, qname, qtype, response))
-{}
-
-Query::~Query() {
-    delete impl_;
-}
-
 void
 Query::process() const {
     const MemoryDataSrc::FindResult result =
-        impl_->memory_datasrc_.findZone(impl_->qname_);
+        memory_datasrc_.findZone(qname_);
     bool keep_doing = true;
 
     if (result.code != result::SUCCESS &&
         result.code != result::PARTIALMATCH) {
-        impl_->response_.setRcode(Rcode::SERVFAIL());
+        response_.setRcode(Rcode::SERVFAIL());
         return;
     }
 
     while (keep_doing) {
         keep_doing = false;
-        Zone::FindResult db_result = result.zone->find(impl_->qname_,
-                                                       impl_->qtype_);
+        Zone::FindResult db_result = result.zone->find(qname_, qtype_);
         switch (db_result.code) {
             case Zone::SUCCESS:
-                impl_->response_.setRcode(Rcode::NOERROR());
-                impl_->response_.addRRset(Message::SECTION_ANSWER,
+                response_.setRcode(Rcode::NOERROR());
+                response_.addRRset(Message::SECTION_ANSWER,
                             boost::const_pointer_cast<RRset>(db_result.rrset));
                 // TODO : fill in authority and addtional sections.
                 break;
@@ -74,23 +51,17 @@ Query::process() const {
                 // TODO : add NS to authority section, fill in additional section.
                 break;
             case Zone::NXDOMAIN:
-                impl_->response_.setRcode(Rcode::NXDOMAIN());
+                response_.setRcode(Rcode::NXDOMAIN());
                 // TODO : add SOA to authority section
                 break;
             case Zone::NXRRSET:
-                impl_->response_.setRcode(Rcode::NXRRSET());
+                response_.setRcode(Rcode::NXRRSET());
                 // TODO : add SOA to authority section
                 break;
             case Zone::CNAME:
             case Zone::DNAME:
                 // TODO : replace qname, continue lookup
-                keep_doing = true;
-                break;
-            // should not happen, catch programming error here.
-            default:
                 break;
-                isc_throw(Unexpected,
-                          "Zone::find return unexpected result.");
         }
     }
 }

src/bin/auth/query.h

@@ -72,10 +72,10 @@ public:
     /// \param response The response message to store the answer to the query.
     Query(const isc::datasrc::MemoryDataSrc& memory_datasrc,
           const isc::dns::Name& qname, const isc::dns::RRType& qtype,
-          isc::dns::Message& response);
-
-    /// The destructor.
-    virtual ~Query();
+          isc::dns::Message& response) :
+        memory_datasrc_(memory_datasrc), qname_(qname), qtype_(qtype),
+        response_(response)
+    {}
 
     /// Process the query.
     ///
@@ -105,8 +105,10 @@ public:
     void process() const;
 
 private:
-    struct QueryImpl;
-    QueryImpl* impl_;
+    const isc::datasrc::MemoryDataSrc& memory_datasrc_;
+    const isc::dns::Name& qname_;
+    const isc::dns::RRType& qtype_;
+    isc::dns::Message& response_;
 };
 
 }

src/bin/auth/tests/query_unittest.cc

@@ -15,6 +15,7 @@
 #include <dns/message.h>
 #include <dns/name.h>
 #include <dns/rcode.h>
+#include <dns/rrttl.h>
 #include <dns/rrtype.h>
 
 #include <datasrc/memory_datasrc.h>
@@ -27,6 +28,82 @@ using namespace isc::dns;
 using namespace isc::datasrc;
 using namespace isc::auth;
 
+RRsetPtr a_rrset = RRsetPtr(new RRset(Name("www.example.com"),
+                                      RRClass::IN(), RRType::A(),
+                                      RRTTL(3600)));
+namespace isc {
+namespace datasrc {
+// This is a mock Zone class for testing.
+// It is a derived class of Zone, and simply hardcode the results of find()
+// return SUCCESS for "www.example.com",
+// return NXDOMAIN for "nxdomain.example.com",
+// return NXRRSET for "nxrrset.example.com",
+// return CNAME for "cname.example.com",
+// else return DNAME
+class MockZone : public Zone{
+public:
+    MockZone(const isc::dns::RRClass& rrclass, const isc::dns::Name& origin);
+
+    // The destructor.
+    virtual ~MockZone();
+    virtual const isc::dns::Name& getOrigin() const;
+    virtual const isc::dns::RRClass& getClass() const;
+
+    FindResult find(const isc::dns::Name& name,
+            const isc::dns::RRType& type) const;
+private:
+    struct MockZoneImpl;
+    MockZoneImpl* impl_;
+};
+
+struct MockZone::MockZoneImpl {
+    MockZoneImpl(const RRClass& zone_class, const Name& origin) :
+        zone_class_(zone_class), origin_(origin)
+    {}
+    RRClass zone_class_;
+    Name origin_;
+};
+
+MockZone::MockZone(const RRClass& zone_class, const Name& origin) :
+    impl_(new MockZoneImpl(zone_class, origin))
+{
+}
+
+MockZone::~MockZone() {
+    delete impl_;
+}
+
+const Name&
+MockZone::getOrigin() const {
+    return (impl_->origin_);
+}
+
+const RRClass&
+MockZone::getClass() const {
+    return (impl_->zone_class_);
+}
+
+Zone::FindResult
+MockZone::find(const Name& name, const RRType&) const {
+    // hardcode the find results
+    if (name == Name("www.example.com")) {
+        return FindResult(SUCCESS, a_rrset);
+    } else if (name == Name("delegation.example.com")) {
+        return FindResult(DELEGATION, RRsetPtr());
+    } else if (name == Name("nxdomain.example.com")) {
+        return FindResult(NXDOMAIN, RRsetPtr());
+    } else if (name == Name("nxrrset.example.com")) {
+        return FindResult(NXRRSET, RRsetPtr());
+    } else if (name == Name("cname.example.com")) {
+        return FindResult(CNAME, RRsetPtr());
+    } else {
+        return FindResult(DNAME, RRsetPtr());
+    }
+}
+
+}
+}
+
 namespace {
 class QueryTest : public ::testing::Test {
 protected:
@@ -53,17 +130,31 @@ TEST_F(QueryTest, noZone) {
 }
 
 TEST_F(QueryTest, matchZone) {
-    // add a matching zone.  since the zone is empty right now, the response
-    // should have NXDOMAIN.
-    memory_datasrc.addZone(ZonePtr(new MemoryZone(qclass, Name("example.com"))));
+    // match qname, normal query
+    memory_datasrc.addZone(ZonePtr(new MockZone(qclass, Name("example.com"))));
     query.process();
+    EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ANSWER,
+                                  Name("www.example.com"), RRClass::IN(),
+                                  RRType::A()));
+
+    // NXDOMAIN
+    const Name nxdomain_name(Name("nxdomain.example.com"));
+    Query nxdomain_query(memory_datasrc, nxdomain_name, qtype, response);
+    nxdomain_query.process();
     EXPECT_EQ(Rcode::NXDOMAIN(), response.getRcode());
+
+    // NXRRSET
+    const Name nxrrset_name(Name("nxrrset.example.com"));
+    Query nxrrset_query(memory_datasrc, nxrrset_name, qtype, response);
+    nxrrset_query.process();
+    EXPECT_EQ(Rcode::NXRRSET(), response.getRcode());
 }
 
 TEST_F(QueryTest, noMatchZone) {
     // there's a zone in the memory datasource but it doesn't match the qname.
     // should result in SERVFAIL.
-    memory_datasrc.addZone(ZonePtr(new MemoryZone(qclass, Name("example.org"))));
+    memory_datasrc.addZone(ZonePtr(new MockZone(qclass, Name("example.org"))));
     query.process();
     EXPECT_EQ(Rcode::SERVFAIL(), response.getRcode());
 }