From: Stephen Morris Date: Mon, 3 Dec 2012 15:35:36 +0000 (+0000) Subject: [2404] More changes as a result of review X-Git-Tag: bind10-1.0.0-beta-release~39^2~8 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=495d4e2f6970ef05f5f764f1fcd6661b5403cefb;p=thirdparty%2Fkea.git [2404] More changes as a result of review Including: * Updates to comments etc. * Addition of Lease4::operator==() (and associated unit tests) --- diff --git a/src/lib/dhcpsrv/dhcpdb_create.mysql b/src/lib/dhcpsrv/dhcpdb_create.mysql index 7103963198..695091dc87 100644 --- a/src/lib/dhcpsrv/dhcpdb_create.mysql +++ b/src/lib/dhcpsrv/dhcpdb_create.mysql @@ -72,6 +72,11 @@ COMMIT; # This table is only modified during schema upgrades. For historical reasons # (related to the names of the columns in the BIND 10 DNS database file), the # first column is called "version" and not "major". +# +# NOTE: this MUST be kept in step with src/lib/dhcpsrv/tests/schema_copy.h, +# which defines the schema for the unit tests. If you are updating +# the version number, the schema has changed: please ensure that +# schema_copy.h has been updated as well. CREATE TABLE schema_version ( version INT PRIMARY KEY NOT NULL, # Major version number minor INT # Minor version number diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc index 5a3dc59296..f7b63732ff 100644 --- a/src/lib/dhcpsrv/lease_mgr.cc +++ b/src/lib/dhcpsrv/lease_mgr.cc @@ -29,7 +29,8 @@ using namespace std; -using namespace isc::dhcp; +namespace isc { +namespace dhcp { Lease6::Lease6(LeaseType type, const isc::asiolink::IOAddress& addr, DuidPtr duid, uint32_t iaid, uint32_t preferred, uint32_t valid, @@ -82,17 +83,47 @@ Lease6::toText() { return (stream.str()); } +bool +Lease4::operator==(const Lease4& other) const { + return ( + addr_ == other.addr_ && + ext_ == other.ext_ && + hwaddr_ == other.hwaddr_ && + *client_id_ == *other.client_id_ && + t1_ == other.t1_ && + t2_ == other.t2_ && + valid_lft_ == other.valid_lft_ && + cltt_ == other.cltt_ && + subnet_id_ == other.subnet_id_ && + fixed_ == other.fixed_ && + hostname_ == other.hostname_ && + fqdn_fwd_ == other.fqdn_fwd_ && + fqdn_rev_ == other.fqdn_rev_ && + comments_ == other.comments_ + ); +} + bool Lease6::operator==(const Lease6& other) const { return ( - type_ == other.type_ && addr_ == other.addr_ && + type_ == other.type_ && prefixlen_ == other.prefixlen_ && iaid_ == other.iaid_ && *duid_ == *other.duid_ && preferred_lft_ == other.preferred_lft_ && valid_lft_ == other.valid_lft_ && + t1_ == other.t1_ && + t2_ == other.t2_ && cltt_ == other.cltt_ && - subnet_id_ == other.subnet_id_ - ); + subnet_id_ == other.subnet_id_ && + fixed_ == other.fixed_ && + hostname_ == other.hostname_ && + fqdn_fwd_ == other.fqdn_fwd_ && + fqdn_rev_ == other.fqdn_rev_ && + comments_ == other.comments_ + ); } + +} // namespace isc::dhcp +} // namespace isc diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index 3fe2d5ce0f..a742a31059 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -183,7 +183,7 @@ struct Lease4 { /// consistent T1 and T2 values. This is specified in seconds since cltt. uint32_t t2_; - /// @brief Ralid lifetime + /// @brief Valid lifetime /// /// Expressed as number of seconds since cltt. uint32_t valid_lft_; @@ -225,6 +225,18 @@ struct Lease4 { /// system administrator. std::string comments_; + /// @brief Compare two leases for equality + /// + /// @param other lease6 object with which to compare + bool operator==(const Lease4& other) const; + + /// @brief Compare two leases for inequality + /// + /// @param other lease6 object with which to compare + bool operator!=(const Lease4& other) const { + return (!operator==(other)); + } + /// @todo: Add DHCPv4 failover related fields here }; @@ -279,7 +291,7 @@ struct Lease6 { uint32_t iaid_; /// @brief Client identifier - boost::shared_ptr duid_; + DuidPtr duid_; /// @brief preferred lifetime /// @@ -373,7 +385,6 @@ struct Lease6 { bool operator!=(const Lease6& other) const { return (!operator==(other)); } - }; /// @brief Pointer to a Lease6 structure. diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index a7394b812e..79fefad0f2 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -885,9 +885,8 @@ MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters) // Enable autocommit. To avoid a flush to disk on every commit, the global // parameter innodb_flush_log_at_trx_commit should be set to 2. This will // cause the changes to be written to the log, but flushed to disk in the - // background every second or so. Setting the parameter to that value will - // speed up the system, but at the risk of losing data if the system - // crashes. + // background every second. Setting the parameter to that value will speed + // up the system, but at the risk of losing data if the system crashes. my_bool result = mysql_autocommit(mysql_, 1); if (result != 0) { isc_throw(DbOperationError, mysql_error(mysql_)); @@ -988,7 +987,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; @@ -997,7 +996,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; @@ -1006,7 +1005,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; @@ -1098,7 +1097,7 @@ MySqlLeaseMgr::prepareStatements() { } // Add leases to the database. The two public methods accept a lease object -// (of different types), bind the contents to the appropriate prepared +// (either V4 of V6), bind the contents to the appropriate prepared // statement, then call common code to execute the statement. bool @@ -1195,9 +1194,6 @@ void MySqlLeaseMgr::getLeaseCollection(StatementIndex stindex, status = mysql_stmt_store_result(statements_[stindex]); checkError(status, stindex, "unable to set up for storing all results"); - // Initialize for returning the data - result.clear(); - // Set up the fetch "release" object to release resources associated // with the call to mysql_stmt_fetch when this method exits, then // retrieve the data. @@ -1659,7 +1655,7 @@ MySqlLeaseMgr::getName() const { try { name = getParameter("name"); } catch (...) { - ; + // Return an empty name } return (name); } diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.h b/src/lib/dhcpsrv/mysql_lease_mgr.h index 700f3f1af5..be86c3a15f 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.h +++ b/src/lib/dhcpsrv/mysql_lease_mgr.h @@ -31,7 +31,7 @@ const uint32_t CURRENT_VERSION_VERSION = 1; const uint32_t CURRENT_VERSION_MINOR = 0; -// Forward declaration of the Lease exchange objects. This class is defined +// Forward declaration of the Lease exchange objects. These classes are defined // in the .cc file. class MySqlLease4Exchange; class MySqlLease6Exchange; @@ -419,13 +419,13 @@ public: DELETE_LEASE4, // Delete from lease4 by address DELETE_LEASE6, // Delete from lease6 by address GET_LEASE4_ADDR, // Get lease4 by address - GET_LEASE4_CLIENTID, // Get lease4 by Client ID - GET_LEASE4_CLIENTID_SUBID, // Get lease4 by Client ID + GET_LEASE4_CLIENTID, // Get lease4 by client ID + GET_LEASE4_CLIENTID_SUBID, // Get lease4 by client ID & subnet ID GET_LEASE4_HWADDR, // Get lease4 by HW address GET_LEASE4_HWADDR_SUBID, // Get lease4 by HW address & subnet ID GET_LEASE6_ADDR, // Get lease6 by address GET_LEASE6_DUID_IAID, // Get lease6 by DUID and IAID - GET_LEASE6_DUID_IAID_SUBID, // Get lease6 by DUID, IAID and Subnet ID + GET_LEASE6_DUID_IAID_SUBID, // Get lease6 by DUID, IAID and subnet ID GET_VERSION, // Obtain version number INSERT_LEASE4, // Add entry to lease4 table INSERT_LEASE6, // Add entry to lease6 table @@ -472,9 +472,9 @@ private: /// @brief Add Lease Common Code /// - /// This method performs the common actions for both flavours of the - /// addLease method. It binds the contents of the lease object to - /// the prepated statement and adds it to the database. + /// This method performs the common actions for both flavours (V4 and V6) + /// of the addLease method. It binds the contents of the lease object to + /// the prepared 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 @@ -495,8 +495,9 @@ private: /// @param stindex Index of statement being executed /// @param bind 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 lease LeaseCollection object returned. Note that any leases in + /// the collection when this method is called are not erased: the + /// new data is appended to the end. /// @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. @@ -518,8 +519,9 @@ private: /// /// @param stindex Index of statement being executed /// @param bind 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. + /// @param lease LeaseCollection object returned. Note that any leases in + /// the collection when this method is called are not erased: the + /// new data is appended to the end. /// /// @throw isc::dhcp::BadValue Data retrieved from the database was invalid. /// @throw isc::dhcp::DbOperationError An operation on the open database has @@ -538,8 +540,8 @@ private: /// /// @param stindex Index of statement being executed /// @param bind 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. + /// @param lease LeaseCollection object returned. Note that any existing + /// data in the collection is erased first. /// /// @throw isc::dhcp::BadValue Data retrieved from the database was invalid. /// @throw isc::dhcp::DbOperationError An operation on the open database has @@ -551,7 +553,7 @@ private: getLeaseCollection(stindex, bind, exchange6_, result); } - /// @brief Get Lease6 Common Code + /// @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, @@ -563,9 +565,9 @@ private: void getLease(StatementIndex stindex, MYSQL_BIND* bind, Lease4Ptr& result) const; - /// @brief Get Lease4 Common Code + /// @brief Get Lease6 Common Code /// - /// This method performs the common actions for the various getLease4() + /// This method performs the common actions for the various getLease46) /// methods. It acts as an interface to the getLeaseCollection() method, /// but retrieveing only a single lease. /// diff --git a/src/lib/dhcpsrv/tests/lease_mgr_factory_unittest.cc b/src/lib/dhcpsrv/tests/lease_mgr_factory_unittest.cc index 40ecba3add..e343c4441f 100644 --- a/src/lib/dhcpsrv/tests/lease_mgr_factory_unittest.cc +++ b/src/lib/dhcpsrv/tests/lease_mgr_factory_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 diff --git a/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc index 82600c6129..d973999dba 100644 --- a/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/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 @@ -238,16 +238,12 @@ public: }; namespace { -// empty class for now, but may be extended once Addr6 becomes bigger -class LeaseMgrTest : public ::testing::Test { -public: - LeaseMgrTest() { - } -}; -// This test checks if the LeaseMgr can be instantiated and that it -// parses parameters string properly. -TEST_F(LeaseMgrTest, getParameter) { +/// @brief getParameter test +/// +/// This test checks if the LeaseMgr can be instantiated and that it +/// parses parameters string properly. +TEST(LeaseMgr, getParameter) { LeaseMgr::ParameterMap pmap; pmap[std::string("param1")] = std::string("value1"); @@ -263,7 +259,9 @@ TEST_F(LeaseMgrTest, getParameter) { // are purely virtual, so we would only call ConcreteLeaseMgr methods. // Those methods are just stubs that do not return anything. -// Lease4 is also defined in lease_mgr.h, so is tested in this file as well. +/// @brief Lease4 Constructor Test +/// +/// Lease4 is also defined in lease_mgr.h, so is tested in this file as well. // This test checks if the Lease4 structure can be instantiated correctly TEST(Lease4, Lease4Constructor) { @@ -278,61 +276,351 @@ TEST(Lease4, Lease4Constructor) { // ...and a time const time_t current_time = time(NULL); - // Other random constants - const uint32_t ADDRESS = 103478; - const uint32_t VALID_LIFETIME = 10986; + // Other random constants. const uint32_t SUBNET_ID = 42; + const uint32_t VALID_LIFETIME = 500; + + // We want to check that various addresses work, so let's iterate over + // these. + const uint32_t ADDRESS[] = { + 0x00000000, 0x01020304, 0x7fffffff, 0x80000000, 0x80000001, 0xffffffff + }; + + for (int i = 0; i < sizeof(ADDRESS) / sizeof(ADDRESS[0]); ++i) { + + // Create the lease + Lease4 lease(ADDRESS[i], HWADDR, sizeof(HWADDR), + CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, current_time, + SUBNET_ID); + + EXPECT_EQ(ADDRESS[i], static_cast(lease.addr_)); + EXPECT_EQ(0, lease.ext_); + EXPECT_TRUE(hwaddr == lease.hwaddr_); + EXPECT_TRUE(clientid == *lease.client_id_); + EXPECT_EQ(0, lease.t1_); + EXPECT_EQ(0, lease.t2_); + EXPECT_EQ(VALID_LIFETIME, lease.valid_lft_); + EXPECT_EQ(current_time, lease.cltt_); + EXPECT_EQ(SUBNET_ID, lease.subnet_id_); + EXPECT_FALSE(lease.fixed_); + EXPECT_TRUE(lease.hostname_.empty()); + EXPECT_FALSE(lease.fqdn_fwd_); + EXPECT_FALSE(lease.fqdn_rev_); + EXPECT_TRUE(lease.comments_.empty()); + } +} + +/// @brief Lease4 Equality Test +/// +/// Checks that the operator==() correctly compares two leases for equality. +/// As operator!=() is also defined for this class, every check on operator==() +/// is followed by the reverse check on operator!=(). +TEST(Lease4, OperatorEquals) { - // Create the lease - Lease4 lease(ADDRESS, HWADDR, sizeof(HWADDR), CLIENTID, sizeof(CLIENTID), - VALID_LIFETIME, current_time, SUBNET_ID); - - EXPECT_EQ(ADDRESS, static_cast(lease.addr_)); - EXPECT_EQ(0, lease.ext_); - EXPECT_TRUE(hwaddr == lease.hwaddr_); - EXPECT_TRUE(clientid == *lease.client_id_); - EXPECT_EQ(0, lease.t1_); - EXPECT_EQ(0, lease.t2_); - EXPECT_EQ(VALID_LIFETIME, lease.valid_lft_); - EXPECT_EQ(current_time, lease.cltt_); - EXPECT_EQ(SUBNET_ID, lease.subnet_id_); - EXPECT_FALSE(lease.fixed_); - EXPECT_TRUE(lease.hostname_.empty()); - EXPECT_FALSE(lease.fqdn_fwd_); - EXPECT_FALSE(lease.fqdn_rev_); - EXPECT_TRUE(lease.comments_.empty()); + // Random values for the tests + const uint32_t ADDRESS = 0x01020304; + const uint8_t HWADDR[] = {0x08, 0x00, 0x2b, 0x02, 0x3f, 0x4e}; + std::vector hwaddr(HWADDR, HWADDR + sizeof(HWADDR)); + const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54}; + std::vector clientid_vec(CLIENTID, CLIENTID + sizeof(CLIENTID)); + ClientId clientid(clientid_vec); + const time_t current_time = time(NULL); + const uint32_t SUBNET_ID = 42; + const uint32_t VALID_LIFETIME = 500; + + // Check when the leases are equal. + Lease4 lease1(ADDRESS, HWADDR, sizeof(HWADDR), + CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, current_time, + SUBNET_ID); + Lease4 lease2(ADDRESS, HWADDR, sizeof(HWADDR), + CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, current_time, + SUBNET_ID); + EXPECT_TRUE(lease1 == lease2); + EXPECT_FALSE(lease1 != lease2); + + // Now vary individual fields in a lease and check that the leases compare + // not equal in every case. + lease1.addr_ = IOAddress(ADDRESS + 1); + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.addr_ = lease2.addr_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.ext_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.ext_ = lease2.ext_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.hwaddr_[0]; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.hwaddr_ = lease2.hwaddr_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++clientid_vec[0]; + lease1.client_id_.reset(new ClientId(clientid_vec)); + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + --clientid_vec[0]; + lease1.client_id_.reset(new ClientId(clientid_vec)); + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.t1_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.t1_ = lease2.t1_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.t2_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.t2_ = lease2.t2_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.valid_lft_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.valid_lft_ = lease2.valid_lft_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.cltt_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.cltt_ = lease2.cltt_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.subnet_id_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.subnet_id_ = lease2.subnet_id_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + lease1.fixed_ = !lease1.fixed_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.fixed_ = lease2.fixed_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + lease1.hostname_ += string("Something random"); + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.hostname_ = lease2.hostname_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + lease1.fqdn_fwd_ = !lease1.fqdn_fwd_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.fqdn_fwd_ = lease2.fqdn_fwd_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + lease1.fqdn_rev_ = !lease1.fqdn_rev_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.fqdn_rev_ = lease2.fqdn_rev_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + lease1.comments_ += string("Something random"); + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.comments_ = lease2.comments_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal } + + // Lease6 is also defined in lease_mgr.h, so is tested in this file as well. // This test checks if the Lease6 structure can be instantiated correctly TEST(Lease6, Lease6Constructor) { - IOAddress addr("2001:db8:1::456"); + // check a variety of addresses with different bits set. + const char* ADDRESS[] = { + "::", "::1", "2001:db8:1::456", + "7fff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", + "8000::", "8000::1", + "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff" + }; + // Other values uint8_t llt[] = {0, 1, 2, 3, 4, 5, 6, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf}; DuidPtr duid(new DUID(llt, sizeof(llt))); - uint32_t iaid = 7; // just a number - SubnetID subnet_id = 8; // just another number - Lease6Ptr x(new Lease6(Lease6::LEASE_IA_NA, addr, - duid, iaid, 100, 200, 50, 80, - subnet_id)); - - EXPECT_TRUE(x->addr_ == addr); - EXPECT_TRUE(*x->duid_ == *duid); - EXPECT_TRUE(x->iaid_ == iaid); - EXPECT_TRUE(x->subnet_id_ == subnet_id); - EXPECT_TRUE(x->type_ == Lease6::LEASE_IA_NA); - EXPECT_TRUE(x->preferred_lft_ == 100); - EXPECT_TRUE(x->valid_lft_ == 200); - EXPECT_TRUE(x->t1_ == 50); - EXPECT_TRUE(x->t2_ == 80); + for (int i = 0; i < sizeof(ADDRESS) / sizeof(ADDRESS[0]); ++i) { + IOAddress addr(ADDRESS[i]); + Lease6Ptr lease(new Lease6(Lease6::LEASE_IA_NA, addr, + duid, iaid, 100, 200, 50, 80, + subnet_id)); + + EXPECT_TRUE(lease->addr_ == addr); + EXPECT_TRUE(*lease->duid_ == *duid); + EXPECT_TRUE(lease->iaid_ == iaid); + EXPECT_TRUE(lease->subnet_id_ == subnet_id); + EXPECT_TRUE(lease->type_ == Lease6::LEASE_IA_NA); + EXPECT_TRUE(lease->preferred_lft_ == 100); + EXPECT_TRUE(lease->valid_lft_ == 200); + EXPECT_TRUE(lease->t1_ == 50); + EXPECT_TRUE(lease->t2_ == 80); + } // Lease6 must be instantiated with a DUID, not with NULL pointer + IOAddress addr(ADDRESS[0]); EXPECT_THROW(new Lease6(Lease6::LEASE_IA_NA, addr, DuidPtr(), iaid, 100, 200, 50, 80, subnet_id), InvalidOperation); } + +/// @brief Lease6 Equality Test +/// +/// Checks that the operator==() correctly compares two leases for equality. +/// As operator!=() is also defined for this class, every check on operator==() +/// is followed by the reverse check on operator!=(). +TEST(Lease6, OperatorEquals) { + + // check a variety of addressemas with different bits set. + const IOAddress addr("2001:db8:1::456"); + uint8_t duid_array[] = {0, 1, 2, 3, 4, 5, 6, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf}; + DuidPtr duid(new DUID(duid_array, sizeof(duid_array))); + uint32_t iaid = 7; // just a number + SubnetID subnet_id = 8; // just another number + + // Check for equality. + Lease6 lease1(Lease6::LEASE_IA_NA, addr, duid, iaid, 100, 200, 50, 80, + subnet_id); + Lease6 lease2(Lease6::LEASE_IA_NA, addr, duid, iaid, 100, 200, 50, 80, + subnet_id); + EXPECT_TRUE(lease1 == lease2); + EXPECT_FALSE(lease1 != lease2); + + // Go through and alter all the fields one by one + + lease1.addr_ = IOAddress("::1"); + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.addr_ = lease2.addr_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + lease1.type_ = Lease6::LEASE_IA_PD; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.type_ = lease2.type_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.prefixlen_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.prefixlen_ = lease2.prefixlen_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.iaid_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.iaid_ = lease2.iaid_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++duid_array[0]; + lease1.duid_.reset(new DUID(duid_array, sizeof(duid_array))); + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + --duid_array[0]; + lease1.duid_.reset(new DUID(duid_array, sizeof(duid_array))); + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.preferred_lft_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.preferred_lft_ = lease2.preferred_lft_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.valid_lft_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.valid_lft_ = lease2.valid_lft_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.t1_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.t1_ = lease2.t1_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.t2_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.t2_ = lease2.t2_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.cltt_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.cltt_ = lease2.cltt_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + ++lease1.subnet_id_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.subnet_id_ = lease2.subnet_id_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + lease1.fixed_ = !lease1.fixed_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.fixed_ = lease2.fixed_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + lease1.hostname_ += string("Something random"); + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.hostname_ = lease2.hostname_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + lease1.fqdn_fwd_ = !lease1.fqdn_fwd_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.fqdn_fwd_ = lease2.fqdn_fwd_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + lease1.fqdn_rev_ = !lease1.fqdn_rev_; + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.fqdn_rev_ = lease2.fqdn_rev_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal + + lease1.comments_ += string("Something random"); + EXPECT_FALSE(lease1 == lease2); + EXPECT_TRUE(lease1 != lease2); + lease1.comments_ = lease2.comments_; + EXPECT_TRUE(lease1 == lease2); // Check that the reversion has made the + EXPECT_FALSE(lease1 != lease2); // ... lease equal +} }; // end of anonymous namespace diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc index e7b64ab3f3..1ee72ff0f5 100644 --- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc @@ -72,7 +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; } @@ -182,9 +182,11 @@ public: ioaddress6_.push_back(ioaddr); } + // Ensure schema is the correct one. destroySchema(); createSchema(); + // Connect to the database try { LeaseMgrFactory::create(validConnectionString()); } catch (...) { @@ -200,9 +202,8 @@ public: /// @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. + /// Rolls back all pending transactions. The deletion of lmptr_ will close + /// the database. Then reopen it and delete everything created by the test. virtual ~MySqlLeaseMgrTest() { lmptr_->rollback(); LeaseMgrFactory::destroy(); @@ -221,8 +222,8 @@ public: /// @brief Initialize Lease4 Fields /// - /// Returns a pointer to a Lease4 structure. Different values are put - /// in the lease according to the address passed. + /// Returns a pointer to a Lease4 structure. Different values are put into + /// the lease according to the address passed. /// /// This is just a convenience function for the test methods. /// @@ -330,8 +331,8 @@ public: /// @brief Initialize Lease6 Fields /// - /// Returns a pointer to a Lease6 structure. Different values are put - /// in the lease according to the address passed. + /// Returns a pointer to a Lease6 structure. Different values are put into + /// the lease according to the address passed. /// /// This is just a convenience function for the test methods. /// @@ -359,8 +360,7 @@ public: lease->type_ = Lease6::LEASE_IA_TA; lease->prefixlen_ = 4; lease->iaid_ = 142; - lease->duid_ = boost::shared_ptr( - new DUID(vector(8, 0x77))); + lease->duid_ = DuidPtr(new DUID(vector(8, 0x77))); lease->preferred_lft_ = 900; lease->valid_lft_ = 8677; lease->cltt_ = 168256; @@ -370,8 +370,7 @@ public: lease->type_ = Lease6::LEASE_IA_TA; lease->prefixlen_ = 0; lease->iaid_ = 42; - lease->duid_ = boost::shared_ptr( - new DUID(vector(8, 0x42))); + lease->duid_ = DuidPtr(new DUID(vector(8, 0x42))); lease->preferred_lft_ = 3600; lease->valid_lft_ = 3677; lease->cltt_ = 123456; @@ -381,8 +380,7 @@ public: lease->type_ = Lease6::LEASE_IA_PD; lease->prefixlen_ = 7; lease->iaid_ = 89; - lease->duid_ = boost::shared_ptr( - new DUID(vector(8, 0x3a))); + lease->duid_ = DuidPtr(new DUID(vector(8, 0x3a))); lease->preferred_lft_ = 1800; lease->valid_lft_ = 5412; lease->cltt_ = 234567; @@ -396,7 +394,7 @@ public: for (uint8_t i = 31; i < 126; ++i) { duid.push_back(i); } - lease->duid_ = boost::shared_ptr(new DUID(duid)); + lease->duid_ = DuidPtr(new DUID(duid)); // The times used in the next tests are deliberately restricted - we // should be able to cope with valid lifetimes up to 0xffffffff. @@ -412,8 +410,7 @@ public: lease->type_ = Lease6::LEASE_IA_PD; lease->prefixlen_ = 15; lease->iaid_ = 42; - lease->duid_ = boost::shared_ptr( - new DUID(vector(8, 0x42))); + lease->duid_ = DuidPtr(new DUID(vector(8, 0x42))); lease->preferred_lft_ = 4800; lease->valid_lft_ = 7736; lease->cltt_ = 222456; @@ -424,8 +421,8 @@ public: lease->type_ = Lease6::LEASE_IA_PD; lease->prefixlen_ = 24; lease->iaid_ = 42; // Same as lease 4 - lease->duid_ = boost::shared_ptr( - new DUID(vector(8, 0x42))); // Same as lease 4 + lease->duid_ = DuidPtr(new DUID(vector(8, 0x42))); + // Same as lease 4 lease->preferred_lft_ = 5400; lease->valid_lft_ = 7832; lease->cltt_ = 227476; @@ -436,8 +433,8 @@ public: lease->type_ = Lease6::LEASE_IA_PD; lease->prefixlen_ = 24; lease->iaid_ = 93; - lease->duid_ = boost::shared_ptr( - new DUID(vector(8, 0x42))); // Same as lease 4 + lease->duid_ = DuidPtr(new DUID(vector(8, 0x42))); + // Same as lease 4 lease->preferred_lft_ = 5400; lease->valid_lft_ = 1832; lease->cltt_ = 627476; @@ -448,8 +445,7 @@ public: lease->type_ = Lease6::LEASE_IA_PD; lease->prefixlen_ = 24; lease->iaid_ = 42; - lease->duid_ = boost::shared_ptr( - new DUID(vector(8, 0xe5))); + lease->duid_ = DuidPtr(new DUID(vector(8, 0xe5))); lease->preferred_lft_ = 5600; lease->valid_lft_ = 7975; lease->cltt_ = 213876; @@ -482,7 +478,10 @@ public: // Check they are different for (int i = 0; i < (leases.size() - 1); ++i) { for (int j = (i + 1); j < leases.size(); ++j) { - ASSERT_TRUE(leases[i] != leases[j]); + stringstream s; + s << "Comparing leases " << i << " & " << j << " for equality"; + SCOPED_TRACE(s.str()); + EXPECT_TRUE(*leases[i] != *leases[j]); } } } @@ -537,6 +536,47 @@ public: vector ioaddress6_; ///< IOAddress forms of IPv6 addresses }; +///@{ +/// @brief Test Utilities +/// +/// The follow are a set of functions used during the tests. + +/// @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 + // odd things happen when they are different: the EXPECT_EQ macro appears to + // call the operator uint32_t() function, which causes an exception to be + // thrown for IPv6 addresses. + EXPECT_EQ(first->addr_.toText(), second->addr_.toText()); + EXPECT_TRUE(first->hwaddr_ == second->hwaddr_); + EXPECT_TRUE(*first->client_id_ == *second->client_id_); + EXPECT_EQ(first->valid_lft_, second->valid_lft_); + EXPECT_EQ(first->cltt_, second->cltt_); + EXPECT_EQ(first->subnet_id_, second->subnet_id_); +} + +/// @brief Compare two Lease6 structures for equality +void +detailCompareLease(const Lease6Ptr& first, const Lease6Ptr& second) { + EXPECT_EQ(first->type_, second->type_); + + // Compare address strings. Comparison of address objects is not used, as + // odd things happen when they are different: the EXPECT_EQ macro appears to + // call the operator uint32_t() function, which causes an exception to be + // thrown for IPv6 addresses. + EXPECT_EQ(first->addr_.toText(), second->addr_.toText()); + EXPECT_EQ(first->prefixlen_, second->prefixlen_); + EXPECT_EQ(first->iaid_, second->iaid_); + EXPECT_TRUE(*first->duid_ == *second->duid_); + EXPECT_EQ(first->preferred_lft_, second->preferred_lft_); + EXPECT_EQ(first->valid_lft_, second->valid_lft_); + EXPECT_EQ(first->cltt_, second->cltt_); + EXPECT_EQ(first->subnet_id_, second->subnet_id_); +} + +///@} + /// @brief Check that database can be opened /// @@ -667,53 +707,9 @@ TEST_F(MySqlLeaseMgrTest, checkVersion) { EXPECT_EQ(CURRENT_VERSION_MINOR, version.second); } -/// @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 - // odd things happen when they are different: the EXPECT_EQ macro appears to - // call the operator uint32_t() function, which causes an exception to be - // thrown for IPv6 addresses. - EXPECT_EQ(first->addr_.toText(), second->addr_.toText()); - EXPECT_TRUE(first->hwaddr_ == second->hwaddr_); - EXPECT_TRUE(*first->client_id_ == *second->client_id_); - EXPECT_EQ(first->valid_lft_, second->valid_lft_); - EXPECT_EQ(first->cltt_, second->cltt_); - EXPECT_EQ(first->subnet_id_, second->subnet_id_); -} - -/// @brief Compare two Lease6 structures for equality -void -detailCompareLease(const Lease6Ptr& first, const Lease6Ptr& second) { - EXPECT_EQ(first->type_, second->type_); - - // Compare address strings. Comparison of address objects is not used, as - // odd things happen when they are different: the EXPECT_EQ macro appears to - // call the operator uint32_t() function, which causes an exception to be - // thrown for IPv6 addresses. - EXPECT_EQ(first->addr_.toText(), second->addr_.toText()); - EXPECT_EQ(first->prefixlen_, second->prefixlen_); - EXPECT_EQ(first->iaid_, second->iaid_); - EXPECT_TRUE(*first->duid_ == *second->duid_); - EXPECT_EQ(first->preferred_lft_, second->preferred_lft_); - EXPECT_EQ(first->valid_lft_, second->valid_lft_); - EXPECT_EQ(first->cltt_, second->cltt_); - EXPECT_EQ(first->subnet_id_, second->subnet_id_); -} - - -/// @brief Basic Lease Checks +/// @brief Basic Lease4 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 - +/// Checks that the addLease, getLease4 (by address) and deleteLease4 works. TEST_F(MySqlLeaseMgrTest, basicLease4) { // Get the leases to be used for the test. vector leases = createLeases4(); @@ -756,15 +752,9 @@ TEST_F(MySqlLeaseMgrTest, basicLease4) { detailCompareLease(leases[2], l_returned); } - -/// @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. +/// @brief Basic Lease6 Checks /// -/// Tests where a collection of leases can be returned are in the test -/// Lease6Collection. +/// Checks that the addLease, getLease6 (by address) and deleteLease6 works. TEST_F(MySqlLeaseMgrTest, basicLease6) { // Get the leases to be used for the test. vector leases = createLeases6(); @@ -836,7 +826,6 @@ TEST_F(MySqlLeaseMgrTest, getLease4AddressSubnetId) { EXPECT_FALSE(l_returned); } - /// @brief Check GetLease4 methods - access by Hardware Address /// /// Adds leases to the database and checks that they can be accessed via @@ -888,8 +877,6 @@ TEST_F(MySqlLeaseMgrTest, getLease4Hwaddr) { EXPECT_EQ(0, returned.size()); } - - /// @brief Check GetLease4 methods - access by Hardware Address & Subnet ID /// /// Adds leases to the database and checks that they can be accessed via @@ -940,7 +927,6 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSubnetId) { isc::dhcp::MultipleRecords); } - /// @brief Check GetLease4 methods - access by Client ID /// /// Adds leases to the database and checks that they can be accessed via @@ -988,7 +974,6 @@ TEST_F(MySqlLeaseMgrTest, getLease4ClientId) { EXPECT_EQ(0, returned.size()); } - /// @brief Check GetLease4 methods - access by Client ID & Subnet ID /// /// Adds leases to the database and checks that they can be accessed via @@ -1026,7 +1011,6 @@ TEST_F(MySqlLeaseMgrTest, getLease4ClientIdSubnetId) { EXPECT_FALSE(returned); } - /// @brief Check GetLease6 methods - access by DUID/IAID /// /// Adds leases to the database and checks that they can be accessed via @@ -1072,8 +1056,6 @@ TEST_F(MySqlLeaseMgrTest, getLease6DuidIaid) { EXPECT_EQ(0, returned.size()); } - - /// @brief Check GetLease6 methods - access by DUID/IAID/SubnetID /// /// Adds leases to the database and checks that they can be accessed via @@ -1111,7 +1093,6 @@ TEST_F(MySqlLeaseMgrTest, getLease6DuidIaidSubnetId) { EXPECT_FALSE(returned); } - /// @brief Lease4 update tests /// /// Checks that we are able to update a lease in the database. @@ -1156,7 +1137,6 @@ TEST_F(MySqlLeaseMgrTest, updateLease4) { EXPECT_THROW(lmptr_->updateLease4(leases[2]), isc::dhcp::NoSuchLease); } - /// @brief Lease6 update tests /// /// Checks that we are able to update a lease in the database. diff --git a/src/lib/dhcpsrv/tests/schema_copy.h b/src/lib/dhcpsrv/tests/schema_copy.h index 85b8a5f62d..48a11cafb1 100644 --- a/src/lib/dhcpsrv/tests/schema_copy.h +++ b/src/lib/dhcpsrv/tests/schema_copy.h @@ -25,7 +25,9 @@ namespace { // by semicolons, and the strings must end with a comma. The final line // statement must be NULL (not in quotes) -// THIS MUST BE KEPT UP TO DATE AND UPDATED IF THE SCHEMA CHANGES +// NOTE: This file mirrors the schema in src/lib/dhcpsrv/dhcpdb_create.mysql. +// If this file is altered, please ensure that any change is compatible +// with the schema in dhcpdb_create.mysql. // Deletion of existing tables.