[2203] changed configurator class to a simple function.

as it's now completely stateless and can work independently.
the common specialization for the main implementation is defined in a
new created .cc file.

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_configurator.h datasrc_configurator.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_configurator.h ../datasrc_configurator.cc
 
 nodist_query_bench_SOURCES = ../auth_messages.h ../auth_messages.cc
 

src/bin/auth/benchmarks/query_bench.cc

@@ -109,7 +109,6 @@ private:
     MockSocketSessionForwarder ddns_forwarder;
 protected:
     AuthSrvPtr server_;
-    DataSourceConfigurator datasrc_configurator_;
 private:
     const BenchQueries& queries_;
     Message& query_message_;
@@ -126,7 +125,7 @@ public:
                           OutputBuffer& buffer) :
         QueryBenchMark(queries, query_message, buffer)
     {
-        datasrc_configurator_.reconfigure(
+        configureDataSource(
             *server_,
             Element::fromJSON("{\"IN\":"
                               "  [{\"type\": \"sqlite3\","
@@ -145,7 +144,7 @@ public:
                          OutputBuffer& buffer) :
         QueryBenchMark(queries, query_message, buffer)
     {
-        datasrc_configurator_.reconfigure(
+        configureDataSource(
             *server_,
             Element::fromJSON("{\"IN\":"
                               "  [{\"type\": \"MasterFiles\","

src/bin/auth/datasrc_configurator.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_configurator.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_configurator.h

@@ -17,110 +17,101 @@
 
 #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 A class to configure the authoritative server's data source lists
+/// \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.
 ///
-/// 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.
+/// 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>
-class DataSourceConfiguratorGeneric {
-private:
+void
+configureDataSourceGeneric(Server& server,
+                           const isc::data::ConstElementPtr& config)
+{
     typedef boost::shared_ptr<List> ListPtr;
-public:
-    /// \brief Default constructor.
-    DataSourceConfiguratorGeneric() {}
+    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;
 
-    /// \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 server The server for which the data sources are to be
-    /// configured.
-    /// \param config The configuration value to parse. It is in the form
-    ///     as an update from the config manager.
-    void reconfigure(Server& server,
-                     const isc::data::ConstElementPtr& config)
-    {
-        // 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);
-                }
+    // 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()));
             }
-            // 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());
+            list->configure(it->second, true);
+            if (need_set) {
+                server.setClientList(rrclass, list);
             }
-        } catch (...) {
-            // Perform a rollback of the changes. The old configuration should
-            // work.
-            for (typename std::vector<RollbackPair>::const_iterator
+        }
+        // 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
+            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;
+             it != rollback_configurations.end(); ++it) {
+            server.getClientList(it->first)->configure(it->second, true);
         }
+        throw;
     }
-};
+}
 
 /// \brief Concrete version of DataSourceConfiguratorGeneric for the
-///     use in authoritative server.
-typedef DataSourceConfiguratorGeneric<AuthSrv,
-        isc::datasrc::ConfigurableClientList>
-    DataSourceConfigurator;
+///     use with authoritative server implementation.
+void
+configureDataSource(AuthSrv& server, const isc::data::ConstElementPtr& config);
 
 #endif
 

src/bin/auth/main.cc

@@ -87,13 +87,13 @@ my_command_handler(const string& command, ConstElementPtr args) {
 }
 
 void
-datasrcConfigHandler(AuthSrv* server, DataSourceConfigurator* configurator,
-                     const std::string&, isc::data::ConstElementPtr config,
+datasrcConfigHandler(AuthSrv* server, const std::string&,
+                     isc::data::ConstElementPtr config,
                      const isc::config::ConfigData&)
 {
     assert(server != NULL);
     if (config->contains("classes")) {
-        configurator->reconfigure(*server, config->get("classes"));
+        configureDataSource(*server, config->get("classes"));
     }
 }
 
@@ -141,7 +141,6 @@ main(int argc, char* argv[]) {
     ModuleCCSession* config_session = NULL;
     XfroutClient xfrout_client(getXfroutSocketPath());
     SocketSessionForwarder ddns_forwarder(getDDNSSocketPath());
-    boost::scoped_ptr<DataSourceConfigurator> datasrc_configurator;
     try {
         string specfile;
         if (getenv("B10_FROM_BUILD")) {
@@ -207,19 +206,16 @@ main(int argc, char* argv[]) {
         auth_server->setTSIGKeyRing(&isc::server_common::keyring);
 
         // Start the data source configuration
-        datasrc_configurator.reset(new DataSourceConfigurator);
         config_session->addRemoteConfig("data_sources",
                                         boost::bind(datasrcConfigHandler,
                                                     auth_server,
-                                                    datasrc_configurator.get(),
                                                     _1, _2, _3),
                                         false);
 
         // HACK: The default is not passed to the handler. This one will
         // get the default (or, current value). Further updates will work
         // the usual way.
-        datasrc_configurator->reconfigure(
-            *auth_server,
+        configureDataSource(*auth_server,
             config_session->getRemoteConfigValue("data_sources", "classes"));
 
         // Now start asynchronous read.
@@ -244,9 +240,10 @@ main(int argc, char* argv[]) {
         xfrin_session->disconnect();
     }
 
-    if (datasrc_configurator) {
-        config_session->removeRemoteConfig("data_sources");
-    }
+    // 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_configurator.h ../datasrc_configurator.cc
 run_unittests_SOURCES += datasrc_util.h datasrc_util.cc
 run_unittests_SOURCES += auth_srv_unittest.cc
 run_unittests_SOURCES += config_unittest.cc

src/bin/auth/tests/auth_srv_unittest.cc

@@ -183,7 +183,6 @@ protected:
     vector<uint8_t> response_data;
     AddressList address_store_;
     TestSocketRequestor sock_requestor_;
-    DataSourceConfigurator datasrc_configurator_;
 };
 
 // A helper function that builds a response to version.bind/TXT/CH that
@@ -723,21 +722,17 @@ TEST_F(AuthSrvTest, notifyWithSessionMessageError) {
 }
 
 void
-updateDatabase(AuthSrv& server, DataSourceConfigurator& datasrc_configurator,
-               const char* params)
-{
+updateDatabase(AuthSrv& server, const char* params) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"IN\": [{"
         "    \"type\": \"sqlite3\","
         "    \"params\": " + string(params) +
         "}]}"));
-    datasrc_configurator.reconfigure(server, config);
+    configureDataSource(server, config);
 }
 
 void
-updateInMemory(AuthSrv& server, DataSourceConfigurator& datasrc_configurator,
-               const char* origin, const char* filename)
-{
+updateInMemory(AuthSrv& server, const char* origin, const char* filename) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"IN\": [{"
         "   \"type\": \"MasterFiles\","
@@ -750,17 +745,17 @@ updateInMemory(AuthSrv& server, DataSourceConfigurator& datasrc_configurator,
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
-    datasrc_configurator.reconfigure(server, config);
+    configureDataSource(server, config);
 }
 
 void
-updateBuiltin(AuthSrv& server, DataSourceConfigurator& datasrc_configurator) {
+updateBuiltin(AuthSrv& server) {
     const ConstElementPtr config(Element::fromJSON("{"
         "\"CH\": [{"
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
-    datasrc_configurator.reconfigure(server, config);
+    configureDataSource(server, config);
 }
 
 // Try giving the server a TSIG signed request and see it can anwer signed as
@@ -771,7 +766,7 @@ TEST_F(AuthSrvTest, DISABLED_TSIGSigned) { // Needs builtin
 TEST_F(AuthSrvTest, TSIGSigned) {
 #endif
     // Prepare key, the client message, etc
-    updateBuiltin(server, datasrc_configurator_);
+    updateBuiltin(server);
     const TSIGKey key("key:c2VjcmV0Cg==:hmac-sha1");
     TSIGContext context(key);
     UnitTestUtil::createRequestMessage(request_message, opcode, default_qid,
@@ -819,7 +814,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQueryViaDNSServer) {
 #else
 TEST_F(AuthSrvTest, builtInQueryViaDNSServer) {
 #endif
-    updateBuiltin(server, datasrc_configurator_);
+    updateBuiltin(server);
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
                                        default_qid, Name("VERSION.BIND."),
                                        RRClass::CH(), RRType::TXT());
@@ -851,7 +846,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQuery) {
 #else
 TEST_F(AuthSrvTest, builtInQuery) {
 #endif
-    updateBuiltin(server, datasrc_configurator_);
+    updateBuiltin(server);
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
                                        default_qid, Name("VERSION.BIND."),
                                        RRClass::CH(), RRType::TXT());
@@ -872,7 +867,7 @@ TEST_F(AuthSrvTest, DISABLED_iqueryViaDNSServer) { // Needs builtin
 #else
 TEST_F(AuthSrvTest, iqueryViaDNSServer) { // Needs builtin
 #endif
-    updateBuiltin(server, datasrc_configurator_);
+    updateBuiltin(server);
     createDataFromFile("iquery_fromWire.wire");
     (*server.getDNSLookupProvider())(*io_message, parse_message,
                                      response_message,
@@ -894,7 +889,7 @@ TEST_F(AuthSrvTest, DISABLED_updateConfig) {
 #else
 TEST_F(AuthSrvTest, updateConfig) {
 #endif
-    updateDatabase(server, datasrc_configurator_, 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
@@ -912,7 +907,7 @@ TEST_F(AuthSrvTest, DISABLED_datasourceFail) {
 #else
 TEST_F(AuthSrvTest, datasourceFail) {
 #endif
-    updateDatabase(server, datasrc_configurator_, 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
@@ -932,11 +927,10 @@ TEST_F(AuthSrvTest, DISABLED_updateConfigFail) {
 TEST_F(AuthSrvTest, updateConfigFail) {
 #endif
     // First, load a valid data source.
-    updateDatabase(server, datasrc_configurator_, CONFIG_TESTDB);
+    updateDatabase(server, CONFIG_TESTDB);
 
     // Next, try to update it with a non-existent one.  This should fail.
-    EXPECT_THROW(updateDatabase(server, datasrc_configurator_,
-                                BADCONFIG_TESTDB),
+    EXPECT_THROW(updateDatabase(server, BADCONFIG_TESTDB),
                  isc::datasrc::DataSourceError);
 
     // The original data source should still exist.
@@ -959,7 +953,7 @@ TEST_F(AuthSrvTest, updateWithInMemoryClient) {
         "   \"params\": {},"
         "   \"cache-enable\": true"
         "}]}"));
-    datasrc_configurator_.reconfigure(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.
@@ -980,8 +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, datasrc_configurator_, "example.",
-                   CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1000,8 +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, datasrc_configurator_, "example.",
-                   CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
     createDataFromFile("nsec3query_fromWire.wire");
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1021,8 +1013,7 @@ TEST_F(AuthSrvTest,
     )
 {
     // Set up the in-memory
-    updateInMemory(server, datasrc_configurator_, "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(),
@@ -1434,8 +1425,7 @@ TEST_F(AuthSrvTest,
     )
 {
     // Set real inmem client to proxy
-    updateInMemory(server, datasrc_configurator_, "example.",
-                   CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
     {
         isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
         boost::shared_ptr<isc::datasrc::ConfigurableClientList>
@@ -1460,12 +1450,10 @@ 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, DataSourceConfigurator& datasrc_configurator,
-           ThrowWhen throw_when, bool isc_exception,
+setupThrow(AuthSrv& server, ThrowWhen throw_when, bool isc_exception,
            ConstRRsetPtr rrset = ConstRRsetPtr())
 {
-    updateInMemory(server, datasrc_configurator, "example.",
-                   CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
 
     isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
     boost::shared_ptr<isc::datasrc::ConfigurableClientList>
@@ -1494,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, datasrc_configurator_, *when, true);
+        setupThrow(server, *when, true);
         processAndCheckSERVFAIL();
         // To be sure, check same for non-isc-exceptions
         createRequestPacket(request_message, IPPROTO_UDP);
-        setupThrow(server, datasrc_configurator_, *when, false);
+        setupThrow(server, *when, false);
         processAndCheckSERVFAIL();
     }
 }
@@ -1514,7 +1502,7 @@ TEST_F(AuthSrvTest,
     )
 {
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
-    setupThrow(server, datasrc_configurator_, 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,
@@ -1538,7 +1526,7 @@ TEST_F(AuthSrvTest,
     ConstRRsetPtr empty_rrset(new RRset(Name("foo.example"),
                                         RRClass::IN(), RRType::TXT(),
                                         RRTTL(0)));
-    setupThrow(server, datasrc_configurator_, 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

@@ -77,7 +77,6 @@ protected:
     MockXfroutClient xfrout_;
     MockSocketSessionForwarder ddns_forwarder_;
     AuthSrv server_;
-    DataSourceConfigurator datasrc_configurator_;
     ConstElementPtr result_;
     // The shutdown command parameter
     ConstElementPtr param_;
@@ -191,7 +190,7 @@ zoneChecks(AuthSrv& server) {
 }
 
 void
-configureZones(AuthSrv& server, DataSourceConfigurator& datasrc_configurator) {
+configureZones(AuthSrv& server) {
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test1.zone.in "
                         TEST_DATA_BUILDDIR "/test1.zone.copied"));
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test2.zone.in "
@@ -209,7 +208,7 @@ configureZones(AuthSrv& server, DataSourceConfigurator& datasrc_configurator) {
         "   \"cache-enable\": true"
         "}]}"));
 
-    datasrc_configurator.reconfigure(server, config);
+    configureDataSource(server, config);
 
     zoneChecks(server);
 }
@@ -235,7 +234,7 @@ newZoneChecks(AuthSrv& server) {
 }
 
 TEST_F(AuthCommandTest, loadZone) {
-    configureZones(server_, datasrc_configurator_);
+    configureZones(server_);
 
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR
                         "/test1-new.zone.in "
@@ -272,7 +271,7 @@ TEST_F(AuthCommandTest,
         "    \"cache-enable\": true,"
         "    \"cache-zones\": [\"example.org\"]"
         "}]}"));
-    datasrc_configurator_.reconfigure(server_, config);
+    configureDataSource(server_, config);
 
     {
         isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
@@ -336,7 +335,7 @@ TEST_F(AuthCommandTest,
         "    \"cache-enable\": true,"
         "    \"cache-zones\": [\"example.com\"]"
         "}]}"));
-    EXPECT_THROW(datasrc_configurator_.reconfigure(server_, config2),
+    EXPECT_THROW(configureDataSource(server_, config2),
                  ConfigurableClientList::ConfigurationError);
 
     result_ = execAuthServerCommand(server_, "loadzone",
@@ -351,7 +350,7 @@ TEST_F(AuthCommandTest,
 }
 
 TEST_F(AuthCommandTest, loadBrokenZone) {
-    configureZones(server_, datasrc_configurator_);
+    configureZones(server_);
 
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR
                         "/test1-broken.zone.in "
@@ -364,7 +363,7 @@ TEST_F(AuthCommandTest, loadBrokenZone) {
 }
 
 TEST_F(AuthCommandTest, loadUnreadableZone) {
-    configureZones(server_, datasrc_configurator_);
+    configureZones(server_);
 
     // install the zone file as unreadable
     ASSERT_EQ(0, system(INSTALL_PROG " -c -m 000 " TEST_DATA_DIR
@@ -387,7 +386,7 @@ TEST_F(AuthCommandTest, loadZoneWithoutDataSrc) {
 }
 
 TEST_F(AuthCommandTest, loadZoneInvalidParams) {
-    configureZones(server_, datasrc_configurator_);
+    configureZones(server_);
 
     // null arg
     result_ = execAuthServerCommand(server_, "loadzone", ElementPtr());

src/bin/auth/tests/datasrc_configurator_unittest.cc

@@ -60,19 +60,23 @@ private:
 
 typedef shared_ptr<FakeList> ListPtr;
 
-// We use the test fixture as both parameters, this makes it possible
-// to easily fake all needed methods and look that they were called.
-typedef DataSourceConfiguratorGeneric<DatasrcConfiguratorTest,
-        FakeList> Configurator;
+void
+testConfigureDataSource(DatasrcConfiguratorTest& test,
+                        const isc::data::ConstElementPtr& config)
+{
+    // 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<DatasrcConfiguratorTest, FakeList>(test,
+                                                                  config);
+}
 
 void
-datasrcConfigHandler(DatasrcConfiguratorTest* fake_server,
-                     Configurator* configurator, const std::string&,
+datasrcConfigHandler(DatasrcConfiguratorTest* fake_server, const std::string&,
                      isc::data::ConstElementPtr config,
                      const isc::config::ConfigData&)
 {
     if (config->contains("classes")) {
-        configurator->reconfigure(*fake_server, config->get("classes"));
+        testConfigureDataSource(*fake_server, config->get("classes"));
     }
 }
 
@@ -113,28 +117,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
+        // 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, config));
-        } else {
-            session.getMessages()->
-                add(createAnswer(0, ElementPtr(new MapElement)));
-        }
+        session.getMessages()->add(createAnswer(0,
+                                                ElementPtr(new MapElement)));
         mccs->addRemoteConfig("data_sources",
                               boost::bind(datasrcConfigHandler,
-                                          this, &configurator_,
-                                          _1, _2, _3), false);
-    }
-    void SetUp() {
-        init();
+                                          this, _1, _2, _3), false);
     }
     ElementPtr buildConfig(const string& config) const {
         const ElementPtr internal(Element::fromJSON(config));
@@ -155,31 +155,12 @@ protected:
     }
     FakeSession session;
     auto_ptr<ModuleCCSession> mccs;
-    Configurator configurator_;
     const string specfile;
     std::map<RRClass, ListPtr> lists_;
     string log_;
     mutable isc::util::thread::Mutex mutex_;
 };
 
-// Check the initialization (and cleanup)
-TEST_F(DatasrcConfiguratorTest, DISABLED_initialization) {
-    // It can't be initialized again
-    EXPECT_THROW(init(), InvalidOperation);
-    EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
-    EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
-    // We can't reconfigure now (not even manually)
-    EXPECT_THROW(configurator_.reconfigure(*this,
-                                           ElementPtr(new MapElement())),
-                 InvalidOperation);
-    // If the server param is NULL, it does not work
-    EXPECT_THROW(Configurator configurator, InvalidParameter);
-    EXPECT_FALSE(session.haveSubscription("data_sources", "*")); // TBD
-    // 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) {
     initializeINList();
@@ -283,7 +264,7 @@ TEST_F(DatasrcConfiguratorTest, rollbackDeletion) {
     const ElementPtr
         config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
                                   "\"CH\": [{\"type\": \"xxx\"}]}"));
-    configurator_.reconfigure(*this, config1);
+    testConfigureDataSource(*this, config1);
     const ElementPtr
         config2(Element::fromJSON("{\"IN\": [{\"type\": 13}]}"));
     // This would delete CH. However, the IN one fails.
@@ -291,7 +272,7 @@ 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(*this, config2), TypeError);
+    EXPECT_THROW(testConfigureDataSource(*this, config2), TypeError);
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
 }
@@ -304,13 +285,13 @@ TEST_F(DatasrcConfiguratorTest, rollbackConfiguration) {
     const ElementPtr
         config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
                                   "\"CH\": [{\"type\": \"xxx\"}]}"));
-    configurator_.reconfigure(*this, 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(*this, config2), TypeError);
+    EXPECT_THROW(testConfigureDataSource(*this, config2), TypeError);
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
 }