Merge branch 'master' into trac384

ChangeLog

@@ -1,3 +1,74 @@
+  153.	[bug]		jelte
+	b10-cfgmgr: Fixed a bug where configuration updates sometimes
+	lost previous settings in the configuration manager.
+	(Trac #427, git 2df894155657754151e0860e2ca9cdbed7317c70)
+
+  152.	[func]*		jinmei
+	b10-auth: Added new configuration variable "statistics-interval"
+	to allow the user to change the timer interval for periodic
+	statistics updates.  The update can also be disabled by setting
+	the value to 0.  Disabling statistics updates will also work as
+	a temporary workaround of a known issue that b10-auth can block in
+	sending statistics and stop responding to queries as a result.
+	(Trac #513, git 285c5ee)
+
+  151.  [bug]		smann
+	lib/log/dummylog.h: 
+	lib/log/dummylog.cc: Modify dlog so that it takes an optional 2nd
+        argument of type bool (true or false). This flag, if set, will cause
+        the message to be printed whether or not -v is chosen.
+        (trac #432, git 880220478c3e8702d56d761b1e0b21b77d08ee5a)
+
+  150.  [bug]		jelte
+	b10-cfgmgr: No longer save the configuration on exit. Configuration
+	is already saved if it is changed successfully, so writing it on
+	exit (and hence, when nothing has changed too) is unnecessary and
+	may even cause problems.
+	(Trac #435, git fd7baa38c08d54d5b5f84930c1684c436d2776dc)
+
+  149.  [bug]		jelte
+	bindctl: Check if the user session has disappeared (either by a
+	timeout or by a server restart), and reauthenticate if so. This
+	fixes the 'cmdctl not running' problem.
+        (trac #431, git b929be82fec5f92e115d8985552f84b4fdd385b9)
+
+  148.	[func]		jelte
+	bindctl: Command results are now pretty-printed (i.e. printed in
+	a more readable form). Empty results are no longer printed at all
+	(used to print '{}'), and the message
+	'send the command to cmd-ctrl' has also been removed.
+	(git 3954c628c13ec90722a2d8816f52a380e0065bae)
+
+  147.	[bug]		jinmei
+	python/isc/config: Fixed a bug that importing custom configuration
+	(in b10-config.db) of a remote module didn't work.
+	(Trac #478, git ea4a481)
+
+  146.	[func]		jelte
+	Command arguments were not validated internally against their
+	specifications. This change fixes that (on the C++ side, Python
+	side depends on an as yet planned addition). Note: this is only
+	an added internal check, the cli already checks format.
+	(Trac #473, git 5474eba181cb2fdd80e2b2200e072cd0a13a4e52)
+
+  145.	[func]*		jinmei
+	b10-auth: added a new command 'loadzone' for (re)loading a
+	specific zone.  The command syntax is generic but it is currently
+	only feasible for class IN in memory data source.  To reload a
+	zone "example.com" via bindctl, execute the command as follows:
+	> Auth loadzone origin = example.com
+	(Trac #467)
+
+  144.	[build]		jinmei
+	Introduced a workaround for clang++ build on FreeBSD (and probably
+	some other OSes).  If building BIND 10 fails with clang++ due to
+	a link error about "__dso_handle", try again from the configure
+	script with CXX_LIBTOOL_LDFLAGS=-L/usr/lib (the path actually
+	doesn't matter; the important part is the -L flag).  This
+	workaround is not automatically enabled as it's difficult to
+	detect the need for it dynamically, and must be enabled via the
+	variable by hand. (Trac #474, git cfde436)
+
   143.	[build]		jinmei
 	Fixed build problems with clang++ in unit tests due to recent
 	changes.  No behavior change. (Trac #448, svn r4133)
@@ -62,21 +133,21 @@
 	(Trac #202, svn r3967)
 
   135.  [func]      each
-	Add b10-recurse. This is an example recursive server that
+	Add b10-resolver. This is an example recursive server that
 	currently does forwarding only and no caching.
 	(Trac #327, svn r3903)
 
   134.  [func]      vorner
-	b10-recurse supports timeouts and retries in forwarder mode.
+	b10-resolver supports timeouts and retries in forwarder mode.
 	(Trac #401, svn r3660)
 
   133.  [func]      vorner
 	New temporary logging function available in isc::log. It is used by
-	b10-recurse.
+	b10-resolver.
 	(Trac #393, r3602)
 
   132.  [func]      vorner
-	The b10-recurse is configured through config manager.
+	The b10-resolver is configured through config manager.
 	It has "listen_on" and "forward_addresses" options.
 	(Trac #389, r3448)
 
@@ -139,7 +210,7 @@ bind10-devel-20101201 released on December 01, 2010
   122.  [func]		stephen
 	src/bin/bind10: Added configuration options to Boss to determine
 	whether to start the authoritative server, recursive server (or
-	both). A dummy recursor has been provided for test purposes.
+	both). A dummy program has been provided for test purposes.
 	(Trac #412, svn r3676)
 
   121.  [func]		jinmei
@@ -200,7 +271,7 @@ bind10-devel-20101201 released on December 01, 2010
 
   112.	[func]		zhang likun
 	Add one mixin class to override the naive serve_forever() provided
-	in python library socketserver. Instead of polling for shutdwon
+	in python library socketserver. Instead of polling for shutdown
 	every poll_interval seconds, one socketpair is used to wake up
 	the waiting server. (Trac #352, svn r3366)
 
@@ -781,8 +852,12 @@ bind10-devel-20100421 released on April 21, 2010
 bind10-devel-20100319 released on March 19, 2010
 
 For complete code revision history, see http://bind10.isc.org/browser
-Specific subversion changesets can be accessed at:
-	http://bind10.isc.org/changeset/rrrr
+Specific git changesets can be accessed at:
+	http://bind10.isc.org/changeset/?reponame=&old=rrrr^&new=rrrr
+or after cloning the original git repository by executing:
+	% git diff rrrr^ rrrr
+Subversion changesets are not accessible any more.  The subversion
+revision numbers will be replaced with corresponding git revisions.
 Trac tickets can be accessed at: https://bind10.isc.org/ticket/nnn
 
 LEGEND

README

@@ -15,7 +15,7 @@ five year plan are described here:
 
 This release includes the bind10 master process, b10-msgq message
 bus, b10-auth authoritative DNS server (with SQLite3 backend),
-b10-recurse forwarding DNS server, b10-cmdctl remote control daemon,
+b10-resolver forwarding DNS server, b10-cmdctl remote control daemon,
 b10-cfgmgr configuration manager, b10-xfrin AXFR inbound service,
 b10-xfrout outgoing AXFR service, b10-zonemgr secondary manager,
 b10-stats statistics collection and reporting daemon, and a new
@@ -193,6 +193,7 @@ config revert:	Revert all changes that have not been committed
 config commit: Commit all changes
 config diff: Show the changes that have not been committed yet
 
+
 EXAMPLE SESSION
 
 ~> bindctl

configure.ac

@@ -9,7 +9,23 @@ AC_CONFIG_HEADERS([config.h])
 
 # Checks for programs.
 AC_PROG_CXX
+
+# Libtool configuration
+#
+# On FreeBSD (and probably some others), clang++ does not meet an autoconf
+# assumption in identifying libtool configuration regarding shared library:
+# the configure script will execute "$CC -shared $CFLAGS -v -o" and expect
+# the output contains -Lxxx or -Ryyy.  This is the case for g++, but not for
+# clang++, and, as a result, it will cause various errors in linking programs
+# or running them with a shared object (such as some of our python scripts).
+# To work around this problem we define a temporary variable
+# "CXX_LIBTOOL_LDFLAGS".  It's expected to be defined as, e.g, "-L/usr/lib"
+# to temporarily fake the output so that it will be compatible with that of
+# g++.
+CFLAGS_SAVED=$CFLAGS
+CFLAGS="$CFLAGS $CXX_LIBTOOL_LDFLAGS"
 AC_PROG_LIBTOOL
+CFLAGS=$CFLAGS_SAVED
 
 # Use C++ language
 AC_LANG([C++])
@@ -67,7 +83,7 @@ case "$host" in
 	CPPFLAGS="$CPPFLAGS -D_XPG4_2 -D__EXTENSIONS__"
 	;;
 *-apple-darwin*)
-	# libtool doesn't work pefectly with Darwin: libtool embeds the
+	# libtool doesn't work perfectly with Darwin: libtool embeds the
 	# final install path in dynamic libraries and our loadable python
 	# modules always refer to that path even if it's loaded within the
 	# source tree.  This prevents pre-install tests from working.
@@ -186,7 +202,7 @@ fi
 
 # Compiler dependent settings: define some mandatory CXXFLAGS here.
 # We also use a separate variable B10_CXXFLAGS.  This will (and should) be
-# used as the default value for each specifc AM_CXXFLAGS:
+# used as the default value for each specific AM_CXXFLAGS:
 # AM_CXXFLAGS = $(B10_CXXFLAGS)
 # AM_CXXFLAGS += ... # add module specific flags
 # We need this so that we can disable some specific compiler warnings per
@@ -539,7 +555,7 @@ fi
 # So, for the moment, we simply disable the use of /dev/poll.  Unless we
 # implement recursive DNS server with randomized ports, we don't need the
 # scalability that /dev/poll can provide, so this decision wouldn't affect
-# run time performance.  Hpefully we can find a better solution or the ASIO
+# run time performance.  Hopefully we can find a better solution or the ASIO
 # code will be updated by the time we really need it.
 AC_CHECK_HEADERS(sys/devpoll.h, ac_cv_have_devpoll=yes, ac_cv_have_devpoll=no)
 if test "X$ac_cv_have_devpoll" = "Xyes" -a "X$GXX" = "Xyes"; then
@@ -577,8 +593,8 @@ AC_CONFIG_FILES([Makefile
                  src/bin/auth/Makefile
                  src/bin/auth/tests/Makefile
                  src/bin/auth/benchmarks/Makefile
-                 src/bin/recurse/Makefile
-                 src/bin/recurse/tests/Makefile
+                 src/bin/resolver/Makefile
+                 src/bin/resolver/tests/Makefile
                  src/bin/xfrin/Makefile
                  src/bin/xfrin/tests/Makefile
                  src/bin/xfrout/Makefile
@@ -652,8 +668,8 @@ AC_OUTPUT([src/bin/cfgmgr/b10-cfgmgr.py
            src/bin/xfrout/xfrout.spec.pre
            src/bin/xfrout/tests/xfrout_test
            src/bin/xfrout/run_b10-xfrout.sh
-           src/bin/recurse/recurse.spec.pre
-           src/bin/recurse/spec_config.h.pre
+           src/bin/resolver/resolver.spec.pre
+           src/bin/resolver/spec_config.h.pre
            src/bin/zonemgr/zonemgr.py
            src/bin/zonemgr/zonemgr.spec.pre
            src/bin/zonemgr/tests/zonemgr_test
@@ -696,7 +712,7 @@ AC_OUTPUT([src/bin/cfgmgr/b10-cfgmgr.py
            chmod +x src/bin/cmdctl/run_b10-cmdctl.sh
            chmod +x src/bin/xfrin/run_b10-xfrin.sh
            chmod +x src/bin/xfrout/run_b10-xfrout.sh
-           chmod +x src/bin/recurse/run_b10-recurse.sh
+           chmod +x src/bin/resolver/run_b10-resolver.sh
            chmod +x src/bin/zonemgr/run_b10-zonemgr.sh
            chmod +x src/bin/stats/tests/stats_test
            chmod +x src/bin/stats/run_b10-stats.sh

src/bin/Makefile.am

@@ -1,4 +1,4 @@
 SUBDIRS = bind10 bindctl cfgmgr loadzone msgq host cmdctl auth xfrin xfrout \
-	usermgr zonemgr stats tests recurse
+	usermgr zonemgr stats tests resolver
 
 check-recursive: all-recursive

src/bin/auth/Makefile.am

@@ -40,6 +40,7 @@ b10_auth_SOURCES = query.cc query.h
 b10_auth_SOURCES += auth_srv.cc auth_srv.h
 b10_auth_SOURCES += change_user.cc change_user.h
 b10_auth_SOURCES += config.cc config.h
+b10_auth_SOURCES += command.cc command.h
 b10_auth_SOURCES += common.h
 b10_auth_SOURCES += statistics.cc statistics.h
 b10_auth_SOURCES += main.cc

src/bin/auth/auth.spec.pre.in

@@ -51,6 +51,11 @@
             }
           }]
         }
+      },
+      { "item_name": "statistics-interval",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 60
       }
     ],
     "commands": [
@@ -63,8 +68,25 @@
         "command_name": "sendstats",
         "command_description": "Send data to a statistics module at once",
         "command_args": []
+      },
+      {
+        "command_name": "loadzone",
+        "command_description": "(Re)load a specified zone",
+        "command_args": [
+          {
+            "item_name": "class", "item_type": "string",
+            "item_optional": true, "item_default": "IN"
+          },
+	  {
+            "item_name": "origin", "item_type": "string",
+            "item_optional": false, "item_default": ""
+          },
+	  {
+            "item_name": "datasrc", "item_type": "string",
+            "item_optional": true, "item_default": "memory"
+          }
+        ]
       }
     ]
   }
 }
-

src/bin/auth/auth_srv.cc

@@ -23,6 +23,8 @@
 #include <iostream>
 #include <vector>
 
+#include <boost/bind.hpp>
+
 #include <asiolink/asiolink.h>
 
 #include <config/ccsession.h>
@@ -87,6 +89,8 @@ public:
     bool processNotify(const IOMessage& io_message, MessagePtr message,
                        OutputBufferPtr buffer);
 
+    IOService io_service_;
+
     /// Currently non-configurable, but will be.
     static const uint16_t DEFAULT_LOCAL_UDPSIZE = 4096;
 
@@ -102,6 +106,9 @@ public:
     /// Hot spot cache
     isc::datasrc::HotCache cache_;
 
+    /// Interval timer for periodic submission of statistics counters.
+    IntervalTimer statistics_timer_;
+
     /// Query counters for statistics
     AuthCounters counters_;
 private:
@@ -125,6 +132,7 @@ AuthSrvImpl::AuthSrvImpl(const bool use_cache,
     config_session_(NULL), verbose_mode_(false),
     xfrin_session_(NULL),
     memory_datasrc_class_(RRClass::IN()),
+    statistics_timer_(io_service_),
     counters_(verbose_mode_),
     xfrout_connected_(false),
     xfrout_client_(xfrout_client)
@@ -200,6 +208,11 @@ AuthSrv::AuthSrv(const bool use_cache, AbstractXfroutClient& xfrout_client) :
     dns_answer_(new MessageAnswer(this))
 {}
 
+void
+AuthSrv::stop() {
+    impl_->io_service_.stop();
+}
+
 AuthSrv::~AuthSrv() {
     delete impl_;
     delete checkin_;
@@ -269,6 +282,11 @@ AuthSrv::getVerbose() const {
     return (impl_->verbose_mode_);
 }
 
+IOService&
+AuthSrv::getIOService() {
+    return (impl_->io_service_);
+}
+
 void
 AuthSrv::setCacheSlots(const size_t slots) {
     impl_->cache_.setSlots(slots);
@@ -299,8 +317,8 @@ AuthSrv::getConfigSession() const {
     return (impl_->config_session_);
 }
 
-AuthSrv::ConstMemoryDataSrcPtr
-AuthSrv::getMemoryDataSrc(const RRClass& rrclass) const {
+AuthSrv::MemoryDataSrcPtr
+AuthSrv::getMemoryDataSrc(const RRClass& rrclass) {
     // XXX: for simplicity, we only support the IN class right now.
     if (rrclass != impl_->memory_datasrc_class_) {
         isc_throw(InvalidParameter,
@@ -332,6 +350,32 @@ AuthSrv::setMemoryDataSrc(const isc::dns::RRClass& rrclass,
     impl_->memory_datasrc_ = memory_datasrc;
 }
 
+uint32_t
+AuthSrv::getStatisticsTimerInterval() const {
+    return (impl_->statistics_timer_.getInterval());
+}
+
+void
+AuthSrv::setStatisticsTimerInterval(uint32_t interval) {
+    if (interval == impl_->statistics_timer_.getInterval()) {
+        return;
+    }
+    if (interval == 0) {
+        impl_->statistics_timer_.cancel();
+    } else {
+        impl_->statistics_timer_.setupTimer(
+            boost::bind(&AuthSrv::submitStatistics, this), interval);
+    }
+    if (impl_->verbose_mode_) {
+        if (interval == 0) {
+            cerr << "[b10-auth] Disabled statistics timer" << endl;
+        } else {
+            cerr << "[b10-auth] Set statistics timer to " << interval
+                 << " seconds" << endl;
+        }
+    }
+}
+
 void
 AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
                         OutputBufferPtr buffer, DNSServer* server)

src/bin/auth/auth_srv.h

@@ -87,6 +87,14 @@ public:
     ~AuthSrv();
     //@}
 
+    /// Stop the server.
+    ///
+    /// It stops the internal event loop of the server and subsequently
+    /// returns the control to the top level context.
+    ///
+    /// This method should never throw an exception.
+    void stop();
+
     /// \brief Process an incoming DNS message, then signal 'server' to resume 
     ///
     /// A DNS query (or other message) has been received by a \c DNSServer
@@ -185,11 +193,8 @@ public:
     /// control commands and configuration updates.
     void setConfigSession(isc::config::ModuleCCSession* config_session);
 
-    /// \brief Assign an ASIO IO Service queue to this Recursor object
-    void setIOService(asiolink::IOService& ios) { io_service_ = &ios; }
-
     /// \brief Return this object's ASIO IO Service queue
-    asiolink::IOService& getIOService() const { return (*io_service_); }
+    asiolink::IOService& getIOService();
 
     /// \brief Return pointer to the DNS Lookup callback function
     asiolink::DNSLookup* getDNSLookupProvider() const { return (dns_lookup_); }
@@ -265,8 +270,7 @@ 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.
-    ConstMemoryDataSrcPtr
-    getMemoryDataSrc(const isc::dns::RRClass& rrclass) const;
+    MemoryDataSrcPtr getMemoryDataSrc(const isc::dns::RRClass& rrclass);
 
     /// Sets or replaces the in-memory data source of the specified RR class.
     ///
@@ -304,6 +308,28 @@ public:
     /// is shutdown.
     void setStatisticsSession(isc::cc::AbstractSession* statistics_session);
 
+    /// Return the interval of periodic submission of statistics in seconds.
+    ///
+    /// If the statistics submission is disabled, it returns 0.
+    ///
+    /// This method never throws an exception.
+    uint32_t getStatisticsTimerInterval() const;
+
+    /// Set the interval of periodic submission of statistics.
+    ///
+    /// If the specified value is non 0, the \c AuthSrv object will submit
+    /// its statistics to the statistics module every \c interval seconds.
+    /// If it's 0, and \c AuthSrv currently submits statistics, the submission
+    /// will be disabled.
+    ///
+    /// This method should normally not throw an exception; however, its
+    /// underlying library routines may involve resource allocation, and
+    /// when it fails it would result in a corresponding standard exception.
+    ///
+    /// \param interval The submission interval in seconds if non 0;
+    /// or a value of 0 to disable the submission.
+    void setStatisticsTimerInterval(uint32_t interval);
+
     /// \brief Submit statistics counters to statistics module.
     ///
     /// This function can throw an exception from
@@ -330,7 +356,6 @@ public:
 
 private:
     AuthSrvImpl* impl_;
-    asiolink::IOService* io_service_;
     asiolink::SimpleCallback* checkin_;
     asiolink::DNSLookup* dns_lookup_;
     asiolink::DNSAnswer* dns_answer_;

src/bin/auth/command.cc

@@ -0,0 +1,252 @@
+// Copyright (C) 2010  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.
+
+#include <string>
+
+#include <boost/scoped_ptr.hpp>
+#include <boost/shared_ptr.hpp>
+
+#include <exceptions/exceptions.h>
+
+#include <dns/rrclass.h>
+
+#include <cc/data.h>
+
+#include <datasrc/memory_datasrc.h>
+
+#include <config/ccsession.h>
+
+#include <auth/auth_srv.h>
+#include <auth/command.h>
+
+using namespace std;
+using boost::shared_ptr;
+using boost::scoped_ptr;
+using namespace isc::dns;
+using namespace isc::data;
+using namespace isc::datasrc;
+using namespace isc::config;
+
+namespace {
+/// An exception that is thrown if an error occurs while handling a command
+/// on an \c AuthSrv object.
+///
+/// Currently it's only used internally, since \c execAuthServerCommand()
+/// (which is the only interface to this module) catches all \c isc::
+/// exceptions and converts them.
+class AuthCommandError : public isc::Exception {
+public:
+    AuthCommandError(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
+/// An abstract base class that represents a single command identifier
+/// for an \c AuthSrv object.
+///
+/// Each of derived classes of \c AuthCommand, which are hidden inside the
+/// implementation, corresponds to a command executed on \c AuthSrv, such as
+/// "shutdown".  The derived class is responsible to execute the corresponding
+/// command with the given command arguments (if any) in its \c exec()
+/// method.
+///
+/// In the initial implementation the existence of the command classes is
+/// hidden inside the implementation since the only public interface is
+/// \c execAuthServerCommand(), which does not expose this class.
+/// In future, we may want to make this framework more dynamic, i.e.,
+/// registering specific derived classes run time outside of this
+/// implementation.  If and when that happens the definition of the abstract
+/// class will be published.
+class AuthCommand {
+    ///
+    /// \name Constructors and Destructor
+    ///
+    /// Note: The copy constructor and the assignment operator are
+    /// intentionally defined as private to make it explicit that this is a
+    /// pure base class.
+    //@{
+private:
+    AuthCommand(const AuthCommand& source);
+    AuthCommand& operator=(const AuthCommand& source);
+protected:
+    /// \brief The default constructor.
+    ///
+    /// This is intentionally defined as \c protected as this base class should
+    /// never be instantiated (except as part of a derived class).
+    AuthCommand() {}
+public:
+    /// The destructor.
+    virtual ~AuthCommand() {}
+    //@}
+
+    /// Execute a single control command.
+    ///
+    /// Specific derived methods can throw exceptions.  When called via
+    /// \c execAuthServerCommand(), all BIND 10 exceptions are caught
+    /// and converted into an error code.
+    /// The derived method may also throw an exception of class
+    /// \c AuthCommandError when it encounters an internal error, such as
+    /// semantics error on the command arguments.
+    ///
+    /// \param server The \c AuthSrv object on which the command is executed.
+    /// \param args Command specific argument.
+    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr args) = 0;
+};
+
+// Handle the "shutdown" command.  No argument is assumed.
+class ShutdownCommand : public AuthCommand {
+public:
+    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr) {
+        server.stop();
+    }
+};
+
+// Handle the "sendstats" command.  No argument is assumed.
+class SendStatsCommand : public AuthCommand {
+public:
+    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr) {
+        if (server.getVerbose()) {
+            cerr << "[b10-auth] command 'sendstats' received" << endl;
+        }
+        server.submitStatistics();
+    }
+};
+
+// Handle the "loadzone" command.
+class LoadZoneCommand : public AuthCommand {
+public:
+    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr args) {
+        // parse and validate the args.
+        if (!validate(server, args)) {
+            return;
+        }
+
+        // Load a new zone and replace the current zone with the new one.
+        // TODO: eventually this should be incremental or done in some way
+        // 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);
+
+        if (server.getVerbose()) {
+            cerr << "[b10-auth] Loaded zone '" << newzone->getOrigin()
+                 << "'/" << newzone->getClass() << endl;
+        }
+    }
+
+private:
+    shared_ptr<MemoryZone> oldzone; // zone to be updated with the new file.
+
+    // A helper private method to parse and validate command parameters.
+    // On success, it sets 'oldzone' 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) {
+        if (args == NULL) {
+            isc_throw(AuthCommandError, "Null argument");
+        }
+
+        // In this initial implementation, we assume memory data source
+        // for class IN by default.
+        ConstElementPtr datasrc_elem = args->get("datasrc");
+        if (datasrc_elem) {
+            if (datasrc_elem->stringValue() == "sqlite3") {
+                if (server.getVerbose()) {
+                    cerr << "[b10-auth] Nothing to do for loading sqlite3"
+                         << endl;
+                }
+                return (false);
+            } else if (datasrc_elem->stringValue() != "memory") {
+                // (note: at this point it's guaranteed that datasrc_elem
+                // is of string type)
+                isc_throw(AuthCommandError,
+                          "Data source type " << datasrc_elem->stringValue()
+                          << " is not supported");
+            }
+        }
+
+        ConstElementPtr class_elem = args->get("class");
+        const RRClass zone_class = class_elem ?
+            RRClass(class_elem->stringValue()) : RRClass::IN();
+
+        AuthSrv::MemoryDataSrcPtr datasrc(server.getMemoryDataSrc(zone_class));
+        if (datasrc == NULL) {
+            isc_throw(AuthCommandError, "Memory data source is disabled");
+        }
+
+        ConstElementPtr origin_elem = args->get("origin");
+        if (!origin_elem) {
+            isc_throw(AuthCommandError, "Zone origin is missing");
+        }
+        const Name origin(origin_elem->stringValue());
+
+        // Get the current zone
+        const MemoryDataSrc::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);
+
+        return (true);
+    }
+};
+
+// The factory of command objects.
+AuthCommand*
+createAuthCommand(const string& command_id) {
+    // For the initial implementation we use a naive if-else blocks
+    // (see also createAuthConfigParser())
+    if (command_id == "shutdown") {
+        return (new ShutdownCommand());
+    } else if (command_id == "sendstats") {
+        return (new SendStatsCommand());
+    } else if (command_id == "loadzone") {
+        return (new LoadZoneCommand());
+    } else if (false && command_id == "_throw_exception") {
+        // This is for testing purpose only and should not appear in the
+        // actual configuration syntax.
+        // XXX: ModuleCCSession doesn't seem to validate commands (unlike
+        // config), so we should disable this case for now.
+        throw runtime_error("throwing for test");
+    }
+
+    isc_throw(AuthCommandError, "Unknown command identifier: " << command_id);
+}
+} // end of unnamed namespace
+
+ConstElementPtr
+execAuthServerCommand(AuthSrv& server, const string& command_id,
+                      ConstElementPtr args)
+{
+    if (server.getVerbose()) {
+        cerr << "[b10-auth] Received '" << command_id << "' command" << endl;
+    }
+
+    try {
+        scoped_ptr<AuthCommand>(createAuthCommand(command_id))->exec(server,
+                                                                     args);
+    } catch (const isc::Exception& ex) {
+        if (server.getVerbose()) {
+            cerr << "[b10-auth] Command '" << command_id
+                 << "' execution failed: " << ex.what() << endl;
+        }
+        return (createAnswer(1, ex.what()));
+    }
+
+    return (createAnswer());
+}

src/bin/auth/command.h

@@ -0,0 +1,61 @@
+// Copyright (C) 2010  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.
+
+#include <string>
+
+#include <cc/data.h>
+
+#ifndef __COMMAND_H
+#define __COMMAND_H 1
+
+class AuthSrv;
+
+/// Execute a control command on \c AuthSrv
+///
+/// It executes the operation identified by \c command_id with arguments
+/// \c args on \c server, and returns the result in the form of the standard
+/// command/config answer message (see \c isc::config::createAnswer()).
+///
+/// This method internally performs minimal validation on \c command_id and
+/// \c args to not cause a surprising result such as a crash, but it is
+/// generally expected that the caller has performed syntax level validation
+/// based on the command specification for the authoritative server.
+/// For example, the caller is responsible \c command_id is a valid command
+/// name for the authoritative server.
+///
+/// The execution of the command may internally trigger an exception; for
+/// instance, if a string that is expected to be a valid domain name is bogus,
+/// the underlying DNS library will throw an exception.  These internal
+/// exceptions will be caught inside this function, and will be converted
+/// to a return value with a non 0 error code.
+/// However, exceptions thrown outside of BIND 10 modules (including standard
+/// exceptions) are expected to be handled at a higher layer, and will be
+/// propagated to the caller.  In particular if memory allocation fails and
+/// \c std::bad_alloc is thrown it will be propagated.
+///
+/// \param server The \c AuthSrv object on which the command is executed.
+/// \param command_id The identifier of the command (such as "shutdown")
+/// \param args Command specific argument in a map type of
+/// \c isc::data::Element, or a \c NULL \c ElementPtr if the argument isn't
+/// specified.
+/// \return The result of the command operation.
+isc::data::ConstElementPtr
+execAuthServerCommand(AuthSrv& server, const std::string& command_id,
+                      isc::data::ConstElementPtr args);
+
+#endif // __COMMAND_H
+
+// Local Variables:
+// mode: c++
+// End:

src/bin/auth/config.cc

@@ -169,6 +169,32 @@ MemoryDatasourceConfig::build(ConstElementPtr config_value) {
     }
 }
 
+/// A derived \c AuthConfigParser class for the "statistics-internal"
+/// configuration identifier.
+class StatisticsIntervalConfig : public AuthConfigParser {
+public:
+    StatisticsIntervalConfig(AuthSrv& server) :
+        server_(server), interval_(0)
+    {}
+    virtual void build(ConstElementPtr config_value) {
+        const int32_t config_interval = config_value->intValue();
+        if (config_interval < 0) {
+            isc_throw(AuthConfigError, "negative statistics-interval value: "
+                      << config_interval);
+        }
+        interval_ = config_interval;
+    }
+    virtual void commit() {
+        // setStatisticsTimerInterval() is not 100% exception free.  But
+        // exceptions should happen only in a very rare situation, so we
+        // let them be thrown and subsequently regard them as a fatal error.
+        server_.setStatisticsTimerInterval(interval_);
+    }
+private:
+    AuthSrv& server_;
+    uint32_t interval_;
+};
+
 /// A special parser for testing: it throws from commit() despite the
 /// suggested convention of the class interface.
 class ThrowerCommitConfig : public AuthConfigParser {
@@ -191,6 +217,8 @@ createAuthConfigParser(AuthSrv& server, const std::string& config_id,
     // that it can be dynamically customized.
     if (config_id == "datasources") {
         return (new DatasourcesConfig(server));
+    } else if (config_id == "statistics-interval") {
+        return (new StatisticsIntervalConfig(server));
     } else if (internal && config_id == "datasources/memory") {
         return (new MemoryDatasourceConfig(server));
     } else if (config_id == "_commit_throw") {

src/bin/auth/main.cc

@@ -25,8 +25,6 @@
 #include <cassert>
 #include <iostream>
 
-#include <boost/bind.hpp>
-
 #include <exceptions/exceptions.h>
 
 #include <dns/buffer.h>
@@ -42,6 +40,7 @@
 #include <auth/spec_config.h>
 #include <auth/common.h>
 #include <auth/config.h>
+#include <auth/command.h>
 #include <auth/change_user.h>
 #include <auth/auth_srv.h>
 #include <asiolink/asiolink.h>
@@ -56,22 +55,15 @@ using namespace asiolink;
 
 namespace {
 
-static bool verbose_mode = false;
+bool verbose_mode = false;
 
 // Default port current 5300 for testing purposes
-static const string PROGRAM = "Auth";
-static const char* DNSPORT = "5300";
-
-// Note: this value must be greater than 0.
-// TODO: make it configurable via command channel.
-const uint32_t STATISTICS_SEND_INTERVAL_SEC = 60;
+const char* DNSPORT = "5300";
 
 /* need global var for config/command handlers.
  * todo: turn this around, and put handlers in the authserver
  * class itself? */
-static AuthSrv *auth_server;
-
-static IOService io_service;
+AuthSrv *auth_server;
 
 ConstElementPtr
 my_config_handler(ConstElementPtr new_config) {
@@ -80,23 +72,8 @@ my_config_handler(ConstElementPtr new_config) {
 
 ConstElementPtr
 my_command_handler(const string& command, ConstElementPtr args) {
-    ConstElementPtr answer = createAnswer();
-
-    if (command == "print_message") {
-        cout << args << endl;
-        /* let's add that message to our answer as well */
-        answer = createAnswer(0, args);
-    } else if (command == "shutdown") {
-        io_service.stop();
-    } else if (command == "sendstats") {
-        if (verbose_mode) {
-            cerr << "[b10-auth] command 'sendstats' received" << endl;
-        }
-        assert(auth_server != NULL);
-        auth_server->submitStatistics();
-    }
-
-    return (answer);
+    assert(auth_server != NULL);
+    return (execAuthServerCommand(*auth_server, command, args));
 }
 
 void
@@ -113,12 +90,6 @@ usage() {
     cerr << "\t-v: verbose output" << endl;
     exit(1);
 }
-
-void
-statisticsTimerCallback(AuthSrv* auth_server) {
-    assert(auth_server != NULL);
-    auth_server->submitStatistics();
-}
 } // end of anonymous namespace
 
 int
@@ -185,7 +156,6 @@ main(int argc, char* argv[]) {
     Session* cc_session = NULL;
     Session* xfrin_session = NULL;
     Session* statistics_session = NULL;
-    IntervalTimer* itimer = NULL;
     bool xfrin_session_established = false; // XXX (see Trac #287)
     bool statistics_session_established = false; // XXX (see Trac #287)
     ModuleCCSession* config_session = NULL;
@@ -210,6 +180,7 @@ main(int argc, char* argv[]) {
         cout << "[b10-auth] Server created." << endl;
 
         SimpleCallback* checkin = auth_server->getCheckinProvider();
+        IOService& io_service = auth_server->getIOService();
         DNSLookup* lookup = auth_server->getDNSLookupProvider();
         DNSAnswer* answer = auth_server->getDNSAnswerProvider();
 
@@ -228,8 +199,7 @@ main(int argc, char* argv[]) {
                                          use_ipv6, checkin, lookup,
                                          answer);
         }
-        auth_server->setIOService(io_service);
-        cout << "[b10-auth] IOService created." << endl;
+        cout << "[b10-auth] DNSServices created." << endl;
 
         cc_session = new Session(io_service.get_io_service());
         cout << "[b10-auth] Configuration session channel created." << endl;
@@ -255,11 +225,6 @@ main(int argc, char* argv[]) {
         statistics_session_established = true;
         cout << "[b10-auth] Statistics session channel established." << endl;
 
-        // XXX: with the current interface to asiolink we have to create
-        // auth_server before io_service while Session needs io_service.
-        // In a next step of refactoring we should make asiolink independent
-        // from auth_server, and create io_service, auth_server, and
-        // sessions in that order.
         auth_server->setXfrinSession(xfrin_session);
         auth_server->setStatisticsSession(statistics_session);
 
@@ -271,14 +236,6 @@ main(int argc, char* argv[]) {
         configureAuthServer(*auth_server, config_session->getFullConfig());
         auth_server->updateConfig(ElementPtr());
 
-        // create interval timer instance
-        itimer = new IntervalTimer(io_service);
-        // set up interval timer
-        // register function to send statistics with interval
-        itimer->setupTimer(boost::bind(statisticsTimerCallback, auth_server),
-                           STATISTICS_SEND_INTERVAL_SEC);
-        cout << "[b10-auth] Interval timer to send statistics set." << endl;
-
         cout << "[b10-auth] Server started." << endl;
         io_service.run();
 
@@ -296,7 +253,6 @@ main(int argc, char* argv[]) {
         xfrin_session->disconnect();
     }
 
-    delete itimer;
     delete statistics_session;
     delete xfrin_session;
     delete config_session;

src/bin/auth/query.cc

@@ -12,6 +12,9 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <vector>
+#include <boost/foreach.hpp>
+
 #include <dns/message.h>
 #include <dns/rcode.h>
 #include <dns/rdataclass.h>
@@ -28,24 +31,24 @@ namespace isc {
 namespace auth {
 
 void
-Query::getAdditional(const isc::datasrc::Zone& zone,
-                     const isc::dns::RRset& rrset) const
-{
-    if (rrset.getType() == RRType::NS()) {
-        // Need to perform the search in the "GLUE OK" mode.
-        RdataIteratorPtr rdata_iterator = rrset.getRdataIterator();
-        for (; !rdata_iterator->isLast(); rdata_iterator->next()) {
-             const Rdata& rdata(rdata_iterator->getCurrent());
-             const generic::NS& ns = dynamic_cast<const generic::NS&>(rdata);
-             findAddrs(zone, ns.getNSName(), Zone::FIND_GLUE_OK);
+Query::getAdditional(const Zone& 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);
+        } else if (rrset.getType() == RRType::MX()) {
+            const generic::MX& mx(dynamic_cast<const generic::MX&>(rdata));
+            findAddrs(zone, mx.getMXName());
         }
     }
 }
 
 void
-Query::findAddrs(const isc::datasrc::Zone& zone,
-                 const isc::dns::Name& qname,
-                 const isc::datasrc::Zone::FindOptions options) const
+Query::findAddrs(const Zone& zone, const Name& qname,
+                 const Zone::FindOptions options) const
 {
     // Out of zone name
     NameComparisonResult result = zone.getOrigin().compare(qname);
@@ -53,17 +56,31 @@ Query::findAddrs(const isc::datasrc::Zone& zone,
         (result.getRelation() != NameComparisonResult::EQUAL))
         return;
 
+    // Omit additional data which has already been provided in the answer
+    // section from the additional.
+    //
+    // All the address rrset with the owner name of qname have been inserted
+    // into ANSWER section.
+    if (qname_ == qname && qtype_ == RRType::ANY())
+        return;
+
     // Find A rrset
-    Zone::FindResult a_result = zone.find(qname, RRType::A(), options);
-    if (a_result.code == Zone::SUCCESS) {
-        response_.addRRset(Message::SECTION_ADDITIONAL,
-                     boost::const_pointer_cast<RRset>(a_result.rrset));
+    if (qname_ != qname || qtype_ != RRType::A()) {
+        Zone::FindResult a_result = zone.find(qname, RRType::A(), options);
+        if (a_result.code == Zone::SUCCESS) {
+            response_.addRRset(Message::SECTION_ADDITIONAL,
+                    boost::const_pointer_cast<RRset>(a_result.rrset));
+        }
     }
+
     // Find AAAA rrset
-    Zone::FindResult aaaa_result = zone.find(qname, RRType::AAAA(), options);
-    if (aaaa_result.code == Zone::SUCCESS) {
-        response_.addRRset(Message::SECTION_ADDITIONAL,
-                     boost::const_pointer_cast<RRset>(aaaa_result.rrset));
+    if (qname_ != qname || qtype_ != RRType::AAAA()) {
+        Zone::FindResult aaaa_result =
+            zone.find(qname, RRType::AAAA(), options);
+        if (aaaa_result.code == Zone::SUCCESS) {
+            response_.addRRset(Message::SECTION_ADDITIONAL,
+                    boost::const_pointer_cast<RRset>(aaaa_result.rrset));
+        }
     }
 }
 
@@ -86,6 +103,22 @@ Query::putSOA(const Zone& zone) const {
 }
 
 void
+Query::getAuthAdditional(const Zone& zone) const {
+    // Fill in authority and addtional sections.
+    Zone::FindResult ns_result = zone.find(zone.getOrigin(), RRType::NS());
+    // zone origin name should have NS records
+    if (ns_result.code != Zone::SUCCESS) {
+        isc_throw(NoApexNS, "There's no apex NS records in zone " <<
+                zone.getOrigin().toText());
+    } else {
+        response_.addRRset(Message::SECTION_AUTHORITY,
+            boost::const_pointer_cast<RRset>(ns_result.rrset));
+        // Handle additional for authority section
+        getAdditional(zone, *ns_result.rrset);
+    }
+}
+
+void
 Query::process() const {
     bool keep_doing = true;
     response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
@@ -112,14 +145,24 @@ Query::process() const {
             case Zone::SUCCESS:
                 response_.setRcode(Rcode::NOERROR());
                 response_.addRRset(Message::SECTION_ANSWER,
-                            boost::const_pointer_cast<RRset>(db_result.rrset));
-                // TODO : fill in authority and addtional sections.
+                    boost::const_pointer_cast<RRset>(db_result.rrset));
+                // Handle additional for answer section
+                getAdditional(*result.zone, *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() ||
+                    (qtype_ != RRType::NS() && qtype_ != RRType::ANY()))
+                {
+                    getAuthAdditional(*result.zone);
+                }
                 break;
             case Zone::DELEGATION:
                 response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
                 response_.setRcode(Rcode::NOERROR());
                 response_.addRRset(Message::SECTION_AUTHORITY,
-                            boost::const_pointer_cast<RRset>(db_result.rrset));
+                    boost::const_pointer_cast<RRset>(db_result.rrset));
                 getAdditional(*result.zone, *db_result.rrset);
                 break;
             case Zone::NXDOMAIN:
@@ -139,5 +182,6 @@ Query::process() const {
         }
     }
 }
+
 }
 }

src/bin/auth/query.h

@@ -66,15 +66,19 @@ namespace auth {
 class Query {
 private:
 
-    /// \short Adds a SOA.
+    /// \brief Adds a SOA.
     ///
     /// Adds a SOA of the zone into the authority zone of response_.
     /// Can throw NoSOA.
     ///
     void putSOA(const isc::datasrc::Zone& zone) const;
 
-    /// Look up additional data (i.e., address records for the names included
-    /// in NS or MX records).
+    /// \brief Look up additional data (i.e., address records for the names
+    /// included in NS or MX records).
+    ///
+    /// Note: Any additional data which has already been provided in the
+    /// answer section (i.e., if the original query happend to be for the
+    /// address of the DNS server), it should be omitted from the additional.
     ///
     /// This method may throw a exception because its underlying methods may
     /// throw exceptions.
@@ -86,7 +90,7 @@ private:
     void getAdditional(const isc::datasrc::Zone& zone,
                        const isc::dns::RRset& rrset) const;
 
-    /// Find address records for a specified name.
+    /// \brief Find address records for a specified name.
     ///
     /// Search the specified zone for AAAA/A RRs of each of the NS/MX RDATA
     /// (domain name), and insert the found ones into the additional section
@@ -98,7 +102,7 @@ private:
     /// The glue records must exactly match the name in the NS RDATA, without
     /// CNAME or wildcard processing.
     ///
-    /// \param zone The Zone wherein the address records is to be found.
+    /// \param zone The \c Zone wherein 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,
@@ -106,6 +110,26 @@ private:
                    const isc::datasrc::Zone::FindOptions options
                    = isc::datasrc::Zone::FIND_DEFAULT) const;
 
+    /// \brief Look up \c Zone's NS and address records for the NS RDATA
+    /// (domain name) for authoritative answer.
+    ///
+    /// On returning an authoritative answer, insert the \c Zone's NS into the
+    /// authority section and AAAA/A RRs of each of the NS RDATA into the
+    /// additional section.
+    ///
+    /// <b>Notes to developer:</b>
+    ///
+    /// We should omit address records which has already been provided in the
+    /// answer section from the additional.
+    ///
+    /// For now, in order to optimize the additional section processing, we
+    /// 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;
+
 public:
     /// Constructor from query parameters.
     ///
@@ -173,6 +197,16 @@ public:
         {}
     };
 
+    /// \short Zone is missing its apex NS records.
+    ///
+    /// We tried to add apex NS records into the authority section, but the
+    /// zone does not contain any.
+    struct NoApexNS: public BadZone {
+        NoApexNS(const char* file, size_t line, const char* what) :
+            BadZone(file, line, what)
+        {}
+    };
+
 private:
     const isc::datasrc::MemoryDataSrc& memory_datasrc_;
     const isc::dns::Name& qname_;

src/bin/auth/statistics.h

@@ -18,6 +18,7 @@
 #define __STATISTICS_H 1
 
 #include <cc/session.h>
+#include <stdint.h>
 
 class AuthCountersImpl;
 

src/bin/auth/tests/Makefile.am

@@ -2,8 +2,9 @@ AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
 AM_CPPFLAGS += -I$(top_builddir)/src/lib/dns -I$(top_srcdir)/src/bin
 AM_CPPFLAGS += -I$(top_builddir)/src/lib/cc
 AM_CPPFLAGS += $(BOOST_INCLUDES)
-AM_CPPFLAGS += -DTEST_DATA_DIR=\"$(top_srcdir)/src/lib/testutils/testdata\"
+AM_CPPFLAGS += -DTEST_DATA_DIR=\"$(abs_top_srcdir)/src/lib/testutils/testdata\"
 AM_CPPFLAGS += -DTEST_DATA_BUILDDIR=\"$(abs_top_builddir)/src/lib/testutils/testdata\"
+AM_CPPFLAGS += -DINSTALL_PROG=\"$(abs_top_srcdir)/install-sh\"
 
 AM_CXXFLAGS = $(B10_CXXFLAGS)
 
@@ -22,9 +23,11 @@ run_unittests_SOURCES += ../auth_srv.h ../auth_srv.cc
 run_unittests_SOURCES += ../query.h ../query.cc
 run_unittests_SOURCES += ../change_user.h ../change_user.cc
 run_unittests_SOURCES += ../config.h ../config.cc
+run_unittests_SOURCES += ../command.h ../command.cc
 run_unittests_SOURCES += ../statistics.h ../statistics.cc
 run_unittests_SOURCES += auth_srv_unittest.cc
 run_unittests_SOURCES += config_unittest.cc
+run_unittests_SOURCES += command_unittest.cc
 run_unittests_SOURCES += query_unittest.cc
 run_unittests_SOURCES += change_user_unittest.cc
 run_unittests_SOURCES += statistics_unittest.cc

src/bin/auth/tests/auth_srv_unittest.cc

@@ -30,6 +30,7 @@
 
 #include <datasrc/memory_datasrc.h>
 #include <auth/auth_srv.h>
+#include <auth/common.h>
 #include <auth/statistics.h>
 
 #include <dns/tests/unittest_util.h>
@@ -640,4 +641,12 @@ TEST_F(AuthSrvTest, queryCounterUnexpected) {
                                        response_obuffer, &dnsserv),
                  isc::Unexpected);
 }
+
+TEST_F(AuthSrvTest, stop) {
+    // normal case is covered in command_unittest.cc.  we should primarily
+    // test it here, but the current design of the stop test takes time,
+    // so we consolidate the cases in the command tests.
+    // If/when the interval timer has finer granularity we'll probably add
+    // our own tests here, so we keep this empty test case.
+}
 }

src/bin/auth/tests/command_unittest.cc

@@ -0,0 +1,290 @@
+// Copyright (C) 2010  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.
+
+#include <cassert>
+#include <cstdlib>
+#include <string>
+#include <stdexcept>
+
+#include <boost/bind.hpp>
+
+#include <gtest/gtest.h>
+
+#include <dns/name.h>
+#include <dns/rrclass.h>
+#include <dns/rrtype.h>
+
+#include <cc/data.h>
+
+#include <config/ccsession.h>
+
+#include <datasrc/memory_datasrc.h>
+
+#include <auth/auth_srv.h>
+#include <auth/config.h>
+#include <auth/command.h>
+
+#include <asiolink/asiolink.h>
+
+#include <testutils/mockups.h>
+
+using namespace std;
+using namespace isc::dns;
+using namespace isc::data;
+using namespace isc::datasrc;
+using namespace isc::config;
+
+namespace {
+class AuthConmmandTest : public ::testing::Test {
+protected:
+    AuthConmmandTest() : server(false, xfrout), rcode(-1) {
+        server.setStatisticsSession(&statistics_session);
+    }
+    void checkAnswer(const int expected_code) {
+        parseAnswer(rcode, result);
+        EXPECT_EQ(expected_code, rcode);
+    }
+    MockSession statistics_session;
+    MockXfroutClient xfrout;
+    AuthSrv server;
+    AuthSrv::ConstMemoryDataSrcPtr memory_datasrc;
+    ConstElementPtr result;
+    int rcode;
+public:
+    void stopServer();          // need to be public for boost::bind
+};
+
+TEST_F(AuthConmmandTest, unknownCommand) {
+    result = execAuthServerCommand(server, "no_such_command",
+                                   ConstElementPtr());
+    parseAnswer(rcode, result);
+    EXPECT_EQ(1, rcode);
+}
+
+TEST_F(AuthConmmandTest, DISABLED_unexpectedException) {
+    // execAuthServerCommand() won't catch standard exceptions.
+    // Skip this test for now: ModuleCCSession doesn't seem to validate
+    // commands.
+    EXPECT_THROW(execAuthServerCommand(server, "_throw_exception",
+                                       ConstElementPtr()),
+                 runtime_error);
+}
+
+TEST_F(AuthConmmandTest, sendStatistics) {
+    result = execAuthServerCommand(server, "sendstats", ConstElementPtr());
+    // Just check some message has been sent.  Detailed tests specific to
+    // statistics are done in its own tests.
+    EXPECT_EQ("Stats", statistics_session.getMessageDest());
+    checkAnswer(0);
+}
+
+void
+AuthConmmandTest::stopServer() {
+    result = execAuthServerCommand(server, "shutdown", ConstElementPtr());
+    parseAnswer(rcode, result);
+    assert(rcode == 0); // make sure the test stops when something is wrong
+}
+
+TEST_F(AuthConmmandTest, shutdown) {
+    asiolink::IntervalTimer itimer(server.getIOService());
+    itimer.setupTimer(boost::bind(&AuthConmmandTest::stopServer, this), 1);
+    server.getIOService().run();
+    EXPECT_EQ(0, rcode);
+}
+
+// A helper function commonly used for the "loadzone" command tests.
+// It configures the server with a memory data source containing two
+// 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->
+              find(Name("ns.test1.example"), RRType::A()).code);
+    EXPECT_EQ(Zone::NXRRSET, server.getMemoryDataSrc(RRClass::IN())->
+              findZone(Name("ns.test1.example")).zone->
+              find(Name("ns.test1.example"), RRType::AAAA()).code);
+    EXPECT_EQ(Zone::SUCCESS, server.getMemoryDataSrc(RRClass::IN())->
+              findZone(Name("ns.test2.example")).zone->
+              find(Name("ns.test2.example"), RRType::A()).code);
+    EXPECT_EQ(Zone::NXRRSET, server.getMemoryDataSrc(RRClass::IN())->
+              findZone(Name("ns.test2.example")).zone->
+              find(Name("ns.test2.example"), RRType::AAAA()).code);
+}
+
+void
+configureZones(AuthSrv& server) {
+    ASSERT_EQ(0, system(INSTALL_PROG " " TEST_DATA_DIR "/test1.zone.in "
+                        TEST_DATA_BUILDDIR "/test1.zone.copied"));
+    ASSERT_EQ(0, system(INSTALL_PROG " " TEST_DATA_DIR "/test2.zone.in "
+                        TEST_DATA_BUILDDIR "/test2.zone.copied"));
+    configureAuthServer(server, Element::fromJSON(
+                            "{\"datasources\": "
+                            " [{\"type\": \"memory\","
+                            "   \"zones\": "
+                            "[{\"origin\": \"test1.example\","
+                            "  \"file\": \""
+                               TEST_DATA_BUILDDIR "/test1.zone.copied\"},"
+                            " {\"origin\": \"test2.example\","
+                            "  \"file\": \""
+                               TEST_DATA_BUILDDIR "/test2.zone.copied\"}"
+                            "]}]}"));
+    zoneChecks(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->
+              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->
+              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->
+              find(Name("ns.test2.example"), RRType::A()).code);
+    EXPECT_EQ(Zone::NXRRSET, server.getMemoryDataSrc(RRClass::IN())->
+              findZone(Name("ns.test2.example")).zone->
+              find(Name("ns.test2.example"), RRType::AAAA()).code);
+}
+
+TEST_F(AuthConmmandTest, loadZone) {
+    configureZones(server);
+
+    ASSERT_EQ(0, system(INSTALL_PROG " " TEST_DATA_DIR
+                        "/test1-new.zone.in "
+                        TEST_DATA_BUILDDIR "/test1.zone.copied"));
+    ASSERT_EQ(0, system(INSTALL_PROG " " TEST_DATA_DIR
+                        "/test2-new.zone.in "
+                        TEST_DATA_BUILDDIR "/test2.zone.copied"));
+
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON(
+                                       "{\"origin\": \"test1.example\"}"));
+    checkAnswer(0);
+    newZoneChecks(server);
+}
+
+TEST_F(AuthConmmandTest, loadBrokenZone) {
+    configureZones(server);
+
+    ASSERT_EQ(0, system(INSTALL_PROG " " TEST_DATA_DIR
+                        "/test1-broken.zone.in "
+                        TEST_DATA_BUILDDIR "/test1.zone.copied"));
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON(
+                                       "{\"origin\": \"test1.example\"}"));
+    checkAnswer(1);
+    zoneChecks(server);     // zone shouldn't be replaced
+}
+
+TEST_F(AuthConmmandTest, loadUnreadableZone) {
+    configureZones(server);
+
+    // install the zone file as unreadable
+    ASSERT_EQ(0, system(INSTALL_PROG " -m 000 " TEST_DATA_DIR
+                        "/test1.zone.in "
+                        TEST_DATA_BUILDDIR "/test1.zone.copied"));
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON(
+                                       "{\"origin\": \"test1.example\"}"));
+    checkAnswer(1);
+    zoneChecks(server);     // zone shouldn't be replaced
+}
+
+TEST_F(AuthConmmandTest, loadZoneWithoutDataSrc) {
+    // try to execute load command without configuring the zone beforehand.
+    // it should fail.
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON(
+                                       "{\"origin\": \"test1.example\"}"));
+    checkAnswer(1);
+}
+
+TEST_F(AuthConmmandTest, loadSqlite3DataSrc) {
+    // For sqlite3 data source we don't have to do anything (the data source
+    // (re)loads itself automatically)
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON(
+                                       "{\"origin\": \"test1.example\","
+                                       " \"datasrc\": \"sqlite3\"}"));
+    checkAnswer(0);
+}
+
+TEST_F(AuthConmmandTest, loadZoneInvalidParams) {
+    configureZones(server);
+
+    // null arg
+    result = execAuthServerCommand(server, "loadzone", ElementPtr());
+    checkAnswer(1);
+
+    // zone class is bogus
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON(
+                                       "{\"origin\": \"test1.example\","
+                                       " \"class\": \"no_such_class\"}"));
+    checkAnswer(1);
+
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON(
+                                       "{\"origin\": \"test1.example\","
+                                       " \"class\": 1}"));
+    checkAnswer(1);
+
+    // unsupported zone class
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON(
+                                       "{\"origin\": \"test1.example\","
+                                       " \"class\": \"CH\"}"));
+    checkAnswer(1);
+
+    // unsupported data source class
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON(
+                                       "{\"origin\": \"test1.example\","
+                                       " \"datasrc\": \"not supported\"}"));
+    checkAnswer(1);
+
+    // data source is bogus
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON(
+                                       "{\"origin\": \"test1.example\","
+                                       " \"datasrc\": 0}"));
+    checkAnswer(1);
+
+    // origin is missing
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON("{}"));
+    checkAnswer(1);
+
+    // zone doesn't exist in the data source
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON("{\"origin\": \"xx\"}"));
+    checkAnswer(1);
+
+    // origin is bogus
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON(
+                                       "{\"origin\": \"...\"}"));
+    checkAnswer(1);
+
+    result = execAuthServerCommand(server, "loadzone",
+                                   Element::fromJSON("{\"origin\": 10}"));
+    checkAnswer(1);
+}
+}

src/bin/auth/tests/config_unittest.cc

@@ -34,6 +34,7 @@
 using namespace isc::dns;
 using namespace isc::data;
 using namespace isc::datasrc;
+using namespace asiolink;
 
 namespace {
 class AuthConfigTest : public ::testing::Test {
@@ -320,4 +321,49 @@ TEST_F(MemoryDatasrcConfigTest, badDatasrcType) {
                                                  " {\"type\": \"memory\"}]")),
                  AuthConfigError);
 }
+
+class StatisticsIntervalConfigTest : public AuthConfigTest {
+protected:
+    StatisticsIntervalConfigTest() :
+        parser(createAuthConfigParser(server, "statistics-interval"))
+    {}
+    ~StatisticsIntervalConfigTest() {
+        delete parser;
+    }
+    AuthConfigParser* parser;
+};
+
+TEST_F(StatisticsIntervalConfigTest, setInterval) {
+    // initially the timer is not configured.
+    EXPECT_EQ(0, server.getStatisticsTimerInterval());
+
+    // initialize the timer
+    parser->build(Element::fromJSON("5"));
+    parser->commit();
+    EXPECT_EQ(5, server.getStatisticsTimerInterval());
+
+    // reset the timer with a new interval
+    delete parser;
+    parser = createAuthConfigParser(server, "statistics-interval");
+    ASSERT_NE(static_cast<void*>(NULL), parser);
+    parser->build(Element::fromJSON("10"));
+    parser->commit();
+    EXPECT_EQ(10, server.getStatisticsTimerInterval());
+
+    // disable the timer again
+    delete parser;
+    parser = createAuthConfigParser(server, "statistics-interval");
+    ASSERT_NE(static_cast<void*>(NULL), parser);
+    parser->build(Element::fromJSON("0"));
+    parser->commit();
+    EXPECT_EQ(0, server.getStatisticsTimerInterval());
+}
+
+TEST_F(StatisticsIntervalConfigTest, badInterval) {
+    EXPECT_THROW(parser->build(Element::fromJSON("\"should be integer\"")),
+                 isc::data::TypeError);
+    EXPECT_THROW(parser->build(Element::fromJSON("2.5")),
+                 isc::data::TypeError);
+    EXPECT_THROW(parser->build(Element::fromJSON("-1")), AuthConfigError);
+}
 }

src/bin/auth/tests/query_unittest.cc

@@ -47,26 +47,34 @@ RRsetPtr glue_aaaa_rrset(RRsetPtr(new RRset(Name("glue.ns.example.com"),
                                             RRClass::IN(), RRType::AAAA(),
                                             RRTTL(3600))));
 RRsetPtr noglue_a_rrset(RRsetPtr(new RRset(Name("noglue.example.com"),
-                                         RRClass::IN(), RRType::A(),
-                                         RRTTL(3600))));
+                                           RRClass::IN(), RRType::A(),
+                                           RRTTL(3600))));
+RRsetPtr delegated_mx_a_rrset(RRsetPtr(new RRset(
+    Name("mx.delegation.example.com"), RRClass::IN(), RRType::A(),
+    RRTTL(3600))));
+
 // This is a mock Zone class for testing.
-// It is a derived class of Zone, and simply hardcode the results of find()
-// return SUCCESS for "www.example.com",
-// return NXDOMAIN for "nxdomain.example.com",
-// return NXRRSET for "nxrrset.example.com",
-// return CNAME for "cname.example.com",
-// otherwise return DNAME
+// It is a derived class of Zone, and simply hardcodes the results of find()
+// See the find() implementation if you want to know its content.
 class MockZone : public Zone {
 public:
-    MockZone(bool has_SOA = true) :
+    MockZone(bool has_SOA = true, bool has_apex_NS = true) :
         origin_(Name("example.com")),
         has_SOA_(has_SOA),
+        has_apex_NS_(has_apex_NS),
         delegation_rrset(RRsetPtr(new RRset(Name("delegation.example.com"),
                                             RRClass::IN(), RRType::NS(),
                                             RRTTL(3600)))),
         cname_rrset(RRsetPtr(new RRset(Name("cname.example.com"),
                                        RRClass::IN(), RRType::CNAME(),
-                                       RRTTL(3600))))
+                                       RRTTL(3600)))),
+        auth_ns_rrset(RRsetPtr(new RRset(Name("example.com"),
+                                         RRClass::IN(), RRType::NS(),
+                                         RRTTL(3600)))),
+        mx_cname_rrset_(new RRset(Name("cnamemailer.example.com"),
+            RRClass::IN(), RRType::CNAME(), RRTTL(3600))),
+        mx_rrset_(new RRset(Name("mx.example.com"), RRClass::IN(),
+            RRType::MX(), RRTTL(3600)))
     {
         delegation_rrset->addRdata(rdata::generic::NS(
                           Name("glue.ns.example.com")));
@@ -78,6 +86,20 @@ public:
                           Name("example.org")));
         cname_rrset->addRdata(rdata::generic::CNAME(
                           Name("www.example.com")));
+        auth_ns_rrset->addRdata(rdata::generic::NS(
+                          Name("glue.ns.example.com")));
+        auth_ns_rrset->addRdata(rdata::generic::NS(
+                          Name("noglue.example.com")));
+        auth_ns_rrset->addRdata(rdata::generic::NS(
+                          Name("example.net")));
+        mx_rrset_->addRdata(isc::dns::rdata::generic::MX(10,
+            Name("www.example.com")));
+        mx_rrset_->addRdata(isc::dns::rdata::generic::MX(20,
+            Name("mailer.example.org")));
+        mx_rrset_->addRdata(isc::dns::rdata::generic::MX(30,
+            Name("mx.delegation.example.com")));
+        mx_cname_rrset_->addRdata(rdata::generic::CNAME(
+            Name("mx.example.com")));
     }
     virtual const isc::dns::Name& getOrigin() const;
     virtual const isc::dns::RRClass& getClass() const;
@@ -89,8 +111,12 @@ public:
 private:
     Name origin_;
     bool has_SOA_;
+    bool has_apex_NS_;
     RRsetPtr delegation_rrset;
     RRsetPtr cname_rrset;
+    RRsetPtr auth_ns_rrset;
+    RRsetPtr mx_cname_rrset_;
+    RRsetPtr mx_rrset_;
 };
 
 const Name&
@@ -108,21 +134,35 @@ MockZone::find(const Name& name, const RRType& type,
                const FindOptions options) const
 {
     // hardcode the find results
-    if (name == Name("www.example.com")) {
+    if (name == Name("www.example.com") && type == RRType::A()) {
         return (FindResult(SUCCESS, a_rrset));
+    } else if (name == Name("www.example.com")) {
+        return (FindResult(NXRRSET, RRsetPtr()));
     } else if (name == Name("glue.ns.example.com") && type == RRType::A() &&
-        options == FIND_GLUE_OK) {
+        (options & FIND_GLUE_OK) != 0) {
         return (FindResult(SUCCESS, glue_a_rrset));
-    } else if (name == Name("noglue.example.com") && type == RRType::A()) {
+    } else if (name == Name("noglue.example.com") && (type == RRType::A() ||
+        type == RRType::ANY())) {
         return (FindResult(SUCCESS, noglue_a_rrset));
     } else if (name == Name("glue.ns.example.com") && type == RRType::AAAA() &&
-        options == FIND_GLUE_OK) {
+        (options & FIND_GLUE_OK) != 0) {
         return (FindResult(SUCCESS, glue_aaaa_rrset));
     } else if (name == Name("example.com") && type == RRType::SOA() &&
         has_SOA_)
     {
         return (FindResult(SUCCESS, soa_rrset));
-    } else if (name == Name("delegation.example.com")) {
+    } else if (name == Name("example.com") && type == RRType::NS() &&
+        has_apex_NS_)
+    {
+        return (FindResult(SUCCESS, auth_ns_rrset));
+    } else if (name == Name("mx.delegation.example.com") &&
+        type == RRType::A() && (options & FIND_GLUE_OK) != 0)
+    {
+        return (FindResult(SUCCESS, delegated_mx_a_rrset));
+    } else if (name == Name("delegation.example.com") ||
+        name.compare(Name("delegation.example.com")).getRelation() ==
+        NameComparisonResult::SUBDOMAIN)
+    {
         return (FindResult(DELEGATION, delegation_rrset));
     } else if (name == Name("ns.example.com")) {
         return (FindResult(DELEGATION, ns_rrset));
@@ -132,6 +172,10 @@ MockZone::find(const Name& name, const RRType& type,
         return (FindResult(NXRRSET, RRsetPtr()));
     } else if ((name == Name("cname.example.com"))) {
         return (FindResult(CNAME, cname_rrset));
+    } else if (name == Name("cnamemailer.example.com")) {
+        return (FindResult(CNAME, mx_cname_rrset_));
+    } else if (name == Name("mx.example.com")) {
+        return (FindResult(SUCCESS, mx_rrset_));
     } else {
         return (FindResult(DNAME, RRsetPtr()));
     }
@@ -157,24 +201,130 @@ protected:
 TEST_F(QueryTest, noZone) {
     // There's no zone in the memory datasource.  So the response should have
     // REFUSED.
-    query.process();
+    EXPECT_NO_THROW(query.process());
     EXPECT_EQ(Rcode::REFUSED(), response.getRcode());
 }
 
-TEST_F(QueryTest, matchZone) {
+TEST_F(QueryTest, exactMatch) {
     // add a matching zone.
     memory_datasrc.addZone(ZonePtr(new MockZone()));
-    query.process();
+    EXPECT_NO_THROW(query.process());
+    // find match rrset
     EXPECT_TRUE(response.getHeaderFlag(Message::HEADERFLAG_AA));
     EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
     EXPECT_TRUE(response.hasRRset(Message::SECTION_ANSWER,
                                   Name("www.example.com"), RRClass::IN(),
                                   RRType::A()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY,
+                                  Name("example.com"), RRClass::IN(),
+                                  RRType::NS()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::A()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::AAAA()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("noglue.example.com"),
+                                  RRClass::IN(), RRType::A()));
+}
+
+TEST_F(QueryTest, exactAddrMatch) {
+    // find match rrset, omit additional data which has already been provided
+    // in the answer section from the additional.
+    memory_datasrc.addZone(ZonePtr(new MockZone()));
+    const Name noglue_name(Name("noglue.example.com"));
+    Query noglue_query(memory_datasrc, noglue_name, qtype, response);
+    EXPECT_NO_THROW(noglue_query.process());
+    EXPECT_TRUE(response.getHeaderFlag(Message::HEADERFLAG_AA));
+    EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ANSWER,
+                                  Name("noglue.example.com"), RRClass::IN(),
+                                  RRType::A()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY,
+                                  Name("example.com"), RRClass::IN(),
+                                  RRType::NS()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::A()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::AAAA()));
+    EXPECT_FALSE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("noglue.example.com"),
+                                  RRClass::IN(), RRType::A()));
+}
+
+TEST_F(QueryTest, apexNSMatch) {
+    // find match rrset, omit authority data which has already been provided
+    // in the answer section from the authority section.
+    memory_datasrc.addZone(ZonePtr(new MockZone()));
+    const Name apex_name(Name("example.com"));
+    Query apex_ns_query(memory_datasrc, apex_name, RRType::NS(), response);
+    EXPECT_NO_THROW(apex_ns_query.process());
+    EXPECT_TRUE(response.getHeaderFlag(Message::HEADERFLAG_AA));
+    EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ANSWER,
+                                  Name("example.com"), RRClass::IN(),
+                                  RRType::NS()));
+    EXPECT_FALSE(response.hasRRset(Message::SECTION_AUTHORITY,
+                                  Name("example.com"), RRClass::IN(),
+                                  RRType::NS()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::A()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::AAAA()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("noglue.example.com"),
+                                  RRClass::IN(), RRType::A()));
+}
+
+TEST_F(QueryTest, exactAnyMatch) {
+    // find match rrset, omit additional data which has already been provided
+    // in the answer section from the additional.
+    memory_datasrc.addZone(ZonePtr(new MockZone()));
+    const Name noglue_name(Name("noglue.example.com"));
+    Query noglue_query(memory_datasrc, noglue_name, RRType::ANY(), response);
+    EXPECT_NO_THROW(noglue_query.process());
+    EXPECT_TRUE(response.getHeaderFlag(Message::HEADERFLAG_AA));
+    EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ANSWER,
+                                  Name("noglue.example.com"), RRClass::IN(),
+                                  RRType::A()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY,
+                                  Name("example.com"), RRClass::IN(),
+                                  RRType::NS()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::A()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::AAAA()));
+    EXPECT_FALSE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("noglue.example.com"),
+                                  RRClass::IN(), RRType::A()));
+}
+
+// This tests that when we need to look up Zone's apex NS records for
+// authoritative answer, and there is no apex NS records. It should
+// throw in that case.
+TEST_F(QueryTest, noApexNS) {
+    // Add a zone without apex NS records
+    memory_datasrc.addZone(ZonePtr(new MockZone(true, false)));
+    const Name noglue_name(Name("noglue.example.com"));
+    Query noglue_query(memory_datasrc, noglue_name, qtype, response);
+    EXPECT_THROW(noglue_query.process(), Query::NoApexNS);
+    // We don't look into the response, as it throwed
+}
 
-    // Delegation
+TEST_F(QueryTest, delegation) {
+    // add a matching zone.
+    memory_datasrc.addZone(ZonePtr(new MockZone()));
     const Name delegation_name(Name("delegation.example.com"));
     Query delegation_query(memory_datasrc, delegation_name, qtype, response);
-    delegation_query.process();
+    EXPECT_NO_THROW(delegation_query.process());
     EXPECT_FALSE(response.getHeaderFlag(Message::HEADERFLAG_AA));
     EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
     EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY,
@@ -199,21 +349,27 @@ TEST_F(QueryTest, matchZone) {
     EXPECT_FALSE(response.hasRRset(Message::SECTION_ADDITIONAL,
                                   Name("example.org"),
                                   RRClass::IN(), RRType::A()));
+}
 
-    // NXDOMAIN
+TEST_F(QueryTest, nxdomain) {
+    // add a matching zone.
+    memory_datasrc.addZone(ZonePtr(new MockZone()));
     const Name nxdomain_name(Name("nxdomain.example.com"));
     Query nxdomain_query(memory_datasrc, nxdomain_name, qtype, response);
-    nxdomain_query.process();
+    EXPECT_NO_THROW(nxdomain_query.process());
     EXPECT_EQ(Rcode::NXDOMAIN(), response.getRcode());
     EXPECT_EQ(0, response.getRRCount(Message::SECTION_ANSWER));
     EXPECT_EQ(0, response.getRRCount(Message::SECTION_ADDITIONAL));
     EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY,
         Name("example.com"), RRClass::IN(), RRType::SOA()));
+}
 
-    // NXRRSET
+TEST_F(QueryTest, nxrrset) {
+    // add a matching zone.
+    memory_datasrc.addZone(ZonePtr(new MockZone()));
     const Name nxrrset_name(Name("nxrrset.example.com"));
     Query nxrrset_query(memory_datasrc, nxrrset_name, qtype, response);
-    nxrrset_query.process();
+    EXPECT_NO_THROW(nxrrset_query.process());
     EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
     EXPECT_EQ(0, response.getRRCount(Message::SECTION_ANSWER));
     EXPECT_EQ(0, response.getRRCount(Message::SECTION_ADDITIONAL));
@@ -250,4 +406,83 @@ TEST_F(QueryTest, noMatchZone) {
     EXPECT_EQ(Rcode::REFUSED(), response.getRcode());
 }
 
+/*
+ * Test MX additional processing.
+ *
+ * The MX RRset has two RRs, one pointing to a known domain with
+ * A record, other to unknown out of zone one.
+ */
+TEST_F(QueryTest, MX) {
+    memory_datasrc.addZone(ZonePtr(new MockZone()));
+    Name qname("mx.example.com");
+    Query mx_query(memory_datasrc, qname, RRType::MX(), response);
+    EXPECT_NO_THROW(mx_query.process());
+    EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ANSWER,
+        Name("mx.example.com"), RRClass::IN(), RRType::MX()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+        Name("www.example.com"), RRClass::IN(), RRType::A()));
+    // We want to skip the additional ones related to authoritative
+    RRsetPtr ns;
+    for (SectionIterator<RRsetPtr> ai(response.beginSection(
+        Message::SECTION_AUTHORITY)); ai != response.endSection(
+        Message::SECTION_AUTHORITY); ++ai)
+    {
+        if ((*ai)->getName() == Name("example.com") && (*ai)->getType() ==
+            RRType::NS())
+        {
+            ns = *ai;
+            break;
+        }
+    }
+    /*
+     * In fact, the MX RRset mentions three names, but we don't know anything
+     * about one of them and one is under a zone cut, so we should have just
+     * one RRset (A for www.example.com)
+     */
+    // We can't use getRRCount, as it counts RRs, not RRsets
+    unsigned additional_count(0);
+    for (SectionIterator<RRsetPtr> ai(response.beginSection(
+        Message::SECTION_ADDITIONAL)); ai != response.endSection(
+        Message::SECTION_ADDITIONAL); ++ai)
+    {
+        // Skip the ones for the NS record
+        if (ns) {
+            for (RdataIteratorPtr nsi(ns->getRdataIterator()); !nsi->isLast();
+                nsi->next())
+            {
+                if ((*ai)->getName() ==
+                    dynamic_cast<const isc::dns::rdata::generic::NS&>(
+                    nsi->getCurrent()).getNSName())
+                {
+                    goto NS_ADDITIONAL_DATA;
+                }
+            }
+        }
+        // It is not related to the NS, then it must be related to the MX
+        ++additional_count;
+        EXPECT_EQ(Name("www.example.com"), (*ai)->getName());
+        EXPECT_EQ(RRType::A(), (*ai)->getType());
+        NS_ADDITIONAL_DATA:;
+    }
+    EXPECT_EQ(1, additional_count);
+}
+
+/*
+ * Test when we ask for MX and encounter an alias (CNAME in this case).
+ *
+ * This should not trigger the additional processing.
+ */
+TEST_F(QueryTest, MXAlias) {
+    memory_datasrc.addZone(ZonePtr(new MockZone()));
+    Name qname("cnamemailer.example.com");
+    Query mx_query(memory_datasrc, qname, RRType::MX(), response);
+    EXPECT_NO_THROW(mx_query.process());
+    EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
+    // We should not have the IP address in additional section
+    // Currently, the section should be completely empty
+    EXPECT_TRUE(response.beginSection(Message::SECTION_ADDITIONAL) ==
+        response.endSection(Message::SECTION_ADDITIONAL));
+}
+
 }

src/bin/bind10/bind10.8

@@ -1,7 +1,7 @@
 '\" t
 .\"     Title: bind10
 .\"    Author: [see the "AUTHORS" section]
-.\" Generator: DocBook XSL Stylesheets v1.76.0 <http://docbook.sf.net/>
+.\" Generator: DocBook XSL Stylesheets v1.75.2 <http://docbook.sf.net/>
 .\"      Date: July 29, 2010
 .\"    Manual: BIND10
 .\"    Source: BIND10
@@ -9,15 +9,6 @@
 .\"
 .TH "BIND10" "8" "July 29, 2010" "BIND10" "BIND10"
 .\" -----------------------------------------------------------------
-.\" * Define some portability stuff
-.\" -----------------------------------------------------------------
-.\" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-.\" http://bugs.debian.org/507673
-.\" http://lists.gnu.org/archive/html/groff/2009-02/msg00013.html
-.\" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-.ie \n(.g .ds Aq \(aq
-.el       .ds Aq '
-.\" -----------------------------------------------------------------
 .\" * set default formatting
 .\" -----------------------------------------------------------------
 .\" disable hyphenation

src/bin/bind10/bind10.py.in

@@ -208,13 +208,13 @@ class BoB:
         self.dns_port = dns_port
         self.forward = forward
         if forward:
-            self.recursive = True
+            self.resolver = True
         else:
-            self.recursive = False
+            self.resolver = False
         self.cc_session = None
         self.ccs = None
         self.cfg_start_auth = True
-        self.cfg_start_recurse = False
+        self.cfg_start_resolver = False
         self.curproc = None
         self.dead_processes = {}
         self.msgq_socket_file = msgq_socket_file
@@ -278,13 +278,13 @@ class BoB:
 
         config_data = self.ccs.get_full_config()
         self.cfg_start_auth = config_data.get("start_auth")
-        self.cfg_start_recurse = config_data.get("start_recurse")
+        self.cfg_start_resolver = config_data.get("start_resolver")
 
         if self.verbose:
             sys.stdout.write("[bind10] - start_auth: %s\n" %
                 str(self.cfg_start_auth))
-            sys.stdout.write("[bind10] - start_recurse: %s\n" %
-                str(self.cfg_start_recurse))
+            sys.stdout.write("[bind10] - start_resolver: %s\n" %
+                str(self.cfg_start_resolver))
 
     def log_starting(self, process, port = None, address = None):
         """
@@ -423,13 +423,13 @@ class BoB:
             Start the Authoritative server
         """
         # XXX: this must be read from the configuration manager in the future
-        if self.recursive:
-            dns_prog = 'b10-recurse'
+        if self.resolver:
+            dns_prog = 'b10-resolver'
         else:
             dns_prog = 'b10-auth'
         dnsargs = [dns_prog]
-        if not self.recursive:
-            # The recursive uses configuration manager for these
+        if not self.resolver:
+            # The resolver uses configuration manager for these
             dnsargs += ['-p', str(self.dns_port)]
             if self.address:
                 dnsargs += ['-a', str(self.address)]
@@ -444,22 +444,22 @@ class BoB:
         self.start_process("b10-auth", dnsargs, c_channel_env,
             self.dns_port, self.address)
 
-    def start_recurse(self, c_channel_env):
+    def start_resolver(self, c_channel_env):
         """
             Start the Resolver.  At present, all these arguments and switches
             are pure speculation.  As with the auth daemon, they should be
             read from the configuration database.
         """
-        self.curproc = "b10-recurse"
+        self.curproc = "b10-resolver"
         # XXX: this must be read from the configuration manager in the future
-        resargs = ['b10-recurse']
+        resargs = ['b10-resolver']
         if self.uid:
             resargs += ['-u', str(self.uid)]
         if self.verbose:
             resargs += ['-v']
 
         # ... and start
-        self.start_process("b10-recurse", resargs, c_channel_env)
+        self.start_process("b10-resolver", resargs, c_channel_env)
 
     def start_xfrout(self, c_channel_env):
         self.start_simple("b10-xfrout", c_channel_env)
@@ -496,8 +496,8 @@ class BoB:
             self.start_auth(c_channel_env)
 
         # ... and resolver (if selected):
-        if self.cfg_start_recurse:
-            self.start_recurse(c_channel_env)
+        if self.cfg_start_resolver:
+            self.start_resolver(c_channel_env)
 
         # Everything after the main components can run as non-root.
         # TODO: this is only temporary - once the privileged socket creator is
@@ -557,7 +557,7 @@ class BoB:
         self.cc_session.group_sendmsg(cmd, 'Cmdctl', 'Cmdctl')
         self.cc_session.group_sendmsg(cmd, "ConfigManager", "ConfigManager")
         self.cc_session.group_sendmsg(cmd, "Auth", "Auth")
-        self.cc_session.group_sendmsg(cmd, "Recurse", "Recurse")
+        self.cc_session.group_sendmsg(cmd, "Resolver", "Resolver")
         self.cc_session.group_sendmsg(cmd, "Xfrout", "Xfrout")
         self.cc_session.group_sendmsg(cmd, "Xfrin", "Xfrin")
         self.cc_session.group_sendmsg(cmd, "Zonemgr", "Zonemgr")

src/bin/bind10/bob.spec

@@ -10,7 +10,7 @@
         "item_default": true
       },
       {
-        "item_name": "start_recurse",
+        "item_name": "start_resolver",
         "item_type": "boolean",
         "item_optional": false,
         "item_default": false

src/bin/bind10/run_bind10.sh.in

@@ -20,7 +20,7 @@ export PYTHON_EXEC
 
 BIND10_PATH=@abs_top_builddir@/src/bin/bind10
 
-PATH=@abs_top_builddir@/src/bin/msgq:@abs_top_builddir@/src/bin/auth:@abs_top_builddir@/src/bin/recurse:@abs_top_builddir@/src/bin/cfgmgr:@abs_top_builddir@/src/bin/cmdctl:@abs_top_builddir@/src/bin/stats:@abs_top_builddir@/src/bin/xfrin:@abs_top_builddir@/src/bin/xfrout:@abs_top_builddir@/src/bin/zonemgr:$PATH
+PATH=@abs_top_builddir@/src/bin/msgq:@abs_top_builddir@/src/bin/auth:@abs_top_builddir@/src/bin/resolver:@abs_top_builddir@/src/bin/cfgmgr:@abs_top_builddir@/src/bin/cmdctl:@abs_top_builddir@/src/bin/stats:@abs_top_builddir@/src/bin/xfrin:@abs_top_builddir@/src/bin/xfrout:@abs_top_builddir@/src/bin/zonemgr:$PATH
 export PATH
 
 PYTHONPATH=@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/xfr/.libs:@abs_top_builddir@/src/lib/log/.libs

src/bin/bind10/tests/bind10_test.py

@@ -89,7 +89,7 @@ class TestBoB(unittest.TestCase):
         self.assertEqual(bob.username, None)
         self.assertEqual(bob.nocache, False)
         self.assertEqual(bob.cfg_start_auth, True)
-        self.assertEqual(bob.cfg_start_recurse, False)
+        self.assertEqual(bob.cfg_start_resolver, False)
 
     def test_init_alternate_socket(self):
         bob = BoB("alt_socket_file")
@@ -106,7 +106,7 @@ class TestBoB(unittest.TestCase):
         self.assertEqual(bob.username, None)
         self.assertEqual(bob.nocache, False)
         self.assertEqual(bob.cfg_start_auth, True)
-        self.assertEqual(bob.cfg_start_recurse, False)
+        self.assertEqual(bob.cfg_start_resolver, False)
 
     def test_init_alternate_dns_port(self):
         bob = BoB(None, 9999)
@@ -123,7 +123,7 @@ class TestBoB(unittest.TestCase):
         self.assertEqual(bob.username, None)
         self.assertEqual(bob.nocache, False)
         self.assertEqual(bob.cfg_start_auth, True)
-        self.assertEqual(bob.cfg_start_recurse, False)
+        self.assertEqual(bob.cfg_start_resolver, False)
 
     def test_init_alternate_address(self):
         bob = BoB(None, 1234, IPAddr('127.127.127.127'))
@@ -140,7 +140,7 @@ class TestBoB(unittest.TestCase):
         self.assertEqual(bob.username, None)
         self.assertEqual(bob.nocache, False)
         self.assertEqual(bob.cfg_start_auth, True)
-        self.assertEqual(bob.cfg_start_recurse, False)
+        self.assertEqual(bob.cfg_start_resolver, False)
 
 # Class for testing the Bob.start_all_processes() method call.
 #
@@ -157,7 +157,7 @@ class StartAllProcessesBob(BoB):
         self.cfgmgr = False
         self.ccsession = False
         self.auth = False
-        self.recurse = False
+        self.resolver = False
         self.xfrout = False
         self.xfrin = False
         self.zonemgr = False
@@ -180,8 +180,8 @@ class StartAllProcessesBob(BoB):
     def start_auth(self, c_channel_env):
         self.auth = True
 
-    def start_recurse(self, c_channel_env):
-        self.recurse = True
+    def start_resolver(self, c_channel_env):
+        self.resolver = True
 
     def start_xfrout(self, c_channel_env):
         self.xfrout = True
@@ -206,14 +206,14 @@ class TestStartAllProcessesBob(unittest.TestCase):
         self.assertEqual(bob.cfgmgr, False)
         self.assertEqual(bob.ccsession, False)
         self.assertEqual(bob.auth, False)
-        self.assertEqual(bob.recurse, False)
+        self.assertEqual(bob.resolver, False)
         self.assertEqual(bob.xfrout, False)
         self.assertEqual(bob.xfrin, False)
         self.assertEqual(bob.zonemgr, False)
         self.assertEqual(bob.stats, False)
         self.assertEqual(bob.cmdctl, False)
 
-    # Checks the processes started when starting neither auth nor recurse
+    # Checks the processes started when starting neither auth nor resolver
     # is specified.
     def test_start_none(self):
         # Created Bob and ensure initialization correct
@@ -223,7 +223,7 @@ class TestStartAllProcessesBob(unittest.TestCase):
         # Start processes and check what was started
         c_channel_env = {}
         bob.cfg_start_auth = False
-        bob.cfg_start_recurse = False
+        bob.cfg_start_resolver = False
 
         bob.start_all_processes(c_channel_env)
 
@@ -231,7 +231,7 @@ class TestStartAllProcessesBob(unittest.TestCase):
         self.assertEqual(bob.cfgmgr, True)
         self.assertEqual(bob.ccsession, True)
         self.assertEqual(bob.auth, False)
-        self.assertEqual(bob.recurse, False)
+        self.assertEqual(bob.resolver, False)
         self.assertEqual(bob.xfrout, False)
         self.assertEqual(bob.xfrin, False)
         self.assertEqual(bob.zonemgr, False)
@@ -247,7 +247,7 @@ class TestStartAllProcessesBob(unittest.TestCase):
         # Start processes and check what was started
         c_channel_env = {}
         bob.cfg_start_auth = True
-        bob.cfg_start_recurse = False
+        bob.cfg_start_resolver = False
 
         bob.start_all_processes(c_channel_env)
 
@@ -255,15 +255,15 @@ class TestStartAllProcessesBob(unittest.TestCase):
         self.assertEqual(bob.cfgmgr, True)
         self.assertEqual(bob.ccsession, True)
         self.assertEqual(bob.auth, True)
-        self.assertEqual(bob.recurse, False)
+        self.assertEqual(bob.resolver, False)
         self.assertEqual(bob.xfrout, True)
         self.assertEqual(bob.xfrin, True)
         self.assertEqual(bob.zonemgr, True)
         self.assertEqual(bob.stats, True)
         self.assertEqual(bob.cmdctl, True)
 
-    # Checks the processes started when starting only the recurse process
-    def test_start_recurse(self):
+    # Checks the processes started when starting only the resolver process
+    def test_start_resolver(self):
         # Created Bob and ensure initialization correct
         bob = StartAllProcessesBob()
         self.check_preconditions(bob)
@@ -271,7 +271,7 @@ class TestStartAllProcessesBob(unittest.TestCase):
         # Start processes and check what was started
         c_channel_env = {}
         bob.cfg_start_auth = False
-        bob.cfg_start_recurse = True
+        bob.cfg_start_resolver = True
 
         bob.start_all_processes(c_channel_env)
 
@@ -279,14 +279,14 @@ class TestStartAllProcessesBob(unittest.TestCase):
         self.assertEqual(bob.cfgmgr, True)
         self.assertEqual(bob.ccsession, True)
         self.assertEqual(bob.auth, False)
-        self.assertEqual(bob.recurse, True)
+        self.assertEqual(bob.resolver, True)
         self.assertEqual(bob.xfrout, False)
         self.assertEqual(bob.xfrin, False)
         self.assertEqual(bob.zonemgr, False)
         self.assertEqual(bob.stats, True)
         self.assertEqual(bob.cmdctl, True)
 
-    # Checks the processes started when starting both auth and recurse process
+    # Checks the processes started when starting both auth and resolver process
     def test_start_both(self):
         # Created Bob and ensure initialization correct
         bob = StartAllProcessesBob()
@@ -295,7 +295,7 @@ class TestStartAllProcessesBob(unittest.TestCase):
         # Start processes and check what was started
         c_channel_env = {}
         bob.cfg_start_auth = True
-        bob.cfg_start_recurse = True
+        bob.cfg_start_resolver = True
 
         bob.start_all_processes(c_channel_env)
 
@@ -303,7 +303,7 @@ class TestStartAllProcessesBob(unittest.TestCase):
         self.assertEqual(bob.cfgmgr, True)
         self.assertEqual(bob.ccsession, True)
         self.assertEqual(bob.auth, True)
-        self.assertEqual(bob.recurse, True)
+        self.assertEqual(bob.resolver, True)
         self.assertEqual(bob.xfrout, True)
         self.assertEqual(bob.xfrin, True)
         self.assertEqual(bob.zonemgr, True)

src/bin/bindctl/bindcmd.py

@@ -217,16 +217,24 @@ class BindCmdInterpreter(Cmd):
         for module_name in self.config_data.get_config_item_list():
             self._prepare_module_commands(self.config_data.get_module_spec(module_name))
 
+    def _send_message(self, url, body):
+        headers = {"cookie" : self.session_id}
+        self.conn.request('GET', url, body, headers)
+        res = self.conn.getresponse()
+        return res.status, res.read()
+
     def send_GET(self, url, body = None):
         '''Send GET request to cmdctl, session id is send with the name
         'cookie' in header.
         '''
-        headers = {"cookie" : self.session_id}
-        self.conn.request('GET', url, body, headers)
-        res = self.conn.getresponse()
-        reply_msg = res.read()
+        status, reply_msg = self._send_message(url, body)
+        if status == http.client.UNAUTHORIZED:
+            if self.login_to_cmdctl():
+                # successful, so try send again
+                status, reply_msg = self._send_message(url, body)
+            
         if reply_msg:
-           return json.loads(reply_msg.decode())
+            return json.loads(reply_msg.decode())
         else:
             return {}
        
@@ -668,9 +676,11 @@ class BindCmdInterpreter(Cmd):
         if (len(cmd.params) != 0):
             cmd_params = json.dumps(cmd.params)
 
-        print("send the command to cmd-ctrld")        
         reply = self.send_POST(url, cmd.params)
         data = reply.read().decode()
-        print("received reply:", data)
+        # The reply is a string containing JSON data,
+        # parse it, then prettyprint
+        if data != "" and data != "{}":
+            print(json.dumps(json.loads(data), sort_keys=True, indent=4))
 
 

src/bin/cfgmgr/b10-cfgmgr.py.in

@@ -61,8 +61,6 @@ def main():
     except ConfigManagerDataReadError as cmdre:
         print("[b10-cfgmgr] " + str(cmdre))
         return 2
-    if cm:
-        return cm.write_config()
     return 0
 
 if __name__ == "__main__":

src/bin/cfgmgr/tests/b10-cfgmgr_test.py.in

@@ -58,7 +58,8 @@ class TestConfigManagerStartup(unittest.TestCase):
         self.assertTrue(b.cm.read_config_called)
         self.assertTrue(b.cm.notify_boss_called)
         self.assertTrue(b.cm.run_called)
-        self.assertTrue(b.cm.write_config_called)
+        # if there are no changes, config is not written
+        self.assertFalse(b.cm.write_config_called)
 
         self.assertTrue(b.cm.running)
         b.signal_handler(None, None)

src/bin/recurse/Makefile.am

@@ -1,57 +0,0 @@
-SUBDIRS = . tests
-
-AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
-AM_CPPFLAGS += -I$(top_srcdir)/src/bin -I$(top_builddir)/src/bin
-AM_CPPFLAGS += -I$(top_srcdir)/src/lib/dns -I$(top_builddir)/src/lib/dns
-AM_CPPFLAGS += -I$(top_srcdir)/src/lib/cc -I$(top_builddir)/src/lib/cc
-AM_CPPFLAGS += -I$(top_srcdir)/src/lib/asiolink
-AM_CPPFLAGS += -I$(top_builddir)/src/lib/asiolink
-AM_CPPFLAGS += $(BOOST_INCLUDES)
-
-AM_CXXFLAGS = $(B10_CXXFLAGS)
-
-if USE_STATIC_LINK
-AM_LDFLAGS = -static
-endif
-
-pkglibexecdir = $(libexecdir)/@PACKAGE@
-
-CLEANFILES = *.gcno *.gcda recurse.spec spec_config.h
-
-man_MANS = b10-recurse.8
-EXTRA_DIST = $(man_MANS) b10-recurse.xml
-
-if ENABLE_MAN
-
-b10-recurse.8: b10-recurse.xml
-	xsltproc --novalid --xinclude --nonet -o $@ http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $(srcdir)/b10-recurse.xml
-
-endif
-
-recurse.spec: recurse.spec.pre
-	$(SED) -e "s|@@LOCALSTATEDIR@@|$(localstatedir)|" recurse.spec.pre >$@
-
-spec_config.h: spec_config.h.pre
-	$(SED) -e "s|@@LOCALSTATEDIR@@|$(localstatedir)|" spec_config.h.pre >$@
-
-BUILT_SOURCES = spec_config.h 
-pkglibexec_PROGRAMS = b10-recurse
-b10_recurse_SOURCES = recursor.cc recursor.h
-b10_recurse_SOURCES += $(top_builddir)/src/bin/auth/change_user.h
-b10_recurse_SOURCES += $(top_builddir)/src/bin/auth/common.h
-b10_recurse_SOURCES += main.cc
-b10_recurse_LDADD =  $(top_builddir)/src/lib/dns/libdns++.la
-b10_recurse_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la
-b10_recurse_LDADD += $(top_builddir)/src/lib/cc/libcc.la
-b10_recurse_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
-b10_recurse_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
-b10_recurse_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
-b10_recurse_LDADD += $(top_builddir)/src/lib/log/liblog.la
-b10_recurse_LDADD += $(top_builddir)/src/bin/auth/change_user.o
-b10_recurse_LDFLAGS = -pthread
-
-# TODO: config.h.in is wrong because doesn't honor pkgdatadir
-# and can't use @datadir@ because doesn't expand default ${prefix}
-b10_recursedir = $(DESTDIR)$(pkgdatadir)
-b10_recurse_DATA = recurse.spec
-

src/bin/resolver/Makefile.am

@@ -0,0 +1,57 @@
+SUBDIRS = . tests
+
+AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
+AM_CPPFLAGS += -I$(top_srcdir)/src/bin -I$(top_builddir)/src/bin
+AM_CPPFLAGS += -I$(top_srcdir)/src/lib/dns -I$(top_builddir)/src/lib/dns
+AM_CPPFLAGS += -I$(top_srcdir)/src/lib/cc -I$(top_builddir)/src/lib/cc
+AM_CPPFLAGS += -I$(top_srcdir)/src/lib/asiolink
+AM_CPPFLAGS += -I$(top_builddir)/src/lib/asiolink
+AM_CPPFLAGS += $(BOOST_INCLUDES)
+
+AM_CXXFLAGS = $(B10_CXXFLAGS)
+
+if USE_STATIC_LINK
+AM_LDFLAGS = -static
+endif
+
+pkglibexecdir = $(libexecdir)/@PACKAGE@
+
+CLEANFILES = *.gcno *.gcda resolver.spec spec_config.h
+
+man_MANS = b10-resolver.8
+EXTRA_DIST = $(man_MANS) b10-resolver.xml
+
+if ENABLE_MAN
+
+b10-resolver.8: b10-resolver.xml
+	xsltproc --novalid --xinclude --nonet -o $@ http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $(srcdir)/b10-resolver.xml
+
+endif
+
+resolver.spec: resolver.spec.pre
+	$(SED) -e "s|@@LOCALSTATEDIR@@|$(localstatedir)|" resolver.spec.pre >$@
+
+spec_config.h: spec_config.h.pre
+	$(SED) -e "s|@@LOCALSTATEDIR@@|$(localstatedir)|" spec_config.h.pre >$@
+
+BUILT_SOURCES = spec_config.h 
+pkglibexec_PROGRAMS = b10-resolver
+b10_resolver_SOURCES = resolver.cc resolver.h
+b10_resolver_SOURCES += $(top_builddir)/src/bin/auth/change_user.h
+b10_resolver_SOURCES += $(top_builddir)/src/bin/auth/common.h
+b10_resolver_SOURCES += main.cc
+b10_resolver_LDADD =  $(top_builddir)/src/lib/dns/libdns++.la
+b10_resolver_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la
+b10_resolver_LDADD += $(top_builddir)/src/lib/cc/libcc.la
+b10_resolver_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
+b10_resolver_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
+b10_resolver_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
+b10_resolver_LDADD += $(top_builddir)/src/lib/log/liblog.la
+b10_resolver_LDADD += $(top_builddir)/src/bin/auth/change_user.o
+b10_resolver_LDFLAGS = -pthread
+
+# TODO: config.h.in is wrong because doesn't honor pkgdatadir
+# and can't use @datadir@ because doesn't expand default ${prefix}
+b10_resolverdir = $(DESTDIR)$(pkgdatadir)
+b10_resolver_DATA = resolver.spec
+

src/bin/recurse/b10-recurse.8

@@ -1,13 +1,13 @@
 '\" t
-.\"     Title: b10-recurse
+.\"     Title: b10-resolver
 .\"    Author: [FIXME: author] [see http://docbook.sf.net/el/author]
 .\" Generator: DocBook XSL Stylesheets v1.75.2 <http://docbook.sf.net/>
-.\"      Date: December 27, 2010
+.\"      Date: January 3, 2011
 .\"    Manual: BIND10
 .\"    Source: BIND10
 .\"  Language: English
 .\"
-.TH "B10\-RECURSE" "8" "December 27, 2010" "BIND10" "BIND10"
+.TH "B10\-RESOLVER" "8" "January 3, 2011" "BIND10" "BIND10"
 .\" -----------------------------------------------------------------
 .\" * set default formatting
 .\" -----------------------------------------------------------------
@@ -19,14 +19,14 @@
 .\" * MAIN CONTENT STARTS HERE *
 .\" -----------------------------------------------------------------
 .SH "NAME"
-b10-recurse \- Recursive DNS server
+b10-resolver \- Recursive DNS server
 .SH "SYNOPSIS"
-.HP \w'\fBb10\-recurse\fR\ 'u
-\fBb10\-recurse\fR [\fB\-u\ \fR\fB\fIusername\fR\fR] [\fB\-v\fR]
+.HP \w'\fBb10\-resolver\fR\ 'u
+\fBb10\-resolver\fR [\fB\-u\ \fR\fB\fIusername\fR\fR] [\fB\-v\fR]
 .SH "DESCRIPTION"
 .PP
 The
-\fBb10\-recurse\fR
+\fBb10\-resolver\fR
 daemon provides the BIND 10 recursive DNS server\&. Normally it is started by the
 \fBbind10\fR(8)
 boss process\&.
@@ -34,7 +34,7 @@ boss process\&.
 This daemon communicates with other BIND 10 components over a
 \fBb10-msgq\fR(8)
 C\-Channel connection\&. If this connection is not established,
-\fBb10\-recurse\fR
+\fBb10\-resolver\fR
 will exit\&.
 .PP
 It also receives its configurations from
@@ -63,7 +63,7 @@ The arguments are as follows:
 \fB\-u \fR\fB\fIusername\fR\fR
 .RS 4
 The user name of the
-\fBb10\-recurse\fR
+\fBb10\-resolver\fR
 daemon\&. If specified, the daemon changes the process owner to the specified user\&. The
 \fIusername\fR
 must be either a valid numeric user ID or a valid user name\&. By default the daemon runs as the user who invokes it\&.
@@ -87,7 +87,7 @@ BIND 10 Guide\&.
 .SH "HISTORY"
 .PP
 The
-\fBb10\-recurse\fR
+\fBb10\-resolver\fR
 daemon was first coded in September 2010\&.
 .SH "COPYRIGHT"
 .br

src/bin/recurse/b10-recurse.xml

@@ -21,17 +21,17 @@
 <refentry>
 
   <refentryinfo>
-    <date>December 27, 2010</date>
+    <date>January 3, 2011</date>
   </refentryinfo>
 
   <refmeta>
-    <refentrytitle>b10-recurse</refentrytitle>
+    <refentrytitle>b10-resolver</refentrytitle>
     <manvolnum>8</manvolnum>
     <refmiscinfo>BIND10</refmiscinfo>
   </refmeta>
 
   <refnamediv>
-    <refname>b10-recurse</refname>
+    <refname>b10-resolver</refname>
     <refpurpose>Recursive DNS server</refpurpose>
   </refnamediv>
 
@@ -44,7 +44,7 @@
 
   <refsynopsisdiv>
     <cmdsynopsis>
-      <command>b10-recurse</command>
+      <command>b10-resolver</command>
       <arg><option>-u <replaceable>username</replaceable></option></arg>
       <arg><option>-v</option></arg>
     </cmdsynopsis>
@@ -52,7 +52,7 @@
 
   <refsect1>
     <title>DESCRIPTION</title>
-    <para>The <command>b10-recurse</command> daemon provides the BIND 10
+    <para>The <command>b10-resolver</command> daemon provides the BIND 10
       recursive DNS server.  Normally it is started by the
       <citerefentry><refentrytitle>bind10</refentrytitle><manvolnum>8</manvolnum></citerefentry>
       boss process.
@@ -62,7 +62,7 @@
       This daemon communicates with other BIND 10 components over a
       <citerefentry><refentrytitle>b10-msgq</refentrytitle><manvolnum>8</manvolnum></citerefentry>
       C-Channel connection.  If this connection is not established,
-      <command>b10-recurse</command> will exit.
+      <command>b10-resolver</command> will exit.
     </para>
 
     <para>
@@ -89,7 +89,7 @@
         <term><option>-u <replaceable>username</replaceable></option></term>
         <listitem>
 	  <para>
-	    The user name of the <command>b10-recurse</command> daemon.
+	    The user name of the <command>b10-resolver</command> daemon.
 	    If specified, the daemon changes the process owner to the
 	    specified user.
 	    The <replaceable>username</replaceable> must be either a
@@ -141,7 +141,7 @@
   <refsect1>
     <title>HISTORY</title>
     <para>
-      The <command>b10-recurse</command> daemon was first coded in
+      The <command>b10-resolver</command> daemon was first coded in
       September 2010.
     </para>
   </refsect1>

src/bin/recurse/main.cc

@@ -44,8 +44,8 @@
 #include <auth/change_user.h>
 #include <auth/common.h>
 
-#include <recurse/spec_config.h>
-#include <recurse/recursor.h>
+#include <resolver/spec_config.h>
+#include <resolver/resolver.h>
 
 #include <log/dummylog.h>
 
@@ -59,14 +59,14 @@ using namespace asiolink;
 namespace {
 
 // Default port current 5300 for testing purposes
-static const string PROGRAM = "Recurse";
+static const string PROGRAM = "Resolver";
 
 IOService io_service;
-static Recursor *recursor;
+static Resolver *resolver;
 
 ConstElementPtr
 my_config_handler(ConstElementPtr new_config) {
-    return (recursor->updateConfig(new_config));
+    return (resolver->updateConfig(new_config));
 }
 
 ConstElementPtr
@@ -86,7 +86,7 @@ my_command_handler(const string& command, ConstElementPtr args) {
 
 void
 usage() {
-    cerr << "Usage:  b10-recurse [-u user] [-v]" << endl;
+    cerr << "Usage:  b10-resolver [-u user] [-v]" << endl;
     cerr << "\t-u: change process UID to the specified user" << endl;
     cerr << "\t-v: verbose output" << endl;
     exit(1);
@@ -95,7 +95,7 @@ usage() {
 
 int
 main(int argc, char* argv[]) {
-    isc::log::dprefix = "b10-recurse";
+    isc::log::dprefix = "b10-resolver";
     int ch;
     const char* uid = NULL;
 
@@ -133,21 +133,21 @@ main(int argc, char* argv[]) {
         string specfile;
         if (getenv("B10_FROM_BUILD")) {
             specfile = string(getenv("B10_FROM_BUILD")) +
-                "/src/bin/recurse/recurse.spec";
+                "/src/bin/resolver/resolver.spec";
         } else {
-            specfile = string(RECURSE_SPECFILE_LOCATION);
+            specfile = string(RESOLVER_SPECFILE_LOCATION);
         }
 
-        recursor = new Recursor();
+        resolver = new Resolver();
         dlog("Server created.");
 
-        SimpleCallback* checkin = recursor->getCheckinProvider();
-        DNSLookup* lookup = recursor->getDNSLookupProvider();
-        DNSAnswer* answer = recursor->getDNSAnswerProvider();
+        SimpleCallback* checkin = resolver->getCheckinProvider();
+        DNSLookup* lookup = resolver->getDNSLookupProvider();
+        DNSAnswer* answer = resolver->getDNSAnswerProvider();
 
         DNSService dns_service(io_service, checkin, lookup, answer);
 
-        recursor->setDNSService(dns_service);
+        resolver->setDNSService(dns_service);
         dlog("IOService created.");
 
         cc_session = new Session(io_service.get_io_service());
@@ -163,19 +163,19 @@ main(int argc, char* argv[]) {
             changeUser(uid);
         }
 
-        recursor->setConfigSession(config_session);
+        resolver->setConfigSession(config_session);
         dlog("Config loaded");
 
         dlog("Server started.");
         io_service.run();
     } catch (const std::exception& ex) {
-        dlog(string("Server failed: ") + ex.what());
+        dlog(string("Server failed: ") + ex.what(),true);
         ret = 1;
     }
 
     delete config_session;
     delete cc_session;
-    delete recursor;
+    delete resolver;
 
     return (ret);
 }

src/bin/recurse/recursor.cc

@@ -45,7 +45,7 @@
 
 #include <log/dummylog.h>
 
-#include <recurse/recursor.h>
+#include <resolver/resolver.h>
 
 using namespace std;
 
@@ -58,49 +58,52 @@ using namespace asiolink;
 
 typedef pair<string, uint16_t> addr_t;
 
-class RecursorImpl {
+class ResolverImpl {
 private:
     // prohibit copy
-    RecursorImpl(const RecursorImpl& source);
-    RecursorImpl& operator=(const RecursorImpl& source);
+    ResolverImpl(const ResolverImpl& source);
+    ResolverImpl& operator=(const ResolverImpl& source);
 public:
-    RecursorImpl() :
+    ResolverImpl() :
         config_session_(NULL),
         rec_query_(NULL)
     {}
 
-    ~RecursorImpl() {
+    ~ResolverImpl() {
         queryShutdown();
     }
 
     void querySetup(DNSService& dnss) {
         assert(!rec_query_); // queryShutdown must be called first
         dlog("Query setup");
-        rec_query_ = new RecursiveQuery(dnss, upstream_);
+        rec_query_ = new RecursiveQuery(dnss, upstream_, timeout_, retries_);
     }
 
     void queryShutdown() {
-        dlog("Query shutdown");
-        delete rec_query_;
-        rec_query_ = NULL;
+        // only shut down if we have actually called querySetup before
+        // (this is not a safety check, just to prevent logging of
+        // actions that are not performed
+        if (rec_query_) {
+            dlog("Query shutdown");
+            delete rec_query_;
+            rec_query_ = NULL;
+        }
     }
 
     void setForwardAddresses(const vector<addr_t>& upstream,
         DNSService *dnss)
     {
-        queryShutdown();
         upstream_ = upstream;
         if (dnss) {
             if (upstream_.empty()) {
                 dlog("Asked to do full recursive, but not implemented yet. "
-                    "I'll do nothing.");
+                    "I'll do nothing.",true);
             } else {
                 dlog("Setting forward addresses:");
                 BOOST_FOREACH(const addr_t& address, upstream) {
                     dlog(" " + address.first + ":" +
                         boost::lexical_cast<string>(address.second));
                 }
-                querySetup(*dnss);
             }
         }
     }
@@ -112,7 +115,7 @@ public:
     /// Currently non-configurable, but will be.
     static const uint16_t DEFAULT_LOCAL_UDPSIZE = 4096;
 
-    /// These members are public because Recursor accesses them directly.
+    /// These members are public because Resolver accesses them directly.
     ModuleCCSession* config_session_;
     /// Addresses of the forward nameserver
     vector<addr_t> upstream_;
@@ -201,10 +204,10 @@ makeErrorMessage(MessagePtr message, OutputBufferPtr buffer,
 
 // This is a derived class of \c DNSLookup, to serve as a
 // callback in the asiolink module.  It calls
-// Recursor::processMessage() on a single DNS message.
+// Resolver::processMessage() on a single DNS message.
 class MessageLookup : public DNSLookup {
 public:
-    MessageLookup(Recursor* srv) : server_(srv) {}
+    MessageLookup(Resolver* srv) : server_(srv) {}
 
     // \brief Handle the DNS Lookup
     virtual void operator()(const IOMessage& io_message, MessagePtr message,
@@ -213,7 +216,7 @@ public:
         server_->processMessage(io_message, message, buffer, server);
     }
 private:
-    Recursor* server_;
+    Resolver* server_;
 };
 
 // This is a derived class of \c DNSAnswer, to serve as a
@@ -260,6 +263,7 @@ public:
                 Message incoming(Message::PARSE);
                 InputBuffer ibuf(buffer->getData(), buffer->getLength());
                 incoming.fromWire(ibuf);
+                message->setRcode(incoming.getRcode());
                 for_each(incoming.beginSection(Message::SECTION_ANSWER),
                          incoming.endSection(Message::SECTION_ANSWER),
                          SectionInserter(message, Message::SECTION_ANSWER));
@@ -300,50 +304,48 @@ public:
 // configuration messages, and executes them if found.
 class ConfigCheck : public SimpleCallback {
 public:
-    ConfigCheck(Recursor* srv) : server_(srv) {}
+    ConfigCheck(Resolver* srv) : server_(srv) {}
     virtual void operator()(const IOMessage&) const {
         if (server_->getConfigSession()->hasQueuedMsgs()) {
             server_->getConfigSession()->checkCommand();
         }
     }
 private:
-    Recursor* server_;
+    Resolver* server_;
 };
 
-Recursor::Recursor() :
-    impl_(new RecursorImpl()),
+Resolver::Resolver() :
+    impl_(new ResolverImpl()),
     checkin_(new ConfigCheck(this)),
     dns_lookup_(new MessageLookup(this)),
     dns_answer_(new MessageAnswer)
 {}
 
-Recursor::~Recursor() {
+Resolver::~Resolver() {
     delete impl_;
     delete checkin_;
     delete dns_lookup_;
     delete dns_answer_;
-    dlog("Deleting the Recursor");
+    dlog("Deleting the Resolver",true);
 }
 
 void
-Recursor::setDNSService(asiolink::DNSService& dnss) {
-    impl_->queryShutdown();
-    impl_->querySetup(dnss);
+Resolver::setDNSService(asiolink::DNSService& dnss) {
     dnss_ = &dnss;
 }
 
 void
-Recursor::setConfigSession(ModuleCCSession* config_session) {
+Resolver::setConfigSession(ModuleCCSession* config_session) {
     impl_->config_session_ = config_session;
 }
 
 ModuleCCSession*
-Recursor::getConfigSession() const {
+Resolver::getConfigSession() const {
     return (impl_->config_session_);
 }
 
 void
-Recursor::processMessage(const IOMessage& io_message, MessagePtr message,
+Resolver::processMessage(const IOMessage& io_message, MessagePtr message,
                         OutputBufferPtr buffer, DNSServer* server)
 {
     dlog("Got a DNS message");
@@ -360,7 +362,7 @@ Recursor::processMessage(const IOMessage& io_message, MessagePtr message,
             return;
         }
     } catch (const Exception& ex) {
-        dlog(string("DNS packet exception: ") + ex.what());
+        dlog(string("DNS packet exception: ") + ex.what(),true);
         server->resume(false);
         return;
     }
@@ -422,7 +424,7 @@ Recursor::processMessage(const IOMessage& io_message, MessagePtr message,
 }
 
 void
-RecursorImpl::processNormalQuery(const Question& question, MessagePtr message,
+ResolverImpl::processNormalQuery(const Question& question, MessagePtr message,
                                  OutputBufferPtr buffer, DNSServer* server)
 {
     dlog("Processing normal query");
@@ -435,7 +437,7 @@ RecursorImpl::processNormalQuery(const Question& question, MessagePtr message,
     if (edns) {
         EDNSPtr edns_response(new EDNS());
         edns_response->setDNSSECAwareness(dnssec_ok);
-        edns_response->setUDPSize(RecursorImpl::DEFAULT_LOCAL_UDPSIZE);
+        edns_response->setUDPSize(ResolverImpl::DEFAULT_LOCAL_UDPSIZE);
         message->setEDNS(edns_response);
     }
     rec_query_->sendQuery(question, buffer, server);
@@ -482,7 +484,7 @@ parseAddresses(ConstElementPtr addresses) {
 }
 
 ConstElementPtr
-Recursor::updateConfig(ConstElementPtr config) {
+Resolver::updateConfig(ConstElementPtr config) {
     dlog("New config comes: " + config->toWire());
 
     try {
@@ -514,35 +516,45 @@ Recursor::updateConfig(ConstElementPtr config) {
         }
         // Everything OK, so commit the changes
         // listenAddresses can fail to bind, so try them first
+        bool need_query_restart = false;
+        
         if (listenAddressesE) {
             setListenAddresses(listenAddresses);
+            need_query_restart = true;
         }
         if (forwardAddressesE) {
             setForwardAddresses(forwardAddresses);
+            need_query_restart = true;
         }
         if (set_timeouts) {
             setTimeouts(timeout, retries);
+            need_query_restart = true;
+        }
+
+        if (need_query_restart) {
+            impl_->queryShutdown();
+            impl_->querySetup(*dnss_);
         }
         return (isc::config::createAnswer());
     } catch (const isc::Exception& error) {
-        dlog(string("error in config: ") + error.what());
+        dlog(string("error in config: ") + error.what(),true);
         return (isc::config::createAnswer(1, error.what()));
     }
 }
 
 void
-Recursor::setForwardAddresses(const vector<addr_t>& addresses)
+Resolver::setForwardAddresses(const vector<addr_t>& addresses)
 {
     impl_->setForwardAddresses(addresses, dnss_);
 }
 
 bool
-Recursor::isForwarding() const {
+Resolver::isForwarding() const {
     return (!impl_->upstream_.empty());
 }
 
 vector<addr_t>
-Recursor::getForwardAddresses() const {
+Resolver::getForwardAddresses() const {
     return (impl_->upstream_);
 }
 
@@ -559,7 +571,7 @@ setAddresses(DNSService *service, const vector<addr_t>& addresses) {
 }
 
 void
-Recursor::setListenAddresses(const vector<addr_t>& addresses) {
+Resolver::setListenAddresses(const vector<addr_t>& addresses) {
     try {
         dlog("Setting listen addresses:");
         BOOST_FOREACH(const addr_t& addr, addresses) {
@@ -577,13 +589,13 @@ Recursor::setListenAddresses(const vector<addr_t>& addresses) {
          * If that fails, bad luck, but we are useless anyway, so just die
          * and let boss start us again.
          */
-        dlog(string("Unable to set new address: ") + e.what());
+        dlog(string("Unable to set new address: ") + e.what(),true);
         try {
             setAddresses(dnss_, impl_->listen_);
         }
         catch (const exception& e2) {
-            dlog(string("Unable to recover from error;"));
-            dlog(string("Rollback failed with: ") + e2.what());
+            dlog(string("Unable to recover from error;"),true);
+            dlog(string("Rollback failed with: ") + e2.what(),true);
             abort();
         }
         throw e; // Let it fly a little bit further
@@ -591,20 +603,18 @@ Recursor::setListenAddresses(const vector<addr_t>& addresses) {
 }
 
 void
-Recursor::setTimeouts(int timeout, unsigned retries) {
+Resolver::setTimeouts(int timeout, unsigned retries) {
     dlog("Setting timeout to " + boost::lexical_cast<string>(timeout) +
         " and retry count to " + boost::lexical_cast<string>(retries));
     impl_->timeout_ = timeout;
     impl_->retries_ = retries;
-    impl_->queryShutdown();
-    impl_->querySetup(*dnss_);
 }
 pair<int, unsigned>
-Recursor::getTimeouts() const {
+Resolver::getTimeouts() const {
     return (pair<int, unsigned>(impl_->timeout_, impl_->retries_));
 }
 
 vector<addr_t>
-Recursor::getListenAddresses() const {
+Resolver::getListenAddresses() const {
     return (impl_->listen_);
 }

src/bin/recurse/recursor.h

@@ -14,8 +14,8 @@
 
 // $Id$
 
-#ifndef __RECURSOR_H
-#define __RECURSOR_H 1
+#ifndef __RESOLVER_H
+#define __RESOLVER_H 1
 
 #include <string>
 #include <vector>
@@ -26,7 +26,7 @@
 
 #include <asiolink/asiolink.h>
 
-class RecursorImpl;
+class ResolverImpl;
 
 /**
  * \short The recursive nameserver.
@@ -37,7 +37,7 @@ class RecursorImpl;
  * answer. It doesn't really know about chasing referrals and similar, it
  * simply plugs the parts that know into the network handling code.
  */
-class Recursor {
+class Resolver {
     ///
     /// \name Constructors, Assignment Operator and Destructor.
     ///
@@ -45,12 +45,12 @@ class Recursor {
     /// intentionally defined as private.
     //@{
 private:
-    Recursor(const Recursor& source);
-    Recursor& operator=(const Recursor& source);
+    Resolver(const Resolver& source);
+    Resolver& operator=(const Resolver& source);
 public:
     /// The constructor.
-    Recursor();
-    ~Recursor();
+    Resolver();
+    ~Resolver();
     //@}
 
     /// \brief Process an incoming DNS message, then signal 'server' to resume 
@@ -76,7 +76,7 @@ public:
     /// \brief Handle commands from the config session
     isc::data::ConstElementPtr updateConfig(isc::data::ConstElementPtr config);
 
-    /// \brief Assign an ASIO IO Service queue to this Recursor object
+    /// \brief Assign an ASIO IO Service queue to this Resolver object
     void setDNSService(asiolink::DNSService& dnss);
 
     /// \brief Return this object's ASIO IO Service queue
@@ -137,14 +137,14 @@ public:
     std::pair<int, unsigned> getTimeouts() const;
 
 private:
-    RecursorImpl* impl_;
+    ResolverImpl* impl_;
     asiolink::DNSService* dnss_;
     asiolink::SimpleCallback* checkin_;
     asiolink::DNSLookup* dns_lookup_;
     asiolink::DNSAnswer* dns_answer_;
 };
 
-#endif // __RECURSOR_H
+#endif // __RESOLVER_H
 
 // Local Variables: 
 // mode: c++

src/bin/recurse/recurse.spec.pre.in

@@ -1,6 +1,6 @@
 {
   "module_spec": {
-    "module_name": "Recurse",
+    "module_name": "Resolver",
     "module_description": "Recursive service",
     "config_data": [
       {

src/bin/recurse/spec_config.h.pre.in

@@ -12,4 +12,4 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#define RECURSE_SPECFILE_LOCATION "@prefix@/share/@PACKAGE@/recurse.spec"
+#define RESOLVER_SPECFILE_LOCATION "@prefix@/share/@PACKAGE@/resolver.spec"

src/bin/recurse/tests/Makefile.am

@@ -19,9 +19,9 @@ if HAVE_GTEST
 TESTS += run_unittests
 run_unittests_SOURCES = $(top_srcdir)/src/lib/dns/tests/unittest_util.h
 run_unittests_SOURCES += $(top_srcdir)/src/lib/dns/tests/unittest_util.cc
-run_unittests_SOURCES += ../recursor.h ../recursor.cc
-run_unittests_SOURCES += recursor_unittest.cc
-run_unittests_SOURCES += recursor_config_unittest.cc
+run_unittests_SOURCES += ../resolver.h ../resolver.cc
+run_unittests_SOURCES += resolver_unittest.cc
+run_unittests_SOURCES += resolver_config_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)

src/bin/recurse/tests/recursor_config_unittest.cc

@@ -20,7 +20,7 @@
 
 #include <asiolink/asiolink.h>
 
-#include <recurse/recursor.h>
+#include <resolver/resolver.h>
 
 #include <dns/tests/unittest_util.h>
 #include <testutils/srv_test.h>
@@ -32,12 +32,12 @@ using namespace asiolink;
 using isc::UnitTestUtil;
 
 namespace {
-class RecursorConfig : public ::testing::Test {
+class ResolverConfig : public ::testing::Test {
     public:
         IOService ios;
         DNSService dnss;
-        Recursor server;
-        RecursorConfig() :
+        Resolver server;
+        ResolverConfig() :
             dnss(ios, NULL, NULL, NULL)
         {
             server.setDNSService(dnss);
@@ -45,7 +45,7 @@ class RecursorConfig : public ::testing::Test {
         void invalidTest(const string &JOSN);
 };
 
-TEST_F(RecursorConfig, forwardAddresses) {
+TEST_F(ResolverConfig, forwardAddresses) {
     // Default value should be fully recursive
     EXPECT_TRUE(server.getForwardAddresses().empty());
     EXPECT_FALSE(server.isForwarding());
@@ -69,7 +69,7 @@ TEST_F(RecursorConfig, forwardAddresses) {
     EXPECT_FALSE(server.isForwarding());
 }
 
-TEST_F(RecursorConfig, forwardAddressConfig) {
+TEST_F(ResolverConfig, forwardAddressConfig) {
     // Try putting there some address
     ElementPtr config(Element::fromJSON("{"
         "\"forward_addresses\": ["
@@ -97,13 +97,13 @@ TEST_F(RecursorConfig, forwardAddressConfig) {
 }
 
 void
-RecursorConfig::invalidTest(const string &JOSN) {
+ResolverConfig::invalidTest(const string &JOSN) {
     ElementPtr config(Element::fromJSON(JOSN));
     EXPECT_FALSE(server.updateConfig(config)->equals(
         *isc::config::createAnswer())) << "Accepted config " << JOSN << endl;
 }
 
-TEST_F(RecursorConfig, invalidForwardAddresses) {
+TEST_F(ResolverConfig, invalidForwardAddresses) {
     // Try torturing it with some invalid inputs
     invalidTest("{"
         "\"forward_addresses\": \"error\""
@@ -128,7 +128,7 @@ TEST_F(RecursorConfig, invalidForwardAddresses) {
         "}]}");
 }
 
-TEST_F(RecursorConfig, listenAddresses) {
+TEST_F(ResolverConfig, listenAddresses) {
     // Default value should be fully recursive
     EXPECT_TRUE(server.getListenAddresses().empty());
 
@@ -149,7 +149,7 @@ TEST_F(RecursorConfig, listenAddresses) {
     EXPECT_TRUE(server.getListenAddresses().empty());
 }
 
-TEST_F(RecursorConfig, DISABLED_listenAddressConfig) {
+TEST_F(ResolverConfig, DISABLED_listenAddressConfig) {
     // Try putting there some address
     ElementPtr config(Element::fromJSON("{"
         "\"listen_on\": ["
@@ -185,7 +185,7 @@ TEST_F(RecursorConfig, DISABLED_listenAddressConfig) {
     EXPECT_EQ(5300, server.getListenAddresses()[0].second);
 }
 
-TEST_F(RecursorConfig, invalidListenAddresses) {
+TEST_F(ResolverConfig, invalidListenAddresses) {
     // Try torturing it with some invalid inputs
     invalidTest("{"
         "\"listen_on\": \"error\""
@@ -211,7 +211,7 @@ TEST_F(RecursorConfig, invalidListenAddresses) {
 }
 
 // Just test it sets and gets the values correctly
-TEST_F(RecursorConfig, timeouts) {
+TEST_F(ResolverConfig, timeouts) {
     server.setTimeouts(0, 1);
     EXPECT_EQ(0, server.getTimeouts().first);
     EXPECT_EQ(1, server.getTimeouts().second);
@@ -220,7 +220,7 @@ TEST_F(RecursorConfig, timeouts) {
     EXPECT_EQ(0, server.getTimeouts().second);
 }
 
-TEST_F(RecursorConfig, timeoutsConfig) {
+TEST_F(ResolverConfig, timeoutsConfig) {
     ElementPtr config = Element::fromJSON("{"
             "\"timeout\": 1000,"
             "\"retries\": 3"
@@ -231,7 +231,7 @@ TEST_F(RecursorConfig, timeoutsConfig) {
     EXPECT_EQ(3, server.getTimeouts().second);
 }
 
-TEST_F(RecursorConfig, invalidTimeoutsConfig) {
+TEST_F(ResolverConfig, invalidTimeoutsConfig) {
     invalidTest("{"
         "\"timeout\": \"error\""
         "}");

src/bin/recurse/tests/recursor_unittest.cc

@@ -14,7 +14,7 @@
 
 #include <dns/name.h>
 
-#include <recurse/recursor.h>
+#include <resolver/resolver.h>
 #include <dns/tests/unittest_util.h>
 #include <testutils/srv_test.h>
 
@@ -25,58 +25,58 @@ using isc::UnitTestUtil;
 namespace {
 const char* const TEST_PORT = "53535";
 
-class RecursorTest : public SrvTestBase{
+class ResolverTest : public SrvTestBase{
 protected:
-    RecursorTest() : server(){}
+    ResolverTest() : server(){}
     virtual void processMessage() {
         server.processMessage(*io_message, parse_message, response_obuffer,
                               &dnsserv);
     }
-    Recursor server;
+    Resolver server;
 };
 
 // Unsupported requests.  Should result in NOTIMP.
-TEST_F(RecursorTest, unsupportedRequest) {
+TEST_F(ResolverTest, unsupportedRequest) {
     unsupportedRequest();
 }
 
 // Multiple questions.  Should result in FORMERR.
-TEST_F(RecursorTest, multiQuestion) {
-    multiQuestion();
+TEST_F(ResolverTest, multiQuestion) {
+    multiQuestion(); 
 }
 
 // Incoming data doesn't even contain the complete header.  Must be silently
 // dropped.
-TEST_F(RecursorTest, shortMessage) {
+TEST_F(ResolverTest, shortMessage) {
     shortMessage();
 }
 
 // Response messages.  Must be silently dropped, whether it's a valid response
 // or malformed or could otherwise cause a protocol error.
-TEST_F(RecursorTest, response) {
-    response();
+TEST_F(ResolverTest, response) {
+     response();
 }
 
 // Query with a broken question
-TEST_F(RecursorTest, shortQuestion) {
+TEST_F(ResolverTest, shortQuestion) {
     shortQuestion();
 }
 
 // Query with a broken answer section
-TEST_F(RecursorTest, shortAnswer) {
+TEST_F(ResolverTest, shortAnswer) {
     shortAnswer();
 }
 
 // Query with unsupported version of EDNS.
-TEST_F(RecursorTest, ednsBadVers) {
+TEST_F(ResolverTest, ednsBadVers) {
     ednsBadVers();
 }
 
-TEST_F(RecursorTest, AXFROverUDP) {
+TEST_F(ResolverTest, AXFROverUDP) {
     axfrOverUDP();
 }
 
-TEST_F(RecursorTest, AXFRFail) {
+TEST_F(ResolverTest, AXFRFail) {
     UnitTestUtil::createRequestMessage(request_message, opcode, default_qid,
                                        Name("example.com"), RRClass::IN(),
                                        RRType::AXFR());
@@ -88,7 +88,7 @@ TEST_F(RecursorTest, AXFRFail) {
                 QR_FLAG, 1, 0, 0, 0);
 }
 
-TEST_F(RecursorTest, notifyFail) {
+TEST_F(ResolverTest, notifyFail) {
     // Notify should always return NOTAUTH
     request_message.clear(Message::RENDER);
     request_message.setOpcode(Opcode::NOTIFY());

src/bin/xfrout/tests/xfrout_test.py

@@ -215,36 +215,38 @@ class TestXfroutSession(unittest.TestCase):
         rrset_soa = self.xfrsess._create_rrset_from_db_record(self.soa_record)
         self.assertEqual(82, get_rrset_len(rrset_soa))
 
-    def test_zone_is_empty(self):
+    def test_zone_has_soa(self):
         global sqlite3_ds
         def mydb1(zone, file):
             return True
         sqlite3_ds.get_zone_soa = mydb1
-        self.assertEqual(self.xfrsess._zone_is_empty(""), False)
+        self.assertTrue(self.xfrsess._zone_has_soa(""))
         def mydb2(zone, file):
             return False
         sqlite3_ds.get_zone_soa = mydb2
-        self.assertEqual(self.xfrsess._zone_is_empty(""), True)
+        self.assertFalse(self.xfrsess._zone_has_soa(""))
 
     def test_zone_exist(self):
         global sqlite3_ds
-        def zone_soa(zone, file):
+        def zone_exist(zone, file):
             return zone
-        sqlite3_ds.get_zone_soa = zone_soa
-        self.assertEqual(self.xfrsess._zone_exist(True), True)
-        self.assertEqual(self.xfrsess._zone_exist(False), False)
+        sqlite3_ds.zone_exist = zone_exist
+        self.assertTrue(self.xfrsess._zone_exist(True))
+        self.assertFalse(self.xfrsess._zone_exist(False))
 
     def test_check_xfrout_available(self):
         def zone_exist(zone):
             return zone
+        def zone_has_soa(zone):
+            return (not zone)
         self.xfrsess._zone_exist = zone_exist
-        self.xfrsess._zone_is_empty = zone_exist
+        self.xfrsess._zone_has_soa = zone_has_soa
         self.assertEqual(self.xfrsess._check_xfrout_available(False).to_text(), "NOTAUTH")
         self.assertEqual(self.xfrsess._check_xfrout_available(True).to_text(), "SERVFAIL")
 
         def zone_empty(zone):
-            return not zone
-        self.xfrsess._zone_is_empty = zone_empty
+            return zone
+        self.xfrsess._zone_has_soa = zone_empty
         def false_func():
             return False
         self.xfrsess.server.increase_transfers_counter = false_func

src/bin/xfrout/xfrout.py.in

@@ -193,30 +193,41 @@ class XfroutSession(BaseRequestHandler):
         self._send_message(sock_fd, msg)
 
 
-    def _zone_is_empty(self, zone):
+    def _zone_has_soa(self, zone):
+        '''Judge if the zone has an SOA record.'''
+        # In some sense, the SOA defines a zone.
+        # If the current name server has authority for the
+        # specific zone, we need to judge if the zone has an SOA record;
+        # if not, we consider the zone has incomplete data, so xfrout can't
+        # serve for it.
         if sqlite3_ds.get_zone_soa(zone, self.server.get_db_file()):
-            return False
-
-        return True
-
-    def _zone_exist(self, zonename):
-        # Find zone in datasource, should this works? maybe should ask
-        # config manager.
-        soa = sqlite3_ds.get_zone_soa(zonename, self.server.get_db_file())
-        if soa:
             return True
+
         return False
 
+    def _zone_exist(self, zonename):
+        '''Judge if the zone is configured by config manager.'''
+        # Currently, if we find the zone in datasource successfully, we
+        # consider the zone is configured, and the current name server has
+        # authority for the specific zone.
+        # TODO: should get zone's configuration from cfgmgr or other place
+        # in future.
+        return sqlite3_ds.zone_exist(zonename, self.server.get_db_file())
 
     def _check_xfrout_available(self, zone_name):
         '''Check if xfr request can be responsed.
            TODO, Get zone's configuration from cfgmgr or some other place
            eg. check allow_transfer setting,
         '''
+        # If the current name server does not have authority for the
+        # zone, xfrout can't serve for it, return rcode NOTAUTH.
         if not self._zone_exist(zone_name):
             return Rcode.NOTAUTH()
 
-        if self._zone_is_empty(zone_name):
+        # If we are an authoritative name server for the zone, but fail
+        # to find the zone's SOA record in datasource, xfrout can't
+        # provide zone transfer for it.
+        if not self._zone_has_soa(zone_name):
             return Rcode.SERVFAIL()
 
         #TODO, check allow_transfer

src/bin/xfrout/xfrout.spec.pre.in

@@ -48,4 +48,4 @@
       ]
   }
 }
-     
+

src/lib/asiolink/README

@@ -17,7 +17,7 @@ including:
     leaving it in place elsewhere.
 
 Currently, the asiolink library only supports DNS servers (i.e., b10-auth
-and b10-recurse).  The plan is to make it more generic and allow it to
+and b10-resolver).  The plan is to make it more generic and allow it to
 support other modules as well.
 
 Some of the classes defined here--for example, IOSocket, IOEndpoint,

src/lib/asiolink/asiolink.cc

@@ -260,7 +260,7 @@ convertAddr(const string& address) {
         isc_throw(IOError, "Invalid IP address '" << &address << "': "
             << err.message());
     }
-    return addr;
+    return (addr);
 }
 
 }
@@ -386,6 +386,11 @@ public:
     void setupTimer(const IntervalTimer::Callback& cbfunc,
                     const uint32_t interval);
     void callback(const asio::error_code& error);
+    void cancel() {
+        timer_.cancel();
+        interval_ = 0;
+    }
+    uint32_t getInterval() const { return (interval_); }
 private:
     // a function to update timer_ when it expires
     void updateTimer();
@@ -398,7 +403,7 @@ private:
 };
 
 IntervalTimerImpl::IntervalTimerImpl(IOService& io_service) :
-    timer_(io_service.get_io_service())
+    interval_(0), timer_(io_service.get_io_service())
 {}
 
 IntervalTimerImpl::~IntervalTimerImpl()
@@ -427,6 +432,10 @@ IntervalTimerImpl::setupTimer(const IntervalTimer::Callback& cbfunc,
 
 void
 IntervalTimerImpl::updateTimer() {
+    if (interval_ == 0) {
+        // timer has been canceled.  Do nothing.
+        return;
+    }
     try {
         // Update expire time to (current time + interval_).
         timer_.expires_from_now(boost::posix_time::seconds(interval_));
@@ -461,4 +470,14 @@ IntervalTimer::setupTimer(const Callback& cbfunc, const uint32_t interval) {
     return (impl_->setupTimer(cbfunc, interval));
 }
 
+void
+IntervalTimer::cancel() {
+    impl_->cancel();
+}
+
+uint32_t
+IntervalTimer::getInterval() const {
+    return (impl_->getInterval());
+}
+
 }

src/lib/asiolink/asiolink.h

@@ -77,7 +77,7 @@ class io_service;
 ///
 /// Notes to developers:
 /// Currently the wrapper interface is fairly specific to use by a
-/// DNS server, i.e., b10-auth or b10-recurse.  But the plan is to
+/// DNS server, i.e., b10-auth or b10-resolver.  But the plan is to
 /// generalize it and have other modules use it as well.
 ///
 /// One obvious drawback of this approach is performance overhead
@@ -657,6 +657,29 @@ public:
     /// \throw isc::Unexpected ASIO library error
     ///
     void setupTimer(const Callback& cbfunc, const uint32_t interval);
+
+    /// Cancel the timer.
+    ///
+    /// If the timer has been set up, this method cancels any asynchronous
+    /// events waiting on the timer and stops the timer itself.
+    /// If the timer has already been canceled, this method effectively does
+    /// nothing.
+    ///
+    /// This method never throws an exception.
+    void cancel();
+
+    /// Return the timer interval.
+    ///
+    /// This method returns the timer interval in seconds if it's running;
+    /// if the timer has been canceled it returns 0.
+    ///
+    /// This method never throws an exception.
+    ///
+    /// Note: We may want to change the granularity of the timer to
+    /// milliseconds or even finer.  If and when this happens the semantics
+    /// of the return value of this method will be changed accordingly.
+    uint32_t getInterval() const;
+
 private:
     IntervalTimerImpl* impl_;
 };

src/lib/asiolink/tests/asiolink_unittest.cc

@@ -751,7 +751,7 @@ TEST_F(ASIOLinkTest, recursiveTimeout) {
 // or not.
 class IntervalTimerTest : public ::testing::Test {
 protected:
-    IntervalTimerTest() : io_service_() {};
+    IntervalTimerTest() : io_service_() {}
     ~IntervalTimerTest() {}
     class TimerCallBack : public std::unary_function<void, void> {
     public:
@@ -811,6 +811,19 @@ protected:
         int count_;
         int prev_counter_;
     };
+    class TimerCallBackCanceller {
+    public:
+        TimerCallBackCanceller(unsigned int& counter, IntervalTimer& itimer) :
+            counter_(counter), itimer_(itimer)
+        {}
+        void operator()() {
+            ++counter_;
+            itimer_.cancel();
+        }
+    private:
+        unsigned int& counter_;
+        IntervalTimer& itimer_;
+    };
     class TimerCallBackOverwriter : public std::unary_function<void, void> {
     public:
         TimerCallBackOverwriter(IntervalTimerTest* test_obj,
@@ -864,6 +877,7 @@ TEST_F(IntervalTimerTest, startIntervalTimer) {
     start = boost::posix_time::microsec_clock::universal_time();
     // setup timer
     itimer.setupTimer(TimerCallBack(this), 1);
+    EXPECT_EQ(1, itimer.getInterval());
     io_service_.run();
     // reaches here after timer expired
     // delta: difference between elapsed time and 1 second
@@ -926,6 +940,23 @@ TEST_F(IntervalTimerTest, destructIntervalTimer) {
     EXPECT_TRUE(timer_cancel_success_);
 }
 
+TEST_F(IntervalTimerTest, cancel) {
+    // Similar to destructIntervalTimer test, but the first timer explicitly
+    // cancels itself on first callback.
+    IntervalTimer itimer_counter(io_service_);
+    IntervalTimer itimer_watcher(io_service_);
+    unsigned int counter = 0;
+    itimer_counter.setupTimer(TimerCallBackCanceller(counter, itimer_counter),
+                              1);
+    itimer_watcher.setupTimer(TimerCallBack(this), 3);
+    io_service_.run();
+    EXPECT_EQ(1, counter);
+    EXPECT_EQ(0, itimer_counter.getInterval());
+
+    // canceling an already canceled timer shouldn't cause any surprise.
+    EXPECT_NO_THROW(itimer_counter.cancel());
+}
+
 TEST_F(IntervalTimerTest, overwriteIntervalTimer) {
     // Note: This test currently takes 4 seconds. The timer should have
     // finer granularity and timer periods in this test should be shorter

src/lib/asiolink/udpdns.cc

@@ -69,7 +69,7 @@ UDPServer::UDPServer(io_service& io_service,
     if (addr.is_v6()) {
         socket_->set_option(asio::ip::v6_only(true));
     }
-    socket_->bind(udp::endpoint(proto, port));
+    socket_->bind(udp::endpoint(addr, port));
 }
 
 /// The function operator is implemented with the "stackless coroutine"

src/lib/config/ccsession.cc

@@ -135,8 +135,6 @@ createCommand(const std::string& command, ConstElementPtr arg) {
     return (cmd);
 }
 
-/// Returns "" and empty ElementPtr() if this does not
-/// look like a command
 std::string
 parseCommand(ConstElementPtr& arg, ConstElementPtr command) {
     if (command &&
@@ -149,7 +147,7 @@ parseCommand(ConstElementPtr& arg, ConstElementPtr command) {
             if (cmd->size() > 1) {
                 arg = cmd->get(1);
             } else {
-                arg = ElementPtr();
+                arg = Element::createMap();
             }
             return (cmd->get(0)->stringValue());
         } else {
@@ -257,7 +255,7 @@ ModuleCCSession::handleConfigUpdate(ConstElementPtr new_config) {
     ElementPtr errors = Element::createList();
     if (!config_handler_) {
         answer = createAnswer(1, module_name_ + " does not have a config handler");
-    } else if (!module_specification_.validate_config(new_config, false,
+    } else if (!module_specification_.validateConfig(new_config, false,
                                                       errors)) {
         std::stringstream ss;
         ss << "Error in config validation: ";
@@ -286,6 +284,51 @@ ModuleCCSession::hasQueuedMsgs() const {
     return (session_.hasQueuedMsgs());
 }
 
+ConstElementPtr
+ModuleCCSession::checkConfigUpdateCommand(const std::string& target_module,
+                                          ConstElementPtr arg)
+{
+    if (target_module == module_name_) {
+        return (handleConfigUpdate(arg));
+    } else {
+        // ok this update is not for us, if we have this module
+        // in our remote config list, update that
+        updateRemoteConfig(target_module, arg);
+        // we're not supposed to answer to this, so return
+        return (ElementPtr());
+    }
+}
+
+ConstElementPtr
+ModuleCCSession::checkModuleCommand(const std::string& cmd_str,
+                                    const std::string& target_module,
+                                    ConstElementPtr arg) const
+{
+    if (target_module == module_name_) {
+        if (command_handler_) {
+            ElementPtr errors = Element::createList();
+            if (module_specification_.validateCommand(cmd_str,
+                                                       arg,
+                                                       errors)) {
+                return (command_handler_(cmd_str, arg));
+            } else {
+                std::stringstream ss;
+                ss << "Error in command validation: ";
+                BOOST_FOREACH(ConstElementPtr error,
+                              errors->listValue()) {
+                    ss << error->stringValue();
+                }
+                return (createAnswer(3, ss.str()));
+            }
+        } else {
+            return (createAnswer(1,
+                                 "Command given but no "
+                                 "command handler for module"));
+        }
+    }
+    return (ElementPtr());
+}
+
 int
 ModuleCCSession::checkCommand() {
     ConstElementPtr cmd, routing, data;
@@ -302,23 +345,9 @@ ModuleCCSession::checkCommand() {
             std::string cmd_str = parseCommand(arg, data);
             std::string target_module = routing->get("group")->stringValue();
             if (cmd_str == "config_update") {
-                if (target_module == module_name_) {
-                    answer = handleConfigUpdate(arg);
-                } else {
-                    // ok this update is not for us, if we have this module
-                    // in our remote config list, update that
-                    updateRemoteConfig(target_module, arg);
-                    // we're not supposed to answer to this, so return
-                    return (0);
-                }
+                answer = checkConfigUpdateCommand(target_module, arg);
             } else {
-                if (target_module == module_name_) {
-                    if (command_handler_) {
-                        answer = command_handler_(cmd_str, arg);
-                    } else {
-                        answer = createAnswer(1, "Command given but no command handler for module");
-                    }
-                }
+                answer = checkModuleCommand(cmd_str, target_module, arg);
             }
         } catch (const CCSessionError& re) {
             // TODO: Once we have logging and timeouts, we should not

src/lib/config/ccsession.h

@@ -91,11 +91,36 @@ isc::data::ConstElementPtr createCommand(const std::string& command,
 /// \brief Parses the given command into a string containing the actual
 ///        command and an ElementPtr containing the optional argument.
 ///
+/// Raises a CCSessionError if this is not a well-formed command
+///
+/// Example code: (command_message is a ConstElementPtr that is
+/// passed here)
+/// \code
+/// ElementPtr command_message = Element::fromJSON("{ \"command\": [\"foo\", { \"bar\": 123 } ] }");
+/// try {
+///     ConstElementPtr args;
+///     std::string command_str = parseCommand(args, command_message);
+///     if (command_str == "foo") {
+///         std::cout << "The command 'foo' was given" << std::endl;
+///         if (args->contains("bar")) {
+///             std::cout << "It had argument name 'bar' set, which has"
+///                       << "value " 
+///                       << args->get("bar")->intValue();
+///         }
+///     } else {
+///         std::cout << "Unknown command '" << command_str << std::endl;
+///     }
+/// } catch (CCSessionError cse) {
+///     std::cerr << "Bad command in CC Session: "
+///     << cse.what() << std::endl;
+/// }
+/// \endcode
+/// 
 /// \param arg This value will be set to the ElementPtr pointing to
-///        the argument, or to an empty ElementPtr if there was none.
+///        the argument, or to an empty Map (ElementPtr) if there was none.
 /// \param command The command message containing the command (as made
 ///        by createCommand()
-/// \return The command string
+/// \return The command name
 std::string parseCommand(isc::data::ConstElementPtr& arg,
                          isc::data::ConstElementPtr command);
 
@@ -244,6 +269,15 @@ private:
     isc::data::ConstElementPtr handleConfigUpdate(
         isc::data::ConstElementPtr new_config);
 
+    isc::data::ConstElementPtr checkConfigUpdateCommand(
+        const std::string& target_module,
+        isc::data::ConstElementPtr arg);
+
+    isc::data::ConstElementPtr checkModuleCommand(
+        const std::string& cmd_str,
+        const std::string& target_module,
+        isc::data::ConstElementPtr arg) const;
+
     isc::data::ConstElementPtr(*config_handler_)(
         isc::data::ConstElementPtr new_config);
     isc::data::ConstElementPtr(*command_handler_)(

src/lib/config/module_spec.cc

@@ -177,17 +177,47 @@ ModuleSpec::getModuleDescription() const {
 }
 
 bool
-ModuleSpec::validate_config(ConstElementPtr data, const bool full) const {
+ModuleSpec::validateConfig(ConstElementPtr data, const bool full) const {
     ConstElementPtr spec = module_specification->find("config_data");
-    return (validate_spec_list(spec, data, full, ElementPtr()));
+    return (validateSpecList(spec, data, full, ElementPtr()));
 }
 
 bool
-ModuleSpec::validate_config(ConstElementPtr data, const bool full,
+ModuleSpec::validateCommand(const std::string& command,
+                             ConstElementPtr args,
+                             ElementPtr errors) const
+{
+    if (args->getType() != Element::map) {
+        errors->add(Element::create("args for command " +
+                                    command + " is not a map"));
+        return (false);
+    }
+
+    ConstElementPtr commands_spec = module_specification->find("commands");
+    if (!commands_spec) {
+        // there are no commands according to the spec.
+        errors->add(Element::create("The given module has no commands"));
+        return (false);
+    }
+
+    BOOST_FOREACH(ConstElementPtr cur_command, commands_spec->listValue()) {
+        if (cur_command->get("command_name")->stringValue() == command) {
+            return (validateSpecList(cur_command->get("command_args"),
+                                       args, true, errors));
+        }
+    }
+
+    // this command is unknown
+    errors->add(Element::create("Unknown command " + command));
+    return (false);
+}
+
+bool
+ModuleSpec::validateConfig(ConstElementPtr data, const bool full,
                             ElementPtr errors) const
 {
     ConstElementPtr spec = module_specification->find("config_data");
-    return (validate_spec_list(spec, data, full, errors));
+    return (validateSpecList(spec, data, full, errors));
 }
 
 ModuleSpec
@@ -264,7 +294,7 @@ check_type(ConstElementPtr spec, ConstElementPtr element) {
 }
 
 bool
-ModuleSpec::validate_item(ConstElementPtr spec, ConstElementPtr data,
+ModuleSpec::validateItem(ConstElementPtr spec, ConstElementPtr data,
                           const bool full, ElementPtr errors) const
 {
     if (!check_type(spec, data)) {
@@ -286,14 +316,14 @@ ModuleSpec::validate_item(ConstElementPtr spec, ConstElementPtr data,
                 return (false);
             }
             if (list_spec->get("item_type")->stringValue() == "map") {
-                if (!validate_item(list_spec, list_el, full, errors)) {
+                if (!validateItem(list_spec, list_el, full, errors)) {
                     return (false);
                 }
             }
         }
     }
     if (data->getType() == Element::map) {
-        if (!validate_spec_list(spec->get("map_item_spec"), data, full, errors)) {
+        if (!validateSpecList(spec->get("map_item_spec"), data, full, errors)) {
             return (false);
         }
     }
@@ -302,7 +332,7 @@ ModuleSpec::validate_item(ConstElementPtr spec, ConstElementPtr data,
 
 // spec is a map with item_name etc, data is a map
 bool
-ModuleSpec::validate_spec(ConstElementPtr spec, ConstElementPtr data,
+ModuleSpec::validateSpec(ConstElementPtr spec, ConstElementPtr data,
                           const bool full, ElementPtr errors) const
 {
     std::string item_name = spec->get("item_name")->stringValue();
@@ -311,7 +341,7 @@ ModuleSpec::validate_spec(ConstElementPtr spec, ConstElementPtr data,
     data_el = data->get(item_name);
     
     if (data_el) {
-        if (!validate_item(spec, data_el, full, errors)) {
+        if (!validateItem(spec, data_el, full, errors)) {
             return (false);
         }
     } else {
@@ -327,13 +357,13 @@ ModuleSpec::validate_spec(ConstElementPtr spec, ConstElementPtr data,
 
 // spec is a list of maps, data is a map
 bool
-ModuleSpec::validate_spec_list(ConstElementPtr spec, ConstElementPtr data,
+ModuleSpec::validateSpecList(ConstElementPtr spec, ConstElementPtr data,
                                const bool full, ElementPtr errors) const
 {
     bool validated = true;
     std::string cur_item_name;
     BOOST_FOREACH(ConstElementPtr cur_spec_el, spec->listValue()) {
-        if (!validate_spec(cur_spec_el, data, full, errors)) {
+        if (!validateSpec(cur_spec_el, data, full, errors)) {
             validated = false;
         }
     }

src/lib/config/module_spec.h

@@ -88,23 +88,66 @@ namespace isc { namespace config {
         /// \param data The base \c Element of the data to check
         /// \return true if the data conforms to the specification,
         /// false otherwise.
-        bool validate_config(isc::data::ConstElementPtr data,
+        bool validateConfig(isc::data::ConstElementPtr data,
                              const bool full = false) const;
 
+        /// Validates the arguments for the given command
+        ///
+        /// This checks the command and argument against the
+        /// specification in the module's .spec file.
+        ///
+        /// A command is considered valid if:
+        /// - it is known (the 'command' string must have an entry in
+        ///                the specification)
+        /// - the args is a MapElement
+        /// - args contains all mandatory arguments
+        /// - args does not contain unknown arguments
+        /// - all arguments in args match their specification
+        /// If all of these are true, this function returns \c true
+        /// If not, this method returns \c false
+        ///
+        /// Example usage:
+        /// \code
+        ///     ElementPtr errors = Element::createList();
+        ///     if (module_specification_.validateCommand(cmd_str,
+        ///                                               arg,
+        ///                                               errors)) {
+        ///         std::cout << "Command is valid" << std::endl;
+        ///     } else {
+        ///         std::cout << "Command is invalid: " << std::endl;
+        ///         BOOST_FOREACH(ConstElementPtr error,
+        ///                       errors->listValue()) {
+        ///             std::cout << error->stringValue() << std::endl;
+        ///         }
+        ///     }
+        /// \endcode
+        ///
+        /// \param command The command to validate the arguments for
+        /// \param args A dict containing the command parameters
+        /// \param errors An ElementPtr pointing to a ListElement. Any
+        ///               errors that are found are added as
+        ///               StringElements to this list
+        /// \return true if the command is known and the parameters are correct
+        ///         false otherwise
+        bool validateCommand(const std::string& command,
+                              isc::data::ConstElementPtr args,
+                              isc::data::ElementPtr errors) const;
+
+
         /// errors must be of type ListElement
-        bool validate_config(isc::data::ConstElementPtr data, const bool full,
+        bool validateConfig(isc::data::ConstElementPtr data, const bool full,
                              isc::data::ElementPtr errors) const;
 
     private:
-        bool validate_item(isc::data::ConstElementPtr spec,
-                           isc::data::ConstElementPtr data,
-                           const bool full,
-                           isc::data::ElementPtr errors) const;
-        bool validate_spec(isc::data::ConstElementPtr spec,
+        bool validateItem(isc::data::ConstElementPtr spec,
+                          isc::data::ConstElementPtr data,
+                          const bool full,
+                          isc::data::ElementPtr errors) const;
+        bool validateSpec(isc::data::ConstElementPtr spec,
                            isc::data::ConstElementPtr data,
                            const bool full,
                            isc::data::ElementPtr errors) const;
-        bool validate_spec_list(isc::data::ConstElementPtr spec,
+        bool validateSpecList(isc::data::ConstElementPtr spec,
                                 isc::data::ConstElementPtr data,
                                 const bool full,
                                 isc::data::ElementPtr errors) const;

src/lib/config/tests/ccsession_unittests.cc

@@ -137,7 +137,7 @@ TEST_F(CCSessionTest, parseCommand) {
 
     cmd = parseCommand(arg, el("{ \"command\": [ \"my_command\" ] }"));
     EXPECT_EQ("my_command", cmd);
-    EXPECT_TRUE(isNull(arg));
+    EXPECT_EQ(*arg, *Element::createMap());
 
     cmd = parseCommand(arg, el("{ \"command\": [ \"my_command\", 1 ] }"));
     EXPECT_EQ("my_command", cmd);
@@ -193,8 +193,8 @@ ConstElementPtr my_command_handler(const std::string& command,
     if (command == "good_command") {
         return (createAnswer());
     } else if (command == "command_with_arg") {
-        if (arg) {
-            if (arg->getType() == Element::integer) {
+        if (arg->contains("number")) {
+            if (arg->get("number")->getType() == Element::integer) {
                 return (createAnswer(0, el("2")));
             } else {
                 return (createAnswer(1, "arg bad type"));
@@ -235,10 +235,10 @@ TEST_F(CCSessionTest, checkCommand) {
     // client will ask for config
     session.getMessages()->add(createAnswer(0, el("{}")));
 
-    EXPECT_FALSE(session.haveSubscription("Spec2", "*"));
-    ModuleCCSession mccs(ccspecfile("spec2.spec"), session, my_config_handler,
+    EXPECT_FALSE(session.haveSubscription("Spec29", "*"));
+    ModuleCCSession mccs(ccspecfile("spec29.spec"), session, my_config_handler,
                          my_command_handler);
-    EXPECT_TRUE(session.haveSubscription("Spec2", "*"));
+    EXPECT_TRUE(session.haveSubscription("Spec29", "*"));
 
     EXPECT_EQ(2, session.getMsgQueue()->size());
     ConstElementPtr msg;
@@ -252,19 +252,19 @@ TEST_F(CCSessionTest, checkCommand) {
     EXPECT_EQ(0, result);
 
     // not a command, should be ignored
-    session.addMessage(el("1"), "Spec2", "*");
+    session.addMessage(el("1"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(0, result);
-
-    session.addMessage(el("{ \"command\": [ \"good_command\" ] }"), "Spec2",
+    session.addMessage(el("{ \"command\": [ \"good_command\" ] }"), "Spec29",
                        "*");
+
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
     EXPECT_EQ("{ \"result\": [ 0 ] }", msg->str());
     EXPECT_EQ(0, result);
 
-    session.addMessage(el("{ \"command\": \"bad_command\" }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": \"bad_command\" }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
@@ -272,53 +272,69 @@ TEST_F(CCSessionTest, checkCommand) {
     EXPECT_EQ(0, result);
 
     session.addMessage(el("{ \"command\": [ \"bad_command\" ] }"),
-                       "Spec2", "*");
+                       "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
     EXPECT_EQ("{ \"result\": [ 1, \"bad command\" ] }", msg->str());
     EXPECT_EQ(0, result);
 
-    session.addMessage(el("{ \"command\": [ \"command_with_arg\", 1 ] }"),
-                       "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"command_with_arg\", {\"number\": 1} ] }"),
+                       "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
     EXPECT_EQ("{ \"result\": [ 0, 2 ] }", msg->str());
     EXPECT_EQ(0, result);
 
-    session.addMessage(el("{ \"command\": [ \"command_with_arg\" ] }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"command_with_arg\" ] }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
     EXPECT_EQ("{ \"result\": [ 1, \"arg missing\" ] }", msg->str());
     EXPECT_EQ(0, result);
 
-    session.addMessage(el("{ \"command\": [ \"command_with_arg\", \"asdf\" ] }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"command_with_arg\", \"asdf\" ] }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
-    EXPECT_EQ("{ \"result\": [ 1, \"arg bad type\" ] }", msg->str());
+    EXPECT_EQ("{ \"result\": [ 3, \"Error in command validation: args for command command_with_arg is not a map\" ] }", msg->str());
     EXPECT_EQ(0, result);
 
     mccs.setCommandHandler(NULL);
-    session.addMessage(el("{ \"command\": [ \"whatever\" ] }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"whatever\" ] }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
     EXPECT_EQ("{ \"result\": [ 1, \"Command given but no command handler for module\" ] }", msg->str());
     EXPECT_EQ(0, result);
+}
+
+// A heuristic workaround for clang++: It doesn't seem to compile the whole
+// test, probably due to its length.  Dividing the tests into two separate
+// test cases seems to work.
+TEST_F(CCSessionTest, checkCommand2) {
+    session.getMessages()->add(createAnswer(0, el("{}")));
+    EXPECT_FALSE(session.haveSubscription("Spec29", "*"));
+    ModuleCCSession mccs(ccspecfile("spec29.spec"), session, my_config_handler,
+                         my_command_handler);
+    EXPECT_TRUE(session.haveSubscription("Spec29", "*"));
+    ConstElementPtr msg;
+    std::string group, to;
+    // checked above, drop em
+    msg = session.getFirstMessage(group, to);
+    msg = session.getFirstMessage(group, to);
 
     EXPECT_EQ(1, mccs.getValue("item1")->intValue());
-    session.addMessage(el("{ \"command\": [ \"config_update\", { \"item1\": 2 } ] }"), "Spec2", "*");
-    result = mccs.checkCommand();
+    session.addMessage(el("{ \"command\": [ \"config_update\", { \"item1\": 2 } ] }"), "Spec29", "*");
+    int result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
     EXPECT_EQ("{ \"result\": [ 0 ] }", msg->str());
     EXPECT_EQ(0, result);
     EXPECT_EQ(2, mccs.getValue("item1")->intValue());
 
-    session.addMessage(el("{ \"command\": [ \"config_update\", { \"item1\": \"asdf\" } ] }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"config_update\", { \"item1\": \"asdf\" } ] }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
@@ -326,7 +342,7 @@ TEST_F(CCSessionTest, checkCommand) {
     EXPECT_EQ(0, result);
     EXPECT_EQ(2, mccs.getValue("item1")->intValue());
 
-    session.addMessage(el("{ \"command\": [ \"config_update\", { \"item1\": 5 } ] }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"config_update\", { \"item1\": 5 } ] }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
@@ -387,9 +403,9 @@ TEST_F(CCSessionTest, ignoreRemoteConfigCommands) {
     // client will ask for config
     session.getMessages()->add(createAnswer(0, el("{  }")));
 
-    EXPECT_FALSE(session.haveSubscription("Spec2", "*"));
-    ModuleCCSession mccs(ccspecfile("spec2.spec"), session, my_config_handler, my_command_handler);
-    EXPECT_TRUE(session.haveSubscription("Spec2", "*"));
+    EXPECT_FALSE(session.haveSubscription("Spec29", "*"));
+    ModuleCCSession mccs(ccspecfile("spec29.spec"), session, my_config_handler, my_command_handler);
+    EXPECT_TRUE(session.haveSubscription("Spec29", "*"));
 
     EXPECT_EQ(2, session.getMsgQueue()->size());
     ConstElementPtr msg;
@@ -404,7 +420,7 @@ TEST_F(CCSessionTest, ignoreRemoteConfigCommands) {
     msg = session.getFirstMessage(group, to);
 
     // Check if commands for the module are handled
-    session.addMessage(el("{ \"command\": [ \"good_command\" ] }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"good_command\" ] }"), "Spec29", "*");
     int result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);

src/lib/config/tests/module_spec_unittests.cc

@@ -30,7 +30,7 @@ std::string specfile(const std::string name) {
 }
 
 void
-module_spec_error(const std::string& file,
+moduleSpecError(const std::string& file,
                const std::string& error1,
                const std::string& error2 = "",
                const std::string& error3 = "")
@@ -52,7 +52,7 @@ TEST(ModuleSpec, ReadingSpecfiles) {
                               ->stringValue(), "Spec1");
     dd = moduleSpecFromFile(specfile("spec2.spec"));
     EXPECT_EQ(dd.getFullSpec()->get("config_data")->size(), 6);
-    module_spec_error("doesnotexist",
+    moduleSpecError("doesnotexist",
                    "Error opening ",
                    specfile("doesnotexist"),
                    ": No such file or directory");
@@ -81,69 +81,65 @@ TEST(ModuleSpec, ReadingSpecfiles) {
 }
 
 TEST(ModuleSpec, SpecfileItems) {
-    module_spec_error("spec3.spec",
+    moduleSpecError("spec3.spec",
                    "item_name missing in { \"item_default\": 1, \"item_optional\": false, \"item_type\": \"integer\" }");
-    module_spec_error("spec4.spec",
+    moduleSpecError("spec4.spec",
                    "item_type missing in { \"item_default\": 1, \"item_name\": \"item1\", \"item_optional\": false }");
-    module_spec_error("spec5.spec",
+    moduleSpecError("spec5.spec",
                    "item_optional missing in { \"item_default\": 1, \"item_name\": \"item1\", \"item_type\": \"integer\" }");
-    module_spec_error("spec6.spec",
+    moduleSpecError("spec6.spec",
                    "item_default missing in { \"item_name\": \"item1\", \"item_optional\": false, \"item_type\": \"integer\" }");
-    module_spec_error("spec9.spec",
+    moduleSpecError("spec9.spec",
                    "item_default not of type integer");
-    module_spec_error("spec10.spec",
+    moduleSpecError("spec10.spec",
                    "item_default not of type real");
-    module_spec_error("spec11.spec",
+    moduleSpecError("spec11.spec",
                    "item_default not of type boolean");
-    module_spec_error("spec12.spec",
+    moduleSpecError("spec12.spec",
                    "item_default not of type string");
-    module_spec_error("spec13.spec",
+    moduleSpecError("spec13.spec",
                    "item_default not of type list");
-    module_spec_error("spec14.spec",
+    moduleSpecError("spec14.spec",
                    "item_default not of type map");
-    module_spec_error("spec15.spec",
+    moduleSpecError("spec15.spec",
                    "badname is not a valid type name");
 }
 
 TEST(ModuleSpec, SpecfileConfigData) {
-    module_spec_error("spec7.spec",
+    moduleSpecError("spec7.spec",
                    "module_name missing in {  }");
-    module_spec_error("spec8.spec",
+    moduleSpecError("spec8.spec",
                    "No module_spec in specification");
-    module_spec_error("spec16.spec",
+    moduleSpecError("spec16.spec",
                    "config_data is not a list of elements");
-    module_spec_error("spec21.spec",
+    moduleSpecError("spec21.spec",
                    "commands is not a list of elements");
 }
 
 TEST(ModuleSpec, SpecfileCommands) {
-    module_spec_error("spec17.spec",
+    moduleSpecError("spec17.spec",
                    "command_name missing in { \"command_args\": [ { \"item_default\": \"\", \"item_name\": \"message\", \"item_optional\": false, \"item_type\": \"string\" } ], \"command_description\": \"Print the given message to stdout\" }");
-    module_spec_error("spec18.spec",
+    moduleSpecError("spec18.spec",
                    "command_args missing in { \"command_description\": \"Print the given message to stdout\", \"command_name\": \"print_message\" }");
-    module_spec_error("spec19.spec",
+    moduleSpecError("spec19.spec",
                    "command_args not of type list");
-    module_spec_error("spec20.spec",
+    moduleSpecError("spec20.spec",
                    "somethingbad is not a valid type name");
-/*
-    module_spec_error("spec22.spec",
-                   "");
-*/
 }
 
 bool
-data_test(const ModuleSpec& dd, const std::string& data_file_name) {
+dataTest(const ModuleSpec& dd, const std::string& data_file_name) {
     std::ifstream data_file;
 
     data_file.open(specfile(data_file_name).c_str());
     ConstElementPtr data = Element::fromJSON(data_file, data_file_name);
     data_file.close();
 
-    return (dd.validate_config(data));
+    return (dd.validateConfig(data));
 }
 
 bool
-data_test_with_errors(const ModuleSpec& dd, const std::string& data_file_name,
+dataTestWithErrors(const ModuleSpec& dd, const std::string& data_file_name,
                       ElementPtr errors)
 {
     std::ifstream data_file;
@@ -152,27 +148,64 @@ data_test_with_errors(const ModuleSpec& dd, const std::string& data_file_name,
     ConstElementPtr data = Element::fromJSON(data_file, data_file_name);
     data_file.close();
 
-    return (dd.validate_config(data, true, errors));
+    return (dd.validateConfig(data, true, errors));
 }
 
 TEST(ModuleSpec, DataValidation) {
     ModuleSpec dd = moduleSpecFromFile(specfile("spec22.spec"));
 
-    EXPECT_TRUE(data_test(dd, "data22_1.data"));
-    EXPECT_FALSE(data_test(dd, "data22_2.data"));
-    EXPECT_FALSE(data_test(dd, "data22_3.data"));
-    EXPECT_FALSE(data_test(dd, "data22_4.data"));
-    EXPECT_FALSE(data_test(dd, "data22_5.data"));
-    EXPECT_TRUE(data_test(dd, "data22_6.data"));
-    EXPECT_TRUE(data_test(dd, "data22_7.data"));
-    EXPECT_FALSE(data_test(dd, "data22_8.data"));
-    EXPECT_FALSE(data_test(dd, "data22_9.data"));
+    EXPECT_TRUE(dataTest(dd, "data22_1.data"));
+    EXPECT_FALSE(dataTest(dd, "data22_2.data"));
+    EXPECT_FALSE(dataTest(dd, "data22_3.data"));
+    EXPECT_FALSE(dataTest(dd, "data22_4.data"));
+    EXPECT_FALSE(dataTest(dd, "data22_5.data"));
+    EXPECT_TRUE(dataTest(dd, "data22_6.data"));
+    EXPECT_TRUE(dataTest(dd, "data22_7.data"));
+    EXPECT_FALSE(dataTest(dd, "data22_8.data"));
+    EXPECT_FALSE(dataTest(dd, "data22_9.data"));
 
     ElementPtr errors = Element::createList();
-    EXPECT_FALSE(data_test_with_errors(dd, "data22_8.data", errors));
+    EXPECT_FALSE(dataTestWithErrors(dd, "data22_8.data", errors));
     EXPECT_EQ("[ \"Type mismatch\" ]", errors->str());
 
     errors = Element::createList();
-    EXPECT_FALSE(data_test_with_errors(dd, "data22_9.data", errors));
+    EXPECT_FALSE(dataTestWithErrors(dd, "data22_9.data", errors));
     EXPECT_EQ("[ \"Unknown item value_does_not_exist\" ]", errors->str());
 }
+
+TEST(ModuleSpec, CommandValidation) {
+    ModuleSpec dd = moduleSpecFromFile(specfile("spec2.spec"));
+    ConstElementPtr arg = Element::fromJSON("{}");
+    ElementPtr errors = Element::createList();
+
+    EXPECT_TRUE(dd.validateCommand("shutdown", arg, errors));
+    EXPECT_EQ(errors->size(), 0);
+
+    errors = Element::createList();
+    EXPECT_FALSE(dd.validateCommand("unknowncommand", arg, errors));
+    EXPECT_EQ(errors->size(), 1);
+    EXPECT_EQ(errors->get(0)->stringValue(), "Unknown command unknowncommand");
+
+    errors = Element::createList();
+    EXPECT_FALSE(dd.validateCommand("print_message", arg, errors));
+    EXPECT_EQ(errors->size(), 1);
+    EXPECT_EQ(errors->get(0)->stringValue(), "Non-optional value missing");
+
+    errors = Element::createList();
+    arg = Element::fromJSON("{ \"message\": \"Hello\" }");
+    EXPECT_TRUE(dd.validateCommand("print_message", arg, errors));
+    EXPECT_EQ(errors->size(), 0);
+
+    errors = Element::createList();
+    arg = Element::fromJSON("{ \"message\": \"Hello\", \"unknown_second_arg\": 1 }");
+    EXPECT_FALSE(dd.validateCommand("print_message", arg, errors));
+    EXPECT_EQ(errors->size(), 1);
+    EXPECT_EQ(errors->get(0)->stringValue(), "Unknown item unknown_second_arg");
+
+    errors = Element::createList();
+    arg = Element::fromJSON("{ \"message\": 1 }");
+    EXPECT_FALSE(dd.validateCommand("print_message", arg, errors));
+    EXPECT_EQ(errors->size(), 1);
+    EXPECT_EQ(errors->get(0)->stringValue(), "Type mismatch");
+
+}

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

@@ -49,3 +49,4 @@ EXTRA_DIST += spec25.spec
 EXTRA_DIST += spec26.spec
 EXTRA_DIST += spec27.spec
 EXTRA_DIST += spec28.spec
+EXTRA_DIST += spec29.spec

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

@@ -0,0 +1,35 @@
+{
+  "module_spec": {
+    "module_name": "Spec29",
+    "config_data": [
+      { "item_name": "item1",
+        "item_type": "integer",
+        "item_optional": false,
+        "item_default": 1
+      }
+    ],
+    "commands": [
+      {
+        "command_name": "good_command",
+        "command_description": "A good command",
+        "command_args": []
+      },
+      {
+        "command_name": "bad_command",
+        "command_description": "A bad command",
+        "command_args": []
+      },
+      {
+        "command_name": "command_with_arg",
+        "command_description": "A command with an (integer) argument",
+        "command_args": [ {
+          "item_name": "number",
+          "item_type": "integer",
+          "item_optional": true,
+          "item_default": 1
+        } ]
+      }
+    ]
+  }
+}
+

src/lib/datasrc/memory_datasrc.cc

@@ -40,6 +40,7 @@ struct MemoryZone::MemoryZoneImpl {
     // Information about the zone
     RRClass zone_class_;
     Name origin_;
+    string file_name_;
 
     // Some type aliases
     /*
@@ -84,8 +85,8 @@ struct MemoryZone::MemoryZoneImpl {
         DomainNode* node;
         switch (domains->insert(name, &node)) {
             // Just check it returns reasonable results
-            case DomainTree::SUCCEED:
-            case DomainTree::ALREADYEXIST:
+            case DomainTree::SUCCESS:
+            case DomainTree::ALREADYEXISTS:
                 break;
             // Something odd got out
             default:
@@ -275,10 +276,21 @@ MemoryZone::load(const string& filename) {
     masterLoad(filename.c_str(), getOrigin(), getClass(),
         boost::bind(&MemoryZoneImpl::addFromLoad, impl_, _1, &tmp));
     // If it went well, put it inside
+    impl_->file_name_ = filename;
     tmp.swap(impl_->domains_);
     // And let the old data die with tmp
 }
 
+void
+MemoryZone::swap(MemoryZone& zone) {
+    std::swap(impl_, zone.impl_);
+}
+
+const string
+MemoryZone::getFileName() const {
+    return (impl_->file_name_);
+}
+
 /// Implementation details for \c MemoryDataSrc hidden from the public
 /// interface.
 ///

src/lib/datasrc/memory_datasrc.h

@@ -15,6 +15,8 @@
 #ifndef __MEMORY_DATA_SOURCE_H
 #define __MEMORY_DATA_SOURCE_H 1
 
+#include <string>
+
 #include <datasrc/zonetable.h>
 
 namespace isc {
@@ -98,6 +100,20 @@ 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.
+    ///
+    /// 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
+    /// string if the zone hasn't loaded any file.
+    const std::string getFileName() const;
+
     /// \brief Load zone from masterfile.
     ///
     /// This loads data from masterfile specified by filename. It replaces
@@ -122,6 +138,15 @@ public:
     ///     This will probably be needed when a better implementation of
     ///     configuration reloading is written.
     void load(const std::string& filename);
+
+    /// Exchanges the content of \c this zone with that of the given \c zone.
+    ///
+    /// 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);
+
 private:
     /// \name Hidden private data
     //@{
@@ -158,10 +183,6 @@ private:
 /// 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.
-///
-/// Currently, \c FindResult::zone is immutable for safety.
-/// In future versions we may want to make it changeable.  For example,
-/// we may want to allow configuration update on an existing zone.
 class MemoryDataSrc {
 public:
     /// \brief A helper structure to represent the search result of
@@ -180,11 +201,11 @@ public:
     /// See the description of \c find() for the semantics of the member
     /// variables.
     struct FindResult {
-        FindResult(result::Result param_code, const ConstZonePtr param_zone) :
+        FindResult(result::Result param_code, const ZonePtr param_zone) :
             code(param_code), zone(param_zone)
         {}
         const result::Result code;
-        const ConstZonePtr zone;
+        const ZonePtr zone;
     };
 
     ///

src/lib/datasrc/rbtree.h

@@ -17,11 +17,11 @@
 
 //! \file datasrc/rbtree.h
 ///
-/// \note The purpose of the RBTree is to provide a generic map with 
-/// domain names as the key that can be used by various BIND 10 modules or
-/// even by other applications.  However, because of some unresolved design
-/// issue, the design and interface are not fixed, and RBTree isn't ready to
-/// be used as a base data structure by other modules.
+/// \note The purpose of the RBTree is to provide a generic map with
+///     domain names as the key that can be used by various BIND 10 modules or
+///     even by other applications.  However, because of some unresolved design
+///     issue, the design and interface are not fixed, and RBTree isn't ready
+///     to be used as a base data structure by other modules.
 
 #include <dns/name.h>
 #include <boost/utility.hpp>
@@ -35,16 +35,18 @@ namespace datasrc {
 
 namespace helper {
 
-/// Helper function to remove the base domain from super domain
+/// \brief Helper function to remove the base domain from super domain.
 ///
-/// the precondition of this function is the super_name contains the
+/// The precondition of this function is the super_name contains the
 /// sub_name so
 /// \code Name a("a.b.c");
 /// Name b("b.c");
 /// Name c = a - b;
 /// \endcode
+/// c will contain "a".
 ///
-/// \note function in this namespace is not intended to be used outside.
+/// \note Functions in this namespace is not intended to be used outside of
+///     RBTree implementation.
 inline isc::dns::Name
 operator-(const isc::dns::Name& super_name, const isc::dns::Name& sub_name) {
     return (super_name.split(0, super_name.getLabelCount() -
@@ -52,66 +54,99 @@ operator-(const isc::dns::Name& super_name, const isc::dns::Name& sub_name) {
 }
 }
 
-template <typename T>
+template <typename T, bool returnEmptyNode>
 class RBTree;
 
-/// \brief \c RBNode use by RBTree to store any data related to one domain name
+/// \brief \c RBNode is used by RBTree to store any data related to one domain
+///     name.
+///
+/// This is meant to be used only from RBTree. It is meaningless to inherit it
+/// or create instances of it from elsewhere. For that reason, the constructor
+/// is private.
+///
+/// It serves three roles. One is to keep structure of the \c RBTree as a
+/// red-black tree. For that purpose, it has left, right and parent pointers
+/// and color member. These are private and accessed only from within the tree.
+///
+/// The second one is to store data for one domain name. The data related
+/// functions can be used to access and set the data.
 ///
-/// It has two roles, the first one is as one node in the \c RBTree,
-/// the second one is to store the data related to one domain name and maintain
-/// the domain name hierarchy struct in one domain name space.
-/// As for the first role, it has left, right, parent and color members
-/// which is used to keep the balance of the \c RBTree.
-/// As for the second role, \c RBNode use down pointer to refer to all its sub
-/// domains, so the name of current node always relative to the up node. since
-/// we only has down pointer without up pointer, so we can only walk down from
-/// top domain to sub domain.
-/// One special kind of node is non-terminal node
-/// which has subdomains with RRset but itself doesn't have any RRsets.
+/// The third role is to keep the hierarchy of domains. The down pointer points
+/// to a subtree of subdomains. Note that we can traverse the hierarchy down,
+/// but not up.
 ///
-/// \note \c RBNode basically used internally by RBTree, it is meaningless to
-/// inherited from it or create it without \c RBTree.
+/// One special kind of node is non-terminal node. It has subdomains with
+/// RRsets, but doesn't have any RRsets itself.
 template <typename T>
 class RBNode : public boost::noncopyable {
+private:
+    /// The RBNode is meant for use from within RBTree, so it has access to
+    /// it.
+    template <typename U, bool returnEmptyNode>
+    friend class RBTree;
+
+    /// \name Constructors
+    ///
+    /// \note The existence of a RBNode without a RBTree is meaningless.
+    ///     Therefore the constructors are private.
+    //@{
+
+    /// \brief Default constructor.
+    ///
+    /// This constructor is provided specifically for generating a special
+    /// "null" node.
+    RBNode();
+
+    /// \brief Constructor from the node name.
+    ///
+    /// \param name The *relative* domain name (if this will live inside
+    ///     a.b.c and is called d.e.a.b.c, then you pass d.e).
+    RBNode(const isc::dns::Name& name);
+    //@}
+
 public:
-    /// only \c RBTree can create and destroy \c RBNode
-    friend class RBTree<T>;
+    /// \brief Alias for shared pointer to the data.
     typedef boost::shared_ptr<T> NodeDataPtr;
 
-    /// \name Destructor
-    /// \note it's seems a little strange that constructor is private
-    /// but deconstructor left public, the reason is for some smart pointer
-    /// like std::auto_ptr, they needs to delete RBNode in sometimes, but
-    /// \code delete *pointer_to_node \endcode shouldn't be called directly
-    //@{
+    /// \brief Destructor
+    ///
+    /// It might seem strange that constructors are private and destructor
+    /// public, but this is needed because of shared pointers need access
+    /// to the destructor.
+    ///
+    /// You should never call anything like:
+    /// \code delete pointer_to_node; \endcode
+    /// The RBTree handles both creation and destructoion of nodes.
     ~RBNode();
-    //@}
 
-    /// \name Test functions
+    /// \name Getter functions.
     //@{
-    /// \brief return the name of current node, it's relative to its top node
+    /// \brief Return the name of current node.
+    ///
+    /// It's relative to its containing node.
     ///
     /// To get the absolute name of one node, the node path from the top node
-    /// to current node has to be recorded
+    /// to current node has to be recorded.
     const isc::dns::Name& getName() const { return (name_); }
 
-    /// \brief return the data store in this node
-    /// \note, since the data is managed by RBNode, developer should not
-    /// free the pointer
+    /// \brief Return the data stored in this node.
+    ///
+    /// You should not delete the data, it is handled by shared pointers.
     NodeDataPtr& getData() { return (data_); }
-    /// \brief return the data stored in this node, read-only version
+    /// \brief Return the data stored in this node.
     const NodeDataPtr& getData() const { return (data_); }
 
-    /// \brief return whether the node has related data
-    /// \note it's meaningless has empty \c RBNode in one RBTree, the only
-    /// exception is for non-terminal node which has sub domain nodes who
-    /// has data(rrset)
+    /// \brief return whether the node has related data.
+    ///
+    /// There can be empty nodes inside the RBTree. They are usually the
+    /// non-terminal domains, but it is possible (yet probably meaningless)
+    /// empty nodes anywhere.
     bool isEmpty() const { return (data_.get() == NULL); }
     //@}
 
-    /// \name Modify functions
+    /// \name Setter functions.
     //@{
-    /// \breif set the data stored in the node
+    /// \brief Set the data stored in the node.
     void setData(const NodeDataPtr& data) { data_ = data; }
     //@}
 
@@ -122,8 +157,6 @@ public:
     /// These methods never throw an exception.
     //@{
     /// Return if callback is enabled at the node.
-    ///
-    /// This method never throws an exception.
     bool isCallbackEnabled() const { return (callback_required_); }
 
     /// Enable callback at the node.
@@ -137,55 +170,45 @@ public:
 private:
     /// \brief Define rbnode color
     enum RBNodeColor {BLACK, RED};
-
-    /// \name Constructors
-    /// \note \c Single RBNode is meaningless without living inside one \c RBTree
-    /// the creation and destroy of one \c RBNode is handle by host \c RBTree, so
-    /// the constructors and destructor of \c RBNode is left private
-    //@{
-    /// \brief Default constructor.
-    ///
-    /// This constructor is provided specifically for generating a special
-    /// "null" node, and is intended be used only internally.
-    RBNode();
-
-    /// \brief Constructor from the node name.
-    ///
-    /// \param name The domain name corresponding to the node.
-    RBNode(const isc::dns::Name& name);
-    //@}
-
     /// This is a factory class method of a special singleton null node.
     static RBNode<T>* NULL_NODE() {
         static RBNode<T> null_node;
         return (&null_node);
     }
 
-    /// data to maintain the rbtree balance
+    /// \name Data to maintain the rbtree structure.
+    //@{
     RBNode<T>*  parent_;
     RBNode<T>*  left_;
     RBNode<T>*  right_;
     RBNodeColor color_;
+    //@}
 
+    /// \brief Relative name of the node.
     isc::dns::Name     name_;
+    /// \brief Data stored here.
     NodeDataPtr       data_;
 
-    /// the down pointer points to the root node of sub domains of current
-    /// domain
-    /// \par Adding down pointer to \c RBNode is for two purpose:
-    /// \li Accelerate the search process, with sub domain tree, it split the
-    /// big flat tree into several hierarchy trees
-    /// \li It save memory useage, so same label won't be saved several times
+    /// \brief The subdomain tree.
+    ///
+    /// This points to the root node of trees of subdomains of this domain.
+    ///
+    /// \par Adding down pointer to \c RBNode has two purposes:
+    /// \li Accelerate the search process, with sub domain tree, it splits the
+    ///     big flat tree into several hierarchy trees.
+    /// \li It saves memory useage as it allows storing only relative names,
+    ///     avoiding storage of the same domain labels multiple times.
     RBNode<T>*  down_;
 
-    // If true, callback should be called at this node in search.
-    // (This may have to become part of more general "attribute flags")
+    /// \brief If callback should be called when traversing this node in
+    /// RBTree::find().
+    ///
+    /// \todo It might be needed to put it into more general attributes field.
     bool callback_required_;
 };
 
 
-// typically each node should has a name associate with it
-// this construction is only used to create \c NULLNODE
+// This is only to support NULL nodes.
 template <typename T>
 RBNode<T>::RBNode() :
     parent_(this),
@@ -216,30 +239,46 @@ template <typename T>
 RBNode<T>::~RBNode() {
 }
 
-// note: the following class description is documented using C-style comments
+
+// note: the following class description is documented using multiline comments
 // because the verbatim diagram contain a backslash, which could be interpreted
-// as part of a multi-line comment with C++ style comments.
+// as escape of newline in singleline comment.
 /**
- *  \brief \c RBTree class represents all the domains with the same suffix,
- *  so it can be used to store the domains in one zone.
- * 
- *  \c RBTree is a generic red black tree, and contains all the nodes with
- *  the same suffix, since each name may have sub domain names
- *  so \c RBTree is a recursive data structure namely tree in tree.
- *  So for one zone, several RBTrees may be involved. But from outside, the sub
- *  tree is opaque for end users.
- * 
- *  \c RBTree split the domain space into hierarchy red black trees, nodes in one
- *  tree has the same base name. The benefit of this struct is that:
- *  - enhance the query performace compared with one big flat red black tree
- *  - decrase the memory footprint to save common labels only once.
- * 
+ *  \brief \c RBTree class represents all the domains with the same suffix.
+ *      It can be used to store the domains in one zone, for example.
+ *
+ *  RBTree is a generic map from domain names to any kind of data. Internally,
+ *  it uses a red-black tree. However, it isn't one tree containing everything.
+ *  Subdomains are trees, so this structure is recursive - trees inside trees.
+ *  But, from the interface point of view, it is opaque data structure.
+ *
+ *  \c RBTree splits the domain space into hierarchy red black trees; nodes
+ * in one tree has the same base name. The benefit of this struct is that:
+ *  - Enhances the query performace compared with one big flat red black tree.
+ *  - Decreases the memory footprint, as it doesn't store the suffix labels
+ *      multiple times.
+ *
+ *  Depending on different usage, rbtree will support different search policy.
+ *  Whether return empty node to end user is one policy among them. Search
+ *  policy is as the last template parameter, the default policy will NOT
+ *  return empty node to end user, pass ture will get empty node during find
+ *  is needed
+ *
+ * \anchor diagram
+ *
+ * with the following names:
+ *  - a
+ *  - b
+ *  - c
+ *  - x.d.e.f
+ *  - z.d.e.f
+ *  - g.h
+ *  - o.w.y.d.e.f
+ *  - p.w.y.d.e.f
+ *  - q.w.y.d.e.f
+ *
+ * the tree will looks like:
  *  \verbatim
-  with the following names:
-      a       x.d.e.f     o.w.y.d.e.f
-      b       z.d.e.f     p.w.y.d.e.f
-      c       g.h         q.w.y.d.e.f
-      the tree will looks like:
                                 b
                               /   \
                              a    d.e.f
@@ -253,33 +292,34 @@ RBNode<T>::~RBNode() {
                                      p
                                     / \
                                    o   q
- *  \endverbatim
- *  \note open problems:
- *  - current find funciton only return non-empty nodes, so there is no difference
- *    between find one not exist name with empty non-terminal nodes, but in DNS query
- *    logic, they are different
+   \endverbatim
  *  \todo
  *  - add remove interface
- *  - add iterator to iterate the whole rbtree while may needed by axfr
- *  - since \c RBNode only has down pointer without up pointer, the node path during finding
- *    should be recorded for later use
+ *  - add iterator to iterate over the whole \c RBTree.  This may be necessary,
+ *    for example, to support AXFR.
+ *  - since \c RBNode only has down pointer without up pointer, the node path
+ *    during finding should be recorded for later use
  */
-template <typename T>
+template <typename T, bool returnEmptyNode = false>
 class RBTree : public boost::noncopyable {
     friend class RBNode<T>;
 public:
-    /// \brief The return value for the \c find() insert() and erase() method
+    /// \brief The return value for the \c find() and insert() methods
     enum Result {
-        SUCCEED, //insert or erase succeed
-        EXACTMATCH, //find the target name
-        PARTIALMATCH, //find part of target name
-        NOTFOUND,  // for find function means no related name found
-                   // for erase function means erase not exist name
-        ALREADYEXIST, //for insert operation, the name to insert already exist
+        SUCCESS,    ///< Insert was successful
+        /// \brief The node returned from find mathes exactly the name given
+        EXACTMATCH,
+        PARTIALMATCH, ///< A superdomain node was found
+        NOTFOUND,   ///< Not even any superdomain was found
+        /// \brief Returned by insert() if a node of the name already exists
+        ALREADYEXISTS,
     };
 
     /// \name Constructor and Destructor
     //@{
+    /// The constructor.
+    ///
+    /// It never throws an exception.
     explicit RBTree();
 
     /// \b Note: RBTree is not intended to be inherited so the destructor
@@ -287,130 +327,157 @@ public:
     ~RBTree();
     //@}
 
-    /// \name Inquery methods
-    //@{
-    /// \brief Find the node that gives a longest match against the given name
-    ///
-    /// This method searches the \c RBTree for a node whose name is a longest
-    /// match against \c name.  The found node, if any, is returned via the
-    /// \c node pointer.
-    /// By default, nodes that don't have data will be ignored, and the result
-    /// can be \c NOTFOUND even if there is a node whose name matches the
-    /// given \c name.
-    /// We'll soon introduce a "no data OK" mode in this method.  It would
-    /// match any node of the tree regardless of whether the node has data
-    /// or not.
-    /// Since the tree is "compressed", i.e., a node can contain multiple
-    /// name labels, there are counter intuitive cases in the "no data OK"
-    /// mode.  For example, see the diagram of the class description.
-    /// Name "y.d.e.f" is logically contained in the tree as part of the
-    /// "compressed" node of "w.y".  But the search logic of this method
-    /// cannot find the logical match, and would return a \c PARTIALMATCH
-    /// result pointing to node "d.e.f".  To correctly identify the real
-    /// longest match, "y.d.e.f" with empty data, the caller needs to
-    /// perform additional steps.
-    ///
-    /// This version of \c find() method is templated to allow the caller
-    /// to specify a "hook" at nodes that give a partial match.
-    /// When the search encounters a node with data that partially matches
-    /// \c name (i.e. node's name is a superdomain of \c name) and has
-    /// enabled callback (via the \c RBNode::enableCallback() method), if
-    /// \c callback is non \c NULL then the callback function is called
-    /// with the argument of a reference to the node and the given
-    /// callback argument (\c callback_arg).  The template parameter specifies
-    /// the type of the callback argument.
-    /// The callback function returns either \c true or \c false, meaning
-    /// the search should stop or continue, respectively.
-    /// If the return value is \c true the search stops immediately at the
-    /// node even if there could be a longer matching name below it.
-    /// In reality, this convoluted callback rule is specifically intended
-    /// to be used to handle a zone cut (delegation) at a name search inside
-    /// a zone, and won't be used in any other cases.
-    /// Other applications of the tree won't need callbacks, and they should
-    /// use the non templated version of the \c find() method.
-    ///
-    /// Since the expected usage of callback is very limited, we do not
-    /// generalize the interface so that it can be an arbitrary functions or
-    /// functor objects in favor of simplicity and efficiency.
-    ///
-    /// This method involves operations on names that can throw an exception.
+    /// \name Find methods
+    ///
+    /// \brief Find the node that gives a longest match against the given name.
+    ///
+    /// \anchor find
+    ///
+    /// These methods search the RBTree for a node whose name is a longest
+    /// against name. The found node, if any, is returned via the node pointer.
+    ///
+    /// By default, nodes that don't have data (see RBNode::isEmpty) are
+    /// ignored and the result can be NOTFOUND even if there's a node whose
+    /// name mathes. The plan is to introduce a "no data OK" mode for this
+    /// method, that would match any node of the tree regardless of wheather
+    /// the node has any data or not.
+    ///
+    /// The case with "no data OK" mode is not as easy as it seems. For example
+    /// in the diagram shown in the class description, the name y.d.e.f is
+    /// logically contained in the tree as part of the node w.y.  It cannot be
+    /// identified simply by checking whether existing nodes (such as
+    /// d.e.f or w.y) has data.
+    ///
+    /// These methods involves operations on names that can throw an exception.
     /// If that happens the exception will be propagated to the caller.
     /// The callback function should generally not throw an exception, but
     /// if it throws, the exception will be propagated to the caller.
     ///
+    /// The \c name parameter says what should be found. The node parameter
+    /// is output only and in case of EXACTMATCH and PARTIALMATCH, it is set
+    /// to a pointer to the found node.
+    ///
+    /// They return:
+    ///  - EXACTMATCH when a node with the same name as requested exists.
+    ///  - PARTIALMATCH when a node with the same name does not exist (or is
+    ///    empty), but there's a (nonempty) superdomain of the requested one.
+    ///    The superdomain with longest name is returned through the node
+    ///    parameter. Beware that if you store a zone in the tree, you may get
+    ///    PARTIALMATCH with zone apex when the given domain name is not there.
+    ///    You should not try to delegate into another zone in that case.
+    ///  - NOTFOUND if there's no node with the same name nor any superdomain
+    ///    of it. In that case, node parameter is left intact.
+    //@{
+
+    /// \brief Find with callback.
+    ///
+    /// \anchor callback
+    ///
+    /// This version of find calls the callback whenever traversing (on the
+    /// way from root down the tree) a marked node on the way down through the
+    /// domain namespace (see RBNode::enableCallback and related functions).
+    ///
+    /// If you return true from the callback, the search is stopped and a
+    /// PARTIALMATCH is returned with the given node. Note that this node
+    /// doesn't really need to be the one with longest possible match.
+    ///
+    /// This callback mechanism was designed with zone cut (delegation)
+    /// processing in mind. The marked nodes would be the ones at delegation
+    /// points. It is not expected that any other applications would need
+    /// callbacks; they should use the versions of find without callbacks.
+    /// The callbacks are not general functors for the same reason - we don't
+    /// expect it to be needed.
+    ///
     /// \param name Target to be found
     /// \param node On success (either \c EXACTMATCH or \c PARTIALMATCH)
-    /// it will store a pointer to the matching node
+    ///     it will store a pointer to the matching node
     /// \param callback If non \c NULL, a call back function to be called
-    /// at "delegation" nodes (see above).
+    ///     at marked nodes (see above).
     /// \param callback_arg A caller supplied argument to be passed to
-    /// \c callback.
-    ///
-    /// \return \c EXACTMATCH A node that whose name is equal to \c name is
-    /// found.  \c *node will be set to point to that node.
-    /// \return \c PARTIALMATCH There is a no exact match, but a superdomain
-    /// of \c name exists.  \c node will be set to point to the node whose
-    /// name is the longest among such superdomains.
-    /// \return \c NOTFOUND There is no exact or partial match against \c name
-    /// \c *node will be intact in this case.
+    ///     \c callback.
+    ///
+    /// \return As described above, but in case of callback returning true,
+    ///     it returns immediately with the current node.
     template <typename CBARG>
     Result find(const isc::dns::Name& name, RBNode<T>** node,
                 bool (*callback)(const RBNode<T>&, CBARG),
                 CBARG callback_arg) const;
 
-    /// Same as the other version, but the returned \c node will be immutable.
+    /// \brief Find with callback returning immutable node.
+    ///
+    /// It has the same behaviour as the find with \ref callback version.
     template <typename CBARG>
     Result find(const isc::dns::Name& name, const RBNode<T>** node,
                 bool (*callback)(const RBNode<T>&, CBARG),
                 CBARG callback_arg) const;
 
-    /// Same as the templated version, but does not use callback.
+    /// \brief Simple find.
     ///
-    /// Applications except the zone implementation should generally use the
-    /// non templated version.
+    /// Acts as described in the \ref find section.
     Result find(const isc::dns::Name& name, RBNode<T>** node) const {
         return (find<void*>(name, node, NULL, NULL));
     }
 
-    /// Same as the templated version, but does not use callback, and the
-    /// returned \c node will be immutable.
+    /// \brieg Simple find returning immutable node.
     ///
-    /// In general, this version should be preferred over the other non
-    /// templated version, unless the caller knows it should modify the
-    /// returned node.
+    /// Acts as described in the \ref find section, but returns immutable node
+    /// pointer.
     Result find(const isc::dns::Name& name, const RBNode<T>** node) const {
         return (find<void*>(name, node, NULL, NULL));
     }
-
-    /// \brief Get the total node count in the tree
-    /// the node count including the node created common suffix node,
-    /// this function will only be used for debuging
-    int getNodeCount() const { return (node_count_);}
     //@}
 
+    /// \brief Get the total number of nodes in the tree
+    ///
+    /// It includes nodes internally created as a result of adding a domain
+    /// name that is a subdomain of an existing node of the tree.
+    /// This function is mainly intended to be used for debugging.
+    int getNodeCount() const { return (node_count_); }
+
     /// \name Debug function
     //@{
-    /// \brief print the nodes in the trees
+    /// \brief Print the nodes in the trees.
+    ///
+    /// \param os A \c std::ostream object to which the tree is printed.
+    /// \param depth A factor of the initial indentation.  Each line
+    /// will begin with space character repeating <code>5 * depth</code>
+    /// times.
     void dumpTree(std::ostream& os, unsigned int depth = 0) const;
     //@}
 
-    /// \name Modify function
+    /// \name Modify functions
     //@{
-    /// \brief Insert the domain name into the tree
-    /// \param name The name to be inserted into the tree
-    /// \param inserted_node If no node with the name in the tree,
-    /// new \c RBNode will be created, otherwise nothing will be done.
-    /// Anyway the pointer point to the node with the name will be assigned to
-    /// inserted_node
+    /// \brief Insert the domain name into the tree.
+    ///
+    /// It either finds an already existing node of the given name or inserts
+    /// a new one, if none exists yet. In any case, the inserted_node parameter
+    /// is set to point to that node. You can fill data into it or modify it.
+    /// So, if you don't know if a node exists or not and you need to modify
+    /// it, just call insert and act by the result.
+    ///
+    /// Please note that the tree can add some empty nodes by itself, so don't
+    /// assume that if you didn't insert a node of that name it doesn't exist.
+    ///
+    /// This method normally involves resource allocation.  If it fails
+    /// the corresponding standard exception will be thrown.
+    ///
+    /// This method does not provide the strong exception guarantee in its
+    /// strict sense; if an exception is thrown in the middle of this
+    /// method, the internal structure may change.  However, it should
+    /// still retain the same property as a mapping container before this
+    /// method is called.  For example, the result of \c find() should be
+    /// the same.  This method provides the weak exception guarantee in its
+    /// normal sense.
+    ///
+    /// \param name The name to be inserted into the tree.
+    /// \param inserted_node This is an output parameter and is set to the
+    ///     node.
+    ///
     /// \return
-    //  - SUCCEED means no node exists in the tree with the name before insert
-    /// - ALREADYEXIST means already has the node with the given name
-    //
-    /// \node To modify the data related with one name but not sure the name has
-    /// inserted or not, it is better to call \c insert,instead of
-    /// \c find(), in case the name isn't exist and needs to insert again
+    ///  - SUCCESS The node was added.
+    ///  - ALREADYEXISTS There was already a node of that name, so it was not
+    ///     added.
     Result insert(const isc::dns::Name& name, RBNode<T>** inserted_node);
-    //@}
 
     /// \brief Swaps two tree's contents.
     ///
@@ -421,6 +488,7 @@ public:
         std::swap(NULLNODE, other.NULLNODE);
         std::swap(node_count_, other.node_count_);
     }
+    //@}
 
 private:
     /// \name RBTree balance functions
@@ -435,14 +503,15 @@ private:
     /// \brief delete tree whose root is equal to node
     void deleteHelper(RBNode<T> *node);
     /// \brief find the node with name
-    /// \param name is the target, up will points to the base domain of
-    /// the tree which name resides, node will point to the target node
-    /// if we has exact same name or partical name in current tree.
-    /// so for example, in zone a, we has
-    /// b.a, c.b.a and d.b.a search c.b.a, up will points to b.a.
-    /// and node will points to c.b.a
-    /// \note parameter up now is not used by any funciton, but we are gonna
-    /// need it soon to implement function like remove
+    ///
+    /// Internal searching function.
+    ///
+    /// \param name What should be found.
+    /// \param up It will point to the node whose down pointer points
+    ///     to the tree containing node. If we looked for o.w.y.d.e.f in the
+    ///     \ref diagram, the up would point to the w.y node.
+    ///     This parameter is not used currently, but it will be soon.
+    /// \param node The found node.
     template <typename CBARG>
     Result findHelper(const isc::dns::Name& name, const RBNode<T>** up,
                       RBNode<T>** node,
@@ -450,9 +519,7 @@ private:
                       CBARG callback_arg) const;
     void dumpTreeHelper(std::ostream& os, const RBNode<T>* node,
                         unsigned int depth) const;
-    /// for indent purpose, add certian mount empty charachter to output stream
-    /// according to the depth. This is a helper function which is only used when
-    /// dump tree
+    /// \brief Indentation helper function for dumpTree
     static void indent(std::ostream& os, unsigned int depth);
 
     /// Split one node into two nodes, keep the old node and create one new
@@ -468,21 +535,22 @@ private:
     unsigned int node_count_;
 };
 
-template <typename T>
-RBTree<T>::RBTree() {
+template <typename T, bool S>
+RBTree<T,S>::RBTree() {
     NULLNODE = RBNode<T>::NULL_NODE();
     root_ = NULLNODE;
     node_count_ = 0;
 }
 
-template <typename T>
-RBTree<T>::~RBTree() {
+template <typename T, bool S>
+RBTree<T,S>::~RBTree() {
     deleteHelper(root_);
     assert(node_count_ == 0);
 }
 
-template <typename T>
-void RBTree<T> ::deleteHelper(RBNode<T> *root) {
+template <typename T, bool S>
+void 
+RBTree<T,S>::deleteHelper(RBNode<T> *root) {
     if (root == NULLNODE) {
         return;
     }
@@ -511,9 +579,10 @@ void RBTree<T> ::deleteHelper(RBNode<T> *root) {
     --node_count_;
 }
 
-template <typename T> template <typename CBARG>
-typename RBTree<T>::Result
-RBTree<T>::find(const isc::dns::Name& name, RBNode<T>** node,
+template <typename T, bool S>
+template <typename CBARG>
+typename RBTree<T,S>::Result
+RBTree<T,S>::find(const isc::dns::Name& name, RBNode<T>** node,
                 bool (*callback)(const RBNode<T>&, CBARG),
                 CBARG callback_arg) const
 {
@@ -521,9 +590,10 @@ RBTree<T>::find(const isc::dns::Name& name, RBNode<T>** node,
     return (findHelper(name, &up_node, node, callback, callback_arg));
 }
 
-template <typename T> template <typename CBARG>
-typename RBTree<T>::Result
-RBTree<T>::find(const isc::dns::Name& name, const RBNode<T>** node,
+template <typename T, bool S>
+template <typename CBARG>
+typename RBTree<T,S>::Result
+RBTree<T,S>::find(const isc::dns::Name& name, const RBNode<T>** node,
                 bool (*callback)(const RBNode<T>&, CBARG),
                 CBARG callback_arg) const
 {
@@ -537,13 +607,14 @@ RBTree<T>::find(const isc::dns::Name& name, const RBNode<T>** node,
     return (ret);
 }
 
-template <typename T> template <typename CBARG>
-typename RBTree<T>::Result
-RBTree<T>::findHelper(const isc::dns::Name& target_name,
-                      const RBNode<T>** up_node,
-                      RBNode<T>** target,
-                      bool (*callback)(const RBNode<T>&, CBARG),
-                      CBARG callback_arg) const
+template <typename T, bool returnEmptyNode>
+template <typename CBARG>
+typename RBTree<T,returnEmptyNode>::Result
+RBTree<T,returnEmptyNode>::findHelper(const isc::dns::Name& target_name,
+                                      const RBNode<T>** up_node,
+                                      RBNode<T>** target,
+                                      bool (*callback)(const RBNode<T>&, CBARG),
+                                      CBARG callback_arg) const
 {
     using namespace helper;
 
@@ -558,7 +629,7 @@ RBTree<T>::findHelper(const isc::dns::Name& target_name,
         const isc::dns::NameComparisonResult::NameRelation relation =
             compare_result.getRelation();
         if (relation == isc::dns::NameComparisonResult::EQUAL) {
-            if (!node->isEmpty()) {
+            if (returnEmptyNode || !node->isEmpty()) {
                 *target = node;
                 ret = EXACTMATCH;
             }
@@ -571,7 +642,7 @@ RBTree<T>::findHelper(const isc::dns::Name& target_name,
                 node = (compare_result.getOrder() < 0) ?
                     node->left_ : node->right_;
             } else if (relation == isc::dns::NameComparisonResult::SUBDOMAIN) {
-                if (!node->isEmpty()) {
+                if (returnEmptyNode || !node->isEmpty()) {
                     ret = RBTree<T>::PARTIALMATCH;
                     *target = node;
                     if (callback != NULL && node->callback_required_) {
@@ -592,9 +663,11 @@ RBTree<T>::findHelper(const isc::dns::Name& target_name,
     return (ret);
 }
 
-template <typename T>
-typename RBTree<T>::Result
-RBTree<T>::insert(const isc::dns::Name& target_name, RBNode<T>** new_node) {
+
+template <typename T, bool returnEmptyNode>
+typename RBTree<T,returnEmptyNode>::Result
+RBTree<T,returnEmptyNode>::insert(const isc::dns::Name& target_name,
+                                  RBNode<T>** new_node) {
     using namespace helper;
     RBNode<T>* parent = NULLNODE;
     RBNode<T>* current = root_;
@@ -611,7 +684,12 @@ RBTree<T>::insert(const isc::dns::Name& target_name, RBNode<T>** new_node) {
             if (new_node != NULL) {
                 *new_node = current;
             }
-            return (ALREADYEXIST);
+
+            if (current->isEmpty() && !returnEmptyNode) {
+                return (SUCCESS);
+            } else {
+                return (ALREADYEXISTS);
+            }
         } else {
             const int common_label_count = compare_result.getCommonLabels();
             if (common_label_count == 1) {
@@ -664,21 +742,25 @@ RBTree<T>::insert(const isc::dns::Name& target_name, RBNode<T>** new_node) {
 
     ++node_count_;
     node.release();
-    return (SUCCEED);
+    return (SUCCESS);
 }
 
-template <typename T>
+
+template <typename T, bool S>
 void
-RBTree<T>::nodeFission(RBNode<T>& node, const isc::dns::Name& base_name) {
+RBTree<T,S>::nodeFission(RBNode<T>& node, const isc::dns::Name& base_name) {
     using namespace helper;
     const isc::dns::Name sub_name = node.name_ - base_name;
     // using auto_ptr here is to avoid memory leak in case of exception raised
     // after the RBNode creation
     std::auto_ptr<RBNode<T> > down_node(new RBNode<T>(sub_name));
+    node.name_ = base_name;
+    // the rest of this function should be exception free so that it keeps
+    // consistent behavior (i.e., a weak form of strong exception guarantee)
+    // even if code after the call to this function throws an exception.
     std::swap(node.data_, down_node->data_);
     std::swap(node.callback_required_, down_node->callback_required_);
     down_node->down_ = node.down_;
-    node.name_ = base_name;
     node.down_ = down_node.get();
     // root node of sub tree, the initial color is BLACK
     down_node->color_ = RBNode<T>::BLACK;
@@ -686,9 +768,10 @@ RBTree<T>::nodeFission(RBNode<T>& node, const isc::dns::Name& base_name) {
     down_node.release();
 }
 
-template <typename T>
+
+template <typename T, bool S>
 void
-RBTree<T>::insertRebalance(RBNode<T>** root, RBNode<T>* node) {
+RBTree<T,S>::insertRebalance(RBNode<T>** root, RBNode<T>* node) {
 
     RBNode<T>* uncle;
     while (node != *root && node->parent_->color_ == RBNode<T>::RED) {
@@ -732,9 +815,9 @@ RBTree<T>::insertRebalance(RBNode<T>** root, RBNode<T>* node) {
 }
 
 
-template <typename T>
+template <typename T, bool S>
 RBNode<T>*
-RBTree<T>::leftRotate(RBNode<T>** root, RBNode<T>* node) {
+RBTree<T,S>::leftRotate(RBNode<T>** root, RBNode<T>* node) {
     RBNode<T>* right = node->right_;
     node->right_ = right->left_;
     if (right->left_ != NULLNODE)
@@ -757,9 +840,9 @@ RBTree<T>::leftRotate(RBNode<T>** root, RBNode<T>* node) {
     return (node);
 }
 
-template <typename T>
+template <typename T, bool S>
 RBNode<T>*
-RBTree<T>::rightRotate(RBNode<T>** root, RBNode<T>* node) {
+RBTree<T,S>::rightRotate(RBNode<T>** root, RBNode<T>* node) {
     RBNode<T>* left = node->left_;
     node->left_ = left->right_;
     if (left->right_ != NULLNODE)
@@ -781,17 +864,18 @@ RBTree<T>::rightRotate(RBNode<T>** root, RBNode<T>* node) {
     return (node);
 }
 
-template <typename T>
+
+template <typename T, bool S>
 void
-RBTree<T>::dumpTree(std::ostream& os, unsigned int depth) const {
+RBTree<T,S>::dumpTree(std::ostream& os, unsigned int depth) const {
     indent(os, depth);
     os << "tree has " << node_count_ << " node(s)\n";
     dumpTreeHelper(os, root_, depth);
 }
 
-template <typename T>
+template <typename T, bool S>
 void
-RBTree<T>::dumpTreeHelper(std::ostream& os, const RBNode<T>* node,
+RBTree<T,S>::dumpTreeHelper(std::ostream& os, const RBNode<T>* node,
                           unsigned int depth) const
 {
     if (node == NULLNODE) {
@@ -816,13 +900,15 @@ RBTree<T>::dumpTreeHelper(std::ostream& os, const RBNode<T>* node,
     dumpTreeHelper(os, node->right_, depth + 1);
 }
 
-template <typename T>
+template <typename T, bool S>
 void
-RBTree<T>::indent(std::ostream& os, unsigned int depth) {
+RBTree<T,S>::indent(std::ostream& os, unsigned int depth) {
     static const unsigned int INDENT_FOR_EACH_DEPTH = 5;
     os << std::string(depth * INDENT_FOR_EACH_DEPTH, ' ');
 }
 
+
+
 }
 }
 

src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -405,4 +405,58 @@ TEST_F(MemoryZoneTest, load) {
         MasterLoadError);
 }
 
+TEST_F(MemoryZoneTest, swap) {
+    // build one zone with some data
+    MemoryZone zone1(class_, origin_);
+    EXPECT_EQ(result::SUCCESS, zone1.add(rr_ns_));
+    EXPECT_EQ(result::SUCCESS, zone1.add(rr_ns_aaaa_));
+
+    // build another zone of a different RR class with some other data
+    const Name other_origin("version.bind");
+    ASSERT_NE(origin_, other_origin); // make sure these two are different
+    MemoryZone zone2(RRClass::CH(), other_origin);
+    EXPECT_EQ(result::SUCCESS,
+              zone2.add(RRsetPtr(new RRset(Name("version.bind"),
+                                           RRClass::CH(), RRType::TXT(),
+                                           RRTTL(0)))));
+
+    zone1.swap(zone2);
+    EXPECT_EQ(other_origin, zone1.getOrigin());
+    EXPECT_EQ(origin_, zone2.getOrigin());
+    EXPECT_EQ(RRClass::CH(), zone1.getClass());
+    EXPECT_EQ(RRClass::IN(), zone2.getClass());
+    // make sure the zone data is swapped, too
+    findTest(origin_, RRType::NS(), Zone::NXDOMAIN, false, ConstRRsetPtr(),
+             &zone1);
+    findTest(other_origin, RRType::TXT(), Zone::SUCCESS, false,
+             ConstRRsetPtr(), &zone1);
+    findTest(origin_, RRType::NS(), Zone::SUCCESS, false, ConstRRsetPtr(),
+             &zone2);
+    findTest(other_origin, RRType::TXT(), Zone::NXDOMAIN, false,
+             ConstRRsetPtr(), &zone2);
+}
+
+TEST_F(MemoryZoneTest, getFileName) {
+    // for an empty zone the file name should also be empty.
+    EXPECT_TRUE(zone_.getFileName().empty());
+
+    // if loading a zone fails the file name shouldn't be set.
+    EXPECT_THROW(zone_.load(TEST_DATA_DIR "/root.zone"), MasterLoadError);
+    EXPECT_TRUE(zone_.getFileName().empty());
+
+    // after a successful load, the specified file name should be set
+    MemoryZone rootzone(class_, Name("."));
+    EXPECT_NO_THROW(rootzone.load(TEST_DATA_DIR "/root.zone"));
+    EXPECT_EQ(TEST_DATA_DIR "/root.zone", rootzone.getFileName());
+    // overriding load, which will fail
+    EXPECT_THROW(rootzone.load(TEST_DATA_DIR "/duplicate_rrset.zone"),
+                 MasterLoadError);
+    // the file name should be intact.
+    EXPECT_EQ(TEST_DATA_DIR "/root.zone", rootzone.getFileName());
+
+    // After swap, file names should also be swapped.
+    zone_.swap(rootzone);
+    EXPECT_EQ(TEST_DATA_DIR "/root.zone", zone_.getFileName());
+    EXPECT_TRUE(rootzone.getFileName().empty());
+}
 }

src/lib/datasrc/tests/rbtree_unittest.cc

@@ -51,30 +51,21 @@ namespace {
 class RBTreeTest : public::testing::Test {
 protected:
     RBTreeTest() : rbtree() {
-        rbtree.insert(Name("c"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(1)));
-        rbtree.insert(Name("b"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(2)));
-        rbtree.insert(Name("a"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(3)));
-        rbtree.insert(Name("x.d.e.f"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(4)));
-        rbtree.insert(Name("z.d.e.f"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(5)));
-        rbtree.insert(Name("g.h"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(6)));
-        rbtree.insert(Name("i.g.h"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(7)));
-        rbtree.insert(Name("o.w.y.d.e.f"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(8)));
-        rbtree.insert(Name("j.z.d.e.f"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(9)));
-        rbtree.insert(Name("p.w.y.d.e.f"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(10)));
-        rbtree.insert(Name("q.w.y.d.e.f"), &rbtnode);
-        rbtnode->setData(RBNode<int>::NodeDataPtr(new int(11)));
+        const char * domain_names[] = {"c", "b", "a", "x.d.e.f", "z.d.e.f", "g.h", "i.g.h", 
+            "o.w.y.d.e.f", "j.z.d.e.f", "p.w.y.d.e.f", "q.w.y.d.e.f"};
+        int name_count = sizeof(domain_names) / sizeof(domain_names[0]);
+        for (int i = 0; i < name_count; ++i) {
+            rbtree.insert(Name(domain_names[i]), &rbtnode);
+            rbtnode->setData(RBNode<int>::NodeDataPtr(new int(i + 1)));
+
+            rbtree_expose_empty_node.insert(Name(domain_names[i]), &rbtnode);
+            rbtnode->setData(RBNode<int>::NodeDataPtr(new int(i + 1)));
+
+        }
     }
+
     RBTree<int> rbtree;
+    RBTree<int, true> rbtree_expose_empty_node;
     RBNode<int>* rbtnode;
     const RBNode<int>* crbtnode;
 };
@@ -90,64 +81,126 @@ TEST_F(RBTreeTest, setGetData) {
 }
 
 TEST_F(RBTreeTest, insertNames) {
-    EXPECT_EQ(RBTree<int>::ALREADYEXIST, rbtree.insert(Name("d.e.f"), &rbtnode));
+    //if don't expose empty node, even the node already exsit which is caused by node fission
+    //we will return succeed
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("d.e.f"), &rbtnode));
     EXPECT_EQ(Name("d.e.f"), rbtnode->getName());
     EXPECT_EQ(13, rbtree.getNodeCount());
 
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("."), &rbtnode));
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, 
+            rbtree_expose_empty_node.insert(Name("d.e.f"), &rbtnode));
+    EXPECT_EQ(Name("d.e.f"), rbtnode->getName());
+    EXPECT_EQ(13, rbtree_expose_empty_node.getNodeCount());
+
+
+    //insert not exist node
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("."), &rbtnode));
     EXPECT_EQ(Name("."), rbtnode->getName());
     EXPECT_EQ(14, rbtree.getNodeCount());
 
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("example.com"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("."), &rbtnode));
+    EXPECT_EQ(Name("."), rbtnode->getName());
+    EXPECT_EQ(14, rbtree_expose_empty_node.getNodeCount());
+    
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("example.com"), &rbtnode));
     EXPECT_EQ(15, rbtree.getNodeCount());
     rbtnode->setData(RBNode<int>::NodeDataPtr(new int(12)));
 
-    // return ALREADYEXIST, since node "example.com" already has been explicitly inserted
-    EXPECT_EQ(RBTree<int>::ALREADYEXIST, rbtree.insert(Name("example.com"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("example.com"), &rbtnode));
+    EXPECT_EQ(15, rbtree_expose_empty_node.getNodeCount());
+    rbtnode->setData(RBNode<int>::NodeDataPtr(new int(12)));
+
+
+    // return ALREADYEXISTS, since node "example.com" already has been explicitly inserted
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree.insert(Name("example.com"), &rbtnode));
     EXPECT_EQ(15, rbtree.getNodeCount());
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree_expose_empty_node.insert(Name("example.com"), &rbtnode));
+    EXPECT_EQ(15, rbtree_expose_empty_node.getNodeCount());
+
 
     // split the node "d.e.f"
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("k.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("k.e.f"), &rbtnode));
     EXPECT_EQ(Name("k"), rbtnode->getName());
     EXPECT_EQ(17, rbtree.getNodeCount());
 
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("k.e.f"), &rbtnode));
+    EXPECT_EQ(Name("k"), rbtnode->getName());
+    EXPECT_EQ(17, rbtree_expose_empty_node.getNodeCount());
+
+
     // split the node "g.h"
-    EXPECT_EQ(RBTree<int>::ALREADYEXIST, rbtree.insert(Name("h"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("h"), &rbtnode));
     EXPECT_EQ(Name("h"), rbtnode->getName());
     EXPECT_EQ(18, rbtree.getNodeCount());
 
+    //node fission will create node "h"
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree_expose_empty_node.insert(Name("h"), &rbtnode));
+    EXPECT_EQ(Name("h"), rbtnode->getName());
+    EXPECT_EQ(18, rbtree_expose_empty_node.getNodeCount());
+
+
     // add child domain
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("m.p.w.y.d.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("m.p.w.y.d.e.f"), &rbtnode));
     EXPECT_EQ(Name("m"), rbtnode->getName());
     EXPECT_EQ(19, rbtree.getNodeCount());
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("n.p.w.y.d.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("n.p.w.y.d.e.f"), &rbtnode));
     EXPECT_EQ(Name("n"), rbtnode->getName());
     EXPECT_EQ(20, rbtree.getNodeCount());
 
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("l.a"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("m.p.w.y.d.e.f"), &rbtnode));
+    EXPECT_EQ(Name("m"), rbtnode->getName());
+    EXPECT_EQ(19, rbtree_expose_empty_node.getNodeCount());
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("n.p.w.y.d.e.f"), &rbtnode));
+    EXPECT_EQ(Name("n"), rbtnode->getName());
+    EXPECT_EQ(20, rbtree_expose_empty_node.getNodeCount());
+
+
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("l.a"), &rbtnode));
     EXPECT_EQ(Name("l"), rbtnode->getName());
     EXPECT_EQ(21, rbtree.getNodeCount());
 
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("r.d.e.f"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("s.d.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("l.a"), &rbtnode));
+    EXPECT_EQ(Name("l"), rbtnode->getName());
+    EXPECT_EQ(21, rbtree_expose_empty_node.getNodeCount());
+
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("r.d.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("s.d.e.f"), &rbtnode));
     EXPECT_EQ(23, rbtree.getNodeCount());
 
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("h.w.y.d.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("r.d.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("s.d.e.f"), &rbtnode));
+    EXPECT_EQ(23, rbtree_expose_empty_node.getNodeCount());
+
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("h.w.y.d.e.f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("h.w.y.d.e.f"), &rbtnode));
 
     // add more nodes one by one to cover leftRotate and rightRotate
-    EXPECT_EQ(RBTree<int>::ALREADYEXIST, rbtree.insert(Name("f"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("m"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("nm"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("om"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("k"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("l"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("fe"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("ge"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("i"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("ae"), &rbtnode));
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("n"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("m"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("nm"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("om"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("k"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("l"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("fe"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("ge"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("i"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("ae"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("n"), &rbtnode));
+    
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS, rbtree_expose_empty_node.insert(Name("f"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("m"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("nm"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("om"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("k"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("l"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("fe"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("ge"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("i"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("ae"), &rbtnode));
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree_expose_empty_node.insert(Name("n"), &rbtnode));
 }
 
+   
 TEST_F(RBTreeTest, findName) {
     // find const rbtnode
     // exact match
@@ -177,7 +230,7 @@ testCallback(const RBNode<int>&, bool* callack_checker) {
 
 TEST_F(RBTreeTest, callback) {
     // by default callback isn't enabled
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("callback.example"),
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("callback.example"),
                                                   &rbtnode));
     rbtnode->setData(RBNode<int>::NodeDataPtr(new int(1)));
     EXPECT_FALSE(rbtnode->isCallbackEnabled());
@@ -192,11 +245,11 @@ TEST_F(RBTreeTest, callback) {
     rbtnode->enableCallback();
     // add more levels below and above the callback node for partial match.
     RBNode<int>* subrbtnode;
-    EXPECT_EQ(RBTree<int>::SUCCEED, rbtree.insert(Name("sub.callback.example"),
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("sub.callback.example"),
                                                   &subrbtnode));
     subrbtnode->setData(RBNode<int>::NodeDataPtr(new int(2)));
     RBNode<int>* parentrbtnode;
-    EXPECT_EQ(RBTree<int>::ALREADYEXIST, rbtree.insert(Name("example"),
+    EXPECT_EQ(RBTree<int>::SUCCESS, rbtree.insert(Name("example"),
                                                        &parentrbtnode));
     //  the chilld/parent nodes shouldn't "inherit" the callback flag.
     // "rbtnode" may be invalid due to the insertion, so we need to re-find

src/lib/datasrc/zonetable.cc

@@ -51,8 +51,8 @@ struct ZoneTable::ZoneTableImpl {
         ZoneNode* node(NULL);
         switch (zones_.insert(zone->getOrigin(), &node)) {
             // This is OK
-            case ZoneTree::SUCCEED:
-            case ZoneTree::ALREADYEXIST:
+            case ZoneTree::SUCCESS:
+            case ZoneTree::ALREADYEXISTS:
                 break;
             // Can Not Happen
             default:
@@ -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, ConstZonePtr()));
+                return (FindResult(result::NOTFOUND, ZonePtr()));
             // Can Not Happen
             default:
                 assert(0);
                 // Because of warning
-                return (FindResult(result::NOTFOUND, ConstZonePtr()));
+                return (FindResult(result::NOTFOUND, ZonePtr()));
         }
 
         // Can Not Happen (remember, NOTFOUND is handled)

src/lib/datasrc/zonetable.h

@@ -41,11 +41,11 @@ namespace datasrc {
 class ZoneTable {
 public:
     struct FindResult {
-        FindResult(result::Result param_code, const ConstZonePtr param_zone) :
+        FindResult(result::Result param_code, const ZonePtr param_zone) :
             code(param_code), zone(param_zone)
         {}
         const result::Result code;
-        const ConstZonePtr zone;
+        const ZonePtr zone;
     };
     ///
     /// \name Constructors and Destructor.

src/lib/log/dummylog.cc

@@ -24,8 +24,8 @@ namespace log {
 bool denabled = false;
 string dprefix;
 
-void dlog(const string& message) {
-    if (denabled) {
+void dlog(const string& message,bool error_flag) {
+    if (denabled || error_flag) {
         if (!dprefix.empty()) {
             cerr << "[" << dprefix << "] ";
         }

src/lib/log/dummylog.h

@@ -52,7 +52,7 @@ extern std::string dprefix;
  * @param message The message to log. The real interface will probably have
  *     more parameters.
  */
-void dlog(const std::string& message);
+void dlog(const std::string& message, bool error_flag=false);
 
 }
 }

src/lib/nsas/README

@@ -2,6 +2,6 @@ For an overview of the Nameserver Address Store, see the requirements and design
 documents at http://bind10.isc.org/wiki/Resolver.
 
 At the time of writing (19 October 2010), the file asiolink.h is present in this
-directory only for the purposes of development.  When the recursor's
+directory only for the purposes of development.  When the resolver's
 asynchronous I/O code has been finished, this will be removed and the NSAS will
 use the "real" code.

src/lib/nsas/asiolink.h

@@ -24,7 +24,7 @@ namespace asiolink {
 
 /// \brief IO Address Dummy Class
 ///
-/// As part of ther recursor, Evan has written the asiolink.h file, which
+/// As part of ther resolver, Evan has written the asiolink.h file, which
 /// encapsulates some of the boost::asio classes.  Until these are checked
 /// into trunk and merged with this branch, these dummy classes should fulfill
 /// their function.

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

@@ -285,7 +285,7 @@ class ModuleCCSession(ConfigData):
         if answer:
             rcode, value = parse_answer(answer)
             if rcode == 0:
-                if value != None and self.get_module_spec().validate_config(False, value):
+                if value != None and module_spec.validate_config(False, value):
                     module_cfg.set_local_config(value);
 
         # all done, add it

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

@@ -26,6 +26,7 @@ import os
 import copy
 import tempfile
 import json
+import errno
 from isc.cc import data
 from isc.config import ccsession, config_data
 
@@ -87,7 +88,12 @@ class ConfigManagerData:
             else:
                 raise ConfigManagerDataReadError("No version information in configuration file " + config.db_filename)
         except IOError as ioe:
-            raise ConfigManagerDataEmpty("No configuration file found")
+            # if IOError is 'no such file or directory', then continue
+            # (raise empty), otherwise fail (raise error)
+            if ioe.errno == errno.ENOENT:
+                raise ConfigManagerDataEmpty("No configuration file found")
+            else:
+                raise ConfigManagerDataReadError("Can't read configuration file: " + str(ioe))
         except ValueError:
             raise ConfigManagerDataReadError("Configuration file out of date or corrupt, please update or remove " + config.db_filename)
         finally:
@@ -270,16 +276,15 @@ class ConfigManager:
         else:
             return ccsession.create_answer(0, self.config.data)
 
-    def _handle_set_config_module(self, cmd):
+    def _handle_set_config_module(self, module_name, cmd):
         # the answer comes (or does not come) from the relevant module
         # so we need a variable to see if we got it
         answer = None
         # todo: use api (and check the data against the definition?)
         old_data = copy.deepcopy(self.config.data)
-        module_name = cmd[0]
         conf_part = data.find_no_exc(self.config.data, module_name)
         if conf_part:
-            data.merge(conf_part, cmd[1])
+            data.merge(conf_part, cmd)
             update_cmd = ccsession.create_command(ccsession.COMMAND_CONFIG_UPDATE,
                                                   conf_part)
             seq = self.cc.group_sendmsg(update_cmd, module_name)
@@ -289,7 +294,7 @@ class ConfigManager:
                 answer = ccsession.create_answer(1, "Timeout waiting for answer from " + module_name)
         else:
             conf_part = data.set(self.config.data, module_name, {})
-            data.merge(conf_part[module_name], cmd[1])
+            data.merge(conf_part[module_name], cmd)
             # send out changed info
             update_cmd = ccsession.create_command(ccsession.COMMAND_CONFIG_UPDATE,
                                                   conf_part[module_name])
@@ -309,29 +314,21 @@ class ConfigManager:
 
     def _handle_set_config_all(self, cmd):
         old_data = copy.deepcopy(self.config.data)
-        data.merge(self.config.data, cmd[0])
-        # send out changed info
         got_error = False
         err_list = []
-        for module in self.config.data:
-            if module != "version" and \
-               (module not in old_data or self.config.data[module] != old_data[module]):
-                update_cmd = ccsession.create_command(ccsession.COMMAND_CONFIG_UPDATE,
-                                                      self.config.data[module])
-                seq = self.cc.group_sendmsg(update_cmd, module)
-                try:
-                    answer, env = self.cc.group_recvmsg(False, seq)
-                    if answer == None:
-                        got_error = True
-                        err_list.append("No answer message from " + module)
-                    else:
-                        rcode, val = ccsession.parse_answer(answer)
-                        if rcode != 0:
-                            got_error = True
-                            err_list.append(val)
-                except isc.cc.SessionTimeout:
+        # The format of the command is a dict with module->newconfig
+        # sets, so we simply call set_config_module for each of those
+        for module in cmd:
+            if module != "version":
+                answer = self._handle_set_config_module(module, cmd[module])
+                if answer == None:
                     got_error = True
-                    err_list.append("CC Timeout waiting on answer message from " + module)
+                    err_list.append("No answer message from " + module)
+                else:
+                    rcode, val = ccsession.parse_answer(answer)
+                    if rcode != 0:
+                        got_error = True
+                        err_list.append(val)
         if not got_error:
             self.write_config()
             return ccsession.create_answer(0)
@@ -343,12 +340,13 @@ class ConfigManager:
     def _handle_set_config(self, cmd):
         """Private function that handles the 'set_config' command"""
         answer = None
+
         if cmd == None:
             return ccsession.create_answer(1, "Wrong number of arguments")
         if len(cmd) == 2:
-            answer = self._handle_set_config_module(cmd)
+            answer = self._handle_set_config_module(cmd[0], cmd[1])
         elif len(cmd) == 1:
-            answer = self._handle_set_config_all(cmd)
+            answer = self._handle_set_config_all(cmd[0])
         else:
             answer = ccsession.create_answer(1, "Wrong number of arguments")
         if not answer:

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

@@ -530,7 +530,19 @@ class TestModuleCCSession(unittest.TestCase):
         self.assertTrue("Spec2" in fake_session.subscriptions)
         mccs = None
         self.assertFalse("Spec2" in fake_session.subscriptions)
-        
+
+    def test_remote_module_with_custom_config(self):
+        fake_session = FakeModuleCCSession()
+        mccs = self.create_session("spec1.spec", None, None, fake_session)
+        # override the default config value for "item1".  add_remote_config()
+        # should incorporate the overridden value, and we should be abel to
+        # get it via get_remote_config_value().
+        fake_session.group_sendmsg({'result': [0, {"item1": 10}]}, 'Spec2')
+        rmodname = mccs.add_remote_config(self.spec_file("spec2.spec"))
+        value, default = mccs.get_remote_config_value(rmodname, "item1")
+        self.assertEqual(10, value)
+        self.assertEqual(False, default)
+
     def test_ignore_command_remote_module(self):
         # Create a Spec1 module and subscribe to remote config for Spec2
         fake_session = FakeModuleCCSession()

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

@@ -288,6 +288,66 @@ class TestConfigManager(unittest.TestCase):
                                 },
                                 {'result': [0]})
 
+    def test_set_config_all(self):
+        my_ok_answer = { 'result': [ 0 ] }
+
+        self.assertEqual({"version": 2}, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value1": 123 }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value1": 123 }
+                         }, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value1": 124 }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value1": 124 }
+                         }, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value2": True }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value1": 124,
+                                    "value2": True
+                                  }
+                         }, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value3": [ 1, 2, 3 ] }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value1": 124,
+                                    "value2": True,
+                                    "value3": [ 1, 2, 3 ]
+                                  }
+                         }, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value2": False }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value1": 124,
+                                    "value2": False,
+                                    "value3": [ 1, 2, 3 ]
+                                  }
+                         }, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value1": None }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value2": False,
+                                    "value3": [ 1, 2, 3 ]
+                                  }
+                         }, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value3": [ 1 ] }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value2": False,
+                                    "value3": [ 1 ]
+                                  }
+                         }, self.cm.config.data)
+
+
     def test_run(self):
         self.fake_session.group_sendmsg({ "command": [ "get_commands_spec" ] }, "ConfigManager")
         self.fake_session.group_sendmsg({ "command": [ "shutdown" ] }, "ConfigManager")

src/lib/python/isc/datasrc/sqlite3_ds.py

@@ -25,25 +25,24 @@ RR_NAME_INDEX = 2
 RR_TTL_INDEX = 4
 RR_RDATA_INDEX = 7
 
-#########################################################################
-# define exceptions
-#########################################################################
 class Sqlite3DSError(Exception):
+    """ Define exceptions."""
     pass
 
-#########################################################################
-# create: set up schema for a newly created zones/records database
-#########################################################################
 def create(cur):
-    """Create new zone database"""
+    """ Set up schema for a newly created zones/records database.
+
+    Arguments:
+        cur - sqlite3 cursor.
+    """
     cur.execute("CREATE TABLE schema_version (version INTEGER NOT NULL)")
     cur.execute("INSERT INTO schema_version VALUES (1)")
-    cur.execute("""CREATE TABLE zones (id INTEGER PRIMARY KEY, 
+    cur.execute("""CREATE TABLE zones (id INTEGER PRIMARY KEY,
                    name STRING NOT NULL COLLATE NOCASE,
-                   rdclass STRING NOT NULL COLLATE NOCASE DEFAULT 'IN', 
+                   rdclass STRING NOT NULL COLLATE NOCASE DEFAULT 'IN',
                    dnssec BOOLEAN NOT NULL DEFAULT 0)""")
     cur.execute("CREATE INDEX zones_byname ON zones (name)")
-    cur.execute("""CREATE TABLE records (id INTEGER PRIMARY KEY, 
+    cur.execute("""CREATE TABLE records (id INTEGER PRIMARY KEY,
                    zone_id INTEGER NOT NULL,
                    name STRING NOT NULL COLLATE NOCASE,
                    rname STRING NOT NULL COLLATE NOCASE,
@@ -53,7 +52,7 @@ def create(cur):
                    rdata STRING NOT NULL)""")
     cur.execute("CREATE INDEX records_byname ON records (name)")
     cur.execute("CREATE INDEX records_byrname ON records (rname)")
-    cur.execute("""CREATE TABLE nsec3 (id INTEGER PRIMARY KEY, 
+    cur.execute("""CREATE TABLE nsec3 (id INTEGER PRIMARY KEY,
                    zone_id INTEGER NOT NULL,
                    hash STRING NOT NULL COLLATE NOCASE,
                    owner STRING NOT NULL COLLATE NOCASE,
@@ -62,17 +61,17 @@ def create(cur):
                    rdata STRING NOT NULL)""")
     cur.execute("CREATE INDEX nsec3_byhash ON nsec3 (hash)")
 
-#########################################################################
-# open: open a database.  if the database is not yet set up, 
-# call create to do so.
-# input:
-#   dbfile - the filename for the sqlite3 database
-# returns:
-#   sqlite3 connection, sqlite3 cursor
-#########################################################################
 def open(dbfile):
-    """Open the database file.  If necessary, set it up"""
-    try: 
+    """ Open a database, if the database is not yet set up, call create
+    to do so. It may raise Sqlite3DSError if failed to open sqlite3
+    database file or find bad database schema version in the database.
+
+    Arguments:
+        dbfile - the filename for the sqlite3 database.
+
+    Return sqlite3 connection, sqlite3 cursor.
+    """
+    try:
         conn = sqlite3.connect(dbfile)
         cur = conn.cursor()
     except Exception as e:
@@ -93,12 +92,15 @@ def open(dbfile):
 
     return conn, cur
 
-#########################################################################
-# get_zone_datas
-#   a generator function producing an iterable set of 
-#   the records in the zone with the given zone name.
-#########################################################################
+
 def get_zone_datas(zonename, dbfile):
+    """ A generator function producing an iterable set of
+    the records in the zone with the given zone name.
+
+    Arguments:
+        zonename - the zone's origin name.
+        dbfile - the filename for the sqlite3 database.
+    """
     conn, cur = open(dbfile)
     zone_id = get_zoneid(zonename, cur)
 
@@ -112,12 +114,14 @@ def get_zone_datas(zonename, dbfile):
     conn.close()
 
 
-#########################################################################
-# get_zone_soa
-#   returns the soa record of the zone with the given zone name. 
-#   If the zone doesn't exist, return None. 
-#########################################################################
 def get_zone_soa(zonename, dbfile):
+    """Return the soa record of the zone with the given zone name.
+    If the zone doesn't exist, return None.
+
+    Arguments:
+        zonename - the zone's origin name.
+        dbfile - the filename for the sqlite3 database.
+    """
     conn, cur = open(dbfile)
     id = get_zoneid(zonename, cur)
     cur.execute("SELECT * FROM records WHERE zone_id = ? and rdtype = ?", [id, 'SOA'])
@@ -128,16 +132,20 @@ def get_zone_soa(zonename, dbfile):
     return datas
 
 
-#########################################################################
-# get_zone_rrset
-#   returns the rrset of the zone with the given zone name, rrset name 
-#   and given rd type. 
-#   If the zone doesn't exist or rd type doesn't exist, return an empty list. 
-#########################################################################
 def get_zone_rrset(zonename, rr_name, rdtype, dbfile):
+    """Return the rrset of the zone with the given zone name, rrset
+    name and given RR type. If the zone doesn't exist or RR type
+    doesn't exist, return an empty list.
+
+    Arguments:
+        zonename - the zone's origin name.
+        rr_name - rr name.
+        rdtype - RR type.
+        dbfile - the filename for the sqlite3 database.
+    """
     conn, cur = open(dbfile)
     id = get_zoneid(zonename, cur)
-    cur.execute("SELECT * FROM records WHERE name = ? and zone_id = ? and rdtype = ?", 
+    cur.execute("SELECT * FROM records WHERE name = ? and zone_id = ? and rdtype = ?",
                 [rr_name, id, rdtype])
     datas = cur.fetchall()
     cur.close()
@@ -145,12 +153,13 @@ def get_zone_rrset(zonename, rr_name, rdtype, dbfile):
     return datas
 
 
-#########################################################################
-# get_zones_info:
-#   returns all the zones' information.
-#########################################################################
-def get_zones_info(db_file):
-    conn, cur = open(db_file)
+def get_zones_info(dbfile):
+    """ Return all the zones' information in the database.
+
+    Arguments:
+        dbfile - the filename for the sqlite3 database.
+    """
+    conn, cur = open(dbfile)
     cur.execute("SELECT name, rdclass FROM zones")
     info = cur.fetchone()
     while info:
@@ -160,44 +169,69 @@ def get_zones_info(db_file):
     cur.close()
     conn.close()
 
-#########################################################################
-# get_zoneid:
-#   returns the zone_id for a given zone name, or an empty
-#   string if the zone is not found
-#########################################################################
-def get_zoneid(zone, cur):
-    cur.execute("SELECT id FROM zones WHERE name = ?", [zone])
+
+def get_zoneid(zonename, cur):
+    """ Get the zone_id for a given zone name.
+
+    Arguments:
+        zonename - the zone's origin name.
+        cur - sqlite3 cursor.
+
+    Return zone id for the given zone name, or an empty string if the
+    zone is not found.
+    """
+    cur.execute("SELECT id FROM zones WHERE name = ?", [zonename])
     row = cur.fetchone()
     if row:
         return row[0]
     else:
         return ''
-    
-#########################################################################
-# reverse_name:
-#   reverse the labels of a DNS name.  (for example,
-#   "bind10.isc.org." would become "org.isc.bind10.")
-#########################################################################
+
+
+def zone_exist(zonename, dbfile):
+    """ Search for the zone with the given zone name in databse. This
+    method may throw a Sqlite3DSError exception because its underlying
+    method open() can throw that exception.
+
+    Arguments:
+        zonename - the zone's origin name.
+        dbfile - the filename for the sqlite3 database.
+
+    Return True if the zone is found, otherwise False.
+    """
+    conn, cur = open(dbfile)
+    zoneid = get_zoneid(zonename, cur)
+    cur.close()
+    conn.close()
+    if zoneid:
+        return True
+    return False
+
+
 def reverse_name(name):
     """Reverse the labels of a domain name; for example,
-    given 'www.isc.org.', return 'org.isc.www.'  This is needed
-    for DNSSEC sort order."""
+    given 'www.example.org.', return 'org.example.www.'  This is needed
+    for DNSSEC sort order.
+
+    Arguments:
+        name - the DNS name will be reversed.
+    """
     new = name.split('.')
     new.reverse()
     if new[0] == '':
         new.pop(0)
     return '.'.join(new)+'.'
 
-#########################################################################
-# load:
-#   load a zone into the SQL database.
-# input:
-#   dbfile: the sqlite3 database fileanme
-#   zone: the zone origin
-#   reader: a generator function producing an iterable set of
-#           name/ttl/class/rrtype/rdata-text tuples
-#########################################################################
+
 def load(dbfile, zone, reader):
+    """  Load a zone into the SQL database.
+
+    Arguments:
+        dbfile - the sqlite3 database filename
+        zone - the zone origin
+        reader - a generator function producing an iterable set of
+        name/ttl/class/rrtype/rdata-text tuples.
+    """
     # if the zone name doesn't contain the trailing dot, automatically add it.
     if zone[-1] != '.':
         zone += '.'

src/lib/python/isc/datasrc/tests/Makefile.am

@@ -1,16 +1,20 @@
 PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
-PYTESTS = master_test.py
+PYTESTS = master_test.py sqlite3_ds_test.py
 EXTRA_DIST = $(PYTESTS)
 
+EXTRA_DIST += testdata/brokendb.sqlite3
+EXTRA_DIST += testdata/example.com.sqlite3
+
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
 if ENABLE_PYTHON_COVERAGE
-	touch $(abs_top_srcdir)/.coverage 
+	touch $(abs_top_srcdir)/.coverage
 	rm -f .coverage
 	${LN_S} $(abs_top_srcdir)/.coverage .coverage
 endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
 	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/python/isc/log \
+	TESTDATA_PATH=$(abs_srcdir)/testdata \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

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

@@ -0,0 +1,43 @@
+# Copyright (C) 2011  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and 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 INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM 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.
+
+from isc.datasrc import sqlite3_ds
+import os
+import socket
+import unittest
+
+TESTDATA_PATH = os.environ['TESTDATA_PATH'] + os.sep
+
+class TestSqlite3_ds(unittest.TestCase):
+    def test_zone_exist(self):
+        # The following file must be non existent and must be non
+        # "creatable"; the sqlite3 library will try to create a new
+        # DB file if it doesn't exist, so to test a failure case the
+        # create operation should also fail. The "nodir", a non
+        # existent directory, is inserted for this purpose.
+        nodir = "/nodir/notexist"
+        self.assertRaises(sqlite3_ds.Sqlite3DSError,
+                          sqlite3_ds.zone_exist, "example.com", nodir)
+        # Open a broken database file
+        self.assertRaises(sqlite3_ds.Sqlite3DSError,
+                          sqlite3_ds.zone_exist, "example.com",
+                          TESTDATA_PATH + "brokendb.sqlite3")
+        self.assertTrue(sqlite3_ds.zone_exist("example.com.",
+                            TESTDATA_PATH + "example.com.sqlite3"))
+        self.assertFalse(sqlite3_ds.zone_exist("example.org.",
+                            TESTDATA_PATH + "example.com.sqlite3"))
+
+if __name__ == '__main__':
+    unittest.main()

src/lib/testutils/testdata/Makefile.am

@@ -1,4 +1,4 @@
-CLEANFILES = *.wire
+CLEANFILES = *.wire *.copied
 
 BUILT_SOURCES = badExampleQuery_fromWire.wire examplequery_fromWire.wire
 BUILT_SOURCES += iqueryresponse_fromWire.wire multiquestion_fromWire.wire
@@ -25,5 +25,11 @@ EXTRA_DIST += example.com.zone example.net.zone example.org.zone example.zone
 EXTRA_DIST += example.com
 EXTRA_DIST += example.sqlite3
 
+EXTRA_DIST += test1.zone.in
+EXTRA_DIST += test1-new.zone.in
+EXTRA_DIST += test1-broken.zone.in
+EXTRA_DIST += test2.zone.in
+EXTRA_DIST += test2-new.zone.in
+
 .spec.wire:
 	$(abs_top_builddir)/src/lib/dns/tests/testdata/gen-wiredata.py -o $@ $<

src/lib/testutils/testdata/test1-broken.zone.in

@@ -0,0 +1,5 @@
+test1.example.    3600    IN  SOA ns.test1.example. admin.test1.example. 1241 3600 1800 2419200 7200
+test1.example.    3600    IN  NS ns.test1.example.
+ns.test1.example.	3600    IN  A 192.0.2.1
+;; out-of-zone RR
+www.example.com.	3600    IN  AAAA 2001:db8::80

src/lib/testutils/testdata/test1-new.zone.in

@@ -0,0 +1,4 @@
+test1.example.    3600    IN  SOA ns.test1.example. admin.test1.example. 1238 3600 1800 2419200 7200
+test1.example.    3600    IN  NS ns.test1.example.
+ns.test1.example.	3600    IN  A 192.0.2.1
+ns.test1.example.	3600    IN  AAAA 2001:db8::1

src/lib/testutils/testdata/test1.zone.in

@@ -0,0 +1,3 @@
+test1.example.    3600    IN  SOA ns.test1.example. admin.test1.example. 1235 3600 1800 2419200 7200
+test1.example.    3600    IN  NS ns.test1.example.
+ns.test1.example.	3600    IN  A 192.0.2.1

src/lib/testutils/testdata/test2-new.zone.in

@@ -0,0 +1,4 @@
+test2.example.    3600    IN  SOA ns.test2.example. admin.test2.example. 1242 3600 1800 2419200 7200
+test2.example.    3600    IN  NS ns.test2.example.
+ns.test2.example.	3600    IN  A 192.0.2.2
+ns.test2.example.	3600    IN  AAAA 2001:db8::2

src/lib/testutils/testdata/test2.zone.in

@@ -0,0 +1,3 @@
+test2.example.    3600    IN  SOA ns.test2.example. admin.test2.example. 1238 3600 1800 2419200 7200
+test2.example.    3600    IN  NS ns.test2.example.
+ns.test2.example.	3600    IN  A 192.0.2.2