From: Tomek Mrugalski Date: Wed, 29 Oct 2014 13:34:22 +0000 (+0100) Subject: [3555] Changes after review: X-Git-Tag: trac3629a_base~5^2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=de28ce63dc83b1b244ac3ff6a21ec2edf98a2cf9;p=thirdparty%2Fkea.git [3555] Changes after review: - better handling of leases without HWAddr - Implement tests for Lease{4,6}::toText() --- diff --git a/src/lib/dhcpsrv/lease.cc b/src/lib/dhcpsrv/lease.cc index 2d455f4f69..ad1a6f46b2 100644 --- a/src/lib/dhcpsrv/lease.cc +++ b/src/lib/dhcpsrv/lease.cc @@ -75,6 +75,8 @@ Lease4::Lease4(const Lease4& other) // Copy the hardware address if it is defined. if (other.hwaddr_) { hwaddr_.reset(new HWAddr(*other.hwaddr_)); + } else { + hwaddr_.reset(); } if (other.client_id_) { @@ -156,6 +158,8 @@ Lease4::operator=(const Lease4& other) { // Copy the hardware address if it is defined. if (other.hwaddr_) { hwaddr_.reset(new HWAddr(*other.hwaddr_)); + } else { + hwaddr_.reset(); } if (other.client_id_) { @@ -218,15 +222,16 @@ std::string Lease6::toText() const { ostringstream stream; + /// @todo: print out DUID stream << "Type: " << typeToText(type_) << "(" - << static_cast(type_) << ") "; + << static_cast(type_) << ")\n"; stream << "Address: " << addr_ << "\n" << "Prefix length: " << static_cast(prefixlen_) << "\n" << "IAID: " << iaid_ << "\n" << "Pref life: " << preferred_lft_ << "\n" << "Valid life: " << valid_lft_ << "\n" << "Cltt: " << cltt_ << "\n" - << "Hardware addr: " << (hwaddr_?hwaddr_->toText(false):"(none)") + << "Hardware addr: " << (hwaddr_?hwaddr_->toText(false):"(none)") << "\n" << "Subnet ID: " << subnet_id_ << "\n"; return (stream.str()); @@ -236,12 +241,13 @@ std::string Lease4::toText() const { ostringstream stream; + /// @todo: print out client-id (if present) stream << "Address: " << addr_ << "\n" << "Valid life: " << valid_lft_ << "\n" << "T1: " << t1_ << "\n" << "T2: " << t2_ << "\n" << "Cltt: " << cltt_ << "\n" - << "Hardware addr: " << (hwaddr_?hwaddr_->toText(false):"(none)") + << "Hardware addr: " << (hwaddr_?hwaddr_->toText(false):"(none)") << "\n" << "Subnet ID: " << subnet_id_ << "\n"; return (stream.str()); diff --git a/src/lib/dhcpsrv/tests/lease_unittest.cc b/src/lib/dhcpsrv/tests/lease_unittest.cc index a5258ed9e7..4cfaffb39b 100644 --- a/src/lib/dhcpsrv/tests/lease_unittest.cc +++ b/src/lib/dhcpsrv/tests/lease_unittest.cc @@ -18,6 +18,7 @@ #include #include #include +#include using namespace isc; using namespace isc::asiolink; @@ -148,6 +149,11 @@ TEST_F(Lease4Test, copyConstructor) { // ... but it should point to different objects. EXPECT_FALSE(lease.hwaddr_ == copied_lease.hwaddr_); + + // Now let's check that the hwaddr pointer is copied even if it's NULL: + lease.hwaddr_.reset(); + Lease4 copied_lease2(lease); + EXPECT_TRUE(lease == copied_lease2); } // This test verfies that the assignment operator copies all Lease4 fields @@ -186,6 +192,11 @@ TEST_F(Lease4Test, operatorAssign) { // ... but it should point to different objects. EXPECT_FALSE(lease.hwaddr_ == copied_lease.hwaddr_); + + // Now let's check that the hwaddr pointer is copied even if it's NULL: + lease.hwaddr_.reset(); + copied_lease = lease; + EXPECT_TRUE(lease == copied_lease); } // This test verifies that the matches() returns true if two leases differ @@ -414,6 +425,37 @@ TEST(Lease4, hasIdenticalFqdn) { false, false))); } +// Verify that toText() method reports Lease4 structure properly. +TEST_F(Lease4Test, toText) { + + const time_t current_time = 12345678; + Lease4 lease(IOAddress("192.0.2.3"), hwaddr_, CLIENTID, sizeof(CLIENTID), + 3600, 123, 456, current_time, 789); + + std::stringstream expected; + expected << "Address: 192.0.2.3\n" + << "Valid life: 3600\n" + << "T1: 123\n" + << "T2: 456\n" + << "Cltt: 12345678\n" + << "Hardware addr: " << hwaddr_->toText(false) << "\n" + << "Subnet ID: 789\n"; + + EXPECT_EQ(expected.str(), lease.toText()); + + // Now let's try with a lease without hardware address. + lease.hwaddr_.reset(); + expected.str(""); + expected << "Address: 192.0.2.3\n" + << "Valid life: 3600\n" + << "T1: 123\n" + << "T2: 456\n" + << "Cltt: 12345678\n" + << "Hardware addr: (none)\n" + << "Subnet ID: 789\n"; + EXPECT_EQ(expected.str(), lease.toText()); +} + /// @brief Creates an instance of the lease with certain FQDN data. /// /// @param hostname Hostname. @@ -809,7 +851,44 @@ TEST(Lease6, hasIdenticalFqdn) { false, false))); } -/// @todo: Add tests for Lease4::toText() -/// @todo: Add tests for Lease6::toText() +// Verify that toText() method reports Lease4 structure properly. +TEST(Lease6, toText) { + + HWAddrPtr hwaddr(new HWAddr(HWADDR, sizeof(HWADDR), HTYPE_ETHER)); + + uint8_t llt[] = {0, 1, 2, 3, 4, 5, 6, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf}; + DuidPtr duid(new DUID(llt, sizeof(llt))); + + Lease6 lease(Lease::TYPE_NA, IOAddress("2001:db8::1"), duid, 123456, + 400, 800, 100, 200, 5678, hwaddr, 128); + lease.cltt_ = 12345678; + + std::stringstream expected; + expected << "Type: IA_NA(" << static_cast(Lease::TYPE_NA) << ")\n" + << "Address: 2001:db8::1\n" + << "Prefix length: 128\n" + << "IAID: 123456\n" + << "Pref life: 400\n" + << "Valid life: 800\n" + << "Cltt: 12345678\n" + << "Hardware addr: " << hwaddr->toText(false) << "\n" + << "Subnet ID: 5678\n"; + + EXPECT_EQ(expected.str(), lease.toText()); + + // Now let's try with a lease without hardware address. + lease.hwaddr_.reset(); + expected.str(""); + expected << "Type: IA_NA(" << static_cast(Lease::TYPE_NA) << ")\n" + << "Address: 2001:db8::1\n" + << "Prefix length: 128\n" + << "IAID: 123456\n" + << "Pref life: 400\n" + << "Valid life: 800\n" + << "Cltt: 12345678\n" + << "Hardware addr: (none)\n" + << "Subnet ID: 5678\n"; + EXPECT_EQ(expected.str(), lease.toText()); +} }; // end of anonymous namespace