update according to review comment : combine findAny and find logic

src/bin/auth/query.cc

@@ -66,7 +66,8 @@ Query::findAddrs(const Zone& zone, const Name& qname,
 
     // Find A rrset
     if (qname_ != qname || qtype_ != RRType::A()) {
-        Zone::FindResult a_result = zone.find(qname, RRType::A(), options);
+        Zone::FindResult a_result = zone.find(qname, RRType::A(), NULL,
+                                              options);
         if (a_result.code == Zone::SUCCESS) {
             response_.addRRset(Message::SECTION_ADDITIONAL,
                     boost::const_pointer_cast<RRset>(a_result.rrset));
@@ -76,7 +77,7 @@ Query::findAddrs(const Zone& zone, const Name& qname,
     // Find AAAA rrset
     if (qname_ != qname || qtype_ != RRType::AAAA()) {
         Zone::FindResult aaaa_result =
-            zone.find(qname, RRType::AAAA(), options);
+            zone.find(qname, RRType::AAAA(), NULL, options);
         if (aaaa_result.code == Zone::SUCCESS) {
             response_.addRRset(Message::SECTION_ADDITIONAL,
                     boost::const_pointer_cast<RRset>(aaaa_result.rrset));

src/bin/auth/tests/query_unittest.cc

@@ -106,6 +106,7 @@ public:
 
     FindResult find(const isc::dns::Name& name,
                     const isc::dns::RRType& type,
+                    RRsetList* target = NULL,
                     const FindOptions options = FIND_DEFAULT) const;
 
 private:
@@ -131,7 +132,7 @@ MockZone::getClass() const {
 
 Zone::FindResult
 MockZone::find(const Name& name, const RRType& type,
-               const FindOptions options) const
+               RRsetList* target, const FindOptions options) const
 {
     // hardcode the find results
     if (name == Name("www.example.com") && type == RRType::A()) {
@@ -155,6 +156,10 @@ MockZone::find(const Name& name, const RRType& type,
         has_apex_NS_)
     {
         return (FindResult(SUCCESS, auth_ns_rrset));
+    } else if (name == Name("example.com") && type == RRType::ANY()) {
+        target->addRRset(soa_rrset);
+        target->addRRset(auth_ns_rrset);
+        return (FindResult(SUCCESS, RRsetPtr()));
     } else if (name == Name("mx.delegation.example.com") &&
         type == RRType::A() && (options & FIND_GLUE_OK) != 0)
     {

src/lib/datasrc/memory_datasrc.cc

@@ -189,7 +189,7 @@ struct MemoryZone::MemoryZoneImpl {
 
     // Implementation of MemoryZone::find
     FindResult find(const Name& name, RRType type,
-                    const FindOptions options) const
+                    RRsetList* target, const FindOptions options) const
     {
         // Get the node
         DomainNode* node(NULL);
@@ -225,12 +225,17 @@ struct MemoryZone::MemoryZoneImpl {
         }
 
         // handle type any query
-        const bool is_any_query = (type == RRType::ANY());
-        if (is_any_query){
-            if (node->getData()->empty()) {
-                return (FindResult(NXRRSET, ConstRRsetPtr()));
-            } else {
+        if (target){
+            if (!node->getData()->empty()) {
+                for (found = node->getData()->begin();
+                     found != node->getData()->end(); found++)
+                {
+                    target->addRRset(
+                        boost::const_pointer_cast<RRset>(found->second));
+                }
                 return (FindResult(SUCCESS, ConstRRsetPtr()));
+            } else {
+                return (FindResult(NXRRSET, ConstRRsetPtr()));
             }
         }
 
@@ -247,26 +252,6 @@ struct MemoryZone::MemoryZoneImpl {
             return (FindResult(NXRRSET, ConstRRsetPtr()));
         }
     }
-
-    // Implementation of MemoryZone::findAny
-    FindResult findAny(const Name& name, RRsetList& target) const
-    {
-        FindResult result(find(name, RRType::ANY(), FIND_DEFAULT));
-        if (result.code == SUCCESS) {
-            DomainNode* node(NULL);
-            FindState state(FIND_DEFAULT);
-            domains_.find(name, &node, zonecutCallback, &state);
-            assert(!node->getData()->empty());
-
-            Domain::const_iterator it;
-            for (it = node->getData()->begin();
-                 it != node->getData()->end(); it++)
-            {
-                target.addRRset(boost::const_pointer_cast<RRset>(it->second));
-            }
-        }
-        return (result);
-    }
 };
 
 MemoryZone::MemoryZone(const RRClass& zone_class, const Name& origin) :
@@ -290,15 +275,9 @@ MemoryZone::getClass() const {
 
 Zone::FindResult
 MemoryZone::find(const Name& name, const RRType& type,
-                 const FindOptions options) const
-{
-    return (impl_->find(name, type, options));
-}
-
-Zone::FindResult
-MemoryZone::findAny(const Name& name, RRsetList& target) const
+                 RRsetList* target, const FindOptions options) const
 {
-    return (impl_->findAny(name, target));
+    return (impl_->find(name, type, target, options));
 }
 
 result::Result

src/lib/datasrc/memory_datasrc.h

@@ -56,35 +56,22 @@ public:
 
     /// \brief Returns the origin of the zone.
     virtual const isc::dns::Name& getOrigin() const;
+
     /// \brief Returns the class of the zone.
     virtual const isc::dns::RRClass& getClass() const;
+
     /// \brief Looks up an RRset in the zone.
     ///
     /// See documentation in \c Zone.
     ///
     /// It returns NULL pointer in case of NXDOMAIN and NXRRSET,
-    /// and also SUCCESS for TYPE_ANY query.
+    /// and also SUCCESS if target is not NULL(TYPE_ANY query).
     /// (the base class documentation does not seem to require that).
     virtual FindResult find(const isc::dns::Name& name,
                             const isc::dns::RRType& type,
+                            isc::dns::RRsetList* target = NULL,
                             const FindOptions options = FIND_DEFAULT) const;
 
-    /// \brief Iterate over RRs with the same owner name in the zone.
-    ///
-    /// It gets all RRsets under the domain name.
-    ///
-    /// It throws DuplicateRRset exception if there are duplicate rrsets under
-    /// the same domain. This method involve internal resource allocation,
-    /// especially for constructing the FindResult, and may throw an exception
-    /// if it fails. It should not throw other types of exceptions.
-    ///
-    /// \param name The domain name to be searched for.
-    /// \param target  Get all RRs under the domain name.
-    /// \return A \c FindResult object enclosing the search result, it returns
-    /// NULL in case of NXDOMAIN, NXRRSET and SUCCESS.
-    FindResult findAny(const isc::dns::Name& name,
-                       isc::dns::RRsetList& target) const;
-
     /// \brief Inserts an rrset into the zone.
     ///
     /// It puts another RRset into the zone.

src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -212,7 +212,8 @@ public:
     void findTest(const Name& name, const RRType& rrtype, Zone::Result result,
                   bool check_answer = true,
                   const ConstRRsetPtr& answer = ConstRRsetPtr(),
-                  MemoryZone *zone = NULL,
+                  RRsetList* target = NULL,
+                  MemoryZone* zone = NULL,
                   Zone::FindOptions options = Zone::FIND_DEFAULT)
     {
         if (!zone) {
@@ -221,7 +222,7 @@ public:
         // The whole block is inside, because we need to check the result and
         // we can't assign to FindResult
         EXPECT_NO_THROW({
-                Zone::FindResult find_result(zone->find(name, rrtype,
+                Zone::FindResult find_result(zone->find(name, rrtype, target,
                                                         options));
                 // Check it returns correct answers
                 EXPECT_EQ(result, find_result.code);
@@ -230,41 +231,6 @@ public:
                 }
             });
     }
-
-    /**
-     * \brief Test one TYPE_ANY find query to the zone.
-     *
-     * Asks a query to the zone and checks it does not throw and returns
-     * expected results. It returns nothing, it just signals failures
-     * to GTEST.
-     *
-     * \param name The name to ask for.
-     * \param result The expected code of the result.
-     * \param check_answer Should a check against equality of the answer be
-     *     done?
-     * \param answer The expected rrset, if any should be returned.
-     * \param zone Check different MemoryZone object than zone_ (if NULL,
-     *     uses zone_)
-     */
-    void findAnyTest(const Name& name, Zone::Result result,
-                     RRsetList& target, bool check_answer = true,
-                     const ConstRRsetPtr& answer = ConstRRsetPtr(),
-                     MemoryZone *zone = NULL)
-    {
-        if (!zone) {
-            zone = &zone_;
-        }
-        // The whole block is inside, because we need to check the result and
-        // we can't assign to FindResult
-        EXPECT_NO_THROW({
-                Zone::FindResult find_result(zone->findAny(name, target));
-                // Check it returns correct answers
-                EXPECT_EQ(result, find_result.code);
-                if (check_answer) {
-                    EXPECT_EQ(answer, find_result.rrset);
-                }
-            });
-    }
 };
 
 /**
@@ -328,19 +294,6 @@ TEST_F(MemoryZoneTest, delegationNS) {
              Zone::DELEGATION, true, rr_child_ns_); // note: !rr_grandchild_ns_
 }
 
-TEST_F(MemoryZoneTest, any) {
-    EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_.add(rr_a_)));
-    EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_.add(rr_ns_)));
-    EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_.add(rr_child_ns_)));
-
-    findTest(Name("example.org"), RRType::ANY(), Zone::SUCCESS,
-             true, ConstRRsetPtr());
-    findTest(Name("child.example.org"), RRType::ANY(), Zone::DELEGATION,
-             true, rr_child_ns_);
-    findTest(Name("example.com"), RRType::ANY(), Zone::NXDOMAIN,
-             true, ConstRRsetPtr());
-}
-
 TEST_F(MemoryZoneTest, findAny) {
     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_.add(rr_a_)));
     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_.add(rr_ns_)));
@@ -348,21 +301,21 @@ TEST_F(MemoryZoneTest, findAny) {
 
     // origin
     RRsetList origin_rrsets;
-    findAnyTest(origin_, Zone::SUCCESS, origin_rrsets,
-                true, ConstRRsetPtr());
+    findTest(origin_, RRType::ANY(), Zone::SUCCESS, true,
+             ConstRRsetPtr(), &origin_rrsets);
     EXPECT_EQ(2, origin_rrsets.size());
     EXPECT_EQ(rr_a_, origin_rrsets.findRRset(RRType::A(), RRClass::IN()));
     EXPECT_EQ(rr_ns_, origin_rrsets.findRRset(RRType::NS(), RRClass::IN()));
 
     // out zone name
     RRsetList out_rrsets;
-    findAnyTest(Name("example.com"), Zone::NXDOMAIN, out_rrsets,
-                true, ConstRRsetPtr());
+    findTest(Name("example.com"), RRType::ANY(), Zone::NXDOMAIN, true,
+             ConstRRsetPtr(), &out_rrsets);
     EXPECT_EQ(0, out_rrsets.size());
 
     RRsetList glue_child_rrsets;
-    findAnyTest(child_glue_name_, Zone::SUCCESS, glue_child_rrsets,
-                true, ConstRRsetPtr());
+    findTest(child_glue_name_, RRType::ANY(), Zone::SUCCESS, true,
+                ConstRRsetPtr(), &glue_child_rrsets);
     EXPECT_EQ(rr_child_glue_, glue_child_rrsets.findRRset(RRType::A(),
                                                      RRClass::IN()));
     EXPECT_EQ(1, glue_child_rrsets.size());
@@ -375,14 +328,14 @@ TEST_F(MemoryZoneTest, findAny) {
 
     // zone cut
     RRsetList child_rrsets;
-    findAnyTest(child_ns_name_, Zone::DELEGATION, child_rrsets,
-                true, rr_child_ns_);
+    findTest(child_ns_name_, RRType::ANY(), Zone::DELEGATION, true,
+             rr_child_ns_, &child_rrsets);
     EXPECT_EQ(0, child_rrsets.size());
 
     // glue for this zone cut
     RRsetList new_glue_child_rrsets;
-    findAnyTest(child_glue_name_, Zone::DELEGATION, new_glue_child_rrsets,
-                true, rr_child_ns_);
+    findTest(child_glue_name_, RRType::ANY(), Zone::DELEGATION, true,
+                rr_child_ns_, &new_glue_child_rrsets);
     EXPECT_EQ(0, new_glue_child_rrsets.size());
 }
 
@@ -404,15 +357,15 @@ TEST_F(MemoryZoneTest, glue) {
 
     // If we do it in the "glue OK" mode, we should find the exact match.
     findTest(child_glue_name_, RRType::A(), Zone::SUCCESS, true,
-             rr_child_glue_, NULL, Zone::FIND_GLUE_OK);
+             rr_child_glue_, NULL, NULL, Zone::FIND_GLUE_OK);
 
     // glue OK + NXRRSET case
     findTest(child_glue_name_, RRType::AAAA(), Zone::NXRRSET, true,
-             ConstRRsetPtr(), NULL, Zone::FIND_GLUE_OK);
+             ConstRRsetPtr(), NULL, NULL, Zone::FIND_GLUE_OK);
 
     // glue OK + NXDOMAIN case
     findTest(Name("www.child.example.org"), RRType::A(), Zone::DELEGATION,
-             true, rr_child_ns_, NULL, Zone::FIND_GLUE_OK);
+             true, rr_child_ns_, NULL, NULL, Zone::FIND_GLUE_OK);
 
     // TODO:
     // glue name would match a wildcard under a zone cut: wildcard match
@@ -421,12 +374,13 @@ TEST_F(MemoryZoneTest, glue) {
 
     // nested cut case.  The glue should be found.
     findTest(grandchild_glue_name_, RRType::AAAA(), Zone::SUCCESS,
-             true, rr_grandchild_glue_, NULL, Zone::FIND_GLUE_OK);    
+             true, rr_grandchild_glue_, NULL, NULL, Zone::FIND_GLUE_OK);
 
     // A non-existent name in nested cut.  This should result in delegation
     // at the highest zone cut.
     findTest(Name("www.grand.child.example.org"), RRType::TXT(),
-             Zone::DELEGATION, true, rr_child_ns_, NULL, Zone::FIND_GLUE_OK);
+             Zone::DELEGATION, true, rr_child_ns_, NULL, NULL,
+             Zone::FIND_GLUE_OK);
 }
 
 // Test adding DNAMEs and resulting delegation handling
@@ -485,14 +439,14 @@ TEST_F(MemoryZoneTest, load) {
 
     // Now see there are some rrsets (we don't look inside, though)
     findTest(Name("."), RRType::SOA(), Zone::SUCCESS, false, ConstRRsetPtr(),
-        &rootzone);
+        NULL, &rootzone);
     findTest(Name("."), RRType::NS(), Zone::SUCCESS, false, ConstRRsetPtr(),
-        &rootzone);
+        NULL, &rootzone);
     findTest(Name("a.root-servers.net."), RRType::A(), Zone::SUCCESS, false,
-        ConstRRsetPtr(), &rootzone);
+        ConstRRsetPtr(), NULL, &rootzone);
     // But this should no longer be here
     findTest(ns_name_, RRType::AAAA(), Zone::NXDOMAIN, true, ConstRRsetPtr(),
-        &rootzone);
+        NULL, &rootzone);
 
     // Try loading zone that is wrong in a different way
     EXPECT_THROW(zone_.load(TEST_DATA_DIR "/duplicate_rrset.zone"),
@@ -521,13 +475,13 @@ TEST_F(MemoryZoneTest, swap) {
     EXPECT_EQ(RRClass::IN(), zone2.getClass());
     // make sure the zone data is swapped, too
     findTest(origin_, RRType::NS(), Zone::NXDOMAIN, false, ConstRRsetPtr(),
-             &zone1);
+             NULL, &zone1);
     findTest(other_origin, RRType::TXT(), Zone::SUCCESS, false,
-             ConstRRsetPtr(), &zone1);
+             ConstRRsetPtr(), NULL, &zone1);
     findTest(origin_, RRType::NS(), Zone::SUCCESS, false, ConstRRsetPtr(),
-             &zone2);
+             NULL, &zone2);
     findTest(other_origin, RRType::TXT(), Zone::NXDOMAIN, false,
-             ConstRRsetPtr(), &zone2);
+             ConstRRsetPtr(), NULL, &zone2);
 }
 
 TEST_F(MemoryZoneTest, getFileName) {

src/lib/datasrc/zone.h

@@ -17,6 +17,7 @@
 #define __ZONE_H 1
 
 #include <datasrc/result.h>
+#include <dns/rrsetlist.h>
 
 namespace isc {
 namespace datasrc {
@@ -159,6 +160,8 @@ public:
     ///   \c CNAME and that CNAME RR.
     /// - If the search name matches a delegation point of DNAME, it returns
     ///   the code of \c DNAME and that DNAME RR.
+    /// - If the query type is ANY, target shouldn't be NULL. It iterate over
+    ///   RRs under the domain, and insert them into target.
     ///
     /// The \c options parameter specifies customized behavior of the search.
     /// Their semantics is as follows:
@@ -174,6 +177,8 @@ public:
     /// A derived version of this method may involve internal resource
     /// allocation, especially for constructing the resulting RRset, and may
     /// throw an exception if it fails.
+    /// It throws DuplicateRRset exception if there are duplicate rrsets under
+    /// the same domain.
     /// It should not throw other types of exceptions.
     ///
     /// Note: It's quite likely that we'll need to specify search options.
@@ -183,10 +188,13 @@ public:
     ///
     /// \param name The domain name to be searched for.
     /// \param type The RR type to be searched for.
+    /// \param target If target is not NULL, insert all RRs under the domain
+    /// into it.
     /// \param options The search options.
     /// \return A \c FindResult object enclosing the search result (see above).
     virtual FindResult find(const isc::dns::Name& name,
                             const isc::dns::RRType& type,
+                            isc::dns::RRsetList* target = NULL,
                             const FindOptions options
                             = FIND_DEFAULT) const = 0;
     //@}