BIND 10 trac2435, updated. 2b05b3926ac85363b1063a12c9ac8d0ac9cda3de [2435] Throw an exception if the database RRsetCollection implementation is used after commit()

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Jan 15 07:46:00 UTC 2013


The branch, trac2435 has been updated
       via  2b05b3926ac85363b1063a12c9ac8d0ac9cda3de (commit)
      from  0c28935d20983b280976f0a62e4b32de735b29a8 (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 2b05b3926ac85363b1063a12c9ac8d0ac9cda3de
Author: Mukund Sivaraman <muks at isc.org>
Date:   Tue Jan 15 13:13:31 2013 +0530

    [2435] Throw an exception if the database RRsetCollection implementation is used after commit()

-----------------------------------------------------------------------

Summary of changes:
 src/lib/datasrc/database.cc                |   13 ++++++++-
 src/lib/datasrc/rrset_collection_base.cc   |    4 +++
 src/lib/datasrc/rrset_collection_base.h    |   41 ++++++++++++++++++++++++++--
 src/lib/datasrc/tests/database_unittest.cc |   20 ++++++++++++++
 src/lib/datasrc/zone.h                     |    5 ++++
 src/lib/dns/rrset_collection_base.h        |    6 ++++
 6 files changed, 85 insertions(+), 4 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index b125295..0a3957d 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -1384,6 +1384,12 @@ public:
     /// \brief Destructor
     virtual ~RRsetCollection() {}
 
+    /// \brief A wrapper around \c disable() so that it can be used as a
+    /// public method. \c disable() is protected.
+    void disableWrapper() {
+        disable();
+    }
+
 protected:
     // TODO: RRsetCollectionBase::Iter is not implemented and the
     // following two methods just throw.
@@ -1469,7 +1475,7 @@ private:
     DiffPhase diff_phase_;
     Serial serial_;
     boost::scoped_ptr<DatabaseClient::Finder> finder_;
-    RRsetCollectionPtr rrset_collection_;
+    boost::shared_ptr<isc::datasrc::RRsetCollection> 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
@@ -1720,6 +1726,11 @@ DatabaseUpdater::commit() {
     accessor_->commit();
     committed_ = true; // make sure the destructor won't trigger rollback
 
+    // Disable the RRsetCollection if it exists.
+    if (rrset_collection_) {
+        rrset_collection_->disableWrapper();
+    }
+
     // We release the accessor immediately after commit is completed so that
     // we don't hold the possible internal resource any longer.
     accessor_.reset();
diff --git a/src/lib/datasrc/rrset_collection_base.cc b/src/lib/datasrc/rrset_collection_base.cc
index aa382f2..b19f62e 100644
--- a/src/lib/datasrc/rrset_collection_base.cc
+++ b/src/lib/datasrc/rrset_collection_base.cc
@@ -27,6 +27,10 @@ isc::datasrc::RRsetCollectionBase::find(const isc::dns::Name& name,
                                         const isc::dns::RRClass& rrclass,
                                         const isc::dns::RRType& rrtype) const
 {
+    if (isDisabled()) {
+        isc_throw(RRsetCollectionError, "This RRsetCollection is disabled.");
+    }
+
     if (rrclass != rrclass_) {
         // We could throw an exception here, but RRsetCollection is
         // expected to support an arbitrary collection of RRsets, and it
diff --git a/src/lib/datasrc/rrset_collection_base.h b/src/lib/datasrc/rrset_collection_base.h
index 0e20d74..7a414df 100644
--- a/src/lib/datasrc/rrset_collection_base.h
+++ b/src/lib/datasrc/rrset_collection_base.h
@@ -43,7 +43,8 @@ public:
     RRsetCollectionBase(ZoneUpdater& updater,
                         const isc::dns::RRClass& rrclass) :
         updater_(updater),
-        rrclass_(rrclass)
+        rrclass_(rrclass),
+        disabled_(false)
     {}
 
     /// \brief Destructor
@@ -55,8 +56,9 @@ public:
     /// 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.
+    /// \throw isc::dns::RRsetCollectionError if \c find() results in
+    /// some underlying datasrc error, or if \c disable() was called.
+    ///
     /// \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.
@@ -65,9 +67,42 @@ public:
                                          const isc::dns::RRClass& rrclass,
                                          const isc::dns::RRType& rrtype) const;
 
+protected:
+    /// \brief Disable the RRsetCollection.
+    ///
+    /// After calling this method, calling operations such as find() or
+    /// using the iterator would result in an \c
+    /// isc::dns::RRsetCollectionError. This method is typically called
+    /// in the \c commit() implementations of some \c ZoneUpdaters.
+    void disable() {
+        disabled_ = true;
+    }
+
+    /// \brief Return if the RRsetCollection is disabled.
+    bool isDisabled() const {
+        return (disabled_);
+    }
+
+    /// \brief See \c isc::dns::RRsetCollectionBase::getBeginning() for
+    /// documentation.
+    ///
+    /// \throw isc::dns::RRsetCollectionError if using the iterator
+    /// results in some underlying datasrc error, or if \c disable() was
+    /// called.
+    virtual IterPtr getBeginning() = 0;
+
+    /// \brief See \c isc::dns::RRsetCollectionBase::getEnd() for
+    /// documentation.
+    ///
+    /// \throw isc::dns::RRsetCollectionError if using the iterator
+    /// results in some underlying datasrc error, or if \c disable() was
+    /// called.
+    virtual IterPtr getEnd() = 0;
+
 private:
     ZoneUpdater& updater_;
     isc::dns::RRClass rrclass_;
+    bool disabled_;
 };
 
 /// \brief A pointer-like type pointing to an
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index 6665642..0ca5a02 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -4307,6 +4307,8 @@ public:
     ZoneUpdaterPtr updater_;
 };
 
+// Test that using addRRset() or deleteRRset() on the ZoneUpdater throws
+// after an RRsetCollection is created.
 TYPED_TEST(RRsetCollectionAndUpdaterTest, updateThrows) {
     // 1. Addition test
 
@@ -4341,4 +4343,22 @@ TYPED_TEST(RRsetCollectionAndUpdaterTest, updateThrows) {
                  isc::InvalidOperation);
 }
 
+// Test that using an RRsetCollection after calling commit() on the
+// ZoneUpdater throws, as the RRsetCollection is disabled.
+TYPED_TEST(RRsetCollectionAndUpdaterTest, useAfterCommitThrows) {
+     isc::datasrc::RRsetCollectionBase& collection =
+         this->updater_->getRRsetCollection();
+
+     // find() must not throw here.
+     collection.find(Name("foo.wild.example.org"), this->qclass_, RRType::A());
+
+     this->updater_->commit();
+
+     // find() must throw RRsetCollectionError here, as the
+     // RRsetCollection is disabled.
+     EXPECT_THROW(collection.find(Name("foo.wild.example.org"),
+                                  this->qclass_, RRType::A()),
+                  RRsetCollectionError);
+}
+
 }
diff --git a/src/lib/datasrc/zone.h b/src/lib/datasrc/zone.h
index aefa902..01d6a83 100644
--- a/src/lib/datasrc/zone.h
+++ b/src/lib/datasrc/zone.h
@@ -822,6 +822,11 @@ public:
     /// of the \c Zonefinder returned by \c getFinder().
     /// Implementations of \c ZoneUpdater may not allow adding or
     /// deleting RRsets after \c getRRsetCollection() is called.
+    /// Implementations of \c ZoneUpdater may disable a previously
+    /// returned \c RRsetCollection after \c commit() is called. If an
+    /// \c RRsetCollection is disabled, using methods such as \c find()
+    /// and using its iterator would cause an exception to be
+    /// thrown. See \c isc::datasrc::RRsetCollectionBase for details.
     virtual isc::datasrc::RRsetCollectionBase& getRRsetCollection() = 0;
 
     /// Add an RRset to a zone via the updater
diff --git a/src/lib/dns/rrset_collection_base.h b/src/lib/dns/rrset_collection_base.h
index b00435d..c2536da 100644
--- a/src/lib/dns/rrset_collection_base.h
+++ b/src/lib/dns/rrset_collection_base.h
@@ -112,10 +112,16 @@ protected:
 
     /// \brief Returns an \c IterPtr wrapping an Iter pointing to the
     /// beginning of the collection.
+    ///
+    /// \throw isc::dns::RRsetCollectionError if using the iterator
+    /// results in some underlying datasrc error.
     virtual IterPtr getBeginning() = 0;
 
     /// \brief Returns an \c IterPtr wrapping an Iter pointing past the
     /// end of the collection.
+    ///
+    /// \throw isc::dns::RRsetCollectionError if using the iterator
+    /// results in some underlying datasrc error.
     virtual IterPtr getEnd() = 0;
 
 public:



More information about the bind10-changes mailing list