[2060] (unrelated) make the finder Context class "more basic"

by defining a generic derived class and moving optional member variables
there.
this essentially addresses the issue of #1767.

src/bin/auth/tests/auth_srv_unittest.cc

@@ -1286,7 +1286,7 @@ public:
         if (fake_rrset_ && fake_rrset_->getName() == name &&
             fake_rrset_->getType() == type)
         {
-            return (ZoneFinderContextPtr(new ZoneFinder::Context(
+            return (ZoneFinderContextPtr(new ZoneFinder::GenericContext(
                                              *this, options,
                                              ResultContext(SUCCESS,
                                                            fake_rrset_))));

src/bin/auth/tests/query_unittest.cc

@@ -442,10 +442,9 @@ public:
                        ConstRRsetPtr rrset)
     {
         nsec_name_ = nsec_name;
-        nsec_context_.reset(new Context(*this,
-                                        FIND_DEFAULT, // a fake value
-                                        ResultContext(code, rrset,
-                                                      RESULT_NSEC_SIGNED)));
+        nsec_context_.reset(
+            new GenericContext(*this, FIND_DEFAULT, // a fake value
+                               ResultContext(code, rrset, RESULT_NSEC_SIGNED)));
     }
 
     // Once called, the findNSEC3 will return the provided result for the next
@@ -487,8 +486,8 @@ protected:
     {
         ConstRRsetPtr rp = stripRRsigs(rrset, options);
         return (ZoneFinderContextPtr(
-                    new Context(*this, options,
-                                ResultContext(code, rp, flags))));
+                    new GenericContext(*this, options,
+                                       ResultContext(code, rp, flags))));
     }
 
 private:
@@ -604,9 +603,9 @@ MockZoneFinder::findAll(const Name& name, std::vector<ConstRRsetPtr>& target,
                 target.push_back(stripRRsigs(found_rrset->second, options));
             }
             return (ZoneFinderContextPtr(
-                        new Context(*this, options,
-                                    ResultContext(SUCCESS, RRsetPtr()),
-                                    target)));
+                        new GenericContext(*this, options,
+                                           ResultContext(SUCCESS, RRsetPtr()),
+                                           target)));
         }
     }
 

src/lib/datasrc/database.cc

@@ -386,10 +386,11 @@ DatabaseClient::Finder::findAll(const isc::dns::Name& name,
                                 std::vector<isc::dns::ConstRRsetPtr>& target,
                                 const FindOptions options)
 {
-    return (ZoneFinderContextPtr(new Context(*this, options,
-                                             findInternal(name, RRType::ANY(),
-                                                          &target, options),
-                                             target)));
+    return (ZoneFinderContextPtr(new GenericContext(
+                                     *this, options,
+                                     findInternal(name, RRType::ANY(),
+                                                  &target, options),
+                                     target)));
 }
 
 ZoneFinderContextPtr
@@ -400,9 +401,10 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
     if (type == RRType::ANY()) {
         isc_throw(isc::Unexpected, "Use findAll to answer ANY");
     }
-    return (ZoneFinderContextPtr(new Context(*this, options,
-                                             findInternal(name, type, NULL,
-                                                          options))));
+    return (ZoneFinderContextPtr(new GenericContext(
+                                     *this, options,
+                                     findInternal(name, type, NULL,
+                                                  options))));
 }
 
 DatabaseClient::Finder::DelegationSearchResult

src/lib/datasrc/memory/zone_finder.cc

@@ -528,17 +528,26 @@ FindNodeResult findNode(const ZoneData& zone_data,
 /// context.
 class InMemoryZoneFinder::Context : public ZoneFinder::Context {
 public:
-    Context(ZoneFinder& finder, ZoneFinder::FindOptions options,
+    Context(InMemoryZoneFinder& finder, ZoneFinder::FindOptions options,
             const RRClass& rrclass, const ZoneFinderResultContext& result) :
-        ZoneFinder::Context(finder, options,
-                            ResultContext(result.code, result.rrset,
-                                          result.flags)),
+        ZoneFinder::Context(options, ResultContext(result.code, result.rrset,
+                                                   result.flags)),
+        finder_(finder),        // NOTE: when #2284 is done we won't need this
         rrclass_(rrclass), zone_data_(result.zone_data),
         found_node_(result.found_node),
         found_rdset_(result.found_rdset)
     {}
 
 protected:
+    // When #2284 is done this can simply return NULL.
+    virtual ZoneFinder* getFinder() { return (&finder_); }
+
+    // We don't use the default protected methods that rely on this method,
+    // so we can simply return NULL.
+    virtual const std::vector<isc::dns::ConstRRsetPtr>* getAllRRsets() const {
+        return (NULL);
+    }
+
     virtual void getAdditionalImpl(const std::vector<RRType>& requested_types,
                                    std::vector<ConstRRsetPtr>& result)
     {
@@ -620,6 +629,7 @@ private:
     }
 
 private:
+    InMemoryZoneFinder& finder_;
     const RRClass rrclass_;
     const ZoneData* const zone_data_;
     const ZoneNode* const found_node_;

src/lib/datasrc/memory_datasrc.cc

@@ -792,13 +792,19 @@ public:
     /// context.
     Context(ZoneFinder& finder, ZoneFinder::FindOptions options,
             const RBNodeResultContext& result) :
-        ZoneFinder::Context(finder, options,
+        ZoneFinder::Context(options,
                             ResultContext(result.code, result.rrset,
                                           result.flags)),
-        rrset_(result.rrset), found_node_(result.found_node)
+        finder_(finder), rrset_(result.rrset), found_node_(result.found_node)
     {}
 
 protected:
+    virtual ZoneFinder* getFinder() { return (&finder_); }
+
+    virtual const std::vector<isc::dns::ConstRRsetPtr>* getAllRRsets() const {
+        return (NULL);
+    }
+
     virtual void getAdditionalImpl(const vector<RRType>& requested_types,
                                    vector<ConstRRsetPtr>& result)
     {
@@ -866,6 +872,7 @@ private:
         }
     }
 
+    ZoneFinder& finder_;
     const ConstRBNodeRRsetPtr rrset_;
     const DomainNode* const found_node_;
 };

src/lib/datasrc/zone.h

@@ -168,48 +168,25 @@ public:
     /// can define a derived class of the base Context and override the
     /// specific virtual method.
     ///
+    /// This base class defines these common protected methods along with
+    /// some helper pure virtual methods that would be necessary for the
+    /// common methods.  If a derived class wants to use the common version
+    /// of the protected method, it needs to provide expected result through
+    /// their implementation of the pure virtual methods.
+    ///
     /// This class object is generally expected to be associated with the
     /// ZoneFinder that originally performed the \c find() call, and expects
     /// the finder is valid throughout the lifetime of this object.  It's
     /// caller's responsibility to ensure that assumption.
     class Context {
     public:
-        /// \brief The constructor for the normal find call.
-        ///
-        /// This constructor is expected to be called from the \c find()
-        /// method when it constructs the return value.
+        /// \brief The constructor.
         ///
-        /// \param finder The ZoneFinder on which find() is called.
         /// \param options The find options specified for the find() call.
         /// \param result The result of the find() call.
-        Context(ZoneFinder& finder, FindOptions options,
-                const ResultContext& result) :
-            code(result.code), rrset(result.rrset),
-            finder_(finder), flags_(result.flags), options_(options)
-        {}
-
-        /// \brief The constructor for the normal findAll call.
-        ///
-        /// This constructor is expected to be called from the \c findAll()
-        /// method when it constructs the return value.
-        ///
-        /// It copies the vector that is to be returned to the caller of
-        /// \c findAll() for possible subsequent use.  Note that it cannot
-        /// simply hold a reference to the vector because the caller may
-        /// alter it after the \c findAll() call.
-        ///
-        /// \param finder The ZoneFinder on which findAll() is called.
-        /// \param options The find options specified for the findAll() call.
-        /// \param result The result of the findAll() call (whose rrset is
-        ///        expected to be NULL).
-        /// \param all_set Reference to the vector given by the caller of
-        ///       \c findAll(), storing the RRsets to be returned.
-        Context(ZoneFinder& finder, FindOptions options,
-                const ResultContext& result,
-                const std::vector<isc::dns::ConstRRsetPtr> &all_set) :
+        Context(FindOptions options, const ResultContext& result) :
             code(result.code), rrset(result.rrset),
-            finder_(finder), flags_(result.flags), options_(options),
-            all_set_(all_set)
+            flags_(result.flags), options_(options)
         {}
 
         /// \brief The destructor.
@@ -353,11 +330,34 @@ public:
         }
 
     protected:
+        /// \brief Return the \c ZoneFinder that created this \c Context.
+        ///
+        /// A derived class implementation can return NULL if it specializes
+        /// other protected methods that require a non NULL result from
+        /// this method.  Otherwise it must return a valid, non NULL pointer
+        /// to the \c ZoneFinder object.
+        virtual ZoneFinder* getFinder() = 0;
+
+        /// \brief Return a vector of RRsets corresponding to findAll() result.
+        ///
+        /// This method returns a set of RRsets that correspond to the
+        /// returned RRsets to a prior \c findAll() call.
+        ///
+        /// A derived class implementation can return NULL if it specializes
+        /// other protected methods that require a non NULL result from
+        /// this method.  Otherwise it must return a valid, non NULL pointer
+        /// to a vector that correspond to the expected set of RRsets.
+        virtual const std::vector<isc::dns::ConstRRsetPtr>*
+        getAllRRsets() const = 0;
+
         /// \brief Actual implementation of getAdditional().
         ///
         /// This base class defines a default implementation that can be
         /// used for any type of data sources.  A data source implementation
         /// can override it.
+        ///
+        /// The default version of this implementation requires both
+        /// \c getFinder() and \c getAllRRsets() return valid results.
         virtual void getAdditionalImpl(
             const std::vector<isc::dns::RRType>& requested_types,
             std::vector<isc::dns::ConstRRsetPtr>& result);
@@ -367,14 +367,72 @@ public:
         /// This base class defines a default implementation that can be
         /// used for any type of data sources.  A data source implementation
         /// can override it.
+        ///
+        /// The default version of this implementation requires
+        /// \c getFinder() return a valid result.
         virtual dns::ConstRRsetPtr getAtOriginImpl(const dns::RRType& type);
 
     private:
-        ZoneFinder& finder_;
+
         const FindResultFlags flags_;
     protected:
         const FindOptions options_;
+    };
+
+    /// \brief Generic ZoneFinder context that works for all implementations.
+    ///
+    /// This is a concrete derived class of \c ZoneFinder::Context that
+    /// only use the generic (default) versions of the protected methods
+    /// and therefore work for any data source implementation.
+    ///
+    /// A data source implementation can use this class to create a
+    /// \c Context object as a return value of \c find() or \c findAll()
+    /// method if it doesn't have to optimize specific protected methods.
+    class GenericContext : public Context {
+    public:
+        /// \brief The constructor for the normal find call.
+        ///
+        /// This constructor is expected to be called from the \c find()
+        /// method when it constructs the return value.
+        ///
+        /// \param finder The ZoneFinder on which find() is called.
+        /// \param options See the \c Context class.
+        /// \param result See the \c Context class.
+        GenericContext(ZoneFinder& finder, FindOptions options,
+                       const ResultContext& result) :
+            Context(options, result), finder_(finder)
+        {}
+
+        /// \brief The constructor for the normal findAll call.
+        ///
+        /// This constructor is expected to be called from the \c findAll()
+        /// method when it constructs the return value.
+        ///
+        /// It copies the vector that is to be returned to the caller of
+        /// \c findAll() for possible subsequent use.  Note that it cannot
+        /// simply hold a reference to the vector because the caller may
+        /// alter it after the \c findAll() call.
+        ///
+        /// \param finder The ZoneFinder on which findAll() is called.
+        /// \param options See the \c Context class.
+        /// \param result See the \c Context class.
+        /// \param all_set Reference to the vector given by the caller of
+        ///       \c findAll(), storing the RRsets to be returned.
+        GenericContext(ZoneFinder& finder, FindOptions options,
+                       const ResultContext& result,
+                       const std::vector<isc::dns::ConstRRsetPtr>& all_set) :
+            Context(options, result), finder_(finder), all_set_(all_set)
+        {}
+
+    protected:
+        virtual ZoneFinder* getFinder() { return (&finder_); }
+        virtual const std::vector<isc::dns::ConstRRsetPtr>*
+        getAllRRsets() const {
+            return (&all_set_);
+        }
+
     private:
+        ZoneFinder& finder_;
         std::vector<isc::dns::ConstRRsetPtr> all_set_;
     };
 

src/lib/datasrc/zone_finder_context.cc

@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <exceptions/exceptions.h>
+
 #include <dns/rdata.h>
 #include <dns/rrset.h>
 #include <dns/rrtype.h>
@@ -87,13 +89,22 @@ ZoneFinder::Context::getAdditionalImpl(const vector<RRType>& requested_types,
 {
     // If rrset is non NULL, it should have been SUCCESS/DELEGATION; otherwise
     // we should have responded to type ANY query.
+    ZoneFinder* finder = getFinder();
+    if (finder == NULL) {
+        // This is a bug of the derived class implementation.
+        isc_throw(isc::Unexpected, "NULL ZoneFinder in finder Context");
+    }
     if (rrset) {
-        getAdditionalForRRset(finder_, *rrset, requested_types, result,
+        getAdditionalForRRset(*finder, *rrset, requested_types, result,
                               options_);
         return;
     }
-    BOOST_FOREACH(ConstRRsetPtr rrset_in_set, all_set_) {
-        getAdditionalForRRset(finder_, *rrset_in_set, requested_types, result,
+    const vector<ConstRRsetPtr>* all_sets = getAllRRsets();
+    if (all_sets == NULL) {     // bug of the derived class implementation.
+        isc_throw(isc::Unexpected, "All RRsets is NULL in finder Context");
+    }
+    BOOST_FOREACH(ConstRRsetPtr rrset_in_set, *getAllRRsets()) {
+        getAdditionalForRRset(*finder, *rrset_in_set, requested_types, result,
                               options_);
     }
 }
@@ -103,7 +114,14 @@ ZoneFinder::Context::getAtOriginImpl(const dns::RRType& type) {
     const ZoneFinder::FindOptions options =
         (options_ & ZoneFinder::FIND_DNSSEC) != 0 ?
         ZoneFinder::FIND_DNSSEC : ZoneFinder::FIND_DEFAULT;
-    ConstZoneFinderContextPtr ctx = finder_.find(finder_.getOrigin(), type,
+
+    ZoneFinder* finder = getFinder();
+    if (finder == NULL) {
+        // This is a bug of the derived class implementation.
+        isc_throw(isc::Unexpected, "NULL ZoneFinder in finder Context");
+    }
+
+    ConstZoneFinderContextPtr ctx = finder->find(finder->getOrigin(), type,
                                                  options);
     if (ctx->code == ZoneFinder::SUCCESS) {
         return (ctx->rrset);