BIND 10 trac2435, updated. 0c28935d20983b280976f0a62e4b32de735b29a8 [2435] Updating after getting RRsetCollection on DatabaseUpdater must fail
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Jan 15 07:07:31 UTC 2013
The branch, trac2435 has been updated
via 0c28935d20983b280976f0a62e4b32de735b29a8 (commit)
via 568655c55d104fcee40289810dd4431188a989f1 (commit)
via f2f653c6f29d56c00fdb09af6ba54bd1ab36f31f (commit)
from 8bd08bf2531ad91b4d3f4ef4b8f304c2dfc9c249 (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 0c28935d20983b280976f0a62e4b32de735b29a8
Author: Mukund Sivaraman <muks at isc.org>
Date: Tue Jan 15 12:34:45 2013 +0530
[2435] Updating after getting RRsetCollection on DatabaseUpdater must fail
commit 568655c55d104fcee40289810dd4431188a989f1
Author: Mukund Sivaraman <muks at isc.org>
Date: Tue Jan 15 11:58:08 2013 +0530
[2435] Return a isc::datasrc::RRsetCollectionBase in ZoneUpdater::getRRsetCollection()
commit f2f653c6f29d56c00fdb09af6ba54bd1ab36f31f
Author: Mukund Sivaraman <muks at isc.org>
Date: Tue Jan 15 11:44:21 2013 +0530
[2435] Introduce an abstract datasrc::RRsetCollectionBase class
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/Makefile.am | 1 +
src/lib/datasrc/database.cc | 86 +++++--------------
src/lib/datasrc/rrset_collection_base.cc | 67 +++++++++++++++
src/lib/datasrc/rrset_collection_base.h | 87 ++++++++++++++++++++
src/lib/datasrc/tests/database_unittest.cc | 52 +++++++++++-
.../datasrc/tests/master_loader_callbacks_test.cc | 2 +-
src/lib/datasrc/tests/zone_loader_unittest.cc | 2 +-
src/lib/datasrc/zone.h | 23 ++++--
src/lib/dns/rrset_collection_base.h | 6 --
9 files changed, 245 insertions(+), 81 deletions(-)
create mode 100644 src/lib/datasrc/rrset_collection_base.cc
create mode 100644 src/lib/datasrc/rrset_collection_base.h
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/Makefile.am b/src/lib/datasrc/Makefile.am
index a4edd4e..dc1007a 100644
--- a/src/lib/datasrc/Makefile.am
+++ b/src/lib/datasrc/Makefile.am
@@ -38,6 +38,7 @@ libb10_datasrc_la_SOURCES += client_list.h client_list.cc
libb10_datasrc_la_SOURCES += memory_datasrc.h memory_datasrc.cc
libb10_datasrc_la_SOURCES += master_loader_callbacks.h
libb10_datasrc_la_SOURCES += master_loader_callbacks.cc
+libb10_datasrc_la_SOURCES += rrset_collection_base.h rrset_collection_base.cc
libb10_datasrc_la_SOURCES += zone_loader.h zone_loader.cc
nodist_libb10_datasrc_la_SOURCES = datasrc_messages.h datasrc_messages.cc
libb10_datasrc_la_LDFLAGS = -no-undefined -version-info 1:0:1
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index 1dbce99..b125295 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -19,6 +19,7 @@
#include <datasrc/database.h>
#include <datasrc/data_source.h>
#include <datasrc/iterator.h>
+#include <datasrc/rrset_collection_base.h>
#include <exceptions/exceptions.h>
#include <dns/name.h>
@@ -1373,79 +1374,16 @@ DatabaseClient::getIterator(const isc::dns::Name& name,
}
/// \brief datasrc implementation of RRsetCollectionBase.
-class RRsetCollection : public isc::dns::RRsetCollectionBase {
+class RRsetCollection : public isc::datasrc::RRsetCollectionBase {
public:
/// \brief Constructor.
- ///
- /// No reference (count via \c shared_ptr) to the \c ZoneUpdater is
- /// acquired. The RRsetCollection must not be used after its
- /// \c ZoneUpdater has been destroyed.
- ///
- /// \param updater The ZoneUpdater to wrap around.
- /// \param rrclass The RRClass of the records in the zone.
RRsetCollection(ZoneUpdater& updater, const isc::dns::RRClass& rrclass) :
- updater_(updater),
- rrclass_(rrclass)
+ isc::datasrc::RRsetCollectionBase(updater, rrclass)
{}
/// \brief Destructor
virtual ~RRsetCollection() {}
- /// \brief Find a matching RRset in the collection.
- ///
- /// Returns the RRset in the collection that exactly matches the
- /// given \c name, \c rrclass and \c rrtype. If no matching RRset
- /// is found, \c NULL is returned.
- ///
- /// \throw isc::dns::RRsetCollectionError if find() results in some
- /// underlying datasrc error.
- /// \param name The name of the RRset to search for.
- /// \param rrclass The class of the RRset to search for.
- /// \param rrtype The type of the RRset to search for.
- /// \returns The RRset if found, \c NULL otherwise.
- virtual isc::dns::ConstRRsetPtr find(const isc::dns::Name& name,
- const isc::dns::RRClass& rrclass,
- const isc::dns::RRType& rrtype) const {
- if (rrclass != rrclass_) {
- // We could throw an exception here, but RRsetCollection is
- // expected to support an arbitrary collection of RRsets,
- // and it can be queried just as arbitrarily. So we just
- // return nothing here.
- return (ConstRRsetPtr());
- }
-
- ZoneFinder& finder = updater_.getFinder();
- try {
- ZoneFinderContextPtr result =
- finder.find(name, rrtype,
- ZoneFinder::NO_WILDCARD | ZoneFinder::FIND_GLUE_OK);
- // We return the result rrset only if the result code is
- // SUCCESS. We return empty if CNAME, DNAME, DELEGATION,
- // etc. are returned by the ZoneFinder.
- //
- // Note that in the case that the queried type itself is
- // CNAME or DNAME, then the finder will return SUCCESS.
- if (result->code == ZoneFinder::SUCCESS) {
- return (result->rrset);
- } else {
- return (ConstRRsetPtr());
- }
- } catch (const OutOfZone&) {
- // As RRsetCollection is an arbitrary set of RRsets, in case
- // the searched name is out of zone, we return nothing
- // instead of propagating the exception.
- return (ConstRRsetPtr());
- } catch (const DataSourceError& e) {
- isc_throw(RRsetCollectionError,
- "ZoneFinder threw a DataSourceError: "
- << e.getMessage().c_str());
- }
- }
-
-private:
- ZoneUpdater& updater_;
- isc::dns::RRClass rrclass_;
-
protected:
// TODO: RRsetCollectionBase::Iter is not implemented and the
// following two methods just throw.
@@ -1498,7 +1436,7 @@ public:
virtual ZoneFinder& getFinder() { return (*finder_); }
- virtual RRsetCollectionBase& getRRsetCollection() {
+ virtual isc::datasrc::RRsetCollectionBase& getRRsetCollection() {
if (!rrset_collection_) {
// This is only assigned the first time and remains for the
// lifetime of the DatabaseUpdater.
@@ -1662,6 +1600,14 @@ isNSEC3KindType(RRType rrtype, const Rdata& rdata) {
void
DatabaseUpdater::addRRset(const AbstractRRset& rrset) {
+ if (rrset_collection_) {
+ isc_throw(InvalidOperation,
+ "Cannot add RRset after an RRsetCollection has been "
+ "requested for ZoneUpdater for "
+ << zone_name_ << "/" << zone_class_ << " on "
+ << db_name_);
+ }
+
validateAddOrDelete("add", rrset, DELETE, ADD);
// It's guaranteed rrset has at least one RDATA at this point.
@@ -1712,6 +1658,14 @@ DatabaseUpdater::addRRset(const AbstractRRset& rrset) {
void
DatabaseUpdater::deleteRRset(const AbstractRRset& rrset) {
+ if (rrset_collection_) {
+ isc_throw(InvalidOperation,
+ "Cannot delete RRset after an RRsetCollection has been "
+ "requested for ZoneUpdater for "
+ << zone_name_ << "/" << zone_class_ << " on "
+ << db_name_);
+ }
+
// If this is the first operation, pretend we are starting a new delete
// sequence after adds. This will simplify the validation below.
if (diff_phase_ == NOT_STARTED) {
diff --git a/src/lib/datasrc/rrset_collection_base.cc b/src/lib/datasrc/rrset_collection_base.cc
new file mode 100644
index 0000000..aa382f2
--- /dev/null
+++ b/src/lib/datasrc/rrset_collection_base.cc
@@ -0,0 +1,67 @@
+// Copyright (C) 2013 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.
+
+#include <datasrc/rrset_collection_base.h>
+#include <datasrc/zone_loader.h>
+#include <exceptions/exceptions.h>
+
+using namespace isc;
+using namespace isc::dns;
+
+namespace isc {
+namespace datasrc {
+
+ConstRRsetPtr
+isc::datasrc::RRsetCollectionBase::find(const isc::dns::Name& name,
+ const isc::dns::RRClass& rrclass,
+ const isc::dns::RRType& rrtype) const
+{
+ if (rrclass != rrclass_) {
+ // We could throw an exception here, but RRsetCollection is
+ // expected to support an arbitrary collection of RRsets, and it
+ // can be queried just as arbitrarily. So we just return nothing
+ // here.
+ return (ConstRRsetPtr());
+ }
+
+ ZoneFinder& finder = updater_.getFinder();
+ try {
+ ZoneFinderContextPtr result =
+ finder.find(name, rrtype,
+ ZoneFinder::NO_WILDCARD | ZoneFinder::FIND_GLUE_OK);
+ // We return the result rrset only if the result code is
+ // SUCCESS. We return empty if CNAME, DNAME, DELEGATION,
+ // etc. are returned by the ZoneFinder.
+ //
+ // Note that in the case that the queried type itself is CNAME
+ // or DNAME, then the finder will return SUCCESS.
+ if (result->code == ZoneFinder::SUCCESS) {
+ return (result->rrset);
+ } else {
+ return (ConstRRsetPtr());
+ }
+ } catch (const OutOfZone&) {
+ // As RRsetCollection is an arbitrary set of RRsets, in case the
+ // searched name is out of zone, we return nothing instead of
+ // propagating the exception.
+ return (ConstRRsetPtr());
+ } catch (const DataSourceError& e) {
+ isc_throw(RRsetCollectionError,
+ "ZoneFinder threw a DataSourceError: "
+ << e.getMessage().c_str());
+ }
+}
+
+} // end of namespace datasrc
+} // end of namespace isc
diff --git a/src/lib/datasrc/rrset_collection_base.h b/src/lib/datasrc/rrset_collection_base.h
new file mode 100644
index 0000000..0e20d74
--- /dev/null
+++ b/src/lib/datasrc/rrset_collection_base.h
@@ -0,0 +1,87 @@
+// Copyright (C) 2013 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 RRSET_COLLECTION_DATASRC_H
+#define RRSET_COLLECTION_DATASRC_H 1
+
+#include <dns/rrset_collection_base.h>
+#include <dns/rrclass.h>
+#include <datasrc/zone.h>
+
+namespace isc {
+namespace datasrc {
+
+/// \brief A forward declaration
+class ZoneUpdater;
+
+/// \brief datasrc derivation of \c isc::dns::RRsetCollectionBase.
+///
+/// This is an abstract class that adds datasrc related detail to
+/// \c isc::dns::RRsetCollectionBase. Derived classes need to complete
+/// the implementation (add iterator support, etc.) before using it.
+class RRsetCollectionBase : public isc::dns::RRsetCollectionBase {
+public:
+ /// \brief Constructor.
+ ///
+ /// No reference (count via \c shared_ptr) to the \c ZoneUpdater is
+ /// acquired. The RRsetCollection must not be used after its
+ /// \c ZoneUpdater has been destroyed.
+ ///
+ /// \param updater The ZoneUpdater to wrap around.
+ /// \param rrclass The RRClass of the records in the zone.
+ RRsetCollectionBase(ZoneUpdater& updater,
+ const isc::dns::RRClass& rrclass) :
+ updater_(updater),
+ rrclass_(rrclass)
+ {}
+
+ /// \brief Destructor
+ virtual ~RRsetCollectionBase() {}
+
+ /// \brief Find a matching RRset in the collection.
+ ///
+ /// Returns the RRset in the collection that exactly matches the
+ /// given \c name, \c rrclass and \c rrtype. If no matching RRset
+ /// is found, \c NULL is returned.
+ ///
+ /// \throw isc::dns::RRsetCollectionError if find() results in some
+ /// underlying datasrc error.
+ /// \param name The name of the RRset to search for.
+ /// \param rrclass The class of the RRset to search for.
+ /// \param rrtype The type of the RRset to search for.
+ /// \returns The RRset if found, \c NULL otherwise.
+ virtual isc::dns::ConstRRsetPtr find(const isc::dns::Name& name,
+ const isc::dns::RRClass& rrclass,
+ const isc::dns::RRType& rrtype) const;
+
+private:
+ ZoneUpdater& updater_;
+ isc::dns::RRClass rrclass_;
+};
+
+/// \brief A pointer-like type pointing to an
+/// \c isc::datasrc::RRsetCollectionBase object.
+///
+/// This type is used to handle RRsetCollections in a polymorphic manner
+/// in libdatasrc.
+typedef boost::shared_ptr<isc::datasrc::RRsetCollectionBase> RRsetCollectionPtr;
+
+} // end of namespace datasrc
+} // end of namespace isc
+
+#endif // RRSET_COLLECTION_DATASRC_H
+
+// Local Variables:
+// mode: c++
+// End:
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index 7d9b1b1..6665642 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -4172,7 +4172,7 @@ public:
{}
ZoneUpdaterPtr updater;
- RRsetCollectionBase& collection;
+ isc::datasrc::RRsetCollectionBase& collection;
};
TYPED_TEST(RRsetCollectionTest, find) {
@@ -4291,4 +4291,54 @@ TEST_F(MockRRsetCollectionTest, findError) {
}, RRsetCollectionError);
}
+TYPED_TEST_CASE(RRsetCollectionAndUpdaterTest, TestAccessorTypes);
+
+// This test fixture is templated so that we can share (most of) the test
+// cases with different types of data sources. Note that in test cases
+// we need to use 'this' to refer to member variables of the test class.
+template <typename ACCESSOR_TYPE>
+class RRsetCollectionAndUpdaterTest : public DatabaseClientTest<ACCESSOR_TYPE> {
+public:
+ RRsetCollectionAndUpdaterTest() :
+ DatabaseClientTest<ACCESSOR_TYPE>(),
+ updater_(this->client_->getUpdater(this->zname_, false))
+ {}
+
+ ZoneUpdaterPtr updater_;
+};
+
+TYPED_TEST(RRsetCollectionAndUpdaterTest, updateThrows) {
+ // 1. Addition test
+
+ // addRRset() must not throw.
+ this->updater_->addRRset(*this->rrset_);
+
+ // Now setup a new updater and call getRRsetCollection() on it.
+ this->updater_.reset();
+ this->updater_ = this->client_->getUpdater(this->zname_, false);
+ (void) this->updater_->getRRsetCollection();
+
+ // addRRset() must throw isc::InvalidOperation here.
+ EXPECT_THROW(this->updater_->addRRset(*this->rrset_),
+ isc::InvalidOperation);
+
+ // 2. Deletion test
+
+ // deleteRRset() must not throw.
+ this->updater_.reset();
+ this->updater_ = this->client_->getUpdater(this->zname_, false);
+ this->updater_->addRRset(*this->rrset_);
+ this->updater_->deleteRRset(*this->rrset_);
+
+ // Now setup a new updater and call getRRsetCollection() on it.
+ this->updater_.reset();
+ this->updater_ = this->client_->getUpdater(this->zname_, false);
+ this->updater_->addRRset(*this->rrset_);
+ (void) this->updater_->getRRsetCollection();
+
+ // deleteRRset() must throw isc::InvalidOperation here.
+ EXPECT_THROW(this->updater_->deleteRRset(*this->rrset_),
+ isc::InvalidOperation);
+}
+
}
diff --git a/src/lib/datasrc/tests/master_loader_callbacks_test.cc b/src/lib/datasrc/tests/master_loader_callbacks_test.cc
index fb4487a..dc44461 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::RRsetCollectionBase& getRRsetCollection() {
+ virtual isc::datasrc::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 bff2b29..943ea2f 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::RRsetCollectionBase& getRRsetCollection() {
+ virtual isc::datasrc::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 36651d4..aefa902 100644
--- a/src/lib/datasrc/zone.h
+++ b/src/lib/datasrc/zone.h
@@ -18,10 +18,10 @@
#include <dns/name.h>
#include <dns/rrset.h>
#include <dns/rrtype.h>
-#include <dns/rrset_collection_base.h>
#include <datasrc/exceptions.h>
#include <datasrc/result.h>
+#include <datasrc/rrset_collection_base.h>
#include <utility>
#include <vector>
@@ -741,6 +741,9 @@ typedef boost::shared_ptr<ZoneFinder::Context> ZoneFinderContextPtr;
/// \c ZoneFinder::Context object.
typedef boost::shared_ptr<ZoneFinder::Context> ConstZoneFinderContextPtr;
+/// \brief A forward declaration
+class RRsetCollectionBase;
+
/// The base class to make updates to a single zone.
///
/// On construction, each derived class object will start a "transaction"
@@ -806,7 +809,7 @@ public:
/// Return an RRsetCollection for the updater.
///
/// This method returns an \c RRsetCollection for the updater,
- /// implementing the \c isc::dns::RRsetCollectionBase
+ /// implementing the \c isc::datasrc::RRsetCollectionBase
/// 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
@@ -816,10 +819,10 @@ public:
/// \c ZoneUpdater implementation.
///
/// The behavior of the RRsetCollection is similar to the behavior
- /// of the \c Zonefinder returned by \c getFinder() with regards to
- /// adding and deleting RRsets via \c addRRset() and \c
- /// deleteRRset().
- virtual isc::dns::RRsetCollectionBase& getRRsetCollection() = 0;
+ /// of the \c Zonefinder returned by \c getFinder().
+ /// Implementations of \c ZoneUpdater may not allow adding or
+ /// deleting RRsets after \c getRRsetCollection() is called.
+ virtual isc::datasrc::RRsetCollectionBase& getRRsetCollection() = 0;
/// Add an RRset to a zone via the updater
///
@@ -868,6 +871,10 @@ public:
/// calls after \c commit() the implementation must throw a
/// \c DataSourceError exception.
///
+ /// Implementations of \c ZoneUpdater may not allow adding or
+ /// deleting RRsets after \c getRRsetCollection() is called. In this
+ /// case, implementations throw an \c InvalidOperation exception.
+ ///
/// If journaling was requested when getting this updater, it will reject
/// to add the RRset if the squence doesn't look like and IXFR (see
/// DataSourceClient::getUpdater). In such case isc::BadValue is thrown.
@@ -939,6 +946,10 @@ public:
/// calls after \c commit() the implementation must throw a
/// \c DataSourceError exception.
///
+ /// Implementations of \c ZoneUpdater may not allow adding or
+ /// deleting RRsets after \c getRRsetCollection() is called. In this
+ /// case, implementations throw an \c InvalidOperation exception.
+ ///
/// If journaling was requested when getting this updater, it will reject
/// to add the RRset if the squence doesn't look like and IXFR (see
/// DataSourceClient::getUpdater). In such case isc::BadValue is thrown.
diff --git a/src/lib/dns/rrset_collection_base.h b/src/lib/dns/rrset_collection_base.h
index f757321..b00435d 100644
--- a/src/lib/dns/rrset_collection_base.h
+++ b/src/lib/dns/rrset_collection_base.h
@@ -171,12 +171,6 @@ public:
}
};
-/// \brief A pointer-like type pointing to an \c RRsetCollection object.
-///
-/// This type is used to handle RRsetCollections in a polymorphic manner
-/// in the BIND 10 codebase.
-typedef boost::shared_ptr<RRsetCollectionBase> RRsetCollectionPtr;
-
} // end of namespace dns
} // end of namespace isc
More information about the bind10-changes
mailing list