diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc index 5f4dc2fb92..5a3dc59296 100644 --- a/src/lib/dhcpsrv/lease_mgr.cc +++ b/src/lib/dhcpsrv/lease_mgr.cc @@ -31,13 +31,13 @@ using namespace std; using namespace isc::dhcp; -Lease6::Lease6(LeaseType type, const isc::asiolink::IOAddress& addr, DuidPtr duid, - uint32_t iaid, uint32_t preferred, uint32_t valid, uint32_t t1, - uint32_t t2, SubnetID subnet_id, uint8_t prefixlen) - :type_(type), addr_(addr), prefixlen_(prefixlen), iaid_(iaid), duid_(duid), - preferred_lft_(preferred), valid_lft_(valid), t1_(t1), t2_(t2), - subnet_id_(subnet_id), fixed_(false), fqdn_fwd_(false), - fqdn_rev_(false) { +Lease6::Lease6(LeaseType type, const isc::asiolink::IOAddress& addr, + DuidPtr duid, uint32_t iaid, uint32_t preferred, uint32_t valid, + uint32_t t1, uint32_t t2, SubnetID subnet_id, uint8_t prefixlen) + : addr_(addr), type_(type), prefixlen_(prefixlen), iaid_(iaid), duid_(duid), + preferred_lft_(preferred), valid_lft_(valid), t1_(t1), t2_(t2), + subnet_id_(subnet_id), fixed_(false), fqdn_fwd_(false), + fqdn_rev_(false) { if (!duid) { isc_throw(InvalidOperation, "DUID must be specified for a lease"); } diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index 6ef54f297b..8b24151601 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -87,6 +87,13 @@ public: isc::Exception(file, line, what) {} }; +/// @brief Multiple lease records found where one expected +class MultipleRecords : public Exception { +public: + MultipleRecords(const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) {} +}; + /// @brief Attempt to update lease that was not there class NoSuchLease : public Exception { public: @@ -101,6 +108,7 @@ public: /// would be required. As this is a critical part of the code that will be used /// extensively, direct access is warranted. struct Lease4 { + /// @brief Constructor /// /// @param addr IPv4 address as unsigned 32-bit integer in network byte @@ -133,69 +141,78 @@ struct Lease4 { /// @brief Address extension /// - /// It is envisaged that in some cases IPv4 address will be accompanied with some - /// additional data. One example of such use are Address + Port solutions (or - /// Port-restricted Addresses), where several clients may get the same address, but - /// different port ranges. This feature is not expected to be widely used. - /// Under normal circumstances, the value should be 0. + /// It is envisaged that in some cases IPv4 address will be accompanied + /// with some additional data. One example of such use are Address + Port + /// solutions (or Port-restricted Addresses), where several clients may get + /// the same address, but different port ranges. This feature is not + /// expected to be widely used. Under normal circumstances, the value + /// should be 0. uint32_t ext_; - /// @brief hardware address + /// @brief Hardware address std::vector hwaddr_; - /// @brief client identifier + /// @brief Client identifier + /// + /// @todo Should this be a pointer to a client ID or the ID itself? + /// Compare with the DUID in the Lease6 structure. boost::shared_ptr client_id_; - /// @brief renewal timer + /// @brief Renewal timer /// - /// Specifies renewal time. Although technically it is a property of IA container, - /// not the address itself, since our data model does not define separate IA - /// entity, we are keeping it in the lease. In case of multiple addresses/prefixes - /// for the same IA, each must have consistent T1 and T2 values. Specified in - /// seconds since cltt. + /// Specifies renewal time. Although technically it is a property of the + /// IA container and not the address itself, since our data model does not + /// define a separate IA entity, we are keeping it in the lease. In the + /// case of multiple addresses/prefixes for the same IA, each must have + /// consistent T1 and T2 values. This is specified in seconds since cltt. uint32_t t1_; - /// @brief rebinding timer + /// @brief Rebinding timer /// - /// Specifies rebinding time. Although technically it is a property of IA container, - /// not the address itself, since our data model does not define separate IA - /// entity, we are keeping it in the lease. In case of multiple addresses/prefixes - /// for the same IA, each must have consistent T1 and T2 values. Specified in - /// seconds since cltt. + /// Specifies rebinding time. Although technically it is a property of the + /// IA container and not the address itself, since our data model does not + /// define a separate IA entity, we are keeping it in the lease. In the + /// case of multiple addresses/prefixes for the same IA, each must have + /// consistent T1 and T2 values. This is pecified in seconds since cltt. uint32_t t2_; - /// @brief valid lifetime + /// @brief Ralid lifetime /// - /// Expressed as number of seconds since cltt + /// Expressed as number of seconds since cltt. uint32_t valid_lft_; - /// @brief client last transmission time + /// @brief Client last transmission time /// - /// Specifies a timestamp, when last transmission from a client was received. + /// Specifies a timestamp giving the time when the last transmission from a + /// client was received. time_t cltt_; /// @brief Subnet identifier /// - /// Specifies subnet-id of the subnet that the lease belongs to + /// Specifies the identification of the subnet to which the lease belongs. SubnetID subnet_id_; - /// @brief Is this a fixed lease? + /// @brief Fixed lease? /// /// Fixed leases are kept after they are released/expired. bool fixed_; - /// @brief client hostname + /// @brief Client hostname /// /// This field may be empty std::string hostname_; - /// @brief did we update AAAA record for this lease? + /// @brief Forward zone updated? + /// + /// Set true if the DNS AAAA record for this lease has been updated. bool fqdn_fwd_; - /// @brief did we update PTR record for this lease? + /// @brief Reverse zone updated? + /// + /// Set true if the DNS PTR record for this lease has been updated. bool fqdn_rev_; - /// @brief Lease comments. + /// @brief Lease comments /// /// Currently not used. It may be used for keeping comments made by the /// system administrator. @@ -210,13 +227,16 @@ typedef boost::shared_ptr Lease4Ptr; /// @brief A collection of IPv4 leases. typedef std::vector Lease4Collection; + + /// @brief Structure that holds a lease for IPv6 address and/or prefix /// -/// For performance reasons it is a simple structure, not a class. Had we chose to -/// make it a class, all fields would have to be made private and getters/setters +/// For performance reasons it is a simple structure, not a class. If we chose +/// make it a class, all fields would have to made private and getters/setters /// would be required. As this is a critical part of the code that will be used -/// extensively, direct access rather than through getters/setters is warranted. struct Lease6 { + + /// @brief Type of lease contents typedef enum { LEASE_IA_NA, /// the lease contains non-temporary IPv6 address LEASE_IA_TA, /// the lease contains temporary IPv6 address @@ -228,86 +248,96 @@ struct Lease6 { uint32_t iaid, uint32_t preferred, uint32_t valid, uint32_t t1, uint32_t t2, SubnetID subnet_id, uint8_t prefixlen_ = 0); - /// @brief specifies lease type (normal addr, temporary addr, prefix) - LeaseType type_; - - /// IPv6 address + /// @brief IPv6 address isc::asiolink::IOAddress addr_; - /// IPv6 prefix length (used only for PD) + /// @brief Lease type + /// + /// One of normal address, temporary address, or prefix. + LeaseType type_; + + /// @brief IPv6 prefix length + /// + /// This is used only for prefix delegations and is ignored otherwise. uint8_t prefixlen_; - /// @brief IAID + /// @brief Identity Association Identifier (IAID) /// - /// Identity Association IDentifier. DHCPv6 stores all addresses and prefixes - /// in IA containers (IA_NA, IA_TA, IA_PD). Most containers may appear more - /// than once in a message. To differentiate between them, IAID field is present + /// DHCPv6 stores all addresses and prefixes in IA containers (IA_NA, + /// IA_TA, IA_PD). Most containers may appear more than once in a message. + /// To differentiate between them, the IAID field is present uint32_t iaid_; - /// @brief client identifier + /// @brief Client identifier boost::shared_ptr duid_; /// @brief preferred lifetime /// - /// This parameter specifies preferred lifetime since the lease was assigned/renewed - /// (cltt), expressed in seconds. + /// This parameter specifies the preferred lifetime since the lease was + /// assigned or renewed (cltt), expressed in seconds. uint32_t preferred_lft_; /// @brief valid lifetime /// - /// This parameter specified valid lifetime since the lease was assigned/renewed - /// (cltt), expressed in seconds. + /// This parameter specifies the valid lifetime since the lease waa + /// assigned/renewed (cltt), expressed in seconds. uint32_t valid_lft_; /// @brief T1 timer /// - /// Specifies renewal time. Although technically it is a property of IA container, - /// not the address itself, since our data model does not define separate IA - /// entity, we are keeping it in the lease. In case of multiple addresses/prefixes - /// for the same IA, each must have consistent T1 and T2 values. Specified in - /// seconds since cltt. - /// This value will also be useful for failover to calculate the next expected - /// client transmission time. + /// Specifies renewal time. Although technically it is a property of the + /// IA container and not the address itself, since our data model does not + /// define a separate IA entity, we are keeping it in the lease. In the + /// case of multiple addresses/prefixes for the same IA, each must have + /// consistent T1 and T2 values. This is specified in seconds since cltt. + /// The value will also be useful for failover to calculate the next + /// expected client transmission time. uint32_t t1_; /// @brief T2 timer /// - /// Specifies rebinding time. Although technically it is a property of IA container, - /// not the address itself, since our data model does not define separate IA - /// entity, we are keeping it in the lease. In case of multiple addresses/prefixes - /// for the same IA, each must have consistent T1 and T2 values. Specified in - /// seconds since cltt. + /// Specifies rebinding time. Although technically it is a property of the + /// IA container and not the address itself, since our data model does not + /// define a separate IA entity, we are keeping it in the lease. In the + /// case of multiple addresses/prefixes for the same IA, each must have + /// consistent T1 and T2 values. This is specified in seconds since cltt. uint32_t t2_; - /// @brief client last transmission time + /// @brief Client last transmission time /// - /// Specifies a timestamp, when last transmission from a client was received. + /// Specifies a timestamp giving the time when the last transmission from a + /// client was received. time_t cltt_; /// @brief Subnet identifier /// - /// Specifies subnet-id of the subnet that the lease belongs to + /// Specifies the identification of the subnet to which the lease belongs. SubnetID subnet_id_; - /// @brief Is this a fixed lease? + /// @brief Fixed lease? /// /// Fixed leases are kept after they are released/expired. bool fixed_; - /// @brief client hostname + /// @brief Client hostname /// /// This field may be empty std::string hostname_; - /// @brief did we update AAAA record for this lease? + /// @brief Forward zone updated? + /// + /// Set true if the DNS AAAA record for this lease has been updated. bool fqdn_fwd_; - /// @brief did we update PTR record for this lease? + /// @brief Reverse zone updated? + /// + /// Set true if the DNS PTR record for this lease has been updated. bool fqdn_rev_; /// @brief Lease comments /// - /// This field is currently not used. + /// Currently not used. It may be used for keeping comments made by the + /// system administrator. std::string comments_; /// @todo: Add DHCPv6 failover related fields here @@ -357,7 +387,7 @@ typedef std::vector Lease6Collection; /// see the documentation of those classes for details. class LeaseMgr { public: - /// Client Hardware address + /// Client hardware address typedef std::vector HWAddr; /// Database configuration parameter map diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 9d2401084f..aa691edc62 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -30,7 +30,8 @@ using namespace std; namespace { ///@{ -/// @brief Maximum Size of Database Fields + +/// @brief Maximum size of database fields /// /// The following constants define buffer sizes for variable length database /// fields. The values should be greater than or equal to the length set in @@ -45,12 +46,23 @@ const size_t ADDRESS6_TEXT_MAX_LEN = 42; ///< Max size of a IPv6 text buffer const size_t DUID_MAX_LEN = 128; ///< Max size of a DUID const size_t HWADDR_MAX_LEN = 128; ///< Max size of a hardware address const size_t CLIENT_ID_MAX_LEN = 128; ///< Max size of a client ID + +/// @brief MySQL True/False constants +/// +/// Declare typed values so as to avoid problems of data conversion. These +/// are local to the file but are given the prefix MLM (MySql Lease Manager) to +/// avoid any likely conflicts with variables named TRUE or FALSE. + +const my_bool MLM_FALSE = 0; ///< False value +const my_bool MLM_TRUE = 1; ///< True value + ///@} /// @brief MySQL Selection Statements /// -/// Each statement is associated with the index, used by the various methods -/// to access the prepared statement. +/// Each statement is associated with an index, which is used to reference the +/// associated prepared statement. + struct TaggedStatement { MySqlLeaseMgr::StatementIndex index; const char* text; @@ -140,11 +152,10 @@ namespace dhcp { /// 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 Lease4 structure. -/// This class handles the creation of that array. +/// structure is identical. 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 +/// to hold things like data length etc. This object holds the intermediate /// variables as well. /// /// @note There are no unit tests for this class. It is tested indirectly @@ -154,38 +165,40 @@ class MySqlLease4Exchange { public: /// @brief Constructor /// - /// Apart from the initialization of false_ and true_, the initialization - /// of addr4_, hwaddr_length_, hwaddr_buffer_, client_id_length_ and - /// client_id_buffer_ are to satisfy cppcheck: none are really needed, as - /// all variables are initialized/set in the methods. - MySqlLease4Exchange() : addr4_(0), hwaddr_length_(0), client_id_length_(0), - false_(0), true_(1) { + /// The initialization of the variables here is nonly to satisfy cppcheck - + /// all variables are initialized/set in the methods before they are used. + MySqlLease4Exchange() : addr4_(0), hwaddr_length_(0), client_id_length_(0) { memset(hwaddr_buffer_, 0, sizeof(hwaddr_buffer_)); memset(client_id_buffer_, 0, sizeof(client_id_buffer_)); } /// @brief Create MYSQL_BIND objects for Lease4 Pointer /// - /// Fills in the MYSQL_BIND objects for the Lease4 passed to it. + /// Fills in the MYSQL_BIND array for sending data in the Lease4 object to + /// the database. /// - /// @param lease Lease object to be added to the database. + /// @param lease Lease object to be added to the database. None of the + /// fields in the lease are modified - the lease data is only read. /// /// @return Vector of MySQL BIND objects representing the data to be added. std::vector createBindForSend(const Lease4Ptr& lease) { + // Store lease object to ensure it remains valid. lease_ = lease; - // Ensure bind_ array clear for constructing the MYSQL_BIND structures - // for this lease. + // Initialize prior to constructing the array of MYSQL_BIND structures. memset(bind_, 0, sizeof(bind_)); + // Set up the structures for the various components of the lease4 + // structure. + // Address: uint32_t - // Address in the Lease structre is an IOAddress object. Convert to - // an integer for storage. + // The address in the Lease structre is an IOAddress object. Convert + // this to an integer for storage. addr4_ = static_cast(lease_->addr_); bind_[0].buffer_type = MYSQL_TYPE_LONG; bind_[0].buffer = reinterpret_cast(&addr4_); - bind_[0].is_unsigned = true_; + bind_[0].is_unsigned = MLM_TRUE; // hwaddr: varbinary // For speed, we avoid copying the data into temporary storage and @@ -207,16 +220,17 @@ public: // valid lifetime: unsigned int bind_[3].buffer_type = MYSQL_TYPE_LONG; bind_[3].buffer = reinterpret_cast(&lease_->valid_lft_); - bind_[3].is_unsigned = true_; + bind_[3].is_unsigned = MLM_TRUE; // expire: timestamp // The lease structure holds the client last transmission time (cltt_) // For convenience for external tools, this is converted to lease - /// expiry time (expire). The relationship is given by: + // expiry time (expire). The relationship is given by: // // expire = cltt_ + valid_lft_ // - // @TODO Handle overflows + // @TODO Handle overflows - a large enough valid_lft_ could cause + // an overflow on a 32-bit system. MySqlLeaseMgr::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_, expire_); bind_[4].buffer_type = MYSQL_TYPE_TIMESTAMP; @@ -227,7 +241,7 @@ public: // Can use lease_->subnet_id_ directly as it is of type uint32_t. bind_[5].buffer_type = MYSQL_TYPE_LONG; bind_[5].buffer = reinterpret_cast(&lease_->subnet_id_); - bind_[5].is_unsigned = true_; + bind_[5].is_unsigned = MLM_TRUE; // Add the data to the vector. Note the end element is one after the // end of the array. @@ -237,22 +251,19 @@ public: /// @brief Create BIND array to receive data /// /// Creates a MYSQL_BIND array to receive Lease4 data from the database. - /// After data is successfully received, getLeaseData() is used to copy + /// After data is successfully received, getLeaseData() can be used to copy /// it to a Lease6 object. /// - /// @return Vector of MySQL BIND objects. std::vector createBindForReceive() { // Ensure both the array of MYSQL_BIND structures and the error array // are clear. memset(bind_, 0, sizeof(bind_)); - memset(error_, 0, sizeof(error_)); // address: uint32_t bind_[0].buffer_type = MYSQL_TYPE_LONG; bind_[0].buffer = reinterpret_cast(&addr4_); - bind_[0].is_unsigned = true_; - bind_[0].error = &error_[0]; + bind_[0].is_unsigned = MLM_TRUE; // hwaddr: varbinary(20) hwaddr_length_ = sizeof(hwaddr_buffer_); @@ -260,7 +271,6 @@ public: bind_[1].buffer = reinterpret_cast(hwaddr_buffer_); bind_[1].buffer_length = hwaddr_length_; bind_[1].length = &hwaddr_length_; - bind_[1].error = &error_[1]; // client_id: varbinary(128) client_id_length_ = sizeof(client_id_buffer_); @@ -268,25 +278,21 @@ public: bind_[2].buffer = reinterpret_cast(client_id_buffer_); bind_[2].buffer_length = client_id_length_; bind_[2].length = &client_id_length_; - bind_[2].error = &error_[2]; // lease_time: unsigned int bind_[3].buffer_type = MYSQL_TYPE_LONG; bind_[3].buffer = reinterpret_cast(&valid_lifetime_); - bind_[3].is_unsigned = true_; - bind_[3].error = &error_[3]; + bind_[3].is_unsigned = MLM_TRUE; // expire: timestamp bind_[4].buffer_type = MYSQL_TYPE_TIMESTAMP; bind_[4].buffer = reinterpret_cast(&expire_); bind_[4].buffer_length = sizeof(expire_); - bind_[4].error = &error_[4]; // subnet_id: unsigned int bind_[5].buffer_type = MYSQL_TYPE_LONG; bind_[5].buffer = reinterpret_cast(&subnet_id_); - bind_[5].is_unsigned = true_; - bind_[5].error = &error_[5]; + bind_[5].is_unsigned = MLM_TRUE; // Add the data to the vector. Note the end element is one after the // end of the array. @@ -296,15 +302,14 @@ public: /// @brief Copy Received Data into Lease6 Object /// /// Called after the MYSQL_BIND array created by createBindForReceive() - /// has been used, this copies data from the internal member vairables + /// has been used, this copies data from the internal member variables /// into a Lease4 object. /// - /// @return Lease6Ptr Pointer to a Lease6 object holding the relevant + /// @return Lease4Ptr Pointer to a Lease6 object holding the relevant /// data. - /// - /// @throw isc::BadValue Unable to convert Lease Type value in database Lease4Ptr getLeaseData() { - // Convert times to units for the lease structure + // Convert times received from the database to times for the lease + // structure time_t cltt = 0; MySqlLeaseMgr::convertFromDatabaseTime(expire_, valid_lifetime_, cltt); @@ -315,22 +320,21 @@ public: private: // Note: All array lengths are equal to the corresponding variable in the - // schema. - // Note: arrays are declared fixed length for speed of creation + // schema. + // Note: Arrays are declared fixed length for speed of creation uint32_t addr4_; ///< IPv4 address - MYSQL_BIND bind_[9]; ///< Bind array + MYSQL_BIND bind_[6]; ///< Bind array std::vector hwaddr_; ///< Hardware address - uint8_t hwaddr_buffer_[HWADDR_MAX_LEN]; ///< Hardware address buffer + uint8_t hwaddr_buffer_[HWADDR_MAX_LEN]; + ///< Hardware address buffer unsigned long hwaddr_length_; ///< Hardware address length std::vector client_id_; ///< Client identification - uint8_t client_id_buffer_[CLIENT_ID_MAX_LEN]; ///< Client ID buffer + uint8_t client_id_buffer_[CLIENT_ID_MAX_LEN]; + ///< Client ID buffer unsigned long client_id_length_; ///< Client ID address length - my_bool error_[6]; ///< For error reporting MYSQL_TIME expire_; ///< Lease expiry time - const my_bool false_; ///< "false" for MySql Lease4Ptr lease_; ///< Pointer to lease object uint32_t subnet_id_; ///< Subnet identification - const my_bool true_; ///< "true_" for MySql uint32_t valid_lifetime_; ///< Lease time }; @@ -341,11 +345,10 @@ private: /// 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. +/// structure is identical. 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 +/// to hold things like data length etc. This object holds the intermediate /// variables as well. /// /// @note There are no unit tests for this class. It is tested indirectly @@ -355,19 +358,17 @@ class MySqlLease6Exchange { public: /// @brief Constructor /// - /// Apart from the initialization of false_ and true_, the initialization - /// of addr6_length_, duid_length_, addr6_buffer_ and duid_buffer_ 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) { + /// The initialization of the variables here is nonly to satisfy cppcheck - + /// all variables are initialized/set in the methods before they are used. + MySqlLease6Exchange() : addr6_length_(0), duid_length_(0) { memset(addr6_buffer_, 0, sizeof(addr6_buffer_)); memset(duid_buffer_, 0, sizeof(duid_buffer_)); } /// @brief Create MYSQL_BIND objects for Lease6 Pointer /// - /// Fills in the MYSQL_BIND objects for the Lease6 passed to it. + /// Fills in the MYSQL_BIND array for sending data in the Lease4 object to + /// the database. /// /// @param lease Lease object to be added to the database. /// @@ -391,13 +392,13 @@ public: // is guaranteed to be valid until the next non-const operation on // addr6_.) // - // Note that the const_cast could be avoided by copying the string to - // a writeable buffer and storing the address of that in the "buffer" - // element. However, this introduces a copy operation (with additional - // overhead) purely to get round the strictures introduced by design of - // the MySQL interface (which uses the area pointed to by "buffer" as - // input when specifying query parameters and as output when retrieving - // data). For that reason, "const_cast" has been used. + // The const_cast could be avoided by copying the string to a writeable + // buffer and storing the address of that in the "buffer" element. + // However, this introduces a copy operation (with additional overhead) + // purely to get round the strictures introduced by design of the + // MySQL interface (which uses the area pointed to by "buffer" as input + // when specifying query parameters and as output when retrieving data). + // For that reason, "const_cast" has been used. bind_[0].buffer_type = MYSQL_TYPE_STRING; bind_[0].buffer = const_cast(addr6_.c_str()); bind_[0].buffer_length = addr6_length_; @@ -415,7 +416,7 @@ public: // valid lifetime: unsigned int bind_[2].buffer_type = MYSQL_TYPE_LONG; bind_[2].buffer = reinterpret_cast(&lease_->valid_lft_); - bind_[2].is_unsigned = true_; + bind_[2].is_unsigned = MLM_TRUE; // expire: timestamp // The lease structure holds the client last transmission time (cltt_) @@ -435,32 +436,32 @@ public: // Can use lease_->subnet_id_ directly as it is of type uint32_t. bind_[4].buffer_type = MYSQL_TYPE_LONG; bind_[4].buffer = reinterpret_cast(&lease_->subnet_id_); - bind_[4].is_unsigned = true_; + bind_[4].is_unsigned = MLM_TRUE; // pref_lifetime: unsigned int // Can use lease_->preferred_lft_ directly as it is of type uint32_t. bind_[5].buffer_type = MYSQL_TYPE_LONG; bind_[5].buffer = reinterpret_cast(&lease_->preferred_lft_); - bind_[5].is_unsigned = true_; + bind_[5].is_unsigned = MLM_TRUE; // lease_type: tinyint - // Must convert to uint8_t as lease_->type_ is a LeaseType variable + // Must convert to uint8_t as lease_->type_ is a LeaseType variable. lease_type_ = lease_->type_; bind_[6].buffer_type = MYSQL_TYPE_TINY; bind_[6].buffer = reinterpret_cast(&lease_type_); - bind_[6].is_unsigned = true_; + bind_[6].is_unsigned = MLM_TRUE; // iaid: unsigned int // Can use lease_->iaid_ directly as it is of type uint32_t. bind_[7].buffer_type = MYSQL_TYPE_LONG; bind_[7].buffer = reinterpret_cast(&lease_->iaid_); - bind_[7].is_unsigned = true_; + bind_[7].is_unsigned = MLM_TRUE; // prefix_len: unsigned tinyint // Can use lease_->prefixlen_ directly as it is uint32_t. bind_[8].buffer_type = MYSQL_TYPE_TINY; bind_[8].buffer = reinterpret_cast(&lease_->prefixlen_); - bind_[8].is_unsigned = true_; + bind_[8].is_unsigned = MLM_TRUE; // Add the data to the vector. Note the end element is one after the // end of the array. @@ -473,24 +474,23 @@ public: /// After data is successfully received, getLeaseData() is used to copy /// it to a Lease6 object. /// - /// @return Vector of MySQL BIND objects. + /// @return Vector of MySQL BIND objects passed to the MySQL data retrieval + /// functions. std::vector createBindForReceive() { // Ensure both the array of MYSQL_BIND structures and the error array // are clear. memset(bind_, 0, sizeof(bind_)); - memset(error_, 0, sizeof(error_)); // address: varchar // A Lease6_ address has a maximum of 39 characters. The array is - // a few bites longer than this to guarantee that we can always null + // a few bytes longer than this to guarantee that we can always null // terminate it. addr6_length_ = sizeof(addr6_buffer_) - 1; bind_[0].buffer_type = MYSQL_TYPE_STRING; bind_[0].buffer = addr6_buffer_; bind_[0].buffer_length = addr6_length_; bind_[0].length = &addr6_length_; - bind_[0].error = &error_[0]; // client_id: varbinary(128) duid_length_ = sizeof(duid_buffer_); @@ -498,49 +498,41 @@ public: bind_[1].buffer = reinterpret_cast(duid_buffer_); bind_[1].buffer_length = duid_length_; bind_[1].length = &duid_length_; - bind_[1].error = &error_[1]; // lease_time: unsigned int bind_[2].buffer_type = MYSQL_TYPE_LONG; bind_[2].buffer = reinterpret_cast(&valid_lifetime_); - bind_[2].is_unsigned = true_; - bind_[2].error = &error_[2]; + bind_[2].is_unsigned = MLM_TRUE; // expire: timestamp bind_[3].buffer_type = MYSQL_TYPE_TIMESTAMP; bind_[3].buffer = reinterpret_cast(&expire_); bind_[3].buffer_length = sizeof(expire_); - bind_[3].error = &error_[3]; // subnet_id: unsigned int bind_[4].buffer_type = MYSQL_TYPE_LONG; bind_[4].buffer = reinterpret_cast(&subnet_id_); - bind_[4].is_unsigned = true_; - bind_[4].error = &error_[4]; + bind_[4].is_unsigned = MLM_TRUE; // pref_lifetime: unsigned int bind_[5].buffer_type = MYSQL_TYPE_LONG; bind_[5].buffer = reinterpret_cast(&pref_lifetime_); - bind_[5].is_unsigned = true_; - bind_[5].error = &error_[5]; + bind_[5].is_unsigned = MLM_TRUE; // lease_type: tinyint bind_[6].buffer_type = MYSQL_TYPE_TINY; bind_[6].buffer = reinterpret_cast(&lease_type_); - bind_[6].is_unsigned = true_; - bind_[6].error = &error_[6]; + bind_[6].is_unsigned = MLM_TRUE; // iaid: unsigned int bind_[7].buffer_type = MYSQL_TYPE_LONG; bind_[7].buffer = reinterpret_cast(&iaid_); - bind_[7].is_unsigned = true_; - bind_[7].error = &error_[7]; + bind_[7].is_unsigned = MLM_TRUE; // prefix_len: unsigned tinyint bind_[8].buffer_type = MYSQL_TYPE_TINY; bind_[8].buffer = reinterpret_cast(&prefixlen_); - bind_[8].is_unsigned = true_; - bind_[8].error = &error_[8]; + bind_[8].is_unsigned = MLM_TRUE; // Add the data to the vector. Note the end element is one after the // end of the array. @@ -558,48 +550,46 @@ public: /// /// @throw isc::BadValue Unable to convert Lease Type value in database Lease6Ptr getLeaseData() { - - // Create the object to be returned. - Lease6Ptr result(new Lease6()); - - // Put the data in the lease object - // The address buffer is declared larger than the buffer size passed // to the access function so that we can always append a null byte. + // Create the IOAddress object corresponding to the received data. addr6_buffer_[addr6_length_] = '\0'; std::string address = addr6_buffer_; + isc::asiolink::IOAddress addr(address); - // Set the other data, converting time as needed. - result->addr_ = isc::asiolink::IOAddress(address); - result->duid_.reset(new DUID(duid_buffer_, duid_length_)); - MySqlLeaseMgr::convertFromDatabaseTime(expire_, valid_lifetime_, - result->cltt_); - result->valid_lft_ = valid_lifetime_; - result->subnet_id_ = subnet_id_; - result->preferred_lft_ = pref_lifetime_; - - // We can't convert from a numeric value to an enum, hence: + // Set the lease type in a variable of the appropriate data type, which + // has been initialized with an arbitrary but valid value. + Lease6::LeaseType type = Lease6::LEASE_IA_NA; switch (lease_type_) { case Lease6::LEASE_IA_NA: - result->type_ = Lease6::LEASE_IA_NA; + type = Lease6::LEASE_IA_NA; break; case Lease6::LEASE_IA_TA: - result->type_ = Lease6::LEASE_IA_TA; + type = Lease6::LEASE_IA_TA; break; case Lease6::LEASE_IA_PD: - result->type_ = Lease6::LEASE_IA_PD; + type = Lease6::LEASE_IA_PD; break; default: isc_throw(BadValue, "invalid lease type returned (" << lease_type_ << ") for lease with address " << - result->addr_.toText() << ". Only 0, 1, or 2 " - "are allowed."); + address << ". Only 0, 1, or 2 are allowed."); } - result->iaid_ = iaid_; - result->prefixlen_ = prefixlen_; + + // Set up DUID, + DuidPtr duid_ptr(new DUID(duid_buffer_, duid_length_)); + + // Create the lease and set the cltt (after converting from the + // expire time retrieved from the database). + Lease6Ptr result(new Lease6(type, addr, duid_ptr, iaid_, + pref_lifetime_, valid_lifetime_, 0, 0, + subnet_id_, prefixlen_)); + time_t cltt = 0; + MySqlLeaseMgr::convertFromDatabaseTime(expire_, valid_lifetime_, cltt); + result->cltt_ = cltt; return (result); } @@ -616,16 +606,13 @@ private: std::vector duid_; ///< Client identification uint8_t duid_buffer_[DUID_MAX_LEN]; ///< Buffer form of DUID unsigned long duid_length_; ///< Length of the DUID - my_bool error_[9]; ///< For error reporting MYSQL_TIME expire_; ///< Lease expiry time - const my_bool false_; ///< "false" for MySql uint32_t iaid_; ///< Identity association ID Lease6Ptr lease_; ///< Pointer to lease object uint8_t lease_type_; ///< Lease type uint8_t prefixlen_; ///< Prefix length uint32_t pref_lifetime_; ///< Preferred lifetime uint32_t subnet_id_; ///< Subnet identification - const my_bool true_; ///< "true_" for MySql uint32_t valid_lifetime_; ///< Lease time }; @@ -634,15 +621,16 @@ private: /// /// When a MySQL statement is exected, to fetch the results the function /// mysql_stmt_fetch() must be called. As well as getting data, this -/// allocates internal state. Subsequent calls to mysql_stmt_fetch -/// can be made, but when all the data is retrieved, mysql_stmt_free_result -/// must be called to free up the resources allocated. +/// allocates internal state. Subsequent calls to mysql_stmt_fetch can be +/// made, but when all the data is retrieved, mysql_stmt_free_result must be +/// called to free up the resources allocated. /// /// Created prior to the first fetch, this class's destructor calls /// mysql_stmt_free_result, so eliminating the need for an explicit release -/// in the method using mysql_stmt_free_result. In this way, it guarantees +/// in the method calling mysql_stmt_free_result. In this way, it guarantees /// that the resources are released even if the MySqlLeaseMgr method concerned /// exits via an exception. + class MySqlFreeResult { public: @@ -682,7 +670,7 @@ MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters) isc_throw(DbOpenError, "unable to initialize MySQL"); } - // Open the database + // Open the database. openDatabase(); // Enable autocommit. For maximum speed, the global parameter @@ -704,8 +692,8 @@ MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::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 it about to go away.) + // 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]); @@ -734,7 +722,7 @@ void MySqlLeaseMgr::convertToDatabaseTime(time_t cltt, uint32_t valid_lifetime, MYSQL_TIME& expire) { - // Calculate expiry time and convert to various date/time fields. + // Calculate expiry time. // @TODO: handle overflows time_t expire_time = cltt + valid_lifetime; @@ -749,8 +737,8 @@ MySqlLeaseMgr::convertToDatabaseTime(time_t cltt, uint32_t valid_lifetime, expire.hour = expire_tm.tm_hour; expire.minute = expire_tm.tm_min; expire.second = expire_tm.tm_sec; - expire.second_part = 0; // No fractional seconds - expire.neg = static_cast(0); // Not negative + expire.second_part = 0; // No fractional seconds + expire.neg = my_bool(0); // Not negative } void @@ -775,7 +763,7 @@ MySqlLeaseMgr::convertFromDatabaseTime(const MYSQL_TIME& expire, -// Database acess method +// Open the database using the parameters passed to the constructor. void MySqlLeaseMgr::openDatabase() { @@ -787,7 +775,7 @@ MySqlLeaseMgr::openDatabase() { shost = getParameter("host"); host = shost.c_str(); } catch (...) { - // No host. Fine, we'll use "localhost" + ; // No host. Fine, we'll use "localhost" } const char* user = NULL; @@ -796,8 +784,7 @@ MySqlLeaseMgr::openDatabase() { suser = getParameter("user"); user = suser.c_str(); } catch (...) { - // No user. Fine, we'll use NULL - ; + ; // No user. Fine, we'll use NULL } const char* password = NULL; @@ -806,8 +793,7 @@ MySqlLeaseMgr::openDatabase() { spassword = getParameter("password"); password = spassword.c_str(); } catch (...) { - // No password. Fine, we'll use NULL - ; + ; // No password. Fine, we'll use NULL } const char* name = NULL; @@ -821,8 +807,11 @@ MySqlLeaseMgr::openDatabase() { } // Set options for the connection: - // - automatic reconnection - my_bool auto_reconnect = 1; + // + // Automatic reconnection: after a period of inactivity, the client will + // disconnect from the database. This option causes it to automatically + // reconnect when another operation is about to be done. + my_bool auto_reconnect = MLM_TRUE; int result = mysql_options(mysql_, MYSQL_OPT_RECONNECT, &auto_reconnect); if (result != 0) { isc_throw(DbOpenError, "unable to set auto-reconnect option: " << @@ -865,11 +854,10 @@ MySqlLeaseMgr::prepareStatement(StatementIndex index, const char* text) { // All OK, so prepare the statement text_statements_[index] = std::string(text); - statements_[index] = mysql_stmt_init(mysql_); if (statements_[index] == NULL) { isc_throw(DbOperationError, "unable to allocate MySQL prepared " - "statement structure" << mysql_error(mysql_)); + "statement structure, reason: " << mysql_error(mysql_)); } int status = mysql_stmt_prepare(statements_[index], text, strlen(text)); @@ -897,7 +885,9 @@ MySqlLeaseMgr::prepareStatements() { } -// Add leases to the database +// Add leases to the database. The two public methods accept a lease object +// (of different types), bind the contents to the appropriate prepared +// statement, then call common code to execute the statement. bool MySqlLeaseMgr::addLease(StatementIndex stindex, std::vector& bind) { @@ -967,59 +957,37 @@ MySqlLeaseMgr::bindAndExecute(StatementIndex stindex, Exchange& exchange, checkError(status, stindex, "unable to execute"); } -// Extraction of leases from the database. Much the code has common logic -// with the difference between V4 and V6 being the data types of the -// objects involved. For this reason, the common logic is inside a -// template method. - -template -void MySqlLeaseMgr::getLease(StatementIndex stindex, MYSQL_BIND* inbind, - Exchange& exchange, LeasePtr& result) const { - - // Bind the input parameters to the statement and bind the output - // to fields in the exchange object, then execute the prepared statement. - bindAndExecute(stindex, exchange, inbind); - - // Fetch the data and set up the "free result" object to release associated - // resources when this method exits. - MySqlFreeResult fetch_release(statements_[stindex]); - int status = mysql_stmt_fetch(statements_[stindex]); - - if (status == 0) { - try { - result = exchange->getLeaseData(); - } catch (const isc::BadValue& ex) { - // Lease type is returned, to rethrow the exception with a bit - // more data. - isc_throw(BadValue, ex.what() << ". Statement is <" << - text_statements_[stindex] << ">"); - } - - // As the address is the primary key in the table, we can't return - // two rows, so we don't bother checking whether multiple rows have - // been returned. - - } else if (status == 1) { - checkError(status, stindex, "unable to fetch results"); - - } else { - // @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 no action is needed. - } -} - -// Extraction of leases from the database. Much the code has common logic -// with the difference between V4 and V6 being the data types of the -// objects involved. For this reason, the common logic is inside a -// template method. +// Extraction of leases from the database. +// +// All getLease() methods ultimately call getLeaseCollection(). This +// binds the input parameters passed to it with the appropriate prepared +// statement and executes the statement. It then gets the results from the +// database. getlease() methods that expect a single result back call it +// with the "single" parameter set true: this causes an exception to be +// generated if multiple records can be retrieved from the result set. (Such +// an occurrence either indicates corruption in the database, or that an +// assumption that a query can only return a single record is incorrect.) +// Methods that require a collection of records have "single" set to the +// default value of false. The logic is the same for both Lease4 and Lease6 +// objects, so the code is templated. +// +// Methods that require a collection of objects access this method through +// two interface methods (also called getLeaseCollection()). These are +// short enough as to be defined in the header file: all they do is to supply +// the appropriate MySqlLeaseXExchange object depending on the type of the +// LeaseCollection objects passed to them. +// +// Methods that require a single object to be returned access the method +// through two interface methods (called getLease()). As well as supplying +// the appropriate exchange object, they convert between lease collection +// holding zero or one leases into an appropriate Lease object. template void MySqlLeaseMgr::getLeaseCollection(StatementIndex stindex, MYSQL_BIND* inbind, Exchange& exchange, - LeaseCollection& result) const { + LeaseCollection& result, + bool single) const { // Bind the input parameters to the statement and bind the output // to fields in the exchange object, then execute the prepared statement. @@ -1037,6 +1005,7 @@ void MySqlLeaseMgr::getLeaseCollection(StatementIndex stindex, // with the call to mysql_stmt_fetch when this method exits, then // retrieve the data. MySqlFreeResult fetch_release(statements_[stindex]); + int count = 0; while ((status = mysql_stmt_fetch(statements_[stindex])) == 0) { try { result.push_back(exchange->getLeaseData()); @@ -1046,11 +1015,17 @@ void MySqlLeaseMgr::getLeaseCollection(StatementIndex stindex, isc_throw(BadValue, ex.what() << ". Statement is <" << 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]); + } } // How did the fetch end? if (status == 1) { - // Error - unable to fecth results + // Error - unable to fetch results checkError(status, stindex, "unable to fetch results"); } else if (status == MYSQL_DATA_TRUNCATED) { // @TODO Handle truncation @@ -1059,6 +1034,46 @@ void MySqlLeaseMgr::getLeaseCollection(StatementIndex stindex, } +void MySqlLeaseMgr::getLease(StatementIndex stindex, MYSQL_BIND* inbind, + Lease4Ptr& result) const { + // Create appropriate collection object and get all leases matching + // the selection criteria. The "single" paraeter is true to indicate + // that the called method should throw an exception if multiple + // matching records are found: this particular method is called when only + // one or zero matches is expected. + Lease4Collection collection; + getLeaseCollection(stindex, inbind, exchange4_, collection, true); + + // Return single record if present, else clear the lease. + if (collection.empty()) { + result.reset(); + } else { + result = *collection.begin(); + } +} + +void MySqlLeaseMgr::getLease(StatementIndex stindex, MYSQL_BIND* inbind, + Lease6Ptr& result) const { + // Create appropriate collection object and get all leases matching + // the selection criteria. The "single" paraeter is true to indicate + // that the called method should throw an exception if multiple + // matching records are found: this particular method is called when only + // one or zero matches is expected. + Lease6Collection collection; + getLeaseCollection(stindex, inbind, exchange6_, collection, true); + + // Return single record if present, else clear the lease. + if (collection.empty()) { + result.reset(); + } else { + result = *collection.begin(); + } +} + + +// Basic lease access methods. Obtain leases from the database using various +// criteria. + Lease4Ptr MySqlLeaseMgr::getLease4(const isc::asiolink::IOAddress& addr) const { // Set up the WHERE clause value @@ -1068,11 +1083,11 @@ MySqlLeaseMgr::getLease4(const isc::asiolink::IOAddress& addr) const { uint32_t addr4 = static_cast(addr); inbind[0].buffer_type = MYSQL_TYPE_LONG; inbind[0].buffer = reinterpret_cast(&addr4); - inbind[0].is_unsigned = static_cast(1); + inbind[0].is_unsigned = MLM_TRUE; // Get the data Lease4Ptr result; - getLease(GET_LEASE4_ADDR, inbind, exchange4_, result); + getLease(GET_LEASE4_ADDR, inbind, result); return (result); } @@ -1117,7 +1132,7 @@ MySqlLeaseMgr::getLease4(const HWAddr& hwaddr) const { // Get the data Lease4Collection result; - getLeaseCollection(GET_LEASE4_HWADDR, inbind, exchange4_, result); + getLeaseCollection(GET_LEASE4_HWADDR, inbind, result); return (result); } @@ -1144,11 +1159,11 @@ MySqlLeaseMgr::getLease4(const HWAddr& hwaddr, SubnetID subnet_id) const { inbind[1].buffer_type = MYSQL_TYPE_LONG; inbind[1].buffer = reinterpret_cast(&subnet_id); - inbind[1].is_unsigned = my_bool(1); + inbind[1].is_unsigned = MLM_TRUE; // Get the data Lease4Ptr result; - getLease(GET_LEASE4_HWADDR_SUBID, inbind, exchange4_, result); + getLease(GET_LEASE4_HWADDR_SUBID, inbind, result); return (result); } @@ -1169,7 +1184,7 @@ MySqlLeaseMgr::getLease4(const ClientId& clientid) const { // Get the data Lease4Collection result; - getLeaseCollection(GET_LEASE4_CLIENTID, inbind, exchange4_, result); + getLeaseCollection(GET_LEASE4_CLIENTID, inbind, result); return (result); } @@ -1190,11 +1205,11 @@ MySqlLeaseMgr::getLease4(const ClientId& clientid, SubnetID subnet_id) const { inbind[1].buffer_type = MYSQL_TYPE_LONG; inbind[1].buffer = reinterpret_cast(&subnet_id); - inbind[1].is_unsigned = my_bool(1); + inbind[1].is_unsigned = MLM_TRUE; // Get the data Lease4Ptr result; - getLease(GET_LEASE4_CLIENTID_SUBID, inbind, exchange4_, result); + getLease(GET_LEASE4_CLIENTID_SUBID, inbind, result); return (result); } @@ -1217,7 +1232,7 @@ MySqlLeaseMgr::getLease6(const isc::asiolink::IOAddress& addr) const { inbind[0].length = &addr6_length; Lease6Ptr result; - getLease(GET_LEASE6_ADDR, inbind, exchange6_, result); + getLease(GET_LEASE6_ADDR, inbind, result); return (result); } @@ -1253,11 +1268,11 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid) const { // IAID inbind[1].buffer_type = MYSQL_TYPE_LONG; inbind[1].buffer = reinterpret_cast(&iaid); - inbind[1].is_unsigned = static_cast(1); + inbind[1].is_unsigned = MLM_TRUE; // ... and get the data Lease6Collection result; - getLeaseCollection(GET_LEASE6_DUID_IAID, inbind, exchange6_, result); + getLeaseCollection(GET_LEASE6_DUID_IAID, inbind, result); return (result); } @@ -1284,39 +1299,30 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid, // IAID inbind[1].buffer_type = MYSQL_TYPE_LONG; inbind[1].buffer = reinterpret_cast(&iaid); - inbind[1].is_unsigned = static_cast(1); + inbind[1].is_unsigned = MLM_TRUE; // Subnet ID inbind[2].buffer_type = MYSQL_TYPE_LONG; inbind[2].buffer = reinterpret_cast(&subnet_id); - inbind[2].is_unsigned = static_cast(1); + inbind[2].is_unsigned = MLM_TRUE; Lease6Ptr result; - getLease(GET_LEASE6_DUID_IAID_SUBID, inbind, exchange6_, result); + getLease(GET_LEASE6_DUID_IAID_SUBID, inbind, result); return (result); } +// Update lease methods. These comprise common code that handles the actual +// update, and type-specific methods that set up the parameters for the prepared +// statement depending on the type of lease. +template void -MySqlLeaseMgr::updateLease4(const Lease4Ptr& lease) { - const StatementIndex stindex = UPDATE_LEASE4; - - // Create the MYSQL_BIND array for the data being updated - std::vector bind = exchange4_->createBindForSend(lease); - - // Set up the WHERE clause and append it to the MYSQL_BIND array - MYSQL_BIND where; - memset(&where, 0, sizeof(where)); - - uint32_t addr4 = static_cast(lease->addr_); - where.buffer_type = MYSQL_TYPE_LONG; - where.buffer = reinterpret_cast(&addr4); - where.is_unsigned = my_bool(1); - bind.push_back(where); +MySqlLeaseMgr::updateLease(StatementIndex stindex, MYSQL_BIND* bind, + const LeasePtr& lease) { // Bind the parameters to the statement - int status = mysql_stmt_bind_param(statements_[stindex], &bind[0]); + int status = mysql_stmt_bind_param(statements_[stindex], bind); checkError(status, stindex, "unable to bind parameters"); // Execute @@ -1338,6 +1344,28 @@ MySqlLeaseMgr::updateLease4(const Lease4Ptr& lease) { } +void +MySqlLeaseMgr::updateLease4(const Lease4Ptr& lease) { + const StatementIndex stindex = UPDATE_LEASE4; + + // Create the MYSQL_BIND array for the data being updated + std::vector bind = exchange4_->createBindForSend(lease); + + // Set up the WHERE clause and append it to the MYSQL_BIND array + MYSQL_BIND where; + memset(&where, 0, sizeof(where)); + + uint32_t addr4 = static_cast(lease->addr_); + where.buffer_type = MYSQL_TYPE_LONG; + where.buffer = reinterpret_cast(&addr4); + where.is_unsigned = MLM_TRUE; + bind.push_back(where); + + // Drop to common update code + updateLease(stindex, &bind[0], lease); +} + + void MySqlLeaseMgr::updateLease6(const Lease6Ptr& lease) { const StatementIndex stindex = UPDATE_LEASE6; @@ -1360,47 +1388,19 @@ MySqlLeaseMgr::updateLease6(const Lease6Ptr& lease) { where.length = &addr6_length; bind.push_back(where); - // Bind the parameters to the statement - int status = mysql_stmt_bind_param(statements_[stindex], &bind[0]); - checkError(status, stindex, "unable to bind parameters"); - - // Execute - status = mysql_stmt_execute(statements_[stindex]); - checkError(status, stindex, "unable to execute"); - - // See how many rows were affected. The statement should only delete a - // single row. - int affected_rows = mysql_stmt_affected_rows(statements_[stindex]); - if (affected_rows == 0) { - isc_throw(NoSuchLease, "unable to update lease for address " << - addr6 << " as it does not exist"); - } else if (affected_rows > 1) { - // Should not happen - primary key constraint should only have selected - // one row. - isc_throw(DbOperationError, "apparently updated more than one lease " - "that had the address " << addr6); - } + // Drop to common update code + updateLease(stindex, &bind[0], lease); } -// TODO: Replace by deleteLease +// Delete lease methods. As with other groups of methods, these comprise +// a per-type method that sets up the relevant MYSQL_BIND array and a +// common method than handles the common processing. + bool -MySqlLeaseMgr::deleteLease4(const isc::asiolink::IOAddress& addr) { - const StatementIndex stindex = DELETE_LEASE4; - - // Set up the WHERE clause value - MYSQL_BIND inbind[1]; - memset(inbind, 0, sizeof(inbind)); - - uint32_t addr4 = static_cast(addr); - - // See the earlier description of the use of "const_cast" when accessing - // the address for an explanation of the reason. - inbind[0].buffer_type = MYSQL_TYPE_LONG; - inbind[0].buffer = reinterpret_cast(&addr4); - inbind[0].is_unsigned = my_bool(1); +MySqlLeaseMgr::deleteLease(StatementIndex stindex, MYSQL_BIND* bind) { // Bind the input parameters to the statement - int status = mysql_stmt_bind_param(statements_[stindex], inbind); + int status = mysql_stmt_bind_param(statements_[stindex], bind); checkError(status, stindex, "unable to bind WHERE clause parameter"); // Execute @@ -1413,9 +1413,27 @@ MySqlLeaseMgr::deleteLease4(const isc::asiolink::IOAddress& addr) { } +bool +MySqlLeaseMgr::deleteLease4(const isc::asiolink::IOAddress& addr) { + + // Set up the WHERE clause value + MYSQL_BIND inbind[1]; + memset(inbind, 0, sizeof(inbind)); + + uint32_t addr4 = static_cast(addr); + + // See the earlier description of the use of "const_cast" when accessing + // the address for an explanation of the reason. + inbind[0].buffer_type = MYSQL_TYPE_LONG; + inbind[0].buffer = reinterpret_cast(&addr4); + inbind[0].is_unsigned = MLM_TRUE; + + return (deleteLease(DELETE_LEASE4, inbind)); +} + + bool MySqlLeaseMgr::deleteLease6(const isc::asiolink::IOAddress& addr) { - const StatementIndex stindex = DELETE_LEASE6; // Set up the WHERE clause value MYSQL_BIND inbind[1]; @@ -1431,19 +1449,10 @@ MySqlLeaseMgr::deleteLease6(const isc::asiolink::IOAddress& addr) { 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"); - - // Execute - status = mysql_stmt_execute(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 (deleteLease(DELETE_LEASE6, inbind)); } +// Miscellaneous database methods. std::string MySqlLeaseMgr::getName() const { diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.h b/src/lib/dhcpsrv/mysql_lease_mgr.h index d7dde46fdc..13d1c3474e 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.h +++ b/src/lib/dhcpsrv/mysql_lease_mgr.h @@ -28,7 +28,7 @@ namespace dhcp { // Define the current database schema values const uint32_t CURRENT_VERSION_VERSION = 0; -const uint32_t CURRENT_VERSION_MINOR = 1; +const uint32_t CURRENT_VERSION_MINOR = 2; // Forward declaration of the Lease exchange objects. This class is defined @@ -69,7 +69,7 @@ public: /// @brief Destructor (closes database) virtual ~MySqlLeaseMgr(); - /// @brief Adds an IPv4 lease. + /// @brief Adds an IPv4 lease /// /// @param lease lease to be added /// @@ -80,7 +80,7 @@ public: /// failed. virtual bool addLease(const Lease4Ptr& lease); - /// @brief Adds an IPv6 lease. + /// @brief Adds an IPv6 lease /// /// @param lease lease to be added /// @@ -210,13 +210,22 @@ public: /// @brief Updates IPv4 lease. /// + /// Updates the record of the lease in the database (as identified by the + /// address) with the data in the passed lease object. + /// /// @param lease4 The lease to be updated. /// - /// If no such lease is present, an exception will be thrown. + /// @throw isc::dhcp::NoSuchLease Attempt to update a lease that did not + /// exist. + /// @throw isc::dhcp::DbOperationError An operation on the open database has + /// failed. virtual void updateLease4(const Lease4Ptr& lease4); /// @brief Updates IPv6 lease. /// + /// Updates the record of the lease in the database (as identified by the + /// address) with the data in the passed lease object. + /// /// @param lease6 The lease to be updated. /// /// @throw isc::dhcp::NoSuchLease Attempt to update a lease that did not @@ -234,6 +243,9 @@ public: /// @param addr IPv4 address of the lease to be deleted. /// /// @return true if deletion was successful, false if no such lease exists + /// + /// @throw isc::dhcp::DbOperationError An operation on the open database has + /// failed. virtual bool deleteLease4(const isc::asiolink::IOAddress& addr); /// @brief Deletes an IPv6 lease. @@ -409,7 +421,8 @@ private: /// @brief Add Lease Common Code /// /// This method performs the common actions for both flavours of the - /// addLease method. + /// addLease method. It binds the contents of the lease object to + /// the prepated statement and adds it to the database. /// /// @param stindex Index of statemnent being executed /// @param bind MYSQL_BIND array that has been created for the type @@ -422,33 +435,6 @@ private: /// failed. bool addLease(StatementIndex stindex, std::vector& bind); - /// @brief Get Lease Common Code - /// - /// This method performs the common actions for obtaining a single lease - /// from the database. - /// - /// @param stindex Index of statement being executed - /// @param inbind MYSQL_BIND array for input parameters - /// @param exchange Exchange object to use - /// @param lease Lease object returned - template - void getLease(StatementIndex stindex, MYSQL_BIND* inbind, - Exchange& exchange, LeasePtr& result) const; - - /// @brief Get Lease Collection Common Code - /// - /// This method performs the common actions for obtaining multiple leases - /// from the database. - /// - /// @param stindex Index of statement being executed - /// @param inbind MYSQL_BIND array for input parameters - /// @param exchange Exchange object to use - /// @param lease LeaseCollection object returned. Note that any data in - /// the collection is cleared before new data is added. - template - void getLeaseCollection(StatementIndex stindex, MYSQL_BIND* inbind, - Exchange& exchange, LeaseCollection& result) const; - /// @brief Binds Parameters and Executes /// /// This method abstracts a lot of common processing from the getXxxx() @@ -472,6 +458,132 @@ private: void bindAndExecute(StatementIndex stindex, Exchange& exchange, MYSQL_BIND* inbind) const; + /// @brief Get Lease Collection Common Code + /// + /// This method performs the common actions for obtaining multiple leases + /// from the database. + /// + /// @param stindex Index of statement being executed + /// @param inbind MYSQL_BIND array for input parameters + /// @param exchange Exchange object to use + /// @param lease LeaseCollection object returned. Note that any data in + /// the collection is cleared before new data is added. + /// @param single If true, only a single data item is to be retrieved. + /// If more than one is present, a MultipleRecords exception will + /// be thrown. + /// + /// @throw isc::dhcp::BadValue Data retrieved from the database was invalid. + /// @throw isc::dhcp::DbOperationError An operation on the open database has + /// failed. + /// @throw isc::dhcp::MultipleRecords Multiple records were retrieved + /// from the database where only one was expected. + template + void getLeaseCollection(StatementIndex stindex, MYSQL_BIND* inbind, + Exchange& exchange, LeaseCollection& result, + bool single = false) const; + + /// @brief Get Lease Collection + /// + /// Gets a collection of Lease4 objects. This is just an interface to + /// the get lease collection common code. + /// + /// @param stindex Index of statement being executed + /// @param inbind MYSQL_BIND array for input parameters + /// @param lease LeaseCollection object returned. Note that any data in + /// the collection is cleared before new data is added. + /// + /// @throw isc::dhcp::BadValue Data retrieved from the database was invalid. + /// @throw isc::dhcp::DbOperationError An operation on the open database has + /// failed. + /// @throw isc::dhcp::MultipleRecords Multiple records were retrieved + /// from the database where only one was expected. + void getLeaseCollection(StatementIndex stindex, MYSQL_BIND* inbind, + Lease4Collection& result) const { + getLeaseCollection(stindex, inbind, exchange4_, result); + } + + /// @brief Get Lease Collection + /// + /// Gets a collection of Lease6 objects. This is just an interface to + /// the get lease collection common code. + /// + /// @param stindex Index of statement being executed + /// @param inbind MYSQL_BIND array for input parameters + /// @param lease LeaseCollection object returned. Note that any data in + /// the collection is cleared before new data is added. + /// + /// @throw isc::dhcp::BadValue Data retrieved from the database was invalid. + /// @throw isc::dhcp::DbOperationError An operation on the open database has + /// failed. + /// @throw isc::dhcp::MultipleRecords Multiple records were retrieved + /// from the database where only one was expected. + void getLeaseCollection(StatementIndex stindex, MYSQL_BIND* inbind, + Lease6Collection& result) const { + getLeaseCollection(stindex, inbind, exchange6_, result); + } + + /// @brief Get Lease6 Common Code + /// + /// This method performs the common actions for the various getLease4() + /// methods. It acts as an interface to the getLeaseCollection() method, + /// but retrieveing only a single lease. + /// + /// @param stindex Index of statement being executed + /// @param inbind MYSQL_BIND array for input parameters + /// @param lease Lease4 object returned + void getLease(StatementIndex stindex, MYSQL_BIND* inbind, + Lease4Ptr& result) const; + + /// @brief Get Lease4 Common Code + /// + /// This method performs the common actions for the various getLease4() + /// methods. It acts as an interface to the getLeaseCollection() method, + /// but retrieveing only a single lease. + /// + /// @param stindex Index of statement being executed + /// @param inbind MYSQL_BIND array for input parameters + /// @param lease Lease6 object returned + void getLease(StatementIndex stindex, MYSQL_BIND* inbind, + Lease6Ptr& result) const; + + /// @brief Update lease common code + /// + /// Holds the common code for updating a lease. It binds the parameters + /// to the prepared statement, executes it, then checks how many rows + /// were affected. + /// + /// @param stindex Index of prepared statement to be executed + /// @param bind Array of MYSQL_BIND objects representing the parameters. + /// (Note that the number is determined by the number of parameters + /// in the statement.) + /// @param lease Pointer to the lease object whose record is being updated. + /// + /// @throw NoSuchLease Could not update a lease because no lease matches + /// the address given. + /// @throw isc::dhcp::DbOperationError An operation on the open database has + /// failed. + template + void updateLease(StatementIndex stindex, MYSQL_BIND* bind, + const LeasePtr& lease); + + /// @brief Delete lease common code + /// + /// Holds the common code for deleting a lease. It binds the parameters + /// to the prepared statement, executes the statement and checks to + /// see how many rows were deleted. + /// + /// @param stindex Index of prepared statement to be executed + /// @param bind Array of MYSQL_BIND objects representing the parameters. + /// (Note that the number is determined by the number of parameters + /// in the statement.) + /// + /// @return true if one or more rows were deleted, false if none were + /// deleted. + /// + /// @throw isc::dhcp::DbOperationError An operation on the open database has + /// failed. + bool deleteLease(StatementIndex stindex, MYSQL_BIND* bind); + /// @brief Check Error and Throw Exception /// /// Virtually all MySQL functions return a status which, if non-zero, @@ -496,15 +608,15 @@ 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".) - MYSQL* mysql_; ///< MySQL context object - std::vector text_statements_; ///< Raw text of statements - std::vector statements_; ///< Prepared statements + /// The exchange objects are used for transfer of data to/from the database. + /// They are pointed-to objects as the contents may change in "const" calls, + /// while the rest of this object does not. (At alternative would be to + /// declare them as "mutable".) boost::scoped_ptr exchange4_; ///< Exchange object boost::scoped_ptr exchange6_; ///< Exchange object + MYSQL* mysql_; ///< MySQL context object + std::vector statements_; ///< Prepared statements + std::vector text_statements_; ///< Raw text of statements }; }; // end of isc::dhcp namespace diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc index 466abe4545..e7b64ab3f3 100644 --- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -32,10 +32,10 @@ using namespace std; namespace { -// Creation of the schema +// This holds statements to create and destroy the schema. #include "schema_copy.h" -// IPv4 and IPv6 addresseses +// IPv4 and IPv6 addresses used in the tests const char* ADDRESS4[] = { "192.0.2.0", "192.0.2.1", "192.0.2.2", "192.0.2.3", "192.0.2.4", "192.0.2.5", "192.0.2.6", "192.0.2.7", @@ -47,8 +47,9 @@ const char* ADDRESS6[] = { NULL }; -// Connection strings. Assume: +// Connection strings. // Database: keatest +// Host: localhost // Username: keatest // Password: keatest const char* VALID_TYPE = "type=mysql"; @@ -71,8 +72,7 @@ string connectionString(const char* type, const char* name, const char* host, if (type != NULL) { result += string(type); } - - if (name != NULL) { +if (name != NULL) { if (! result.empty()) { result += space; } @@ -113,7 +113,7 @@ validConnectionString() { // @brief Clear everything from the database // // There is no error checking in this code: if something fails, one of the -// tests will fall over. +// tests will (should) fall over. void destroySchema() { // Initialise MYSQL handle; @@ -147,7 +147,7 @@ void createSchema() { (void) mysql_real_connect(&handle, "localhost", "keatest", "keatest", "keatest", 0, NULL, 0); - // Get rid of everything in it. + // Execute creation statements. for (int i = 0; create_statement[i] != NULL; ++i) { (void) mysql_query(&handle, create_statement[i]); } @@ -156,20 +156,16 @@ void createSchema() { (void) mysql_close(&handle); } -// Note: Doxygen "///" not used - even though Doxygen is used to -// document class and methods - to avoid the comments appearing -// in the programming manual. - -// @brief Test Fixture Class -// -// Opens the database prior to each test and closes it afterwards. -// All pending transactions are deleted prior to closure. +/// @brief Test fixture class for testing MySQL Lease Manager +/// +/// Opens the database prior to each test and closes it afterwards. +/// All pending transactions are deleted prior to closure. class MySqlLeaseMgrTest : public ::testing::Test { public: - // @brief Constructor - // - // Deletes everything from the database and opens it. + /// @brief Constructor + /// + /// Deletes everything from the database and opens it. MySqlLeaseMgrTest() { // Initialize address strings and IOAddresses for (int i = 0; ADDRESS4[i] != NULL; ++i) { @@ -202,38 +198,38 @@ public: lmptr_ = &(LeaseMgrFactory::instance()); } - // @brief Destructor - // - // Rolls back all pending transactions. The deletion of the - // lmptr_ member variable will close the database. Then - // reopen it and delete everything created by the test. + /// @brief Destructor + /// + /// Rolls back all pending transactions. The deletion of the + /// lmptr_ member variable will close the database. Then + /// reopen it and delete everything created by the test. virtual ~MySqlLeaseMgrTest() { lmptr_->rollback(); LeaseMgrFactory::destroy(); destroySchema(); } - // @brief Reopen the database - // - // Closes the database and re-open it. Anything committed should be - // visible. + /// @brief Reopen the database + /// + /// Closes the database and re-open it. Anything committed should be + /// visible. void reopen() { LeaseMgrFactory::destroy(); LeaseMgrFactory::create(validConnectionString()); lmptr_ = &(LeaseMgrFactory::instance()); } - // @brief Initialize Lease4 Fields - // - // Returns a pointer to a Lease4 structure. Different values are put - // in the lease according to the address passed. - // - // This is just a convenience function for the test methods. - // - // @param address Address to use for the initialization - // - // @return Lease4Ptr. This will not point to anything if the initialization - // failed (e.g. unknown address). + /// @brief Initialize Lease4 Fields + /// + /// Returns a pointer to a Lease4 structure. Different values are put + /// in the lease according to the address passed. + /// + /// This is just a convenience function for the test methods. + /// + /// @param address Address to use for the initialization + /// + /// @return Lease4Ptr. This will not point to anything if the initialization + /// failed (e.g. unknown address). Lease4Ptr initializeLease4(std::string address) { Lease4Ptr lease(new Lease4()); @@ -332,17 +328,17 @@ public: return (lease); } - // @brief Initialize Lease6 Fields - // - // Returns a pointer to a Lease6 structure. Different values are put - // in the lease according to the address passed. - // - // This is just a convenience function for the test methods. - // - // @param address Address to use for the initialization - // - // @return Lease6Ptr. This will not point to anything if the initialization - // failed (e.g. unknown address). + /// @brief Initialize Lease6 Fields + /// + /// Returns a pointer to a Lease6 structure. Different values are put + /// in the lease according to the address passed. + /// + /// This is just a convenience function for the test methods. + /// + /// @param address Address to use for the initialization + /// + /// @return Lease6Ptr. This will not point to anything if the initialization + /// failed (e.g. unknown address). Lease6Ptr initializeLease6(std::string address) { Lease6Ptr lease(new Lease6()); @@ -363,31 +359,34 @@ public: lease->type_ = Lease6::LEASE_IA_TA; lease->prefixlen_ = 4; lease->iaid_ = 142; - lease->duid_ = boost::shared_ptr(new DUID(vector(8, 0x77))); - lease->preferred_lft_ = 900; // Preferred lifetime - lease->valid_lft_ = 8677; // Actual lifetime - lease->cltt_ = 168256; // Current time of day - lease->subnet_id_ = 23; // Arbitrary number + lease->duid_ = boost::shared_ptr( + new DUID(vector(8, 0x77))); + lease->preferred_lft_ = 900; + lease->valid_lft_ = 8677; + lease->cltt_ = 168256; + lease->subnet_id_ = 23; } else if (address == straddress6_[1]) { lease->type_ = Lease6::LEASE_IA_TA; lease->prefixlen_ = 0; lease->iaid_ = 42; - lease->duid_ = boost::shared_ptr(new DUID(vector(8, 0x42))); - lease->preferred_lft_ = 3600; // Preferred lifetime - lease->valid_lft_ = 3677; // Actual lifetime - lease->cltt_ = 123456; // Current time of day - lease->subnet_id_ = 73; // Arbitrary number + lease->duid_ = boost::shared_ptr( + new DUID(vector(8, 0x42))); + lease->preferred_lft_ = 3600; + lease->valid_lft_ = 3677; + lease->cltt_ = 123456; + lease->subnet_id_ = 73; } else if (address == straddress6_[2]) { lease->type_ = Lease6::LEASE_IA_PD; lease->prefixlen_ = 7; lease->iaid_ = 89; - lease->duid_ = boost::shared_ptr(new DUID(vector(8, 0x3a))); - lease->preferred_lft_ = 1800; // Preferred lifetime - lease->valid_lft_ = 5412; // Actual lifetime - lease->cltt_ = 234567; // Current time of day - lease->subnet_id_ = 73; // Same as for straddress6_1 + lease->duid_ = boost::shared_ptr( + new DUID(vector(8, 0x3a))); + lease->preferred_lft_ = 1800; + lease->valid_lft_ = 5412; + lease->cltt_ = 234567; + lease->subnet_id_ = 73; // Same as lease 1 } else if (address == straddress6_[3]) { lease->type_ = Lease6::LEASE_IA_NA; @@ -403,54 +402,58 @@ public: // should be able to cope with valid lifetimes up to 0xffffffff. // However, this will lead to overflows. // @TODO: test overflow conditions when code has been fixed - lease->preferred_lft_ = 7200; // Preferred lifetime - lease->valid_lft_ = 7000; // Actual lifetime - lease->cltt_ = 234567; // Current time of day - lease->subnet_id_ = 37; // Different from L1 and L2 + lease->preferred_lft_ = 7200; + lease->valid_lft_ = 7000; + lease->cltt_ = 234567; + lease->subnet_id_ = 37; } else if (address == straddress6_[4]) { // Same DUID and IAID as straddress6_1 lease->type_ = Lease6::LEASE_IA_PD; lease->prefixlen_ = 15; lease->iaid_ = 42; - lease->duid_ = boost::shared_ptr(new DUID(vector(8, 0x42))); - lease->preferred_lft_ = 4800; // Preferred lifetime - lease->valid_lft_ = 7736; // Actual lifetime - lease->cltt_ = 222456; // Current time of day - lease->subnet_id_ = 75; // Arbitrary number + lease->duid_ = boost::shared_ptr( + new DUID(vector(8, 0x42))); + lease->preferred_lft_ = 4800; + lease->valid_lft_ = 7736; + lease->cltt_ = 222456; + lease->subnet_id_ = 671; } else if (address == straddress6_[5]) { // Same DUID and IAID as straddress6_1 lease->type_ = Lease6::LEASE_IA_PD; lease->prefixlen_ = 24; - lease->iaid_ = 42; - lease->duid_ = boost::shared_ptr(new DUID(vector(8, 0x42))); - lease->preferred_lft_ = 5400; // Preferred lifetime - lease->valid_lft_ = 7832; // Actual lifetime - lease->cltt_ = 227476; // Current time of day - lease->subnet_id_ = 175; // Arbitrary number + lease->iaid_ = 42; // Same as lease 4 + lease->duid_ = boost::shared_ptr( + new DUID(vector(8, 0x42))); // Same as lease 4 + lease->preferred_lft_ = 5400; + lease->valid_lft_ = 7832; + lease->cltt_ = 227476; + lease->subnet_id_ = 175; } else if (address == straddress6_[6]) { // Same DUID as straddress6_1 lease->type_ = Lease6::LEASE_IA_PD; lease->prefixlen_ = 24; lease->iaid_ = 93; - lease->duid_ = boost::shared_ptr(new DUID(vector(8, 0x42))); - lease->preferred_lft_ = 5400; // Preferred lifetime - lease->valid_lft_ = 1832; // Actual lifetime - lease->cltt_ = 627476; // Current time of day - lease->subnet_id_ = 112; // Arbitrary number + lease->duid_ = boost::shared_ptr( + new DUID(vector(8, 0x42))); // Same as lease 4 + lease->preferred_lft_ = 5400; + lease->valid_lft_ = 1832; + lease->cltt_ = 627476; + lease->subnet_id_ = 112; } else if (address == straddress6_[7]) { // Same IAID as straddress6_1 lease->type_ = Lease6::LEASE_IA_PD; lease->prefixlen_ = 24; lease->iaid_ = 42; - lease->duid_ = boost::shared_ptr(new DUID(vector(8, 0xe5))); - lease->preferred_lft_ = 5600; // Preferred lifetime - lease->valid_lft_ = 7975; // Actual lifetime - lease->cltt_ = 213876; // Current time of day - lease->subnet_id_ = 19; // Arbitrary number + lease->duid_ = boost::shared_ptr( + new DUID(vector(8, 0xe5))); + lease->preferred_lft_ = 5600; + lease->valid_lft_ = 7975; + lease->cltt_ = 213876; + lease->subnet_id_ = 19; } else { // Unknown address, return an empty pointer. @@ -461,13 +464,13 @@ public: return (lease); } - // @brief Check Leases Present and Different - // - // Checks a vector of lease pointers and ensures that all the leases - // they point to are present and different. If not, a GTest assertion - // will fail. - // - // @param leases Vector of pointers to leases + /// @brief Check Leases present and different + /// + /// Checks a vector of lease pointers and ensures that all the leases + /// they point to are present and different. If not, a GTest assertion + /// will fail. + /// + /// @param leases Vector of pointers to leases template void checkLeasesDifferent(const std::vector leases) const { @@ -484,11 +487,11 @@ public: } } - // @brief Creates Leases for the test - // - // Creates all leases for the test and checks that they are different. - // - // @return vector Vector of pointers to leases + /// @brief Creates leases for the test + /// + /// Creates all leases for the test and checks that they are different. + /// + /// @return vector Vector of pointers to leases vector createLeases4() { // Create leases for each address @@ -504,11 +507,11 @@ public: return (leases); } - // @brief Creates Leases for the test - // - // Creates all leases for the test and checks that they are different. - // - // @return vector Vector of pointers to leases + /// @brief Creates leases for the test + /// + /// Creates all leases for the test and checks that they are different. + /// + /// @return vector Vector of pointers to leases vector createLeases6() { // Create leases for each address @@ -527,21 +530,20 @@ public: // Member variables - LeaseMgr* lmptr_; // Pointer to the lease manager - - vector straddress4_; // String forms of IPv4 addresses - vector ioaddress4_; // IOAddress forms of IPv4 addresses - vector straddress6_; // String forms of IPv6 addresses - vector ioaddress6_; // IOAddress forms of IPv6 addresses + LeaseMgr* lmptr_; ///< Pointer to the lease manager + vector straddress4_; ///< String forms of IPv4 addresses + vector ioaddress4_; ///< IOAddress forms of IPv4 addresses + vector straddress6_; ///< String forms of IPv6 addresses + vector ioaddress6_; ///< IOAddress forms of IPv6 addresses }; -// @brief Check that Database Can Be Opened -// -// This test checks if the MySqlLeaseMgr can be instantiated. This happens -// only if the database can be opened. Note that this is not part of the -// MySqlLeaseMgr test fixure set. This test checks that the database can be -// opened: the fixtures assume that and check basic operations. +/// @brief Check that database can be opened +/// +/// This test checks if the MySqlLeaseMgr can be instantiated. This happens +/// only if the database can be opened. Note that this is not part of the +/// MySqlLeaseMgr test fixure set. This test checks that the database can be +/// opened: the fixtures assume that and check basic operations. TEST(MySqlOpenTest, OpenDatabase) { @@ -599,25 +601,25 @@ TEST(MySqlOpenTest, OpenDatabase) { destroySchema(); } -// @brief Check the getType() method -// -// getType() returns a string giving the type of the backend, which should -// always be "mysql". +/// @brief Check the getType() method +/// +/// getType() returns a string giving the type of the backend, which should +/// always be "mysql". TEST_F(MySqlLeaseMgrTest, getType) { EXPECT_EQ(std::string("mysql"), lmptr_->getType()); } -// @brief Check conversion functions -// -// The server works using cltt and valid_filetime. In the database, the -// information is stored as expire_time and valid-lifetime, which are -// related by -// -// expire_time = cltt + valid_lifetime -// -// This test checks that the conversion is correct. It does not check that the -// data is entered into the database correctly, only that the MYSQL_TIME -// structure used for the entry is correctly set up. +/// @brief Check conversion functions +/// +/// The server works using cltt and valid_filetime. In the database, the +/// information is stored as expire_time and valid-lifetime, which are +/// related by +/// +/// expire_time = cltt + valid_lifetime +/// +/// This test checks that the conversion is correct. It does not check that the +/// data is entered into the database correctly, only that the MYSQL_TIME +/// structure used for the entry is correctly set up. TEST_F(MySqlLeaseMgrTest, checkTimeConversion) { const time_t cltt = time(NULL); const uint32_t valid_lft = 86400; // 1 day @@ -649,14 +651,14 @@ TEST_F(MySqlLeaseMgrTest, checkTimeConversion) { } -// @brief Check getName() returns correct database name +/// @brief Check getName() returns correct database name TEST_F(MySqlLeaseMgrTest, getName) { EXPECT_EQ(std::string("keatest"), lmptr_->getName()); // @TODO: check for the negative } -// @brief Check that getVersion() returns the expected version +/// @brief Check that getVersion() returns the expected version TEST_F(MySqlLeaseMgrTest, checkVersion) { // Check version pair version; @@ -665,7 +667,7 @@ TEST_F(MySqlLeaseMgrTest, checkVersion) { EXPECT_EQ(CURRENT_VERSION_MINOR, version.second); } -// @brief Compare two Lease4 structures for equality +/// @brief Compare two Lease4 structures for equality void detailCompareLease(const Lease4Ptr& first, const Lease4Ptr& second) { // Compare address strings. Comparison of address objects is not used, as @@ -680,7 +682,7 @@ detailCompareLease(const Lease4Ptr& first, const Lease4Ptr& second) { EXPECT_EQ(first->subnet_id_, second->subnet_id_); } -// @brief Compare two Lease6 structures for equality +/// @brief Compare two Lease6 structures for equality void detailCompareLease(const Lease6Ptr& first, const Lease6Ptr& second) { EXPECT_EQ(first->type_, second->type_); @@ -700,17 +702,17 @@ detailCompareLease(const Lease6Ptr& first, const Lease6Ptr& second) { } -// @brief Basic Lease Checks -// -// Checks that the add/get/delete works. All are done within one -// test so that "rollback" can be used to remove trace of the tests -// from the database. -// -// Tests where a collection of leases can be returned are in the test -// Lease4Collection. -// -// @param leases Vector of leases used in the tests -// @param ioaddress Vector of IOAddresses used in the tests +/// @brief Basic Lease Checks +/// +/// Checks that the add/get/delete works. All are done within one +/// test so that "rollback" can be used to remove trace of the tests +/// from the database. +/// +/// Tests where a collection of leases can be returned are in the test +/// Lease4Collection. +/// +/// @param leases Vector of leases used in the tests +/// @param ioaddress Vector of IOAddresses used in the tests TEST_F(MySqlLeaseMgrTest, basicLease4) { // Get the leases to be used for the test. @@ -755,14 +757,14 @@ TEST_F(MySqlLeaseMgrTest, basicLease4) { } -// @brief Check individual Lease6 methods -// -// Checks that the add/get/delete works. All are done within one -// test so that "rollback" can be used to remove trace of the tests -// from the database. -// -// Tests where a collection of leases can be returned are in the test -// Lease6Collection. +/// @brief Check individual Lease6 methods +/// +/// Checks that the add/get/delete works. All are done within one +/// test so that "rollback" can be used to remove trace of the tests +/// from the database. +/// +/// Tests where a collection of leases can be returned are in the test +/// Lease6Collection. TEST_F(MySqlLeaseMgrTest, basicLease6) { // Get the leases to be used for the test. vector leases = createLeases6(); @@ -805,10 +807,10 @@ TEST_F(MySqlLeaseMgrTest, basicLease6) { detailCompareLease(leases[2], l_returned); } -// @brief Check GetLease4 methods - Access by Address and SubnetID -// -// Adds leases to the database and checks that they can be accessed via -// a the hardware address +/// @brief Check GetLease4 methods - access by Address and SubnetID +/// +/// Adds leases to the database and checks that they can be accessed via +/// a the hardware address TEST_F(MySqlLeaseMgrTest, getLease4AddressSubnetId) { // Get the leases to be used for the test. vector leases = createLeases4(); @@ -835,10 +837,10 @@ TEST_F(MySqlLeaseMgrTest, getLease4AddressSubnetId) { } -// @brief Check GetLease4 methods - Access by Hardware Address -// -// Adds leases to the database and checks that they can be accessed via -// a combination of DIUID and IAID. +/// @brief Check GetLease4 methods - access by Hardware Address +/// +/// Adds leases to the database and checks that they can be accessed via +/// a combination of DIUID and IAID. TEST_F(MySqlLeaseMgrTest, getLease4Hwaddr) { // Get the leases to be used for the test and add to the database vector leases = createLeases4(); @@ -888,11 +890,10 @@ TEST_F(MySqlLeaseMgrTest, getLease4Hwaddr) { -// @brief Check GetLease4 methods - Access by Hardware Address & Subnet ID -// -// Adds leases to the database and checks that they can be accessed via -// a combination of hardware address and subnet ID - +/// @brief Check GetLease4 methods - access by Hardware Address & Subnet ID +/// +/// Adds leases to the database and checks that they can be accessed via +/// a combination of hardware address and subnet ID TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSubnetId) { // Get the leases to be used for the test and add to the database vector leases = createLeases4(); @@ -925,13 +926,25 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSubnetId) { returned = lmptr_->getLease4(invalid_hwaddr, leases[1]->subnet_id_ + 1); EXPECT_FALSE(returned); + + // Add a second lease with the same values as the first and check that + // an attempt to access the database by these parameters throws a + // "multiple records" exception. (We expect there to be only one record + // with that combination, so getting them via getLeaseX() (as opposed + // to getLeaseXCollection() should throw an exception.) + EXPECT_TRUE(lmptr_->deleteLease4(leases[2]->addr_)); + leases[1]->addr_ = leases[2]->addr_; + EXPECT_TRUE(lmptr_->addLease(leases[1])); + EXPECT_THROW(returned = lmptr_->getLease4(leases[1]->hwaddr_, + leases[1]->subnet_id_), + isc::dhcp::MultipleRecords); } -// @brief Check GetLease4 methods - Access by Client ID -// -// Adds leases to the database and checks that they can be accessed via -// the Client ID +/// @brief Check GetLease4 methods - access by Client ID +/// +/// Adds leases to the database and checks that they can be accessed via +/// the Client ID. TEST_F(MySqlLeaseMgrTest, getLease4ClientId) { // Get the leases to be used for the test and add to the database vector leases = createLeases4(); @@ -976,11 +989,10 @@ TEST_F(MySqlLeaseMgrTest, getLease4ClientId) { } -// @brief Check GetLease4 methods - Access by Client ID & Subnet ID -// -// Adds leases to the database and checks that they can be accessed via -// a combination of client and subnet IDs. - +/// @brief Check GetLease4 methods - access by Client ID & Subnet ID +/// +/// Adds leases to the database and checks that they can be accessed via +/// a combination of client and subnet IDs. TEST_F(MySqlLeaseMgrTest, getLease4ClientIdSubnetId) { // Get the leases to be used for the test and add to the database vector leases = createLeases4(); @@ -1015,10 +1027,10 @@ TEST_F(MySqlLeaseMgrTest, getLease4ClientIdSubnetId) { } -// @brief Check GetLease6 methods - Access by DUID/IAID -// -// Adds leases to the database and checks that they can be accessed via -// a combination of DIUID and IAID. +/// @brief Check GetLease6 methods - access by DUID/IAID +/// +/// Adds leases to the database and checks that they can be accessed via +/// a combination of DIUID and IAID. TEST_F(MySqlLeaseMgrTest, getLease6DuidIaid) { // Get the leases to be used for the test. vector leases = createLeases6(); @@ -1062,10 +1074,10 @@ TEST_F(MySqlLeaseMgrTest, getLease6DuidIaid) { -// @brief Check GetLease6 methods - Access by DUID/IAID/SubnetID -// -// Adds leases to the database and checks that they can be accessed via -// a combination of DIUID and IAID. +/// @brief Check GetLease6 methods - access by DUID/IAID/SubnetID +/// +/// Adds leases to the database and checks that they can be accessed via +/// a combination of DIUID and IAID. TEST_F(MySqlLeaseMgrTest, getLease6DuidIaidSubnetId) { // Get the leases to be used for the test and add them to the database. vector leases = createLeases6(); @@ -1100,8 +1112,9 @@ TEST_F(MySqlLeaseMgrTest, getLease6DuidIaidSubnetId) { } -// Update lease4 tests - +/// @brief Lease4 update tests +/// +/// Checks that we are able to update a lease in the database. TEST_F(MySqlLeaseMgrTest, updateLease4) { // Get the leases to be used for the test and add them to the database. vector leases = createLeases4(); @@ -1144,10 +1157,9 @@ TEST_F(MySqlLeaseMgrTest, updateLease4) { } - -// @brief Lease6 Update Tests -// -// Checks that we are able to update a lease in the database. +/// @brief Lease6 update tests +/// +/// Checks that we are able to update a lease in the database. TEST_F(MySqlLeaseMgrTest, updateLease6) { // Get the leases to be used for the test. vector leases = createLeases6(); @@ -1157,7 +1169,6 @@ TEST_F(MySqlLeaseMgrTest, updateLease6) { EXPECT_TRUE(lmptr_->addLease(leases[1])); lmptr_->commit(); - reopen(); Lease6Ptr l_returned = lmptr_->getLease6(ioaddress6_[1]); ASSERT_TRUE(l_returned); detailCompareLease(leases[1], l_returned); @@ -1168,7 +1179,6 @@ TEST_F(MySqlLeaseMgrTest, updateLease6) { leases[1]->valid_lft_ *= 2; lmptr_->updateLease6(leases[1]); lmptr_->commit(); - reopen(); // ... and check what is returned is what is expected. l_returned.reset(); diff --git a/src/lib/dhcpsrv/tests/schema_copy.h b/src/lib/dhcpsrv/tests/schema_copy.h index 3e739daacd..72f6f88bcf 100644 --- a/src/lib/dhcpsrv/tests/schema_copy.h +++ b/src/lib/dhcpsrv/tests/schema_copy.h @@ -75,7 +75,7 @@ const char* create_statement[] = { "minor INT" ")", - "INSERT INTO schema_version VALUES (0, 1)", + "INSERT INTO schema_version VALUES (0, 2)", NULL };