From: Tomek Mrugalski Date: Fri, 21 Aug 2015 16:27:41 +0000 (+0200) Subject: [3681_rebase] MySQLLeaseMgr uses, but does not derive from MySqlConnection. X-Git-Tag: trac3874_base~33^2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e626cf0d384da8b34ad5cc523e841fa4f337fe45;p=thirdparty%2Fkea.git [3681_rebase] MySQLLeaseMgr uses, but does not derive from MySqlConnection. --- diff --git a/src/lib/dhcpsrv/mysql_connection.cc b/src/lib/dhcpsrv/mysql_connection.cc index 1db80eb1b2..138dc78fcf 100755 --- a/src/lib/dhcpsrv/mysql_connection.cc +++ b/src/lib/dhcpsrv/mysql_connection.cc @@ -185,5 +185,21 @@ MySqlConnection::prepareStatements(const TaggedStatement tagged_statements[], } } +/// @brief Destructor +MySqlConnection::~MySqlConnection() { + // Free up the prepared statements, ignoring errors. (What would we do + // about them? We're destroying this object and are not really concerned + // with errors on a database connection that is about to go away.) + for (int i = 0; i < statements_.size(); ++i) { + if (statements_[i] != NULL) { + (void) mysql_stmt_close(statements_[i]); + statements_[i] = NULL; + } + } + statements_.clear(); + text_statements_.clear(); + +} + } // namespace isc::dhcp } // namespace isc diff --git a/src/lib/dhcpsrv/mysql_connection.h b/src/lib/dhcpsrv/mysql_connection.h index 69b804ff90..03dfdba208 100755 --- a/src/lib/dhcpsrv/mysql_connection.h +++ b/src/lib/dhcpsrv/mysql_connection.h @@ -136,10 +136,11 @@ private: /// @brief Common MySQL Connector Pool /// -/// This class provides common operations for MySQL database connection -/// used by both MySqlLeaseMgr and MySqlHostDataSource. It manages connecting -/// to the database and preparing compiled statements. - +/// This class provides common operations for MySQL database connection +/// used by both MySqlLeaseMgr and MySqlHostDataSource. It manages connecting +/// to the database and preparing compiled statements. Its fields are +/// public, because they are used (both set and retrieved) in classes +/// that use instances of MySqlConnection. class MySqlConnection : public DatabaseConnection { public: @@ -151,8 +152,7 @@ public: } /// @brief Destructor - virtual ~MySqlConnection() { - } + virtual ~MySqlConnection(); /// @brief Prepare Single Statement /// @@ -190,12 +190,22 @@ public: /// @throw DbOpenError Error opening the database void openDatabase(); -protected: + /// @brief Prepared statements + /// + /// This field is public, because it is used heavily from MySqlConnection + /// and will be from MySqlHostDataSource. + std::vector statements_; - std::vector statements_; ///< Prepared statements - std::vector text_statements_; ///< Raw text of statements + /// @brief Raw text of statements + /// + /// This field is public, because it is used heavily from MySqlConnection + /// and will be from MySqlHostDataSource. + std::vector text_statements_; /// @brief MySQL connection handle + /// + /// This field is public, because it is used heavily from MySqlConnection + /// and will be from MySqlHostDataSource. MySqlHolder mysql_; }; diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 2c0c0677d7..e1e8bb360f 100755 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -1151,23 +1151,23 @@ private: // MySqlLeaseMgr Constructor and Destructor MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters) - : MySqlConnection(parameters) { + : conn_(parameters) { // Open the database. - openDatabase(); + conn_.openDatabase(); // Enable autocommit. To avoid a flush to disk on every commit, the global // parameter innodb_flush_log_at_trx_commit should be set to 2. This will // cause the changes to be written to the log, but flushed to disk in the // background every second. Setting the parameter to that value will speed // up the system, but at the risk of losing data if the system crashes. - my_bool result = mysql_autocommit(mysql_, 1); + my_bool result = mysql_autocommit(conn_.mysql_, 1); if (result != 0) { - isc_throw(DbOperationError, mysql_error(mysql_)); + isc_throw(DbOperationError, mysql_error(conn_.mysql_)); } // Prepare all statements likely to be used. - prepareStatements(tagged_statements, MySqlLeaseMgr::NUM_STATEMENTS); + conn_.prepareStatements(tagged_statements, MySqlLeaseMgr::NUM_STATEMENTS); // Create the exchange objects for use in exchanging data between the // program and the database. @@ -1177,16 +1177,6 @@ MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters) 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 - // with errors on a database connection that is about to go away.) - for (int i = 0; i < statements_.size(); ++i) { - if (statements_[i] != NULL) { - (void) mysql_stmt_close(statements_[i]); - statements_[i] = NULL; - } - } - // There is no need to close the database in this destructor: it is // closed in the destructor of the mysql_ member variable. } @@ -1275,17 +1265,17 @@ MySqlLeaseMgr::addLeaseCommon(StatementIndex stindex, std::vector& bind) { // Bind the parameters to the statement - int status = mysql_stmt_bind_param(statements_[stindex], &bind[0]); + int status = mysql_stmt_bind_param(conn_.statements_[stindex], &bind[0]); checkError(status, stindex, "unable to bind parameters"); // Execute the statement - status = mysql_stmt_execute(statements_[stindex]); + status = mysql_stmt_execute(conn_.statements_[stindex]); if (status != 0) { // Failure: check for the special case of duplicate entry. If this is // the case, we return false to indicate that the row was not added. // Otherwise we throw an exception. - if (mysql_errno(mysql_) == ER_DUP_ENTRY) { + if (mysql_errno(conn_.mysql_) == ER_DUP_ENTRY) { return (false); } checkError(status, stindex, "unable to execute"); @@ -1353,43 +1343,43 @@ void MySqlLeaseMgr::getLeaseCollection(StatementIndex stindex, bool single) const { // Bind the selection parameters to the statement - int status = mysql_stmt_bind_param(statements_[stindex], bind); + int status = mysql_stmt_bind_param(conn_.statements_[stindex], bind); checkError(status, stindex, "unable to bind WHERE clause parameter"); // Set up the MYSQL_BIND array for the data being returned and bind it to // the statement. std::vector outbind = exchange->createBindForReceive(); - status = mysql_stmt_bind_result(statements_[stindex], &outbind[0]); + status = mysql_stmt_bind_result(conn_.statements_[stindex], &outbind[0]); checkError(status, stindex, "unable to bind SELECT clause parameters"); // Execute the statement - status = mysql_stmt_execute(statements_[stindex]); + status = mysql_stmt_execute(conn_.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]); + status = mysql_stmt_store_result(conn_.statements_[stindex]); checkError(status, stindex, "unable to set up for storing all results"); // Set up the fetch "release" object to release resources associated // with the call to mysql_stmt_fetch when this method exits, then // retrieve the data. - MySqlFreeResult fetch_release(statements_[stindex]); + MySqlFreeResult fetch_release(conn_.statements_[stindex]); int count = 0; - while ((status = mysql_stmt_fetch(statements_[stindex])) == 0) { + while ((status = mysql_stmt_fetch(conn_.statements_[stindex])) == 0) { try { result.push_back(exchange->getLeaseData()); } catch (const isc::BadValue& ex) { // Rethrow the exception with a bit more data. isc_throw(BadValue, ex.what() << ". Statement is <" << - text_statements_[stindex] << ">"); + conn_.text_statements_[stindex] << ">"); } if (single && (++count > 1)) { isc_throw(MultipleRecords, "multiple records were found in the " "database where only one was expected for query " - << text_statements_[stindex]); + << conn_.text_statements_[stindex]); } } @@ -1399,7 +1389,7 @@ void MySqlLeaseMgr::getLeaseCollection(StatementIndex stindex, checkError(status, stindex, "unable to fetch results"); } else if (status == MYSQL_DATA_TRUNCATED) { // Data truncated - throw an exception indicating what was at fault - isc_throw(DataTruncated, text_statements_[stindex] + isc_throw(DataTruncated, conn_.text_statements_[stindex] << " returned truncated data: columns affected are " << exchange->getErrorColumns()); } @@ -1732,16 +1722,16 @@ MySqlLeaseMgr::updateLeaseCommon(StatementIndex stindex, MYSQL_BIND* bind, const LeasePtr& lease) { // Bind the parameters to the statement - int status = mysql_stmt_bind_param(statements_[stindex], bind); + int status = mysql_stmt_bind_param(conn_.statements_[stindex], bind); checkError(status, stindex, "unable to bind parameters"); // Execute - status = mysql_stmt_execute(statements_[stindex]); + status = mysql_stmt_execute(conn_.statements_[stindex]); checkError(status, stindex, "unable to execute"); // See how many rows were affected. The statement should only update a // single row. - int affected_rows = mysql_stmt_affected_rows(statements_[stindex]); + int affected_rows = mysql_stmt_affected_rows(conn_.statements_[stindex]); if (affected_rows == 0) { isc_throw(NoSuchLease, "unable to update lease for address " << lease->addr_ << " as it does not exist"); @@ -1818,16 +1808,16 @@ bool MySqlLeaseMgr::deleteLeaseCommon(StatementIndex stindex, MYSQL_BIND* bind) { // Bind the input parameters to the statement - int status = mysql_stmt_bind_param(statements_[stindex], bind); + int status = mysql_stmt_bind_param(conn_.statements_[stindex], bind); checkError(status, stindex, "unable to bind WHERE clause parameter"); // Execute - status = mysql_stmt_execute(statements_[stindex]); + status = mysql_stmt_execute(conn_.statements_[stindex]); checkError(status, stindex, "unable to execute"); // See how many rows were affected. Note that the statement may delete // multiple rows. - return (mysql_stmt_affected_rows(statements_[stindex]) > 0); + return (mysql_stmt_affected_rows(conn_.statements_[stindex]) > 0); } @@ -1870,7 +1860,7 @@ std::string MySqlLeaseMgr::getName() const { std::string name = ""; try { - name = getParameter("name"); + name = conn_.getParameter("name"); } catch (...) { // Return an empty name } @@ -1895,11 +1885,11 @@ MySqlLeaseMgr::getVersion() const { uint32_t minor; // Minor version number // Execute the prepared statement - int status = mysql_stmt_execute(statements_[stindex]); + int status = mysql_stmt_execute(conn_.statements_[stindex]); if (status != 0) { isc_throw(DbOperationError, "unable to execute <" - << text_statements_[stindex] << "> - reason: " << - mysql_error(mysql_)); + << conn_.text_statements_[stindex] << "> - reason: " << + mysql_error(conn_.mysql_)); } // Bind the output of the statement to the appropriate variables. @@ -1916,19 +1906,19 @@ MySqlLeaseMgr::getVersion() const { bind[1].buffer = &minor; bind[1].buffer_length = sizeof(minor); - status = mysql_stmt_bind_result(statements_[stindex], bind); + status = mysql_stmt_bind_result(conn_.statements_[stindex], bind); if (status != 0) { isc_throw(DbOperationError, "unable to bind result set: " << - mysql_error(mysql_)); + mysql_error(conn_.mysql_)); } // Fetch the data and set up the "release" object to release associated // resources when this method exits then retrieve the data. - MySqlFreeResult fetch_release(statements_[stindex]); - status = mysql_stmt_fetch(statements_[stindex]); + MySqlFreeResult fetch_release(conn_.statements_[stindex]); + status = mysql_stmt_fetch(conn_.statements_[stindex]); if (status != 0) { isc_throw(DbOperationError, "unable to obtain result set: " << - mysql_error(mysql_)); + mysql_error(conn_.mysql_)); } return (std::make_pair(major, minor)); @@ -1938,8 +1928,8 @@ MySqlLeaseMgr::getVersion() const { void MySqlLeaseMgr::commit() { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MYSQL_COMMIT); - if (mysql_commit(mysql_) != 0) { - isc_throw(DbOperationError, "commit failed: " << mysql_error(mysql_)); + if (mysql_commit(conn_.mysql_) != 0) { + isc_throw(DbOperationError, "commit failed: " << mysql_error(conn_.mysql_)); } } @@ -1947,8 +1937,8 @@ MySqlLeaseMgr::commit() { void MySqlLeaseMgr::rollback() { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MYSQL_ROLLBACK); - if (mysql_rollback(mysql_) != 0) { - isc_throw(DbOperationError, "rollback failed: " << mysql_error(mysql_)); + if (mysql_rollback(conn_.mysql_) != 0) { + isc_throw(DbOperationError, "rollback failed: " << mysql_error(conn_.mysql_)); } } diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.h b/src/lib/dhcpsrv/mysql_lease_mgr.h index 53bcef1ac9..b03418f72a 100755 --- a/src/lib/dhcpsrv/mysql_lease_mgr.h +++ b/src/lib/dhcpsrv/mysql_lease_mgr.h @@ -46,7 +46,7 @@ class MySqlLease6Exchange; /// database. Use of this backend presupposes that a MySQL database is /// available and that the Kea schema has been created within it. -class MySqlLeaseMgr : public LeaseMgr, public MySqlConnection { +class MySqlLeaseMgr : public LeaseMgr { public: /// @brief Constructor @@ -71,7 +71,7 @@ public: /// @throw isc::dhcp::DbOpenError Error opening the database /// @throw isc::dhcp::DbOperationError An operation on the open database has /// failed. - MySqlLeaseMgr(const ParameterMap& parameters); + MySqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters); /// @brief Destructor (closes database) virtual ~MySqlLeaseMgr(); @@ -432,6 +432,8 @@ public: }; private: + /// @brief MySQL connection + MySqlConnection conn_; /// @brief Add Lease Common Code /// @@ -594,9 +596,9 @@ private: const char* what) const { if (status != 0) { isc_throw(DbOperationError, what << " for <" << - text_statements_[index] << ">, reason: " << - mysql_error(mysql_) << " (error code " << - mysql_errno(mysql_) << ")"); + conn_.text_statements_[index] << ">, reason: " << + mysql_error(conn_.mysql_) << " (error code " << + mysql_errno(conn_.mysql_) << ")"); } } diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index ea8739bced..511fcfc754 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -943,8 +943,8 @@ private: }; PgSqlLeaseMgr::PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters) - : LeaseMgr(), DatabaseConnection(parameters), exchange4_(new PgSqlLease4Exchange()), - exchange6_(new PgSqlLease6Exchange()), conn_(NULL) { + : LeaseMgr(), exchange4_(new PgSqlLease4Exchange()), + exchange6_(new PgSqlLease6Exchange()), dbconn_(parameters), conn_(NULL) { openDatabase(); prepareStatements(); } @@ -1000,7 +1000,7 @@ PgSqlLeaseMgr::openDatabase() { string dbconnparameters; string shost = "localhost"; try { - shost = getParameter("host"); + shost = dbconn_.getParameter("host"); } catch(...) { // No host. Fine, we'll use "localhost" } @@ -1009,7 +1009,7 @@ PgSqlLeaseMgr::openDatabase() { string suser; try { - suser = getParameter("user"); + suser = dbconn_.getParameter("user"); dbconnparameters += " user = '" + suser + "'"; } catch(...) { // No user. Fine, we'll use NULL @@ -1017,7 +1017,7 @@ PgSqlLeaseMgr::openDatabase() { string spassword; try { - spassword = getParameter("password"); + spassword = dbconn_.getParameter("password"); dbconnparameters += " password = '" + spassword + "'"; } catch(...) { // No password. Fine, we'll use NULL @@ -1025,7 +1025,7 @@ PgSqlLeaseMgr::openDatabase() { string sname; try { - sname= getParameter("name"); + sname= dbconn_.getParameter("name"); dbconnparameters += " dbname = '" + sname + "'"; } catch(...) { // No database name. Throw a "NoDatabaseName" exception @@ -1498,7 +1498,7 @@ string PgSqlLeaseMgr::getName() const { string name = ""; try { - name = getParameter("name"); + name = dbconn_.getParameter("name"); } catch (...) { // Return an empty name } diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.h b/src/lib/dhcpsrv/pgsql_lease_mgr.h index 7cd90b82e6..8ef303088d 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.h +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.h @@ -121,7 +121,7 @@ const uint32_t PG_CURRENT_MINOR = 0; /// This class provides the \ref isc::dhcp::LeaseMgr interface to the PostgreSQL /// database. Use of this backend presupposes that a PostgreSQL database is /// available and that the Kea schema has been created within it. -class PgSqlLeaseMgr : public LeaseMgr, DatabaseConnection { +class PgSqlLeaseMgr : public LeaseMgr { public: /// @brief Constructor @@ -619,6 +619,12 @@ private: boost::scoped_ptr exchange4_; ///< Exchange object boost::scoped_ptr exchange6_; ///< Exchange object + /// Database connection object + /// + /// @todo: Implement PgSQLConnection object and collapse + /// dbconn_ and conn_ into a single object. + DatabaseConnection dbconn_; + /// PostgreSQL connection handle PGconn* conn_; };