[1579] suggested changes: made FindDNSSECContext more focused.

removed origin_ or complete flags_ from the class members.  non DNSSEC-specific
processing was recovered from the original version.
also made FindDNSSECContext private inside ZoneFinder; it doesn't have to
be publicly usable.
also made indentation policy more consistent for the class.

src/lib/datasrc/database.cc

@@ -601,11 +601,10 @@ DatabaseClient::Finder::findDelegationPoint(const isc::dns::Name& name,
 //
 // If none of the above applies in any level, the search fails with NXDOMAIN.
 ZoneFinder::ResultContext
-DatabaseClient::Finder::findWildcardMatch(const isc::dns::Name& name,
-                                          const isc::dns::RRType& type,
-                                          const DelegationSearchResult& dresult,
-                                          std::vector<isc::dns::ConstRRsetPtr>*
-                                          target, FindDNSSECContext& dnssec_ctx)
+DatabaseClient::Finder::findWildcardMatch(
+    const Name& name, const RRType& type, const FindOptions options,
+    const DelegationSearchResult& dresult, vector<ConstRRsetPtr>* target,
+    FindDNSSECContext& dnssec_ctx)
 {
     // Note that during the search we are going to search not only for the
     // requested type, but also for types that indicate a delegation -
@@ -633,7 +632,7 @@ DatabaseClient::Finder::findWildcardMatch(const isc::dns::Name& name,
             if (dresult.first_ns) {
                 // About to use first_ns.  The only way this can be set is if
                 // we are searching for glue, so do a sanity check.
-                if ((dnssec_ctx.getOptions() & FIND_GLUE_OK) == 0) {
+                if ((options & FIND_GLUE_OK) == 0) {
                     isc_throw(Unexpected, "Inconsistent conditions during "
                               "cancel of wilcard search for " <<
                               name.toText() << ": find_ns non-null when not "
@@ -649,8 +648,8 @@ DatabaseClient::Finder::findWildcardMatch(const isc::dns::Name& name,
             } else if (!hasSubdomains(name.split(i - 1).toText())) {
                 // The wildcard match is the best one, find the final result
                 // at it.  Note that wildcard should never be the zone origin.
-                return (findOnNameResult(name, type, found, &wildcard,
-                                         target, dnssec_ctx));
+                return (findOnNameResult(name, type, options, false, found,
+                                         &wildcard, target, dnssec_ctx));
             } else {
 
                 // more specified match found, cancel wildcard match
@@ -714,19 +713,14 @@ DatabaseClient::Finder::logAndCreateResult(
 
 DatabaseClient::Finder::FindDNSSECContext::FindDNSSECContext(
     DatabaseClient::Finder* finderp,
-    const isc::datasrc::ZoneFinder::FindOptions options,
-    const Name& origin) :
+    const FindOptions options) :
     finderp_(finderp),
-    options_(options),
-    origin_(origin),
+    need_dnssec_((options & FIND_DNSSEC) != 0),
+    is_nsec3_(false),
+    is_nsec_(false),
     initialized_(false)
 {}
 
-ZoneFinder::FindOptions
-DatabaseClient::Finder::FindDNSSECContext::getOptions() const {
-    return (options_);
-}
-
 void
 DatabaseClient::Finder::FindDNSSECContext::init() {
     if (finderp_ == NULL) {
@@ -734,19 +728,13 @@ DatabaseClient::Finder::FindDNSSECContext::init() {
     }
     if (!initialized_) {
         initialized_ = true;
-        // If NSEC3PARAM rrset exists, the zone looks like signed with NSEC3
-        is_nsec3_ = ((options_ & FIND_DNSSEC) == 0) ? false :
-            finderp_->isNSEC3();
-        if (is_nsec3_) {
-            // If the zone is signed with NSEC3, no need to check NSEC. If NSEC
-            // and NSEC3 coexist, according to RFC 5155 10-4, it should return
-            // result as NSEC3.
-            is_nsec_ = false;
-        } else {
+        if (need_dnssec_) {
+            // If NSEC3PARAM rrset exists, the zone looks like signed with
+            // NSEC3
+            is_nsec3_ = finderp_->isNSEC3();
             // If no NSEC3PARAM and it is DNSSEC query, check whether NSEC
             // exist in apex of zone
-            is_nsec_ = ((options_ & FIND_DNSSEC) == 0) ? false :
-                finderp_->isNSEC();
+            is_nsec_ = is_nsec3_ ? false : finderp_->isNSEC();
         }
     }
 }
@@ -804,29 +792,10 @@ DatabaseClient::Finder::FindDNSSECContext::getNSECRRset(const Name &name) const
     }
 }
 
-DatabaseClient::Finder::FoundRRsets
-DatabaseClient::Finder::FindDNSSECContext::getDNSSECRRset(const Name& name,
-                                                          const RRType& type)
-{
-    if (finderp_ == NULL) {
-        isc_throw(DataSourceError, "no Finder to query");
-    }
-    const bool is_origin = isOrigin(name);
-    WantedTypes final_types(isNSEC() ? FINAL_TYPES() : FINAL_TYPES_NO_NSEC());
-    final_types.insert(type);
-    return (finderp_->getRRsets(name.toText(), final_types, !is_origin, NULL,
-                                type == RRType::ANY()));
-}
-
-bool
-DatabaseClient::Finder::FindDNSSECContext::isOrigin(const Name &name) const {
-    return (name == origin_);
-}
-
 ZoneFinder::FindResultFlags
 DatabaseClient::Finder::FindDNSSECContext::getResultFlags() {
     // If it is not DNSSEC query, it should return RESULT_DEFAULT
-    if ((options_ & FIND_DNSSEC) == 0) {
+    if (!need_dnssec_) {
         return (RESULT_DEFAULT);
     }
     // If it is a DNSSEC query and the zone is signed with NSEC3, it should
@@ -843,6 +812,8 @@ DatabaseClient::Finder::FindDNSSECContext::getResultFlags() {
 ZoneFinder::ResultContext
 DatabaseClient::Finder::findOnNameResult(const Name& name,
                                          const RRType& type,
+                                         const FindOptions options,
+                                         const bool is_origin,
                                          const FoundRRsets& found,
                                          const string* wildname,
                                          std::vector<isc::dns::ConstRRsetPtr>*
@@ -853,7 +824,7 @@ DatabaseClient::Finder::findOnNameResult(const Name& name,
     // If the zone file is signed with NSEC3, need to set RESULT_NSEC3_SIGNED
     // flag in the flags. It is good for upper caller to deal with the query
     // response message.
-    if (wild & dnssec_ctx.isNSEC3()) {
+    if (wild && dnssec_ctx.isNSEC3()) {
         flags = (flags | RESULT_NSEC3_SIGNED);
     }
     // Get iterators for the different types of records we are interested in -
@@ -869,8 +840,7 @@ DatabaseClient::Finder::findOnNameResult(const Name& name,
         flags = (flags | RESULT_NSEC_SIGNED);
     }
 
-    if (!dnssec_ctx.isOrigin(name) &&
-        ((dnssec_ctx.getOptions() & FIND_GLUE_OK) == 0) &&
+    if (!is_origin && (options & FIND_GLUE_OK) == 0 &&
         nsi != found.second.end()) {
         // A NS RRset was found at the domain we were searching for.  As it is
         // not at the origin of the zone, it is a delegation and indicates that
@@ -938,16 +908,17 @@ DatabaseClient::Finder::findOnNameResult(const Name& name,
         // NULL for 'wildname'.
         return (logAndCreateResult(name, NULL, type, NXRRSET, nsec_rrset,
                                    DATASRC_DATABASE_FOUND_NXRRSET_NSEC,
-                                   (flags | RESULT_NSEC_SIGNED)));
+                                   flags | RESULT_NSEC_SIGNED));
     }
     return (logAndCreateResult(name, wildname, type, NXRRSET, nsec_rrset,
                                wild ? DATASRC_DATABASE_WILDCARD_NXRRSET :
                                DATASRC_DATABASE_FOUND_NXRRSET,
-                               (flags | dnssec_ctx.getResultFlags())));
+                               flags | dnssec_ctx.getResultFlags()));
 }
 
 ZoneFinder::ResultContext
 DatabaseClient::Finder::findNoNameResult(const Name& name, const RRType& type,
+                                         FindOptions options,
                                          const DelegationSearchResult& dresult,
                                          std::vector<isc::dns::ConstRRsetPtr>*
                                          target, FindDNSSECContext& dnssec_ctx)
@@ -965,18 +936,19 @@ DatabaseClient::Finder::findNoNameResult(const Name& name, const RRType& type,
             arg(accessor_->getDBName()).arg(name);
         const ConstRRsetPtr nsec = dnssec_ctx.isNSEC() ? findNSECCover(name) :
             ConstRRsetPtr();
-        if(dnssec_ctx.isNSEC() && !nsec){
+        if (dnssec_ctx.isNSEC() && !nsec) {
             isc_throw(DataSourceError,
                       "no NSEC RR covers in the NSEC signed zone");
         }
         return (ResultContext(NXRRSET, nsec, dnssec_ctx.getResultFlags()));
-    } else if ((dnssec_ctx.getOptions() & NO_WILDCARD) == 0) {
+    } else if ((options & NO_WILDCARD) == 0) {
         // It's not an empty non-terminal and wildcard matching is not
         // disabled, so check for wildcards. If there is a wildcard match
         // (i.e. all results except NXDOMAIN) return it; otherwise fall
         // through to the NXDOMAIN case below.
         const ResultContext wcontext =
-            findWildcardMatch(name, type, dresult, target, dnssec_ctx);
+            findWildcardMatch(name, type, options, dresult, target,
+                              dnssec_ctx);
         if (wcontext.code != NXDOMAIN) {
             return (wcontext);
         }
@@ -988,7 +960,7 @@ DatabaseClient::Finder::findNoNameResult(const Name& name, const RRType& type,
               arg(accessor_->getDBName()).arg(name).arg(type).arg(getClass());
     const ConstRRsetPtr nsec = dnssec_ctx.isNSEC() ? findNSECCover(name) :
         ConstRRsetPtr();
-    if(dnssec_ctx.isNSEC() && !nsec){
+    if (dnssec_ctx.isNSEC() && !nsec) {
         isc_throw(DataSourceError, "no NSEC RR covers in the NSEC signed zone");
     }
     return (ResultContext(NXDOMAIN, nsec, dnssec_ctx.getResultFlags()));
@@ -1057,16 +1029,23 @@ DatabaseClient::Finder::findInternal(const Name& name, const RRType& type,
     // - Requested name is a delegation point (NS only but not at the zone
     //   apex - DNAME is ignored here as it redirects DNS names subordinate to
     //   the owner name - the owner name itself is not redirected.)
-    FindDNSSECContext dnssec_ctx(this, options, getOrigin());
-    const FoundRRsets found = dnssec_ctx.getDNSSECRRset(name, type);
+    const bool is_origin = (name == getOrigin());
+    WantedTypes final_types(FINAL_TYPES());
+    final_types.insert(type);
+    const FoundRRsets found = getRRsets(name.toText(), final_types,
+                                        !is_origin, NULL,
+                                        type == RRType::ANY());
+    FindDNSSECContext dnssec_ctx(this, options);
     if (found.first) {
         // Something found at the domain name.  Look into it further to get
         // the final result.
-        return (findOnNameResult(name, type, found, NULL, target, dnssec_ctx));
+        return (findOnNameResult(name, type, options, is_origin, found, NULL,
+                                 target, dnssec_ctx));
     } else {
         // Did not find anything at all at the domain name, so check for
         // subdomains or wildcards.
-        return (findNoNameResult(name, type, dresult, target, dnssec_ctx));
+        return (findNoNameResult(name, type, options, dresult, target,
+                                 dnssec_ctx));
     }
 }
 

src/lib/datasrc/database.h

@@ -688,127 +688,7 @@ public:
     ///
     /// Methods directly corresponds to the ones in ZoneFinder.
     class Finder : public ZoneFinder {
-
-        /// \brief Helper to the findInterval.
-        ///
-        /// Get the ResultFlags for findInterval. If the zone is signed with
-        /// NSEC3, it will return RESULT_NSEC3_SIGNED. If it is signed with
-        /// NSEC, it wll return RESULT_NSEC_SIGNED. Otherwise it will return
-        /// RESULT_DEFAULT. It wraps getRRsets function to do some special
-        /// search, like searching NSEC RRset by getNSECRRset function,
-        /// searching DNSSEC related RRset and RRsig by getNSECRRset.
-        class FindDNSSECContext {
-            public:
-
-                /// \brief Constructor for FindDNSSECContext class.
-                ///
-                /// It initalize a helper for findInterval function.
-                ///
-                /// \param finderp The Finder piont for search.
-                /// \param options Search options.
-                /// \param origin The origin name for this finder.
-                FindDNSSECContext(Finder* finderp, const FindOptions options,
-                                  const isc::dns::Name& origin);
-
-                /// \brief Get result flags of this query.
-                /// \return ResultFlags for this query. If the zone file is
-                /// signed with NSEC, is will return RESULT_NSEC_SIGNED with
-                /// dnssec query. If the zone file is signed with NSEC3, it
-                /// will return RESULT_NSEC3_SIGNED with dnssec query, others
-                /// it should return RESULT_DEFAULT.
-                ZoneFinder::FindResultFlags getResultFlags();
-
-                typedef std::pair<bool, std::map<dns::RRType, dns::RRsetPtr> >
-                    FoundRRsets;
-                /// \brief Get DNSSEC RRset for the queried name.
-                ///
-                /// It should return the RRsets and RRsigs for the queried
-                /// name with designated type.
-                ///
-                /// \param name The queried name.
-                /// \param type The queried type.
-                /// \return RRsets and RRsigs that are matched.
-                FoundRRsets getDNSSECRRset(const isc::dns::Name& name,
-                                           const isc::dns::RRType& type);
-
-                /// \brief Get the needed NSEC RRset.
-                ///
-                /// It should return the needed NSEC RRset.
-                ///
-                /// \param name The name which the NSEC RRset belong to.
-                /// \return the needed NSEC RRsets.
-                isc::dns::ConstRRsetPtr getNSECRRset(const isc::dns::Name&
-                                                     name) const;
-
-                /// \brief Get the needed NSEC RRset.
-                ///
-                /// It should return the needed NSEC RRset.
-                ///
-                /// \param found_set The RRset which contain the NSEC an other
-                /// type RRs.
-                /// \return the needed NSEC RRsets.
-                isc::dns::ConstRRsetPtr getNSECRRset(const FoundRRsets&
-                                                     found_set) const;
-
-                /// \brief Check whether the zone file is signed with NSECi3.
-                ///
-                /// It checks whether the zone file is signed with NSEC3. If
-                /// yes, return true, otherwise return false.
-                ///
-                /// \return True for NSEC3, false otherwise.
-                bool isNSEC3();
-
-                /// \brief Check whether the zone file is signed with NSEC.
-                ///
-                /// It checks whether the zone file is signed with NSEC, If
-                /// yes, return true, otherwise return false.
-                ///
-                /// \return True for NSEC, false otherwise.
-                bool isNSEC();
-
-                /// \brief Check whether the name is origin name of the zone.
-                ///
-                /// It checks whether the name is origin name of the zone. Yes
-                /// for the origin name, false for not.
-                ///
-                /// \param name The queried name.
-                /// \return True for origin name, false for not.
-                bool isOrigin(const isc::dns::Name& name) const;
-
-                /// \brief get the options of queried.
-                ///
-                /// It return the queried options.
-                ///
-                /// \return It return the queried options.
-                ZoneFinder::FindOptions getOptions() const;
-            private:
-
-                /// \brief Init the attributes in this entity.
-                ///
-                /// It should init the attributes of this entity. Check whether
-                /// it is the NSEC or NSEC3 zone file if it is a dnssec query.
-                ///
-                /// \note If the entity is initialized, no need to init it
-                /// again.
-                void init();
-
-                /// \brief Check whether the entity is initalized.
-                ///
-                /// It should return true if the entity is inited, else return
-                /// false.
-                ///
-                /// \return True for inited, else return false.
-                bool isInited();
-                DatabaseClient::Finder* finderp_;
-                ZoneFinder::ZoneFinder::FindOptions options_;
-                FindResultFlags flags_;
-                isc::dns::Name origin_;
-                bool is_nsec3_;
-                bool is_nsec_;
-                bool initialized_;
-        };
-
-        public:
+    public:
         /// \brief Constructor
         ///
         /// \param database The database (shared with DatabaseClient) to
@@ -971,7 +851,94 @@ public:
                               const std::string* construct_name = NULL,
                               bool any = false);
 
-    private:
+        /// \brief Helper to the findInterval.
+        ///
+        /// Get the ResultFlags for findInterval. If the zone is signed with
+        /// NSEC3, it will return RESULT_NSEC3_SIGNED. If it is signed with
+        /// NSEC, it wll return RESULT_NSEC_SIGNED. Otherwise it will return
+        /// RESULT_DEFAULT. It wraps getRRsets function to do some special
+        /// search, like searching NSEC RRset by getNSECRRset function,
+        /// searching DNSSEC related RRset and RRsig by getNSECRRset.
+        class FindDNSSECContext {
+        public:
+            /// \brief Constructor for FindDNSSECContext class.
+            ///
+            /// It initalize a helper for findInterval function.
+            ///
+            /// \param finderp The Finder piont for search.
+            /// \param options Search options.
+            FindDNSSECContext(Finder* finderp, const FindOptions options);
+
+            /// \brief Get result flags of this query.
+            /// \return ResultFlags for this query. If the zone file is
+            /// signed with NSEC, is will return RESULT_NSEC_SIGNED with
+            /// dnssec query. If the zone file is signed with NSEC3, it
+            /// will return RESULT_NSEC3_SIGNED with dnssec query, others
+            /// it should return RESULT_DEFAULT.
+            ZoneFinder::FindResultFlags getResultFlags();
+
+            /// \brief Get the needed NSEC RRset.
+            ///
+            /// It should return the needed NSEC RRset.
+            ///
+            /// \param name The name which the NSEC RRset belong to.
+            /// \return the needed NSEC RRsets.
+            isc::dns::ConstRRsetPtr getNSECRRset(const isc::dns::Name&
+                                                 name) const;
+
+            /// \brief Get the needed NSEC RRset.
+            ///
+            /// It should return the needed NSEC RRset.
+            ///
+            /// \param found_set The RRset which contain the NSEC an other
+            /// type RRs.
+            /// \return the needed NSEC RRsets.
+            isc::dns::ConstRRsetPtr getNSECRRset(const FoundRRsets&
+                                                 found_set) const;
+
+            /// \brief Check whether the zone file is signed with NSECi3.
+            ///
+            /// It checks whether the zone file is signed with NSEC3. If
+            /// yes, return true, otherwise return false.
+            ///
+            /// \return True for NSEC3, false otherwise.
+            bool isNSEC3();
+
+            /// \brief Check whether the zone file is signed with NSEC.
+            ///
+            /// It checks whether the zone file is signed with NSEC, If
+            /// yes, return true, otherwise return false.
+            ///
+            /// \return True for NSEC, false otherwise.
+            bool isNSEC();
+
+        private:
+            /// \brief Init the attributes in this entity.
+            ///
+            /// It should init the attributes of this entity. Check whether
+            /// it is the NSEC or NSEC3 zone file if it is a dnssec query.
+            ///
+            /// \note If the entity is initialized, no need to init it
+            /// again.
+            void init();
+
+            /// \brief Check whether the entity is initialized.
+            ///
+            /// It should return true if the entity is inited, else return
+            /// false.
+            ///
+            /// \return True for inited, else return false.
+            bool isInited();
+
+            DatabaseClient::Finder* const finderp_;
+            const bool need_dnssec_;
+
+            FindResultFlags flags_;
+            bool is_nsec3_;
+            bool is_nsec_;
+            bool initialized_;
+        };
+
         /// \brief Search result of \c findDelegationPoint().
         ///
         /// This is a tuple combining the result of the search - a status code
@@ -1067,6 +1034,8 @@ public:
         ///
         /// \param name The name to find
         /// \param type The RRType to find
+        /// \param options Options about how to search. See the documentation
+        ///        for ZoneFinder::FindOptions.
         /// \param dresult Result of the search through the zone for a
         ///        delegation.
         /// \param target If the type happens to be ANY, it will insert all
@@ -1083,6 +1052,7 @@ public:
         ///         NXDOMAIN or an NXRRSET.
         ResultContext findWildcardMatch(const isc::dns::Name& name,
                                         const isc::dns::RRType& type,
+                                        const FindOptions options,
                                         const DelegationSearchResult& dresult,
                                         std::vector<isc::dns::ConstRRsetPtr>*
                                         target, FindDNSSECContext& dnssec_ctx);
@@ -1106,6 +1076,9 @@ public:
         ///
         /// \param name The name to find
         /// \param type The RRType to find
+        /// \param options Options about how to search. See the documentation
+        ///        for ZoneFinder::FindOptions.
+        /// \param is_origin If name is the zone's origin name.
         /// \param found A set of found RRsets in the search for the name
         ///        and type.  It could contain one or more of the requested
         ///        type, CNAME, NS, and NSEC RRsets of the name.
@@ -1126,6 +1099,8 @@ public:
         ///         method.
         ResultContext findOnNameResult(const isc::dns::Name& name,
                                        const isc::dns::RRType& type,
+                                       const FindOptions options,
+                                       const bool is_origin,
                                        const FoundRRsets& found,
                                        const std::string* wildname,
                                        std::vector<isc::dns::ConstRRsetPtr>*
@@ -1163,6 +1138,7 @@ public:
         ///         success due to an exact match).
         ResultContext findNoNameResult(const isc::dns::Name& name,
                                        const isc::dns::RRType& type,
+                                       FindOptions options,
                                        const DelegationSearchResult& dresult,
                                        std::vector<isc::dns::ConstRRsetPtr>*
                                        target, FindDNSSECContext& dnssec_ctx);
@@ -1218,8 +1194,6 @@ public:
         /// To find stuff in the result of getRRsets.
         typedef std::map<dns::RRType, dns::RRsetPtr>::const_iterator
             FoundIterator;
-
-
     };
 
     /// \brief Find a zone in the database