BIND 10 trac1891, updated. bcb62dd7e4d8a3bca40e3725aa457074ada36134 [1891] enabled some DatabaseClientTest for add/del NSEC3s for SQLite3, too.

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Apr 18 21:27:40 UTC 2012


The branch, trac1891 has been updated
       via  bcb62dd7e4d8a3bca40e3725aa457074ada36134 (commit)
       via  400ff18a8c98cd0c1c393ef32e1d76bb4222dce6 (commit)
      from  e7c91fe725369cabe19bd227e06176a117955461 (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 bcb62dd7e4d8a3bca40e3725aa457074ada36134
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Wed Apr 18 14:26:05 2012 -0700

    [1891] enabled some DatabaseClientTest for add/del NSEC3s for SQLite3, too.
    
    They were originally planned to be enabled with the completion of this branch,
    but I forgot to enable them before review.  As expected, they just passed
    for SQLite3, too.

commit 400ff18a8c98cd0c1c393ef32e1d76bb4222dce6
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Wed Apr 18 14:18:35 2012 -0700

    [1891] added bindXXX methods to StatementProcessor as sqlite_bind_xxx wrapper.
    
    As suggested in review.
    
    This will make the caller code simpler.  It's also safer in that the caller
    doesn't have to manipulate the statement separetely (in the previous way
    StatementProcessor and the caller could use different statements).
    
    Also made naming more consistent: in the very initial version the class was
    called "Executer" and then renamed to Processor.  Some variables of this class
    were still named "exec", etc, but it's more consistent if they are named
    like "proc".

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

Summary of changes:
 src/lib/datasrc/sqlite3_accessor.cc        |   95 +++++++++++++--------------
 src/lib/datasrc/tests/database_unittest.cc |   16 ++---
 2 files changed, 52 insertions(+), 59 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc
index 2dde0ff..f3947f5 100644
--- a/src/lib/datasrc/sqlite3_accessor.cc
+++ b/src/lib/datasrc/sqlite3_accessor.cc
@@ -219,6 +219,10 @@ private:
 // statement, which is completed with a single "step" (normally within a
 // single call to an SQLite3Database method).  In particular, it cannot be
 // used for "SELECT" variants, which generally expect multiple matching rows.
+//
+// The bindXXX methods are straightforward wrappers for the corresponding
+// sqlite3_bind_xxx functions that make bindings with the given parameters
+// on the statement maintained in this class.
 class StatementProcessor {
 public:
     // desc will be used on failure in the what() message of the resulting
@@ -235,6 +239,33 @@ public:
         sqlite3_reset(stmt_);
     }
 
+    void bindInt(int index, int val) {
+        if (sqlite3_bind_int(stmt_, index, val) != SQLITE_OK) {
+            isc_throw(DataSourceError,
+                      "failed to bind SQLite3 parameter: " <<
+                      sqlite3_errmsg(dbparameters_.db_));
+        }
+    }
+
+    void bindInt64(int index, sqlite3_int64 val) {
+        if (sqlite3_bind_int64(stmt_, index, val) != SQLITE_OK) {
+            isc_throw(DataSourceError,
+                      "failed to bind SQLite3 parameter: " <<
+                      sqlite3_errmsg(dbparameters_.db_));
+        }
+    }
+
+    // For simplicity, we assume val is a NULL-terminated string, and the
+    // entire non NUL characters are to be bound.  The destructor parameter
+    // is normally either SQLITE_TRANSIENT or SQLITE_STATIC.
+    void bindText(int index, const char* val, void(*destructor)(void*)) {
+        if (sqlite3_bind_text(stmt_, index, val, -1, destructor)
+            != SQLITE_OK) {
+            isc_throw(DataSourceError, "failed to bind SQLite3 parameter: " <<
+                      sqlite3_errmsg(dbparameters_.db_));
+        }
+    }
+
     void exec() {
         if (sqlite3_step(stmt_) != SQLITE_DONE) {
             sqlite3_reset(stmt_);
@@ -1040,18 +1071,11 @@ SQLite3Accessor::startUpdateZone(const string& zone_name, const bool replace) {
             for (size_t i = 0;
                  i < sizeof(delzone_stmts) / sizeof(delzone_stmts[0]);
                  ++i) {
-                StatementProcessor delzone_exec(*dbparameters_,
+                StatementProcessor delzone_proc(*dbparameters_,
                                                 delzone_stmts[i].first,
                                                 delzone_stmts[i].second);
-                sqlite3_stmt* stmt =
-                    dbparameters_->getStatement(delzone_stmts[i].first);
-                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_));
-                }
-                delzone_exec.exec();
+                delzone_proc.bindInt(1, zone_info.second);
+                delzone_proc.exec();
             }
         } catch (const DataSourceError&) {
             // Once we start a transaction, if something unexpected happens
@@ -1120,29 +1144,19 @@ void
 doUpdate(SQLite3Parameters& dbparams, StatementID stmt_id,
          COLUMNS_TYPE update_params, const char* exec_desc)
 {
-    sqlite3_stmt* const stmt = dbparams.getStatement(stmt_id);
-    StatementProcessor executer(dbparams, stmt_id, exec_desc);
+    StatementProcessor proc(dbparams, stmt_id, exec_desc);
 
     int param_id = 0;
-    if (sqlite3_bind_int(stmt, ++param_id, dbparams.updated_zone_id)
-        != SQLITE_OK) {
-        isc_throw(DataSourceError, "failed to bind SQLite3 parameter: " <<
-                  sqlite3_errmsg(dbparams.db_));
-    }
+    proc.bindInt(++param_id, dbparams.updated_zone_id);
     const size_t column_count =
         sizeof(update_params) / sizeof(update_params[0]);
     for (int i = 0; i < column_count; ++i) {
         // The old sqlite3 data source API assumes NULL for an empty column.
         // We need to provide compatibility at least for now.
-        if (sqlite3_bind_text(stmt, ++param_id,
-                              update_params[i].empty() ? NULL :
-                              update_params[i].c_str(),
-                              -1, SQLITE_TRANSIENT) != SQLITE_OK) {
-            isc_throw(DataSourceError, "failed to bind SQLite3 parameter: " <<
-                      sqlite3_errmsg(dbparams.db_));
-        }
+        proc.bindText(++param_id, update_params[i].empty() ? NULL :
+                      update_params[i].c_str(), SQLITE_TRANSIENT);
     }
-    executer.exec();
+    proc.exec();
 }
 }
 
@@ -1218,33 +1232,16 @@ SQLite3Accessor::addRecordDiff(int zone_id, uint32_t serial,
                   << dbparameters_->updated_zone_id);
     }
 
-    sqlite3_stmt* const stmt = dbparameters_->getStatement(ADD_RECORD_DIFF);
-    StatementProcessor executer(*dbparameters_, ADD_RECORD_DIFF,
-                                "add record diff");
+    StatementProcessor proc(*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_));
-    }
+    proc.bindInt(++param_id, zone_id);
+    proc.bindInt64(++param_id, serial);
+    proc.bindInt(++param_id, operation);
     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_));
-        }
+        proc.bindText(++param_id, params[i].c_str(), SQLITE_TRANSIENT);
     }
-    executer.exec();
+    proc.exec();
 }
 
 std::string
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index 5b3a5dc..602ed24 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -3002,10 +3002,9 @@ TYPED_TEST(DatabaseClientTest, addRRsetToNewZone) {
     this->checkLastAdded(rrset_added);
 }
 
-// Below we define a set of NSEC3 update tests.   Right now this only works
-// for the mock DB, but the plan is to make it a TYPED_TEST to share the case
-// with SQLite3 implementation, too.
-
+//
+// Below we define a set of NSEC3 update tests.
+//
 // Commonly used data for NSEC3 update tests below.
 const char* const nsec3_hash = "1BB7SO0452U1QHL98UISNDD9218GELR5";
 const char* const nsec3_rdata = "1 1 12 AABBCCDD "
@@ -3045,7 +3044,7 @@ nsec3Check(const vector<ConstRRsetPtr>& expected_rrsets,
                 actual_rrsets.begin(), actual_rrsets.end());
 }
 
-TEST_F(MockDatabaseClientTest, addDeleteNSEC3InZone) {
+TYPED_TEST(DatabaseClientTest, addDeleteNSEC3InZone) {
     // Add one NSEC3 RR to the zone, delete it, and add another one.
     this->updater_ = this->client_->getUpdater(this->zname_, true);
     const ConstRRsetPtr nsec3_rrset =
@@ -3066,7 +3065,7 @@ TEST_F(MockDatabaseClientTest, addDeleteNSEC3InZone) {
                *this->current_accessor_);
 }
 
-TEST_F(MockDatabaseClientTest, addDeleteNSEC3AndRRSIGToZone) {
+TYPED_TEST(DatabaseClientTest, addDeleteNSEC3AndRRSIGToZone) {
     // Add one NSEC3 RR and its RRSIG to the zone, delete the RRSIG and add
     // a new one.
     this->updater_ = this->client_->getUpdater(this->zname_, true);
@@ -3639,10 +3638,7 @@ TYPED_TEST(DatabaseClientTest, journal) {
     this->checkJournal(expected);
 }
 
-// At the moment this only works for the mock accessor.  Once sqlite3
-// accessor supports updating NSEC3, this should be merged to the previous
-// test
-TEST_F(MockDatabaseClientTest, journalForNSEC3) {
+TYPED_TEST(DatabaseClientTest, journalForNSEC3) {
     // Similar to the previous test, but adding/deleting NSEC3 RRs, just to
     // confirm that NSEC3 is not special for managing diffs.
     const ConstRRsetPtr nsec3_rrset =



More information about the bind10-changes mailing list