[1068] use fixed sized arrays for addRecordToZone() and deleteRecordInZone()

instead of vector, mainly to be consistent with other interfaces.

src/lib/datasrc/database.h

@@ -340,12 +340,12 @@ public:
     /// in the actual derived method.
     ///
     /// \exception DataSourceError Invalid call without starting a transaction,
-    /// the columns parameter has an invalid number of elements, or other
-    /// internal database error.
+    /// or other internal database error.
     ///
-    /// \param columns A vector of strings that defines a record to be added
+    /// \param columns An array of strings that defines a record to be added
     /// to the zone.
-    virtual void addRecordToZone(const std::vector<std::string>& columns) = 0;
+    virtual void addRecordToZone(
+        const std::string (&columns)[ADD_COLUMN_COUNT]) = 0;
 
     /// Delete a single record from the zone to be updated.
     ///
@@ -377,13 +377,12 @@ public:
     /// it simply ignores the result.
     ///
     /// \exception DataSourceError Invalid call without starting a transaction,
-    /// the columns parameter has an invalid number of elements, or other
-    /// internal database error.
+    /// or other internal database error.
     ///
-    /// \param params A vector of strings that defines a record to be deleted
+    /// \param params An array of strings that defines a record to be deleted
     /// from the zone.
     virtual void deleteRecordInZone(
-        const std::vector<std::string>& params) = 0;
+        const std::string (&params)[DEL_PARAM_COUNT]) = 0;
 
     /// Commit updates to the zone.
     ///

src/lib/datasrc/sqlite3_accessor.cc

@@ -516,9 +516,10 @@ SQLite3Accessor::rollbackUpdateZone() {
 
 namespace {
 // Commonly used code sequence for adding/deleting record
+template <typename COLUMNS_TYPE>
 void
 doUpdate(SQLite3Parameters& dbparams, StatementID stmt_id,
-         const vector<string>& update_params, const char* exec_desc)
+         COLUMNS_TYPE update_params, const char* exec_desc)
 {
     sqlite3_stmt* const stmt = dbparams.statements_[stmt_id];
     StatementExecuter executer(dbparams, stmt_id, exec_desc);
@@ -529,8 +530,10 @@ doUpdate(SQLite3Parameters& dbparams, StatementID stmt_id,
         isc_throw(DataSourceError, "failed to bind SQLite3 parameter: " <<
                   sqlite3_errmsg(dbparams.db_));
     }
-    BOOST_FOREACH(const string& column, update_params) {
-        if (sqlite3_bind_text(stmt, ++param_id, column.c_str(), -1,
+    const size_t column_count =
+        sizeof(update_params) / sizeof(update_params[0]);
+    for (int i = 0; i < column_count; ++i) {
+        if (sqlite3_bind_text(stmt, ++param_id, update_params[i].c_str(), -1,
                               SQLITE_TRANSIENT) != SQLITE_OK) {
             isc_throw(DataSourceError, "failed to bind SQLite3 parameter: " <<
                       sqlite3_errmsg(dbparams.db_));
@@ -541,31 +544,23 @@ doUpdate(SQLite3Parameters& dbparams, StatementID stmt_id,
 }
 
 void
-SQLite3Accessor::addRecordToZone(const vector<string>& columns) {
+SQLite3Accessor::addRecordToZone(const string (&columns)[ADD_COLUMN_COUNT]) {
     if (!dbparameters_->updating_zone) {
         isc_throw(DataSourceError, "adding record to SQLite3 "
                   "data source without transaction");
     }
-    if (columns.size() != ADD_COLUMN_COUNT) {
-        isc_throw(DataSourceError, "adding incompatible number of columns "
-                  "to SQLite3 data source: " << columns.size());
-    }
-
-    doUpdate(*dbparameters_, ADD_RECORD, columns, "add record to zone");
+    doUpdate<const string (&)[DatabaseAccessor::ADD_COLUMN_COUNT]>(
+        *dbparameters_, ADD_RECORD, columns, "add record to zone");
 }
 
 void
-SQLite3Accessor::deleteRecordInZone(const vector<string>& params) {
+SQLite3Accessor::deleteRecordInZone(const string (&params)[DEL_PARAM_COUNT]) {
     if (!dbparameters_->updating_zone) {
         isc_throw(DataSourceError, "deleting record in SQLite3 "
                   "data source without transaction");
     }
-    if (params.size() != DEL_PARAM_COUNT) {
-        isc_throw(DataSourceError, "incompatible # of parameters for "
-                  "deleting in SQLite3 data source: " << params.size());
-    }
-
-    doUpdate(*dbparameters_, DEL_RECORD, params, "delete record from zone");
+    doUpdate<const string (&)[DatabaseAccessor::DEL_PARAM_COUNT]>(
+        *dbparameters_, DEL_RECORD, params, "delete record from zone");
 }
 
 }

src/lib/datasrc/sqlite3_accessor.h

@@ -140,9 +140,11 @@ public:
     /// considered a bug of the higher level application program.
     virtual void rollbackUpdateZone();
 
-    virtual void addRecordToZone(const std::vector<std::string>& columns);
+    virtual void addRecordToZone(
+        const std::string (&columns)[ADD_COLUMN_COUNT]);
 
-    virtual void deleteRecordInZone(const std::vector<std::string>& params);
+    virtual void deleteRecordInZone(
+        const std::string (&params)[DEL_PARAM_COUNT]);
 
     /// The SQLite3 implementation of this method returns a string starting
     /// with a fixed prefix of "sqlite3_" followed by the DB file name

src/lib/datasrc/tests/database_unittest.cc

@@ -64,8 +64,8 @@ public:
     }
     virtual void commitUpdateZone() {}
     virtual void rollbackUpdateZone() {}
-    virtual void addRecordToZone(const std::vector<std::string>&) {}
-    virtual void deleteRecordInZone(const std::vector<std::string>&) {}
+    virtual void addRecordToZone(const string (&)[ADD_COLUMN_COUNT]) {}
+    virtual void deleteRecordInZone(const string (&)[DEL_PARAM_COUNT]) {}
 
     virtual const std::string& getDBName() const {
         return (database_name_);

src/lib/datasrc/tests/sqlite3_accessor_unittest.cc

@@ -12,6 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <algorithm>
 #include <vector>
 
 #include <datasrc/sqlite3_accessor.h>
@@ -313,7 +314,8 @@ protected:
 
     int zone_id;
     std::string get_columns[DatabaseAccessor::COLUMN_COUNT];
-    std::vector<std::string> update_columns;
+    std::string add_columns[DatabaseAccessor::ADD_COLUMN_COUNT];
+    std::string del_params[DatabaseAccessor::DEL_PARAM_COUNT];
 
     vector<const char* const*> expected_stored; // placeholder for checkRecords
     vector<const char* const*> empty_stored; // indicate no corresponding data
@@ -451,9 +453,9 @@ TEST_F(SQLite3Update, addRecord) {
     checkRecords(*accessor, zone_id, "newdata.example.com.", empty_stored);
 
     zone_id = accessor->startUpdateZone("example.com.", false).second;
-    update_columns.assign(new_data,
-                          new_data + DatabaseAccessor::ADD_COLUMN_COUNT);
-    accessor->addRecordToZone(update_columns);
+    copy(new_data, new_data + DatabaseAccessor::ADD_COLUMN_COUNT,
+         add_columns);
+    accessor->addRecordToZone(add_columns);
 
     expected_stored.clear();
     expected_stored.push_back(new_data);
@@ -466,9 +468,9 @@ TEST_F(SQLite3Update, addRecord) {
 
 TEST_F(SQLite3Update, addThenRollback) {
     zone_id = accessor->startUpdateZone("example.com.", false).second;
-    update_columns.assign(new_data,
-                          new_data + DatabaseAccessor::ADD_COLUMN_COUNT);
-    accessor->addRecordToZone(update_columns);
+    copy(new_data, new_data + DatabaseAccessor::ADD_COLUMN_COUNT,
+         add_columns);
+    accessor->addRecordToZone(add_columns);
 
     expected_stored.clear();
     expected_stored.push_back(new_data);
@@ -489,28 +491,17 @@ TEST_F(SQLite3Update, duplicateAdd) {
 
     // Adding exactly the same data.  As this backend is "dumb", another
     // row of the same content will be inserted.
-    update_columns.assign(dup_data,
-                          dup_data + DatabaseAccessor::ADD_COLUMN_COUNT);
+    copy(dup_data, dup_data + DatabaseAccessor::ADD_COLUMN_COUNT,
+         add_columns);
     zone_id = accessor->startUpdateZone("example.com.", false).second;
-    accessor->addRecordToZone(update_columns);
+    accessor->addRecordToZone(add_columns);
     expected_stored.push_back(dup_data);
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
 }
 
 TEST_F(SQLite3Update, invalidAdd) {
     // An attempt of add before an explicit start of transaction
-    EXPECT_THROW(accessor->addRecordToZone(update_columns), DataSourceError);
-
-    // Short column vector
-    update_columns.clear();
-    zone_id = accessor->startUpdateZone("example.com.", false).second;
-    EXPECT_THROW(accessor->addRecordToZone(update_columns), DataSourceError);
-
-    // Too many columns
-    for (int i = 0; i < DatabaseAccessor::ADD_COLUMN_COUNT + 1; ++i) {
-        update_columns.push_back("");
-    }
-    EXPECT_THROW(accessor->addRecordToZone(update_columns), DataSourceError);
+    EXPECT_THROW(accessor->addRecordToZone(add_columns), DataSourceError);
 }
 
 TEST_F(SQLite3Update, deleteRecord) {
@@ -518,9 +509,9 @@ TEST_F(SQLite3Update, deleteRecord) {
 
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
 
-    update_columns.assign(deleted_data, deleted_data +
-                          DatabaseAccessor::DEL_PARAM_COUNT);
-    accessor->deleteRecordInZone(update_columns);
+    copy(deleted_data, deleted_data + DatabaseAccessor::DEL_PARAM_COUNT,
+         del_params);
+    accessor->deleteRecordInZone(del_params);
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
 
     // Commit the change, and confirm the deleted data still isn't there.
@@ -531,9 +522,9 @@ TEST_F(SQLite3Update, deleteRecord) {
 TEST_F(SQLite3Update, deleteThenRollback) {
     zone_id = accessor->startUpdateZone("example.com.", false).second;
 
-    update_columns.assign(deleted_data, deleted_data +
-                          DatabaseAccessor::DEL_PARAM_COUNT);
-    accessor->deleteRecordInZone(update_columns);
+    copy(deleted_data, deleted_data + DatabaseAccessor::DEL_PARAM_COUNT,
+         del_params);
+    accessor->deleteRecordInZone(del_params);
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
 
     // Rollback the change, and confirm the data still exists.
@@ -543,47 +534,36 @@ TEST_F(SQLite3Update, deleteThenRollback) {
 
 TEST_F(SQLite3Update, deleteNonexistent) {
     zone_id = accessor->startUpdateZone("example.com.", false).second;
-    update_columns.assign(deleted_data, deleted_data +
-                          DatabaseAccessor::DEL_PARAM_COUNT);
+    copy(deleted_data, deleted_data + DatabaseAccessor::DEL_PARAM_COUNT,
+         del_params);
 
     // Replace the name with a non existent one, then try to delete it.
     // nothing should happen.
-    update_columns[0] = "no-such-name.example.com.";
+    del_params[DatabaseAccessor::DEL_NAME] = "no-such-name.example.com.";
     checkRecords(*accessor, zone_id, "no-such-name.example.com.",
                  empty_stored);
-    accessor->deleteRecordInZone(update_columns);
+    accessor->deleteRecordInZone(del_params);
     checkRecords(*accessor, zone_id, "no-such-name.example.com.",
                  empty_stored);
 
     // Name exists but the RR type is different.  Delete attempt shouldn't
     // delete only by name.
-    update_columns.assign(deleted_data, deleted_data +
-                          DatabaseAccessor::DEL_PARAM_COUNT);
-    update_columns[1] = "AAAA";
-    accessor->deleteRecordInZone(update_columns);
+    copy(deleted_data, deleted_data + DatabaseAccessor::DEL_PARAM_COUNT,
+         del_params);
+    del_params[DatabaseAccessor::DEL_TYPE] = "AAAA";
+    accessor->deleteRecordInZone(del_params);
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
 
     // Similar to the previous case, but RDATA is different.
-    update_columns.assign(deleted_data, deleted_data +
-                          DatabaseAccessor::DEL_PARAM_COUNT);
-    update_columns[2] = "192.0.2.2";
-    accessor->deleteRecordInZone(update_columns);
+    copy(deleted_data, deleted_data + DatabaseAccessor::DEL_PARAM_COUNT,
+         del_params);
+    del_params[DatabaseAccessor::DEL_RDATA] = "192.0.2.2";
+    accessor->deleteRecordInZone(del_params);
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
 }
 
 TEST_F(SQLite3Update, invalidDelete) {
     // An attempt of delete before an explicit start of transaction
-    EXPECT_THROW(accessor->deleteRecordInZone(update_columns), DataSourceError);
-
-    // Short column vector
-    update_columns.clear();
-    zone_id = accessor->startUpdateZone("example.com.", false).second;
-    EXPECT_THROW(accessor->deleteRecordInZone(update_columns), DataSourceError);
-
-    // Too many parameters
-    for (int i = 0; i < DatabaseAccessor::DEL_PARAM_COUNT + 1; ++i) {
-        update_columns.push_back("");
-    }
-    EXPECT_THROW(accessor->deleteRecordInZone(update_columns), DataSourceError);
+    EXPECT_THROW(accessor->deleteRecordInZone(del_params), DataSourceError);
 }
 } // end anonymous namespace