Commit 78a9c39a authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[#683,!390] Addressed review comments.

parent ce489f0e
......@@ -11,6 +11,7 @@
#include <cc/data.h>
#include <asiolink/io_address.h>
#include <dhcpsrv/cfgmgr.h>
#include <dhcpsrv/dhcpsrv_exceptions.h>
#include <dhcpsrv/lease_mgr.h>
#include <dhcpsrv/lease_mgr_factory.h>
#include <dhcpsrv/subnet_id.h>
......@@ -124,7 +125,8 @@ public:
/// @brief lease6-bulk-apply command handler
///
/// Provides the implementation for the @ref isc::lease_cmds::LeaseCmds::lease6BulkHandler.
/// Provides the implementation for the
/// @ref isc::lease_cmds::LeaseCmds::lease6BulkApplyHandler.
///
/// @param handle Callout context - which is expected to contain the
/// add command JSON text in the "command" argument
......@@ -271,10 +273,10 @@ public:
/// @param lease_type lease type.
/// @param lease_address lease address.
/// @param duid DUID of the client.
ElementPtr getFailedLeaseMap(const SubnetID& subnet_id,
const Lease::Type& lease_type,
const IOAddress& lease_address,
const DuidPtr&duid) const;
ElementPtr createFailedLeaseMap(const SubnetID& subnet_id,
const Lease::Type& lease_type,
const IOAddress& lease_address,
const DuidPtr&duid) const;
};
int
......@@ -919,8 +921,8 @@ LeaseCmdsImpl::lease6BulkApplyHandler(CalloutHandle& handle) {
if (!failed_deleted_list) {
failed_deleted_list = Element::createList();
}
failed_deleted_list->add(getFailedLeaseMap(p.subnet_id, p.lease_type,
p.addr, p.duid));
failed_deleted_list->add(createFailedLeaseMap(p.subnet_id, p.lease_type,
p.addr, p.duid));
}
}
}
......@@ -934,20 +936,17 @@ LeaseCmdsImpl::lease6BulkApplyHandler(CalloutHandle& handle) {
for (auto lease : parsed_leases_list) {
Lease6Parser parser;
bool force_update;
try {
// Check if the lease already exists.
auto existing_lease = LeaseMgrFactory::instance().getLease6(lease->type_,
lease->addr_);
// If the lease exists, we should update it. Otherwise, we add
// the new lease.
if (existing_lease) {
try {
// Try to update.
LeaseMgrFactory::instance().updateLease6(lease);
} else {
} catch (const NoSuchLease& ex) {
// Lease to be updated not found, so add it.
LeaseMgrFactory::instance().addLease(lease);
}
++success_count;
} catch (...) {
......@@ -955,10 +954,10 @@ LeaseCmdsImpl::lease6BulkApplyHandler(CalloutHandle& handle) {
if (!failed_leases_list) {
failed_leases_list = Element::createList();
}
failed_leases_list->add(getFailedLeaseMap(lease->subnet_id_,
lease->type_,
lease->addr_,
lease->duid_));
failed_leases_list->add(createFailedLeaseMap(lease->subnet_id_,
lease->type_,
lease->addr_,
lease->duid_));
}
}
}
......@@ -1264,10 +1263,10 @@ LeaseCmdsImpl::getIPv6AddressForDelete(const Parameters& parameters) const {
}
ElementPtr
LeaseCmdsImpl::getFailedLeaseMap(const SubnetID& subnet_id,
const Lease::Type& lease_type,
const IOAddress& lease_address,
const DuidPtr&duid) const {
LeaseCmdsImpl::createFailedLeaseMap(const SubnetID& subnet_id,
const Lease::Type& lease_type,
const IOAddress& lease_address,
const DuidPtr&duid) const {
auto failed_lease_map = Element::createMap();
failed_lease_map->set("subnet-id",
Element::create(static_cast<long int>(subnet_id)));
......
......@@ -3999,7 +3999,7 @@ TEST_F(LeaseCmdsTest, Lease6BulkApply) {
// with the lease6-bulk-apply.
TEST_F(LeaseCmdsTest, Lease6BulkApplyAddsOnly) {
initLeaseMgr(true, true); // (true = v6, true = create leases)
initLeaseMgr(true, false); // (true = v6, true = create leases)
// Now send the command.
string cmd =
......@@ -4032,6 +4032,49 @@ TEST_F(LeaseCmdsTest, Lease6BulkApplyAddsOnly) {
EXPECT_TRUE(lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:2::123")));
}
// This test verifies that it is possible to update leases with
// the lease6-bulk-apply.
TEST_F(LeaseCmdsTest, Lease6BulkApplyUpdatesOnly) {
initLeaseMgr(true, true); // (true = v6, true = create leases)
// Now send the command.
string cmd =
"{\n"
" \"command\": \"lease6-bulk-apply\",\n"
" \"arguments\": {"
" \"leases\": ["
" {"
" \"subnet-id\": 66,\n"
" \"ip-address\": \"2001:db8:1::1\",\n"
" \"duid\": \"11:11:11:11:11:11\",\n"
" \"iaid\": 1234\n"
" },"
" {"
" \"subnet-id\": 66,\n"
" \"ip-address\": \"2001:db8:1::2\",\n"
" \"duid\": \"22:22:22:22:22:22\",\n"
" \"iaid\": 1234\n"
" }"
" ]"
" }"
"}";
string exp_rsp = "Bulk apply of 2 IPv6 leases completed.";
// The status expected is success.
testCommand(cmd, CONTROL_RESULT_SUCCESS, exp_rsp);
// Check that the leases we inserted are stored.
Lease6Ptr lease1 = lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::1"));
Lease6Ptr lease2 = lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::2"));
ASSERT_TRUE(lease1);
ASSERT_TRUE(lease2);
// The IAIDs should have been updated for the existing leases.
EXPECT_EQ(1234, lease1->iaid_);
EXPECT_EQ(1234, lease2->iaid_);
}
// This test verifies that it is possible to only delete leases
// with the lease6-bulk-apply.
TEST_F(LeaseCmdsTest, Lease6BulkApplyDeletesOnly) {
......@@ -4091,7 +4134,18 @@ TEST_F(LeaseCmdsTest, Lease6BulkApplyDeleteNonExiting) {
string exp_rsp = "Bulk apply of 0 IPv6 leases completed.";
// The status expected is success.
testCommand(cmd, CONTROL_RESULT_EMPTY, exp_rsp);
auto resp = testCommand(cmd, CONTROL_RESULT_EMPTY, exp_rsp);
ASSERT_TRUE(resp);
ASSERT_EQ(Element::map, resp->getType());
auto args = resp->get("arguments");
ASSERT_TRUE(args);
ASSERT_EQ(Element::map, args->getType());
auto failed_deleted_leases = args->get("failed-deleted-leases");
ASSERT_TRUE(failed_deleted_leases);
ASSERT_EQ(Element::list, failed_deleted_leases->getType());
ASSERT_EQ(2, failed_deleted_leases->size());
}
// Check that changes for other leases are not applied if one of
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment