From: JINMEI Tatuya Date: Thu, 25 Aug 2011 00:09:00 +0000 (-0700) Subject: [1068] use fixed sized arrays for addRecordToZone() and deleteRecordInZone() X-Git-Tag: perftcpdns_before_epoll~191^2~17^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=df047c5ccb5c81f9a3d36f7fc38a19bc7c8f2ac2;p=thirdparty%2Fkea.git [1068] use fixed sized arrays for addRecordToZone() and deleteRecordInZone() instead of vector, mainly to be consistent with other interfaces. --- diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h index 712e6026aa..efa370663a 100644 --- a/src/lib/datasrc/database.h +++ b/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& 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& params) = 0; + const std::string (¶ms)[DEL_PARAM_COUNT]) = 0; /// Commit updates to the zone. /// diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc index ab34c3b94e..0e6ffdccef 100644 --- a/src/lib/datasrc/sqlite3_accessor.cc +++ b/src/lib/datasrc/sqlite3_accessor.cc @@ -516,9 +516,10 @@ SQLite3Accessor::rollbackUpdateZone() { namespace { // Commonly used code sequence for adding/deleting record +template void doUpdate(SQLite3Parameters& dbparams, StatementID stmt_id, - const vector& 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& 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( + *dbparameters_, ADD_RECORD, columns, "add record to zone"); } void -SQLite3Accessor::deleteRecordInZone(const vector& params) { +SQLite3Accessor::deleteRecordInZone(const string (¶ms)[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( + *dbparameters_, DEL_RECORD, params, "delete record from zone"); } } diff --git a/src/lib/datasrc/sqlite3_accessor.h b/src/lib/datasrc/sqlite3_accessor.h index 471f6a1e80..fd3cdfdf11 100644 --- a/src/lib/datasrc/sqlite3_accessor.h +++ b/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& columns); + virtual void addRecordToZone( + const std::string (&columns)[ADD_COLUMN_COUNT]); - virtual void deleteRecordInZone(const std::vector& params); + virtual void deleteRecordInZone( + const std::string (¶ms)[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 diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc index f832b1d652..c3d7b12cc2 100644 --- a/src/lib/datasrc/tests/database_unittest.cc +++ b/src/lib/datasrc/tests/database_unittest.cc @@ -64,8 +64,8 @@ public: } virtual void commitUpdateZone() {} virtual void rollbackUpdateZone() {} - virtual void addRecordToZone(const std::vector&) {} - virtual void deleteRecordInZone(const std::vector&) {} + 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_); diff --git a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc index 11a896d845..5725c81219 100644 --- a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc +++ b/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 #include #include @@ -313,7 +314,8 @@ protected: int zone_id; std::string get_columns[DatabaseAccessor::COLUMN_COUNT]; - std::vector update_columns; + std::string add_columns[DatabaseAccessor::ADD_COLUMN_COUNT]; + std::string del_params[DatabaseAccessor::DEL_PARAM_COUNT]; vector expected_stored; // placeholder for checkRecords vector 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