[2342] Abstracted common MySQL binding code into a separate method.

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<MYSQL_BIND>& 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<MYSQL_BIND> 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<MYSQL_BIND>(&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<MYSQL_BIND>& bindvec) {
+    /// @return Vector of MySQL BIND objects.
+    std::vector<MYSQL_BIND> 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<char*>(&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<char*>(&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<MYSQL_BIND>(&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<MYSQL_BIND> bind;
-    exchange.createBindForSend(lease, bind);
+    std::vector<MYSQL_BIND> 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<MYSQL_BIND> 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<MYSQL_BIND> 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<uint8_t>& 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<char*>(&iaid);
     inbind[1].is_unsigned = static_cast<my_bool>(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<MYSQL_BIND> 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<char*>(&subnet_id);
     inbind[2].is_unsigned = static_cast<my_bool>(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<MYSQL_BIND> 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<MYSQL_BIND> bind;
-    exchange.createBindForSend(lease, bind);
+    std::vector<MYSQL_BIND> 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<uint32_t, uint32_t>
 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) {

src/lib/dhcp/mysql_lease_mgr.h

@@ -16,6 +16,7 @@
 #define MYSQL_LEASE_MGR_H
 
 #include <time.h>
+#include <boost/scoped_ptr.hpp>
 #include <mysql.h>
 #include <dhcp/lease_mgr.h>
 
@@ -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<MySqlLease6Exchange> exchange6_;
     MYSQL*              mysql_;                 ///< MySQL context object
     std::vector<std::string> text_statements_;  ///< Raw text of statements
     std::vector<MYSQL_STMT*> statements_;       ///< Prepared statements