From: Michal 'vorner' Vaner Date: Mon, 3 Dec 2012 11:24:47 +0000 (+0100) Subject: [2377] Change the callback interface X-Git-Tag: bind10-1.0.0-beta-release~24^2~57 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b548f4ed6cc20a5d8bb802aed646af007c0653b8;p=thirdparty%2Fkea.git [2377] Change the callback interface As we pass exactly one RR each time, RRset is overkill. We just pass bunch of parameters to the callback. --- diff --git a/src/lib/datasrc/master_loader_callbacks.cc b/src/lib/datasrc/master_loader_callbacks.cc index 54e4d0e744..04b8940327 100644 --- a/src/lib/datasrc/master_loader_callbacks.cc +++ b/src/lib/datasrc/master_loader_callbacks.cc @@ -48,6 +48,19 @@ logWarning(const isc::dns::Name& name, const isc::dns::RRClass& rrclass, arg(name).arg(rrclass).arg(reason); } +void +addRR(const isc::dns::Name& name, const isc::dns::RRClass& rrclass, + const isc::dns::RRType& type, const isc::dns::RRTTL& ttl, + const isc::dns::rdata::RdataPtr& data, ZoneUpdater* updater) +{ + // We get description of one RR. The updater takes RRset, so we + // wrap it up and push there. It should collate the RRsets of the + // same name and type together, since the addRRset should "merge". + isc::dns::BasicRRset rrset(name, rrclass, type, ttl); + rrset.addRdata(data); + updater->addRRset(rrset); +} + } isc::dns::MasterLoaderCallbacks @@ -61,12 +74,9 @@ createMasterLoaderCallbacks(const isc::dns::Name& name, rrclass, _1, _2, _3))); } -isc::dns::AddRRsetCallback +isc::dns::AddRRCallback createMasterLoaderAddCallback(ZoneUpdater& updater) { - return (boost::bind(&ZoneUpdater::addRRset, &updater, - // The callback provides a shared pointer, we - // need the object. This bind unpacks the object. - boost::bind(&isc::dns::RRsetPtr::operator*, _1))); + return (boost::bind(addRR, _1, _2, _3, _4, _5, &updater)); } } diff --git a/src/lib/datasrc/master_loader_callbacks.h b/src/lib/datasrc/master_loader_callbacks.h index c2583037b5..ae827c96b5 100644 --- a/src/lib/datasrc/master_loader_callbacks.h +++ b/src/lib/datasrc/master_loader_callbacks.h @@ -58,7 +58,7 @@ createMasterLoaderCallbacks(const isc::dns::Name& name, /// \param updater The zone updater to use. /// \return The callback to be passed to MasterLoader. /// \throw std::bad_alloc when allocation fails. -isc::dns::AddRRsetCallback +isc::dns::AddRRCallback createMasterLoaderAddCallback(ZoneUpdater& updater); } diff --git a/src/lib/datasrc/tests/master_loader_callbacks_test.cc b/src/lib/datasrc/tests/master_loader_callbacks_test.cc index f814288b83..f55596145f 100644 --- a/src/lib/datasrc/tests/master_loader_callbacks_test.cc +++ b/src/lib/datasrc/tests/master_loader_callbacks_test.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include @@ -40,8 +41,8 @@ public: // the correct ones, according to a predefined set in a list. virtual void addRRset(const isc::dns::AbstractRRset& rrset) { ASSERT_FALSE(expected_rrsets_.empty()); - // In our tests, pointer equality is enough. - EXPECT_EQ(expected_rrsets_.front().get(), &rrset); + + EXPECT_EQ(expected_rrsets_.front().get()->toText(), rrset.toText()); // And remove this RRset, as it has been used. expected_rrsets_.pop_front(); } @@ -67,14 +68,22 @@ protected: isc::dns::RRClass::IN(), &ok_)) {} // Generate a new RRset, put it to the updater and return it. - isc::dns::RRsetPtr generateRRset() { + void generateRRset(isc::dns::AddRRCallback callback) { const isc::dns::RRsetPtr result(new isc::dns::RRset(isc::dns::Name("example.org"), isc::dns::RRClass::IN(), isc::dns::RRType::A(), isc::dns::RRTTL(3600))); + const isc::dns::rdata::RdataPtr + data(isc::dns::rdata::createRdata(isc::dns::RRType::A(), + isc::dns::RRClass::IN(), + "192.0.2.1")); + + result->addRdata(data); updater_.expected_rrsets_.push_back(result); - return (result); + + callback(result->getName(), result->getClass(), result->getType(), + result->getTTL(), data); } // An updater to be passed to the context MockUpdater updater_; @@ -112,11 +121,11 @@ TEST_F(MasterLoaderCallbackTest, callbacks) { // Try adding some RRsets. TEST_F(MasterLoaderCallbackTest, addRRset) { - isc::dns::AddRRsetCallback + isc::dns::AddRRCallback callback(createMasterLoaderAddCallback(updater_)); // Put some of them in. - EXPECT_NO_THROW(callback(generateRRset())); - EXPECT_NO_THROW(callback(generateRRset())); + EXPECT_NO_THROW(generateRRset(callback)); + EXPECT_NO_THROW(generateRRset(callback)); // They all get pushed there right away, so there are none in the queue EXPECT_TRUE(updater_.expected_rrsets_.empty()); } diff --git a/src/lib/dns/master_loader.cc b/src/lib/dns/master_loader.cc index e9aad9267a..06cf7c55d1 100644 --- a/src/lib/dns/master_loader.cc +++ b/src/lib/dns/master_loader.cc @@ -18,7 +18,6 @@ #include #include #include -#include #include using std::string; @@ -32,7 +31,7 @@ public: const Name& zone_origin, const RRClass& zone_class, const MasterLoaderCallbacks& callbacks, - const AddRRsetCallback& add_callback, + const AddRRCallback& add_callback, MasterLoader::Options options) : lexer_(), zone_origin_(zone_origin), @@ -88,13 +87,7 @@ public: // the Rdata. The errors should have been reported by // callbacks_ already, so we just need to not report the RR if (data != rdata::RdataPtr()) { - // Create the RRset. We don't need the RRSIG, so we are good - // with the basic one - const RRsetPtr rrset(new BasicRRset(name, rrclass, rrtype, - ttl)); - rrset->addRdata(data); - // OK now, so give the RRset with single RR to the caller - add_callback_(rrset); + add_callback_(name, rrclass, rrtype, ttl, data); // Good, we loaded another one ++count; @@ -108,16 +101,15 @@ private: const Name& zone_origin_; const RRClass zone_class_; MasterLoaderCallbacks callbacks_; - AddRRsetCallback add_callback_; + AddRRCallback add_callback_; MasterLoader::Options options_; - RRsetPtr current_rrset_; }; MasterLoader::MasterLoader(const char* master_file, const Name& zone_origin, const RRClass& zone_class, const MasterLoaderCallbacks& callbacks, - const AddRRsetCallback& add_callback, + const AddRRCallback& add_callback, Options options) { impl_ = new MasterLoaderImpl(master_file, zone_origin, diff --git a/src/lib/dns/master_loader.h b/src/lib/dns/master_loader.h index 61ac7aff2f..dbc3d965c3 100644 --- a/src/lib/dns/master_loader.h +++ b/src/lib/dns/master_loader.h @@ -33,7 +33,7 @@ public: const Name& zone_origin, const RRClass& zone_class, const MasterLoaderCallbacks& callbacks, - const AddRRsetCallback& add_callback, + const AddRRCallback& add_callback, Options options = DEFAULT); ~MasterLoader(); diff --git a/src/lib/dns/master_loader_callbacks.h b/src/lib/dns/master_loader_callbacks.h index 77d9ac5585..44ddbfa7d1 100644 --- a/src/lib/dns/master_loader_callbacks.h +++ b/src/lib/dns/master_loader_callbacks.h @@ -23,9 +23,14 @@ namespace isc { namespace dns { - -class AbstractRRset; -typedef boost::shared_ptr RRsetPtr; +class Name; +class RRClass; +class RRType; +class RRTTL; +namespace rdata { +class Rdata; +typedef boost::shared_ptr RdataPtr; +} /// \brief Type of callback to add a RRset. /// @@ -36,7 +41,10 @@ typedef boost::shared_ptr RRsetPtr; /// \param RRset The rrset to add. It does not contain the accompanying /// RRSIG (if the zone is signed), they are reported with separate /// calls to the callback. -typedef boost::function AddRRsetCallback; +typedef boost::function + AddRRCallback; /// \brief Set of issue callbacks for a loader. /// diff --git a/src/lib/dns/tests/master_loader_unittest.cc b/src/lib/dns/tests/master_loader_unittest.cc index d0a3339366..7b275792d7 100644 --- a/src/lib/dns/tests/master_loader_unittest.cc +++ b/src/lib/dns/tests/master_loader_unittest.cc @@ -55,7 +55,11 @@ public: } } - void addRRset(const RRsetPtr& rrset) { + void addRRset(const Name& name, const RRClass& rrclass, + const RRType& rrtype, const RRTTL& rrttl, + const rdata::RdataPtr& data) { + const RRsetPtr rrset(new BasicRRset(name, rrclass, rrtype, rrttl)); + rrset->addRdata(data); rrsets_.push_back(rrset); } @@ -66,7 +70,8 @@ public: file).c_str(), origin, rrclass, callbacks_, boost::bind(&MasterLoaderTest::addRRset, - this, _1), options)); + this, _1, _2, _3, _4, _5), + options)); } // Check the next RR in the ones produced by the loader