From: Razvan Becheriu Date: Thu, 10 Feb 2022 12:51:34 +0000 (+0200) Subject: [#95] addressed some review comments and formatting X-Git-Tag: Kea-2.1.3~33 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e32b6f36704091cfee5fc8cf7165c9924059dccc;p=thirdparty%2Fkea.git [#95] addressed some review comments and formatting --- diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc index d204989e13..51412e012e 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc @@ -3294,6 +3294,7 @@ TaggedStatementArray tagged_statements = { { { MySqlConfigBackendDHCPv4Impl::INSERT_OPTION4_SERVER, MYSQL_INSERT_OPTION_SERVER(dhcp4) }, + // Insert client class. { MySqlConfigBackendDHCPv4Impl::INSERT_CLIENT_CLASS4, "INSERT INTO dhcp4_client_class(" @@ -3311,14 +3312,17 @@ TaggedStatementArray tagged_statements = { { " modification_ts" ") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)" }, + // Insert association of a client class with a server. { MySqlConfigBackendDHCPv4Impl::INSERT_CLIENT_CLASS4_SERVER, MYSQL_INSERT_CLIENT_CLASS_SERVER(dhcp4) }, + // Insert client class dependency. { MySqlConfigBackendDHCPv4Impl::INSERT_CLIENT_CLASS4_DEPENDENCY, MYSQL_INSERT_CLIENT_CLASS_DEPENDENCY(dhcp4) }, + // Insert server with server tag and description. { MySqlConfigBackendDHCPv4Impl::INSERT_SERVER4, MYSQL_INSERT_SERVER(dhcp4) diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc index cc24a178d9..25d50048e2 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc @@ -11,6 +11,7 @@ #include #include #include + #include #include #include @@ -255,7 +256,7 @@ MySqlConfigBackendImpl::deleteFromTable(const int index, // When deleting multiple objects we must not use ANY server. if (server_selector.amAny()) { isc_throw(InvalidOperation, "deleting multiple objects for ANY server is not" - " supported"); + " supported"); } MySqlBindingCollection in_bindings; @@ -497,7 +498,7 @@ MySqlConfigBackendImpl::createUpdateOptionDef(const db::ServerSelector& server_s if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" - " (unassigned) is unsupported at the moment"); + " (unassigned) is unsupported at the moment"); } auto tag = getServerTag(server_selector, "creating or updating option definition"); @@ -566,7 +567,7 @@ MySqlConfigBackendImpl::getOption(const int index, if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" - " (unassigned) is unsupported at the moment"); + " (unassigned) is unsupported at the moment"); } auto tag = getServerTag(server_selector, "fetching global option"); @@ -631,7 +632,7 @@ MySqlConfigBackendImpl::getOption(const int index, if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" - " (unassigned) is unsupported at the moment"); + " (unassigned) is unsupported at the moment"); } auto tag = getServerTag(server_selector, "fetching subnet level option"); @@ -662,7 +663,7 @@ MySqlConfigBackendImpl::getOption(const int index, if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" - " (unassigned) is unsupported at the moment"); + " (unassigned) is unsupported at the moment"); } std::string msg = "fetching "; @@ -701,7 +702,7 @@ MySqlConfigBackendImpl::getOption(const int index, if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" - " (unassigned) is unsupported at the moment"); + " (unassigned) is unsupported at the moment"); } auto tag = getServerTag(server_selector, "fetching shared network level option"); diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc b/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc index f1a8cec7a5..7e59ae1efa 100644 --- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc +++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc @@ -49,7 +49,7 @@ using namespace isc::util; namespace isc { namespace dhcp { -/// @brief Implementation of the PgSQL Configuration Backend. +/// @brief Implementation of the PostgreSQL Configuration Backend. class PgSqlConfigBackendDHCPv4Impl : public PgSqlConfigBackendImpl { public: @@ -265,7 +265,7 @@ public: } transaction.commit(); - } + } /// @brief Sends query to the database to retrieve multiple subnets. /// @@ -884,6 +884,7 @@ public: } transaction.commit(); + return (count); } @@ -919,6 +920,7 @@ public: // Commit the transaction. transaction.commit(); + return (count); } @@ -1029,6 +1031,7 @@ public: return (true); } + }; namespace { @@ -1167,7 +1170,10 @@ TaggedStatementArray tagged_statements = { { // Select all subnets. { // PgSqlConfigBackendDHCPv4Impl::GET_ALL_SUBNETS4, - 0, { OID_NONE }, + 0, + { + OID_NONE + }, "GET_ALL_SUBNETS4", PGSQL_GET_SUBNET4_NO_TAG() }, @@ -1175,7 +1181,10 @@ TaggedStatementArray tagged_statements = { { // Select all unassigned subnets. { // PgSqlConfigBackendDHCPv4Impl::GET_ALL_SUBNETS4_UNASSIGNED, - 0, { OID_NONE }, + 0, + { + OID_NONE + }, "GET_ALL_SUBNETS4_UNASSIGNED", PGSQL_GET_SUBNET4_UNASSIGNED() }, @@ -1277,7 +1286,10 @@ TaggedStatementArray tagged_statements = { { // Select all shared networks. { // PgSqlConfigBackendDHCPv4Impl::GET_ALL_SHARED_NETWORKS4, - 0, { OID_NONE }, + 0, + { + OID_NONE + }, "GET_ALL_SHARED_NETWORKS4", PGSQL_GET_SHARED_NETWORK4_NO_TAG() }, @@ -1285,7 +1297,10 @@ TaggedStatementArray tagged_statements = { { // Select all unassigned shared networks. { // PgSqlConfigBackendDHCPv4Impl::GET_ALL_SHARED_NETWORKS4_UNASSIGNED, - 0, { OID_NONE }, + 0, + { + OID_NONE + }, "GET_ALL_SHARED_NETWORKS4_UNASSIGNED", PGSQL_GET_SHARED_NETWORK4_UNASSIGNED() }, @@ -1339,7 +1354,6 @@ TaggedStatementArray tagged_statements = { { // Retrieves modified option definitions. { // PgSqlConfigBackendDHCPv4Impl::GET_MODIFIED_OPTION_DEFS4, - // server tag is $1 2, { OID_VARCHAR, // 1 server_tag @@ -1365,7 +1379,6 @@ TaggedStatementArray tagged_statements = { { // Retrieves all global options. { // PgSqlConfigBackendDHCPv4Impl::GET_ALL_OPTIONS4, - // server tag is $1 1, { OID_VARCHAR // 1 server_tag @@ -1377,7 +1390,6 @@ TaggedStatementArray tagged_statements = { { // Retrieves modified options. { // PgSqlConfigBackendDHCPv4Impl::GET_MODIFIED_OPTIONS4, - // server tag is $1 2, { OID_VARCHAR, // 1 server_tag @@ -1390,7 +1402,6 @@ TaggedStatementArray tagged_statements = { { // Retrieves an option for a given subnet, option code and space. { // PgSqlConfigBackendDHCPv4Impl::GET_OPTION4_SUBNET_ID_CODE_SPACE, - // server tag is $1 4, { OID_VARCHAR, // 1 server_tag @@ -1405,7 +1416,6 @@ TaggedStatementArray tagged_statements = { { // Retrieves an option for a given pool, option code and space. { // PgSqlConfigBackendDHCPv4Impl::GET_OPTION4_POOL_ID_CODE_SPACE, - // server tag is $1 4, { OID_VARCHAR, // 1 server_tag @@ -1445,7 +1455,10 @@ TaggedStatementArray tagged_statements = { { // Select all client classes. { // PgSqlConfigBackendDHCPv4Impl::GET_ALL_CLIENT_CLASSES4, - 0, { OID_NONE }, + 0, + { + OID_NONE + }, "GET_ALL_CLIENT_CLASSES4", PGSQL_GET_CLIENT_CLASS4_WITH_TAG() }, @@ -1453,7 +1466,10 @@ TaggedStatementArray tagged_statements = { { // Select all unassigned client classes. { // PgSqlConfigBackendDHCPv4Impl::GET_ALL_CLIENT_CLASSES4_UNASSIGNED, - 0, { OID_NONE }, + 0, + { + OID_NONE + }, "GET_ALL_CLIENT_CLASSES4_UNASSIGNED", PGSQL_GET_CLIENT_CLASS4_UNASSIGNED() }, @@ -1479,6 +1495,7 @@ TaggedStatementArray tagged_statements = { { "GET_MODIFIED_CLIENT_CLASSES4_UNASSIGNED", PGSQL_GET_CLIENT_CLASS4_UNASSIGNED(AND c.modification_ts >= $1) }, + // Retrieves the most recent audit entries. { // PgSqlConfigBackendDHCPv4Impl::GET_AUDIT_ENTRIES4_TIME, @@ -1506,7 +1523,10 @@ TaggedStatementArray tagged_statements = { { // Retrieves all servers. { // PgSqlConfigBackendDHCPv4Impl::GET_ALL_SERVERS4, - 0, { OID_NONE }, + 0, + { + OID_NONE + }, "GET_ALL_SERVERS4", PGSQL_GET_ALL_SERVERS(dhcp4) }, @@ -1553,7 +1573,7 @@ TaggedStatementArray tagged_statements = { { OID_VARCHAR, // 8 interface OID_BOOL, // 9 match_client_id OID_TIMESTAMP, // 10 modification_ts - OID_TEXT, // 11 next_server --- cast as INET + OID_TEXT, // 11 next_server - cast as inet OID_INT8, // 12 rebind_timer OID_TEXT, // 13 relay OID_INT8, // 14 renew_timer @@ -1566,8 +1586,8 @@ TaggedStatementArray tagged_statements = { { OID_INT8, // 21 min_valid_lifetime OID_INT8, // 22 max_valid_lifetime OID_BOOL, // 23 calculate_tee_times - OID_TEXT, // 24 t1_percent --- cast as FLOAT - OID_TEXT, // 25 t2_percent --- cast as FLOAT + OID_TEXT, // 24 t1_percent - cast as float + OID_TEXT, // 25 t2_percent - cast as float OID_BOOL, // 26 authoritative OID_BOOL, // 27 ddns_send_updates OID_BOOL, // 28 ddns_override_no_update @@ -1577,7 +1597,7 @@ TaggedStatementArray tagged_statements = { { OID_VARCHAR, // 32 ddns_qualifying_suffix OID_BOOL, // 33 reservations_in_subnet OID_BOOL, // 34 reservations_out_of_pool - OID_TEXT, // 35 cache_threshold -- cast as FLOAT + OID_TEXT, // 35 cache_threshold - cast as float OID_INT8 // 36 cache_max_age" }, "INSERT_SUBNET4", @@ -1680,7 +1700,7 @@ TaggedStatementArray tagged_statements = { { OID_TEXT, // 17 t2_percent - cast as float OID_BOOL, // 18 authoritative, OID_VARCHAR, // 19 boot_file_name, - OID_TEXT, // 20 next_server - cast as inet + OID_TEXT, // 20 next_server - cast as inaet OID_VARCHAR, // 21 server_hostname, OID_BOOL, // 22 ddns_send_updates, OID_BOOL, // 23 ddns_override_no_update, @@ -1761,7 +1781,7 @@ TaggedStatementArray tagged_statements = { { OID_VARCHAR, // 7 encapsulate OID_VARCHAR, // 8 record_types OID_VARCHAR, // 9 user_context - OID_INT8 // 10 class_id" -- column is missing from dhcpX_option_def tables + OID_INT8 // 10 class_id" - column is missing from dhcpX_option_def tables }, "INSERT_OPTION_DEF4", PGSQL_INSERT_OPTION_DEF(dhcp4) @@ -1941,7 +1961,7 @@ TaggedStatementArray tagged_statements = { { OID_VARCHAR, // 8 interface OID_BOOL, // 9 match_client_id OID_TIMESTAMP, // 10 modification_ts - OID_TEXT, // 11 next_server - cast as INET + OID_TEXT, // 11 next_server - cast as inet OID_INT8, // 12 rebind_timer OID_TEXT, // 13 relay OID_INT8, // 14 renew_timer @@ -1954,8 +1974,8 @@ TaggedStatementArray tagged_statements = { { OID_INT8, // 21 min_valid_lifetime OID_INT8, // 22 max_valid_lifetime OID_BOOL, // 23 calculate_tee_times - OID_TEXT, // 24 t1_percent - cast as FLOAT - OID_TEXT, // 25 t2_percent - cast as FLOAT + OID_TEXT, // 24 t1_percent - cast as float + OID_TEXT, // 25 t2_percent - cast as float OID_BOOL, // 26 authoritative OID_BOOL, // 27 ddns_send_updates OID_BOOL, // 28 ddns_override_no_update @@ -1965,7 +1985,7 @@ TaggedStatementArray tagged_statements = { { OID_VARCHAR, // 32 ddns_qualifying_suffix OID_BOOL, // 33 reservations_in_subnet OID_BOOL, // 34 reservations_out_of_pool - OID_TEXT, // 35 cache_threshold - cast as FLOAT + OID_TEXT, // 35 cache_threshold - cast as float OID_INT8, // 36 cache_max_age" OID_INT8, // 37 subnet_id (of subnet to update) OID_VARCHAR // 38 subnet_prefix (of subnet to update) @@ -2342,7 +2362,10 @@ TaggedStatementArray tagged_statements = { { // Delete all global parameters which are unassigned to any servers. { // PgSqlConfigBackendDHCPv4Impl::DELETE_ALL_GLOBAL_PARAMETERS4_UNASSIGNED, - 0, { OID_NONE }, + 0, + { + OID_NONE + }, "DELETE_ALL_GLOBAL_PARAMETERS4_UNASSIGNED", PGSQL_DELETE_GLOBAL_PARAMETER_UNASSIGNED(dhcp4) }, @@ -2526,11 +2549,13 @@ TaggedStatementArray tagged_statements = { { PGSQL_DELETE_OPTION_DEF(dhcp4) }, - // Delete all option definitions which are assigned to no servers. { // PgSqlConfigBackendDHCPv4Impl::DELETE_ALL_OPTION_DEFS4_UNASSIGNED, - 0, { OID_NONE }, + 0, + { + OID_NONE + }, "DELETE_ALL_OPTION_DEFS4_UNASSIGNED", PGSQL_DELETE_OPTION_DEF_UNASSIGNED(dhcp4) }, @@ -2562,9 +2587,12 @@ TaggedStatementArray tagged_statements = { { // Delete all global options which are unassigned to any servers. { // PgSqlConfigBackendDHCPv4Impl::DELETE_ALL_GLOBAL_OPTIONS4_UNASSIGNED, - 0, { OID_NONE }, + 0, + { + OID_NONE + }, "DELETE_ALL_GLOBAL_OPTIONS4_UNASSIGNED", - PGSQL_DELETE_OPTION_UNASSIGNED(dhcp4, AND scope_id = 0) + PGSQL_DELETE_OPTION_UNASSIGNED(dhcp4, AND o.scope_id = 0) }, // Delete single option from a subnet. @@ -2679,7 +2707,10 @@ TaggedStatementArray tagged_statements = { { // Delete all unassigned client classes. { // PgSqlConfigBackendDHCPv4Impl::DELETE_ALL_CLIENT_CLASSES4_UNASSIGNED, - 0, { OID_NONE }, + 0, + { + OID_NONE + }, "DELETE_ALL_CLIENT_CLASSES4_UNASSIGNED", PGSQL_DELETE_CLIENT_CLASS_UNASSIGNED(dhcp4) }, @@ -2748,7 +2779,7 @@ PgSqlConfigBackendDHCPv4Impl::PgSqlConfigBackendDHCPv4Impl(const DatabaseConnect tagged_statements.end()); // @todo As part of enabling read-only CB access, statements need to // be limited: -// tagged_statements.begin() + WRITE_STMTS_BEGIN); +// tagged_statements.begin() + WRITE_STMTS_BEGIN); // Create unique timer name per instance. timer_name_ = "PgSqlConfigBackend4["; diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc index 262d2fd800..e9eb6838cf 100644 --- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc +++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc @@ -61,7 +61,8 @@ PgSqlConfigBackendImpl::ScopedAuditRevision::ScopedAuditRevision( bool cascade_transaction) : impl_(impl) { impl_->createAuditRevision(index, server_selector, - boost::posix_time::microsec_clock::local_time(), log_message, + boost::posix_time::microsec_clock::local_time(), + log_message, cascade_transaction); } @@ -316,6 +317,7 @@ PgSqlConfigBackendImpl::getGlobalParameters(const int index, local_parameters.replace(existing, last_param); return; } + } // If there is no such parameter yet or the existing parameter @@ -416,6 +418,7 @@ PgSqlConfigBackendImpl::getAllOptions(const int /* index */, const Option::Universe& /* universe */, const ServerSelector& server_selector) { OptionContainer options; + auto tags = server_selector.getTags(); isc_throw(NotImplemented, NOT_IMPL_STR); @@ -463,7 +466,8 @@ PgSqlConfigBackendImpl::getOption(const int index, isc_throw(NotImplemented, NOT_IMPL_STR); getOptions(index, in_bindings, universe, options); - return (options.empty() ? OptionDescriptorPtr() : OptionDescriptor::create(*options.begin())); + return (options.empty() ? OptionDescriptorPtr() : + OptionDescriptor::create(*options.begin())); } OptionDescriptorPtr @@ -494,7 +498,8 @@ PgSqlConfigBackendImpl::getOption(const int index, isc_throw(NotImplemented, NOT_IMPL_STR); getOptions(index, in_bindings, universe, options); - return (options.empty() ? OptionDescriptorPtr() : OptionDescriptor::create(*options.begin())); + return (options.empty() ? OptionDescriptorPtr() : + OptionDescriptor::create(*options.begin())); } OptionDescriptorPtr @@ -516,7 +521,8 @@ PgSqlConfigBackendImpl::getOption(const int index, PsqlBindArray in_bindings; isc_throw(NotImplemented, NOT_IMPL_STR); getOptions(index, in_bindings, universe, options); - return (options.empty() ? OptionDescriptorPtr() : OptionDescriptor::create(*options.begin())); + return (options.empty() ? OptionDescriptorPtr() : + OptionDescriptor::create(*options.begin())); } void @@ -722,6 +728,5 @@ PgSqlConfigBackendImpl::addOptionValueBinding(PsqlBindArray& bindings, } } - } // namespace dhcp } // end of namespace isc diff --git a/src/hooks/dhcp/pgsql_cb/tests/pgsql_cb_dhcp4_unittest.cc b/src/hooks/dhcp/pgsql_cb/tests/pgsql_cb_dhcp4_unittest.cc index 3bfe3ebd24..78a118d16a 100644 --- a/src/hooks/dhcp/pgsql_cb/tests/pgsql_cb_dhcp4_unittest.cc +++ b/src/hooks/dhcp/pgsql_cb/tests/pgsql_cb_dhcp4_unittest.cc @@ -38,7 +38,7 @@ namespace ph = std::placeholders; namespace { -/// @brief Test implementation of the PgSQL configuration backend. +/// @brief Test implementation of the PostgreSQL configuration backend. /// /// It exposes protected members of the @c PgSqlConfigBackendDHCPv4. class TestPgSqlConfigBackendDHCPv4 : public PgSqlConfigBackendDHCPv4 { @@ -59,10 +59,10 @@ public: class PgSqlConfigBackendDHCPv4Test : public GenericConfigBackendDHCPv4Test { public: /// @brief Constructor. - PgSqlConfigBackendDHCPv4Test() {}; + PgSqlConfigBackendDHCPv4Test() {} /// @brief Destructor. - virtual ~PgSqlConfigBackendDHCPv4Test() {}; + virtual ~PgSqlConfigBackendDHCPv4Test() {} /// @brief Creates the PostgreSQL back end schema virtual void createSchema() { @@ -92,7 +92,7 @@ public: return (ConfigBackendDHCPv4Ptr(new TestPgSqlConfigBackendDHCPv4(params))); } - /// @brief Counts rows in a selected table in PgSQL database. + /// @brief Counts rows in a selected table in PostgreSQL database. /// /// This method can be used to verify that some configuration elements were /// deleted from a selected table as a result of cascade delete or a trigger. @@ -129,7 +129,7 @@ TEST_F(PgSqlConfigBackendDHCPv4Test, getPort) { getPortTest(); } -TEST_F(PgSqlConfigBackendDHCPv4Test, createUpdateDeleteServer) { +TEST_F(PgSqlConfigBackendDHCPv4Test, createUpdateDeleteServerTest) { createUpdateDeleteServerTest(); } diff --git a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc index c23a4c73d5..bb991fb319 100644 --- a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc +++ b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc @@ -1014,8 +1014,8 @@ GenericConfigBackendDHCPv4Test::nullKeyErrorTest() { // Create a global parameter (it should work with any object type). StampedValuePtr global_parameter = StampedValue::create("global", "value"); - ASSERT_THROW (cbptr_->createUpdateGlobalParameter4(ServerSelector::ONE("server1"), - global_parameter), NullKeyError); + ASSERT_THROW(cbptr_->createUpdateGlobalParameter4(ServerSelector::ONE("server1"), + global_parameter), NullKeyError); } void @@ -1066,7 +1066,7 @@ GenericConfigBackendDHCPv4Test::getSubnet4Test() { // The subnet shouldn't have been added, even though one of the servers exists. Subnet4Ptr returned_subnet; ASSERT_NO_THROW_LOG(returned_subnet = cbptr_->getSubnet4(ServerSelector::ONE("server2"), - subnet2->getID())); + subnet2->getID())); EXPECT_FALSE(returned_subnet); // Insert two subnets, one for all servers and one for server2. @@ -1163,7 +1163,7 @@ GenericConfigBackendDHCPv4Test::getSubnet4Test() { // Update the subnet in the database (both use the same prefix). subnet2.reset(new Subnet4(IOAddress("192.0.3.0"), 24, 30, 40, 60, 8192)); - ASSERT_NO_THROW_LOG(cbptr_->createUpdateSubnet4(ServerSelector::ONE("server2"), subnet2)); + ASSERT_NO_THROW_LOG(cbptr_->createUpdateSubnet4(ServerSelector::ONE("server2"), subnet2)); // Fetch again and verify. returned_subnet = cbptr_->getSubnet4(ServerSelector::ONE("server2"), subnet2->toText()); @@ -1174,7 +1174,7 @@ GenericConfigBackendDHCPv4Test::getSubnet4Test() { // with different subnets. This should throw. // Subnets are 10.0.0.0/8 id 1024 and 192.0.3.0/24 id 8192 subnet2.reset(new Subnet4(IOAddress("10.0.0.0"), 8, 30, 40, 60, 8192)); - EXPECT_THROW(cbptr_->createUpdateSubnet4(ServerSelector::ONE("server2"), subnet2), + EXPECT_THROW(cbptr_->createUpdateSubnet4(ServerSelector::ONE("server2"), subnet2), DuplicateEntry); } @@ -1399,7 +1399,7 @@ GenericConfigBackendDHCPv4Test::getAllSubnets4Test() { (*subnet_it)->toElement()->str()); } - // Attempt to remove the non existing subnet should return 0. + // Attempt to remove the non existing subnet should return 0. EXPECT_EQ(0, cbptr_->deleteSubnet4(ServerSelector::ALL(), 22)); EXPECT_EQ(0, cbptr_->deleteSubnet4(ServerSelector::ALL(), "155.0.3.0/24")); @@ -1740,7 +1740,7 @@ GenericConfigBackendDHCPv4Test::unassignedSubnet4Test() { // Trying to fetch the subnet by server tag should return no result. Subnet4Ptr returned_subnet; ASSERT_NO_THROW_LOG(returned_subnet = cbptr_->getSubnet4(ServerSelector::ONE("server1"), - subnet->getID())); + subnet->getID())); EXPECT_FALSE(returned_subnet); // The same if we use other calls. @@ -2534,7 +2534,7 @@ GenericConfigBackendDHCPv4Test::getModifiedSharedNetworks4Test() { } // Fetch shared networks with timestamp later than today. Only one - // shared network should be returned. + // shared network should be returned. SharedNetwork4Collection networks = cbptr_->getModifiedSharedNetworks4(ServerSelector::ALL(), timestamps_["after today"]); @@ -2670,7 +2670,7 @@ GenericConfigBackendDHCPv4Test::deleteSharedNetwork4SelectorsTest() { // Not supported selectors. EXPECT_THROW(cbptr_->deleteSharedNetwork4(ServerSelector::MULTIPLE({ "server1", "server2" }), - "level1"), + "level1"), isc::InvalidOperation); // Not implemented selectors. @@ -2734,7 +2734,7 @@ GenericConfigBackendDHCPv4Test::unassignedSharedNetworkTest() { // We should get the shared network if we ask for unassigned. ASSERT_NO_THROW_LOG(returned_network = cbptr_->getSharedNetwork4(ServerSelector::UNASSIGNED(), - "level1")); + "level1")); ASSERT_TRUE(returned_network); // Also if we ask for all unassigned networks it should be returned. @@ -2828,7 +2828,7 @@ GenericConfigBackendDHCPv4Test::sharedNetworkOptionsTest() { // Remove the shared network. This should not affect options assigned to the // other shared network. ASSERT_NO_THROW_LOG(cbptr_->deleteSharedNetwork4(ServerSelector::ALL(), - test_networks_[1]->getName())); + test_networks_[1]->getName())); EXPECT_EQ(1, countRows("dhcp4_shared_network")); EXPECT_EQ(1, countRows("dhcp4_options")); @@ -2840,7 +2840,7 @@ GenericConfigBackendDHCPv4Test::sharedNetworkOptionsTest() { // Delete this shared network. This should not affect the option associated // with the remaining shared network. ASSERT_NO_THROW_LOG(cbptr_->deleteSharedNetwork4(ServerSelector::ALL(), - test_networks_[0]->getName())); + test_networks_[0]->getName())); EXPECT_EQ(1, countRows("dhcp4_shared_network")); EXPECT_EQ(1, countRows("dhcp4_options")); } @@ -3822,7 +3822,7 @@ GenericConfigBackendDHCPv4Test::createUpdateDeleteSharedNetworkOption4Test() { opt_boot_file_name); returned_network = cbptr_->getSharedNetwork4(ServerSelector::ALL(), - shared_network->getName()); + shared_network->getName()); ASSERT_TRUE(returned_network); OptionDescriptor returned_opt_boot_file_name = diff --git a/src/lib/pgsql/pgsql_exchange.cc b/src/lib/pgsql/pgsql_exchange.cc index e1f2b0c638..baa8385c6c 100644 --- a/src/lib/pgsql/pgsql_exchange.cc +++ b/src/lib/pgsql/pgsql_exchange.cc @@ -450,7 +450,7 @@ PgSqlExchange::getInetValue4(const PgSqlResult& r, const int row, isc_throw(BadValue, "not a v4 address"); } - return(addr); + return (addr); } catch (const std::exception& ex) { isc_throw(DbOperationError, "Cannot convert data: " << data << " for: " << getColumnLabel(r, col) << " row:" << row @@ -469,7 +469,7 @@ PgSqlExchange::getInetValue6(const PgSqlResult& r, const int row, isc_throw(BadValue, "not a v6 address"); } - return(addr); + return (addr); } catch (const std::exception& ex) { isc_throw(DbOperationError, "Cannot convert data: " << data << " for: " << getColumnLabel(r, col) << " row:" << row @@ -516,7 +516,7 @@ PgSqlExchange::convertFromBytea(const PgSqlResult& r, const int row, << getColumnLabel(r, col) << " row:" << row); } - // Copy from the allocated buffer to caller's buffer the free up + // Copy from the allocated buffer to caller's buffer then free up // the allocated buffer. memcpy(buffer, bytes, bytes_converted); PQfreemem(bytes); @@ -579,7 +579,7 @@ PgSqlExchange::getTripletValue(const PgSqlResult& r, const int row, } uint32_t max_value = value; - if (!isColumnNull(r, row, max_col)) { + if (!isColumnNull(r, row, max_col)) { getColumnValue(r, row, max_col, max_value); } @@ -658,7 +658,7 @@ PgSqlResultRowWorker::getDouble(const size_t col) { const char* PgSqlResultRowWorker::getRawColumnValue(const size_t col) { - return(PgSqlExchange::getRawColumnValue(r_, row_, col)); + return (PgSqlExchange::getRawColumnValue(r_, row_, col)); } uint64_t @@ -689,12 +689,12 @@ PgSqlResultRowWorker::getBytes(const size_t col, std::vector& value) { isc::asiolink::IOAddress PgSqlResultRowWorker::getInet4(const size_t col) { - return(PgSqlExchange::getInetValue4(r_, row_, col)); + return (PgSqlExchange::getInetValue4(r_, row_, col)); } isc::asiolink::IOAddress PgSqlResultRowWorker::getInet6(const size_t col) { - return(PgSqlExchange::getInetValue6(r_, row_, col)); + return (PgSqlExchange::getInetValue6(r_, row_, col)); } boost::posix_time::ptime @@ -708,23 +708,23 @@ data::ElementPtr PgSqlResultRowWorker::getJSON(const size_t col) { data::ElementPtr value; getColumnValue(col, value); - return(value); + return (value); } isc::util::Triplet PgSqlResultRowWorker::getTriplet(const size_t col) { - return(PgSqlExchange::getTripletValue(r_, row_, col)); + return (PgSqlExchange::getTripletValue(r_, row_, col)); } isc::util::Triplet PgSqlResultRowWorker::getTriplet(const size_t def_col, const size_t min_col, const size_t max_col) { - return(PgSqlExchange::getTripletValue(r_, row_, def_col, min_col, max_col)); + return (PgSqlExchange::getTripletValue(r_, row_, def_col, min_col, max_col)); } std::string PgSqlResultRowWorker::dumpRow() { - return(PgSqlExchange::dumpRow(r_, row_)); + return (PgSqlExchange::dumpRow(r_, row_)); } } // end of isc::db namespace diff --git a/src/lib/pgsql/pgsql_exchange.h b/src/lib/pgsql/pgsql_exchange.h index 22233f7e89..ef135ce849 100644 --- a/src/lib/pgsql/pgsql_exchange.h +++ b/src/lib/pgsql/pgsql_exchange.h @@ -765,7 +765,7 @@ public: /// value /// /// @throw DbOperationError if the value cannot be fetched or is - /// invalid. + /// invalid. static void convertFromBytea(const PgSqlResult& r, const int row, const size_t col, std::vector& value); @@ -774,9 +774,9 @@ public: /// /// @param r the result set containing the query results /// @param row the row number within the result set - /// @param col the column number within the row If the column + /// @param col the column number within the row. If the column /// is null, the Triplet is returned as unspecified. - /// @param[out] value Triplet to receive the column value + /// @return Triplet to receive the column value /// /// @throw DbOperationError if the value cannot be fetched or is /// invalid. @@ -796,7 +796,7 @@ public: /// minium value. /// @param max_col the column number within the row that contains the /// maximum value. - /// @param[out] value Triplet to receive the column value + /// @return Triplet to receive the column value /// /// @throw DbOperationError if the value cannot be fetched or is /// invalid. @@ -931,13 +931,13 @@ public: /// @brief Fetches a timestamp column as a ptime. /// /// @param col the column number within the row - /// @param[out] value ptime parameter to receive the converted timestamp + /// @return ptime parameter to receive the converted timestamp boost::posix_time::ptime getTimestamp(const size_t col); /// @brief Fetches a JSON column as an ElementPtr. /// /// @param col the column number within the row - /// @param[out] value ElementPtr parameter to receive the column value + /// @return ElementPtr parameter to receive the column value data::ElementPtr getJSON(const size_t col); /// @brief Fetches a uint32_t value into a Triplet using a single @@ -945,7 +945,7 @@ public: /// /// @param col the column number within the row If the column /// is null, the Triplet is returned as unspecified. - /// @param[out] value Triplet to receive the column value + /// @return Triplet to receive the column value isc::util::Triplet getTriplet(const size_t col); /// @brief Fetches a uint32_t value into a Triplet using a three @@ -958,19 +958,20 @@ public: /// minium value. /// @param max_col the column number within the row that contains the /// maximum value. - /// @param[out] value Triplet to receive the column value + /// @return Triplet to receive the column value isc::util::Triplet getTriplet(const size_t def_col, const size_t min_col, const size_t max_col); /// @brief Diagnostic tool which dumps the Result row contents as a string /// - /// @return A string depiction of the row contents. + /// @return A string representation of the row contents. std::string dumpRow(); private: /// @brief Result set containing the row. const PgSqlResult& r_; + /// @brief Index of the desired row. size_t row_; }; diff --git a/src/lib/pgsql/tests/pgsql_basics.cc b/src/lib/pgsql/tests/pgsql_basics.cc index 1c4850adc9..c12326945b 100644 --- a/src/lib/pgsql/tests/pgsql_basics.cc +++ b/src/lib/pgsql/tests/pgsql_basics.cc @@ -143,5 +143,4 @@ PgSqlBasicsTest::fetchRows(PgSqlResultPtr& r, int exp_rows, int line) { ASSERT_EQ(r->getRows(), exp_rows) << "fetch at line: " << line << " wrong row count, expected: " << exp_rows << " , have: " << r->getRows(); - } diff --git a/src/lib/pgsql/tests/pgsql_basics.h b/src/lib/pgsql/tests/pgsql_basics.h index 02c5184f93..3165208e6e 100644 --- a/src/lib/pgsql/tests/pgsql_basics.h +++ b/src/lib/pgsql/tests/pgsql_basics.h @@ -153,8 +153,8 @@ public: #define FETCH_ROWS(a,b) (fetchRows(a,b,__LINE__)) #define WIPE_ROWS(a) (RUN_SQL(a, "DELETE FROM BASICS", PGRES_COMMAND_OK)) -}; -}; -}; +} +} +} #endif diff --git a/src/lib/pgsql/tests/pgsql_exchange_unittest.cc b/src/lib/pgsql/tests/pgsql_exchange_unittest.cc index 5166c0de51..54f5d180c9 100644 --- a/src/lib/pgsql/tests/pgsql_exchange_unittest.cc +++ b/src/lib/pgsql/tests/pgsql_exchange_unittest.cc @@ -1092,7 +1092,7 @@ TEST(PsqlBindArray, popBackTest) { /// @brief Verify that we can read and write IPv4 addresses /// using INET columns. TEST_F(PgSqlBasicsTest, inetTest4) { - // Create a prepared statement for inserting a SMALLINT + // Create a prepared statement for inserting an IPv4 address const char* st_name = "inet4_insert"; PgSqlTaggedStatement statement[] = { { 1, { OID_TEXT }, st_name, @@ -1101,7 +1101,7 @@ TEST_F(PgSqlBasicsTest, inetTest4) { "select * from BASICS where inet4_col = cast($1 as inet)" } }; - ASSERT_NO_THROW(conn_->prepareStatement(statement[0])); + ASSERT_NO_THROW_LOG(conn_->prepareStatement(statement[0])); ASSERT_NO_THROW_LOG(conn_->prepareStatement(statement[1])); // Build our reference list of reference values @@ -1161,14 +1161,17 @@ TEST_F(PgSqlBasicsTest, inetTest4) { /// @brief Verify that we can read and write IPv6 addresses /// using INET columns. TEST_F(PgSqlBasicsTest, inetTest6) { - // Create a prepared statement for inserting a SMALLINT + // Create a prepared statement for inserting an IPv6 address const char* st_name = "inet6_insert"; PgSqlTaggedStatement statement[] = { { 1, { OID_TEXT }, st_name, - "INSERT INTO BASICS (inet6_col) values (cast($1 as inet))" } + "INSERT INTO BASICS (inet6_col) values (cast($1 as inet))" }, + { 1, { OID_TEXT }, "check_where", + "select * from BASICS where inet6_col = cast($1 as inet)" } }; - ASSERT_NO_THROW(conn_->prepareStatement(statement[0])); + ASSERT_NO_THROW_LOG(conn_->prepareStatement(statement[0])); + ASSERT_NO_THROW_LOG(conn_->prepareStatement(statement[1])); // Build our reference list of reference values std::vectorinets; @@ -1189,16 +1192,25 @@ TEST_F(PgSqlBasicsTest, inetTest6) { // Iterate over the rows, verifying each value against its reference int row = 0; + asiolink::IOAddress fetched_inet("::"); for ( ; row < inets.size(); ++row ) { // Verify the column is not null. ASSERT_FALSE(PgSqlExchange::isColumnNull(*r, row, INET6_COL)); // Fetch and verify the column value - asiolink::IOAddress fetched_inet("::"); ASSERT_NO_THROW(fetched_inet = PgSqlExchange::getInetValue6(*r, row, INET6_COL)); EXPECT_EQ(fetched_inet, inets[row]); } + // Verify that casting from string to inet works in where clauses. + r.reset(); + bind_array.reset(new PsqlBindArray()); + bind_array->addInet6(inets[1]); + RUN_PREP(r, statement[1], bind_array, PGRES_TUPLES_OK); + ASSERT_EQ(r->getRows(),1); + ASSERT_NO_THROW(fetched_inet = PgSqlExchange::getInetValue6(*r, 0, INET6_COL)); + EXPECT_EQ(fetched_inet, inets[1]); + // Clean out the table WIPE_ROWS(r); @@ -1216,7 +1228,7 @@ TEST_F(PgSqlBasicsTest, inetTest6) { /// @brief Verify that we can read and write floats TEST_F(PgSqlBasicsTest, floatTest) { - // Create a prepared statement for inserting a SMALLINT + // Create a prepared statement for inserting a FLOAT const char* st_name = "float_insert"; PgSqlTaggedStatement statement[] = { { 1, { OID_TEXT }, st_name, @@ -1271,7 +1283,7 @@ TEST_F(PgSqlBasicsTest, floatTest) { /// @brief Verify that we can read and write JSON columns. TEST_F(PgSqlBasicsTest, jsonTest) { - // Create a prepared statement for inserting a SMALLINT + // Create a prepared statement for inserting a JSON const char* st_name = "json_insert"; PgSqlTaggedStatement statement[] = { { 1, { OID_TEXT }, st_name, @@ -1326,7 +1338,7 @@ TEST_F(PgSqlBasicsTest, jsonTest) { /// @brief Verify that we can read and write integer Triplets. TEST_F(PgSqlBasicsTest, tripleTest) { - // Create a prepared statement for inserting a SMALLINT + // Create a prepared statement for inserting a Triplet const char* st_name = "triplets_insert"; PgSqlTaggedStatement statement[] = { { 3, { OID_INT4, OID_INT4, OID_INT4 }, st_name, @@ -1373,7 +1385,7 @@ TEST_F(PgSqlBasicsTest, tripleTest) { // Now test making a triplet with all three columns. ASSERT_NO_THROW_LOG( fetched = PgSqlExchange::getTripletValue(*r, row, - INT_COL, MIN_INT_COL, MAX_INT_COL)); + INT_COL, MIN_INT_COL, MAX_INT_COL)); if (expected.unspecified()) { EXPECT_TRUE(fetched.unspecified()); } else { @@ -1392,7 +1404,7 @@ TEST_F(PgSqlBasicsTest, tripleTest) { /// @brief Verify PgResultRowWorker operations. TEST_F(PgSqlBasicsTest, resultRowWorker) { - // Create a prepared statement for inserting a SMALLINT + // Create a prepared statement for inserting multiple types const char* st_name = "row_insert"; PgSqlTaggedStatement statement[] = { { 14, @@ -1426,7 +1438,7 @@ TEST_F(PgSqlBasicsTest, resultRowWorker) { " min_int_col, " " max_int_col, " " inet6_col) " - " VALUES ( $1, $2, $3, $4, $5, $6, $7, $8, cast($9 as inet), " + " VALUES ($1, $2, $3, $4, $5, $6, $7, $8, cast($9 as inet), " " cast($10 as float), cast($11 as json), $12, $13, $14)" } }; @@ -1439,7 +1451,7 @@ TEST_F(PgSqlBasicsTest, resultRowWorker) { bool exp_bool(true); b->add(exp_bool); - std::vector exp_bytes({ 0x01, 0x02, 0x03, 0x04}); + std::vector exp_bytes({0x01, 0x02, 0x03, 0x04}); b->add(exp_bytes); uint64_t exp_bigint = 9876; @@ -1458,7 +1470,7 @@ TEST_F(PgSqlBasicsTest, resultRowWorker) { ptime exp_timestamp(date(2021, Jul, 18), duration); b->addTimestamp(exp_timestamp); - std::string exp_varchar = "really just a string"; + const char* exp_varchar = "really just a string"; b->add(exp_varchar); asiolink::IOAddress exp_inet4("192.168.1.35"); @@ -1486,7 +1498,7 @@ TEST_F(PgSqlBasicsTest, resultRowWorker) { FETCH_ROWS(r, 1); // Create a row worker. - PgSqlResultRowWorkerPtr worker; + PgSqlResultRowWorkerPtr worker; // Creating the row worker for the first (and only) row should succeed. ASSERT_NO_THROW_LOG(worker.reset(new PgSqlResultRowWorker(*r, 0))); @@ -1512,7 +1524,7 @@ TEST_F(PgSqlBasicsTest, resultRowWorker) { EXPECT_EQ(exp_inet6, worker->getInet6(INET6_COL)); // Get a triplet using int_col as the sole value. - Tripletfetched_triplet = worker->getTriplet(5); + Tripletfetched_triplet = worker->getTriplet(INT_COL); EXPECT_EQ(exp_int, fetched_triplet.get()); // Get a triplet using int_col, min_col, and max_col values. @@ -1526,5 +1538,4 @@ TEST_F(PgSqlBasicsTest, resultRowWorker) { DbOperationError, "row: 1, out of range: 0..1"); } - -}; // namespace +} // namespace