[master] Merge branch 'trac1287'

src/lib/datasrc/database.cc

@@ -704,30 +704,67 @@ namespace {
  */
 class DatabaseIterator : public ZoneIterator {
 public:
-    DatabaseIterator(const DatabaseAccessor::IteratorContextPtr& context,
-             const RRClass& rrclass) :
-        context_(context),
+    DatabaseIterator(shared_ptr<DatabaseAccessor> accessor,
+                     const Name& zone_name,
+                     const RRClass& rrclass) :
+        accessor_(accessor),
         class_(rrclass),
         ready_(true)
     {
+        // Get the zone
+        const pair<bool, int> zone(accessor_->getZone(zone_name.toText()));
+        if (!zone.first) {
+            // No such zone, can't continue
+            isc_throw(DataSourceError, "Zone " + zone_name.toText() +
+                      " can not be iterated, because it doesn't exist "
+                      "in this data source");
+        }
+
+        // Start a separate transaction.
+        accessor_->startTransaction();
+
+        // Find the SOA of the zone (may or may not succeed).  Note that
+        // this must be done before starting the iteration context.
+        soa_ = DatabaseClient::Finder(accessor_, zone.second, zone_name).
+            find(zone_name, RRType::SOA(), NULL).rrset;
+
+        // Request the context
+        context_ = accessor_->getAllRecords(zone.second);
+        // It must not return NULL, that's a bug of the implementation
+        if (!context_) {
+            isc_throw(isc::Unexpected, "Iterator context null at " +
+                      zone_name.toText());
+        }
+
         // Prepare data for the next time
         getData();
     }
 
+    virtual ~DatabaseIterator() {
+        if (ready_) {
+            accessor_->commit();
+        }
+    }
+
+    virtual ConstRRsetPtr getSOA() const {
+        return (soa_);
+    }
+
     virtual isc::dns::ConstRRsetPtr getNextRRset() {
         if (!ready_) {
             isc_throw(isc::Unexpected, "Iterating past the zone end");
         }
         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) {
@@ -745,6 +782,7 @@ public:
             arg(rrset->getName()).arg(rrset->getType());
         return (rrset);
     }
+
 private:
     // Load next row of data
     void getData() {
@@ -756,10 +794,14 @@ private:
         rdata_ = data[DatabaseAccessor::RDATA_COLUMN];
     }
 
+    // The dedicated accessor
+    shared_ptr<DatabaseAccessor> accessor_;
     // The context
-    const DatabaseAccessor::IteratorContextPtr context_;
+    DatabaseAccessor::IteratorContextPtr context_;
     // Class of the zone
-    RRClass class_;
+    const RRClass class_;
+    // SOA of the zone, if any (it should normally exist)
+    ConstRRsetPtr soa_;
     // Status
     bool ready_, data_ready_;
     // Data of the next row
@@ -770,30 +812,13 @@ private:
 
 ZoneIteratorPtr
 DatabaseClient::getIterator(const isc::dns::Name& name) const {
-    // Get the zone
-    std::pair<bool, int> zone(accessor_->getZone(name.toText()));
-    if (!zone.first) {
-        // No such zone, can't continue
-        isc_throw(DataSourceError, "Zone " + name.toText() +
-                  " can not be iterated, because it doesn't exist "
-                  "in this data source");
-    }
-    // Request the context
-    DatabaseAccessor::IteratorContextPtr
-        context(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 " +
-                  name.toText());
-    }
-    // Create the iterator and return it
-    // TODO: Once #1062 is merged with this, we need to get the
-    // actual zone class from the connection, as the DatabaseClient
-    // doesn't know it and the iterator needs it (so it wouldn't query
-    // it each time)
+    ZoneIteratorPtr iterator = ZoneIteratorPtr(new DatabaseIterator(
+                                                   accessor_->clone(), name,
+                                                   rrclass_));
     LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ITERATE).
         arg(name);
-    return (ZoneIteratorPtr(new DatabaseIterator(context, RRClass::IN())));
+
+    return (iterator);
 }
 
 //
@@ -815,13 +840,13 @@ public:
     virtual ~DatabaseUpdater() {
         if (!committed_) {
             try {
-                accessor_->rollbackUpdateZone();
+                accessor_->rollback();
                 logger.info(DATASRC_DATABASE_UPDATER_ROLLBACK)
                     .arg(zone_name_).arg(zone_class_).arg(db_name_);
             } catch (const DataSourceError& e) {
                 // We generally expect that rollback always succeeds, and
                 // it should in fact succeed in a way we execute it.  But
-                // as the public API allows rollbackUpdateZone() to fail and
+                // as the public API allows rollback() to fail and
                 // throw, we should expect it.  Obviously we cannot re-throw
                 // it.  The best we can do is to log it as a critical error.
                 logger.error(DATASRC_DATABASE_UPDATER_ROLLBACKFAIL)
@@ -937,7 +962,7 @@ DatabaseUpdater::commit() {
                   << zone_name_ << "/" << zone_class_ << " on "
                   << db_name_);
     }
-    accessor_->commitUpdateZone();
+    accessor_->commit();
     committed_ = true; // make sure the destructor won't trigger rollback
 
     // We release the accessor immediately after commit is completed so that

src/lib/datasrc/database.h

@@ -260,13 +260,14 @@ public:
     /// \c commitUpdateZone()); if it's false, the existing records will be
     /// intact unless explicitly deleted by \c deleteRecordInZone().
     ///
-    /// A single \c DatabaseAccessor instance can perform at most one update
+    /// A single \c DatabaseAccessor instance can perform at most one
     /// transaction; a duplicate call to this method before
-    /// \c commitUpdateZone() or \c rollbackUpdateZone() will result in
-    /// a \c DataSourceError exception.  If multiple update attempts need
-    /// to be performed concurrently (and if the underlying database allows
-    /// such operation), separate \c DatabaseAccessor instance must be
-    /// created.
+    /// \c commitUpdateZone() or \c rollbackUpdateZone(), or a call to this
+    /// method within another transaction started by \c startTransaction()
+    /// will result in a \c DataSourceError exception.
+    /// If multiple update attempts need to be performed concurrently (and
+    /// if the underlying database allows such operation), separate
+    /// \c DatabaseAccessor instance must be created.
     ///
     /// \note The underlying database may not allow concurrent updates to
     /// the same database instance even if different "connections" (or
@@ -295,8 +296,9 @@ public:
     /// \c getZone(); for example, a specific implementation may use a
     /// completely new zone ID when \c replace is true.
     ///
-    /// \exception DataSourceError Duplicate call to this method, or some
-    /// internal database related error.
+    /// \exception DataSourceError Duplicate call to this method, call to
+    /// this method within another transaction, or some internal database
+    /// related error.
     ///
     /// \param zone_name A string representation of the zone name to be updated
     /// \param replace Whether to replace the entire zone (see above)
@@ -382,12 +384,32 @@ public:
     virtual void deleteRecordInZone(
         const std::string (&params)[DEL_PARAM_COUNT]) = 0;
 
-    /// Commit updates to the zone.
+    /// Start a general transaction.
     ///
-    /// This method completes a transaction of making updates to the zone
-    /// in the context started by startUpdateZone.
+    /// Each derived class version of this method starts a database
+    /// transaction in a way specific to the database details.  Any subsequent
+    /// operations on the accessor are guaranteed to be not susceptible to
+    /// any update attempts made during the transaction.  The transaction
+    /// must be terminated by either \c commit() or \c rollback().
     ///
-    /// A successful call to \c startUpdateZone() must have preceded to
+    /// In practice, this transaction is intended to be used to perform
+    /// a set of atomic reads and work as a read-only lock.  So, in many
+    /// cases \c commit() and \c rollback() will have the same effect.
+    ///
+    /// This transaction cannot coexist with an update transaction started
+    /// by \c startUpdateZone().  Such an attempt will result in
+    /// \c DataSourceError.
+    ///
+    /// \exception DataSourceError An attempt of nested transaction, or some
+    /// internal database related error.
+    virtual void startTransaction() = 0;
+
+    /// Commit a transaction.
+    ///
+    /// This method completes a transaction started by \c startTransaction
+    /// or \c startUpdateZone.
+    ///
+    /// A successful call to one of the "start" methods must have preceded to
     /// this call; otherwise a \c DataSourceError exception will be thrown.
     /// Once this method successfully completes, the transaction isn't
     /// considered to exist any more.  So a new transaction can now be
@@ -403,17 +425,16 @@ public:
     ///
     /// \exception DataSourceError Call without a transaction, duplicate call
     /// to the method or internal database error.
-    virtual void commitUpdateZone() = 0;
+    virtual void commit() = 0;
 
-    /// Rollback updates to the zone made so far.
+    /// Rollback any changes in a transaction made so far.
     ///
-    /// This method rollbacks a transaction of making updates to the zone
-    /// in the context started by startUpdateZone.  When it succeeds
-    /// (it normally should, but see below), the underlying database should
-    /// be reverted to the point before performing the corresponding
-    /// \c startUpdateZone().
+    /// This method rollbacks a transaction started by \c startTransaction or
+    /// \c startUpdateZone.  When it succeeds (it normally should, but see
+    /// below), the underlying database should be reverted to the point
+    /// before performing the corresponding "start" method.
     ///
-    /// A successful call to \c startUpdateZone() must have preceded to
+    /// A successful call to one of the "start" method must have preceded to
     /// this call; otherwise a \c DataSourceError exception will be thrown.
     /// Once this method successfully completes, the transaction isn't
     /// considered to exist any more.  So a new transaction can now be
@@ -430,7 +451,7 @@ public:
     ///
     /// \exception DataSourceError Call without a transaction, duplicate call
     /// to the method or internal database error.
-    virtual void rollbackUpdateZone() = 0;
+    virtual void rollback() = 0;
 
     /// Clone the accessor with the same configuration.
     ///

src/lib/datasrc/iterator.h

@@ -12,10 +12,15 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#ifndef __DATASRC_ZONE_ITERATOR_H
+#define __DATASRC_ZONE_ITERATOR_H 1
+
 #include <dns/rrset.h>
 
 #include <boost/noncopyable.hpp>
 
+#include <datasrc/zone.h>
+
 namespace isc {
 namespace datasrc {
 
@@ -55,7 +60,46 @@ public:
      *     gets to the end of the zone.
      */
     virtual isc::dns::ConstRRsetPtr getNextRRset() = 0;
+
+    /**
+     * \brief Return the SOA record of the zone in the iterator context.
+     *
+     * This method returns the zone's SOA record (if any, and a valid zone
+     * should have it) in the form of an RRset object.  This SOA is identical
+     * to that (again, if any) contained in the sequence of RRsets returned
+     * by the iterator.  In that sense this method is redundant, but is
+     * provided as a convenient utility for the application of the
+     * iterator; the application may need to know the SOA serial or the
+     * SOA RR itself for the purpose of protocol handling or skipping the
+     * expensive iteration processing.
+     *
+     * If the zone doesn't have an SOA (which is broken, but some data source
+     * may allow that situation), this method returns NULL.  Also, in the
+     * normal and valid case, the SOA should have exactly one RDATA, but
+     * this API does not guarantee it as some data source may accept such an
+     * abnormal condition.  It's up to the caller whether to check the number
+     * of RDATA and how to react to the unexpected case.
+     *
+     * Each concrete derived method must ensure that the SOA returned by this
+     * method is identical to the zone's SOA returned via the iteration.
+     * For example, even if another thread or process updates the SOA while
+     * the iterator is working, the result of this method must not be
+     * affected by the update.  For database based data sources, this can
+     * be done by making the entire iterator operation as a single database
+     * transaction, but the actual implementation can differ.
+     *
+     * \exception None
+     *
+     * \return A shared pointer to an SOA RRset that would be returned
+     * from the iteration.  It will be NULL if the zone doesn't have an SOA.
+     */
+    virtual isc::dns::ConstRRsetPtr getSOA() const = 0;
 };
 
 }
 }
+#endif  // __DATASRC_ZONE_ITERATOR_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/datasrc/memory_datasrc.cc

@@ -780,6 +780,10 @@ public:
 
         return (result);
     }
+
+    virtual ConstRRsetPtr getSOA() const {
+        isc_throw(NotImplemented, "Not imelemented");
+    }
 };
 
 } // End of anonymous namespace

src/lib/datasrc/sqlite3_accessor.cc

@@ -73,7 +73,7 @@ const char* const text_statements[NUM_STATEMENTS] = {
     "DELETE FROM records WHERE zone_id=?1 AND name=?2 " // DEL_RECORD
     "AND rdtype=?3 AND rdata=?4",
     "SELECT rdtype, ttl, sigtype, rdata, name FROM records " // ITERATE
-    "WHERE zone_id = ?1 ORDER BY name, rdtype",
+    "WHERE zone_id = ?1 ORDER BY rname, rdtype",
     /*
      * This one looks for previous name with NSEC record. It is done by
      * using the reversed name. The NSEC is checked because we need to
@@ -86,7 +86,8 @@ const char* const text_statements[NUM_STATEMENTS] = {
 
 struct SQLite3Parameters {
     SQLite3Parameters() :
-        db_(NULL), version_(-1), updating_zone(false), updated_zone_id(-1)
+        db_(NULL), version_(-1), in_transaction(false), updating_zone(false),
+        updated_zone_id(-1)
     {
         for (int i = 0; i < NUM_STATEMENTS; ++i) {
             statements_[i] = NULL;
@@ -96,8 +97,9 @@ struct SQLite3Parameters {
     sqlite3* db_;
     int version_;
     sqlite3_stmt* statements_[NUM_STATEMENTS];
-    bool updating_zone;         // whether or not updating the zone
-    int updated_zone_id;        // valid only when updating_zone is true
+    bool in_transaction; // whether or not a transaction has been started
+    bool updating_zone;          // whether or not updating the zone
+    int updated_zone_id;        // valid only when in_transaction is true
 };
 
 // This is a helper class to encapsulate the code logic of executing
@@ -543,6 +545,10 @@ SQLite3Accessor::startUpdateZone(const string& zone_name, const bool replace) {
         isc_throw(DataSourceError,
                   "duplicate zone update on SQLite3 data source");
     }
+    if (dbparameters_->in_transaction) {
+        isc_throw(DataSourceError,
+                  "zone update attempt in another SQLite3 transaction");
+    }
 
     const pair<bool, int> zone_info(getZone(zone_name));
     if (!zone_info.first) {
@@ -550,7 +556,7 @@ SQLite3Accessor::startUpdateZone(const string& zone_name, const bool replace) {
     }
 
     StatementProcessor(*dbparameters_, BEGIN,
-                       "start an SQLite3 transaction").exec();
+                       "start an SQLite3 update transaction").exec();
 
     if (replace) {
         try {
@@ -577,6 +583,7 @@ SQLite3Accessor::startUpdateZone(const string& zone_name, const bool replace) {
         }
     }
 
+    dbparameters_->in_transaction = true;
     dbparameters_->updating_zone = true;
     dbparameters_->updated_zone_id = zone_info.second;
 
@@ -584,28 +591,40 @@ SQLite3Accessor::startUpdateZone(const string& zone_name, const bool replace) {
 }
 
 void
-SQLite3Accessor::commitUpdateZone() {
-    if (!dbparameters_->updating_zone) {
-        isc_throw(DataSourceError, "committing zone update on SQLite3 "
+SQLite3Accessor::startTransaction() {
+    if (dbparameters_->in_transaction) {
+        isc_throw(DataSourceError,
+                  "duplicate transaction on SQLite3 data source");
+    }
+
+    StatementProcessor(*dbparameters_, BEGIN,
+                       "start an SQLite3 transaction").exec();
+    dbparameters_->in_transaction = true;
+}
+
+void
+SQLite3Accessor::commit() {
+    if (!dbparameters_->in_transaction) {
+        isc_throw(DataSourceError, "performing commit on SQLite3 "
                   "data source without transaction");
     }
 
     StatementProcessor(*dbparameters_, COMMIT,
                        "commit an SQLite3 transaction").exec();
-    dbparameters_->updating_zone = false;
+    dbparameters_->in_transaction = false;
     dbparameters_->updated_zone_id = -1;
 }
 
 void
-SQLite3Accessor::rollbackUpdateZone() {
-    if (!dbparameters_->updating_zone) {
-        isc_throw(DataSourceError, "rolling back zone update on SQLite3 "
+SQLite3Accessor::rollback() {
+    if (!dbparameters_->in_transaction) {
+        isc_throw(DataSourceError, "performing rollback on SQLite3 "
                   "data source without transaction");
     }
 
     StatementProcessor(*dbparameters_, ROLLBACK,
                        "rollback an SQLite3 transaction").exec();
-    dbparameters_->updating_zone = false;
+    dbparameters_->in_transaction = false;
     dbparameters_->updated_zone_id = -1;
 }
 

src/lib/datasrc/sqlite3_accessor.h

@@ -131,6 +131,8 @@ public:
     virtual std::pair<bool, int> startUpdateZone(const std::string& zone_name,
                                                  bool replace);
 
+    virtual void startTransaction();
+
     /// \note we are quite impatient here: it's quite possible that the COMMIT
     /// fails due to other process performing SELECT on the same database
     /// (consider the case where COMMIT is done by xfrin or dynamic update
@@ -139,7 +141,7 @@ public:
     /// attempt and/or increase timeout before giving up the COMMIT, even
     /// if it still doesn't guarantee 100% success.  Right now this
     /// implementation throws a \c DataSourceError exception in such a case.
-    virtual void commitUpdateZone();
+    virtual void commit();
 
     /// \note In SQLite3 rollback can fail if there's another unfinished
     /// statement is performed for the same database structure.
@@ -147,7 +149,7 @@ public:
     /// guaranteed to be prevented at the API level.  If it ever happens, this
     /// method throws a \c DataSourceError exception.  It should be
     /// considered a bug of the higher level application program.
-    virtual void rollbackUpdateZone();
+    virtual void rollback();
 
     virtual void addRecordToZone(
         const std::string (&columns)[ADD_COLUMN_COUNT]);

src/lib/datasrc/tests/database_unittest.cc

@@ -154,9 +154,13 @@ const char* const TEST_RECORDS[][5] = {
 
     // Put some data into apex (including NS) so we can check our NS
     // doesn't break anything
+    {"example.org.", "SOA", "3600", "", "ns1.example.org. admin.example.org. "
+     "1234 3600 1800 2419200 7200" },
     {"example.org.", "NS", "3600", "", "ns.example.com."},
     {"example.org.", "A", "3600", "", "192.0.2.1"},
     {"example.org.", "NSEC", "3600", "", "acnamesig1.example.org. NS A NSEC RRSIG"},
+    {"example.org.", "RRSIG", "3600", "", "SOA 5 3 3600 20000101000000 "
+              "20000201000000 12345 example.org. FAKEFAKEFAKE"},
     {"example.org.", "RRSIG", "3600", "", "NSEC 5 3 3600 20000101000000 "
               "20000201000000 12345 example.org. FAKEFAKEFAKE"},
     {"example.org.", "RRSIG", "3600", "", "NS 5 3 3600 20000101000000 "
@@ -216,15 +220,17 @@ 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) {
         // return dummy value.  unused anyway.
         return (pair<bool, int>(true, 0));
     }
-    virtual void commitUpdateZone() {}
-    virtual void rollbackUpdateZone() {}
+    virtual void startTransaction() {}
+    virtual void commit() {}
+    virtual void rollback() {}
     virtual void addRecordToZone(const string (&)[ADD_COLUMN_COUNT]) {}
     virtual void deleteRecordInZone(const string (&)[DEL_PARAM_COUNT]) {}
 
@@ -273,7 +279,7 @@ class MockAccessor : public NopAccessor {
                      NameCompare > Domains;
 
 public:
-    MockAccessor() : rollbacked_(false) {
+    MockAccessor() : rollbacked_(false), did_transaction_(false) {
         readonly_records_ = &readonly_records_master_;
         update_records_ = &update_records_master_;
         empty_records_ = &empty_records_master_;
@@ -289,6 +295,24 @@ public:
         return (cloned_accessor);
     }
 
+    virtual void startTransaction() {
+        // 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()).
+        // Obviously as a consequence, if a test case tries to make multiple
+        // transactions on a single mock accessor it will fail.
+
+        // Check any attempt of multiple transactions
+        if (did_transaction_) {
+            isc_throw(isc::Unexpected, "MockAccessor::startTransaction() "
+                      "called multiple times - likely a bug in the test");
+        }
+
+        readonly_records_copy_ = *readonly_records_;
+        readonly_records_ = &readonly_records_copy_;
+        did_transaction_ = true;
+    }
+
 private:
     class MockNameIteratorContext : public IteratorContext {
     public:
@@ -360,38 +384,52 @@ private:
     class MockIteratorContext : public IteratorContext {
     private:
         int step;
+        const Domains& domains_;
     public:
-        MockIteratorContext() :
-            step(0)
+        MockIteratorContext(const Domains& domains) :
+            step(0), domains_(domains)
         { }
         virtual bool getNext(string (&data)[COLUMN_COUNT]) {
+            // A special case: if the given set of domains is already empty,
+            // we always return false.
+            if (domains_.empty()) {
+                return (false);
+            }
+
+            // Return faked data for tests
             switch (step ++) {
                 case 0:
                     data[DatabaseAccessor::NAME_COLUMN] = "example.org";
+                    data[DatabaseAccessor::TYPE_COLUMN] = "A";
+                    data[DatabaseAccessor::TTL_COLUMN] = "3600";
+                    data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.1";
+                    return (true);
+                case 1:
+                    data[DatabaseAccessor::NAME_COLUMN] = "example.org";
                     data[DatabaseAccessor::TYPE_COLUMN] = "SOA";
-                    data[DatabaseAccessor::TTL_COLUMN] = "300";
+                    data[DatabaseAccessor::TTL_COLUMN] = "3600";
                     data[DatabaseAccessor::RDATA_COLUMN] = "ns1.example.org. admin.example.org. "
                         "1234 3600 1800 2419200 7200";
                     return (true);
-                case 1:
+                case 2:
                     data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
                     data[DatabaseAccessor::TYPE_COLUMN] = "A";
                     data[DatabaseAccessor::TTL_COLUMN] = "300";
                     data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.1";
                     return (true);
-                case 2:
+                case 3:
                     data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
                     data[DatabaseAccessor::TYPE_COLUMN] = "A";
                     data[DatabaseAccessor::TTL_COLUMN] = "300";
                     data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.2";
                     return (true);
-                case 3:
+                case 4:
                     data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
                     data[DatabaseAccessor::TYPE_COLUMN] = "AAAA";
                     data[DatabaseAccessor::TTL_COLUMN] = "300";
                     data[DatabaseAccessor::RDATA_COLUMN] = "2001:db8::1";
                     return (true);
-                case 4:
+                case 5:
                     data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
                     data[DatabaseAccessor::TYPE_COLUMN] = "AAAA";
                     data[DatabaseAccessor::TTL_COLUMN] = "300";
@@ -400,7 +438,7 @@ private:
                 default:
                     ADD_FAILURE() <<
                         "Request past the end of iterator context";
-                case 5:
+                case 6:
                     return (false);
             }
         }
@@ -443,7 +481,8 @@ private:
 public:
     virtual IteratorContextPtr getAllRecords(int id) const {
         if (id == READONLY_ZONE_ID) {
-            return (IteratorContextPtr(new MockIteratorContext()));
+            return (IteratorContextPtr(new MockIteratorContext(
+                                           *readonly_records_)));
         } else if (id == 13) {
             return (IteratorContextPtr());
         } else if (id == 0) {
@@ -463,7 +502,11 @@ public:
                         new MockNameIteratorContext(*this, id, name,
                                                     subdomains)));
         } else {
-            isc_throw(isc::Unexpected, "Unknown zone ID");
+            // This iterator is bogus, but for the cases tested below that's
+            // sufficient.
+            return (IteratorContextPtr(
+                        new MockNameIteratorContext(*this, READONLY_ZONE_ID,
+                                                    name, subdomains)));
         }
     }
 
@@ -486,10 +529,10 @@ public:
 
         return (pair<bool, int>(true, WRITABLE_ZONE_ID));
     }
-    virtual void commitUpdateZone() {
+    virtual void commit() {
         *readonly_records_ = *update_records_;
     }
-    virtual void rollbackUpdateZone() {
+    virtual void rollback() {
         // Special hook: if something with a name of "throw.example.org"
         // has been added, trigger an imaginary unexpected event with an
         // exception.
@@ -603,20 +646,20 @@ private:
     // The following member variables are storage and/or update work space
     // of the test zone.  The "master"s are the real objects that contain
     // the data, and they are shared among all accessors cloned from
-    // an initially created one.  The pointer members allow the sharing.
+    // an initially created one.  The "copy" data will be used for read-only
+    // transaction.  The pointer members allow the sharing.
     // "readonly" is for normal lookups.  "update" is the workspace for
     // updates.  When update starts it will be initialized either as an
     // empty set (when replacing the entire zone) or as a copy of the
     // "readonly" one.  "empty" is a sentinel to produce negative results.
     Domains readonly_records_master_;
+    Domains readonly_records_copy_;
     Domains* readonly_records_;
     Domains update_records_master_;
     Domains* update_records_;
     const Domains empty_records_master_;
     const Domains* empty_records_;
 
-    // used as temporary storage during the building of the fake data
-
     // used as temporary storage after searchForRecord() and during
     // getNextRecord() calls, as well as during the building of the
     // fake data
@@ -632,6 +675,9 @@ private:
     // Remember the mock accessor that was last cloned
     boost::shared_ptr<MockAccessor> latest_clone_;
 
+    // Internal flag for duplicate check
+    bool did_transaction_;
+
     const Domains& getMockRecords(int zone_id) const {
         if (zone_id == READONLY_ZONE_ID) {
             return (*readonly_records_);
@@ -860,7 +906,7 @@ public:
 
             addRecordToZone(columns);
         }
-        commitUpdateZone();
+        commit();
     }
 };
 
@@ -951,56 +997,64 @@ TEST_F(MockDatabaseClientTest, emptyIterator) {
     EXPECT_THROW(it->getNextRRset(), isc::Unexpected);
 }
 
+// checks if the given rrset matches the
+// given name, class, type and rdatas
+void
+checkRRset(isc::dns::ConstRRsetPtr rrset,
+           const isc::dns::Name& name,
+           const isc::dns::RRClass& rrclass,
+           const isc::dns::RRType& rrtype,
+           const isc::dns::RRTTL& rrttl,
+           const std::vector<std::string>& rdatas) {
+    isc::dns::RRsetPtr expected_rrset(
+        new isc::dns::RRset(name, rrclass, rrtype, rrttl));
+    for (unsigned int i = 0; i < rdatas.size(); ++i) {
+        expected_rrset->addRdata(
+            isc::dns::rdata::createRdata(rrtype, rrclass,
+                                         rdatas[i]));
+    }
+    isc::testutils::rrsetCheck(expected_rrset, rrset);
+}
+
 // Iterate through a zone
 TYPED_TEST(DatabaseClientTest, iterator) {
     ZoneIteratorPtr it(this->client_->getIterator(Name("example.org")));
     ConstRRsetPtr rrset(it->getNextRRset());
     ASSERT_NE(ConstRRsetPtr(), rrset);
 
+    // The first name should be the zone origin.
+    EXPECT_EQ(this->zname_, rrset->getName());
+
     // The rest of the checks work only for the mock accessor.
     if (!this->is_mock_) {
         return;
     }
 
-    EXPECT_EQ(Name("example.org"), rrset->getName());
-    EXPECT_EQ(RRClass::IN(), rrset->getClass());
-    EXPECT_EQ(RRType::SOA(), rrset->getType());
-    EXPECT_EQ(RRTTL(300), rrset->getTTL());
-    RdataIteratorPtr rit(rrset->getRdataIterator());
-    ASSERT_FALSE(rit->isLast());
-    rit->next();
-    EXPECT_TRUE(rit->isLast());
+    this->expected_rdatas_.clear();
+    this->expected_rdatas_.push_back("192.0.2.1");
+    checkRRset(rrset, Name("example.org"), this->qclass_, RRType::A(),
+               this->rrttl_, this->expected_rdatas_);
 
     rrset = it->getNextRRset();
-    ASSERT_NE(ConstRRsetPtr(), rrset);
-    EXPECT_EQ(Name("x.example.org"), rrset->getName());
-    EXPECT_EQ(RRClass::IN(), rrset->getClass());
-    EXPECT_EQ(RRType::A(), rrset->getType());
-    EXPECT_EQ(RRTTL(300), rrset->getTTL());
-    rit = rrset->getRdataIterator();
-    ASSERT_FALSE(rit->isLast());
-    EXPECT_EQ("192.0.2.1", rit->getCurrent().toText());
-    rit->next();
-    ASSERT_FALSE(rit->isLast());
-    EXPECT_EQ("192.0.2.2", rit->getCurrent().toText());
-    rit->next();
-    EXPECT_TRUE(rit->isLast());
+    this->expected_rdatas_.clear();
+    this->expected_rdatas_.push_back("ns1.example.org. admin.example.org. "
+                                     "1234 3600 1800 2419200 7200");
+    checkRRset(rrset, Name("example.org"), this->qclass_, RRType::SOA(),
+               this->rrttl_, this->expected_rdatas_);
 
     rrset = it->getNextRRset();
-    ASSERT_NE(ConstRRsetPtr(), rrset);
-    EXPECT_EQ(Name("x.example.org"), rrset->getName());
-    EXPECT_EQ(RRClass::IN(), rrset->getClass());
-    EXPECT_EQ(RRType::AAAA(), rrset->getType());
-    EXPECT_EQ(RRTTL(300), rrset->getTTL());
-    EXPECT_EQ(ConstRRsetPtr(), it->getNextRRset());
-    rit = rrset->getRdataIterator();
-    ASSERT_FALSE(rit->isLast());
-    EXPECT_EQ("2001:db8::1", rit->getCurrent().toText());
-    rit->next();
-    ASSERT_FALSE(rit->isLast());
-    EXPECT_EQ("2001:db8::2", rit->getCurrent().toText());
-    rit->next();
-    EXPECT_TRUE(rit->isLast());
+    this->expected_rdatas_.clear();
+    this->expected_rdatas_.push_back("192.0.2.1");
+    this->expected_rdatas_.push_back("192.0.2.2");
+    checkRRset(rrset, Name("x.example.org"), this->qclass_, RRType::A(),
+               RRTTL(300), this->expected_rdatas_);
+
+    rrset = it->getNextRRset();
+    this->expected_rdatas_.clear();
+    this->expected_rdatas_.push_back("2001:db8::1");
+    this->expected_rdatas_.push_back("2001:db8::2");
+    checkRRset(rrset, Name("x.example.org"), this->qclass_, RRType::AAAA(),
+               RRTTL(300), this->expected_rdatas_);
 }
 
 // This has inconsistent TTL in the set (the rest, like nonsense in
@@ -1011,23 +1065,96 @@ TEST_F(MockDatabaseClientTest, badIterator) {
     EXPECT_EQ(it->getNextRRset()->getTTL(), isc::dns::RRTTL(300));
 }
 
-// checks if the given rrset matches the
-// given name, class, type and rdatas
-void
-checkRRset(isc::dns::ConstRRsetPtr rrset,
-           const isc::dns::Name& name,
-           const isc::dns::RRClass& rrclass,
-           const isc::dns::RRType& rrtype,
-           const isc::dns::RRTTL& rrttl,
-           const std::vector<std::string>& rdatas) {
-    isc::dns::RRsetPtr expected_rrset(
-        new isc::dns::RRset(name, rrclass, rrtype, rrttl));
-    for (unsigned int i = 0; i < rdatas.size(); ++i) {
-        expected_rrset->addRdata(
-            isc::dns::rdata::createRdata(rrtype, rrclass,
-                                         rdatas[i]));
+TYPED_TEST(DatabaseClientTest, getSOAFromIterator) {
+    vector<string> soa_data;
+    soa_data.push_back("ns1.example.org. admin.example.org. "
+                       "1234 3600 1800 2419200 7200");
+
+    ZoneIteratorPtr it(this->client_->getIterator(this->zname_));
+    ASSERT_TRUE(it);
+    checkRRset(it->getSOA(), this->zname_, this->qclass_, RRType::SOA(),
+               this->rrttl_, soa_data);
+
+    // Iterate over the zone until we find an SOA.  Although there's a broken
+    // RDATA that would trigger an exception in getNextRRset(), we should
+    // reach the SOA as the sequence should be sorted and the SOA is at
+    // the origin name (which has no bogus data).
+    ConstRRsetPtr rrset;
+    while ((rrset = it->getNextRRset()) != ConstRRsetPtr() &&
+           rrset->getType() != RRType::SOA()) {
+        ;
     }
-    isc::testutils::rrsetCheck(expected_rrset, rrset);
+    ASSERT_TRUE(rrset);
+    // It should be identical to the result of getSOA().
+    isc::testutils::rrsetCheck(it->getSOA(), rrset);
+}
+
+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);
+
+    // Try to empty the zone after getting the iterator.  Depending on the
+    // underlying data source, it may result in an exception due to the
+    // transaction for the iterator.  In either case the integrity of the
+    // iterator result should be reserved.
+    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;
+    while ((rrset = it->getNextRRset()) != ConstRRsetPtr() &&
+           rrset->getType() != RRType::SOA()) {
+        ;
+    }
+    ASSERT_TRUE(rrset);
+    // It should be identical to the result of getSOA().
+    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, updateAfterDeleteIterator) {
+    // 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

src/lib/datasrc/tests/sqlite3_accessor_unittest.cc

@@ -130,18 +130,6 @@ TEST_F(SQLite3AccessorTest, iterator) {
     std::string data[DatabaseAccessor::COLUMN_COUNT];
     // Get and check the first and only record
     EXPECT_TRUE(context->getNext(data));
-    EXPECT_EQ("DNAME", data[DatabaseAccessor::TYPE_COLUMN]);
-    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
-    EXPECT_EQ("dname.example.info.", data[DatabaseAccessor::RDATA_COLUMN]);
-    EXPECT_EQ("dname.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
-
-    EXPECT_TRUE(context->getNext(data));
-    EXPECT_EQ("DNAME", data[DatabaseAccessor::TYPE_COLUMN]);
-    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
-    EXPECT_EQ("dname2.example.info.", data[DatabaseAccessor::RDATA_COLUMN]);
-    EXPECT_EQ("dname2.foo.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
-
-    EXPECT_TRUE(context->getNext(data));
     EXPECT_EQ("MX", data[DatabaseAccessor::TYPE_COLUMN]);
     EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
     EXPECT_EQ("10 mail.example.org.", data[DatabaseAccessor::RDATA_COLUMN]);
@@ -174,16 +162,22 @@ TEST_F(SQLite3AccessorTest, iterator) {
     EXPECT_EQ("example.org.", data[DatabaseAccessor::NAME_COLUMN]);
 
     EXPECT_TRUE(context->getNext(data));
-    EXPECT_EQ("A", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("DNAME", data[DatabaseAccessor::TYPE_COLUMN]);
     EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
-    EXPECT_EQ("192.0.2.10", data[DatabaseAccessor::RDATA_COLUMN]);
-    EXPECT_EQ("mail.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+    EXPECT_EQ("dname.example.info.", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("dname.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+
+    EXPECT_TRUE(context->getNext(data));
+    EXPECT_EQ("DNAME", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("dname2.example.info.", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("dname2.foo.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
 
     EXPECT_TRUE(context->getNext(data));
     EXPECT_EQ("A", data[DatabaseAccessor::TYPE_COLUMN]);
     EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
-    EXPECT_EQ("192.0.2.101", data[DatabaseAccessor::RDATA_COLUMN]);
-    EXPECT_EQ("ns.sub.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+    EXPECT_EQ("192.0.2.10", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("mail.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
 
     EXPECT_TRUE(context->getNext(data));
     EXPECT_EQ("NS", data[DatabaseAccessor::TYPE_COLUMN]);
@@ -194,6 +188,12 @@ TEST_F(SQLite3AccessorTest, iterator) {
     EXPECT_TRUE(context->getNext(data));
     EXPECT_EQ("A", data[DatabaseAccessor::TYPE_COLUMN]);
     EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("192.0.2.101", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("ns.sub.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+
+    EXPECT_TRUE(context->getNext(data));
+    EXPECT_EQ("A", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
     EXPECT_EQ("192.0.2.1", data[DatabaseAccessor::RDATA_COLUMN]);
     EXPECT_EQ("www.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
 
@@ -550,7 +550,7 @@ TEST_F(SQLite3Update, emptyUpdate) {
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
     zone_id = accessor->startUpdateZone("example.com.", false).second;
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
-    accessor->commitUpdateZone();
+    accessor->commit();
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
 }
 
@@ -561,7 +561,7 @@ TEST_F(SQLite3Update, flushZone) {
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
     zone_id = accessor->startUpdateZone("example.com.", true).second;
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
-    accessor->commitUpdateZone();
+    accessor->commit();
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
 }
 
@@ -575,7 +575,7 @@ TEST_F(SQLite3Update, readWhileUpdate) {
 
     // Once the changes are committed, the other accessor will see the new
     // data.
-    accessor->commitUpdateZone();
+    accessor->commit();
     checkRecords(*another_accessor, zone_id, "foo.bar.example.com.",
                  empty_stored);
 }
@@ -585,7 +585,7 @@ TEST_F(SQLite3Update, rollback) {
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
 
     // Rollback will revert the change made by startUpdateZone(, true).
-    accessor->rollbackUpdateZone();
+    accessor->rollback();
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
 }
 
@@ -599,7 +599,7 @@ TEST_F(SQLite3Update, rollbackFailure) {
     EXPECT_TRUE(iterator->getNext(columns));
 
     accessor->startUpdateZone("example.com.", true);
-    EXPECT_THROW(accessor->rollbackUpdateZone(), DataSourceError);
+    EXPECT_THROW(accessor->rollback(), DataSourceError);
 }
 
 TEST_F(SQLite3Update, commitConflict) {
@@ -612,8 +612,8 @@ TEST_F(SQLite3Update, commitConflict) {
     // which will prevent commit.
     zone_id = accessor->startUpdateZone("example.com.", true).second;
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
-    EXPECT_THROW(accessor->commitUpdateZone(), DataSourceError);
-    accessor->rollbackUpdateZone();   // rollback should still succeed
+    EXPECT_THROW(accessor->commit(), DataSourceError);
+    accessor->rollback();   // rollback should still succeed
 
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
 }
@@ -631,9 +631,9 @@ TEST_F(SQLite3Update, updateConflict) {
 
     // Once we rollback the other attempt of change, we should be able to
     // start and commit the transaction using the main accessor.
-    another_accessor->rollbackUpdateZone();
+    another_accessor->rollback();
     accessor->startUpdateZone("example.com.", true);
-    accessor->commitUpdateZone();
+    accessor->commit();
 }
 
 TEST_F(SQLite3Update, duplicateUpdate) {
@@ -643,11 +643,11 @@ TEST_F(SQLite3Update, duplicateUpdate) {
 }
 
 TEST_F(SQLite3Update, commitWithoutTransaction) {
-    EXPECT_THROW(accessor->commitUpdateZone(), DataSourceError);
+    EXPECT_THROW(accessor->commit(), DataSourceError);
 }
 
 TEST_F(SQLite3Update, rollbackWithoutTransaction) {
-    EXPECT_THROW(accessor->rollbackUpdateZone(), DataSourceError);
+    EXPECT_THROW(accessor->rollback(), DataSourceError);
 }
 
 TEST_F(SQLite3Update, addRecord) {
@@ -664,7 +664,7 @@ TEST_F(SQLite3Update, addRecord) {
     checkRecords(*accessor, zone_id, "newdata.example.com.", expected_stored);
 
     // Commit the change, and confirm the new data is still there.
-    accessor->commitUpdateZone();
+    accessor->commit();
     checkRecords(*accessor, zone_id, "newdata.example.com.", expected_stored);
 }
 
@@ -678,7 +678,7 @@ TEST_F(SQLite3Update, addThenRollback) {
     expected_stored.push_back(new_data);
     checkRecords(*accessor, zone_id, "newdata.example.com.", expected_stored);
 
-    accessor->rollbackUpdateZone();
+    accessor->rollback();
     checkRecords(*accessor, zone_id, "newdata.example.com.", empty_stored);
 }
 
@@ -717,7 +717,7 @@ TEST_F(SQLite3Update, deleteRecord) {
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
 
     // Commit the change, and confirm the deleted data still isn't there.
-    accessor->commitUpdateZone();
+    accessor->commit();
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
 }
 
@@ -730,7 +730,7 @@ TEST_F(SQLite3Update, deleteThenRollback) {
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
 
     // Rollback the change, and confirm the data still exists.
-    accessor->rollbackUpdateZone();
+    accessor->rollback();
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
 }
 
@@ -768,4 +768,80 @@ TEST_F(SQLite3Update, invalidDelete) {
     // An attempt of delete before an explicit start of transaction
     EXPECT_THROW(accessor->deleteRecordInZone(del_params), DataSourceError);
 }
+
+TEST_F(SQLite3Update, emptyTransaction) {
+    // A generic transaction without doing anything inside it.  Just check
+    // it doesn't throw or break the database.
+    checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+    accessor->startTransaction();
+    checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+    accessor->commit();
+    checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+}
+
+TEST_F(SQLite3Update, duplicateTransaction) {
+    accessor->startTransaction();
+    EXPECT_THROW(accessor->startTransaction(), DataSourceError);
+}
+
+TEST_F(SQLite3Update, transactionInUpdate) {
+    accessor->startUpdateZone("example.com.", true);
+    EXPECT_THROW(accessor->startTransaction(), DataSourceError);
+}
+
+TEST_F(SQLite3Update, updateInTransaction) {
+    accessor->startTransaction();
+    EXPECT_THROW(accessor->startUpdateZone("example.com.", true),
+                 DataSourceError);
+}
+
+TEST_F(SQLite3Update, updateWithTransaction) {
+    // Start a read-only transaction, wherein we execute two reads.
+    // Meanwhile we start a write (update) transaction.  The commit attempt
+    // for the write transaction will due to the lock held by the read
+    // transaction.  The database should be intact.
+    another_accessor->startTransaction();
+    checkRecords(*another_accessor, zone_id, "foo.bar.example.com.",
+                 expected_stored);
+
+    ASSERT_TRUE(accessor->startUpdateZone("example.com.", true).first);
+    EXPECT_THROW(accessor->commit(), DataSourceError);
+
+    checkRecords(*another_accessor, zone_id, "foo.bar.example.com.",
+                 expected_stored);
+    another_accessor->commit(); // this shouldn't throw
+}
+
+TEST_F(SQLite3Update, updateWithoutTransaction) {
+    // Similar to the previous test, but reads are not protected in a
+    // transaction.  So the write transaction will succeed and flush the DB,
+    // and the result of the second read is different from the first.
+    checkRecords(*another_accessor, zone_id, "foo.bar.example.com.",
+                 expected_stored);
+
+    ASSERT_TRUE(accessor->startUpdateZone("example.com.", true).first);
+    accessor->commit();
+
+    checkRecords(*another_accessor, zone_id, "foo.bar.example.com.",
+                 empty_stored);
+}
+
+TEST_F(SQLite3Update, concurrentTransactions) {
+    // Two read-only transactions coexist (unlike the read vs write)
+    // Start one transaction.
+    accessor->startTransaction();
+    checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+
+    // Start a new one.
+    another_accessor->startTransaction();
+
+    // The second transaction doesn't affect the first or vice versa.
+    checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+    checkRecords(*another_accessor, zone_id, "foo.bar.example.com.",
+                 expected_stored);
+
+    // Commit should be successful for both transactions.
+    accessor->commit();
+    another_accessor->commit();
+}
 } // end anonymous namespace

src/lib/python/isc/datasrc/iterator_inc.cc

@@ -31,4 +31,37 @@ the end of the zone.\n\
 Raises an isc.datasrc.Error exception if it is called again after returning\n\
 None\n\
 ";
+
+// Modifications:
+//  - ConstRRset->RRset
+//  - NULL->None
+//  - removed notes about derived classes (which doesn't apply for python)
+const char* const ZoneIterator_getSOA_doc = "\
+get_soa() -> isc.dns.RRset\n\
+\n\
+Return the SOA record of the zone in the iterator context.\n\
+\n\
+This method returns the zone's SOA record (if any, and a valid zone\n\
+should have it) in the form of an RRset object. This SOA is identical\n\
+to that (again, if any) contained in the sequence of RRsets returned\n\
+by the iterator. In that sense this method is redundant, but is\n\
+provided as a convenient utility for the application of the iterator;\n\
+the application may need to know the SOA serial or the SOA RR itself\n\
+for the purpose of protocol handling or skipping the expensive\n\
+iteration processing.\n\
+\n\
+If the zone doesn't have an SOA (which is broken, but some data source\n\
+may allow that situation), this method returns None. Also, in the\n\
+normal and valid case, the SOA should have exactly one RDATA, but this\n\
+API does not guarantee it as some data source may accept such an\n\
+abnormal condition. It's up to the caller whether to check the number\n\
+of RDATA and how to react to the unexpected case.\n\
+\n\
+Exceptions:\n\
+  None\n\
+\n\
+Return Value(s): An SOA RRset object that would be\n\
+returned from the iteration. It will be None if the zone doesn't have\n\
+an SOA.\n\
+";
 } // unnamed namespace

src/lib/python/isc/datasrc/iterator_python.cc

@@ -132,10 +132,35 @@ ZoneIterator_next(PyObject* self) {
     }
 }
 
+PyObject*
+ZoneIterator_getSOA(PyObject* po_self, PyObject*) {
+    s_ZoneIterator* self = static_cast<s_ZoneIterator*>(po_self);
+    try {
+        isc::dns::ConstRRsetPtr rrset = self->cppobj->getSOA();
+        if (!rrset) {
+            Py_RETURN_NONE;
+        }
+        return (createRRsetObject(*rrset));
+    } catch (const isc::Exception& isce) {
+        // isc::Unexpected is thrown when we call getNextRRset() when we are
+        // already done iterating ('iterating past end')
+        // We could also simply return None again
+        PyErr_SetString(getDataSourceException("Error"), isce.what());
+        return (NULL);
+    } catch (const std::exception& exc) {
+        PyErr_SetString(getDataSourceException("Error"), exc.what());
+        return (NULL);
+    } catch (...) {
+        PyErr_SetString(getDataSourceException("Error"),
+                        "Unexpected exception");
+        return (NULL);
+    }
+}
+
 PyMethodDef ZoneIterator_methods[] = {
-    { "get_next_rrset",
-      reinterpret_cast<PyCFunction>(ZoneIterator_getNextRRset), METH_NOARGS,
+    { "get_next_rrset", ZoneIterator_getNextRRset, METH_NOARGS,
       ZoneIterator_getNextRRset_doc },
+    { "get_soa", ZoneIterator_getSOA, METH_NOARGS, ZoneIterator_getSOA_doc },
     { NULL, NULL, 0, NULL }
 };
 

src/lib/python/isc/datasrc/tests/datasrc_test.py

@@ -189,6 +189,20 @@ class DataSrcClient(unittest.TestCase):
 
         self.assertRaises(TypeError, dsc.get_iterator, "asdf")
 
+    def test_iterator_soa(self):
+        dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
+        iterator = dsc.get_iterator(isc.dns.Name("sql1.example.com."))
+        expected_soa = isc.dns.RRset(isc.dns.Name("sql1.example.com."),
+                                     isc.dns.RRClass.IN(),
+                                     isc.dns.RRType.SOA(),
+                                     isc.dns.RRTTL(3600))
+        expected_soa.add_rdata(isc.dns.Rdata(isc.dns.RRType.SOA(),
+                                             isc.dns.RRClass.IN(),
+                                             "master.example.com. " +
+                                             "admin.example.com. 678 " +
+                                             "3600 1800 2419200 7200"))
+        self.assertTrue(rrsets_equal(expected_soa, iterator.get_soa()))
+
     def test_construct(self):
         # can't construct directly
         self.assertRaises(TypeError, isc.datasrc.ZoneFinder)
@@ -512,6 +526,17 @@ class DataSrcUpdater(unittest.TestCase):
         dsc.get_updater(isc.dns.Name("example.com"), True)
         self.assertEqual(orig_ref, sys.getrefcount(dsc))
 
+    def test_iterate_over_empty_zone(self):
+        # empty the test zone first
+        dsc = isc.datasrc.DataSourceClient("sqlite3", WRITE_ZONE_DB_CONFIG)
+        updater = dsc.get_updater(isc.dns.Name("example.com"), True)
+        updater.commit()
+
+        # Check the iterator behavior for the empty zone.
+        iterator = dsc.get_iterator(isc.dns.Name("example.com."))
+        self.assertEqual(None, iterator.get_soa())
+        self.assertEqual(None, iterator.get_next_rrset())
+
 if __name__ == "__main__":
     isc.log.init("bind10")
     unittest.main()