BIND 10 master, updated. 1aa233fab1d74dc776899df61181806679d14013 [master] Merge branch 'trac1329'

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Nov 7 21:33:09 UTC 2011


The branch, master has been updated
       via  1aa233fab1d74dc776899df61181806679d14013 (commit)
       via  27b7f9d36113514773777eb94bf66a3ef8c49a82 (commit)
       via  6716721a7c10737d86a4a29af530d54a458f83ca (commit)
       via  e8aa8b8b994146dfff6d29435a66c88dcf79eb69 (commit)
       via  dee6a4739aee15e8899da2e35d179cb1d8623e76 (commit)
       via  50672f2d6073e813fb80250398b6e6a2b93c915d (commit)
       via  1a90f118bf69d6239ca290f712bfeb89a9027efd (commit)
       via  5d290088a1b996011217cf801e37600d5bcd037e (commit)
       via  3d59d6a24e3a84c3ca453721649e6adfab863c0e (commit)
       via  a95b528af25a2b3bda91f9b88c04a20b0b783208 (commit)
       via  58e8ca7d1c5d8f4b69aa174405e4ef280b8012cc (commit)
       via  aa13f832395794bab3647ed375ac8a6e2d26e55f (commit)
       via  0ea04c4bb216cc822be49626d4b0269956fd070e (commit)
      from  9697c6b3cc3e49d96efc6777c1dba5ecb00eb785 (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 1aa233fab1d74dc776899df61181806679d14013
Merge: 9697c6b 27b7f9d
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Mon Nov 7 13:27:36 2011 -0800

    [master] Merge branch 'trac1329'

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

Summary of changes:
 src/lib/datasrc/database.h                         |  105 ++++++++-
 src/lib/datasrc/sqlite3_accessor.cc                |  178 ++++++++++---
 src/lib/datasrc/sqlite3_accessor.h                 |   17 ++
 src/lib/datasrc/tests/Makefile.am                  |    1 +
 src/lib/datasrc/tests/database_unittest.cc         |    2 +
 src/lib/datasrc/tests/sqlite3_accessor_unittest.cc |  272 +++++++++++++++++++-
 src/lib/datasrc/tests/testdata/test.sqlite3        |  Bin 43008 -> 44032 bytes
 .../{test.sqlite3 => test.sqlite3.nodiffs}         |  Bin 43008 -> 43008 bytes
 8 files changed, 530 insertions(+), 45 deletions(-)
 copy src/lib/datasrc/tests/testdata/{test.sqlite3 => test.sqlite3.nodiffs} (100%)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h
index d80a4ab..b9379b7 100644
--- a/src/lib/datasrc/database.h
+++ b/src/lib/datasrc/database.h
@@ -85,7 +85,7 @@ public:
      * Definitions of the fields to be passed to addRecordToZone().
      *
      * Each derived implementation of addRecordToZone() should expect
-     * the "columns" vector to be filled with the values as described in this
+     * the "columns" array to be filled with the values as described in this
      * enumeration, in this order.
      */
     enum AddRecordColumns {
@@ -103,7 +103,7 @@ public:
      * Definitions of the fields to be passed to deleteRecordInZone().
      *
      * Each derived implementation of deleteRecordInZone() should expect
-     * the "params" vector to be filled with the values as described in this
+     * the "params" array to be filled with the values as described in this
      * enumeration, in this order.
      */
     enum DeleteRecordParams {
@@ -114,6 +114,31 @@ public:
     };
 
     /**
+     * Operation mode when adding a record diff.
+     *
+     * This is used as the "operation" parameter value of addRecordDiff().
+     */
+    enum DiffOperation {
+        DIFF_ADD = 0,           ///< This diff is for adding an RR
+        DIFF_DELETE = 1         ///< This diff is for deleting an RR
+    };
+
+    /**
+     * Definitions of the fields to be passed to addRecordDiff().
+     *
+     * Each derived implementation of addRecordDiff() should expect
+     * the "params" array to be filled with the values as described in this
+     * enumeration, in this order.
+     */
+    enum DiffRecordParams {
+        DIFF_NAME = 0, ///< The owner name of the record (a domain name)
+        DIFF_TYPE = 1, ///< The RRType of the record (A/NS/TXT etc.)
+        DIFF_TTL = 2,  ///< The TTL of the record (in numeric form)
+        DIFF_RDATA = 3, ///< Full text representation of the record's RDATA
+        DIFF_PARAM_COUNT = 4    ///< Number of parameters
+    };
+
+    /**
      * \brief Destructor
      *
      * It is empty, but needs a virtual one, since we will use the derived
@@ -453,6 +478,82 @@ public:
     /// to the method or internal database error.
     virtual void rollback() = 0;
 
+    /// Install a single RR diff in difference sequences for zone update.
+    ///
+    /// This method inserts parameters of an update operation for a single RR
+    /// (either adding or deleting one) in the underlying database.
+    /// (These parameters would normally be a separate database table, but
+    /// actual realization can differ in specific implementations).
+    /// The information given via this method generally corresponds to either
+    /// a single call to \c addRecordToZone() or \c deleteRecordInZone(),
+    /// and this method is expected to be called immediately after (or before)
+    /// a call to either of those methods.
+    ///
+    /// Note, however, that this method passes more detailed information
+    /// than those update methods: it passes "serial", even if the diff
+    /// is not for the SOA RR; it passes TTL for a diff that deletes an RR
+    /// while in \c deleteRecordInZone() it's omitted.  This is because
+    /// the stored diffs are expected to be retrieved in the form that
+    /// \c getRecordDiffs() is expected to meet.  This means if the caller
+    /// wants to use this method with other update operations, it must
+    /// ensure the additional information is ready when this method is called.
+    ///
+    /// \note \c getRecordDiffs() is not yet implemented.
+    ///
+    /// The caller of this method must ensure that the added diffs via
+    /// this method in a single transaction form an IXFR-style difference
+    /// sequences: Each difference sequence is a sequence of RRs:
+    /// an older version of SOA (to be deleted), zero or more other deleted
+    /// RRs, the post-transaction SOA (to be added), and zero or more other
+    /// added RRs.  So, for example, the first call to this method in a
+    /// transaction must always be deleting an SOA.  Also, the \c serial
+    /// parameter must be equal to the value of the serial field of the
+    /// SOA that was last added or deleted (if the call is to add or delete
+    /// an SOA RR, \c serial must be identical to the serial of that SOA).
+    /// The underlying derived class implementation may or may not check
+    /// this condition, but if the caller doesn't meet the condition
+    /// a subsequent call to \c getRecordDiffs() will not work as expected.
+    ///
+    /// Any call to this method must be in a transaction, and, for now,
+    /// it must be a transaction triggered by \c startUpdateZone() (that is,
+    /// it cannot be a transaction started by \c startTransaction()).
+    /// All calls to this method are considered to be part of an atomic
+    /// transaction: Until \c commit() is performed, the added diffs are
+    /// not visible outside the transaction; if \c rollback() is performed,
+    /// all added diffs are canceled; and the added sequences are not
+    /// affected by any concurrent attempt of adding diffs (conflict resolution
+    /// is up to the database implementation).
+    ///
+    /// Also for now, all diffs are assumed to be for the zone that is
+    /// being updated in the context of \c startUpdateZone().  So the
+    /// \c zone_id parameter must be identical to the zone ID returned by
+    /// \c startUpdateZone().
+    ///
+    /// In a future version we may loosen this condition so that diffs can be
+    /// added in a generic transaction and may not even have to belong to
+    /// a single zone.  For this possible extension \c zone_id parameter is
+    /// included even if it's redundant under the current restriction.
+    ///
+    /// The support for adding (or retrieving) diffs is optional; if it's
+    /// not supported in a specific data source, this method for the
+    /// corresponding derived class will throw an \c NotImplemented exception.
+    ///
+    /// \exception DataSourceError Invalid call without starting a transaction,
+    /// zone ID doesn't match the zone being updated, or other internal
+    /// database error.
+    /// \exception NotImplemented Adding diffs is not supported in the
+    /// data source.
+    /// \exception Other The concrete derived method may throw other
+    /// data source specific exceptions.
+    ///
+    /// \param zone_id The zone for the diff to be added.
+    /// \param serial The SOA serial to which the diff belongs.
+    /// \param operation Either \c DIFF_ADD or \c DIFF_DELETE.
+    /// \param params An array of strings that defines a record for the diff.
+    virtual void addRecordDiff(
+        int zone_id, uint32_t serial, DiffOperation operation,
+        const std::string (&params)[DIFF_PARAM_COUNT]) = 0;
+
     /// Clone the accessor with the same configuration.
     ///
     /// Each derived class implementation of this method will create a new
diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc
index cf1593a..efa5717 100644
--- a/src/lib/datasrc/sqlite3_accessor.cc
+++ b/src/lib/datasrc/sqlite3_accessor.cc
@@ -52,7 +52,9 @@ enum StatementID {
     DEL_RECORD = 8,
     ITERATE = 9,
     FIND_PREVIOUS = 10,
-    NUM_STATEMENTS = 11
+    ADD_RECORD_DIFF = 11,
+    GET_RECORD_DIFF = 12,       // This is temporary for testing "add diff"
+    NUM_STATEMENTS = 13
 };
 
 const char* const text_statements[NUM_STATEMENTS] = {
@@ -81,7 +83,12 @@ const char* const text_statements[NUM_STATEMENTS] = {
      */
     "SELECT name FROM records " // FIND_PREVIOUS
     "WHERE zone_id=?1 AND rdtype = 'NSEC' AND "
-    "rname < $2 ORDER BY rname DESC LIMIT 1"
+    "rname < $2 ORDER BY rname DESC LIMIT 1",
+    "INSERT INTO diffs "        // ADD_RECORD_DIFF
+    "(zone_id, version, operation, name, rrtype, ttl, rdata) "
+    "VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)"
+    , "SELECT name, rrtype, ttl, rdata, version, operation " // GET_RECORD_DIFF
+    "FROM diffs WHERE zone_id = ?1 ORDER BY id, operation"
 };
 
 struct SQLite3Parameters {
@@ -94,12 +101,45 @@ struct SQLite3Parameters {
         }
     }
 
+    // This method returns the specified ID of SQLITE3 statement.  If it's
+    // not yet prepared it internally creates a new one.  This way we can
+    // avoid preparing unnecessary statements and minimize the overhead.
+    sqlite3_stmt*
+    getStatement(int id) {
+        assert(id < NUM_STATEMENTS);
+        if (statements_[id] == NULL) {
+            assert(db_ != NULL);
+            sqlite3_stmt* prepared = NULL;
+            if (sqlite3_prepare_v2(db_, text_statements[id], -1, &prepared,
+                                   NULL) != SQLITE_OK) {
+                isc_throw(SQLite3Error, "Could not prepare SQLite statement: "
+                          << text_statements[id] <<
+                          ": " << sqlite3_errmsg(db_));
+            }
+            statements_[id] = prepared;
+        }
+        return (statements_[id]);
+    }
+
+    void
+    finalizeStatements() {
+        for (int i = 0; i < NUM_STATEMENTS; ++i) {
+            if (statements_[i] != NULL) {
+                sqlite3_finalize(statements_[i]);
+                statements_[i] = NULL;
+            }
+        }
+    }
+
     sqlite3* db_;
     int version_;
-    sqlite3_stmt* statements_[NUM_STATEMENTS];
     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
+private:
+    // statements_ are private and must be accessed via getStatement() outside
+    // of this structure.
+    sqlite3_stmt* statements_[NUM_STATEMENTS];
 };
 
 // This is a helper class to encapsulate the code logic of executing
@@ -116,18 +156,19 @@ public:
     // DataSourceError exception.
     StatementProcessor(SQLite3Parameters& dbparameters, StatementID stmt_id,
                        const char* desc) :
-        dbparameters_(dbparameters), stmt_id_(stmt_id), desc_(desc)
+        dbparameters_(dbparameters), stmt_(dbparameters.getStatement(stmt_id)),
+        desc_(desc)
     {
-        sqlite3_clear_bindings(dbparameters_.statements_[stmt_id_]);
+        sqlite3_clear_bindings(stmt_);
     }
 
     ~StatementProcessor() {
-        sqlite3_reset(dbparameters_.statements_[stmt_id_]);
+        sqlite3_reset(stmt_);
     }
 
     void exec() {
-        if (sqlite3_step(dbparameters_.statements_[stmt_id_]) != SQLITE_DONE) {
-            sqlite3_reset(dbparameters_.statements_[stmt_id_]);
+        if (sqlite3_step(stmt_) != SQLITE_DONE) {
+            sqlite3_reset(stmt_);
             isc_throw(DataSourceError, "failed to " << desc_ << ": " <<
                       sqlite3_errmsg(dbparameters_.db_));
         }
@@ -135,7 +176,7 @@ public:
 
 private:
     SQLite3Parameters& dbparameters_;
-    const StatementID stmt_id_;
+    sqlite3_stmt* stmt_;
     const char* const desc_;
 };
 
@@ -170,10 +211,6 @@ namespace {
 class Initializer {
 public:
     ~Initializer() {
-        for (int i = 0; i < NUM_STATEMENTS; ++i) {
-            sqlite3_finalize(params_.statements_[i]);
-        }
-
         if (params_.db_ != NULL) {
             sqlite3_close(params_.db_);
         }
@@ -206,6 +243,11 @@ const char* const SCHEMA_LIST[] = {
     "ttl INTEGER NOT NULL, rdtype STRING NOT NULL COLLATE NOCASE, "
     "rdata STRING NOT NULL)",
     "CREATE INDEX nsec3_byhash ON nsec3 (hash)",
+    "CREATE TABLE diffs (id INTEGER PRIMARY KEY, "
+    "zone_id INTEGER NOT NULL, version INTEGER NOT NULL, "
+    "operation INTEGER NOT NULL, name STRING NOT NULL COLLATE NOCASE, "
+    "rrtype STRING NOT NULL COLLATE NOCASE, ttl INTEGER NOT NULL, "
+    "rdata STRING NOT NULL)",
     NULL
 };
 
@@ -214,7 +256,7 @@ prepare(sqlite3* const db, const char* const statement) {
     sqlite3_stmt* prepared = NULL;
     if (sqlite3_prepare_v2(db, statement, -1, &prepared, NULL) != SQLITE_OK) {
         isc_throw(SQLite3Error, "Could not prepare SQLite statement: " <<
-                  statement);
+                  statement << ": " << sqlite3_errmsg(db));
     }
     return (prepared);
 }
@@ -304,10 +346,6 @@ checkAndSetupSchema(Initializer* initializer) {
         schema_version = create_database(db);
     }
     initializer->params_.version_ = schema_version;
-
-    for (int i = 0; i < NUM_STATEMENTS; ++i) {
-        initializer->params_.statements_[i] = prepare(db, text_statements[i]);
-    }
 }
 
 }
@@ -345,12 +383,7 @@ SQLite3Accessor::close(void) {
                   "SQLite data source is being closed before open");
     }
 
-    // XXX: sqlite3_finalize() could fail.  What should we do in that case?
-    for (int i = 0; i < NUM_STATEMENTS; ++i) {
-        sqlite3_finalize(dbparameters_->statements_[i]);
-        dbparameters_->statements_[i] = NULL;
-    }
-
+    dbparameters_->finalizeStatements();
     sqlite3_close(dbparameters_->db_);
     dbparameters_->db_ = NULL;
 }
@@ -358,7 +391,7 @@ SQLite3Accessor::close(void) {
 std::pair<bool, int>
 SQLite3Accessor::getZone(const std::string& name) const {
     int rc;
-    sqlite3_stmt* const stmt = dbparameters_->statements_[ZONE];
+    sqlite3_stmt* const stmt = dbparameters_->getStatement(ZONE);
 
     // Take the statement (simple SELECT id FROM zones WHERE...)
     // and prepare it (bind the parameters to it)
@@ -522,7 +555,7 @@ private:
 
     const IteratorType iterator_type_;
     boost::shared_ptr<const SQLite3Accessor> accessor_;
-    sqlite3_stmt *statement_;
+    sqlite3_stmt* statement_;
     const std::string name_;
 };
 
@@ -563,10 +596,9 @@ SQLite3Accessor::startUpdateZone(const string& zone_name, const bool replace) {
             StatementProcessor delzone_exec(*dbparameters_, DEL_ZONE_RECORDS,
                                             "delete zone records");
 
-            sqlite3_clear_bindings(
-                dbparameters_->statements_[DEL_ZONE_RECORDS]);
-            if (sqlite3_bind_int(dbparameters_->statements_[DEL_ZONE_RECORDS],
-                                 1, zone_info.second) != SQLITE_OK) {
+            sqlite3_stmt* stmt = dbparameters_->getStatement(DEL_ZONE_RECORDS);
+            sqlite3_clear_bindings(stmt);
+            if (sqlite3_bind_int(stmt, 1, zone_info.second) != SQLITE_OK) {
                 isc_throw(DataSourceError,
                           "failed to bind SQLite3 parameter: " <<
                           sqlite3_errmsg(dbparameters_->db_));
@@ -635,7 +667,7 @@ void
 doUpdate(SQLite3Parameters& dbparams, StatementID stmt_id,
          COLUMNS_TYPE update_params, const char* exec_desc)
 {
-    sqlite3_stmt* const stmt = dbparams.statements_[stmt_id];
+    sqlite3_stmt* const stmt = dbparams.getStatement(stmt_id);
     StatementProcessor executer(dbparams, stmt_id, exec_desc);
 
     int param_id = 0;
@@ -681,34 +713,98 @@ SQLite3Accessor::deleteRecordInZone(const string (&params)[DEL_PARAM_COUNT]) {
         *dbparameters_, DEL_RECORD, params, "delete record from zone");
 }
 
+void
+SQLite3Accessor::addRecordDiff(int zone_id, uint32_t serial,
+                               DiffOperation operation,
+                               const std::string (&params)[DIFF_PARAM_COUNT])
+{
+    if (!dbparameters_->updating_zone) {
+        isc_throw(DataSourceError, "adding record diff without update "
+                  "transaction on " << getDBName());
+    }
+    if (zone_id != dbparameters_->updated_zone_id) {
+        isc_throw(DataSourceError, "bad zone ID for adding record diff on "
+                  << getDBName() << ": " << zone_id << ", must be "
+                  << dbparameters_->updated_zone_id);
+    }
+
+    sqlite3_stmt* const stmt = dbparameters_->getStatement(ADD_RECORD_DIFF);
+    StatementProcessor executer(*dbparameters_, ADD_RECORD_DIFF,
+                                "add record diff");
+    int param_id = 0;
+    if (sqlite3_bind_int(stmt, ++param_id, zone_id)
+        != SQLITE_OK) {
+        isc_throw(DataSourceError, "failed to bind SQLite3 parameter: " <<
+                  sqlite3_errmsg(dbparameters_->db_));
+    }
+    if (sqlite3_bind_int64(stmt, ++param_id, serial)
+        != SQLITE_OK) {
+        isc_throw(DataSourceError, "failed to bind SQLite3 parameter: " <<
+                  sqlite3_errmsg(dbparameters_->db_));
+    }
+    if (sqlite3_bind_int(stmt, ++param_id, operation)
+        != SQLITE_OK) {
+        isc_throw(DataSourceError, "failed to bind SQLite3 parameter: " <<
+                  sqlite3_errmsg(dbparameters_->db_));
+    }
+    for (int i = 0; i < DIFF_PARAM_COUNT; ++i) {
+        if (sqlite3_bind_text(stmt, ++param_id, params[i].c_str(),
+                              -1, SQLITE_TRANSIENT) != SQLITE_OK) {
+            isc_throw(DataSourceError, "failed to bind SQLite3 parameter: " <<
+                      sqlite3_errmsg(dbparameters_->db_));
+        }
+    }
+    executer.exec();
+}
+
+vector<vector<string> >
+SQLite3Accessor::getRecordDiff(int zone_id) {
+    sqlite3_stmt* const stmt = dbparameters_->getStatement(GET_RECORD_DIFF);
+    sqlite3_bind_int(stmt, 1, zone_id);
+
+    vector<vector<string> > result;
+    while (sqlite3_step(stmt) == SQLITE_ROW) {
+        vector<string> row_result;
+        for (int i = 0; i < 6; ++i) {
+            row_result.push_back(convertToPlainChar(sqlite3_column_text(stmt,
+                                                                        i),
+                                                    dbparameters_->db_));
+        }
+        result.push_back(row_result);
+    }
+    sqlite3_reset(stmt);
+
+    return (result);
+}
+
 std::string
 SQLite3Accessor::findPreviousName(int zone_id, const std::string& rname)
     const
 {
-    sqlite3_reset(dbparameters_->statements_[FIND_PREVIOUS]);
-    sqlite3_clear_bindings(dbparameters_->statements_[FIND_PREVIOUS]);
+    sqlite3_stmt* const stmt = dbparameters_->getStatement(FIND_PREVIOUS);
+    sqlite3_reset(stmt);
+    sqlite3_clear_bindings(stmt);
 
-    if (sqlite3_bind_int(dbparameters_->statements_[FIND_PREVIOUS], 1,
-                         zone_id) != SQLITE_OK) {
+    if (sqlite3_bind_int(stmt, 1, zone_id) != SQLITE_OK) {
         isc_throw(SQLite3Error, "Could not bind zone ID " << zone_id <<
                   " to SQL statement (find previous): " <<
                   sqlite3_errmsg(dbparameters_->db_));
     }
-    if (sqlite3_bind_text(dbparameters_->statements_[FIND_PREVIOUS], 2,
-                          rname.c_str(), -1, SQLITE_STATIC) != SQLITE_OK) {
+    if (sqlite3_bind_text(stmt, 2, rname.c_str(), -1, SQLITE_STATIC) !=
+        SQLITE_OK) {
         isc_throw(SQLite3Error, "Could not bind name " << rname <<
                   " to SQL statement (find previous): " <<
                   sqlite3_errmsg(dbparameters_->db_));
     }
 
     std::string result;
-    const int rc = sqlite3_step(dbparameters_->statements_[FIND_PREVIOUS]);
+    const int rc = sqlite3_step(stmt);
     if (rc == SQLITE_ROW) {
         // We found it
-        result = convertToPlainChar(sqlite3_column_text(dbparameters_->
-            statements_[FIND_PREVIOUS], 0), dbparameters_->db_);
+        result = convertToPlainChar(sqlite3_column_text(stmt, 0),
+                                    dbparameters_->db_);
     }
-    sqlite3_reset(dbparameters_->statements_[FIND_PREVIOUS]);
+    sqlite3_reset(stmt);
 
     if (rc == SQLITE_DONE) {
         // No NSEC records here, this DB doesn't support DNSSEC or
diff --git a/src/lib/datasrc/sqlite3_accessor.h b/src/lib/datasrc/sqlite3_accessor.h
index 3d1c85d..6b5369c 100644
--- a/src/lib/datasrc/sqlite3_accessor.h
+++ b/src/lib/datasrc/sqlite3_accessor.h
@@ -157,6 +157,23 @@ public:
     virtual void deleteRecordInZone(
         const std::string (&params)[DEL_PARAM_COUNT]);
 
+    /// This derived version of the method prepares an SQLite3 statement
+    /// for adding the diff first time it's called, and if it fails throws
+    // an \c SQLite3Error exception.
+    virtual void addRecordDiff(
+        int zone_id, uint32_t serial, DiffOperation operation,
+        const std::string (&params)[DIFF_PARAM_COUNT]);
+
+    // A short term method for tests until we implement more complete
+    // API to retrieve diffs (#1330).  It returns all records of the diffs
+    // table whose zone_id column is identical to the given value.
+    // Since this is a short term workaround, it ignores some corner cases
+    // (such as an SQLite3 execution failure) and is not very efficient,
+    // in favor of brevity.  Once #1330 is completed, this method must be
+    // removed, and the tests using this method must be rewritten using the
+    // official API.
+    std::vector<std::vector<std::string> > getRecordDiff(int zone_id);
+
     /// The SQLite3 implementation of this method returns a string starting
     /// with a fixed prefix of "sqlite3_" followed by the DB file name
     /// removing any path name.  For example, for the DB file
diff --git a/src/lib/datasrc/tests/Makefile.am b/src/lib/datasrc/tests/Makefile.am
index 3d2ba6d..e5cca0a 100644
--- a/src/lib/datasrc/tests/Makefile.am
+++ b/src/lib/datasrc/tests/Makefile.am
@@ -76,4 +76,5 @@ EXTRA_DIST += testdata/sql1.example.com.signed
 EXTRA_DIST += testdata/sql2.example.com.signed
 EXTRA_DIST += testdata/test-root.sqlite3
 EXTRA_DIST += testdata/test.sqlite3
+EXTRA_DIST += testdata/test.sqlite3.nodiffs
 EXTRA_DIST += testdata/rwtest.sqlite3
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index 9775321..99d6667 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -233,6 +233,8 @@ public:
     virtual void rollback() {}
     virtual void addRecordToZone(const string (&)[ADD_COLUMN_COUNT]) {}
     virtual void deleteRecordInZone(const string (&)[DEL_PARAM_COUNT]) {}
+    virtual void addRecordDiff(int, uint32_t, DiffOperation,
+                               const std::string (&)[DIFF_PARAM_COUNT]) {}
 
     virtual const std::string& getDBName() const {
         return (database_name_);
diff --git a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
index 5d66737..90b2ac1 100644
--- a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
+++ b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
@@ -22,6 +22,7 @@
 #include <dns/rrclass.h>
 
 #include <gtest/gtest.h>
+#include <boost/lexical_cast.hpp>
 #include <boost/scoped_ptr.hpp>
 #include <fstream>
 #include <sqlite3.h>
@@ -29,6 +30,7 @@
 using namespace std;
 using namespace isc::datasrc;
 using boost::shared_ptr;
+using boost::lexical_cast;
 using isc::data::ConstElementPtr;
 using isc::data::Element;
 using isc::dns::RRClass;
@@ -214,8 +216,7 @@ TEST(SQLite3Open, getDBNameExampleROOT) {
     EXPECT_EQ(SQLITE_DBNAME_EXAMPLE_ROOT, accessor.getDBName());
 }
 
-// Simple function to cound the number of records for
-// any name
+// Simple function to match records
 void
 checkRecordRow(const std::string columns[],
                const std::string& field0,
@@ -518,6 +519,7 @@ protected:
     std::string get_columns[DatabaseAccessor::COLUMN_COUNT];
     std::string add_columns[DatabaseAccessor::ADD_COLUMN_COUNT];
     std::string del_params[DatabaseAccessor::DEL_PARAM_COUNT];
+    std::string diff_params[DatabaseAccessor::DIFF_PARAM_COUNT];
 
     vector<const char* const*> expected_stored; // placeholder for checkRecords
     vector<const char* const*> empty_stored; // indicate no corresponding data
@@ -844,4 +846,270 @@ TEST_F(SQLite3Update, concurrentTransactions) {
     accessor->commit();
     another_accessor->commit();
 }
+
+//
+// Commonly used data for diff related tests.  The last two entries are
+// a textual representation of "version" and a textual representation of
+// diff operation (either DIFF_ADD_TEXT or DIFF_DELETE_TEXT).  We use this
+// format for the convenience of generating test data and checking the results.
+//
+const char* const DIFF_ADD_TEXT = "0";
+const char* const DIFF_DELETE_TEXT = "1";
+const char* const diff_begin_data[] = {
+    "example.com.", "SOA", "3600",
+    "ns.example.com. admin.example.com. 1234 3600 1800 2419200 7200",
+    "1234", DIFF_DELETE_TEXT
+};
+const char* const diff_del_a_data[] = {
+    "dns01.example.com.", "A", "3600", "192.0.2.1", "1234", DIFF_DELETE_TEXT
+};
+const char* const diff_end_data[] = {
+    "example.com.", "SOA", "3600",
+    "ns.example.com. admin.example.com. 1300 3600 1800 2419200 7200",
+    "1300", DIFF_ADD_TEXT
+};
+const char* const diff_add_a_data[] = {
+    "dns01.example.com.", "A", "3600", "192.0.2.10", "1234", DIFF_ADD_TEXT
+};
+
+// The following two are helper functions to convert textual test data
+// to integral zone ID and diff operation.
+int
+getVersion(const char* const diff_data[]) {
+    return (lexical_cast<int>(diff_data[DatabaseAccessor::DIFF_PARAM_COUNT]));
+}
+
+DatabaseAccessor::DiffOperation
+getOperation(const char* const diff_data[]) {
+    return (static_cast<DatabaseAccessor::DiffOperation>(
+                lexical_cast<int>(
+                    diff_data[DatabaseAccessor::DIFF_PARAM_COUNT + 1])));
+}
+
+// Common checker function that compares expected and actual sequence of
+// diffs.
+void
+checkDiffs(const vector<const char* const*>& expected,
+           const vector<vector<string> >& actual)
+{
+    EXPECT_EQ(expected.size(), actual.size());
+    const size_t n_diffs = std::min(expected.size(), actual.size());
+    for (size_t i = 0; i < n_diffs; ++i) {
+        for (int j = 0; j < actual[i].size(); ++j) {
+            EXPECT_EQ(expected[i][j], actual[i][j]);
+        }
+    }
+}
+
+TEST_F(SQLite3Update, addRecordDiff) {
+    // A simple case of adding diffs: just changing the SOA, and confirm
+    // the diffs are stored as expected.
+    zone_id = accessor->startUpdateZone("example.com.", false).second;
+
+    copy(diff_begin_data, diff_begin_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    accessor->addRecordDiff(zone_id, getVersion(diff_begin_data),
+                            getOperation(diff_begin_data), diff_params);
+
+    copy(diff_end_data, diff_end_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    accessor->addRecordDiff(zone_id, getVersion(diff_end_data),
+                            getOperation(diff_end_data), diff_params);
+
+    // Until the diffs are committed, they are not visible to other accessors.
+    EXPECT_TRUE(another_accessor->getRecordDiff(zone_id).empty());
+
+    accessor->commit();
+
+    expected_stored.clear();
+    expected_stored.push_back(diff_begin_data);
+    expected_stored.push_back(diff_end_data);
+    checkDiffs(expected_stored, accessor->getRecordDiff(zone_id));
+    // Now it should be visible to others, too.
+    checkDiffs(expected_stored, another_accessor->getRecordDiff(zone_id));
+}
+
+TEST_F(SQLite3Update, addRecordOfLargeSerial) {
+    // This is essentially the same as the previous test, but using a
+    // very large "version" (SOA serial), which is actually the possible
+    // largest value to confirm the internal code doesn't have an overflow bug
+    // or other failure due to the larger value.
+    zone_id = accessor->startUpdateZone("example.com.", false).second;
+
+    const char* const begin_data[] = {
+        "example.com.", "SOA", "3600",
+        "ns.example.com. admin.example.com. 4294967295 3600 1800 2419200 7200",
+        "4294967295", DIFF_DELETE_TEXT
+    };
+
+    copy(begin_data, begin_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    // For "serial" parameter, we intentionally hardcode the value rather
+    // than converting it from the data.
+    accessor->addRecordDiff(zone_id, 0xffffffff, getOperation(diff_begin_data),
+                            diff_params);
+    copy(diff_end_data, diff_end_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    accessor->addRecordDiff(zone_id, getVersion(diff_end_data),
+                            getOperation(diff_end_data), diff_params);
+
+    accessor->commit();
+
+    expected_stored.clear();
+    expected_stored.push_back(begin_data);
+    expected_stored.push_back(diff_end_data);
+    checkDiffs(expected_stored, accessor->getRecordDiff(zone_id));
+}
+
+TEST_F(SQLite3Update, addDiffWithoutUpdate) {
+    // Right now we require startUpdateZone() prior to performing
+    // addRecordDiff.
+    copy(diff_begin_data, diff_begin_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    EXPECT_THROW(accessor->addRecordDiff(0, getVersion(diff_begin_data),
+                                         getOperation(diff_begin_data),
+                                         diff_params),
+                 DataSourceError);
+
+    // For now, we don't allow adding diffs in a general transaction either.
+    accessor->startTransaction();
+    EXPECT_THROW(accessor->addRecordDiff(0, getVersion(diff_begin_data),
+                                         getOperation(diff_begin_data),
+                                         diff_params),
+                 DataSourceError);
+}
+
+TEST_F(SQLite3Update, addDiffWithBadZoneID) {
+    // For now, we require zone ID passed to addRecordDiff be equal to
+    // that for the zone being updated.
+    zone_id = accessor->startUpdateZone("example.com.", false).second;
+    copy(diff_begin_data, diff_begin_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    EXPECT_THROW(accessor->addRecordDiff(zone_id + 1,
+                                         getVersion(diff_begin_data),
+                                         getOperation(diff_begin_data),
+                                         diff_params),
+                 DataSourceError);
+}
+
+TEST_F(SQLite3Update, addDiffRollback) {
+    // Rollback tentatively added diffs.  This is no different from the
+    // update case, but we test it explicitly just in case.
+    zone_id = accessor->startUpdateZone("example.com.", false).second;
+
+    copy(diff_begin_data, diff_begin_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    accessor->addRecordDiff(zone_id, getVersion(diff_begin_data),
+                            getOperation(diff_begin_data), diff_params);
+    accessor->rollback();
+
+    EXPECT_TRUE(accessor->getRecordDiff(zone_id).empty());
+}
+
+TEST_F(SQLite3Update, addDiffInBadOrder) {
+    // At this level, the API is naive, and doesn't care if the diff sequence
+    // is a valid IXFR order.
+    zone_id = accessor->startUpdateZone("example.com.", false).second;
+
+    // Add diff of 'end', then 'begin'
+    copy(diff_end_data, diff_end_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    accessor->addRecordDiff(zone_id, getVersion(diff_end_data),
+                            getOperation(diff_end_data), diff_params);
+
+    copy(diff_begin_data, diff_begin_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    accessor->addRecordDiff(zone_id, getVersion(diff_begin_data),
+                            getOperation(diff_begin_data), diff_params);
+
+    accessor->commit();
+
+    expected_stored.clear();
+    expected_stored.push_back(diff_end_data);
+    expected_stored.push_back(diff_begin_data);
+    checkDiffs(expected_stored, accessor->getRecordDiff(zone_id));
+}
+
+TEST_F(SQLite3Update, addDiffWithUpdate) {
+    // A more realistic example: add corresponding diffs while updating zone.
+    // Implementation wise, there should be no reason this could fail if
+    // the basic tests so far pass.  But we check it in case we miss something.
+
+    const char* const old_a_record[] = {
+        "dns01.example.com.", "A", "192.0.2.1"
+    };
+    const char* const new_a_record[] = {
+        "dns01.example.com.", "com.example.dns01.", "3600", "A", "",
+        "192.0.2.10"
+    };
+    const char* const old_soa_record[] = {
+        "example.com.", "SOA",
+        "ns.example.com. admin.example.com. 1234 3600 1800 2419200 7200",
+    };
+    const char* const new_soa_record[] = {
+        "dns01.example.com.", "com.example.dns01.", "3600", "A", "",
+        "ns.example.com. admin.example.com. 1300 3600 1800 2419200 7200",
+    };
+
+    zone_id = accessor->startUpdateZone("example.com.", false).second;
+
+    // Delete SOA (and add that diff)
+    copy(old_soa_record, old_soa_record + DatabaseAccessor::DEL_PARAM_COUNT,
+         del_params);
+    accessor->deleteRecordInZone(del_params);
+    copy(diff_begin_data, diff_begin_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    accessor->addRecordDiff(zone_id, getVersion(diff_begin_data),
+                            getOperation(diff_begin_data), diff_params);
+
+    // Delete A
+    copy(old_a_record, old_a_record + DatabaseAccessor::DEL_PARAM_COUNT,
+         del_params);
+    accessor->deleteRecordInZone(del_params);
+    copy(diff_del_a_data, diff_del_a_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    accessor->addRecordDiff(zone_id, getVersion(diff_del_a_data),
+                            getOperation(diff_del_a_data), diff_params);
+
+    // Add SOA
+    copy(new_soa_record, new_soa_record + DatabaseAccessor::ADD_COLUMN_COUNT,
+         add_columns);
+    accessor->addRecordToZone(add_columns);
+    copy(diff_end_data, diff_end_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    accessor->addRecordDiff(zone_id, getVersion(diff_end_data),
+                            getOperation(diff_end_data), diff_params);
+
+    // Add A
+    copy(new_a_record, new_a_record + DatabaseAccessor::ADD_COLUMN_COUNT,
+         add_columns);
+    accessor->addRecordToZone(add_columns);
+    copy(diff_add_a_data, diff_add_a_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    accessor->addRecordDiff(zone_id, getVersion(diff_add_a_data),
+                            getOperation(diff_add_a_data), diff_params);
+
+    accessor->commit();
+
+    expected_stored.clear();
+    expected_stored.push_back(diff_begin_data);
+    expected_stored.push_back(diff_del_a_data);
+    expected_stored.push_back(diff_end_data);
+    expected_stored.push_back(diff_add_a_data);
+
+    checkDiffs(expected_stored, accessor->getRecordDiff(zone_id));
+}
+
+TEST_F(SQLite3Update, addDiffWithNoTable) {
+    // An attempt of adding diffs to an old version of database that doesn't
+    // have a diffs table.  This will fail in preparing the statement.
+    initAccessor(SQLITE_DBFILE_EXAMPLE + ".nodiffs", "IN");
+    zone_id = accessor->startUpdateZone("example.com.", false).second;
+    copy(diff_begin_data, diff_begin_data + DatabaseAccessor::DIFF_PARAM_COUNT,
+         diff_params);
+    EXPECT_THROW(accessor->addRecordDiff(zone_id, getVersion(diff_begin_data),
+                                         getOperation(diff_begin_data),
+                                         diff_params),
+                 SQLite3Error);
+}
 } // end anonymous namespace
diff --git a/src/lib/datasrc/tests/testdata/test.sqlite3 b/src/lib/datasrc/tests/testdata/test.sqlite3
index cc8cfc3..521cf31 100644
Binary files a/src/lib/datasrc/tests/testdata/test.sqlite3 and b/src/lib/datasrc/tests/testdata/test.sqlite3 differ
diff --git a/src/lib/datasrc/tests/testdata/test.sqlite3.nodiffs b/src/lib/datasrc/tests/testdata/test.sqlite3.nodiffs
new file mode 100644
index 0000000..cc8cfc3
Binary files /dev/null and b/src/lib/datasrc/tests/testdata/test.sqlite3.nodiffs differ




More information about the bind10-changes mailing list