Parcourir la source

[2203] refactoring 2nd step: configurator can now be a separate object.

i.e., it's not a singleton any more.
testReconfigure() method isn't needed any more because it doesn't hold
CC session internally.
DatasrcConfiguratorTest.initialization test currently fails and is
disabled for now.  The plan is to make the class completely stateless,
at which point we don't even have to think about initialization or cleanup,
and then the test will be able to be removed.
JINMEI Tatuya il y a 12 ans
Parent
commit
05136b55f9

+ 7 - 7
src/bin/auth/benchmarks/query_bench.cc

@@ -81,6 +81,7 @@ protected:
     QueryBenchMark(const BenchQueries& queries, Message& query_message,
     QueryBenchMark(const BenchQueries& queries, Message& query_message,
                    OutputBuffer& buffer) :
                    OutputBuffer& buffer) :
         server_(new AuthSrv(xfrout_client, ddns_forwarder)),
         server_(new AuthSrv(xfrout_client, ddns_forwarder)),
+        datasrc_configurator_(server_.get()),
         queries_(queries),
         queries_(queries),
         query_message_(query_message),
         query_message_(query_message),
         buffer_(buffer),
         buffer_(buffer),
@@ -109,6 +110,7 @@ private:
     MockSocketSessionForwarder ddns_forwarder;
     MockSocketSessionForwarder ddns_forwarder;
 protected:
 protected:
     AuthSrvPtr server_;
     AuthSrvPtr server_;
+    DataSourceConfigurator datasrc_configurator_;
 private:
 private:
     const BenchQueries& queries_;
     const BenchQueries& queries_;
     Message& query_message_;
     Message& query_message_;
@@ -125,8 +127,7 @@ public:
                           OutputBuffer& buffer) :
                           OutputBuffer& buffer) :
         QueryBenchMark(queries, query_message, buffer)
         QueryBenchMark(queries, query_message, buffer)
     {
     {
-        DataSourceConfigurator::testReconfigure(
-            server_.get(),
+        datasrc_configurator_.reconfigure(
             Element::fromJSON("{\"IN\":"
             Element::fromJSON("{\"IN\":"
                               "  [{\"type\": \"sqlite3\","
                               "  [{\"type\": \"sqlite3\","
                               "    \"params\": {"
                               "    \"params\": {"
@@ -139,13 +140,12 @@ class MemoryQueryBenchMark  : public QueryBenchMark {
 public:
 public:
     MemoryQueryBenchMark(const char* const zone_file,
     MemoryQueryBenchMark(const char* const zone_file,
                          const char* const zone_origin,
                          const char* const zone_origin,
-                          const BenchQueries& queries,
-                          Message& query_message,
-                          OutputBuffer& buffer) :
+                         const BenchQueries& queries,
+                         Message& query_message,
+                         OutputBuffer& buffer) :
         QueryBenchMark(queries, query_message, buffer)
         QueryBenchMark(queries, query_message, buffer)
     {
     {
-        DataSourceConfigurator::testReconfigure(
-            server_.get(),
+        datasrc_configurator_.reconfigure(
             Element::fromJSON("{\"IN\":"
             Element::fromJSON("{\"IN\":"
                               "  [{\"type\": \"MasterFiles\","
                               "  [{\"type\": \"MasterFiles\","
                               "    \"cache-enable\": true, "
                               "    \"cache-enable\": true, "

+ 15 - 61
src/bin/auth/datasrc_configurator.h

@@ -30,32 +30,25 @@
 /// keep the local copy of data source clients in the list in the authoritative
 /// keep the local copy of data source clients in the list in the authoritative
 /// server.
 /// 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.
 /// 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
 /// You don't need to pay attention to it, use the DataSourceConfigurator
 /// type alias instead.
 /// type alias instead.
 template<class Server, class List>
 template<class Server, class List>
 class DataSourceConfiguratorGeneric {
 class DataSourceConfiguratorGeneric {
 private:
 private:
-    /// \brief Disallow creation of instances
-    DataSourceConfiguratorGeneric();
-    /// \brief Internal method to hook into the ModuleCCSession
+    Server* server_;
+    typedef boost::shared_ptr<List> ListPtr;
+public:
+    /// \brief Constructor.
     ///
     ///
-    /// 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"));
+    /// \throw isc::InvalidParameter if server is NULL
+    /// \param server The server to configure.
+    DataSourceConfiguratorGeneric(Server* server) : server_(server) {
+        if (server == NULL) {
+            isc_throw(isc::InvalidParameter, "The server must not be NULL");
         }
         }
     }
     }
-    static Server* server_;
-    typedef boost::shared_ptr<List> ListPtr;
-public:
+
     /// \brief Initializes the class.
     /// \brief Initializes the class.
     ///
     ///
     /// This configures which session and server should be used.
     /// This configures which session and server should be used.
@@ -66,22 +59,13 @@ public:
     /// session dies, otherwise it might access them after they
     /// session dies, otherwise it might access them after they
     /// are destroyed.
     /// are destroyed.
     ///
     ///
-    /// \param session The session to hook into and to access the configuration
-    ///     through.
     /// \param server It is the server to configure.
     /// \param server It is the server to configure.
-    /// \throw isc::InvalidOperation if this is called when already initialized.
+    /// \throw isc::InvalidOperation if this is called when already
+    /// initialized.
     /// \throw isc::InvalidParameter if any of the parameters is NULL
     /// \throw isc::InvalidParameter if any of the parameters is NULL
     /// \throw isc::config::ModuleCCError if the remote configuration is not
     /// \throw isc::config::ModuleCCError if the remote configuration is not
     ///     available for some reason.
     ///     available for some reason.
-    static void init(Server* server) {
-        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;
+    void init() {
     }
     }
 
 
     /// \brief Deinitializes the class.
     /// \brief Deinitializes the class.
@@ -91,7 +75,7 @@ public:
     ///
     ///
     /// This can be called even if it is not initialized currently. You
     /// This can be called even if it is not initialized currently. You
     /// can initialize it again after this.
     /// can initialize it again after this.
-    static void cleanup() {
+    void cleanup() {
         server_ = NULL;
         server_ = NULL;
     }
     }
 
 
@@ -105,7 +89,7 @@ public:
     /// \param config The configuration value to parse. It is in the form
     /// \param config The configuration value to parse. It is in the form
     ///     as an update from the config manager.
     ///     as an update from the config manager.
     /// \throw InvalidOperation if it is called when not initialized.
     /// \throw InvalidOperation if it is called when not initialized.
-    static void reconfigure(const isc::data::ConstElementPtr& config) {
+    void reconfigure(const isc::data::ConstElementPtr& config) {
         if (server_ == NULL) {
         if (server_ == NULL) {
             isc_throw(isc::InvalidOperation,
             isc_throw(isc::InvalidOperation,
                       "Can't reconfigure while not initialized by init()");
                       "Can't reconfigure while not initialized by init()");
@@ -171,38 +155,8 @@ public:
             throw;
             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>
-Server* DataSourceConfiguratorGeneric<Server, List>::server_(NULL);
-
 /// \brief Concrete version of DataSourceConfiguratorGeneric for the
 /// \brief Concrete version of DataSourceConfiguratorGeneric for the
 ///     use in authoritative server.
 ///     use in authoritative server.
 typedef DataSourceConfiguratorGeneric<AuthSrv,
 typedef DataSourceConfiguratorGeneric<AuthSrv,

+ 27 - 18
src/bin/auth/main.cc

@@ -14,17 +14,6 @@
 
 
 #include <config.h>
 #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 <exceptions/exceptions.h>
 
 
 #include <util/buffer.h>
 #include <util/buffer.h>
@@ -52,6 +41,20 @@
 #include <server_common/keyring.h>
 #include <server_common/keyring.h>
 #include <server_common/socket_request.h>
 #include <server_common/socket_request.h>
 
 
+#include <boost/bind.hpp>
+#include <boost/scoped_ptr.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 std;
 using namespace isc::asiodns;
 using namespace isc::asiodns;
 using namespace isc::asiolink;
 using namespace isc::asiolink;
@@ -84,12 +87,12 @@ my_command_handler(const string& command, ConstElementPtr args) {
 }
 }
 
 
 void
 void
-datasrcConfigHandler(const std::string&,
+datasrcConfigHandler(DataSourceConfigurator* configurator, const std::string&,
                      isc::data::ConstElementPtr config,
                      isc::data::ConstElementPtr config,
                      const isc::config::ConfigData&)
                      const isc::config::ConfigData&)
 {
 {
     if (config->contains("classes")) {
     if (config->contains("classes")) {
-        DataSourceConfigurator::reconfigure(config->get("classes"));
+        configurator->reconfigure(config->get("classes"));
     }
     }
 }
 }
 
 
@@ -137,6 +140,7 @@ main(int argc, char* argv[]) {
     ModuleCCSession* config_session = NULL;
     ModuleCCSession* config_session = NULL;
     XfroutClient xfrout_client(getXfroutSocketPath());
     XfroutClient xfrout_client(getXfroutSocketPath());
     SocketSessionForwarder ddns_forwarder(getDDNSSocketPath());
     SocketSessionForwarder ddns_forwarder(getDDNSSocketPath());
+    boost::scoped_ptr<DataSourceConfigurator> datasrc_configurator;
     try {
     try {
         string specfile;
         string specfile;
         if (getenv("B10_FROM_BUILD")) {
         if (getenv("B10_FROM_BUILD")) {
@@ -202,13 +206,17 @@ main(int argc, char* argv[]) {
         auth_server->setTSIGKeyRing(&isc::server_common::keyring);
         auth_server->setTSIGKeyRing(&isc::server_common::keyring);
 
 
         // Start the data source configuration
         // Start the data source configuration
-        DataSourceConfigurator::init(auth_server);
-        config_session->addRemoteConfig("data_sources", datasrcConfigHandler,
+        datasrc_configurator.reset(new DataSourceConfigurator(auth_server));
+        config_session->addRemoteConfig("data_sources",
+                                        boost::bind(datasrcConfigHandler,
+                                                    datasrc_configurator.get(),
+                                                    _1, _2, _3),
                                         false);
                                         false);
+
         // HACK: The default is not passed to the handler. This one will
         // HACK: The default is not passed to the handler. This one will
         // get the default (or, current value). Further updates will work
         // get the default (or, current value). Further updates will work
         // the usual way.
         // the usual way.
-        DataSourceConfigurator::reconfigure(
+        datasrc_configurator->reconfigure(
             config_session->getRemoteConfigValue("data_sources", "classes"));
             config_session->getRemoteConfigValue("data_sources", "classes"));
 
 
         // Now start asynchronous read.
         // Now start asynchronous read.
@@ -233,8 +241,9 @@ main(int argc, char* argv[]) {
         xfrin_session->disconnect();
         xfrin_session->disconnect();
     }
     }
 
 
-    //DataSourceConfigurator::cleanup();
-    config_session->removeRemoteConfig("data_sources");
+    if (datasrc_configurator) {
+        config_session->removeRemoteConfig("data_sources");
+    }
     delete xfrin_session;
     delete xfrin_session;
     delete config_session;
     delete config_session;
     delete cc_session;
     delete cc_session;

+ 33 - 26
src/bin/auth/tests/auth_srv_unittest.cc

@@ -98,7 +98,8 @@ protected:
         // The empty string is expected value of the parameter of
         // The empty string is expected value of the parameter of
         // requestSocket, not the app_name (there's no fallback, it checks
         // requestSocket, not the app_name (there's no fallback, it checks
         // the empty string is passed).
         // the empty string is passed).
-        sock_requestor_(dnss_, address_store_, 53210, "")
+        sock_requestor_(dnss_, address_store_, 53210, ""),
+        datasrc_configurator_(&server)
     {
     {
         server.setDNSService(dnss_);
         server.setDNSService(dnss_);
         server.setXfrinSession(&notify_session);
         server.setXfrinSession(&notify_session);
@@ -183,6 +184,7 @@ protected:
     vector<uint8_t> response_data;
     vector<uint8_t> response_data;
     AddressList address_store_;
     AddressList address_store_;
     TestSocketRequestor sock_requestor_;
     TestSocketRequestor sock_requestor_;
+    DataSourceConfigurator datasrc_configurator_;
 };
 };
 
 
 // A helper function that builds a response to version.bind/TXT/CH that
 // A helper function that builds a response to version.bind/TXT/CH that
@@ -722,17 +724,21 @@ TEST_F(AuthSrvTest, notifyWithSessionMessageError) {
 }
 }
 
 
 void
 void
-updateDatabase(AuthSrv* server, const char* params) {
+updateDatabase(DataSourceConfigurator& datasrc_configurator,
+               const char* params)
+{
     const ConstElementPtr config(Element::fromJSON("{"
     const ConstElementPtr config(Element::fromJSON("{"
         "\"IN\": [{"
         "\"IN\": [{"
         "    \"type\": \"sqlite3\","
         "    \"type\": \"sqlite3\","
         "    \"params\": " + string(params) +
         "    \"params\": " + string(params) +
         "}]}"));
         "}]}"));
-    DataSourceConfigurator::testReconfigure(server, config);
+    datasrc_configurator.reconfigure(config);
 }
 }
 
 
 void
 void
-updateInMemory(AuthSrv* server, const char* origin, const char* filename) {
+updateInMemory(DataSourceConfigurator& datasrc_configurator,
+               const char* origin, const char* filename)
+{
     const ConstElementPtr config(Element::fromJSON("{"
     const ConstElementPtr config(Element::fromJSON("{"
         "\"IN\": [{"
         "\"IN\": [{"
         "   \"type\": \"MasterFiles\","
         "   \"type\": \"MasterFiles\","
@@ -745,17 +751,17 @@ updateInMemory(AuthSrv* server, const char* origin, const char* filename) {
         "   \"type\": \"static\","
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
         "}]}"));
-    DataSourceConfigurator::testReconfigure(server, config);
+    datasrc_configurator.reconfigure(config);
 }
 }
 
 
 void
 void
-updateBuiltin(AuthSrv* server) {
+updateBuiltin(DataSourceConfigurator& datasrc_configurator) {
     const ConstElementPtr config(Element::fromJSON("{"
     const ConstElementPtr config(Element::fromJSON("{"
         "\"CH\": [{"
         "\"CH\": [{"
         "   \"type\": \"static\","
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
         "}]}"));
-    DataSourceConfigurator::testReconfigure(server, config);
+    datasrc_configurator.reconfigure(config);
 }
 }
 
 
 // Try giving the server a TSIG signed request and see it can anwer signed as
 // Try giving the server a TSIG signed request and see it can anwer signed as
@@ -766,7 +772,7 @@ TEST_F(AuthSrvTest, DISABLED_TSIGSigned) { // Needs builtin
 TEST_F(AuthSrvTest, TSIGSigned) {
 TEST_F(AuthSrvTest, TSIGSigned) {
 #endif
 #endif
     // Prepare key, the client message, etc
     // Prepare key, the client message, etc
-    updateBuiltin(&server);
+    updateBuiltin(datasrc_configurator_);
     const TSIGKey key("key:c2VjcmV0Cg==:hmac-sha1");
     const TSIGKey key("key:c2VjcmV0Cg==:hmac-sha1");
     TSIGContext context(key);
     TSIGContext context(key);
     UnitTestUtil::createRequestMessage(request_message, opcode, default_qid,
     UnitTestUtil::createRequestMessage(request_message, opcode, default_qid,
@@ -814,7 +820,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQueryViaDNSServer) {
 #else
 #else
 TEST_F(AuthSrvTest, builtInQueryViaDNSServer) {
 TEST_F(AuthSrvTest, builtInQueryViaDNSServer) {
 #endif
 #endif
-    updateBuiltin(&server);
+    updateBuiltin(datasrc_configurator_);
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
                                        default_qid, Name("VERSION.BIND."),
                                        default_qid, Name("VERSION.BIND."),
                                        RRClass::CH(), RRType::TXT());
                                        RRClass::CH(), RRType::TXT());
@@ -846,7 +852,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQuery) {
 #else
 #else
 TEST_F(AuthSrvTest, builtInQuery) {
 TEST_F(AuthSrvTest, builtInQuery) {
 #endif
 #endif
-    updateBuiltin(&server);
+    updateBuiltin(datasrc_configurator_);
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
                                        default_qid, Name("VERSION.BIND."),
                                        default_qid, Name("VERSION.BIND."),
                                        RRClass::CH(), RRType::TXT());
                                        RRClass::CH(), RRType::TXT());
@@ -867,7 +873,7 @@ TEST_F(AuthSrvTest, DISABLED_iqueryViaDNSServer) { // Needs builtin
 #else
 #else
 TEST_F(AuthSrvTest, iqueryViaDNSServer) { // Needs builtin
 TEST_F(AuthSrvTest, iqueryViaDNSServer) { // Needs builtin
 #endif
 #endif
-    updateBuiltin(&server);
+    updateBuiltin(datasrc_configurator_);
     createDataFromFile("iquery_fromWire.wire");
     createDataFromFile("iquery_fromWire.wire");
     (*server.getDNSLookupProvider())(*io_message, parse_message,
     (*server.getDNSLookupProvider())(*io_message, parse_message,
                                      response_message,
                                      response_message,
@@ -889,7 +895,7 @@ TEST_F(AuthSrvTest, DISABLED_updateConfig) {
 #else
 #else
 TEST_F(AuthSrvTest, updateConfig) {
 TEST_F(AuthSrvTest, updateConfig) {
 #endif
 #endif
-    updateDatabase(&server, CONFIG_TESTDB);
+    updateDatabase(datasrc_configurator_, CONFIG_TESTDB);
 
 
     // query for existent data in the installed data source.  The resulting
     // 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
     // response should have the AA flag on, and have an RR in each answer
@@ -907,7 +913,7 @@ TEST_F(AuthSrvTest, DISABLED_datasourceFail) {
 #else
 #else
 TEST_F(AuthSrvTest, datasourceFail) {
 TEST_F(AuthSrvTest, datasourceFail) {
 #endif
 #endif
-    updateDatabase(&server, CONFIG_TESTDB);
+    updateDatabase(datasrc_configurator_, CONFIG_TESTDB);
 
 
     // This query will hit a corrupted entry of the data source (the zoneload
     // 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
     // tool and the data source itself naively accept it).  This will result
@@ -927,10 +933,10 @@ TEST_F(AuthSrvTest, DISABLED_updateConfigFail) {
 TEST_F(AuthSrvTest, updateConfigFail) {
 TEST_F(AuthSrvTest, updateConfigFail) {
 #endif
 #endif
     // First, load a valid data source.
     // First, load a valid data source.
-    updateDatabase(&server, CONFIG_TESTDB);
+    updateDatabase(datasrc_configurator_, CONFIG_TESTDB);
 
 
     // Next, try to update it with a non-existent one.  This should fail.
     // Next, try to update it with a non-existent one.  This should fail.
-    EXPECT_THROW(updateDatabase(&server, BADCONFIG_TESTDB),
+    EXPECT_THROW(updateDatabase(datasrc_configurator_, BADCONFIG_TESTDB),
                  isc::datasrc::DataSourceError);
                  isc::datasrc::DataSourceError);
 
 
     // The original data source should still exist.
     // The original data source should still exist.
@@ -953,7 +959,7 @@ TEST_F(AuthSrvTest, updateWithInMemoryClient) {
         "   \"params\": {},"
         "   \"params\": {},"
         "   \"cache-enable\": true"
         "   \"cache-enable\": true"
         "}]}"));
         "}]}"));
-    DataSourceConfigurator::testReconfigure(&server, config);
+    datasrc_configurator_.reconfigure(config);
     // after successful configuration, we should have one (with empty zoneset).
     // after successful configuration, we should have one (with empty zoneset).
 
 
     // The memory data source is empty, should return REFUSED rcode.
     // The memory data source is empty, should return REFUSED rcode.
@@ -974,7 +980,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientNoDNSSEC) {
     // query handler class, and confirm it returns no error and a non empty
     // query handler class, and confirm it returns no error and a non empty
     // answer section.  Detailed examination on the response content
     // answer section.  Detailed examination on the response content
     // for various types of queries are tested in the query tests.
     // for various types of queries are tested in the query tests.
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(datasrc_configurator_, "example.", CONFIG_INMEMORY_EXAMPLE);
 
 
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
     server.processMessage(*io_message, *parse_message, *response_obuffer,
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -993,7 +999,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientDNSSEC) {
     // Similar to the previous test, but the query has the DO bit on.
     // 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 response should contain RRSIGs, and should have more RRs than
     // the previous case.
     // the previous case.
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(datasrc_configurator_, "example.", CONFIG_INMEMORY_EXAMPLE);
 
 
     createDataFromFile("nsec3query_fromWire.wire");
     createDataFromFile("nsec3query_fromWire.wire");
     server.processMessage(*io_message, *parse_message, *response_obuffer,
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1013,7 +1019,7 @@ TEST_F(AuthSrvTest,
     )
     )
 {
 {
     // Set up the in-memory
     // Set up the in-memory
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(datasrc_configurator_, "example.", CONFIG_INMEMORY_EXAMPLE);
 
 
     // This shouldn't affect the result of class CH query
     // This shouldn't affect the result of class CH query
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
     UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
@@ -1425,7 +1431,7 @@ TEST_F(AuthSrvTest,
     )
     )
 {
 {
     // Set real inmem client to proxy
     // Set real inmem client to proxy
-    updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(datasrc_configurator_, "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>
         boost::shared_ptr<isc::datasrc::ConfigurableClientList>
@@ -1450,10 +1456,11 @@ TEST_F(AuthSrvTest,
 // If non null rrset is given, it will be passed to the proxy so it can
 // If non null rrset is given, it will be passed to the proxy so it can
 // return some faked response.
 // return some faked response.
 void
 void
-setupThrow(AuthSrv* server, ThrowWhen throw_when, bool isc_exception,
+setupThrow(AuthSrv* server, DataSourceConfigurator& datasrc_configurator,
+           ThrowWhen throw_when, bool isc_exception,
            ConstRRsetPtr rrset = ConstRRsetPtr())
            ConstRRsetPtr rrset = ConstRRsetPtr())
 {
 {
-    updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE);
+    updateInMemory(datasrc_configurator, "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>
     boost::shared_ptr<isc::datasrc::ConfigurableClientList>
@@ -1482,11 +1489,11 @@ TEST_F(AuthSrvTest,
                                              RRClass::IN(), RRType::TXT());
                                              RRClass::IN(), RRType::TXT());
     for (ThrowWhen* when(throws); *when != THROW_NEVER; ++when) {
     for (ThrowWhen* when(throws); *when != THROW_NEVER; ++when) {
         createRequestPacket(request_message, IPPROTO_UDP);
         createRequestPacket(request_message, IPPROTO_UDP);
-        setupThrow(&server, *when, true);
+        setupThrow(&server, datasrc_configurator_, *when, true);
         processAndCheckSERVFAIL();
         processAndCheckSERVFAIL();
         // To be sure, check same for non-isc-exceptions
         // To be sure, check same for non-isc-exceptions
         createRequestPacket(request_message, IPPROTO_UDP);
         createRequestPacket(request_message, IPPROTO_UDP);
-        setupThrow(&server, *when, false);
+        setupThrow(&server, datasrc_configurator_, *when, false);
         processAndCheckSERVFAIL();
         processAndCheckSERVFAIL();
     }
     }
 }
 }
@@ -1502,7 +1509,7 @@ TEST_F(AuthSrvTest,
     )
     )
 {
 {
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
-    setupThrow(&server, THROW_AT_GET_CLASS, true);
+    setupThrow(&server, datasrc_configurator_, THROW_AT_GET_CLASS, true);
 
 
     // getClass is not called so it should just answer
     // getClass is not called so it should just answer
     server.processMessage(*io_message, *parse_message, *response_obuffer,
     server.processMessage(*io_message, *parse_message, *response_obuffer,
@@ -1526,7 +1533,7 @@ TEST_F(AuthSrvTest,
     ConstRRsetPtr empty_rrset(new RRset(Name("foo.example"),
     ConstRRsetPtr empty_rrset(new RRset(Name("foo.example"),
                                         RRClass::IN(), RRType::TXT(),
                                         RRClass::IN(), RRType::TXT(),
                                         RRTTL(0)));
                                         RRTTL(0)));
-    setupThrow(&server, THROW_NEVER, true, empty_rrset);
+    setupThrow(&server, datasrc_configurator_, THROW_NEVER, true, empty_rrset);
 
 
     // Repeat the query processing two times.  Due to the faked RRset,
     // Repeat the query processing two times.  Due to the faked RRset,
     // toWire() should throw, and it should result in SERVFAIL.
     // toWire() should throw, and it should result in SERVFAIL.

+ 10 - 8
src/bin/auth/tests/command_unittest.cc

@@ -64,6 +64,7 @@ class AuthCommandTest : public ::testing::Test {
 protected:
 protected:
     AuthCommandTest() :
     AuthCommandTest() :
         server_(xfrout_, ddns_forwarder_),
         server_(xfrout_, ddns_forwarder_),
+        datasrc_configurator_(&server_),
         rcode_(-1),
         rcode_(-1),
         expect_rcode_(0),
         expect_rcode_(0),
         itimer_(server_.getIOService())
         itimer_(server_.getIOService())
@@ -77,6 +78,7 @@ protected:
     MockXfroutClient xfrout_;
     MockXfroutClient xfrout_;
     MockSocketSessionForwarder ddns_forwarder_;
     MockSocketSessionForwarder ddns_forwarder_;
     AuthSrv server_;
     AuthSrv server_;
+    DataSourceConfigurator datasrc_configurator_;
     ConstElementPtr result_;
     ConstElementPtr result_;
     // The shutdown command parameter
     // The shutdown command parameter
     ConstElementPtr param_;
     ConstElementPtr param_;
@@ -190,7 +192,7 @@ zoneChecks(AuthSrv& server) {
 }
 }
 
 
 void
 void
-configureZones(AuthSrv& server) {
+configureZones(AuthSrv& server, DataSourceConfigurator& datasrc_configurator) {
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test1.zone.in "
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test1.zone.in "
                         TEST_DATA_BUILDDIR "/test1.zone.copied"));
                         TEST_DATA_BUILDDIR "/test1.zone.copied"));
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test2.zone.in "
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test2.zone.in "
@@ -208,7 +210,7 @@ configureZones(AuthSrv& server) {
         "   \"cache-enable\": true"
         "   \"cache-enable\": true"
         "}]}"));
         "}]}"));
 
 
-    DataSourceConfigurator::testReconfigure(&server, config);
+    datasrc_configurator.reconfigure(config);
 
 
     zoneChecks(server);
     zoneChecks(server);
 }
 }
@@ -234,7 +236,7 @@ newZoneChecks(AuthSrv& server) {
 }
 }
 
 
 TEST_F(AuthCommandTest, loadZone) {
 TEST_F(AuthCommandTest, loadZone) {
-    configureZones(server_);
+    configureZones(server_, datasrc_configurator_);
 
 
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR
                         "/test1-new.zone.in "
                         "/test1-new.zone.in "
@@ -271,7 +273,7 @@ TEST_F(AuthCommandTest,
         "    \"cache-enable\": true,"
         "    \"cache-enable\": true,"
         "    \"cache-zones\": [\"example.org\"]"
         "    \"cache-zones\": [\"example.org\"]"
         "}]}"));
         "}]}"));
-    DataSourceConfigurator::testReconfigure(&server_, config);
+    datasrc_configurator_.reconfigure(config);
 
 
     {
     {
         isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
         isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());
@@ -335,7 +337,7 @@ TEST_F(AuthCommandTest,
         "    \"cache-enable\": true,"
         "    \"cache-enable\": true,"
         "    \"cache-zones\": [\"example.com\"]"
         "    \"cache-zones\": [\"example.com\"]"
         "}]}"));
         "}]}"));
-    EXPECT_THROW(DataSourceConfigurator::testReconfigure(&server_, config2),
+    EXPECT_THROW(datasrc_configurator_.reconfigure(config2),
                  ConfigurableClientList::ConfigurationError);
                  ConfigurableClientList::ConfigurationError);
 
 
     result_ = execAuthServerCommand(server_, "loadzone",
     result_ = execAuthServerCommand(server_, "loadzone",
@@ -350,7 +352,7 @@ TEST_F(AuthCommandTest,
 }
 }
 
 
 TEST_F(AuthCommandTest, loadBrokenZone) {
 TEST_F(AuthCommandTest, loadBrokenZone) {
-    configureZones(server_);
+    configureZones(server_, datasrc_configurator_);
 
 
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR
                         "/test1-broken.zone.in "
                         "/test1-broken.zone.in "
@@ -363,7 +365,7 @@ TEST_F(AuthCommandTest, loadBrokenZone) {
 }
 }
 
 
 TEST_F(AuthCommandTest, loadUnreadableZone) {
 TEST_F(AuthCommandTest, loadUnreadableZone) {
-    configureZones(server_);
+    configureZones(server_, datasrc_configurator_);
 
 
     // install the zone file as unreadable
     // install the zone file as unreadable
     ASSERT_EQ(0, system(INSTALL_PROG " -c -m 000 " TEST_DATA_DIR
     ASSERT_EQ(0, system(INSTALL_PROG " -c -m 000 " TEST_DATA_DIR
@@ -386,7 +388,7 @@ TEST_F(AuthCommandTest, loadZoneWithoutDataSrc) {
 }
 }
 
 
 TEST_F(AuthCommandTest, loadZoneInvalidParams) {
 TEST_F(AuthCommandTest, loadZoneInvalidParams) {
-    configureZones(server_);
+    configureZones(server_, datasrc_configurator_);
 
 
     // null arg
     // null arg
     result_ = execAuthServerCommand(server_, "loadzone", ElementPtr());
     result_ = execAuthServerCommand(server_, "loadzone", ElementPtr());

+ 19 - 17
src/bin/auth/tests/datasrc_configurator_unittest.cc

@@ -19,9 +19,12 @@
 #include <util/threads/lock.h>
 #include <util/threads/lock.h>
 
 
 #include <gtest/gtest.h>
 #include <gtest/gtest.h>
-#include <memory>
+
+#include <boost/bind.hpp>
 #include <boost/shared_ptr.hpp>
 #include <boost/shared_ptr.hpp>
 
 
+#include <memory>
+
 using namespace isc;
 using namespace isc;
 using namespace isc::cc;
 using namespace isc::cc;
 using namespace isc::config;
 using namespace isc::config;
@@ -63,12 +66,12 @@ typedef DataSourceConfiguratorGeneric<DatasrcConfiguratorTest,
         FakeList> Configurator;
         FakeList> Configurator;
 
 
 void
 void
-datasrcConfigHandler(const std::string&,
+datasrcConfigHandler(Configurator* configurator, const std::string&,
                      isc::data::ConstElementPtr config,
                      isc::data::ConstElementPtr config,
                      const isc::config::ConfigData&)
                      const isc::config::ConfigData&)
 {
 {
     if (config->contains("classes")) {
     if (config->contains("classes")) {
-        Configurator::reconfigure(config->get("classes"));
+        configurator->reconfigure(config->get("classes"));
     }
     }
 }
 }
 
 
@@ -99,6 +102,7 @@ protected:
     DatasrcConfiguratorTest() :
     DatasrcConfiguratorTest() :
         session(ElementPtr(new ListElement), ElementPtr(new ListElement),
         session(ElementPtr(new ListElement), ElementPtr(new ListElement),
                 ElementPtr(new ListElement)),
                 ElementPtr(new ListElement)),
+        configurator_(this),
         specfile(string(TEST_OWN_DATA_DIR) + "/spec.spec")
         specfile(string(TEST_OWN_DATA_DIR) + "/spec.spec")
     {
     {
         initSession();
         initSession();
@@ -111,7 +115,6 @@ protected:
     void TearDown() {
     void TearDown() {
         // Make sure no matter what we did, it is cleaned up.
         // Make sure no matter what we did, it is cleaned up.
         mccs->removeRemoteConfig("data_sources");
         mccs->removeRemoteConfig("data_sources");
-        Configurator::cleanup();
     }
     }
     void init(const ElementPtr& config = ElementPtr()) {
     void init(const ElementPtr& config = ElementPtr()) {
         session.getMessages()->
         session.getMessages()->
@@ -125,8 +128,9 @@ protected:
             session.getMessages()->
             session.getMessages()->
                 add(createAnswer(0, ElementPtr(new MapElement)));
                 add(createAnswer(0, ElementPtr(new MapElement)));
         }
         }
-        Configurator::init(this);
-        mccs->addRemoteConfig("data_sources", datasrcConfigHandler, false);
+        mccs->addRemoteConfig("data_sources",
+                              boost::bind(datasrcConfigHandler, &configurator_,
+                                          _1, _2, _3), false);
     }
     }
     void SetUp() {
     void SetUp() {
         init();
         init();
@@ -150,6 +154,7 @@ protected:
     }
     }
     FakeSession session;
     FakeSession session;
     auto_ptr<ModuleCCSession> mccs;
     auto_ptr<ModuleCCSession> mccs;
+    Configurator configurator_;
     const string specfile;
     const string specfile;
     std::map<RRClass, ListPtr> lists_;
     std::map<RRClass, ListPtr> lists_;
     string log_;
     string log_;
@@ -157,20 +162,17 @@ protected:
 };
 };
 
 
 // Check the initialization (and cleanup)
 // Check the initialization (and cleanup)
-TEST_F(DatasrcConfiguratorTest, initialization) {
+TEST_F(DatasrcConfiguratorTest, DISABLED_initialization) {
     // It can't be initialized again
     // It can't be initialized again
     EXPECT_THROW(init(), InvalidOperation);
     EXPECT_THROW(init(), InvalidOperation);
     EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
     EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
-    // Deinitialize to make the tests reasonable
-    mccs->removeRemoteConfig("data_sources");
-    Configurator::cleanup();
     EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
     EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
     // We can't reconfigure now (not even manually)
     // We can't reconfigure now (not even manually)
-    EXPECT_THROW(Configurator::reconfigure(ElementPtr(new MapElement())),
+    EXPECT_THROW(configurator_.reconfigure(ElementPtr(new MapElement())),
                  InvalidOperation);
                  InvalidOperation);
     // If the server param is NULL, it does not work
     // If the server param is NULL, it does not work
-    EXPECT_THROW(Configurator::init(NULL), InvalidParameter);
-    EXPECT_FALSE(session.haveSubscription("data_sources", "*"));
+    EXPECT_THROW(Configurator(NULL), InvalidParameter);
+    EXPECT_FALSE(session.haveSubscription("data_sources", "*")); // TBD
     // But we can initialize it again now
     // But we can initialize it again now
     EXPECT_NO_THROW(init());
     EXPECT_NO_THROW(init());
     EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
     EXPECT_TRUE(session.haveSubscription("data_sources", "*"));
@@ -279,7 +281,7 @@ TEST_F(DatasrcConfiguratorTest, rollbackDeletion) {
     const ElementPtr
     const ElementPtr
         config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
         config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
                                   "\"CH\": [{\"type\": \"xxx\"}]}"));
                                   "\"CH\": [{\"type\": \"xxx\"}]}"));
-    Configurator::reconfigure(config1);
+    configurator_.reconfigure(config1);
     const ElementPtr
     const ElementPtr
         config2(Element::fromJSON("{\"IN\": [{\"type\": 13}]}"));
         config2(Element::fromJSON("{\"IN\": [{\"type\": 13}]}"));
     // This would delete CH. However, the IN one fails.
     // This would delete CH. However, the IN one fails.
@@ -287,7 +289,7 @@ TEST_F(DatasrcConfiguratorTest, rollbackDeletion) {
     // and there's no known way to cause an exception during the
     // and there's no known way to cause an exception during the
     // deletions, it is not a true rollback, but the result should
     // deletions, it is not a true rollback, but the result should
     // be the same.
     // be the same.
-    EXPECT_THROW(Configurator::reconfigure(config2), TypeError);
+    EXPECT_THROW(configurator_.reconfigure(config2), TypeError);
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
 }
 }
@@ -300,13 +302,13 @@ TEST_F(DatasrcConfiguratorTest, rollbackConfiguration) {
     const ElementPtr
     const ElementPtr
         config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
         config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], "
                                   "\"CH\": [{\"type\": \"xxx\"}]}"));
                                   "\"CH\": [{\"type\": \"xxx\"}]}"));
-    Configurator::reconfigure(config1);
+    configurator_.reconfigure(config1);
     // Now, the CH happens first. But nevertheless, it should be
     // Now, the CH happens first. But nevertheless, it should be
     // restored to the previoeus version.
     // restored to the previoeus version.
     const ElementPtr
     const ElementPtr
         config2(Element::fromJSON("{\"IN\": [{\"type\": 13}], "
         config2(Element::fromJSON("{\"IN\": [{\"type\": 13}], "
                                   "\"CH\": [{\"type\": \"yyy\"}]}"));
                                   "\"CH\": [{\"type\": \"yyy\"}]}"));
-    EXPECT_THROW(Configurator::reconfigure(config2), TypeError);
+    EXPECT_THROW(configurator_.reconfigure(config2), TypeError);
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
     EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf());
 }
 }