[master] Merge branch 'trac2203'

src/bin/auth/Makefile.am

@@ -55,7 +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_configurator.h
+b10_auth_SOURCES += datasrc_config.h datasrc_config.cc
 b10_auth_SOURCES += main.cc
 
 nodist_b10_auth_SOURCES = auth_messages.h auth_messages.cc

src/bin/auth/benchmarks/Makefile.am

@@ -17,6 +17,7 @@ query_bench_SOURCES += ../auth_srv.h ../auth_srv.cc
 query_bench_SOURCES += ../auth_config.h ../auth_config.cc
 query_bench_SOURCES += ../statistics.h ../statistics.cc
 query_bench_SOURCES += ../auth_log.h ../auth_log.cc
+query_bench_SOURCES += ../datasrc_config.h ../datasrc_config.cc
 
 nodist_query_bench_SOURCES = ../auth_messages.h ../auth_messages.cc
 

src/bin/auth/benchmarks/query_bench.cc

@@ -30,7 +30,7 @@
 
 #include <auth/auth_srv.h>
 #include <auth/auth_config.h>
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 #include <auth/query.h>
 
 #include <asiodns/asiodns.h>
@@ -125,8 +125,8 @@ public:
                           OutputBuffer& buffer) :
         QueryBenchMark(queries, query_message, buffer)
     {
-        DataSourceConfigurator::testReconfigure(
+        configureDataSource(
-            server_.get(),
+            *server_,
             Element::fromJSON("{\"IN\":"
                               "  [{\"type\": \"sqlite3\","
                               "    \"params\": {"
@@ -139,13 +139,13 @@ class MemoryQueryBenchMark  : public QueryBenchMark {
 public:
     MemoryQueryBenchMark(const char* const zone_file,
                          const char* const zone_origin,
-                          const BenchQueries& queries,
+                         const BenchQueries& queries,
-                          Message& query_message,
+                         Message& query_message,
-                          OutputBuffer& buffer) :
+                         OutputBuffer& buffer) :
         QueryBenchMark(queries, query_message, buffer)
     {
-        DataSourceConfigurator::testReconfigure(
+        configureDataSource(
-            server_.get(),
+            *server_,
             Element::fromJSON("{\"IN\":"
                               "  [{\"type\": \"MasterFiles\","
                               "    \"cache-enable\": true, "

src/bin/auth/datasrc_config.cc

@@ -0,0 +1,26 @@
+// 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_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.
+void
+configureDataSource(AuthSrv& server, const isc::data::ConstElementPtr& config)
+{
+    return (configureDataSourceGeneric<AuthSrv,
+            isc::datasrc::ConfigurableClientList>(server, config));
+}

src/bin/auth/datasrc_config.h

@@ -0,0 +1,120 @@
+// 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_CONFIG_H
+#define DATASRC_CONFIG_H
+
+#include "auth_srv.h"
+
+#include <cc/data.h>
+#include <datasrc/client_list.h>
+#include <util/threads/lock.h>
+
+#include <boost/shared_ptr.hpp>
+
+#include <set>
+
+/// \brief Configure the authoritative server's data source lists
+///
+/// This will hook into the data_sources module configuration and it will
+/// keep the local copy of data source clients in the list in the authoritative
+/// server.
+///
+/// This function is templated. This is simply because of easier testing.
+/// You don't need to pay attention to it, use the configureDataSource
+/// specialization instead.
+///
+/// \param server It is the server to configure.
+/// \param config The configuration value to parse. It is in the form
+///     as an update from the config manager.
+template<class Server, class List>
+void
+configureDataSourceGeneric(Server& server,
+                           const isc::data::ConstElementPtr& config)
+{
+    typedef boost::shared_ptr<List> ListPtr;
+    typedef std::map<std::string, isc::data::ConstElementPtr> Map;
+    typedef std::pair<isc::dns::RRClass, ListPtr> RollbackPair;
+    typedef std::pair<isc::dns::RRClass, isc::data::ConstElementPtr>
+        RollbackConfiguration;
+
+    // Lock the client lists, we're going to manipulate them.
+    isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
+
+    // Some structures to be able to perform a rollback
+    std::vector<RollbackPair> rollback_sets;
+    std::vector<RollbackConfiguration> rollback_configurations;
+    try {
+        // Get the configuration and current state.
+        const Map& map(config->mapValue());
+        const std::vector<isc::dns::RRClass>
+            activeVector(server.getClientListClasses());
+        std::set<isc::dns::RRClass> active(activeVector.begin(),
+                                           activeVector.end());
+        // Go through the configuration and change everything.
+        for (Map::const_iterator it(map.begin()); it != map.end(); ++it) {
+            const isc::dns::RRClass rrclass(it->first);
+            active.erase(rrclass);
+            ListPtr list(server.getClientList(rrclass));
+            bool need_set(false);
+            if (list) {
+                rollback_configurations.
+                    push_back(RollbackConfiguration(rrclass,
+                                                    list->getConfiguration()));
+            } else {
+                list.reset(new List(rrclass));
+                need_set = true;
+                rollback_sets.push_back(RollbackPair(rrclass, ListPtr()));
+            }
+            list->configure(it->second, true);
+            if (need_set) {
+                server.setClientList(rrclass, list);
+            }
+        }
+        // Remove the ones that are not in the configuration.
+        for (std::set<isc::dns::RRClass>::iterator it(active.begin());
+             it != active.end(); ++it) {
+            // There seems to be no way the setClientList could throw.
+            // But this is just to make sure in case it did to restore
+            // the original.
+            rollback_sets.push_back(
+                RollbackPair(*it, server.getClientList(*it)));
+            server.setClientList(*it, ListPtr());
+        }
+    } catch (...) {
+        // Perform a rollback of the changes. The old configuration should
+        // work.
+        for (typename std::vector<RollbackPair>::const_iterator
+                 it(rollback_sets.begin()); it != rollback_sets.end(); ++it) {
+            server.setClientList(it->first, it->second);
+        }
+        for (typename std::vector<RollbackConfiguration>::const_iterator
+                 it(rollback_configurations.begin());
+             it != rollback_configurations.end(); ++it) {
+            server.getClientList(it->first)->configure(it->second, true);
+        }
+        throw;
+    }
+}
+
+/// \brief Concrete version of configureDataSource() for the
+///     use with authoritative server implementation.
+void
+configureDataSource(AuthSrv& server, const isc::data::ConstElementPtr& config);
+
+#endif  // DATASRC_CONFIG_H
+
+// Local Variables:
+// mode: c++
+// End:

src/bin/auth/datasrc_configurator.h

@@ -1,226 +0,0 @@
-// 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_CONFIGURATOR_H
-#define DATASRC_CONFIGURATOR_H
-
-#include "auth_srv.h"
-
-#include <datasrc/client_list.h>
-#include <config/ccsession.h>
-#include <cc/data.h>
-#include <util/threads/lock.h>
-
-#include <set>
-
-/// \brief A class to configure the authoritative server's data source lists
-///
-/// This will hook into the data_sources module configuration and it will
-/// keep the local copy of data source clients in the list in the authoritative
-/// server.
-///
-/// The class is slightly unusual. Due to some technical limitations, the hook
-/// needs to be static method. Therefore it is not possible to create instances
-/// of the class.
-///
-/// Also, the class is a template. This is simply because of easier testing.
-/// You don't need to pay attention to it, use the DataSourceConfigurator
-/// type alias instead.
-template<class Server, class List>
-class DataSourceConfiguratorGeneric {
-private:
-    /// \brief Disallow creation of instances
-    DataSourceConfiguratorGeneric();
-    /// \brief Internal method to hook into the ModuleCCSession
-    ///
-    /// It simply calls reconfigure.
-    static void reconfigureInternal(const std::string&,
-                                    isc::data::ConstElementPtr config,
-                                    const isc::config::ConfigData&)
-    {
-        if (config->contains("classes")) {
-            reconfigure(config->get("classes"));
-        }
-    }
-    static Server* server_;
-    static isc::config::ModuleCCSession* session_;
-    typedef boost::shared_ptr<List> ListPtr;
-public:
-    /// \brief Initializes the class.
-    ///
-    /// This configures which session and server should be used.
-    /// It hooks to the session now and downloads the configuration.
-    /// It is synchronous (it may block for some time).
-    ///
-    /// Note that you need to call cleanup before the server or
-    /// session dies, otherwise it might access them after they
-    /// are destroyed.
-    ///
-    /// \param session The session to hook into and to access the configuration
-    ///     through.
-    /// \param server It is the server to configure.
-    /// \throw isc::InvalidOperation if this is called when already initialized.
-    /// \throw isc::InvalidParameter if any of the parameters is NULL
-    /// \throw isc::config::ModuleCCError if the remote configuration is not
-    ///     available for some reason.
-    static void init(isc::config::ModuleCCSession* session,
-                     Server* server)
-    {
-        if (session == NULL) {
-            isc_throw(isc::InvalidParameter, "The session must not be NULL");
-        }
-        if (server == NULL) {
-            isc_throw(isc::InvalidParameter, "The server must not be NULL");
-        }
-        if (server_ != NULL) {
-            isc_throw(isc::InvalidOperation,
-                      "The configurator is already initialized");
-        }
-        server_ = server;
-        session_ = session;
-        session->addRemoteConfig("data_sources", reconfigureInternal, false);
-    }
-    /// \brief Deinitializes the class.
-    ///
-    /// This detaches from the session and removes the server from internal
-    /// storage. The current configuration in the server is preserved.
-    ///
-    /// This can be called even if it is not initialized currently. You
-    /// can initialize it again after this.
-    static void cleanup() {
-        if (session_ != NULL) {
-            session_->removeRemoteConfig("data_sources");
-        }
-        session_ = NULL;
-        server_ = NULL;
-    }
-    /// \brief Reads new configuration and replaces the old one.
-    ///
-    /// It instructs the server to replace the lists with new ones as needed.
-    /// You don't need to call it directly (but you could, though the benefit
-    /// is unknown and it would be questionable at least). It is called
-    /// automatically on normal updates.
-    ///
-    /// \param config The configuration value to parse. It is in the form
-    ///     as an update from the config manager.
-    /// \throw InvalidOperation if it is called when not initialized.
-    static void reconfigure(const isc::data::ConstElementPtr& config) {
-        if (server_ == NULL) {
-            isc_throw(isc::InvalidOperation,
-                      "Can't reconfigure while not initialized by init()");
-        }
-        // Lock the client lists, we're going to manipulate them.
-        isc::util::thread::Mutex::Locker locker(server_->getClientListMutex());
-        typedef std::map<std::string, isc::data::ConstElementPtr> Map;
-        typedef std::pair<isc::dns::RRClass, ListPtr> RollbackPair;
-        typedef std::pair<isc::dns::RRClass, isc::data::ConstElementPtr>
-            RollbackConfiguration;
-        // Some structures to be able to perform a rollback
-        std::vector<RollbackPair> rollback_sets;
-        std::vector<RollbackConfiguration> rollback_configurations;
-        try {
-            // Get the configuration and current state.
-            const Map& map(config->mapValue());
-            const std::vector<isc::dns::RRClass>
-                activeVector(server_->getClientListClasses());
-            std::set<isc::dns::RRClass> active(activeVector.begin(),
-                                               activeVector.end());
-            // Go through the configuration and change everything.
-            for (Map::const_iterator it(map.begin()); it != map.end(); ++it) {
-                isc::dns::RRClass rrclass(it->first);
-                active.erase(rrclass);
-                ListPtr list(server_->getClientList(rrclass));
-                bool need_set(false);
-                if (list) {
-                    rollback_configurations.
-                        push_back(RollbackConfiguration(rrclass,
-                            list->getConfiguration()));
-                } else {
-                    list.reset(new List(rrclass));
-                    need_set = true;
-                    rollback_sets.push_back(RollbackPair(rrclass, ListPtr()));
-                }
-                list->configure(it->second, true);
-                if (need_set) {
-                    server_->setClientList(rrclass, list);
-                }
-            }
-            // Remove the ones that are not in the configuration.
-            for (std::set<isc::dns::RRClass>::iterator it(active.begin());
-                 it != active.end(); ++it) {
-                // There seems to be no way the setClientList could throw.
-                // But this is just to make sure in case it did to restore
-                // the original.
-                rollback_sets.push_back(
-                    RollbackPair(*it, server_->getClientList(*it)));
-                server_->setClientList(*it, ListPtr());
-            }
-        } catch (...) {
-            // Perform a rollback of the changes. The old configuration should
-            // work.
-            for (typename std::vector<RollbackPair>::const_iterator
-                 it(rollback_sets.begin()); it != rollback_sets.end(); ++it) {
-                server_->setClientList(it->first, it->second);
-            }
-            for (typename std::vector<RollbackConfiguration>::const_iterator
-                 it(rollback_configurations.begin());
-                 it != rollback_configurations.end(); ++it) {
-                server_->getClientList(it->first)->configure(it->second, true);
-            }
-            throw;
-        }
-    }
-    /// \brief Version of reconfigure for easier testing.
-    ///
-    /// This method can be used to reconfigure a server without first
-    /// initializing the configurator. This does not need a session.
-    /// Otherwise, it acts the same as reconfigure.
-    ///
-    /// This is not meant for production code. Do not use there.
-    ///
-    /// \param server The server to configure.
-    /// \param config The config to use.
-    /// \throw isc::InvalidOperation if the configurator is initialized.
-    /// \throw anything that reconfigure does.
-    static void testReconfigure(Server* server,
-                                const isc::data::ConstElementPtr& config)
-    {
-        if (server_ != NULL) {
-            isc_throw(isc::InvalidOperation, "Currently initialized.");
-        }
-        try {
-            server_ = server;
-            reconfigure(config);
-            server_ = NULL;
-        } catch (...) {
-            server_ = NULL;
-            throw;
-        }
-    }
-};
-
-template<class Server, class List>
-isc::config::ModuleCCSession*
-DataSourceConfiguratorGeneric<Server, List>::session_(NULL);
-
-template<class Server, class List>
-Server* DataSourceConfiguratorGeneric<Server, List>::server_(NULL);
-
-/// \brief Concrete version of DataSourceConfiguratorGeneric for the
-///     use in authoritative server.
-typedef DataSourceConfiguratorGeneric<AuthSrv,
-        isc::datasrc::ConfigurableClientList>
-    DataSourceConfigurator;
-
-#endif

src/bin/auth/main.cc

@@ -14,17 +14,6 @@
 
 #include <config.h>
 
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <sys/select.h>
-#include <netdb.h>
-#include <netinet/in.h>
-#include <stdlib.h>
-#include <errno.h>
-
-#include <cassert>
-#include <iostream>
-
 #include <exceptions/exceptions.h>
 
 #include <util/buffer.h>
@@ -45,13 +34,26 @@
 #include <auth/command.h>
 #include <auth/auth_srv.h>
 #include <auth/auth_log.h>
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 #include <asiodns/asiodns.h>
 #include <asiolink/asiolink.h>
 #include <log/logger_support.h>
 #include <server_common/keyring.h>
 #include <server_common/socket_request.h>
 
+#include <boost/bind.hpp>
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/select.h>
+#include <netdb.h>
+#include <netinet/in.h>
+#include <stdlib.h>
+#include <errno.h>
+
+#include <cassert>
+#include <iostream>
+
 using namespace std;
 using namespace isc::asiodns;
 using namespace isc::asiolink;
@@ -70,7 +72,7 @@ namespace {
 /* need global var for config/command handlers.
  * todo: turn this around, and put handlers in the authserver
  * class itself? */
-AuthSrv *auth_server;
+AuthSrv* auth_server;
 
 ConstElementPtr
 my_config_handler(ConstElementPtr new_config) {
@@ -83,6 +85,29 @@ my_command_handler(const string& command, ConstElementPtr args) {
     return (execAuthServerCommand(*auth_server, command, args));
 }
 
+void
+datasrcConfigHandler(AuthSrv* server, bool* first_time,
+                     ModuleCCSession* config_session, const std::string&,
+                     isc::data::ConstElementPtr config,
+                     const isc::config::ConfigData&)
+{
+    assert(server != NULL);
+    if (config->contains("classes")) {
+        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;
+            configureDataSource(*auth_server,
+                                config_session->getRemoteConfigValue(
+                                    "data_sources", "classes"));
+        } else {
+            configureDataSource(*server, config->get("classes"));
+        }
+    }
+}
+
 void
 usage() {
     cerr << "Usage:  b10-auth [-v]"
@@ -191,13 +216,15 @@ main(int argc, char* argv[]) {
         isc::server_common::initKeyring(*config_session);
         auth_server->setTSIGKeyRing(&isc::server_common::keyring);
 
-        // Start the data source configuration
+        // Start the data source configuration.  We pass first_time and
-        DataSourceConfigurator::init(config_session, auth_server);
+        // config_session for the hack described in datasrcConfigHandler.
-        // HACK: The default is not passed to the handler. This one will
+        bool first_time = true;
-        // get the default (or, current value). Further updates will work
+        config_session->addRemoteConfig("data_sources",
-        // the usual way.
+                                        boost::bind(datasrcConfigHandler,
-        DataSourceConfigurator::reconfigure(
+                                                    auth_server, &first_time,
-            config_session->getRemoteConfigValue("data_sources", "classes"));
+                                                    config_session,
+                                                    _1, _2, _3),
+                                        false);
 
         // Now start asynchronous read.
         config_session->start();
@@ -221,7 +248,10 @@ main(int argc, char* argv[]) {
         xfrin_session->disconnect();
     }
 
-    DataSourceConfigurator::cleanup();
+    // If we haven't registered callback for data sources, this will be just
+    // no-op.
+    config_session->removeRemoteConfig("data_sources");
+
     delete xfrin_session;
     delete config_session;
     delete cc_session;

src/bin/auth/tests/Makefile.am

@@ -42,6 +42,7 @@ run_unittests_SOURCES += ../auth_config.h ../auth_config.cc
 run_unittests_SOURCES += ../command.h ../command.cc
 run_unittests_SOURCES += ../common.h ../common.cc
 run_unittests_SOURCES += ../statistics.h ../statistics.cc
+run_unittests_SOURCES += ../datasrc_config.h ../datasrc_config.cc
 run_unittests_SOURCES += datasrc_util.h datasrc_util.cc
 run_unittests_SOURCES += auth_srv_unittest.cc
 run_unittests_SOURCES += config_unittest.cc
@@ -50,7 +51,7 @@ 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 += datasrc_configurator_unittest.cc
+run_unittests_SOURCES += datasrc_config_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
 
 nodist_run_unittests_SOURCES = ../auth_messages.h ../auth_messages.cc

src/bin/auth/tests/auth_srv_unittest.cc

@@ -36,7 +36,7 @@
 #include <auth/command.h>
 #include <auth/common.h>
 #include <auth/statistics.h>
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 
 #include <util/unittests/mock_socketsession.h>
 #include <util/threads/lock.h>
@@ -722,17 +722,17 @@ TEST_F(AuthSrvTest, notifyWithSessionMessageError) {
 }
 
 void
-updateDatabase(AuthSrv* server, const char* params) {
+updateDatabase(AuthSrv& server, const char* params) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"IN\": [{"
         "    \"type\": \"sqlite3\","
         "    \"params\": " + string(params) +
         "}]}"));
-    DataSourceConfigurator::testReconfigure(server, config);
+    configureDataSource(server, config);
 }
 
 void
-updateInMemory(AuthSrv* server, const char* origin, const char* filename) {
+updateInMemory(AuthSrv& server, const char* origin, const char* filename) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"IN\": [{"
         "   \"type\": \"MasterFiles\","
@@ -745,17 +745,17 @@ updateInMemory(AuthSrv* server, const char* origin, const char* filename) {
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
-    DataSourceConfigurator::testReconfigure(server, config);
+    configureDataSource(server, config);
 }
 
 void
-updateBuiltin(AuthSrv* server) {
+updateBuiltin(AuthSrv& server) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"CH\": [{"
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
-    DataSourceConfigurator::testReconfigure(server, config);
+    configureDataSource(server, config);
 }
 
 // Try giving the server a TSIG signed request and see it can anwer signed as
@@ -766,7 +766,7 @@ TEST_F(AuthSrvTest, DISABLED_TSIGSigned) { // Needs builtin
 TEST_F(AuthSrvTest, TSIGSigned) {
 #endif
     // Prepare key, the client message, etc
-    updateBuiltin(&server);
+    updateBuiltin(server);
     const TSIGKey key("key:c2VjcmV0Cg==:hmac-sha1");
     TSIGContext context(key);
     UnitTestUtil::createRequestMessage(request_message, opcode, default_qid,
@@ -814,7 +814,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQueryViaDNSServer) {
 #else
 TEST_F(AuthSrvTest, builtInQueryViaDNSServer) {
 #endif
-    updateBuiltin(&server);
+    updateBuiltin(server);
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
                                        default_qid, Name("VERSION.BIND."),
                                        RRClass::CH(), RRType::TXT());
@@ -846,7 +846,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQuery) {
 #else
 TEST_F(AuthSrvTest, builtInQuery) {
 #endif
-    updateBuiltin(&server);
+    updateBuiltin(server);
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
                                        default_qid, Name("VERSION.BIND."),
                                        RRClass::CH(), RRType::TXT());
@@ -867,7 +867,7 @@ TEST_F(AuthSrvTest, DISABLED_iqueryViaDNSServer) { // Needs builtin
 #else
 TEST_F(AuthSrvTest, iqueryViaDNSServer) { // Needs builtin
 #endif
-    updateBuiltin(&server);
+    updateBuiltin(server);
     createDataFromFile("iquery_fromWire.wire");
     (*server.getDNSLookupProvider())(*io_message, parse_message,
                                      response_message,
@@ -889,7 +889,7 @@ TEST_F(AuthSrvTest, DISABLED_updateConfig) {
 #else
 TEST_F(AuthSrvTest, updateConfig) {
 #endif
-    updateDatabase(&server, CONFIG_TESTDB);
+    updateDatabase(server, CONFIG_TESTDB);
 
     // query for existent data in the installed data source.  The resulting
     // response should have the AA flag on, and have an RR in each answer
@@ -907,7 +907,7 @@ TEST_F(AuthSrvTest, DISABLED_datasourceFail) {
 #else
 TEST_F(AuthSrvTest, datasourceFail) {
 #endif
-    updateDatabase(&server, CONFIG_TESTDB);
+    updateDatabase(server, CONFIG_TESTDB);
 
     // This query will hit a corrupted entry of the data source (the zoneload
     // tool and the data source itself naively accept it).  This will result
@@ -927,10 +927,10 @@ TEST_F(AuthSrvTest, DISABLED_updateConfigFail) {
 TEST_F(AuthSrvTest, updateConfigFail) {
 #endif
     // First, load a valid data source.
-    updateDatabase(&server, CONFIG_TESTDB);
+    updateDatabase(server, CONFIG_TESTDB);
 
     // Next, try to update it with a non-existent one.  This should fail.
-    EXPECT_THROW(updateDatabase(&server, BADCONFIG_TESTDB),
+    EXPECT_THROW(updateDatabase(server, BADCONFIG_TESTDB),
                  isc::datasrc::DataSourceError);
 
     // The original data source should still exist.
@@ -953,7 +953,7 @@ TEST_F(AuthSrvTest, updateWithInMemoryClient) {
         "   \"params\": {},"
         "   \"cache-enable\": true"
         "}]}"));
-    DataSourceConfigurator::testReconfigure(&server, config);
+    configureDataSource(server, config);
     // after successful configuration, we should have one (with empty zoneset).
 
     // The memory data source is empty, should return REFUSED rcode.
@@ -974,7 +974,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientNoDNSSEC) {
     // query handler class, and confirm it returns no error and a non empty
     // answer section.  Detailed examination on the response content
     // for various types of queries are tested in the query tests.
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -993,7 +993,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientDNSSEC) {
     // Similar to the previous test, but the query has the DO bit on.
     // The response should contain RRSIGs, and should have more RRs than
     // the previous case.
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
     createDataFromFile("nsec3query_fromWire.wire");
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1013,7 +1013,7 @@ TEST_F(AuthSrvTest,
     )
 {
     // Set up the in-memory
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
     // This shouldn't affect the result of class CH query
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
@@ -1425,7 +1425,7 @@ TEST_F(AuthSrvTest,
     )
 {
     // Set real inmem client to proxy
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
     {
         isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
         boost::shared_ptr<isc::datasrc::ConfigurableClientList>
@@ -1450,16 +1450,16 @@ TEST_F(AuthSrvTest,
 // If non null rrset is given, it will be passed to the proxy so it can
 // return some faked response.
 void
-setupThrow(AuthSrv* server, ThrowWhen throw_when, bool isc_exception,
+setupThrow(AuthSrv& server, ThrowWhen throw_when, bool isc_exception,
            ConstRRsetPtr rrset = ConstRRsetPtr())
 {
     updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
-    isc::util::thread::Mutex::Locker locker(server->getClientListMutex());
+    isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
     boost::shared_ptr<isc::datasrc::ConfigurableClientList>
-        list(new FakeList(server->getClientList(RRClass::IN()), throw_when,
+        list(new FakeList(server.getClientList(RRClass::IN()), throw_when,
                           isc_exception, rrset));
-    server->setClientList(RRClass::IN(), list);
+    server.setClientList(RRClass::IN(), list);
 }
 
 TEST_F(AuthSrvTest,
@@ -1482,11 +1482,11 @@ TEST_F(AuthSrvTest,
                                              RRClass::IN(), RRType::TXT());
     for (ThrowWhen* when(throws); *when != THROW_NEVER; ++when) {
         createRequestPacket(request_message, IPPROTO_UDP);
-        setupThrow(&server, *when, true);
+        setupThrow(server, *when, true);
         processAndCheckSERVFAIL();
         // To be sure, check same for non-isc-exceptions
         createRequestPacket(request_message, IPPROTO_UDP);
-        setupThrow(&server, *when, false);
+        setupThrow(server, *when, false);
         processAndCheckSERVFAIL();
     }
 }
@@ -1502,7 +1502,7 @@ TEST_F(AuthSrvTest,
     )
 {
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
-    setupThrow(&server, THROW_AT_GET_CLASS, true);
+    setupThrow(server, THROW_AT_GET_CLASS, true);
 
     // getClass is not called so it should just answer
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1526,7 +1526,7 @@ TEST_F(AuthSrvTest,
     ConstRRsetPtr empty_rrset(new RRset(Name("foo.example"),
                                         RRClass::IN(), RRType::TXT(),
                                         RRTTL(0)));
-    setupThrow(&server, THROW_NEVER, true, empty_rrset);
+    setupThrow(server, THROW_NEVER, true, empty_rrset);
 
     // Repeat the query processing two times.  Due to the faked RRset,
     // toWire() should throw, and it should result in SERVFAIL.

src/bin/auth/tests/command_unittest.cc

@@ -19,7 +19,7 @@
 #include <auth/auth_srv.h>
 #include <auth/auth_config.h>
 #include <auth/command.h>
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 
 #include <dns/name.h>
 #include <dns/rrclass.h>
@@ -208,7 +208,7 @@ configureZones(AuthSrv& server) {
         "   \"cache-enable\": true"
         "}]}"));
 
-    DataSourceConfigurator::testReconfigure(&server, config);
+    configureDataSource(server, config);
 
     zoneChecks(server);
 }
@@ -271,7 +271,7 @@ TEST_F(AuthCommandTest,
         "    \"cache-enable\": true,"
         "    \"cache-zones\": [\"example.org\"]"
         "}]}"));
-    DataSourceConfigurator::testReconfigure(&server_, config);
+    configureDataSource(server_, config);
 
     {
         isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
@@ -335,7 +335,7 @@ TEST_F(AuthCommandTest,
         "    \"cache-enable\": true,"
         "    \"cache-zones\": [\"example.com\"]"
         "}]}"));
-    EXPECT_THROW(DataSourceConfigurator::testReconfigure(&server_, config2),
+    EXPECT_THROW(configureDataSource(server_, config2),
                  ConfigurableClientList::ConfigurationError);
 
     result_ = execAuthServerCommand(server_, "loadzone",

src/bin/auth/tests/datasrc_config_unittest.cc

@@ -12,16 +12,19 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <auth/datasrc_configurator.h>
+#include <auth/datasrc_config.h>
 
 #include <config/tests/fake_session.h>
 #include <config/ccsession.h>
 #include <util/threads/lock.h>
 
 #include <gtest/gtest.h>
-#include <memory>
+
+#include <boost/bind.hpp>
 #include <boost/shared_ptr.hpp>
 
+#include <memory>
+
 using namespace isc;
 using namespace isc::cc;
 using namespace isc::config;
@@ -32,7 +35,7 @@ using namespace boost;
 
 namespace {
 
-class DatasrcConfiguratorTest;
+class DatasrcConfigTest;
 
 class FakeList {
 public:
@@ -57,12 +60,26 @@ private:
 
 typedef shared_ptr<FakeList> ListPtr;
 
-// We use the test fixture as both parameters, this makes it possible
+void
-// to easily fake all needed methods and look that they were called.
+testConfigureDataSource(DatasrcConfigTest& test,
-typedef DataSourceConfiguratorGeneric<DatasrcConfiguratorTest,
+                        const isc::data::ConstElementPtr& config)
-        FakeList> Configurator;
+{
+    // We use the test fixture for the Server type.  This makes it possible
+    // to easily fake all needed methods and look that they were called.
+    configureDataSourceGeneric<DatasrcConfigTest, FakeList>(test, config);
+}
 
-class DatasrcConfiguratorTest : public ::testing::Test {
+void
+datasrcConfigHandler(DatasrcConfigTest* fake_server, const std::string&,
+                     isc::data::ConstElementPtr config,
+                     const isc::config::ConfigData&)
+{
+    if (config->contains("classes")) {
+        testConfigureDataSource(*fake_server, config->get("classes"));
+    }
+}
+
+class DatasrcConfigTest : public ::testing::Test {
 public:
     // These pretend to be the server
     ListPtr getClientList(const RRClass& rrclass) {
@@ -86,7 +103,7 @@ public:
         return (mutex_);
     }
 protected:
-    DatasrcConfiguratorTest() :
+    DatasrcConfigTest() :
         session(ElementPtr(new ListElement), ElementPtr(new ListElement),
                 ElementPtr(new ListElement)),
         specfile(string(TEST_OWN_DATA_DIR) + "/spec.spec")
@@ -99,25 +116,24 @@ protected:
                                        false));
     }
     void TearDown() {
-        // Make sure no matter what we did, it is cleaned up.
+        // Make sure no matter what we did, it is cleaned up.  Also check
-        Configurator::cleanup();
+        // we really have subscribed to the configuration, and after removing
+        // it we actually cancel it.
+        EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
+        mccs->removeRemoteConfig("data_sources");
+        EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
     }
-    void init(const ElementPtr& config = ElementPtr()) {
+    void SetUp() {
         session.getMessages()->
             add(createAnswer(0,
                              moduleSpecFromFile(string(PLUGIN_DATA_PATH) +
                                                 "/datasrc.spec").
                              getFullSpec()));
-        if (config) {
+        session.getMessages()->add(createAnswer(0,
-            session.getMessages()->add(createAnswer(0, config));
+                                                ElementPtr(new MapElement)));
-        } else {
+        mccs->addRemoteConfig("data_sources",
-            session.getMessages()->
+                              boost::bind(datasrcConfigHandler,
-                add(createAnswer(0, ElementPtr(new MapElement)));
+                                          this, _1, _2, _3), false);
-        }
-        Configurator::init(mccs.get(), this);
-    }
-    void SetUp() {
-        init();
     }
     ElementPtr buildConfig(const string& config) const {
         const ElementPtr internal(Element::fromJSON(config));
@@ -126,10 +142,10 @@ protected:
         return (external);
     }
     void initializeINList() {
-        const ElementPtr
+        const ConstElementPtr
             config(buildConfig("{\"IN\": [{\"type\": \"xxx\"}]}"));
-        session.addMessage(createCommand("config_update", config), "data_sources",
+        session.addMessage(createCommand("config_update", config),
-                           "*");
+                           "data_sources", "*");
         mccs->checkCommand();
         // Check it called the correct things (check that there's no IN yet and
         // set a new one.
@@ -144,33 +160,12 @@ protected:
     mutable isc::util::thread::Mutex mutex_;
 };
 
-// Check the initialization (and cleanup)
-TEST_F(DatasrcConfiguratorTest, initialization) {
-    // It can't be initialized again
-    EXPECT_THROW(init(), InvalidOperation);
-    EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
-    // Deinitialize to make the tests reasonable
-    Configurator::cleanup();
-    EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
-    // We can't reconfigure now (not even manually)
-    EXPECT_THROW(Configurator::reconfigure(ElementPtr(new MapElement())),
-                 InvalidOperation);
-    // If one of them is NULL, it does not work
-    EXPECT_THROW(Configurator::init(NULL, this), InvalidParameter);
-    EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
-    EXPECT_THROW(Configurator::init(mccs.get(), NULL), InvalidParameter);
-    EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
-    // But we can initialize it again now
-    EXPECT_NO_THROW(init());
-    EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
-}
-
 // Push there a configuration with a single list.
-TEST_F(DatasrcConfiguratorTest, createList) {
+TEST_F(DatasrcConfigTest, createList) {
     initializeINList();
 }
 
-TEST_F(DatasrcConfiguratorTest, modifyList) {
+TEST_F(DatasrcConfigTest, modifyList) {
     // First, initialize the list
     initializeINList();
     // And now change the configuration of the list
@@ -188,7 +183,7 @@ TEST_F(DatasrcConfiguratorTest, modifyList) {
 }
 
 // Check we can have multiple lists at once
-TEST_F(DatasrcConfiguratorTest, multiple) {
+TEST_F(DatasrcConfigTest, multiple) {
     const ElementPtr
         config(buildConfig("{\"IN\": [{\"type\": \"yyy\"}], "
                                  "\"CH\": [{\"type\": \"xxx\"}]}"));
@@ -208,7 +203,7 @@ TEST_F(DatasrcConfiguratorTest, multiple) {
 //
 // It's almost like above, but we initialize first with single-list
 // config.
-TEST_F(DatasrcConfiguratorTest, updateAdd) {
+TEST_F(DatasrcConfigTest, updateAdd) {
     initializeINList();
     const ElementPtr
         config(buildConfig("{\"IN\": [{\"type\": \"yyy\"}], "
@@ -226,7 +221,7 @@ TEST_F(DatasrcConfiguratorTest, updateAdd) {
 }
 
 // We delete a class list in this test.
-TEST_F(DatasrcConfiguratorTest, updateDelete) {
+TEST_F(DatasrcConfigTest, updateDelete) {
     initializeINList();
     const ElementPtr
         config(buildConfig("{}"));
@@ -243,7 +238,7 @@ TEST_F(DatasrcConfiguratorTest, updateDelete) {
 }
 
 // Check that we can rollback an addition if something else fails
-TEST_F(DatasrcConfiguratorTest, rollbackAddition) {
+TEST_F(DatasrcConfigTest, rollbackAddition) {
     initializeINList();
     // The configuration is wrong. However, the CH one will get done first.
     const ElementPtr
@@ -262,13 +257,13 @@ TEST_F(DatasrcConfiguratorTest, rollbackAddition) {
 }
 
 // Check that we can rollback a deletion if something else fails
-TEST_F(DatasrcConfiguratorTest, rollbackDeletion) {
+TEST_F(DatasrcConfigTest, rollbackDeletion) {
     initializeINList();
     // Put the CH there
     const ElementPtr
         config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
                                   "\"CH\": [{\"type\": \"xxx\"}]}"));
-    Configurator::reconfigure(config1);
+    testConfigureDataSource(*this, config1);
     const ElementPtr
         config2(Element::fromJSON("{\"IN\": [{\"type\": 13}]}"));
     // This would delete CH. However, the IN one fails.
@@ -276,26 +271,26 @@ TEST_F(DatasrcConfiguratorTest, rollbackDeletion) {
     // and there's no known way to cause an exception during the
     // deletions, it is not a true rollback, but the result should
     // be the same.
-    EXPECT_THROW(Configurator::reconfigure(config2), TypeError);
+    EXPECT_THROW(testConfigureDataSource(*this, config2), TypeError);
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
 }
 
 // Check that we can roll back configuration change if something
 // fails later on.
-TEST_F(DatasrcConfiguratorTest, rollbackConfiguration) {
+TEST_F(DatasrcConfigTest, rollbackConfiguration) {
     initializeINList();
     // Put the CH there
     const ElementPtr
         config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
                                   "\"CH\": [{\"type\": \"xxx\"}]}"));
-    Configurator::reconfigure(config1);
+    testConfigureDataSource(*this, config1);
     // Now, the CH happens first. But nevertheless, it should be
     // restored to the previoeus version.
     const ElementPtr
         config2(Element::fromJSON("{\"IN\": [{\"type\": 13}], "
                                   "\"CH\": [{\"type\": \"yyy\"}]}"));
-    EXPECT_THROW(Configurator::reconfigure(config2), TypeError);
+    EXPECT_THROW(testConfigureDataSource(*this, config2), TypeError);
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
 }

src/lib/config/ccsession.cc

@@ -338,7 +338,7 @@ getRelatedLoggers(ConstElementPtr loggers) {
 
     // Now find the wildcard names (the one that start with "*").
     BOOST_FOREACH(ConstElementPtr cur_logger, loggers->listValue()) {
-        std::string cur_name = cur_logger->get("name")->stringValue();
+        const std::string cur_name = cur_logger->get("name")->stringValue();
         // If name is '*', or starts with '*.', replace * with root
         // logger name.
         if (cur_name == "*" || cur_name.length() > 1 &&
@@ -671,9 +671,7 @@ ModuleCCSession::fetchRemoteSpec(const std::string& module, bool is_filename) {
 
 std::string
 ModuleCCSession::addRemoteConfig(const std::string& spec_name,
-                                 void (*handler)(const std::string& module,
+                                 RemoteHandler handler,
-                                                 ConstElementPtr,
-                                                 const ConfigData&),
                                  bool spec_is_filename)
 {
     // First get the module name, specification and default config

src/lib/config/ccsession.h

@@ -283,7 +283,7 @@ public:
      *                  the configuration manager must know it). If
      *                  spec_is_filename is true (the default), then a
      *                  filename is assumed, otherwise a module name.
-     * \param handler The handler function called whenever there's a change.
+     * \param handler The handler functor called whenever there's a change.
      *                Called once initally from this function. May be NULL
      *                if you don't want any handler to be called and you're
      *                fine with requesting the data through
@@ -296,11 +296,11 @@ public:
      * \return The name of the module specified in the given specification
      *         file
      */
+    typedef boost::function<void(const std::string&,
+                                 isc::data::ConstElementPtr,
+                                 const ConfigData&)> RemoteHandler;
     std::string addRemoteConfig(const std::string& spec_name,
-                                void (*handler)(const std::string& module_name,
+                                RemoteHandler handler = RemoteHandler(),
-                                                isc::data::ConstElementPtr
-                                                update,
-                                                const ConfigData& config_data) = NULL,
                                 bool spec_is_filename = true);
 
     /**
@@ -513,9 +513,6 @@ private:
         const std::string& command,
         isc::data::ConstElementPtr args);
 
-    typedef void (*RemoteHandler)(const std::string&,
-                                  isc::data::ConstElementPtr,
-                                  const ConfigData&);
     std::map<std::string, ConfigData> remote_module_configs_;
     std::map<std::string, RemoteHandler> remote_module_handlers_;