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

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();

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

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

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);
+}
+
 }

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

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: