From: Mukund Sivaraman Date: Tue, 15 Jan 2013 05:53:43 +0000 (+0530) Subject: [2435] Make ZoneUpdater::getRRsetCollection() return a RRsetCollectionBase& X-Git-Tag: bind10-1.0.0-rc-release~108 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=caf2b100bf61d75cd98935bb59b6b39a718289a9;p=thirdparty%2Fkea.git [2435] Make ZoneUpdater::getRRsetCollection() return a RRsetCollectionBase& ... because in case of some updaters, the returned RRsetCollectionBase is very well a singleton for that updater instance, and it does not make sense to return multiple RRsetCollections for it. --- diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc index bfa029cd05..421e3e2018 100644 --- a/src/lib/datasrc/database.cc +++ b/src/lib/datasrc/database.cc @@ -1498,8 +1498,13 @@ public: virtual ZoneFinder& getFinder() { return (*finder_); } - virtual RRsetCollectionPtr getRRsetCollection() { - return (RRsetCollectionPtr(new RRsetCollection(*this, zone_class_))); + virtual RRsetCollectionBase& getRRsetCollection() { + if (!rrset_collection_) { + // This is only assigned the first time and remains for the + // lifetime of the DatabaseUpdater. + rrset_collection_.reset(new RRsetCollection(*this, zone_class_)); + } + return (*rrset_collection_); } virtual void addRRset(const AbstractRRset& rrset); @@ -1526,6 +1531,7 @@ private: DiffPhase diff_phase_; Serial serial_; boost::scoped_ptr finder_; + RRsetCollectionPtr rrset_collection_; // This is a set of validation checks commonly used for addRRset() and // deleteRRset to minimize duplicate code logic and to make the main diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc index 06c399f547..7d9b1b148a 100644 --- a/src/lib/datasrc/tests/database_unittest.cc +++ b/src/lib/datasrc/tests/database_unittest.cc @@ -4172,13 +4172,13 @@ public: {} ZoneUpdaterPtr updater; - RRsetCollectionPtr collection; + RRsetCollectionBase& collection; }; TYPED_TEST(RRsetCollectionTest, find) { // Test the find() that returns ConstRRsetPtr - ConstRRsetPtr rrset = this->collection->find(Name("www.example.org."), - RRClass::IN(), RRType::A()); + ConstRRsetPtr rrset = this->collection.find(Name("www.example.org."), + RRClass::IN(), RRType::A()); ASSERT_TRUE(rrset); EXPECT_EQ(RRType::A(), rrset->getType()); EXPECT_EQ(RRTTL(3600), rrset->getTTL()); @@ -4186,59 +4186,59 @@ TYPED_TEST(RRsetCollectionTest, find) { EXPECT_EQ(Name("www.example.org"), rrset->getName()); // foo.example.org doesn't exist - rrset = this->collection->find(Name("foo.example.org"), this->qclass_, - RRType::A()); + rrset = this->collection.find(Name("foo.example.org"), this->qclass_, + RRType::A()); EXPECT_FALSE(rrset); // www.example.org exists, but not with MX - rrset = this->collection->find(Name("www.example.org"), this->qclass_, - RRType::MX()); + rrset = this->collection.find(Name("www.example.org"), this->qclass_, + RRType::MX()); EXPECT_FALSE(rrset); // www.example.org exists, with AAAA - rrset = this->collection->find(Name("www.example.org"), this->qclass_, - RRType::AAAA()); + rrset = this->collection.find(Name("www.example.org"), this->qclass_, + RRType::AAAA()); EXPECT_TRUE(rrset); // www.example.org with AAAA does not exist in RRClass::CH() - rrset = this->collection->find(Name("www.example.org"), RRClass::CH(), - RRType::AAAA()); + rrset = this->collection.find(Name("www.example.org"), RRClass::CH(), + RRType::AAAA()); EXPECT_FALSE(rrset); // Out-of-zone find()s must not throw. - rrset = this->collection->find(Name("www.example.com"), this->qclass_, - RRType::A()); + rrset = this->collection.find(Name("www.example.com"), this->qclass_, + RRType::A()); EXPECT_FALSE(rrset); // "cname.example.org." with type CNAME should return the CNAME RRset - rrset = this->collection->find(Name("cname.example.org"), this->qclass_, - RRType::CNAME()); + rrset = this->collection.find(Name("cname.example.org"), this->qclass_, + RRType::CNAME()); ASSERT_TRUE(rrset); EXPECT_EQ(RRType::CNAME(), rrset->getType()); EXPECT_EQ(Name("cname.example.org"), rrset->getName()); // "cname.example.org." with type A should return nothing - rrset = this->collection->find(Name("cname.example.org"), this->qclass_, - RRType::A()); + rrset = this->collection.find(Name("cname.example.org"), this->qclass_, + RRType::A()); EXPECT_FALSE(rrset); // "dname.example.org." with type DNAME should return the DNAME RRset - rrset = this->collection->find(Name("dname.example.org"), this->qclass_, - RRType::DNAME()); + rrset = this->collection.find(Name("dname.example.org"), this->qclass_, + RRType::DNAME()); ASSERT_TRUE(rrset); EXPECT_EQ(RRType::DNAME(), rrset->getType()); EXPECT_EQ(Name("dname.example.org"), rrset->getName()); // "below.dname.example.org." with type AAAA should return nothing - rrset = this->collection->find(Name("below.dname.example.org"), - this->qclass_, RRType::AAAA()); + rrset = this->collection.find(Name("below.dname.example.org"), + this->qclass_, RRType::AAAA()); EXPECT_FALSE(rrset); // TODO: "below.dname.example.org." with type A does not return the // record (see top of file). It needs to be checked if this is what // we want. - rrset = this->collection->find(Name("below.dname.example.org"), - this->qclass_, RRType::A()); + rrset = this->collection.find(Name("below.dname.example.org"), + this->qclass_, RRType::A()); // Is this correct behavior? EXPECT_FALSE(rrset); @@ -4247,8 +4247,8 @@ TYPED_TEST(RRsetCollectionTest, find) { // return the NS record. Without FIND_GLUE_OK, ZoneFinder's find() // would return DELEGATION and the find() below would return // nothing. - rrset = this->collection->find(Name("delegation.example.org"), - this->qclass_, RRType::NS()); + rrset = this->collection.find(Name("delegation.example.org"), + this->qclass_, RRType::NS()); ASSERT_TRUE(rrset); EXPECT_EQ(RRType::NS(), rrset->getType()); EXPECT_EQ(Name("delegation.example.org"), rrset->getName()); @@ -4257,14 +4257,14 @@ TYPED_TEST(RRsetCollectionTest, find) { // searching for some "foo.wildcard.example.org." would make // ZoneFinder's find() return NXDOMAIN, and the find() below should // return nothing. - rrset = this->collection->find(Name("foo.wild.example.org"), - this->qclass_, RRType::A()); + rrset = this->collection.find(Name("foo.wild.example.org"), + this->qclass_, RRType::A()); EXPECT_FALSE(rrset); // Searching directly for "*.wild.example.org." should return the // record. - rrset = this->collection->find(Name("*.wild.example.org"), - this->qclass_, RRType::A()); + rrset = this->collection.find(Name("*.wild.example.org"), + this->qclass_, RRType::A()); ASSERT_TRUE(rrset); EXPECT_EQ(RRType::A(), rrset->getType()); EXPECT_EQ(Name("*.wild.example.org"), rrset->getName()); @@ -4272,8 +4272,8 @@ TYPED_TEST(RRsetCollectionTest, find) { TYPED_TEST(RRsetCollectionTest, iteratorTest) { // Iterators are currently not implemented. - EXPECT_THROW(this->collection->begin(), isc::NotImplemented); - EXPECT_THROW(this->collection->end(), isc::NotImplemented); + EXPECT_THROW(this->collection.begin(), isc::NotImplemented); + EXPECT_THROW(this->collection.end(), isc::NotImplemented); } typedef RRsetCollectionTest MockRRsetCollectionTest; @@ -4286,8 +4286,8 @@ TEST_F(MockRRsetCollectionTest, findError) { // The "dsexception.example.org." name is rigged by the MockAccessor // to throw a DataSourceError. EXPECT_THROW({ - this->collection->find(Name("dsexception.example.org"), this->qclass_, - RRType::A()); + this->collection.find(Name("dsexception.example.org"), this->qclass_, + RRType::A()); }, RRsetCollectionError); } diff --git a/src/lib/datasrc/tests/master_loader_callbacks_test.cc b/src/lib/datasrc/tests/master_loader_callbacks_test.cc index ecc0f283d0..fb4487ad48 100644 --- a/src/lib/datasrc/tests/master_loader_callbacks_test.cc +++ b/src/lib/datasrc/tests/master_loader_callbacks_test.cc @@ -65,7 +65,7 @@ public: virtual ZoneFinder& getFinder() { isc_throw(isc::NotImplemented, "Not to be called in this test"); } - virtual isc::dns::RRsetCollectionPtr getRRsetCollection() { + virtual isc::dns::RRsetCollectionBase& getRRsetCollection() { isc_throw(isc::NotImplemented, "Not to be called in this test"); } virtual void deleteRRset(const isc::dns::AbstractRRset&) { diff --git a/src/lib/datasrc/tests/zone_loader_unittest.cc b/src/lib/datasrc/tests/zone_loader_unittest.cc index b8517f4bc3..bff2b29c39 100644 --- a/src/lib/datasrc/tests/zone_loader_unittest.cc +++ b/src/lib/datasrc/tests/zone_loader_unittest.cc @@ -89,7 +89,7 @@ public: virtual ZoneFinder& getFinder() { return (finder_); } - virtual isc::dns::RRsetCollectionPtr getRRsetCollection() { + virtual isc::dns::RRsetCollectionBase& getRRsetCollection() { isc_throw(isc::NotImplemented, "Method not used in tests"); } virtual void addRRset(const isc::dns::AbstractRRset& rrset) { diff --git a/src/lib/datasrc/zone.h b/src/lib/datasrc/zone.h index c7b50bcf21..36651d4f11 100644 --- a/src/lib/datasrc/zone.h +++ b/src/lib/datasrc/zone.h @@ -807,10 +807,11 @@ public: /// /// This method returns an \c RRsetCollection for the updater, /// implementing the \c isc::dns::RRsetCollectionBase - /// interface. While the returned RRsetCollection object is - /// existing, the corresponding \c ZoneUpdater must not be - /// destroyed. The returned RRsetCollection object may be used to - /// search RRsets from the ZoneUpdater. The actual + /// interface. Typically, the returned \c RRsetCollection is a + /// singleton for its \c ZoneUpdater. The returned RRsetCollection + /// object must not be used after its corresponding \c ZoneUpdater + /// has been destroyed. The returned RRsetCollection object may be + /// used to search RRsets from the ZoneUpdater. The actual /// \c RRsetCollection returned has a behavior dependent on the /// \c ZoneUpdater implementation. /// @@ -818,7 +819,7 @@ public: /// of the \c Zonefinder returned by \c getFinder() with regards to /// adding and deleting RRsets via \c addRRset() and \c /// deleteRRset(). - virtual isc::dns::RRsetCollectionPtr getRRsetCollection() = 0; + virtual isc::dns::RRsetCollectionBase& getRRsetCollection() = 0; /// Add an RRset to a zone via the updater ///