From: Stephen Morris Date: Tue, 6 Nov 2012 12:58:10 +0000 (+0000) Subject: [2342] Abstracted common MySQL binding code into a separate method. X-Git-Tag: trac2487_base~1^2~40 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3dc78f0e8380b2d5e21c8e07f392a8dde4c3d456;p=thirdparty%2Fkea.git [2342] Abstracted common MySQL binding code into a separate method. --- diff --git a/src/lib/dhcp/mysql_lease_mgr.cc b/src/lib/dhcp/mysql_lease_mgr.cc index d3523e9a0f..6e3e308f88 100644 --- a/src/lib/dhcp/mysql_lease_mgr.cc +++ b/src/lib/dhcp/mysql_lease_mgr.cc @@ -95,21 +95,23 @@ TaggedStatement tagged_statements[] = { namespace isc { namespace dhcp { + + /// @brief Exchange MySQL and Lease6 Data /// /// On any MySQL operation, arrays of MYSQL_BIND structures must be built to /// describe the parameters in the prepared statements. Where information is /// inserted or retrieved - INSERT, UPDATE, SELECT - a large amount of that /// structure is identical - it defines data values in the Lease6 structure. -/// -/// This class handles the creation of that array. For maximum flexibility, -/// the data is appended to an array of MYSQL_BIND elemements, so allowing -/// additional elements to be prepended/appended to it. +/// This class handles the creation of that array. /// /// Owing to the MySQL API, the process requires some intermediate variables /// to hold things like length etc. This object holds the intermediate /// variables as well. +// Note - there are no unit tests for this class. It is tested indirectly +// in all MySqlLeaseMgr::xxx6() calls where it is used. + class MySqlLease6Exchange { public: /// @brief Constructor @@ -117,7 +119,8 @@ public: /// Apart from the initialization of false_ and true_, the other /// initializations are to satisfy cppcheck: none are really needed, as all /// variables are initialized/set in the methods. - MySqlLease6Exchange() : addr6_length_(0), duid_length_(0), false_(0), true_(1) { + MySqlLease6Exchange() : addr6_length_(0), duid_length_(0), + false_(0), true_(1) { memset(addr6_buffer_, 0, sizeof(addr6_buffer_)); memset(duid_buffer_, 0, sizeof(duid_buffer_)); } @@ -126,12 +129,10 @@ public: /// /// Fills in the MYSQL_BIND objects for the Lease6 passed to it. /// - /// @param lease Lease object to be added to the database - /// @param bindvec Vector of MySQL BIND objects: the elements describing the - /// lease are appended to this vector. The data added to the vector - /// only remain valid while both the lease and this object are valid. - void - createBindForSend(const Lease6Ptr& lease, std::vector& bindvec) { + /// @param lease Lease object to be added to the database. + /// + /// @return Vector of MySQL BIND objects representing the data to be added. + std::vector createBindForSend(const Lease6Ptr& lease) { // Store lease object to ensure it remains valid. lease_ = lease; @@ -209,7 +210,7 @@ public: // Add the data to the vector. Note the end element is one after the // end of the array. - bindvec.insert(bindvec.end(), &bind_[0], &bind_[9]); + return (std::vector(&bind_[0], &bind_[9])); } /// @brief Create BIND array to receive data @@ -218,11 +219,8 @@ public: /// After data is successfully received, getLeaseData() is used to copy /// it to a Lease6 object. /// - /// @param bindvec Vector of MySQL BIND objects: the elements describing the - /// lease are appended to this vector. The data added to the vector - /// only remain valid while both the lease and this object are valid. - - void createBindForReceive(std::vector& bindvec) { + /// @return Vector of MySQL BIND objects. + std::vector createBindForReceive() { // Ensure both the array of MYSQL_BIND structures and the error array // are clear. @@ -283,7 +281,7 @@ public: bind_[7].buffer = reinterpret_cast(&iaid_); bind_[7].is_unsigned = true_; bind_[7].error = &error_[7]; - + // prefix_len: unsigned tinyint bind_[8].buffer_type = MYSQL_TYPE_TINY; bind_[8].buffer = reinterpret_cast(&prefixlen_); @@ -292,7 +290,7 @@ public: // Add the data to the vector. Note the end element is one after the // end of the array. - bindvec.insert(bindvec.end(), &bind_[0], &bind_[9]); + return(std::vector(&bind_[0], &bind_[9])); } /// @brief Copy Received Data into Lease6 Object @@ -353,7 +351,7 @@ public: } private: - // Note: All array legths are equal to the corresponding variable in the + // Note: All array lengths are equal to the corresponding variable in the // schema. std::string addr6_; ///< String form of address char addr6_buffer_[ADDRESS6_TEXT_MAX_LEN]; ///< Character @@ -377,6 +375,8 @@ private: }; +// MySqlLeaseMgr Methods + MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters) : LeaseMgr(parameters), mysql_(NULL) { @@ -397,8 +397,13 @@ MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters) // Prepare all statements likely to be used. prepareStatements(); + + // Create the exchange object for use in exchanging data between the + // program and the database. + exchange6_.reset(new MySqlLease6Exchange()); } + MySqlLeaseMgr::~MySqlLeaseMgr() { // Free up the prepared statements, ignoring errors. (What would we do // about them - we're destroying this object and are not really concerned @@ -470,6 +475,7 @@ MySqlLeaseMgr::convertFromDatabaseTime(const MYSQL_TIME& expire, cltt = mktime(&expire_tm) - valid_lifetime; } + void MySqlLeaseMgr::openDatabase() { @@ -539,6 +545,7 @@ MySqlLeaseMgr::openDatabase() { } } + void MySqlLeaseMgr::prepareStatement(StatementIndex index, const char* text) { // Validate that there is space for the statement in the statements array @@ -565,6 +572,7 @@ MySqlLeaseMgr::prepareStatement(StatementIndex index, const char* text) { } } + void MySqlLeaseMgr::prepareStatements() { // Allocate space for all statements @@ -581,6 +589,7 @@ MySqlLeaseMgr::prepareStatements() { } } + bool MySqlLeaseMgr::addLease(const Lease4Ptr& /* lease */) { isc_throw(NotImplemented, "MySqlLeaseMgr::addLease(const Lease4Ptr&) " @@ -588,14 +597,13 @@ MySqlLeaseMgr::addLease(const Lease4Ptr& /* lease */) { return (false); } + bool MySqlLeaseMgr::addLease(const Lease6Ptr& lease) { const StatementIndex stindex = INSERT_LEASE6; // Create the MYSQL_BIND array for the lease - MySqlLease6Exchange exchange; - std::vector bind; - exchange.createBindForSend(lease, bind); + std::vector bind = exchange6_->createBindForSend(lease); // Bind the parameters to the statement int status = mysql_stmt_bind_param(statements_[stindex], &bind[0]); @@ -618,6 +626,7 @@ MySqlLeaseMgr::addLease(const Lease6Ptr& lease) { return (true); } + Lease4Ptr MySqlLeaseMgr::getLease4(const isc::asiolink::IOAddress& /* addr */, SubnetID /* subnet_id */) const { @@ -626,6 +635,7 @@ MySqlLeaseMgr::getLease4(const isc::asiolink::IOAddress& /* addr */, return (Lease4Ptr()); } + Lease4Ptr MySqlLeaseMgr::getLease4(const isc::asiolink::IOAddress& /* addr */) const { isc_throw(NotImplemented, "MySqlLeaseMgr::getLease4(const IOAddress&) " @@ -633,6 +643,7 @@ MySqlLeaseMgr::getLease4(const isc::asiolink::IOAddress& /* addr */) const { return (Lease4Ptr()); } + Lease4Collection MySqlLeaseMgr::getLease4(const HWAddr& /* hwaddr */) const { isc_throw(NotImplemented, "MySqlLeaseMgr::getLease4(const HWAddr&) " @@ -640,6 +651,7 @@ MySqlLeaseMgr::getLease4(const HWAddr& /* hwaddr */) const { return (Lease4Collection()); } + Lease4Ptr MySqlLeaseMgr::getLease4(const HWAddr& /* hwaddr */, SubnetID /* subnet_id */) const { @@ -648,6 +660,7 @@ MySqlLeaseMgr::getLease4(const HWAddr& /* hwaddr */, return (Lease4Ptr()); } + Lease4Collection MySqlLeaseMgr::getLease4(const ClientId& /* clientid */) const { isc_throw(NotImplemented, "MySqlLeaseMgr::getLease4(const ClientID&) " @@ -655,6 +668,7 @@ MySqlLeaseMgr::getLease4(const ClientId& /* clientid */) const { return (Lease4Collection()); } + Lease4Ptr MySqlLeaseMgr::getLease4(const ClientId& /* clientid */, SubnetID /* subnet_id */) const { @@ -663,6 +677,32 @@ MySqlLeaseMgr::getLease4(const ClientId& /* clientid */, return (Lease4Ptr()); } + +// A convenience function used in the various getLease6() methods. It binds +// the selection parameters to the prepared statement, and binds the variables +// that will receive the data. These are stored in the MySqlLease6Exchange +// object associated with the lease manager and converted to a Lease6 object +// when retrieved. +void +MySqlLeaseMgr::bind6AndExecute(StatementIndex stindex, MYSQL_BIND* inbind) const { + + // Bind the input parameters to the statement + int status = mysql_stmt_bind_param(statements_[stindex], inbind); + checkError(status, stindex, "unable to bind WHERE clause parameter"); + + // Set up the SELECT clause + std::vector outbind = exchange6_->createBindForReceive(); + + // Bind the output parameters to the statement + status = mysql_stmt_bind_result(statements_[stindex], &outbind[0]); + checkError(status, stindex, "unable to bind SELECT caluse parameters"); + + // Execute the statement + status = mysql_stmt_execute(statements_[stindex]); + checkError(status, stindex, "unable to execute"); +} + + Lease6Ptr MySqlLeaseMgr::getLease6(const isc::asiolink::IOAddress& addr) const { const StatementIndex stindex = GET_LEASE6_ADDR; @@ -679,30 +719,17 @@ MySqlLeaseMgr::getLease6(const isc::asiolink::IOAddress& addr) const { inbind[0].buffer_length = addr6_length; inbind[0].length = &addr6_length; - // Bind the input parameters to the statement - int status = mysql_stmt_bind_param(statements_[stindex], inbind); - checkError(status, stindex, "unable to bind WHERE clause parameter"); - - // Set up the SELECT clause - MySqlLease6Exchange exchange; - std::vector outbind; - exchange.createBindForReceive(outbind); - - // Bind the output parameters to the statement - status = mysql_stmt_bind_result(statements_[stindex], &outbind[0]); - checkError(status, stindex, "unable to bind SELECT caluse parameters"); - - // Execute the statement - status = mysql_stmt_execute(statements_[stindex]); - checkError(status, stindex, "unable to execute"); + // Bind the input parameters to the statement and bind the output + // to fields in the exchange object, then execute the prepared statement. + bind6AndExecute(stindex, inbind); // Fetch the data. - status = mysql_stmt_fetch(statements_[stindex]); + int status = mysql_stmt_fetch(statements_[stindex]); Lease6Ptr result; if (status == 0) { try { - result = exchange.getLeaseData(); + result = exchange6_->getLeaseData(); } catch (const isc::BadValue& ex) { // Free up result set. @@ -724,7 +751,7 @@ MySqlLeaseMgr::getLease6(const isc::asiolink::IOAddress& addr) const { // @TODO Handle truncation // We are ignoring truncation for now, so the only other result is // no data was found. In that case, we return a null Lease6 structure. - // This has already been set, so the action is a no-op. + // This has already been set, so no action is needed. } // Free data structures associated with information returned. @@ -732,6 +759,7 @@ MySqlLeaseMgr::getLease6(const isc::asiolink::IOAddress& addr) const { return (result); } + Lease6Collection MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid) const { const StatementIndex stindex = GET_LEASE6_DUID_IAID; @@ -742,7 +770,7 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid) const { // DUID. The complex casting is needed to obtain the "const" vector of // uint8_t from the DUID, point to the start of it (discarding the - // "const"ness) and finally casing it to "char*" for the MySQL buffer + // "const"ness) and finally casting it to "char*" for the MySQL buffer // element. const vector& duid_vector = duid.getDuid(); unsigned long duid_length = duid_vector.size(); @@ -757,26 +785,13 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid) const { inbind[1].buffer = reinterpret_cast(&iaid); inbind[1].is_unsigned = static_cast(1); - // Bind the input parameters to the statement - int status = mysql_stmt_bind_param(statements_[stindex], inbind); - checkError(status, stindex, "unable to bind WHERE clause parameter"); - - // Set up the SELECT clause - MySqlLease6Exchange exchange; - std::vector outbind; - exchange.createBindForReceive(outbind); + // Bind the input parameters to the statement and bind the output + // to fields in the exchange object, then execute the prepared statement. + bind6AndExecute(stindex, inbind); - // Bind the output parameters to the statement - status = mysql_stmt_bind_result(statements_[stindex], &outbind[0]); - checkError(status, stindex, "unable to bind SELECT clause parameters"); - - // Execute the query. - status = mysql_stmt_execute(statements_[stindex]); - checkError(status, stindex, "unable to execute"); - - // Ensure that all the lease information is retrieved in one go to avoid overhead - // of going back and forth between client and server. - status = mysql_stmt_store_result(statements_[stindex]); + // Ensure that all the lease information is retrieved in one go to avoid + // overhead of going back and forth between client and server. + int status = mysql_stmt_store_result(statements_[stindex]); checkError(status, stindex, "unable to set up for storing all results"); // Fetch the data. There could be multiple rows, so we need to iterate @@ -784,7 +799,7 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid) const { Lease6Collection result; while ((status = mysql_stmt_fetch(statements_[stindex])) == 0) { try { - Lease6Ptr lease = exchange.getLeaseData(); + Lease6Ptr lease = exchange6_->getLeaseData(); result.push_back(lease); } catch (const isc::BadValue& ex) { @@ -811,6 +826,7 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid) const { return (result); } + Lease6Ptr MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid, SubnetID subnet_id) const { @@ -842,31 +858,18 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid, inbind[2].buffer = reinterpret_cast(&subnet_id); inbind[2].is_unsigned = static_cast(1); - // Bind the input parameters to the statement - int status = mysql_stmt_bind_param(statements_[stindex], inbind); - checkError(status, stindex, "unable to bind WHERE clause parameter"); - - // Set up the SELECT clause - MySqlLease6Exchange exchange; - std::vector outbind; - exchange.createBindForReceive(outbind); - - // Bind the output parameters to the statement - status = mysql_stmt_bind_result(statements_[stindex], &outbind[0]); - checkError(status, stindex, "unable to bind SELECT clause parameters"); - - // Execute the query. - status = mysql_stmt_execute(statements_[stindex]); - checkError(status, stindex, "unable to execute"); + // Bind the input parameters to the statement and bind the output + // to fields in the exchange object, then execute the prepared statement. + bind6AndExecute(stindex, inbind); Lease6Ptr result; - status = mysql_stmt_fetch(statements_[stindex]); + int status = mysql_stmt_fetch(statements_[stindex]); if (status == 0) { try { - result = exchange.getLeaseData(); + result = exchange6_->getLeaseData(); - // TODO: check for more than one row returned. At present, just ignore - // the excess and take the first. + // TODO: check for more than one row returned. At present, just + // ignore the excess and take the first. } catch (const isc::BadValue& ex) { // Free up result set. @@ -897,20 +900,20 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid, return (result); } + void MySqlLeaseMgr::updateLease4(const Lease4Ptr& /* lease4 */) { isc_throw(NotImplemented, "MySqlLeaseMgr::updateLease4(const Lease4Ptr&) " "not implemented yet"); } + void MySqlLeaseMgr::updateLease6(const Lease6Ptr& lease) { const StatementIndex stindex = UPDATE_LEASE6; // Create the MYSQL_BIND array for the data being updated - MySqlLease6Exchange exchange; - std::vector bind; - exchange.createBindForSend(lease, bind); + std::vector bind = exchange6_->createBindForSend(lease); // Set up the WHERE clause value MYSQL_BIND where; @@ -947,6 +950,7 @@ MySqlLeaseMgr::updateLease6(const Lease6Ptr& lease) { } } + bool MySqlLeaseMgr::deleteLease4(const isc::asiolink::IOAddress& /* addr */) { isc_throw(NotImplemented, "MySqlLeaseMgr::deleteLease4(const IOAddress&) " @@ -954,6 +958,7 @@ MySqlLeaseMgr::deleteLease4(const isc::asiolink::IOAddress& /* addr */) { return (false); } + bool MySqlLeaseMgr::deleteLease6(const isc::asiolink::IOAddress& addr) { const StatementIndex stindex = DELETE_LEASE6; @@ -981,10 +986,9 @@ MySqlLeaseMgr::deleteLease6(const isc::asiolink::IOAddress& addr) { // See how many rows were affected. Note that the statement may delete // multiple rows. return (mysql_stmt_affected_rows(statements_[stindex]) > 0); - - return false; } + std::string MySqlLeaseMgr::getName() const { std::string name = ""; @@ -996,11 +1000,13 @@ MySqlLeaseMgr::getName() const { return (name); } + std::string MySqlLeaseMgr::getDescription() const { - return (std::string("")); + return (std::string("MySQL Database")); } + std::pair MySqlLeaseMgr::getVersion() const { const StatementIndex stindex = GET_VERSION; @@ -1048,6 +1054,7 @@ MySqlLeaseMgr::getVersion() const { return (std::make_pair(major, minor)); } + void MySqlLeaseMgr::commit() { if (mysql_commit(mysql_) != 0) { @@ -1055,6 +1062,7 @@ MySqlLeaseMgr::commit() { } } + void MySqlLeaseMgr::rollback() { if (mysql_rollback(mysql_) != 0) { diff --git a/src/lib/dhcp/mysql_lease_mgr.h b/src/lib/dhcp/mysql_lease_mgr.h index 6a542f62ae..6b5773ba23 100644 --- a/src/lib/dhcp/mysql_lease_mgr.h +++ b/src/lib/dhcp/mysql_lease_mgr.h @@ -16,6 +16,7 @@ #define MYSQL_LEASE_MGR_H #include +#include #include #include @@ -27,6 +28,12 @@ namespace dhcp { const uint32_t CURRENT_VERSION_VERSION = 0; const uint32_t CURRENT_VERSION_MINOR = 1; + +// Forward declaration of the Lease6 exchange object. This class is defined +// in the .cc file. +class MySqlLease6Exchange; + + /// @brief MySQL Lease Manager /// /// This is a concrete API for the backend for the MySQL database. @@ -374,6 +381,25 @@ private: /// @exception DbOpenError Error opening the database void openDatabase(); + /// @brief Binds Parameters and Executes + /// + /// This method abstracts a lot of common processing from the getXxxx() + /// methods. It binds the parameters passed to it to the appropriate + /// prepared statement, and binds the variables in the exchange6 object to + /// the output parameters of the statement. It then executes the prepared + /// statement. + /// + /// The data can be retrieved using mysql_stmt_fetch and the getLeaseData() + /// method on the exchange6 object. + /// + /// @param stindex Index of prepared statement to be executed + /// @param inbind Array of MYSQL_BIND objects representing the parameters. + /// (Note that the number is determined by the number of parameters + /// in the statement.) + /// + /// @exception DbOperationError Error doing a database operation. + void bind6AndExecute(StatementIndex stindex, MYSQL_BIND* inbind) const; + /// @brief Check Error and Throw Exception /// /// Virtually all MySQL functions return a status which, if non-zero, @@ -396,6 +422,12 @@ private: } // Members + + /// Used for transfer of data to/from the database. This is a pointed-to + /// object as its contents may change in "const" calls, while the rest + /// of this object does not. (At alternative would be to declare it as + /// "mutable".) + boost::scoped_ptr exchange6_; MYSQL* mysql_; ///< MySQL context object std::vector text_statements_; ///< Raw text of statements std::vector statements_; ///< Prepared statements