[2140] LeaseMgr tweaks.

src/lib/dhcp/lease_mgr.cc

@@ -31,6 +31,10 @@ using namespace isc::dhcp;
 
 LeaseMgr::LeaseMgr(const std::string& dbconfig) {
 
+    if (dbconfig.length() == 0) {
+        return;
+    }
+
     vector<string> tokens;
 
     boost::split(tokens, dbconfig, boost::is_any_of("\t "));
@@ -47,8 +51,14 @@ LeaseMgr::LeaseMgr(const std::string& dbconfig) {
         }
 
     }
+}
 
-    /// @todo: Parse dbconfig string
+std::string LeaseMgr::getParameter(const std::string& name) const {
+    map<string, string>::const_iterator param = parameters_.find(name);
+    if (param == parameters_.end()) {
+        isc_throw(BadValue, "Parameter not found");
+    }
+    return (param->second);
 }
 
 LeaseMgr::~LeaseMgr() {

src/lib/dhcp/lease_mgr.h

@@ -31,7 +31,8 @@ namespace dhcp {
 /// would be required. As this is a critical part of the code that will be used
 /// extensively, direct access is warranted.
 struct Lease4 {
-    uint32_t addr_; /// IPv4 address
+    /// IPv4 address
+    isc::asiolink::IOAddress addr_;
 
     /// @brief Address extension
     ///
@@ -40,13 +41,13 @@ struct Lease4 {
     /// 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_; /// address extension
+    uint32_t ext_;
 
     /// @brief hardware address
     std::vector<uint8_t> hwaddr_;
 
     /// @brief client identifier
-    std::vector<uint8_t> client_id_;
+    boost::shared_ptr<ClientId> client_id_;
 
     /// @brief valid lifetime
     ///
@@ -140,7 +141,7 @@ struct Lease6 {
     /// client has its own unique DUID (DHCP Unique IDentifier). Furthermore, client's
     /// HW address is not always available, because client may be behind a relay (relay
     /// stores only link-local address).
-    std::vector<uint8_t> hwaddr;
+    std::vector<uint8_t> hwaddr_;
 
     /// @brief client identifier
     boost::shared_ptr<DUID> duid_;
@@ -149,17 +150,21 @@ struct Lease6 {
     ///
     /// This parameter specifies preferred lifetime since the lease was assigned/renewed
     /// (cltt), expressed in seconds.
-    uint32_t preferred_lft;
+    uint32_t preferred_lft_;
 
     /// @brief valid lifetime
-    uint32_t valid_lft;
+    ///
+    /// This parameter specified valid lifetime since the lease was 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.
+    /// for the same IA, each must have consistent T1 and T2 values. Specified in
+    /// seconds since cltt.
     uint32_t t1_;
 
     /// @brief T2 timer
@@ -167,7 +172,8 @@ struct Lease6 {
     /// 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.
+    /// for the same IA, each must have consistent T1 and T2 values. Specified in
+    /// seconds since cltt.
     uint32_t t2_;
 
     /// @brief Recycle timer
@@ -176,45 +182,45 @@ struct Lease6 {
     /// or expired. This timer specifies number of seconds that it should be kept
     /// after release or expiration, in case the client returns. This feature is not
     /// currently used and this value is set to 0.
-    uint32_t recycle_time;
+    uint32_t recycle_time_;
 
     /// @brief client last transmission time
     ///
     /// Specifies a timestamp, when last transmission from a client was received.
-    time_t cltt;
+    time_t cltt_;
 
     /// @brief Pool identifier
     ///
     /// Specifies pool-id of the pool that the lease belongs to
-    uint32_t pool_id;
+    uint32_t pool_id_;
 
     /// @brief Is this a fixed lease?
     ///
     /// Fixed leases are kept after they are released/expired.
-    bool fixed;
+    bool fixed_;
 
     /// @brief client hostname
     ///
     /// This field may be empty
-    std::string hostname;
+    std::string hostname_;
 
     /// @brief did we update AAAA record for this lease?
-    bool fqdn_fwd;
+    bool fqdn_fwd_;
 
     /// @brief did we update PTR record for this lease?
-    bool fqdn_rev;
+    bool fqdn_rev_;
 
     /// @brief additional options stored with this lease
     ///
     /// That field is currently not used. We may keep extra options assigned
     /// for leasequery and possibly other purposes.
-    /// @todo: Define this as a container of options
-    std::string options;
+    /// @todo We need a way to store options in the databased.
+    Option::OptionCollection options_;
 
-    /// @brief /// comments on that lease
+    /// @brief Lease comments
     ///
-    /// (currently not used)
-    std::string comments;
+    /// This field is currently not used.
+    std::string comments_;
 };
 
 /// @brief Pointer to a Lease6 structure.
@@ -331,14 +337,31 @@ public:
     virtual std::string getDescription() const = 0;
 
     /// @brief Returns backend version.
+    ///
+    /// @todo: We will need to implement 3 version functions eventually:
+    /// A. abstract API version
+    /// B. backend version
+    /// C. database version (stored in the database scheme)
+    ///
+    /// and then check that:
+    /// 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 happen
     virtual std::string getVersion() const = 0;
 
-    /// @todo: Add pool management here
-
     /// @todo: Add host management here
+    /// As host reservation is outside of scope for 2012, support for hosts
+    /// is currently postponed.
+
 protected:
+    /// @brief returns value of the parameter
+    std::string getParameter(const std::string& name) const;
 
     /// @brief list of parameters passed in dbconfig
+    ///
+    /// That will be mostly used for storing database name, username,
+    /// password and other parameters required for DB access. It is not
+    /// intended to keep any DHCP-related parameters.
     std::map<std::string, std::string> parameters_;
 };
 

src/lib/dhcp/tests/lease_mgr_unittest.cc

@@ -25,6 +25,11 @@ using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
 
+// This is a concrete implementation of a Lease database.
+// It does not do anything useful now, and is used for abstract LeaseMgr
+// class testing. It may later evolve into more useful backend if the
+// need arises. We can reuse code from memfile benchmark. See code in
+// tests/tools/dhcp-ubench/memfile_bench.{cc|h}
 class Memfile_LeaseMgr : public LeaseMgr {
 public:
 
@@ -124,6 +129,9 @@ public:
 
     /// @brief Returns backend version.
     std::string getVersion() const { return "test-version"; }
+
+    using LeaseMgr::getParameter;
+
 protected:
 
 
@@ -185,51 +193,6 @@ std::string Memfile_LeaseMgr::getDescription() const {
                    "purpose is to test abstract lease manager API."));
 }
 
-#if 0
-bool Memfile_LeaseMgr::addLease(Lease4Ptr lease) {
-    if (ip4Hash_.find(lease->addr) != ip4Hash_.end()) {
-        // there is such an address already in the hash
-        return false;
-    }
-    ip4Hash_.insert(pair<uint32_t, Lease4Ptr>(lease->addr, lease));
-    lease->hostname = "add";
-    writeLease(lease);
-    return (true);
-}
-
-Lease4Ptr Memfile_LeaseMgr::getLease(uint32_t addr) {
-    leaseIt x = ip4Hash_.find(addr);
-    if (x != ip4Hash_.end()) {
-        return x->second; // found
-    }
-
-    // not found
-    return Lease4Ptr();
-}
-
-Lease4Ptr Memfile_LeaseMgr::updateLease(uint32_t addr, uint32_t new_cltt) {
-    leaseIt x = ip4Hash_.find(addr);
-    if (x != ip4Hash_.end()) {
-        x->second->cltt = new_cltt;
-        x->second->hostname = "update";
-        writeLease(x->second);
-        return x->second;
-    }
-    return Lease4Ptr();
-}
-
-bool Memfile_LeaseMgr::deleteLease(uint32_t addr) {
-    leaseIt x = ip4Hash_.find(addr);
-    if (x != ip4Hash_.end()) {
-        x->second->hostname = "delete";
-        writeLease(x->second);
-        ip4Hash_.erase(x);
-        return true;
-    }
-    return false;
-}
-#endif
-
 namespace {
 // empty class for now, but may be extended once Addr6 becomes bigger
 class LeaseMgrTest : public ::testing::Test {
@@ -240,9 +203,20 @@ public:
 
 TEST_F(LeaseMgrTest, constructor) {
 
-    LeaseMgr * leaseMgr = new Memfile_LeaseMgr("param1=value1");
+    // should not throw any exceptions here
+    Memfile_LeaseMgr * leaseMgr = new Memfile_LeaseMgr("");
+    delete leaseMgr;
+
+    leaseMgr = new Memfile_LeaseMgr("param1=value1 param2=value2");
+
+    EXPECT_EQ("value1", leaseMgr->getParameter("param1"));
+    EXPECT_EQ("value2", leaseMgr->getParameter("param2"));
+    EXPECT_THROW(leaseMgr->getParameter("param3"), BadValue);
 
     delete leaseMgr;
 }
 
+// There's no point in calling any other methods in LeaseMgr, as they
+// are purely virtual, so we would only call Memfile_LeaseMgr methods.
+
 }; // end of anonymous namespace