Merge branch 'master' into trac1128

ChangeLog

@@ -1,3 +1,9 @@
+276.	[func]		stephen
+	Although the top-level loggers are named after the program (e.g.
+	b10-auth, b10-resolver), allow the logger configuration to omit the
+	"b10-" prefix and use just the module name.
+	(Trac #1003, git a01cd4ac5a68a1749593600c0f338620511cae2d)
+
 275.	[func]		jinmei
 	Added support for TSIG key matching in ACLs.  The xfrout ACL can
 	now refer to TSIG key names using the "key" attribute.  For

src/bin/auth/auth_config.cc

@@ -107,7 +107,7 @@ DatasourcesConfig::commit() {
     // server implementation details, and isn't scalable wrt the number of
     // data source types, and should eventually be improved.
     // Currently memory data source for class IN is the only possibility.
-    server_.setMemoryDataSrc(RRClass::IN(), AuthSrv::MemoryDataSrcPtr());
+    server_.setInMemoryClient(RRClass::IN(), AuthSrv::InMemoryClientPtr());
 
     BOOST_FOREACH(shared_ptr<AuthConfigParser> datasrc_config, datasources_) {
         datasrc_config->commit();
@@ -125,12 +125,12 @@ public:
     {}
     virtual void build(ConstElementPtr config_value);
     virtual void commit() {
-        server_.setMemoryDataSrc(rrclass_, memory_datasrc_);
+        server_.setInMemoryClient(rrclass_, memory_client_);
     }
 private:
     AuthSrv& server_;
     RRClass rrclass_;
-    AuthSrv::MemoryDataSrcPtr memory_datasrc_;
+    AuthSrv::InMemoryClientPtr memory_client_;
 };
 
 void
@@ -143,8 +143,8 @@ MemoryDatasourceConfig::build(ConstElementPtr config_value) {
     // We'd eventually optimize building zones (in case of reloading) by
     // selectively loading fresh zones.  Right now we simply check the
     // RR class is supported by the server implementation.
-    server_.getMemoryDataSrc(rrclass_);
-    memory_datasrc_ = AuthSrv::MemoryDataSrcPtr(new MemoryDataSrc());
+    server_.getInMemoryClient(rrclass_);
+    memory_client_ = AuthSrv::InMemoryClientPtr(new InMemoryClient());
 
     ConstElementPtr zones_config = config_value->get("zones");
     if (!zones_config) {
@@ -163,9 +163,10 @@ MemoryDatasourceConfig::build(ConstElementPtr config_value) {
             isc_throw(AuthConfigError, "Missing zone file for zone: "
                       << origin->str());
         }
-        shared_ptr<MemoryZone> new_zone(new MemoryZone(rrclass_,
+        shared_ptr<InMemoryZoneFinder> zone_finder(new
+                                                   InMemoryZoneFinder(rrclass_,
             Name(origin->stringValue())));
-        const result::Result result = memory_datasrc_->addZone(new_zone);
+        const result::Result result = memory_client_->addZone(zone_finder);
         if (result == result::EXIST) {
             isc_throw(AuthConfigError, "zone "<< origin->str()
                       << " already exists");
@@ -177,7 +178,7 @@ MemoryDatasourceConfig::build(ConstElementPtr config_value) {
          * need the load method to be split into some kind of build and
          * commit/abort parts.
          */
-        new_zone->load(file->stringValue());
+        zone_finder->load(file->stringValue());
     }
 }
 

src/bin/auth/auth_srv.cc

@@ -108,8 +108,8 @@ public:
     AbstractSession* xfrin_session_;
 
     /// In-memory data source.  Currently class IN only for simplicity.
-    const RRClass memory_datasrc_class_;
-    AuthSrv::MemoryDataSrcPtr memory_datasrc_;
+    const RRClass memory_client_class_;
+    AuthSrv::InMemoryClientPtr memory_client_;
 
     /// Hot spot cache
     isc::datasrc::HotCache cache_;
@@ -145,7 +145,7 @@ AuthSrvImpl::AuthSrvImpl(const bool use_cache,
                          AbstractXfroutClient& xfrout_client) :
     config_session_(NULL),
     xfrin_session_(NULL),
-    memory_datasrc_class_(RRClass::IN()),
+    memory_client_class_(RRClass::IN()),
     statistics_timer_(io_service_),
     counters_(),
     keyring_(NULL),
@@ -329,34 +329,34 @@ AuthSrv::getConfigSession() const {
     return (impl_->config_session_);
 }
 
-AuthSrv::MemoryDataSrcPtr
-AuthSrv::getMemoryDataSrc(const RRClass& rrclass) {
+AuthSrv::InMemoryClientPtr
+AuthSrv::getInMemoryClient(const RRClass& rrclass) {
     // XXX: for simplicity, we only support the IN class right now.
-    if (rrclass != impl_->memory_datasrc_class_) {
+    if (rrclass != impl_->memory_client_class_) {
         isc_throw(InvalidParameter,
                   "Memory data source is not supported for RR class "
                   << rrclass);
     }
-    return (impl_->memory_datasrc_);
+    return (impl_->memory_client_);
 }
 
 void
-AuthSrv::setMemoryDataSrc(const isc::dns::RRClass& rrclass,
-                          MemoryDataSrcPtr memory_datasrc)
+AuthSrv::setInMemoryClient(const isc::dns::RRClass& rrclass,
+                           InMemoryClientPtr memory_client)
 {
     // XXX: see above
-    if (rrclass != impl_->memory_datasrc_class_) {
+    if (rrclass != impl_->memory_client_class_) {
         isc_throw(InvalidParameter,
                   "Memory data source is not supported for RR class "
                   << rrclass);
-    } else if (!impl_->memory_datasrc_ && memory_datasrc) {
+    } else if (!impl_->memory_client_ && memory_client) {
         LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_MEM_DATASRC_ENABLED)
                   .arg(rrclass);
-    } else if (impl_->memory_datasrc_ && !memory_datasrc) {
+    } else if (impl_->memory_client_ && !memory_client) {
         LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_MEM_DATASRC_DISABLED)
                   .arg(rrclass);
     }
-    impl_->memory_datasrc_ = memory_datasrc;
+    impl_->memory_client_ = memory_client;
 }
 
 uint32_t
@@ -505,10 +505,10 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, MessagePtr message,
         // If a memory data source is configured call the separate
         // Query::process()
         const ConstQuestionPtr question = *message->beginQuestion();
-        if (memory_datasrc_ && memory_datasrc_class_ == question->getClass()) {
+        if (memory_client_ && memory_client_class_ == question->getClass()) {
             const RRType& qtype = question->getType();
             const Name& qname = question->getName();
-            auth::Query(*memory_datasrc_, qname, qtype, *message).process();
+            auth::Query(*memory_client_, qname, qtype, *message).process();
         } else {
             datasrc::Query query(*message, cache_, dnssec_ok);
             data_sources_.doQuery(query);

src/bin/auth/auth_srv.h

@@ -17,7 +17,7 @@
 
 #include <string>
 
-// For MemoryDataSrcPtr below.  This should be a temporary definition until
+// For InMemoryClientPtr below.  This should be a temporary definition until
 // we reorganize the data source framework.
 #include <boost/shared_ptr.hpp>
 
@@ -39,7 +39,7 @@
 
 namespace isc {
 namespace datasrc {
-class MemoryDataSrc;
+class InMemoryClient;
 }
 namespace xfr {
 class AbstractXfroutClient;
@@ -133,7 +133,7 @@ public:
     /// If there is a data source installed, it will be replaced with the
     /// new one.
     ///
-    /// In the current implementation, the SQLite data source and MemoryDataSrc
+    /// In the current implementation, the SQLite data source and InMemoryClient
     /// are assumed.
     /// We can enable memory data source and get the path of SQLite database by
     /// the \c config parameter.  If we disabled memory data source, the SQLite
@@ -233,16 +233,16 @@ public:
     ///
     void setXfrinSession(isc::cc::AbstractSession* xfrin_session);
 
-    /// A shared pointer type for \c MemoryDataSrc.
+    /// A shared pointer type for \c InMemoryClient.
     ///
     /// This is defined inside the \c AuthSrv class as it's supposed to be
     /// a short term interface until we integrate the in-memory and other
     /// data source frameworks.
-    typedef boost::shared_ptr<isc::datasrc::MemoryDataSrc> MemoryDataSrcPtr;
+    typedef boost::shared_ptr<isc::datasrc::InMemoryClient> InMemoryClientPtr;
 
-    /// An immutable shared pointer type for \c MemoryDataSrc.
-    typedef boost::shared_ptr<const isc::datasrc::MemoryDataSrc>
-    ConstMemoryDataSrcPtr;
+    /// An immutable shared pointer type for \c InMemoryClient.
+    typedef boost::shared_ptr<const isc::datasrc::InMemoryClient>
+    ConstInMemoryClientPtr;
 
     /// Returns the in-memory data source configured for the \c AuthSrv,
     /// if any.
@@ -260,11 +260,11 @@ public:
     /// \param rrclass The RR class of the requested in-memory data source.
     /// \return A pointer to the in-memory data source, if configured;
     /// otherwise NULL.
-    MemoryDataSrcPtr getMemoryDataSrc(const isc::dns::RRClass& rrclass);
+    InMemoryClientPtr getInMemoryClient(const isc::dns::RRClass& rrclass);
 
     /// Sets or replaces the in-memory data source of the specified RR class.
     ///
-    /// As noted in \c getMemoryDataSrc(), some RR classes may not be
+    /// As noted in \c getInMemoryClient(), some RR classes may not be
     /// supported, in which case an exception of class \c InvalidParameter
     /// will be thrown.
     /// This method never throws an exception otherwise.
@@ -275,9 +275,9 @@ public:
     /// in-memory data source.
     ///
     /// \param rrclass The RR class of the in-memory data source to be set.
-    /// \param memory_datasrc A (shared) pointer to \c MemoryDataSrc to be set.
-    void setMemoryDataSrc(const isc::dns::RRClass& rrclass,
-                          MemoryDataSrcPtr memory_datasrc);
+    /// \param memory_datasrc A (shared) pointer to \c InMemoryClient to be set.
+    void setInMemoryClient(const isc::dns::RRClass& rrclass,
+                           InMemoryClientPtr memory_client);
 
     /// \brief Set the communication session with Statistics.
     ///

src/bin/auth/command.cc

@@ -136,19 +136,21 @@ public:
         // that doesn't block other server operations.
         // TODO: we may (should?) want to check the "last load time" and
         // the timestamp of the file and skip loading if the file isn't newer.
-        shared_ptr<MemoryZone> newzone(new MemoryZone(oldzone->getClass(),
-                                                      oldzone->getOrigin()));
-        newzone->load(oldzone->getFileName());
-        oldzone->swap(*newzone);
+        shared_ptr<InMemoryZoneFinder> zone_finder(
+            new InMemoryZoneFinder(old_zone_finder->getClass(),
+                                   old_zone_finder->getOrigin()));
+        zone_finder->load(old_zone_finder->getFileName());
+        old_zone_finder->swap(*zone_finder);
         LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_LOAD_ZONE)
-                  .arg(newzone->getOrigin()).arg(newzone->getClass());
+                  .arg(zone_finder->getOrigin()).arg(zone_finder->getClass());
     }
 
 private:
-    shared_ptr<MemoryZone> oldzone; // zone to be updated with the new file.
+    // zone finder to be updated with the new file.
+    shared_ptr<InMemoryZoneFinder> old_zone_finder;
 
     // A helper private method to parse and validate command parameters.
-    // On success, it sets 'oldzone' to the zone to be updated.
+    // On success, it sets 'old_zone_finder' to the zone to be updated.
     // It returns true if everything is okay; and false if the command is
     // valid but there's no need for further process.
     bool validate(AuthSrv& server, isc::data::ConstElementPtr args) {
@@ -176,7 +178,7 @@ private:
         const RRClass zone_class = class_elem ?
             RRClass(class_elem->stringValue()) : RRClass::IN();
 
-        AuthSrv::MemoryDataSrcPtr datasrc(server.getMemoryDataSrc(zone_class));
+        AuthSrv::InMemoryClientPtr datasrc(server.getInMemoryClient(zone_class));
         if (datasrc == NULL) {
             isc_throw(AuthCommandError, "Memory data source is disabled");
         }
@@ -188,13 +190,14 @@ private:
         const Name origin(origin_elem->stringValue());
 
         // Get the current zone
-        const MemoryDataSrc::FindResult result = datasrc->findZone(origin);
+        const InMemoryClient::FindResult result = datasrc->findZone(origin);
         if (result.code != result::SUCCESS) {
             isc_throw(AuthCommandError, "Zone " << origin <<
                       " is not found in data source");
         }
 
-        oldzone = boost::dynamic_pointer_cast<MemoryZone>(result.zone);
+        old_zone_finder = boost::dynamic_pointer_cast<InMemoryZoneFinder>(
+            result.zone_finder);
 
         return (true);
     }

src/bin/auth/query.cc

@@ -19,7 +19,7 @@
 #include <dns/rcode.h>
 #include <dns/rdataclass.h>
 
-#include <datasrc/memory_datasrc.h>
+#include <datasrc/client.h>
 
 #include <auth/query.h>
 
@@ -31,14 +31,14 @@ namespace isc {
 namespace auth {
 
 void
-Query::getAdditional(const Zone& zone, const RRset& rrset) const {
+Query::getAdditional(const ZoneFinder& zone, const RRset& rrset) const {
     RdataIteratorPtr rdata_iterator(rrset.getRdataIterator());
     for (; !rdata_iterator->isLast(); rdata_iterator->next()) {
         const Rdata& rdata(rdata_iterator->getCurrent());
         if (rrset.getType() == RRType::NS()) {
             // Need to perform the search in the "GLUE OK" mode.
             const generic::NS& ns = dynamic_cast<const generic::NS&>(rdata);
-            findAddrs(zone, ns.getNSName(), Zone::FIND_GLUE_OK);
+            findAddrs(zone, ns.getNSName(), ZoneFinder::FIND_GLUE_OK);
         } else if (rrset.getType() == RRType::MX()) {
             const generic::MX& mx(dynamic_cast<const generic::MX&>(rdata));
             findAddrs(zone, mx.getMXName());
@@ -47,8 +47,8 @@ Query::getAdditional(const Zone& zone, const RRset& rrset) const {
 }
 
 void
-Query::findAddrs(const Zone& zone, const Name& qname,
-                 const Zone::FindOptions options) const
+Query::findAddrs(const ZoneFinder& zone, const Name& qname,
+                 const ZoneFinder::FindOptions options) const
 {
     // Out of zone name
     NameComparisonResult result = zone.getOrigin().compare(qname);
@@ -66,9 +66,9 @@ Query::findAddrs(const Zone& zone, const Name& qname,
 
     // Find A rrset
     if (qname_ != qname || qtype_ != RRType::A()) {
-        Zone::FindResult a_result = zone.find(qname, RRType::A(), NULL,
+        ZoneFinder::FindResult a_result = zone.find(qname, RRType::A(), NULL,
                                               options);
-        if (a_result.code == Zone::SUCCESS) {
+        if (a_result.code == ZoneFinder::SUCCESS) {
             response_.addRRset(Message::SECTION_ADDITIONAL,
                     boost::const_pointer_cast<RRset>(a_result.rrset));
         }
@@ -76,9 +76,9 @@ Query::findAddrs(const Zone& zone, const Name& qname,
 
     // Find AAAA rrset
     if (qname_ != qname || qtype_ != RRType::AAAA()) {
-        Zone::FindResult aaaa_result =
+        ZoneFinder::FindResult aaaa_result =
             zone.find(qname, RRType::AAAA(), NULL, options);
-        if (aaaa_result.code == Zone::SUCCESS) {
+        if (aaaa_result.code == ZoneFinder::SUCCESS) {
             response_.addRRset(Message::SECTION_ADDITIONAL,
                     boost::const_pointer_cast<RRset>(aaaa_result.rrset));
         }
@@ -86,10 +86,10 @@ Query::findAddrs(const Zone& zone, const Name& qname,
 }
 
 void
-Query::putSOA(const Zone& zone) const {
-    Zone::FindResult soa_result(zone.find(zone.getOrigin(),
+Query::putSOA(const ZoneFinder& zone) const {
+    ZoneFinder::FindResult soa_result(zone.find(zone.getOrigin(),
         RRType::SOA()));
-    if (soa_result.code != Zone::SUCCESS) {
+    if (soa_result.code != ZoneFinder::SUCCESS) {
         isc_throw(NoSOA, "There's no SOA record in zone " <<
             zone.getOrigin().toText());
     } else {
@@ -104,11 +104,12 @@ Query::putSOA(const Zone& zone) const {
 }
 
 void
-Query::getAuthAdditional(const Zone& zone) const {
+Query::getAuthAdditional(const ZoneFinder& zone) const {
     // Fill in authority and addtional sections.
-    Zone::FindResult ns_result = zone.find(zone.getOrigin(), RRType::NS());
+    ZoneFinder::FindResult ns_result = zone.find(zone.getOrigin(),
+                                                 RRType::NS());
     // zone origin name should have NS records
-    if (ns_result.code != Zone::SUCCESS) {
+    if (ns_result.code != ZoneFinder::SUCCESS) {
         isc_throw(NoApexNS, "There's no apex NS records in zone " <<
                 zone.getOrigin().toText());
     } else {
@@ -125,8 +126,8 @@ Query::process() const {
     const bool qtype_is_any = (qtype_ == RRType::ANY());
 
     response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
-    const MemoryDataSrc::FindResult result =
-        memory_datasrc_.findZone(qname_);
+    const DataSourceClient::FindResult result =
+        datasrc_client_.findZone(qname_);
 
     // If we have no matching authoritative zone for the query name, return
     // REFUSED.  In short, this is to be compatible with BIND 9, but the
@@ -145,11 +146,10 @@ Query::process() const {
     while (keep_doing) {
         keep_doing = false;
         std::auto_ptr<RRsetList> target(qtype_is_any ? new RRsetList : NULL);
-        const Zone::FindResult db_result(result.zone->find(qname_, qtype_,
-            target.get()));
-
+        const ZoneFinder::FindResult db_result(
+            result.zone_finder->find(qname_, qtype_, target.get()));
         switch (db_result.code) {
-            case Zone::DNAME: {
+            case ZoneFinder::DNAME: {
                 // First, put the dname into the answer
                 response_.addRRset(Message::SECTION_ANSWER,
                     boost::const_pointer_cast<RRset>(db_result.rrset));
@@ -191,7 +191,7 @@ Query::process() const {
                 response_.addRRset(Message::SECTION_ANSWER, cname);
                 break;
             }
-            case Zone::CNAME:
+            case ZoneFinder::CNAME:
                 /*
                  * We don't do chaining yet. Therefore handling a CNAME is
                  * mostly the same as handling SUCCESS, but we didn't get
@@ -204,46 +204,46 @@ Query::process() const {
                 response_.addRRset(Message::SECTION_ANSWER,
                     boost::const_pointer_cast<RRset>(db_result.rrset));
                 break;
-            case Zone::SUCCESS:
+            case ZoneFinder::SUCCESS:
                 if (qtype_is_any) {
                     // If quety type is ANY, insert all RRs under the domain
                     // into answer section.
                     BOOST_FOREACH(RRsetPtr rrset, *target) {
                         response_.addRRset(Message::SECTION_ANSWER, rrset);
                         // Handle additional for answer section
-                        getAdditional(*result.zone, *rrset.get());
+                        getAdditional(*result.zone_finder, *rrset.get());
                     }
                 } else {
                     response_.addRRset(Message::SECTION_ANSWER,
                         boost::const_pointer_cast<RRset>(db_result.rrset));
                     // Handle additional for answer section
-                    getAdditional(*result.zone, *db_result.rrset);
+                    getAdditional(*result.zone_finder, *db_result.rrset);
                 }
                 // If apex NS records haven't been provided in the answer
                 // section, insert apex NS records into the authority section
                 // and AAAA/A RRS of each of the NS RDATA into the additional
                 // section.
-                if (qname_ != result.zone->getOrigin() ||
-                    db_result.code != Zone::SUCCESS ||
+                if (qname_ != result.zone_finder->getOrigin() ||
+                    db_result.code != ZoneFinder::SUCCESS ||
                     (qtype_ != RRType::NS() && !qtype_is_any))
                 {
-                    getAuthAdditional(*result.zone);
+                    getAuthAdditional(*result.zone_finder);
                 }
                 break;
-            case Zone::DELEGATION:
+            case ZoneFinder::DELEGATION:
                 response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
                 response_.addRRset(Message::SECTION_AUTHORITY,
                     boost::const_pointer_cast<RRset>(db_result.rrset));
-                getAdditional(*result.zone, *db_result.rrset);
+                getAdditional(*result.zone_finder, *db_result.rrset);
                 break;
-            case Zone::NXDOMAIN:
+            case ZoneFinder::NXDOMAIN:
                 // Just empty answer with SOA in authority section
                 response_.setRcode(Rcode::NXDOMAIN());
-                putSOA(*result.zone);
+                putSOA(*result.zone_finder);
                 break;
-            case Zone::NXRRSET:
+            case ZoneFinder::NXRRSET:
                 // Just empty answer with SOA in authority section
-                putSOA(*result.zone);
+                putSOA(*result.zone_finder);
                 break;
         }
     }

src/bin/auth/query.h

@@ -26,7 +26,7 @@ class RRset;
 }
 
 namespace datasrc {
-class MemoryDataSrc;
+class DataSourceClient;
 }
 
 namespace auth {
@@ -36,10 +36,8 @@ namespace auth {
 ///
 /// Many of the design details for this class are still in flux.
 /// We'll revisit and update them as we add more functionality, for example:
-/// - memory_datasrc parameter of the constructor.  It is a data source that
-///   uses in memory dedicated backend.
 /// - as a related point, we may have to pass the RR class of the query.
-///   in the initial implementation the RR class is an attribute of memory
+///   in the initial implementation the RR class is an attribute of
 ///   datasource and omitted.  It's not clear if this assumption holds with
 ///   generic data sources.  On the other hand, it will help keep
 ///   implementation simpler, and we might rather want to modify the design
@@ -51,7 +49,7 @@ namespace auth {
 ///   separate attribute setter.
 /// - likewise, we'll eventually need to do per zone access control, for which
 ///   we need querier's information such as its IP address.
-/// - memory_datasrc and response may better be parameters to process() instead
+/// - datasrc_client and response may better be parameters to process() instead
 ///   of the constructor.
 ///
 /// <b>Note:</b> The class name is intentionally the same as the one used in
@@ -71,7 +69,7 @@ private:
     /// Adds a SOA of the zone into the authority zone of response_.
     /// Can throw NoSOA.
     ///
-    void putSOA(const isc::datasrc::Zone& zone) const;
+    void putSOA(const isc::datasrc::ZoneFinder& zone) const;
 
     /// \brief Look up additional data (i.e., address records for the names
     /// included in NS or MX records).
@@ -83,11 +81,11 @@ private:
     /// This method may throw a exception because its underlying methods may
     /// throw exceptions.
     ///
-    /// \param zone The Zone wherein the additional data to the query is bo be
-    /// found.
+    /// \param zone The ZoneFinder through which the additional data for the
+    /// query is to be found.
     /// \param rrset The RRset (i.e., NS or MX rrset) which require additional
     /// processing.
-    void getAdditional(const isc::datasrc::Zone& zone,
+    void getAdditional(const isc::datasrc::ZoneFinder& zone,
                        const isc::dns::RRset& rrset) const;
 
     /// \brief Find address records for a specified name.
@@ -102,18 +100,19 @@ private:
     /// The glue records must exactly match the name in the NS RDATA, without
     /// CNAME or wildcard processing.
     ///
-    /// \param zone The \c Zone wherein the address records is to be found.
+    /// \param zone The \c ZoneFinder through which the address records is to
+    /// be found.
     /// \param qname The name in rrset RDATA.
     /// \param options The search options.
-    void findAddrs(const isc::datasrc::Zone& zone,
+    void findAddrs(const isc::datasrc::ZoneFinder& zone,
                    const isc::dns::Name& qname,
-                   const isc::datasrc::Zone::FindOptions options
-                   = isc::datasrc::Zone::FIND_DEFAULT) const;
+                   const isc::datasrc::ZoneFinder::FindOptions options
+                   = isc::datasrc::ZoneFinder::FIND_DEFAULT) const;
 
-    /// \brief Look up \c Zone's NS and address records for the NS RDATA
-    /// (domain name) for authoritative answer.
+    /// \brief Look up a zone's NS RRset and their address records for an
+    /// authoritative answer.
     ///
-    /// On returning an authoritative answer, insert the \c Zone's NS into the
+    /// On returning an authoritative answer, insert a zone's NS into the
     /// authority section and AAAA/A RRs of each of the NS RDATA into the
     /// additional section.
     ///
@@ -126,24 +125,24 @@ private:
     /// include AAAA/A RRs under a zone cut in additional section. (BIND 9
     /// excludes under-cut RRs; NSD include them.)
     ///
-    /// \param zone The \c Zone wherein the additional data to the query is to
-    /// be found.
-    void getAuthAdditional(const isc::datasrc::Zone& zone) const;
+    /// \param zone The \c ZoneFinder through which the NS and additional data
+    /// for the query are to be found.
+    void getAuthAdditional(const isc::datasrc::ZoneFinder& zone) const;
 
 public:
     /// Constructor from query parameters.
     ///
     /// This constructor never throws an exception.
     ///
-    /// \param memory_datasrc The memory datasource wherein the answer to the query is
+    /// \param datasrc_client The datasource wherein the answer to the query is
     /// to be found.
     /// \param qname The query name
     /// \param qtype The RR type of the query
     /// \param response The response message to store the answer to the query.
-    Query(const isc::datasrc::MemoryDataSrc& memory_datasrc,
+    Query(const isc::datasrc::DataSourceClient& datasrc_client,
           const isc::dns::Name& qname, const isc::dns::RRType& qtype,
           isc::dns::Message& response) :
-        memory_datasrc_(memory_datasrc), qname_(qname), qtype_(qtype),
+        datasrc_client_(datasrc_client), qname_(qname), qtype_(qtype),
         response_(response)
     {}
 
@@ -157,7 +156,7 @@ public:
     /// successful search would result in adding a corresponding RRset to
     /// the answer section of the response.
     ///
-    /// If no matching zone is found in the memory datasource, the RCODE of
+    /// If no matching zone is found in the datasource, the RCODE of
     /// SERVFAIL will be set in the response.
     /// <b>Note:</b> this is different from the error code that BIND 9 returns
     /// by default when it's configured as an authoritative-only server (and
@@ -208,7 +207,7 @@ public:
     };
 
 private:
-    const isc::datasrc::MemoryDataSrc& memory_datasrc_;
+    const isc::datasrc::DataSourceClient& datasrc_client_;
     const isc::dns::Name& qname_;
     const isc::dns::RRType& qtype_;
     isc::dns::Message& response_;

src/bin/auth/tests/auth_srv_unittest.cc

@@ -651,17 +651,17 @@ TEST_F(AuthSrvTest, updateConfigFail) {
                 QR_FLAG | AA_FLAG, 1, 1, 1, 0);
 }
 
-TEST_F(AuthSrvTest, updateWithMemoryDataSrc) {
+TEST_F(AuthSrvTest, updateWithInMemoryClient) {
     // Test configuring memory data source.  Detailed test cases are covered
     // in the configuration tests.  We only check the AuthSrv interface here.
 
     // By default memory data source isn't enabled
-    EXPECT_EQ(AuthSrv::MemoryDataSrcPtr(), server.getMemoryDataSrc(rrclass));
+    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
     updateConfig(&server,
                  "{\"datasources\": [{\"type\": \"memory\"}]}", true);
     // after successful configuration, we should have one (with empty zoneset).
-    ASSERT_NE(AuthSrv::MemoryDataSrcPtr(), server.getMemoryDataSrc(rrclass));
-    EXPECT_EQ(0, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    ASSERT_NE(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_EQ(0, server.getInMemoryClient(rrclass)->getZoneCount());
 
     // The memory data source is empty, should return REFUSED rcode.
     createDataFromFile("examplequery_fromWire.wire");
@@ -672,7 +672,7 @@ TEST_F(AuthSrvTest, updateWithMemoryDataSrc) {
                 opcode.getCode(), QR_FLAG, 1, 0, 0, 0);
 }
 
-TEST_F(AuthSrvTest, chQueryWithMemoryDataSrc) {
+TEST_F(AuthSrvTest, chQueryWithInMemoryClient) {
     // Configure memory data source for class IN
     updateConfig(&server, "{\"datasources\": "
                  "[{\"class\": \"IN\", \"type\": \"memory\"}]}", true);

src/bin/auth/tests/command_unittest.cc

@@ -60,7 +60,6 @@ protected:
     MockSession statistics_session;
     MockXfroutClient xfrout;
     AuthSrv server;
-    AuthSrv::ConstMemoryDataSrcPtr memory_datasrc;
     ConstElementPtr result;
     int rcode;
 public:
@@ -110,18 +109,18 @@ TEST_F(AuthCommandTest, shutdown) {
 // zones, and checks the zones are correctly loaded.
 void
 zoneChecks(AuthSrv& server) {
-    EXPECT_TRUE(server.getMemoryDataSrc(RRClass::IN()));
-    EXPECT_EQ(Zone::SUCCESS, server.getMemoryDataSrc(RRClass::IN())->
-              findZone(Name("ns.test1.example")).zone->
+    EXPECT_TRUE(server.getInMemoryClient(RRClass::IN()));
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getInMemoryClient(RRClass::IN())->
+              findZone(Name("ns.test1.example")).zone_finder->
               find(Name("ns.test1.example"), RRType::A()).code);
-    EXPECT_EQ(Zone::NXRRSET, server.getMemoryDataSrc(RRClass::IN())->
-              findZone(Name("ns.test1.example")).zone->
+    EXPECT_EQ(ZoneFinder::NXRRSET, server.getInMemoryClient(RRClass::IN())->
+              findZone(Name("ns.test1.example")).zone_finder->
               find(Name("ns.test1.example"), RRType::AAAA()).code);
-    EXPECT_EQ(Zone::SUCCESS, server.getMemoryDataSrc(RRClass::IN())->
-              findZone(Name("ns.test2.example")).zone->
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getInMemoryClient(RRClass::IN())->
+              findZone(Name("ns.test2.example")).zone_finder->
               find(Name("ns.test2.example"), RRType::A()).code);
-    EXPECT_EQ(Zone::NXRRSET, server.getMemoryDataSrc(RRClass::IN())->
-              findZone(Name("ns.test2.example")).zone->
+    EXPECT_EQ(ZoneFinder::NXRRSET, server.getInMemoryClient(RRClass::IN())->
+              findZone(Name("ns.test2.example")).zone_finder->
               find(Name("ns.test2.example"), RRType::AAAA()).code);
 }
 
@@ -147,21 +146,21 @@ configureZones(AuthSrv& server) {
 
 void
 newZoneChecks(AuthSrv& server) {
-    EXPECT_TRUE(server.getMemoryDataSrc(RRClass::IN()));
-    EXPECT_EQ(Zone::SUCCESS, server.getMemoryDataSrc(RRClass::IN())->
-              findZone(Name("ns.test1.example")).zone->
+    EXPECT_TRUE(server.getInMemoryClient(RRClass::IN()));
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getInMemoryClient(RRClass::IN())->
+              findZone(Name("ns.test1.example")).zone_finder->
               find(Name("ns.test1.example"), RRType::A()).code);
     // now test1.example should have ns/AAAA
-    EXPECT_EQ(Zone::SUCCESS, server.getMemoryDataSrc(RRClass::IN())->
-              findZone(Name("ns.test1.example")).zone->
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getInMemoryClient(RRClass::IN())->
+              findZone(Name("ns.test1.example")).zone_finder->
               find(Name("ns.test1.example"), RRType::AAAA()).code);
 
     // test2.example shouldn't change
-    EXPECT_EQ(Zone::SUCCESS, server.getMemoryDataSrc(RRClass::IN())->
-              findZone(Name("ns.test2.example")).zone->
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getInMemoryClient(RRClass::IN())->
+              findZone(Name("ns.test2.example")).zone_finder->
               find(Name("ns.test2.example"), RRType::A()).code);
-    EXPECT_EQ(Zone::NXRRSET, server.getMemoryDataSrc(RRClass::IN())->
-              findZone(Name("ns.test2.example")).zone->
+    EXPECT_EQ(ZoneFinder::NXRRSET, server.getInMemoryClient(RRClass::IN())->
+              findZone(Name("ns.test2.example")).zone_finder->
               find(Name("ns.test2.example"), RRType::AAAA()).code);
 }
 

src/bin/auth/tests/config_unittest.cc

@@ -57,12 +57,12 @@ protected:
 
 TEST_F(AuthConfigTest, datasourceConfig) {
     // By default, we don't have any in-memory data source.
-    EXPECT_EQ(AuthSrv::MemoryDataSrcPtr(), server.getMemoryDataSrc(rrclass));
+    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
     configureAuthServer(server, Element::fromJSON(
                             "{\"datasources\": [{\"type\": \"memory\"}]}"));
     // after successful configuration, we should have one (with empty zoneset).
-    ASSERT_NE(AuthSrv::MemoryDataSrcPtr(), server.getMemoryDataSrc(rrclass));
-    EXPECT_EQ(0, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    ASSERT_NE(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_EQ(0, server.getInMemoryClient(rrclass)->getZoneCount());
 }
 
 TEST_F(AuthConfigTest, databaseConfig) {
@@ -82,7 +82,7 @@ TEST_F(AuthConfigTest, versionConfig) {
 }
 
 TEST_F(AuthConfigTest, exceptionGuarantee) {
-    EXPECT_EQ(AuthSrv::MemoryDataSrcPtr(), server.getMemoryDataSrc(rrclass));
+    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
     // This configuration contains an invalid item, which will trigger
     // an exception.
     EXPECT_THROW(configureAuthServer(
@@ -92,7 +92,7 @@ TEST_F(AuthConfigTest, exceptionGuarantee) {
                          " \"no_such_config_var\": 1}")),
                  AuthConfigError);
     // The server state shouldn't change
-    EXPECT_EQ(AuthSrv::MemoryDataSrcPtr(), server.getMemoryDataSrc(rrclass));
+    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
 }
 
 TEST_F(AuthConfigTest, exceptionConversion) {
@@ -154,22 +154,22 @@ protected:
 TEST_F(MemoryDatasrcConfigTest, addZeroDataSrc) {
     parser->build(Element::fromJSON("[]"));
     parser->commit();
-    EXPECT_EQ(AuthSrv::MemoryDataSrcPtr(), server.getMemoryDataSrc(rrclass));
+    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
 }
 
 TEST_F(MemoryDatasrcConfigTest, addEmpty) {
     // By default, we don't have any in-memory data source.
-    EXPECT_EQ(AuthSrv::MemoryDataSrcPtr(), server.getMemoryDataSrc(rrclass));
+    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
     parser->build(Element::fromJSON("[{\"type\": \"memory\"}]"));
     parser->commit();
-    EXPECT_EQ(0, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_EQ(0, server.getInMemoryClient(rrclass)->getZoneCount());
 }
 
 TEST_F(MemoryDatasrcConfigTest, addZeroZone) {
     parser->build(Element::fromJSON("[{\"type\": \"memory\","
                                     "  \"zones\": []}]"));
     parser->commit();
-    EXPECT_EQ(0, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_EQ(0, server.getInMemoryClient(rrclass)->getZoneCount());
 }
 
 TEST_F(MemoryDatasrcConfigTest, addOneZone) {
@@ -179,10 +179,10 @@ TEST_F(MemoryDatasrcConfigTest, addOneZone) {
                       "               \"file\": \"" TEST_DATA_DIR
                       "/example.zone\"}]}]")));
     EXPECT_NO_THROW(parser->commit());
-    EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_EQ(1, server.getInMemoryClient(rrclass)->getZoneCount());
     // Check it actually loaded something
-    EXPECT_EQ(Zone::SUCCESS, server.getMemoryDataSrc(rrclass)->findZone(
-        Name("ns.example.com.")).zone->find(Name("ns.example.com."),
+    EXPECT_EQ(ZoneFinder::SUCCESS, server.getInMemoryClient(rrclass)->findZone(
+        Name("ns.example.com.")).zone_finder->find(Name("ns.example.com."),
         RRType::A()).code);
 }
 
@@ -199,7 +199,7 @@ TEST_F(MemoryDatasrcConfigTest, addMultiZones) {
                       "               \"file\": \"" TEST_DATA_DIR
                       "/example.net.zone\"}]}]")));
     EXPECT_NO_THROW(parser->commit());
-    EXPECT_EQ(3, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_EQ(3, server.getInMemoryClient(rrclass)->getZoneCount());
 }
 
 TEST_F(MemoryDatasrcConfigTest, replace) {
@@ -209,9 +209,9 @@ TEST_F(MemoryDatasrcConfigTest, replace) {
                       "               \"file\": \"" TEST_DATA_DIR
                       "/example.zone\"}]}]")));
     EXPECT_NO_THROW(parser->commit());
-    EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_EQ(1, server.getInMemoryClient(rrclass)->getZoneCount());
     EXPECT_EQ(isc::datasrc::result::SUCCESS,
-              server.getMemoryDataSrc(rrclass)->findZone(
+              server.getInMemoryClient(rrclass)->findZone(
                   Name("example.com")).code);
 
     // create a new parser, and install a new set of configuration.  It
@@ -227,9 +227,9 @@ TEST_F(MemoryDatasrcConfigTest, replace) {
                       "               \"file\": \"" TEST_DATA_DIR
                       "/example.net.zone\"}]}]")));
     EXPECT_NO_THROW(parser->commit());
-    EXPECT_EQ(2, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_EQ(2, server.getInMemoryClient(rrclass)->getZoneCount());
     EXPECT_EQ(isc::datasrc::result::NOTFOUND,
-              server.getMemoryDataSrc(rrclass)->findZone(
+              server.getInMemoryClient(rrclass)->findZone(
                   Name("example.com")).code);
 }
 
@@ -241,9 +241,9 @@ TEST_F(MemoryDatasrcConfigTest, exception) {
                       "               \"file\": \"" TEST_DATA_DIR
                       "/example.zone\"}]}]")));
     EXPECT_NO_THROW(parser->commit());
-    EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_EQ(1, server.getInMemoryClient(rrclass)->getZoneCount());
     EXPECT_EQ(isc::datasrc::result::SUCCESS,
-              server.getMemoryDataSrc(rrclass)->findZone(
+              server.getInMemoryClient(rrclass)->findZone(
                   Name("example.com")).code);
 
     // create a new parser, and try to load something. It will throw,
@@ -262,9 +262,9 @@ TEST_F(MemoryDatasrcConfigTest, exception) {
     // commit it
 
     // The original should be untouched
-    EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_EQ(1, server.getInMemoryClient(rrclass)->getZoneCount());
     EXPECT_EQ(isc::datasrc::result::SUCCESS,
-              server.getMemoryDataSrc(rrclass)->findZone(
+              server.getInMemoryClient(rrclass)->findZone(
                   Name("example.com")).code);
 }
 
@@ -275,13 +275,13 @@ TEST_F(MemoryDatasrcConfigTest, remove) {
                       "               \"file\": \"" TEST_DATA_DIR
                       "/example.zone\"}]}]")));
     EXPECT_NO_THROW(parser->commit());
-    EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_EQ(1, server.getInMemoryClient(rrclass)->getZoneCount());
 
     delete parser;
     parser = createAuthConfigParser(server, "datasources"); 
     EXPECT_NO_THROW(parser->build(Element::fromJSON("[]")));
     EXPECT_NO_THROW(parser->commit());
-    EXPECT_EQ(AuthSrv::MemoryDataSrcPtr(), server.getMemoryDataSrc(rrclass));
+    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
 }
 
 TEST_F(MemoryDatasrcConfigTest, adDuplicateZones) {

src/bin/auth/tests/query_unittest.cc

@@ -93,9 +93,9 @@ const char* const other_zone_rrs =
     "mx.delegation.example.com. 3600 IN A 192.0.2.100\n";
 
 // This is a mock Zone class for testing.
-// It is a derived class of Zone for the convenient of tests.
+// It is a derived class of ZoneFinder for the convenient of tests.
 // Its find() method emulates the common behavior of protocol compliant
-// zone classes, but simplifies some minor cases and also supports broken
+// ZoneFinder classes, but simplifies some minor cases and also supports broken
 // behavior.
 // For simplicity, most names are assumed to be "in zone"; there's only
 // one zone cut at the point of name "delegation.example.com".
@@ -103,9 +103,9 @@ const char* const other_zone_rrs =
 // will result in DNAME.
 // This mock zone doesn't handle empty non terminal nodes (if we need to test
 // such cases find() should have specialized code for it).
-class MockZone : public Zone {
+class MockZoneFinder : public ZoneFinder {
 public:
-    MockZone() :
+    MockZoneFinder() :
         origin_(Name("example.com")),
         delegation_name_("delegation.example.com"),
         dname_name_("dname.example.com"),
@@ -120,7 +120,7 @@ public:
             other_zone_rrs;
 
         masterLoad(zone_stream, origin_, rrclass_,
-                   boost::bind(&MockZone::loadRRset, this, _1));
+                   boost::bind(&MockZoneFinder::loadRRset, this, _1));
     }
     virtual const isc::dns::Name& getOrigin() const { return (origin_); }
     virtual const isc::dns::RRClass& getClass() const { return (rrclass_); }
@@ -163,9 +163,9 @@ private:
     const RRClass rrclass_;
 };
 
-Zone::FindResult
-MockZone::find(const Name& name, const RRType& type,
-               RRsetList* target, const FindOptions options) const
+ZoneFinder::FindResult
+MockZoneFinder::find(const Name& name, const RRType& type,
+                     RRsetList* target, const FindOptions options) const
 {
     // Emulating a broken zone: mandatory apex RRs are missing if specifically
     // configured so (which are rare cases).
@@ -233,11 +233,15 @@ protected:
         response.setRcode(Rcode::NOERROR());
         response.setOpcode(Opcode::QUERY());
         // create and add a matching zone.
-        mock_zone = new MockZone();
-        memory_datasrc.addZone(ZonePtr(mock_zone));
+        mock_finder = new MockZoneFinder();
+        memory_client.addZone(ZoneFinderPtr(mock_finder));
     }
-    MockZone* mock_zone;
-    MemoryDataSrc memory_datasrc;
+    MockZoneFinder* mock_finder;
+    // We use InMemoryClient here. We could have some kind of mock client
+    // here, but historically, the Query supported only InMemoryClient
+    // (originally named MemoryDataSrc) and was tested with it, so we keep
+    // it like this for now.
+    InMemoryClient memory_client;
     const Name qname;
     const RRClass qclass;
     const RRType qtype;
@@ -286,14 +290,14 @@ responseCheck(Message& response, const isc::dns::Rcode& rcode,
 TEST_F(QueryTest, noZone) {
     // There's no zone in the memory datasource.  So the response should have
     // REFUSED.
-    MemoryDataSrc empty_memory_datasrc;
-    Query nozone_query(empty_memory_datasrc, qname, qtype, response);
+    InMemoryClient empty_memory_client;
+    Query nozone_query(empty_memory_client, qname, qtype, response);
     EXPECT_NO_THROW(nozone_query.process());
     EXPECT_EQ(Rcode::REFUSED(), response.getRcode());
 }
 
 TEST_F(QueryTest, exactMatch) {
-    Query query(memory_datasrc, qname, qtype, response);
+    Query query(memory_client, qname, qtype, response);
     EXPECT_NO_THROW(query.process());
     // find match rrset
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 3, 3,
@@ -303,7 +307,7 @@ TEST_F(QueryTest, exactMatch) {
 TEST_F(QueryTest, exactAddrMatch) {
     // find match rrset, omit additional data which has already been provided
     // in the answer section from the additional.
-    EXPECT_NO_THROW(Query(memory_datasrc, Name("noglue.example.com"), qtype,
+    EXPECT_NO_THROW(Query(memory_client, Name("noglue.example.com"), qtype,
                           response).process());
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 3, 2,
@@ -315,7 +319,7 @@ TEST_F(QueryTest, exactAddrMatch) {
 TEST_F(QueryTest, apexNSMatch) {
     // find match rrset, omit authority data which has already been provided
     // in the answer section from the authority section.
-    EXPECT_NO_THROW(Query(memory_datasrc, Name("example.com"), RRType::NS(),
+    EXPECT_NO_THROW(Query(memory_client, Name("example.com"), RRType::NS(),
                           response).process());
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 3, 0, 3,
@@ -326,7 +330,7 @@ TEST_F(QueryTest, apexNSMatch) {
 TEST_F(QueryTest, exactAnyMatch) {
     // find match rrset, omit additional data which has already been provided
     // in the answer section from the additional.
-    EXPECT_NO_THROW(Query(memory_datasrc, Name("noglue.example.com"),
+    EXPECT_NO_THROW(Query(memory_client, Name("noglue.example.com"),
                           RRType::ANY(), response).process());
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 3, 2,
@@ -339,18 +343,18 @@ TEST_F(QueryTest, exactAnyMatch) {
 TEST_F(QueryTest, apexAnyMatch) {
     // find match rrset, omit additional data which has already been provided
     // in the answer section from the additional.
-    EXPECT_NO_THROW(Query(memory_datasrc, Name("example.com"),
+    EXPECT_NO_THROW(Query(memory_client, Name("example.com"),
                           RRType::ANY(), response).process());
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 4, 0, 3,
                   "example.com. 3600 IN SOA . . 0 0 0 0 0\n"
                   "example.com. 3600 IN NS glue.delegation.example.com.\n"
                   "example.com. 3600 IN NS noglue.example.com.\n"
                   "example.com. 3600 IN NS example.net.\n",
-                  NULL, ns_addrs_txt, mock_zone->getOrigin());
+                  NULL, ns_addrs_txt, mock_finder->getOrigin());
 }
 
 TEST_F(QueryTest, mxANYMatch) {
-    EXPECT_NO_THROW(Query(memory_datasrc, Name("mx.example.com"),
+    EXPECT_NO_THROW(Query(memory_client, Name("mx.example.com"),
                           RRType::ANY(), response).process());
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 3, 3, 4,
                   mx_txt, zone_ns_txt,
@@ -358,17 +362,17 @@ TEST_F(QueryTest, mxANYMatch) {
 }
 
 TEST_F(QueryTest, glueANYMatch) {
-    EXPECT_NO_THROW(Query(memory_datasrc, Name("delegation.example.com"),
+    EXPECT_NO_THROW(Query(memory_client, Name("delegation.example.com"),
                           RRType::ANY(), response).process());
     responseCheck(response, Rcode::NOERROR(), 0, 0, 4, 3,
                   NULL, delegation_txt, ns_addrs_txt);
 }
 
 TEST_F(QueryTest, nodomainANY) {
-    EXPECT_NO_THROW(Query(memory_datasrc, Name("nxdomain.example.com"),
+    EXPECT_NO_THROW(Query(memory_client, Name("nxdomain.example.com"),
                           RRType::ANY(), response).process());
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 1, 0,
-                  NULL, soa_txt, NULL, mock_zone->getOrigin());
+                  NULL, soa_txt, NULL, mock_finder->getOrigin());
 }
 
 // This tests that when we need to look up Zone's apex NS records for
@@ -376,15 +380,15 @@ TEST_F(QueryTest, nodomainANY) {
 // throw in that case.
 TEST_F(QueryTest, noApexNS) {
     // Disable apex NS record
-    mock_zone->setApexNSFlag(false);
+    mock_finder->setApexNSFlag(false);
 
-    EXPECT_THROW(Query(memory_datasrc, Name("noglue.example.com"), qtype,
+    EXPECT_THROW(Query(memory_client, Name("noglue.example.com"), qtype,
                        response).process(), Query::NoApexNS);
     // We don't look into the response, as it threw
 }
 
 TEST_F(QueryTest, delegation) {
-    EXPECT_NO_THROW(Query(memory_datasrc, Name("delegation.example.com"),
+    EXPECT_NO_THROW(Query(memory_client, Name("delegation.example.com"),
                           qtype, response).process());
 
     responseCheck(response, Rcode::NOERROR(), 0, 0, 4, 3,
@@ -392,18 +396,18 @@ TEST_F(QueryTest, delegation) {
 }
 
 TEST_F(QueryTest, nxdomain) {
-    EXPECT_NO_THROW(Query(memory_datasrc, Name("nxdomain.example.com"), qtype,
+    EXPECT_NO_THROW(Query(memory_client, Name("nxdomain.example.com"), qtype,
                           response).process());
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 1, 0,
-                  NULL, soa_txt, NULL, mock_zone->getOrigin());
+                  NULL, soa_txt, NULL, mock_finder->getOrigin());
 }
 
 TEST_F(QueryTest, nxrrset) {
-    EXPECT_NO_THROW(Query(memory_datasrc, Name("www.example.com"),
+    EXPECT_NO_THROW(Query(memory_client, Name("www.example.com"),
                           RRType::TXT(), response).process());
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 1, 0,
-                  NULL, soa_txt, NULL, mock_zone->getOrigin());
+                  NULL, soa_txt, NULL, mock_finder->getOrigin());
 }
 
 /*
@@ -412,22 +416,22 @@ TEST_F(QueryTest, nxrrset) {
  */
 TEST_F(QueryTest, noSOA) {
     // disable zone's SOA RR.
-    mock_zone->setSOAFlag(false);
+    mock_finder->setSOAFlag(false);
 
     // The NX Domain
-    EXPECT_THROW(Query(memory_datasrc, Name("nxdomain.example.com"),
+    EXPECT_THROW(Query(memory_client, Name("nxdomain.example.com"),
                        qtype, response).process(), Query::NoSOA);
     // Of course, we don't look into the response, as it throwed
 
     // NXRRSET
-    EXPECT_THROW(Query(memory_datasrc, Name("nxrrset.example.com"),
+    EXPECT_THROW(Query(memory_client, Name("nxrrset.example.com"),
                        qtype, response).process(), Query::NoSOA);
 }
 
 TEST_F(QueryTest, noMatchZone) {
     // there's a zone in the memory datasource but it doesn't match the qname.
     // should result in REFUSED.
-    Query(memory_datasrc, Name("example.org"), qtype, response).process();
+    Query(memory_client, Name("example.org"), qtype, response).process();
     EXPECT_EQ(Rcode::REFUSED(), response.getRcode());
 }
 
@@ -438,7 +442,7 @@ TEST_F(QueryTest, noMatchZone) {
  * A record, other to unknown out of zone one.
  */
 TEST_F(QueryTest, MX) {
-    Query(memory_datasrc, Name("mx.example.com"), RRType::MX(),
+    Query(memory_client, Name("mx.example.com"), RRType::MX(),
           response).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 3, 3, 4,
@@ -452,7 +456,7 @@ TEST_F(QueryTest, MX) {
  * This should not trigger the additional processing for the exchange.
  */
 TEST_F(QueryTest, MXAlias) {
-    Query(memory_datasrc, Name("cnamemx.example.com"), RRType::MX(),
+    Query(memory_client, Name("cnamemx.example.com"), RRType::MX(),
           response).process();
 
     // there shouldn't be no additional RRs for the exchanges (we have 3
@@ -472,7 +476,7 @@ TEST_F(QueryTest, MXAlias) {
  * returned.
  */
 TEST_F(QueryTest, CNAME) {
-    Query(memory_datasrc, Name("cname.example.com"), RRType::A(),
+    Query(memory_client, Name("cname.example.com"), RRType::A(),
         response).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 0, 0,
@@ -482,7 +486,7 @@ TEST_F(QueryTest, CNAME) {
 TEST_F(QueryTest, explicitCNAME) {
     // same owner name as the CNAME test but explicitly query for CNAME RR.
     // expect the same response as we don't provide a full chain yet.
-    Query(memory_datasrc, Name("cname.example.com"), RRType::CNAME(),
+    Query(memory_client, Name("cname.example.com"), RRType::CNAME(),
         response).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 3, 3,
@@ -494,7 +498,7 @@ TEST_F(QueryTest, CNAME_NX_RRSET) {
     // note: with chaining, what should be expected is not trivial:
     // BIND 9 returns the CNAME in answer and SOA in authority, no additional.
     // NSD returns the CNAME, NS in authority, A/AAAA for NS in additional.
-    Query(memory_datasrc, Name("cname.example.com"), RRType::TXT(),
+    Query(memory_client, Name("cname.example.com"), RRType::TXT(),
         response).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 0, 0,
@@ -503,7 +507,7 @@ TEST_F(QueryTest, CNAME_NX_RRSET) {
 
 TEST_F(QueryTest, explicitCNAME_NX_RRSET) {
     // same owner name as the NXRRSET test but explicitly query for CNAME RR.
-    Query(memory_datasrc, Name("cname.example.com"), RRType::CNAME(),
+    Query(memory_client, Name("cname.example.com"), RRType::CNAME(),
         response).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 3, 3,
@@ -517,7 +521,7 @@ TEST_F(QueryTest, CNAME_NX_DOMAIN) {
     // RCODE being NXDOMAIN.
     // NSD returns the CNAME, NS in authority, A/AAAA for NS in additional,
     // RCODE being NOERROR.
-    Query(memory_datasrc, Name("cnamenxdom.example.com"), RRType::A(),
+    Query(memory_client, Name("cnamenxdom.example.com"), RRType::A(),
         response).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 0, 0,
@@ -526,7 +530,7 @@ TEST_F(QueryTest, CNAME_NX_DOMAIN) {
 
 TEST_F(QueryTest, explicitCNAME_NX_DOMAIN) {
     // same owner name as the NXDOMAIN test but explicitly query for CNAME RR.
-    Query(memory_datasrc, Name("cnamenxdom.example.com"), RRType::CNAME(),
+    Query(memory_client, Name("cnamenxdom.example.com"), RRType::CNAME(),
         response).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 3, 3,
@@ -542,7 +546,7 @@ TEST_F(QueryTest, CNAME_OUT) {
      * Then the same test should be done with .org included there and
      * see what it does (depends on what we want to do)
      */
-    Query(memory_datasrc, Name("cnameout.example.com"), RRType::A(),
+    Query(memory_client, Name("cnameout.example.com"), RRType::A(),
         response).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 0, 0,
@@ -551,7 +555,7 @@ TEST_F(QueryTest, CNAME_OUT) {
 
 TEST_F(QueryTest, explicitCNAME_OUT) {
     // same owner name as the OUT test but explicitly query for CNAME RR.
-    Query(memory_datasrc, Name("cnameout.example.com"), RRType::CNAME(),
+    Query(memory_client, Name("cnameout.example.com"), RRType::CNAME(),
         response).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 3, 3,
@@ -567,7 +571,7 @@ TEST_F(QueryTest, explicitCNAME_OUT) {
  * pointing to NXRRSET and NXDOMAIN cases (similarly as with CNAME).
  */
 TEST_F(QueryTest, DNAME) {
-    Query(memory_datasrc, Name("www.dname.example.com"), RRType::A(),
+    Query(memory_client, Name("www.dname.example.com"), RRType::A(),
         response).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 2, 0, 0,
@@ -583,7 +587,7 @@ TEST_F(QueryTest, DNAME) {
  * DNAME.
  */
 TEST_F(QueryTest, DNAME_ANY) {
-    Query(memory_datasrc, Name("www.dname.example.com"), RRType::ANY(),
+    Query(memory_client, Name("www.dname.example.com"), RRType::ANY(),
         response).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 2, 0, 0,
@@ -592,7 +596,7 @@ TEST_F(QueryTest, DNAME_ANY) {
 
 // Test when we ask for DNAME explicitly, it does no synthetizing.
 TEST_F(QueryTest, explicitDNAME) {
-    Query(memory_datasrc, Name("dname.example.com"), RRType::DNAME(),
+    Query(memory_client, Name("dname.example.com"), RRType::DNAME(),
         response).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 3, 3,
@@ -604,7 +608,7 @@ TEST_F(QueryTest, explicitDNAME) {
  * the CNAME, it should return the RRset.
  */
 TEST_F(QueryTest, DNAME_A) {
-    Query(memory_datasrc, Name("dname.example.com"), RRType::A(),
+    Query(memory_client, Name("dname.example.com"), RRType::A(),
         response).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 3, 3,
@@ -616,11 +620,11 @@ TEST_F(QueryTest, DNAME_A) {
  * It should not synthetize the CNAME.
  */
 TEST_F(QueryTest, DNAME_NX_RRSET) {
-    EXPECT_NO_THROW(Query(memory_datasrc, Name("dname.example.com"),
+    EXPECT_NO_THROW(Query(memory_client, Name("dname.example.com"),
         RRType::TXT(), response).process());
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 1, 0,
-        NULL, soa_txt, NULL, mock_zone->getOrigin());
+        NULL, soa_txt, NULL, mock_finder->getOrigin());
 }
 
 /*
@@ -636,7 +640,7 @@ TEST_F(QueryTest, LongDNAME) {
         "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa."
         "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa."
         "dname.example.com.");
-    EXPECT_NO_THROW(Query(memory_datasrc, longname, RRType::A(),
+    EXPECT_NO_THROW(Query(memory_client, longname, RRType::A(),
         response).process());
 
     responseCheck(response, Rcode::YXDOMAIN(), AA_FLAG, 1, 0, 0,
@@ -655,7 +659,7 @@ TEST_F(QueryTest, MaxLenDNAME) {
         "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa."
         "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa."
         "dname.example.com.");
-    EXPECT_NO_THROW(Query(memory_datasrc, longname, RRType::A(),
+    EXPECT_NO_THROW(Query(memory_client, longname, RRType::A(),
         response).process());
 
     // Check the answer is OK

src/bin/bind10/tests/Makefile.am

@@ -7,7 +7,7 @@ PYTESTS = bind10_test.py sockcreator_test.py
 # required by loadable python modules.
 LIBRARY_PATH_PLACEHOLDER =
 if SET_ENV_LIBRARY_PATH
-LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$$$(ENV_LIBRARY_PATH)
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$(abs_top_builddir)/src/lib/util/io/.libs:$$$(ENV_LIBRARY_PATH)
 endif
 
 # test using command-line arguments, so use check-local target instead of TESTS

src/bin/bind10/tests/sockcreator_test.py.in

@@ -124,6 +124,11 @@ class FakeCreator:
 class ParserTests(unittest.TestCase):
     """
     Testcases for the Parser class.
+
+    A lot of these test could be done by
+    `with self.assertRaises(CreatorError) as cm`. But some versions of python
+    take the scope wrong and don't work, so we use the primitive way of
+    try-except.
     """
     def __terminate(self):
         creator = FakeCreator([('s', b'T'), ('r', b'')])
@@ -139,6 +144,17 @@ class ParserTests(unittest.TestCase):
         """
         self.__terminate()
 
+    def __terminate_raises(self, parser):
+        """
+        Check that terminate() raises a fatal exception.
+        """
+        try:
+            parser.terminate()
+            self.fail("Not raised")
+        except CreatorError as ce:
+            self.assertTrue(ce.fatal)
+            self.assertEqual(None, ce.errno)
+
     def test_terminate_error1(self):
         """
         Test it reports an exception when there's error terminating the creator.
@@ -146,10 +162,7 @@ class ParserTests(unittest.TestCase):
         """
         creator = FakeCreator([('s', b'T'), ('e', None)])
         parser = Parser(creator)
-        with self.assertRaises(CreatorError) as cm:
-            parser.terminate()
-        self.assertTrue(cm.exception.fatal)
-        self.assertEqual(None, cm.exception.errno)
+        self.__terminate_raises(parser)
 
     def test_terminate_error2(self):
         """
@@ -158,20 +171,7 @@ class ParserTests(unittest.TestCase):
         """
         creator = FakeCreator([('e', None)])
         parser = Parser(creator)
-        with self.assertRaises(CreatorError) as cm:
-            parser.terminate()
-        self.assertTrue(cm.exception.fatal)
-        self.assertEqual(None, cm.exception.errno)
-
-    def test_terminate_twice(self):
-        """
-        Test we can't terminate twice.
-        """
-        parser = self.__terminate()
-        with self.assertRaises(CreatorError) as cm:
-            parser.terminate()
-        self.assertTrue(cm.exception.fatal)
-        self.assertEqual(None, cm.exception.errno)
+        self.__terminate_raises(parser)
 
     def test_terminate_error3(self):
         """
@@ -180,10 +180,14 @@ class ParserTests(unittest.TestCase):
         """
         creator = FakeCreator([('s', b'T'), ('r', b'Extra data')])
         parser = Parser(creator)
-        with self.assertRaises(CreatorError) as cm:
-            parser.terminate()
-        self.assertTrue(cm.exception.fatal)
-        self.assertEqual(None, cm.exception.errno)
+        self.__terminate_raises(parser)
+
+    def test_terminate_twice(self):
+        """
+        Test we can't terminate twice.
+        """
+        parser = self.__terminate()
+        self.__terminate_raises(parser)
 
     def test_crash(self):
         """
@@ -192,12 +196,14 @@ class ParserTests(unittest.TestCase):
         """
         creator = FakeCreator([('s', b'SU4\0\0\0\0\0\0'), ('r', b'')])
         parser = Parser(creator)
-        with self.assertRaises(CreatorError) as cm:
+        try:
             parser.get_socket(IPAddr('0.0.0.0'), 0, 'UDP')
-        self.assertTrue(creator.all_used())
-        # Is the exception correct?
-        self.assertTrue(cm.exception.fatal)
-        self.assertEqual(None, cm.exception.errno)
+            self.fail("Not raised")
+        except CreatorError as ce:
+            self.assertTrue(creator.all_used())
+            # Is the exception correct?
+            self.assertTrue(ce.fatal)
+            self.assertEqual(None, ce.errno)
 
     def test_error(self):
         """
@@ -210,20 +216,24 @@ class ParserTests(unittest.TestCase):
         creator = FakeCreator([('s', b'SU4\0\0\0\0\0\0'), ('r', b'ES' +
             intpart[:1]), ('r', intpart[1:])])
         parser = Parser(creator)
-        with self.assertRaises(CreatorError) as cm:
+        try:
             parser.get_socket(IPAddr('0.0.0.0'), 0, 'UDP')
-        self.assertTrue(creator.all_used())
-        # Is the exception correct?
-        self.assertFalse(cm.exception.fatal)
-        self.assertEqual(42, cm.exception.errno)
+            self.fail("Not raised")
+        except CreatorError as ce:
+            self.assertTrue(creator.all_used())
+            # Is the exception correct?
+            self.assertFalse(ce.fatal)
+            self.assertEqual(42, ce.errno)
 
     def __error(self, plan):
         creator = FakeCreator(plan)
         parser = Parser(creator)
-        with self.assertRaises(CreatorError) as cm:
+        try:
             parser.get_socket(IPAddr('0.0.0.0'), 0, socket.SOCK_DGRAM)
-        self.assertTrue(creator.all_used())
-        self.assertTrue(cm.exception.fatal)
+            self.fail("Not raised")
+        except CreatorError as ce:
+            self.assertTrue(creator.all_used())
+            self.assertTrue(ce.fatal)
 
     def test_error_send(self):
         self.__error([('e', None)])
@@ -251,10 +261,12 @@ class ParserTests(unittest.TestCase):
         Test we can't request sockets after it was terminated.
         """
         parser = self.__terminate()
-        with self.assertRaises(CreatorError) as cm:
+        try:
             parser.get_socket(IPAddr('0.0.0.0'), 0, 'UDP')
-        self.assertTrue(cm.exception.fatal)
-        self.assertEqual(None, cm.exception.errno)
+            self.fail("Not raised")
+        except CreatorError as ce:
+            self.assertTrue(ce.fatal)
+            self.assertEqual(None, ce.errno)
 
     def test_invalid_socktype(self):
         """

src/bin/bindctl/bindcmd.py

@@ -398,6 +398,8 @@ class BindCmdInterpreter(Cmd):
                 print("Error: " + str(dte))
             except isc.cc.data.DataNotFoundError as dnfe:
                 print("Error: " + str(dnfe))
+            except isc.cc.data.DataAlreadyPresentError as dape:
+                print("Error: " + str(dape))
             except KeyError as ke:
                 print("Error: missing " + str(ke))
         else:
@@ -634,7 +636,15 @@ class BindCmdInterpreter(Cmd):
                     # we have more data to show
                     line += "/"
                 else:
-                    line += "\t" + json.dumps(value_map['value'])
+                    # if type is named_set, don't print value if None
+                    # (it is either {} meaning empty, or None, meaning
+                    # there actually is data, but not to be shown with
+                    # the current command
+                    if value_map['type'] == 'named_set' and\
+                       value_map['value'] is None:
+                        line += "/\t"
+                    else:
+                        line += "\t" + json.dumps(value_map['value'])
                 line += "\t" + value_map['type']
                 line += "\t"
                 if value_map['default']:
@@ -649,10 +659,9 @@ class BindCmdInterpreter(Cmd):
                 data, default = self.config_data.get_value(identifier)
                 print(json.dumps(data))
         elif cmd.command == "add":
-            if 'value' in cmd.params:
-                self.config_data.add_value(identifier, cmd.params['value'])
-            else:
-                self.config_data.add_value(identifier)
+            self.config_data.add_value(identifier,
+                                       cmd.params.get('value_or_name'),
+                                       cmd.params.get('value_for_set'))
         elif cmd.command == "remove":
             if 'value' in cmd.params:
                 self.config_data.remove_value(identifier, cmd.params['value'])
@@ -679,7 +688,7 @@ class BindCmdInterpreter(Cmd):
             except isc.config.ModuleCCSessionError as mcse:
                 print(str(mcse))
         elif cmd.command == "diff":
-            print(self.config_data.get_local_changes());
+            print(self.config_data.get_local_changes())
         elif cmd.command == "go":
             self.go(identifier)
 

src/bin/bindctl/bindctl_main.py.in

@@ -50,17 +50,28 @@ def prepare_config_commands(tool):
     cmd.add_param(param)
     module.add_command(cmd)
 
-    cmd = CommandInfo(name = "add", desc = "Add an entry to configuration list. If no value is given, a default value is added.")
+    cmd = CommandInfo(name = "add", desc =
+        "Add an entry to configuration list or a named set. "
+        "When adding to a list, the command has one optional argument, "
+        "a value to add to the list. The value must be in correct JSON "
+        "and complete. When adding to a named set, it has one "
+        "mandatory parameter (the name to add), and an optional "
+        "parameter value, similar to when adding to a list. "
+        "In either case, when no value is given, an entry will be "
+        "constructed with default values.")
     param = ParamInfo(name = "identifier", type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
-    param = ParamInfo(name = "value", type = "string", optional=True, desc = "Specifies a value to add to the list. It must be in correct JSON format and complete.")
+    param = ParamInfo(name = "value_or_name", type = "string", optional=True, desc = "Specifies a value to add to the list, or the name when adding to a named set. It must be in correct JSON format and complete.")
+    cmd.add_param(param)
+    module.add_command(cmd)
+    param = ParamInfo(name = "value_for_set", type = "string", optional=True, desc = "Specifies an optional value to add to the named map. It must be in correct JSON format and complete.")
     cmd.add_param(param)
     module.add_command(cmd)
 
-    cmd = CommandInfo(name = "remove", desc = "Remove entry from configuration list.")
+    cmd = CommandInfo(name = "remove", desc = "Remove entry from configuration list or named set.")
     param = ParamInfo(name = "identifier", type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
-    param = ParamInfo(name = "value", type = "string", optional=True, desc = "Specifies a value to remove from the list. It must be in correct JSON format and complete.")
+    param = ParamInfo(name = "value", type = "string", optional=True, desc = "When identifier is a list, specifies a value to remove from the list. It must be in correct JSON format and complete. When it is a named set, specifies the name to remove.")
     cmd.add_param(param)
     module.add_command(cmd)
 

src/bin/dhcp6/tests/Makefile.am

@@ -8,7 +8,7 @@ EXTRA_DIST = $(PYTESTS)
 # required by loadable python modules.
 LIBRARY_PATH_PLACEHOLDER =
 if SET_ENV_LIBRARY_PATH
-LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$$$(ENV_LIBRARY_PATH)
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$(abs_top_builddir)/src/lib/util/io/.libs:$$$(ENV_LIBRARY_PATH)
 endif
 
 # test using command-line arguments, so use check-local target instead of TESTS

src/bin/zonemgr/zonemgr.py.in

@@ -38,6 +38,10 @@ from optparse import OptionParser, OptionValueError
 from isc.config.ccsession import *
 import isc.util.process
 
+# Initialize logging for called modules.
+# TODO: Log messages properly
+isc.log.init("b10-zonemgr")
+
 isc.util.process.rename()
 
 # If B10_FROM_BUILD is set in the environment, we use data files

src/lib/cc/data.cc

@@ -511,6 +511,8 @@ Element::nameToType(const std::string& type_name) {
         return (Element::list);
     } else if (type_name == "map") {
         return (Element::map);
+    } else if (type_name == "named_set") {
+        return (Element::map);
     } else if (type_name == "null") {
         return (Element::null);
     } else if (type_name == "any") {

src/lib/config/ccsession.cc

@@ -18,12 +18,15 @@
 #include <stdlib.h>
 #include <string.h>
 #include <sys/time.h>
+#include <ctype.h>
 
-#include <iostream>
-#include <fstream>
-#include <sstream>
+#include <algorithm>
 #include <cerrno>
+#include <fstream>
+#include <iostream>
 #include <set>
+#include <sstream>
+#include <string>
 
 #include <boost/bind.hpp>
 #include <boost/foreach.hpp>
@@ -175,6 +178,36 @@ ConstElementPtr getValueOrDefault(ConstElementPtr config_part,
     }
 }
 
+// Prefix name with "b10-".
+//
+// In BIND 10, modules have names taken from the .spec file, which are typically
+// names starting with a capital letter (e.g. "Resolver", "Auth" etc.).  The
+// names of the associated binaries are derived from the module names, being
+// prefixed "b10-" and having the first letter of the module name lower-cased
+// (e.g. "b10-resolver", "b10-auth").  (It is a required convention that there
+// be this relationship between the names.)
+//
+// Within the binaries the root loggers are named after the binaries themselves.
+// (The reason for this is that the name of the logger is included in the
+// message logged, so making it clear which message comes from which BIND 10
+// process.) As logging is configured using module names, the configuration code
+// has to match these with the corresponding logger names. This function
+// converts a module name to a root logger name by lowercasing the first letter
+// of the module name and prepending "b10-".
+//
+// \param instring String to convert.  (This may be empty, in which case
+//        "b10-" will be returned.)
+//
+// \return Converted string.
+std::string
+b10Prefix(const std::string& instring) {
+    std::string result = instring;
+    if (!result.empty()) {
+        result[0] = tolower(result[0]);
+    }
+    return (std::string("b10-") + result);
+}
+
 // Reads a output_option subelement of a logger configuration,
 // and sets the values thereing to the given OutputOption struct,
 // or defaults values if they are not provided (from config_data).
@@ -215,6 +248,7 @@ readLoggersConf(std::vector<isc::log::LoggerSpecification>& specs,
                 ConstElementPtr logger,
                 const ConfigData& config_data)
 {
+    // Read name, adding prefix as required.
     std::string lname = logger->get("name")->stringValue();
 
     ConstElementPtr severity_el = getValueOrDefault(logger,
@@ -247,6 +281,27 @@ readLoggersConf(std::vector<isc::log::LoggerSpecification>& specs,
     specs.push_back(logger_spec);
 }
 
+// Copies the map for a logger, changing the name of the logger in the process.
+// This is used because the map being copied is "const", so in order to
+// change the name we need to create a new one.
+//
+// \param cur_logger Logger being copied.
+// \param new_name New value of the "name" element at the top level.
+//
+// \return Pointer to the map with the updated element.
+ConstElementPtr
+copyLogger(ConstElementPtr& cur_logger, const std::string& new_name) {
+
+    // Since we'll only be updating one first-level element and subsequent
+    // use won't change the contents of the map, a shallow map copy is enough.
+    ElementPtr new_logger(Element::createMap());
+    new_logger->setValue(cur_logger->mapValue());
+    new_logger->set("name", Element::create(new_name));
+
+    return (new_logger);
+}
+
+
 } // end anonymous namespace
 
 
@@ -259,38 +314,60 @@ getRelatedLoggers(ConstElementPtr loggers) {
     ElementPtr result = isc::data::Element::createList();
 
     BOOST_FOREACH(ConstElementPtr cur_logger, loggers->listValue()) {
+        // Need to add the b10- prefix to names ready from the spec file.
         const std::string cur_name = cur_logger->get("name")->stringValue();
-        if (cur_name == root_name || cur_name.find(root_name + ".") == 0) {
-            our_names.insert(cur_name);
-            result->add(cur_logger);
+        const std::string mod_name = b10Prefix(cur_name);
+        if (mod_name == root_name || mod_name.find(root_name + ".") == 0) {
+
+            // Note this name so that we don't add a wildcard that matches it.
+            our_names.insert(mod_name);
+
+            // We want to store the logger with the modified name (i.e. with
+            // the b10- prefix).  As we are dealing with const loggers, we
+            // store a modified copy of the data.
+            result->add(copyLogger(cur_logger, mod_name));
+            LOG_DEBUG(config_logger, DBG_CONFIG_PROCESS, CONFIG_LOG_EXPLICIT)
+                      .arg(cur_name);
+
+        } else if (!cur_name.empty() && (cur_name[0] != '*')) {
+            // Not a wildcard logger and we are ignoring it.
+            LOG_DEBUG(config_logger, DBG_CONFIG_PROCESS,
+                      CONFIG_LOG_IGNORE_EXPLICIT).arg(cur_name);
         }
     }
 
-    // now find the * names
+    // Now find the wildcard names (the one that start with "*").
     BOOST_FOREACH(ConstElementPtr cur_logger, loggers->listValue()) {
         std::string cur_name = cur_logger->get("name")->stringValue();
-        // if name is '*', or starts with '*.', replace * with root
-        // logger name
+        // If name is '*', or starts with '*.', replace * with root
+        // logger name.
         if (cur_name == "*" || cur_name.length() > 1 &&
             cur_name[0] == '*' && cur_name[1] == '.') {
 
-            cur_name = root_name + cur_name.substr(1);
-            // now add it to the result list, but only if a logger with
-            // that name was not configured explicitely
-            if (our_names.find(cur_name) == our_names.end()) {
-                // we substitute the name here already, but as
-                // we are dealing with consts, we copy the data
-                ElementPtr new_logger(Element::createMap());
-                // since we'll only be updating one first-level element,
-                // and we return as const again, a shallow map copy is
-                // enough
-                new_logger->setValue(cur_logger->mapValue());
-                new_logger->set("name", Element::create(cur_name));
-                result->add(new_logger);
+            // Substitute the "*" with the root name
+            std::string mod_name = cur_name;
+            mod_name.replace(0, 1, root_name);
+
+            // Now add it to the result list, but only if a logger with
+            // that name was not configured explicitly.
+            if (our_names.find(mod_name) == our_names.end()) {
+
+                // We substitute the name here, but as we are dealing with
+                // consts, we need to copy the data.
+                result->add(copyLogger(cur_logger, mod_name));
+                LOG_DEBUG(config_logger, DBG_CONFIG_PROCESS,
+                          CONFIG_LOG_WILD_MATCH).arg(cur_name);
+
+            } else if (!cur_name.empty() && (cur_name[0] == '*')) {
+                // Is a wildcard and we are ignoring it (because the wildcard
+                // expands to a specification that we already encountered when
+                // processing explicit names).
+                LOG_DEBUG(config_logger, DBG_CONFIG_PROCESS,
+                          CONFIG_LOG_IGNORE_WILD).arg(cur_name);
             }
         }
     }
-    return result;
+    return (result);
 }
 
 void
@@ -318,7 +395,7 @@ ModuleSpec
 ModuleCCSession::readModuleSpecification(const std::string& filename) {
     std::ifstream file;
     ModuleSpec module_spec;
-    
+
     // this file should be declared in a @something@ directive
     file.open(filename.c_str());
     if (!file) {
@@ -385,7 +462,7 @@ ModuleCCSession::ModuleCCSession(
         LOG_ERROR(config_logger, CONFIG_MOD_SPEC_REJECT).arg(answer->str());
         isc_throw(CCSessionInitError, answer->str());
     }
-    
+
     setLocalConfig(Element::fromJSON("{}"));
     // get any stored configuration from the manager
     if (config_handler_) {
@@ -511,7 +588,7 @@ int
 ModuleCCSession::checkCommand() {
     ConstElementPtr cmd, routing, data;
     if (session_.group_recvmsg(routing, data, true)) {
-        
+
         /* ignore result messages (in case we're out of sync, to prevent
          * pingpongs */
         if (data->getType() != Element::map || data->contains("result")) {

src/lib/config/ccsession.h

@@ -377,10 +377,10 @@ default_logconfig_handler(const std::string& module_name,
 /// \brief Returns the loggers related to this module
 ///
 /// This function does two things;
-/// - it drops the configuration parts for loggers for other modules
+/// - it drops the configuration parts for loggers for other modules.
 /// - it replaces the '*' in the name of the loggers by the name of
 ///   this module, but *only* if the expanded name is not configured
-///   explicitely
+///   explicitly.
 ///
 /// Examples: if this is the module b10-resolver,
 /// For the config names ['*', 'b10-auth']

src/lib/config/config_log.h

@@ -32,6 +32,14 @@ namespace config {
 /// space.
 extern isc::log::Logger config_logger;    // isc::config::config_logger is the CONFIG logger
 
+/// \brief Debug Levels
+///
+/// Debug levels used in the configuration library
+enum {
+    DBG_CONFIG_PROCESS = 40     // Enumerate configuration elements as they
+                                // ... are processed.
+};
+
 } // namespace config
 } // namespace isc
 

src/lib/config/config_messages.mes

@@ -37,6 +37,31 @@ manager is appended to the log error. The most likely cause is that
 the module is of a different (command specification) version than the
 running configuration manager.
 
+% CONFIG_LOG_EXPLICIT will use logging configuration for explicitly-named logger %1
+This is a debug message.  When processing the "loggers" part of the
+configuration file, the configuration library found an entry for the named
+logger that matches the logger specification for the program.  The logging
+configuration for the program will be updated with the information.
+
+% CONFIG_LOG_IGNORE_EXPLICIT ignoring logging configuration for explicitly-named logger %1
+This is a debug message.  When processing the "loggers" part of the
+configuration file, the configuration library found an entry for the
+named logger.  As this does not match the logger specification for the
+program, it has been ignored.
+
+% CONFIG_LOG_IGNORE_WILD ignoring logging configuration for wildcard logger %1
+This is a debug message.  When processing the "loggers" part of the
+configuration file, the configuration library found the named wildcard
+entry (one containing the "*" character) that matched a logger already
+matched by an explicitly named entry.  The configuration is ignored.
+
+% CONFIG_LOG_WILD_MATCH will use logging configuration for wildcard logger %1
+This is a debug message.  When processing the "loggers" part of
+the configuration file, the configuration library found the named
+wildcard entry (one containing the "*" character) that matches a logger
+specification in the program. The logging configuration for the program
+will be updated with the information.
+
 % CONFIG_JSON_PARSE JSON parse error in %1: %2
 There was an error parsing the JSON file. The given file does not appear
 to be in valid JSON format. Please verify that the filename is correct

src/lib/config/module_spec.cc

@@ -67,10 +67,13 @@ check_config_item(ConstElementPtr spec) {
         check_leaf_item(spec, "list_item_spec", Element::map, true);
         check_config_item(spec->get("list_item_spec"));
     }
-    // todo: add stuff for type map
-    if (Element::nameToType(spec->get("item_type")->stringValue()) == Element::map) {
+
+    if (spec->get("item_type")->stringValue() == "map") {
         check_leaf_item(spec, "map_item_spec", Element::list, true);
         check_config_item_list(spec->get("map_item_spec"));
+    } else if (spec->get("item_type")->stringValue() == "named_set") {
+        check_leaf_item(spec, "named_set_item_spec", Element::map, true);
+        check_config_item(spec->get("named_set_item_spec"));
     }
 }
 
@@ -286,7 +289,8 @@ check_type(ConstElementPtr spec, ConstElementPtr element) {
             return (cur_item_type == "list");
             break;
         case Element::map:
-            return (cur_item_type == "map");
+            return (cur_item_type == "map" ||
+                    cur_item_type == "named_set");
             break;
     }
     return (false);
@@ -323,8 +327,20 @@ ModuleSpec::validateItem(ConstElementPtr spec, ConstElementPtr data,
         }
     }
     if (data->getType() == Element::map) {
-        if (!validateSpecList(spec->get("map_item_spec"), data, full, errors)) {
-            return (false);
+        // either a normal 'map' or a 'named set' (determined by which
+        // subspecification it has)
+        if (spec->contains("map_item_spec")) {
+            if (!validateSpecList(spec->get("map_item_spec"), data, full, errors)) {
+                return (false);
+            }
+        } else {
+            typedef std::pair<std::string, ConstElementPtr> maptype;
+
+            BOOST_FOREACH(maptype m, data->mapValue()) {
+                if (!validateItem(spec->get("named_set_item_spec"), m.second, full, errors)) {
+                    return (false);
+                }
+            }
         }
     }
     return (true);

src/lib/config/tests/ccsession_unittests.cc

@@ -44,7 +44,9 @@ el(const std::string& str) {
 
 class CCSessionTest : public ::testing::Test {
 protected:
-    CCSessionTest() : session(el("[]"), el("[]"), el("[]")) {
+    CCSessionTest() : session(el("[]"), el("[]"), el("[]")),
+                      root_name(isc::log::getRootLoggerName())
+    {
         // upon creation of a ModuleCCSession, the class
         // sends its specification to the config manager.
         // it expects an ok answer back, so everytime we
@@ -52,8 +54,11 @@ protected:
         // ok answer.
         session.getMessages()->add(createAnswer());
     }
-    ~CCSessionTest() {}
+    ~CCSessionTest() {
+        isc::log::setRootLoggerName(root_name);
+    }
     FakeSession session;
+    const std::string root_name;
 };
 
 TEST_F(CCSessionTest, createAnswer) {
@@ -652,41 +657,44 @@ void doRelatedLoggersTest(const char* input, const char* expected) {
 TEST(LogConfigTest, relatedLoggersTest) {
     // make sure logger configs for 'other' programs are ignored,
     // and that * is substituted correctly
-    // The default root logger name is "bind10"
+    // We'll use a root logger name of "b10-test".
+    isc::log::setRootLoggerName("b10-test");
+
     doRelatedLoggersTest("[{ \"name\": \"other_module\" }]",
                          "[]");
     doRelatedLoggersTest("[{ \"name\": \"other_module.somelib\" }]",
                          "[]");
-    doRelatedLoggersTest("[{ \"name\": \"bind10_other\" }]",
+    doRelatedLoggersTest("[{ \"name\": \"test_other\" }]",
                          "[]");
-    doRelatedLoggersTest("[{ \"name\": \"bind10_other.somelib\" }]",
+    doRelatedLoggersTest("[{ \"name\": \"test_other.somelib\" }]",
                          "[]");
     doRelatedLoggersTest("[ { \"name\": \"other_module\" },"
-                         "  { \"name\": \"bind10\" }]",
-                         "[ { \"name\": \"bind10\" } ]");
-    doRelatedLoggersTest("[ { \"name\": \"bind10\" }]",
-                         "[ { \"name\": \"bind10\" } ]");
-    doRelatedLoggersTest("[ { \"name\": \"bind10.somelib\" }]",
-                         "[ { \"name\": \"bind10.somelib\" } ]");
+                         "  { \"name\": \"test\" }]",
+                         "[ { \"name\": \"b10-test\" } ]");
+    doRelatedLoggersTest("[ { \"name\": \"test\" }]",
+                         "[ { \"name\": \"b10-test\" } ]");
+    doRelatedLoggersTest("[ { \"name\": \"test.somelib\" }]",
+                         "[ { \"name\": \"b10-test.somelib\" } ]");
     doRelatedLoggersTest("[ { \"name\": \"other_module.somelib\" },"
-                         "  { \"name\": \"bind10.somelib\" }]",
-                         "[ { \"name\": \"bind10.somelib\" } ]");
+                         "  { \"name\": \"test.somelib\" }]",
+                         "[ { \"name\": \"b10-test.somelib\" } ]");
     doRelatedLoggersTest("[ { \"name\": \"other_module.somelib\" },"
-                         "  { \"name\": \"bind10\" },"
-                         "  { \"name\": \"bind10.somelib\" }]",
-                         "[ { \"name\": \"bind10\" },"
-                         "  { \"name\": \"bind10.somelib\" } ]");
+                         "  { \"name\": \"test\" },"
+                         "  { \"name\": \"test.somelib\" }]",
+                         "[ { \"name\": \"b10-test\" },"
+                         "  { \"name\": \"b10-test.somelib\" } ]");
     doRelatedLoggersTest("[ { \"name\": \"*\" }]",
-                         "[ { \"name\": \"bind10\" } ]");
+                         "[ { \"name\": \"b10-test\" } ]");
     doRelatedLoggersTest("[ { \"name\": \"*.somelib\" }]",
-                         "[ { \"name\": \"bind10.somelib\" } ]");
+                         "[ { \"name\": \"b10-test.somelib\" } ]");
     doRelatedLoggersTest("[ { \"name\": \"*\", \"severity\": \"DEBUG\" },"
-                         "  { \"name\": \"bind10\", \"severity\": \"WARN\"}]",
-                         "[ { \"name\": \"bind10\", \"severity\": \"WARN\"} ]");
+                         "  { \"name\": \"test\", \"severity\": \"WARN\"}]",
+                         "[ { \"name\": \"b10-test\", \"severity\": \"WARN\"} ]");
     doRelatedLoggersTest("[ { \"name\": \"*\", \"severity\": \"DEBUG\" },"
                          "  { \"name\": \"some_module\", \"severity\": \"WARN\"}]",
-                         "[ { \"name\": \"bind10\", \"severity\": \"DEBUG\"} ]");
-
+                         "[ { \"name\": \"b10-test\", \"severity\": \"DEBUG\"} ]");
+    doRelatedLoggersTest("[ { \"name\": \"b10-test\" }]",
+                         "[]");
     // make sure 'bad' things like '*foo.x' or '*lib' are ignored
     // (cfgmgr should have already caught it in the logconfig plugin
     // check, and is responsible for reporting the error)
@@ -696,8 +704,8 @@ TEST(LogConfigTest, relatedLoggersTest) {
                          "[ ]");
     doRelatedLoggersTest("[ { \"name\": \"*foo\" },"
                          "  { \"name\": \"*foo.lib\" },"
-                         "  { \"name\": \"bind10\" } ]",
-                         "[ { \"name\": \"bind10\" } ]");
+                         "  { \"name\": \"test\" } ]",
+                         "[ { \"name\": \"b10-test\" } ]");
 }
 
 }

src/lib/config/tests/module_spec_unittests.cc

@@ -211,3 +211,12 @@ TEST(ModuleSpec, CommandValidation) {
     EXPECT_EQ(errors->get(0)->stringValue(), "Type mismatch");
 
 }
+
+TEST(ModuleSpec, NamedSetValidation) {
+    ModuleSpec dd = moduleSpecFromFile(specfile("spec32.spec"));
+
+    ElementPtr errors = Element::createList();
+    EXPECT_TRUE(dataTestWithErrors(dd, "data32_1.data", errors));
+    EXPECT_FALSE(dataTest(dd, "data32_2.data"));
+    EXPECT_FALSE(dataTest(dd, "data32_3.data"));
+}

src/lib/config/tests/testdata/Makefile.am

@@ -22,6 +22,9 @@ EXTRA_DIST += data22_7.data
 EXTRA_DIST += data22_8.data
 EXTRA_DIST += data22_9.data
 EXTRA_DIST += data22_10.data
+EXTRA_DIST += data32_1.data
+EXTRA_DIST += data32_2.data
+EXTRA_DIST += data32_3.data
 EXTRA_DIST += spec1.spec
 EXTRA_DIST += spec2.spec
 EXTRA_DIST += spec3.spec
@@ -53,3 +56,4 @@ EXTRA_DIST += spec28.spec
 EXTRA_DIST += spec29.spec
 EXTRA_DIST += spec30.spec
 EXTRA_DIST += spec31.spec
+EXTRA_DIST += spec32.spec

src/lib/config/tests/testdata/data32_1.data

@@ -0,0 +1,3 @@
+{
+    "named_set_item": { "foo": 1, "bar": 2 }
+}

src/lib/config/tests/testdata/data32_2.data

@@ -0,0 +1,3 @@
+{
+    "named_set_item": { "foo": "wrongtype", "bar": 2 }
+}

src/lib/config/tests/testdata/data32_3.data

@@ -0,0 +1,3 @@
+{
+    "named_set_item": []
+}

src/lib/config/tests/testdata/spec32.spec

@@ -0,0 +1,19 @@
+{
+  "module_spec": {
+    "module_name": "Spec32",
+    "config_data": [
+      { "item_name": "named_set_item",
+        "item_type": "named_set",
+        "item_optional": false,
+        "item_default": { "a": 1, "b": 2 },
+        "named_set_item_spec": {
+          "item_name": "named_set_element",
+          "item_type": "integer",
+          "item_optional": false,
+          "item_default": 3
+        }
+      }
+    ]
+  }
+}
+

src/lib/datasrc/Makefile.am

@@ -21,6 +21,7 @@ libdatasrc_la_SOURCES += memory_datasrc.h memory_datasrc.cc
 libdatasrc_la_SOURCES += zone.h
 libdatasrc_la_SOURCES += result.h
 libdatasrc_la_SOURCES += logger.h logger.cc
+libdatasrc_la_SOURCES += client.h
 nodist_libdatasrc_la_SOURCES = datasrc_messages.h datasrc_messages.cc
 
 libdatasrc_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la

src/lib/datasrc/client.h

@@ -0,0 +1,150 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __DATA_SOURCE_CLIENT_H
+#define __DATA_SOURCE_CLIENT_H 1
+
+#include <datasrc/zone.h>
+
+namespace isc {
+namespace datasrc {
+
+/// \brief The base class of data source clients.
+///
+/// This is an abstract base class that defines the common interface for
+/// various types of data source clients.  A data source client is a top level
+/// access point to a data source, allowing various operations on the data
+/// source such as lookups, traversing or updates.  The client class itself
+/// has limited focus and delegates the responsibility for these specific
+/// operations to other classes; in general methods of this class act as
+/// factories of these other classes.
+///
+/// The following derived classes are currently (expected to be) provided:
+/// - \c InMemoryClient: A client of a conceptual data source that stores
+/// all necessary data in memory for faster lookups
+/// - \c DatabaseClient: A client that uses a real database backend (such as
+/// an SQL database).  It would internally hold a connection to the underlying
+/// database system.
+///
+/// \note It is intentional that while the term these derived classes don't
+/// contain "DataSource" unlike their base class.  It's also noteworthy
+/// that the naming of the base class is somewhat redundant because the
+/// namespace \c datasrc would indicate that it's related to a data source.
+/// The redundant naming comes from the observation that namespaces are
+/// often omitted with \c using directives, in which case "Client"
+/// would be too generic.  On the other hand, concrete derived classes are
+/// generally not expected to be referenced directly from other modules and
+/// applications, so we'll give them more concise names such as InMemoryClient.
+///
+/// A single \c DataSourceClient object is expected to handle only a single
+/// RR class even if the underlying data source contains records for multiple
+/// RR classes.  Likewise, (when we support views) a \c DataSourceClient
+/// object is expected to handle only a single view.
+///
+/// If the application uses multiple threads, each thread will need to
+/// create and use a separate DataSourceClient.  This is because some
+/// database backend doesn't allow multiple threads to share the same
+/// connection to the database.
+///
+/// \note For a client using an in memory backend, this may result in
+/// having a multiple copies of the same data in memory, increasing the
+/// memory footprint substantially.  Depending on how to support multiple
+/// CPU cores for concurrent lookups on the same single data source (which
+/// is not fully fixed yet, and for which multiple threads may be used),
+/// this design may have to be revisited.
+///
+/// This class (and therefore its derived classes) are not copyable.
+/// This is because the derived classes would generally contain attributes
+/// that are not easy to copy (such as a large size of in memory data or a
+/// network connection to a database server).  In order to avoid a surprising
+/// disruption with a naive copy it's prohibited explicitly.  For the expected
+/// usage of the client classes the restriction should be acceptable.
+///
+/// \todo This class is not complete. It needs more factory methods, for
+///     accessing the whole zone, updating it, loading it, etc.
+class DataSourceClient : boost::noncopyable {
+public:
+    /// \brief A helper structure to represent the search result of
+    /// \c find().
+    ///
+    /// This is a straightforward pair of the result code and a share pointer
+    /// to the found zone to represent the result of \c find().
+    /// We use this in order to avoid overloading the return value for both
+    /// the result code ("success" or "not found") and the found object,
+    /// i.e., avoid using \c NULL to mean "not found", etc.
+    ///
+    /// This is a simple value class with no internal state, so for
+    /// convenience we allow the applications to refer to the members
+    /// directly.
+    ///
+    /// See the description of \c find() for the semantics of the member
+    /// variables.
+    struct FindResult {
+        FindResult(result::Result param_code,
+                   const ZoneFinderPtr param_zone_finder) :
+            code(param_code), zone_finder(param_zone_finder)
+        {}
+        const result::Result code;
+        const ZoneFinderPtr zone_finder;
+    };
+
+    ///
+    /// \name Constructors and Destructor.
+    ///
+protected:
+    /// Default constructor.
+    ///
+    /// This is intentionally defined as protected as this base class
+    /// should never be instantiated directly.
+    ///
+    /// The constructor of a concrete derived class may throw an exception.
+    /// This interface does not specify which exceptions can happen (at least
+    /// at this moment), and the caller should expect any type of exception
+    /// and react accordingly.
+    DataSourceClient() {}
+
+public:
+    /// The destructor.
+    virtual ~DataSourceClient() {}
+    //@}
+
+    /// Returns a \c ZoneFinder for a zone that best matches the given name.
+    ///
+    /// A concrete derived version of this method gets access to its backend
+    /// data source to search for a zone whose origin gives the longest match
+    /// against \c name.  It returns the search result in the form of a
+    /// \c FindResult object as follows:
+    /// - \c code: The result code of the operation.
+    ///   - \c result::SUCCESS: A zone that gives an exact match is found
+    ///   - \c result::PARTIALMATCH: A zone whose origin is a
+    ///   super domain of \c name is found (but there is no exact match)
+    ///   - \c result::NOTFOUND: For all other cases.
+    /// - \c zone_finder: Pointer to a \c ZoneFinder object for the found zone
+    /// if one is found; otherwise \c NULL.
+    ///
+    /// A specific derived version of this method may throw an exception.
+    /// This interface does not specify which exceptions can happen (at least
+    /// at this moment), and the caller should expect any type of exception
+    /// and react accordingly.
+    ///
+    /// \param name A domain name for which the search is performed.
+    /// \return A \c FindResult object enclosing the search result (see above).
+    virtual FindResult findZone(const isc::dns::Name& name) const = 0;
+};
+}
+}
+#endif  // DATA_SOURCE_CLIENT_H
+// Local Variables:
+// mode: c++
+// End:

src/lib/datasrc/memory_datasrc.cc

@@ -32,10 +32,10 @@ using namespace isc::dns;
 namespace isc {
 namespace datasrc {
 
-// Private data and hidden methods of MemoryZone
-struct MemoryZone::MemoryZoneImpl {
+// Private data and hidden methods of InMemoryZoneFinder
+struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
     // Constructor
-    MemoryZoneImpl(const RRClass& zone_class, const Name& origin) :
+    InMemoryZoneFinderImpl(const RRClass& zone_class, const Name& origin) :
         zone_class_(zone_class), origin_(origin), origin_data_(NULL),
         domains_(true)
     {
@@ -223,7 +223,7 @@ struct MemoryZone::MemoryZoneImpl {
      * Implementation of longer methods. We put them here, because the
      * access is without the impl_-> and it will get inlined anyway.
      */
-    // Implementation of MemoryZone::add
+    // Implementation of InMemoryZoneFinder::add
     result::Result add(const ConstRRsetPtr& rrset, DomainTree* domains) {
         // Sanitize input.  This will cause an exception to be thrown
         // if the input RRset is empty.
@@ -409,7 +409,7 @@ struct MemoryZone::MemoryZoneImpl {
         }
     }
 
-    // Implementation of MemoryZone::find
+    // Implementation of InMemoryZoneFinder::find
     FindResult find(const Name& name, RRType type,
                     RRsetList* target, const FindOptions options) const
     {
@@ -593,50 +593,50 @@ struct MemoryZone::MemoryZoneImpl {
     }
 };
 
-MemoryZone::MemoryZone(const RRClass& zone_class, const Name& origin) :
-    impl_(new MemoryZoneImpl(zone_class, origin))
+InMemoryZoneFinder::InMemoryZoneFinder(const RRClass& zone_class, const Name& origin) :
+    impl_(new InMemoryZoneFinderImpl(zone_class, origin))
 {
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_CREATE).arg(origin).
         arg(zone_class);
 }
 
-MemoryZone::~MemoryZone() {
+InMemoryZoneFinder::~InMemoryZoneFinder() {
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_DESTROY).arg(getOrigin()).
         arg(getClass());
     delete impl_;
 }
 
 const Name&
-MemoryZone::getOrigin() const {
+InMemoryZoneFinder::getOrigin() const {
     return (impl_->origin_);
 }
 
 const RRClass&
-MemoryZone::getClass() const {
+InMemoryZoneFinder::getClass() const {
     return (impl_->zone_class_);
 }
 
-Zone::FindResult
-MemoryZone::find(const Name& name, const RRType& type,
+ZoneFinder::FindResult
+InMemoryZoneFinder::find(const Name& name, const RRType& type,
                  RRsetList* target, const FindOptions options) const
 {
     return (impl_->find(name, type, target, options));
 }
 
 result::Result
-MemoryZone::add(const ConstRRsetPtr& rrset) {
+InMemoryZoneFinder::add(const ConstRRsetPtr& rrset) {
     return (impl_->add(rrset, &impl_->domains_));
 }
 
 
 void
-MemoryZone::load(const string& filename) {
+InMemoryZoneFinder::load(const string& filename) {
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_LOAD).arg(getOrigin()).
         arg(filename);
     // Load it into a temporary tree
-    MemoryZoneImpl::DomainTree tmp;
+    InMemoryZoneFinderImpl::DomainTree tmp;
     masterLoad(filename.c_str(), getOrigin(), getClass(),
-        boost::bind(&MemoryZoneImpl::addFromLoad, impl_, _1, &tmp));
+        boost::bind(&InMemoryZoneFinderImpl::addFromLoad, impl_, _1, &tmp));
     // If it went well, put it inside
     impl_->file_name_ = filename;
     tmp.swap(impl_->domains_);
@@ -644,61 +644,61 @@ MemoryZone::load(const string& filename) {
 }
 
 void
-MemoryZone::swap(MemoryZone& zone) {
+InMemoryZoneFinder::swap(InMemoryZoneFinder& zone_finder) {
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_SWAP).arg(getOrigin()).
-        arg(zone.getOrigin());
-    std::swap(impl_, zone.impl_);
+        arg(zone_finder.getOrigin());
+    std::swap(impl_, zone_finder.impl_);
 }
 
 const string
-MemoryZone::getFileName() const {
+InMemoryZoneFinder::getFileName() const {
     return (impl_->file_name_);
 }
 
-/// Implementation details for \c MemoryDataSrc hidden from the public
+/// Implementation details for \c InMemoryClient hidden from the public
 /// interface.
 ///
-/// For now, \c MemoryDataSrc only contains a \c ZoneTable object, which
-/// consists of (pointers to) \c MemoryZone objects, we may add more
+/// For now, \c InMemoryClient only contains a \c ZoneTable object, which
+/// consists of (pointers to) \c InMemoryZoneFinder objects, we may add more
 /// member variables later for new features.
-class MemoryDataSrc::MemoryDataSrcImpl {
+class InMemoryClient::InMemoryClientImpl {
 public:
-    MemoryDataSrcImpl() : zone_count(0) {}
+    InMemoryClientImpl() : zone_count(0) {}
     unsigned int zone_count;
     ZoneTable zone_table;
 };
 
-MemoryDataSrc::MemoryDataSrc() : impl_(new MemoryDataSrcImpl)
+InMemoryClient::InMemoryClient() : impl_(new InMemoryClientImpl)
 {}
 
-MemoryDataSrc::~MemoryDataSrc() {
+InMemoryClient::~InMemoryClient() {
     delete impl_;
 }
 
 unsigned int
-MemoryDataSrc::getZoneCount() const {
+InMemoryClient::getZoneCount() const {
     return (impl_->zone_count);
 }
 
 result::Result
-MemoryDataSrc::addZone(ZonePtr zone) {
-    if (!zone) {
+InMemoryClient::addZone(ZoneFinderPtr zone_finder) {
+    if (!zone_finder) {
         isc_throw(InvalidParameter,
-                  "Null pointer is passed to MemoryDataSrc::addZone()");
+                  "Null pointer is passed to InMemoryClient::addZone()");
     }
 
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_ADD_ZONE).
-        arg(zone->getOrigin()).arg(zone->getClass().toText());
+        arg(zone_finder->getOrigin()).arg(zone_finder->getClass().toText());
 
-    const result::Result result = impl_->zone_table.addZone(zone);
+    const result::Result result = impl_->zone_table.addZone(zone_finder);
     if (result == result::SUCCESS) {
         ++impl_->zone_count;
     }
     return (result);
 }
 
-MemoryDataSrc::FindResult
-MemoryDataSrc::findZone(const isc::dns::Name& name) const {
+InMemoryClient::FindResult
+InMemoryClient::findZone(const isc::dns::Name& name) const {
     LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_FIND_ZONE).arg(name);
     return (FindResult(impl_->zone_table.findZone(name).code,
                        impl_->zone_table.findZone(name).zone));

src/lib/datasrc/memory_datasrc.h

@@ -17,7 +17,10 @@
 
 #include <string>
 
+#include <boost/noncopyable.hpp>
+
 #include <datasrc/zonetable.h>
+#include <datasrc/client.h>
 
 namespace isc {
 namespace dns {
@@ -27,18 +30,17 @@ class RRsetList;
 
 namespace datasrc {
 
-/// A derived zone class intended to be used with the memory data source.
-class MemoryZone : public Zone {
+/// A derived zone finder class intended to be used with the memory data source.
+///
+/// Conceptually this "finder" maintains a local in-memory copy of all RRs
+/// of a single zone from some kind of source (right now it's a textual
+/// master file, but it could also be another data source with a database
+/// backend).  This is why the class has methods like \c load() or \c add().
+///
+/// This class is non copyable.
+class InMemoryZoneFinder : boost::noncopyable, public ZoneFinder {
     ///
     /// \name Constructors and Destructor.
-    ///
-    /// \b Note:
-    /// The copy constructor and the assignment operator are intentionally
-    /// defined as private, making this class non copyable.
-    //@{
-private:
-    MemoryZone(const MemoryZone& source);
-    MemoryZone& operator=(const MemoryZone& source);
 public:
     /// \brief Constructor from zone parameters.
     ///
@@ -48,10 +50,11 @@ public:
     ///
     /// \param rrclass The RR class of the zone.
     /// \param origin The origin name of the zone.
-    MemoryZone(const isc::dns::RRClass& rrclass, const isc::dns::Name& origin);
+    InMemoryZoneFinder(const isc::dns::RRClass& rrclass,
+                       const isc::dns::Name& origin);
 
     /// The destructor.
-    virtual ~MemoryZone();
+    virtual ~InMemoryZoneFinder();
     //@}
 
     /// \brief Returns the origin of the zone.
@@ -128,14 +131,14 @@ public:
     /// Return the master file name of the zone
     ///
     /// This method returns the name of the zone's master file to be loaded.
-    /// The returned string will be an empty unless the zone has successfully
-    /// loaded a zone.
+    /// The returned string will be an empty unless the zone finder has
+    /// successfully loaded a zone.
     ///
     /// This method should normally not throw an exception.  But the creation
     /// of the return string may involve a resource allocation, and if it
     /// fails, the corresponding standard exception will be thrown.
     ///
-    /// \return The name of the zone file loaded in the zone, or an empty
+    /// \return The name of the zone file loaded in the zone finder, or an empty
     /// string if the zone hasn't loaded any file.
     const std::string getFileName() const;
 
@@ -164,143 +167,102 @@ public:
     ///     configuration reloading is written.
     void load(const std::string& filename);
 
-    /// Exchanges the content of \c this zone with that of the given \c zone.
+    /// Exchanges the content of \c this zone finder with that of the given
+    /// \c zone_finder.
     ///
     /// This method never throws an exception.
     ///
-    /// \param zone Another \c MemoryZone object which is to be swapped with
-    /// \c this zone.
-    void swap(MemoryZone& zone);
+    /// \param zone_finder Another \c InMemoryZone object which is to
+    /// be swapped with \c this zone finder.
+    void swap(InMemoryZoneFinder& zone_finder);
 
 private:
     /// \name Hidden private data
     //@{
-    struct MemoryZoneImpl;
-    MemoryZoneImpl* impl_;
+    struct InMemoryZoneFinderImpl;
+    InMemoryZoneFinderImpl* impl_;
     //@}
 };
 
-/// \brief A data source that uses in memory dedicated backend.
+/// \brief A data source client that holds all necessary data in memory.
 ///
-/// The \c MemoryDataSrc class represents a data source and provides a
-/// basic interface to help DNS lookup processing. For a given domain
-/// name, its \c findZone() method searches the in memory dedicated backend
-/// for the zone that gives a longest match against that name.
+/// The \c InMemoryClient class provides an access to a conceptual data
+/// source that maintains all necessary data in a memory image, thereby
+/// allowing much faster lookups.  The in memory data is a copy of some
+/// real physical source - in the current implementation a list of zones
+/// are populated as a result of \c addZone() calls; zone data is given
+/// in a standard master file (but there's a plan to use database backends
+/// as a source of the in memory data).
 ///
-/// The in memory dedicated backend are assumed to be of the same RR class,
-/// but the \c MemoryDataSrc class does not enforce the assumption through
+/// Although every data source client is assumed to be of the same RR class,
+/// the \c InMemoryClient class does not enforce the assumption through
 /// its interface.
 /// For example, the \c addZone() method does not check if the new zone is of
-/// the same RR class as that of the others already in the dedicated backend.
+/// the same RR class as that of the others already in memory.
 /// It is caller's responsibility to ensure this assumption.
 ///
 /// <b>Notes to developer:</b>
 ///
-/// For now, we don't make it a derived class of AbstractDataSrc because the
-/// interface is so different (we'll eventually consider this as part of the
-/// generalization work).
-///
 /// The addZone() method takes a (Boost) shared pointer because it would be
 /// inconvenient to require the caller to maintain the ownership of zones,
 /// while it wouldn't be safe to delete unnecessary zones inside the dedicated
 /// backend.
 ///
-/// The findZone() method takes a domain name and returns the best matching \c
-/// MemoryZone in the form of (Boost) shared pointer, so that it can provide
-/// the general interface for all data sources.
-class MemoryDataSrc {
+/// The findZone() method takes a domain name and returns the best matching 
+/// \c InMemoryZoneFinder in the form of (Boost) shared pointer, so that it can
+/// provide the general interface for all data sources.
+class InMemoryClient : public DataSourceClient {
 public:
-    /// \brief A helper structure to represent the search result of
-    /// <code>MemoryDataSrc::find()</code>.
-    ///
-    /// This is a straightforward pair of the result code and a share pointer
-    /// to the found zone to represent the result of \c find().
-    /// We use this in order to avoid overloading the return value for both
-    /// the result code ("success" or "not found") and the found object,
-    /// i.e., avoid using \c NULL to mean "not found", etc.
-    ///
-    /// This is a simple value class with no internal state, so for
-    /// convenience we allow the applications to refer to the members
-    /// directly.
-    ///
-    /// See the description of \c find() for the semantics of the member
-    /// variables.
-    struct FindResult {
-        FindResult(result::Result param_code, const ZonePtr param_zone) :
-            code(param_code), zone(param_zone)
-        {}
-        const result::Result code;
-        const ZonePtr zone;
-    };
-
     ///
     /// \name Constructors and Destructor.
     ///
-    /// \b Note:
-    /// The copy constructor and the assignment operator are intentionally
-    /// defined as private, making this class non copyable.
     //@{
-private:
-    MemoryDataSrc(const MemoryDataSrc& source);
-    MemoryDataSrc& operator=(const MemoryDataSrc& source);
 
-public:
     /// Default constructor.
     ///
     /// This constructor internally involves resource allocation, and if
     /// it fails, a corresponding standard exception will be thrown.
     /// It never throws an exception otherwise.
-    MemoryDataSrc();
+    InMemoryClient();
 
     /// The destructor.
-    ~MemoryDataSrc();
+    ~InMemoryClient();
     //@}
 
-    /// Return the number of zones stored in the data source.
+    /// Return the number of zones stored in the client.
     ///
     /// This method never throws an exception.
     ///
-    /// \return The number of zones stored in the data source.
+    /// \return The number of zones stored in the client.
     unsigned int getZoneCount() const;
 
-    /// Add a \c Zone to the \c MemoryDataSrc.
+    /// Add a zone (in the form of \c ZoneFinder) to the \c InMemoryClient.
     ///
-    /// \c Zone must not be associated with a NULL pointer; otherwise
+    /// \c zone_finder must not be associated with a NULL pointer; otherwise
     /// an exception of class \c InvalidParameter will be thrown.
     /// If internal resource allocation fails, a corresponding standard
     /// exception will be thrown.
     /// This method never throws an exception otherwise.
     ///
-    /// \param zone A \c Zone object to be added.
-    /// \return \c result::SUCCESS If the zone is successfully
-    /// added to the memory data source.
+    /// \param zone_finder A \c ZoneFinder object to be added.
+    /// \return \c result::SUCCESS If the zone_finder is successfully
+    /// added to the client.
     /// \return \c result::EXIST The memory data source already
     /// stores a zone that has the same origin.
-    result::Result addZone(ZonePtr zone);
+    result::Result addZone(ZoneFinderPtr zone_finder);
 
-    /// Find a \c Zone that best matches the given name in the \c MemoryDataSrc.
-    ///
-    /// It searches the internal storage for a \c Zone that gives the
-    /// longest match against \c name, and returns the result in the
-    /// form of a \c FindResult object as follows:
-    /// - \c code: The result code of the operation.
-    ///   - \c result::SUCCESS: A zone that gives an exact match
-    //    is found
-    ///   - \c result::PARTIALMATCH: A zone whose origin is a
-    //    super domain of \c name is found (but there is no exact match)
-    ///   - \c result::NOTFOUND: For all other cases.
-    /// - \c zone: A "Boost" shared pointer to the found \c Zone object if one
-    //  is found; otherwise \c NULL.
-    ///
-    /// This method never throws an exception.
+    /// Returns a \c ZoneFinder for a zone_finder that best matches the given
+    /// name.
     ///
-    /// \param name A domain name for which the search is performed.
-    /// \return A \c FindResult object enclosing the search result (see above).
-    FindResult findZone(const isc::dns::Name& name) const;
+    /// This derived version of the method never throws an exception.
+    /// For other details see \c DataSourceClient::findZone().
+    virtual FindResult findZone(const isc::dns::Name& name) const;
 
 private:
-    class MemoryDataSrcImpl;
-    MemoryDataSrcImpl* impl_;
+    // TODO: Do we still need the PImpl if nobody should manipulate this class
+    // directly any more (it should be handled through DataSourceClient)?
+    class InMemoryClientImpl;
+    InMemoryClientImpl* impl_;
 };
 }
 }

src/lib/datasrc/rbtree.h

@@ -704,9 +704,9 @@ public:
     /// \brief Find with callback and node chain.
     ///
     /// This version of \c find() is specifically designed for the backend
-    /// of the \c MemoryZone class, and implements all necessary features
-    /// for that purpose.  Other applications shouldn't need these additional
-    /// features, and should normally use the simpler versions.
+    /// of the \c InMemoryZoneFinder class, and implements all necessary
+    /// features for that purpose.  Other applications shouldn't need these
+    /// additional features, and should normally use the simpler versions.
     ///
     /// This version of \c find() calls the callback whenever traversing (on
     /// the way from root down the tree) a marked node on the way down through

src/lib/datasrc/tests/zonetable_unittest.cc

@@ -18,7 +18,7 @@
 #include <dns/rrclass.h>
 
 #include <datasrc/zonetable.h>
-// We use MemoryZone to put something into the table
+// We use InMemoryZone to put something into the table
 #include <datasrc/memory_datasrc.h>
 
 #include <gtest/gtest.h>
@@ -28,31 +28,32 @@ using namespace isc::datasrc;
 
 namespace {
 TEST(ZoneTest, init) {
-    MemoryZone zone(RRClass::IN(), Name("example.com"));
+    InMemoryZoneFinder zone(RRClass::IN(), Name("example.com"));
     EXPECT_EQ(Name("example.com"), zone.getOrigin());
     EXPECT_EQ(RRClass::IN(), zone.getClass());
 
-    MemoryZone ch_zone(RRClass::CH(), Name("example"));
+    InMemoryZoneFinder ch_zone(RRClass::CH(), Name("example"));
     EXPECT_EQ(Name("example"), ch_zone.getOrigin());
     EXPECT_EQ(RRClass::CH(), ch_zone.getClass());
 }
 
 TEST(ZoneTest, find) {
-    MemoryZone zone(RRClass::IN(), Name("example.com"));
-    EXPECT_EQ(Zone::NXDOMAIN,
+    InMemoryZoneFinder zone(RRClass::IN(), Name("example.com"));
+    EXPECT_EQ(ZoneFinder::NXDOMAIN,
               zone.find(Name("www.example.com"), RRType::A()).code);
 }
 
 class ZoneTableTest : public ::testing::Test {
 protected:
-    ZoneTableTest() : zone1(new MemoryZone(RRClass::IN(),
-                                           Name("example.com"))),
-                      zone2(new MemoryZone(RRClass::IN(),
-                                           Name("example.net"))),
-                      zone3(new MemoryZone(RRClass::IN(), Name("example")))
+    ZoneTableTest() : zone1(new InMemoryZoneFinder(RRClass::IN(),
+                                                   Name("example.com"))),
+                      zone2(new InMemoryZoneFinder(RRClass::IN(),
+                                                   Name("example.net"))),
+                      zone3(new InMemoryZoneFinder(RRClass::IN(),
+                                                   Name("example")))
     {}
     ZoneTable zone_table;
-    ZonePtr zone1, zone2, zone3;
+    ZoneFinderPtr zone1, zone2, zone3;
 };
 
 TEST_F(ZoneTableTest, addZone) {
@@ -60,7 +61,8 @@ TEST_F(ZoneTableTest, addZone) {
     EXPECT_EQ(result::EXIST, zone_table.addZone(zone1));
     // names are compared in a case insensitive manner.
     EXPECT_EQ(result::EXIST, zone_table.addZone(
-                  ZonePtr(new MemoryZone(RRClass::IN(), Name("EXAMPLE.COM")))));
+                  ZoneFinderPtr(new InMemoryZoneFinder(RRClass::IN(),
+                                                       Name("EXAMPLE.COM")))));
 
     EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone2));
     EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone3));
@@ -68,11 +70,11 @@ TEST_F(ZoneTableTest, addZone) {
     // Zone table is indexed only by name.  Duplicate origin name with
     // different zone class isn't allowed.
     EXPECT_EQ(result::EXIST, zone_table.addZone(
-                  ZonePtr(new MemoryZone(RRClass::CH(),
-                                         Name("example.com")))));
+                  ZoneFinderPtr(new InMemoryZoneFinder(RRClass::CH(),
+                                                       Name("example.com")))));
 
     /// Bogus zone (NULL)
-    EXPECT_THROW(zone_table.addZone(ZonePtr()), isc::InvalidParameter);
+    EXPECT_THROW(zone_table.addZone(ZoneFinderPtr()), isc::InvalidParameter);
 }
 
 TEST_F(ZoneTableTest, DISABLED_removeZone) {
@@ -95,7 +97,7 @@ TEST_F(ZoneTableTest, findZone) {
 
     EXPECT_EQ(result::NOTFOUND,
               zone_table.findZone(Name("example.org")).code);
-    EXPECT_EQ(ConstZonePtr(),
+    EXPECT_EQ(ConstZoneFinderPtr(),
               zone_table.findZone(Name("example.org")).zone);
 
     // there's no exact match.  the result should be the longest match,
@@ -107,7 +109,7 @@ TEST_F(ZoneTableTest, findZone) {
 
     // make sure the partial match is indeed the longest match by adding
     // a zone with a shorter origin and query again.
-    ZonePtr zone_com(new MemoryZone(RRClass::IN(), Name("com")));
+    ZoneFinderPtr zone_com(new InMemoryZoneFinder(RRClass::IN(), Name("com")));
     EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone_com));
     EXPECT_EQ(Name("example.com"),
               zone_table.findZone(Name("www.example.com")).zone->getOrigin());

src/lib/datasrc/zone.h

@@ -27,7 +27,7 @@ namespace datasrc {
 /// a DNS zone as part of data source.
 ///
 /// At the moment this is provided mainly for making the \c ZoneTable class
-/// and the authoritative query logic  testable, and only provides a minimal
+/// and the authoritative query logic testable, and only provides a minimal
 /// set of features.
 /// This is why this class is defined in the same header file, but it may
 /// have to move to a separate header file when we understand what is
@@ -53,9 +53,9 @@ namespace datasrc {
 ///
 /// <b>Note:</b> Unlike some other abstract base classes we don't name the
 /// class beginning with "Abstract".  This is because we want to have
-/// commonly used definitions such as \c Result and \c ZonePtr, and we want
-/// to make them look more intuitive.
-class Zone {
+/// commonly used definitions such as \c Result and \c ZoneFinderPtr, and we
+/// want to make them look more intuitive.
+class ZoneFinder {
 public:
     /// Result codes of the \c find() method.
     ///
@@ -119,10 +119,10 @@ protected:
     ///
     /// This is intentionally defined as \c protected as this base class should
     /// never be instantiated (except as part of a derived class).
-    Zone() {}
+    ZoneFinder() {}
 public:
     /// The destructor.
-    virtual ~Zone() {}
+    virtual ~ZoneFinder() {}
     //@}
 
     ///
@@ -201,11 +201,11 @@ public:
     //@}
 };
 
-/// \brief A pointer-like type pointing to a \c Zone object.
-typedef boost::shared_ptr<Zone> ZonePtr;
+/// \brief A pointer-like type pointing to a \c ZoneFinder object.
+typedef boost::shared_ptr<ZoneFinder> ZoneFinderPtr;
 
-/// \brief A pointer-like type pointing to a \c Zone object.
-typedef boost::shared_ptr<const Zone> ConstZonePtr;
+/// \brief A pointer-like type pointing to a \c ZoneFinder object.
+typedef boost::shared_ptr<const ZoneFinder> ConstZoneFinderPtr;
 
 }
 }

src/lib/datasrc/zonetable.cc

@@ -28,8 +28,8 @@ namespace datasrc {
 /// \short Private data and implementation of ZoneTable
 struct ZoneTable::ZoneTableImpl {
     // Type aliases to make it shorter
-    typedef RBTree<Zone> ZoneTree;
-    typedef RBNode<Zone> ZoneNode;
+    typedef RBTree<ZoneFinder> ZoneTree;
+    typedef RBNode<ZoneFinder> ZoneNode;
     // The actual storage
     ZoneTree zones_;
 
@@ -40,7 +40,7 @@ struct ZoneTable::ZoneTableImpl {
      */
 
     // Implementation of ZoneTable::addZone
-    result::Result addZone(ZonePtr zone) {
+    result::Result addZone(ZoneFinderPtr zone) {
         // Sanity check
         if (!zone) {
             isc_throw(InvalidParameter,
@@ -85,12 +85,12 @@ struct ZoneTable::ZoneTableImpl {
                 break;
             // We have no data there, so translate the pointer to NULL as well
             case ZoneTree::NOTFOUND:
-                return (FindResult(result::NOTFOUND, ZonePtr()));
+                return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
             // Can Not Happen
             default:
                 assert(0);
                 // Because of warning
-                return (FindResult(result::NOTFOUND, ZonePtr()));
+                return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
         }
 
         // Can Not Happen (remember, NOTFOUND is handled)
@@ -108,7 +108,7 @@ ZoneTable::~ZoneTable() {
 }
 
 result::Result
-ZoneTable::addZone(ZonePtr zone) {
+ZoneTable::addZone(ZoneFinderPtr zone) {
     return (impl_->addZone(zone));
 }
 

src/lib/datasrc/zonetable.h

@@ -41,11 +41,11 @@ namespace datasrc {
 class ZoneTable {
 public:
     struct FindResult {
-        FindResult(result::Result param_code, const ZonePtr param_zone) :
+        FindResult(result::Result param_code, const ZoneFinderPtr param_zone) :
             code(param_code), zone(param_zone)
         {}
         const result::Result code;
-        const ZonePtr zone;
+        const ZoneFinderPtr zone;
     };
     ///
     /// \name Constructors and Destructor.
@@ -83,7 +83,7 @@ public:
     /// added to the zone table.
     /// \return \c result::EXIST The zone table already contains
     /// zone of the same origin.
-    result::Result addZone(ZonePtr zone);
+    result::Result addZone(ZoneFinderPtr zone);
 
     /// Remove a \c Zone of the given origin name from the \c ZoneTable.
     ///

src/lib/python/isc/cc/data.py

@@ -22,8 +22,22 @@
 
 import json
 
-class DataNotFoundError(Exception): pass
-class DataTypeError(Exception): pass
+class DataNotFoundError(Exception):
+    """Raised if an identifier does not exist according to a spec file,
+       or if an item is addressed that is not in the current (or default)
+       config (such as a nonexistent list or map element)"""
+    pass
+
+class DataAlreadyPresentError(Exception):
+    """Raised if there is an attemt to add an element to a list or a
+       map that is already present in that list or map (i.e. if 'add'
+       is used when it should be 'set')"""
+    pass
+
+class DataTypeError(Exception):
+    """Raised if there is an attempt to set an element that is of a
+       different type than the type specified in the specification."""
+    pass
 
 def remove_identical(a, b):
     """Removes the values from dict a that are the same as in dict b.

src/lib/python/isc/config/ccsession.py

@@ -312,7 +312,7 @@ class ModuleCCSession(ConfigData):
         module_spec = isc.config.module_spec_from_file(spec_file_name)
         module_cfg = ConfigData(module_spec)
         module_name = module_spec.get_module_name()
-        self._session.group_subscribe(module_name);
+        self._session.group_subscribe(module_name)
 
         # Get the current config for that module now
         seq = self._session.group_sendmsg(create_command(COMMAND_GET_CONFIG, { "module_name": module_name }), "ConfigManager")
@@ -327,7 +327,7 @@ class ModuleCCSession(ConfigData):
             rcode, value = parse_answer(answer)
             if rcode == 0:
                 if value != None and module_spec.validate_config(False, value):
-                    module_cfg.set_local_config(value);
+                    module_cfg.set_local_config(value)
                     if config_update_callback is not None:
                         config_update_callback(value, module_cfg)
 
@@ -377,7 +377,7 @@ class ModuleCCSession(ConfigData):
                         if self.get_module_spec().validate_config(False,
                                                                   value,
                                                                   errors):
-                            self.set_local_config(value);
+                            self.set_local_config(value)
                             if self._config_handler:
                                 self._config_handler(value)
                         else:
@@ -414,8 +414,8 @@ class UIModuleCCSession(MultiConfigData):
             self.set_specification(isc.config.ModuleSpec(specs[module]))
 
     def update_specs_and_config(self):
-        self.request_specifications();
-        self.request_current_config();
+        self.request_specifications()
+        self.request_current_config()
 
     def request_current_config(self):
         """Requests the current configuration from the configuration
@@ -425,47 +425,90 @@ class UIModuleCCSession(MultiConfigData):
             raise ModuleCCSessionError("Bad config version")
         self._set_current_config(config)
 
-
-    def add_value(self, identifier, value_str = None):
-        """Add a value to a configuration list. Raises a DataTypeError
-           if the value does not conform to the list_item_spec field
-           of the module config data specification. If value_str is
-           not given, we add the default as specified by the .spec
-           file."""
-        module_spec = self.find_spec_part(identifier)
-        if (type(module_spec) != dict or "list_item_spec" not in module_spec):
-            raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list")
-
+    def _add_value_to_list(self, identifier, value):
         cur_list, status = self.get_value(identifier)
         if not cur_list:
             cur_list = []
 
-        # Hmm. Do we need to check for duplicates?
-        value = None
-        if value_str is not None:
-            value = isc.cc.data.parse_value_str(value_str)
-        else:
+        if value is None:
             if "item_default" in module_spec["list_item_spec"]:
                 value = module_spec["list_item_spec"]["item_default"]
 
         if value is None:
-            raise isc.cc.data.DataNotFoundError("No value given and no default for " + str(identifier))
-            
+            raise isc.cc.data.DataNotFoundError(
+                "No value given and no default for " + str(identifier))
+
         if value not in cur_list:
             cur_list.append(value)
             self.set_value(identifier, cur_list)
+        else:
+            raise isc.cc.data.DataAlreadyPresentError(value +
+                                                      " already in "
+                                                      + identifier)
+
+    def _add_value_to_named_set(self, identifier, value, item_value):
+        if type(value) != str:
+            raise isc.cc.data.DataTypeError("Name for named_set " +
+                                            identifier +
+                                            " must be a string")
+        # fail on both None and empty string
+        if not value:
+            raise isc.cc.data.DataNotFoundError(
+                    "Need a name to add a new item to named_set " +
+                    str(identifier))
+        else:
+            cur_map, status = self.get_value(identifier)
+            if not cur_map:
+                cur_map = {}
+            if value not in cur_map:
+                cur_map[value] = item_value
+                self.set_value(identifier, cur_map)
+            else:
+                raise isc.cc.data.DataAlreadyPresentError(value +
+                                                          " already in "
+                                                          + identifier)
 
-    def remove_value(self, identifier, value_str):
-        """Remove a value from a configuration list. The value string
-           must be a string representation of the full item. Raises
-           a DataTypeError if the value at the identifier is not a list,
-           or if the given value_str does not match the list_item_spec
-           """
+    def add_value(self, identifier, value_str = None, set_value_str = None):
+        """Add a value to a configuration list. Raises a DataTypeError
+           if the value does not conform to the list_item_spec field
+           of the module config data specification. If value_str is
+           not given, we add the default as specified by the .spec
+           file. Raises a DataNotFoundError if the given identifier
+           is not specified in the specification as a map or list.
+           Raises a DataAlreadyPresentError if the specified element
+           already exists."""
         module_spec = self.find_spec_part(identifier)
-        if (type(module_spec) != dict or "list_item_spec" not in module_spec):
-            raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list")
+        if module_spec is None:
+            raise isc.cc.data.DataNotFoundError("Unknown item " + str(identifier))
+
+        # the specified element must be a list or a named_set
+        if 'list_item_spec' in module_spec:
+            value = None
+            # in lists, we might get the value with spaces, making it
+            # the third argument. In that case we interpret both as
+            # one big string meant as the value
+            if value_str is not None:
+                if set_value_str is not None:
+                    value_str += set_value_str
+                value = isc.cc.data.parse_value_str(value_str)
+            self._add_value_to_list(identifier, value)
+        elif 'named_set_item_spec' in module_spec:
+            item_name = None
+            item_value = None
+            if value_str is not None:
+                item_name =  isc.cc.data.parse_value_str(value_str)
+            if set_value_str is not None:
+                item_value = isc.cc.data.parse_value_str(set_value_str)
+            else:
+                if 'item_default' in module_spec['named_set_item_spec']:
+                    item_value = module_spec['named_set_item_spec']['item_default']
+            self._add_value_to_named_set(identifier, item_name,
+                                         item_value)
+        else:
+            raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list or a named set")
 
-        if value_str is None:
+    def _remove_value_from_list(self, identifier, value):
+        if value is None:
             # we are directly removing an list index
             id, list_indices = isc.cc.data.split_identifier_list_indices(identifier)
             if list_indices is None:
@@ -473,17 +516,52 @@ class UIModuleCCSession(MultiConfigData):
             else:
                 self.set_value(identifier, None)
         else:
-            value = isc.cc.data.parse_value_str(value_str)
-            isc.config.config_data.check_type(module_spec, [value])
             cur_list, status = self.get_value(identifier)
-            #if not cur_list:
-            #    cur_list = isc.cc.data.find_no_exc(self.config.data, identifier)
             if not cur_list:
                 cur_list = []
-            if value in cur_list:
+            elif value in cur_list:
                 cur_list.remove(value)
             self.set_value(identifier, cur_list)
 
+    def _remove_value_from_named_set(self, identifier, value):
+        if value is None:
+            raise isc.cc.data.DataNotFoundError("Need a name to remove an item from named_set " + str(identifier))
+        elif type(value) != str:
+            raise isc.cc.data.DataTypeError("Name for named_set " + identifier + " must be a string")
+        else:
+            cur_map, status = self.get_value(identifier)
+            if not cur_map:
+                cur_map = {}
+            if value in cur_map:
+                del cur_map[value]
+            else:
+                raise isc.cc.data.DataNotFoundError(value + " not found in named_set " + str(identifier))
+
+    def remove_value(self, identifier, value_str):
+        """Remove a value from a configuration list or named set.
+        The value string must be a string representation of the full
+        item. Raises a DataTypeError if the value at the identifier
+        is not a list, or if the given value_str does not match the
+        list_item_spec """
+        module_spec = self.find_spec_part(identifier)
+        if module_spec is None:
+            raise isc.cc.data.DataNotFoundError("Unknown item " + str(identifier))
+
+        value = None
+        if value_str is not None:
+            value = isc.cc.data.parse_value_str(value_str)
+
+        if 'list_item_spec' in module_spec:
+            if value is not None:
+                isc.config.config_data.check_type(module_spec['list_item_spec'], value)
+            self._remove_value_from_list(identifier, value)
+        elif 'named_set_item_spec' in module_spec:
+            self._remove_value_from_named_set(identifier, value)
+        else:
+            raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list or a named_set")
+
+
+
     def commit(self):
         """Commit all local changes, send them through b10-cmdctl to
            the configuration manager"""

src/lib/python/isc/config/config_data.py

@@ -145,6 +145,8 @@ def _find_spec_part_single(cur_spec, id_part):
             return cur_spec['list_item_spec']
         # not found
         raise isc.cc.data.DataNotFoundError(id + " not found")
+    elif type(cur_spec) == dict and 'named_set_item_spec' in cur_spec.keys():
+        return cur_spec['named_set_item_spec']
     elif type(cur_spec) == list:
         for cur_spec_item in cur_spec:
             if cur_spec_item['item_name'] == id:
@@ -191,11 +193,14 @@ def spec_name_list(spec, prefix="", recurse=False):
                     result.extend(spec_name_list(map_el['map_item_spec'], prefix + map_el['item_name'], recurse))
                 else:
                     result.append(prefix + name)
+        elif 'named_set_item_spec' in spec:
+            # we added a '/' above, but in this one case we don't want it
+            result.append(prefix[:-1])
         else:
             for name in spec:
                 result.append(prefix + name + "/")
                 if recurse:
-                    result.extend(spec_name_list(spec[name],name, recurse))
+                    result.extend(spec_name_list(spec[name], name, recurse))
     elif type(spec) == list:
         for list_el in spec:
             if 'item_name' in list_el:
@@ -207,7 +212,7 @@ def spec_name_list(spec, prefix="", recurse=False):
             else:
                 raise ConfigDataError("Bad specification")
     else:
-        raise ConfigDataError("Bad specication")
+        raise ConfigDataError("Bad specification")
     return result
 
 class ConfigData:
@@ -255,7 +260,7 @@ class ConfigData:
 
     def get_local_config(self):
         """Returns the non-default config values in a dict"""
-        return self.data;
+        return self.data
 
     def get_item_list(self, identifier = None, recurse = False):
         """Returns a list of strings containing the full identifiers of
@@ -412,7 +417,39 @@ class MultiConfigData:
                 item_id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
                 id_list = module + "/" + id_prefix + "/" + item_id
                 id_prefix += "/" + id_part
-                if list_indices is not None:
+                part_spec = find_spec_part(self._specifications[module].get_config_spec(), id_prefix)
+                if part_spec['item_type'] == 'named_set':
+                    # For named sets, the identifier is partly defined
+                    # by which values are actually present, and not
+                    # purely by the specification.
+                    # So if there is a part of the identifier left,
+                    # we need to look up the value, then see if that
+                    # contains the next part of the identifier we got
+                    if len(id_parts) == 0:
+                        if 'item_default' in part_spec:
+                            return part_spec['item_default']
+                        else:
+                            return None
+                    id_part = id_parts.pop(0)
+
+                    named_set_value, type = self.get_value(id_list)
+                    if id_part in named_set_value:
+                        if len(id_parts) > 0:
+                            # we are looking for the *default* value.
+                            # so if not present in here, we need to
+                            # lookup the one from the spec
+                            rest_of_id = "/".join(id_parts)
+                            result = isc.cc.data.find_no_exc(named_set_value[id_part], rest_of_id)
+                            if result is None:
+                                spec_part = self.find_spec_part(identifier)
+                                if 'item_default' in spec_part:
+                                    return spec_part['item_default']
+                            return result
+                        else:
+                            return named_set_value[id_part]
+                    else:
+                        return None
+                elif list_indices is not None:
                     # there's actually two kinds of default here for
                     # lists; they can have a default value (like an
                     # empty list), but their elements can  also have
@@ -449,7 +486,12 @@ class MultiConfigData:
                     
             spec = find_spec_part(self._specifications[module].get_config_spec(), id)
             if 'item_default' in spec:
-                return spec['item_default']
+                # one special case, named_set
+                if spec['item_type'] == 'named_set':
+                    print("is " + id_part + " in named set?")
+                    return spec['item_default']
+                else:
+                    return spec['item_default']
             else:
                 return None
 
@@ -493,7 +535,7 @@ class MultiConfigData:
                 spec_part_list = spec_part['list_item_spec']
                 list_value, status = self.get_value(identifier)
                 if list_value is None:
-                    raise isc.cc.data.DataNotFoundError(identifier)
+                    raise isc.cc.data.DataNotFoundError(identifier + " not found")
 
                 if type(list_value) != list:
                     # the identifier specified a single element
@@ -509,12 +551,38 @@ class MultiConfigData:
                         for i in range(len(list_value)):
                             self._append_value_item(result, spec_part_list, "%s[%d]" % (identifier, i), all)
             elif item_type == "map":
+                value, status = self.get_value(identifier)
                 # just show the specific contents of a map, we are
                 # almost never interested in just its name
                 spec_part_map = spec_part['map_item_spec']
                 self._append_value_item(result, spec_part_map, identifier, all)
+            elif item_type == "named_set":
+                value, status = self.get_value(identifier)
+
+                # show just the one entry, when either the map is empty,
+                # or when this is element is not requested specifically
+                if len(value.keys()) == 0:
+                    entry = _create_value_map_entry(identifier,
+                                                    item_type,
+                                                    {}, status)
+                    result.append(entry)
+                elif not first and not all:
+                    entry = _create_value_map_entry(identifier,
+                                                    item_type,
+                                                    None, status)
+                    result.append(entry)
+                else:
+                    spec_part_named_set = spec_part['named_set_item_spec']
+                    for entry in value:
+                        self._append_value_item(result,
+                                                spec_part_named_set,
+                                                identifier + "/" + entry,
+                                                all)
             else:
                 value, status = self.get_value(identifier)
+                if status == self.NONE and not spec_part['item_optional']:
+                    raise isc.cc.data.DataNotFoundError(identifier + " not found")
+
                 entry = _create_value_map_entry(identifier,
                                                 item_type,
                                                 value, status)
@@ -569,7 +637,7 @@ class MultiConfigData:
                     spec_part = spec_part['list_item_spec']
                 check_type(spec_part, value)
         else:
-            raise isc.cc.data.DataNotFoundError(identifier)
+            raise isc.cc.data.DataNotFoundError(identifier + " not found")
 
         # Since we do not support list diffs (yet?), we need to
         # copy the currently set list of items to _local_changes
@@ -579,15 +647,50 @@ class MultiConfigData:
         cur_id_part = '/'
         for id_part in id_parts:
             id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
+            cur_value, status = self.get_value(cur_id_part + id)
+            # Check if the value was there in the first place
+            if status == MultiConfigData.NONE and cur_id_part != "/":
+                raise isc.cc.data.DataNotFoundError(id_part +
+                                                    " not found in " +
+                                                    cur_id_part)
             if list_indices is not None:
-                cur_list, status = self.get_value(cur_id_part + id)
+                # And check if we don't set something outside of any
+                # list
+                cur_list = cur_value
+                for list_index in list_indices:
+                    if list_index >= len(cur_list):
+                        raise isc.cc.data.DataNotFoundError("No item " +
+                                  str(list_index) + " in " + id_part)
+                    else:
+                        cur_list = cur_list[list_index]
                 if status != MultiConfigData.LOCAL:
                     isc.cc.data.set(self._local_changes,
                                     cur_id_part + id,
-                                    cur_list)
+                                    cur_value)
             cur_id_part = cur_id_part + id_part + "/"
         isc.cc.data.set(self._local_changes, identifier, value)
- 
+
+    def _get_list_items(self, item_name):
+        """This method is used in get_config_item_list, to add list
+           indices and named_set names to the completion list. If
+           the given item_name is for a list or named_set, it'll
+           return a list of those (appended to item_name), otherwise
+           the list will only contain the item_name itself."""
+        spec_part = self.find_spec_part(item_name)
+        if 'item_type' in spec_part and \
+           spec_part['item_type'] == 'named_set':
+            subslash = ""
+            if spec_part['named_set_item_spec']['item_type'] == 'map' or\
+               spec_part['named_set_item_spec']['item_type'] == 'named_set':
+                subslash = "/"
+            values, status = self.get_value(item_name)
+            if len(values) > 0:
+                return [ item_name + "/" + v + subslash for v in values.keys() ]
+            else:
+                return [ item_name ]
+        else:
+            return [ item_name ]
+
     def get_config_item_list(self, identifier = None, recurse = False):
         """Returns a list of strings containing the item_names of
            the child items at the given identifier. If no identifier is
@@ -598,7 +701,11 @@ class MultiConfigData:
             if identifier.startswith("/"):
                 identifier = identifier[1:]
             spec = self.find_spec_part(identifier)
-            return spec_name_list(spec, identifier + "/", recurse)
+            spec_list = spec_name_list(spec, identifier + "/", recurse)
+            result_list = []
+            for spec_name in spec_list:
+                result_list.extend(self._get_list_items(spec_name))
+            return result_list
         else:
             if recurse:
                 id_list = []

src/lib/python/isc/config/module_spec.py

@@ -229,7 +229,7 @@ def _check_item_spec(config_item):
     item_type = config_item["item_type"]
     if type(item_type) != str:
         raise ModuleSpecError("item_type in " + item_name + " is not a string: " + str(type(item_type)))
-    if item_type not in ["integer", "real", "boolean", "string", "list", "map", "any"]:
+    if item_type not in ["integer", "real", "boolean", "string", "list", "map", "named_set", "any"]:
         raise ModuleSpecError("unknown item_type in " + item_name + ": " + item_type)
     if "item_optional" in config_item:
         if type(config_item["item_optional"]) != bool:
@@ -293,6 +293,10 @@ def _validate_type(spec, value, errors):
         if errors != None:
             errors.append(str(value) + " should be a map")
         return False
+    elif data_type == "named_set" and type(value) != dict:
+        if errors != None:
+            errors.append(str(value) + " should be a map")
+        return False
     else:
         return True
 
@@ -308,8 +312,16 @@ def _validate_item(spec, full, data, errors):
                 if not _validate_item(list_spec, full, data_el, errors):
                     return False
     elif type(data) == dict:
-        if not _validate_spec_list(spec['map_item_spec'], full, data, errors):
-            return False
+        if 'map_item_spec' in spec:
+            if not _validate_spec_list(spec['map_item_spec'], full, data, errors):
+                return False
+        else:
+            named_set_spec = spec['named_set_item_spec']
+            for data_el in data.values():
+                if not _validate_type(named_set_spec, data_el, errors):
+                    return False
+                if not _validate_item(named_set_spec, full, data_el, errors):
+                    return False
     return True
 
 def _validate_spec(spec, full, data, errors):

src/lib/python/isc/config/tests/ccsession_test.py

@@ -695,6 +695,12 @@ class TestUIModuleCCSession(unittest.TestCase):
         fake_conn.set_get_answer('/config_data', { 'version': BIND10_CONFIG_DATA_VERSION })
         return UIModuleCCSession(fake_conn)
 
+    def create_uccs_named_set(self, fake_conn):
+        module_spec = isc.config.module_spec_from_file(self.spec_file("spec32.spec"))
+        fake_conn.set_get_answer('/module_spec', { module_spec.get_module_name(): module_spec.get_full_spec()})
+        fake_conn.set_get_answer('/config_data', { 'version': BIND10_CONFIG_DATA_VERSION })
+        return UIModuleCCSession(fake_conn)
+
     def test_init(self):
         fake_conn = fakeUIConn()
         fake_conn.set_get_answer('/module_spec', {})
@@ -715,12 +721,14 @@ class TestUIModuleCCSession(unittest.TestCase):
     def test_add_remove_value(self):
         fake_conn = fakeUIConn()
         uccs = self.create_uccs2(fake_conn)
+
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, 1, "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, "no_such_item", "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, "Spec2/item1", "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.remove_value, 1, "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.remove_value, "no_such_item", "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.remove_value, "Spec2/item1", "a")
+
         self.assertEqual({}, uccs._local_changes)
         uccs.add_value("Spec2/item5", "foo")
         self.assertEqual({'Spec2': {'item5': ['a', 'b', 'foo']}}, uccs._local_changes)
@@ -730,11 +738,36 @@ class TestUIModuleCCSession(unittest.TestCase):
         uccs.remove_value("Spec2/item5", "foo")
         uccs.add_value("Spec2/item5", "foo")
         self.assertEqual({'Spec2': {'item5': ['foo']}}, uccs._local_changes)
-        uccs.add_value("Spec2/item5", "foo")
+        self.assertRaises(isc.cc.data.DataAlreadyPresentError,
+                          uccs.add_value, "Spec2/item5", "foo")
         self.assertEqual({'Spec2': {'item5': ['foo']}}, uccs._local_changes)
+        self.assertRaises(isc.cc.data.DataNotFoundError,
+                          uccs.remove_value, "Spec2/item5[123]", None)
         uccs.remove_value("Spec2/item5[0]", None)
         self.assertEqual({'Spec2': {'item5': []}}, uccs._local_changes)
 
+    def test_add_remove_value_named_set(self):
+        fake_conn = fakeUIConn()
+        uccs = self.create_uccs_named_set(fake_conn)
+        value, status = uccs.get_value("/Spec32/named_set_item")
+        self.assertEqual({'a': 1, 'b': 2}, value)
+        uccs.add_value("/Spec32/named_set_item", "foo")
+        value, status = uccs.get_value("/Spec32/named_set_item")
+        self.assertEqual({'a': 1, 'b': 2, 'foo': 3}, value)
+
+        uccs.remove_value("/Spec32/named_set_item", "a")
+        uccs.remove_value("/Spec32/named_set_item", "foo")
+        value, status = uccs.get_value("/Spec32/named_set_item")
+        self.assertEqual({'b': 2}, value)
+
+        self.assertRaises(isc.cc.data.DataNotFoundError,
+                          uccs.set_value,
+                          "/Spec32/named_set_item/no_such_item",
+                          4)
+        self.assertRaises(isc.cc.data.DataNotFoundError,
+                          uccs.remove_value, "/Spec32/named_set_item",
+                          "no_such_item")
+
     def test_commit(self):
         fake_conn = fakeUIConn()
         uccs = self.create_uccs2(fake_conn)

src/lib/python/isc/config/tests/config_data_test.py

@@ -236,6 +236,7 @@ class TestConfigData(unittest.TestCase):
         value, default = self.cd.get_value("item6/value2")
         self.assertEqual(None, value)
         self.assertEqual(False, default)
+        self.assertRaises(isc.cc.data.DataNotFoundError, self.cd.get_value, "item6/no_such_item")
 
     def test_get_default_value(self):
         self.assertEqual(1, self.cd.get_default_value("item1"))
@@ -360,7 +361,7 @@ class TestMultiConfigData(unittest.TestCase):
 
     def test_get_current_config(self):
         cf = { 'module1': { 'item1': 2, 'item2': True } }
-        self.mcd._set_current_config(cf);
+        self.mcd._set_current_config(cf)
         self.assertEqual(cf, self.mcd.get_current_config())
 
     def test_get_local_changes(self):
@@ -421,6 +422,17 @@ class TestMultiConfigData(unittest.TestCase):
         value = self.mcd.get_default_value("Spec2/no_such_item/asdf")
         self.assertEqual(None, value)
 
+        module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec32.spec")
+        self.mcd.set_specification(module_spec)
+        value = self.mcd.get_default_value("Spec32/named_set_item")
+        self.assertEqual({ 'a': 1, 'b': 2}, value)
+        value = self.mcd.get_default_value("Spec32/named_set_item/a")
+        self.assertEqual(1, value)
+        value = self.mcd.get_default_value("Spec32/named_set_item/b")
+        self.assertEqual(2, value)
+        value = self.mcd.get_default_value("Spec32/named_set_item/no_such_item")
+        self.assertEqual(None, value)
+
     def test_get_value(self):
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
         self.mcd.set_specification(module_spec)
@@ -544,6 +556,29 @@ class TestMultiConfigData(unittest.TestCase):
         maps = self.mcd.get_value_maps("/Spec22/value9")
         self.assertEqual(expected, maps)
 
+    def test_get_value_maps_named_set(self):
+        module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec32.spec")
+        self.mcd.set_specification(module_spec)
+        maps = self.mcd.get_value_maps()
+        self.assertEqual([{'default': False, 'type': 'module',
+                           'name': 'Spec32', 'value': None,
+                           'modified': False}], maps)
+        maps = self.mcd.get_value_maps("/Spec32/named_set_item")
+        self.assertEqual([{'default': True, 'type': 'integer',
+                           'name': 'Spec32/named_set_item/a',
+                           'value': 1, 'modified': False},
+                          {'default': True, 'type': 'integer',
+                           'name': 'Spec32/named_set_item/b',
+                           'value': 2, 'modified': False}], maps)
+        maps = self.mcd.get_value_maps("/Spec32/named_set_item/a")
+        self.assertEqual([{'default': True, 'type': 'integer',
+                           'name': 'Spec32/named_set_item/a',
+                           'value': 1, 'modified': False}], maps)
+        maps = self.mcd.get_value_maps("/Spec32/named_set_item/b")
+        self.assertEqual([{'default': True, 'type': 'integer',
+                           'name': 'Spec32/named_set_item/b',
+                           'value': 2, 'modified': False}], maps)
+
     def test_set_value(self):
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
         self.mcd.set_specification(module_spec)
@@ -582,6 +617,24 @@ class TestMultiConfigData(unittest.TestCase):
         config_items = self.mcd.get_config_item_list("Spec2", True)
         self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
 
+    def test_get_config_item_list_named_set(self):
+        config_items = self.mcd.get_config_item_list()
+        self.assertEqual([], config_items)
+        module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec32.spec")
+        self.mcd.set_specification(module_spec)
+        config_items = self.mcd.get_config_item_list()
+        self.assertEqual(['Spec32'], config_items)
+        config_items = self.mcd.get_config_item_list(None, False)
+        self.assertEqual(['Spec32'], config_items)
+        config_items = self.mcd.get_config_item_list(None, True)
+        self.assertEqual(['Spec32/named_set_item'], config_items)
+        self.mcd.set_value('Spec32/named_set_item', { "aaaa": 4, "aabb": 5, "bbbb": 6})
+        config_items = self.mcd.get_config_item_list("/Spec32/named_set_item", True)
+        self.assertEqual(['Spec32/named_set_item/aaaa',
+                          'Spec32/named_set_item/aabb',
+                          'Spec32/named_set_item/bbbb',
+                         ], config_items)
+
 if __name__ == '__main__':
     unittest.main()
 

src/lib/python/isc/config/tests/module_spec_test.py

@@ -98,6 +98,9 @@ class TestModuleSpec(unittest.TestCase):
         self.assertEqual(True, self.validate_data("spec22.spec", "data22_6.data"))
         self.assertEqual(True, self.validate_data("spec22.spec", "data22_7.data"))
         self.assertEqual(False, self.validate_data("spec22.spec", "data22_8.data"))
+        self.assertEqual(True, self.validate_data("spec32.spec", "data32_1.data"))
+        self.assertEqual(False, self.validate_data("spec32.spec", "data32_2.data"))
+        self.assertEqual(False, self.validate_data("spec32.spec", "data32_3.data"))
 
     def validate_command_params(self, specfile_name, datafile_name, cmd_name):
         dd = self.read_spec_file(specfile_name);