[1287] use a cloned accessor for iterator. added various minor test cases.

src/lib/datasrc/database.cc

@@ -704,10 +704,11 @@ namespace {
  */
 class DatabaseIterator : public ZoneIterator {
 public:
-    DatabaseIterator(shared_ptr<DatabaseAccessor> /*accessor*/,
+    DatabaseIterator(shared_ptr<DatabaseAccessor> accessor,
                      ConstRRsetPtr soa,
                      const DatabaseAccessor::IteratorContextPtr& context,
                      const RRClass& rrclass) :
+        accessor_(accessor),
         context_(context),
         class_(rrclass),
         soa_(soa),
@@ -717,6 +718,12 @@ public:
         getData();
     }
 
+    virtual ~DatabaseIterator() {
+        if (ready_) {
+            accessor_->commit();
+        }
+    }
+
     virtual ConstRRsetPtr getSOA() const {
         return (soa_);
     }
@@ -727,14 +734,15 @@ public:
         }
         if (!data_ready_) {
             // At the end of zone
+            accessor_->commit();
             ready_ = false;
             LOG_DEBUG(logger, DBG_TRACE_DETAILED,
                       DATASRC_DATABASE_ITERATE_END);
             return (ConstRRsetPtr());
         }
-        string name_str(name_), rtype_str(rtype_), ttl(ttl_);
-        Name name(name_str);
-        RRType rtype(rtype_str);
+        const string name_str(name_), rtype_str(rtype_), ttl(ttl_);
+        const Name name(name_str);
+        const RRType rtype(rtype_str);
         RRsetPtr rrset(new RRset(name, class_, rtype, RRTTL(ttl)));
         while (data_ready_ && name_ == name_str && rtype_str == rtype_) {
             if (ttl_ != ttl) {
@@ -767,6 +775,8 @@ private:
         rdata_ = data[DatabaseAccessor::RDATA_COLUMN];
     }
 
+    // The dedicated accessor
+    shared_ptr<DatabaseAccessor> accessor_;
     // The context
     const DatabaseAccessor::IteratorContextPtr context_;
     // Class of the zone
@@ -783,8 +793,9 @@ private:
 
 ZoneIteratorPtr
 DatabaseClient::getIterator(const isc::dns::Name& name) const {
+    shared_ptr<DatabaseAccessor> iterator_accessor(accessor_->clone());
     // Get the zone
-    std::pair<bool, int> zone(accessor_->getZone(name.toText()));
+    std::pair<bool, int> zone(iterator_accessor->getZone(name.toText()));
     if (!zone.first) {
         // No such zone, can't continue
         isc_throw(DataSourceError, "Zone " + name.toText() +
@@ -792,16 +803,16 @@ DatabaseClient::getIterator(const isc::dns::Name& name) const {
                   "in this data source");
     }
 
-    accessor_->startTransaction();
+    iterator_accessor->startTransaction();
 
     // Find the SOA of the zone (may or may not succeed).  Note that
     // this must be done before starting the iteration context.
-    ConstRRsetPtr soa = DatabaseClient::Finder(accessor_, zone.second, name).
+    ConstRRsetPtr soa = DatabaseClient::Finder(iterator_accessor, zone.second, name).
         find(name, RRType::SOA(), NULL).rrset;
 
     // Request the context
     DatabaseAccessor::IteratorContextPtr
-        context(accessor_->getAllRecords(zone.second));
+        context(iterator_accessor->getAllRecords(zone.second));
     // It must not return NULL, that's a bug of the implementation
     if (context == DatabaseAccessor::IteratorContextPtr()) {
         isc_throw(isc::Unexpected, "Iterator context null at " +
@@ -814,7 +825,7 @@ DatabaseClient::getIterator(const isc::dns::Name& name) const {
     // it each time)
     LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ITERATE).
         arg(name);
-    return (ZoneIteratorPtr(new DatabaseIterator(accessor_, soa, context,
+    return (ZoneIteratorPtr(new DatabaseIterator(iterator_accessor, soa, context,
                                                  RRClass::IN())));
 }
 

src/lib/datasrc/tests/database_unittest.cc

@@ -220,7 +220,8 @@ public:
     }
 
     virtual shared_ptr<DatabaseAccessor> clone() {
-        return (shared_ptr<DatabaseAccessor>()); // bogus data, but unused
+        // This accessor is stateless, so we can simply return a new instance.
+        return (shared_ptr<DatabaseAccessor>(new NopAccessor));
     }
 
     virtual std::pair<bool, int> startUpdateZone(const std::string&, bool) {
@@ -298,7 +299,7 @@ public:
         // Currently we only use this transaction for simple read-only
         // operations.  So we just make a local copy of the data (we don't
         // care about what happens after commit() or rollback()).
-        readonly_records_copy_ = readonly_records_master_;
+        readonly_records_copy_ = *readonly_records_;
         readonly_records_ = &readonly_records_copy_;
     }
 
@@ -1077,7 +1078,18 @@ TYPED_TEST(DatabaseClientTest, getSOAFromIterator) {
     isc::testutils::rrsetCheck(it->getSOA(), rrset);
 }
 
-TYPED_TEST(DatabaseClientTest, getSOAThenUpdate) {
+TYPED_TEST(DatabaseClientTest, noSOAFromIterator) {
+    // First, empty the zone.
+    this->updater_ = this->client_->getUpdater(this->zname_, true);
+    this->updater_->commit();
+
+    // Then getSOA() should return NULL.
+    ZoneIteratorPtr it(this->client_->getIterator(this->zname_));
+    ASSERT_TRUE(it);
+    EXPECT_FALSE(it->getSOA());
+}
+
+TYPED_TEST(DatabaseClientTest, iterateThenUpdate) {
     ZoneIteratorPtr it(this->client_->getIterator(this->zname_));
     ASSERT_TRUE(it);
 
@@ -1088,6 +1100,10 @@ TYPED_TEST(DatabaseClientTest, getSOAThenUpdate) {
     try {
         this->updater_ = this->client_->getUpdater(this->zname_, true);
         this->updater_->commit();
+
+        // Confirm at least it doesn't contain any SOA
+        EXPECT_EQ(ZoneFinder::NXDOMAIN,
+                  this->getFinder()->find(this->zname_, RRType::SOA()).code);
     } catch (const DataSourceError&) {}
 
     ConstRRsetPtr rrset;
@@ -1100,6 +1116,36 @@ TYPED_TEST(DatabaseClientTest, getSOAThenUpdate) {
     isc::testutils::rrsetCheck(it->getSOA(), rrset);
 }
 
+TYPED_TEST(DatabaseClientTest, updateThenIterateThenUpdate) {
+    // First clear the zone.
+    this->updater_ = this->client_->getUpdater(this->zname_, true);
+    this->updater_->commit();
+
+    // Then iterate over it.  It should immediately reach the end, at which
+    // point the transaction should be committed.
+    ZoneIteratorPtr it(this->client_->getIterator(this->zname_));
+    ASSERT_TRUE(it);
+    EXPECT_FALSE(it->getNextRRset());
+
+    // So another update attempt should succeed, too.
+    this->updater_ = this->client_->getUpdater(this->zname_, true);
+    this->updater_->commit();
+}
+
+TYPED_TEST(DatabaseClientTest, updateAfterDelteIterator) {
+    // Similar to the previous case, but we delete the iterator in the
+    // middle of zone.  The transaction should be canceled (actually no
+    // different from commit though) at that point.
+    ZoneIteratorPtr it(this->client_->getIterator(this->zname_));
+    ASSERT_TRUE(it);
+    EXPECT_TRUE(it->getNextRRset());
+    it.reset();
+
+    // So another update attempt should succeed.
+    this->updater_ = this->client_->getUpdater(this->zname_, true);
+    this->updater_->commit();
+}
+
 void
 doFindTest(ZoneFinder& finder,
            const isc::dns::Name& name,