[2109] refactor creation of TreeNodeRRsets

and make RRClass a construction-time argument

src/lib/datasrc/memory/tests/zone_finder_unittest.cc

@@ -182,7 +182,7 @@ public:
         class_(RRClass::IN()),
         origin_("example.org"),
         zone_data_(ZoneData::create(mem_sgmt_, origin_)),
-        zone_finder_(*zone_data_)
+        zone_finder_(*zone_data_, class_)
     {
         // Build test RRsets.  Below, we construct an RRset for
         // each textual RR(s) of zone_data, and assign it to the corresponding

src/lib/datasrc/memory/zone_finder.cc

@@ -29,25 +29,33 @@ using namespace isc::dns;
 using namespace isc::datasrc::memory;
 using namespace isc::datasrc;
 
-namespace {
+namespace isc {
+namespace datasrc {
+namespace memory {
 
+namespace {
 // (temporary?) convenience function to create a treenoderrset
 // given an rdataset; we should probably have some pool so these
 // do not need to be allocated dynamically
 // For now they are still dynamically allocated, and have a fixed rrclass (...)
 TreeNodeRRsetPtr
 createTreeNodeRRset(const ZoneNode* node,
-                    const RdataSet* rdataset)
+                    const RdataSet* rdataset,
+                    const RRClass& rrclass)
 {
-    return TreeNodeRRsetPtr(new TreeNodeRRset(RRClass::IN(), node,
-                                              rdataset, true));
+    if (rdataset != NULL) {
+        return TreeNodeRRsetPtr(new TreeNodeRRset(rrclass, node,
+                                                  rdataset, true));
+    } else {
+        return TreeNodeRRsetPtr();
+    }
 }
 
 struct FindState {
     FindState(bool glue_ok) :
         zonecut_node_(NULL),
         dname_node_(NULL),
-        rrset_(TreeNodeRRsetPtr()),
+        rrset_(NULL),
         glue_ok_(glue_ok)
     {}
 
@@ -59,7 +67,8 @@ struct FindState {
     const ZoneNode* dname_node_;
 
     // Delegation RRset (NS or DNAME), if found.
-    TreeNodeRRsetPtr rrset_;
+    //TreeNodeRRsetPtr rrset_;
+    const RdataSet* rrset_;
 
     // Whether to continue search below a delegation point.
     // Set at construction time.
@@ -78,7 +87,7 @@ bool cutCallback(const ZoneNode& node, FindState* state) {
     if (found_dname != NULL) {
         LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_MEM_DNAME_ENCOUNTERED);
         state->dname_node_ = &node;
-        state->rrset_ = createTreeNodeRRset(&node, found_dname);
+        state->rrset_ = found_dname;
         return (true);
     }
 
@@ -100,7 +109,7 @@ bool cutCallback(const ZoneNode& node, FindState* state) {
         // It cannot happen for us (at least for now), so we don't do
         // that check.
         state->zonecut_node_ = &node;
-        state->rrset_ = createTreeNodeRRset(&node, found_ns);
+        state->rrset_ = found_ns;
 
         // Unless glue is allowed the search stops here, so we return
         // false; otherwise return true to continue the search.
@@ -116,94 +125,38 @@ bool cutCallback(const ZoneNode& node, FindState* state) {
     return (false);
 }
 
-} // unnamed namespace
-
-
-namespace isc {
-namespace datasrc {
-namespace memory {
-
-namespace {
-    // convenience function to fill in the final details
-    isc::datasrc::memory::ZoneFinderResultContext
-    createFindResult(const ZoneData& zone_data,
-                     ZoneFinder::Result code,
-                     TreeNodeRRsetPtr rrset,
-                     const ZoneNode* node,
-                     bool wild = false) {
-        ZoneFinder::FindResultFlags flags = ZoneFinder::RESULT_DEFAULT;
-
-        if (wild) {
-            flags = flags | ZoneFinder::RESULT_WILDCARD;
-        }
-        if (code == ZoneFinder::NXRRSET || code == ZoneFinder::NXDOMAIN || wild) {
-            if (zone_data.isNSEC3Signed()) {
-                flags = flags | ZoneFinder::RESULT_NSEC3_SIGNED;
-            } else if (zone_data.isSigned()) {
-                flags = flags | ZoneFinder::RESULT_NSEC_SIGNED;
-            }
+// convenience function to fill in the final details
+isc::datasrc::memory::ZoneFinderResultContext
+createFindResult(const RRClass& rrclass,
+                 const ZoneData& zone_data,
+                 ZoneFinder::Result code,
+                 const RdataSet* rrset,
+                 const ZoneNode* node,
+                 bool wild = false) {
+    ZoneFinder::FindResultFlags flags = ZoneFinder::RESULT_DEFAULT;
+
+    if (wild) {
+        flags = flags | ZoneFinder::RESULT_WILDCARD;
+    }
+    if (code == ZoneFinder::NXRRSET || code == ZoneFinder::NXDOMAIN || wild) {
+        if (zone_data.isNSEC3Signed()) {
+            flags = flags | ZoneFinder::RESULT_NSEC3_SIGNED;
+        } else if (zone_data.isSigned()) {
+            flags = flags | ZoneFinder::RESULT_NSEC_SIGNED;
         }
-
-        return (ZoneFinderResultContext(code, rrset, flags, node));
     }
-} // end anonymous namespace
 
-class InMemoryZoneFinder::Context : public ZoneFinder::Context {
-public:
-    /// \brief Constructor.
-    ///
-    /// Note that we don't have a specific constructor for the findAll() case.
-    /// For (successful) type ANY query, found_node points to the
-    /// corresponding RB node, which is recorded within this specialized
-    /// context.
-    Context(ZoneFinder& finder, ZoneFinder::FindOptions options,
-            ZoneFinderResultContext result) :
-        ZoneFinder::Context(finder, options,
-                            ResultContext(result.code, result.rrset,
-                                          result.flags)),
-        rrset_(result.rrset), found_node_(result.found_node)
-    {}
-
-private:
-    const TreeNodeRRsetPtr rrset_;
-    const ZoneNode* const found_node_;
-};
-
-boost::shared_ptr<ZoneFinder::Context>
-InMemoryZoneFinder::find(const isc::dns::Name& name,
-                const isc::dns::RRType& type,
-                const FindOptions options)
-{
-    // call find_internal, and wrap the result in a ContextPtr
-    return ZoneFinderContextPtr(new Context(*this, options,
-                                            find_internal(name,
-                                                          type,
-                                                          NULL,
-                                                          options)));
-}
-
-boost::shared_ptr<ZoneFinder::Context>
-InMemoryZoneFinder::findAll(const isc::dns::Name& name,
-        std::vector<isc::dns::ConstRRsetPtr>& target,
-        const FindOptions options)
-{
-    // call find_internal, and wrap the result in a ContextPtr
-    return ZoneFinderContextPtr(new Context(*this, options,
-                                            find_internal(name,
-                                                          RRType::ANY(),
-                                                          &target,
-                                                          options)));
+    return (ZoneFinderResultContext(code, createTreeNodeRRset(node, rrset, rrclass), flags, node));
 }
 
-namespace {
-
-TreeNodeRRsetPtr
+const RdataSet*
 getClosestNSEC(const ZoneData& zone_data,
                ZoneNodeChain& chain,
+               const ZoneNode** nsec_node,
                ZoneFinder::FindOptions options)
 {
     if (!zone_data.isSigned() || (options & ZoneFinder::FIND_DNSSEC) == 0 || zone_data.isNSEC3Signed()) {
-        return (TreeNodeRRsetPtr());
+        return (NULL);
     }
 
     const ZoneNode* prev_node;
@@ -211,7 +164,8 @@ getClosestNSEC(const ZoneData& zone_data,
         if (!prev_node->isEmpty()) {
             const RdataSet* found = RdataSet::find(prev_node->getData(), RRType::NSEC());
             if (found != NULL) {
-                return (createTreeNodeRRset(prev_node, found));
+                *nsec_node = prev_node;
+                return (found);
             }
         }
     }
@@ -220,14 +174,14 @@ getClosestNSEC(const ZoneData& zone_data,
     assert(false);
     // Even though there is an assert here, strict compilers
     // will still need some return value.
-    return (TreeNodeRRsetPtr());
+    return (NULL);
 }
 
 // A helper function for the NXRRSET case in find().  If the zone is
 // NSEC-signed and DNSSEC records are requested, try to find NSEC
 // on the given node, and return it if found; return NULL for all other
 // cases.
-TreeNodeRRsetPtr
+const RdataSet*
 getNSECForNXRRSET(const ZoneData& zone_data,
                   ZoneFinder::FindOptions options,
                   const ZoneNode* node)
@@ -237,10 +191,10 @@ getNSECForNXRRSET(const ZoneData& zone_data,
         (options & ZoneFinder::FIND_DNSSEC) != 0) {
         const RdataSet* found = RdataSet::find(node->getData(), RRType::NSEC());
         if (found != NULL) {
-            return (createTreeNodeRRset(node, found));
+            return (found);
         }
     }
-    return (TreeNodeRRsetPtr());
+    return (NULL);
 }
 
 
@@ -255,8 +209,8 @@ public:
     static const unsigned int FIND_ZONECUT = 2;
 
     FindNodeResult(ZoneFinder::Result code_param,
-                   ZoneNode* node_param,
-                   TreeNodeRRsetPtr rrset_param,
+                   const ZoneNode* node_param,
+                   const RdataSet* rrset_param,
                    unsigned int flags_param = 0) :
         code(code_param),
         node(node_param),
@@ -265,7 +219,7 @@ public:
     {}
     const ZoneFinder::Result code;
     const ZoneNode* node;
-    TreeNodeRRsetPtr rrset;
+    const RdataSet* rrset;
     const unsigned int flags;
 };
 
@@ -289,25 +243,27 @@ FindNodeResult findNode(const ZoneData& zone_data,
         assert(node != NULL);
         if (state.dname_node_ != NULL) { // DNAME
             LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_DNAME_FOUND).
-                arg(state.rrset_->getName());
-            return (FindNodeResult(ZoneFinder::DNAME, NULL, state.rrset_));
+                arg(state.dname_node_->getName());
+            return (FindNodeResult(ZoneFinder::DNAME, state.dname_node_, state.rrset_));
         }
         if (state.zonecut_node_ != NULL) { // DELEGATION due to NS
             LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_DELEG_FOUND).
-                arg(state.rrset_->getName());
-            return (FindNodeResult(ZoneFinder::DELEGATION, NULL, state.rrset_));
+                arg(state.zonecut_node_->getName());
+            return (FindNodeResult(ZoneFinder::DELEGATION, state.zonecut_node_, state.rrset_));
         }
         if (chain.getLastComparisonResult().getRelation() ==
             NameComparisonResult::SUPERDOMAIN) { // empty node, so NXRRSET
             LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_SUPER_STOP).arg(name);
-            return (FindNodeResult(ZoneFinder::NXRRSET, node,
-                                   getClosestNSEC(zone_data, chain, options)));
+            const ZoneNode* nsec_node;
+            const RdataSet* nsec_rds = getClosestNSEC(zone_data, chain, &nsec_node, options);
+            return (FindNodeResult(ZoneFinder::NXRRSET, nsec_node, nsec_rds));
         }
         // TODO: wildcard (see memory_datasrc.cc:480)
         // Nothing really matched.
         LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_NOT_FOUND).arg(name);
-        return (FindNodeResult(ZoneFinder::NXDOMAIN, node,
-                               getClosestNSEC(zone_data, chain, options)));
+        const ZoneNode* nsec_node;
+        const RdataSet* nsec_rds = getClosestNSEC(zone_data, chain, &nsec_node, options);
+        return (FindNodeResult(ZoneFinder::NXDOMAIN, nsec_node, nsec_rds));
     } else {
         // If the name is neither an exact or partial match, it is
         // out of bailiwick, which is considered an error.
@@ -318,6 +274,53 @@ FindNodeResult findNode(const ZoneData& zone_data,
 
 } // end anonymous namespace
 
+class InMemoryZoneFinder::Context : public ZoneFinder::Context {
+public:
+    /// \brief Constructor.
+    ///
+    /// Note that we don't have a specific constructor for the findAll() case.
+    /// For (successful) type ANY query, found_node points to the
+    /// corresponding RB node, which is recorded within this specialized
+    /// context.
+    Context(ZoneFinder& finder, ZoneFinder::FindOptions options,
+            ZoneFinderResultContext result) :
+        ZoneFinder::Context(finder, options,
+                            ResultContext(result.code, result.rrset,
+                                          result.flags)),
+        rrset_(result.rrset), found_node_(result.found_node)
+    {}
+
+private:
+    const TreeNodeRRsetPtr rrset_;
+    const ZoneNode* const found_node_;
+};
+
+boost::shared_ptr<ZoneFinder::Context>
+InMemoryZoneFinder::find(const isc::dns::Name& name,
+                const isc::dns::RRType& type,
+                const FindOptions options)
+{
+    // call find_internal, and wrap the result in a ContextPtr
+    return ZoneFinderContextPtr(new Context(*this, options,
+                                            find_internal(name,
+                                                          type,
+                                                          NULL,
+                                                          options)));
+}
+
+boost::shared_ptr<ZoneFinder::Context>
+InMemoryZoneFinder::findAll(const isc::dns::Name& name,
+        std::vector<isc::dns::ConstRRsetPtr>& target,
+        const FindOptions options)
+{
+    // call find_internal, and wrap the result in a ContextPtr
+    return ZoneFinderContextPtr(new Context(*this, options,
+                                            find_internal(name,
+                                                          RRType::ANY(),
+                                                          &target,
+                                                          options)));
+}
+
 ZoneFinderResultContext
 InMemoryZoneFinder::find_internal(const isc::dns::Name& name,
                                   const isc::dns::RRType& type,
@@ -330,7 +333,7 @@ InMemoryZoneFinder::find_internal(const isc::dns::Name& name,
     const FindNodeResult node_result =
         findNode(zone_data_, name, chain, options);
     if (node_result.code != SUCCESS) {
-        return (createFindResult(zone_data_, node_result.code, node_result.rrset, NULL));
+        return (createFindResult(rrclass_, zone_data_, node_result.code, node_result.rrset, node_result.node));
     }
 
     const ZoneNode* node = node_result.node;
@@ -343,9 +346,11 @@ InMemoryZoneFinder::find_internal(const isc::dns::Name& name,
     if (node->isEmpty()) {
         LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_DOMAIN_EMPTY).
             arg(name);
-        return (createFindResult(zone_data_, NXRRSET,
-                                 getClosestNSEC(zone_data_, chain, options),
-                                 node));
+        const ZoneNode* nsec_node;
+        const RdataSet* nsec_rds = getClosestNSEC(zone_data_, chain, &nsec_node, options);
+        return (createFindResult(rrclass_, zone_data_, NXRRSET,
+                                 nsec_rds,
+                                 nsec_node));
     }
 
     const RdataSet* found;
@@ -361,7 +366,7 @@ InMemoryZoneFinder::find_internal(const isc::dns::Name& name,
             LOG_DEBUG(logger, DBG_TRACE_DATA,
                       DATASRC_MEM_EXACT_DELEGATION).arg(name);
             // TODO: rename argument (wildcards, see #2110)
-            return (createFindResult(zone_data_, DELEGATION, createTreeNodeRRset(node, found), node));
+            return (createFindResult(rrclass_, zone_data_, DELEGATION, found, node));
         }
     }
 
@@ -370,12 +375,12 @@ InMemoryZoneFinder::find_internal(const isc::dns::Name& name,
         // Empty domain will be handled as NXRRSET by normal processing
         const RdataSet* cur_rds = node->getData();
         while (cur_rds != NULL) {
-            target->push_back(createTreeNodeRRset(node, cur_rds));
+            target->push_back(createTreeNodeRRset(node, cur_rds, rrclass_));
             cur_rds = cur_rds->getNext();
         }
         LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_ANY_SUCCESS).
             arg(name);
-        return (createFindResult(zone_data_, SUCCESS, TreeNodeRRsetPtr(), node));
+        return (createFindResult(rrclass_, zone_data_, SUCCESS, NULL, node));
     }
 
     const RdataSet* currds = node->getData();
@@ -387,17 +392,17 @@ InMemoryZoneFinder::find_internal(const isc::dns::Name& name,
         // Good, it is here
         LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_SUCCESS).arg(name).
             arg(type);
-        return (createFindResult(zone_data_, SUCCESS, createTreeNodeRRset(node, found), node));
+        return (createFindResult(rrclass_, zone_data_, SUCCESS, found, node));
     } else {
         // Next, try CNAME.
         found = RdataSet::find(node->getData(), RRType::CNAME());
         if (found != NULL) {
             LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_CNAME).arg(name);
-            return (createFindResult(zone_data_, CNAME, createTreeNodeRRset(node, found), node));
+            return (createFindResult(rrclass_, zone_data_, CNAME, found, node));
         }
     }
     // No exact match or CNAME.  Get NSEC if necessary and return NXRRSET.
-    return (createFindResult(zone_data_, NXRRSET, getNSECForNXRRSET(zone_data_, options, node), node));
+    return (createFindResult(rrclass_, zone_data_, NXRRSET, getNSECForNXRRSET(zone_data_, options, node), node));
 }
 
 isc::datasrc::ZoneFinder::FindNSEC3Result
@@ -407,6 +412,6 @@ InMemoryZoneFinder::findNSEC3(const isc::dns::Name& name, bool recursive) {
     isc_throw(isc::NotImplemented, "not completed yet! please implement me");
 }
 
-}
-}
-}
+} // namespace memory
+} // namespace datasrc
+} // namespace isc

src/lib/datasrc/memory/zone_finder.h

@@ -50,7 +50,7 @@ public:
 
 class InMemoryZoneFinder : boost::noncopyable, public ZoneFinder {
 public:
-    InMemoryZoneFinder(const ZoneData& zone_data) : zone_data_(zone_data) {}
+    InMemoryZoneFinder(const ZoneData& zone_data, const isc::dns::RRClass& rrclass) : zone_data_(zone_data), rrclass_(rrclass) {}
 
     virtual boost::shared_ptr<ZoneFinder::Context> find(
         const isc::dns::Name& name,
@@ -84,6 +84,7 @@ private:
                                           FIND_DEFAULT);
 
     const ZoneData& zone_data_;
+    const isc::dns::RRClass& rrclass_;
 };
 
 } // namespace memory