Commit e6beeb3f authored by Francis Dupont's avatar Francis Dupont

[#1196] Addressed comments

parent 898beae2
......@@ -2035,8 +2035,8 @@ PgSqlHostDataSourceImpl::PgSqlHostDataSourceImpl(const PgSqlConnection::Paramete
: parameters_(parameters) {
// Validate the schema version first.
std::pair<uint32_t, uint32_t> code_version(PGSQL_SCHEMA_VERSION_MAJOR,
PGSQL_SCHEMA_VERSION_MINOR);
std::pair<uint32_t, uint32_t> code_version(PG_SCHEMA_VERSION_MAJOR,
PG_SCHEMA_VERSION_MINOR);
std::pair<uint32_t, uint32_t> db_version = getVersion();
if (code_version != db_version) {
isc_throw(DbOpenError,
......
......@@ -1086,8 +1086,7 @@ public:
/// calls will return false.
///
/// Checks against negative values for the state count and logs once
/// a warning message. Unfortunately not getting the message is not
/// a proof that detailed counters are correct.
/// a warning message.
///
/// @param row Storage for the fetched row
///
......@@ -1212,8 +1211,8 @@ PgSqlLeaseMgr::PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters)
: parameters_(parameters) {
// Validate schema version first.
std::pair<uint32_t, uint32_t> code_version(PGSQL_SCHEMA_VERSION_MAJOR,
PGSQL_SCHEMA_VERSION_MINOR);
std::pair<uint32_t, uint32_t> code_version(PG_SCHEMA_VERSION_MAJOR,
PG_SCHEMA_VERSION_MINOR);
std::pair<uint32_t, uint32_t> db_version = getVersion();
if (code_version != db_version) {
isc_throw(DbOpenError,
......@@ -1263,8 +1262,8 @@ PgSqlLeaseMgr::createContext() const {
std::string
PgSqlLeaseMgr::getDBVersion() {
std::stringstream tmp;
tmp << "PostgreSQL backend " << PGSQL_SCHEMA_VERSION_MAJOR;
tmp << "." << PGSQL_SCHEMA_VERSION_MINOR;
tmp << "PostgreSQL backend " << PG_SCHEMA_VERSION_MAJOR;
tmp << "." << PG_SCHEMA_VERSION_MINOR;
tmp << ", library " << PQlibVersion();
return (tmp.str());
}
......
......@@ -858,14 +858,14 @@ TEST_F(CqlLeaseMgrTest, leaseStatsQuery6) {
testLeaseStatsQuery6();
}
/// @brief Tests v4 lease stats to never go negative.
TEST_F(CqlLeaseMgrTest, leaseStatsQueryNegative4) {
testLeaseStatsQueryNegative4();
/// @brief Tests v4 lease stats to be attributed to the wrong subnet.
TEST_F(CqlLeaseMgrTest, leaseStatsQueryAttribution4) {
testLeaseStatsQueryAttribution4();
}
/// @brief Tests v6 lease stats to never go negative.
TEST_F(CqlLeaseMgrTest, leaseStatsQueryNegative6) {
testLeaseStatsQueryNegative6();
/// @brief Tests v6 lease stats to be attributed to the wrong subnet.
TEST_F(CqlLeaseMgrTest, leaseStatsQueryAttribution6) {
testLeaseStatsQueryAttribution6();
}
} // namespace
......@@ -3630,7 +3630,7 @@ GenericLeaseMgrTest::testLeaseStatsQuery6() {
}
void
GenericLeaseMgrTest::testLeaseStatsQueryNegative4() {
GenericLeaseMgrTest::testLeaseStatsQueryAttribution4() {
// Create two subnets for the same range.
CfgSubnets4Ptr cfg = CfgMgr::instance().getStagingCfg()->getCfgSubnets4();
Subnet4Ptr subnet;
......@@ -3638,7 +3638,8 @@ GenericLeaseMgrTest::testLeaseStatsQueryNegative4() {
subnet.reset(new Subnet4(IOAddress("192.0.1.0"), 24, 1, 2, 3, 1));
cfg->add(subnet);
subnet.reset(new Subnet4(IOAddress("192.0.1.1"), 24, 1, 2, 3, 2));
// Note it is even allowed to use 192.0.1.1/24 here...
subnet.reset(new Subnet4(IOAddress("192.0.1.0"), 25, 1, 2, 3, 2));
cfg->add(subnet);
ASSERT_NO_THROW(CfgMgr::instance().commit());
......@@ -3671,7 +3672,7 @@ GenericLeaseMgrTest::testLeaseStatsQueryNegative4() {
}
void
GenericLeaseMgrTest::testLeaseStatsQueryNegative6() {
GenericLeaseMgrTest::testLeaseStatsQueryAttribution6() {
// Create two subnets.
CfgSubnets6Ptr cfg = CfgMgr::instance().getStagingCfg()->getCfgSubnets6();
Subnet6Ptr subnet;
......
......@@ -467,23 +467,25 @@ public:
///
void testLeaseStatsQuery6();
/// @brief Checks if v4 LeaseStatsQuery can get negative counters
/// @brief Checks if v4 LeaseStatsQuery can get bad attribution.
///
/// It creates two subnets with leases and move one from the first
/// to the second. If counters are not updated this can lead to
/// negative counters.
/// bad attribution i.e. a lease is counted in a subnet when it
/// belongs to another one.
///
void testLeaseStatsQueryNegative4();
void testLeaseStatsQueryAttribution4();
/// @brief Checks if v6 LeaseStatsQuery can get negative counters
/// @brief Checks if v6 LeaseStatsQuery can get bad attribution.
///
/// It creates two subnets with leases and move one from the first
/// to the second. If counters are not updated this can lead to
/// negative counters.
/// bad attribution i.e. a lease is counted in a subnet when it
/// belongs to another one.
///
/// @note We can check the lease type change too but in the real
/// world this never happens.
void testLeaseStatsQueryNegative6();
void testLeaseStatsQueryAttribution6();
/// @brief Compares LeaseQueryStats content to expected set of rows
///
......
......@@ -2258,16 +2258,16 @@ TEST_F(MemfileLeaseMgrTest, leaseStatsQuery6) {
testLeaseStatsQuery6();
}
/// @brief Tests v4 lease stats to never go negative.
TEST_F(MemfileLeaseMgrTest, leaseStatsQueryNegative4) {
/// @brief Tests v4 lease stats to be attributed to the wrong subnet.
TEST_F(MemfileLeaseMgrTest, leaseStatsQueryAttribution4) {
startBackend(V4);
testLeaseStatsQueryNegative4();
testLeaseStatsQueryAttribution4();
}
/// @brief Tests v6 lease stats to never go negative.
TEST_F(MemfileLeaseMgrTest, leaseStatsQueryNegative6) {
/// @brief Tests v6 lease stats to be attributed to the wrong subnet.
TEST_F(MemfileLeaseMgrTest, leaseStatsQueryAttribution6) {
startBackend(V6);
testLeaseStatsQueryNegative6();
testLeaseStatsQueryAttribution6();
}
} // namespace
......@@ -1015,26 +1015,26 @@ TEST_F(MySqlLeaseMgrTest, leaseStatsQuery6MultiThreading) {
testLeaseStatsQuery6();
}
/// @brief Tests v4 lease stats to never go negative.
TEST_F(MySqlLeaseMgrTest, leaseStatsQueryNegative4) {
testLeaseStatsQueryNegative4();
/// @brief Tests v4 lease stats to be attributed to the wrong subnet.
TEST_F(MySqlLeaseMgrTest, leaseStatsQueryAttribution4) {
testLeaseStatsQueryAttribution4();
}
/// @brief Tests v4 lease stats to never go negative.
TEST_F(MySqlLeaseMgrTest, leaseStatsQueryNegative4MultiThreading) {
/// @brief Tests v4 lease stats to be attributed to the wrong subnet.
TEST_F(MySqlLeaseMgrTest, leaseStatsQueryAttribution4MultiThreading) {
MultiThreadingMgr::instance().setMode(true);
testLeaseStatsQueryNegative4();
testLeaseStatsQueryAttribution4();
}
/// @brief Tests v6 lease stats to never go negative.
TEST_F(MySqlLeaseMgrTest, leaseStatsQueryNegative6) {
testLeaseStatsQueryNegative6();
/// @brief Tests v6 lease stats to be attributed to the wrong subnet.
TEST_F(MySqlLeaseMgrTest, leaseStatsQueryAttribution6) {
testLeaseStatsQueryAttribution6();
}
/// @brief Tests v6 lease stats to never go negative.
TEST_F(MySqlLeaseMgrTest, leaseStatsQueryNegative6MultiThreading) {
/// @brief Tests v6 lease stats to be attributed to the wrong subnet.
TEST_F(MySqlLeaseMgrTest, leaseStatsQueryAttribution6MultiThreading) {
MultiThreadingMgr::instance().setMode(true);
testLeaseStatsQueryNegative6();
testLeaseStatsQueryAttribution6();
}
} // namespace
......@@ -246,8 +246,8 @@ TEST_F(PgSqlLeaseMgrTest, checkVersion) {
// Check version
pair<uint32_t, uint32_t> version;
ASSERT_NO_THROW(version = lmptr_->getVersion());
EXPECT_EQ(PGSQL_SCHEMA_VERSION_MAJOR, version.first);
EXPECT_EQ(PGSQL_SCHEMA_VERSION_MINOR, version.second);
EXPECT_EQ(PG_SCHEMA_VERSION_MAJOR, version.first);
EXPECT_EQ(PG_SCHEMA_VERSION_MINOR, version.second);
}
////////////////////////////////////////////////////////////////////////////////
......@@ -971,26 +971,26 @@ TEST_F(PgSqlLeaseMgrTest, leaseStatsQuery6MultiThreading) {
testLeaseStatsQuery6();
}
/// @brief Tests v4 lease stats to never go negative.
TEST_F(PgSqlLeaseMgrTest, leaseStatsQueryNegative4) {
testLeaseStatsQueryNegative4();
/// @brief Tests v4 lease stats to be attributed to the wrong subnet.
TEST_F(PgSqlLeaseMgrTest, leaseStatsQueryAttribution4) {
testLeaseStatsQueryAttribution4();
}
/// @brief Tests v4 lease stats to never go negative.
TEST_F(PgSqlLeaseMgrTest, leaseStatsQueryNegative4MultiThreading) {
/// @brief Tests v4 lease stats to be attributed to the wrong subnet.
TEST_F(PgSqlLeaseMgrTest, leaseStatsQueryAttribution4MultiThreading) {
MultiThreadingMgr::instance().setMode(true);
testLeaseStatsQueryNegative4();
testLeaseStatsQueryAttribution4();
}
/// @brief Tests v6 lease stats to never go negative.
TEST_F(PgSqlLeaseMgrTest, leaseStatsQueryNegative6) {
testLeaseStatsQueryNegative6();
/// @brief Tests v6 lease stats to be attributed to the wrong subnet.
TEST_F(PgSqlLeaseMgrTest, leaseStatsQueryAttribution6) {
testLeaseStatsQueryAttribution6();
}
/// @brief Tests v6 lease stats to never go negative.
TEST_F(PgSqlLeaseMgrTest, leaseStatsQueryNegative6MultiThreading) {
/// @brief Tests v6 lease stats to be attributed to the wrong subnet.
TEST_F(PgSqlLeaseMgrTest, leaseStatsQueryAttribution6MultiThreading) {
MultiThreadingMgr::instance().setMode(true);
testLeaseStatsQueryNegative6();
testLeaseStatsQueryAttribution6();
}
} // namespace
......@@ -18,8 +18,8 @@ namespace isc {
namespace db {
/// @brief Define PostgreSQL backend version: 6.1
const uint32_t PGSQL_SCHEMA_VERSION_MAJOR = 6;
const uint32_t PGSQL_SCHEMA_VERSION_MINOR = 1;
const uint32_t PG_SCHEMA_VERSION_MAJOR = 6;
const uint32_t PG_SCHEMA_VERSION_MINOR = 1;
// Maximum number of parameters that can be used a statement
// @todo This allows us to use an initializer list (since we can't
......
......@@ -56,7 +56,7 @@ bool wipePgSQLData(bool show_err) {
cmd << " sh " << DATABASE_SCRIPTS_DIR << "/pgsql/wipe_data.sh";
// Add expected schema version as the wipe script's first argument.
cmd << " " << PGSQL_SCHEMA_VERSION_MAJOR << "." << PGSQL_SCHEMA_VERSION_MINOR;
cmd << " " << PG_SCHEMA_VERSION_MAJOR << "." << PG_SCHEMA_VERSION_MINOR;
// Now add command line arguments for psql.
cmd << " --set ON_ERROR_STOP=1 -A -t -h localhost -q -U keatest -d keatest";
......
......@@ -76,8 +76,8 @@ void createPgSQLSchema(bool show_err = false, bool force = false);
/// <TEST_ADMIN_SCRIPTS_DIR>/pgsql/wipe_data.sh
///
/// This will fail if there is no schema, if the existing schema
/// version is incorrect (i.e. does not match PGSQL_SCHEMA_VERSION_MAJOR
/// and PGSQL_SCHEMA_VERSION_MINOR), or a SQL error occurs. Otherwise,
/// version is incorrect (i.e. does not match PG_SCHEMA_VERSION_MAJOR
/// and PG_SCHEMA_VERSION_MINOR), or a SQL error occurs. Otherwise,
/// the script is should delete all transient data, leaving intact
/// reference tables.
///
......
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