BIND 10 trac2207, updated. c0e090f453d3a1627ccd0591e0ec0abc3e80a01b [2207] Deprecate InstallAction
BIND 10 source code commits
bind10-changes at lists.isc.org
Sat Oct 13 11:07:14 UTC 2012
The branch, trac2207 has been updated
via c0e090f453d3a1627ccd0591e0ec0abc3e80a01b (commit)
from 70aa7bfa24600d5492056c860838d94c9243c6aa (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit c0e090f453d3a1627ccd0591e0ec0abc3e80a01b
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Sat Oct 13 13:03:26 2012 +0200
[2207] Deprecate InstallAction
As we want to directly modify the ZoneTable in ZoneReloaderLocal
(because it is memory-type dependant), we don't need the InstallAction
for anything. If we don't need it, we don't pass it as constructor
parameter -- it'll be called from a factory anyway and the factory may
know we don't need it.
Also adding a setTable() method to ZoneTableHeader. The header had no
way to specify the table currently and we need it for the tests. The
method is currently tentative, we'll probably replace it by something.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/memory/zone_reloader.cc | 15 ++--
src/lib/datasrc/memory/zone_reloader.h | 40 +---------
src/lib/datasrc/memory/zone_table_segment.h | 20 +++++
.../datasrc/tests/memory/zone_reloader_unittest.cc | 77 +++-----------------
4 files changed, 43 insertions(+), 109 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/zone_reloader.cc b/src/lib/datasrc/memory/zone_reloader.cc
index 3027a70..7a3bd54 100644
--- a/src/lib/datasrc/memory/zone_reloader.cc
+++ b/src/lib/datasrc/memory/zone_reloader.cc
@@ -26,11 +26,11 @@ namespace memory {
ZoneReloaderLocal::ZoneReloaderLocal(ZoneTableSegment* segment,
const LoadAction& load_action,
- const InstallAction& install_action,
+ const dns::Name& origin,
const dns::RRClass& rrclass) :
segment_(segment),
load_action_(load_action),
- install_action_(install_action),
+ origin_(origin),
rrclass_(rrclass),
zone_data_(NULL),
loaded_(false),
@@ -67,12 +67,15 @@ ZoneReloaderLocal::install() {
}
data_ready_ = false;
- auto_ptr<ZoneSegment> zone_segment(new ZoneSegment(zone_data_));
- zone_data_ = install_action_(ZoneSegmentID(), zone_segment.get());
+ ZoneTable* table(segment_->getHeader().getTable());
+ if (table == NULL) {
+ isc_throw(isc::Unexpected, "No zone table present");
+ }
+ ZoneTable::AddResult result(table->addZone(segment_->getMemorySegment(),
+ rrclass_, origin_, zone_data_));
- // The ownership was passed to the callback, no need to clear it now.
- zone_segment.release();
+ zone_data_ = result.zone_data;
}
void
diff --git a/src/lib/datasrc/memory/zone_reloader.h b/src/lib/datasrc/memory/zone_reloader.h
index 904b603..70572f7 100644
--- a/src/lib/datasrc/memory/zone_reloader.h
+++ b/src/lib/datasrc/memory/zone_reloader.h
@@ -13,6 +13,7 @@
// PERFORMANCE OF THIS SOFTWARE.
#include <dns/rrclass.h>
+#include <dns/name.h>
#include <boost/function.hpp>
@@ -80,29 +81,6 @@ public:
virtual void cleanup() = 0;
};
-// TODO: Fully define this. It is supposed to be passed to the install_action
-// callback, but what does it actually represent? Is it the actuall zone data
-// there?
-//
-// The current interface is temporary, so the tests work. It will probably
-// change (and we may even fold this class to some other, because there
-// seem to be too many classes around holding zone already).
-//
-// FIXME: Who is responsible for releasing of the segment itself?
-class ZoneSegment {
-public:
- explicit ZoneSegment(ZoneData* data) :
- data_(data)
- {}
- ZoneData* getZoneData() {
- return (data_);
- }
-private:
- ZoneData* data_;
-};
-// TODO: Somehow specify what the ID is
-class ZoneSegmentID {};
-
/// \brief Callback to load data into the memory
///
/// This callback should create new ZoneData (allocated from the passed
@@ -111,17 +89,6 @@ class ZoneSegmentID {};
///
/// It must not return NULL.
typedef boost::function<ZoneData*(util::MemorySegment&)> LoadAction;
-/// \brief Install the zone somewhere.
-///
-/// The goal of the callback is to take the zone data (contained in the
-/// ZoneSegment and identified by ZoneSegmentID) and put it somewhere
-/// to use. The return value should contain the old copy of the zone, if
-/// there was any (it may be NULL). The updater will then destroy it.
-///
-/// Upon successful completion, the ownership of the new zone is passed
-/// to the callback and the old to the updater.
-typedef boost::function<ZoneData* (const ZoneSegmentID&,
- ZoneSegment*)> InstallAction;
/// \brief Reloader implementation which loads data locally.
///
@@ -138,8 +105,7 @@ public:
/// \param install_action The callback used to install the loaded zone.
/// \param rrclass The class of the zone.
ZoneReloaderLocal(ZoneTableSegment* segment, const LoadAction& load_action,
- const InstallAction& install_action,
- const dns::RRClass& rrclass);
+ const dns::Name& name, const dns::RRClass& rrclass);
/// \brief Destructor
~ZoneReloaderLocal();
/// \brief Loads the data.
@@ -169,7 +135,7 @@ public:
private:
ZoneTableSegment* segment_;
LoadAction load_action_;
- InstallAction install_action_;
+ dns::Name origin_;
dns::RRClass rrclass_;
ZoneData* zone_data_;
// The load was performed
diff --git a/src/lib/datasrc/memory/zone_table_segment.h b/src/lib/datasrc/memory/zone_table_segment.h
index 7fd1310..3c927e7 100644
--- a/src/lib/datasrc/memory/zone_table_segment.h
+++ b/src/lib/datasrc/memory/zone_table_segment.h
@@ -45,6 +45,26 @@ public:
return (table.get());
}
+ /// \brief Method to set the internal table
+ ///
+ /// The interface is tentative, we don't know if this is the correct place
+ /// and way to set the data. But for now, we need something to be there
+ /// at least for the tests. So we have this. For this reason, there are
+ /// no tests for this method directly. Do not use in actuall
+ /// implementation.
+ ///
+ /// It can be used only once, to initially set it. It can't replace the
+ /// one already there.
+ ///
+ /// \param table Pointer to the table to use.
+ /// \throw isc::Unexpected if called the second time.
+ void setTable(ZoneTable* table) {
+ if (this->table.get() != NULL) {
+ isc_throw(isc::Unexpected, "Replacing table");
+ }
+ this->table = table;
+ }
+
private:
boost::interprocess::offset_ptr<ZoneTable> table;
};
diff --git a/src/lib/datasrc/tests/memory/zone_reloader_unittest.cc b/src/lib/datasrc/tests/memory/zone_reloader_unittest.cc
index ba9d5c2..a841f29 100644
--- a/src/lib/datasrc/tests/memory/zone_reloader_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_reloader_unittest.cc
@@ -46,18 +46,22 @@ public:
ZoneReloaderLocal(segment_.get(),
bind(&ZoneReloaderLocalTest::loadAction, this,
_1),
- bind(&ZoneReloaderLocalTest::installAction, this,
- _1, _2),
- RRClass::IN())),
+ Name("example.org"), RRClass::IN())),
load_called_(false),
- install_called_(false),
load_throw_(false),
- install_throw_(false),
load_null_(false)
- {}
+ {
+ // TODO: The setTable is only a temporary interface
+ segment_->getHeader().
+ setTable(ZoneTable::create(segment_->getMemorySegment(),
+ RRClass::IN()));
+ }
void TearDown() {
// Release the reloader
reloader_.reset();
+ // Release the table we used
+ ZoneTable::destroy(segment_->getMemorySegment(),
+ segment_->getHeader().getTable(), RRClass::IN());
// And check we freed all memory
EXPECT_TRUE(segment_->getMemorySegment().allMemoryDeallocated());
}
@@ -65,9 +69,7 @@ protected:
scoped_ptr<ZoneTableSegment> segment_;
scoped_ptr<ZoneReloaderLocal> reloader_;
bool load_called_;
- bool install_called_;
bool load_throw_;
- bool install_throw_;
bool load_null_;
private:
ZoneData* loadAction(isc::util::MemorySegment& segment) {
@@ -88,32 +90,6 @@ private:
// goes inside.
return (ZoneData::create(segment, Name("example.org")));
}
- ZoneData* installAction(const ZoneSegmentID&, ZoneSegment* segment) {
- install_called_ = true;
-
- // Check we got something
- if (segment == NULL) {
- ADD_FAILURE() << "Zone segment is NULL in install action";
- return (NULL);
- }
- EXPECT_NE(static_cast<const ZoneData*>(NULL), segment->getZoneData());
-
- if (install_throw_) {
- // In case we throw, we do so before releasing the memory there,
- // as in that case we don't claim the ownership of the data.
- throw TestException();
- }
-
- // We received ownership of the parameters here. And we don't really need
- // them in the tests, so we just release them.
- ZoneData::destroy(segment_->getMemorySegment(), segment->getZoneData(),
- RRClass::IN());
- delete segment;
-
- // And we are supposed to pass the old version back. So we create
- // new zone data and pass it.
- return (ZoneData::create(segment_->getMemorySegment(), Name("exmaple.org")));
- }
};
// We call it the way we are supposed to, check every callback is called in the
@@ -121,17 +97,14 @@ private:
TEST_F(ZoneReloaderLocalTest, correctCall) {
// Nothing called before we call it
EXPECT_FALSE(load_called_);
- EXPECT_FALSE(install_called_);
// Just the load gets called now
EXPECT_NO_THROW(reloader_->load());
EXPECT_TRUE(load_called_);
- EXPECT_FALSE(install_called_);
load_called_ = false;
EXPECT_NO_THROW(reloader_->install());
EXPECT_FALSE(load_called_);
- EXPECT_TRUE(install_called_);
// We don't check explicitly how this works, but call it to free memory. If
// everything is freed should be checked inside the TearDown.
@@ -142,18 +115,15 @@ TEST_F(ZoneReloaderLocalTest, loadTwice) {
// Load it the first time
EXPECT_NO_THROW(reloader_->load());
EXPECT_TRUE(load_called_);
- EXPECT_FALSE(install_called_);
load_called_ = false;
// The second time, it should not be possible
EXPECT_THROW(reloader_->load(), isc::Unexpected);
EXPECT_FALSE(load_called_);
- EXPECT_FALSE(install_called_);
// The object should not be damaged, try installing and clearing now
EXPECT_NO_THROW(reloader_->install());
EXPECT_FALSE(load_called_);
- EXPECT_TRUE(install_called_);
// We don't check explicitly how this works, but call it to free memory. If
// everything is freed should be checked inside the TearDown.
@@ -167,18 +137,16 @@ TEST_F(ZoneReloaderLocalTest, loadLater) {
EXPECT_NO_THROW(reloader_->load());
EXPECT_NO_THROW(reloader_->install());
// Reset so we see nothing is called now
- install_called_ = load_called_ = false;
+ load_called_ = false;
EXPECT_THROW(reloader_->load(), isc::Unexpected);
EXPECT_FALSE(load_called_);
- EXPECT_FALSE(install_called_);
// Cleanup and try loading again. Still shouldn't work.
EXPECT_NO_THROW(reloader_->cleanup());
EXPECT_THROW(reloader_->load(), isc::Unexpected);
EXPECT_FALSE(load_called_);
- EXPECT_FALSE(install_called_);
}
// Try calling install at various bad times
@@ -186,17 +154,14 @@ TEST_F(ZoneReloaderLocalTest, invalidInstall) {
// Nothing loaded yet
EXPECT_THROW(reloader_->install(), isc::Unexpected);
EXPECT_FALSE(load_called_);
- EXPECT_FALSE(install_called_);
EXPECT_NO_THROW(reloader_->load());
load_called_ = false;
// This install is OK
EXPECT_NO_THROW(reloader_->install());
- install_called_ = false;
// But we can't call it second time now
EXPECT_THROW(reloader_->install(), isc::Unexpected);
EXPECT_FALSE(load_called_);
- EXPECT_FALSE(install_called_);
}
// We check we can clean without installing first and nothing bad
@@ -207,11 +172,9 @@ TEST_F(ZoneReloaderLocalTest, cleanWithoutInstall) {
EXPECT_NO_THROW(reloader_->cleanup());
EXPECT_TRUE(load_called_);
- EXPECT_FALSE(install_called_);
// We cleaned up, no way to install now
EXPECT_THROW(reloader_->install(), isc::Unexpected);
- EXPECT_FALSE(install_called_);
}
// Test the case when load callback throws
@@ -222,29 +185,11 @@ TEST_F(ZoneReloaderLocalTest, loadThrows) {
// We can't install now
EXPECT_THROW(reloader_->install(), isc::Unexpected);
EXPECT_TRUE(load_called_);
- EXPECT_FALSE(install_called_);
// But we can cleanup
EXPECT_NO_THROW(reloader_->cleanup());
}
-// Test we free all our memory even when we throw from install
-TEST_F(ZoneReloaderLocalTest, installThrows) {
- install_throw_ = true;
- EXPECT_NO_THROW(reloader_->load());
-
- EXPECT_THROW(reloader_->install(), TestException);
- EXPECT_TRUE(load_called_);
- EXPECT_TRUE(install_called_);
-
- // We can't try again
- install_throw_ = false;
- EXPECT_THROW(reloader_->install(), isc::Unexpected);
-
- // But it is not completely broken
- EXPECT_NO_THROW(reloader_->cleanup());
-}
-
// Check the reloader defends itsefl when load action returns NULL
TEST_F(ZoneReloaderLocalTest, loadNull) {
load_null_ = true;
More information about the bind10-changes
mailing list