BIND 10 trac2342, updated. 39c82ec533e3642015ce742a132b31350e91c928 [2342] Address remaining issues in review comment 6.

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Oct 24 18:20:15 UTC 2012


The branch, trac2342 has been updated
       via  39c82ec533e3642015ce742a132b31350e91c928 (commit)
       via  3e52b2fec4e3fed3898964ad0ae10d899b31c8bb (commit)
       via  942be75acdf13e402a5dc5582a16d782ac23d4eb (commit)
      from  b0ed08d21421ca321502cda1e9811a4bfe560275 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 39c82ec533e3642015ce742a132b31350e91c928
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Oct 24 19:18:33 2012 +0100

    [2342] Address remaining issues in review comment 6.

commit 3e52b2fec4e3fed3898964ad0ae10d899b31c8bb
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Oct 24 15:36:54 2012 +0100

    [2342] Fix problems with time conversion
    
    ... traced to not initializing a "struct" properly.

commit 942be75acdf13e402a5dc5582a16d782ac23d4eb
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Oct 24 14:27:43 2012 +0100

    [2342] Changed Lease6 "client_id" field to "duid"

-----------------------------------------------------------------------

Summary of changes:
 src/lib/dhcp/Makefile.am                       |   18 +-
 src/lib/dhcp/dhcpdb_create.mysql               |    4 +-
 src/lib/dhcp/lease_mgr_factory.h               |    3 +
 src/lib/dhcp/mysql_lease_mgr.cc                |  260 ++++++++++++++----------
 src/lib/dhcp/mysql_lease_mgr.h                 |   71 ++++---
 src/lib/dhcp/tests/Makefile.am                 |   19 +-
 src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc |   16 +-
 7 files changed, 227 insertions(+), 164 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/Makefile.am b/src/lib/dhcp/Makefile.am
index 945eaee..ff3b937 100644
--- a/src/lib/dhcp/Makefile.am
+++ b/src/lib/dhcp/Makefile.am
@@ -24,12 +24,7 @@ libb10_dhcp___la_SOURCES += iface_mgr.cc iface_mgr.h
 libb10_dhcp___la_SOURCES += iface_mgr_bsd.cc
 libb10_dhcp___la_SOURCES += iface_mgr_linux.cc
 libb10_dhcp___la_SOURCES += iface_mgr_sun.cc
-libb10_dhcp___la_SOURCES += lease_mgr.cc lease_mgr.h
-libb10_dhcp___la_SOURCES += lease_mgr_factory.cc lease_mgr_factory.h
 libb10_dhcp___la_SOURCES += libdhcp++.cc libdhcp++.h
-if HAVE_MYSQL
-libb10_dhcp___la_SOURCES += mysql_lease_mgr.cc mysql_lease_mgr.h
-endif
 libb10_dhcp___la_SOURCES += option4_addrlst.cc option4_addrlst.h
 libb10_dhcp___la_SOURCES += option6_addrlst.cc option6_addrlst.h
 libb10_dhcp___la_SOURCES += option6_iaaddr.cc option6_iaaddr.h
@@ -43,21 +38,26 @@ libb10_dhcp___la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
 libb10_dhcp___la_LIBADD   = $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
 libb10_dhcp___la_LIBADD  += $(top_builddir)/src/lib/util/libb10-util.la
 libb10_dhcp___la_LDFLAGS  = -no-undefined -version-info 2:0:0
-if HAVE_MYSQL
-libb10_dhcp___la_LDFLAGS += $(MYSQL_LIBS)
-endif
 
 libb10_dhcpsrv_la_SOURCES  = cfgmgr.cc cfgmgr.h
+libb10_dhcpsrv_la_SOURCES += addr_utilities.cc addr_utilities.h
+libb10_dhcpsrv_la_SOURCES += lease_mgr.cc lease_mgr.h
+libb10_dhcpsrv_la_SOURCES += lease_mgr_factory.cc lease_mgr_factory.h
+if HAVE_MYSQL
+libb10_dhcpsrv_la_SOURCES += mysql_lease_mgr.cc mysql_lease_mgr.h
+endif
 libb10_dhcpsrv_la_SOURCES += pool.cc pool.h
 libb10_dhcpsrv_la_SOURCES += subnet.cc subnet.h
 libb10_dhcpsrv_la_SOURCES += triplet.h
-libb10_dhcpsrv_la_SOURCES += addr_utilities.cc addr_utilities.h
 
 libb10_dhcpsrv_la_CXXFLAGS = $(AM_CXXFLAGS)
 libb10_dhcpsrv_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
 libb10_dhcpsrv_la_LIBADD   = $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
 libb10_dhcpsrv_la_LIBADD  += $(top_builddir)/src/lib/util/libb10-util.la
 libb10_dhcpsrv_la_LDFLAGS  = -no-undefined -version-info 2:0:0
+if HAVE_MYSQL
+libb10_dhcpsrv_la_LDFLAGS += $(MYSQL_LIBS)
+endif
 
 EXTRA_DIST  = README
 
diff --git a/src/lib/dhcp/dhcpdb_create.mysql b/src/lib/dhcp/dhcpdb_create.mysql
index 41bea24..b888b59 100644
--- a/src/lib/dhcp/dhcpdb_create.mysql
+++ b/src/lib/dhcp/dhcpdb_create.mysql
@@ -45,8 +45,8 @@ CREATE TABLE lease4 (
 CREATE TABLE lease6 (
     address VARCHAR(40) PRIMARY KEY NOT NULL,   # IPv6 address
     hwaddr VARBINARY(20),                       # Hardware address
-    client_id VARBINARY(128),                   # Client ID
-    lease_time INT UNSIGNED,                    # Length of the lease (seconds)
+    duid VARBINARY(128),                        # DUID
+    valid_lifetime INT UNSIGNED,                # Length of the lease (seconds)
     expire TIMESTAMP,                           # Expiration time of the lease
     subnet_id INT UNSIGNED,                     # Subnet identification
     pref_lifetime INT UNSIGNED,                 # Preferred lifetime
diff --git a/src/lib/dhcp/lease_mgr_factory.h b/src/lib/dhcp/lease_mgr_factory.h
index 164d2f9..abd1bba 100644
--- a/src/lib/dhcp/lease_mgr_factory.h
+++ b/src/lib/dhcp/lease_mgr_factory.h
@@ -50,6 +50,9 @@ public:
 ///
 /// Strictly speaking these functions could be stand-alone functions.  However,
 /// it is convenient to encapsulate them in a class for naming purposes.
+///
+/// @TODO: Will need to develop some form of registration mechanism for
+///        user-supplied backends (so that there is no need to modify the code).
 class LeaseMgrFactory {
 public:
     /// @brief Create an instance of a lease manager.
diff --git a/src/lib/dhcp/mysql_lease_mgr.cc b/src/lib/dhcp/mysql_lease_mgr.cc
index 1f56ec9..8fb73bd 100644
--- a/src/lib/dhcp/mysql_lease_mgr.cc
+++ b/src/lib/dhcp/mysql_lease_mgr.cc
@@ -22,6 +22,7 @@
 #include <dhcp/mysql_lease_mgr.h>
 #include <asiolink/io_address.h>
 
+using namespace isc;
 using namespace std;
 
 namespace {
@@ -48,9 +49,15 @@ namespace dhcp {
 
 /// @brief Exchange MySQL and Lease6 Data
 ///
-/// On the INSERT, SELECT and UPDATE statements, an array of MYSQL_BIND
-/// structures must be built to reflect the data being inserted or retrieved
-/// from the database.
+/// 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 - one array describes the
+/// data being exchanged with the database.  The other array describes the
+/// WHERE clause of the statement.
+///
+/// The array describing the information exchanged is common between the
+/// INSERT, UPDATE and SELECT statements, and this class handles the creation
+/// of that array and the insertion/extraction of data into/from it.
 ///
 /// Owing to the MySQL API, the process requires some intermediate variables
 /// to hold things like length etc.  This object holds the intermediate
@@ -67,9 +74,7 @@ public:
 
     /// @brief Create MYSQL_BIND objects for Lease6 Pointer
     ///
-    /// Fills in the bind_ objects for the Lease6 passed to it.
-    ///
-    /// The MySQL documentation 
+    /// Fills in the MYSQL_BIND objects for the Lease6 passed to it.
     ///
     /// @param lease Lease object to be added to the database
     ///
@@ -78,7 +83,7 @@ public:
     ///         valid only for as long as (1) this object is in existence and
     ///         (2) the lease object passed to it is in existence.  The
     ///         caller should NOT delete it.
-    MYSQL_BIND* CreateBindForSend(const Lease6Ptr& lease) {
+    MYSQL_BIND* createBindForSend(const Lease6Ptr& lease) {
         // Store lease object to ensure it remains valid.
         lease_ = lease;
 
@@ -102,32 +107,28 @@ public:
         bind_[1].buffer_length = hwaddr_length_;
         bind_[1].length = &hwaddr_length_;
 
-        // client_id: varchar(128)
-        clientid_ = lease_->duid_->getDuid();
-        clientid_length_ = clientid_.size();
+        // duid: varchar(128)
+        duid_ = lease_->duid_->getDuid();
+        duid_length_ = duid_.size();
 
         bind_[2].buffer_type = MYSQL_TYPE_BLOB;
-        bind_[2].buffer = reinterpret_cast<char*>(&(clientid_[0]));
-        bind_[2].buffer_length = clientid_length_;
-        bind_[2].length = &clientid_length_;
-
-        // The lease structure holds the client last transmission time (cltt_)
-        // and the valid lifetime (valid_lft_).  For convenience for external
-        // tools, the data stored in the database is expiry time (expire) and
-        // lease time (lease+time).  The relationship is given by:
-        //
-        // lease_time - valid_lft_
-        // expire = cltt_ + valid_lft_
-        //
-        MySqlLeaseMgr::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_,
-                                             expire_, lease_time_);
+        bind_[2].buffer = reinterpret_cast<char*>(&(duid_[0]));
+        bind_[2].buffer_length = duid_length_;
+        bind_[2].length = &duid_length_;
 
         // lease_time: unsigned int
         bind_[3].buffer_type = MYSQL_TYPE_LONG;
-        bind_[3].buffer = reinterpret_cast<char*>(&lease_time_);
+        bind_[3].buffer = reinterpret_cast<char*>(&lease->valid_lft_);
         bind_[3].is_unsigned = 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:
+        //
+        // expire = cltt_ + valid_lft_
+        MySqlLeaseMgr::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_,
+                                             expire_);
         bind_[4].buffer_type = MYSQL_TYPE_TIMESTAMP;
         bind_[4].buffer = reinterpret_cast<char*>(&expire_);
         bind_[4].buffer_length = sizeof(expire_);
@@ -149,7 +150,7 @@ public:
         lease_type_ = lease_->type_;
         bind_[7].buffer_type = MYSQL_TYPE_TINY;
         bind_[7].buffer = reinterpret_cast<char*>(&lease_type_);
-        bind_[7].is_unsigned = static_cast<my_bool>(1);
+        bind_[7].is_unsigned = true_;
 
         // iaid: unsigned int
         // Can use lease_->iaid_ directly as it is of type uint32_t.
@@ -201,16 +202,16 @@ public:
         bind_[2].error = &error_[1];
 
         // client_id: varbinary(128)
-        clientid_length_ = sizeof(clientid_buffer_);
+        duid_length_ = sizeof(duid_buffer_);
         bind_[2].buffer_type = MYSQL_TYPE_BLOB;
-        bind_[2].buffer = reinterpret_cast<char*>(clientid_buffer_);
-        bind_[2].buffer_length = clientid_length_;
-        bind_[2].length = &clientid_length_;
+        bind_[2].buffer = reinterpret_cast<char*>(duid_buffer_);
+        bind_[2].buffer_length = duid_length_;
+        bind_[2].length = &duid_length_;
         bind_[2].error = &error_[2];
 
         // lease_time: unsigned int
         bind_[3].buffer_type = MYSQL_TYPE_LONG;
-        bind_[3].buffer = reinterpret_cast<char*>(&lease_time_);
+        bind_[3].buffer = reinterpret_cast<char*>(&valid_lifetime_);
         bind_[3].is_unsigned = true_;
         bind_[3].error = &error_[3];
 
@@ -243,7 +244,7 @@ public:
         bind_[8].buffer = reinterpret_cast<char*>(&iaid_);
         bind_[8].is_unsigned = true_;
         bind_[8].error = &error_[8];
-
+ 
         // prefix_len: unsigned tinyint
         bind_[9].buffer_type = MYSQL_TYPE_TINY;
         bind_[9].buffer = reinterpret_cast<char*>(&prefixlen_);
@@ -269,16 +270,20 @@ public:
         Lease6Ptr result(new Lease6());
 
         // Success - 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.
         addr6_buffer_[addr6_length_] = '\0';
         std::string address = addr6_buffer_;
-        result->addr_ = isc::asiolink::IOAddress(address);
 
+        // Set the other data, converting time as needed.
+        result->addr_ = isc::asiolink::IOAddress(address);
         result->hwaddr_ = vector<uint8_t>(&hwaddr_buffer_[0],
                                           &hwaddr_buffer_[hwaddr_length_]);
-        result->duid_.reset(new DUID(clientid_buffer_, clientid_length_));
-        MySqlLeaseMgr::convertFromDatabaseTime(expire_, lease_time_,
-                                               result->cltt_,
-                                               result->valid_lft_);
+        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_;
 
@@ -297,7 +302,10 @@ public:
                 break;
 
             default:
-                isc_throw(BadValue, "invalid lease type returned");
+                isc_throw(BadValue, "invalid lease type returned (" <<
+                          lease_type_ << ") for lease with address " <<
+                          result->addr_.toText() << ". Only 0, 1, or 2 "
+                          "are allowed.");
         }
         result->iaid_ = iaid_;
         result->prefixlen_ = prefixlen_;
@@ -313,9 +321,9 @@ private:
                                         ///< array form of V6 address
     unsigned long   addr6_length_;      ///< Length of the address
     MYSQL_BIND      bind_[10];          ///< Static array for speed of access
-    std::vector<uint8_t> clientid_;     ///< Client identification
-    uint8_t         clientid_buffer_[DUID_MAX_LEN]; ///< Buffer form of DUID
-    unsigned long   clientid_length_;   ///< Length of client ID
+    std::vector<uint8_t> 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_[10];         ///< For error reporting
     MYSQL_TIME      expire_;            ///< Lease expiry time
     const my_bool   false_;             ///< "false" for MySql
@@ -323,7 +331,7 @@ private:
     unsigned long   hwaddr_length_;     ///< Length of hardware address
     uint32_t        iaid_;              ///< Identity association ID
     Lease6Ptr       lease_;             ///< Pointer to lease object
-    uint32_t        lease_time_;        ///< Lease time
+    uint32_t        valid_lifetime_;    ///< Lease time
     uint8_t         lease_type_;        ///< Lease type
     uint8_t         prefixlen_;         ///< Prefix length
     uint32_t        pref_lifetime_;     ///< Preferred lifetime
@@ -332,20 +340,64 @@ private:
 };
 
 
+MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters) 
+    : LeaseMgr(parameters), mysql_(NULL) {
+
+    // Allocate context for MySQL - it is destroyed in the destructor.
+    mysql_ = mysql_init(NULL);
+    if (mysql_ == NULL) {
+        isc_throw(DbOpenError, "unable to initialize MySQL");
+    }
+
+    // Open the database
+    openDatabase();
+
+    // Disable autocommit
+    my_bool result = mysql_autocommit(mysql_, 0);
+    if (result != 0) {
+        isc_throw(DbOperationError, mysql_error(mysql_));
+    }
+
+    // Prepare all statements likely to be used.
+    prepareStatements();
+}
+
+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.)
+    for (int i = 0; i < statements_.size(); ++i) {
+        if (statements_[i] != NULL) {
+            (void) mysql_stmt_close(statements_[i]);
+            statements_[i] = NULL;
+        }
+    }
+
+    // Close the database
+    mysql_close(mysql_);
+    mysql_ = NULL;
+}
+
 
 // Time conversion methods.
 //
 // Note that the MySQL TIMESTAMP data type (used for "expire") converts data
 // from the current timezone to UTC for storage, and from UTC to the current
-// timezone for retrieval.  This means that the external interface - cltt -
-// must be local time.
+// timezone for retrieval.
+//
+// This causes no problems providing that:
+// a) cltt is given in local time
+// b) We let the system take care of timezone conversion when converting
+//    from a time read from the database into a local time.
 
 void
-MySqlLeaseMgr::convertToDatabaseTime(time_t cltt, uint32_t valid_lft,
-                                    MYSQL_TIME& expire, uint32_t& lease_time) {
+MySqlLeaseMgr::convertToDatabaseTime(time_t cltt, uint32_t valid_lifetime,
+                                    MYSQL_TIME& expire) {
 
     // Calculate expiry time and convert to various date/time fields.
-    time_t expire_time = cltt + valid_lft;
+    time_t expire_time = cltt + valid_lifetime;
+
+    // Convert to broken-out time
     struct tm expire_tm;
     (void) localtime_r(&expire_time, &expire_tm);
 
@@ -358,19 +410,15 @@ MySqlLeaseMgr::convertToDatabaseTime(time_t cltt, uint32_t valid_lft,
     expire.second = expire_tm.tm_sec;
     expire.second_part = 0;                    // No fractional seconds
     expire.neg = static_cast<my_bool>(0);      // Not negative
-
-    // Set the lease time.
-    lease_time = valid_lft;
 }
 
 void
 MySqlLeaseMgr::convertFromDatabaseTime(const MYSQL_TIME& expire,
-                                       uint32_t lease_time, time_t& cltt,
-                                       uint32_t& valid_lft) {
-    valid_lft = lease_time;
+                                       uint32_t valid_lifetime, time_t& cltt) {
 
     // Copy across fields from MYSQL_TIME structure.
     struct tm expire_tm;
+    memset(&expire_tm, 0, sizeof(expire_tm));
 
     expire_tm.tm_year = expire.year - 1900;
     expire_tm.tm_mon = expire.month - 1;
@@ -378,23 +426,23 @@ MySqlLeaseMgr::convertFromDatabaseTime(const MYSQL_TIME& expire,
     expire_tm.tm_hour = expire.hour;
     expire_tm.tm_min = expire.minute;
     expire_tm.tm_sec = expire.second;
+    expire_tm.tm_isdst = -1;    // Let the system work out about DST 
 
     // Convert to local time
-    cltt = mktime(&expire_tm) - valid_lft;
+    cltt = mktime(&expire_tm) - valid_lifetime;
 }
 
 void
 MySqlLeaseMgr::openDatabase() {
 
     // Set up the values of the parameters
-    const char* host = NULL;
+    const char* host = "localhost";
     string shost;
     try {
         shost = getParameter("host");
         host = shost.c_str();
     } catch (...) {
-        // No host.  Fine, we'll use NULL
-        ;
+        // No host.  Fine, we'll use "localhost"
     }
 
     const char* user = NULL;
@@ -440,12 +488,13 @@ MySqlLeaseMgr::prepareStatement(StatementIndex index, const char* text) {
     // Validate that there is space for the statement in the statements array
     // and that nothing has been placed there before.
     if ((index >= statements_.size()) || (statements_[index] != NULL)) {
-        isc_throw(InvalidParameter, "invalid prepared statement index or "
-                  "statement index not null");
+        isc_throw(InvalidParameter, "invalid prepared statement index (" <<
+                  static_cast<int>(index) << ") or indexed prepared " <<
+                  "statement is not null");
     }
 
     // All OK, so prepare the statement
-    raw_statements_[index] = std::string(text);
+    text_statements_[index] = std::string(text);
 
     statements_[index] = mysql_stmt_init(mysql_);
     if (statements_[index] == NULL) {
@@ -466,65 +515,30 @@ MySqlLeaseMgr::prepareStatements() {
     statements_.clear();
     statements_.resize(NUM_STATEMENTS, NULL);
     
-    raw_statements_.clear();
-    raw_statements_.resize(NUM_STATEMENTS, std::string(""));
+    text_statements_.clear();
+    text_statements_.resize(NUM_STATEMENTS, std::string(""));
 
     // Now allocate the statements
     prepareStatement(DELETE_LEASE6,
                      "DELETE FROM lease6 WHERE address = ?");
     prepareStatement(GET_LEASE6,
-                     "SELECT address, hwaddr, client_id, "
-                         "lease_time, expire, subnet_id, pref_lifetime, "
+                     "SELECT address, hwaddr, duid, "
+                         "valid_lifetime, expire, subnet_id, pref_lifetime, "
                          "lease_type, iaid, prefix_len "
                          "FROM lease6 WHERE address = ?");
     prepareStatement(GET_VERSION,
                      "SELECT version, minor FROM schema_version");
     prepareStatement(INSERT_LEASE6,
-                     "INSERT INTO lease6(address, hwaddr, client_id, "
-                         "lease_time, expire, subnet_id, pref_lifetime, "
+                     "INSERT INTO lease6(address, hwaddr, duid, "
+                         "valid_lifetime, expire, subnet_id, pref_lifetime, "
                          "lease_type, iaid, prefix_len) "
                          "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)");
 }
 
-
-MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters) 
-    : LeaseMgr(parameters), mysql_(NULL) {
-
-    // Allocate context for MySQL - it is destroyed in the destructor.
-    mysql_ = mysql_init(NULL);
-
-    // Open the database
-    openDatabase();
-
-    // Disable autocommit
-    my_bool result = mysql_autocommit(mysql_, 0);
-    if (result != 0) {
-        isc_throw(DbOperationError, mysql_error(mysql_));
-    }
-
-    // Prepare all statements likely to be used.
-    prepareStatements();
-}
-
-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.)
-    for (int i = 0; i < statements_.size(); ++i) {
-        if (statements_[i] != NULL) {
-            (void) mysql_stmt_close(statements_[i]);
-            statements_[i] = NULL;
-        }
-    }
-
-    // Close the database
-    mysql_close(mysql_);
-    mysql_ = NULL;
-}
-
 bool
 MySqlLeaseMgr::addLease(const Lease4Ptr& /* lease */) {
-
+    isc_throw(NotImplemented, "MySqlLeaseMgr::addLease(const Lease4Ptr&) "
+              "not implemented yet");
     return (false);
 }
 
@@ -533,7 +547,7 @@ MySqlLeaseMgr::addLease(const Lease6Ptr& lease) {
 
     // Create the MYSQL_BIND array for the lease
     MySqlLease6Exchange exchange;
-    MYSQL_BIND* bind = exchange.CreateBindForSend(lease);
+    MYSQL_BIND* bind = exchange.createBindForSend(lease);
 
     // Bind the parameters to the statement
     int status = mysql_stmt_bind_param(statements_[INSERT_LEASE6], bind);
@@ -559,33 +573,45 @@ MySqlLeaseMgr::addLease(const Lease6Ptr& lease) {
 Lease4Ptr
 MySqlLeaseMgr::getLease4(const isc::asiolink::IOAddress& /* addr */,
                          SubnetID /* subnet_id */) const {
+    isc_throw(NotImplemented, "MySqlLeaseMgr::getLease4(const IOAddress&, SubnetID) "
+              "not implemented yet");
     return (Lease4Ptr());
 }
 
 Lease4Ptr
 MySqlLeaseMgr::getLease4(const isc::asiolink::IOAddress& /* addr */) const {
+    isc_throw(NotImplemented, "MySqlLeaseMgr::getLease4(const IOAddress&) "
+              "not implemented yet");
     return (Lease4Ptr());
 }
 
 Lease4Collection
 MySqlLeaseMgr::getLease4(const HWAddr& /* hwaddr */) const {
+    isc_throw(NotImplemented, "MySqlLeaseMgr::getLease4(const HWAddr&) "
+              "not implemented yet");
     return (Lease4Collection());
 }
 
 Lease4Ptr
 MySqlLeaseMgr::getLease4(const HWAddr& /* hwaddr */,
                          SubnetID /* subnet_id */) const {
+    isc_throw(NotImplemented, "MySqlLeaseMgr::getLease4(const HWAddr&, SubnetID) "
+              "not implemented yet");
     return (Lease4Ptr());
 }
 
 Lease4Collection
 MySqlLeaseMgr::getLease4(const ClientId& /* clientid */) const {
+    isc_throw(NotImplemented, "MySqlLeaseMgr::getLease4(const ClientID&) "
+              "not implemented yet");
     return (Lease4Collection());
 }
 
 Lease4Ptr
 MySqlLeaseMgr::getLease4(const ClientId& /* clientid */,
                          SubnetID /* subnet_id */) const {
+    isc_throw(NotImplemented, "MySqlLeaseMgr::getLease4(const ClientID&, SubnetID) "
+              "not implemented yet");
     return (Lease4Ptr());
 }
 
@@ -626,14 +652,14 @@ MySqlLeaseMgr::getLease6(const isc::asiolink::IOAddress& addr) const {
     if (status == 0) {
         try {
             result = exchange.getLeaseData();
-        } catch (isc::BadValue) {
+        } catch (const isc::BadValue& ex) {
             // Free up result set.
 
             (void) mysql_stmt_free_result(statements_[GET_LEASE6]);
             // Lease type is returned, to rethrow the exception with a bit
             // more data.
-            isc_throw(BadValue, "invalid lease type returned for <" <<
-                      raw_statements_[GET_LEASE6] << ">");
+            isc_throw(BadValue, ex.what() << ". Statement is <" <<
+                      text_statements_[GET_LEASE6] << ">");
         }
 
         // As the address is the primary key in the table, we can't return
@@ -655,25 +681,35 @@ MySqlLeaseMgr::getLease6(const isc::asiolink::IOAddress& addr) const {
 
 Lease6Collection
 MySqlLeaseMgr::getLease6(const DUID& /* duid */, uint32_t /* iaid */) const {
+    isc_throw(NotImplemented, "MySqlLeaseMgr::getLease6(const DUID&) "
+              "not implemented yet");
     return (Lease6Collection());
 }
 
 Lease6Ptr
 MySqlLeaseMgr::getLease6(const DUID& /* duid */, uint32_t /* iaid */,
                          SubnetID /* subnet_id */) const {
+    isc_throw(NotImplemented, "MySqlLeaseMgr::getLease4(const DUID&, SubnetID) "
+              "not implemented yet");
     return (Lease6Ptr());
 }
 
 void
 MySqlLeaseMgr::updateLease4(const Lease4Ptr& /* lease4 */) {
+    isc_throw(NotImplemented, "MySqlLeaseMgr::updateLease4(const Lease4Ptr&) "
+              "not implemented yet");
 }
 
 void
 MySqlLeaseMgr::updateLease6(const Lease6Ptr& /* lease6 */) {
+    isc_throw(NotImplemented, "MySqlLeaseMgr::updateLease6(const Lease6Ptr&) "
+              "not implemented yet");
 }
 
 bool
 MySqlLeaseMgr::deleteLease4(const isc::asiolink::IOAddress& /* addr */) {
+    isc_throw(NotImplemented, "MySqlLeaseMgr::deleteLease4(const IOAddress&) "
+              "not implemented yet");
     return (false);
 }
 
@@ -709,7 +745,13 @@ MySqlLeaseMgr::deleteLease6(const isc::asiolink::IOAddress& addr) {
 
 std::string
 MySqlLeaseMgr::getName() const {
-    return (std::string(""));
+    std::string name = "";
+    try {
+        name = getParameter("name");
+    } catch (...) {
+        ;
+    }
+    return (name);
 }
 
 std::string
@@ -726,7 +768,7 @@ MySqlLeaseMgr::getVersion() const {
     int status = mysql_stmt_execute(statements_[GET_VERSION]);
     if (status != 0) {
         isc_throw(DbOperationError, "unable to execute <"
-                  << raw_statements_[GET_VERSION] << "> - reason: " <<
+                  << text_statements_[GET_VERSION] << "> - reason: " <<
                   mysql_error(mysql_));
     }
 
diff --git a/src/lib/dhcp/mysql_lease_mgr.h b/src/lib/dhcp/mysql_lease_mgr.h
index 4a7ba74..349b3bb 100644
--- a/src/lib/dhcp/mysql_lease_mgr.h
+++ b/src/lib/dhcp/mysql_lease_mgr.h
@@ -22,7 +22,7 @@
 namespace isc {
 namespace dhcp {
 
-/// @brief Abstract Lease Manager
+/// @brief MySQL Lease Manager
 ///
 /// This is a concrete API for the backend for the MySQL database.
 class MySqlLeaseMgr : public LeaseMgr {
@@ -31,10 +31,10 @@ public:
     ///
     /// Uses the following keywords in the parameters passed to it to
     /// connect to the database:
-    /// - name - Name of the database to which to connect
-    /// - host - Host name to which to connect
-    /// - user - Username under which to connect.
-    /// - password - Password for "user" on the database.
+    /// - name - Name of the database to which to connect (mandatory)
+    /// - host - Host to which to connect (optional, defaults to "localhost")
+    /// - user - Username under which to connect (optional)
+    /// - password - Password for "user" on the database (optional)
     ///
     /// If the database is successfully opened, the version number in the
     /// schema_version table will be checked against hard-coded value in
@@ -44,6 +44,10 @@ public:
     ///
     /// @param parameters A data structure relating keywords and values
     ///        concerned with the database.
+    ///
+    /// @exception DbOpenError Error opening the database
+    /// @exception DbOperationError An operation on the open database has
+    ///            failed.
     MySqlLeaseMgr(const ParameterMap& parameters);
 
     /// @brief Destructor (closes database)
@@ -85,7 +89,7 @@ public:
     /// @brief Returns an IPv4 lease for specified IPv4 address
     ///
     /// This method return a lease that is associated with a given address.
-    /// For other query types (by hardware addr, by client-id) there can be
+    /// For other query types (by hardware addr, by DUID) there can be
     /// several leases in different subnets (e.g. for mobile clients that
     /// got address in different subnets). However, for a single address
     /// there can be only one lease, so this method returns a pointer to
@@ -156,6 +160,11 @@ public:
     /// @param addr address of the searched lease
     ///
     /// @return smart pointer to the lease (or NULL if a lease is not found)
+    ///
+    /// @exception BadValue record retrieved from database had an invalid
+    ///            lease type field.
+    /// @exception DbOperationError MySQL operation failed, exception will give
+    ///            text indicating the reason.
     virtual Lease6Ptr getLease6(const isc::asiolink::IOAddress& addr) const;
 
     /// @brief Returns existing IPv6 leases for a given DUID+IA combination
@@ -208,6 +217,9 @@ public:
     /// @param addr IPv4 address of the lease to be deleted.
     ///
     /// @return true if deletion was successful, false if no such lease exists
+    ///
+    /// @exception DbOperationError MySQL operation failed, exception will give
+    ///            text indicating the reason.
     virtual bool deleteLease6(const isc::asiolink::IOAddress& addr);
 
     /// @brief Returns backend name.
@@ -234,6 +246,9 @@ public:
     /// B>=A and B=C (it is ok to have newer backend, as it should be backward
     /// compatible)
     /// Also if B>C, some database upgrade procedure may be triggered
+    ///
+    /// @exception DbOperationError MySQL operation failed, exception will give
+    ///            text indicating the reason.
     virtual std::pair<uint32_t, uint32_t> getVersion() const;
 
     /// @brief Commit Transactions
@@ -262,36 +277,32 @@ public:
 
     /// @brief Convert Lease Time to Database Times
     ///
-    /// Within the DHCP servers, times are stored as cltt (client last transmit
-    /// time) and valid_lft (valid lifetime).  In the database, the information
-    /// is stored as lease_time (lease time) and expire (time of expiry of the
-    /// lease).  They are related by the equations:
+    /// Within the DHCP servers, times are stored as client last transmit time
+    /// and valid lifetime.  In the database, the information is stored as
+    /// valid lifetime and "expire" (time of expiry of the lease).  They are
+    /// related by the equation:
     ///
-    /// - lease_time = valid_lft
-    /// - expire = cltt + valid_lft
+    /// - expire = client last transmit time + valid lifetime
     ///
     /// This method converts from the times in the lease object into times
     /// able to be added to the database.
     ///
     /// @param cltt Client last transmit time
-    /// @param valid_lft Valid lifetime
+    /// @param valid_lifetime Valid lifetime
     /// @param expire Reference to MYSQL_TIME object where the expiry time of
     ///        the lease will be put.
-    /// @param lease_time Reference to the time_t object where the lease time
-    ///         will be put.
     static
-    void convertToDatabaseTime(time_t cltt, uint32_t valid_lft,
-                               MYSQL_TIME& expire, uint32_t& lease_time);
+    void convertToDatabaseTime(time_t cltt, uint32_t valid_lifetime,
+                               MYSQL_TIME& expire);
 
     /// @brief Convert Database Time to Lease Times
     ///
-    /// Within the database, time is stored as lease_time (lease time) and
-    /// expire (time of expiry of the lease).  In the DHCP server, the
-    /// information is stored as cltt (client last transmit time) and
-    /// valid_lft (valid lifetime).  These are related by the equations:
+    /// Within the database, time is stored as "expire" (time of expiry of the
+    /// lease) and valid lifetime.  In the DHCP server, the information is
+    /// stored client last transmit time and valid lifetime.  These are related
+    /// by the equation:
     ///
-    /// - valid_lft = lease_time
-    /// - cltt = expire - lease_time
+    /// - client last transmit time = expire - valid_lifetime
     ///
     /// This method converts from the times in the database into times
     /// able to be inserted into the lease object.
@@ -301,10 +312,9 @@ public:
     /// @param lease_time lifetime of the lease.
     /// @param cltt Reference to location where client last transmit time
     ///        is put.
-    /// @param valid_lft Reference to location where valid lifetime is put.
     static
-    void convertFromDatabaseTime(const MYSQL_TIME& expire, uint32_t lease_time,
-                                 time_t& cltt, uint32_t& valid_lft);
+    void convertFromDatabaseTime(const MYSQL_TIME& expire, 
+                                 uint32_t valid_lifetime, time_t& cltt);
 
     ///@}
 
@@ -340,6 +350,11 @@ private:
     ///
     /// Creates the prepared statements for all of the SQL statements used
     /// by the MySQL backend.
+    ///
+    /// @exception DbOperationError MySQL operation failed, exception will give
+    ///            text indicating the reason.
+    /// @exception InvalidParameter 'index' is not valid for the vector.  This
+    ///            represents an internal error within the code.
     void prepareStatements();
 
     /// @brief Open Database
@@ -365,7 +380,7 @@ private:
                            const char* what) const {
         if (status != 0) {
             isc_throw(DbOperationError, what << " for <" <<
-                      raw_statements_[index] << ">, reason: " <<
+                      text_statements_[index] << ">, reason: " <<
                       mysql_error(mysql_) << " (error code " <<
                       mysql_errno(mysql_) << ")");
         }
@@ -373,7 +388,7 @@ private:
 
     // Members
     MYSQL*              mysql_;                 ///< MySQL context object
-    std::vector<std::string> raw_statements_;   ///< Raw text of statements
+    std::vector<std::string> text_statements_;  ///< Raw text of statements
     std::vector<MYSQL_STMT*> statements_;       ///< Prepared statements
 };
 
diff --git a/src/lib/dhcp/tests/Makefile.am b/src/lib/dhcp/tests/Makefile.am
index 931940f..05f02b8 100644
--- a/src/lib/dhcp/tests/Makefile.am
+++ b/src/lib/dhcp/tests/Makefile.am
@@ -31,11 +31,6 @@ TESTS += libdhcp++_unittests libdhcpsrv_unittests
 libdhcp___unittests_SOURCES  = run_unittests.cc
 libdhcp___unittests_SOURCES += duid_unittest.cc
 libdhcp___unittests_SOURCES += iface_mgr_unittest.cc
-libdhcp___unittests_SOURCES += lease_mgr_factory_unittest.cc
-libdhcp___unittests_SOURCES += lease_mgr_unittest.cc
-if HAVE_MYSQL
-libdhcp___unittests_SOURCES += mysql_lease_mgr_unittest.cc
-endif
 libdhcp___unittests_SOURCES += libdhcp++_unittest.cc
 libdhcp___unittests_SOURCES += option4_addrlst_unittest.cc
 libdhcp___unittests_SOURCES += option6_addrlst_unittest.cc
@@ -48,14 +43,15 @@ libdhcp___unittests_SOURCES += pkt6_unittest.cc
 libdhcp___unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) $(LOG4CPLUS_INCLUDES)
 libdhcp___unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
 libdhcp___unittests_CXXFLAGS = $(AM_CXXFLAGS)
-if HAVE_MYSQL
-libdhcp___unittests_CPPFLAGS += $(MYSQL_CPPFLAGS)
-libdhcp___unittests_LDFLAGS  += $(MYSQL_LIBS)
-endif
 
 libdhcpsrv_unittests_SOURCES  = run_unittests.cc
 libdhcpsrv_unittests_SOURCES += addr_utilities_unittest.cc
 libdhcpsrv_unittests_SOURCES += cfgmgr_unittest.cc
+libdhcpsrv_unittests_SOURCES += lease_mgr_factory_unittest.cc
+libdhcpsrv_unittests_SOURCES += lease_mgr_unittest.cc
+if HAVE_MYSQL
+libdhcpsrv_unittests_SOURCES += mysql_lease_mgr_unittest.cc
+endif
 libdhcpsrv_unittests_SOURCES += pool_unittest.cc
 libdhcpsrv_unittests_SOURCES += subnet_unittest.cc
 libdhcpsrv_unittests_SOURCES += triplet_unittest.cc
@@ -67,7 +63,12 @@ libdhcpsrv_unittests_LDADD  = $(GTEST_LDADD)
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcpsrv.la
+libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
+if HAVE_MYSQL
+libdhcpsrv_unittests_CPPFLAGS += $(MYSQL_CPPFLAGS)
+libdhcpsrv_unittests_LDFLAGS  += $(MYSQL_LIBS)
+endif
 
 
 if USE_CLANGPP
diff --git a/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc
index 2739a90..e8305e4 100644
--- a/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc
@@ -132,21 +132,16 @@ TEST_F(MySqlLeaseMgrTest, CheckTimeConversion) {
     const time_t cltt = time(NULL);
     const uint32_t valid_lft = 86400;       // 1 day
     MYSQL_TIME expire;
-    uint32_t lease_time;
 
-    MySqlLeaseMgr::convertToDatabaseTime(cltt, valid_lft, expire, lease_time);
-    EXPECT_EQ(valid_lft, lease_time);
+    MySqlLeaseMgr::convertToDatabaseTime(cltt, valid_lft, expire);
     EXPECT_LE(2012, expire.year);       // Code was written in 2012
     EXPECT_EQ(0, expire.second_part);
     EXPECT_EQ(0, expire.neg);
 
     // Convert back
     time_t converted_cltt = 0;
-    uint32_t converted_valid_lft = 0;
-    MySqlLeaseMgr::convertFromDatabaseTime(expire, lease_time, converted_cltt,
-                                      converted_valid_lft);
+    MySqlLeaseMgr::convertFromDatabaseTime(expire, valid_lft, converted_cltt);
     EXPECT_EQ(cltt, converted_cltt);
-    EXPECT_EQ(valid_lft, converted_valid_lft);
 }
 
 /// @brief Check that getVersion() works
@@ -315,4 +310,11 @@ TEST_F(MySqlLeaseMgrTest, BasicLease6) {
     detailCompareLease6(l2, l_returned);
 }
 
+/// @brief Check getName() returns correct database name
+TEST_F(MySqlLeaseMgrTest, getName) {
+    EXPECT_EQ(std::string("keatest"), lmptr_->getName());
+
+    // @TODO: check for the negative
+}
+
 }; // end of anonymous namespace



More information about the bind10-changes mailing list