Commit 24b7fb0e authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[4212] Addressed review comments.

- Introduced constants for MySQL fetch results
- Simplified loops in generic SQL unit tests
- Removed some typos
- Removed unused variables
parent 51610a48
......@@ -30,21 +30,10 @@ using namespace std;
namespace isc {
namespace dhcp {
/// @brief Maximum size of database fields
///
/// The following constants define buffer sizes for variable length database
/// fields. The values should be greater than or equal to the length set in
/// the schema definition.
///
/// The exception is the length of any VARCHAR fields: buffers for these should
/// be set greater than or equal to the length of the field plus 1: this allows
/// for the insertion of a trailing null whatever data is returned.
const my_bool MLM_FALSE = 0; ///< False value
const my_bool MLM_TRUE = 1; ///< True value
///@}
const my_bool MLM_FALSE = 0;
const my_bool MLM_TRUE = 1;
const int MLM_MYSQL_FETCH_SUCCESS = 0;
const int MLM_MYSQL_FETCH_FAILURE = 1;
// Open the database using the parameters passed to the constructor.
......
......@@ -24,20 +24,31 @@
namespace isc {
namespace dhcp {
/// @brief MySQL True/False constants
/// @name MySQL constants.
///
/// Declare typed values so as to avoid problems of data conversion. These
/// are local to the file but are given the prefix MLM (MySql Lease Manager) to
/// avoid any likely conflicts with variables in header files named TRUE or
/// FALSE.
//@{
/// @brief MySQL false value.
extern const my_bool MLM_FALSE;
/// @brief MySQL true value.
extern const my_bool MLM_TRUE;
// Define the current database schema values
/// @brief MySQL fetch success code.
extern const int MLM_MYSQL_FETCH_SUCCESS;
/// @brief MySQL fetch failure code.
extern const int MLM_MYSQL_FETCH_FAILURE;
//@}
/// @name Current database schema version values.
//@{
const uint32_t CURRENT_VERSION_VERSION = 3;
const uint32_t CURRENT_VERSION_MINOR = 0;
//@}
/// @brief Fetch and Release MySQL Results
///
/// When a MySQL statement is expected, to fetch the results the function
......
......@@ -842,21 +842,12 @@ public:
private:
uint64_t host_id_; /// Host unique identifier
uint64_t reservation_id_; /// Reservation unique identifier
size_t host_id_length_; /// Length of the host unique ID
std::string address_; ///< Address (or prefix)
size_t address_len_; ///< Length of the textual address representation
uint8_t prefix_len_; ///< Length of the prefix (128 for addresses)
uint8_t type_;
uint8_t iaid_;
uint64_t host_id_len_;
// NULL flags for subnets id, ipv4 address, hostname and client classes
my_bool host_id_null_;
my_bool address_null_;
my_bool prefix_len_null_;
my_bool iaid_null_;
IPv6Resrv resv_;
MYSQL_BIND bind_[RESRV_COLUMNS];
......@@ -999,7 +990,8 @@ MySqlHostDataSource::getIPv6ReservationCollection(StatementIndex stindex,
// retrieve the data. mysql_stmt_fetch return value equal to 0 represents
// successful data fetch.
MySqlFreeResult fetch_release(conn_.statements_[stindex]);
while ((status = mysql_stmt_fetch(conn_.statements_[stindex])) == 0) {
while ((status = mysql_stmt_fetch(conn_.statements_[stindex])) ==
MLM_MYSQL_FETCH_SUCCESS) {
try {
result.insert(IPv6ResrvTuple(exchange->getIPv6ReservData().getType(),
exchange->getIPv6ReservData()));
......@@ -1013,9 +1005,10 @@ MySqlHostDataSource::getIPv6ReservationCollection(StatementIndex stindex,
// How did the fetch end?
// If mysql_stmt_fetch return value is equal to 1 an error occurred.
if (status == 1) {
if (status == MLM_MYSQL_FETCH_FAILURE) {
// Error - unable to fetch results
checkError(status, stindex, "unable to fetch results");
} else if (status == MYSQL_DATA_TRUNCATED) {
// Data truncated - throw an exception indicating what was at fault
isc_throw(DataTruncated, conn_.text_statements_[stindex]
......@@ -1106,9 +1099,10 @@ MySqlHostDataSource::getHostCollection(StatementIndex stindex, MYSQL_BIND* bind,
// How did the fetch end?
// If mysql_stmt_fetch return value is equal to 1 an error occurred.
if (status == 1) {
if (status == MLM_MYSQL_FETCH_FAILURE) {
// Error - unable to fetch results
checkError(status, stindex, "unable to fetch results");
} else if (status == MYSQL_DATA_TRUNCATED) {
// Data truncated - throw an exception indicating what was at fault
isc_throw(DataTruncated, conn_.text_statements_[stindex]
......
......@@ -250,8 +250,7 @@ void GenericHostDataSourceTest::compareHosts(const ConstHostPtr& host1,
// Compare IPv6 reservations
compareReservations6(host1->getIPv6Reservations(),
host2->getIPv6Reservations(),
true);
host2->getIPv6Reservations());
// And compare client classification details
compareClientClasses(host1->getClientClasses4(),
......@@ -282,21 +281,34 @@ GenericHostDataSourceTest::DuidToHWAddr(const DuidPtr& duid) {
void
GenericHostDataSourceTest::compareReservations6(IPv6ResrvRange resrv1,
IPv6ResrvRange resrv2,
bool expect_match) {
IPv6ResrvRange resrv2) {
// Compare number of reservations for both hosts
if (std::distance(resrv1.first, resrv1.second) !=
std::distance(resrv2.first, resrv2.second)){
// Number of reservations is not equal.
// Let's see if it's a problem.
if (expect_match) {
ADD_FAILURE()<< "Reservation comparison failed, "
"hosts got different number of reservations.";
}
ADD_FAILURE()<< "Reservation comparison failed, "
"hosts got different number of reservations.";
return;
}
// Iterate over the range of reservations to find a match in the
// reference range.
for (IPv6ResrvIterator r1 = resrv1.first; r1 != resrv1.second; ++r1) {
IPv6ResrvIterator r2 = resrv2.first;
for (; r2 != resrv2.second; ++r2) {
// IPv6Resrv object implements equality operator.
if (r1->second == r2->second) {
break;
}
}
// If r2 iterator reached the end of the range it means that there
// is no match.
if (r2 == resrv2.second) {
ADD_FAILURE() << "No match found for reservation: "
<< resrv1.first->second.getPrefix().toText();
}
}
if (std::distance(resrv1.first, resrv1.second) > 0) {
for (; resrv1.first != resrv1.second; resrv1.first++) {
IPv6ResrvIterator iter = resrv2.first;
......@@ -307,7 +319,7 @@ GenericHostDataSourceTest::compareReservations6(IPv6ResrvRange resrv1,
break;
}
iter++;
if (iter == resrv2.second && expect_match) {
if (iter == resrv2.second) {
ADD_FAILURE()<< "Reservation comparison failed, "
"no match for reservation: "
<< resrv1.first->second.getPrefix().toText();
......@@ -824,10 +836,10 @@ void GenericHostDataSourceTest::testAddr6AndPrefix(){
ASSERT_TRUE(from_hds);
// Check if reservations are the same
compareReservations6(host->getIPv6Reservations(), from_hds->getIPv6Reservations(), true);
compareReservations6(host->getIPv6Reservations(), from_hds->getIPv6Reservations());
}
void GenericHostDataSourceTest::testMultipletReservations(){
void GenericHostDataSourceTest::testMultipleReservations(){
// Make sure we have the pointer to the host data source.
ASSERT_TRUE(hdsptr_);
uint8_t len = 128;
......@@ -857,7 +869,7 @@ void GenericHostDataSourceTest::testMultipletReservations(){
compareHosts(host, from_hds);
}
void GenericHostDataSourceTest::testMultipletReservationsDifferentOrder(){
void GenericHostDataSourceTest::testMultipleReservationsDifferentOrder(){
// Make sure we have the pointer to the host data source.
ASSERT_TRUE(hdsptr_);
uint8_t len = 128;
......@@ -882,7 +894,7 @@ void GenericHostDataSourceTest::testMultipletReservationsDifferentOrder(){
host2->addReservation(resv1);
// Check if reservations are the same
compareReservations6(host1->getIPv6Reservations(), host2->getIPv6Reservations(), true);
compareReservations6(host1->getIPv6Reservations(), host2->getIPv6Reservations());
}
......
......@@ -112,8 +112,7 @@ public:
///
/// @param resv1 first IPv6 reservations list
/// @param resv2 second IPv6 reservations list
void compareReservations6(IPv6ResrvRange resv1, IPv6ResrvRange resv2,
bool expect_match);
void compareReservations6(IPv6ResrvRange resv1, IPv6ResrvRange resv2);
/// @brief Compares two client classes
///
......@@ -212,12 +211,12 @@ public:
void testAddr6AndPrefix();
/// @brief Tests if host with multiple IPv6 reservations can be added and then
/// retrieved correctly.
void testMultipletReservations();
/// retrieved correctly.
void testMultipleReservations();
/// @brief Tests if compareIPv6Reservations() method treats same pool of
/// reservations but added in different order as equal.
void testMultipletReservationsDifferentOrder();
/// reservations but added in different order as equal.
void testMultipleReservationsDifferentOrder();
/// @brief Test if host reservations made for different IPv6 subnets
/// are handled correctly.
......
......@@ -322,13 +322,13 @@ TEST_F(MySqlHostDataSourceTest, addr6AndPrefix) {
// Tests if host with multiple IPv6 reservations can be added and then
// retrieved correctly. Test checks reservations comparing.
TEST_F(MySqlHostDataSourceTest, multipleReservations){
testMultipletReservations();
testMultipleReservations();
}
// Tests if compareIPv6Reservations() method treats same pool of reservations
// but added in different order as equal.
TEST_F(MySqlHostDataSourceTest, multipleReservationsDifferentOrder){
testMultipletReservationsDifferentOrder();
testMultipleReservationsDifferentOrder();
}
// Test verifies if multiple client classes for IPv4 can be stored.
......
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