From: Razvan Becheriu Date: Wed, 16 Feb 2022 18:18:25 +0000 (+0200) Subject: [#95] fixed comments and indentation X-Git-Tag: Kea-2.1.3~12 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=267af65edb8535730eda79747b163f9319fc8aac;p=thirdparty%2Fkea.git [#95] fixed comments and indentation --- diff --git a/src/bin/admin/tests/pgsql_tests.sh.in b/src/bin/admin/tests/pgsql_tests.sh.in index ae92bb8b66..54341c26fc 100644 --- a/src/bin/admin/tests/pgsql_tests.sh.in +++ b/src/bin/admin/tests/pgsql_tests.sh.in @@ -468,7 +468,7 @@ pgsql_upgrade_test() { # Check 7.0 to 8.0 upgrade pgsql_upgrade_7_0_to_8_0 - # Check 7.0 to 8.0 upgrade + # Check 8.0 to 9.0 upgrade pgsql_upgrade_8_0_to_9_0 # Let's wipe the whole database diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc index 51412e012e..81d975bd38 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc @@ -193,8 +193,8 @@ public: const std::string& name) { StampedValueCollection parameters; - auto tags = server_selector.getTags(); - for (auto tag : tags) { + auto const& tags = server_selector.getTags(); + for (auto const& tag : tags) { MySqlBindingCollection in_bindings = { MySqlBinding::createString(tag.get()), MySqlBinding::createString(name) @@ -679,7 +679,7 @@ public: last_subnet->addPool(last_pool); } - // Parse pool specific option from 25 to 36. + // Parse pool-specific option from 25 to 36. if (last_pool && !out_bindings[25]->amNull() && (last_pool_option_id < out_bindings[25]->getInteger())) { last_pool_option_id = out_bindings[25]->getInteger(); @@ -690,7 +690,7 @@ public: } } - // Parse subnet specific option from 37 to 48. + // Parse subnet-specific option from 37 to 48. if (!out_bindings[37]->amNull() && (last_option_id < out_bindings[37]->getInteger())) { last_option_id = out_bindings[37]->getInteger(); @@ -729,10 +729,8 @@ public: MySqlBindingCollection in_bindings = { MySqlBinding::createInteger(subnet_id) }; auto index = GET_SUBNET4_ID_NO_TAG; - if (server_selector.amUnassigned()) { index = GET_SUBNET4_ID_UNASSIGNED; - } else if (server_selector.amAny()) { index = GET_SUBNET4_ID_ANY; } @@ -763,10 +761,8 @@ public: MySqlBindingCollection in_bindings = { MySqlBinding::createString(subnet_prefix) }; auto index = GET_SUBNET4_PREFIX_NO_TAG; - if (server_selector.amUnassigned()) { index = GET_SUBNET4_PREFIX_UNASSIGNED; - } else if (server_selector.amAny()) { index = GET_SUBNET4_PREFIX_ANY; } @@ -958,10 +954,9 @@ public: MySqlBinding::createInteger(pool_end_address.toUint32()) }; getPools(GET_POOL4_RANGE_ANY, in_bindings, pools, pool_ids); - } else { - auto tags = server_selector.getTags(); - for (auto tag : tags) { + auto const& tags = server_selector.getTags(); + for (auto const& tag : tags) { MySqlBindingCollection in_bindings = { MySqlBinding::createString(tag.get()), MySqlBinding::createInteger(pool_start_address.toUint32()), @@ -1622,10 +1617,8 @@ public: MySqlBindingCollection in_bindings = { MySqlBinding::createString(name) }; auto index = GET_SHARED_NETWORK4_NAME_NO_TAG; - if (server_selector.amUnassigned()) { index = GET_SHARED_NETWORK4_NAME_UNASSIGNED; - } else if (server_selector.amAny()) { index = GET_SHARED_NETWORK4_NAME_ANY; } @@ -3846,8 +3839,8 @@ StampedValueCollection MySqlConfigBackendDHCPv4::getAllGlobalParameters4(const ServerSelector& server_selector) const { LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_GET_ALL_GLOBAL_PARAMETERS4); StampedValueCollection parameters; - auto tags = server_selector.getTags(); - for (auto tag : tags) { + auto const& tags = server_selector.getTags(); + for (auto const& tag : tags) { MySqlBindingCollection in_bindings = { MySqlBinding::createString(tag.get()) }; impl_->getGlobalParameters(MySqlConfigBackendDHCPv4Impl::GET_ALL_GLOBAL_PARAMETERS4, in_bindings, parameters); @@ -3863,8 +3856,8 @@ MySqlConfigBackendDHCPv4::getModifiedGlobalParameters4(const db::ServerSelector& LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_GET_MODIFIED_GLOBAL_PARAMETERS4) .arg(util::ptimeToText(modification_time)); StampedValueCollection parameters; - auto tags = server_selector.getTags(); - for (auto tag : tags) { + auto const& tags = server_selector.getTags(); + for (auto const& tag : tags) { MySqlBindingCollection in_bindings = { MySqlBinding::createString(tag.get()), MySqlBinding::createTimestamp(modification_time) diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc index 25d50048e2..f4a05637f3 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc @@ -178,7 +178,7 @@ MySqlConfigBackendImpl::createAuditRevision(const int index, /// audit entry is when there is a single server tag, i.e. "all" or explicit /// server name. In fact, these are the most common two cases. std::string tag = ServerTag::ALL; - auto tags = server_selector.getTags(); + auto const& tags = server_selector.getTags(); if (tags.size() == 1) { tag = tags.begin()->get(); } @@ -217,9 +217,8 @@ MySqlConfigBackendImpl::getRecentAuditEntries(const int index, MySqlBinding::createString(AUDIT_ENTRY_LOG_MESSAGE_BUF_LENGTH) // log_message }; - auto tags = server_selector.getTags(); - - for (auto tag : tags) { + auto const& tags = server_selector.getTags(); + for (auto const& tag : tags) { // There are only a few input bindings MySqlBindingCollection in_bindings = { @@ -313,6 +312,7 @@ MySqlConfigBackendImpl::getGlobalParameters(const int index, // server_tag ServerTag last_param_server_tag(out_bindings[5]->getString()); last_param->setServerTag(last_param_server_tag.get()); + // If we're fetching parameters for a given server (explicit server // tag is provided), it takes precedence over the same parameter // specified for all servers. Therefore, we check if the given @@ -371,10 +371,10 @@ MySqlConfigBackendImpl::getOptionDef(const int index, void MySqlConfigBackendImpl::getAllOptionDefs(const int index, - const ServerSelector& server_selector, - OptionDefContainer& option_defs) { - auto tags = server_selector.getTags(); - for (auto tag : tags) { + const ServerSelector& server_selector, + OptionDefContainer& option_defs) { + auto const& tags = server_selector.getTags(); + for (auto const& tag : tags) { MySqlBindingCollection in_bindings = { MySqlBinding::createString(tag.get()) }; @@ -387,8 +387,8 @@ MySqlConfigBackendImpl::getModifiedOptionDefs(const int index, const ServerSelector& server_selector, const boost::posix_time::ptime& modification_time, OptionDefContainer& option_defs) { - auto tags = server_selector.getTags(); - for (auto tag : tags) { + auto const& tags = server_selector.getTags(); + for (auto const& tag : tags) { MySqlBindingCollection in_bindings = { MySqlBinding::createString(tag.get()), MySqlBinding::createTimestamp(modification_time) @@ -592,8 +592,8 @@ MySqlConfigBackendImpl::getAllOptions(const int index, const ServerSelector& server_selector) { OptionContainer options; - auto tags = server_selector.getTags(); - for (auto tag : tags) { + auto const& tags = server_selector.getTags(); + for (auto const& tag : tags) { MySqlBindingCollection in_bindings = { MySqlBinding::createString(tag.get()) }; @@ -610,8 +610,8 @@ MySqlConfigBackendImpl::getModifiedOptions(const int index, const boost::posix_time::ptime& modification_time) { OptionContainer options; - auto tags = server_selector.getTags(); - for (auto tag : tags) { + auto const& tags = server_selector.getTags(); + for (auto const& tag : tags) { MySqlBindingCollection in_bindings = { MySqlBinding::createString(tag.get()), MySqlBinding::createTimestamp(modification_time) @@ -908,6 +908,7 @@ MySqlConfigBackendImpl::processOptionDefRow(MySqlBindingCollection::iterator fir isc_throw(BadValue, "invalid record_types value " << (*(first_binding + 8))->getString()); } + // This element must contain a list of integers specifying // types of the record fields. for (auto i = 0; i < record_types_element->size(); ++i) { @@ -915,6 +916,7 @@ MySqlConfigBackendImpl::processOptionDefRow(MySqlBindingCollection::iterator fir if (type_element->getType() != Element::integer) { isc_throw(BadValue, "record type values must be integers"); } + def->addRecordField(static_cast(type_element->intValue())); } } @@ -929,10 +931,10 @@ void MySqlConfigBackendImpl::attachElementToServers(const int index, const ServerSelector& server_selector, const MySqlBindingPtr& first_binding, - const MySqlBindingPtr& in_bindings...) { + const MySqlBindingPtr& in_bindings) { // Create the vector from the parameter pack. MySqlBindingCollection in_server_bindings = { first_binding, in_bindings }; - for (auto tag : server_selector.getTags()) { + for (auto const& tag : server_selector.getTags()) { in_server_bindings.push_back(MySqlBinding::createString(tag.get())); // Handles the case where the server does not exists. try { @@ -1035,7 +1037,8 @@ MySqlConfigBackendImpl::createUpdateServer(const int& create_audit_revision, const ServerPtr& server) { // The server tag 'all' is reserved. if (server->getServerTag().amAll()) { - isc_throw(InvalidOperation, "'all' is a name reserved for the server tag which" + isc_throw(InvalidOperation, + "'all' is a name reserved for the server tag which" " associates the configuration elements with all servers connecting" " to the database and a server with this name may not be created"); } diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h index b34e23fbe3..0c332e636e 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h @@ -196,7 +196,7 @@ public: /// is more than one server tag associated with the selector. std::string getServerTag(const db::ServerSelector& server_selector, const std::string& operation) const { - auto tags = server_selector.getTags(); + auto const& tags = server_selector.getTags(); if (tags.size() != 1) { isc_throw(InvalidOperation, "expected exactly one server tag to be specified" " while " << operation << ". Got: " @@ -212,8 +212,8 @@ public: /// This method is useful for logging purposes. std::string getServerTagsAsText(const db::ServerSelector& server_selector) const { std::ostringstream s; - auto server_tags = server_selector.getTags(); - for (auto tag : server_tags) { + auto const& server_tags = server_selector.getTags(); + for (auto const& tag : server_tags) { if (s.tellp() != 0) { s << ", "; } @@ -447,10 +447,11 @@ public: /// /// @return Pointer to the returned option or NULL if such option /// doesn't exist. - OptionDescriptorPtr - getOption(const int index, const Option::Universe& universe, - const db::ServerSelector& server_selector, const uint16_t code, - const std::string& space); + OptionDescriptorPtr getOption(const int index, + const Option::Universe& universe, + const db::ServerSelector& server_selector, + const uint16_t code, + const std::string& space); /// @brief Sends query to retrieve all global options. /// @@ -458,9 +459,9 @@ public: /// @param universe Option universe, i.e. V4 or V6. /// @param server_selector Server selector. /// @return Container holding returned options. - OptionContainer - getAllOptions(const int index, const Option::Universe& universe, - const db::ServerSelector& server_selector); + OptionContainer getAllOptions(const int index, + const Option::Universe& universe, + const db::ServerSelector& server_selector); /// @brief Sends query to retrieve global options with modification /// time later than specified timestamp. @@ -469,10 +470,10 @@ public: /// @param universe Option universe, i.e. V4 or V6. /// @param server_selector Server selector. /// @return Container holding returned options. - OptionContainer - getModifiedOptions(const int index, const Option::Universe& universe, - const db::ServerSelector& server_selector, - const boost::posix_time::ptime& modification_time); + OptionContainer getModifiedOptions(const int index, + const Option::Universe& universe, + const db::ServerSelector& server_selector, + const boost::posix_time::ptime& modification_time); /// @brief Sends query to retrieve single option by code and option space /// for a given subnet id. @@ -574,9 +575,8 @@ public: /// @param universe V4 or V6. /// @param first_binding Iterator of the output binding containing /// option_id. - OptionDescriptorPtr - processOptionRow(const Option::Universe& universe, - db::MySqlBindingCollection::iterator first_binding); + OptionDescriptorPtr processOptionRow(const Option::Universe& universe, + db::MySqlBindingCollection::iterator first_binding); /// @brief Returns DHCP option definition instance from output bindings. /// @@ -610,7 +610,7 @@ public: void attachElementToServers(const int index, const db::ServerSelector& server_selector, const db::MySqlBindingPtr& first_binding, - const db::MySqlBindingPtr& in_bindings...); + const db::MySqlBindingPtr& in_bindings); /// @brief Creates input binding for relay addresses. /// @@ -759,7 +759,7 @@ public: } // Go over the collection of elements. - for (auto elem = index.begin(); elem != index.end(); ) { + for (auto elem = index.begin(); elem != index.end();) { // If we're asking for shared networks matching all servers, // we have to make sure that the fetched element has "all" @@ -783,9 +783,9 @@ public: // Server selector contains explicit server tags, so // let's see if the returned elements includes any of // them. - auto tags = server_selector.getTags(); + auto const& tags = server_selector.getTags(); bool tag_found = false; - for (auto tag : tags) { + for (auto const& tag : tags) { if ((*elem)->hasServerTag(tag) || (*elem)->hasAllServerTag()) { tag_found = true; @@ -836,7 +836,7 @@ public: return (parameters_); } - /// @brief Sets IO service to be used by the MySql config backend. + /// @brief Sets IO service to be used by the MySQL config backend. /// /// @param IOService object, used for all ASIO operations. static void setIOService(const isc::asiolink::IOServicePtr& io_service) { @@ -864,7 +864,7 @@ private: /// @brief Connection parameters isc::db::DatabaseConnection::ParameterMap parameters_; - /// The IOService object, used for all ASIO operations. + /// @brief The IOService object, used for all ASIO operations. static isc::asiolink::IOServicePtr io_service_; }; diff --git a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc index 1957f4ebb8..6afd5a9f68 100644 --- a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc +++ b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc @@ -75,7 +75,7 @@ public: destroyMySQLSchema(); } - /// @brief Returns a valid PostgreSQL back end specific connection + /// @brief Returns a valid MySQL back end specific connection /// string std::string validConnectionString() { return (validMySQLConnectionString()); diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc b/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc index 644a4f1be6..8a7e7b9a39 100644 --- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc +++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc @@ -199,6 +199,7 @@ public: StampedValuePtr getGlobalParameter4(const ServerSelector& server_selector, const std::string& name) { StampedValueCollection parameters; + auto const& tags = server_selector.getTags(); for (auto const& tag : tags) { PsqlBindArray in_bindings; @@ -217,6 +218,7 @@ public: /// @param value StampedValue describing the parameter to create/update. void createUpdateGlobalParameter4(const db::ServerSelector& server_selector, const StampedValuePtr& value) { + if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" " (unassigned) is unsupported at the moment"); @@ -258,7 +260,7 @@ public: PsqlBindArray attach_bindings; uint64_t pid = getLastInsertId("dhcp4_global_parameter", "id"); attach_bindings.add(pid); // id of newly inserted global. - attach_bindings.add(value->getModificationTime()); + attach_bindings.addTimestamp(value->getModificationTime()); attachElementToServers(PgSqlConfigBackendDHCPv4Impl::INSERT_GLOBAL_PARAMETER4_SERVER, server_selector, attach_bindings); } @@ -301,7 +303,7 @@ public: last_subnet = *subnets.rbegin(); } // Subnet_id is column 0. - SubnetID subnet_id = worker.getBigInt(0) ; + SubnetID subnet_id = worker.getInt(0) ; // Subnet has been returned. Assuming that subnets are ordered by // subnet identifier, if the subnet identifier of the current row @@ -347,9 +349,9 @@ public: // 4o6_interface_id at 3. if (!worker.isColumnNull(3)) { - std::string interface_id_str = worker.getString(3); - OptionBuffer dhcp4o6_interface_id_buf(interface_id_str.begin(), - interface_id_str.end()); + std::string dhcp4o6_interface_id = worker.getString(3); + OptionBuffer dhcp4o6_interface_id_buf(dhcp4o6_interface_id.begin(), + dhcp4o6_interface_id.end()); OptionPtr option_dhcp4o6_interface_id = Option::create(Option::V6, D6O_INTERFACE_ID, dhcp4o6_interface_id_buf); last_subnet->get4o6().setInterfaceId(option_dhcp4o6_interface_id); @@ -357,9 +359,8 @@ public: // 4o6_subnet at 4. if (!worker.isColumnNull(4)) { - std::string dhcp4o6_prefix_str = worker.getString(4); std::pair dhcp4o6_subnet_prefix_pair = - Subnet6::parsePrefix(dhcp4o6_prefix_str); + Subnet6::parsePrefix(worker.getString(4)); last_subnet->get4o6().setSubnet4o6(dhcp4o6_subnet_prefix_pair.first, dhcp4o6_subnet_prefix_pair.second); } @@ -599,7 +600,6 @@ public: tossNonMatchingElements(server_selector, subnet_index); } - /// @brief Sends query to retrieve single subnet by id. /// /// @param server_selector Server selector. @@ -609,6 +609,7 @@ public: /// doesn't exist. Subnet4Ptr getSubnet4(const ServerSelector& server_selector, const SubnetID& subnet_id) { + if (server_selector.hasMultipleTags()) { isc_throw(InvalidOperation, "expected one server tag to be specified" " while fetching a subnet. Got: " @@ -703,7 +704,6 @@ public: auto index = (server_selector.amUnassigned() ? GET_MODIFIED_SUBNETS4_UNASSIGNED : GET_MODIFIED_SUBNETS4); getSubnets4(index, server_selector, in_bindings, subnets); - } /// @brief Sends query to retrieve all subnets belonging to a shared network. @@ -754,6 +754,7 @@ public: // pool start_address (1) // pool end_address (2) last_pool_id = id; + last_pool = Pool4::create(worker.getInet4(1), worker.getInet4(2)); // pool subnet_id (3) (ignored) @@ -818,7 +819,7 @@ public: auto const& tags = server_selector.getTags(); for (auto const& tag : tags) { PsqlBindArray in_bindings; - in_bindings.add(tag.get()); + in_bindings.addTempString(tag.get()); in_bindings.addInet4(pool_start_address); in_bindings.addInet4(pool_end_address); @@ -835,6 +836,7 @@ public: } pool_id = 0; + return (Pool4Ptr()); } @@ -844,6 +846,7 @@ public: /// @param subnet Pointer to the subnet to be inserted or updated. void createUpdateSubnet4(const ServerSelector& server_selector, const Subnet4Ptr& subnet) { + if (server_selector.amAny()) { isc_throw(InvalidOperation, "creating or updating a subnet for ANY" " server is not supported"); @@ -864,6 +867,7 @@ public: if (dhcp4o6_interface_id) { in_bindings.addTempString(std::string(dhcp4o6_interface_id->getData().begin(), dhcp4o6_interface_id->getData().end())); + } else { in_bindings.addNull(); } @@ -986,13 +990,14 @@ public: // Insert associations with the servers. PsqlBindArray attach_bindings; attach_bindings.add(subnet->getID()); - attach_bindings.add(subnet->getModificationTime()); + attach_bindings.addTimestamp(subnet->getModificationTime()); attachElementToServers(PgSqlConfigBackendDHCPv4Impl::INSERT_SUBNET4_SERVER, server_selector, attach_bindings); // (Re)create pools. for (auto pool : subnet->getPools(Lease::TYPE_V4)) { - createPool4(server_selector, boost::dynamic_pointer_cast(pool), subnet); + createPool4(server_selector, boost::dynamic_pointer_cast(pool), + subnet); } // (Re)create options. @@ -1002,7 +1007,8 @@ public: for (auto desc = options->begin(); desc != options->end(); ++desc) { OptionDescriptorPtr desc_copy = OptionDescriptor::create(*desc); desc_copy->space_name_ = option_space; - createUpdateOption4(server_selector, subnet->getID(), desc_copy, true); + createUpdateOption4(server_selector, subnet->getID(), desc_copy, + true); } } @@ -1188,7 +1194,7 @@ public: last_network->allowClientClass(worker.getString(2)); } - // Interface at 3. + // interface at 3. if (!worker.isColumnNull(3)) { last_network->setIface(worker.getString(3)); } @@ -1381,6 +1387,7 @@ public: /// network doesn't exist. SharedNetwork4Ptr getSharedNetwork4(const ServerSelector& server_selector, const std::string& name) { + if (server_selector.hasMultipleTags()) { isc_throw(InvalidOperation, "expected one server tag to be specified" " while fetching a shared network. Got: " @@ -1410,7 +1417,7 @@ public: /// structure where shared networks should be inserted. void getAllSharedNetworks4(const ServerSelector& server_selector, SharedNetwork4Collection& shared_networks) { - if (server_selector.amAny()) { + if (server_selector.amAny()) { isc_throw(InvalidOperation, "fetching all shared networks for ANY " "server is not supported"); } @@ -1443,7 +1450,6 @@ public: auto index = (server_selector.amUnassigned() ? PgSqlConfigBackendDHCPv4Impl::GET_MODIFIED_SHARED_NETWORKS4_UNASSIGNED : PgSqlConfigBackendDHCPv4Impl::GET_MODIFIED_SHARED_NETWORKS4); - getSharedNetworks4(index, server_selector, in_bindings, shared_networks); } @@ -1453,6 +1459,7 @@ public: /// @param subnet Pointer to the shared network to be inserted or updated. void createUpdateSharedNetwork4(const ServerSelector& server_selector, const SharedNetwork4Ptr& shared_network) { + if (server_selector.amAny()) { isc_throw(InvalidOperation, "creating or updating a shared network for ANY" " server is not supported"); @@ -1542,7 +1549,7 @@ public: // Associate the shared network with the servers. PsqlBindArray attach_bindings; attach_bindings.addTempString(shared_network->getName()); - attach_bindings.add(shared_network->getModificationTime()); + attach_bindings.addTimestamp(shared_network->getModificationTime()); attachElementToServers(PgSqlConfigBackendDHCPv4Impl::INSERT_SHARED_NETWORK4_SERVER, server_selector, attach_bindings); @@ -1582,7 +1589,7 @@ public: PsqlBindArray attach_bindings; attach_bindings.add(option_id); // id of newly inserted global. - attach_bindings.add(modification_ts); + attach_bindings.addTimestamp(modification_ts); // Associate the option with the servers. attachElementToServers(PgSqlConfigBackendDHCPv4Impl::INSERT_OPTION4_SERVER, @@ -1595,6 +1602,7 @@ public: /// @param option Pointer to the option descriptor encapsulating the option. void createUpdateOption4(const ServerSelector& server_selector, const OptionDescriptorPtr& option) { + if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" " (unassigned) is unsupported at the moment"); @@ -1663,6 +1671,7 @@ public: const SubnetID& subnet_id, const OptionDescriptorPtr& option, const bool cascade_update) { + if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" " (unassigned) is unsupported at the moment"); @@ -1752,6 +1761,7 @@ public: const uint64_t pool_id, const OptionDescriptorPtr& option, const bool cascade_update) { + if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" " (unassigned) is unsupported at the moment"); @@ -1819,6 +1829,7 @@ public: const std::string& shared_network_name, const OptionDescriptorPtr& option, const bool cascade_update) { + if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" " (unassigned) is unsupported at the moment"); @@ -1891,6 +1902,7 @@ public: /// @param option_def Pointer to the option definition to be inserted or updated. void createUpdateOptionDef4(const ServerSelector& server_selector, const OptionDefinitionPtr& option_def) { + createUpdateOptionDef(server_selector, option_def, DHCP4_OPTION_SPACE, PgSqlConfigBackendDHCPv4Impl::GET_OPTION_DEF4_CODE_SPACE, PgSqlConfigBackendDHCPv4Impl::INSERT_OPTION_DEF4, @@ -2094,7 +2106,6 @@ public: "deleting options for a shared network", "shared network specific options deleted", true, in_bindings)); - } /// @brief Deletes options belonging to a client class from the database. diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc index 5e67bf744a..9cf8f624a2 100644 --- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc +++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc @@ -138,7 +138,7 @@ PgSqlConfigBackendImpl::createAuditRevision(const int index, /// audit entry is when there is a single server tag, i.e. "all" or explicit /// server name. In fact, these are the most common two cases. std::string tag = ServerTag::ALL; - const auto& tags = server_selector.getTags(); + auto const& tags = server_selector.getTags(); if (tags.size() == 1) { tag = tags.begin()->get(); } @@ -157,7 +157,6 @@ PgSqlConfigBackendImpl::clearAuditRevision() { if (audit_revision_ref_count_ <= 0) { isc_throw(Unexpected, "attempted to clear audit revision that does not exist - coding error"); } - --audit_revision_ref_count_; } @@ -345,6 +344,7 @@ PgSqlConfigBackendImpl::getOptionDef(const int index, const ServerSelector& server_selector, const uint16_t code, const std::string& space) { + if (server_selector.amUnassigned()) { isc_throw(NotImplemented, "managing configuration for no particular server" " (unassigned) is unsupported at the moment"); @@ -391,8 +391,9 @@ PgSqlConfigBackendImpl::getModifiedOptionDefs(const int index, void PgSqlConfigBackendImpl::getOptionDefs(const int index, const PsqlBindArray& in_bindings, - OptionDefContainer& option_defs ) { + OptionDefContainer& option_defs) { uint64_t last_def_id = 0; + OptionDefContainer local_option_defs; // Run select query. @@ -402,7 +403,6 @@ PgSqlConfigBackendImpl::getOptionDefs(const int index, // Create a worker for the row. PgSqlResultRowWorker worker(r, row); - // Get pointer to last fetched option definition. OptionDefinitionPtr last_def; if (!local_option_defs.empty()) { @@ -473,9 +473,10 @@ PgSqlConfigBackendImpl::createUpdateOptionDef(const db::ServerSelector& server_s const int& create_audit_revision, const int& insert_option_def_server, const std::string& client_class_name) { + 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"); @@ -563,7 +564,7 @@ PgSqlConfigBackendImpl::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"); @@ -600,7 +601,6 @@ PgSqlConfigBackendImpl::getModifiedOptions(const int index, const Option::Universe& universe, const ServerSelector& server_selector, const boost::posix_time::ptime& modification_time) { - OptionContainer options; PsqlBindArray in_bindings; @@ -1011,7 +1011,7 @@ PgSqlConfigBackendImpl::attachElementToServers(const int index, const PsqlBindArray& in_bindings) { // Copy the bindings because we're going to modify them. PsqlBindArray server_bindings = in_bindings; - for (auto tag : server_selector.getTags()) { + for (auto const& tag : server_selector.getTags()) { // Add the server tag to end of the bindings. std::string server_tag = tag.get(); server_bindings.add(server_tag); @@ -1056,7 +1056,7 @@ PgSqlConfigBackendImpl::setRelays(PgSqlResultRowWorker& worker, size_t col, Netw "be valid strings"); } - network.addRelayAddress(IOAddress(relay_element->get(i)->stringValue())); + network.addRelayAddress(IOAddress(relay_address_element->stringValue())); } } diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.h b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.h index 7983f3e8f5..fc436488d3 100644 --- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.h +++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.h @@ -37,12 +37,13 @@ namespace dhcp { /// @brief Base class for PostgreSQL Config Backend implementations. /// /// This class contains common methods for manipulating data in the -/// Postgres database, used by all servers. +/// PostgreSQL database, used by all servers. /// /// All POSIX times specified in the methods belonging to this /// class must be local times. class PgSqlConfigBackendImpl { protected: + /// @brief RAII object used to protect against creating multiple /// audit revisions during cascade configuration updates. /// @@ -68,13 +69,14 @@ protected: /// transaction with the database. class ScopedAuditRevision { public: + /// @brief Constructor. /// /// Creates new audit revision and sets the flag in the - /// Postgres CB implementation object which prevents new audit + /// PostgreSQL CB implementation object which prevents new audit /// revisions to be created while this instance exists. /// - /// @param impl pointer to the Postgres CB implementation. + /// @param impl pointer to the PostgreSQL CB implementation. /// @param index index of the query to set session variables /// used for creation of the audit revision and the audit /// entries. @@ -98,11 +100,13 @@ protected: ~ScopedAuditRevision(); private: - /// @brief Pointer to the Postgres CB implementation. + + /// @brief Pointer to the PostgreSQL CB implementation. PgSqlConfigBackendImpl* impl_; }; public: + /// @brief Constructor. /// /// @param parameters A data structure relating keywords and values @@ -112,7 +116,7 @@ public: /// use when fetching the last insert id for a given table. explicit PgSqlConfigBackendImpl(const db::DatabaseConnection::ParameterMap& parameters, const db::DbCallback db_reconnect_callback, - const size_t last_insert_id_index_); + const size_t last_insert_id_index); /// @brief Destructor. virtual ~PgSqlConfigBackendImpl(); @@ -130,14 +134,13 @@ public: /// @return Server tag. /// @throw InvalidOperation if the server selector is unassigned or if there /// is more than one server tag associated with the selector. - std::string - getServerTag(const db::ServerSelector& server_selector, const std::string& operation) const { - auto tags = server_selector.getTags(); + std::string getServerTag(const db::ServerSelector& server_selector, + const std::string& operation) const { + auto const& tags = server_selector.getTags(); if (tags.size() != 1) { isc_throw(InvalidOperation, "expected exactly one server tag to be specified" - " while " - << operation - << ". Got: " << getServerTagsAsText(server_selector)); + " while " << operation << ". Got: " + << getServerTagsAsText(server_selector)); } return (tags.begin()->get()); @@ -149,8 +152,8 @@ public: /// This method is useful for logging purposes. std::string getServerTagsAsText(const db::ServerSelector& server_selector) const { std::ostringstream s; - auto server_tags = server_selector.getTags(); - for (auto tag : server_tags) { + auto const& server_tags = server_selector.getTags(); + for (auto const& tag : server_tags) { if (s.tellp() != 0) { s << ", "; } @@ -599,7 +602,7 @@ public: } /// @brief Iterates over the class names in a JSON list element at a - /// given column, invoking a setter function each one. + /// given column, invoking a setter function for each one. /// /// Has no effect if the column is null or is an empty list. /// @@ -642,8 +645,9 @@ public: /// @param bindings Reference to the PgSQL input bindings. /// @param [out] servers Reference to the container where fetched servers /// will be inserted. - void - getServers(const int index, const db::PsqlBindArray& bindings, db::ServerCollection& servers); + void getServers(const int index, + const db::PsqlBindArray& bindings, + db::ServerCollection& servers); /// @brief Creates or updates a server. /// @@ -703,9 +707,9 @@ public: /// @param server_selector Server selector. /// @param index Reference to the index holding the returned configuration /// elements to be processed. - template - void - tossNonMatchingElements(const db::ServerSelector& server_selector, CollectionIndex& index) { + template + void tossNonMatchingElements(const db::ServerSelector& server_selector, + CollectionIndex& index) { // Don't filter the matching server tags if the server selector is // set to ANY. if (server_selector.amAny()) { @@ -737,10 +741,11 @@ public: // Server selector contains explicit server tags, so // let's see if the returned elements includes any of // them. - auto tags = server_selector.getTags(); + auto const& tags = server_selector.getTags(); bool tag_found = false; - for (auto tag : tags) { - if ((*elem)->hasServerTag(tag) || (*elem)->hasAllServerTag()) { + for (auto const& tag : tags) { + if ((*elem)->hasServerTag(tag) || + (*elem)->hasAllServerTag()) { tag_found = true; break; } @@ -761,7 +766,7 @@ public: /// @brief Returns backend type in the textual format. /// - /// @return "pgsql". + /// @return "postgresql". std::string getType() const; /// @brief Returns backend host. @@ -789,7 +794,7 @@ public: return (parameters_); } - /// @brief Sets IO service to be used by the Postgres config backend. + /// @brief Sets IO service to be used by the PostgreSQL config backend. /// /// @param IOService object, used for all ASIO operations. static void setIOService(const isc::asiolink::IOServicePtr& io_service) { @@ -861,14 +866,16 @@ public: /// @return Number of affected rows. uint64_t updateDeleteQuery(size_t index, const db::PsqlBindArray& in_bindings); - /// @brief Represents connection to the Postgres database. + /// @brief Represents connection to the PostgreSQL database. db::PgSqlConnection conn_; protected: + /// @brief Timer name used to register database reconnect timer. std::string timer_name_; private: + /// @brief Reference counter for @ScopedAuditRevision instances. int audit_revision_ref_count_; diff --git a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc index e5b67a897d..31b3396248 100644 --- a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc +++ b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc @@ -467,7 +467,7 @@ GenericConfigBackendDHCPv4Test::testNewAuditEntry(const std::string& exp_object_ // Server tag is 'all'. tag = "all"; } else { - auto tags = server_selector.getTags(); + const auto& tags = server_selector.getTags(); // This test is not meant to handle multiple server tags all at once. if (tags.size() > 1) { ADD_FAILURE() << "Test error: do not use multiple server tags"; @@ -515,7 +515,7 @@ GenericConfigBackendDHCPv4Test::testNewAuditEntry(const std::vector 1) { ADD_FAILURE() << "Test error: do not use multiple server tags"; @@ -1355,7 +1355,7 @@ GenericConfigBackendDHCPv4Test::getSubnet4SharedNetworkTest() { // Store shared network in the database. ASSERT_NO_THROW_LOG(cbptr_->createUpdateSharedNetwork4(ServerSelector::ALL(), - shared_network)); + shared_network)); // Store subnet associated with the shared network in the database. ASSERT_NO_THROW_LOG(cbptr_->createUpdateSubnet4(ServerSelector::ALL(), subnet)); @@ -2484,7 +2484,7 @@ GenericConfigBackendDHCPv4Test::getAllSharedNetworks4Test() { }, { "dhcp4_subnet", - AuditEntry::ModificationType::UPDATE, "deleted all shared networks" + AuditEntry::ModificationType::UPDATE, "deleted all shared networks" } }); diff --git a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.h b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.h index 934ef68655..d15a5291cc 100644 --- a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.h +++ b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.h @@ -145,7 +145,7 @@ public: /// checks that number and content of the expected new entries have been /// added to the end of this collection. /// - /// @param exp_entries a list of the new audit entries expected. + /// @param exp_entries a list of the expected new audit entries. /// @param server_selector Server selector to be used for next query. void testNewAuditEntry(const std::vector& exp_entries, const db::ServerSelector& server_selector); diff --git a/src/lib/pgsql/pgsql_exchange.cc b/src/lib/pgsql/pgsql_exchange.cc index d531f41299..3451b3a070 100644 --- a/src/lib/pgsql/pgsql_exchange.cc +++ b/src/lib/pgsql/pgsql_exchange.cc @@ -62,6 +62,7 @@ void PsqlBindArray::insert(const std::string& value, size_t index) { } bound_strs_.push_back(ConstStringPtr(new std::string(value))); + values_.insert(values_.begin() + index, bound_strs_.back()->c_str()); lengths_.insert(lengths_.begin() + index, value.size()); formats_.insert(formats_.begin() + index, TEXT_FMT); @@ -85,7 +86,7 @@ void PsqlBindArray::add(const std::vector& data) { void PsqlBindArray::addTempBinary(const std::vector& data) { bound_strs_.push_back(ConstStringPtr(new std::string( - (reinterpret_cast(data.data())), data.size()))); + reinterpret_cast(data.data()), data.size()))); values_.push_back(reinterpret_cast(bound_strs_.back()->data())); lengths_.push_back(data.size()); @@ -108,14 +109,14 @@ void PsqlBindArray::addTempBuffer(const uint8_t* data, const size_t len) { } bound_strs_.push_back(ConstStringPtr(new std::string( - reinterpret_cast(data),len))); + reinterpret_cast(data), len))); + values_.push_back(bound_strs_.back()->data()); lengths_.push_back(len); formats_.push_back(BINARY_FMT); } - -void PsqlBindArray::add(const bool& value) { +void PsqlBindArray::add(const bool& value) { add(value ? TRUE_STR : FALSE_STR); } @@ -175,6 +176,7 @@ PsqlBindArray::addMax(const Triplet& triplet) { /// mean duplicating strings where it isn't strictly necessary. void PsqlBindArray::addTempString(const std::string& str) { bound_strs_.push_back(ConstStringPtr(new std::string(str))); + PsqlBindArray::add((bound_strs_.back())->c_str()); } @@ -236,7 +238,7 @@ PsqlBindArray::addTimestamp(const boost::posix_time::ptime& timestamp) { // // Sadly boost::posix_time::to_time_t() was not added until 1.58, // so do it ourselves. - ptime epoch(boost::gregorian::date(1970,1,1)); + ptime epoch(boost::gregorian::date(1970, 1, 1)); time_duration::sec_type since_epoch = (timestamp - epoch).total_seconds(); time_t input_time(since_epoch); @@ -325,7 +327,6 @@ PsqlBindArray::amNull(size_t index) const { return ( (values_.at(index) == NULL) && (lengths_.at(index) == 0) ); } - std::string PgSqlExchange::convertToDatabaseTime(const time_t input_time) { struct tm tinfo; @@ -361,7 +362,7 @@ time_t PgSqlExchange::convertFromDatabaseTime(const std::string& db_time_val) { // Convert string time value to time_t time_t new_time; - try { + try { new_time = (boost::lexical_cast(db_time_val)); } catch (const std::exception& ex) { isc_throw(BadValue, "Database time value is invalid: " << db_time_val); @@ -471,7 +472,6 @@ PgSqlExchange::getInetValue4(const PgSqlResult& r, const int row, } } - isc::asiolink::IOAddress PgSqlExchange::getInetValue6(const PgSqlResult& r, const int row, const size_t col) { diff --git a/src/lib/pgsql/pgsql_exchange.h b/src/lib/pgsql/pgsql_exchange.h index 13de2621ef..4221463a78 100644 --- a/src/lib/pgsql/pgsql_exchange.h +++ b/src/lib/pgsql/pgsql_exchange.h @@ -283,7 +283,7 @@ struct PsqlBindArray { /// the buffer values. /// /// @param data buffer of binary data. - /// @param len number of bytes of data in buffer + /// @param len number of bytes of data in buffer /// @throw DbOperationError if data is NULL. void addTempBuffer(const uint8_t* data, const size_t len); diff --git a/src/share/database/scripts/pgsql/dhcpdb_create.pgsql b/src/share/database/scripts/pgsql/dhcpdb_create.pgsql index 6871b4ddad..7558b620eb 100644 --- a/src/share/database/scripts/pgsql/dhcpdb_create.pgsql +++ b/src/share/database/scripts/pgsql/dhcpdb_create.pgsql @@ -4286,7 +4286,7 @@ ALTER TABLE dhcp6_subnet_server -- Fix constraint typo on dhcp4_option_def_server ALTER TABLE dhcp4_option_def_server DROP CONSTRAINT dhcp4_option_def_server_option_def_id_fkey, - ADD CONSTRAINT dhcp4_option_def_server_option_def_id_fkey + ADD CONSTRAINT dhcp4_option_def_server_option_def_id_fkey FOREIGN KEY (option_def_id) REFERENCES dhcp4_option_def(id) ON DELETE CASCADE; -- DROP shared-network ADEL triggers that should not exist. @@ -4302,9 +4302,10 @@ DROP TRIGGER IF EXISTS dhcp6_shared_network_ADEL on dhcp6_shared_network CASCADE -- a parent object when an option is modified. -- -- The following parameters are passed to the procedure: --- - modification_type: 'create', 'update' or 'delete' +-- - modification_type: "create", "update" or "delete" -- - scope_id: identifier of the option scope, e.g. --- global, subnet specific etc. +-- global, subnet specific etc. See dhcp_option_scope +-- for specific values. -- - option_id: identifier of the option. -- - p_subnet_id: identifier of the subnet if the option -- belongs to the subnet. @@ -4338,7 +4339,6 @@ DECLARE snid BIGINT; sid BIGINT; cascade_transaction BOOLEAN := true; - ct TEXT; BEGIN -- Cascade transaction flag is set to true to prevent creation of -- the audit entries for the options when the options are @@ -4393,13 +4393,16 @@ END;$$; -- -- ----------------------------------------------------- -- +-- Stored procedure which updates modification timestamp of +-- a parent object when an option is modified. +-- -- The following parameters are passed to the procedure: -- - modification_type: "create", "update" or "delete" -- - scope_id: identifier of the option scope, e.g. -- global, subnet specific etc. See dhcp_option_scope -- for specific values. -- - option_id: identifier of the option. --- - subnet_id: identifier of the subnet if the option +-- - p_subnet_id: identifier of the subnet if the option -- belongs to the subnet. -- - host_id: identifier of the host if the option -- - belongs to the host. @@ -4409,7 +4412,7 @@ END;$$; -- belongs to the pool. -- - pd_pool_id: identifier of the pool if the option -- belongs to the pd pool. --- - modification_ts: modification timestamp of the +-- - p_modification_ts: modification timestamp of the -- option. -- Some arguments are prefixed with "p_" to avoid ambiguity -- with column names in SQL statements. PostgreSQL does not @@ -4434,7 +4437,6 @@ DECLARE snid BIGINT; sid BIGINT; cascade_transaction BOOLEAN := false; - BEGIN -- Cascade transaction flag is set to true to prevent creation of -- the audit entries for the options when the options are @@ -4448,6 +4450,7 @@ BEGIN -- entire subnet. The only case when the object_type will be -- set to 'dhcp6_options' is when a global option is added. -- Global options do not have the owner. + cascade_transaction := get_session_boolean('kea.cascade_transaction'); IF cascade_transaction = false THEN -- todo: host manager hasn't been updated to use audit diff --git a/src/share/database/scripts/pgsql/upgrade_008_to_009.sh.in b/src/share/database/scripts/pgsql/upgrade_008_to_009.sh.in index f9cf2e308a..87315bb8d0 100644 --- a/src/share/database/scripts/pgsql/upgrade_008_to_009.sh.in +++ b/src/share/database/scripts/pgsql/upgrade_008_to_009.sh.in @@ -58,7 +58,7 @@ ALTER TABLE dhcp6_subnet_server -- Fix constraint typo on dhcp4_option_def_server ALTER TABLE dhcp4_option_def_server DROP CONSTRAINT dhcp4_option_def_server_option_def_id_fkey, - ADD CONSTRAINT dhcp4_option_def_server_option_def_id_fkey + ADD CONSTRAINT dhcp4_option_def_server_option_def_id_fkey FOREIGN KEY (option_def_id) REFERENCES dhcp4_option_def(id) ON DELETE CASCADE; -- DROP shared-network ADEL triggers that should not exist. @@ -74,9 +74,10 @@ DROP TRIGGER IF EXISTS dhcp6_shared_network_ADEL on dhcp6_shared_network CASCADE -- a parent object when an option is modified. -- -- The following parameters are passed to the procedure: --- - modification_type: 'create', 'update' or 'delete' +-- - modification_type: "create", "update" or "delete" -- - scope_id: identifier of the option scope, e.g. --- global, subnet specific etc. +-- global, subnet specific etc. See dhcp_option_scope +-- for specific values. -- - option_id: identifier of the option. -- - p_subnet_id: identifier of the subnet if the option -- belongs to the subnet. @@ -110,7 +111,6 @@ DECLARE snid BIGINT; sid BIGINT; cascade_transaction BOOLEAN := true; - ct TEXT; BEGIN -- Cascade transaction flag is set to true to prevent creation of -- the audit entries for the options when the options are @@ -165,13 +165,16 @@ END;\$\$; -- -- ----------------------------------------------------- -- +-- Stored procedure which updates modification timestamp of +-- a parent object when an option is modified. +-- -- The following parameters are passed to the procedure: -- - modification_type: "create", "update" or "delete" -- - scope_id: identifier of the option scope, e.g. -- global, subnet specific etc. See dhcp_option_scope -- for specific values. -- - option_id: identifier of the option. --- - subnet_id: identifier of the subnet if the option +-- - p_subnet_id: identifier of the subnet if the option -- belongs to the subnet. -- - host_id: identifier of the host if the option -- - belongs to the host. @@ -181,7 +184,7 @@ END;\$\$; -- belongs to the pool. -- - pd_pool_id: identifier of the pool if the option -- belongs to the pd pool. --- - modification_ts: modification timestamp of the +-- - p_modification_ts: modification timestamp of the -- option. -- Some arguments are prefixed with "p_" to avoid ambiguity -- with column names in SQL statements. PostgreSQL does not @@ -206,7 +209,6 @@ DECLARE snid BIGINT; sid BIGINT; cascade_transaction BOOLEAN := false; - BEGIN -- Cascade transaction flag is set to true to prevent creation of -- the audit entries for the options when the options are @@ -220,6 +222,7 @@ BEGIN -- entire subnet. The only case when the object_type will be -- set to 'dhcp6_options' is when a global option is added. -- Global options do not have the owner. + cascade_transaction := get_session_boolean('kea.cascade_transaction'); IF cascade_transaction = false THEN -- todo: host manager hasn't been updated to use audit