BIND 10 trac3186, updated. a1b91fbe2702b3fb60ac639f3ce7286c525c739a [3186] Added doxygen commentary to user check hook lib
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Oct 21 12:31:18 UTC 2013
The branch, trac3186 has been updated
via a1b91fbe2702b3fb60ac639f3ce7286c525c739a (commit)
from e76a6be3b3ffbc619ddb36b56e2a30d998ac9c36 (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 a1b91fbe2702b3fb60ac639f3ce7286c525c739a
Author: Thomas Markwalder <tmark at isc.org>
Date: Mon Oct 21 08:29:21 2013 -0400
[3186] Added doxygen commentary to user check hook lib
Added doxygen throughout, cleaned up unit tests. Removed CLIENT_ID type
as it was unnecessary.
-----------------------------------------------------------------------
Summary of changes:
src/hooks/dhcp/user_chk/Makefile.am | 2 +-
src/hooks/dhcp/user_chk/load_unload.cc | 30 +++-
src/hooks/dhcp/user_chk/subnet_select_co.cc | 131 +++++++++++++---
src/hooks/dhcp/user_chk/tests/Makefile.am | 2 +-
src/hooks/dhcp/user_chk/tests/test_users_1.txt | 1 -
.../dhcp/user_chk/tests/user_file_unittests.cc | 73 +++++++--
.../dhcp/user_chk/tests/user_registry_unittests.cc | 145 ++++++++++--------
src/hooks/dhcp/user_chk/tests/user_unittests.cc | 34 +++-
src/hooks/dhcp/user_chk/tests/userid_unittests.cc | 88 +++--------
src/hooks/dhcp/user_chk/user.cc | 20 +--
src/hooks/dhcp/user_chk/user.h | 162 ++++++++++++++++++--
src/hooks/dhcp/user_chk/user_data_source.cc | 46 ------
src/hooks/dhcp/user_chk/user_data_source.h | 57 +++++--
src/hooks/dhcp/user_chk/user_file.cc | 61 +++++---
src/hooks/dhcp/user_chk/user_file.h | 93 +++++++++--
src/hooks/dhcp/user_chk/user_registry.cc | 20 ++-
src/hooks/dhcp/user_chk/user_registry.h | 61 +++++++-
src/hooks/dhcp/user_chk/version.cc | 1 +
18 files changed, 733 insertions(+), 294 deletions(-)
delete mode 100644 src/hooks/dhcp/user_chk/user_data_source.cc
-----------------------------------------------------------------------
diff --git a/src/hooks/dhcp/user_chk/Makefile.am b/src/hooks/dhcp/user_chk/Makefile.am
index 868e306..cce30b6 100644
--- a/src/hooks/dhcp/user_chk/Makefile.am
+++ b/src/hooks/dhcp/user_chk/Makefile.am
@@ -33,7 +33,7 @@ libdhcp_user_chk_la_SOURCES += load_unload.cc
libdhcp_user_chk_la_SOURCES += subnet_select_co.cc
libdhcp_user_chk_la_SOURCES += user.cc user.h
libdhcp_user_chk_la_SOURCES += user_chk_log.cc user_chk_log.h
-libdhcp_user_chk_la_SOURCES += user_data_source.cc user_data_source.h
+libdhcp_user_chk_la_SOURCES += user_data_source.h
libdhcp_user_chk_la_SOURCES += user_file.cc user_file.h
libdhcp_user_chk_la_SOURCES += user_registry.cc user_registry.h
libdhcp_user_chk_la_SOURCES += version.cc
diff --git a/src/hooks/dhcp/user_chk/load_unload.cc b/src/hooks/dhcp/user_chk/load_unload.cc
index adc1945..bd14057 100644
--- a/src/hooks/dhcp/user_chk/load_unload.cc
+++ b/src/hooks/dhcp/user_chk/load_unload.cc
@@ -11,7 +11,8 @@
// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
-// load_unload.cc
+
+/// @file This file defines the load and unload hooks library functions.
#include <hooks/hooks.h>
#include <user_registry.h>
@@ -23,13 +24,28 @@
using namespace isc::hooks;
+/// @brief Pointer to the registry instance.
UserRegistryPtr user_registry;
+
+/// @brief Output filestream for recording user check outcomes.
std::fstream user_chk_output;
+
+/// @brief For now, hard-code registry input file name.
const char* registry_fname = "/tmp/user_registry.txt";
+
+/// @brief For now, hard-code user check outcome file name.
const char* user_chk_output_fname = "/tmp/user_check_output.txt";
extern "C" {
+/// @brief Called by the Hooks library manager when the library is loaded.
+///
+/// Instantiates the UserRegistry and opens the outcome file. Failure in
+/// either results in a failed return code.
+///
+/// @param unused library handle parameter required by Hooks API.
+///
+/// @return Returns 0 upon success, non-zero upon failure.
int load(LibraryHandle&) {
// non-zero indicates an error.
int ret_val = 0;
@@ -49,14 +65,17 @@ int load(LibraryHandle&) {
// Open up the output file for user_chk results.
user_chk_output.open(user_chk_output_fname,
std::fstream::out | std::fstream::app);
- int sav_errno = errno;
+
if (!user_chk_output) {
+ // Grab the system error message.
+ const char* errmsg = strerror(errno);
isc_throw(isc::Unexpected, "Cannot open output file: "
<< user_chk_output_fname
- << " reason: " << strerror(sav_errno));
+ << " reason: " << errmsg);
}
}
catch (const std::exception& ex) {
+ // Log the error and return failure.
std::cout << "DHCP UserCheckHook could not be loaded: "
<< ex.what() << std::endl;
ret_val = 1;
@@ -65,6 +84,11 @@ int load(LibraryHandle&) {
return (ret_val);
}
+/// @brief Called by the Hooks library manager when the library is unloaded.
+///
+/// Destroys the UserRegistry and closes the outcome file.
+///
+/// @return Always returns 0.
int unload() {
try {
user_registry.reset();
diff --git a/src/hooks/dhcp/user_chk/subnet_select_co.cc b/src/hooks/dhcp/user_chk/subnet_select_co.cc
index fe1aaf4..c75fd3f 100644
--- a/src/hooks/dhcp/user_chk/subnet_select_co.cc
+++ b/src/hooks/dhcp/user_chk/subnet_select_co.cc
@@ -1,3 +1,19 @@
+// Copyright (C) 2013 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
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+/// @file Defines the subnet4_select and subnet6_select callout functions.
+
#include <hooks/hooks.h>
#include <dhcp/pkt4.h>
#include <dhcp/dhcp6.h>
@@ -17,10 +33,50 @@ extern std::fstream user_chk_output;
extern const char* registry_fname;
extern const char* user_chk_output_fname;
-
extern "C" {
-
+/// @brief Adds an entry to the end of the user check outcome file.
+///
+/// Each user entry is written in an ini-like format, with one name-value pair
+/// per line as follows:
+///
+/// id_type=<id type>
+/// client=<id str>
+/// subnet=<subnet str>
+/// registered=<is registered>"
+///
+/// where:
+/// <id type> text label of the id type: "HW_ADDR" or "DUID"
+/// <id str> user's id formatted as either isc::dhcp::Hwaddr.toText() or
+/// isc::dhcp::DUID.toText()
+/// <subnet str> selected subnet formatted as isc::dhcp::Subnet4::toText() or
+/// isc::dhcp::Subnet6::toText() as appropriate.
+/// <is registered> "yes" or "no"
+///
+/// Sample IPv4 entry would like this:
+///
+/// @code
+/// id_type=DUID
+/// client=00:01:00:01:19:ef:e6:3b:00:0c:01:02:03:04
+/// subnet=2001:db8:2::/64
+/// registered=yes
+/// id_type=duid
+/// @endcode
+///
+/// Sample IPv4 entry would like this:
+///
+/// @code
+/// id_type=DUID
+/// id_type=HW_ADDR
+/// client=hwtype=1 00:0c:01:02:03:05
+/// subnet=152.77.5.0/24
+/// registered=no
+/// @endcode
+///
+/// @param id_type_str text label identify the id type
+/// @param id_val_str text representation of the user id
+/// @param subnet_str text representation of the selected subnet
+/// @param registered boolean indicating if the user is registered or not
void generate_output_record(const std::string& id_type_str,
const std::string& id_val_str,
const std::string& subnet_str,
@@ -32,10 +88,25 @@ void generate_output_record(const std::string& id_type_str,
<< "registered=" << (registered ? "yes" : "no")
<< std::endl;
+ // @todo Flush is here to ensure output is immediate for demo purposes.
+ // Performance would generally dictate not using it.
flush(user_chk_output);
}
-// This callout is called at the "subnet4_select" hook.
+/// @brief This callout is called at the "subnet4_select" hook.
+///
+/// This function searches the UserRegistry for the client indicated by the
+/// inbound IPv4 DHCP packet. If the client is found in the registry output
+/// the generate outcome record and return.
+///
+/// If the client is not found in the registry replace the selected subnet
+/// with the restricted access subnet, then generate the outcome record and
+/// return. By convention, it is assumed that last subnet in the list of
+/// available subnets is the restricted access subnet.
+///
+/// @param handle CalloutHandle which provides access to context.
+///
+/// @return 0 upon success, non-zero otherwise.
int subnet4_select(CalloutHandle& handle) {
if (!user_registry) {
std::cout << "DHCP UserCheckHook : subnet4_select UserRegistry is null"
@@ -55,22 +126,23 @@ int subnet4_select(CalloutHandle& handle) {
// Look for the user.
UserPtr registered_user = user_registry->findUser(*hwaddr);
if (registered_user) {
- // User is in the registry, so leave the pre-selected
- // subnet alone.
+ // User is in the registry, so leave the pre-selected subnet alone.
Subnet4Ptr subnet;
handle.getArgument("subnet4", subnet);
- generate_output_record("mac", hwaddr->toText(), subnet->toText(),
- true);
+ // Add the outcome entry to the output file.
+ generate_output_record(UserId::HW_ADDRESS_STR, hwaddr->toText(),
+ subnet->toText(), true);
} else {
- // User is not in the registry, so assign them to
- // the last subnet in the collection. By convention
- // we are assuming this is the restricted subnet.
+ // User is not in the registry, so assign them to the last subnet
+ // in the collection. By convention we are assuming this is the
+ // restricted subnet.
const isc::dhcp::Subnet4Collection *subnets = NULL;
handle.getArgument("subnet4collection", subnets);
Subnet4Ptr subnet = subnets->back();
handle.setArgument("subnet4", subnet);
- generate_output_record("mac", hwaddr->toText(), subnet->toText(),
- false);
+ // Add the outcome entry to the output file.
+ generate_output_record(UserId::HW_ADDRESS_STR, hwaddr->toText(),
+ subnet->toText(), false);
}
} catch (const std::exception& ex) {
std::cout << "DHCP UserCheckHook : subnet6_select unexpected error: "
@@ -80,7 +152,21 @@ int subnet4_select(CalloutHandle& handle) {
return (0);
}
-// This callout is called at the "subnet6_select" hook.
+
+/// @brief This callout is called at the "subnet6_select" hook.
+///
+/// This function searches the UserRegistry for the client indicated by the
+/// inbound IPv6 DHCP packet. If the client is found in the registry generate
+/// the outcome record and return.
+///
+/// If the client is not found in the registry replace the selected subnet
+/// with the restricted access subnet, then generate the outcome record and
+/// return. By convention, it is assumed that last subnet in the list of
+/// available subnets is the restricted access subnet.
+///
+/// @param handle CalloutHandle which provides access to context.
+///
+/// @return 0 upon success, non-zero otherwise.
int subnet6_select(CalloutHandle& handle) {
if (!user_registry) {
std::cout << "DHCP UserCheckHook : subnet6_select UserRegistry is null"
@@ -108,22 +194,23 @@ int subnet6_select(CalloutHandle& handle) {
// Look for the user.
UserPtr registered_user = user_registry->findUser(*duid);
if (registered_user) {
- // User is in the registry, so leave the pre-selected
- // subnet alone.
+ // User is in the registry, so leave the pre-selected subnet alone.
Subnet6Ptr subnet;
handle.getArgument("subnet6", subnet);
- generate_output_record("duid", duid->toText(), subnet->toText(),
- true);
+ // Add the outcome entry to the output file.
+ generate_output_record(UserId::DUID_STR, duid->toText(),
+ subnet->toText(), true);
} else {
- // User is not in the registry, so assign them to
- // the last subnet in the collection. By convention
- // we are assuming this is the restricted subnet.
+ // User is not in the registry, so assign them to the last subnet
+ // in the collection. By convention we are assuming this is the
+ // restricted subnet.
const isc::dhcp::Subnet6Collection *subnets = NULL;
handle.getArgument("subnet6collection", subnets);
Subnet6Ptr subnet = subnets->back();
handle.setArgument("subnet6", subnet);
- generate_output_record("duid", duid->toText(), subnet->toText(),
- false);
+ // Add the outcome entry to the output file.
+ generate_output_record(UserId::DUID_STR, duid->toText(),
+ subnet->toText(), false);
}
} catch (const std::exception& ex) {
std::cout << "DHCP UserCheckHook : subnet6_select unexpected error: "
diff --git a/src/hooks/dhcp/user_chk/tests/Makefile.am b/src/hooks/dhcp/user_chk/tests/Makefile.am
index cbd2029..28393b4 100644
--- a/src/hooks/dhcp/user_chk/tests/Makefile.am
+++ b/src/hooks/dhcp/user_chk/tests/Makefile.am
@@ -37,7 +37,7 @@ libdhcp_user_chk_unittests_SOURCES += ../user.cc ../user.h
libdhcp_user_chk_unittests_SOURCES += ../user_chk_log.cc ../user_chk_log.h
# Until logging in dynamically loaded libraries is fixed, exclude these.
#libdhcp_user_chk_unittests_SOURCES += ../user_chk_messages.cc ../user_chk_messages.h
-libdhcp_user_chk_unittests_SOURCES += ../user_data_source.cc ../user_data_source.h
+libdhcp_user_chk_unittests_SOURCES += ../user_data_source.h
libdhcp_user_chk_unittests_SOURCES += ../user_file.cc ../user_file.h
libdhcp_user_chk_unittests_SOURCES += ../user_registry.cc ../user_registry.h
libdhcp_user_chk_unittests_SOURCES += run_unittests.cc
diff --git a/src/hooks/dhcp/user_chk/tests/test_users_1.txt b/src/hooks/dhcp/user_chk/tests/test_users_1.txt
index fd60fe7..5fa718f 100644
--- a/src/hooks/dhcp/user_chk/tests/test_users_1.txt
+++ b/src/hooks/dhcp/user_chk/tests/test_users_1.txt
@@ -1,3 +1,2 @@
{ "type" : "HW_ADDR", "id" : "01AC00F03344", "opt1" : "true" }
-{ "type" : "CLIENT_ID", "id" : "0899e0cc0707", "opt1" : "false" }
{ "type" : "DUID", "id" : "225060de0a0b", "opt1" : "true" }
diff --git a/src/hooks/dhcp/user_chk/tests/user_file_unittests.cc b/src/hooks/dhcp/user_chk/tests/user_file_unittests.cc
index 4659e03..9507fab 100644
--- a/src/hooks/dhcp/user_chk/tests/user_file_unittests.cc
+++ b/src/hooks/dhcp/user_chk/tests/user_file_unittests.cc
@@ -25,16 +25,26 @@ using namespace std;
namespace {
+/// @brief Convenience method for reliably building test file path names.
+///
+/// Function prefixes the given file name with a path to unit tests directory
+/// so we can reliably find test data files.
+///
+/// @param name base file name of the test file
std::string testFilePath(const std::string& name) {
return (std::string(USER_CHK_TEST_DIR) + "/" + name);
}
+/// @brief Tests the UserFile constructor.
TEST(UserFile, construction) {
+ // Verify that a UserFile with no file name is rejected.
ASSERT_THROW(UserFile(""), UserFileError);
+ // Verify that a UserFile with a non-blank file name is accepted.
ASSERT_NO_THROW(UserFile("someName"));
}
+/// @brief Tests opening and closing UserFile
TEST(UserFile, openFile) {
UserFilePtr user_file;
@@ -47,8 +57,9 @@ TEST(UserFile, openFile) {
EXPECT_FALSE(user_file->isOpen());
// Construct a user file that should exist.
- ASSERT_NO_THROW(user_file.reset(new
- UserFile(testFilePath("test_users_1.txt"))));
+ ASSERT_NO_THROW(user_file.reset(new UserFile
+ (testFilePath("test_users_1.txt"))));
+ // File should not be open.
EXPECT_FALSE(user_file->isOpen());
// Verify that we can open it.
@@ -67,15 +78,57 @@ TEST(UserFile, openFile) {
EXPECT_TRUE(user_file->isOpen());
}
+
+/// @brief Tests makeUser with invalid user strings
+TEST(UserFile, makeUser) {
+ const char* invalid_strs[]= {
+ // Missinge type element.
+ "{ \"id\" : \"01AC00F03344\" }",
+ // Invalid id type string value.
+ "{ \"type\" : \"BOGUS\", \"id\" : \"01AC00F03344\"}",
+ // Non-string id type
+ "{ \"type\" : 1, \"id\" : \"01AC00F03344\"}",
+ // Missing id element.
+ "{ \"type\" : \"HW_ADDR\" }",
+ // Odd number of digits in id value.
+ "{ \"type\" : \"HW_ADDR\", \"id\" : \"1AC00F03344\"}",
+ // Invalid characters in id value.
+ "{ \"type\" : \"HW_ADDR\", \"id\" : \"THIS IS BAD!\"}",
+ // Empty id value.
+ "{ \"type\" : \"HW_ADDR\", \"id\" : \"\"}",
+ // Non-string id.
+ "{ \"type\" : \"HW_ADDR\", \"id\" : 01AC00F03344 }",
+ // Option with non-string value
+ "{ \"type\" : \"HW_ADDR\", \"id\" : \"01AC00F03344\", \"opt\" : 4 }",
+ NULL
+ };
+
+ // Create a UseFile to work with.
+ UserFilePtr user_file;
+ ASSERT_NO_THROW(user_file.reset(new UserFile("noFile")));
+
+ // Iterate over the list of invalid user strings and verify
+ // each one fails.
+ const char** tmp = invalid_strs;;
+ while (*tmp) {
+ EXPECT_THROW(user_file->makeUser(*tmp), UserFileError)
+ << "Invalid str not caught: ["
+ << *tmp << "]" << std::endl;
+ ++tmp;
+ }
+}
+
+/// @brief Test reading from UserFile
TEST(UserFile, readFile) {
UserFilePtr user_file;
- // Construct an open a known file.
- ASSERT_NO_THROW(user_file.reset(new
- UserFile(testFilePath("test_users_1.txt"))));
+ // Construct and then open a known file.
+ ASSERT_NO_THROW(user_file.reset(new UserFile
+ (testFilePath("test_users_1.txt"))));
ASSERT_NO_THROW(user_file->open());
EXPECT_TRUE(user_file->isOpen());
+ // File should contain two valid users. Read and verify each.
UserPtr user;
int i = 0;
do {
@@ -87,17 +140,13 @@ TEST(UserFile, readFile) {
EXPECT_EQ("true", user->getProperty("opt1"));
break;
case 1:
- EXPECT_EQ(UserId::CLIENT_ID, user->getUserId().getType());
- EXPECT_EQ("0899e0cc0707", user->getUserId().toText());
- EXPECT_EQ("false", user->getProperty("opt1"));
- break;
- case 2:
EXPECT_EQ(UserId::DUID, user->getUserId().getType());
EXPECT_EQ("225060de0a0b", user->getUserId().toText());
EXPECT_EQ("true", user->getProperty("opt1"));
break;
default:
- // this is an error, TBD
+ // Third time around, we are at EOF User should be null.
+ ASSERT_FALSE(user);
break;
}
} while (user);
@@ -106,6 +155,4 @@ TEST(UserFile, readFile) {
ASSERT_NO_THROW(user_file->close());
}
-
-
} // end of anonymous namespace
diff --git a/src/hooks/dhcp/user_chk/tests/user_registry_unittests.cc b/src/hooks/dhcp/user_chk/tests/user_registry_unittests.cc
index 2061b8b..1f250c8 100644
--- a/src/hooks/dhcp/user_chk/tests/user_registry_unittests.cc
+++ b/src/hooks/dhcp/user_chk/tests/user_registry_unittests.cc
@@ -27,65 +27,72 @@ using namespace std;
namespace {
+/// @brief Convenience method for reliably building test file path names.
+///
+/// Function prefixes the given file name with a path to unit tests directory
+/// so we can reliably find test data files.
+///
+/// @param name base file name of the test file
std::string testFilePath(const std::string& name) {
return (std::string(USER_CHK_TEST_DIR) + "/" + name);
}
+/// @brief Tests UserRegistry construction.
TEST(UserRegistry, constructor) {
+ // Currently there is only the default constructor which does not throw.
UserRegistryPtr reg;
ASSERT_NO_THROW(reg.reset(new UserRegistry()));
}
+/// @brief Tests mechanics of adding, finding, removing Users.
TEST(UserRegistry, userBasics) {
+ // Create an empty registry.
UserRegistryPtr reg;
ASSERT_NO_THROW(reg.reset(new UserRegistry()));
- UserIdPtr id, id2;
- // Make user ids.
- ASSERT_NO_THROW(id.reset(new UserId(UserId::HW_ADDRESS, "01010101")));
- ASSERT_NO_THROW(id2.reset(new UserId(UserId::HW_ADDRESS, "02020202")));
-
- UserPtr user, user2;
// Verify that a blank user cannot be added.
+ UserPtr user;
ASSERT_THROW(reg->addUser(user), UserRegistryError);
- // Make new users.
+ // Make a new id and user.
+ UserIdPtr id;
+ ASSERT_NO_THROW(id.reset(new UserId(UserId::HW_ADDRESS, "01010101")));
ASSERT_NO_THROW(user.reset(new User(*id)));
- ASSERT_NO_THROW(user2.reset(new User(*id)));
- // Add first user.
+ // Verify that we can add a user.
ASSERT_NO_THROW(reg->addUser(user));
- // Verify user added can be found.
+ // Verify that the user can be found.
UserPtr found_user;
ASSERT_NO_THROW(found_user = reg->findUser(*id));
EXPECT_TRUE(found_user);
EXPECT_EQ(found_user->getUserId(), *id);
- // Verify user not added cannot be found.
+ // Verify that searching for a non-existant user returns empty user pointer.
+ UserIdPtr id2;
+ ASSERT_NO_THROW(id2.reset(new UserId(UserId::HW_ADDRESS, "02020202")));
ASSERT_NO_THROW(found_user = reg->findUser(*id2));
EXPECT_FALSE(found_user);
- // Verfiy user can no longer be found.
+ // Verify that the user can be deleted.
ASSERT_NO_THROW(reg->removeUser(*id));
ASSERT_NO_THROW(found_user = reg->findUser(*id));
EXPECT_FALSE(found_user);
}
+/// @brief Tests finding users by isc::dhcp::HWaddr instance.
TEST(UserRegistry, findByHWAddr) {
+ // Create the registry.
UserRegistryPtr reg;
ASSERT_NO_THROW(reg.reset(new UserRegistry()));
- // Make a user.
+ // Make a new user and add it.
UserPtr user;
ASSERT_NO_THROW(user.reset(new User(UserId::HW_ADDRESS, "01010101")));
-
- // Verify user can be added.
ASSERT_NO_THROW(reg->addUser(user));
// Make a HWAddr instance using the same id value.
- isc::dhcp::HWAddr hwaddr(user->getUserId().getId(),
- isc::dhcp::HTYPE_ETHER);
+ isc::dhcp::HWAddr hwaddr(user->getUserId().getId(), isc::dhcp::HTYPE_ETHER);
// Verify user can be found by HWAddr.
UserPtr found_user;
@@ -94,100 +101,116 @@ TEST(UserRegistry, findByHWAddr) {
EXPECT_EQ(found_user->getUserId(), user->getUserId());
}
+/// @brief Tests finding users by isc::dhcp::DUID instance.
TEST(UserRegistry, findByDUID) {
+ // Create the registry.
UserRegistryPtr reg;
ASSERT_NO_THROW(reg.reset(new UserRegistry()));
- // Make a user.
+ // Make a new user and add it.
UserPtr user;
ASSERT_NO_THROW(user.reset(new User(UserId::DUID, "01010101")));
-
- // Verify user can be added.
ASSERT_NO_THROW(reg->addUser(user));
- // Make a HWAddr instance using the same id value.
+ // Make a DUID instance using the same id value.
isc::dhcp::DUID duid(user->getUserId().getId());
- // Verify user can be found by HWAddr.
+ // Verify user can be found by DUID.
UserPtr found_user;
ASSERT_NO_THROW(found_user = reg->findUser(duid));
EXPECT_TRUE(found_user);
EXPECT_EQ(found_user->getUserId(), user->getUserId());
}
-TEST(UserRegistry, findByClientId) {
- UserRegistryPtr reg;
- ASSERT_NO_THROW(reg.reset(new UserRegistry()));
-
- // Make a user.
- UserPtr user;
- ASSERT_NO_THROW(user.reset(new User(UserId::CLIENT_ID, "01010101")));
-
- // Verify user can be added.
- ASSERT_NO_THROW(reg->addUser(user));
-
- // Make a HWAddr instance using the same id value.
- isc::dhcp::ClientId client_id(user->getUserId().getId());
-
- // Verify user can be found by HWAddr.
- UserPtr found_user;
- ASSERT_NO_THROW(found_user = reg->findUser(client_id));
- EXPECT_TRUE(found_user);
- EXPECT_EQ(found_user->getUserId(), user->getUserId());
-}
-
+/// @brief Tests mixing users of different types.
TEST(UserRegistry, oneOfEach) {
+ // Create the registry.
UserRegistryPtr reg;
ASSERT_NO_THROW(reg.reset(new UserRegistry()));
// Make user ids.
- UserIdPtr idh, idc, idd;
+ UserIdPtr idh, idd;
ASSERT_NO_THROW(idh.reset(new UserId(UserId::HW_ADDRESS, "01010101")));
- ASSERT_NO_THROW(idc.reset(new UserId(UserId::CLIENT_ID, "02020202")));
ASSERT_NO_THROW(idd.reset(new UserId(UserId::DUID, "03030303")));
+ // Make and add HW_ADDRESS user.
UserPtr user;
user.reset(new User(*idh));
ASSERT_NO_THROW(reg->addUser(user));
- user.reset(new User(*idc));
- ASSERT_NO_THROW(reg->addUser(user));
+ // Make and add DUID user.
user.reset(new User(*idd));
ASSERT_NO_THROW(reg->addUser(user));
+ // Verify we can find both.
UserPtr found_user;
ASSERT_NO_THROW(found_user = reg->findUser(*idh));
- ASSERT_NO_THROW(found_user = reg->findUser(*idc));
ASSERT_NO_THROW(found_user = reg->findUser(*idd));
}
-TEST(UserRegistry, userFileTest) {
+/// @brief Tests loading the registry from a file.
+TEST(UserRegistry, refreshFromFile) {
+ // Create the registry.
UserRegistryPtr reg;
ASSERT_NO_THROW(reg.reset(new UserRegistry()));
- // Create the data source.
UserDataSourcePtr user_file;
- ASSERT_NO_THROW(user_file.reset(new
- UserFile(testFilePath("test_users_1.txt"))));
+
+ // Verify that data source cannot be set to null source.
+ ASSERT_THROW(reg->setSource(user_file), UserRegistryError);
+
+ // Create the data source.
+ ASSERT_NO_THROW(user_file.reset(new UserFile
+ (testFilePath("test_users_1.txt"))));
// Set the registry's data source and refresh the registry.
ASSERT_NO_THROW(reg->setSource(user_file));
ASSERT_NO_THROW(reg->refresh());
// Verify we can find all the expected users.
- UserPtr found_user;
UserIdPtr id;
ASSERT_NO_THROW(id.reset(new UserId(UserId::HW_ADDRESS, "01ac00f03344")));
- ASSERT_NO_THROW(found_user = reg->findUser(*id));
- EXPECT_TRUE(found_user);
-
- ASSERT_NO_THROW(id.reset(new UserId(UserId::CLIENT_ID, "0899e0cc0707")));
- ASSERT_NO_THROW(found_user = reg->findUser(*id));
- EXPECT_TRUE(found_user);
+ EXPECT_TRUE(reg->findUser(*id));
ASSERT_NO_THROW(id.reset(new UserId(UserId::DUID, "225060de0a0b")));
- ASSERT_NO_THROW(found_user = reg->findUser(*id));
- EXPECT_TRUE(found_user);
+ EXPECT_TRUE(reg->findUser(*id));
+}
+
+/// @brief Tests preservation of registry upon refresh failure.
+TEST(UserRegistry, refreshFail) {
+ // Create the registry.
+ UserRegistryPtr reg;
+ ASSERT_NO_THROW(reg.reset(new UserRegistry()));
+
+ // Create the data source.
+ UserDataSourcePtr user_file;
+ ASSERT_NO_THROW(user_file.reset(new UserFile
+ (testFilePath("test_users_1.txt"))));
+
+ // Set the registry's data source and refresh the registry.
+ ASSERT_NO_THROW(reg->setSource(user_file));
+ ASSERT_NO_THROW(reg->refresh());
+
+ // Make user ids of expected users.
+ UserIdPtr id1, id2;
+ ASSERT_NO_THROW(id1.reset(new UserId(UserId::HW_ADDRESS, "01ac00f03344")));
+ ASSERT_NO_THROW(id2.reset(new UserId(UserId::DUID, "225060de0a0b")));
+
+ // Verify we can find all the expected users.
+ EXPECT_TRUE(reg->findUser(*id1));
+ EXPECT_TRUE(reg->findUser(*id2));
+
+ // Replace original data source with a new one containing an invalid entry.
+ ASSERT_NO_THROW(user_file.reset(new UserFile
+ (testFilePath("test_users_err.txt"))));
+ ASSERT_NO_THROW(reg->setSource(user_file));
+
+ // Refresh should throw due to invalid data.
+ EXPECT_THROW(reg->refresh(), UserRegistryError);
+
+ // Verify we can still find all the original users.
+ EXPECT_TRUE(reg->findUser(*id1));
+ EXPECT_TRUE(reg->findUser(*id2));
}
} // end of anonymous namespace
diff --git a/src/hooks/dhcp/user_chk/tests/user_unittests.cc b/src/hooks/dhcp/user_chk/tests/user_unittests.cc
index ca61e24..7049711 100644
--- a/src/hooks/dhcp/user_chk/tests/user_unittests.cc
+++ b/src/hooks/dhcp/user_chk/tests/user_unittests.cc
@@ -24,49 +24,73 @@ using namespace std;
namespace {
+/// @brief Tests User construction variants.
+/// Note, since all constructors accept or rely on UserId, invalid id
+/// variants are tested under UserId not here.
TEST(UserTest, construction) {
std::string test_address("01FF02AC030B0709");
+ // Create a user id.
UserIdPtr id;
ASSERT_NO_THROW(id.reset(new UserId(UserId::HW_ADDRESS, test_address)));
- UserPtr user;
// Verify construction from a UserId.
+ UserPtr user;
ASSERT_NO_THROW(user.reset(new User(*id)));
- // Verify construction from a type and an address vector.
+ // Verify construction from an id type and an address vector.
ASSERT_NO_THROW(user.reset(new User(UserId::HW_ADDRESS, id->getId())));
ASSERT_NO_THROW(user.reset(new User(UserId::DUID, id->getId())));
- ASSERT_NO_THROW(user.reset(new User(UserId::CLIENT_ID, id->getId())));
- // Verify construction from a type and an address string.
+ // Verify construction from an id type and an address string.
ASSERT_NO_THROW(user.reset(new User(UserId::HW_ADDRESS, test_address)));
ASSERT_NO_THROW(user.reset(new User(UserId::DUID, test_address)));
- ASSERT_NO_THROW(user.reset(new User(UserId::CLIENT_ID, test_address)));
}
+/// @brief Tests property map fundamentals.
TEST(UserTest, properties) {
+ // Create a user.
UserPtr user;
ASSERT_NO_THROW(user.reset(new User(UserId::DUID, "01020304050607")));
+ // Verify that we can add and retrieve a property.
std::string value = "";
EXPECT_NO_THROW(user->setProperty("one","1"));
EXPECT_NO_THROW(value = user->getProperty("one"));
EXPECT_EQ("1", value);
+ // Verify that we can update and then retrieve the property.
EXPECT_NO_THROW(user->setProperty("one","1.0"));
EXPECT_NO_THROW(value = user->getProperty("one"));
EXPECT_EQ("1.0", value);
+ // Verify that we can remove and then NOT find the property.
EXPECT_NO_THROW(user->delProperty("one"));
EXPECT_NO_THROW(value = user->getProperty("one"));
EXPECT_TRUE(value.empty());
+ // Verify that a blank property name is NOT permitted.
EXPECT_THROW(user->setProperty("", "blah"), isc::BadValue);
+ // Verify that a blank property value IS permitted.
EXPECT_NO_THROW(user->setProperty("one", ""));
EXPECT_NO_THROW(value = user->getProperty("one"));
EXPECT_TRUE(value.empty());
+
+ PropertyMap map;
+ map["one"]="1.0";
+ map["two"]="2.0";
+
+ ASSERT_NO_THROW(user->setProperties(map));
+
+ EXPECT_NO_THROW(value = user->getProperty("one"));
+ EXPECT_EQ("1.0", value);
+
+ EXPECT_NO_THROW(value = user->getProperty("two"));
+ EXPECT_EQ("2.0", value);
+
+ const PropertyMap& map2 = user->getProperties();
+ EXPECT_EQ(map2, map);
}
} // end of anonymous namespace
diff --git a/src/hooks/dhcp/user_chk/tests/userid_unittests.cc b/src/hooks/dhcp/user_chk/tests/userid_unittests.cc
index 679fbd7..817e011 100644
--- a/src/hooks/dhcp/user_chk/tests/userid_unittests.cc
+++ b/src/hooks/dhcp/user_chk/tests/userid_unittests.cc
@@ -24,50 +24,54 @@ using namespace std;
namespace {
+/// @brief Test invalid constructors.
TEST(UserIdTest, invalidConstructors) {
// Verify that constructor does not allow empty id vector.
std::vector<uint8_t> empty_bytes;
ASSERT_THROW(UserId(UserId::HW_ADDRESS, empty_bytes), isc::BadValue);
ASSERT_THROW(UserId(UserId::DUID, empty_bytes), isc::BadValue);
- ASSERT_THROW(UserId(UserId::CLIENT_ID, empty_bytes), isc::BadValue);
// Verify that constructor does not allow empty id string.
ASSERT_THROW(UserId(UserId::HW_ADDRESS, ""), isc::BadValue);
ASSERT_THROW(UserId(UserId::DUID, ""), isc::BadValue);
- ASSERT_THROW(UserId(UserId::CLIENT_ID, ""), isc::BadValue);
}
+/// @brief Test making and using HW_ADDRESS type UserIds
TEST(UserIdTest, hwAddress_type) {
-
+ // Verify text label look up for HW_ADDRESS enum.
EXPECT_EQ(std::string(UserId::HW_ADDRESS_STR),
UserId::lookupTypeStr(UserId::HW_ADDRESS));
+ // Verify enum look up for HW_ADDRESS text label.
EXPECT_EQ(UserId::HW_ADDRESS,
UserId::lookupType(UserId::HW_ADDRESS_STR));
+ // Build a test address vector.
uint8_t tmp[] = { 0x01, 0xFF, 0x02, 0xAC, 0x03, 0x0B, 0x07, 0x08 };
std::vector<uint8_t> bytes(tmp, tmp + (sizeof(tmp)/sizeof(uint8_t)));
+ // Verify construction from an HW_ADDRESS id type and address vector.
UserIdPtr id;
ASSERT_NO_THROW(id.reset(new UserId(UserId::HW_ADDRESS, bytes)));
+ // Verify that the id can be fetched.
EXPECT_EQ(id->getType(), UserId::HW_ADDRESS);
EXPECT_EQ(bytes, id->getId());
- // Verify a == b;
+ // Check relational oeprators when a == b.
UserIdPtr id2;
ASSERT_NO_THROW(id2.reset(new UserId(UserId::HW_ADDRESS, id->toText())));
EXPECT_TRUE(*id == *id2);
EXPECT_FALSE(*id != *id2);
EXPECT_FALSE(*id < *id2);
- // Verify a < b;
+ // Check relational oeprators when a < b.
ASSERT_NO_THROW(id2.reset(new UserId(UserId::HW_ADDRESS,
"01FF02AC030B0709")));
EXPECT_FALSE(*id == *id2);
EXPECT_TRUE(*id != *id2);
EXPECT_TRUE(*id < *id2);
- // Verify a > b;
+ // Check relational oeprators when a > b.
ASSERT_NO_THROW(id2.reset(new UserId(UserId::HW_ADDRESS,
"01FF02AC030B0707")));
EXPECT_FALSE(*id == *id2);
@@ -75,108 +79,60 @@ TEST(UserIdTest, hwAddress_type) {
EXPECT_FALSE(*id < *id2);
}
+/// @brief Test making and using DUID type UserIds
TEST(UserIdTest, duid_type) {
+ // Verify text label look up for DUID enum.
EXPECT_EQ(std::string(UserId::DUID_STR),
UserId::lookupTypeStr(UserId::DUID));
+ // Verify enum look up for DUID text label.
EXPECT_EQ(UserId::DUID,
UserId::lookupType(UserId::DUID_STR));
+ // Build a test DUID vector.
uint8_t tmp[] = { 0x01, 0xFF, 0x02, 0xAC, 0x03, 0x0B, 0x07, 0x08 };
std::vector<uint8_t> bytes(tmp, tmp + (sizeof(tmp)/sizeof(uint8_t)));
+ // Verify construction from an DUID id type and address vector.
UserIdPtr id;
ASSERT_NO_THROW(id.reset(new UserId(UserId::DUID, bytes)));
+ // Verify that the id can be fetched.
EXPECT_EQ(id->getType(), UserId::DUID);
EXPECT_EQ(bytes, id->getId());
- // Verify a == b;
+ // Check relational oeprators when a == b.
UserIdPtr id2;
ASSERT_NO_THROW(id2.reset(new UserId(UserId::DUID, id->toText())));
EXPECT_TRUE(*id == *id2);
EXPECT_FALSE(*id != *id2);
EXPECT_FALSE(*id < *id2);
- // Verify a < b;
+ // Check relational oeprators when a < b.
ASSERT_NO_THROW(id2.reset(new UserId(UserId::DUID, "01FF02AC030B0709")));
EXPECT_FALSE(*id == *id2);
EXPECT_TRUE(*id != *id2);
EXPECT_TRUE(*id < *id2);
- // Verify a > b;
+ // Check relational oeprators when a > b.
ASSERT_NO_THROW(id2.reset(new UserId(UserId::DUID, "01FF02AC030B0707")));
EXPECT_FALSE(*id == *id2);
EXPECT_TRUE(*id != *id2);
EXPECT_FALSE(*id < *id2);
}
-TEST(UserIdTest, client_id_type) {
- EXPECT_EQ(std::string(UserId::CLIENT_ID_STR),
- UserId::lookupTypeStr(UserId::CLIENT_ID));
-
- EXPECT_EQ(UserId::CLIENT_ID,
- UserId::lookupType(UserId::CLIENT_ID_STR));
-
- uint8_t tmp[] = { 0x01, 0xFF, 0x02, 0xAC, 0x03, 0x0B, 0x07, 0x08 };
- std::vector<uint8_t> bytes(tmp, tmp + (sizeof(tmp)/sizeof(uint8_t)));
-
- UserIdPtr id;
- ASSERT_NO_THROW(id.reset(new UserId(UserId::CLIENT_ID, bytes)));
- EXPECT_EQ(id->getType(), UserId::CLIENT_ID);
- EXPECT_EQ(bytes, id->getId());
-
- // Verify a == b;
- UserIdPtr id2;
- ASSERT_NO_THROW(id2.reset(new UserId(UserId::CLIENT_ID, id->toText())));
- EXPECT_TRUE(*id == *id2);
- EXPECT_FALSE(*id != *id2);
- EXPECT_FALSE(*id < *id2);
-
- // Verify a < b;
- ASSERT_NO_THROW(id2.reset(new UserId(UserId::CLIENT_ID,
- "01FF02AC030B0709")));
- EXPECT_FALSE(*id == *id2);
- EXPECT_TRUE(*id != *id2);
- EXPECT_TRUE(*id < *id2);
-
- // Verify a > b;
- ASSERT_NO_THROW(id2.reset(new UserId(UserId::CLIENT_ID,
- "01FF02AC030B0707")));
- EXPECT_FALSE(*id == *id2);
- EXPECT_TRUE(*id != *id2);
- EXPECT_FALSE(*id < *id2);
-}
-
+/// @brief Tests that UserIds of different types compare correctly.
TEST(UserIdTest, mixed_type_compare) {
- UserIdPtr hw, duid, client;
- // Address are the same
+ UserIdPtr hw, duid;
+ // Create UserIds with different types, but same id data.
ASSERT_NO_THROW(hw.reset(new UserId(UserId::HW_ADDRESS,
"01FF02AC030B0709")));
ASSERT_NO_THROW(duid.reset(new UserId(UserId::DUID,
"01FF02AC030B0709")));
- ASSERT_NO_THROW(client.reset(new UserId(UserId::CLIENT_ID,
- "01FF02AC030B0709")));
// Verify that UserIdType influences logical comparators.
- EXPECT_TRUE(*hw < *duid);
- EXPECT_TRUE(*duid < *client);
-
-
- // Now use different addresses.
- ASSERT_NO_THROW(hw.reset(new UserId(UserId::HW_ADDRESS,
- "01010101")));
- ASSERT_NO_THROW(duid.reset(new UserId(UserId::DUID,
- "02020202")));
- ASSERT_NO_THROW(client.reset(new UserId(UserId::CLIENT_ID,
- "03030303")));
-
EXPECT_FALSE(*hw == *duid);
EXPECT_TRUE(*hw != *duid);
EXPECT_TRUE(*hw < *duid);
-
- EXPECT_FALSE(*duid == *client);
- EXPECT_TRUE(*duid != *client);
- EXPECT_TRUE(*duid < *client);
}
diff --git a/src/hooks/dhcp/user_chk/user.cc b/src/hooks/dhcp/user_chk/user.cc
index 108e20f..8d53408 100644
--- a/src/hooks/dhcp/user_chk/user.cc
+++ b/src/hooks/dhcp/user_chk/user.cc
@@ -22,9 +22,9 @@
#include <sstream>
//********************************* UserId ******************************
+
const char* UserId::HW_ADDRESS_STR = "HW_ADDR";
const char* UserId::DUID_STR = "DUID";
-const char* UserId::CLIENT_ID_STR = "CLIENT_ID";
UserId::UserId(UserIdType id_type, const std::vector<uint8_t>& id)
: id_type_(id_type), id_(id) {
@@ -35,13 +35,12 @@ UserId::UserId(UserIdType id_type, const std::vector<uint8_t>& id)
UserId::UserId(UserIdType id_type, const std::string & id_str) :
id_type_(id_type) {
-
if (id_str.empty()) {
isc_throw(isc::BadValue, "UserId id string may not be blank");
}
- // logic to convert id_str etc...
- // input str is expected to be 2-digits per bytes, no delims
+ // Convert the id string to vector.
+ // Input is expected to be 2-digits per bytes, no delimiters.
std::vector<uint8_t> addr_bytes;
decodeHex(id_str, addr_bytes);
@@ -51,10 +50,6 @@ UserId::UserId(UserIdType id_type, const std::string & id_str) :
isc::dhcp::HWAddr hwaddr(addr_bytes, isc::dhcp::HTYPE_ETHER);
break;
}
- case CLIENT_ID: {
- isc::dhcp::ClientId client_id(addr_bytes);
- break;
- }
case DUID: {
isc::dhcp::DUID duid(addr_bytes);
break;
@@ -148,9 +143,6 @@ UserId::lookupTypeStr(UserIdType type) {
case DUID:
tmp = DUID_STR;
break;
- case CLIENT_ID:
- tmp = CLIENT_ID_STR;
- break;
default:
isc_throw(isc::BadValue, "Invalid UserIdType:" << type);
break;
@@ -165,8 +157,6 @@ UserId::lookupType(const std::string& type_str) {
return (HW_ADDRESS);
} else if (type_str.compare(DUID_STR) == 0) {
return (DUID);
- } else if (type_str.compare(CLIENT_ID_STR) == 0) {
- return (CLIENT_ID);
}
isc_throw(isc::BadValue, "Invalid UserIdType string:" << type_str);
@@ -210,6 +200,7 @@ void User::setProperty(const std::string& name, const std::string& value) {
isc_throw (isc::BadValue, "User property name cannot be blank");
}
+ // Note that if the property exists its value will be updated.
properties_[name]=value;
}
@@ -220,6 +211,9 @@ User::getProperty(const std::string& name) const {
return ((*it).second);
}
+ // By returning an empty string rather than throwing, we allow the
+ // flexibility of defaulting to blank if not specified. Let the caller
+ // decide if that is valid or not.
return ("");
}
diff --git a/src/hooks/dhcp/user_chk/user.h b/src/hooks/dhcp/user_chk/user.h
index f949069..656ee17 100644
--- a/src/hooks/dhcp/user_chk/user.h
+++ b/src/hooks/dhcp/user_chk/user.h
@@ -21,33 +21,79 @@
#include <stdint.h>
#include <vector>
+/// @file user.h This file defines classes: UserId and User.
+/// @brief These classes are used to describe and recognize DHCP lease
+/// requestors (i.e. clients).
+
+/// @brief Encapsulates a unique identifier for a DHCP client.
+/// This class provides a generic wrapper around the information used to
+/// uniquely identify the requester in a DHCP request packet. It provides
+/// the necessary operators such that it can be used as a key within STL
+/// containers such as maps. It supports both IPv4 and IPv6 clients.
class UserId {
public:
- // Use explicit value to ensure consistent numeric ordering for key
- // comparisions.
+ /// @brief Defines the supported types of user ids.
+ // Use explicit values to ensure consistent numeric ordering for key
+ // comparisons.
enum UserIdType {
+ /// @brief Hardware addresses (MAC) are used for IPv4 clients.
HW_ADDRESS = 0,
- DUID = 1,
- CLIENT_ID = 2
+ /// @brief DUIDs are used for IPv6 clients.
+ DUID = 1
};
+ /// @brief Defines the text label hardware address id type.
static const char* HW_ADDRESS_STR;
+ /// @brief Define the text label DUID id type.
static const char* DUID_STR;
- static const char* CLIENT_ID_STR;
+ /// @brief Constructor
+ ///
+ /// Constructs a UserId from an id type and id vector.
+ ///
+ /// @param id_type The type of user id contained in vector
+ /// @param id a vector of unsigned bytes containing the id
+ ///
+ /// @throw isc::BadValue if the vector is empty.
UserId(UserIdType id_type, const std::vector<uint8_t>& id);
+ /// @brief Constructor
+ ///
+ /// Constructs a UserId from an id type and id string.
+ ///
+ /// @param id_type The type of user id contained in string.
+ /// The string is expected to contain an even number of hex digits
+ /// without delimiters.
+ ///
+ /// @param id a vector of unsigned bytes containing the id
+ ///
+ /// @throw isc::BadValue if the string is empty, contains non
+ /// valid hex digits, or an odd number of hex digits.
UserId(UserIdType id_type, const std::string& id_str);
+ /// @brief Destructor.
~UserId();
/// @brief Returns a const reference to the actual id value
const std::vector<uint8_t>& getId() const;
- /// @brief Returns the UserIdType
+ /// @brief Returns the user id type
UserIdType getType() const;
- /// @brief Returns textual representation of a id (e.g. 00:01:02:03:ff)
+ /// @brief Returns textual representation of the id
+ ///
+ /// Outputs a string of hex digits representing the id with an
+ /// optional delimiter between digit pairs (i.e. bytes).
+ ///
+ /// Without a delimiter:
+ /// "0c220F"
+ ///
+ /// with colon as a delimiter:
+ /// "0c:22:0F"
+ ///
+ /// @param delim_char The delimiter to place in between
+ /// "bytes". It defaults to none.
+ /// (e.g. 00010203ff)
std::string toText(char delim_char=0x0) const;
/// @brief Compares two UserIds for equality
@@ -56,14 +102,31 @@ public:
/// @brief Compares two UserIds for inequality
bool operator !=(const UserId & other) const;
- /// @brief Performs less than comparision of two UserIds
+ /// @brief Performs less than comparison of two UserIds
bool operator <(const UserId & other) const;
+ /// @brief Returns the text label for a given id type
+ ///
+ /// @param type The id type value for which the label is desired
+ ///
+ /// @throw isc::BadValue if type is not valid.
static std::string lookupTypeStr(UserIdType type);
+ /// @brief Returns the id type for a given text label
+ ///
+ /// @param type_str The text label for which the id value is desired
+ ///
+ /// @throw isc::BadValue if type_str is not a valid text label.
static UserIdType lookupType(const std::string& type_str);
private:
+ /// @brief Converts a string of hex digits to vector of bytes
+ ///
+ /// @param input string of hex digits to convert
+ /// @param bytes vector in which to place the result
+ ///
+ /// @throw isc::BadValue if input string contains invalid hex digits
+ /// or has an odd number of digits.
void decodeHex(const std::string& input, std::vector<uint8_t>& bytes) const;
/// @brief The type of id value
@@ -74,43 +137,122 @@ private:
};
+/// @brief Outputs the UserId contents in a string to the given stream.
+///
+/// The output string has the form "<type>=<id>" where:
+///
+/// <type> is the text label returned by UserId::lookupTypeStr()
+/// <id> is the output of UserId::toText() without a delimiter.
+///
+/// Examples:
+/// HW_ADDR=0c0e0a01ff06
+/// DUID=0001000119efe63b000c01020306
+///
+/// @param os output stream to which to write
+/// @param user_id source object to output
std::ostream&
operator<<(std::ostream& os, const UserId& user_id);
+/// @brief Defines a smart pointer to UserId
typedef boost::shared_ptr<UserId> UserIdPtr;
+/// @brief Defines a map of string values keyed by string labels.
typedef std::map<std::string, std::string> PropertyMap;
+/// @brief Represents a unique DHCP user
+/// This class is used to represent a specific DHCP user who is identified by a
+/// unique id and who possesses a set of properties.
class User {
public:
-
+ /// @brief Constructor
+ ///
+ /// Constructs a new User from a given id with an empty set of properties.
+ ///
+ /// @param user_id Id to assign to the user
+ ///
+ /// @throw isc::BadValue if user id is blank.
User(const UserId & user_id);
+ /// @brief Constructor
+ ///
+ /// Constructs a new User from a given id type and vector containing the
+ /// id data with an empty set of properties.
+ ///
+ /// @param user_id Type of id contained in the id vector
+ /// @param id Vector of data representing the user's id
+ ///
+ /// @throw isc::BadValue if user id vector is empty.
User(UserId::UserIdType id_type, const std::vector<uint8_t>& id);
+ /// @brief Constructor
+ ///
+ /// Constructs a new User from a given id type and string containing the
+ /// id data with an empty set of properties.
+ ///
+ /// @param user_id Type of id contained in the id vector
+ /// @param id string of hex digits representing the user's id
+ ///
+ /// @throw isc::BadValue if user id string is empty or invalid
User(UserId::UserIdType id_type, const std::string& id_str);
+ /// @brief Destructor
~User();
+ /// @brief Returns a reference to the map of properties.
+ ///
+ /// Note that this reference can go out of scope and should not be
+ /// relied upon other than for momentary use.
const PropertyMap& getProperties() const;
+ /// @brief Sets the user's properties from a given property map
+ ///
+ /// Replaces the contents of the user's property map with the given
+ /// property map.
+ ///
+ /// @param properties property map to assign to the user
void setProperties(const PropertyMap& properties);
+ /// @brief Sets a given property to the given value
+ ///
+ /// Adds or updates the given property to the given value.
+ ///
+ /// @param name string by which the property is identified (keyed)
+ /// @param value string data to associate with the property
+ ///
+ /// @throw isc::BadValue if name is blank.
void setProperty(const std::string& name, const std::string& value);
+ /// @brief Fetches the string value for a given property name.
+ ///
+ /// @param name property name to fetch
+ ///
+ /// @return Returns the string value for the given name or an empty string
+ /// if the property is not found in the property map.
std::string getProperty(const std::string& name) const;
+ /// @brief Removes the given property from the property map.
+ ///
+ /// Removes the given property from the map if found. If not, no harm no
+ /// foul.
+ ///
+ /// @param name property name to remove
void delProperty(const std::string& name);
+ /// @brief Returns the user's id.
+ ///
+ /// Note that this reference can go out of scope and should not be
+ /// relied upon other than for momentary use.
const UserId& getUserId() const;
private:
-
+ /// @brief The user's id.
UserId user_id_;
+ /// @brief The user's property map.
PropertyMap properties_;
};
+/// @brief Defines a smart pointer to a User.
typedef boost::shared_ptr<User> UserPtr;
#endif
diff --git a/src/hooks/dhcp/user_chk/user_data_source.cc b/src/hooks/dhcp/user_chk/user_data_source.cc
deleted file mode 100644
index 3a4faa2..0000000
--- a/src/hooks/dhcp/user_chk/user_data_source.cc
+++ /dev/null
@@ -1,46 +0,0 @@
-// Copyright (C) 2013 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
-// copyright notice and this permission notice appear in all copies.
-//
-// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
-// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
-// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
-// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
-// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
-// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
-// PERFORMANCE OF THIS SOFTWARE.
-
-#include <user_data_source.h>
-
-UserDataSource::UserDataSource() : open_flag_(false) {
-}
-
-UserDataSource::~UserDataSource() {
-}
-
-void
-UserDataSource::open() {
- open_flag_ = false;
-}
-
-UserPtr
-UserDataSource::readNextUser() {
- return (UserPtr());
-}
-
-void
-UserDataSource::close() {
- open_flag_ = false;
-}
-
-bool
-UserDataSource::isOpen() const {
- return open_flag_;
-}
-
-void
-UserDataSource::setOpenFlag(bool value) {
- open_flag_ = value;
-}
diff --git a/src/hooks/dhcp/user_chk/user_data_source.h b/src/hooks/dhcp/user_chk/user_data_source.h
index 696e8e0..a1842cf 100644
--- a/src/hooks/dhcp/user_chk/user_data_source.h
+++ b/src/hooks/dhcp/user_chk/user_data_source.h
@@ -14,29 +14,62 @@
#ifndef _USER_DATA_SOURCE_H
#define _USER_DATA_SOURCE_H
+/// @file user_data_source.h Defines the base class, UserDataSource.
+#include <exceptions/exceptions.h>
#include <user.h>
-class UserDataSource {
-
+/// @brief Thrown if UserDataSource encounters an error
+class UserDataSourceError : public isc::Exception {
public:
- UserDataSource();
-
- virtual ~UserDataSource();
+ UserDataSourceError(const char* file, size_t line,
+ const char* what) :
+ isc::Exception(file, line, what) { };
+};
- virtual void open();
+/// @brief Defines an interface for reading user data into a registry.
+/// This is an abstract class which defines the interface for reading Users
+/// from an IO source such as a file.
+class UserDataSource {
+public:
+ /// @brief Constructor.
+ UserDataSource() {};
- virtual UserPtr readNextUser();
+ /// @brief Virtual Destructor.
+ virtual ~UserDataSource() {};
- virtual void close();
+ /// @brief Opens the data source.
+ ///
+ /// Prepares the data source for reading. Upon successful completion the
+ /// data source is ready to read from the beginning of its content.
+ ///
+ /// @throw UserDataSourceError if the source fails to open.
+ virtual void open() = 0;
- bool isOpen() const;
+ /// @brief Fetches the next user from the data source.
+ ///
+ /// Reads the next User from the data source and returns it. If no more
+ /// data is available it should return an empty (null) user.
+ ///
+ /// @throw UserDataSourceError if an error occurs.
+ virtual UserPtr readNextUser() = 0;
- void setOpenFlag(bool value);
+ /// @brief Closes that data source.
+ ///
+ /// Closes the data source.
+ ///
+ /// This method must not throw exceptions.
+ virtual void close() = 0;
-private:
- bool open_flag_;
+ /// @brief Returns true if the data source is open.
+ ///
+ /// This method should return true once the data source has been
+ /// successfully opened and until it has been closed.
+ ///
+ /// It is assumed to be exception safe.
+ virtual bool isOpen() const = 0;
};
+/// @brief Defines a smart pointer to a UserDataSource.
typedef boost::shared_ptr<UserDataSource> UserDataSourcePtr;
#endif
diff --git a/src/hooks/dhcp/user_chk/user_file.cc b/src/hooks/dhcp/user_chk/user_file.cc
index 384cb52..361727c 100644
--- a/src/hooks/dhcp/user_chk/user_file.cc
+++ b/src/hooks/dhcp/user_chk/user_file.cc
@@ -19,7 +19,7 @@
#include <boost/foreach.hpp>
#include <errno.h>
-UserFile::UserFile(const std::string& fname) : fname_(fname) {
+UserFile::UserFile(const std::string& fname) : fname_(fname), ifs_() {
if (fname_.empty()) {
isc_throw(UserFileError, "file name cannot be blank");
}
@@ -41,8 +41,6 @@ UserFile::open() {
isc_throw(UserFileError, "cannot open file:" << fname_
<< " reason: " << strerror(sav_error));
}
-
- setOpenFlag(true);
}
UserPtr
@@ -54,17 +52,17 @@ UserFile::readNextUser() {
if (ifs_.good()) {
char buf[4096];
- // get the next line
+ // Get the next line.
ifs_.getline(buf, sizeof(buf));
- // we got something, try to make a user out of it.
+ // We got something, try to make a user out of it.
if (ifs_.gcount() > 0) {
return(makeUser(buf));
}
}
- // returns an empty user
- return (UserDataSource::readNextUser());
+ // Returns an empty user on EOF.
+ return (UserPtr());
}
UserPtr
@@ -88,26 +86,37 @@ UserFile::makeUser(const std::string& user_string) {
std::string id_type_str;
std::string id_str;
+ // Iterate over the elements, saving of "type" and "id" to their
+ // respective locals. Anything else is assumed to be an option so
+ // add it to the local property map.
std::pair<std::string, isc::data::ConstElementPtr> element_pair;
BOOST_FOREACH (element_pair, elements->mapValue()) {
+ // Get the element's label.
std::string label = element_pair.first;
- std::string value = "";
- element_pair.second->getValue(value);
+
+ // Currently everything must be a string.
+ if (element_pair.second->getType() != isc::data::Element::string) {
+ isc_throw (UserFileError, "UserFile entry: " << user_string
+ << "has non-string value for : " << label);
+ }
+
+ std::string value = element_pair.second->stringValue();
if (label == "type") {
id_type_str = value;
} else if (label == "id") {
id_str = value;
} else {
- if (properties.find(label) != properties.end()) {
- isc_throw (UserFileError,
- "UserFile entry contains duplicate values: "
- << user_string);
- }
+ // JSON parsing reduces any duplicates to the last value parsed,
+ // so we will never see duplicates here.
+ std::cout << "adding propetry: " << label << ":[" << value << "]"
+ << std::endl;
+
properties[label]=value;
}
}
+ // First we attempt to translate the id type.
UserId::UserIdType id_type;
try {
id_type = UserId::lookupType(id_type_str);
@@ -116,25 +125,37 @@ UserFile::makeUser(const std::string& user_string) {
<< user_string << " " << ex.what());
}
+ // Id type is valid, so attempt to make the user based on that and
+ // the value we have for "id".
UserPtr user;
try {
user.reset(new User(id_type, id_str));
} catch (const std::exception& ex) {
isc_throw (UserFileError, "UserFile cannot create user form entry: "
<< user_string << " " << ex.what());
- }
-
+ }
+ // We have a new User, so add in the properties and return it.
user->setProperties(properties);
return (user);
}
+bool
+UserFile::isOpen() const {
+ return (ifs_.is_open());
+}
+
void
UserFile::close() {
- if (ifs_.is_open()) {
- ifs_.close();
+ try {
+ if (ifs_.is_open()) {
+ ifs_.close();
+ }
+ } catch (const std::exception& ex) {
+ // Highly unlikely to occur but let's at least spit out an error.
+ // Beyond that we swallow it for tidiness.
+ std::cout << "UserFile unexpected error closing the file: "
+ << fname_ << " : " << ex.what() << std::endl;
}
-
- setOpenFlag(false);
}
diff --git a/src/hooks/dhcp/user_chk/user_file.h b/src/hooks/dhcp/user_chk/user_file.h
index 4b3020f..4bfb038 100644
--- a/src/hooks/dhcp/user_chk/user_file.h
+++ b/src/hooks/dhcp/user_chk/user_file.h
@@ -11,11 +11,10 @@
// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
-
#ifndef _USER_FILE_H
#define _USER_FILE_H
-#include <exceptions/exceptions.h>
+/// @file user_file.h Defines the class, UserFile, which implements the UserDataSource interface for text files.
#include <user_data_source.h>
#include <user.h>
@@ -26,36 +25,106 @@
using namespace std;
-/// @brief Thrown UserFile encounters an error
-class UserFileError : public isc::Exception {
+/// @brief Thrown a UserFile encounters an error.
+/// Note that it derives from UserDataSourceError to comply with the interface.
+class UserFileError : public UserDataSourceError {
public:
UserFileError(const char* file, size_t line,
const char* what) :
- isc::Exception(file, line, what) { };
+ UserDataSourceError(file, line, what) { };
};
+/// @brief Provides a UserDataSource implementation for JSON text files.
+/// This class allows a text file of JSON entries to be treated as a source of
+/// User entries. The format of the file is one user entry per line, where
+/// each line contains a JSON string as follows:
+///
+/// { "type" : "<user type>", "id" : "<user_id>" (options) }
+///
+/// where:
+///
+/// <user_type> text label of the id type: "HW_ADDR" or "DUID"
+/// <user_id> the user's id as a string of hex digits without delimiters
+/// (options) zero or more string elements as name-value pairs, separated by
+/// commas: "opt1" : "val1", "other_opt", "77" ...
+///
+/// Each entry must have a valid entry for "type" and a valid entry or "id".
+///
+/// If an entry contains duplicate option names, that option will be assigend
+/// the last value found. This is typical JSON behavior.
+/// Currently, only string option values (i.e. enclosed in quotes) are
+/// supported.
+///
+/// Example file entries might look like this:
+/// @code
+///
+/// { "type" : "HW_ADDR", "id" : "01AC00F03344", "opt1" : "true" }
+/// { "type" : "DUID", "id" : "225060de0a0b", "opt1" : "false" }
+///
+/// @endcode
class UserFile : public UserDataSource {
public:
+ /// @brief Constructor
+ ///
+ /// Create a UserFile for the given file name without opening the file.
+ /// @param fname pathname to the input file.
+ ///
+ /// @throw UserFileError if given file name is empty.
UserFile(const std::string& fname);
+ /// @brief Destructor.
+ ////
+ /// The destructor does call the close method.
virtual ~UserFile();
- // must throw if open fails.
- void open();
-
- UserPtr readNextUser();
-
- void close();
-
+ /// @brief Opens the input file for reading.
+ ///
+ /// Upon successful completion, the file is opened and positioned to start
+ /// reading from the beginning of the file.
+ ///
+ /// @throw UserFileError if the file cannot be opened.
+ virtual void open();
+
+ /// @brief Fetches the next user from the file.
+ ///
+ /// Reads the next user entry from the file and attempts to create a
+ /// new User from the text therein. If there is no more data to be read
+ /// it returns an empty UserPtr.
+ ///
+ /// @return A UserPtr pointing to the new User or an empty pointer on EOF.
+ ///
+ /// @throw UserFileError if an error occurs while reading.
+ virtual UserPtr readNextUser();
+
+ /// @brief Closes the underlying file.
+ ///
+ /// Method is exception safe.
+ virtual void close();
+
+ /// @brief Returns true if the file is open.
+ ///
+ /// @return True if the underlying file is open, false otherwise.
+ virtual bool isOpen() const;
+
+ /// @brief Creates a new User instance from JSON text.
+ ///
+ /// @param user_string string the JSON text for a user entry.
+ ///
+ /// @return A pointer to the newly created User instance.
+ ///
+ /// @throw UserFileError if the entry is invalid.
UserPtr makeUser(const std::string& user_string);
private:
+ /// @brief Pathname of the input text file.
string fname_;
+ /// @brief Input file stream.
std::ifstream ifs_;
};
+/// @brief Defines a smart pointer to a UserFile.
typedef boost::shared_ptr<UserFile> UserFilePtr;
#endif
diff --git a/src/hooks/dhcp/user_chk/user_registry.cc b/src/hooks/dhcp/user_chk/user_registry.cc
index 4df69ac..44b98a5 100644
--- a/src/hooks/dhcp/user_chk/user_registry.cc
+++ b/src/hooks/dhcp/user_chk/user_registry.cc
@@ -64,12 +64,6 @@ UserRegistry::findUser(const isc::dhcp::HWAddr& hwaddr) const {
}
const UserPtr&
-UserRegistry::findUser(const isc::dhcp::ClientId& client_id) const {
- UserId id(UserId::CLIENT_ID, client_id.getClientId());
- return (findUser(id));
-}
-
-const UserPtr&
UserRegistry::findUser(const isc::dhcp::DUID& duid) const {
UserId id(UserId::DUID, duid.getDuid());
return (findUser(id));
@@ -81,10 +75,15 @@ void UserRegistry::refresh() {
"UserRegistry: cannot refresh, no data source");
}
+ // If the source isn't open, open it.
if (!source_->isOpen()) {
source_->open();
}
+ // Make a copy in case something goes wrong midstream.
+ UserMap backup(users_);
+
+ // Empty the registry then read users from source until source is empty.
clearall();
try {
UserPtr user;
@@ -92,11 +91,15 @@ void UserRegistry::refresh() {
addUser(user);
}
} catch (const std::exception& ex) {
+ // Source was compromsised so restore registry from backup.
+ users_ = backup;
+ // Close the source.
source_->close();
isc_throw (UserRegistryError, "UserRegistry: refresh failed during read"
<< ex.what());
}
+ // Close the source.
source_->close();
}
@@ -105,6 +108,11 @@ void UserRegistry::clearall() {
}
void UserRegistry::setSource(UserDataSourcePtr& source) {
+ if (!source) {
+ isc_throw (UserRegistryError,
+ "UserRegistry: data source cannot be set to null");
+ }
+
source_ = source;
}
diff --git a/src/hooks/dhcp/user_chk/user_registry.h b/src/hooks/dhcp/user_chk/user_registry.h
index d8db0c5..a0fb7dc 100644
--- a/src/hooks/dhcp/user_chk/user_registry.h
+++ b/src/hooks/dhcp/user_chk/user_registry.h
@@ -14,6 +14,8 @@
#ifndef _USER_REGISTRY_H
#define _USER_REGISTRY_H
+/// @file user_registry.h Defines the class, UserRegistry.
+
#include <dhcp/hwaddr.h>
#include <dhcp/duid.h>
#include <exceptions/exceptions.h>
@@ -32,40 +34,95 @@ public:
isc::Exception(file, line, what) { };
};
+/// @brief Defines a map of unique Users keyed by UserId.
typedef std::map<UserId,UserPtr> UserMap;
+/// @brief Embodies an update-able, searchable list of unique users
+/// This class provides the means to create and maintain a searchable list
+/// of unique users. List entries are pointers to instances of User, keyed
+/// by their UserIds.
+/// Users may be added and removed from the list individually or the list
+/// may be updated by loading it from a data source, such as a file.
class UserRegistry {
public:
+ /// @brief Constructor
+ ///
+ /// Creates a new registry with an empty list of users and no data source.
UserRegistry();
+ /// @brief Destructor
~UserRegistry();
+ /// @brief Adds a given user to the registry.
+ ///
+ /// @param user A pointer to the user to add
+ ///
+ /// @throw UserRegistryError if the user is null or if the user already
+ /// exists in the registry.
void addUser(UserPtr& user);
+ /// @brief Finds a user in the registry by user id
+ ///
+ /// @param id The user id for which to search
+ ///
+ /// @return A pointer to the user if found or an null pointer if not.
const UserPtr& findUser(const UserId& id) const;
+ /// @brief Removes a user from the registry by user id
+ ///
+ /// Removes the user entry if found, if not simply return.
+ ///
+ /// @param id The user id of the user to remove
void removeUser(const UserId& id);
+ /// @brief Finds a user in the registry by hardware address
+ ///
+ /// @param hwaddr The hardware address for which to search
+ ///
+ /// @return A pointer to the user if found or an null pointer if not.
const UserPtr& findUser(const isc::dhcp::HWAddr& hwaddr) const;
- const UserPtr& findUser(const isc::dhcp::ClientId& client_id) const;
-
+ /// @brief Finds a user in the registry by DUID
+ ///
+ /// @param duid The DUID for which to search
+ ///
+ /// @return A pointer to the user if found or an null pointer if not.
const UserPtr& findUser(const isc::dhcp::DUID& duid) const;
+ /// @brief Updates the registry from its data source.
+ ///
+ /// This method will replace the contents of the registry with new content
+ /// read from its data source. It will attempt to open the source and
+ /// then add users from the source to the registry until the source is
+ /// exhausted. If an error occurs accessing the source the registry
+ /// contents will be restored to that of before the call to refresh.
+ ///
+ /// @throw UserRegistryError if the data source has not been set (is null)
+ /// or if an error occurs accessing the data source.
void refresh();
+ /// @brief Removes all entries from the registry.
void clearall();
+ /// @brief Returns a reference to the data source.
const UserDataSourcePtr& getSource();
+ /// @brief Sets the data source to the given value.
+ ///
+ /// @param source reference to the data source to use.
+ ///
+ /// @throw UserRegistryError if new source value is null.
void setSource(UserDataSourcePtr& source);
private:
+ /// @brief The registry of users.
UserMap users_;
+ /// @brief The current data source of users.
UserDataSourcePtr source_;
};
+/// @brief Define a smart pointer to a UserRegistry.
typedef boost::shared_ptr<UserRegistry> UserRegistryPtr;
#endif
diff --git a/src/hooks/dhcp/user_chk/version.cc b/src/hooks/dhcp/user_chk/version.cc
index e82bba1..eaa9d50 100644
--- a/src/hooks/dhcp/user_chk/version.cc
+++ b/src/hooks/dhcp/user_chk/version.cc
@@ -17,6 +17,7 @@
extern "C" {
+/// @brief Version function required by Hooks API for compatibility checks.
int version() {
return (BIND10_HOOKS_VERSION);
}
More information about the bind10-changes
mailing list