Merge branch 'master' into trac2236

Conflicts:
	src/bin/auth/auth_srv.cc
	src/bin/auth/tests/auth_srv_unittest.cc

ChangeLog

@@ -1,3 +1,11 @@
+495.	[func]		team
+	b10-auth now handles reconfiguration of data sources in
+	background using a separate thread.  This means even if the new
+	configuration includes a large amount of data to be loaded into
+	memory (very large zones and/or a very large number of zones),
+	the reconfiguration doesn't block query handling.
+	(Multiple Trac tickets up to #2211)
+
 494.	[bug]		jinmei
 	Fixed a problem that shutting down BIND 10 kept some of the
 	processes alive.  It was two-fold: when the main bind10 process
@@ -54,7 +62,7 @@
 
 488.	[build]		jinmei
 	On configure, changed the search order for Python executable.
-	It first ties more specific file names such as "python3.2" before
+	It first tries more specific file names such as "python3.2" before
 	more generic "python3".  This will prevent configure failure on
 	Mac OS X that installs Python3 via recent versions of Homebrew.
 	(Trac #2339, git 88db890d8d1c64de49be87f03c24a2021bcf63da)

configure.ac

@@ -1126,6 +1126,7 @@ AC_CONFIG_FILES([Makefile
                  src/bin/bindctl/Makefile
                  src/bin/bindctl/tests/Makefile
                  src/bin/cfgmgr/Makefile
+                 src/bin/cfgmgr/local_plugins/Makefile
                  src/bin/cfgmgr/plugins/Makefile
                  src/bin/cfgmgr/plugins/tests/Makefile
                  src/bin/cfgmgr/tests/Makefile

src/bin/auth/Makefile.am

@@ -55,6 +55,7 @@ b10_auth_SOURCES += auth_config.cc auth_config.h
 b10_auth_SOURCES += command.cc command.h
 b10_auth_SOURCES += common.h common.cc
 b10_auth_SOURCES += statistics.cc statistics.h
+b10_auth_SOURCES += datasrc_clients_mgr.h
 b10_auth_SOURCES += datasrc_config.h datasrc_config.cc
 b10_auth_SOURCES += main.cc
 

src/bin/auth/auth_log.cc

@@ -21,6 +21,12 @@ namespace auth {
 
 isc::log::Logger auth_logger("auth");
 
+const int DBG_AUTH_START = DBGLVL_START_SHUT;
+const int DBG_AUTH_SHUT = DBGLVL_START_SHUT;
+const int DBG_AUTH_OPS = DBGLVL_COMMAND;
+const int DBG_AUTH_DETAIL = DBGLVL_TRACE_BASIC;
+const int DBG_AUTH_MESSAGES = DBGLVL_TRACE_DETAIL_DATA;
+
 } // namespace auth
 } // namespace isc
 

src/bin/auth/auth_log.h

@@ -28,21 +28,21 @@ namespace auth {
 /// output.
 
 // Debug messages indicating normal startup are logged at this debug level.
-const int DBG_AUTH_START = DBGLVL_START_SHUT;
+extern const int DBG_AUTH_START;
 // Debug messages upon shutdown
-const int DBG_AUTH_SHUT = DBGLVL_START_SHUT;
+extern const int DBG_AUTH_SHUT;
 
 // Debug level used to log setting information (such as configuration changes).
-const int DBG_AUTH_OPS = DBGLVL_COMMAND;
+extern const int DBG_AUTH_OPS;
 
 // Trace detailed operations, including errors raised when processing invalid
 // packets.  (These are not logged at severities of WARN or higher for fear
 // that a set of deliberately invalid packets set to the authoritative server
 // could overwhelm the logging.)
-const int DBG_AUTH_DETAIL = DBGLVL_TRACE_BASIC;
+extern const int DBG_AUTH_DETAIL;
 
 // This level is used to log the contents of packets received and sent.
-const int DBG_AUTH_MESSAGES = DBGLVL_TRACE_DETAIL_DATA;
+extern const int DBG_AUTH_MESSAGES;
 
 /// Define the logger for the "auth" module part of b10-auth.  We could define
 /// a logger in each file, but we would want to define a common name to avoid

src/bin/auth/auth_messages.mes

@@ -57,6 +57,87 @@ At attempt to update the configuration the server with information
 from the configuration database has failed, the reason being given in
 the message.
 
+% AUTH_DATASRC_CLIENTS_BUILDER_COMMAND data source builder received command: %1
+A debug message, showing when the separate thread for maintaining data
+source clients receives a command from the manager.
+
+% AUTH_DATASRC_CLIENTS_BUILDER_FAILED data source builder thread stopped due to an exception: %1
+The separate thread for maintaining data source clients has been
+terminated due to some uncaught exception.  When this happens, the
+thread immediately terminates the entire process because the manager
+cannot always catch this condition in a timely fashion and it would be
+worse to keep running with such a half-broken state.  This is really
+an unexpected event and should generally indicate an internal bug.
+It's advisable to file a bug report when this message is logged (and
+b10-auth subsequently stops).
+
+% AUTH_DATASRC_CLIENTS_BUILDER_FAILED_UNEXPECTED data source builder thread stopped due to an unexpected exception
+This is similar to AUTH_DATASRC_CLIENTS_BUILDER_FAILED, but the
+exception type indicates it's not thrown either within the BIND 10
+implementation or other standard-compliant libraries.  This may rather
+indicate some run time failure than program errors.  As in the other
+failure case, the thread terminates the entire process immediately
+after logging this message.
+
+% AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_CONFIG_ERROR Error in data source configuration: %1
+The thread for maintaining data source clients has received a command to
+reconfigure, but the parameter data (the new configuration) contains an
+error. The most likely cause is that the datasource-specific configuration
+data is not what the data source expects. The system is still running with
+the data sources that were previously configured (i.e. as if the
+configuration has not changed), and the configuration data needs to be
+checked.
+The specific problem is printed in the log message.
+
+% AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_DATASRC_ERROR Error setting up data source: %1
+The thread for maintaining data source clients has received a command to
+reconfigure, but a data source failed to set up. This may be a problem with
+the data that is configured (e.g. unreadable files, inconsistent data,
+parser problems, database connection problems, etc.), but it could be a bug
+in the data source implementation as well. The system is still running with
+the data sources that were previously configured (i.e. as if the
+configuration has not changed).
+The specific problem is printed in the log message.
+
+% AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_ERROR Internal error setting up data source: %1
+The thread for maintaining data source clients has received a command to
+reconfigure, but raised an exception while setting up data sources. This is
+most likely an internal error in a data source, or a bug in the data source
+or the system itself, but it is probably a good idea to verify the
+configuration first. The system is still running with the data sources that
+were previously configured (i.e. as if the configuration has not changed).
+The specific problem is printed in the log message.
+
+% AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_STARTED data source reconfiguration started
+The thread for maintaining data source clients has received a command to
+reconfigure, and has now started this process.
+
+% AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_SUCCESS data source reconfiguration completed succesfully
+The thread for maintaining data source clients has finished reconfiguring
+the data source clients, and is now running with the new configuration.
+
+% AUTH_DATASRC_CLIENTS_BUILDER_STARTED data source builder thread started
+A separate thread for maintaining data source clients has been started.
+
+% AUTH_DATASRC_CLIENTS_BUILDER_STOPPED data source builder thread stopped
+The separate thread for maintaining data source clients has been stopped.
+
+% AUTH_DATASRC_CLIENTS_SHUTDOWN_ERROR error on waiting for data source builder thread: %1
+This indicates that the separate thread for maintaining data source
+clients had been terminated due to an uncaught exception, and the
+manager notices that at its own termination.  This is not an expected
+event, because the thread is implemented so it catches all exceptions
+internally.  So, if this message is logged it's most likely some internal
+bug, and it would be nice to file a bug report.
+
+% AUTH_DATASRC_CLIENTS_SHUTDOWN_UNEXPECTED_ERROR Unexpected error on waiting for data source builder thread
+Some exception happens while waiting for the termination of the
+separate thread for maintaining data source clients.  This shouldn't
+happen in normal conditions; it should be either fatal system level
+errors such as severe memory shortage or some internal bug.  If that
+happens, and if it's not in the middle of terminating b10-auth, it's
+probably better to stop and restart it.
+
 % AUTH_DATA_SOURCE data source database file: %1
 This is a debug message produced by the authoritative server when it accesses a
 datebase data source, listing the file that is being accessed.

src/bin/auth/auth_srv.cc

@@ -26,7 +26,6 @@
 #include <exceptions/exceptions.h>
 
 #include <util/buffer.h>
-#include <util/threads/sync.h>
 
 #include <dns/edns.h>
 #include <dns/exceptions.h>
@@ -53,6 +52,7 @@
 #include <auth/query.h>
 #include <auth/statistics.h>
 #include <auth/auth_log.h>
+#include <auth/datasrc_clients_mgr.h>
 
 #include <boost/bind.hpp>
 #include <boost/lexical_cast.hpp>
@@ -268,26 +268,8 @@ public:
     /// The TSIG keyring
     const shared_ptr<TSIGKeyRing>* keyring_;
 
-    /// The data source client list
-    AuthSrv::DataSrcClientListsPtr datasrc_client_lists_;
-
-    shared_ptr<ConfigurableClientList> getDataSrcClientList(
-        const RRClass& rrclass)
-    {
-#ifdef ENABLE_DEBUG
-        // Debug-build only check
-        if (!mutex_.locked()) {
-            isc_throw(isc::Unexpected, "Not locked!");
-        }
-#endif
-        const std::map<RRClass, shared_ptr<ConfigurableClientList> >::
-            const_iterator it(datasrc_client_lists_->find(rrclass));
-        if (it == datasrc_client_lists_->end()) {
-            return (shared_ptr<ConfigurableClientList>());
-        } else {
-            return (it->second);
-        }
-    }
+    /// The data source client list manager
+    auth::DataSrcClientsMgr datasrc_clients_mgr_;
 
     /// Bind the ModuleSpec object in config_session_ with
     /// isc:config::ModuleSpec::validateStatistics.
@@ -317,8 +299,6 @@ public:
                       isc::dns::Message& message,
                       bool done);
 
-    mutable util::thread::Mutex mutex_;
-
 private:
     bool xfrout_connected_;
     AbstractXfroutClient& xfrout_client_;
@@ -338,8 +318,6 @@ AuthSrvImpl::AuthSrvImpl(AbstractXfroutClient& xfrout_client,
     xfrin_session_(NULL),
     counters_(),
     keyring_(NULL),
-    datasrc_client_lists_(new std::map<RRClass,
-                          shared_ptr<ConfigurableClientList> >()),
     ddns_base_forwarder_(ddns_forwarder),
     ddns_forwarder_(NULL),
     xfrout_connected_(false),
@@ -490,6 +468,11 @@ AuthSrv::getIOService() {
     return (impl_->io_service_);
 }
 
+isc::auth::DataSrcClientsMgr&
+AuthSrv::getDataSrcClientsMgr() {
+    return (impl_->datasrc_clients_mgr_);
+}
+
 void
 AuthSrv::setXfrinSession(AbstractSession* xfrin_session) {
     impl_->xfrin_session_ = xfrin_session;
@@ -648,15 +631,15 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, Message& message,
         local_edns->setUDPSize(AuthSrvImpl::DEFAULT_LOCAL_UDPSIZE);
         message.setEDNS(local_edns);
     }
-    // Lock the client lists and keep them under the lock until the processing
-    // and rendering is done (this is the same mutex as from
-    // AuthSrv::getDataSrcClientListMutex()).
-    isc::util::thread::Mutex::Locker locker(mutex_);
+    // Get access to data source client list through the holder and keep the
+    // holder until the processing and rendering is done to avoid inter-thread
+    // race.
+    auth::DataSrcClientsMgr::Holder datasrc_holder(datasrc_clients_mgr_);
 
     try {
         const ConstQuestionPtr question = *message.beginQuestion();
         const shared_ptr<datasrc::ClientList>
-            list(getDataSrcClientList(question->getClass()));
+            list(datasrc_holder.findClientList(question->getClass()));
         if (list) {
             const RRType& qtype = question->getType();
             const Name& qname = question->getName();
@@ -935,28 +918,6 @@ AuthSrv::destroyDDNSForwarder() {
     }
 }
 
-AuthSrv::DataSrcClientListsPtr
-AuthSrv::swapDataSrcClientLists(DataSrcClientListsPtr new_lists) {
-#ifdef ENABLE_DEBUG
-    // Debug-build only check
-    if (!impl_->mutex_.locked()) {
-        isc_throw(isc::Unexpected, "Not locked!");
-    }
-#endif
-    std::swap(new_lists, impl_->datasrc_client_lists_);
-    return (new_lists);
-}
-
-shared_ptr<ConfigurableClientList>
-AuthSrv::getDataSrcClientList(const RRClass& rrclass) {
-    return (impl_->getDataSrcClientList(rrclass));
-}
-
-util::thread::Mutex&
-AuthSrv::getDataSrcClientListMutex() const {
-    return (impl_->mutex_);
-}
-
 void
 AuthSrv::setTCPRecvTimeout(size_t timeout) {
     dnss_->setTCPRecvTimeout(timeout);

src/bin/auth/auth_srv.h

@@ -19,6 +19,7 @@
 
 #include <datasrc/factory.h>
 #include <datasrc/client_list.h>
+#include <datasrc/datasrc_config.h>
 
 #include <dns/message.h>
 #include <dns/opcode.h>
@@ -34,7 +35,9 @@
 
 #include <asiolink/asiolink.h>
 #include <server_common/portconfig.h>
+
 #include <auth/statistics.h>
+#include <auth/datasrc_clients_mgr.h>
 
 #include <boost/shared_ptr.hpp>
 
@@ -193,6 +196,11 @@ public:
     /// \brief Return pointer to the Checkin callback function
     isc::asiolink::SimpleCallback* getCheckinProvider() const { return (checkin_); }
 
+    /// \brief Return data source clients manager.
+    ///
+    /// \throw None
+    isc::auth::DataSrcClientsMgr& getDataSrcClientsMgr();
+
     /// \brief Set the communication session with a separate process for
     /// outgoing zone transfers.
     ///
@@ -302,83 +310,10 @@ public:
     /// If there was no forwarder yet, this method does nothing.
     void destroyDDNSForwarder();
 
-    /// \brief Shortcut typedef used for swapDataSrcClientLists().
-    typedef boost::shared_ptr<std::map<
-        isc::dns::RRClass, boost::shared_ptr<
-                               isc::datasrc::ConfigurableClientList> > >
-    DataSrcClientListsPtr;
-
-    /// \brief Swap the currently used set of data source client lists with
-    /// given one.
-    ///
-    /// The "set" of lists is actually given in the form of map from
-    /// RRClasses to shared pointers to isc::datasrc::ConfigurableClientList.
-    ///
-    /// This method returns the swapped set of lists, which was previously
-    /// used by the server.
-    ///
-    /// This method is intended to be used by a separate method to update
-    /// the data source configuration "at once".  The caller must hold
-    /// a lock for the mutex object returned by  \c getDataSrcClientListMutex()
-    /// before calling this method.
-    ///
-    /// The ownership of the returned pointer is transferred to the caller.
-    /// The caller is generally expected to release the resources used in
-    /// the old lists.  Note that it could take longer time if some of the
-    /// data source clients contain a large size of in-memory data.
-    ///
-    /// The caller can pass a NULL pointer.  This effectively disables
-    /// any data source for the server.
-    ///
-    /// \param new_lists Shared pointer to a new set of data source client
-    /// lists.
-    /// \return The previous set of lists.  It can be NULL.
-    DataSrcClientListsPtr swapDataSrcClientLists(DataSrcClientListsPtr
-                                                 new_lists);
-
-    /// \brief Returns the currently used client list for the class.
-    ///
-    /// \param rrclass The class for which to get the list.
-    /// \return The list, or NULL if no list is set for the class.
-    boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-        getDataSrcClientList(const isc::dns::RRClass& rrclass);
-
-    /// \brief Return a mutex for the client lists.
-    ///
-    /// Background loading of data uses threads. Therefore we need to protect
-    /// the client lists by a mutex, so they don't change (or get destroyed)
-    /// during query processing. Get (and lock) this mutex whenever you do
-    /// something with the lists and keep it locked until you finish. This
-    /// is correct:
-    /// \code
-    /// {
-    ///  Mutex::Locker locker(auth->getDataSrcClientListMutex());
-    ///  boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-    ///    list(auth->getDataSrcClientList(RRClass::IN()));
-    ///  // Do some processing here
-    /// }
-    /// \endcode
-    ///
-    /// But this is not (it releases the mutex too soon):
-    /// \code
-    /// boost::shared_ptr<isc::datasrc::ConfigurableClientList> list;
-    /// {
-    ///     Mutex::Locker locker(auth->getDataSrcClientListMutex());
-    ///     list = auth->getDataSrcClientList(RRClass::IN()));
-    /// }
-    /// // Do some processing here
-    /// \endcode
-    ///
-    /// \note This method is const even if you are allowed to modify
-    ///    (lock) the mutex. It's because locking of the mutex is not really
-    ///    a modification of the server object and it is needed to protect the
-    ///    lists even on read-only operations.
-    isc::util::thread::Mutex& getDataSrcClientListMutex() const;
-
     /// \brief Sets the timeout for incoming TCP connections
     ///
     /// Incoming TCP connections that have not sent their data
-    /// withing this time are dropped.
+    /// within this time are dropped.
     ///
     /// \param timeout The timeout (in milliseconds). If se to
     /// zero, no timeouts are used, and the connection will remain

src/bin/auth/benchmarks/query_bench.cc

@@ -18,7 +18,6 @@
 #include <bench/benchmark_util.h>
 
 #include <util/buffer.h>
-#include <util/threads/sync.h>
 
 #include <dns/message.h>
 #include <dns/name.h>
@@ -33,6 +32,7 @@
 #include <auth/auth_srv.h>
 #include <auth/auth_config.h>
 #include <auth/datasrc_config.h>
+#include <auth/datasrc_clients_mgr.h>
 #include <auth/query.h>
 
 #include <asiodns/asiodns.h>
@@ -127,9 +127,9 @@ public:
                           OutputBuffer& buffer) :
         QueryBenchMark(queries, query_message, buffer)
     {
-        isc::util::thread::Mutex::Locker locker(
-                server_->getDataSrcClientListMutex());
-        server_->swapDataSrcClientLists(
+        // Note: setDataSrcClientLists() may be deprecated, but until then
+        // we use it because we want to be synchronized with the server.
+        server_->getDataSrcClientsMgr().setDataSrcClientLists(
             configureDataSource(
                 Element::fromJSON("{\"IN\":"
                                   "  [{\"type\": \"sqlite3\","
@@ -148,9 +148,7 @@ public:
                          OutputBuffer& buffer) :
         QueryBenchMark(queries, query_message, buffer)
     {
-        isc::util::thread::Mutex::Locker locker(
-                server_->getDataSrcClientListMutex());
-        server_->swapDataSrcClientLists(
+        server_->getDataSrcClientsMgr().setDataSrcClientLists(
             configureDataSource(
                 Element::fromJSON("{\"IN\":"
                                   "  [{\"type\": \"MasterFiles\","

src/bin/auth/command.cc

@@ -15,13 +15,13 @@
 #include <auth/command.h>
 #include <auth/auth_log.h>
 #include <auth/auth_srv.h>
+#include <auth/datasrc_clients_mgr.h>
 
 #include <cc/data.h>
 #include <datasrc/client_list.h>
 #include <config/ccsession.h>
 #include <exceptions/exceptions.h>
 #include <dns/rrclass.h>
-#include <util/threads/sync.h>
 
 #include <string>
 
@@ -188,14 +188,11 @@ public:
         if (!origin_elem) {
             isc_throw(AuthCommandError, "Zone origin is missing");
         }
-        Name origin(origin_elem->stringValue());
+        const Name origin(origin_elem->stringValue());
 
-        // We're going to work with the client lists. They may be used
-        // from a different thread too, protect them.
-        isc::util::thread::Mutex::Locker locker(
-            server.getDataSrcClientListMutex());
-        const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-            list(server.getDataSrcClientList(zone_class));
+        DataSrcClientsMgr::Holder holder(server.getDataSrcClientsMgr());
+        boost::shared_ptr<ConfigurableClientList> list =
+            holder.findClientList(zone_class);
 
         if (!list) {
             isc_throw(AuthCommandError, "There's no client list for "

src/bin/auth/datasrc_clients_mgr.h

@@ -0,0 +1,464 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef DATASRC_CLIENTS_MGR_H
+#define DATASRC_CLIENTS_MGR_H 1
+
+#include <util/threads/thread.h>
+#include <util/threads/sync.h>
+
+#include <log/logger_support.h>
+#include <log/log_dbglevels.h>
+
+#include <dns/rrclass.h>
+
+#include <cc/data.h>
+
+#include <datasrc/data_source.h>
+#include <datasrc/client_list.h>
+
+#include <auth/auth_log.h>
+#include <auth/datasrc_config.h>
+
+#include <boost/array.hpp>
+#include <boost/bind.hpp>
+#include <boost/shared_ptr.hpp>
+#include <boost/noncopyable.hpp>
+
+#include <exception>
+#include <list>
+#include <utility>
+
+namespace isc {
+namespace auth {
+
+namespace datasrc_clientmgr_internal {
+// This namespace is essentially private for DataSrcClientsMgr(Base) and
+// DataSrcClientsBuilder(Base).  This is exposed in the public header
+// only because these classes are templated (for testing purposes) and
+// class internal has to be defined here.
+
+/// \brief ID of commands from the DataSrcClientsMgr to DataSrcClientsBuilder.
+enum CommandID {
+    NOOP,         ///< Do nothing.  Only useful for tests; no argument
+    RECONFIGURE,  ///< Reconfigure the datasource client lists,
+                  ///  the argument to the command is the full new
+                  ///  datasources configuration.
+    SHUTDOWN,     ///< Shutdown the builder; no argument
+    NUM_COMMANDS
+};
+
+/// \brief The data type passed from DataSrcClientsMgr to
+/// DataSrcClientsBuilder.
+///
+/// The first element of the pair is the command ID, and the second element
+/// is its argument.  If the command doesn't take an argument it should be
+/// a null pointer.
+typedef std::pair<CommandID, data::ConstElementPtr> Command;
+} // namespace datasrc_clientmgr_internal
+
+/// \brief Frontend to the manager object for data source clients.
+///
+/// This class provides interfaces for configuring and updating a set of
+/// data source clients "in the background".  The user of this class can
+/// assume any operation on this class can be done effectively non-blocking,
+/// not suspending any delay-sensitive operations such as DNS query
+/// processing.  The only exception is the time when this class object
+/// is destroyed (normally as a result of an implicit call to the destructor);
+/// in the current implementation it can take time depending on what is
+/// running "in the background" at the time of the call.
+///
+/// Internally, an object of this class invokes a separate thread to perform
+/// time consuming operations such as loading large zone data into memory,
+/// but such details are completely hidden from the user of this class.
+///
+/// This class is templated only so that we can test the class without
+/// involving actual threads or mutex.  Normal applications will only
+/// need one specific specialization that has a typedef of
+/// \c DataSrcClientsMgr.
+template <typename ThreadType, typename BuilderType, typename MutexType,
+          typename CondVarType>
+class DataSrcClientsMgrBase : boost::noncopyable {
+private:
+    typedef std::map<dns::RRClass,
+                     boost::shared_ptr<datasrc::ConfigurableClientList> >
+    ClientListsMap;
+
+public:
+    /// \brief Thread-safe accessor to the data source client lists.
+    ///
+    /// This class provides a simple wrapper for searching the client lists
+    /// stored in the DataSrcClientsMgr in a thread-safe manner.
+    /// It ensures the result of \c getClientList() can be used without
+    /// causing a race condition with other threads that can possibly use
+    /// the same manager throughout the lifetime of the holder object.
+    ///
+    /// This also means the holder object is expected to have a short lifetime.
+    /// The application shouldn't try to keep it unnecessarily long.
+    /// It's normally expected to create the holder object on the stack
+    /// of a small scope and automatically let it be destroyed at the end
+    /// of the scope.
+    class Holder {
+    public:
+        Holder(DataSrcClientsMgrBase& mgr) :
+            mgr_(mgr), locker_(mgr_.map_mutex_)
+        {}
+
+        /// \brief Find a data source client list of a specified RR class.
+        ///
+        /// It returns a pointer to the list stored in the manager if found,
+        /// otherwise it returns NULL.  The manager keeps the ownership of
+        /// the pointed object.  Also, it's not safe to get access to the
+        /// object beyond the scope of the holder object.
+        ///
+        /// \note Since the ownership isn't transferred the return value
+        /// could be a bare pointer (and it's probably better in several
+        /// points).  Unfortunately, some unit tests currently don't work
+        /// unless this method effectively shares the ownership with the
+        /// tests.  That's the only reason why we return a shared pointer
+        /// for now.  We should eventually fix it and change the return value
+        /// type (see Trac ticket #2395).  Other applications must not
+        /// assume the ownership is actually shared.
+        boost::shared_ptr<datasrc::ConfigurableClientList> findClientList(
+            const dns::RRClass& rrclass)
+        {
+            const ClientListsMap::const_iterator
+                it = mgr_.clients_map_->find(rrclass);
+            if (it == mgr_.clients_map_->end()) {
+                return (boost::shared_ptr<datasrc::ConfigurableClientList>());
+            } else {
+                return (it->second);
+            }
+        }
+    private:
+        DataSrcClientsMgrBase& mgr_;
+        typename MutexType::Locker locker_;
+    };
+
+    /// \brief Constructor.
+    ///
+    /// It internally invokes a separate thread and waits for further
+    /// operations from the user application.
+    ///
+    /// This method is basically exception free except in case of really
+    /// rare system-level errors.  When that happens the only reasonable
+    /// action that the application can take would be to terminate the program
+    /// in practice.
+    ///
+    /// \throw std::bad_alloc internal memory allocation failure.
+    /// \throw isc::Unexpected general unexpected system errors.
+    DataSrcClientsMgrBase() :
+        clients_map_(new ClientListsMap),
+        builder_(&command_queue_, &cond_, &queue_mutex_, &clients_map_,
+                 &map_mutex_),
+        builder_thread_(boost::bind(&BuilderType::run, &builder_))
+    {}
+
+    /// \brief The destructor.
+    ///
+    /// It tells the internal thread to stop and waits for it completion.
+    /// In the current implementation, it can block for some unpredictably
+    /// long period depending on what the thread is doing at that time
+    /// (in future we may want to implement a rapid way of killing the thread
+    /// and/or provide a separate interface for waiting so that the application
+    /// can choose the timing).
+    ///
+    /// The waiting operation can result in an exception, but this method
+    /// catches any of them so this method itself is exception free.
+    ~DataSrcClientsMgrBase() {
+        // We share class member variables with the builder, which will be
+        // invalidated after the call to the destructor, so we need to make
+        // sure the builder thread is terminated.  Depending on the timing
+        // this could take a long time; if we don't want that to happen in
+        // this context, we may want to introduce a separate 'shutdown()'
+        // method.
+        // Also, since we don't want to propagate exceptions from a destructor,
+        // we catch any possible ones.  In fact the only really expected one
+        // is Thread::UncaughtException when the builder thread died due to
+        // an exception.  We specifically log it and just ignore others.
+        try {
+            sendCommand(datasrc_clientmgr_internal::SHUTDOWN,
+                        data::ConstElementPtr());
+            builder_thread_.wait();
+        } catch (const util::thread::Thread::UncaughtException& ex) {
+            // technically, logging this could throw, which will be propagated.
+            // But such an exception would be a fatal one anyway, so we
+            // simply let it go through.
+            LOG_ERROR(auth_logger, AUTH_DATASRC_CLIENTS_SHUTDOWN_ERROR).
+                arg(ex.what());
+        } catch (...) {
+            LOG_ERROR(auth_logger,
+                      AUTH_DATASRC_CLIENTS_SHUTDOWN_UNEXPECTED_ERROR);
+        }
+
+        cleanup();              // see below
+    }
+
+    /// \brief Handle new full configuration for data source clients.
+    ///
+    /// This method simply passes the new configuration to the builder
+    /// and immediately returns.  This method is basically exception free
+    /// as long as the caller passes a non NULL value for \c config_arg;
+    /// it doesn't validate the argument further.
+    ///
+    /// \brief isc::InvalidParameter config_arg is NULL.
+    /// \brief std::bad_alloc
+    ///
+    /// \param config_arg The new data source configuration.  Must not be NULL.
+    void reconfigure(data::ConstElementPtr config_arg) {
+        if (!config_arg) {
+            isc_throw(InvalidParameter, "Invalid null config argument");
+        }
+        sendCommand(datasrc_clientmgr_internal::RECONFIGURE, config_arg);
+        reconfigureHook();      // for test's customization
+    }
+
+    /// \brief Set the underlying data source client lists to new lists.
+    ///
+    /// This is provided only for some existing tests until we support a
+    /// cleaner way to use faked data source clients.  Non test code or
+    /// newer tests must not use this.
+    void setDataSrcClientLists(datasrc::ClientListMapPtr new_lists) {
+        typename MutexType::Locker locker(map_mutex_);
+        clients_map_ = new_lists;
+    }
+
+private:
+    // This is expected to be called at the end of the destructor.  It
+    // actually does nothing, but provides a customization point for
+    // specialized class for tests so that the tests can inspect the last
+    // state of the class.
+    void cleanup() {}
+
+    // same as cleanup(), for reconfigure().
+    void reconfigureHook() {}
+
+    void sendCommand(datasrc_clientmgr_internal::CommandID command,
+                     data::ConstElementPtr arg)
+    {
+        // The lock will be held until the end of this method.  Only
+        // push_back has to be protected, but we can avoid having an extra
+        // block this way.
+        typename MutexType::Locker locker(queue_mutex_);
+        command_queue_.push_back(
+            datasrc_clientmgr_internal::Command(command, arg));
+        cond_.signal();
+    }
+
+    //
+    // The following are shared with the builder.
+    //
+    // The list is used as a one-way queue: back-in, front-out
+    std::list<datasrc_clientmgr_internal::Command> command_queue_;
+    CondVarType cond_;          // condition variable for queue operations
+    MutexType queue_mutex_;     // mutex to protect the queue
+    datasrc::ClientListMapPtr clients_map_;
+                                // map of actual data source client objects
+    MutexType map_mutex_;       // mutex to protect the clients map
+
+    BuilderType builder_;
+    ThreadType builder_thread_; // for safety this should be placed last
+};
+
+namespace datasrc_clientmgr_internal {
+
+/// \brief A class that maintains a set of data source clients.
+///
+/// An object of this class is supposed to run on a dedicated thread, whose
+/// main function is a call to its \c run() method.  It runs in a loop
+/// waiting for commands from the manager and handles each command (including
+/// reloading a new version of zone data into memory or fully reconfiguration
+/// of specific set of data source clients).  When it receives a SHUTDOWN
+/// command, it exits from the loop, which will terminate the thread.
+///
+/// While this class is defined in a publicly visible namespace, it's
+/// essentially private to \c DataSrcClientsMgr.  Except for tests,
+/// applications should not directly access this class.
+///
+/// This class is templated so that we can test it without involving actual
+/// threads or locks.
+template <typename MutexType, typename CondVarType>
+class DataSrcClientsBuilderBase : boost::noncopyable {
+public:
+    /// \brief Constructor.
+    ///
+    /// It simply sets up a local copy of shared data with the manager.
+    ///
+    /// Note: this will take actual set (map) of data source clients and
+    /// a mutex object for it in #2210 or #2212.
+    ///
+    /// \throw None
+    DataSrcClientsBuilderBase(std::list<Command>* command_queue,
+                              CondVarType* cond, MutexType* queue_mutex,
+                              datasrc::ClientListMapPtr* clients_map,
+                              MutexType* map_mutex
+        ) :
+        command_queue_(command_queue), cond_(cond), queue_mutex_(queue_mutex),
+        clients_map_(clients_map), map_mutex_(map_mutex)
+    {}
+
+    /// \brief The main loop.
+    void run();
+
+    /// \brief Handle one command from the manager.
+    ///
+    /// This is a dedicated subroutine of run() and is essentially private,
+    /// but is defined as a separate public method so we can test each
+    /// command test individually.  In any case, this class itself is
+    /// generally considered private.
+    ///
+    /// \return true if the builder should keep running; false otherwise.
+    bool handleCommand(const Command& command);
+
+private:
+    // NOOP command handler.  We use this so tests can override it; the default
+    // implementation really does nothing.
+    void doNoop() {}
+
+    void doReconfigure(const data::ConstElementPtr& config) {
+        if (config) {
+            LOG_INFO(auth_logger,
+                     AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_STARTED);
+            try {
+                // Define new_clients_map outside of the block that
+                // has the lock scope; this way, after the swap,
+                // the lock is guaranteed to be released before
+                // the old data is destroyed, minimizing the lock
+                // duration.
+                datasrc::ClientListMapPtr new_clients_map =
+                    configureDataSource(config);
+                {
+                    typename MutexType::Locker locker(*map_mutex_);
+                    new_clients_map.swap(*clients_map_);
+                } // lock is released by leaving scope
+                LOG_INFO(auth_logger,
+                         AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_SUCCESS);
+            } catch (const datasrc::ConfigurableClientList::ConfigurationError&
+                     config_error) {
+                LOG_ERROR(auth_logger,
+                    AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_CONFIG_ERROR).
+                    arg(config_error.what());
+            } catch (const datasrc::DataSourceError& ds_error) {
+                LOG_ERROR(auth_logger,
+                    AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_DATASRC_ERROR).
+                    arg(ds_error.what());
+            } catch (const isc::Exception& isc_error) {
+                LOG_ERROR(auth_logger,
+                    AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_ERROR).
+                    arg(isc_error.what());
+            }
+            // other exceptions are propagated, see
+            // http://bind10.isc.org/ticket/2210#comment:13
+
+            // old clients_map_ data is released by leaving scope
+        }
+    }
+
+    // The following are shared with the manager
+    std::list<Command>* command_queue_;
+    CondVarType* cond_;
+    MutexType* queue_mutex_;
+    datasrc::ClientListMapPtr* clients_map_;
+    MutexType* map_mutex_;
+};
+
+// Shortcut typedef for normal use
+typedef DataSrcClientsBuilderBase<util::thread::Mutex, util::thread::CondVar>
+DataSrcClientsBuilder;
+
+template <typename MutexType, typename CondVarType>
+void
+DataSrcClientsBuilderBase<MutexType, CondVarType>::run() {
+    LOG_INFO(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_STARTED);
+
+    try {
+        bool keep_running = true;
+        while (keep_running) {
+            std::list<Command> current_commands;
+            {
+                // Move all new commands to local queue under the protection of
+                // queue_mutex_.
+                typename MutexType::Locker locker(*queue_mutex_);
+                while (command_queue_->empty()) {
+                    cond_->wait(*queue_mutex_);
+                }
+                current_commands.swap(*command_queue_);
+            } // the lock is released here.
+
+            while (keep_running && !current_commands.empty()) {
+                keep_running = handleCommand(current_commands.front());
+                current_commands.pop_front();
+            }
+        }
+
+        LOG_INFO(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_STOPPED);
+    } catch (const std::exception& ex) {
+        // We explicitly catch exceptions so we can log it as soon as possible.
+        LOG_FATAL(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_FAILED).
+            arg(ex.what());
+        std::terminate();
+    } catch (...) {
+        LOG_FATAL(auth_logger, AUTH_DATASRC_CLIENTS_BUILDER_FAILED_UNEXPECTED);
+        std::terminate();
+    }
+}
+
+template <typename MutexType, typename CondVarType>
+bool
+DataSrcClientsBuilderBase<MutexType, CondVarType>::handleCommand(
+    const Command& command)
+{
+    const CommandID cid = command.first;
+    if (cid >= NUM_COMMANDS) {
+        // This shouldn't happen except for a bug within this file.
+        isc_throw(Unexpected, "internal bug: invalid command, ID: " << cid);
+    }
+
+    const boost::array<const char*, NUM_COMMANDS> command_desc = {
+        {"NOOP", "RECONFIGURE", "SHUTDOWN"}
+    };
+    LOG_DEBUG(auth_logger, DBGLVL_TRACE_BASIC,
+              AUTH_DATASRC_CLIENTS_BUILDER_COMMAND).arg(command_desc.at(cid));
+    switch (command.first) {
+    case RECONFIGURE:
+        doReconfigure(command.second);
+        break;
+    case SHUTDOWN:
+        return (false);
+    case NOOP:
+        doNoop();
+        break;
+    case NUM_COMMANDS:
+        assert(false);          // we rejected this case above
+    }
+    return (true);
+}
+} // namespace datasrc_clientmgr_internal
+
+/// \brief Shortcut type for normal data source clients manager.
+///
+/// In fact, for non test applications this is the only type of this kind
+/// to be considered.
+typedef DataSrcClientsMgrBase<
+    util::thread::Thread,
+    datasrc_clientmgr_internal::DataSrcClientsBuilder,
+    util::thread::Mutex, util::thread::CondVar> DataSrcClientsMgr;
+} // namespace auth
+} // namespace isc
+
+#endif  // DATASRC_CLIENTS_MGR_H
+
+// Local Variables:
+// mode: c++
+// End:

src/bin/auth/datasrc_config.cc

@@ -13,12 +13,11 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <cc/data.h>
-#include "auth_srv.h"
 #include "datasrc_config.h"
 
 // This is a trivial specialization for the commonly used version.
 // Defined in .cc to avoid accidental creation of multiple copies.
-AuthSrv::DataSrcClientListsPtr
+isc::datasrc::ClientListMapPtr
 configureDataSource(const isc::data::ConstElementPtr& config) {
     return (configureDataSourceGeneric<
             isc::datasrc::ConfigurableClientList>(config));

src/bin/auth/datasrc_config.h

@@ -15,15 +15,13 @@
 #ifndef DATASRC_CONFIG_H
 #define DATASRC_CONFIG_H
 
-#include "auth_srv.h"
-
 #include <cc/data.h>
 #include <datasrc/client_list.h>
 
 #include <boost/shared_ptr.hpp>
 
 #include <utility>
-#include <set>
+#include <map>
 
 /// \brief Configure data source client lists
 ///
@@ -48,6 +46,8 @@
 /// \param config The configuration value to parse. It is in the form
 ///     as an update from the config manager.
 /// \return A map from RR classes to configured lists.
+/// \throw ConfigurationError if the config element is not in the expected
+///        format (A map of lists)
 template<class List>
 boost::shared_ptr<std::map<isc::dns::RRClass,
                            boost::shared_ptr<List> > > // = ListMap below
@@ -58,7 +58,6 @@ configureDataSourceGeneric(const isc::data::ConstElementPtr& config) {
 
     boost::shared_ptr<ListMap> new_lists(new ListMap);
 
-    // Go through the configuration and create corresponding list.
     const Map& map(config->mapValue());
     for (Map::const_iterator it(map.begin()); it != map.end(); ++it) {
         const isc::dns::RRClass rrclass(it->first);
@@ -73,7 +72,7 @@ configureDataSourceGeneric(const isc::data::ConstElementPtr& config) {
 
 /// \brief Concrete version of configureDataSource() for the
 ///     use with authoritative server implementation.
-AuthSrv::DataSrcClientListsPtr
+isc::datasrc::ClientListMapPtr
 configureDataSource(const isc::data::ConstElementPtr& config);
 
 #endif  // DATASRC_CONFIG_H

src/bin/auth/main.cc

@@ -18,7 +18,6 @@
 
 #include <util/buffer.h>
 #include <util/io/socketsession.h>
-#include <util/threads/sync.h>
 
 #include <dns/message.h>
 #include <dns/messagerenderer.h>
@@ -36,6 +35,8 @@
 #include <auth/auth_srv.h>
 #include <auth/auth_log.h>
 #include <auth/datasrc_config.h>
+#include <auth/datasrc_clients_mgr.h>
+
 #include <asiodns/asiodns.h>
 #include <asiolink/asiolink.h>
 #include <log/logger_support.h>
@@ -93,32 +94,23 @@ datasrcConfigHandler(AuthSrv* server, bool* first_time,
                      const isc::config::ConfigData&)
 {
     assert(server != NULL);
-    if (config->contains("classes")) {
-        AuthSrv::DataSrcClientListsPtr lists;
-
-        if (*first_time) {
-            // HACK: The default is not passed to the handler in the first
-            // callback. This one will get the default (or, current value).
-            // Further updates will work the usual way.
-            assert(config_session != NULL);
-            *first_time = false;
-            lists = configureDataSource(
-                config_session->getRemoteConfigValue("data_sources",
-                                                     "classes"));
-        } else {
-            lists = configureDataSource(config->get("classes"));
-        }
 
-        // Replace the server's lists.  The returned lists will be stored
-        // in a local variable 'lists', and will be destroyed outside of
-        // the temporary block for the lock scope.  That way we can minimize
-        // the range of the critical section.
-        {
-            isc::util::thread::Mutex::Locker locker(
-                server->getDataSrcClientListMutex());
-            lists = server->swapDataSrcClientLists(lists);
-        }
-        // The previous lists are destroyed here.
+    // Note: remote config handler is requested to be exception free.
+    // While the code below is not 100% exception free, such an exception
+    // is really fatal and the server should actually stop.  So we don't
+    // bother to catch them; the exception would be propagated to the
+    // top level of the server and terminate it.
+
+    if (*first_time) {
+        // HACK: The default is not passed to the handler in the first
+        // callback. This one will get the default (or, current value).
+        // Further updates will work the usual way.
+        assert(config_session != NULL);
+        *first_time = false;
+        server->getDataSrcClientsMgr().reconfigure(
+            config_session->getRemoteConfigValue("data_sources", "classes"));
+    } else if (config->contains("classes")) {
+        server->getDataSrcClientsMgr().reconfigure(config->get("classes"));
     }
 }
 

src/bin/auth/tests/Makefile.am

@@ -51,6 +51,9 @@ run_unittests_SOURCES += command_unittest.cc
 run_unittests_SOURCES += common_unittest.cc
 run_unittests_SOURCES += query_unittest.cc
 run_unittests_SOURCES += statistics_unittest.cc
+run_unittests_SOURCES += test_datasrc_clients_mgr.h test_datasrc_clients_mgr.cc
+run_unittests_SOURCES += datasrc_clients_builder_unittest.cc
+run_unittests_SOURCES += datasrc_clients_mgr_unittest.cc
 run_unittests_SOURCES += datasrc_config_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
 

src/bin/auth/tests/auth_srv_unittest.cc

@@ -39,7 +39,6 @@
 #include <auth/datasrc_config.h>
 
 #include <util/unittests/mock_socketsession.h>
-#include <util/threads/sync.h>
 #include <dns/tests/unittest_util.h>
 #include <testutils/dnsmessage_test.h>
 #include <testutils/srv_test.h>
@@ -70,6 +69,7 @@ using namespace isc::util::unittests;
 using namespace isc::dns::rdata;
 using namespace isc::data;
 using namespace isc::xfr;
+using namespace isc::auth;
 using namespace isc::asiodns;
 using namespace isc::asiolink;
 using namespace isc::testutils;
@@ -726,11 +726,11 @@ TEST_F(AuthSrvTest, notifyWithSessionMessageError) {
 }
 
 void
-installDataSrcClientLists(AuthSrv& server,
-                          AuthSrv::DataSrcClientListsPtr lists)
-{
-    thread::Mutex::Locker locker(server.getDataSrcClientListMutex());
-    server.swapDataSrcClientLists(lists);
+installDataSrcClientLists(AuthSrv& server, ClientListMapPtr lists) {
+    // For now, we use explicit swap than reconfigure() because the latter
+    // involves a separate thread and cannot guarantee the new config is
+    // available for the subsequent test.
+    server.getDataSrcClientsMgr().setDataSrcClientLists(lists);
 }
 
 void
@@ -1438,16 +1438,16 @@ TEST_F(AuthSrvTest,
 {
     // Set real inmem client to proxy
     updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    boost::shared_ptr<isc::datasrc::ConfigurableClientList> list;
+    DataSrcClientsMgr& mgr = server.getDataSrcClientsMgr();
     {
-        isc::util::thread::Mutex::Locker locker(
-            server.getDataSrcClientListMutex());
-        boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-            list(new FakeList(server.getDataSrcClientList(RRClass::IN()),
-                              THROW_NEVER, false));
-        AuthSrv::DataSrcClientListsPtr lists(new std::map<RRClass, ListPtr>);
-        lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
-        server.swapDataSrcClientLists(lists);
+        DataSrcClientsMgr::Holder holder(mgr);
+        list.reset(new FakeList(holder.findClientList(RRClass::IN()),
+                                THROW_NEVER, false));
     }
+    ClientListMapPtr lists(new std::map<RRClass, ListPtr>);
+    lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
+    server.getDataSrcClientsMgr().setDataSrcClientLists(lists);
 
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1470,14 +1470,16 @@ setupThrow(AuthSrv& server, ThrowWhen throw_when, bool isc_exception,
 {
     updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
-    isc::util::thread::Mutex::Locker locker(
-        server.getDataSrcClientListMutex());
-    boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-        list(new FakeList(server.getDataSrcClientList(RRClass::IN()),
-                          throw_when, isc_exception, rrset));
-    AuthSrv::DataSrcClientListsPtr lists(new std::map<RRClass, ListPtr>);
+    boost::shared_ptr<isc::datasrc::ConfigurableClientList> list;
+    DataSrcClientsMgr& mgr = server.getDataSrcClientsMgr();
+    {           // we need to limit the scope so swap is outside of it
+        DataSrcClientsMgr::Holder holder(mgr);
+        list.reset(new FakeList(holder.findClientList(RRClass::IN()),
+                                throw_when, isc_exception, rrset));
+    }
+    ClientListMapPtr lists(new std::map<RRClass, ListPtr>);
     lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
-    server.swapDataSrcClientLists(lists);
+    mgr.setDataSrcClientLists(lists);
 }
 
 TEST_F(AuthSrvTest,
@@ -1784,61 +1786,4 @@ TEST_F(AuthSrvTest, DDNSForwardCreateDestroy) {
                 Opcode::UPDATE().getCode(), QR_FLAG, 0, 0, 0, 0);
 }
 
-// Check the client list accessors
-TEST_F(AuthSrvTest, clientList) {
-    // We need to lock the mutex to make the (get|set)ClientList happy.
-    // There's a debug-build only check in them to make sure everything
-    // locks them and we call them directly here.
-    isc::util::thread::Mutex::Locker locker(
-        server.getDataSrcClientListMutex());
-
-    AuthSrv::DataSrcClientListsPtr lists; // initially empty
-
-    // The lists don't exist. Therefore, the list of RRClasses is empty.
-    EXPECT_TRUE(server.swapDataSrcClientLists(lists)->empty());
-
-    // Put something in.
-    const ListPtr list(new ConfigurableClientList(RRClass::IN()));
-    const ListPtr list2(new ConfigurableClientList(RRClass::CH()));
-
-    lists.reset(new std::map<RRClass, ListPtr>);
-    lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
-    lists->insert(pair<RRClass, ListPtr>(RRClass::CH(), list2));
-    server.swapDataSrcClientLists(lists);
-
-    // And the lists can be retrieved.
-    EXPECT_EQ(list, server.getDataSrcClientList(RRClass::IN()));
-    EXPECT_EQ(list2, server.getDataSrcClientList(RRClass::CH()));
-
-    // Replace the lists with new lists containing only one list.
-    lists.reset(new std::map<RRClass, ListPtr>);
-    lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
-    lists = server.swapDataSrcClientLists(lists);
-
-    // Old one had two lists.  That confirms our swap for IN and CH classes
-    // (i.e., no other entries were there).
-    EXPECT_EQ(2, lists->size());
-
-    // The CH list really got deleted.
-    EXPECT_EQ(list, server.getDataSrcClientList(RRClass::IN()));
-    EXPECT_FALSE(server.getDataSrcClientList(RRClass::CH()));
-}
-
-#ifdef ENABLE_DEBUG
-
-// We just test the mutex can be locked (exactly once).
-TEST_F(AuthSrvTest, mutex) {
-    isc::util::thread::Mutex::Locker l1(server.getDataSrcClientListMutex());
-    // TODO: Once we have non-debug build, this one will not work, since
-    // we currently use the fact that we can't lock twice from the same
-    // thread. In the non-debug mode, this would deadlock.
-    // Skip then.
-    EXPECT_THROW({
-        isc::util::thread::Mutex::Locker l2(
-            server.getDataSrcClientListMutex());
-    }, isc::InvalidOperation);
-}
-
-#endif // ENABLE_DEBUG
-
 }

src/bin/auth/tests/command_unittest.cc

@@ -16,10 +16,7 @@
 
 #include "datasrc_util.h"
 
-#include <util/threads/sync.h>
-
 #include <auth/auth_srv.h>
-#include <auth/auth_config.h>
 #include <auth/command.h>
 #include <auth/datasrc_config.h>
 
@@ -58,6 +55,7 @@ using namespace isc::datasrc;
 using namespace isc::config;
 using namespace isc::util::unittests;
 using namespace isc::testutils;
+using namespace isc::auth;
 using namespace isc::auth::unittest;
 
 namespace {
@@ -176,29 +174,26 @@ TEST_F(AuthCommandTest, shutdownIncorrectPID) {
 // zones, and checks the zones are correctly loaded.
 void
 zoneChecks(AuthSrv& server) {
-    isc::util::thread::Mutex::Locker locker(
-        server.getDataSrcClientListMutex());
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
-              find(Name("ns.test1.example")).finder_->
-              find(Name("ns.test1.example"), RRType::A())->code);
-    EXPECT_EQ(ZoneFinder::NXRRSET, server.getDataSrcClientList(RRClass::IN())->
-              find(Name("ns.test1.example")).finder_->
-              find(Name("ns.test1.example"), RRType::AAAA())->code);
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
-              find(Name("ns.test2.example")).finder_->
-              find(Name("ns.test2.example"), RRType::A())->code);
-    EXPECT_EQ(ZoneFinder::NXRRSET, server.getDataSrcClientList(RRClass::IN())->
-              find(Name("ns.test2.example")).finder_->
-              find(Name("ns.test2.example"), RRType::AAAA())->code);
+    const RRClass rrclass(RRClass::IN());
+
+    DataSrcClientsMgr::Holder holder(server.getDataSrcClientsMgr());
+    EXPECT_EQ(ZoneFinder::SUCCESS,
+              holder.findClientList(rrclass)->find(Name("ns.test1.example"))
+              .finder_->find(Name("ns.test1.example"), RRType::A())->code);
+    EXPECT_EQ(ZoneFinder::NXRRSET,
+              holder.findClientList(rrclass)->find(Name("ns.test1.example")).
+              finder_->find(Name("ns.test1.example"), RRType::AAAA())->code);
+    EXPECT_EQ(ZoneFinder::SUCCESS,
+              holder.findClientList(rrclass)->find(Name("ns.test2.example")).
+              finder_->find(Name("ns.test2.example"), RRType::A())->code);
+    EXPECT_EQ(ZoneFinder::NXRRSET,
+              holder.findClientList(rrclass)->find(Name("ns.test2.example")).
+              finder_->find(Name("ns.test2.example"), RRType::AAAA())->code);
 }
 
 void
-installDataSrcClientLists(AuthSrv& server,
-                          AuthSrv::DataSrcClientListsPtr lists)
-{
-    isc::util::thread::Mutex::Locker locker(
-        server.getDataSrcClientListMutex());
-    server.swapDataSrcClientLists(lists);
+installDataSrcClientLists(AuthSrv& server, ClientListMapPtr lists) {
+    server.getDataSrcClientsMgr().setDataSrcClientLists(lists);
 }
 
 void
@@ -227,22 +222,24 @@ configureZones(AuthSrv& server) {
 
 void
 newZoneChecks(AuthSrv& server) {
-    isc::util::thread::Mutex::Locker locker(
-        server.getDataSrcClientListMutex());
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
+    const RRClass rrclass(RRClass::IN());
+
+    DataSrcClientsMgr::Holder holder(server.getDataSrcClientsMgr());
+    EXPECT_EQ(ZoneFinder::SUCCESS, holder.findClientList(rrclass)->
               find(Name("ns.test1.example")).finder_->
               find(Name("ns.test1.example"), RRType::A())->code);
+
     // now test1.example should have ns/AAAA
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS, holder.findClientList(rrclass)->
               find(Name("ns.test1.example")).finder_->
               find(Name("ns.test1.example"), RRType::AAAA())->code);
 
     // test2.example shouldn't change
-    EXPECT_EQ(ZoneFinder::SUCCESS, server.getDataSrcClientList(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS, holder.findClientList(rrclass)->
               find(Name("ns.test2.example")).finder_->
               find(Name("ns.test2.example"), RRType::A())->code);
     EXPECT_EQ(ZoneFinder::NXRRSET,
-              server.getDataSrcClientList(RRClass::IN())->
+              holder.findClientList(rrclass)->
               find(Name("ns.test2.example")).finder_->
               find(Name("ns.test2.example"), RRType::AAAA())->code);
 }
@@ -288,11 +285,11 @@ TEST_F(AuthCommandTest,
     installDataSrcClientLists(server_, configureDataSource(config));
 
     {
-        isc::util::thread::Mutex::Locker locker(
-            server_.getDataSrcClientListMutex());
+        DataSrcClientsMgr::Holder holder(server_.getDataSrcClientsMgr());
+
         // Check that the A record at www.example.org does not exist
         EXPECT_EQ(ZoneFinder::NXDOMAIN,
-                  server_.getDataSrcClientList(RRClass::IN())->
+                  holder.findClientList(RRClass::IN())->
                   find(Name("example.org")).finder_->
                   find(Name("www.example.org"), RRType::A())->code);
 
@@ -313,7 +310,7 @@ TEST_F(AuthCommandTest,
         sql_updater->commit();
 
         EXPECT_EQ(ZoneFinder::NXDOMAIN,
-                  server_.getDataSrcClientList(RRClass::IN())->
+                  holder.findClientList(RRClass::IN())->
                   find(Name("example.org")).finder_->
                   find(Name("www.example.org"), RRType::A())->code);
     }
@@ -325,11 +322,10 @@ TEST_F(AuthCommandTest,
     checkAnswer(0, "Successful load");
 
     {
-        isc::util::thread::Mutex::Locker locker(
-            server_.getDataSrcClientListMutex());
+        DataSrcClientsMgr::Holder holder(server_.getDataSrcClientsMgr());
         // And now it should be present too.
         EXPECT_EQ(ZoneFinder::SUCCESS,
-                  server_.getDataSrcClientList(RRClass::IN())->
+                  holder.findClientList(RRClass::IN())->
                   find(Name("example.org")).finder_->
                   find(Name("www.example.org"), RRType::A())->code);
     }
@@ -340,11 +336,10 @@ TEST_F(AuthCommandTest,
     checkAnswer(1, "example.com");
 
     {
-        isc::util::thread::Mutex::Locker locker(
-            server_.getDataSrcClientListMutex());
+        DataSrcClientsMgr::Holder holder(server_.getDataSrcClientsMgr());
         // The previous zone is not hurt in any way
         EXPECT_EQ(ZoneFinder::SUCCESS,
-                  server_.getDataSrcClientList(RRClass::IN())->
+                  holder.findClientList(RRClass::IN())->
                   find(Name("example.org")).finder_->
                   find(Name("example.org"), RRType::SOA())->code);
     }
@@ -363,11 +358,10 @@ TEST_F(AuthCommandTest,
         Element::fromJSON("{\"origin\": \"example.com\"}"));
     checkAnswer(1, "Unreadable");
 
-    isc::util::thread::Mutex::Locker locker(
-        server_.getDataSrcClientListMutex());
+    DataSrcClientsMgr::Holder holder(server_.getDataSrcClientsMgr());
     // The previous zone is not hurt in any way
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              server_.getDataSrcClientList(RRClass::IN())->
+              holder.findClientList(RRClass::IN())->
               find(Name("example.org")).finder_->
               find(Name("example.org"), RRType::SOA())->code);
 }

src/bin/auth/tests/datasrc_clients_builder_unittest.cc

@@ -0,0 +1,196 @@
+// Copyright (C) 2012  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 <cc/data.h>
+
+#include <auth/datasrc_clients_mgr.h>
+#include "test_datasrc_clients_mgr.h"
+
+#include <gtest/gtest.h>
+
+#include <boost/function.hpp>
+
+using isc::data::ConstElementPtr;
+using namespace isc::datasrc;
+using namespace isc::auth::datasrc_clientmgr_internal;
+
+namespace {
+class DataSrcClientsBuilderTest : public ::testing::Test {
+protected:
+    DataSrcClientsBuilderTest() :
+        builder(&command_queue, &cond, &queue_mutex, &clients_map, &map_mutex),
+        cond(command_queue, delayed_command_queue),
+        shutdown_cmd(SHUTDOWN, ConstElementPtr()),
+        noop_cmd(NOOP, ConstElementPtr())
+    {}
+
+    TestDataSrcClientsBuilder builder;
+    std::list<Command> command_queue; // test command queue
+    std::list<Command> delayed_command_queue; // commands available after wait
+    ClientListMapPtr clients_map; // configured clients
+    TestCondVar cond;
+    TestMutex queue_mutex;
+    TestMutex map_mutex;
+    const Command shutdown_cmd;
+    const Command noop_cmd;
+};
+
+TEST_F(DataSrcClientsBuilderTest, runSingleCommand) {
+    // A simplest case, just to check the basic behavior.
+    command_queue.push_back(shutdown_cmd);
+    builder.run();
+    EXPECT_TRUE(command_queue.empty());
+    EXPECT_EQ(0, cond.wait_count); // no wait because command queue is not empty
+    EXPECT_EQ(1, queue_mutex.lock_count);
+    EXPECT_EQ(1, queue_mutex.unlock_count);
+}
+
+TEST_F(DataSrcClientsBuilderTest, runMultiCommands) {
+    // Two NOOP commands followed by SHUTDOWN.  We should see two doNoop()
+    // calls.
+    command_queue.push_back(noop_cmd);
+    command_queue.push_back(noop_cmd);
+    command_queue.push_back(shutdown_cmd);
+    builder.run();
+    EXPECT_TRUE(command_queue.empty());
+    EXPECT_EQ(1, queue_mutex.lock_count);
+    EXPECT_EQ(1, queue_mutex.unlock_count);
+    EXPECT_EQ(2, queue_mutex.noop_count);
+}
+
+TEST_F(DataSrcClientsBuilderTest, exception) {
+    // Let the noop command handler throw exceptions and see if we can see
+    // them.  Right now, we simply abort to prevent the system from running
+    // with half-broken state.  Eventually we should introduce a better
+    // error handling.
+    command_queue.push_back(noop_cmd);
+    queue_mutex.throw_from_noop = TestMutex::EXCLASS;
+    EXPECT_DEATH_IF_SUPPORTED({builder.run();}, "");
+
+    command_queue.push_back(noop_cmd);
+    queue_mutex.throw_from_noop = TestMutex::INTEGER;
+    EXPECT_DEATH_IF_SUPPORTED({builder.run();}, "");
+}
+
+TEST_F(DataSrcClientsBuilderTest, condWait) {
+    // command_queue is originally empty, so it will require waiting on
+    // condvar.  specialized wait() will make the delayed command available.
+    delayed_command_queue.push_back(shutdown_cmd);
+    builder.run();
+
+    // There should be one call to wait()
+    EXPECT_EQ(1, cond.wait_count);
+    // wait() effectively involves one more set of lock/unlock, so we have
+    // two in total
+    EXPECT_EQ(2, queue_mutex.lock_count);
+    EXPECT_EQ(2, queue_mutex.unlock_count);
+}
+
+TEST_F(DataSrcClientsBuilderTest, reconfigure) {
+    // Full testing of different configurations is not here, but we
+    // do check a few cases of correct and erroneous input, to verify
+    // the error handling
+
+    // A command structure we'll modify to send different commands
+    Command reconfig_cmd(RECONFIGURE, ConstElementPtr());
+
+    // Initially, no clients should be there
+    EXPECT_EQ(ClientListMapPtr(), clients_map);
+
+    // A config that doesn't do much except be accepted
+    ConstElementPtr good_config = isc::data::Element::fromJSON(
+        "{"
+        "\"IN\": [{"
+        "   \"type\": \"MasterFiles\","
+        "   \"params\": {},"
+        "   \"cache-enable\": true"
+        "}]"
+        "}"
+    );
+
+    // A configuration that is 'correct' in the top-level, but contains
+    // bad data for the type it specifies
+    ConstElementPtr bad_config = isc::data::Element::fromJSON(
+        "{"
+        "\"IN\": [{"
+        "   \"type\": \"MasterFiles\","
+        "   \"params\": { \"foo\": [ 1, 2, 3, 4  ]},"
+        "   \"cache-enable\": true"
+        "}]"
+        "}"
+    );
+
+    reconfig_cmd.second = good_config;
+    EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
+    EXPECT_EQ(1, clients_map->size());
+    EXPECT_EQ(1, map_mutex.lock_count);
+
+    // Store the nonempty clients map we now have
+    ClientListMapPtr working_config_clients(clients_map);
+
+    // If a 'bad' command argument got here, the config validation should
+    // have failed already, but still, the handler should return true,
+    // and the clients_map should not be updated.
+    reconfig_cmd.second = isc::data::Element::create("{ \"foo\": \"bar\" }");
+    EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
+    EXPECT_EQ(working_config_clients, clients_map);
+    // Building failed, so map mutex should not have been locked again
+    EXPECT_EQ(1, map_mutex.lock_count);
+
+    // The same for a configuration that has bad data for the type it
+    // specifies
+    reconfig_cmd.second = bad_config;
+    builder.handleCommand(reconfig_cmd);
+    EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
+    EXPECT_EQ(working_config_clients, clients_map);
+    // Building failed, so map mutex should not have been locked again
+    EXPECT_EQ(1, map_mutex.lock_count);
+
+    // The same goes for an empty parameter (it should at least be
+    // an empty map)
+    reconfig_cmd.second = ConstElementPtr();
+    EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
+    EXPECT_EQ(working_config_clients, clients_map);
+    EXPECT_EQ(1, map_mutex.lock_count);
+
+    // Reconfigure again with the same good clients, the result should
+    // be a different map than the original, but not an empty one.
+    reconfig_cmd.second = good_config;
+    EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
+    EXPECT_NE(working_config_clients, clients_map);
+    EXPECT_EQ(1, clients_map->size());
+    EXPECT_EQ(2, map_mutex.lock_count);
+
+    // And finally, try an empty config to disable all datasource clients
+    reconfig_cmd.second = isc::data::Element::createMap();
+    EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
+    EXPECT_EQ(0, clients_map->size());
+    EXPECT_EQ(3, map_mutex.lock_count);
+
+    // Also check if it has been cleanly unlocked every time
+    EXPECT_EQ(3, map_mutex.unlock_count);
+}
+
+TEST_F(DataSrcClientsBuilderTest, shutdown) {
+    EXPECT_FALSE(builder.handleCommand(shutdown_cmd));
+}
+
+TEST_F(DataSrcClientsBuilderTest, badCommand) {
+    // out-of-range command ID
+    EXPECT_THROW(builder.handleCommand(Command(NUM_COMMANDS,
+                                               ConstElementPtr())),
+                 isc::Unexpected);
+}
+
+} // unnamed namespace

src/bin/auth/tests/datasrc_clients_mgr_unittest.cc

@@ -0,0 +1,205 @@
+// Copyright (C) 2012  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 <exceptions/exceptions.h>
+
+#include <dns/rrclass.h>
+
+#include <cc/data.h>
+
+#include <datasrc/client_list.h>
+
+#include <auth/datasrc_clients_mgr.h>
+#include "test_datasrc_clients_mgr.h"
+
+#include <gtest/gtest.h>
+
+#include <boost/function.hpp>
+
+using namespace isc::dns;
+using namespace isc::data;
+using namespace isc::datasrc;
+using namespace isc::auth;
+using namespace isc::auth::datasrc_clientmgr_internal;
+
+namespace {
+void
+shutdownCheck() {
+    // Check for common points on shutdown.  The manager should have acquired
+    // the lock, put a SHUTDOWN command to the queue, and should have signaled
+    // the builder.
+    EXPECT_EQ(1, FakeDataSrcClientsBuilder::queue_mutex->lock_count);
+    EXPECT_EQ(1, FakeDataSrcClientsBuilder::cond->signal_count);
+    EXPECT_EQ(1, FakeDataSrcClientsBuilder::command_queue->size());
+    const Command& cmd = FakeDataSrcClientsBuilder::command_queue->front();
+    EXPECT_EQ(SHUTDOWN, cmd.first);
+    EXPECT_FALSE(cmd.second);   // no argument
+
+    // Finally, the manager should wait for the thread to terminate.
+    EXPECT_TRUE(FakeDataSrcClientsBuilder::thread_waited);
+}
+
+// Commonly used pattern of checking member variables shared between the
+// manager and builder.
+void
+checkSharedMembers(size_t expected_queue_lock_count,
+                   size_t expected_queue_unlock_count,
+                   size_t expected_map_lock_count,
+                   size_t expected_map_unlock_count,
+                   size_t expected_cond_signal_count,
+                   size_t expected_command_queue_size)
+{
+    EXPECT_EQ(expected_queue_lock_count,
+              FakeDataSrcClientsBuilder::queue_mutex->lock_count);
+    EXPECT_EQ(expected_queue_unlock_count,
+              FakeDataSrcClientsBuilder::queue_mutex->unlock_count);
+    EXPECT_EQ(expected_map_lock_count,
+              FakeDataSrcClientsBuilder::map_mutex->lock_count);
+    EXPECT_EQ(expected_map_unlock_count,
+              FakeDataSrcClientsBuilder::map_mutex->unlock_count);
+    EXPECT_EQ(expected_cond_signal_count,
+              FakeDataSrcClientsBuilder::cond->signal_count);
+    EXPECT_EQ(expected_command_queue_size,
+              FakeDataSrcClientsBuilder::command_queue->size());
+}
+
+TEST(DataSrcClientsMgrTest, start) {
+    // When we create a manager, builder's run() method should be called.
+    FakeDataSrcClientsBuilder::started = false;
+    {
+        TestDataSrcClientsMgr mgr;
+        EXPECT_TRUE(FakeDataSrcClientsBuilder::started);
+        EXPECT_TRUE(FakeDataSrcClientsBuilder::command_queue->empty());
+
+        // Check pre-destroy conditions
+        EXPECT_EQ(0, FakeDataSrcClientsBuilder::cond->signal_count);
+        EXPECT_FALSE(FakeDataSrcClientsBuilder::thread_waited);
+    } // mgr and builder have been destroyed by this point.
+
+    // We stopped the manager implicitly (without shutdown()).  The manager
+    // will internally notify it
+    shutdownCheck();
+}
+
+TEST(DataSrcClientsMgrTest, shutdownWithUncaughtException) {
+    // Emulating the case when the builder exists on exception.  shutdown()
+    // will encounter UncaughtException exception and catch it.
+    EXPECT_NO_THROW({
+            TestDataSrcClientsMgr mgr;
+            FakeDataSrcClientsBuilder::thread_throw_on_wait =
+                FakeDataSrcClientsBuilder::THROW_UNCAUGHT_EX;
+        });
+}
+
+TEST(DataSrcClientsMgrTest, shutdownWithException) {
+    EXPECT_NO_THROW({
+            TestDataSrcClientsMgr mgr;
+            FakeDataSrcClientsBuilder::thread_throw_on_wait =
+                FakeDataSrcClientsBuilder::THROW_OTHER;
+        });
+}
+
+TEST(DataSrcClientsMgrTest, reconfigure) {
+    TestDataSrcClientsMgr mgr;
+
+    // Check pre-command condition
+    checkSharedMembers(0, 0, 0, 0, 0, 0);
+
+    // A valid reconfigure argument
+    ConstElementPtr reconfigure_arg = Element::fromJSON(
+        "{""\"IN\": [{\"type\": \"MasterFiles\", \"params\": {},"
+        "             \"cache-enable\": true}]}");
+
+    // On reconfigure(), it just send the RECONFIGURE command to the builder
+    // thread with the given argument intact.
+    mgr.reconfigure(reconfigure_arg);
+
+    // The manager should have acquired the queue lock, send RECONFIGURE
+    // command with the arg, wake up the builder thread by signal.  It doesn't
+    // touch or refer to the map, so it shouldn't acquire the map lock.
+    checkSharedMembers(1, 1, 0, 0, 1, 1);
+    const Command& cmd1 = FakeDataSrcClientsBuilder::command_queue->front();
+    EXPECT_EQ(RECONFIGURE, cmd1.first);
+    EXPECT_EQ(reconfigure_arg, cmd1.second);
+
+    // Non-null, but semantically invalid argument.  The manager doesn't do
+    // this check, so it should result in the same effect.
+    FakeDataSrcClientsBuilder::command_queue->clear();
+    reconfigure_arg = isc::data::Element::create("{ \"foo\": \"bar\" }");
+    mgr.reconfigure(reconfigure_arg);
+    checkSharedMembers(2, 2, 0, 0, 2, 1);
+    const Command& cmd2 = FakeDataSrcClientsBuilder::command_queue->front();
+    EXPECT_EQ(RECONFIGURE, cmd2.first);
+    EXPECT_EQ(reconfigure_arg, cmd2.second);
+
+    // Passing NULL argument is immediately rejected
+    EXPECT_THROW(mgr.reconfigure(ConstElementPtr()), isc::InvalidParameter);
+    checkSharedMembers(2, 2, 0, 0, 2, 1); // no state change
+}
+
+TEST(DataSrcClientsMgrTest, holder) {
+    TestDataSrcClientsMgr mgr;
+
+    {
+        // Initially it's empty, so findClientList() will always return NULL
+        TestDataSrcClientsMgr::Holder holder(mgr);
+        EXPECT_FALSE(holder.findClientList(RRClass::IN()));
+        EXPECT_FALSE(holder.findClientList(RRClass::CH()));
+        // map should be protected here
+        EXPECT_EQ(1, FakeDataSrcClientsBuilder::map_mutex->lock_count);
+        EXPECT_EQ(0, FakeDataSrcClientsBuilder::map_mutex->unlock_count);
+    }
+    // map lock has been released
+    EXPECT_EQ(1, FakeDataSrcClientsBuilder::map_mutex->unlock_count);
+
+    // Put something in, that should become visible.
+    ConstElementPtr reconfigure_arg = Element::fromJSON(
+        "{\"IN\": [{\"type\": \"MasterFiles\", \"params\": {},"
+        "           \"cache-enable\": true}],"
+        " \"CH\": [{\"type\": \"MasterFiles\", \"params\": {},"
+        "           \"cache-enable\": true}]}");
+    mgr.reconfigure(reconfigure_arg);
+    {
+        TestDataSrcClientsMgr::Holder holder(mgr);
+        EXPECT_TRUE(holder.findClientList(RRClass::IN()));
+        EXPECT_TRUE(holder.findClientList(RRClass::CH()));
+    }
+    // We need to clear command queue by hand
+    FakeDataSrcClientsBuilder::command_queue->clear();
+
+    // Replace the lists with new lists containing only one list.
+    // The CH will disappear again.
+    reconfigure_arg = Element::fromJSON(
+        "{\"IN\": [{\"type\": \"MasterFiles\", \"params\": {},"
+        "           \"cache-enable\": true}]}");
+    mgr.reconfigure(reconfigure_arg);
+    {
+        TestDataSrcClientsMgr::Holder holder(mgr);
+        EXPECT_TRUE(holder.findClientList(RRClass::IN()));
+        EXPECT_FALSE(holder.findClientList(RRClass::CH()));
+    }
+
+    // Duplicate lock acquisition is prohibited (only test mgr can detect
+    // this reliably, so this test may not be that useful)
+    TestDataSrcClientsMgr::Holder holder1(mgr);
+    EXPECT_THROW(TestDataSrcClientsMgr::Holder holder2(mgr), isc::Unexpected);
+}
+
+TEST(DataSrcClientsMgrTest, realThread) {
+    // Using the non-test definition with a real thread.  Just checking
+    // no disruption happens.
+    DataSrcClientsMgr mgr;
+}
+
+} // unnamed namespace

src/bin/auth/tests/datasrc_config_unittest.cc

@@ -16,7 +16,6 @@
 
 #include <config/tests/fake_session.h>
 #include <config/ccsession.h>
-#include <util/threads/sync.h>
 
 #include <gtest/gtest.h>
 
@@ -78,12 +77,8 @@ datasrcConfigHandler(DatasrcConfigTest* fake_server, const std::string&,
 
 class DatasrcConfigTest : public ::testing::Test {
 public:
-    // These pretend to be the server
-    isc::util::thread::Mutex& getDataSrcClientListMutex() const {
-        return (mutex_);
-    }
-    void swapDataSrcClientLists(shared_ptr<std::map<dns::RRClass, ListPtr> >
-                                new_lists)
+    void setDataSrcClientLists(shared_ptr<std::map<dns::RRClass, ListPtr> >
+                               new_lists)
     {
         lists_.clear();         // first empty it
 
@@ -156,7 +151,6 @@ protected:
     const string specfile;
     std::map<RRClass, ListPtr> lists_;
     string log_;
-    mutable isc::util::thread::Mutex mutex_;
 };
 
 void
@@ -167,7 +161,7 @@ testConfigureDataSource(DatasrcConfigTest& test,
     // possible to easily look that they were called.
     shared_ptr<std::map<dns::RRClass, ListPtr> > lists =
         configureDataSourceGeneric<FakeList>(config);
-    test.swapDataSrcClientLists(lists);
+    test.setDataSrcClientLists(lists);
 }
 
 // Push there a configuration with a single list.

src/bin/auth/tests/test_datasrc_clients_mgr.cc

@@ -0,0 +1,93 @@
+// Copyright (C) 2012  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 <exceptions/exceptions.h>
+#include <auth/datasrc_config.h>
+
+#include "test_datasrc_clients_mgr.h"
+
+#include <cassert>
+
+namespace isc {
+namespace auth {
+namespace datasrc_clientmgr_internal {
+
+// Define static DataSrcClientsBuilder member variables.
+bool FakeDataSrcClientsBuilder::started = false;
+std::list<Command>* FakeDataSrcClientsBuilder::command_queue = NULL;
+std::list<Command> FakeDataSrcClientsBuilder::command_queue_copy;
+TestCondVar* FakeDataSrcClientsBuilder::cond = NULL;
+TestCondVar FakeDataSrcClientsBuilder::cond_copy;
+TestMutex* FakeDataSrcClientsBuilder::queue_mutex = NULL;
+isc::datasrc::ClientListMapPtr*
+    FakeDataSrcClientsBuilder::clients_map = NULL;
+TestMutex* FakeDataSrcClientsBuilder::map_mutex = NULL;
+TestMutex FakeDataSrcClientsBuilder::queue_mutex_copy;
+bool FakeDataSrcClientsBuilder::thread_waited = false;
+FakeDataSrcClientsBuilder::ExceptionFromWait
+FakeDataSrcClientsBuilder::thread_throw_on_wait =
+    FakeDataSrcClientsBuilder::NOTHROW;
+
+template<>
+void
+TestDataSrcClientsBuilder::doNoop() {
+    ++queue_mutex_->noop_count;
+    switch (queue_mutex_->throw_from_noop) {
+    case TestMutex::NONE:
+        break;                  // no throw
+    case TestMutex::EXCLASS:
+        isc_throw(Exception, "test exception");
+    case TestMutex::INTEGER:
+        throw 42;
+    }
+}
+} // namespace datasrc_clientmgr_internal
+
+template<>
+void
+TestDataSrcClientsMgr::cleanup() {
+    using namespace datasrc_clientmgr_internal;
+    // Make copy of some of the manager's member variables and reset the
+    // corresponding pointers.  The currently pointed objects are in the
+    // manager object, which are going to be invalidated.
+
+    FakeDataSrcClientsBuilder::command_queue_copy = command_queue_;
+    FakeDataSrcClientsBuilder::command_queue =
+        &FakeDataSrcClientsBuilder::command_queue_copy;
+    FakeDataSrcClientsBuilder::queue_mutex_copy = queue_mutex_;
+    FakeDataSrcClientsBuilder::queue_mutex =
+        &FakeDataSrcClientsBuilder::queue_mutex_copy;
+    FakeDataSrcClientsBuilder::cond_copy = cond_;
+    FakeDataSrcClientsBuilder::cond =
+        &FakeDataSrcClientsBuilder::cond_copy;
+}
+
+template<>
+void
+TestDataSrcClientsMgr::reconfigureHook() {
+    using namespace datasrc_clientmgr_internal;
+
+    // Simply replace the local map, ignoring bogus config value.
+    assert(command_queue_.front().first == RECONFIGURE);
+    try {
+        clients_map_ = configureDataSource(command_queue_.front().second);
+    } catch (...) {}
+}
+
+} // namespace auth
+} // namespace isc
+
+// Local Variables:
+// mode: c++
+// End:

src/bin/auth/tests/test_datasrc_clients_mgr.h

@@ -0,0 +1,222 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef TEST_DATASRC_CLIENTS_MGR_H
+#define TEST_DATASRC_CLIENTS_MGR_H 1
+
+#include <exceptions/exceptions.h>
+
+#include <auth/datasrc_clients_mgr.h>
+#include <datasrc/datasrc_config.h>
+
+#include <boost/function.hpp>
+
+#include <list>
+
+// In this file we provide specialization of thread, mutex, condition variable,
+// and DataSrcClientsBuilder for convenience of tests.  They don't use
+// actual threads or mutex, and allow tests to inspect some internal states
+// of the corresponding objects.
+//
+// In many cases, tests can use TestDataSrcClientsMgr (defined below) where
+// DataSrcClientsMgr is needed.
+
+// Below we extend the isc::auth::datasrc_clientmgr_internal namespace to
+// specialize the doNoop() method.
+namespace isc {
+namespace auth {
+namespace datasrc_clientmgr_internal {
+class TestMutex {
+public:
+    // for throw_from_noop.
+    // None: no throw from specialized doNoop()
+    // EXCLASS: throw some exception class object
+    // INTEGER: throw an integer
+    enum ExceptionFromNoop { NONE, EXCLASS, INTEGER };
+
+    TestMutex() : lock_count(0), unlock_count(0), noop_count(0),
+                  throw_from_noop(NONE)
+    {}
+    class Locker {
+    public:
+        Locker(TestMutex& mutex) : mutex_(mutex) {
+            if (mutex.lock_count != mutex.unlock_count) {
+                isc_throw(Unexpected,
+                          "attempt of duplicate lock acquisition");
+            }
+
+            ++mutex.lock_count;
+            if (mutex.lock_count > 100) { // 100 is an arbitrary choice
+                isc_throw(Unexpected,
+                          "too many test mutex count, likely a bug in test");
+            }
+        }
+        ~Locker() {
+            ++mutex_.unlock_count;
+        }
+    private:
+        TestMutex& mutex_;
+    };
+    size_t lock_count; // number of lock acquisitions; tests can check this
+    size_t unlock_count; // number of lock releases; tests can check this
+    size_t noop_count;          // allow doNoop() to modify this
+    ExceptionFromNoop throw_from_noop; // tests can set this to control doNoop
+};
+
+class TestCondVar {
+public:
+    TestCondVar() : wait_count(0), signal_count(0), command_queue_(NULL),
+                    delayed_command_queue_(NULL)
+    {}
+    TestCondVar(std::list<Command>& command_queue,
+                std::list<Command>& delayed_command_queue) :
+        wait_count(0),
+        signal_count(0),
+        command_queue_(&command_queue),
+        delayed_command_queue_(&delayed_command_queue)
+    {
+    }
+    void wait(TestMutex& mutex) {
+        // bookkeeping
+        ++mutex.unlock_count;
+        ++wait_count;
+        ++mutex.lock_count;
+
+        if (wait_count > 100) { // 100 is an arbitrary choice
+            isc_throw(Unexpected,
+                      "too many cond wait count, likely a bug in test");
+        }
+
+        // make the delayed commands available
+        command_queue_->splice(command_queue_->end(), *delayed_command_queue_);
+    }
+    void signal() {
+        ++signal_count;
+    }
+    size_t wait_count; // number of calls to wait(); tests can check this
+    size_t signal_count; // number of calls to signal(); tests can check this
+private:
+    std::list<Command>* command_queue_;
+    std::list<Command>* delayed_command_queue_;
+};
+
+// Convenient shortcut
+typedef DataSrcClientsBuilderBase<TestMutex, TestCondVar>
+TestDataSrcClientsBuilder;
+
+// We specialize this command handler for the convenience of tests.
+// It abuses our specialized Mutex to count the number of calls of this method.
+template<>
+void
+TestDataSrcClientsBuilder::doNoop();
+
+// A specialization of DataSrcClientsBuilder that allows tests to inspect
+// its internal states via static class variables.  Using static is suboptimal,
+// but DataSrcClientsMgr is highly encapsulated, this seems to be the best
+// possible compromise.
+class FakeDataSrcClientsBuilder {
+public:
+    // true iff a builder has started.
+    static bool started;
+
+    // These three correspond to the resource shared with the manager.
+    // xxx_copy will be set in the manager's destructor to record the
+    // final state of the manager.
+    static std::list<Command>* command_queue;
+    static TestCondVar* cond;
+    static TestMutex* queue_mutex;
+    static isc::datasrc::ClientListMapPtr* clients_map;
+    static TestMutex* map_mutex;
+    static std::list<Command> command_queue_copy;
+    static TestCondVar cond_copy;
+    static TestMutex queue_mutex_copy;
+
+    // true iff the manager waited on the thread running the builder.
+    static bool thread_waited;
+
+    // If set to true by a test, TestThread::wait() throws an exception
+    // exception.
+    enum ExceptionFromWait { NOTHROW, THROW_UNCAUGHT_EX, THROW_OTHER };
+    static ExceptionFromWait thread_throw_on_wait;
+
+    FakeDataSrcClientsBuilder(
+        std::list<Command>* command_queue,
+        TestCondVar* cond,
+        TestMutex* queue_mutex,
+        isc::datasrc::ClientListMapPtr* clients_map,
+        TestMutex* map_mutex)
+    {
+        FakeDataSrcClientsBuilder::started = false;
+        FakeDataSrcClientsBuilder::command_queue = command_queue;
+        FakeDataSrcClientsBuilder::cond = cond;
+        FakeDataSrcClientsBuilder::queue_mutex = queue_mutex;
+        FakeDataSrcClientsBuilder::clients_map = clients_map;
+        FakeDataSrcClientsBuilder::map_mutex = map_mutex;
+        FakeDataSrcClientsBuilder::thread_waited = false;
+        FakeDataSrcClientsBuilder::thread_throw_on_wait = NOTHROW;
+    }
+    void run() {
+        FakeDataSrcClientsBuilder::started = true;
+    }
+};
+
+// A fake thread class that doesn't really invoke thread but simply calls
+// the given main function (synchronously).  Tests can tweak the wait()
+// behavior via some static variables so it will throw some exceptions.
+class TestThread {
+public:
+    TestThread(const boost::function<void()>& main) {
+        main();
+    }
+    void wait() {
+        FakeDataSrcClientsBuilder::thread_waited = true;
+        switch (FakeDataSrcClientsBuilder::thread_throw_on_wait) {
+        case FakeDataSrcClientsBuilder::NOTHROW:
+            break;
+        case FakeDataSrcClientsBuilder::THROW_UNCAUGHT_EX:
+            isc_throw(util::thread::Thread::UncaughtException,
+                      "TestThread wait() saw an exception");
+        case FakeDataSrcClientsBuilder::THROW_OTHER:
+            isc_throw(Unexpected,
+                      "General emulated failure in TestThread wait()");
+        }
+    }
+};
+} // namespace datasrc_clientmgr_internal
+
+// Convenient shortcut
+typedef DataSrcClientsMgrBase<
+    datasrc_clientmgr_internal::TestThread,
+    datasrc_clientmgr_internal::FakeDataSrcClientsBuilder,
+    datasrc_clientmgr_internal::TestMutex,
+    datasrc_clientmgr_internal::TestCondVar> TestDataSrcClientsMgr;
+
+// A specialization of manager's "cleanup" called at the end of the
+// destructor.  We use this to record the final values of some of the class
+// member variables.
+template<>
+void
+TestDataSrcClientsMgr::cleanup();
+
+template<>
+void
+TestDataSrcClientsMgr::reconfigureHook();
+} // namespace auth
+} // namespace isc
+
+#endif  // TEST_DATASRC_CLIENTS_MGR_H
+
+// Local Variables:
+// mode: c++
+// End:

src/bin/cfgmgr/Makefile.am

@@ -1,4 +1,4 @@
-SUBDIRS = . plugins tests
+SUBDIRS = . plugins local_plugins tests
 
 pkglibexecdir = $(libexecdir)/@PACKAGE@
 

src/bin/cfgmgr/local_plugins/Makefile.am

@@ -0,0 +1,11 @@
+# Nothing is installed from this directory.  This local_plugins
+# directory overrides the plugins directory when lettuce is run, and the
+# spec file here is used to serve the static zone from the source tree
+# for testing (instead of installation prefix).
+
+noinst_DATA = datasrc.spec
+
+datasrc.spec: ../plugins/datasrc.spec.pre
+	$(SED) -e "s|@@STATIC_ZONE_FILE@@|$(abs_top_builddir)/src/lib/datasrc/static.zone|;s|@@SQLITE3_DATABASE_FILE@@|$(abs_top_builddir)/local.zone.sqlite3|" ../plugins/datasrc.spec.pre >$@
+
+CLEANFILES = datasrc.spec

src/bin/cfgmgr/plugins/Makefile.am

@@ -3,7 +3,7 @@ SUBDIRS = tests
 EXTRA_DIST = README logging.spec tsig_keys.spec
 
 datasrc.spec: datasrc.spec.pre
-	$(SED) -e "s|@@PKGDATADIR@@|$(pkgdatadir)|;s|@@LOCALSTATEDIR@@|$(localstatedir)|" datasrc.spec.pre >$@
+	$(SED) -e "s|@@STATIC_ZONE_FILE@@|$(pkgdatadir)/static.zone|;s|@@SQLITE3_DATABASE_FILE@@|$(localstatedir)/$(PACKAGE)/zone.sqlite3|" datasrc.spec.pre >$@
 
 config_plugindir = @prefix@/share/@PACKAGE@/config_plugins
 config_plugin_DATA = logging.spec tsig_keys.spec datasrc.spec

src/bin/cfgmgr/plugins/datasrc.spec.pre.in

@@ -12,7 +12,7 @@
                         {
                             "type": "sqlite3",
                             "params": {
-                                "database_file": "@@LOCALSTATEDIR@@/@PACKAGE@/zone.sqlite3"
+                                "database_file": "@@SQLITE3_DATABASE_FILE@@"
                             }
                         }
                     ],
@@ -20,7 +20,7 @@
                         {
                             "type": "static",
                             "cache-enable": false,
-                            "params": "@@PKGDATADIR@@/static.zone"
+                            "params": "@@STATIC_ZONE_FILE@@"
                         }
                     ]
                 },

src/bin/dbutil/tests/dbutil_test.sh.in

@@ -161,7 +161,7 @@ get_schema() {
 # @param $2 Expected backup file
 upgrade_ok_test() {
     copy_file $1 $tempfile
-    ../run_dbutil.sh --upgrade --noconfirm $tempfile
+    ${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile
     if [ $? -eq 0 ]
     then
         # Compare schema with the reference
@@ -199,7 +199,7 @@ upgrade_ok_test() {
 # @param $2 Expected backup file
 upgrade_fail_test() {
     copy_file $1 $tempfile
-    ../run_dbutil.sh --upgrade --noconfirm $tempfile
+    ${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile
     failzero $?
     check_backup $1 $backupfile
 }
@@ -222,7 +222,7 @@ record_count_test() {
     records_count=`sqlite3 $tempfile 'select count(*) from records'`
     zones_count=`sqlite3 $tempfile 'select count(*) from zones'`
 
-    ../run_dbutil.sh --upgrade --noconfirm $tempfile
+    ${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile
     if [ $? -ne 0 ]
     then
         # Reason for failure should already have been output
@@ -268,12 +268,12 @@ record_count_test() {
 # @param $2 Expected version string
 check_version() {
     copy_file $1 $verfile
-    ../run_dbutil.sh --check $verfile
+    ${SHELL} ../run_dbutil.sh --check $verfile
     if [ $? -gt 2 ]
     then
         fail "version check failed on database $1; return code $?"
     else
-        ../run_dbutil.sh --check $verfile 2>&1 | grep "$2" > /dev/null
+        ${SHELL} ../run_dbutil.sh --check $verfile 2>&1 | grep "$2" > /dev/null
         if [ $? -ne 0 ]
         then
             fail "database $1 not at expected version $2 (output: $?)"
@@ -293,7 +293,7 @@ check_version() {
 # @param $2 Backup file
 check_version_fail() {
     copy_file $1 $verfile
-    ../run_dbutil.sh --check $verfile
+    ${SHELL} ../run_dbutil.sh --check $verfile
     failzero $?
     check_no_backup $tempfile $backupfile
 }
@@ -305,12 +305,12 @@ rm -f $tempfile $backupfile
 
 # Test 1 - check that the utility fails if the database does not exist
 echo "1.1. Non-existent database - check"
-../run_dbutil.sh --check $tempfile
+${SHELL} ../run_dbutil.sh --check $tempfile
 failzero $?
 check_no_backup $tempfile $backupfile
 
 echo "1.2. Non-existent database - upgrade"
-../run_dbutil.sh --upgrade --noconfirm $tempfile
+${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile
 failzero $?
 check_no_backup $tempfile $backupfile
 rm -f $tempfile $backupfile
@@ -324,7 +324,7 @@ rm -f $tempfile $backupfile
 
 echo "2.2. Database is an empty file - upgrade"
 touch $tempfile
-../run_dbutil.sh --upgrade --noconfirm $tempfile
+${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile
 failzero $?
 # A backup is performed before anything else, so the backup should exist.
 check_backup $tempfile $backupfile
@@ -338,7 +338,7 @@ rm -f $tempfile $backupfile
 
 echo "3.2. Database is not an SQLite file - upgrade"
 echo "This is not an sqlite3 database" > $tempfile
-../run_dbutil.sh --upgrade --noconfirm $tempfile
+${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile
 failzero $?
 # ...and as before, a backup should have been created
 check_backup $tempfile $backupfile
@@ -421,31 +421,31 @@ rm -f $tempfile $backupfile ${backupfile}-1 ${backupfile}-2
 
 echo "13.1 Command-line errors"
 copy_file $testdata/old_v1.sqlite3 $tempfile
-../run_dbutil.sh $tempfile
+${SHELL} ../run_dbutil.sh $tempfile
 failzero $?
-../run_dbutil.sh --upgrade --check $tempfile
+${SHELL} ../run_dbutil.sh --upgrade --check $tempfile
 failzero $?
-../run_dbutil.sh --noconfirm --check $tempfile
+${SHELL} ../run_dbutil.sh --noconfirm --check $tempfile
 failzero $?
-../run_dbutil.sh --check
+${SHELL} ../run_dbutil.sh --check
 failzero $?
-../run_dbutil.sh --upgrade --noconfirm
+${SHELL} ../run_dbutil.sh --upgrade --noconfirm
 failzero $?
-../run_dbutil.sh --check $tempfile $backupfile
+${SHELL} ../run_dbutil.sh --check $tempfile $backupfile
 failzero $?
-../run_dbutil.sh --upgrade --noconfirm $tempfile $backupfile
+${SHELL} ../run_dbutil.sh --upgrade --noconfirm $tempfile $backupfile
 failzero $?
 rm -f $tempfile $backupfile
 
 echo "13.2 verbose flag"
 copy_file $testdata/old_v1.sqlite3 $tempfile
-../run_dbutil.sh --upgrade --noconfirm --verbose $tempfile
+${SHELL} ../run_dbutil.sh --upgrade --noconfirm --verbose $tempfile
 passzero $?
 rm -f $tempfile $backupfile
 
 echo "13.3 Interactive prompt - yes"
 copy_file $testdata/old_v1.sqlite3 $tempfile
-../run_dbutil.sh --upgrade $tempfile << .
+${SHELL} ../run_dbutil.sh --upgrade $tempfile << .
 Yes
 .
 passzero $?
@@ -454,7 +454,7 @@ rm -f $tempfile $backupfile
 
 echo "13.4 Interactive prompt - no"
 copy_file $testdata/old_v1.sqlite3 $tempfile
-../run_dbutil.sh --upgrade $tempfile << .
+${SHELL} ../run_dbutil.sh --upgrade $tempfile << .
 no
 .
 passzero $?
@@ -464,7 +464,7 @@ rm -f $tempfile $backupfile
 
 echo "13.5 quiet flag"
 copy_file $testdata/old_v1.sqlite3 $tempfile
-../run_dbutil.sh --check --quiet $tempfile 2>&1 | grep .
+${SHELL} ../run_dbutil.sh --check --quiet $tempfile 2>&1 | grep .
 failzero $?
 rm -f $tempfile $backupfile
 

src/bin/resolver/resolver_messages.mes

@@ -108,7 +108,7 @@ This error is issued when a resolver configuration update has specified
 a negative retry count: only zero or positive values are valid.  The
 configuration update was abandoned and the parameters were not changed.
 
-% RESOLVER_NON_IN_PACKET non-IN class request received, returning REFUSED message
+% RESOLVER_NON_IN_PACKET non-IN class (%1) request received, returning REFUSED message
 This debug message is issued when resolver has received a DNS packet that
 was not IN (Internet) class.  The resolver cannot handle such packets,
 so is returning a REFUSED response to the sender.

src/lib/datasrc/client_list.h

@@ -37,7 +37,6 @@ typedef boost::shared_ptr<DataSourceClient> DataSourceClientPtr;
 class DataSourceClientContainer;
 typedef boost::shared_ptr<DataSourceClientContainer>
     DataSourceClientContainerPtr;
-
 // XXX: it's better to even hide the existence of the "memory" namespace.
 // We should probably consider pimpl for details of ConfigurableClientList
 // and hide real definitions except for itself and tests.
@@ -391,6 +390,11 @@ protected:
     DataSources data_sources_;
 };
 
+/// \brief Shortcut typedef for maps of client_lists.
+typedef boost::shared_ptr<std::map<
+    isc::dns::RRClass, boost::shared_ptr<ConfigurableClientList> > >
+        ClientListMapPtr;
+
 } // namespace datasrc
 } // namespace isc
 

src/lib/datasrc/memory/Makefile.am

@@ -24,6 +24,9 @@ libdatasrc_memory_la_SOURCES += zone_table_segment_local.h zone_table_segment_lo
 libdatasrc_memory_la_SOURCES += zone_data_updater.h zone_data_updater.cc
 libdatasrc_memory_la_SOURCES += zone_data_loader.h zone_data_loader.cc
 libdatasrc_memory_la_SOURCES += memory_client.h memory_client.cc
+libdatasrc_memory_la_SOURCES += zone_writer.h
+libdatasrc_memory_la_SOURCES += zone_writer_local.h zone_writer_local.cc
+libdatasrc_memory_la_SOURCES += load_action.h
 
 nodist_libdatasrc_memory_la_SOURCES = memory_messages.h memory_messages.cc
 

src/lib/datasrc/memory/load_action.h

@@ -0,0 +1,47 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef LOAD_ACTION_H
+#define LOAD_ACTION_H
+
+#include <boost/function.hpp>
+
+namespace isc {
+// Forward declarations
+namespace util{
+class MemorySegment;
+}
+namespace datasrc {
+namespace memory {
+class ZoneData;
+
+/// \brief Callback to load data into the memory
+///
+/// This is called from the ZoneWriter whenever there's need to load the
+/// zone data. The callback should allocate new ZoneData and fill it with
+/// the zone content. It is up to the callback to know where or how to
+/// load the data, or even the origin and class of the zone (it is assumed
+/// the callback will be some kind of functor).
+///
+/// All data should be allocated from the passed MemorySegment. The ownership
+/// is passed onto the caller.
+///
+/// It must not return NULL.
+typedef boost::function<ZoneData*(util::MemorySegment&)> LoadAction;
+
+}
+}
+}
+
+#endif

src/lib/datasrc/memory/zone_table_segment.h

@@ -16,6 +16,7 @@
 #define __ZONE_TABLE_SEGMENT_H__
 
 #include <datasrc/memory/zone_table.h>
+#include "load_action.h"
 #include <cc/data.h>
 #include <util/memory_segment.h>
 
@@ -24,8 +25,14 @@
 #include <stdlib.h>
 
 namespace isc {
+// Some forward declarations
+namespace dns {
+class Name;
+class RRClass;
+}
 namespace datasrc {
 namespace memory {
+class ZoneWriter;
 
 /// \brief Memory-management independent entry point that contains a
 /// pointer to a zone table in memory.
@@ -45,6 +52,26 @@ public:
         return (table.get());
     }
 
+    /// \brief Method to set the internal table
+    ///
+    /// The interface is tentative, we don't know if this is the correct place
+    /// and way to set the data. But for now, we need something to be there
+    /// at least for the tests. So we have this. For this reason, there are
+    /// no tests for this method directly. Do not use in actual
+    /// implementation.
+    ///
+    /// It can be used only once, to initially set it. It can't replace the
+    /// one already there.
+    ///
+    /// \param table Pointer to the table to use.
+    /// \throw isc::Unexpected if called the second time.
+    void setTable(ZoneTable* table) {
+        if (this->table.get() != NULL) {
+            isc_throw(isc::Unexpected, "Replacing table");
+        }
+        this->table = table;
+    }
+
 private:
     boost::interprocess::offset_ptr<ZoneTable> table;
 };
@@ -101,6 +128,21 @@ public:
     ///
     /// \param segment The segment to destroy.
     static void destroy(ZoneTableSegment* segment);
+
+    /// \brief Create a zone write corresponding to this segment
+    ///
+    /// This creates a new write that can be used to update zones
+    /// inside this zone table segment.
+    ///
+    /// \param loadAction Callback to provide the actual data.
+    /// \param origin The origin of the zone to reload.
+    /// \param rrclass The class of the zone to reload.
+    /// \return New instance of a zone writer. The ownership is passed
+    ///     onto the caller and the caller needs to \c delete it when
+    ///     it's done with the writer.
+    virtual ZoneWriter* getZoneWriter(const LoadAction& load_action,
+                                      const dns::Name& origin,
+                                      const dns::RRClass& rrclass) = 0;
 };
 
 } // namespace memory

src/lib/datasrc/memory/zone_table_segment_local.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <datasrc/memory/zone_table_segment_local.h>
+#include "zone_writer_local.h"
 
 using namespace isc::util;
 
@@ -38,6 +39,14 @@ ZoneTableSegmentLocal::getMemorySegment() {
      return (mem_sgmt_);
 }
 
+ZoneWriter*
+ZoneTableSegmentLocal::getZoneWriter(const LoadAction& load_action,
+                                     const dns::Name& name,
+                                     const dns::RRClass& rrclass)
+{
+    return (new ZoneWriterLocal(this, load_action, name, rrclass));
+}
+
 } // namespace memory
 } // namespace datasrc
 } // namespace isc

src/lib/datasrc/memory/zone_table_segment_local.h

@@ -54,6 +54,10 @@ public:
     /// implementation (a MemorySegmentLocal instance).
     virtual isc::util::MemorySegment& getMemorySegment();
 
+    /// \brief Concrete implementation of ZoneTableSegment::getZoneWriter
+    virtual ZoneWriter* getZoneWriter(const LoadAction& load_action,
+                                      const dns::Name& origin,
+                                      const dns::RRClass& rrclass);
 private:
     ZoneTableHeader header_;
     isc::util::MemorySegmentLocal mem_sgmt_;

src/lib/datasrc/memory/zone_writer.h

@@ -0,0 +1,92 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef MEM_ZONE_WRITER_H
+#define MEM_ZONE_WRITER_H
+
+#include "load_action.h"
+
+namespace isc {
+namespace datasrc {
+namespace memory {
+
+/// \brief Does an update to a zone.
+///
+/// This abstract base class represents the work of a reload of a zone.
+/// The work is divided into three stages -- load(), install() and cleanup().
+/// They should be called in this order for the effect to take place.
+///
+/// We divide them so the update of zone data can be done asynchronously,
+/// in a different thread. The install() operation is the only one that needs
+/// to be done in a critical section.
+///
+/// Each derived class implementation must provide the strong exception
+/// guarantee for each public method. That is, when any of the methods
+/// throws, the entire state should stay the same as before the call
+/// (how to achieve that may be implementation dependant).
+class ZoneWriter {
+public:
+    /// \brief Virtual destructor.
+    virtual ~ZoneWriter() {};
+
+    /// \brief Get the zone data into memory.
+    ///
+    /// This is the part that does the time-consuming loading into the memory.
+    /// This can be run in a separate thread, for example. It has no effect on
+    /// the data actually served, it only prepares them for future use.
+    ///
+    /// This is the first method you should call on the object. Never call it
+    /// multiple times.
+    ///
+    /// \note As this contains reading of files or other data sources, or with
+    ///     some other source of the data to load, it may throw quite anything.
+    ///     If it throws, do not call any other methods on the object and
+    ///     discard it.
+    /// \note After successful load(), you have to call cleanup() some time
+    ///     later.
+    /// \throw isc::InvalidOperation if called second time.
+    virtual void load() = 0;
+
+    /// \brief Put the changes to effect.
+    ///
+    /// This replaces the old version of zone with the one previously prepared
+    /// by load(). It takes ownership of the old zone data, if any.
+    ///
+    /// You may call it only after successful load() and at most once.
+    ///
+    /// The operation is expected to be fast and is meant to be used inside
+    /// a critical section.
+    ///
+    /// This may throw in rare cases, depending on the concrete implementation.
+    /// If it throws, you still need to call cleanup().
+    ///
+    /// \throw isc::InvalidOperation if called without previous load() or for
+    ///     the second time or cleanup() was called already.
+    virtual void install() = 0;
+
+    /// \brief Clean up resources.
+    ///
+    /// This releases all resources held by owned zone data. That means the
+    /// one loaded by load() in case install() was not called or was not
+    /// successful, or the one replaced in install().
+    ///
+    /// Generally, this should never throw.
+    virtual void cleanup() = 0;
+};
+
+}
+}
+}
+
+#endif

src/lib/datasrc/memory/zone_writer_local.cc

@@ -0,0 +1,93 @@
+// Copyright (C) 2012  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 "zone_writer_local.h"
+#include "zone_data.h"
+#include "zone_table_segment_local.h"
+
+#include <memory>
+
+using std::auto_ptr;
+
+namespace isc {
+namespace datasrc {
+namespace memory {
+
+ZoneWriterLocal::ZoneWriterLocal(ZoneTableSegmentLocal* segment,
+                                 const LoadAction& load_action,
+                                 const dns::Name& origin,
+                                 const dns::RRClass& rrclass) :
+    segment_(segment),
+    load_action_(load_action),
+    origin_(origin),
+    rrclass_(rrclass),
+    zone_data_(NULL),
+    state_(ZW_UNUSED)
+{}
+
+ZoneWriterLocal::~ZoneWriterLocal() {
+    // Clean up everything there might be left if someone forgot, just
+    // in case.
+    cleanup();
+}
+
+void
+ZoneWriterLocal::load() {
+    if (state_ != ZW_UNUSED) {
+        isc_throw(isc::InvalidOperation, "Trying to load twice");
+    }
+
+    zone_data_ = load_action_(segment_->getMemorySegment());
+
+    if (zone_data_ == NULL) {
+        // Bug inside load_action_.
+        isc_throw(isc::InvalidOperation, "No data returned from load action");
+    }
+
+    state_ = ZW_LOADED;
+}
+
+void
+ZoneWriterLocal::install() {
+    if (state_ != ZW_LOADED) {
+        isc_throw(isc::InvalidOperation, "No data to install");
+    }
+
+
+    ZoneTable* table(segment_->getHeader().getTable());
+    if (table == NULL) {
+        isc_throw(isc::Unexpected, "No zone table present");
+    }
+    const ZoneTable::AddResult result(table->addZone(
+                                          segment_->getMemorySegment(),
+                                          rrclass_, origin_, zone_data_));
+
+    state_ = ZW_INSTALLED;
+    zone_data_ = result.zone_data;
+}
+
+void
+ZoneWriterLocal::cleanup() {
+    // We eat the data (if any) now.
+
+    if (zone_data_ != NULL) {
+        ZoneData::destroy(segment_->getMemorySegment(), zone_data_, rrclass_);
+        zone_data_ = NULL;
+        state_ = ZW_CLEANED;
+    }
+}
+
+}
+}
+}

src/lib/datasrc/memory/zone_writer_local.h

@@ -0,0 +1,95 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef MEM_ZONE_WRITER_LOCAL_H
+#define MEM_ZONE_WRITER_LOCAL_H
+
+#include "zone_writer.h"
+
+#include <dns/rrclass.h>
+#include <dns/name.h>
+
+namespace isc {
+namespace datasrc {
+namespace memory {
+
+class ZoneData;
+class ZoneTableSegmentLocal;
+
+/// \brief Writer implementation which loads data locally.
+///
+/// This implementation prepares a clean zone data and lets one callback
+/// to fill it and another to install it somewhere. The class does mostly
+/// nothing (and delegates the work to the callbacks), just stores little bit
+/// of state between the calls.
+class ZoneWriterLocal : public ZoneWriter {
+public:
+    /// \brief Constructor
+    ///
+    /// \param segment The zone table segment to store the zone into.
+    /// \param load_action The callback used to load data.
+    /// \param install_action The callback used to install the loaded zone.
+    /// \param rrclass The class of the zone.
+    ZoneWriterLocal(ZoneTableSegmentLocal* segment,
+                    const LoadAction& load_action, const dns::Name& name,
+                    const dns::RRClass& rrclass);
+
+    /// \brief Destructor
+    ~ZoneWriterLocal();
+
+    /// \brief Loads the data.
+    ///
+    /// This calls the load_action (passed to constructor) and stores the
+    /// data for future use.
+    ///
+    /// \throw isc::InvalidOperation if it is called the second time in
+    ///     lifetime of the object.
+    /// \throw Whatever the load_action throws, it is propagated up.
+    virtual void load();
+
+    /// \brief Installs the zone.
+    ///
+    /// It modifies the zone table accessible through the segment (passed to
+    /// constructor).
+    ///
+    /// \throw isc::InvalidOperation if it is called the second time in
+    ///     lifetime of the object or if load() was not called previously or if
+    ///     cleanup() was already called.
+    virtual void install();
+
+    /// \brief Clean up memory.
+    ///
+    /// Cleans up the memory used by load()ed zone if not yet installed, or
+    /// the old zone replaced by install().
+    virtual void cleanup();
+private:
+    ZoneTableSegmentLocal* segment_;
+    LoadAction load_action_;
+    dns::Name origin_;
+    dns::RRClass rrclass_;
+    ZoneData* zone_data_;
+    enum State {
+        ZW_UNUSED,
+        ZW_LOADED,
+        ZW_INSTALLED,
+        ZW_CLEANED
+    };
+    State state_;
+};
+
+}
+}
+}
+
+#endif

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

@@ -33,6 +33,7 @@ run_unittests_SOURCES += memory_segment_test.h
 run_unittests_SOURCES += segment_object_holder_unittest.cc
 run_unittests_SOURCES += memory_client_unittest.cc
 run_unittests_SOURCES += zone_table_segment_unittest.cc
+run_unittests_SOURCES += zone_writer_unittest.cc
 
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)

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

@@ -13,8 +13,14 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <datasrc/memory/zone_table_segment.h>
+#include <datasrc/memory/zone_writer_local.h>
 #include <gtest/gtest.h>
 
+#include <boost/scoped_ptr.hpp>
+
+using boost::scoped_ptr;
+using isc::dns::Name;
+using isc::dns::RRClass;
 using namespace isc::datasrc::memory;
 using namespace isc::data;
 using namespace isc::util;
@@ -80,4 +86,22 @@ TEST_F(ZoneTableSegmentTest, getMemorySegment) {
     EXPECT_TRUE(mem_sgmt.allMemoryDeallocated());
 }
 
+ZoneData*
+loadAction(MemorySegment&) {
+    // The function won't be called, so this is OK
+    return (NULL);
+}
+
+// Test we can get a writer.
+TEST_F(ZoneTableSegmentTest, getZoneWriter) {
+    scoped_ptr<ZoneWriter>
+        writer(segment_->getZoneWriter(loadAction, Name("example.org"),
+                                       RRClass::IN()));
+    // We have to get something
+    EXPECT_NE(static_cast<void*>(NULL), writer.get());
+    // And for now, it should be the local writer
+    EXPECT_NE(static_cast<void*>(NULL),
+              dynamic_cast<ZoneWriterLocal*>(writer.get()));
+}
+
 } // anonymous namespace

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

@@ -0,0 +1,249 @@
+// Copyright (C) 2012  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 <datasrc/memory/zone_writer_local.h>
+#include <datasrc/memory/zone_table_segment_local.h>
+#include <datasrc/memory/zone_data.h>
+
+#include <cc/data.h>
+#include <dns/rrclass.h>
+#include <dns/name.h>
+
+#include <gtest/gtest.h>
+
+#include <boost/scoped_ptr.hpp>
+#include <boost/bind.hpp>
+
+using boost::scoped_ptr;
+using boost::bind;
+using isc::dns::RRClass;
+using isc::dns::Name;
+using namespace isc::datasrc::memory;
+
+namespace {
+
+class TestException {};
+
+class ZoneWriterLocalTest : public ::testing::Test {
+public:
+    ZoneWriterLocalTest() :
+        // FIXME: The NullElement probably isn't the best one, but we don't
+        // know how the config will look, so it just fills the argument
+        // (which is currently ignored)
+        segment_(ZoneTableSegment::create(isc::data::NullElement())),
+        writer_(new
+            ZoneWriterLocal(dynamic_cast<ZoneTableSegmentLocal*>(segment_.
+                                                                 get()),
+                            bind(&ZoneWriterLocalTest::loadAction, this, _1),
+                            Name("example.org"), RRClass::IN())),
+        load_called_(false),
+        load_throw_(false),
+        load_null_(false),
+        load_data_(false)
+    {
+        // TODO: The setTable is only a temporary interface
+        segment_->getHeader().
+            setTable(ZoneTable::create(segment_->getMemorySegment(),
+                                       RRClass::IN()));
+    }
+    void TearDown() {
+        // Release the writer
+        writer_.reset();
+        // Release the table we used
+        ZoneTable::destroy(segment_->getMemorySegment(),
+                           segment_->getHeader().getTable(), RRClass::IN());
+        // And check we freed all memory
+        EXPECT_TRUE(segment_->getMemorySegment().allMemoryDeallocated());
+    }
+protected:
+    scoped_ptr<ZoneTableSegment> segment_;
+    scoped_ptr<ZoneWriterLocal> writer_;
+    bool load_called_;
+    bool load_throw_;
+    bool load_null_;
+    bool load_data_;
+private:
+    ZoneData* loadAction(isc::util::MemorySegment& segment) {
+        // Make sure it is the correct segment passed. We know the
+        // exact instance, can compare pointers to them.
+        EXPECT_EQ(&segment_->getMemorySegment(), &segment);
+        // We got called
+        load_called_ = true;
+        if (load_throw_) {
+            throw TestException();
+        }
+
+        if (load_null_) {
+            // Be nasty to the caller and return NULL, which is forbidden
+            return (NULL);
+        }
+        ZoneData* data = ZoneData::create(segment, Name("example.org"));
+        if (load_data_) {
+            // Put something inside. The node itself should be enough for
+            // the tests.
+            ZoneNode* node(NULL);
+            data->insertName(segment, Name("subdomain.example.org"), &node);
+            EXPECT_NE(static_cast<ZoneNode*>(NULL), node);
+        }
+        return (data);
+    }
+};
+
+// We call it the way we are supposed to, check every callback is called in the
+// right moment.
+TEST_F(ZoneWriterLocalTest, correctCall) {
+    // Nothing called before we call it
+    EXPECT_FALSE(load_called_);
+
+    // Just the load gets called now
+    EXPECT_NO_THROW(writer_->load());
+    EXPECT_TRUE(load_called_);
+    load_called_ = false;
+
+    EXPECT_NO_THROW(writer_->install());
+    EXPECT_FALSE(load_called_);
+
+    // We don't check explicitly how this works, but call it to free memory. If
+    // everything is freed should be checked inside the TearDown.
+    EXPECT_NO_THROW(writer_->cleanup());
+}
+
+TEST_F(ZoneWriterLocalTest, loadTwice) {
+    // Load it the first time
+    EXPECT_NO_THROW(writer_->load());
+    EXPECT_TRUE(load_called_);
+    load_called_ = false;
+
+    // The second time, it should not be possible
+    EXPECT_THROW(writer_->load(), isc::InvalidOperation);
+    EXPECT_FALSE(load_called_);
+
+    // The object should not be damaged, try installing and clearing now
+    EXPECT_NO_THROW(writer_->install());
+    EXPECT_FALSE(load_called_);
+
+    // We don't check explicitly how this works, but call it to free memory. If
+    // everything is freed should be checked inside the TearDown.
+    EXPECT_NO_THROW(writer_->cleanup());
+}
+
+// Try loading after call to install and call to cleanup. Both is
+// forbidden.
+TEST_F(ZoneWriterLocalTest, loadLater) {
+    // Load first, so we can install
+    EXPECT_NO_THROW(writer_->load());
+    EXPECT_NO_THROW(writer_->install());
+    // Reset so we see nothing is called now
+    load_called_ = false;
+
+    EXPECT_THROW(writer_->load(), isc::InvalidOperation);
+    EXPECT_FALSE(load_called_);
+
+    // Cleanup and try loading again. Still shouldn't work.
+    EXPECT_NO_THROW(writer_->cleanup());
+
+    EXPECT_THROW(writer_->load(), isc::InvalidOperation);
+    EXPECT_FALSE(load_called_);
+}
+
+// Try calling install at various bad times
+TEST_F(ZoneWriterLocalTest, invalidInstall) {
+    // Nothing loaded yet
+    EXPECT_THROW(writer_->install(), isc::InvalidOperation);
+    EXPECT_FALSE(load_called_);
+
+    EXPECT_NO_THROW(writer_->load());
+    load_called_ = false;
+    // This install is OK
+    EXPECT_NO_THROW(writer_->install());
+    // But we can't call it second time now
+    EXPECT_THROW(writer_->install(), isc::InvalidOperation);
+    EXPECT_FALSE(load_called_);
+}
+
+// We check we can clean without installing first and nothing bad
+// happens. We also misuse the testcase to check we can't install
+// after cleanup.
+TEST_F(ZoneWriterLocalTest, cleanWithoutInstall) {
+    EXPECT_NO_THROW(writer_->load());
+    EXPECT_NO_THROW(writer_->cleanup());
+
+    EXPECT_TRUE(load_called_);
+
+    // We cleaned up, no way to install now
+    EXPECT_THROW(writer_->install(), isc::InvalidOperation);
+}
+
+// Test the case when load callback throws
+TEST_F(ZoneWriterLocalTest, loadThrows) {
+    load_throw_ = true;
+    EXPECT_THROW(writer_->load(), TestException);
+
+    // We can't install now
+    EXPECT_THROW(writer_->install(), isc::InvalidOperation);
+    EXPECT_TRUE(load_called_);
+
+    // But we can cleanup
+    EXPECT_NO_THROW(writer_->cleanup());
+}
+
+// Check the strong exception guarantee - if it throws, nothing happened
+// to the content.
+TEST_F(ZoneWriterLocalTest, retry) {
+    // First attempt fails due to some exception.
+    load_throw_ = true;
+    EXPECT_THROW(writer_->load(), TestException);
+    // This one shall succeed.
+    load_called_ = load_throw_ = false;
+    // We want some data inside.
+    load_data_ = true;
+    EXPECT_NO_THROW(writer_->load());
+    // And this one will fail again. But the old data will survive.
+    load_data_ = false;
+    EXPECT_THROW(writer_->load(), isc::InvalidOperation);
+
+    // The rest still works correctly
+    EXPECT_NO_THROW(writer_->install());
+    ZoneTable* const table(segment_->getHeader().getTable());
+    const ZoneTable::FindResult found(table->findZone(Name("example.org")));
+    ASSERT_EQ(isc::datasrc::result::SUCCESS, found.code);
+    // For some reason it doesn't seem to work by the ZoneNode typedef, using
+    // the full definition instead. At least on some compilers.
+    const isc::datasrc::memory::DomainTreeNode<RdataSet>* node;
+    EXPECT_EQ(isc::datasrc::memory::DomainTree<RdataSet>::EXACTMATCH,
+              found.zone_data->getZoneTree().
+              find(Name("subdomain.example.org"), &node));
+    EXPECT_NO_THROW(writer_->cleanup());
+}
+
+// Check the writer defends itsefl when load action returns NULL
+TEST_F(ZoneWriterLocalTest, loadNull) {
+    load_null_ = true;
+    EXPECT_THROW(writer_->load(), isc::InvalidOperation);
+
+    // We can't install that
+    EXPECT_THROW(writer_->install(), isc::InvalidOperation);
+
+    // It should be possible to clean up safely
+    EXPECT_NO_THROW(writer_->cleanup());
+}
+
+// Check the object cleans up in case we forget it.
+TEST_F(ZoneWriterLocalTest, autoCleanUp) {
+    // Load data and forget about it. It should get released
+    // when the writer itself is destroyed.
+    EXPECT_NO_THROW(writer_->load());
+}
+
+}

src/lib/dhcp/Makefile.am

@@ -22,10 +22,14 @@ libb10_dhcp___la_SOURCES += iface_mgr_linux.cc
 libb10_dhcp___la_SOURCES += iface_mgr_bsd.cc
 libb10_dhcp___la_SOURCES += iface_mgr_sun.cc
 libb10_dhcp___la_SOURCES += option.cc option.h
+libb10_dhcp___la_SOURCES += option_data_types.h
+libb10_dhcp___la_SOURCES += option_definition.cc option_definition.h
 libb10_dhcp___la_SOURCES += option6_ia.cc option6_ia.h
 libb10_dhcp___la_SOURCES += option6_iaaddr.cc option6_iaaddr.h
 libb10_dhcp___la_SOURCES += option6_addrlst.cc option6_addrlst.h
 libb10_dhcp___la_SOURCES += option4_addrlst.cc option4_addrlst.h
+libb10_dhcp___la_SOURCES += option6_int.h
+libb10_dhcp___la_SOURCES += option6_int_array.h
 libb10_dhcp___la_SOURCES += dhcp6.h dhcp4.h
 libb10_dhcp___la_SOURCES += pkt6.cc pkt6.h
 libb10_dhcp___la_SOURCES += pkt4.cc pkt4.h

src/lib/dhcp/option4_addrlst.h

@@ -20,6 +20,7 @@
 #include <vector>
 #include <boost/shared_ptr.hpp>
 #include <boost/shared_array.hpp>
+#include <asiolink/io_address.h>
 #include <util/buffer.h>
 #include <dhcp/option.h>
 

src/lib/dhcp/option6_int.h

@@ -0,0 +1,189 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef OPTION6_INT_H_
+#define OPTION6_INT_H_
+
+#include <dhcp/libdhcp++.h>
+#include <dhcp/option.h>
+#include <dhcp/option_data_types.h>
+#include <util/io_utilities.h>
+
+#include <stdint.h>
+
+namespace isc {
+namespace dhcp {
+
+/// This template class represents DHCPv6 option with single value.
+/// This value is of integer type and can be any of the following:
+/// - uint8_t,
+/// - uint16_t,
+/// - uint32_t,
+/// - int8_t,
+/// - int16_t,
+/// - int32_t.
+///
+/// @param T data field type (see above).
+template<typename T>
+class Option6Int: public Option {
+
+public:
+    /// @brief Constructor.
+    ///
+    /// @param type option type.
+    /// @param value option value.
+    ///
+    /// @throw isc::dhcp::InvalidDataType if data field type provided
+    /// as template parameter is not a supported integer type.
+    Option6Int(uint16_t type, T value)
+        : Option(Option::V6, type), value_(value) {
+        if (!OptionDataTypes<T>::valid) {
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
+        }
+    }
+
+    /// @brief Constructor.
+    ///
+    /// This constructor creates option from a buffer. This construtor
+    /// may throw exception if \ref unpack function throws during buffer
+    /// parsing.
+    ///
+    /// @param type option type.
+    /// @param begin iterator to first byte of option data.
+    /// @param end iterator to end of option data (first byte after option end).
+    ///
+    /// @throw isc::OutOfRange if provided buffer is shorter than data size.
+    /// @throw isc::dhcp::InvalidDataType if data field type provided
+    /// as template parameter is not a supported integer type.
+    Option6Int(uint16_t type, OptionBufferConstIter begin,
+               OptionBufferConstIter end)
+        : Option(Option::V6, type) {
+        if (!OptionDataTypes<T>::valid) {
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
+        }
+        unpack(begin, end);
+    }
+
+    /// Writes option in wire-format to buf, returns pointer to first unused
+    /// byte after stored option.
+    ///
+    /// @param [out] buf buffer (option will be stored here)
+    ///
+    /// @throw isc::dhcp::InvalidDataType if size of a data field type is not
+    /// equal to 1, 2 or 4 bytes. The data type is not checked in this function
+    /// because it is checked in a constructor.
+    void pack(isc::util::OutputBuffer& buf) {
+        buf.writeUint16(type_);
+        buf.writeUint16(len() - OPTION6_HDR_LEN);
+        // Depending on the data type length we use different utility functions
+        // writeUint16 or writeUint32 which write the data in the network byte
+        // order to the provided buffer. The same functions can be safely used
+        // for either unsiged or signed integers so there is not need to create
+        // special cases for intX_t types.
+        switch (OptionDataTypes<T>::len) {
+        case 1:
+            buf.writeUint8(value_);
+            break;
+        case 2:
+            buf.writeUint16(value_);
+            break;
+        case 4:
+            buf.writeUint32(value_);
+            break;
+        default:
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
+        }
+        LibDHCP::packOptions6(buf, options_);
+    }
+
+    /// @brief Parses received buffer
+    ///
+    /// Parses received buffer and returns offset to the first unused byte after
+    /// parsed option.
+    ///
+    /// @param begin iterator to first byte of option data
+    /// @param end iterator to end of option data (first byte after option end)
+    ///
+    /// @throw isc::OutOfRange if provided buffer is shorter than data size.
+    /// @throw isc::dhcp::InvalidDataType if size of a data field type is not
+    /// equal to 1, 2 or 4 bytes. The data type is not checked in this function
+    /// because it is checked in a constructor.
+    virtual void unpack(OptionBufferConstIter begin, OptionBufferConstIter end) {
+        if (distance(begin, end) < sizeof(T)) {
+            isc_throw(OutOfRange, "Option " << getType() << " truncated");
+        }
+        // @todo consider what to do if buffer is longer than data type.
+
+        // Depending on the data type length we use different utility functions
+        // readUint16 or readUint32 which read the data laid in the network byte
+        // order from the provided buffer. The same functions can be safely used
+        // for either unsiged or signed integers so there is not need to create
+        // special cases for intX_t types.
+        int data_size_len = OptionDataTypes<T>::len;
+        switch (data_size_len) {
+        case 1:
+            value_ = *begin;
+            break;
+        case 2:
+            value_ = isc::util::readUint16(&(*begin));
+            break;
+        case 4:
+            value_ = isc::util::readUint32(&(*begin));
+            break;
+        default:
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
+        }
+        // Use local variable to set a new value for this iterator.
+        // When using OptionDataTypes<T>::len directly some versions
+        // of clang complain about unresolved reference to
+        // OptionDataTypes structure during linking.
+        begin += data_size_len;
+        LibDHCP::unpackOptions6(OptionBuffer(begin, end), options_);
+    }
+
+    /// @brief Set option value.
+    ///
+    /// @param value new option value.
+    void setValue(T value) { value_ = value; }
+
+    /// @brief Return option value.
+    ///
+    /// @return option value.
+    T getValue() const { return value_; }
+
+    /// @brief returns complete length of option
+    ///
+    /// Returns length of this option, including option header and suboptions
+    ///
+    /// @return length of this option
+    virtual uint16_t len() {
+        uint16_t length = OPTION6_HDR_LEN + sizeof(T);
+        // length of all suboptions
+        for (Option::OptionCollection::iterator it = options_.begin();
+             it != options_.end();
+             ++it) {
+            length += (*it).second->len();
+        }
+        return (length);
+    }
+
+private:
+
+    T value_;  ///< Value conveyed by the option.
+};
+
+} // isc::dhcp namespace
+} // isc namespace
+
+#endif /* OPTION6_INT_H_ */

src/lib/dhcp/option6_int_array.h

@@ -0,0 +1,228 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef OPTION6_INT_ARRAY_H_
+#define OPTION6_INT_ARRAY_H_
+
+#include <dhcp/libdhcp++.h>
+#include <dhcp/option.h>
+#include <dhcp/option_data_types.h>
+#include <util/io_utilities.h>
+
+#include <stdint.h>
+
+namespace isc {
+namespace dhcp {
+
+/// This template class represents DHCPv6 option with array of
+/// integer values. The type of the elements in the array can be
+/// any of the following:
+/// - uint8_t,
+/// - uint16_t,
+/// - uint32_t,
+/// - int8_t,
+/// - int16_t,
+/// - int32_t.
+///
+/// @warning Since this option may convey variable number of integer
+/// values, sub-options are should not be added in this option as
+/// there is no way to distinguish them from other data. The API will
+/// allow addition of sub-options but they will be ignored during
+/// packing and unpacking option data.
+///
+/// @param T data field type (see above).
+template<typename T>
+class Option6IntArray: public Option {
+
+public:
+
+    /// @brief Constructor.
+    ///
+    /// Creates option with empty values vector.
+    ///
+    /// @param type option type.
+    ///
+    /// @throw isc::dhcp::InvalidDataType if data field type provided
+    /// as template parameter is not a supported integer type.
+    Option6IntArray(uint16_t type)
+        : Option(Option::V6, type),
+          values_(0) {
+        if (!OptionDataTypes<T>::valid) {
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
+        }
+    }
+
+    /// @brief Constructor.
+    ///
+    /// @param type option type.
+    /// @param buf buffer with option data (must not be empty).
+    ///
+    /// @throw isc::OutOfRange if provided buffer is empty or its length
+    /// is not multiple of size of the data type in bytes.
+    /// @throw isc::dhcp::InvalidDataType if data field type provided
+    /// as template parameter is not a supported integer type.
+    Option6IntArray(uint16_t type, const OptionBuffer& buf)
+        : Option(Option::V6, type) {
+        if (!OptionDataTypes<T>::valid) {
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
+        }
+        unpack(buf.begin(), buf.end());
+    }
+
+    /// @brief Constructor.
+    ///
+    /// This constructor creates option from a buffer. This construtor
+    /// may throw exception if \ref unpack function throws during buffer
+    /// parsing.
+    ///
+    /// @param type option type.
+    /// @param begin iterator to first byte of option data.
+    /// @param end iterator to end of option data (first byte after option end).
+    ///
+    /// @throw isc::OutOfRange if provided buffer is empty or its length
+    /// is not multiple of size of the data type in bytes.
+    /// @throw isc::dhcp::InvalidDataType if data field type provided
+    /// as template parameter is not a supported integer type.
+    Option6IntArray(uint16_t type, OptionBufferConstIter begin,
+                    OptionBufferConstIter end)
+        : Option(Option::V6, type) {
+        if (!OptionDataTypes<T>::valid) {
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
+        }
+        unpack(begin, end);
+    }
+
+    /// Writes option in wire-format to buf, returns pointer to first unused
+    /// byte after stored option.
+    ///
+    /// @param [out] buf buffer (option will be stored here)
+    ///
+    /// @throw isc::dhcp::InvalidDataType if size of a data fields type is not
+    /// equal to 1, 2 or 4 bytes. The data type is not checked in this function
+    /// because it is checked in a constructor.
+    void pack(isc::util::OutputBuffer& buf) {
+        buf.writeUint16(type_);
+        buf.writeUint16(len() - OPTION6_HDR_LEN);
+        for (int i = 0; i < values_.size(); ++i) {
+            // Depending on the data type length we use different utility functions
+            // writeUint16 or writeUint32 which write the data in the network byte
+            // order to the provided buffer. The same functions can be safely used
+            // for either unsiged or signed integers so there is not need to create
+            // special cases for intX_t types.
+            switch (OptionDataTypes<T>::len) {
+            case 1:
+                buf.writeUint8(values_[i]);
+                break;
+            case 2:
+                buf.writeUint16(values_[i]);
+                break;
+            case 4:
+                buf.writeUint32(values_[i]);
+                break;
+            default:
+                isc_throw(dhcp::InvalidDataType, "non-integer type");
+            }
+        }
+        // We don't pack sub-options here because we have array-type option.
+        // We don't allow sub-options in array-type options as there is no
+        // way to distinguish them from the data fields on option reception.
+    }
+
+    /// @brief Parses received buffer
+    ///
+    /// Parses received buffer and returns offset to the first unused byte after
+    /// parsed option.
+    ///
+    /// @param begin iterator to first byte of option data
+    /// @param end iterator to end of option data (first byte after option end)
+    ///
+    /// @throw isc::dhcp::InvalidDataType if size of a data fields type is not
+    /// equal to 1, 2 or 4 bytes. The data type is not checked in this function
+    /// because it is checked in a constructor.
+    virtual void unpack(OptionBufferConstIter begin, OptionBufferConstIter end) {
+        if (distance(begin, end) == 0) {
+            isc_throw(OutOfRange, "option " << getType() << " empty");
+        }
+        if (distance(begin, end) % sizeof(T) != 0) {
+            isc_throw(OutOfRange, "option " << getType() << " truncated");
+        }
+        // @todo consider what to do if buffer is longer than data type.
+
+        values_.clear();
+        while (begin != end) {
+            // Depending on the data type length we use different utility functions
+            // readUint16 or readUint32 which read the data laid in the network byte
+            // order from the provided buffer. The same functions can be safely used
+            // for either unsiged or signed integers so there is not need to create
+            // special cases for intX_t types.
+            int data_size_len = OptionDataTypes<T>::len;
+            switch (data_size_len) {
+            case 1:
+                values_.push_back(*begin);
+                break;
+            case 2:
+                values_.push_back(isc::util::readUint16(&(*begin)));
+                break;
+            case 4:
+                values_.push_back(isc::util::readUint32(&(*begin)));
+                break;
+            default:
+                isc_throw(dhcp::InvalidDataType, "non-integer type");
+            }
+            // Use local variable to set a new value for this iterator.
+            // When using OptionDataTypes<T>::len directly some versions
+            // of clang complain about unresolved reference to
+            // OptionDataTypes structure during linking.
+            begin += data_size_len;
+        }
+        // We do not unpack sub-options here because we have array-type option.
+        // Such option have variable number of data fields, thus there is no
+        // way to assess where sub-options start.
+    }
+
+    /// @brief Return collection of option values.
+    ///
+    /// @return collection of values.
+    const std::vector<T>& getValues() const { return (values_); }
+
+    /// @brief Set option values.
+    ///
+    /// @param values collection of values to be set for option.
+    void setValues(const std::vector<T>& values) { values_ = values; }
+
+    /// @brief returns complete length of option
+    ///
+    /// Returns length of this option, including option header and suboptions
+    ///
+    /// @return length of this option
+    virtual uint16_t len() {
+        uint16_t length = OPTION6_HDR_LEN + values_.size() * sizeof(T);
+        // length of all suboptions
+        for (Option::OptionCollection::iterator it = options_.begin();
+             it != options_.end();
+             ++it) {
+            length += (*it).second->len();
+        }
+        return (length);
+    }
+
+private:
+
+    std::vector<T> values_;
+};
+
+} // isc::dhcp namespace
+} // isc namespace
+
+#endif /* OPTION6_INT_ARRAY_H_ */

src/lib/dhcp/option_data_types.h

@@ -0,0 +1,89 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef OPTION_DATA_TYPES_H_
+#define OPTION_DATA_TYPES_H_
+
+#include <exceptions/exceptions.h>
+
+#include <stdint.h>
+
+namespace isc {
+namespace dhcp {
+
+/// @brief Exception to be thrown when invalid type specified as template parameter.
+class InvalidDataType : public Exception {
+public:
+    InvalidDataType(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
+/// @brief Trait class for integer data types supported in DHCP option definitions.
+///
+/// This is useful to check whether the type specified as template parameter
+/// is supported by classes like Option6Int, Option6IntArray and some template
+/// factory functions in OptionDefinition class.
+template<typename T>
+struct OptionDataTypes {
+    static const bool valid = false;
+    static const int len = 0;
+};
+
+/// int8_t type is supported.
+template<>
+struct OptionDataTypes<int8_t> {
+    static const bool valid = true;
+    static const int len = 1;
+};
+
+/// int16_t type is supported.
+template<>
+struct OptionDataTypes<int16_t> {
+    static const bool valid = true;
+    static const int len = 2;
+};
+
+/// int32_t type is supported.
+template<>
+struct OptionDataTypes<int32_t> {
+    static const bool valid = true;
+    static const int len = 4;
+};
+
+/// uint8_t type is supported.
+template<>
+struct OptionDataTypes<uint8_t> {
+    static const bool valid = true;
+    static const int len = 1;
+};
+
+/// uint16_t type is supported.
+template<>
+struct OptionDataTypes<uint16_t> {
+    static const bool valid = true;
+    static const int len = 2;
+};
+
+/// uint32_t type is supported.
+template<>
+struct OptionDataTypes<uint32_t> {
+    static const bool valid = true;
+    static const int len = 4;
+};
+
+
+} // isc::dhcp namespace
+} // isc namespace
+
+#endif /* OPTION_DATA_TYPES_H_ */

src/lib/dhcp/option_definition.cc

@@ -0,0 +1,252 @@
+// Copyright (C) 2012 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 <dhcp/dhcp6.h>
+#include <dhcp/option_definition.h>
+#include <dhcp/option4_addrlst.h>
+#include <dhcp/option6_addrlst.h>
+#include <dhcp/option6_ia.h>
+#include <dhcp/option6_iaaddr.h>
+#include <dhcp/option6_int.h>
+#include <dhcp/option6_int_array.h>
+
+using namespace std;
+using namespace isc::util;
+
+namespace isc {
+namespace dhcp {
+
+OptionDefinition::DataTypeUtil::DataTypeUtil() {
+    data_types_["empty"] = EMPTY_TYPE;
+    data_types_["boolean"] = BOOLEAN_TYPE;
+    data_types_["int8"] = INT8_TYPE;
+    data_types_["int16"] = INT16_TYPE;
+    data_types_["int32"] = INT32_TYPE;
+    data_types_["uint8"] = UINT8_TYPE;
+    data_types_["uint16"] = UINT16_TYPE;
+    data_types_["uint32"] = UINT32_TYPE;
+    data_types_["ipv4-address"] = IPV4_ADDRESS_TYPE;
+    data_types_["ipv6-address"] = IPV6_ADDRESS_TYPE;
+    data_types_["string"] = STRING_TYPE;
+    data_types_["fqdn"] = FQDN_TYPE;
+    data_types_["record"] = RECORD_TYPE;
+}
+
+OptionDefinition::DataType
+OptionDefinition::DataTypeUtil::getDataType(const std::string& data_type) {
+    std::map<std::string, DataType>::const_iterator data_type_it =
+        data_types_.find(data_type);
+    if (data_type_it != data_types_.end()) {
+        return (data_type_it->second);
+    }
+    return UNKNOWN_TYPE;
+}
+
+OptionDefinition::OptionDefinition(const std::string& name,
+                                 const uint16_t code,
+                                 const std::string& type,
+                                 const bool array_type /* = false */)
+    : name_(name),
+      code_(code),
+      type_(UNKNOWN_TYPE),
+      array_type_(array_type) {
+    // Data type is held as enum value by this class.
+    // Use the provided option type string to get the
+    // corresponding enum value.
+    type_ = DataTypeUtil::instance().getDataType(type);
+}
+
+OptionDefinition::OptionDefinition(const std::string& name,
+                                   const uint16_t code,
+                                   const DataType type,
+                                   const bool array_type /* = false */)
+    : name_(name),
+      code_(code),
+      type_(type),
+      array_type_(array_type) {
+}
+
+void
+OptionDefinition::addRecordField(const std::string& data_type_name) {
+    DataType data_type = DataTypeUtil::instance().getDataType(data_type_name);
+    addRecordField(data_type);
+}
+
+void
+OptionDefinition::addRecordField(const DataType data_type) {
+    if (type_ != RECORD_TYPE) {
+        isc_throw(isc::InvalidOperation, "'record' option type must be used"
+                  " to add data fields to the record");
+    }
+    if (data_type >= UNKNOWN_TYPE) {
+        isc_throw(isc::BadValue, "attempted to add invalid data type to the record");
+    }
+    record_fields_.push_back(data_type);
+}
+
+Option::Factory*
+OptionDefinition::getFactory() const {
+    // @todo This function must be extended to return more factory
+    // functions that create instances of more specialized options.
+    // This requires us to first implement those more specialized
+    // options that will be derived from Option class.
+    if (type_ == IPV6_ADDRESS_TYPE && array_type_) {
+        return (factoryAddrList6);
+    } else if (type_ == IPV4_ADDRESS_TYPE && array_type_) {
+        return (factoryAddrList4);
+    } else if (type_ == EMPTY_TYPE) {
+        return (factoryEmpty);
+    } else if (code_ == D6O_IA_NA && haveIA6Format()) {
+        return (factoryIA6);
+    } else if (code_ == D6O_IAADDR && haveIAAddr6Format()) {
+        return (factoryIAAddr6);
+    } else if (type_ == UINT8_TYPE) {
+        if (array_type_) {
+            return (factoryGeneric);
+        } else {
+            return (factoryInteger<uint8_t>);
+        }
+    } else if (type_ == UINT16_TYPE) {
+        if (array_type_) {
+            return (factoryIntegerArray<uint16_t>);
+        } else {
+            return (factoryInteger<uint16_t>);
+        }
+    } else if (type_ == UINT32_TYPE) {
+        if (array_type_) {
+            return (factoryIntegerArray<uint32_t>);
+        } else {
+            return (factoryInteger<uint32_t>);
+        }
+    }
+    // Factory generic returns instance of Option class. However, once we
+    // implement CustomOption class we may want to return factory function
+    // that will create instance of CustomOption rather than Option.
+    // CustomOption will allow to access particular data fields within the
+    // option rather than raw data buffer.
+    return (factoryGeneric);
+}
+
+void
+OptionDefinition::sanityCheckUniverse(const Option::Universe expected_universe,
+                                      const Option::Universe actual_universe) {
+    if (expected_universe != actual_universe) {
+        isc_throw(isc::BadValue, "invalid universe specified for the option");
+    }
+}
+
+void
+OptionDefinition::validate() const {
+    // Option name must not be empty.
+    if (name_.empty()) {
+        isc_throw(isc::BadValue, "option name must not be empty");
+    }
+    // Option name must not contain spaces.
+    if (name_.find(" ") != string::npos) {
+        isc_throw(isc::BadValue, "option name must not contain spaces");
+    }
+    // Unsupported option types are not allowed.
+    if (type_ >= UNKNOWN_TYPE) {
+        isc_throw(isc::OutOfRange, "option type value " << type_
+                  << " is out of range");
+    }
+}
+
+bool
+OptionDefinition::haveIAx6Format(OptionDefinition::DataType first_type) const {
+   return (haveType(RECORD_TYPE) &&
+           record_fields_.size() == 3 &&
+           record_fields_[0] == first_type &&
+           record_fields_[1] == UINT32_TYPE &&
+           record_fields_[2] == UINT32_TYPE);
+}
+
+bool
+OptionDefinition::haveIA6Format() const {
+    // Expect that IA_NA option format is defined as record.
+    // Although it consists of 3 elements of the same (uint32)
+    // type it can't be defined as array of uint32 elements because
+    // arrays do not impose limitations on number of elements in
+    // the array while this limitation is needed for IA_NA - need
+    // exactly 3 elements.
+    return (haveIAx6Format(UINT32_TYPE));
+}
+
+bool
+OptionDefinition::haveIAAddr6Format() const {
+    return (haveIAx6Format(IPV6_ADDRESS_TYPE));
+}
+
+OptionPtr
+OptionDefinition::factoryAddrList4(Option::Universe u, uint16_t type,
+                                   const OptionBuffer& buf) {
+    sanityCheckUniverse(u, Option::V4);
+    boost::shared_ptr<Option4AddrLst> option(new Option4AddrLst(type, buf.begin(),
+                                                                buf.begin() + buf.size()));
+    return (option);
+}
+
+OptionPtr
+OptionDefinition::factoryAddrList6(Option::Universe u, uint16_t type,
+                                   const OptionBuffer& buf) {
+    sanityCheckUniverse(u, Option::V6);
+    boost::shared_ptr<Option6AddrLst> option(new Option6AddrLst(type, buf.begin(),
+                                                                buf.begin() + buf.size()));
+    return (option);
+}
+
+
+OptionPtr
+OptionDefinition::factoryEmpty(Option::Universe u, uint16_t type, const OptionBuffer& buf) {
+    if (buf.size() > 0) {
+        isc_throw(isc::BadValue, "input option buffer must be empty"
+                  " when creating empty option instance");
+    }
+    OptionPtr option(new Option(u, type));
+    return (option);
+}
+
+OptionPtr
+OptionDefinition::factoryGeneric(Option::Universe u, uint16_t type, const OptionBuffer& buf) {
+    OptionPtr option(new Option(u, type, buf));
+    return (option);
+}
+
+OptionPtr
+OptionDefinition::factoryIA6(Option::Universe u, uint16_t type, const OptionBuffer& buf) {
+    sanityCheckUniverse(u, Option::V6);
+    if (buf.size() != Option6IA::OPTION6_IA_LEN) {
+        isc_throw(isc::OutOfRange, "input option buffer has invalid size, expeted "
+                  << Option6IA::OPTION6_IA_LEN << " bytes");
+    }
+    boost::shared_ptr<Option6IA> option(new Option6IA(type, buf.begin(),
+                                                      buf.begin() + buf.size()));
+    return (option);
+}
+
+OptionPtr
+OptionDefinition::factoryIAAddr6(Option::Universe u, uint16_t type, const OptionBuffer& buf) {
+    sanityCheckUniverse(u, Option::V6);
+    if (buf.size() != Option6IAAddr::OPTION6_IAADDR_LEN) {
+        isc_throw(isc::OutOfRange, "input option buffer has invalid size, expeted "
+                  << Option6IAAddr::OPTION6_IAADDR_LEN << " bytes");
+    }
+    boost::shared_ptr<Option6IAAddr> option(new Option6IAAddr(type, buf.begin(),
+                                                      buf.begin() + buf.size()));
+    return (option);
+}
+
+
+} // end of isc::dhcp namespace
+} // end of isc namespace

src/lib/dhcp/option_definition.h

@@ -0,0 +1,383 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef OPTION_DEFINITION_H_
+#define OPTION_DEFINITION_H_
+
+#include <dhcp/option_data_types.h>
+#include <dhcp/option6_int.h>
+#include <dhcp/option6_int_array.h>
+#include <dhcp/option.h>
+
+namespace isc {
+namespace dhcp {
+
+/// @brief Base class representing a DHCP option definition.
+///
+/// This is a base class representing a DHCP option definition, which describes
+/// the format of the option. In particular, it defines:
+/// - option name,
+/// - option code,
+/// - data fields order and their types,
+/// - sub options space that the particular option encapsulates.
+///
+/// The option type specifies the data type(s) which an option conveys.  If
+/// this is a single value the option type points to the data type of the
+/// value. For example, DHCPv6 option 8 comprises a two-byte option code, a
+/// two-byte option length and two-byte field that carries a uint16 value
+/// (RFC 3315 - http://ietf.org/rfc/rfc3315.txt).  In such a case, the option
+/// type is defined as "uint16".
+///
+/// When the option has a more complex structure, the option type may be
+/// defined as "array", "record" or even "array of records".
+///
+/// Array types should be used when the option contains multiple contiguous
+/// data values of the same type laid. For example, DHCPv6 option 6 includes
+/// multiple fields holding uint16 codes of requested DHCPv6 options (RFC 3315).
+/// Such an option can be represented with this class by setting the option
+/// type to "uint16" and the array indicator (array_type) to true.  The number
+/// of elements in the array is effectively unlimited (although it is actually
+/// limited by the maximal DHCPv6 option length).
+///
+/// Should the option comprise data fields of different types, the "record"
+/// option type is used. In such cases the data field types within the record
+/// are specified using \ref OptionDefinition::addRecordField.
+///
+/// When the OptionDefinition object has been sucessfully created, it can be
+/// queried to return the appropriate option factory function for the specified
+/// specified option format. There are a number of "standard" factory functions
+/// that cover well known (common) formats.  If the particular format does not
+/// match any common format the generic factory function is returned.
+///
+/// The following data type strings are supported:
+/// - "empty" (option does not contain data fields)
+/// - "boolean"
+/// - "int8"
+/// - "int16"
+/// - "int32"
+/// - "uint8"
+/// - "uint16"
+/// - "uint32"
+/// - "ipv4-address" (IPv4 Address)
+/// - "ipv6-address" (IPV6 Address)
+/// - "string"
+/// - "fqdn" (fully qualified name)
+/// - "record" (set of data fields of different types)
+///
+/// @todo Extend the comment to describe "generic factories".
+/// @todo Extend this class to use custom namespaces.
+/// @todo Extend this class with more factory functions.
+class OptionDefinition {
+public:
+
+    /// Data types of DHCP option fields.
+    enum DataType {
+        EMPTY_TYPE,
+        BOOLEAN_TYPE,
+        INT8_TYPE,
+        INT16_TYPE,
+        INT32_TYPE,
+        UINT8_TYPE,
+        UINT16_TYPE,
+        UINT32_TYPE,
+        IPV4_ADDRESS_TYPE,
+        IPV6_ADDRESS_TYPE,
+        STRING_TYPE,
+        FQDN_TYPE,
+        RECORD_TYPE,
+        UNKNOWN_TYPE
+    };
+
+    /// List of fields within the record.
+    typedef std::vector<DataType> RecordFieldsCollection;
+    /// Const iterator for record data fields.
+    typedef std::vector<DataType>::const_iterator RecordFieldsConstIter;
+
+private:
+
+    /// @brief Utility class for operations on DataTypes.
+    ///
+    /// This class is implemented as the singleton because the list of
+    /// supported data types need only be loaded only once into memory as it
+    /// can persist for all option definitions.
+    ///
+    /// @todo This class can be extended to return the string value
+    /// representing the data type from the enum value.
+    class DataTypeUtil {
+    public:
+
+        /// @brief Return the sole instance of this class.
+        ///
+        /// @return instance of this class.
+        static DataTypeUtil& instance() {
+            static DataTypeUtil instance;
+            return (instance);
+        }
+
+        /// @brief Convert type given as string value to option data type.
+        ///
+        /// @param data_type_name data type string.
+        ///
+        /// @return option data type.
+        DataType getDataType(const std::string& data_type_name);
+
+    private:
+        /// @brief Private constructor.
+        ///
+        /// Constructor initializes the internal data structures, e.g.
+        /// mapping between data type name and the corresponding enum.
+        /// This constructor is private to ensure that exactly one
+        /// instance of this class can be created using \ref instance
+        /// function.
+        DataTypeUtil();
+
+        /// Map of data types, maps name of the type to enum value.
+        std::map<std::string, DataType> data_types_;
+    };
+
+public:
+    /// @brief Constructor.
+    ///
+    /// @param name option name.
+    /// @param code option code.
+    /// @param type option data type as string.
+    /// @param array_type array indicator, if true it indicates that the
+    /// option fields are the array.
+    OptionDefinition(const std::string& name,
+                     const uint16_t code,
+                     const std::string& type,
+                     const bool array_type = false);
+
+    /// @brief Constructor.
+    ///
+    /// @param name option name.
+    /// @param code option code.
+    /// @param type option data type.
+    /// @param array_type array indicator, if true it indicates that the
+    /// option fields are the array.
+    OptionDefinition(const std::string& name,
+                     const uint16_t code,
+                     const DataType type,
+                     const bool array_type = false);
+
+    /// @brief Adds data field to the record.
+    ///
+    /// @param data_type_name name of the data type for the field.
+    ///
+    /// @throw isc::InvalidOperation if option type is not set to RECORD_TYPE.
+    /// @throw isc::BadValue if specified invalid data type.
+    void addRecordField(const std::string& data_type_name);
+
+    /// @brief Adds data field to the record.
+    ///
+    /// @param data_type data type for the field.
+    ///
+    /// @throw isc::InvalidOperation if option type is not set to RECORD_TYPE.
+    /// @throw isc::BadValue if specified invalid data type.
+    void addRecordField(const DataType data_type);
+
+    /// @brief Return array type indicator.
+    ///
+    /// The method returns the bool value to indicate whether the option is a
+    /// a single value or an array of values.
+    ///
+    /// @return true if option comprises an array of values.
+    bool getArrayType() const { return (array_type_); }
+
+    /// @brief Return option code.
+    ///
+    /// @return option code.
+    uint16_t getCode() const { return (code_); }
+
+    /// @brief Return factory function for the given definition.
+    ///
+    /// @return pointer to factory function.
+    Option::Factory* getFactory() const;
+
+    /// @brief Return option name.
+    ///
+    /// @return option name.
+    const std::string& getName() const { return (name_); }
+
+    /// @brief Return list of record fields.
+    ///
+    /// @return list of record fields.
+    const RecordFieldsCollection& getRecordFields() const { return (record_fields_); }
+
+    /// @brief Return option data type.
+    ///
+    /// @return option data type.
+    DataType getType() const { return (type_); };
+
+    /// @brief Check if the option definition is valid.
+    ///
+    /// @throw isc::OutOfRange if invalid option type was specified.
+    /// @throw isc::BadValue if invalid option name was specified,
+    /// e.g. empty or containing spaces.
+    void validate() const;
+
+    /// @brief Check if specified format is IA_NA option format.
+    ///
+    /// @return true if specified format is IA_NA option format.
+    bool haveIA6Format() const;
+
+    /// @brief Check if specified format is IAADDR option format.
+    ///
+    /// @return true if specified format is IAADDR option format.
+    bool haveIAAddr6Format() const;
+
+    /// @brief Factory to create option with address list.
+    ///
+    /// @param u universe (must be V4).
+    /// @param type option type.
+    /// @param buf option buffer with a list of IPv4 addresses.
+    ///
+    /// @throw isc::OutOfRange if length of the provided option buffer
+    /// is not multiple of IPV4 address length.
+    static OptionPtr factoryAddrList4(Option::Universe u, uint16_t type,
+                                      const OptionBuffer& buf);
+
+    /// @brief Factory to create option with address list.
+    ///
+    /// @param u universe (must be V6).
+    /// @param type option type.
+    /// @param buf option buffer with a list of IPv6 addresses.
+    ///
+    /// @throw isc::OutOfaRange if length of provided option buffer
+    /// is not multiple of IPV6 address length.
+    static OptionPtr factoryAddrList6(Option::Universe u, uint16_t type,
+                                      const OptionBuffer& buf);
+
+    /// @brief Empty option factory.
+    ///
+    /// @param u universe (V6 or V4).
+    /// @param type option type.
+    /// @param buf option buffer (must be empty).
+    static OptionPtr factoryEmpty(Option::Universe u, uint16_t type,
+                                  const OptionBuffer& buf);
+
+    /// @brief Factory to create generic option.
+    ///
+    /// @param u universe (V6 or V4).
+    /// @param type option type.
+    /// @param buf option buffer.
+    static OptionPtr factoryGeneric(Option::Universe u, uint16_t type,
+                                    const OptionBuffer& buf);
+
+    /// @brief Factory for IA-type of option.
+    ///
+    /// @param u universe (must be V6).
+    /// @param type option type.
+    /// @param buf option buffer.
+    ///
+    /// @throw isc::OutOfRange if provided option buffer is too short or
+    /// too long. Expected size is 12 bytes.
+    /// @throw isc::BadValue if specified universe value is not V6.
+    static OptionPtr factoryIA6(Option::Universe u, uint16_t type,
+                                const OptionBuffer& buf);
+
+    /// @brief Factory for IAADDR-type of option.
+    ///
+    /// @param u universe (must be V6).
+    /// @param type option type.
+    /// @param buf option buffer.
+    ///
+    /// @throw isc::OutOfRange if provided option buffer is too short or
+    /// too long. Expected size is 24 bytes.
+    /// @throw isc::BadValue if specified universe value is not V6.
+    static OptionPtr factoryIAAddr6(Option::Universe u, uint16_t type,
+                                const OptionBuffer& buf);
+
+    /// @brief Factory function to create option with integer value.
+    ///
+    /// @param type option type.
+    /// @param buf option buffer.
+    /// @tparam T type of the data field (must be one of the uintX_t or intX_t).
+    ///
+    /// @throw isc::OutOfRange if provided option buffer length is invalid.
+    template<typename T>
+    static OptionPtr factoryInteger(Option::Universe, uint16_t type, const OptionBuffer& buf) {
+        if (buf.size() > sizeof(T)) {
+            isc_throw(isc::OutOfRange, "provided option buffer is too large, expected: "
+                      << sizeof(T) << " bytes");
+        }
+        OptionPtr option(new Option6Int<T>(type, buf.begin(), buf.end()));
+        return (option);
+    }
+
+    /// @brief Factory function to create option with array of integer values.
+    ///
+    /// @param type option type.
+    /// @param buf option buffer.
+    /// @tparam T type of the data field (must be one of the uintX_t or intX_t).
+    ///
+    /// @throw isc::OutOfRange if provided option buffer length is invalid.
+    template<typename T>
+    static OptionPtr factoryIntegerArray(Option::Universe, uint16_t type, const OptionBuffer& buf) {
+        if (buf.size() == 0) {
+            isc_throw(isc::OutOfRange, "option buffer length must be greater than zero");
+        } else if (buf.size() % OptionDataTypes<T>::len != 0) {
+            isc_throw(isc::OutOfRange, "option buffer length must be multiple of "
+                      << OptionDataTypes<T>::len << " bytes");
+        }
+        OptionPtr option(new Option6IntArray<T>(type, buf.begin(), buf.end()));
+        return (option);
+    }
+
+private:
+
+    /// @brief Check if specified option format is a record with 3 fields
+    /// where first one is custom, and two others are uint32.
+    ///
+    /// This is a helper function for functions that detect IA_NA and IAAddr
+    /// option formats.
+    ///
+    /// @param first_type type of the first data field.
+    ///
+    /// @return true if actual option format matches expected format.
+    bool haveIAx6Format(const OptionDefinition::DataType first_type) const;
+
+    /// @brief Check if specified type matches option definition type.
+    ///
+    /// @return true if specified type matches option definition type.
+    inline bool haveType(const DataType type) const {
+        return (type == type_);
+    }
+
+    /// @brief Sanity check universe value.
+    ///
+    /// @param expected_universe expected universe value.
+    /// @param actual_universe actual universe value.
+    ///
+    /// @throw isc::BadValue if expected universe and actual universe don't match.
+   static inline void sanityCheckUniverse(const Option::Universe expected_universe,
+                                          const Option::Universe actual_universe); 
+
+    /// Option name.
+    std::string name_;
+    /// Option code.
+    uint16_t code_;
+    /// Option data type.
+    DataType type_;
+    /// Indicates wheter option is a single value or array.
+    bool array_type_;
+    /// Collection of data fields within the record.
+    RecordFieldsCollection record_fields_;
+};
+
+
+} // namespace isc::dhcp
+} // namespace isc
+
+#endif

src/lib/dhcp/subnet.cc

@@ -41,6 +41,17 @@ bool Subnet::inRange(const isc::asiolink::IOAddress& addr) const {
     return ((first <= addr) && (addr <= last));
 }
 
+void
+Subnet::addOption(OptionPtr& option, bool persistent /* = false */) {
+    validateOption(option);
+    options_.push_back(OptionDescriptor(option, persistent));
+}
+
+void
+Subnet::delOptions() {
+    options_.clear();
+}
+
 Subnet4::Subnet4(const isc::asiolink::IOAddress& prefix, uint8_t length,
                  const Triplet<uint32_t>& t1,
                  const Triplet<uint32_t>& t2,
@@ -85,6 +96,15 @@ Pool4Ptr Subnet4::getPool4(const isc::asiolink::IOAddress& hint /* = IOAddress("
     return (candidate);
 }
 
+void
+Subnet4::validateOption(const OptionPtr& option) const {
+    if (!option) {
+        isc_throw(isc::BadValue, "option configured for subnet must not be NULL");
+    } else if (option->getUniverse() != Option::V4) {
+        isc_throw(isc::BadValue, "expected V4 option to be added to the subnet");
+    }
+}
+
 Subnet6::Subnet6(const isc::asiolink::IOAddress& prefix, uint8_t length,
                  const Triplet<uint32_t>& t1,
                  const Triplet<uint32_t>& t2,
@@ -131,5 +151,13 @@ Pool6Ptr Subnet6::getPool6(const isc::asiolink::IOAddress& hint /* = IOAddress("
     return (candidate);
 }
 
+void
+Subnet6::validateOption(const OptionPtr& option) const {
+    if (!option) {
+        isc_throw(isc::BadValue, "option configured for subnet must not be NULL");
+    } else if (option->getUniverse() != Option::V6) {
+        isc_throw(isc::BadValue, "expected V6 option to be added to the subnet");
+    }
+}
 } // end of isc::dhcp namespace
 } // end of isc namespace

src/lib/dhcp/subnet.h

@@ -15,10 +15,16 @@
 #ifndef SUBNET_H
 #define SUBNET_H
 
-#include <boost/shared_ptr.hpp>
 #include <asiolink/io_address.h>
 #include <dhcp/pool.h>
 #include <dhcp/triplet.h>
+#include <dhcp/option.h>
+#include <boost/shared_ptr.hpp>
+#include <boost/multi_index_container.hpp>
+#include <boost/multi_index/hashed_index.hpp>
+#include <boost/multi_index/sequenced_index.hpp>
+#include <boost/multi_index/mem_fun.hpp>
+#include <boost/multi_index/member.hpp>
 
 namespace isc {
 namespace dhcp {
@@ -30,14 +36,174 @@ namespace dhcp {
 /// attached to it. In most cases all devices attached to a single link can
 /// share the same parameters. Therefore Subnet holds several values that are
 /// typically shared by all hosts: renew timer (T1), rebind timer (T2) and
-/// leased addresses lifetime (valid-lifetime).
-///
-/// @todo: Implement support for options here
+/// leased addresses lifetime (valid-lifetime). It also holds the set
+/// of DHCP option instances configured for the subnet. These options are
+/// included in DHCP messages being sent to clients which are connected
+/// to the particular subnet.
 class Subnet {
 public:
+
+    /// @brief Option descriptor.
+    ///
+    /// Option descriptor holds information about option configured for
+    /// a particular subnet. This information comprises the actual option
+    /// instance and information whether this option is sent to DHCP client
+    /// only on request (persistent = false) or always (persistent = true).
+    struct OptionDescriptor {
+        /// Option instance.
+        OptionPtr option;
+        /// Persistent flag, if true option is always sent to the client,
+        /// if false option is sent to the client on request.
+        bool persistent;
+
+        /// @brief Constructor.
+        ///
+        /// @param opt option
+        /// @param persist if true option is always sent.
+        OptionDescriptor(OptionPtr& opt, bool persist)
+            : option(opt), persistent(persist) {};
+    };
+
+    /// @brief Extractor class to extract key with another key.
+    ///
+    /// This class solves the problem of accessing index key values
+    /// that are stored in objects nested in other objects.
+    /// Each OptionDescriptor structure contains the OptionPtr object.
+    /// The value retured by one of its accessors (getType) is used
+    /// as an indexing value in the multi_index_container defined below.
+    /// There is no easy way to mark that value returned by Option::getType
+    /// should be an index of this multi_index_container. There are standard
+    /// key extractors such as 'member' or 'mem_fun' but they are not
+    /// sufficient here. The former can be used to mark that member of
+    /// the structure that is held in the container should be used as an
+    /// indexing value. The latter can be used if the indexing value is
+    /// a product of the class being held in the container. In this complex
+    /// scenario when the indexing value is a product of the function that
+    /// is wrapped by the structure, this new extractor template has to be
+    /// defined. The template class provides a 'chain' of two extractors
+    /// to access the value returned by nested object and to use it as
+    /// indexing value.
+    /// For some more examples of complex keys see:
+    /// http://www.cs.brown.edu/~jwicks/boost/libs/multi_index/doc/index.html
+    ///
+    /// @tparam KeyExtractor1 extractor used to access data in
+    /// OptionDescriptor::option
+    /// @tparam KeyExtractor2 extractor used to access
+    /// OptionDescriptor::option member.
+    template<typename KeyExtractor1, typename KeyExtractor2>
+    class KeyFromKey {
+    public:
+        typedef typename KeyExtractor1::result_type result_type;
+
+        /// @brief Constructor.
+        KeyFromKey()
+            : key1_(KeyExtractor1()), key2_(KeyExtractor2()) { };
+
+        /// @brief Extract key with another key.
+        ///
+        /// @param arg the key value.
+        ///
+        /// @tparam key value type.
+        template<typename T>
+        result_type operator() (T& arg) const {
+            return (key1_(key2_(arg)));
+        }
+    private:
+        KeyExtractor1 key1_; ///< key 1.
+        KeyExtractor2 key2_; ///< key 2.
+    };
+
+    /// @brief Multi index container for DHCP option descriptors.
+    ///
+    /// This container comprises three indexes to access option
+    /// descriptors:
+    /// - sequenced index: used to access elements in the order they
+    /// have been added to the container,
+    /// - option type index: used to search option descriptors containing
+    /// options with specific option code (aka option type).
+    /// - persistency flag index: used to search option descriptors with
+    /// 'persistent' flag set to true.
+    ///
+    /// This container is the equivalent of three separate STL containers:
+    /// - std::list of all options,
+    /// - std::multimap of options with option code used as a multimap key,
+    /// - std::multimap of option descriptors with option persistency flag
+    /// used as a multimap key.
+    /// The major advantage of this container over 3 separate STL containers
+    /// is automatic synchronization of all indexes when elements are added,
+    /// removed or modified in the container. With separate containers,
+    /// the synchronization would have to be guaranteed by the Subnet class
+    /// code. This would increase code complexity and presumably it would
+    /// be much harder to add new search criteria (indexes).
+    ///
+    /// @todo we may want to search for options using option spaces when
+    /// they are implemented.
+    ///
+    /// @see http://www.boost.org/doc/libs/1_51_0/libs/multi_index/doc/index.html
+    typedef boost::multi_index_container<
+        // Container comprises elements of OptionDescriptor type.
+        OptionDescriptor,
+        // Here we start enumerating various indexes.
+        boost::multi_index::indexed_by<
+            // Sequenced index allows accessing elements in the same way
+            // as elements in std::list.
+            // Sequenced is an index #0.
+            boost::multi_index::sequenced<>,
+            // Start definition of index #1.
+            boost::multi_index::hashed_non_unique<
+                // KeyFromKey is the index key extractor that allows accessing
+                // option type being held by the OptionPtr through
+                // OptionDescriptor structure.
+                KeyFromKey<
+                    // Use option type as the index key. The type is held
+                    // in OptionPtr object so we have to call Option::getType
+                    // to retrieve this key for each element.
+                    boost::multi_index::mem_fun<
+                        Option,
+                        uint16_t,
+                        &Option::getType
+                    >,
+                    // Indicate that OptionPtr is a member of
+                    // OptionDescriptor structure.
+                    boost::multi_index::member<
+                        OptionDescriptor,
+                        OptionPtr,
+                        &OptionDescriptor::option
+                    >
+                 >
+            >,
+            // Start definition of index #2.
+            // Use 'persistent' struct member as a key.
+            boost::multi_index::hashed_non_unique<
+                boost::multi_index::member<
+                    OptionDescriptor,
+                    bool,
+                    &OptionDescriptor::persistent
+                >
+            >
+        >
+    > OptionContainer;
+
+    /// Type of the index #1 - option type.
+    typedef OptionContainer::nth_index<1>::type OptionContainerTypeIndex;
+    /// Type of the index #2 - option persistency flag.
+    typedef OptionContainer::nth_index<2>::type OptionContainerPersistIndex;
+
     /// @brief checks if specified address is in range
     bool inRange(const isc::asiolink::IOAddress& addr) const;
 
+    /// @brief Add new option instance to the collection.
+    ///
+    /// @param option option instance.
+    /// @param persistent if true, send an option regardless if client
+    /// requested it or not.
+    ///
+    /// @throw isc::BadValue if invalid option provided.
+    void addOption(OptionPtr& option, bool persistent = false);
+
+    /// @brief Delete all options configured for the subnet.
+    void delOptions();
+
     /// @brief return valid-lifetime for addresses in that prefix
     Triplet<uint32_t> getValid() const {
         return (valid_);
@@ -53,6 +219,15 @@ public:
         return (t2_);
     }
 
+    /// @brief Return a collection of options.
+    ///
+    /// @return reference to collection of options configured for a subnet.
+    /// The returned reference is valid as long as the Subnet object which
+    /// returned it still exists.
+    const OptionContainer& getOptions() {
+        return (options_);
+    }
+
 protected:
     /// @brief protected constructor
     //
@@ -63,6 +238,12 @@ protected:
            const Triplet<uint32_t>& t2,
            const Triplet<uint32_t>& valid_lifetime);
 
+    /// @brief virtual destructor
+    ///
+    /// A virtual destructor is needed because other classes
+    /// derive from this class.
+    virtual ~Subnet() { };
+
     /// @brief returns the next unique Subnet-ID
     ///
     /// @return the next unique Subnet-ID
@@ -71,6 +252,11 @@ protected:
         return (id++);
     }
 
+    /// @brief Check if option is valid and can be added to a subnet.
+    ///
+    /// @param option option to be validated.
+    virtual void validateOption(const OptionPtr& option) const = 0;
+
     /// @brief subnet-id
     ///
     /// Subnet-id is a unique value that can be used to find or identify
@@ -91,6 +277,9 @@ protected:
 
     /// @brief a tripet (min/default/max) holding allowed valid lifetime values
     Triplet<uint32_t> valid_;
+
+    /// @brief a collection of DHCP options configured for a subnet.
+    OptionContainer options_;
 };
 
 /// @brief A configuration holder for IPv4 subnet.
@@ -133,6 +322,14 @@ public:
     }
 
 protected:
+
+    /// @brief Check if option is valid and can be added to a subnet.
+    ///
+    /// @param option option to be validated.
+    ///
+    /// @throw isc::BadValue if provided option is invalid.
+    virtual void validateOption(const OptionPtr& option) const;
+
     /// @brief collection of pools in that list
     Pool4Collection pools_;
 };
@@ -193,6 +390,14 @@ public:
     }
 
 protected:
+
+    /// @brief Check if option is valid and can be added to a subnet.
+    ///
+    /// @param option option to be validated.
+    ///
+    /// @throw isc::BadValue if provided option is invalid.
+    virtual void validateOption(const OptionPtr& option) const;
+
     /// @brief collection of pools in that list
     Pool6Collection pools_;
 

src/lib/dhcp/tests/Makefile.am

@@ -33,7 +33,10 @@ libdhcp___unittests_SOURCES += option6_iaaddr_unittest.cc
 libdhcp___unittests_SOURCES += option6_ia_unittest.cc
 libdhcp___unittests_SOURCES += option6_addrlst_unittest.cc
 libdhcp___unittests_SOURCES += option4_addrlst_unittest.cc
+libdhcp___unittests_SOURCES += option6_int_unittest.cc
+libdhcp___unittests_SOURCES += option6_int_array_unittest.cc
 libdhcp___unittests_SOURCES += option_unittest.cc
+libdhcp___unittests_SOURCES += option_definition_unittest.cc
 libdhcp___unittests_SOURCES += pkt6_unittest.cc
 libdhcp___unittests_SOURCES += pkt4_unittest.cc
 libdhcp___unittests_SOURCES += duid_unittest.cc
@@ -54,6 +57,7 @@ libdhcpsrv_unittests_LDADD  = $(GTEST_LDADD)
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcpsrv.la
+libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
 
 

src/lib/dhcp/tests/option6_int_array_unittest.cc

@@ -0,0 +1,420 @@
+// Copyright (C) 2012 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 <config.h>
+#include <dhcp/dhcp6.h>
+#include <dhcp/option.h>
+#include <dhcp/option6_int_array.h>
+#include <dhcp/option6_iaaddr.h>
+#include <util/buffer.h>
+
+#include <boost/pointer_cast.hpp>
+#include <gtest/gtest.h>
+
+using namespace std;
+using namespace isc;
+using namespace isc::dhcp;
+using namespace isc::asiolink;
+using namespace isc::util;
+
+namespace {
+
+/// @brief Option6IntArray test class.
+class Option6IntArrayTest : public ::testing::Test {
+public:
+    /// @brief Constructor.
+    ///
+    /// Initializes the option buffer with some data.
+    Option6IntArrayTest(): buf_(255), out_buf_(255) {
+        for (int i = 0; i < 255; i++) {
+            buf_[i] = 255 - i;
+        }
+    }
+
+    /// @brief Test parsing buffer into array of int8_t or uint8_t values.
+    ///
+    /// @warning this function does not perform type check. Make
+    /// sure that only int8_t or uint8_t type is used.
+    ///
+    /// @tparam T int8_t or uint8_t.
+    template<typename T>
+    void bufferToIntTest8() {
+        // Create option that conveys array of multiple uint8_t or int8_t values.
+        // In fact there is no need to use this template class for array
+        // of uint8_t values because Option class is sufficient - it
+        // returns the buffer which is actually the array of uint8_t.
+        // However, since we allow using uint8_t types with this template
+        // class we have to test it here.
+        boost::shared_ptr<Option6IntArray<T> > opt;
+        const int opt_len = 10;
+        const uint16_t opt_code = 80;
+
+        // Constructor throws exception if provided buffer is empty.
+        EXPECT_THROW(
+            Option6IntArray<T>(opt_code, buf_.begin(), buf_.begin()),
+            isc::OutOfRange
+        );
+
+        // Provided buffer is not empty so it should not throw exception.
+        ASSERT_NO_THROW(
+            opt = boost::shared_ptr<
+                Option6IntArray<T> >(new Option6IntArray<T>(opt_code, buf_.begin(),
+                                                            buf_.begin() + opt_len))
+        );
+
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(opt_code, opt->getType());
+        // Option should return the collection of int8_t or uint8_t values that
+        // we can match with the buffer we used to create the option.
+        std::vector<T> values = opt->getValues();
+        // We need to copy values from the buffer to apply sign if signed
+        // type is used.
+        std::vector<T> reference_values;
+        for (int i = 0; i < opt_len; ++i) {
+            // Values have been read from the buffer in network
+            // byte order. We put them back in the same order here.
+            reference_values.push_back(static_cast<T>(buf_[i]));
+        }
+
+        // Compare the values against the reference buffer.
+        ASSERT_EQ(opt_len, values.size());
+        EXPECT_TRUE(std::equal(reference_values.begin(), reference_values.begin()
+                               + opt_len, values.begin()));
+
+        // test for pack()
+        opt->pack(out_buf_);
+
+        // Data length is 10 bytes.
+        EXPECT_EQ(10, opt->len() - opt->getHeaderLen());
+        EXPECT_EQ(opt_code, opt->getType());
+        // The total length is 10 bytes for data and 4 bytes for header.
+        ASSERT_EQ(14, out_buf_.getLength());
+
+        // Check if pack worked properly:
+        InputBuffer out(out_buf_.getData(), out_buf_.getLength());
+        // if option type is correct
+        EXPECT_EQ(opt_code, out.readUint16());
+        // if option length is correct
+        EXPECT_EQ(10, out.readUint16());
+        // if data is correct
+        std::vector<uint8_t> out_data;
+        ASSERT_NO_THROW(out.readVector(out_data, opt_len));
+        ASSERT_EQ(opt_len, out_data.size());
+        EXPECT_TRUE(std::equal(buf_.begin(), buf_.begin() + opt_len, out_data.begin()));;
+    }
+
+    /// @brief Test parsing buffer into array of int16_t or uint16_t values.
+    ///
+    /// @warning this function does not perform type check. Make
+    /// sure that only int16_t or uint16_t type is used.
+    ///
+    /// @tparam T int16_t or uint16_t.
+    template<typename T>
+    void bufferToIntTest16() {
+        // Create option that conveys array of multiple uint16_t or int16_t values.
+        boost::shared_ptr<Option6IntArray<T> > opt;
+        const int opt_len = 20;
+        const uint16_t opt_code = 81;
+
+        // Constructor throws exception if provided buffer is empty.
+        EXPECT_THROW(
+            Option6IntArray<T>(opt_code, buf_.begin(), buf_.begin()),
+            isc::OutOfRange
+        );
+
+        // Constructor throws exception if provided buffer's length is not
+        // multiple of 2-bytes.
+        EXPECT_THROW(
+            Option6IntArray<T>(opt_code, buf_.begin(), buf_.begin() + 5),
+            isc::OutOfRange
+        );
+
+        // Now the buffer length is correct.
+        ASSERT_NO_THROW(
+            opt = boost::shared_ptr<
+                Option6IntArray<T> >(new Option6IntArray<T>(opt_code, buf_.begin(),
+                                                            buf_.begin() + opt_len))
+        );
+
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(opt_code, opt->getType());
+        // Option should return vector of uint16_t values which should be
+        // constructed from the buffer we provided.
+        std::vector<T> values = opt->getValues();
+        ASSERT_EQ(opt_len, values.size() * sizeof(T));
+        // Create reference values from the buffer so as we can
+        // simply compare two vectors.
+        std::vector<T> reference_values;
+        for (int i = 0; i < opt_len; i += 2) {
+            reference_values.push_back((buf_[i] << 8) |
+                                       buf_[i + 1]);
+        }
+        EXPECT_TRUE(std::equal(reference_values.begin(), reference_values.end(),
+                               values.begin()));
+
+        // Test for pack()
+        opt->pack(out_buf_);
+
+        // Data length is 20 bytes.
+        EXPECT_EQ(20, opt->len() - opt->getHeaderLen());
+        EXPECT_EQ(opt_code, opt->getType());
+        // The total length is 20 bytes for data and 4 bytes for header.
+        ASSERT_EQ(24, out_buf_.getLength());
+
+        // Check if pack worked properly:
+        InputBuffer out(out_buf_.getData(), out_buf_.getLength());
+        // if option type is correct
+        EXPECT_EQ(opt_code, out.readUint16());
+        // if option length is correct
+        EXPECT_EQ(20, out.readUint16());
+        // if data is correct
+        std::vector<uint8_t> out_data;
+        ASSERT_NO_THROW(out.readVector(out_data, opt_len));
+        ASSERT_EQ(opt_len, out_data.size());
+        EXPECT_TRUE(std::equal(buf_.begin(), buf_.begin() + opt_len, out_data.begin()));;
+    }
+
+    /// @brief Test parsing buffer into array of int32_t or uint32_t values.
+    ///
+    /// @warning this function does not perform type check. Make
+    /// sure that only int32_t or uint32_t type is used.
+    ///
+    /// @tparam T int32_t or uint32_t.
+    template<typename T>
+    void bufferToIntTest32() {
+        // Create option that conveys array of multiple uint16_t values.
+        boost::shared_ptr<Option6IntArray<T> > opt;
+        const int opt_len = 40;
+        const uint16_t opt_code = 82;
+
+        // Constructor throws exception if provided buffer is empty.
+        EXPECT_THROW(
+            Option6IntArray<T>(opt_code, buf_.begin(), buf_.begin()),
+            isc::OutOfRange
+        );
+
+        // Constructor throws exception if provided buffer's length is not
+        // multiple of 4-bytes.
+        EXPECT_THROW(
+            Option6IntArray<T>(opt_code, buf_.begin(), buf_.begin() + 9),
+            isc::OutOfRange
+        );
+
+        // Now the buffer length is correct.
+        ASSERT_NO_THROW(
+            opt = boost::shared_ptr<
+                Option6IntArray<T> >(new Option6IntArray<T>(opt_code, buf_.begin(),
+                                                            buf_.begin() + opt_len))
+        );
+
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(opt_code, opt->getType());
+        // Option should return vector of uint32_t values which should be
+        // constructed from the buffer we provided.
+        std::vector<T> values = opt->getValues();
+        ASSERT_EQ(opt_len, values.size() * sizeof(T));
+        // Create reference values from the buffer so as we can
+        // simply compare two vectors.
+        std::vector<T> reference_values;
+        for (int i = 0; i < opt_len; i += 4) {
+            reference_values.push_back((buf_[i] << 24) |
+                                       (buf_[i + 1] << 16 & 0x00FF0000) |
+                                       (buf_[i + 2] << 8 & 0xFF00) |
+                                       (buf_[i + 3] & 0xFF));
+        }
+        EXPECT_TRUE(std::equal(reference_values.begin(), reference_values.end(),
+                               values.begin()));
+
+        // Test for pack()
+        opt->pack(out_buf_);
+
+        // Data length is 40 bytes.
+        EXPECT_EQ(40, opt->len() - opt->getHeaderLen());
+        EXPECT_EQ(opt_code, opt->getType());
+        // The total length is 40 bytes for data and 4 bytes for header.
+        ASSERT_EQ(44, out_buf_.getLength());
+
+        // Check if pack worked properly:
+        InputBuffer out(out_buf_.getData(), out_buf_.getLength());
+        // if option type is correct
+        EXPECT_EQ(opt_code, out.readUint16());
+        // if option length is correct
+        EXPECT_EQ(40, out.readUint16());
+        // if data is correct
+        std::vector<uint8_t> out_data;
+        ASSERT_NO_THROW(out.readVector(out_data, opt_len));
+        ASSERT_EQ(opt_len, out_data.size());
+        EXPECT_TRUE(std::equal(buf_.begin(), buf_.begin() + opt_len, out_data.begin()));;
+    }
+
+
+    OptionBuffer buf_;     ///< Option buffer
+    OutputBuffer out_buf_; ///< Output buffer
+};
+
+/// @todo: below, there is a bunch of tests for options that
+/// convey unsigned values. We should maybe extend these tests for
+/// signed types too.
+
+TEST_F(Option6IntArrayTest, useInvalidType) {
+    const uint16_t opt_code = 80;
+    EXPECT_THROW(
+        boost::scoped_ptr<
+            Option6IntArray<bool> >(new Option6IntArray<bool>(opt_code, OptionBuffer(5))),
+        InvalidDataType
+    );
+
+    EXPECT_THROW(
+        boost::scoped_ptr<
+            Option6IntArray<int64_t> >(new Option6IntArray<int64_t>(opt_code,
+                                                                    OptionBuffer(10))),
+        InvalidDataType
+    );
+
+}
+
+TEST_F(Option6IntArrayTest, bufferToUint8) {
+    bufferToIntTest8<uint8_t>();
+}
+
+TEST_F(Option6IntArrayTest, bufferToInt8) {
+    bufferToIntTest8<int8_t>();
+}
+
+TEST_F(Option6IntArrayTest, bufferToUint16) {
+    bufferToIntTest16<uint16_t>();
+}
+
+TEST_F(Option6IntArrayTest, bufferToInt16) {
+    bufferToIntTest16<int16_t>();
+}
+
+TEST_F(Option6IntArrayTest, bufferToUint32) {
+    bufferToIntTest32<uint32_t>();
+}
+
+TEST_F(Option6IntArrayTest, bufferToInt32) {
+    bufferToIntTest32<int32_t>();
+}
+
+TEST_F(Option6IntArrayTest, setValuesUint8) {
+    const uint16_t opt_code = 100;
+    // Create option with empty vector of values.
+    boost::shared_ptr<Option6IntArray<uint8_t> > opt(new Option6IntArray<uint8_t>(opt_code));
+    // Initialize vector with some data and pass to the option.
+    std::vector<uint8_t> values;
+    for (int i = 0; i < 10; ++i) {
+        values.push_back(numeric_limits<uint8_t>::max() - i);
+    }
+    opt->setValues(values);
+
+    // Check if universe, option type and data was set correctly.
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(opt_code, opt->getType());
+    std::vector<uint8_t> returned_values = opt->getValues();
+    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+}
+
+TEST_F(Option6IntArrayTest, setValuesInt8) {
+    const uint16_t opt_code = 100;
+    // Create option with empty vector of values.
+    boost::shared_ptr<Option6IntArray<int8_t> > opt(new Option6IntArray<int8_t>(opt_code));
+    // Initialize vector with some data and pass to the option.
+    std::vector<int8_t> values;
+    for (int i = 0; i < 10; ++i) {
+        values.push_back(numeric_limits<int8_t>::min() + i);
+    }
+    opt->setValues(values);
+
+    // Check if universe, option type and data was set correctly.
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(opt_code, opt->getType());
+    std::vector<int8_t> returned_values = opt->getValues();
+    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+}
+
+TEST_F(Option6IntArrayTest, setValuesUint16) {
+    const uint16_t opt_code = 101;
+    // Create option with empty vector of values.
+    boost::shared_ptr<Option6IntArray<uint16_t> > opt(new Option6IntArray<uint16_t>(opt_code));
+    // Initialize vector with some data and pass to the option.
+    std::vector<uint16_t> values;
+    for (int i = 0; i < 10; ++i) {
+        values.push_back(numeric_limits<uint16_t>::max() - i);
+    }
+    opt->setValues(values);
+
+    // Check if universe, option type and data was set correctly.
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(opt_code, opt->getType());
+    std::vector<uint16_t> returned_values = opt->getValues();
+    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+}
+
+TEST_F(Option6IntArrayTest, setValuesInt16) {
+    const uint16_t opt_code = 101;
+    // Create option with empty vector of values.
+    boost::shared_ptr<Option6IntArray<int16_t> > opt(new Option6IntArray<int16_t>(opt_code));
+    // Initialize vector with some data and pass to the option.
+    std::vector<int16_t> values;
+    for (int i = 0; i < 10; ++i) {
+        values.push_back(numeric_limits<int16_t>::min() + i);
+    }
+    opt->setValues(values);
+
+    // Check if universe, option type and data was set correctly.
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(opt_code, opt->getType());
+    std::vector<int16_t> returned_values = opt->getValues();
+    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+}
+
+TEST_F(Option6IntArrayTest, setValuesUint32) {
+    const uint32_t opt_code = 101;
+    // Create option with empty vector of values.
+    boost::shared_ptr<Option6IntArray<uint32_t> > opt(new Option6IntArray<uint32_t>(opt_code));
+    // Initialize vector with some data and pass to the option.
+    std::vector<uint32_t> values;
+    for (int i = 0; i < 10; ++i) {
+        values.push_back(numeric_limits<uint32_t>::max() - i);
+    }
+    opt->setValues(values);
+
+    // Check if universe, option type and data was set correctly.
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(opt_code, opt->getType());
+    std::vector<uint32_t> returned_values = opt->getValues();
+    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+}
+
+TEST_F(Option6IntArrayTest, setValuesInt32) {
+    const uint32_t opt_code = 101;
+    // Create option with empty vector of values.
+    boost::shared_ptr<Option6IntArray<int32_t> > opt(new Option6IntArray<int32_t>(opt_code));
+    // Initialize vector with some data and pass to the option.
+    std::vector<int32_t> values;
+    for (int i = 0; i < 10; ++i) {
+        values.push_back(numeric_limits<int32_t>::min() + i);
+    }
+    opt->setValues(values);
+
+    // Check if universe, option type and data was set correctly.
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(opt_code, opt->getType());
+    std::vector<int32_t> returned_values = opt->getValues();
+    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+}
+
+
+} // anonymous namespace

src/lib/dhcp/tests/option6_int_unittest.cc

@@ -0,0 +1,413 @@
+// Copyright (C) 2012 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 <config.h>
+#include <dhcp/dhcp6.h>
+#include <dhcp/option.h>
+#include <dhcp/option6_int.h>
+#include <dhcp/option6_iaaddr.h>
+#include <util/buffer.h>
+
+#include <boost/pointer_cast.hpp>
+#include <gtest/gtest.h>
+
+using namespace std;
+using namespace isc;
+using namespace isc::dhcp;
+using namespace isc::asiolink;
+using namespace isc::util;
+
+namespace {
+
+/// @brief Option6Int test class.
+class Option6IntTest : public ::testing::Test {
+public:
+    /// @brief Constructor.
+    ///
+    /// Initializes the option buffer with some data.
+    Option6IntTest(): buf_(255), out_buf_(255) {
+        for (int i = 0; i < 255; i++) {
+            buf_[i] = 255 - i;
+        }
+    }
+
+    /// @brief Basic test for int8 and uint8 types.
+    ///
+    /// @note this function does not perform type check. Make
+    /// sure that only int8_t or uint8_t type is used.
+    ///
+    /// @tparam T int8_t or uint8_t.
+    template<typename T>
+    void basicTest8() {
+        // Create option that conveys single 8 bit integer value.
+        boost::shared_ptr<Option6Int<T> > opt;
+        // Initialize buffer with this value.
+        buf_[0] = 0xa1;
+        // Constructor may throw in case provided buffer is too short.
+        ASSERT_NO_THROW(
+            opt = boost::shared_ptr<Option6Int<T> >(new Option6Int<T>(D6O_PREFERENCE,
+                                                                      buf_.begin(),
+                                                                      buf_.end()))
+        );
+
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(D6O_PREFERENCE, opt->getType());
+        // Option should return the same value that we initialized the first
+        // byte of the buffer with.
+        EXPECT_EQ(static_cast<T>(0xa1), opt->getValue());
+
+        // test for pack()
+        opt->pack(out_buf_);
+
+        // Data length is 1 byte.
+        EXPECT_EQ(1, opt->len() - opt->getHeaderLen());
+        EXPECT_EQ(D6O_PREFERENCE, opt->getType());
+        // The total length is 1 byte for data and 4 bytes for header.
+        EXPECT_EQ(5, out_buf_.getLength());
+
+        // Check if pack worked properly:
+        InputBuffer out(out_buf_.getData(), out_buf_.getLength());
+        // if option type is correct
+        EXPECT_EQ(D6O_PREFERENCE, out.readUint16());
+        // if option length is correct
+        EXPECT_EQ(1, out.readUint16());
+        // if data is correct
+        EXPECT_EQ(0xa1, out.readUint8() );
+    }
+
+    /// @brief Basic test for int16 and uint16 types.
+    ///
+    /// @note this function does not perform type check. Make
+    /// sure that only int16_t or uint16_t type is used.
+    ///
+    /// @tparam T int16_t or uint16_t.
+    template<typename T>
+    void basicTest16() {
+        // Create option that conveys single 16-bit integer value.
+        boost::shared_ptr<Option6Int<T> > opt;
+        // Initialize buffer with uint16_t value.
+        buf_[0] = 0xa1;
+        buf_[1] = 0xa2;
+        // Constructor may throw in case provided buffer is too short.
+        ASSERT_NO_THROW(
+            opt = boost::shared_ptr<Option6Int<T> >(new Option6Int<T>(D6O_ELAPSED_TIME,
+                                                                      buf_.begin(),
+                                                                      buf_.end()))
+        );
+
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(D6O_ELAPSED_TIME, opt->getType());
+        // Option should return the value equal to the contents of first
+        // and second byte of the buffer.
+        EXPECT_EQ(static_cast<T>(0xa1a2), opt->getValue());
+
+        // Test for pack()
+        opt->pack(out_buf_);
+
+        // Data length is 2 bytes.
+        EXPECT_EQ(2, opt->len() - opt->getHeaderLen());
+        EXPECT_EQ(D6O_ELAPSED_TIME, opt->getType());
+        // The total length is 2 byte for data and 4 bytes for header.
+        EXPECT_EQ(6, out_buf_.getLength());
+
+        // Check if pack worked properly:
+        InputBuffer out(out_buf_.getData(), out_buf_.getLength());
+        // if option type is correct
+        EXPECT_EQ(D6O_ELAPSED_TIME, out.readUint16());
+        // if option length is correct
+        EXPECT_EQ(2, out.readUint16());
+        // if data is correct
+        EXPECT_EQ(0xa1a2, out.readUint16() );
+    }
+
+    /// @brief Basic test for int32 and uint32 types.
+    ///
+    /// @note this function does not perform type check. Make
+    /// sure that only int32_t or uint32_t type is used.
+    ///
+    /// @tparam T int32_t or uint32_t.
+    template<typename T>
+    void basicTest32() {
+        // Create option that conveys single 32-bit integer value.
+        boost::shared_ptr<Option6Int<T> > opt;
+        // Initialize buffer with 32-bit integer value.
+        buf_[0] = 0xa1;
+        buf_[1] = 0xa2;
+        buf_[2] = 0xa3;
+        buf_[3] = 0xa4;
+        // Constructor may throw in case provided buffer is too short.
+        ASSERT_NO_THROW(
+                        opt = boost::shared_ptr<Option6Int<T> >(new Option6Int<T>(D6O_CLT_TIME,
+                                                                                  buf_.begin(),
+                                                                                  buf_.end()))
+                        );
+
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(D6O_CLT_TIME, opt->getType());
+        // Option should return the value equal to the value made of
+        // first 4 bytes of the buffer.
+        EXPECT_EQ(static_cast<T>(0xa1a2a3a4), opt->getValue());
+
+        // Test for pack()
+        opt->pack(out_buf_);
+
+        // Data length is 4 bytes.
+        EXPECT_EQ(4, opt->len() - opt->getHeaderLen());
+        EXPECT_EQ(D6O_CLT_TIME, opt->getType());
+        // The total length is 4 bytes for data and 4 bytes for header.
+        EXPECT_EQ(8, out_buf_.getLength());
+
+        // Check if pack worked properly:
+        InputBuffer out(out_buf_.getData(), out_buf_.getLength());
+        // if option type is correct
+        EXPECT_EQ(D6O_CLT_TIME, out.readUint16());
+        // if option length is correct
+        EXPECT_EQ(4, out.readUint16());
+        // if data is correct
+        EXPECT_EQ(0xa1a2a3a4, out.readUint32());
+    }
+
+    OptionBuffer buf_;     ///< Option buffer
+    OutputBuffer out_buf_; ///< Output buffer
+};
+
+/// @todo: below, there is a bunch of tests for options that
+/// convey unsigned value. We should maybe extend these tests for
+/// signed types too.
+
+TEST_F(Option6IntTest, useInvalidType) {
+    EXPECT_THROW(
+        boost::scoped_ptr<Option6Int<bool> >(new Option6Int<bool>(D6O_ELAPSED_TIME, 10)),
+        InvalidDataType
+    );
+
+    EXPECT_THROW(
+        boost::scoped_ptr<Option6Int<int64_t> >(new Option6Int<int64_t>(D6O_ELAPSED_TIME, 10)),
+        InvalidDataType
+    );
+
+}
+
+TEST_F(Option6IntTest, basicUint8) {
+    basicTest8<uint8_t>();
+}
+
+TEST_F(Option6IntTest, basicUint16) {
+    basicTest16<uint16_t>();
+}
+
+TEST_F(Option6IntTest, basicUint32) {
+    basicTest32<uint32_t>();
+}
+
+TEST_F(Option6IntTest, basicInt8) {
+    basicTest8<int8_t>();
+}
+
+TEST_F(Option6IntTest, basicInt16) {
+    basicTest16<int16_t>();
+}
+
+TEST_F(Option6IntTest, basicInt32) {
+    basicTest32<int32_t>();
+}
+
+TEST_F(Option6IntTest, setValueUint8) {
+    boost::shared_ptr<Option6Int<uint8_t> > opt(new Option6Int<uint8_t>(D6O_PREFERENCE, 123));
+    // Check if constructor intitialized the option value correctly.
+    EXPECT_EQ(123, opt->getValue());
+    // Override the value.
+    opt->setValue(111);
+
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(D6O_PREFERENCE, opt->getType());
+    // Check if the value has been overriden.
+    EXPECT_EQ(111, opt->getValue());
+}
+
+TEST_F(Option6IntTest, setValueInt8) {
+    boost::shared_ptr<Option6Int<int8_t> > opt(new Option6Int<int8_t>(D6O_PREFERENCE, -123));
+    // Check if constructor intitialized the option value correctly.
+    EXPECT_EQ(-123, opt->getValue());
+    // Override the value.
+    opt->setValue(-111);
+
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(D6O_PREFERENCE, opt->getType());
+    // Check if the value has been overriden.
+    EXPECT_EQ(-111, opt->getValue());
+}
+
+
+TEST_F(Option6IntTest, setValueUint16) {
+    boost::shared_ptr<Option6Int<uint16_t> > opt(new Option6Int<uint16_t>(D6O_ELAPSED_TIME, 123));
+    // Check if constructor intitialized the option value correctly.
+    EXPECT_EQ(123, opt->getValue());
+    // Override the value.
+    opt->setValue(0x0102);
+
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(D6O_ELAPSED_TIME, opt->getType());
+    // Check if the value has been overriden.
+    EXPECT_EQ(0x0102, opt->getValue());
+}
+
+TEST_F(Option6IntTest, setValueInt16) {
+    boost::shared_ptr<Option6Int<int16_t> > opt(new Option6Int<int16_t>(D6O_ELAPSED_TIME, -16500));
+    // Check if constructor intitialized the option value correctly.
+    EXPECT_EQ(-16500, opt->getValue());
+    // Override the value.
+    opt->setValue(-20100);
+
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(D6O_ELAPSED_TIME, opt->getType());
+    // Check if the value has been overriden.
+    EXPECT_EQ(-20100, opt->getValue());
+}
+
+TEST_F(Option6IntTest, setValueUint32) {
+    boost::shared_ptr<Option6Int<uint32_t> > opt(new Option6Int<uint32_t>(D6O_CLT_TIME, 123));
+    // Check if constructor intitialized the option value correctly.
+    EXPECT_EQ(123, opt->getValue());
+    // Override the value.
+    opt->setValue(0x01020304);
+
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(D6O_CLT_TIME, opt->getType());
+    // Check if the value has been overriden.
+    EXPECT_EQ(0x01020304, opt->getValue());
+}
+
+TEST_F(Option6IntTest, setValueint32) {
+    boost::shared_ptr<Option6Int<int32_t> > opt(new Option6Int<int32_t>(D6O_CLT_TIME, -120100));
+    // Check if constructor intitialized the option value correctly.
+    EXPECT_EQ(-120100, opt->getValue());
+    // Override the value.
+    opt->setValue(-125000);
+
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(D6O_CLT_TIME, opt->getType());
+    // Check if the value has been overriden.
+    EXPECT_EQ(-125000, opt->getValue());
+}
+
+TEST_F(Option6IntTest, packSuboptions) {
+    // option code is really uint16_t, but using uint8_t
+    // for easier conversion to uint8_t array.
+    uint8_t opt_code = 80;
+
+    boost::shared_ptr<Option6Int<uint32_t> > opt(new Option6Int<uint32_t>(opt_code, 0x01020304));
+    OptionPtr sub1(new Option(Option::V6, 0xcafe));
+
+    boost::shared_ptr<Option6IAAddr> addr1(
+        new Option6IAAddr(D6O_IAADDR, IOAddress("2001:db8:1234:5678::abcd"), 0x5000, 0x7000));
+
+    opt->addOption(sub1);
+    opt->addOption(addr1);
+
+    ASSERT_EQ(28, addr1->len());
+    ASSERT_EQ(4, sub1->len());
+    ASSERT_EQ(40, opt->len());
+
+    uint8_t expected[] = {
+        0, opt_code, // type
+        0, 36, // length
+        0x01, 0x02, 0x03, 0x04, // uint32_t value
+
+        // iaaddr suboption
+        D6O_IAADDR / 256, D6O_IAADDR % 256, // type
+        0, 24, // len
+        0x20, 0x01, 0xd, 0xb8, 0x12,0x34, 0x56, 0x78,
+        0, 0, 0, 0, 0, 0, 0xab, 0xcd, // IP address
+        0, 0, 0x50, 0, // preferred-lifetime
+        0, 0, 0x70, 0, // valid-lifetime
+
+        // suboption
+        0xca, 0xfe, // type
+        0, 0 // len
+    };
+
+    // Create on-wire format of option and suboptions.
+    opt->pack(out_buf_);
+    // Compare the on-wire data with the reference buffer.
+    ASSERT_EQ(40, out_buf_.getLength());
+    EXPECT_EQ(0, memcmp(out_buf_.getData(), expected, 40));
+}
+
+
+TEST_F(Option6IntTest, unpackSuboptions) {
+    // option code is really uint16_t, but using uint8_t
+    // for easier conversion to uint8_t array.
+    const uint8_t opt_code = 80;
+    // Prepare reference data.
+    uint8_t expected[] = {
+        0, opt_code, // type
+        0, 34, // length
+        0x01, 0x02, // uint16_t value
+
+        // iaaddr suboption
+        D6O_IAADDR / 256, D6O_IAADDR % 256, // type
+        0, 24, // len
+        0x20, 0x01, 0xd, 0xb8, 0x12,0x34, 0x56, 0x78,
+        0, 0, 0, 0, 0, 0, 0xab, 0xcd, // IP address
+        0, 0, 0x50, 0, // preferred-lifetime
+        0, 0, 0x70, 0, // valid-lifetime
+
+        // suboption
+        0xca, 0xfe, // type
+        0, 0 // len
+    };
+    ASSERT_EQ(38, sizeof(expected));
+
+    memcpy(&buf_[0], expected, sizeof(expected));
+
+    boost::shared_ptr<Option6Int<uint16_t> > opt;
+    EXPECT_NO_THROW(
+        opt = boost::shared_ptr<
+            Option6Int<uint16_t> >(new Option6Int<uint16_t>(opt_code, buf_.begin() + 4,
+                                                            buf_.begin() + sizeof(expected)));
+    );
+    ASSERT_TRUE(opt);
+
+    EXPECT_EQ(opt_code, opt->getType());
+    EXPECT_EQ(0x0102, opt->getValue());
+
+    // Checks for address option
+    OptionPtr subopt = opt->getOption(D6O_IAADDR);
+    ASSERT_TRUE(subopt);
+    boost::shared_ptr<Option6IAAddr> addr(boost::dynamic_pointer_cast<Option6IAAddr>(subopt));
+    ASSERT_TRUE(addr);
+
+    EXPECT_EQ(D6O_IAADDR, addr->getType());
+    EXPECT_EQ(28, addr->len());
+    EXPECT_EQ(0x5000, addr->getPreferred());
+    EXPECT_EQ(0x7000, addr->getValid());
+    EXPECT_EQ("2001:db8:1234:5678::abcd", addr->getAddress().toText());
+
+    // Checks for dummy option
+    subopt = opt->getOption(0xcafe);
+    ASSERT_TRUE(subopt); // should be non-NULL
+
+    EXPECT_EQ(0xcafe, subopt->getType());
+    EXPECT_EQ(4, subopt->len());
+    // There should be no data at all
+    EXPECT_EQ(0, subopt->getData().size());
+
+    // Try to get non-existent option.
+    subopt = opt->getOption(1);
+    // Expecting NULL which means that option does not exist.
+    ASSERT_FALSE(subopt);
+}
+
+} // anonymous namespace

src/lib/dhcp/tests/option_definition_unittest.cc

@@ -0,0 +1,610 @@
+// Copyright (C) 2012 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 <config.h>
+
+#include <exceptions/exceptions.h>
+#include <asiolink/io_address.h>
+#include <dhcp/dhcp4.h>
+#include <dhcp/dhcp6.h>
+#include <dhcp/option4_addrlst.h>
+#include <dhcp/option6_addrlst.h>
+#include <dhcp/option6_ia.h>
+#include <dhcp/option6_iaaddr.h>
+#include <dhcp/option6_int.h>
+#include <dhcp/option6_int_array.h>
+#include <dhcp/option_definition.h>
+
+#include <gtest/gtest.h>
+#include <boost/shared_ptr.hpp>
+#include <boost/pointer_cast.hpp>
+
+using namespace std;
+using namespace isc;
+using namespace isc::dhcp;
+using namespace isc::util;
+
+namespace {
+
+/// @brief OptionDefinition test class.
+///
+/// This class does not do anything useful but we keep
+/// it around for the future.
+class OptionDefinitionTest : public ::testing::Test {
+public:
+    // @brief Constructor.
+    OptionDefinitionTest() { }
+};
+
+TEST_F(OptionDefinitionTest, constructor) {
+    // Specify the option data type as string. This should get converted
+    // to enum value returned by getType().
+    OptionDefinition opt_def1("OPTION_CLIENTID", 1, "string");
+    EXPECT_EQ("OPTION_CLIENTID", opt_def1.getName());
+    EXPECT_EQ(1, opt_def1.getCode());
+    EXPECT_EQ(OptionDefinition::STRING_TYPE,  opt_def1.getType());
+    EXPECT_FALSE(opt_def1.getArrayType());
+    EXPECT_NO_THROW(opt_def1.validate());
+
+    // Specify the option data type as an enum value.
+    OptionDefinition opt_def2("OPTION_RAPID_COMMIT", 14,
+                              OptionDefinition::EMPTY_TYPE);
+    EXPECT_EQ("OPTION_RAPID_COMMIT", opt_def2.getName());
+    EXPECT_EQ(14, opt_def2.getCode());
+    EXPECT_EQ(OptionDefinition::EMPTY_TYPE, opt_def2.getType());
+    EXPECT_FALSE(opt_def2.getArrayType());
+    EXPECT_NO_THROW(opt_def1.validate());
+
+    // Check if it is possible to set that option is an array.
+    OptionDefinition opt_def3("OPTION_NIS_SERVERS", 27,
+                              OptionDefinition::IPV6_ADDRESS_TYPE,
+                              true);
+    EXPECT_EQ("OPTION_NIS_SERVERS", opt_def3.getName());
+    EXPECT_EQ(27, opt_def3.getCode());
+    EXPECT_EQ(OptionDefinition::IPV6_ADDRESS_TYPE, opt_def3.getType());
+    EXPECT_TRUE(opt_def3.getArrayType());
+    EXPECT_NO_THROW(opt_def3.validate());
+
+    // The created object is invalid if invalid data type is specified but
+    // constructor shouldn't throw exception. The object is validated after
+    // it has been created.
+    EXPECT_NO_THROW(
+        OptionDefinition opt_def4("OPTION_SERVERID",
+                                  OptionDefinition::UNKNOWN_TYPE + 10,
+                                  OptionDefinition::STRING_TYPE);
+    );
+}
+
+TEST_F(OptionDefinitionTest, addRecordField) {
+    // We can only add fields to record if the option type has been
+    // specified as 'record'. We try all other types but 'record'
+    // here and expect exception to be thrown.
+    for (int i = 0; i < OptionDefinition::UNKNOWN_TYPE; ++i) {
+        // Do not try for 'record' type because this is the only
+        // type for which adding record will succeed.
+        if (i == OptionDefinition::RECORD_TYPE) {
+            continue;
+        }
+        OptionDefinition opt_def("OPTION_IAADDR", 5,
+                                 static_cast<OptionDefinition::DataType>(i));
+        EXPECT_THROW(opt_def.addRecordField("uint8"), isc::InvalidOperation);
+    }
+
+    // Positive scenario starts here.
+    OptionDefinition opt_def("OPTION_IAADDR", 5, "record");
+    EXPECT_NO_THROW(opt_def.addRecordField("ipv6-address"));
+    EXPECT_NO_THROW(opt_def.addRecordField("uint32"));
+    // It should not matter if we specify field type by its name or using enum.
+    EXPECT_NO_THROW(opt_def.addRecordField(OptionDefinition::UINT32_TYPE));
+
+    // Check what we have actually added.
+    OptionDefinition::RecordFieldsCollection fields = opt_def.getRecordFields();
+    ASSERT_EQ(3, fields.size());
+    EXPECT_EQ(OptionDefinition::IPV6_ADDRESS_TYPE, fields[0]);
+    EXPECT_EQ(OptionDefinition::UINT32_TYPE, fields[1]);
+    EXPECT_EQ(OptionDefinition::UINT32_TYPE, fields[2]);
+
+    // Let's try some more negative scenarios: use invalid data types.
+    EXPECT_THROW(opt_def.addRecordField("unknown_type_xyz"), isc::BadValue);
+    OptionDefinition::DataType invalid_type =
+        static_cast<OptionDefinition::DataType>(OptionDefinition::UNKNOWN_TYPE + 10);
+    EXPECT_THROW(opt_def.addRecordField(invalid_type), isc::BadValue);
+}
+
+TEST_F(OptionDefinitionTest, validate) {
+    // Not supported option type string is not allowed.
+    OptionDefinition opt_def1("OPTION_CLIENTID", D6O_CLIENTID, "non-existent-type");
+    EXPECT_THROW(opt_def1.validate(), isc::OutOfRange);
+
+    // Not supported option type enum value is not allowed.
+    OptionDefinition opt_def2("OPTION_CLIENTID", D6O_CLIENTID, OptionDefinition::UNKNOWN_TYPE);
+    EXPECT_THROW(opt_def2.validate(), isc::OutOfRange);
+
+    OptionDefinition opt_def3("OPTION_CLIENTID", D6O_CLIENTID,
+                              static_cast<OptionDefinition::DataType>(OptionDefinition::UNKNOWN_TYPE
+                                                                      + 2));
+    EXPECT_THROW(opt_def3.validate(), isc::OutOfRange);
+    
+    // Empty option name is not allowed.
+    OptionDefinition opt_def4("", D6O_CLIENTID, "string");
+    EXPECT_THROW(opt_def4.validate(), isc::BadValue);
+
+    // Option name must not contain spaces.
+    OptionDefinition opt_def5(" OPTION_CLIENTID", D6O_CLIENTID, "string");
+    EXPECT_THROW(opt_def5.validate(), isc::BadValue);
+
+    OptionDefinition opt_def6("OPTION CLIENTID", D6O_CLIENTID, "string");
+    EXPECT_THROW(opt_def6.validate(), isc::BadValue);
+}
+
+TEST_F(OptionDefinitionTest, factoryAddrList6) {
+    OptionDefinition opt_def("OPTION_NIS_SERVERS", D6O_NIS_SERVERS,
+                             "ipv6-address", true);
+    Option::Factory* factory(NULL);
+    EXPECT_NO_THROW(factory = opt_def.getFactory());
+    ASSERT_TRUE(factory != NULL);
+
+    // Create a list of some V6 addresses.
+    std::vector<asiolink::IOAddress> addrs;
+    addrs.push_back(asiolink::IOAddress("2001:0db8::ff00:0042:8329"));
+    addrs.push_back(asiolink::IOAddress("2001:0db8::ff00:0042:2319"));
+    addrs.push_back(asiolink::IOAddress("::1"));
+    addrs.push_back(asiolink::IOAddress("::2"));
+
+    // Write addresses to the buffer.
+    OptionBuffer buf(addrs.size() * asiolink::V6ADDRESS_LEN);
+    for (int i = 0; i < addrs.size(); ++i) {
+        asio::ip::address_v6::bytes_type addr_bytes =
+            addrs[i].getAddress().to_v6().to_bytes();
+        ASSERT_EQ(asiolink::V6ADDRESS_LEN, addr_bytes.size());
+        std::copy(addr_bytes.begin(), addr_bytes.end(),
+                  buf.begin() + i * asiolink::V6ADDRESS_LEN);
+    }
+    // Create DHCPv6 option from this buffer. Once option is created it is
+    // supposed to have internal list of addresses that it parses out from
+    // the provided buffer.
+    OptionPtr option_v6;
+    ASSERT_NO_THROW(
+        option_v6 = factory(Option::V6, D6O_NIS_SERVERS, buf);
+    );
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6AddrLst));
+    boost::shared_ptr<Option6AddrLst> option_cast_v6 =
+        boost::static_pointer_cast<Option6AddrLst>(option_v6);
+    ASSERT_TRUE(option_cast_v6);
+    // Get the list of parsed addresses from the option object.
+    std::vector<asiolink::IOAddress> addrs_returned =
+        option_cast_v6->getAddresses();
+    // The list of addresses must exactly match addresses that we
+    // stored in the buffer to create the option from it.
+    EXPECT_TRUE(std::equal(addrs.begin(), addrs.end(), addrs_returned.begin()));
+
+    // The provided buffer's length must be a multiple of V6 address length.
+    // Let's extend the buffer by one byte so as this condition is not
+    // fulfilled anymore.
+    buf.insert(buf.end(), 1, 1);
+    // It should throw exception then.
+    EXPECT_THROW(
+        factory(Option::V6, D6O_NIS_SERVERS, buf),
+        isc::OutOfRange
+    );
+}
+
+TEST_F(OptionDefinitionTest, factoryAddrList4) {
+    OptionDefinition opt_def("OPTION_NAME_SERVERS", D6O_NIS_SERVERS,
+                             "ipv4-address", true);
+    Option::Factory* factory(NULL);
+    EXPECT_NO_THROW(factory = opt_def.getFactory());
+    ASSERT_TRUE(factory != NULL);
+
+    // Create a list of some V6 addresses.
+    std::vector<asiolink::IOAddress> addrs;
+    addrs.push_back(asiolink::IOAddress("192.168.0.1"));
+    addrs.push_back(asiolink::IOAddress("172.16.1.1"));
+    addrs.push_back(asiolink::IOAddress("127.0.0.1"));
+    addrs.push_back(asiolink::IOAddress("213.41.23.12"));
+
+    // Write addresses to the buffer.
+    OptionBuffer buf(addrs.size() * asiolink::V4ADDRESS_LEN);
+    for (int i = 0; i < addrs.size(); ++i) {
+        asio::ip::address_v4::bytes_type addr_bytes =
+            addrs[i].getAddress().to_v4().to_bytes();
+        ASSERT_EQ(asiolink::V4ADDRESS_LEN, addr_bytes.size());
+        std::copy(addr_bytes.begin(), addr_bytes.end(),
+                  buf.begin() + i * asiolink::V4ADDRESS_LEN);
+    }
+    // Create DHCPv6 option from this buffer. Once option is created it is
+    // supposed to have internal list of addresses that it parses out from
+    // the provided buffer.
+    OptionPtr option_v4;
+    ASSERT_NO_THROW(
+        option_v4 = factory(Option::V4, DHO_NAME_SERVERS, buf)
+    );
+    ASSERT_TRUE(typeid(*option_v4) == typeid(Option4AddrLst));
+    // Get the list of parsed addresses from the option object.
+    boost::shared_ptr<Option4AddrLst> option_cast_v4 =
+        boost::static_pointer_cast<Option4AddrLst>(option_v4);
+    std::vector<asiolink::IOAddress> addrs_returned =
+        option_cast_v4->getAddresses();
+    // The list of addresses must exactly match addresses that we
+    // stored in the buffer to create the option from it.
+    EXPECT_TRUE(std::equal(addrs.begin(), addrs.end(), addrs_returned.begin()));
+
+    // The provided buffer's length must be a multiple of V4 address length.
+    // Let's extend the buffer by one byte so as this condition is not
+    // fulfilled anymore.
+    buf.insert(buf.end(), 1, 1);
+    // It should throw exception then.
+    EXPECT_THROW(factory(Option::V4, DHO_NIS_SERVERS, buf), isc::OutOfRange);
+}
+
+TEST_F(OptionDefinitionTest, factoryEmpty) {
+    OptionDefinition opt_def("OPTION_RAPID_COMMIT", D6O_RAPID_COMMIT, "empty");
+    Option::Factory* factory(NULL);
+    EXPECT_NO_THROW(factory = opt_def.getFactory());
+    ASSERT_TRUE(factory != NULL);
+
+    // Create option instance and provide empty buffer as expected.
+    OptionPtr option_v6;
+    ASSERT_NO_THROW(
+        option_v6 = factory(Option::V6, D6O_RAPID_COMMIT, OptionBuffer())
+    );
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option));
+    // Expect 'empty' DHCPv6 option.
+    EXPECT_EQ(Option::V6, option_v6->getUniverse());
+    EXPECT_EQ(4, option_v6->getHeaderLen());
+    EXPECT_EQ(0, option_v6->getData().size());
+
+    // Repeat the same test scenario for DHCPv4 option.
+    EXPECT_THROW(factory(Option::V4, 214, OptionBuffer(2)),isc::BadValue);
+
+    OptionPtr option_v4;
+    ASSERT_NO_THROW(option_v4 = factory(Option::V4, 214, OptionBuffer()));
+    // Expect 'empty' DHCPv4 option.
+    EXPECT_EQ(Option::V4, option_v4->getUniverse());
+    EXPECT_EQ(2, option_v4->getHeaderLen());
+    EXPECT_EQ(0, option_v4->getData().size());
+
+    // This factory produces empty option (consisting of option type
+    // and length). Attempt to provide some data in the buffer should
+    // result in exception.
+    EXPECT_THROW(factory(Option::V6, D6O_RAPID_COMMIT,OptionBuffer(2)),isc::BadValue);
+}
+
+TEST_F(OptionDefinitionTest, factoryIA6) {
+    // This option consists of IAID, T1 and T2 fields (each 4 bytes long).
+    const int option6_ia_len = 12;
+
+    // Get the factory function pointer.
+    OptionDefinition opt_def("OPTION_IA_NA", D6O_IA_NA, "record", true);
+    // Each data field is uint32.
+    for (int i = 0; i < 3; ++i) {
+        EXPECT_NO_THROW(opt_def.addRecordField("uint32"));
+    }
+    Option::Factory* factory(NULL);
+    EXPECT_NO_THROW(factory = opt_def.getFactory());
+    ASSERT_TRUE(factory != NULL);
+
+    // Check the positive scenario.
+    OptionBuffer buf(12);
+    for (int i = 0; i < buf.size(); ++i) {
+        buf[i] = i;
+    }
+    OptionPtr option_v6;
+    ASSERT_NO_THROW(option_v6 = factory(Option::V6, D6O_IA_NA, buf));
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6IA));
+    boost::shared_ptr<Option6IA> option_cast_v6 =
+        boost::static_pointer_cast<Option6IA>(option_v6);
+    EXPECT_EQ(0x00010203, option_cast_v6->getIAID());
+    EXPECT_EQ(0x04050607, option_cast_v6->getT1());
+    EXPECT_EQ(0x08090A0B, option_cast_v6->getT2());
+
+    // This should work for DHCPv6 only, try passing invalid universe value.
+    EXPECT_THROW(
+        factory(Option::V4, D6O_IA_NA, OptionBuffer(option6_ia_len)),
+        isc::BadValue
+    );
+    // The length of the buffer must be 12 bytes.
+    // Check too short buffer.
+    EXPECT_THROW(
+        factory(Option::V6, D6O_IA_NA, OptionBuffer(option6_ia_len - 1)),
+        isc::OutOfRange
+     );
+    // Check too long buffer.
+    EXPECT_THROW(
+        factory(Option::V6, D6O_IA_NA, OptionBuffer(option6_ia_len + 1)),
+        isc::OutOfRange
+    );
+}
+
+TEST_F(OptionDefinitionTest, factoryIAAddr6) {
+    // This option consists of IPV6 Address (16 bytes) and preferred-lifetime and
+    // valid-lifetime fields (each 4 bytes long).
+    const int option6_iaaddr_len = 24;
+
+    OptionDefinition opt_def("OPTION_IAADDR", D6O_IAADDR, "record");
+    ASSERT_NO_THROW(opt_def.addRecordField("ipv6-address"));
+    ASSERT_NO_THROW(opt_def.addRecordField("uint32"));
+    ASSERT_NO_THROW(opt_def.addRecordField("uint32"));
+    Option::Factory* factory(NULL);
+    EXPECT_NO_THROW(factory = opt_def.getFactory());
+    ASSERT_TRUE(factory != NULL);
+
+    // Check the positive scenario.
+    OptionPtr option_v6;
+    asiolink::IOAddress addr_v6("2001:0db8::ff00:0042:8329");
+    OptionBuffer buf(asiolink::V6ADDRESS_LEN);
+    ASSERT_TRUE(addr_v6.getAddress().is_v6());
+    asio::ip::address_v6::bytes_type addr_bytes =
+        addr_v6.getAddress().to_v6().to_bytes();
+    ASSERT_EQ(asiolink::V6ADDRESS_LEN, addr_bytes.size());
+    std::copy(addr_bytes.begin(), addr_bytes.end(), buf.begin());
+
+    for (int i = 0; i < option6_iaaddr_len - asiolink::V6ADDRESS_LEN; ++i) {
+        buf.push_back(i);
+    }
+    ASSERT_NO_THROW(option_v6 = factory(Option::V6, D6O_IAADDR, buf));
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6IAAddr));
+    boost::shared_ptr<Option6IAAddr> option_cast_v6 =
+        boost::static_pointer_cast<Option6IAAddr>(option_v6);
+    EXPECT_EQ(addr_v6, option_cast_v6->getAddress());
+    EXPECT_EQ(0x00010203, option_cast_v6->getPreferred());
+    EXPECT_EQ(0x04050607, option_cast_v6->getValid());
+
+    // This should work for DHCPv6 only, try passing invalid universe value.
+    EXPECT_THROW(
+        factory(Option::V4, D6O_IAADDR, OptionBuffer(option6_iaaddr_len)),
+        isc::BadValue
+    );
+    // The length of the buffer must be 12 bytes.
+    // Check too short buffer.
+    EXPECT_THROW(
+        factory(Option::V6, D6O_IAADDR, OptionBuffer(option6_iaaddr_len - 1)),
+        isc::OutOfRange
+     );
+    // Check too long buffer.
+    EXPECT_THROW(
+        factory(Option::V6, D6O_IAADDR, OptionBuffer(option6_iaaddr_len + 1)),
+        isc::OutOfRange
+    );
+}
+
+TEST_F(OptionDefinitionTest, factoryIntegerInvalidType) {
+    // The template function factoryInteger<> accepts integer values only
+    // as template typename. Here we try passing different type and
+    // see if it rejects it.
+    EXPECT_THROW(
+        OptionDefinition::factoryInteger<bool>(Option::V6, D6O_PREFERENCE, OptionBuffer(1)),
+        isc::dhcp::InvalidDataType
+    );
+}
+
+TEST_F(OptionDefinitionTest, factoryUint8) {
+    OptionDefinition opt_def("OPTION_PREFERENCE", D6O_PREFERENCE, "uint8");
+    Option::Factory* factory(NULL);
+    EXPECT_NO_THROW(factory = opt_def.getFactory());
+    ASSERT_TRUE(factory != NULL);
+
+    OptionPtr option_v6;
+    // Try to use correct buffer length = 1 byte.
+    ASSERT_NO_THROW(
+        option_v6 = factory(Option::V6, D6O_PREFERENCE, OptionBuffer(1, 1));
+    );
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6Int<uint8_t>));
+    // Validate the value.
+    boost::shared_ptr<Option6Int<uint8_t> > option_cast_v6 =
+        boost::static_pointer_cast<Option6Int<uint8_t> >(option_v6);
+    EXPECT_EQ(1, option_cast_v6->getValue());
+
+    // Try to provide too large buffer. Expect exception.
+    EXPECT_THROW(
+        option_v6 = factory(Option::V6, D6O_PREFERENCE, OptionBuffer(3)),
+        isc::OutOfRange
+    );
+
+    // Try to provide zero-length buffer. Expect exception.
+    EXPECT_THROW(
+        option_v6 = factory(Option::V6, D6O_PREFERENCE, OptionBuffer()),
+        isc::OutOfRange
+    );
+
+    // @todo Add more cases for DHCPv4
+}
+
+TEST_F(OptionDefinitionTest, factoryUint16) {
+    OptionDefinition opt_def("OPTION_ELAPSED_TIME", D6O_ELAPSED_TIME, "uint16");
+    Option::Factory* factory(NULL);
+    EXPECT_NO_THROW(factory = opt_def.getFactory());
+    ASSERT_TRUE(factory != NULL);
+
+    OptionPtr option_v6;
+    // Try to use correct buffer length = 2 bytes.
+    OptionBuffer buf;
+    buf.push_back(1);
+    buf.push_back(2);
+    ASSERT_NO_THROW(
+        option_v6 = factory(Option::V6, D6O_ELAPSED_TIME, buf);
+    );
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6Int<uint16_t>));
+    // Validate the value.
+    boost::shared_ptr<Option6Int<uint16_t> > option_cast_v6 =
+        boost::static_pointer_cast<Option6Int<uint16_t> >(option_v6);
+    EXPECT_EQ(0x0102, option_cast_v6->getValue());
+
+    // Try to provide too large buffer. Expect exception.
+    EXPECT_THROW(
+        option_v6 = factory(Option::V6, D6O_ELAPSED_TIME, OptionBuffer(3)),
+        isc::OutOfRange
+    );
+    // Try to provide zero-length buffer. Expect exception.
+    EXPECT_THROW(
+        option_v6 = factory(Option::V6, D6O_ELAPSED_TIME, OptionBuffer(1)),
+        isc::OutOfRange
+    );
+
+    // @todo Add more cases for DHCPv4
+}
+
+TEST_F(OptionDefinitionTest, factoryUint32) {
+    OptionDefinition opt_def("OPTION_CLT_TIME", D6O_CLT_TIME, "uint32");
+    Option::Factory* factory(NULL);
+    EXPECT_NO_THROW(factory = opt_def.getFactory());
+    ASSERT_TRUE(factory != NULL);
+
+    OptionPtr option_v6;
+    OptionBuffer buf;
+    buf.push_back(1);
+    buf.push_back(2);
+    buf.push_back(3);
+    buf.push_back(4);
+    ASSERT_NO_THROW(
+        option_v6 = factory(Option::V6, D6O_CLT_TIME, buf);
+    );
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6Int<uint32_t>));
+    // Validate the value.
+    boost::shared_ptr<Option6Int<uint32_t> > option_cast_v6 =
+        boost::static_pointer_cast<Option6Int<uint32_t> >(option_v6);
+    EXPECT_EQ(0x01020304, option_cast_v6->getValue());
+
+    // Try to provide too large buffer. Expect exception.
+    EXPECT_THROW(
+        option_v6 = factory(Option::V6, D6O_CLT_TIME, OptionBuffer(5)),
+        isc::OutOfRange
+    );
+    // Try to provide zero-length buffer. Expect exception.
+    EXPECT_THROW(
+        option_v6 = factory(Option::V6, D6O_CLT_TIME, OptionBuffer(2)),
+        isc::OutOfRange
+    );
+
+    // @todo Add more cases for DHCPv4
+}
+
+TEST_F(OptionDefinitionTest, factoryUint16Array) {
+    // Let's define some dummy option.
+    const uint16_t opt_code = 79;
+    OptionDefinition opt_def("OPTION_UINT16_ARRAY", opt_code, "uint16", true);
+    Option::Factory* factory(NULL);
+    EXPECT_NO_THROW(factory = opt_def.getFactory());
+    ASSERT_TRUE(factory != NULL);
+
+    OptionPtr option_v6;
+    // Positive scenario, initiate the buffer with length being
+    // multiple of uint16_t size.
+    // buffer elements will be: 0x112233.
+    OptionBuffer buf(6);
+    for (int i = 0; i < 6; ++i) {
+        buf[i] = i / 2;
+    }
+    // Constructor should succeed because buffer has correct size.
+    EXPECT_NO_THROW(
+        option_v6 = factory(Option::V6, opt_code, buf);
+    );
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6IntArray<uint16_t>));
+    boost::shared_ptr<Option6IntArray<uint16_t> > option_cast_v6 =
+        boost::static_pointer_cast<Option6IntArray<uint16_t> >(option_v6);
+    // Get the values from the initiated options and validate.
+    std::vector<uint16_t> values = option_cast_v6->getValues();
+    for (int i = 0; i < values.size(); ++i) {
+        // Expected value is calculated using on the same pattern
+        // as the one we used to initiate buffer:
+        // for i=0, expected = 0x00, for i = 1, expected == 0x11 etc.
+        uint16_t expected = (i << 8) | i;
+        EXPECT_EQ(expected, values[i]);
+    }
+
+    // Provided buffer size must be greater than zero. Check if we
+    // get exception if we provide zero-length buffer.
+    EXPECT_THROW(
+        option_v6 = factory(Option::V6, opt_code, OptionBuffer()),
+        isc::OutOfRange
+    );
+    // Buffer length must be multiple of data type size.
+    EXPECT_THROW(
+        option_v6 = factory(Option::V6, opt_code, OptionBuffer(5)),
+        isc::OutOfRange
+    );
+}
+
+TEST_F(OptionDefinitionTest, factoryUint32Array) {
+    // Let's define some dummy option.
+    const uint16_t opt_code = 80;
+
+    OptionDefinition opt_def("OPTION_UINT32_ARRAY", opt_code, "uint32", true);
+    Option::Factory* factory(NULL);
+    EXPECT_NO_THROW(factory = opt_def.getFactory());
+    ASSERT_TRUE(factory != NULL);
+
+    OptionPtr option_v6;
+    // Positive scenario, initiate the buffer with length being
+    // multiple of uint16_t size.
+    // buffer elements will be: 0x111122223333.
+    OptionBuffer buf(12);
+    for (int i = 0; i < buf.size(); ++i) {
+        buf[i] = i / 4;
+    }
+    // Constructor should succeed because buffer has correct size.
+    EXPECT_NO_THROW(
+        option_v6 = factory(Option::V6, opt_code, buf);
+    );
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6IntArray<uint32_t>));
+    boost::shared_ptr<Option6IntArray<uint32_t> > option_cast_v6 =
+        boost::static_pointer_cast<Option6IntArray<uint32_t> >(option_v6);
+    // Get the values from the initiated options and validate.
+    std::vector<uint32_t> values = option_cast_v6->getValues();
+    for (int i = 0; i < values.size(); ++i) {
+        // Expected value is calculated using on the same pattern
+        // as the one we used to initiate buffer:
+        // for i=0, expected = 0x0000, for i = 1, expected == 0x1111 etc.
+        uint32_t expected = 0x01010101 * i;
+        EXPECT_EQ(expected, values[i]);
+    }
+
+    // Provided buffer size must be greater than zero. Check if we
+    // get exception if we provide zero-length buffer.
+    EXPECT_THROW(
+        option_v6 = factory(Option::V6, opt_code, OptionBuffer()),
+        isc::OutOfRange
+    );
+    // Buffer length must be multiple of data type size.
+    EXPECT_THROW(
+        option_v6 = factory(Option::V6, opt_code, OptionBuffer(5)),
+        isc::OutOfRange
+    );
+}
+
+TEST_F(OptionDefinitionTest, recognizeFormat) {
+    // IA_NA option format.
+    OptionDefinition opt_def1("OPTION_IA_NA", D6O_IA_NA, "record");
+    for (int i = 0; i < 3; ++i) {
+        opt_def1.addRecordField("uint32");
+    }
+    EXPECT_TRUE(opt_def1.haveIA6Format());
+    // Create non-matching format to check that this function does not
+    // return 'true' all the time.
+    OptionDefinition opt_def2("OPTION_IA_NA", D6O_IA_NA, "uint16");
+    EXPECT_FALSE(opt_def2.haveIA6Format());
+
+    // IAADDR option format.
+    OptionDefinition opt_def3("OPTION_IAADDR", D6O_IAADDR, "record");
+    opt_def3.addRecordField("ipv6-address");
+    opt_def3.addRecordField("uint32");
+    opt_def3.addRecordField("uint32");
+    EXPECT_TRUE(opt_def3.haveIAAddr6Format());
+    // Create non-matching format to check that this function does not
+    // return 'true' all the time.
+    OptionDefinition opt_def4("OPTION_IAADDR", D6O_IAADDR, "uint32", true);
+    EXPECT_FALSE(opt_def4.haveIAAddr6Format());
+}
+
+} // anonymous namespace

src/lib/dhcp/tests/subnet_unittest.cc

@@ -15,6 +15,7 @@
 
 #include <config.h>
 #include <dhcp/subnet.h>
+#include <dhcp/option.h>
 #include <exceptions/exceptions.h>
 #include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
@@ -104,6 +105,24 @@ TEST(Subnet4Test, Subnet4_Pool4_checks) {
     EXPECT_THROW(subnet->addPool4(pool3), BadValue);
 }
 
+TEST(Subnet4Test, addInvalidOption) {
+    // Create the V4 subnet.
+    Subnet4Ptr subnet(new Subnet4(IOAddress("192.0.2.0"), 8, 1, 2, 3));
+
+    // Some dummy option code.
+    uint16_t code = 100;
+    // Create option with invalid universe (V6 instead of V4).
+    // Attempt to add this option should result in exception.
+    OptionPtr option1(new Option(Option::V6, code, OptionBuffer(10, 0xFF)));
+    EXPECT_THROW(subnet->addOption(option1), isc::BadValue);
+
+    // Create NULL pointer option. Attempt to add NULL option
+    // should result in exception.
+    OptionPtr option2;
+    ASSERT_FALSE(option2);
+    EXPECT_THROW(subnet->addOption(option2), isc::BadValue);
+}
+
 // Tests for Subnet6
 
 TEST(Subnet6Test, constructor) {
@@ -187,4 +206,146 @@ TEST(Subnet6Test, Subnet6_Pool6_checks) {
     EXPECT_THROW(subnet->addPool6(pool4), BadValue);
 }
 
+TEST(Subnet6Test, addOptions) {
+    // Create as subnet to add options to it.
+    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 56, 1, 2, 3, 4));
+
+    // Differentiate options by their codes (100-109)
+    for (uint16_t code = 100; code < 110; ++code) {
+        OptionPtr option(new Option(Option::V6, code, OptionBuffer(10, 0xFF)));
+        ASSERT_NO_THROW(subnet->addOption(option));
+    }
+
+    // Get options from the Subnet and check if all 10 are there.
+    Subnet::OptionContainer options = subnet->getOptions();
+    ASSERT_EQ(10, options.size());
+
+    // Validate codes of added options.
+    uint16_t expected_code = 100;
+    for (Subnet::OptionContainer::const_iterator option_desc = options.begin();
+         option_desc != options.end(); ++option_desc) {
+        ASSERT_TRUE(option_desc->option);
+        EXPECT_EQ(expected_code, option_desc->option->getType());
+        ++expected_code;
+    }
+
+    subnet->delOptions();
+
+    options = subnet->getOptions();
+    EXPECT_EQ(0, options.size());
+}
+
+TEST(Subnet6Test, addNonUniqueOptions) {
+    // Create as subnet to add options to it.
+    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 56, 1, 2, 3, 4));
+
+    // Create a set of options with non-unique codes.
+    for (int i = 0;  i < 2; ++i) {
+        // In the inner loop we create options with unique codes (100-109).
+        for (uint16_t code = 100; code < 110; ++code) {
+            OptionPtr option(new Option(Option::V6, code, OptionBuffer(10, 0xFF)));
+            ASSERT_NO_THROW(subnet->addOption(option));
+        }
+    }
+
+    // Sanity check that all options are there.
+    Subnet::OptionContainer options = subnet->getOptions();
+    ASSERT_EQ(20, options.size());
+
+    // Use container index #1 to get the options by their codes.
+    Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+    // Look for the codes 100-109.
+    for (uint16_t code = 100; code < 110; ++ code) {
+        // For each code we should get two instances of options.
+        std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
+                  Subnet::OptionContainerTypeIndex::const_iterator> range =
+            idx.equal_range(code);
+        // Distance between iterators indicates how many options
+        // have been retured for the particular code.
+        ASSERT_EQ(2, distance(range.first, range.second));
+        // Check that returned options actually have the expected option code.
+        for (Subnet::OptionContainerTypeIndex::const_iterator option_desc = range.first;
+             option_desc != range.second; ++option_desc) {
+            ASSERT_TRUE(option_desc->option);
+            EXPECT_EQ(code, option_desc->option->getType());
+        }
+    }
+
+    // Let's try to find some non-exiting option.
+    const uint16_t non_existing_code = 150;
+    std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
+              Subnet::OptionContainerTypeIndex::const_iterator> range =
+        idx.equal_range(non_existing_code);
+    // Empty set is expected.
+    EXPECT_EQ(0, distance(range.first, range.second));
+
+    subnet->delOptions();
+
+    options = subnet->getOptions();
+    EXPECT_EQ(0, options.size());
+}
+
+TEST(Subnet6Test, addInvalidOption) {
+    // Create as subnet to add options to it.
+    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 56, 1, 2, 3, 4));
+
+    // Some dummy option code.
+    uint16_t code = 100;
+    // Create option with invalid universe (V4 instead of V6).
+    // Attempt to add this option should result in exception.
+    OptionPtr option1(new Option(Option::V4, code, OptionBuffer(10, 0xFF)));
+    EXPECT_THROW(subnet->addOption(option1), isc::BadValue);
+
+    // Create NULL pointer option. Attempt to add NULL option
+    // should result in exception.
+    OptionPtr option2;
+    ASSERT_FALSE(option2);
+    EXPECT_THROW(subnet->addOption(option2), isc::BadValue);
+}
+
+TEST(Subnet6Test, addPersistentOption) {
+    // Create as subnet to add options to it.
+    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 56, 1, 2, 3, 4));
+
+    // Add 10 options to the subnet with option codes 100 - 109.
+    for (uint16_t code = 100; code < 110; ++code) {
+        OptionPtr option(new Option(Option::V6, code, OptionBuffer(10, 0xFF)));
+        // We create 10 options and want some of them to be flagged
+        // persistent and some non-persistent. Persistent options are
+        // those that server sends to clients regardless if they ask
+        // for them or not. We pick 3 out of 10 options and mark them
+        // non-persistent and 7 other options persistent.
+        // Code values: 102, 105 and 108 are divisable by 3
+        // and options with these codes will be flagged non-persistent.
+        // Options with other codes will be flagged persistent.
+        bool persistent = (code % 3) ? true : false;
+        ASSERT_NO_THROW(subnet->addOption(option, persistent));
+    }
+
+    // Get added options from the subnet.
+    Subnet::OptionContainer options = subnet->getOptions();
+
+    // options.get<2> returns reference to container index #2. This
+    // index is used to access options by the 'persistent' flag.
+    Subnet::OptionContainerPersistIndex& idx = options.get<2>();
+
+    // Get all persistent options.
+    std::pair<Subnet::OptionContainerPersistIndex::const_iterator,
+              Subnet::OptionContainerPersistIndex::const_iterator> range_persistent =
+        idx.equal_range(true);
+    // 3 out of 10 options have been flagged persistent.
+    ASSERT_EQ(7, distance(range_persistent.first, range_persistent.second));
+
+    // Get all non-persistent options.
+    std::pair<Subnet::OptionContainerPersistIndex::const_iterator,
+              Subnet::OptionContainerPersistIndex::const_iterator> range_non_persistent =
+        idx.equal_range(false);
+    // 7 out of 10 options have been flagged persistent.
+    ASSERT_EQ(3, distance(range_non_persistent.first, range_non_persistent.second));
+
+    subnet->delOptions();
+
+    options = subnet->getOptions();
+    EXPECT_EQ(0, options.size());
+}
 };

src/lib/python/bind10_config.py.in

@@ -51,7 +51,7 @@ def reload():
     #  tree the programs in the tree (not installed ones) will be used.
     #
     # B10_FROM_SOURCE_LOCALSTATEDIR is specifically intended to be used for
-    # tests where we want to use variuos types of configuration within the test
+    # tests where we want to use various types of configuration within the test
     # environment.  (We may want to make it even more generic so that the path
     # is passed from the boss process)
     if "B10_FROM_SOURCE" in os.environ:
@@ -60,6 +60,8 @@ def reload():
         else:
             DATA_PATH = os.environ["B10_FROM_SOURCE"]
         PLUGIN_PATHS = [os.environ["B10_FROM_SOURCE"] +
+                            '/src/bin/cfgmgr/local_plugins',
+                             os.environ["B10_FROM_SOURCE"] +
                             '/src/bin/cfgmgr/plugins']
         programdirs = ['auth', 'cfgmgr', 'cmdctl', 'ddns', 'dhcp6', 'msgq',
                        'resolver', 'sockcreator', 'stats', 'xfrin', 'xfrout',

tests/lettuce/configurations/auth/.gitignore

@@ -0,0 +1 @@
+/auth_basic.config

tests/lettuce/configurations/auth/auth_basic.config.orig

@@ -0,0 +1,22 @@
+{
+    "version": 2,
+    "Logging": {
+        "loggers": [ {
+            "debuglevel": 99,
+            "severity": "DEBUG",
+            "name": "*"
+        } ]
+    },
+    "Auth": {
+        "listen_on": [ {
+            "port": 47806,
+            "address": "127.0.0.1"
+        } ]
+    },
+    "Boss": {
+        "components": {
+            "b10-auth": { "kind": "needed", "special": "auth" },
+            "b10-cmdctl": { "special": "cmdctl", "kind": "needed" }
+        }
+    }
+}

tests/lettuce/features/auth_basic.feature

@@ -0,0 +1,20 @@
+Feature: Basic Authoritative DNS server
+    This feature set is for testing the execution of the b10-auth
+    component using its default datasource configurations. This
+    will start it and perform queries against it.
+
+    Scenario: Query builtin bind zone
+        Given I have bind10 running with configuration auth/auth_basic.config
+        And wait for bind10 stderr message BIND10_STARTED_CC
+        And wait for bind10 stderr message CMDCTL_STARTED
+        And wait for bind10 stderr message AUTH_SERVER_STARTED
+
+        bind10 module Auth should be running
+        And bind10 module Resolver should not be running
+
+        A query for example.com should have rcode REFUSED
+        A query for version.bind type TXT class CH should have rcode NOERROR
+        A query for authors.bind type TXT class CH should have rcode NOERROR
+
+        # TODO: to be compatible with BIND 9
+        # A query for nonexistent.bind type TXT class CH should have rcode REFUSED

tests/lettuce/features/terrain/terrain.py

@@ -49,6 +49,8 @@ copylist = [
      "configurations/example.org.config"],
     ["configurations/bindctl/bindctl.config.orig",
      "configurations/bindctl/bindctl.config"],
+    ["configurations/auth/auth_basic.config.orig",
+     "configurations/auth/auth_basic.config"],
     ["configurations/resolver/resolver_basic.config.orig",
      "configurations/resolver/resolver_basic.config"],
     ["configurations/multi_instance/multi_auth.config.orig",