Commit d7e42bb0 authored by JINMEI Tatuya's avatar JINMEI Tatuya
Browse files

[2204] completely replaced setClientList with swapDataSrcClientLists.

the test cases using setClientList were updated so they use
swapDataSrcClientLists (some of them work as a test for the "swap" itself).
now we don't need setClientList, so it was removed.
parent 919fe74c
......@@ -931,29 +931,13 @@ AuthSrv::destroyDDNSForwarder() {
}
}
void
AuthSrv::setClientList(const RRClass& rrclass,
const shared_ptr<ConfigurableClientList>& list)
{
// TODO: Debug-build only check
if (!impl_->mutex_.locked()) {
isc_throw(isc::Unexpected, "Not locked");
}
if (list) {
(*impl_->datasrc_client_lists_)[rrclass] = list;
} else {
impl_->datasrc_client_lists_->erase(rrclass);
}
}
AuthSrv::DataSrcClientListsPtr
AuthSrv::swapDataSrcClientLists(DataSrcClientListsPtr new_lists) {
{
thread::Mutex::Locker locker(impl_->mutex_);
std::swap(new_lists, impl_->datasrc_client_lists_);
// TODO: Debug-build only check
if (!impl_->mutex_.locked()) {
isc_throw(isc::Unexpected, "Not locked!");
}
std::swap(new_lists, impl_->datasrc_client_lists_);
return (new_lists);
}
......
......@@ -302,24 +302,36 @@ public:
/// If there was no forwarder yet, this method does nothing.
void destroyDDNSForwarder();
/// \brief Sets the currently used list for data sources of given
/// class.
///
/// Replaces the internally used client list with a new one. Other
/// classes are not changed.
///
/// \param rrclass The class to modify.
/// \param list Shared pointer to the client list. If it is NULL,
/// the list is removed instead.
void setClientList(const isc::dns::RRClass& rrclass, const
boost::shared_ptr<isc::datasrc::ConfigurableClientList>&
list);
typedef boost::shared_ptr<std::map<
isc::dns::RRClass, boost::shared_ptr<
isc::datasrc::ConfigurableClientList> > >
DataSrcClientListsPtr;
/// \brief Swap the currently used set of data source client lists with
/// given one.
///
/// The "set" of lists is actually given in the form of map from
/// RRClasses to shared pointers to isc::datasrc::ConfigurableClientList.
///
/// This method returns the swapped set of lists, which was previously
/// used by the server.
///
/// This method is intended to be used by a separate method to update
/// the data source configuration "at once". The caller must hold
/// a lock for the mutex object returned by \c getClientListMutex()
/// before calling this method.
///
/// The ownership of the returned pointer is transferred to the caller.
/// The caller is generally expected to release the resources used in
/// the old lists. Note that it could take longer time if some of the
/// data source clients contain a large size of in-memory data.
///
/// The caller can pass a NULL pointer. This effectively disables
/// any data source for the server.
///
/// \param new_lists Shared pointer to a new set of data source client
/// lists.
/// \return The previous set of lists. It can be NULL.
DataSrcClientListsPtr swapDataSrcClientLists(DataSrcClientListsPtr
new_lists);
......
......@@ -17,6 +17,8 @@
#include "auth_srv.h"
#include <util/threads/lock.h>
#include <cc/data.h>
#include <datasrc/client_list.h>
......@@ -62,8 +64,12 @@ configureDataSourceGeneric(Server& server,
}
// Replace the server's lists. By ignoring the return value we let the
// old lists be destroyed.
server.swapDataSrcClientLists(new_lists);
// old lists be destroyed. Lock will be released immediately after the
// swap.
{
isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
server.swapDataSrcClientLists(new_lists);
}
}
/// \brief Concrete version of configureDataSource() for the
......
......@@ -63,6 +63,7 @@
using namespace std;
using namespace isc::cc;
using namespace isc::dns;
using namespace isc::datasrc;
using namespace isc::util;
using namespace isc::util::io::internal;
using namespace isc::util::unittests;
......@@ -90,6 +91,9 @@ const char* const STATIC_DSRC_FILE = DSRC_DIR "/static.zone";
// a signed example zone.
const char* const CONFIG_INMEMORY_EXAMPLE = TEST_DATA_DIR "/rfc5155-example.zone.signed";
// shortcut commonly used in tests
typedef boost::shared_ptr<ConfigurableClientList> ListPtr;
class AuthSrvTest : public SrvTestBase {
protected:
AuthSrvTest() :
......@@ -1431,7 +1435,9 @@ TEST_F(AuthSrvTest,
boost::shared_ptr<isc::datasrc::ConfigurableClientList>
list(new FakeList(server.getClientList(RRClass::IN()), THROW_NEVER,
false));
server.setClientList(RRClass::IN(), list);
AuthSrv::DataSrcClientListsPtr lists(new std::map<RRClass, ListPtr>);
lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
server.swapDataSrcClientLists(lists);
}
createDataFromFile("nsec3query_nodnssec_fromWire.wire");
......@@ -1459,7 +1465,9 @@ setupThrow(AuthSrv& server, ThrowWhen throw_when, bool isc_exception,
boost::shared_ptr<isc::datasrc::ConfigurableClientList>
list(new FakeList(server.getClientList(RRClass::IN()), throw_when,
isc_exception, rrset));
server.setClientList(RRClass::IN(), list);
AuthSrv::DataSrcClientListsPtr lists(new std::map<RRClass, ListPtr>);
lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
server.swapDataSrcClientLists(lists);
}
TEST_F(AuthSrvTest,
......@@ -1772,34 +1780,37 @@ TEST_F(AuthSrvTest, clientList) {
// There's a debug-build only check in them to make sure everything
// locks them and we call them directly here.
isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
AuthSrv::DataSrcClientListsPtr lists; // initially empty
// The lists don't exist. Therefore, the list of RRClasses is empty.
// We also have no IN list.
EXPECT_TRUE(server.getClientListClasses().empty());
EXPECT_EQ(boost::shared_ptr<const isc::datasrc::ClientList>(),
server.getClientList(RRClass::IN()));
EXPECT_TRUE(server.swapDataSrcClientLists(lists)->empty());
// Put something in.
const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
list(new isc::datasrc::ConfigurableClientList(RRClass::IN()));
const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
list2(new isc::datasrc::ConfigurableClientList(RRClass::CH()));
server.setClientList(RRClass::IN(), list);
server.setClientList(RRClass::CH(), list2);
// There are two things in the list and they are IN and CH
vector<RRClass> classes(server.getClientListClasses());
ASSERT_EQ(2, classes.size());
EXPECT_EQ(RRClass::IN(), classes[0]);
EXPECT_EQ(RRClass::CH(), classes[1]);
const ListPtr list(new ConfigurableClientList(RRClass::IN()));
const ListPtr list2(new ConfigurableClientList(RRClass::CH()));
lists.reset(new std::map<RRClass, ListPtr>);
lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
lists->insert(pair<RRClass, ListPtr>(RRClass::CH(), list2));
server.swapDataSrcClientLists(lists);
// And the lists can be retrieved.
EXPECT_EQ(list, server.getClientList(RRClass::IN()));
EXPECT_EQ(list2, server.getClientList(RRClass::CH()));
// Remove one of them
server.setClientList(RRClass::CH(),
boost::shared_ptr<isc::datasrc::ConfigurableClientList>());
// This really got deleted, including the class.
classes = server.getClientListClasses();
ASSERT_EQ(1, classes.size());
EXPECT_EQ(RRClass::IN(), classes[0]);
// Replace the lists with new lists containing only one list.
lists.reset(new std::map<RRClass, ListPtr>);
lists->insert(pair<RRClass, ListPtr>(RRClass::IN(), list));
lists = server.swapDataSrcClientLists(lists);
// Old one had two lists. That confirms our swap for IN and CH classes
// (i.e., no other entries were there).
EXPECT_EQ(2, lists->size());
// The CH list really got deleted.
EXPECT_EQ(list, server.getClientList(RRClass::IN()));
EXPECT_FALSE(server.getClientList(RRClass::CH()));
}
// We just test the mutex can be locked (exactly once).
......
......@@ -81,7 +81,10 @@ datasrcConfigHandler(DatasrcConfigTest* fake_server, const std::string&,
class DatasrcConfigTest : public ::testing::Test {
public:
// To pretend to be the server:
// These pretend to be the server
isc::util::thread::Mutex& getClientListMutex() const {
return (mutex_);
}
void swapDataSrcClientLists(shared_ptr<std::map<dns::RRClass, ListPtr> >
new_lists)
{
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment