Browse Source

[1207] remove old-style inmem-ds getter and setter

Jelte Jansen 13 years ago
parent
commit
0b5f0023e7

+ 3 - 2
src/bin/auth/auth_config.cc

@@ -108,7 +108,8 @@ DatasourcesConfig::commit() {
     // server implementation details, and isn't scalable wrt the number of
     // data source types, and should eventually be improved.
     // Currently memory data source for class IN is the only possibility.
-    server_.setInMemoryClient(RRClass::IN(), AuthSrv::InMemoryClientPtr());
+    server_.setInMemoryClient(RRClass::IN(),
+        isc::datasrc::DataSourceClientContainerPtr());
 
     BOOST_FOREACH(boost::shared_ptr<AuthConfigParser> datasrc_config,
                   datasources_) {
@@ -145,7 +146,7 @@ MemoryDatasourceConfig::build(ConstElementPtr config_value) {
     // We'd eventually optimize building zones (in case of reloading) by
     // selectively loading fresh zones.  Right now we simply check the
     // RR class is supported by the server implementation.
-    server_.getInMemoryClient(rrclass_);
+    (void)server_.getInMemoryClientContainer(rrclass_);
     memory_client_ = isc::datasrc::DataSourceClientContainerPtr(
         new isc::datasrc::DataSourceClientContainer("memory", config_value));
 }

+ 16 - 40
src/bin/auth/auth_srv.cc

@@ -141,7 +141,7 @@ public:
 
     /// In-memory data source.  Currently class IN only for simplicity.
     const RRClass memory_client_class_;
-    AuthSrv::InMemoryClientPtr memory_client_;
+    //AuthSrv::InMemoryClientPtr memory_client_;
     isc::datasrc::DataSourceClientContainerPtr memory_client_container_;
     isc::datasrc::InMemoryClient* memory_client_p_;
 
@@ -392,20 +392,8 @@ AuthSrv::getConfigSession() const {
     return (impl_->config_session_);
 }
 
-AuthSrv::InMemoryClientPtr
-AuthSrv::getInMemoryClient(const RRClass& rrclass) {
-    // XXX: for simplicity, we only support the IN class right now.
-    if (rrclass != impl_->memory_client_class_) {
-        isc_throw(InvalidParameter,
-                  "Memory data source is not supported for RR class "
-                  << rrclass);
-    }
-    return (impl_->memory_client_);
-}
-
 isc::datasrc::DataSourceClientContainerPtr
 AuthSrv::getInMemoryClientContainer(const RRClass& rrclass) {
-    // XXX: for simplicity, we only support the IN class right now.
     if (rrclass != impl_->memory_client_class_) {
         isc_throw(InvalidParameter,
                   "Memory data source is not supported for RR class "
@@ -416,59 +404,47 @@ AuthSrv::getInMemoryClientContainer(const RRClass& rrclass) {
 
 isc::datasrc::InMemoryClient*
 AuthSrv::getInMemoryClientP(const RRClass& rrclass) {
-    // XXX: for simplicity, we only support the IN class right now.
     if (rrclass != impl_->memory_client_class_) {
         isc_throw(InvalidParameter,
                   "Memory data source is not supported for RR class "
                   << rrclass);
     }
     if (!impl_->memory_client_p_) {
-        isc_throw(Exception, "no memory client set");
+        isc_throw(InvalidOperation, "no memory client set");
     }
-    return (impl_->memory_client_p_);
+    return static_cast<isc::datasrc::InMemoryClient*>(
+        &getInMemoryClientContainer(rrclass)->getInstance());
 }
 
-void
-AuthSrv::setInMemoryClient(const isc::dns::RRClass& rrclass,
-                           InMemoryClientPtr memory_client)
-{
-    // XXX: see above
-    if (rrclass != impl_->memory_client_class_) {
-        isc_throw(InvalidParameter,
-                  "Memory data source is not supported for RR class "
-                  << rrclass);
-    } else if (!impl_->memory_client_ && memory_client) {
-        LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_MEM_DATASRC_ENABLED)
-                  .arg(rrclass);
-    } else if (impl_->memory_client_ && !memory_client) {
-        LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_MEM_DATASRC_DISABLED)
-                  .arg(rrclass);
-    }
-    impl_->memory_client_ = memory_client;
-    impl_->memory_client_p_ = memory_client.get();
+bool
+AuthSrv::hasInMemoryClient() {
+    return (impl_->memory_client_p_ != NULL);
 }
 
 void
 AuthSrv::setInMemoryClient(const isc::dns::RRClass& rrclass,
     isc::datasrc::DataSourceClientContainerPtr memory_client)
 {
-    // XXX: see above
     if (rrclass != impl_->memory_client_class_) {
         isc_throw(InvalidParameter,
                   "Memory data source is not supported for RR class "
                   << rrclass);
-    } else if (!impl_->memory_client_ && memory_client) {
+    } else if (!impl_->memory_client_container_ && memory_client) {
         LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_MEM_DATASRC_ENABLED)
                   .arg(rrclass);
-    } else if (impl_->memory_client_ && !memory_client) {
+    } else if (impl_->memory_client_container_ && !memory_client) {
         LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_MEM_DATASRC_DISABLED)
                   .arg(rrclass);
     }
     impl_->memory_client_container_ = memory_client;
-    impl_->memory_client_p_ =
-        static_cast<isc::datasrc::InMemoryClient*>(&memory_client->getInstance());
     // temp fix for tests; fool tests with a fake inmemoryclientptr
-    impl_->memory_client_ = AuthSrv::InMemoryClientPtr(new InMemoryClient());
+    if (memory_client) {
+        impl_->memory_client_p_ =
+            static_cast<isc::datasrc::InMemoryClient*>(
+                &memory_client->getInstance());
+    } else {
+        impl_->memory_client_p_ = NULL;
+    }
 }
 
 

+ 28 - 7
src/bin/auth/auth_srv.h

@@ -263,9 +263,32 @@ public:
     /// \param rrclass The RR class of the requested in-memory data source.
     /// \return A pointer to the in-memory data source, if configured;
     /// otherwise NULL.
-    InMemoryClientPtr getInMemoryClient(const isc::dns::RRClass& rrclass);
-    isc::datasrc::InMemoryClient* getInMemoryClientP(const isc::dns::RRClass& rrclass);
-    isc::datasrc::DataSourceClientContainerPtr getInMemoryClientContainer(const isc::dns::RRClass& rrclass);
+    isc::datasrc::InMemoryClient* getInMemoryClientP(
+        const isc::dns::RRClass& rrclass);
+
+    /// Returns the DataSourceClientContainer of the in-memory datasource
+    ///
+    /// \exception InvalidParameter if the given class does not match
+    ///            the one in the memory data source
+    /// \exception InvalidOperation if the memory datasource has not been set
+    ///            (callers can check with \c hasMemoryDataSource())
+    ///
+    /// \param rrclass The RR class of the requested in-memory data source.
+    /// \return A shared pointer to the in-memory data source, if configured;
+    /// otherwise an empty shared pointer.
+    isc::datasrc::DataSourceClientContainerPtr getInMemoryClientContainer(
+        const isc::dns::RRClass& rrclass);
+
+    /// Checks if the in-memory data source has been set.
+    ///
+    /// Right now, only one datasource at a time is effectively supported.
+    /// This is a helper method to check whether it is the in-memory one.
+    /// This is mostly useful for current testing, and is expected to be
+    /// removed (or changed in behaviour) soon, when the general
+    /// multi-data-source framework is completed.
+    ///
+    /// \return True if the in-memory datasource has been set.
+    bool hasInMemoryClient();
 
     /// Sets or replaces the in-memory data source of the specified RR class.
     ///
@@ -276,14 +299,12 @@ public:
     ///
     /// If there is already an in memory data source configured, it will be
     /// replaced with the newly specified one.
-    /// \c memory_datasrc can be NULL, in which case it will (re)disable the
-    /// in-memory data source.
+    /// \c memory_datasrc can be an empty shared pointer, in which case it
+    /// will (re)disable the in-memory data source.
     ///
     /// \param rrclass The RR class of the in-memory data source to be set.
     /// \param memory_datasrc A (shared) pointer to \c InMemoryClient to be set.
     void setInMemoryClient(const isc::dns::RRClass& rrclass,
-                           InMemoryClientPtr memory_client);
-    void setInMemoryClient(const isc::dns::RRClass& rrclass,
         isc::datasrc::DataSourceClientContainerPtr memory_client);
 
     /// \brief Set the communication session with Statistics.

+ 35 - 10
src/bin/auth/tests/auth_srv_unittest.cc

@@ -835,11 +835,11 @@ TEST_F(AuthSrvTest, updateWithInMemoryClient) {
     // in the configuration tests.  We only check the AuthSrv interface here.
 
     // By default memory data source isn't enabled
-    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_FALSE(server.hasInMemoryClient());
     updateConfig(&server,
                  "{\"datasources\": [{\"type\": \"memory\"}]}", true);
     // after successful configuration, we should have one (with empty zoneset).
-    ASSERT_NE(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_TRUE(server.hasInMemoryClient());
     EXPECT_EQ(0, server.getInMemoryClientP(rrclass)->getZoneCount());
 
     // The memory data source is empty, should return REFUSED rcode.
@@ -857,7 +857,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientNoDNSSEC) {
     // answer section.  Detailed examination on the response content
     // for various types of queries are tested in the query tests.
     updateConfig(&server, CONFIG_INMEMORY_EXAMPLE, true);
-    ASSERT_NE(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_TRUE(server.hasInMemoryClient());
     EXPECT_EQ(1, server.getInMemoryClientP(rrclass)->getZoneCount());
 
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
@@ -874,7 +874,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientDNSSEC) {
     // The response should contain RRSIGs, and should have more RRs than
     // the previous case.
     updateConfig(&server, CONFIG_INMEMORY_EXAMPLE, true);
-    ASSERT_NE(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_TRUE(server.hasInMemoryClient());
     EXPECT_EQ(1, server.getInMemoryClientP(rrclass)->getZoneCount());
 
     createDataFromFile("nsec3query_fromWire.wire");
@@ -1247,6 +1247,27 @@ private:
     ConstRRsetPtr fake_rrset_;
 };
 
+class FakeContainer : public isc::datasrc::DataSourceClientContainer {
+public:
+    /// \brief Creates a fake container for the given in-memory client
+    ///
+    /// The initializer creates a fresh instance of a memory datasource,
+    /// which is ignored for the rest (but we do not allow 'null' containers
+    /// atm, and this is only needed in these tests)
+    FakeContainer(AuthSrv::InMemoryClientPtr client) :
+        DataSourceClientContainer("memory",
+                                  Element::fromJSON("{\"type\": \"memory\"}")),
+        client_(client)
+    {}
+
+    isc::datasrc::DataSourceClient& getInstance() {
+        return (*client_);
+    }
+
+private:
+    AuthSrv::InMemoryClientPtr client_;
+};
+
 } // end anonymous namespace for throwing proxy classes
 
 // Test for the tests
@@ -1260,9 +1281,12 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientProxy) {
     AuthSrv::InMemoryClientPtr fake_client(
         new FakeInMemoryClient(server.getInMemoryClientContainer(rrclass),
                                THROW_NEVER, false));
-    ASSERT_NE(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
-    server.setInMemoryClient(rrclass, fake_client);
-
+    EXPECT_TRUE(server.hasInMemoryClient());
+/*
+    isc::datasrc::DataSourceClientContainerPtr fake_client_container(
+        new FakeContainer(fake_client));
+    server.setInMemoryClient(rrclass, fake_client_container);
+*/
     createDataFromFile("nsec3query_nodnssec_fromWire.wire");
     server.processMessage(*io_message, *parse_message, *response_obuffer,
                           &dnsserv);
@@ -1291,10 +1315,11 @@ setupThrow(AuthSrv* server, const char *config, ThrowWhen throw_when,
         new FakeInMemoryClient(
             server->getInMemoryClientContainer(isc::dns::RRClass::IN()),
             throw_when, isc_exception, rrset));
+    isc::datasrc::DataSourceClientContainerPtr fake_client_container(
+        new FakeContainer(fake_client));
 
-    ASSERT_NE(AuthSrv::InMemoryClientPtr(),
-              server->getInMemoryClient(isc::dns::RRClass::IN()));
-    server->setInMemoryClient(isc::dns::RRClass::IN(), fake_client);
+    ASSERT_TRUE(server->hasInMemoryClient());
+    server->setInMemoryClient(isc::dns::RRClass::IN(), fake_client_container);
 }
 
 TEST_F(AuthSrvTest, queryWithThrowingProxyServfails) {

+ 21 - 12
src/bin/auth/tests/command_unittest.cc

@@ -68,7 +68,7 @@ protected:
     }
     void checkAnswer(const int expected_code) {
         parseAnswer(rcode_, result_);
-        EXPECT_EQ(expected_code, rcode_) << result_->str();
+        ASSERT_EQ(expected_code, rcode_) << result_->str();
     }
     MockSession statistics_session_;
     MockXfroutClient xfrout_;
@@ -295,9 +295,15 @@ TEST_F(AuthCommandTest,
 
     // The loadzone command needs the zone to be already loaded, because
     // it is used for reloading only
-    AuthSrv::InMemoryClientPtr dsrc(new InMemoryClient());
-    dsrc->addZone(ZoneFinderPtr(new InMemoryZoneFinder(RRClass::IN(),
-                                                       Name("example.org"))));
+    //AuthSrv::InMemoryClientPtr dsrc(new InMemoryClient());
+    isc::datasrc::DataSourceClientContainerPtr dsrc(
+        new isc::datasrc::DataSourceClientContainer("memory",
+            Element::fromJSON("{\"type\": \"memory\"}")));
+    // The 'public' factory API does not allow for direct internal calls such
+    // as addZone, so purely for this test we do a quick cast
+    static_cast<InMemoryClient*>(&dsrc->getInstance())->addZone(
+        ZoneFinderPtr(new InMemoryZoneFinder(RRClass::IN(),
+                                             Name("example.org"))));
     server_.setInMemoryClient(RRClass::IN(), dsrc);
 
     // Now send the command to reload it
@@ -307,20 +313,23 @@ TEST_F(AuthCommandTest,
 
     // Get the zone and look if there are data in it (the original one was
     // empty)
-    ASSERT_TRUE(server_.getInMemoryClient(RRClass::IN()));
-    EXPECT_EQ(ZoneFinder::SUCCESS, server_.getInMemoryClient(RRClass::IN())->
+    ASSERT_TRUE(server_.getInMemoryClientContainer(RRClass::IN()));
+    EXPECT_EQ(ZoneFinder::SUCCESS, server_.getInMemoryClientP(RRClass::IN())->
               findZone(Name("example.org")).zone_finder->
               find(Name("example.org"), RRType::SOA())->code);
 
     // Some error cases. First, the zone has no configuration.
-    dsrc->addZone(ZoneFinderPtr(new InMemoryZoneFinder(RRClass::IN(),
-                                                       Name("example.com"))));
+    // The 'public' factory API does not allow for direct internal calls such
+    // as addZone, so purely for this test we do a quick cast
+    static_cast<InMemoryClient*>(&dsrc->getInstance())->addZone(
+        ZoneFinderPtr(new InMemoryZoneFinder(RRClass::IN(),
+                                             Name("example.com"))));
     result_ = execAuthServerCommand(server_, "loadzone",
         Element::fromJSON("{\"origin\": \"example.com\"}"));
     checkAnswer(1);
 
     // The previous zone is not hurt in any way
-    EXPECT_EQ(ZoneFinder::SUCCESS, server_.getInMemoryClient(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS, server_.getInMemoryClientP(RRClass::IN())->
               findZone(Name("example.org")).zone_finder->
               find(Name("example.org"), RRType::SOA())->code);
 
@@ -330,7 +339,7 @@ TEST_F(AuthCommandTest,
     checkAnswer(1);
 
     // The previous zone is not hurt in any way
-    EXPECT_EQ(ZoneFinder::SUCCESS, server_.getInMemoryClient(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS, server_.getInMemoryClientP(RRClass::IN())->
               findZone(Name("example.org")).zone_finder->
               find(Name("example.org"), RRType::SOA())->code);
     // Configure an unreadable zone. Should fail, but leave the original zone
@@ -353,7 +362,7 @@ TEST_F(AuthCommandTest,
         Element::fromJSON("{\"origin\": \"example.com\"}"));
     checkAnswer(1);
     // The previous zone is not hurt in any way
-    EXPECT_EQ(ZoneFinder::SUCCESS, server_.getInMemoryClient(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS, server_.getInMemoryClientP(RRClass::IN())->
               findZone(Name("example.org")).zone_finder->
               find(Name("example.org"), RRType::SOA())->code);
 
@@ -368,7 +377,7 @@ TEST_F(AuthCommandTest,
     module_session.setLocalConfig(broken);
     checkAnswer(1);
     // The previous zone is not hurt in any way
-    EXPECT_EQ(ZoneFinder::SUCCESS, server_.getInMemoryClient(RRClass::IN())->
+    EXPECT_EQ(ZoneFinder::SUCCESS, server_.getInMemoryClientContainer(RRClass::IN())->getInstance().
               findZone(Name("example.org")).zone_finder->
               find(Name("example.org"), RRType::SOA())->code);
 }

+ 7 - 7
src/bin/auth/tests/config_unittest.cc

@@ -71,11 +71,11 @@ private:
 
 TEST_F(AuthConfigTest, datasourceConfig) {
     // By default, we don't have any in-memory data source.
-    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_FALSE(server.hasInMemoryClient());
     configureAuthServer(server, Element::fromJSON(
                             "{\"datasources\": [{\"type\": \"memory\"}]}"));
     // after successful configuration, we should have one (with empty zoneset).
-    ASSERT_NE(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_TRUE(server.hasInMemoryClient());
     EXPECT_EQ(0, server.getInMemoryClientP(rrclass)->getZoneCount());
 }
 
@@ -96,7 +96,7 @@ TEST_F(AuthConfigTest, versionConfig) {
 }
 
 TEST_F(AuthConfigTest, exceptionGuarantee) {
-    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_FALSE(server.hasInMemoryClient());
     // This configuration contains an invalid item, which will trigger
     // an exception.
     EXPECT_THROW(configureAuthServer(
@@ -106,7 +106,7 @@ TEST_F(AuthConfigTest, exceptionGuarantee) {
                          " \"no_such_config_var\": 1}")),
                  AuthConfigError);
     // The server state shouldn't change
-    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_FALSE(server.hasInMemoryClient());
 }
 
 TEST_F(AuthConfigTest, exceptionConversion) {
@@ -176,12 +176,12 @@ protected:
 TEST_F(MemoryDatasrcConfigTest, addZeroDataSrc) {
     parser->build(Element::fromJSON("[]"));
     parser->commit();
-    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_FALSE(server.hasInMemoryClient());
 }
 
 TEST_F(MemoryDatasrcConfigTest, addEmpty) {
     // By default, we don't have any in-memory data source.
-    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_FALSE(server.hasInMemoryClient());
     parser->build(Element::fromJSON("[{\"type\": \"memory\"}]"));
     parser->commit();
     EXPECT_EQ(0, server.getInMemoryClientP(rrclass)->getZoneCount());
@@ -354,7 +354,7 @@ TEST_F(MemoryDatasrcConfigTest, remove) {
     parser = createAuthConfigParser(server, "datasources"); 
     EXPECT_NO_THROW(parser->build(Element::fromJSON("[]")));
     EXPECT_NO_THROW(parser->commit());
-    EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_FALSE(server.hasInMemoryClient());
 }
 
 TEST_F(MemoryDatasrcConfigTest, addDuplicateZones) {

+ 2 - 2
src/lib/datasrc/factory.h

@@ -158,13 +158,13 @@ public:
                               isc::data::ConstElementPtr config);
 
     /// \brief Destructor
-    ~DataSourceClientContainer();
+    virtual ~DataSourceClientContainer();
 
     /// \brief Accessor to the instance
     ///
     /// \return Reference to the DataSourceClient instance contained in this
     ///         container.
-    DataSourceClient& getInstance() { return (*instance_); }
+    virtual DataSourceClient& getInstance() { return (*instance_); }
 
 private:
     DataSourceClient* instance_;