[2282] Merge branch 'trac2219' into trac2282

configure.ac

@@ -1207,11 +1207,11 @@ AC_CONFIG_FILES([Makefile
                  src/lib/exceptions/tests/Makefile
                  src/lib/datasrc/Makefile
                  src/lib/datasrc/memory/Makefile
-                 src/lib/datasrc/memory/tests/Makefile
-                 src/lib/datasrc/memory/tests/testdata/Makefile
                  src/lib/datasrc/memory/benchmarks/Makefile
                  src/lib/datasrc/tests/Makefile
                  src/lib/datasrc/tests/testdata/Makefile
+                 src/lib/datasrc/tests/memory/Makefile
+                 src/lib/datasrc/tests/memory/testdata/Makefile
                  src/lib/xfr/Makefile
                  src/lib/xfr/tests/Makefile
                  src/lib/log/Makefile

src/bin/auth/Makefile.am

@@ -57,12 +57,6 @@ b10_auth_SOURCES += common.h common.cc
 b10_auth_SOURCES += statistics.cc statistics.h
 b10_auth_SOURCES += datasrc_configurator.h
 b10_auth_SOURCES += main.cc
-# This is a temporary workaround for #1206, where the InMemoryClient has been
-# moved to an ldopened library. We could add that library to LDADD, but that
-# is nonportable. This should've been moot after #1207, but there is still
-# one dependency; the in-memory-specific zone loader call is still in
-# auth.
-b10_auth_SOURCES += ${top_srcdir}/src/lib/datasrc/memory_datasrc.cc
 
 nodist_b10_auth_SOURCES = auth_messages.h auth_messages.cc
 EXTRA_DIST += auth_messages.mes

src/bin/auth/benchmarks/Makefile.am

@@ -17,12 +17,6 @@ query_bench_SOURCES += ../auth_srv.h ../auth_srv.cc
 query_bench_SOURCES += ../auth_config.h ../auth_config.cc
 query_bench_SOURCES += ../statistics.h ../statistics.cc
 query_bench_SOURCES += ../auth_log.h ../auth_log.cc
-# This is a temporary workaround for #1206, where the InMemoryClient has been
-# moved to an ldopened library. We could add that library to LDADD, but that
-# is nonportable. When #1207 is done this becomes moot anyway, and the
-# specific workaround is not needed anymore, so we can then remove this
-# line again.
-query_bench_SOURCES += ${top_srcdir}/src/lib/datasrc/memory_datasrc.cc
 
 nodist_query_bench_SOURCES = ../auth_messages.h ../auth_messages.cc
 

src/bin/auth/tests/Makefile.am

@@ -52,13 +52,6 @@ run_unittests_SOURCES += query_unittest.cc
 run_unittests_SOURCES += statistics_unittest.cc
 run_unittests_SOURCES += datasrc_configurator_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
-# This is a temporary workaround for #1206, where the InMemoryClient has been
-# moved to an ldopened library. We could add that library to LDADD, but that
-# is nonportable. This should've been moot after #1207, but there is still
-# one dependency; the in-memory-specific zone loader call is still in
-# auth.
-run_unittests_SOURCES += ${top_srcdir}/src/lib/datasrc/memory_datasrc.cc
-
 
 nodist_run_unittests_SOURCES = ../auth_messages.h ../auth_messages.cc
 

src/bin/auth/tests/auth_srv_unittest.cc

@@ -15,6 +15,7 @@
 #include <config.h>
 
 #include <util/io/sockaddr_util.h>
+#include <util/memory_segment_local.h>
 
 #include <dns/message.h>
 #include <dns/messagerenderer.h>
@@ -1393,16 +1394,19 @@ public:
              const isc::datasrc::DataSourceClientPtr
                  client(new FakeClient(info.data_src_client_ != NULL ?
                                        info.data_src_client_ :
-                                       info.cache_.get(),
+                                       info.getCacheClient(),
                                        throw_when, isc_exception, fake_rrset));
              clients_.push_back(client);
-             data_sources_.push_back(DataSourceInfo(client.get(),
-                 isc::datasrc::DataSourceClientContainerPtr(), false));
+             data_sources_.push_back(
+                 DataSourceInfo(client.get(),
+                                isc::datasrc::DataSourceClientContainerPtr(),
+                                false, RRClass::IN(), mem_sgmt_));
         }
     }
 private:
     const boost::shared_ptr<isc::datasrc::ConfigurableClientList> real_;
     vector<isc::datasrc::DataSourceClientPtr> clients_;
+    MemorySegmentLocal mem_sgmt_;
 };
 
 } // end anonymous namespace for throwing proxy classes

src/lib/datasrc/Makefile.am

@@ -1,4 +1,4 @@
-SUBDIRS = . memory tests
+SUBDIRS = memory . tests
 
 AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
 AM_CPPFLAGS += -I$(top_srcdir)/src/lib/dns -I$(top_builddir)/src/lib/dns
@@ -64,6 +64,7 @@ libb10_datasrc_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.
 libb10_datasrc_la_LIBADD += $(top_builddir)/src/lib/dns/libb10-dns++.la
 libb10_datasrc_la_LIBADD += $(top_builddir)/src/lib/log/libb10-log.la
 libb10_datasrc_la_LIBADD += $(top_builddir)/src/lib/cc/libb10-cc.la
+libb10_datasrc_la_LIBADD += $(builddir)/memory/libdatasrc_memory.la
 libb10_datasrc_la_LIBADD += $(SQLITE_LIBS)
 
 BUILT_SOURCES = datasrc_config.h datasrc_messages.h datasrc_messages.cc

src/lib/datasrc/client_list.cc

@@ -12,10 +12,12 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <util/memory_segment_local.h>
+
 #include "client_list.h"
 #include "client.h"
 #include "factory.h"
-#include "memory_datasrc.h"
+#include "memory/memory_client.h"
 #include "logger.h"
 #include <dns/masterload.h>
 
@@ -25,32 +27,58 @@
 using namespace isc::data;
 using namespace isc::dns;
 using namespace std;
+using isc::util::MemorySegment;
 using boost::lexical_cast;
 using boost::shared_ptr;
 using boost::dynamic_pointer_cast;
+using isc::datasrc::memory::InMemoryClient;
 
 namespace isc {
 namespace datasrc {
 
 ConfigurableClientList::DataSourceInfo::DataSourceInfo(
     DataSourceClient* data_src_client,
-    const DataSourceClientContainerPtr& container, bool has_cache) :
+    const DataSourceClientContainerPtr& container, bool has_cache,
+    const RRClass& rrclass, MemorySegment& mem_sgmt) :
     data_src_client_(data_src_client),
     container_(container)
 {
     if (has_cache) {
-        cache_.reset(new InMemoryClient);
+        cache_.reset(new InMemoryClient(mem_sgmt, rrclass));
     }
 }
 
-ConfigurableClientList::DataSourceInfo::DataSourceInfo(bool has_cache) :
+ConfigurableClientList::DataSourceInfo::DataSourceInfo(
+    const RRClass& rrclass, MemorySegment& mem_sgmt, bool has_cache) :
     data_src_client_(NULL)
 {
     if (has_cache) {
-        cache_.reset(new InMemoryClient);
+        cache_.reset(new InMemoryClient(mem_sgmt, rrclass));
     }
 }
 
+const DataSourceClient*
+ConfigurableClientList::DataSourceInfo::getCacheClient() const {
+    return (cache_.get());
+}
+
+ConfigurableClientList::ConfigurableClientList(const RRClass& rrclass) :
+    rrclass_(rrclass),
+    mem_sgmt_(new util::MemorySegmentLocal),
+    configuration_(new isc::data::ListElement),
+    allow_cache_(false)
+{}
+
+ConfigurableClientList::~ConfigurableClientList() {
+    // Explicitly clear the contained data source clients, and check memory
+    // leak.  assert() (with abort on failure) may be too harsh, but
+    // it's probably better to find more leaks initially.  Once it's stabilized
+    // we should probably revisit it.
+
+    data_sources_.clear();
+    assert(mem_sgmt_->allMemoryDeallocated());
+}
+
 void
 ConfigurableClientList::configure(const ConstElementPtr& config,
                                   bool allow_cache)
@@ -98,14 +126,16 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                     isc_throw(ConfigurationError, "The cache must be enabled "
                               "for the MasterFiles type");
                 }
-                new_data_sources.push_back(DataSourceInfo(true));
+                new_data_sources.push_back(DataSourceInfo(rrclass_, *mem_sgmt_,
+                                                          true));
             } else {
                 // Ask the factory to create the data source for us
                 const DataSourcePair ds(this->getDataSourceClient(type,
                                                                   paramConf));
                 // And put it into the vector
                 new_data_sources.push_back(DataSourceInfo(ds.first, ds.second,
-                                                          want_cache));
+                                                          want_cache, rrclass_,
+                                                          *mem_sgmt_));
             }
 
             if (want_cache) {
@@ -141,13 +171,10 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                 for (vector<string>::const_iterator it(zones_origins.begin());
                      it != zones_origins.end(); ++it) {
                     const Name origin(*it);
-                    shared_ptr<InMemoryZoneFinder>
-                        finder(new
-                            InMemoryZoneFinder(rrclass_, origin));
                     if (type == "MasterFiles") {
                         try {
-                            finder->load(paramConf->get(*it)->stringValue());
-                            cache->addZone(finder);
+                            cache->load(origin,
+                                        paramConf->get(*it)->stringValue());
                         } catch (const isc::dns::MasterLoadError& mle) {
                             LOG_ERROR(logger, DATASRC_MASTERLOAD_ERROR)
                                 .arg(mle.what());
@@ -165,8 +192,7 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                             isc_throw(isc::Unexpected, "Got NULL iterator "
                                       "for zone " << origin);
                         }
-                        finder->load(*iterator);
-                        cache->addZone(finder);
+                        cache->load(origin, *iterator);
                     }
                 }
             }
@@ -324,14 +350,11 @@ ConfigurableClientList::reload(const Name& name) {
     }
     // Try to convert the finder to in-memory one. If it is the cache,
     // it should work.
-    shared_ptr<InMemoryZoneFinder>
-        finder(dynamic_pointer_cast<InMemoryZoneFinder>(result.finder));
-    const DataSourceInfo* info(result.info);
     // It is of a different type or there's no cache.
-    if (!info->cache_ || !finder) {
+    if (!result.info->cache_) {
         return (ZONE_NOT_CACHED);
     }
-    DataSourceClient* client(info->data_src_client_);
+    DataSourceClient* client(result.info->data_src_client_);
     if (client) {
         // Now do the final reload. If it does not exist in client,
         // DataSourceError is thrown, which is exactly the result what we
@@ -340,15 +363,15 @@ ConfigurableClientList::reload(const Name& name) {
         if (!iterator) {
             isc_throw(isc::Unexpected, "Null iterator from " << name);
         }
-        finder->load(*iterator);
+        result.info->cache_->load(name, *iterator);
     } else {
         // The MasterFiles special case
-        const string filename(finder->getFileName());
+        const string filename(result.info->cache_->getFileName(name));
         if (filename.empty()) {
             isc_throw(isc::Unexpected, "Confused about missing both filename "
                       "and data source");
         }
-        finder->load(filename);
+        result.info->cache_->load(name, filename);
     }
     return (ZONE_RELOADED);
 }

src/lib/datasrc/client_list.h

@@ -15,6 +15,8 @@
 #ifndef DATASRC_CONTAINER_H
 #define DATASRC_CONTAINER_H
 
+#include <util/memory_segment.h>
+
 #include <dns/name.h>
 #include <dns/rrclass.h>
 #include <cc/data.h>
@@ -22,6 +24,7 @@
 
 #include <vector>
 #include <boost/shared_ptr.hpp>
+#include <boost/scoped_ptr.hpp>
 #include <boost/noncopyable.hpp>
 
 namespace isc {
@@ -34,7 +37,13 @@ typedef boost::shared_ptr<DataSourceClient> DataSourceClientPtr;
 class DataSourceClientContainer;
 typedef boost::shared_ptr<DataSourceClientContainer>
     DataSourceClientContainerPtr;
+
+// XXX: it's better to even hide the existence of the "memory" namespace.
+// We should probably consider pimpl for details of ConfigurableClientList
+// and hide real definitions except for itself and tests.
+namespace memory {
 class InMemoryClient;
+}
 
 /// \brief The list of data source clients.
 ///
@@ -209,11 +218,11 @@ public:
     /// \brief Constructor
     ///
     /// \param rrclass For which class the list should work.
-    ConfigurableClientList(const isc::dns::RRClass &rrclass) :
-        rrclass_(rrclass),
-        configuration_(new isc::data::ListElement),
-        allow_cache_(false)
-    {}
+    ConfigurableClientList(const isc::dns::RRClass& rrclass);
+
+    /// \brief Destructor
+    virtual ~ConfigurableClientList();
+
     /// \brief Exception thrown when there's an error in configuration.
     class ConfigurationError : public Exception {
     public:
@@ -290,24 +299,27 @@ public:
     /// \todo The content yet to be defined.
     struct DataSourceInfo {
         // Plays a role of default constructor too (for vector)
-        DataSourceInfo(bool has_cache = false);
+        DataSourceInfo(const dns::RRClass& rrclass,
+                       util::MemorySegment& mem_sgmt,
+                       bool has_cache = false);
         DataSourceInfo(DataSourceClient* data_src_client,
                        const DataSourceClientContainerPtr& container,
-                       bool has_cache);
+                       bool has_cache, const dns::RRClass& rrclass,
+                       util::MemorySegment& mem_sgmt);
         DataSourceClient* data_src_client_;
         DataSourceClientContainerPtr container_;
-        boost::shared_ptr<InMemoryClient> cache_;
+
+        // Accessor to cache_ in the form of DataSourceClient, hiding
+        // the existence of InMemoryClient as much as possible.  We should
+        // really consider cleaner abstraction, but for now it works.
+        // This is also only intended to be used in auth unit tests right now.
+        // No other applications or tests may use it.
+        const DataSourceClient* getCacheClient() const;
+        boost::shared_ptr<memory::InMemoryClient> cache_;
     };
 
     /// \brief The collection of data sources.
     typedef std::vector<DataSourceInfo> DataSources;
-protected:
-    /// \brief The data sources held here.
-    ///
-    /// All our data sources are stored here. It is protected to let the
-    /// tests in. You should consider it private if you ever want to
-    /// derive this class (which is not really recommended anyway).
-    DataSources data_sources_;
 
     /// \brief Convenience type alias.
     ///
@@ -357,10 +369,26 @@ private:
     void findInternal(MutableResult& result, const dns::Name& name,
                       bool want_exact_match, bool want_finder) const;
     const isc::dns::RRClass rrclass_;
+
+    /// \brief Memory segment for in-memory cache.
+    ///
+    /// Note that this must be placed before data_sources_ so it won't be
+    /// destroyed before the built objects in the destructor.
+    boost::scoped_ptr<util::MemorySegment> mem_sgmt_;
+
     /// \brief Currently active configuration.
     isc::data::ConstElementPtr configuration_;
+
     /// \brief The last set value of allow_cache.
     bool allow_cache_;
+
+protected:
+    /// \brief The data sources held here.
+    ///
+    /// All our data sources are stored here. It is protected to let the
+    /// tests in. You should consider it private if you ever want to
+    /// derive this class (which is not really recommended anyway).
+    DataSources data_sources_;
 };
 
 } // namespace datasrc

src/lib/datasrc/memory/Makefile.am

@@ -1,4 +1,4 @@
-SUBDIRS = . tests benchmarks
+SUBDIRS = . benchmarks
 
 AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
 AM_CPPFLAGS += -I$(top_srcdir)/src/lib/dns -I$(top_builddir)/src/lib/dns

src/lib/datasrc/memory/domaintree.h

@@ -373,11 +373,24 @@ private:
         }
     }
 
+public:
+    /// \brief returns if the node is a subtree's root node
+    ///
+    /// This method takes a node and returns \c true if it is the root
+    /// node of the subtree it belongs to.
+    ///
+    /// This method never throws an exception.
     bool isSubTreeRoot() const {
         return ((flags_ & FLAG_SUBTREE_ROOT) != 0);
     }
 
-public:
+    /// \brief returns the root of its subtree
+    ///
+    /// This method takes a node and returns the root of its subtree.
+    ///
+    /// This method never throws an exception.
+    const DomainTreeNode<T>* getSubTreeRoot() const;
+
     /// \brief returns the parent of the root of its subtree
     ///
     /// This method takes a node and returns the parent of the root of
@@ -388,7 +401,14 @@ public:
     /// This method never throws an exception.
     const DomainTreeNode<T>* getUpperNode() const;
 
-private:
+    /// \brief returns the largest node of this node's subtree
+    ///
+    /// This method takes a node and returns the largest node in its
+    /// subtree.
+    ///
+    /// This method never throws an exception.
+    const DomainTreeNode<T>* getLargestInSubTree() const;
+
     /// \brief return the next node which is bigger than current node
     /// in the same subtree
     ///
@@ -423,6 +443,7 @@ private:
     /// This method never throws an exception.
     const DomainTreeNode<T>* predecessor() const;
 
+private:
     /// \brief private shared implementation of successor and predecessor
     ///
     /// As the two mentioned functions are merely mirror images of each other,
@@ -538,7 +559,7 @@ DomainTreeNode<T>::~DomainTreeNode() {
 
 template <typename T>
 const DomainTreeNode<T>*
-DomainTreeNode<T>::getUpperNode() const {
+DomainTreeNode<T>::getSubTreeRoot() const {
     const DomainTreeNode<T>* current = this;
 
     // current would never be equal to NULL here (in a correct tree
@@ -547,7 +568,24 @@ DomainTreeNode<T>::getUpperNode() const {
         current = current->getParent();
     }
 
-    return (current->getParent());
+    return (current);
+}
+
+template <typename T>
+const DomainTreeNode<T>*
+DomainTreeNode<T>::getUpperNode() const {
+    return (getSubTreeRoot()->getParent());
+}
+
+template <typename T>
+const DomainTreeNode<T>*
+DomainTreeNode<T>::getLargestInSubTree() const {
+    const DomainTreeNode<T>* sroot = getSubTreeRoot();
+    while (sroot->getRight() != NULL) {
+        sroot = sroot->getRight();
+    }
+
+    return (sroot);
 }
 
 template <typename T>

src/lib/datasrc/memory/memory_client.cc

@@ -22,6 +22,7 @@
 #include <datasrc/memory/domaintree.h>
 #include <datasrc/memory/segment_object_holder.h>
 #include <datasrc/memory/treenode_rrset.h>
+#include <datasrc/memory/zone_finder.h>
 
 #include <util/memory_segment_local.h>
 
@@ -100,9 +101,6 @@ public:
         FileNameTree::destroy(mem_sgmt_, file_name_tree_, deleter);
 
         ZoneTable::destroy(mem_sgmt_, zone_table_, rrclass_);
-
-        // see above for the assert().
-        assert(mem_sgmt_.allMemoryDeallocated());
     }
 
     util::MemorySegment& mem_sgmt_;
@@ -341,12 +339,15 @@ public:
             }
         }
 
+        // Make just the NSEC3 hash label uppercase, and insert the
+        // entire name into the NSEC3Data ZoneTree.
         string fst_label = rrset->getName().split(0, 1).toText(true);
         transform(fst_label.begin(), fst_label.end(), fst_label.begin(),
                   ::toupper);
+        const string rest = rrset->getName().split(1).toText(true);
 
         ZoneNode* node;
-        nsec3_data->insertName(mem_sgmt_, Name(fst_label), &node);
+        nsec3_data->insertName(mem_sgmt_, Name(fst_label + "." + rest), &node);
 
         RdataEncoder encoder;
 
@@ -606,7 +607,7 @@ InMemoryClient::InMemoryClientImpl::load(
     }
 
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEMORY_MEM_ADD_ZONE).
-        arg(zone_name).arg(rrclass_.toText());
+        arg(zone_name).arg(rrclass_);
 
     // Set the filename in file_name_tree_ now, so that getFileName()
     // can use it (during zone reloading).
@@ -686,21 +687,25 @@ InMemoryClient::getZoneCount() const {
     return (impl_->zone_count_);
 }
 
-isc::datasrc::memory::ZoneTable::FindResult
-InMemoryClient::findZone2(const isc::dns::Name& zone_name) const {
+isc::datasrc::DataSourceClient::FindResult
+InMemoryClient::findZone(const isc::dns::Name& zone_name) const {
     LOG_DEBUG(logger, DBG_TRACE_DATA,
               DATASRC_MEMORY_MEM_FIND_ZONE).arg(zone_name);
+
     ZoneTable::FindResult result(impl_->zone_table_->findZone(zone_name));
-    return (result);
+
+    ZoneFinderPtr finder;
+    if (result.code != result::NOTFOUND) {
+        finder.reset(new InMemoryZoneFinder(*result.zone_data, getClass()));
+    }
+
+    return (DataSourceClient::FindResult(result.code, finder));
 }
 
-isc::datasrc::DataSourceClient::FindResult
-InMemoryClient::findZone(const isc::dns::Name&) const {
-    // This variant of findZone() is not implemented and should be
-    // removed eventually. It currently throws an exception. It is
-    // required right now to derive from DataSourceClient.
-    isc_throw(isc::NotImplemented,
-              "This variant of findZone() is not implemented.");
+isc::datasrc::memory::ZoneTable::FindResult
+InMemoryClient::findZone2(const isc::dns::Name& zone_name) const {
+    ZoneTable::FindResult result(impl_->zone_table_->findZone(zone_name));
+    return (result);
 }
 
 result::Result

src/lib/datasrc/memory/memory_client.h

@@ -20,10 +20,6 @@
 #include <datasrc/iterator.h>
 #include <datasrc/client.h>
 #include <datasrc/memory/zone_table.h>
-
-// for isc::datasrc::ZoneTable::FindResult returned by findZone(). This
-// variant of findZone() is not implemented and should be removed
-// eventually.
 #include <datasrc/zonetable.h>
 
 #include <string>
@@ -206,6 +202,13 @@ public:
         { }
     };
 
+    /// Returns a \c ZoneFinder result that best matches the given name.
+    ///
+    /// This derived version of the method never throws an exception.
+    /// For other details see \c DataSourceClient::findZone().
+    virtual isc::datasrc::DataSourceClient::FindResult
+    findZone(const isc::dns::Name& name) const;
+
     /// Returns a \c ZoneTable result that best matches the given name.
     ///
     /// This derived version of the method never throws an exception.
@@ -213,12 +216,6 @@ public:
     virtual isc::datasrc::memory::ZoneTable::FindResult
     findZone2(const isc::dns::Name& name) const;
 
-    // This variant of findZone() is not implemented and should be
-    // removed eventually. It currently throws an exception. It is
-    // required right now to derive from DataSourceClient.
-    virtual isc::datasrc::DataSourceClient::FindResult
-    findZone(const isc::dns::Name& name) const;
-
     /// \brief Implementation of the getIterator method
     virtual isc::datasrc::ZoneIteratorPtr
     getIterator(const isc::dns::Name& name, bool separate_rrs = false) const;

src/lib/datasrc/memory/treenode_rrset.cc

@@ -79,10 +79,6 @@ TreeNodeRRset::setTTL(const RRTTL&) {
 
 std::string
 TreeNodeRRset::toText() const {
-    // Create TTL from internal data
-    util::InputBuffer ttl_buffer(rdataset_->getTTLData(), sizeof(uint32_t));
-    const RRTTL ttl(ttl_buffer);
-
     // Dump the main RRset, if not empty
     std::string ret;
     RRsetPtr tmp_rrset;
@@ -92,7 +88,7 @@ TreeNodeRRset::toText() const {
     {
         if (!tmp_rrset) {
             tmp_rrset = RRsetPtr(new RRset(getName(), rrclass_, getType(),
-                                           ttl));
+                                           getTTL()));
         }
         tmp_rrset->addRdata(rit->getCurrent());
     }
@@ -101,17 +97,7 @@ TreeNodeRRset::toText() const {
     }
 
     // Dump any RRSIGs
-    tmp_rrset.reset();
-    for (RdataIteratorPtr rit = getSigRdataIterator();
-         !rit->isLast();
-         rit->next())
-    {
-        if (!tmp_rrset) {
-            tmp_rrset = RRsetPtr(new RRset(getName(), rrclass_,
-                                           RRType::RRSIG(), ttl));
-        }
-        tmp_rrset->addRdata(rit->getCurrent());
-    }
+    tmp_rrset = getRRsig();
     if (tmp_rrset) {
         ret += tmp_rrset->toText();
     }
@@ -292,7 +278,26 @@ TreeNodeRRset::getSigRdataIterator() const {
 
 RRsetPtr
 TreeNodeRRset::getRRsig() const {
-    isc_throw(Unexpected, "unexpected method called on TreeNodeRRset");
+    // Shortcut: if DNSSEC is disabled for the RRset, simply return NULL.
+    // The generic code below provides the same behavior, but with much more
+    // overhead.
+    if (!dnssec_ok_) {
+        return (RRsetPtr());
+    }
+
+    RRsetPtr tmp_rrset;
+    for (RdataIteratorPtr rit = getSigRdataIterator();
+         !rit->isLast();
+         rit->next())
+    {
+        if (!tmp_rrset) {
+            tmp_rrset = RRsetPtr(new RRset(getName(), rrclass_,
+                                           RRType::RRSIG(), getTTL()));
+        }
+        tmp_rrset->addRdata(rit->getCurrent());
+    }
+
+    return (tmp_rrset);
 }
 
 void

src/lib/datasrc/memory/treenode_rrset.h

@@ -190,8 +190,6 @@ public:
     virtual dns::RdataIteratorPtr getRdataIterator() const;
 
     /// \brief Specialized version of \c getRRsig() for \c TreeNodeRRset.
-    ///
-    /// It throws \c isc::Unexpected unconditionally.
     virtual dns::RRsetPtr getRRsig() const;
 
     virtual unsigned int getRRsigDataCount() const {

src/lib/datasrc/memory/zone_finder.cc

@@ -23,11 +23,18 @@
 #include <dns/rrset.h>
 #include <dns/rrtype.h>
 
+#include <util/buffer.h>
+#include <util/encode/base32hex.h>
+#include <util/hash/sha1.h>
+
 #include <datasrc/logger.h>
 
 using namespace isc::dns;
 using namespace isc::datasrc::memory;
 using namespace isc::datasrc;
+using namespace isc::util;
+using namespace isc::util::encode;
+using namespace isc::util::hash;
 
 namespace isc {
 namespace datasrc {
@@ -51,18 +58,21 @@ TreeNodeRRsetPtr
 createTreeNodeRRset(const ZoneNode* node,
                     const RdataSet* rdataset,
                     const RRClass& rrclass,
+                    ZoneFinder::FindOptions options,
                     const Name* realname = NULL)
 {
+    const bool dnssec = ((options & ZoneFinder::FIND_DNSSEC) != 0);
     if (node != NULL && rdataset != NULL) {
         if (realname != NULL) {
-            return TreeNodeRRsetPtr(new TreeNodeRRset(*realname, rrclass, node,
-                                                      rdataset, true));
+            return (TreeNodeRRsetPtr(new TreeNodeRRset(*realname, rrclass,
+                                                       node, rdataset,
+                                                       dnssec)));
         } else {
-            return TreeNodeRRsetPtr(new TreeNodeRRset(rrclass, node,
-                                                      rdataset, true));
+            return (TreeNodeRRsetPtr(new TreeNodeRRset(rrclass, node, rdataset,
+                                                       dnssec)));
         }
     } else {
-        return TreeNodeRRsetPtr();
+        return (TreeNodeRRsetPtr());
     }
 }
 
@@ -162,10 +172,12 @@ isc::datasrc::memory::ZoneFinderResultContext
 createFindResult(const RRClass& rrclass,
                  const ZoneData& zone_data,
                  ZoneFinder::Result code,
-                 const RdataSet* rrset,
+                 const RdataSet* rdset,
                  const ZoneNode* node,
+                 ZoneFinder::FindOptions options,
                  bool wild = false,
-                 const Name* qname = NULL) {
+                 const Name* qname = NULL)
+{
     ZoneFinder::FindResultFlags flags = ZoneFinder::RESULT_DEFAULT;
     const Name* rename = NULL;
 
@@ -182,8 +194,9 @@ createFindResult(const RRClass& rrclass,
         }
     }
 
-    return (ZoneFinderResultContext(code, createTreeNodeRRset(node, rrset,
-                                                              rrclass, rename),
+    return (ZoneFinderResultContext(code, createTreeNodeRRset(node, rdset,
+                                                              rrclass, options,
+                                                              rename),
                                     flags, node));
 }
 
@@ -446,15 +459,59 @@ FindNodeResult findNode(const ZoneData& zone_data,
 
 } // end anonymous namespace
 
-// Specialization of the ZoneFinder::Context for the in-memory finder.
+inline void
+iterateSHA1(SHA1Context* ctx, const uint8_t* input, size_t inlength,
+            const uint8_t* salt, size_t saltlen,
+            uint8_t output[SHA1_HASHSIZE])
+{
+    SHA1Reset(ctx);
+    SHA1Input(ctx, input, inlength);
+    SHA1Input(ctx, salt, saltlen); // this works whether saltlen == or > 0
+    SHA1Result(ctx, output);
+}
+
+std::string
+InMemoryZoneFinderNSEC3Calculate(const Name& name,
+                                 const uint16_t iterations,
+                                 const uint8_t* salt,
+                                 size_t salt_len) {
+    // We first need to normalize the name by converting all upper case
+    // characters in the labels to lower ones.
+    OutputBuffer obuf(Name::MAX_WIRE);
+    Name name_copy(name);
+    name_copy.downcase();
+    name_copy.toWire(obuf);
+
+    const uint8_t* const salt_buf = (salt_len > 0) ? salt : NULL;
+    std::vector<uint8_t> digest(SHA1_HASHSIZE);
+    uint8_t* const digest_buf = &digest[0];
+
+    SHA1Context sha1_ctx;
+    iterateSHA1(&sha1_ctx, static_cast<const uint8_t*>(obuf.getData()),
+                obuf.getLength(), salt_buf, salt_len, digest_buf);
+    for (unsigned int n = 0; n < iterations; ++n) {
+        iterateSHA1(&sha1_ctx, digest_buf, SHA1_HASHSIZE,
+                    salt_buf, salt_len,
+                    digest_buf);
+    }
+
+    return (encodeBase32Hex(digest));
+}
+
+/// \brief Specialization of the ZoneFinder::Context for the in-memory finder.
+///
+/// \note Right now we don't implement optimization using this specialized
+/// version, but assuming we'll do pretty soon we'll keep and use the
+/// definition.  The note below will apply at that point (and at that point
+/// we should remove the other constructor for findAll()).
+///
+/// Note that we don't have a specific constructor for the findAll() case.
+/// For (successful) type ANY query, found_node points to the
+/// corresponding zone node, which is recorded within this specialized
+/// context.
 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.
+    /// \brief Constructor for normal find().
     Context(ZoneFinder& finder, ZoneFinder::FindOptions options,
             ZoneFinderResultContext result) :
         ZoneFinder::Context(finder, options,
@@ -463,6 +520,16 @@ public:
         rrset_(result.rrset), found_node_(result.found_node)
     {}
 
+    /// \brief Constructor for findAll().
+    Context(ZoneFinder& finder, ZoneFinder::FindOptions options,
+            ZoneFinderResultContext result,
+            std::vector<isc::dns::ConstRRsetPtr>& target) :
+        ZoneFinder::Context(finder, options,
+                            ResultContext(result.code, result.rrset,
+                                          result.flags), target),
+        rrset_(result.rrset), found_node_(result.found_node)
+    {}
+
 private:
     const TreeNodeRRsetPtr rrset_;
     const ZoneNode* const found_node_;
@@ -473,11 +540,9 @@ InMemoryZoneFinder::find(const isc::dns::Name& name,
                 const isc::dns::RRType& type,
                 const FindOptions options)
 {
-    return ZoneFinderContextPtr(new Context(*this, options,
-                                            find_internal(name,
-                                                          type,
-                                                          NULL,
-                                                          options)));
+    return (ZoneFinderContextPtr(new Context(*this, options,
+                                             find_internal(name, type,
+                                                           NULL, options))));
 }
 
 boost::shared_ptr<ZoneFinder::Context>
@@ -485,11 +550,12 @@ InMemoryZoneFinder::findAll(const isc::dns::Name& name,
         std::vector<isc::dns::ConstRRsetPtr>& target,
         const FindOptions options)
 {
-    return ZoneFinderContextPtr(new Context(*this, options,
-                                            find_internal(name,
-                                                          RRType::ANY(),
-                                                          &target,
-                                                          options)));
+    return (ZoneFinderContextPtr(new Context(*this, options,
+                                             find_internal(name,
+                                                           RRType::ANY(),
+                                                           &target,
+                                                           options),
+                                             target)));
 }
 
 ZoneFinderResultContext
@@ -505,7 +571,8 @@ InMemoryZoneFinder::find_internal(const isc::dns::Name& name,
         findNode(zone_data_, name, node_path, options);
     if (node_result.code != SUCCESS) {
         return (createFindResult(rrclass_, zone_data_, node_result.code,
-                                 node_result.rrset, node_result.node));
+                                 node_result.rrset, node_result.node,
+                                 options));
     }
 
     const ZoneNode* node = node_result.node;
@@ -524,25 +591,26 @@ InMemoryZoneFinder::find_internal(const isc::dns::Name& name,
         const RdataSet* nsec_rds = getClosestNSEC(zone_data_, node_path,
                                                   &nsec_node, options);
         return (createFindResult(rrclass_, zone_data_, NXRRSET,
-                                 nsec_rds,
-                                 nsec_node,
-                                 wild));
+                                 nsec_rds, nsec_node, options, wild));
     }
 
     const RdataSet* found;
 
     // If the node callback is enabled, this may be a zone cut.  If it
     // has a NS RR, we should return a delegation, but not in the apex.
-    // There is one exception: the case for DS query, which should always
-    // be considered in-zone lookup.
+    // There are two exceptions:
+    // - the case for DS query, which should always be considered in-zone
+    //   lookup.
+    // - when we are looking for glue records (FIND_GLUE_OK)
     if (node->getFlag(ZoneNode::FLAG_CALLBACK) &&
-            node != zone_data_.getOriginNode() && type != RRType::DS()) {
+        (options & FIND_GLUE_OK) == 0 &&
+        node != zone_data_.getOriginNode() && type != RRType::DS()) {
         found = RdataSet::find(node->getData(), RRType::NS());
         if (found != NULL) {
             LOG_DEBUG(logger, DBG_TRACE_DATA,
                       DATASRC_MEM_EXACT_DELEGATION).arg(name);
             return (createFindResult(rrclass_, zone_data_, DELEGATION,
-                                     found, node, wild, &name));
+                                     found, node, options, wild, &name));
         }
     }
 
@@ -552,13 +620,13 @@ InMemoryZoneFinder::find_internal(const isc::dns::Name& name,
         const RdataSet* cur_rds = node->getData();
         while (cur_rds != NULL) {
             target->push_back(createTreeNodeRRset(node, cur_rds, rrclass_,
-                                                  &name));
+                                                  options, &name));
             cur_rds = cur_rds->getNext();
         }
         LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_ANY_SUCCESS).
             arg(name);
         return (createFindResult(rrclass_, zone_data_, SUCCESS, NULL, node,
-                                 wild, &name));
+                                 options, wild, &name));
     }
 
     found = RdataSet::find(node->getData(), type);
@@ -567,7 +635,7 @@ InMemoryZoneFinder::find_internal(const isc::dns::Name& name,
         LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_SUCCESS).arg(name).
             arg(type);
         return (createFindResult(rrclass_, zone_data_, SUCCESS, found, node,
-                                 wild, &name));
+                                 options, wild, &name));
     } else {
         // Next, try CNAME.
         found = RdataSet::find(node->getData(), RRType::CNAME());
@@ -575,20 +643,153 @@ InMemoryZoneFinder::find_internal(const isc::dns::Name& name,
 
             LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_CNAME).arg(name);
             return (createFindResult(rrclass_, zone_data_, CNAME, found, node,
-                                     wild, &name));
+                                     options, wild, &name));
         }
     }
     // No exact match or CNAME.  Get NSEC if necessary and return NXRRSET.
     return (createFindResult(rrclass_, zone_data_, NXRRSET,
                              getNSECForNXRRSET(zone_data_, options, node),
-                             node, wild, &name));
+                             node, options, wild, &name));
 }
 
 isc::datasrc::ZoneFinder::FindNSEC3Result
 InMemoryZoneFinder::findNSEC3(const isc::dns::Name& name, bool recursive) {
-    (void)name;
-    (void)recursive;
-    isc_throw(isc::NotImplemented, "not completed yet! please implement me");
+    LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_FINDNSEC3).arg(name).
+        arg(recursive ? "recursive" : "non-recursive");
+
+    if (!zone_data_.isNSEC3Signed()) {
+        isc_throw(DataSourceError,
+                  "findNSEC3 attempt for non NSEC3 signed zone: " <<
+                  getOrigin() << "/" << getClass());
+    }
+
+    const NameComparisonResult cmp_result = name.compare(getOrigin());
+    if (cmp_result.getRelation() != NameComparisonResult::EQUAL &&
+        cmp_result.getRelation() != NameComparisonResult::SUBDOMAIN) {
+        isc_throw(OutOfZone, "findNSEC3 attempt for out-of-zone name: "
+                  << name << ", zone: " << getOrigin() << "/"
+                  << getClass());
+    }
+
+    // Convenient shortcuts
+    const ZoneFinder::FindOptions options =
+        ZoneFinder::FIND_DNSSEC; // NSEC3 implies DNSSEC
+    const unsigned int olabels = getOrigin().getLabelCount();
+    const unsigned int qlabels = name.getLabelCount();
+    const NSEC3Data* nsec3_data = zone_data_.getNSEC3Data();
+
+    const ZoneNode* covering_node(NULL); // placeholder of the next closer proof
+    // Examine all names from the query name to the origin name, stripping
+    // the deepest label one by one, until we find a name that has a matching
+    // NSEC3 hash.
+    for (unsigned int labels = qlabels; labels >= olabels; --labels) {
+        const std::string hlabel = (nsec3_calculate_)
+            ((labels == qlabels ?
+              name : name.split(qlabels - labels, labels)),
+             nsec3_data->iterations,
+             nsec3_data->getSaltData(),
+             nsec3_data->getSaltLen());
+
+        LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_FINDNSEC3_TRYHASH).
+            arg(name).arg(labels).arg(hlabel);
+
+        const ZoneTree& tree = nsec3_data->getNSEC3Tree();
+
+        ZoneNode* node(NULL);
+        ZoneChain chain;
+
+        ZoneTree::Result result =
+            tree.find(Name(hlabel + "." + getOrigin().toText()), &node, chain);
+
+        if (result == ZoneTree::EXACTMATCH) {
+            // We found an exact match.
+            RdataSet* set = node->getData();
+            ConstRRsetPtr closest = createTreeNodeRRset(node, set, getClass(),
+                                                        options);
+            ConstRRsetPtr next =
+                createTreeNodeRRset(covering_node,
+                                    (covering_node != NULL ?
+                                     covering_node->getData() : NULL),
+                                    getClass(), options);
+
+            LOG_DEBUG(logger, DBG_TRACE_BASIC,
+                      DATASRC_MEM_FINDNSEC3_MATCH).arg(name).arg(labels).
+                arg(*closest);
+
+            return (FindNSEC3Result(true, labels, closest, next));
+        } else {
+            const NameComparisonResult& last_cmp =
+                chain.getLastComparisonResult();
+            const ZoneNode* last_node = chain.getLastComparedNode();
+            assert(last_cmp.getOrder() != 0);
+
+            // find() finished in between one of these and last_node:
+            const ZoneNode* previous_node = last_node->predecessor();
+            const ZoneNode* next_node = last_node->successor();
+
+            // If the given hash is larger than the largest stored hash or
+            // the first label doesn't match the target, identify the "previous"
+            // hash value and remember it as the candidate next closer proof.
+            if (((last_cmp.getOrder() < 0) && (previous_node == NULL)) ||
+                ((last_cmp.getOrder() > 0) && (next_node == NULL))) {
+                covering_node = last_node->getLargestInSubTree();
+            } else {
+                // Otherwise, H(found_entry-1) < given_hash < H(found_entry).
+                // The covering proof is the first one (and it's valid
+                // because found is neither begin nor end)
+                covering_node = previous_node;
+            }
+
+            if (!recursive) {   // in non recursive mode, we are done.
+                ConstRRsetPtr closest =
+                    createTreeNodeRRset(covering_node,
+                                        (covering_node != NULL ?
+                                         covering_node->getData() :
+                                         NULL),
+                                        getClass(), options);
+
+                if (closest) {
+                    LOG_DEBUG(logger, DBG_TRACE_BASIC,
+                              DATASRC_MEM_FINDNSEC3_COVER).
+                        arg(name).arg(*closest);
+                }
+
+                return (FindNSEC3Result(false, labels,
+                                        closest, ConstRRsetPtr()));
+            }
+        }
+    }
+
+    isc_throw(DataSourceError, "recursive findNSEC3 mode didn't stop, likely "
+              "a broken NSEC3 zone: " << getOrigin() << "/"
+              << getClass());
+}
+
+Name
+InMemoryZoneFinder::getOrigin() const {
+    size_t data_len;
+    const uint8_t* data;
+
+    // Normally the label sequence of the origin node should be absolute,
+    // in which case we can simply generate the origin name from the labels.
+    const LabelSequence node_labels = zone_data_.getOriginNode()->getLabels();
+    if (node_labels.isAbsolute()) {
+        data = node_labels.getData(&data_len);
+    } else {
+        // In future we may allow adding out-of-zone names in the zone tree.
+        // For example, to hold out-of-zone NS names so we can establish a
+        // shortcut link to them as an optimization.  If and when that happens
+        // the origin node may not have an absolute label (consider the zone
+        // is example.org and we add ns.noexample.org).  In that case
+        // we first need to construct the absolute label sequence and then
+        // construct the name.
+        uint8_t labels_buf[LabelSequence::MAX_SERIALIZED_LENGTH];
+        const LabelSequence name_labels =
+            zone_data_.getOriginNode()->getAbsoluteLabels(labels_buf);
+        data = name_labels.getData(&data_len);
+    }
+    util::InputBuffer buffer(data, data_len);
+    return (Name(buffer));
 }
 
 } // namespace memory

src/lib/datasrc/memory/zone_finder.h

@@ -48,6 +48,12 @@ public:
     const ZoneNode* const found_node;
 };
 
+std::string
+InMemoryZoneFinderNSEC3Calculate(const isc::dns::Name& name,
+                                 const uint16_t iterations,
+                                 const uint8_t* salt,
+                                 size_t salt_len);
+
 /// A derived zone finder class intended to be used with the memory data
 /// source, using ZoneData for its contents.
 class InMemoryZoneFinder : boost::noncopyable, public ZoneFinder {
@@ -66,7 +72,8 @@ public:
     InMemoryZoneFinder(const ZoneData& zone_data,
                        const isc::dns::RRClass& rrclass) :
         zone_data_(zone_data),
-        rrclass_(rrclass)
+        rrclass_(rrclass),
+        nsec3_calculate_(InMemoryZoneFinderNSEC3Calculate)
     {}
 
     /// \brief Find an RRset in the datasource
@@ -92,16 +99,13 @@ public:
     findNSEC3(const isc::dns::Name& name, bool recursive);
 
     /// \brief Returns the origin of the zone.
-    virtual isc::dns::Name getOrigin() const {
-        return zone_data_.getOriginNode()->getName();
-    }
+    virtual isc::dns::Name getOrigin() const;
 
     /// \brief Returns the RR class of the zone.
     virtual isc::dns::RRClass getClass() const {
-        return rrclass_;
+        return (rrclass_);
     }
 
-
 private:
     /// \brief In-memory version of finder context.
     ///
@@ -118,7 +122,14 @@ private:
         FIND_DEFAULT);
 
     const ZoneData& zone_data_;
-    const isc::dns::RRClass& rrclass_;
+    const isc::dns::RRClass rrclass_;
+
+protected:
+    typedef std::string (NSEC3CalculateFn) (const isc::dns::Name& name,
+                                            const uint16_t iterations,
+                                            const uint8_t* salt,
+                                            size_t salt_len);
+    NSEC3CalculateFn* nsec3_calculate_;
 };
 
 } // namespace memory

src/lib/datasrc/tests/Makefile.am

@@ -1,4 +1,4 @@
-SUBDIRS = testdata
+SUBDIRS = . memory testdata
 
 AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
 AM_CPPFLAGS += -I$(top_builddir)/src/lib/dns -I$(top_srcdir)/src/lib/dns

src/lib/datasrc/tests/client_list_unittest.cc

@@ -12,11 +12,14 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <util/memory_segment_local.h>
+
 #include <datasrc/client_list.h>
 #include <datasrc/client.h>
 #include <datasrc/iterator.h>
 #include <datasrc/data_source.h>
-#include <datasrc/memory_datasrc.h>
+#include <datasrc/memory/memory_client.h>
+#include <datasrc/memory/zone_finder.h>
 
 #include <dns/rrclass.h>
 #include <dns/rrttl.h>
@@ -28,6 +31,8 @@
 #include <fstream>
 
 using namespace isc::datasrc;
+using isc::datasrc::memory::InMemoryClient;
+using isc::datasrc::memory::InMemoryZoneFinder;
 using namespace isc::data;
 using namespace isc::dns;
 using namespace boost;
@@ -67,35 +72,47 @@ public:
     };
     class Iterator : public ZoneIterator {
     public:
-        Iterator(const Name& origin) :
+        Iterator(const Name& origin, bool include_ns) :
             origin_(origin),
-            finished_(false),
-            soa_(new RRset(origin_, RRClass::IN(), RRType::SOA(), RRTTL(3600)))
+            soa_(new RRset(origin_, RRClass::IN(), RRType::SOA(),
+                           RRTTL(3600)))
         {
             // The RData here is bogus, but it is not used to anything. There
             // just needs to be some.
             soa_->addRdata(rdata::generic::SOA(Name::ROOT_NAME(),
                                                Name::ROOT_NAME(),
                                                0, 0, 0, 0, 0));
+            rrsets_.push_back(soa_);
+
+            if (include_ns) {
+                ns_.reset(new RRset(origin_, RRClass::IN(), RRType::NS(),
+                                    RRTTL(3600)));
+                ns_->addRdata(rdata::generic::NS(Name::ROOT_NAME()));
+                rrsets_.push_back(ns_);
+            }
+            rrsets_.push_back(ConstRRsetPtr());
+
+            it_ = rrsets_.begin();
         }
         virtual isc::dns::ConstRRsetPtr getNextRRset() {
-            if (finished_) {
-                return (ConstRRsetPtr());
-            } else {
-                finished_ = true;
-                return (soa_);
-            }
+            ConstRRsetPtr result = *it_;
+            ++it_;
+            return (result);
         }
         virtual isc::dns::ConstRRsetPtr getSOA() const {
             return (soa_);
         }
     private:
         const Name origin_;
-        bool finished_;
-        const isc::dns::RRsetPtr soa_;
+        const RRsetPtr soa_;
+        RRsetPtr ns_;
+        std::vector<ConstRRsetPtr> rrsets_;
+        std::vector<ConstRRsetPtr>::const_iterator it_;
     };
     // Constructor from a list of zones.
-    MockDataSourceClient(const char* zone_names[]) {
+    MockDataSourceClient(const char* zone_names[]) :
+        have_ns_(true), use_baditerator_(true)
+    {
         for (const char** zone(zone_names); *zone; ++zone) {
             zones.insert(Name(*zone));
         }
@@ -105,7 +122,8 @@ public:
     MockDataSourceClient(const string& type,
                          const ConstElementPtr& configuration) :
         type_(type),
-        configuration_(configuration)
+        configuration_(configuration),
+        have_ns_(true), use_baditerator_(true)
     {
         EXPECT_NE("MasterFiles", type) << "MasterFiles is a special case "
             "and it never should be created as a data source client";
@@ -146,23 +164,27 @@ public:
         isc_throw(isc::NotImplemented, "Not implemented");
     }
     virtual ZoneIteratorPtr getIterator(const Name& name, bool) const {
-        if (name == Name("noiter.org")) {
+        if (use_baditerator_ && name == Name("noiter.org")) {
             isc_throw(isc::NotImplemented, "Asked not to be implemented");
-        } else if (name == Name("null.org")) {
+        } else if (use_baditerator_ && name == Name("null.org")) {
             return (ZoneIteratorPtr());
         } else {
             FindResult result(findZone(name));
             if (result.code == isc::datasrc::result::SUCCESS) {
-                return (ZoneIteratorPtr(new Iterator(name)));
+                return (ZoneIteratorPtr(new Iterator(name, have_ns_)));
             } else {
                 isc_throw(DataSourceError, "No such zone");
             }
         }
     }
+    void disableNS() { have_ns_ = false; }
+    void disableBadIterator() { use_baditerator_ = false; }
     const string type_;
     const ConstElementPtr configuration_;
 private:
     set<Name> zones;
+    bool have_ns_; // control the iterator behavior wrt whether to include NS
+    bool use_baditerator_; // whether to use bogus zone iterators for tests
 };
 
 
@@ -220,8 +242,9 @@ const size_t ds_count = (sizeof(ds_zones) / sizeof(*ds_zones));
 class ListTest : public ::testing::Test {
 public:
     ListTest() :
+        rrclass_(RRClass::IN()),
         // The empty list corresponds to a list with no elements inside
-        list_(new TestedList(RRClass::IN())),
+        list_(new TestedList(rrclass_)),
         config_elem_(Element::fromJSON("["
             "{"
             "   \"type\": \"test_type\","
@@ -238,28 +261,35 @@ public:
             shared_ptr<MockDataSourceClient>
                 ds(new MockDataSourceClient(ds_zones[i]));
             ds_.push_back(ds);
-            ds_info_.push_back(ConfigurableClientList::DataSourceInfo(ds.get(),
-                DataSourceClientContainerPtr(), false));
+            ds_info_.push_back(ConfigurableClientList::DataSourceInfo(
+                                   ds.get(), DataSourceClientContainerPtr(),
+                                   false, rrclass_, mem_sgmt_));
         }
     }
-    void prepareCache(size_t index, const Name& zone, bool prefill = false) {
-        const shared_ptr<InMemoryClient> cache(new InMemoryClient());
-        const shared_ptr<InMemoryZoneFinder>
-            finder(new InMemoryZoneFinder(RRClass::IN(), zone));
-        if (prefill) {
-            RRsetPtr soa(new RRset(zone, RRClass::IN(), RRType::SOA(),
-                                   RRTTL(3600)));
-            // The RData here is bogus, but it is not used to anything. There
-            // just needs to be some.
-            soa->addRdata(rdata::generic::SOA(Name::ROOT_NAME(),
-                                              Name::ROOT_NAME(),
-                                              0, 0, 0, 0, 0));
-            finder->add(soa);
-        }
-        // If we don't do prefill, we leave the zone empty. This way,
-        // we can check when it was reloaded.
-        cache->addZone(finder);
-        list_->getDataSources()[index].cache_ = cache;
+
+    // Install a "fake" cached zone using a temporary underlying data source
+    // client.
+    void prepareCache(size_t index, const Name& zone) {
+        // Prepare the temporary data source client
+        const char* zones[2];
+        const std::string zonename_txt = zone.toText();
+        zones[0] = zonename_txt.c_str();
+        zones[1] = NULL;
+        MockDataSourceClient mock_client(zones);
+        // Disable some default features of the mock to distinguish the
+        // temporary case from normal case.
+        mock_client.disableNS();
+        mock_client.disableBadIterator();
+
+        // Create cache from the temporary data source, and push it to the
+        // client list.
+        const shared_ptr<InMemoryClient> cache(new InMemoryClient(mem_sgmt_,
+                                                                  rrclass_));
+        cache->load(zone, *mock_client.getIterator(zone, false));
+
+        ConfigurableClientList::DataSourceInfo& dsrc_info =
+                list_->getDataSources()[index];
+        dsrc_info.cache_ = cache;
     }
     // Check the positive result is as we expect it.
     void positiveResult(const ClientList::FindResult& result,
@@ -331,6 +361,8 @@ public:
         EXPECT_EQ(cache, list_->getDataSources()[index].cache_ !=
                   shared_ptr<InMemoryClient>());
     }
+    const RRClass rrclass_;
+    isc::util::MemorySegmentLocal mem_sgmt_;
     shared_ptr<TestedList> list_;
     const ClientList::FindResult negative_result_;
     vector<shared_ptr<MockDataSourceClient> > ds_;
@@ -815,39 +847,38 @@ TEST_F(ListTest, BadMasterFile) {
 // Test we can reload a zone
 TEST_F(ListTest, reloadSuccess) {
     list_->configure(config_elem_zones_, true);
-    Name name("example.org");
+    const Name name("example.org");
     prepareCache(0, name);
-    // Not there yet. It would be NXDOMAIN, but it is in apex and
-    // it returns NXRRSET instead.
+    // The cache currently contains a tweaked version of zone, which doesn't
+    // have apex NS.  So the lookup should result in NXRRSET.
     EXPECT_EQ(ZoneFinder::NXRRSET,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
-    // Now reload. It should be there now.
+              list_->find(name).finder_->find(name, RRType::NS())->code);
+    // Now reload the full zone. It should be there now.
     EXPECT_EQ(ConfigurableClientList::ZONE_RELOADED, list_->reload(name));
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              list_->find(name).finder_->find(name, RRType::NS())->code);
 }
 
 // The cache is not enabled. The load should be rejected.
 TEST_F(ListTest, reloadNotEnabled) {
     list_->configure(config_elem_zones_, false);
-    Name name("example.org");
+    const Name name("example.org");
     // We put the cache in even when not enabled. This won't confuse the thing.
     prepareCache(0, name);
-    // Not there yet. It would be NXDOMAIN, but it is in apex and
-    // it returns NXRRSET instead.
+    // See the reloadSuccess test.  This should result in NXRRSET.
     EXPECT_EQ(ZoneFinder::NXRRSET,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              list_->find(name).finder_->find(name, RRType::NS())->code);
     // Now reload. It should reject it.
     EXPECT_EQ(ConfigurableClientList::CACHE_DISABLED, list_->reload(name));
     // Nothing changed here
     EXPECT_EQ(ZoneFinder::NXRRSET,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              list_->find(name).finder_->find(name, RRType::NS())->code);
 }
 
 // Test several cases when the zone does not exist
 TEST_F(ListTest, reloadNoSuchZone) {
     list_->configure(config_elem_zones_, true);
-    Name name("example.org");
+    const Name name("example.org");
     // We put the cache in even when not enabled. This won't confuse the
     // reload method, as that one looks at the real state of things, not
     // at the configuration.
@@ -867,27 +898,27 @@ TEST_F(ListTest, reloadNoSuchZone) {
               list_->find(Name("example.cz")).dsrc_client_);
     EXPECT_EQ(static_cast<isc::datasrc::DataSourceClient*>(NULL),
               list_->find(Name("sub.example.com"), true).dsrc_client_);
-    // Not reloaded
+    // Not reloaded, so NS shouldn't be visible yet.
     EXPECT_EQ(ZoneFinder::NXRRSET,
               list_->find(Name("example.com")).finder_->
-              find(Name("example.com"), RRType::SOA())->code);
+              find(Name("example.com"), RRType::NS())->code);
 }
 
 // Check we gracefuly throw an exception when a zone disappeared in
 // the underlying data source when we want to reload it
 TEST_F(ListTest, reloadZoneGone) {
     list_->configure(config_elem_, true);
-    Name name("example.org");
+    const Name name("example.org");
     // We put in a cache for non-existant zone. This emulates being loaded
     // and then the zone disappearing. We prefill the cache, so we can check
     // it.
-    prepareCache(0, name, true);
-    // The zone contains something
+    prepareCache(0, name);
+    // The (cached) zone contains zone's SOA
     EXPECT_EQ(ZoneFinder::SUCCESS,
               list_->find(name).finder_->find(name, RRType::SOA())->code);
     // The zone is not there, so abort the reload.
     EXPECT_THROW(list_->reload(name), DataSourceError);
-    // The zone is not hurt.
+    // The (cached) zone is not hurt.
     EXPECT_EQ(ZoneFinder::SUCCESS,
               list_->find(name).finder_->find(name, RRType::SOA())->code);
 }
@@ -895,8 +926,8 @@ TEST_F(ListTest, reloadZoneGone) {
 // The underlying data source throws. Check we don't modify the state.
 TEST_F(ListTest, reloadZoneThrow) {
     list_->configure(config_elem_zones_, true);
-    Name name("noiter.org");
-    prepareCache(0, name, true);
+    const Name name("noiter.org");
+    prepareCache(0, name);
     // The zone contains stuff now
     EXPECT_EQ(ZoneFinder::SUCCESS,
               list_->find(name).finder_->find(name, RRType::SOA())->code);
@@ -909,8 +940,8 @@ TEST_F(ListTest, reloadZoneThrow) {
 
 TEST_F(ListTest, reloadNullIterator) {
     list_->configure(config_elem_zones_, true);
-    Name name("null.org");
-    prepareCache(0, name, true);
+    const Name name("null.org");
+    prepareCache(0, name);
     // The zone contains stuff now
     EXPECT_EQ(ZoneFinder::SUCCESS,
               list_->find(name).finder_->find(name, RRType::SOA())->code);

src/lib/datasrc/memory/tests/Makefile.am

@@ -36,8 +36,7 @@ run_unittests_SOURCES += memory_client_unittest.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
 
-run_unittests_LDADD = $(builddir)/../libdatasrc_memory.la
-run_unittests_LDADD += $(top_builddir)/src/lib/datasrc/libb10-datasrc.la
+run_unittests_LDADD = $(top_builddir)/src/lib/datasrc/libb10-datasrc.la
 run_unittests_LDADD += $(top_builddir)/src/lib/dns/libb10-dns++.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/libb10-util.la

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

@@ -257,7 +257,7 @@ TEST_F(DomainTreeTest, subTreeRoot) {
     // "g.h" is not a subtree root
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
               dtree_expose_empty_node.find(Name("g.h"), &dtnode));
-    EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+    EXPECT_FALSE(dtnode->isSubTreeRoot());
 
     // fission the node "g.h"
     EXPECT_EQ(TestDomainTree::ALREADYEXISTS,
@@ -266,12 +266,12 @@ TEST_F(DomainTreeTest, subTreeRoot) {
 
     // the node "h" (h.down_ -> "g") should not be a subtree root. "g"
     // should be a subtree root.
-    EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+    EXPECT_FALSE(dtnode->isSubTreeRoot());
 
     // "g.h" should be a subtree root now.
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
               dtree_expose_empty_node.find(Name("g.h"), &dtnode));
-    EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+    EXPECT_TRUE(dtnode->isSubTreeRoot());
 }
 
 TEST_F(DomainTreeTest, additionalNodeFission) {
@@ -287,7 +287,7 @@ TEST_F(DomainTreeTest, additionalNodeFission) {
     // "t.0" is not a subtree root
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
               dtree_expose_empty_node.find(Name("t.0"), &dtnode));
-    EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+    EXPECT_FALSE(dtnode->isSubTreeRoot());
 
     // fission the node "t.0"
     EXPECT_EQ(TestDomainTree::ALREADYEXISTS,
@@ -296,12 +296,12 @@ TEST_F(DomainTreeTest, additionalNodeFission) {
 
     // the node "0" ("0".down_ -> "t") should not be a subtree root. "t"
     // should be a subtree root.
-    EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+    EXPECT_FALSE(dtnode->isSubTreeRoot());
 
     // "t.0" should be a subtree root now.
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
               dtree_expose_empty_node.find(Name("t.0"), &dtnode));
-    EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+    EXPECT_TRUE(dtnode->isSubTreeRoot());
 }
 
 TEST_F(DomainTreeTest, findName) {
@@ -687,6 +687,16 @@ const char* const upper_node_names[] = {
     "w.y.d.e.f", "w.y.d.e.f", "d.e.f", "z.d.e.f",
     ".", "g.h", "g.h"};
 
+const char* const subtree_root_node_names[] = {
+    "b", "b", "b", "b", "w.y.d.e.f", "w.y.d.e.f", "p.w.y.d.e.f",
+    "p.w.y.d.e.f", "p.w.y.d.e.f", "w.y.d.e.f", "j.z.d.e.f",
+    "b", "i.g.h", "i.g.h"};
+
+const char* const largest_node_names[] = {
+    "g.h", "g.h", "g.h", "g.h", "z.d.e.f", "z.d.e.f", "q.w.y.d.e.f",
+    "q.w.y.d.e.f", "q.w.y.d.e.f", "z.d.e.f", "j.z.d.e.f",
+    "g.h", "k.g.h", "k.g.h"};
+
 TEST_F(DomainTreeTest, getUpperNode) {
     TestDomainTreeNodeChain node_path;
     const TestDomainTreeNode* node = NULL;
@@ -716,6 +726,64 @@ TEST_F(DomainTreeTest, getUpperNode) {
     EXPECT_EQ(static_cast<void*>(NULL), node);
 }
 
+TEST_F(DomainTreeTest, getSubTreeRoot) {
+    TestDomainTreeNodeChain node_path;
+    const TestDomainTreeNode* node = NULL;
+    EXPECT_EQ(TestDomainTree::EXACTMATCH,
+              dtree_expose_empty_node.find(Name(names[0]),
+                                            &node,
+                                            node_path));
+    for (int i = 0; i < name_count; ++i) {
+        EXPECT_NE(static_cast<void*>(NULL), node);
+
+        const TestDomainTreeNode* sr_node = node->getSubTreeRoot();
+        if (subtree_root_node_names[i] != NULL) {
+            const TestDomainTreeNode* sr_node2 = NULL;
+            EXPECT_EQ(TestDomainTree::EXACTMATCH,
+                dtree_expose_empty_node.find(Name(subtree_root_node_names[i]),
+                                             &sr_node2));
+            EXPECT_NE(static_cast<void*>(NULL), sr_node2);
+            EXPECT_EQ(sr_node, sr_node2);
+        } else {
+            EXPECT_EQ(static_cast<void*>(NULL), sr_node);
+        }
+
+        node = dtree_expose_empty_node.nextNode(node_path);
+    }
+
+    // We should have reached the end of the tree.
+    EXPECT_EQ(static_cast<void*>(NULL), node);
+}
+
+TEST_F(DomainTreeTest, getLargestInSubTree) {
+    TestDomainTreeNodeChain node_path;
+    const TestDomainTreeNode* node = NULL;
+    EXPECT_EQ(TestDomainTree::EXACTMATCH,
+              dtree_expose_empty_node.find(Name(names[0]),
+                                            &node,
+                                            node_path));
+    for (int i = 0; i < name_count; ++i) {
+        EXPECT_NE(static_cast<void*>(NULL), node);
+
+        const TestDomainTreeNode* largest_node = node->getLargestInSubTree();
+        if (largest_node_names[i] != NULL) {
+            const TestDomainTreeNode* largest_node2 = NULL;
+            EXPECT_EQ(TestDomainTree::EXACTMATCH,
+                dtree_expose_empty_node.find(Name(largest_node_names[i]),
+                                             &largest_node2));
+            EXPECT_NE(static_cast<void*>(NULL), largest_node2);
+            EXPECT_EQ(largest_node, largest_node2);
+        } else {
+            EXPECT_EQ(static_cast<void*>(NULL), largest_node);
+        }
+
+        node = dtree_expose_empty_node.nextNode(node_path);
+    }
+
+    // We should have reached the end of the tree.
+    EXPECT_EQ(static_cast<void*>(NULL), node);
+}
+
 TEST_F(DomainTreeTest, nextNode) {
     TestDomainTreeNodeChain node_path;
     const TestDomainTreeNode* node = NULL;

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

@@ -702,12 +702,6 @@ TEST_F(MemoryClientTest, add) {
     EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
 }
 
-TEST_F(MemoryClientTest, findZoneThrowsNotImplemented) {
-    // This method is not implemented.
-    EXPECT_THROW(client_->findZone(Name(".")),
-                 isc::NotImplemented);
-}
-
 TEST_F(MemoryClientTest, findZone2) {
     client_->load(Name("example.org"),
                   TEST_DATA_DIR "/example.org-rrsigs.zone");

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

@@ -52,7 +52,7 @@ namespace isc {
 namespace datasrc{
 namespace memory {
 
-#include "../rdata_serialization_priv.cc"
+#include <datasrc/memory/rdata_serialization_priv.cc>
 
 }
 }

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

@@ -25,6 +25,7 @@
 
 #include <gtest/gtest.h>
 
+#include <boost/bind.hpp>
 #include <boost/shared_ptr.hpp>
 
 #include <string>
@@ -137,11 +138,22 @@ protected:
     RdataSet* wildcard_rdataset_; // for wildcard (type doesn't matter much)
 };
 
+void
+compareRRSIGData(RdataIteratorPtr rit, const void* data, size_t data_len) {
+    ASSERT_FALSE(rit->isLast());
+
+    OutputBuffer buffer(0);
+    rit->getCurrent().toWire(buffer);
+    matchWireData(data, data_len, buffer.getData(), buffer.getLength());
+    rit->next();
+}
+
 // Check some trivial fields of a constructed TreeNodeRRset (passed as
 // AbstractRRset as we'd normally use it in polymorphic way).
 // Other complicated fields are checked through rendering tests.
 void
-checkBasicFields(const AbstractRRset& actual_rrset, const Name& expected_name,
+checkBasicFields(const AbstractRRset& actual_rrset, const RdataSet* rdataset,
+                 const Name& expected_name,
                  const RRClass& expected_class, const RRType& expected_type,
                  const uint32_t expected_ttl,
                  size_t expected_rdatacount, size_t expected_sigcount)
@@ -152,6 +164,28 @@ checkBasicFields(const AbstractRRset& actual_rrset, const Name& expected_name,
     EXPECT_EQ(RRTTL(expected_ttl), actual_rrset.getTTL());
     EXPECT_EQ(expected_rdatacount, actual_rrset.getRdataCount());
     EXPECT_EQ(expected_sigcount, actual_rrset.getRRsigDataCount());
+
+    // getRRsig() should return non NULL iff the RRset is expected to be signed
+    if (expected_sigcount == 0) {
+        EXPECT_FALSE(actual_rrset.getRRsig());
+    } else {
+        ConstRRsetPtr actual_sigrrset = actual_rrset.getRRsig();
+        ASSERT_TRUE(actual_sigrrset);
+        EXPECT_EQ(expected_name, actual_sigrrset->getName());
+        EXPECT_EQ(expected_class, actual_sigrrset->getClass());
+        EXPECT_EQ(RRType::RRSIG(), actual_sigrrset->getType());
+        EXPECT_EQ(RRTTL(expected_ttl), actual_sigrrset->getTTL());
+        EXPECT_EQ(expected_sigcount, actual_sigrrset->getRdataCount());
+
+        // Compare each RRSIG RDATA
+        RdataIteratorPtr rit = actual_sigrrset->getRdataIterator();
+        RdataReader reader(expected_class, expected_type,
+                           rdataset->getDataBuf(), expected_rdatacount,
+                           expected_sigcount, &RdataReader::emptyNameAction,
+                           boost::bind(compareRRSIGData, rit, _1, _2));
+        while (reader.nextSig() != RdataReader::RRSET_BOUNDARY) {}
+        EXPECT_TRUE(rit->isLast()); // should check all RDATAs
+    }
 }
 
 // The following two are trivial wrapper to create a shared pointer
@@ -178,30 +212,37 @@ createRRset(const Name& realname, const RRClass& rrclass, const ZoneNode* node,
 TEST_F(TreeNodeRRsetTest, create) {
     // Constructed with RRSIG, and it should be visible.
     checkBasicFields(*createRRset(rrclass_, www_node_, a_rdataset_, true),
-                     www_name_, rrclass_, RRType::A(), 3600, 2, 1);
+                     a_rdataset_, www_name_, rrclass_, RRType::A(), 3600, 2,
+                     1);
     // Constructed with RRSIG, and it should be invisible.
     checkBasicFields(*createRRset(rrclass_, www_node_, a_rdataset_, false),
-                     www_name_, rrclass_, RRType::A(), 3600, 2, 0);
+                     a_rdataset_, www_name_, rrclass_, RRType::A(), 3600, 2,
+                     0);
     // Constructed without RRSIG, and it would be visible (but of course won't)
     checkBasicFields(*createRRset(rrclass_, origin_node_, ns_rdataset_, true),
-                     origin_name_, rrclass_, RRType::NS(), 3600, 1, 0);
+                     ns_rdataset_, origin_name_, rrclass_, RRType::NS(), 3600,
+                     1, 0);
     // Constructed without RRSIG, and it should be visible
     checkBasicFields(*createRRset(rrclass_, origin_node_, ns_rdataset_, false),
-                     origin_name_, rrclass_, RRType::NS(), 3600, 1, 0);
+                     ns_rdataset_, origin_name_, rrclass_, RRType::NS(), 3600,
+                     1, 0);
     // RRSIG-only case (note the RRset's type is covered type)
     checkBasicFields(*createRRset(rrclass_, www_node_, rrsig_only_rdataset_,
                                   true),
-                     www_name_, rrclass_, RRType::TXT(), 3600, 0, 1);
+                     rrsig_only_rdataset_, www_name_, rrclass_, RRType::TXT(),
+                     3600, 0, 1);
     // RRSIG-only case (note the RRset's type is covered type), but it's
     // invisible
     checkBasicFields(*createRRset(rrclass_, www_node_, rrsig_only_rdataset_,
                                   false),
-                     www_name_, rrclass_, RRType::TXT(), 3600, 0, 0);
+                     rrsig_only_rdataset_, www_name_, rrclass_, RRType::TXT(),
+                     3600, 0, 0);
     // Wildcard substitution
     checkBasicFields(*createRRset(match_name_, rrclass_,
                                   wildcard_node_, wildcard_rdataset_,
                                   true),
-                     match_name_, rrclass_, RRType::A(), 3600, 2, 1);
+                     wildcard_rdataset_, match_name_, rrclass_, RRType::A(),
+                     3600, 2, 1);
 }
 
 // The following two templated functions are helper to encapsulate the
@@ -578,7 +619,6 @@ TEST_F(TreeNodeRRsetTest, unexpectedMethods) {
     EXPECT_THROW(rrset.setName(Name("example")), isc::Unexpected);
     EXPECT_THROW(rrset.addRdata(createRdata(RRType::A(), rrclass_, "0.0.0.0")),
                  isc::Unexpected);
-    EXPECT_THROW(rrset.getRRsig(), isc::Unexpected);
     RdataPtr sig_rdata = createRdata(
         RRType::RRSIG(), rrclass_,
         "A 5 2 3600 20120814220826 20120715220826 5300 example.com. FAKE");

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

@@ -61,59 +61,57 @@ const char* const xyw_hash = "2vptu5timamqttgl4luu9kg21e0aor3s";
 // For zzz.example.org.
 const char* const zzz_hash = "R53BQ7CC2UVMUBFU5OCMM6PERS9TK9EN";
 
-// A simple faked NSEC3 hash calculator with a dedicated creator for it.
-//
-// This is used in some NSEC3-related tests below.
-// Also see NOTE at inclusion of "../../tests/faked_nsec3.h"
-class TestNSEC3HashCreator : public NSEC3HashCreator {
-    class TestNSEC3Hash : public NSEC3Hash {
-    private:
-        typedef map<Name, string> NSEC3HashMap;
-        typedef NSEC3HashMap::value_type NSEC3HashPair;
-        NSEC3HashMap map_;
-    public:
-        TestNSEC3Hash() {
-            // Build pre-defined hash
-            map_[Name("example.org")] = apex_hash;
-            map_[Name("www.example.org")] = "2S9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
-            map_[Name("xxx.example.org")] = "Q09MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
-            map_[Name("yyy.example.org")] = "0A9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
-            map_[Name("x.y.w.example.org")] =
-                "2VPTU5TIMAMQTTGL4LUU9KG21E0AOR3S";
-            map_[Name("y.w.example.org")] = "K8UDEMVP1J2F7EG6JEBPS17VP3N8I58H";
-            map_[Name("w.example.org")] = w_hash;
-            map_[Name("zzz.example.org")] = zzz_hash;
-            map_[Name("smallest.example.org")] =
-                "00000000000000000000000000000000";
-            map_[Name("largest.example.org")] =
-                "UUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUU";
-        }
-        virtual string calculate(const Name& name) const {
-            const NSEC3HashMap::const_iterator found = map_.find(name);
-            if (found != map_.end()) {
-                return (found->second);
-            }
-            isc_throw(isc::Unexpected, "unexpected name for NSEC3 test: "
-                      << name);
-        }
-        virtual bool match(const generic::NSEC3PARAM&) const {
-            return (true);
-        }
-        virtual bool match(const generic::NSEC3&) const {
-            return (true);
-        }
-    };
+typedef map<Name, string> NSEC3HashMap;
+typedef NSEC3HashMap::value_type NSEC3HashPair;
+NSEC3HashMap nsec3_hash_map;
+
+// A faked NSEC3 hash calculator for convenience. Tests that need to use
+// the faked hashed values should call setFakeNSEC3Calculate() on the
+// MyZoneFinder object at the beginning of the test (at least before
+// adding any NSEC3/NSEC3PARAM RR).
+std::string
+fakeNSEC3Calculate(const Name& name,
+                   const uint16_t,
+                   const uint8_t*,
+                   size_t) {
+    const NSEC3HashMap::const_iterator found = nsec3_hash_map.find(name);
+    if (found != nsec3_hash_map.end()) {
+        return (found->second);
+    }
+
+    isc_throw(isc::Unexpected,
+              "unexpected name for NSEC3 test: " << name);
+}
 
+class MyZoneFinder : public memory::InMemoryZoneFinder {
+private:
 public:
-    virtual NSEC3Hash* create(const generic::NSEC3PARAM&) const {
-        return (new TestNSEC3Hash);
+    MyZoneFinder(const ZoneData& zone_data,
+                 const isc::dns::RRClass& rrclass) :
+         memory::InMemoryZoneFinder(zone_data, rrclass)
+    {
+        // Build pre-defined hash
+        nsec3_hash_map.clear();
+        nsec3_hash_map[Name("example.org")] = apex_hash;
+        nsec3_hash_map[Name("www.example.org")] = "2S9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
+        nsec3_hash_map[Name("xxx.example.org")] = "Q09MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
+        nsec3_hash_map[Name("yyy.example.org")] = "0A9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
+        nsec3_hash_map[Name("x.y.w.example.org")] =
+            "2VPTU5TIMAMQTTGL4LUU9KG21E0AOR3S";
+        nsec3_hash_map[Name("y.w.example.org")] = "K8UDEMVP1J2F7EG6JEBPS17VP3N8I58H";
+        nsec3_hash_map[Name("w.example.org")] = w_hash;
+        nsec3_hash_map[Name("zzz.example.org")] = zzz_hash;
+        nsec3_hash_map[Name("smallest.example.org")] =
+            "00000000000000000000000000000000";
+        nsec3_hash_map[Name("largest.example.org")] =
+            "UUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUU";
     }
-    virtual NSEC3Hash* create(const generic::NSEC3&) const {
-        return (new TestNSEC3Hash);
+
+    void setFakeNSEC3Calculate() {
+        nsec3_calculate_ = fakeNSEC3Calculate;
     }
 };
 
-
 /// \brief expensive rrset converter
 ///
 /// converts any specialized rrset (which may not have implemented some
@@ -182,6 +180,8 @@ public:
             {"example.org. 300 IN NS ns.example.org.", &rr_ns_},
             {"example.org. 300 IN A 192.0.2.1", &rr_a_},
             {"ns.example.org. 300 IN A 192.0.2.2", &rr_ns_a_},
+            // This one will place rr_ns_a_ at a zone cut, making it a glue:
+            {"ns.example.org. 300 IN NS 192.0.2.2", &rr_ns_ns_},
             {"ns.example.org. 300 IN AAAA 2001:db8::2", &rr_ns_aaaa_},
             {"cname.example.org. 300 IN CNAME canonical.example.org",
              &rr_cname_},
@@ -259,14 +259,46 @@ public:
     void addZoneDataNSEC3(const ConstRRsetPtr rrset) {
         assert(rrset->getType() == RRType::NSEC3());
 
-        const Rdata* rdata = &rrset->getRdataIterator()->getCurrent();
-        const generic::NSEC3* nsec3_rdata =
-            dynamic_cast<const generic::NSEC3*>(rdata);
-        NSEC3Data* nsec3_data = NSEC3Data::create(mem_sgmt_, *nsec3_rdata);
-        // in case we happen to be replacing, destroy old
-        NSEC3Data* old_data = zone_data_->setNSEC3Data(nsec3_data);
-        if (old_data != NULL) {
-            NSEC3Data::destroy(mem_sgmt_, old_data, rrset->getClass());
+        const generic::NSEC3& nsec3_rdata =
+             dynamic_cast<const generic::NSEC3&>(
+                  rrset->getRdataIterator()->getCurrent());
+
+        NSEC3Data* nsec3_data = zone_data_->getNSEC3Data();
+        if (nsec3_data == NULL) {
+             nsec3_data = NSEC3Data::create(mem_sgmt_, nsec3_rdata);
+             zone_data_->setNSEC3Data(nsec3_data);
+        } else {
+             size_t salt_len = nsec3_data->getSaltLen();
+             const uint8_t* salt_data = nsec3_data->getSaltData();
+             const vector<uint8_t>& salt_data_2 = nsec3_rdata.getSalt();
+
+             if ((nsec3_rdata.getHashalg() != nsec3_data->hashalg) ||
+                 (nsec3_rdata.getIterations() != nsec3_data->iterations) ||
+                 (salt_data_2.size() != salt_len) ||
+                 (std::memcmp(&salt_data_2[0], salt_data, salt_len) != 0)) {
+                  isc_throw(isc::Unexpected,
+                            "NSEC3 with inconsistent parameters: " <<
+                            rrset->toText());
+             }
+        }
+
+        // Make just the NSEC3 hash label uppercase, and insert the
+        // entire name into the NSEC3Data ZoneTree.
+        string fst_label = rrset->getName().split(0, 1).toText(true);
+        transform(fst_label.begin(), fst_label.end(), fst_label.begin(),
+                  ::toupper);
+        const string rest = rrset->getName().split(1).toText(true);
+
+        ZoneNode *node;
+        nsec3_data->insertName(mem_sgmt_, Name(fst_label + "." + rest), &node);
+
+        // We assume that rrsig has already been checked to match rrset
+        // by the caller.
+        RdataSet *set = RdataSet::create(mem_sgmt_, encoder_,
+                                         rrset, ConstRRsetPtr());
+        RdataSet *old_set = node->setData(set);
+        if (old_set != NULL) {
+             RdataSet::destroy(mem_sgmt_, class_, old_set);
         }
         zone_data_->setSigned(true);
     }
@@ -320,7 +352,7 @@ public:
     // The zone finder to torture by tests
     MemorySegmentTest mem_sgmt_;
     memory::ZoneData* zone_data_;
-    memory::InMemoryZoneFinder zone_finder_;
+    MyZoneFinder zone_finder_;
     isc::datasrc::memory::RdataEncoder encoder_;
 
     // Placeholder for storing RRsets to be checked with rrsetsCheck()
@@ -340,6 +372,7 @@ public:
         rr_ns_aaaa_,
         // A of example.org
         rr_a_;
+    RRsetPtr rr_ns_ns_;         // used to make rr_ns_a_ a glue.
     RRsetPtr rr_cname_;         // CNAME in example.org (RDATA will be added)
     RRsetPtr rr_cname_a_; // for mixed CNAME + A case
     RRsetPtr rr_dname_;         // DNAME in example.org (RDATA will be added)
@@ -369,12 +402,6 @@ public:
     RRsetPtr rr_ns_nsec_;
     RRsetPtr rr_wild_nsec_;
 
-    // A faked NSEC3 hash calculator for convenience.
-    // Tests that need to use the faked hashed values should call
-    // setNSEC3HashCreator() with a pointer to this variable at the beginning
-    // of the test (at least before adding any NSEC3/NSEC3PARAM RR).
-    TestNSEC3HashCreator nsec3_hash_creator_;
-
     /**
      * \brief Test one find query to the zone finder.
      *
@@ -435,9 +462,12 @@ public:
                         ConstRRsetPtr result_rrset(
                             convertRRset(find_result->rrset));
                         rrsetCheck(answer, result_rrset);
-                        if (answer_sig) {
+                        if (answer_sig &&
+                            (options & ZoneFinder::FIND_DNSSEC) != 0) {
                             ASSERT_TRUE(result_rrset->getRRsig());
                             rrsetCheck(answer_sig, result_rrset->getRRsig());
+                        } else {
+                            EXPECT_FALSE(result_rrset->getRRsig());
                         }
                     }
                 } else if (check_wild_answer) {
@@ -527,6 +557,13 @@ public:
  */
 TEST_F(InMemoryZoneFinderTest, constructor) {
     ASSERT_EQ(origin_, zone_finder_.getOrigin());
+
+    // Some unusual (abnormal case): if we add a super domain name of the
+    // zone somehow, the label of the origin node won't be absolute.
+    // getOrigin() should still be the correct one.
+    ZoneNode *node;
+    zone_data_->insertName(mem_sgmt_, Name("org"), &node);
+    ASSERT_EQ(origin_, zone_finder_.getOrigin());
 }
 
 TEST_F(InMemoryZoneFinderTest, findCNAME) {
@@ -684,6 +721,9 @@ TEST_F(InMemoryZoneFinderTest, glue) {
     EXPECT_NO_THROW(addZoneData(rr_grandchild_ns_));
     // glue under the deeper zone cut
     EXPECT_NO_THROW(addZoneData(rr_grandchild_glue_));
+    // glue 'at the' zone cut
+    EXPECT_NO_THROW(addZoneData(rr_ns_a_));
+    EXPECT_NO_THROW(addZoneData(rr_ns_ns_));
 
     // by default glue is hidden due to the zone cut
     findTest(rr_child_glue_->getName(), RRType::A(), ZoneFinder::DELEGATION,
@@ -716,6 +756,13 @@ TEST_F(InMemoryZoneFinderTest, glue) {
     findTest(Name("www.grand.child.example.org"), RRType::TXT(),
              ZoneFinder::DELEGATION, true, rr_child_ns_,
              ZoneFinder::RESULT_DEFAULT, NULL, ZoneFinder::FIND_GLUE_OK);
+
+    // Glue at a zone cut
+    findTest(Name("ns.example.org"), RRType::A(),
+             ZoneFinder::DELEGATION, true, rr_ns_ns_);
+    findTest(Name("ns.example.org"), RRType::A(), ZoneFinder::SUCCESS,
+             true, rr_ns_a_, ZoneFinder::RESULT_DEFAULT,
+             NULL, ZoneFinder::FIND_GLUE_OK);
 }
 
 /**
@@ -729,6 +776,9 @@ InMemoryZoneFinderTest::findCheck(ZoneFinder::FindResultFlags expected_flags,
 {
     // Fill some data inside
     // Now put all the data we have there. It should throw nothing
+    rr_a_->addRRsig(createRdata(RRType::RRSIG(), RRClass::IN(),
+                                "A 5 3 3600 20120814220826 20120715220826 "
+                                "1234 example.com. FAKE"));
     EXPECT_NO_THROW(addZoneData(rr_ns_));
     EXPECT_NO_THROW(addZoneData(rr_ns_a_));
     EXPECT_NO_THROW(addZoneData(rr_ns_aaaa_));
@@ -747,6 +797,12 @@ InMemoryZoneFinderTest::findCheck(ZoneFinder::FindResultFlags expected_flags,
     findTest(rr_ns_a_->getName(), RRType::A(), ZoneFinder::SUCCESS, true,
              rr_ns_a_);
 
+    // Similar test for a signed RRset.  We should see the RRSIG iff
+    // FIND_DNSSEC option is specified.
+    findTest(rr_a_->getName(), RRType::A(), ZoneFinder::SUCCESS, true, rr_a_);
+    findTest(rr_a_->getName(), RRType::A(), ZoneFinder::SUCCESS, true,
+             rr_a_, ZoneFinder::RESULT_DEFAULT, NULL, ZoneFinder::FIND_DNSSEC);
+
     // These domains don't exist. (and one is out of the zone).  In an
     // NSEC-signed zone with DNSSEC records requested, it should return the
     // covering NSEC for the query name (the actual NSEC in the test data may
@@ -1427,10 +1483,9 @@ TEST_F(InMemoryZoneFinderTest, cancelWildcardNSEC) {
 }
 
 
-// DISABLED: nsec3 will be re-added in #2118
-TEST_F(InMemoryZoneFinderTest, DISABLED_findNSEC3) {
+TEST_F(InMemoryZoneFinderTest, findNSEC3) {
     // Set up the faked hash calculator.
-    setNSEC3HashCreator(&nsec3_hash_creator_);
+    zone_finder_.setFakeNSEC3Calculate();
 
     // Add a few NSEC3 records:
     // apex (example.org.): hash=0P..
@@ -1453,10 +1508,9 @@ TEST_F(InMemoryZoneFinderTest, DISABLED_findNSEC3) {
     performNSEC3Test(zone_finder_);
 }
 
-// DISABLED: NSEC3 will be re-added in #2218
-TEST_F(InMemoryZoneFinderTest, DISABLED_findNSEC3ForBadZone) {
+TEST_F(InMemoryZoneFinderTest, findNSEC3ForBadZone) {
     // Set up the faked hash calculator.
-    setNSEC3HashCreator(&nsec3_hash_creator_);
+    zone_finder_.setFakeNSEC3Calculate();
 
     // If the zone has nothing about NSEC3 (neither NSEC3 or NSEC3PARAM),
     // findNSEC3() should be rejected.

src/lib/datasrc/tests/zone_finder_context_unittest.cc

@@ -14,12 +14,14 @@
 
 #include <exceptions/exceptions.h>
 
+#include <util/memory_segment_local.h>
+
 #include <dns/masterload.h>
 #include <dns/name.h>
 #include <dns/rrclass.h>
 
 #include <datasrc/zone.h>
-#include <datasrc/memory_datasrc.h>
+#include <datasrc/memory/memory_client.h>
 #include <datasrc/database.h>
 #include <datasrc/sqlite3_accessor.h>
 
@@ -39,8 +41,10 @@
 using namespace std;
 using boost::shared_ptr;
 
+using namespace isc::util;
 using namespace isc::dns;
 using namespace isc::datasrc;
+using isc::datasrc::memory::InMemoryClient;
 using namespace isc::testutils;
 
 namespace {
@@ -54,18 +58,16 @@ typedef shared_ptr<DataSourceClient> DataSourceClientPtr;
 // This is the type used as the test parameter.  Note that this is
 // intentionally a plain old type (i.e. a function pointer), not a class;
 // otherwise it could cause initialization fiasco at the instantiation time.
-typedef DataSourceClientPtr (*ClientCreator)(RRClass, const Name&);
+typedef DataSourceClientPtr (*ClientCreator)(MemorySegment&, RRClass,
+                                             const Name&);
 
 // Creator for the in-memory client to be tested
 DataSourceClientPtr
-createInMemoryClient(RRClass zclass, const Name& zname) {
-    shared_ptr<InMemoryClient> client(new InMemoryClient);
-
-    shared_ptr<InMemoryZoneFinder> finder(
-        new InMemoryZoneFinder(zclass, zname));
-    finder->load(TEST_ZONE_FILE);
-
-    client->addZone(finder);
+createInMemoryClient(MemorySegment& mem_sgmt, RRClass zclass,
+                     const Name& zname)
+{
+    shared_ptr<InMemoryClient> client(new InMemoryClient(mem_sgmt, zclass));
+    client->load(zname, TEST_ZONE_FILE);
 
     return (client);
 }
@@ -76,7 +78,7 @@ addRRset(ZoneUpdaterPtr updater, ConstRRsetPtr rrset) {
 }
 
 DataSourceClientPtr
-createSQLite3Client(RRClass zclass, const Name& zname) {
+createSQLite3Client(MemorySegment&, RRClass zclass, const Name& zname) {
     // We always begin with an empty template SQLite3 DB file and install
     // the zone data from the zone file to ensure both cases have the
     // same test data.
@@ -103,7 +105,7 @@ class ZoneFinderContextTest :
 {
 protected:
     ZoneFinderContextTest() : qclass_(RRClass::IN()), qzone_("example.org") {
-        client_ = (*GetParam())(qclass_, qzone_);
+        client_ = (*GetParam())(mem_sgmt_, qclass_, qzone_);
         REQUESTED_A.push_back(RRType::A());
         REQUESTED_AAAA.push_back(RRType::AAAA());
         REQUESTED_BOTH.push_back(RRType::A());
@@ -114,6 +116,7 @@ protected:
         ASSERT_TRUE(finder_);
     }
 
+    MemorySegmentLocal mem_sgmt_;
     const RRClass qclass_;
     const Name qzone_;
     DataSourceClientPtr client_;

src/lib/dns/labelsequence.cc

@@ -45,8 +45,7 @@ LabelSequence::LabelSequence(const void* buf) {
     // Check the integrity on the offsets and the name data
     const uint8_t* dp = data_;
     for (size_t cur_offset = 0; cur_offset < offsets_len; ++cur_offset) {
-        if (offsets_[cur_offset] > Name::MAX_LABELLEN ||
-            dp - data_ != offsets_[cur_offset]) {
+        if (dp - data_ != offsets_[cur_offset] || *dp > Name::MAX_LABELLEN) {
             isc_throw(BadValue,
                       "Broken offset or name data in serialized "
                       "LabelSequence data");

src/lib/dns/tests/labelsequence_unittest.cc

@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <util/buffer.h>
+
 #include <dns/labelsequence.h>
 #include <dns/name.h>
 #include <exceptions/exceptions.h>
@@ -772,6 +774,37 @@ TEST_F(LabelSequenceTest, serialize) {
         1, 0, 7, 'e', 'x', 'a', 'm', 'p', 'l', 'e' };
     expected.push_back(DataPair(sizeof(expected_data5), expected_data5));
 
+    // Labels containing a longest possible label
+    const Name name_longlabel(std::string(63, 'x')); // 63 'x's
+    LabelSequence ls_longlabel(name_longlabel);
+    actual_labelseqs.push_back(ls_longlabel);
+    vector<uint8_t> expected_data6;
+    expected_data6.push_back(2); // 2 labels
+    expected_data6.push_back(0); // 1st offset
+    expected_data6.push_back(64); // 2nd offset
+    expected_data6.push_back(63); // 1st label length
+    expected_data6.insert(expected_data6.end(), 63, 'x'); // 1st label: 63 'x's
+    expected_data6.push_back(0); // 2nd label: trailing 0
+    expected.push_back(DataPair(expected_data6.size(), &expected_data6[0]));
+
+    // Max number of labels and longest possible name
+    EXPECT_EQ(Name::MAX_WIRE, n_maxlabel.getLength());
+    LabelSequence ls_maxlabel(n_maxlabel);
+    actual_labelseqs.push_back(ls_maxlabel);
+    vector<uint8_t> expected_data7;
+    expected_data7.push_back(Name::MAX_LABELS); // number of labels
+    for (size_t i = 0; i < Name::MAX_LABELS; ++i) {
+        expected_data7.push_back(i * 2); // each label has length and 1 byte
+    }
+    // Copy wire data of the name
+    isc::util::OutputBuffer ob(0);
+    n_maxlabel.toWire(ob);
+    expected_data7.insert(expected_data7.end(),
+                          static_cast<const uint8_t*>(ob.getData()),
+                          static_cast<const uint8_t*>(ob.getData()) +
+                          ob.getLength());
+    expected.push_back(DataPair(expected_data7.size(), &expected_data7[0]));
+
     // For each data set, serialize the labels and compare the data to the
     // expected one.
     vector<DataPair>::const_iterator it = expected.begin();

src/lib/python/isc/datasrc/tests/clientlist_test.py

@@ -28,6 +28,16 @@ class ClientListTest(unittest.TestCase):
     contain the ConfigurableClientList only.
     """
 
+    def tearDown(self):
+        # The unit test module could keep internal objects alive longer than
+        # we expect.  But cached zone finder and cache client cannot stay
+        # longer than the originating client list.  So we explicitly clean
+        # them up here.  The ordering is important: clist must be destroyed
+        # last.
+        self.dsrc = None
+        self.finder = None
+        self.clist = None
+
     def test_constructors(self):
         """
         Test the constructor. It should accept an RRClass. Check it
@@ -50,16 +60,16 @@ class ClientListTest(unittest.TestCase):
         ones are acceptend and invalid rejected. We check the changes
         have effect.
         """
-        clist = isc.datasrc.ConfigurableClientList(isc.dns.RRClass.IN())
+        self.clist = isc.datasrc.ConfigurableClientList(isc.dns.RRClass.IN())
         # This should be NOP now
-        clist.configure("[]", True)
+        self.clist.configure("[]", True)
         # Check the zone is not there yet
-        dsrc, finder, exact = clist.find(isc.dns.Name("example.org"))
+        dsrc, finder, exact = self.clist.find(isc.dns.Name("example.org"))
         self.assertIsNone(dsrc)
         self.assertIsNone(finder)
         self.assertFalse(exact)
         # We can use this type, as it is not loaded dynamically.
-        clist.configure('''[{
+        self.clist.configure('''[{
             "type": "MasterFiles",
             "params": {
                 "example.org": "''' + TESTDATA_PATH + '''example.org.zone"
@@ -68,38 +78,39 @@ class ClientListTest(unittest.TestCase):
         }]''', True)
         # Check the zone is there now. Proper tests of find are in other
         # test methods.
-        dsrc, finder, exact = clist.find(isc.dns.Name("example.org"))
-        self.assertIsNotNone(dsrc)
-        self.assertTrue(isinstance(dsrc, isc.datasrc.DataSourceClient))
-        self.assertIsNotNone(finder)
-        self.assertTrue(isinstance(finder, isc.datasrc.ZoneFinder))
+        self.dsrc, self.finder, exact = \
+            self.clist.find(isc.dns.Name("example.org"))
+        self.assertIsNotNone(self.dsrc)
+        self.assertTrue(isinstance(self.dsrc, isc.datasrc.DataSourceClient))
+        self.assertIsNotNone(self.finder)
+        self.assertTrue(isinstance(self.finder, isc.datasrc.ZoneFinder))
         self.assertTrue(exact)
-        self.assertRaises(isc.datasrc.Error, clist.configure, '"bad type"',
-                          True)
-        self.assertRaises(isc.datasrc.Error, clist.configure, '''[{
+        self.assertRaises(isc.datasrc.Error, self.clist.configure,
+                          '"bad type"', True)
+        self.assertRaises(isc.datasrc.Error, self.clist.configure, '''[{
             "type": "bad type"
         }]''', True)
-        self.assertRaises(isc.datasrc.Error, clist.configure, '''[{
+        self.assertRaises(isc.datasrc.Error, self.clist.configure, '''[{
             bad JSON,
         }]''', True)
-        self.assertRaises(TypeError, clist.configure, [], True)
-        self.assertRaises(TypeError, clist.configure, "[]")
-        self.assertRaises(TypeError, clist.configure, "[]", "true")
+        self.assertRaises(TypeError, self.clist.configure, [], True)
+        self.assertRaises(TypeError, self.clist.configure, "[]")
+        self.assertRaises(TypeError, self.clist.configure, "[]", "true")
 
     def test_find(self):
         """
         Test the find accepts the right arguments, some of them can be omitted,
         etc.
         """
-        clist = isc.datasrc.ConfigurableClientList(isc.dns.RRClass.IN())
-        clist.configure('''[{
+        self.clist = isc.datasrc.ConfigurableClientList(isc.dns.RRClass.IN())
+        self.clist.configure('''[{
             "type": "MasterFiles",
             "params": {
                 "example.org": "''' + TESTDATA_PATH + '''example.org.zone"
             },
             "cache-enable": true
         }]''', True)
-        dsrc, finder, exact = clist.find(isc.dns.Name("sub.example.org"))
+        dsrc, finder, exact = self.clist.find(isc.dns.Name("sub.example.org"))
         self.assertIsNotNone(dsrc)
         self.assertTrue(isinstance(dsrc, isc.datasrc.DataSourceClient))
         self.assertIsNotNone(finder)
@@ -112,33 +123,33 @@ class ClientListTest(unittest.TestCase):
         self.assertEqual(2, sys.getrefcount(dsrc))
         # We check an exact match in test_configure already
         self.assertFalse(exact)
-        dsrc, finder, exact = clist.find(isc.dns.Name("sub.example.org"),
-                                         False)
-        self.assertIsNotNone(dsrc)
-        self.assertTrue(isinstance(dsrc, isc.datasrc.DataSourceClient))
-        self.assertIsNotNone(finder)
-        self.assertTrue(isinstance(finder, isc.datasrc.ZoneFinder))
+        self.dsrc, self.finder, exact = \
+            self.clist.find(isc.dns.Name("sub.example.org"), False)
+        self.assertIsNotNone(self.dsrc)
+        self.assertTrue(isinstance(self.dsrc, isc.datasrc.DataSourceClient))
+        self.assertIsNotNone(self.finder)
+        self.assertTrue(isinstance(self.finder, isc.datasrc.ZoneFinder))
         self.assertFalse(exact)
-        dsrc, finder, exact = clist.find(isc.dns.Name("sub.example.org"),
-                                         True)
-        self.assertIsNone(dsrc)
-        self.assertIsNone(finder)
+        self.dsrc, self.finder, exact = \
+            self.clist.find(isc.dns.Name("sub.example.org"), True)
+        self.assertIsNone(self.dsrc)
+        self.assertIsNone(self.finder)
         self.assertFalse(exact)
-        dsrc, finder, exact = clist.find(isc.dns.Name("sub.example.org"),
-                                         False, False)
-        self.assertIsNotNone(dsrc)
-        self.assertTrue(isinstance(dsrc, isc.datasrc.DataSourceClient))
-        self.assertIsNotNone(finder)
-        self.assertTrue(isinstance(finder, isc.datasrc.ZoneFinder))
+        self.dsrc, self.finder, exact = \
+            self.clist.find(isc.dns.Name("sub.example.org"), False, False)
+        self.assertIsNotNone(self.dsrc)
+        self.assertTrue(isinstance(self.dsrc, isc.datasrc.DataSourceClient))
+        self.assertIsNotNone(self.finder)
+        self.assertTrue(isinstance(self.finder, isc.datasrc.ZoneFinder))
         self.assertFalse(exact)
-        dsrc, finder, exact = clist.find(isc.dns.Name("sub.example.org"),
-                                         True, False)
-        self.assertIsNone(dsrc)
-        self.assertIsNone(finder)
+        self.dsrc, self.finder, exact = \
+            self.clist.find(isc.dns.Name("sub.example.org"), True, False)
+        self.assertIsNone(self.dsrc)
+        self.assertIsNone(self.finder)
         self.assertFalse(exact)
         # Some invalid inputs
-        self.assertRaises(TypeError, clist.find, "example.org")
-        self.assertRaises(TypeError, clist.find)
+        self.assertRaises(TypeError, self.clist.find, "example.org")
+        self.assertRaises(TypeError, self.clist.find)
 
 if __name__ == "__main__":
     isc.log.init("bind10")