From: Mukund Sivaraman Date: Tue, 30 Apr 2013 05:48:57 +0000 (+0530) Subject: [2850] Make getZoneWriter() non-virtual X-Git-Tag: bind10-1.2.0beta1-release~467^2~89 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=58a2bdc6b084fdb4ac57726b305f49463370cc2c;p=thirdparty%2Fkea.git [2850] Make getZoneWriter() non-virtual This also makes ZoneWriter a concrete class (and removes ZoneWriterLocal). --- diff --git a/src/lib/datasrc/memory/Makefile.am b/src/lib/datasrc/memory/Makefile.am index c0ee688a89..32c2bc45d6 100644 --- a/src/lib/datasrc/memory/Makefile.am +++ b/src/lib/datasrc/memory/Makefile.am @@ -25,8 +25,7 @@ libdatasrc_memory_la_SOURCES += zone_table_segment_local.h zone_table_segment_lo libdatasrc_memory_la_SOURCES += zone_data_updater.h zone_data_updater.cc libdatasrc_memory_la_SOURCES += zone_data_loader.h zone_data_loader.cc libdatasrc_memory_la_SOURCES += memory_client.h memory_client.cc -libdatasrc_memory_la_SOURCES += zone_writer.h -libdatasrc_memory_la_SOURCES += zone_writer_local.h zone_writer_local.cc +libdatasrc_memory_la_SOURCES += zone_writer.h zone_writer.cc libdatasrc_memory_la_SOURCES += load_action.h libdatasrc_memory_la_SOURCES += util_internal.h diff --git a/src/lib/datasrc/memory/zone_table_segment.cc b/src/lib/datasrc/memory/zone_table_segment.cc index 1253102801..f3c962641d 100644 --- a/src/lib/datasrc/memory/zone_table_segment.cc +++ b/src/lib/datasrc/memory/zone_table_segment.cc @@ -14,6 +14,7 @@ #include #include +#include #include @@ -40,6 +41,14 @@ ZoneTableSegment::destroy(ZoneTableSegment *segment) { delete segment; } +ZoneWriter* +ZoneTableSegment::getZoneWriter(const LoadAction& load_action, + const dns::Name& name, + const dns::RRClass& rrclass) +{ + return (new ZoneWriter(this, load_action, name, rrclass)); +} + } // namespace memory } // namespace datasrc } // namespace isc diff --git a/src/lib/datasrc/memory/zone_table_segment.h b/src/lib/datasrc/memory/zone_table_segment.h index 1440a2ef59..26d1ab5b48 100644 --- a/src/lib/datasrc/memory/zone_table_segment.h +++ b/src/lib/datasrc/memory/zone_table_segment.h @@ -128,9 +128,9 @@ public: /// \param segment The segment to destroy. static void destroy(ZoneTableSegment* segment); - /// \brief Create a zone write corresponding to this segment + /// \brief Create a zone writer /// - /// This creates a new write that can be used to update zones + /// This creates a new writer that can be used to update zones /// inside this zone table segment. /// /// \param loadAction Callback to provide the actual data. @@ -139,9 +139,9 @@ public: /// \return New instance of a zone writer. The ownership is passed /// onto the caller and the caller needs to \c delete it when /// it's done with the writer. - virtual ZoneWriter* getZoneWriter(const LoadAction& load_action, - const dns::Name& origin, - const dns::RRClass& rrclass) = 0; + ZoneWriter* getZoneWriter(const LoadAction& load_action, + const dns::Name& origin, + const dns::RRClass& rrclass); }; } // namespace memory diff --git a/src/lib/datasrc/memory/zone_table_segment_local.cc b/src/lib/datasrc/memory/zone_table_segment_local.cc index fdaf678e6f..de69d4a9a6 100644 --- a/src/lib/datasrc/memory/zone_table_segment_local.cc +++ b/src/lib/datasrc/memory/zone_table_segment_local.cc @@ -13,7 +13,6 @@ // PERFORMANCE OF THIS SOFTWARE. #include -#include "zone_writer_local.h" using namespace isc::dns; using namespace isc::util; @@ -56,14 +55,6 @@ ZoneTableSegmentLocal::getMemorySegment() { return (mem_sgmt_); } -ZoneWriter* -ZoneTableSegmentLocal::getZoneWriter(const LoadAction& load_action, - const dns::Name& name, - const dns::RRClass& rrclass) -{ - return (new ZoneWriterLocal(this, load_action, name, rrclass)); -} - } // namespace memory } // namespace datasrc } // namespace isc diff --git a/src/lib/datasrc/memory/zone_table_segment_local.h b/src/lib/datasrc/memory/zone_table_segment_local.h index e08ca3978b..f289493608 100644 --- a/src/lib/datasrc/memory/zone_table_segment_local.h +++ b/src/lib/datasrc/memory/zone_table_segment_local.h @@ -53,10 +53,6 @@ public: /// implementation (a MemorySegmentLocal instance). virtual isc::util::MemorySegment& getMemorySegment(); - /// \brief Concrete implementation of ZoneTableSegment::getZoneWriter - virtual ZoneWriter* getZoneWriter(const LoadAction& load_action, - const dns::Name& origin, - const dns::RRClass& rrclass); private: isc::util::MemorySegmentLocal mem_sgmt_; ZoneTableHeader header_; diff --git a/src/lib/datasrc/memory/zone_writer_local.cc b/src/lib/datasrc/memory/zone_writer.cc similarity index 83% rename from src/lib/datasrc/memory/zone_writer_local.cc rename to src/lib/datasrc/memory/zone_writer.cc index 0cd95876c5..86d09e9cda 100644 --- a/src/lib/datasrc/memory/zone_writer_local.cc +++ b/src/lib/datasrc/memory/zone_writer.cc @@ -12,9 +12,8 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. -#include "zone_writer_local.h" +#include "zone_writer.h" #include "zone_data.h" -#include "zone_table_segment_local.h" #include @@ -24,10 +23,10 @@ namespace isc { namespace datasrc { namespace memory { -ZoneWriterLocal::ZoneWriterLocal(ZoneTableSegmentLocal* segment, - const LoadAction& load_action, - const dns::Name& origin, - const dns::RRClass& rrclass) : +ZoneWriter::ZoneWriter(ZoneTableSegment* segment, + const LoadAction& load_action, + const dns::Name& origin, + const dns::RRClass& rrclass) : segment_(segment), load_action_(load_action), origin_(origin), @@ -36,14 +35,14 @@ ZoneWriterLocal::ZoneWriterLocal(ZoneTableSegmentLocal* segment, state_(ZW_UNUSED) {} -ZoneWriterLocal::~ZoneWriterLocal() { +ZoneWriter::~ZoneWriter() { // Clean up everything there might be left if someone forgot, just // in case. cleanup(); } void -ZoneWriterLocal::load() { +ZoneWriter::load() { if (state_ != ZW_UNUSED) { isc_throw(isc::InvalidOperation, "Trying to load twice"); } @@ -59,7 +58,7 @@ ZoneWriterLocal::load() { } void -ZoneWriterLocal::install() { +ZoneWriter::install() { if (state_ != ZW_LOADED) { isc_throw(isc::InvalidOperation, "No data to install"); } @@ -78,7 +77,7 @@ ZoneWriterLocal::install() { } void -ZoneWriterLocal::cleanup() { +ZoneWriter::cleanup() { // We eat the data (if any) now. if (zone_data_ != NULL) { diff --git a/src/lib/datasrc/memory/zone_writer.h b/src/lib/datasrc/memory/zone_writer.h index 0e8f285fff..cfba6ccfef 100644 --- a/src/lib/datasrc/memory/zone_writer.h +++ b/src/lib/datasrc/memory/zone_writer.h @@ -15,30 +15,43 @@ #ifndef MEM_ZONE_WRITER_H #define MEM_ZONE_WRITER_H +#include "zone_table_segment.h" #include "load_action.h" +#include +#include + namespace isc { namespace datasrc { namespace memory { /// \brief Does an update to a zone. /// -/// This abstract base class represents the work of a reload of a zone. -/// The work is divided into three stages -- load(), install() and cleanup(). -/// They should be called in this order for the effect to take place. +/// This represents the work of a reload of a zone. The work is divided +/// into three stages -- load(), install() and cleanup(). They should +/// be called in this order for the effect to take place. /// /// We divide them so the update of zone data can be done asynchronously, /// in a different thread. The install() operation is the only one that needs /// to be done in a critical section. /// -/// Each derived class implementation must provide the strong exception -/// guarantee for each public method. That is, when any of the methods -/// throws, the entire state should stay the same as before the call -/// (how to achieve that may be implementation dependant). +/// This class provides strong exception guarantee for each public +/// method. That is, when any of the methods throws, the entire state +/// stays the same as before the call. class ZoneWriter { public: - /// \brief Virtual destructor. - virtual ~ZoneWriter() {}; + /// \brief Constructor + /// + /// \param segment The zone table segment to store the zone into. + /// \param load_action The callback used to load data. + /// \param install_action The callback used to install the loaded zone. + /// \param rrclass The class of the zone. + ZoneWriter(ZoneTableSegment* segment, + const LoadAction& load_action, const dns::Name& name, + const dns::RRClass& rrclass); + + /// \brief Destructor. + ~ZoneWriter(); /// \brief Get the zone data into memory. /// @@ -56,7 +69,7 @@ public: /// \note After successful load(), you have to call cleanup() some time /// later. /// \throw isc::InvalidOperation if called second time. - virtual void load() = 0; + void load(); /// \brief Put the changes to effect. /// @@ -68,12 +81,12 @@ public: /// The operation is expected to be fast and is meant to be used inside /// a critical section. /// - /// This may throw in rare cases, depending on the concrete implementation. - /// If it throws, you still need to call cleanup(). + /// This may throw in rare cases. If it throws, you still need to + /// call cleanup(). /// /// \throw isc::InvalidOperation if called without previous load() or for /// the second time or cleanup() was called already. - virtual void install() = 0; + void install(); /// \brief Clean up resources. /// @@ -82,7 +95,21 @@ public: /// successful, or the one replaced in install(). /// /// Generally, this should never throw. - virtual void cleanup() = 0; + void cleanup(); + +private: + ZoneTableSegment* segment_; + LoadAction load_action_; + dns::Name origin_; + dns::RRClass rrclass_; + ZoneData* zone_data_; + enum State { + ZW_UNUSED, + ZW_LOADED, + ZW_INSTALLED, + ZW_CLEANED + }; + State state_; }; } diff --git a/src/lib/datasrc/memory/zone_writer_local.h b/src/lib/datasrc/memory/zone_writer_local.h deleted file mode 100644 index 7231a5738a..0000000000 --- a/src/lib/datasrc/memory/zone_writer_local.h +++ /dev/null @@ -1,95 +0,0 @@ -// 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. - -#ifndef MEM_ZONE_WRITER_LOCAL_H -#define MEM_ZONE_WRITER_LOCAL_H - -#include "zone_writer.h" - -#include -#include - -namespace isc { -namespace datasrc { -namespace memory { - -class ZoneData; -class ZoneTableSegmentLocal; - -/// \brief Writer implementation which loads data locally. -/// -/// This implementation prepares a clean zone data and lets one callback -/// to fill it and another to install it somewhere. The class does mostly -/// nothing (and delegates the work to the callbacks), just stores little bit -/// of state between the calls. -class ZoneWriterLocal : public ZoneWriter { -public: - /// \brief Constructor - /// - /// \param segment The zone table segment to store the zone into. - /// \param load_action The callback used to load data. - /// \param install_action The callback used to install the loaded zone. - /// \param rrclass The class of the zone. - ZoneWriterLocal(ZoneTableSegmentLocal* segment, - const LoadAction& load_action, const dns::Name& name, - const dns::RRClass& rrclass); - - /// \brief Destructor - ~ZoneWriterLocal(); - - /// \brief Loads the data. - /// - /// This calls the load_action (passed to constructor) and stores the - /// data for future use. - /// - /// \throw isc::InvalidOperation if it is called the second time in - /// lifetime of the object. - /// \throw Whatever the load_action throws, it is propagated up. - virtual void load(); - - /// \brief Installs the zone. - /// - /// It modifies the zone table accessible through the segment (passed to - /// constructor). - /// - /// \throw isc::InvalidOperation if it is called the second time in - /// lifetime of the object or if load() was not called previously or if - /// cleanup() was already called. - virtual void install(); - - /// \brief Clean up memory. - /// - /// Cleans up the memory used by load()ed zone if not yet installed, or - /// the old zone replaced by install(). - virtual void cleanup(); -private: - ZoneTableSegmentLocal* segment_; - LoadAction load_action_; - dns::Name origin_; - dns::RRClass rrclass_; - ZoneData* zone_data_; - enum State { - ZW_UNUSED, - ZW_LOADED, - ZW_INSTALLED, - ZW_CLEANED - }; - State state_; -}; - -} -} -} - -#endif diff --git a/src/lib/datasrc/tests/memory/zone_table_segment_test.h b/src/lib/datasrc/tests/memory/zone_table_segment_test.h index 2078036376..accd961199 100644 --- a/src/lib/datasrc/tests/memory/zone_table_segment_test.h +++ b/src/lib/datasrc/tests/memory/zone_table_segment_test.h @@ -18,7 +18,7 @@ #include #include #include -#include +#include namespace isc { namespace datasrc { @@ -57,51 +57,12 @@ public: const dns::Name& name, const dns::RRClass& rrclass) { - return (new Writer(this, load_action, name, rrclass)); + return (new ZoneWriter(this, load_action, name, rrclass)); } private: isc::util::MemorySegment& mem_sgmt_; ZoneTableHeader header_; - - // A writer for this segment. The implementation is similar - // to ZoneWriterLocal, but all the error handling is stripped - // for simplicity. Also, we do everything inside the - // install(), for the same reason. We just need something - // inside the tests, not a full-blown implementation - // for background loading. - class Writer : public ZoneWriter { - public: - Writer(ZoneTableSegmentTest* segment, const LoadAction& load_action, - const dns::Name& name, const dns::RRClass& rrclass) : - segment_(segment), - load_action_(load_action), - name_(name), - rrclass_(rrclass) - {} - - void load() {} - - void install() { - ZoneTable* table(segment_->getHeader().getTable()); - const ZoneTable::AddResult - result(table->addZone(segment_->getMemorySegment(), rrclass_, - name_, - load_action_(segment_-> - getMemorySegment()))); - if (result.zone_data != NULL) { - ZoneData::destroy(segment_->getMemorySegment(), - result.zone_data, rrclass_); - } - } - - virtual void cleanup() {} - private: - ZoneTableSegmentTest* segment_; - LoadAction load_action_; - dns::Name name_; - dns::RRClass rrclass_; - }; }; } // namespace test diff --git a/src/lib/datasrc/tests/memory/zone_table_segment_unittest.cc b/src/lib/datasrc/tests/memory/zone_table_segment_unittest.cc index 51d5e0f573..8efb97193b 100644 --- a/src/lib/datasrc/tests/memory/zone_table_segment_unittest.cc +++ b/src/lib/datasrc/tests/memory/zone_table_segment_unittest.cc @@ -12,7 +12,7 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. -#include +#include #include #include @@ -92,9 +92,6 @@ TEST_F(ZoneTableSegmentTest, getZoneWriter) { RRClass::IN())); // We have to get something EXPECT_NE(static_cast(NULL), writer.get()); - // And for now, it should be the local writer - EXPECT_NE(static_cast(NULL), - dynamic_cast(writer.get())); } } // anonymous namespace diff --git a/src/lib/datasrc/tests/memory/zone_writer_unittest.cc b/src/lib/datasrc/tests/memory/zone_writer_unittest.cc index 5d2cd0a2d5..38e4be1e41 100644 --- a/src/lib/datasrc/tests/memory/zone_writer_unittest.cc +++ b/src/lib/datasrc/tests/memory/zone_writer_unittest.cc @@ -12,7 +12,7 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. -#include +#include #include #include @@ -34,15 +34,14 @@ namespace { class TestException {}; -class ZoneWriterLocalTest : public ::testing::Test { +class ZoneWriterTest : public ::testing::Test { public: - ZoneWriterLocalTest() : + ZoneWriterTest() : segment_(ZoneTableSegment::create(RRClass::IN(), "local")), writer_(new - ZoneWriterLocal(dynamic_cast(segment_. - get()), - bind(&ZoneWriterLocalTest::loadAction, this, _1), - Name("example.org"), RRClass::IN())), + ZoneWriter(segment_.get(), + bind(&ZoneWriterTest::loadAction, this, _1), + Name("example.org"), RRClass::IN())), load_called_(false), load_throw_(false), load_null_(false), @@ -54,7 +53,7 @@ public: } protected: scoped_ptr segment_; - scoped_ptr writer_; + scoped_ptr writer_; bool load_called_; bool load_throw_; bool load_null_; @@ -88,7 +87,7 @@ private: // We call it the way we are supposed to, check every callback is called in the // right moment. -TEST_F(ZoneWriterLocalTest, correctCall) { +TEST_F(ZoneWriterTest, correctCall) { // Nothing called before we call it EXPECT_FALSE(load_called_); @@ -105,7 +104,7 @@ TEST_F(ZoneWriterLocalTest, correctCall) { EXPECT_NO_THROW(writer_->cleanup()); } -TEST_F(ZoneWriterLocalTest, loadTwice) { +TEST_F(ZoneWriterTest, loadTwice) { // Load it the first time EXPECT_NO_THROW(writer_->load()); EXPECT_TRUE(load_called_); @@ -126,7 +125,7 @@ TEST_F(ZoneWriterLocalTest, loadTwice) { // Try loading after call to install and call to cleanup. Both is // forbidden. -TEST_F(ZoneWriterLocalTest, loadLater) { +TEST_F(ZoneWriterTest, loadLater) { // Load first, so we can install EXPECT_NO_THROW(writer_->load()); EXPECT_NO_THROW(writer_->install()); @@ -144,7 +143,7 @@ TEST_F(ZoneWriterLocalTest, loadLater) { } // Try calling install at various bad times -TEST_F(ZoneWriterLocalTest, invalidInstall) { +TEST_F(ZoneWriterTest, invalidInstall) { // Nothing loaded yet EXPECT_THROW(writer_->install(), isc::InvalidOperation); EXPECT_FALSE(load_called_); @@ -161,7 +160,7 @@ TEST_F(ZoneWriterLocalTest, invalidInstall) { // We check we can clean without installing first and nothing bad // happens. We also misuse the testcase to check we can't install // after cleanup. -TEST_F(ZoneWriterLocalTest, cleanWithoutInstall) { +TEST_F(ZoneWriterTest, cleanWithoutInstall) { EXPECT_NO_THROW(writer_->load()); EXPECT_NO_THROW(writer_->cleanup()); @@ -172,7 +171,7 @@ TEST_F(ZoneWriterLocalTest, cleanWithoutInstall) { } // Test the case when load callback throws -TEST_F(ZoneWriterLocalTest, loadThrows) { +TEST_F(ZoneWriterTest, loadThrows) { load_throw_ = true; EXPECT_THROW(writer_->load(), TestException); @@ -186,7 +185,7 @@ TEST_F(ZoneWriterLocalTest, loadThrows) { // Check the strong exception guarantee - if it throws, nothing happened // to the content. -TEST_F(ZoneWriterLocalTest, retry) { +TEST_F(ZoneWriterTest, retry) { // First attempt fails due to some exception. load_throw_ = true; EXPECT_THROW(writer_->load(), TestException); @@ -214,7 +213,7 @@ TEST_F(ZoneWriterLocalTest, retry) { } // Check the writer defends itsefl when load action returns NULL -TEST_F(ZoneWriterLocalTest, loadNull) { +TEST_F(ZoneWriterTest, loadNull) { load_null_ = true; EXPECT_THROW(writer_->load(), isc::InvalidOperation); @@ -226,7 +225,7 @@ TEST_F(ZoneWriterLocalTest, loadNull) { } // Check the object cleans up in case we forget it. -TEST_F(ZoneWriterLocalTest, autoCleanUp) { +TEST_F(ZoneWriterTest, autoCleanUp) { // Load data and forget about it. It should get released // when the writer itself is destroyed. EXPECT_NO_THROW(writer_->load());