From: Michal 'vorner' Vaner Date: Wed, 10 Apr 2013 13:31:43 +0000 (+0200) Subject: [2877] Don't overload interface X-Git-Tag: bind10-1.1.0beta2-release~8^2~28^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=ccf16f3b82314b94b3aaffe2f58eac7464f404a0;p=thirdparty%2Fkea.git [2877] Don't overload interface Provide separate list of parameter strings for deleting NSEC3 records. It is cleaner to have separate enum for listing the parameters needed to delete NSEC3 than re-using generic delete parameters and having the hash passed twice. --- diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc index aa8c939c55..2cff63edb6 100644 --- a/src/lib/datasrc/database.cc +++ b/src/lib/datasrc/database.cc @@ -1707,15 +1707,16 @@ DatabaseUpdater::deleteRRset(const AbstractRRset& rrset) { LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_DELETEDIFF). arg(cvtr.getName()).arg(cvtr.getType()).arg(rdata_txt); } - const string params[Accessor::DEL_PARAM_COUNT] = - { nsec3_type ? cvtr.getNSEC3Name() : cvtr.getName(), - cvtr.getType(), rdata_txt, - nsec3_type ? cvtr.getNSEC3Name() : cvtr.getRevName() }; if (nsec3_type) { + const string params[Accessor::DEL_NSEC3_PARAM_COUNT] = + { cvtr.getNSEC3Name(), cvtr.getType(), rdata_txt }; accessor_->deleteNSEC3RecordInZone(params); LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_DELETENSEC3). arg(cvtr.getNSEC3Name()).arg(rdata_txt); } else { + const string params[Accessor::DEL_PARAM_COUNT] = + { cvtr.getName(), cvtr.getType(), rdata_txt, + cvtr.getRevName() }; accessor_->deleteRecordInZone(params); LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_DELETERR). arg(cvtr.getName()).arg(cvtr.getType()).arg(rdata_txt); diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h index 88a4140039..299d322287 100644 --- a/src/lib/datasrc/database.h +++ b/src/lib/datasrc/database.h @@ -116,8 +116,7 @@ public: ADD_NSEC3_COLUMN_COUNT = 4 ///< Number of columns }; - /// \brief Definitions of the fields to be passed to deleteRecordInZone() - /// and deleteNSEC3RecordInZone() + /// \brief Definitions of the fields to be passed to deleteRecordInZone(). /// /// Each derived implementation of deleteRecordInZone() should expect /// the "params" array to be filled with the values as described in this @@ -132,16 +131,29 @@ public: /// in that sense redundant. But both are provided so the underlying /// implementation doesn't have to deal with DNS level concepts. enum DeleteRecordParams { - DEL_NAME = 0, ///< The owner name of the record (a domain name) - ///< or the hash label for deleteNSEC3RecordInZone() + DEL_NAME = 0, ///< The owner name of the record (a domain name). DEL_TYPE = 1, ///< The RRType of the record (A/NS/TXT etc.) DEL_RDATA = 2, ///< Full text representation of the record's RDATA DEL_RNAME = 3, ///< As DEL_NAME, but with the labels of domain name - ///< in reverse order (eg. org.example.). With NSEC3, - ///< it is the same as DEL_NAME. + ///< in reverse order (eg. org.example.). DEL_PARAM_COUNT = 4 ///< Number of parameters }; + /// \brief Definitions of the fields to be passed to + /// deleteNSEC3RecordInZone(). + /// + /// Each derived implementation of deleteNSEC3RecordInZone() should expect + /// the "params" array to be filled with the values as described in this + /// enumeration, in this order. + enum DeleteNSEC3RecordParams { + DEL_NSEC3_HASH = 0, ///< The hash (1st) label of the owren name, + ///< excluding the dot character. + DEL_NSEC3_TYPE = 1, ///< The type of RR. Either RRSIG or NSEC3. + DEL_NSEC3_RDATA = 2, ///< Full text representation of the record's RDATA. + ///< Must match the one in the database. + DEL_NSEC3_PARAM_COUNT = 3 ///< Number of parameters. + }; + /// \brief Operation mode when adding a record diff. /// /// This is used as the "operation" parameter value of addRecordDiff(). @@ -588,11 +600,8 @@ public: /// \c addRecordToZone() and \c addNSEC3RecordToZone(), and the same /// notes apply to this method. /// - /// This method uses the same set of parameters to specify the record - /// to be deleted as \c deleteRecordInZone(), but the \c DEL_NAME column - /// is expected to only store the hash label of the owner name. - /// This is the same as \c ADD_NSEC3_HASH column for - /// \c addNSEC3RecordToZone(). + /// This method uses the \c DeleteNSEC3RecordParams enum to specify the + /// values. /// /// \exception DataSourceError Invalid call without starting a transaction, /// or other internal database error. @@ -602,7 +611,7 @@ public: /// \param params An array of strings that defines a record to be deleted /// from the NSEC3 namespace of the zone. virtual void deleteNSEC3RecordInZone( - const std::string (¶ms)[DEL_PARAM_COUNT]) = 0; + const std::string (¶ms)[DEL_NSEC3_PARAM_COUNT]) = 0; /// \brief Start a general transaction. /// diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc index 1653168521..1a90fa0986 100644 --- a/src/lib/datasrc/sqlite3_accessor.cc +++ b/src/lib/datasrc/sqlite3_accessor.cc @@ -1311,20 +1311,14 @@ SQLite3Accessor::deleteRecordInZone(const string (¶ms)[DEL_PARAM_COUNT]) { void SQLite3Accessor::deleteNSEC3RecordInZone( - const string (¶ms)[DEL_PARAM_COUNT]) + const string (¶ms)[DEL_NSEC3_PARAM_COUNT]) { if (!dbparameters_->updating_zone) { isc_throw(DataSourceError, "deleting NSEC3-related record in SQLite3 " "data source without transaction"); } - const size_t SQLITE3_DEL_PARAM_COUNT = DEL_PARAM_COUNT - 1; - const string sqlite3_params[SQLITE3_DEL_PARAM_COUNT] = { - params[DEL_NAME], - params[DEL_TYPE], - params[DEL_RDATA] - }; - doUpdate( - *dbparameters_, DEL_NSEC3_RECORD, sqlite3_params, + doUpdate( + *dbparameters_, DEL_NSEC3_RECORD, params, "delete NSEC3 record from zone"); } diff --git a/src/lib/datasrc/sqlite3_accessor.h b/src/lib/datasrc/sqlite3_accessor.h index 4d05ef6aaf..3d457b3fa5 100644 --- a/src/lib/datasrc/sqlite3_accessor.h +++ b/src/lib/datasrc/sqlite3_accessor.h @@ -230,7 +230,7 @@ public: const std::string (¶ms)[DEL_PARAM_COUNT]); virtual void deleteNSEC3RecordInZone( - const std::string (¶ms)[DEL_PARAM_COUNT]); + const std::string (¶ms)[DEL_NSEC3_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 diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc index cac5e337b7..1b9fbb6c54 100644 --- a/src/lib/datasrc/tests/database_unittest.cc +++ b/src/lib/datasrc/tests/database_unittest.cc @@ -167,7 +167,8 @@ public: virtual void addNSEC3RecordToZone(const string (&)[ADD_NSEC3_COLUMN_COUNT]) {} virtual void deleteRecordInZone(const string (&)[DEL_PARAM_COUNT]) {} - virtual void deleteNSEC3RecordInZone(const string (&)[DEL_PARAM_COUNT]) {} + virtual void deleteNSEC3RecordInZone(const string + (&)[DEL_NSEC3_PARAM_COUNT]) {} virtual void addRecordDiff(int, uint32_t, DiffOperation, const std::string (&)[DIFF_PARAM_COUNT]) {} @@ -634,8 +635,9 @@ private: }; // Common subroutine for deleteRecordinZone and deleteNSEC3RecordInZone. + template void deleteRecord(Domains& domains, - const string (¶ms)[DEL_PARAM_COUNT]) + const string (¶ms)[param_count]) { vector >& records = domains[params[DatabaseAccessor::DEL_NAME]]; @@ -655,7 +657,7 @@ public: } virtual void deleteNSEC3RecordInZone( - const string (¶ms)[DEL_PARAM_COUNT]) + const string (¶ms)[DEL_NSEC3_PARAM_COUNT]) { deleteRecord(*update_nsec3_namespace_, params); } diff --git a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc index f8c1d78ac7..786218a11b 100644 --- a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc +++ b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc @@ -823,8 +823,7 @@ const char* const nsec3_sig_data[DatabaseAccessor::ADD_NSEC3_COLUMN_COUNT] = { const char* const nsec3_deleted_data[] = { // Delete parameters for nsec3_data apex_hash, nsec3_data[DatabaseAccessor::ADD_NSEC3_TYPE], - nsec3_data[DatabaseAccessor::ADD_NSEC3_RDATA], - apex_hash + nsec3_data[DatabaseAccessor::ADD_NSEC3_RDATA] }; class SQLite3Update : public SQLite3AccessorTest { @@ -854,6 +853,7 @@ protected: std::string add_columns[DatabaseAccessor::ADD_COLUMN_COUNT]; std::string add_nsec3_columns[DatabaseAccessor::ADD_NSEC3_COLUMN_COUNT]; std::string del_params[DatabaseAccessor::DEL_PARAM_COUNT]; + std::string del_nsec3_params[DatabaseAccessor::DEL_NSEC3_PARAM_COUNT]; std::string diff_params[DatabaseAccessor::DIFF_PARAM_COUNT]; vector expected_stored; // placeholder for checkRecords @@ -1193,8 +1193,9 @@ TEST_F(SQLite3Update, deleteNSEC3Record) { // Delete it, and confirm that. copy(nsec3_deleted_data, - nsec3_deleted_data + DatabaseAccessor::DEL_PARAM_COUNT, del_params); - accessor->deleteNSEC3RecordInZone(del_params); + nsec3_deleted_data + DatabaseAccessor::DEL_NSEC3_PARAM_COUNT, + del_nsec3_params); + accessor->deleteNSEC3RecordInZone(del_nsec3_params); checkNSEC3Records(*accessor, zone_id, apex_hash, empty_stored); // Commit the change, and confirm the deleted data still isn't there. @@ -1251,7 +1252,7 @@ TEST_F(SQLite3Update, invalidDelete) { EXPECT_THROW(accessor->deleteRecordInZone(del_params), DataSourceError); // Same for NSEC3. - EXPECT_THROW(accessor->deleteNSEC3RecordInZone(del_params), + EXPECT_THROW(accessor->deleteNSEC3RecordInZone(del_nsec3_params), DataSourceError); }