]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#3770] addressed review comments
authorRazvan Becheriu <razvan@isc.org>
Mon, 21 Jul 2025 15:08:25 +0000 (18:08 +0300)
committerRazvan Becheriu <razvan@isc.org>
Mon, 21 Jul 2025 15:08:25 +0000 (18:08 +0300)
14 files changed:
src/bin/admin/tests/mysql_tests.sh.in
src/hooks/dhcp/mysql/mysql_cb_dhcp4.cc
src/hooks/dhcp/mysql/mysql_cb_impl.cc
src/hooks/dhcp/mysql/mysql_host_data_source.cc
src/hooks/dhcp/pgsql/pgsql_cb_dhcp4.h
src/hooks/dhcp/pgsql/pgsql_cb_impl.cc
src/hooks/dhcp/pgsql/pgsql_cb_impl.h
src/lib/dhcpsrv/cfg_option.cc
src/lib/dhcpsrv/config_backend_pool_dhcp4.cc
src/lib/dhcpsrv/tests/cfg_option_unittest.cc
src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc
src/lib/dhcpsrv/testutils/test_config_backend_dhcp4.cc
src/lib/dhcpsrv/testutils/test_config_backend_dhcp4.h
src/share/database/scripts/mysql/upgrade_030_to_031.sh.in

index da770a75b1c9ebbd0dc7e6fa46a0230cbb62a7b2..c4331707b1e421a3caf65b12c452064072dad66c 100755 (executable)
@@ -1889,7 +1889,7 @@ SET @disable_audit = 0"
     # Check upgrade from 29.0 to 30.0.
     mysql_upgrade_29_to_30_test
 
-    # Check upgrade from 29.0 to 30.0.
+    # Check upgrade from 30.0 to 31.0.
     mysql_upgrade_30_to_31_test
 
     # Let's wipe the whole database
@@ -3994,7 +3994,7 @@ mysql_migrate_client_class_test() {
 
 # Verifies migration of dhcpX_options.client_classes column to not null.
 mysql_migrate_dhcpX_options_client_classes() {
-    test_start "mysql.mysql_migrate_dhcp4_options_client_classes"
+    test_start "mysql.mysql_migrate_dhcpX_options_client_classes"
 
     # Let's wipe the whole database
     mysql_wipe
@@ -4031,6 +4031,7 @@ mysql_migrate_dhcpX_options_client_classes() {
     # Verify the inserted record counts.
     qry="select count(*) from dhcp4_options;"
     run_statement "#get 4_option_count before update" "$qry" 2
+
     qry="select count(*) from dhcp6_options;"
     run_statement "#get 6_option_count before update" "$qry" 2
 
index 1c7ca0a3dd10077a19f851cf028c468f7173a0ce..aff111d6ca83c86aef86f3b0b002a0e4855f62fb 100644 (file)
@@ -3692,7 +3692,7 @@ TaggedStatementArray tagged_statements = { {
     { MySqlConfigBackendDHCPv4Impl::DELETE_OPTION4_SHARED_NETWORK,
       MYSQL_DELETE_OPTION_NO_TAG(dhcp4,
                           WHERE (o.scope_id = 4 AND o.shared_network_name = ? AND o.code = ? AND o.space = ?
-                                AND o.client_classes LIKE ?))
+                                 AND o.client_classes LIKE ?))
     },
 
     // Delete options belonging to a subnet.
index 0b01dc677b18bc63f809e89cad9d0e1ebdd8f527..ebf2d74dc09c912287af5129d73c3ea8e3a79566 100644 (file)
@@ -562,7 +562,7 @@ MySqlConfigBackendImpl::getOption(const int index,
                                   const ServerSelector& server_selector,
                                   const uint16_t code,
                                   const std::string& space,
-                                  const ClientClassesPtr client_classes) {
+                                  const ClientClassesPtr client_classes /* = ClientClassesPtr() */) {
 
     if (server_selector.amUnassigned()) {
         isc_throw(NotImplemented, "managing configuration for no particular server"
@@ -581,7 +581,7 @@ MySqlConfigBackendImpl::getOption(const int index,
     }
     in_bindings.push_back(MySqlBinding::createString(space));
 
-    /// @todo Remove the if when v6 is ready for this.
+    /// @todo TKM Remove the if when v6 is ready.
     if (universe == Option::V4) {
         in_bindings.push_back(createClientClassesForWhereClause(client_classes));
     }
index d4bfe0000240c6632c58a3b9f1b9c429c34dace4..6644671d73579145c6a3d7f512bee8bb222ad45a 100644 (file)
@@ -973,7 +973,7 @@ private:
                 user_context.assign(user_context_);
             }
 
-            // Convert clietn classes to string.
+            // Convert client classes to string.
             std::string client_classes;
             if (client_classes_null_ == MLM_FALSE) {
                 client_classes_[client_classes_length_] = '\0';
index 6c7d91b6435a3b8fa317ad6739b9ae8234009abb..87f2755ffd4b3ce54bf7de394ffc74317fb4eef4 100644 (file)
@@ -507,10 +507,8 @@ public:
     /// @return Number of deleted options.
     /// @throw NotImplemented if server selector is "unassigned".
     virtual uint64_t
-    deleteOption4(const db::ServerSelector& server_selector,
-                  const SubnetID& subnet_id,
-                  const uint16_t code,
-                  const std::string& space,
+    deleteOption4(const db::ServerSelector& server_selector, const SubnetID& subnet_id,
+                  const uint16_t code, const std::string& space,
                   const ClientClassesPtr client_classes = ClientClassesPtr());
 
     /// @brief Deletes pool level option.
index a80540914c01e2c020a3baee124ac7c571a224c1..c41995ce74766838c94fe5131256812e33d60806 100644 (file)
@@ -548,8 +548,7 @@ PgSqlConfigBackendImpl::getOption(const int index,
                                   const ServerSelector& server_selector,
                                   const uint16_t code,
                                   const std::string& space,
-                                  const ClientClassesPtr client_classes 
-                                  /* = ClientClassesPtr() */) {
+                                  const ClientClassesPtr client_classes /* = ClientClassesPtr() */) {
     if (server_selector.amUnassigned()) {
         isc_throw(NotImplemented, "managing configuration for no particular server"
                                   " (unassigned) is unsupported at the moment");
index eea15d5a2dacfa25da47efe0d5be2b86b7f271ef..c5ae6da35625d03b805405e6bf8912ced390ed08 100644 (file)
@@ -645,8 +645,7 @@ public:
     /// @param bindings PsqlBindArray to which the option value should be added.
     /// @param client_classes ClientClasses collection containing the class names.
     void addClientClassesForWhereClause(db::PsqlBindArray& bindings,
-                                        const ClientClassesPtr client_classes
-                                        = ClientClassesPtr());
+                                        const ClientClassesPtr client_classes = ClientClassesPtr());
 
     /// @brief Iterates over the class names in a JSON list element at a
     /// given column, adding each name to the given ClientClasses instance.
index 76bc6b01c75da1b77540a597bf65995e6b96d109..3db4eb99ad3e1cd7fc1496bd2f0983c3fc3296dc 100644 (file)
@@ -140,7 +140,7 @@ CfgOption::replace(const OptionDescriptor& desc, const std::string& option_space
     if (od_itr == idx6.end()) {
         isc_throw(isc::BadValue, "cannot replace option: "
                   << option_space << ":" << desc.option_->getType()
-                  << " , client-classes: " << desc.client_classes_.toText()
+                  << ", client-classes: " << desc.client_classes_.toText()
                   << ", it does not exist");
     }
 
@@ -264,7 +264,7 @@ CfgOption::createDescriptorOption(CfgOptionDefPtr cfg_def, const std::string& sp
     }
 
     // Indicate we replaced the definition.
-    return(true);
+    return (true);
 }
 
 void
@@ -456,7 +456,7 @@ CfgOption::del(const std::string& option_space, const uint16_t option_code,
         idx6.erase(range.first, range.second);
     }
 
-    return(count);
+    return (count);
 }
 
 size_t
index 7dc8aab9e6ff331ceda0e00088701baa39d7bbf5..bd4f7004a6856d98af1d3e0b37079fce2354bf3c 100644 (file)
@@ -139,8 +139,7 @@ ConfigBackendPoolDHCPv4::getOption4(const BackendSelector& backend_selector,
                                     const ServerSelector& server_selector,
                                     const uint16_t code,
                                     const std::string& space,
-                                    const ClientClassesPtr client_classes
-                                    /* = ClientClassesPtr */) const {
+                                    const ClientClassesPtr client_classes /* = ClientClassesPtr() */) const {
     OptionDescriptorPtr option;
     getPropertyPtrConst<OptionDescriptorPtr, uint16_t, const std::string&>
         (&ConfigBackendDHCPv4::getOption4, backend_selector, server_selector,
@@ -434,7 +433,7 @@ ConfigBackendPoolDHCPv4::deleteOption4(const BackendSelector& backend_selector,
                                        const ServerSelector& server_selector,
                                        const uint16_t code,
                                        const std::string& space,
-                                       const ClientClassesPtr client_classes /* = ClientClassesPtr */) {
+                                       const ClientClassesPtr client_classes /* = ClientClassesPtr() */) {
 
     return (createUpdateDeleteProperty<uint64_t, uint16_t, const std::string&>
             (&ConfigBackendDHCPv4::deleteOption4, backend_selector, server_selector,
@@ -447,7 +446,7 @@ ConfigBackendPoolDHCPv4::deleteOption4(const BackendSelector& backend_selector,
                                        const std::string& shared_network_name,
                                        const uint16_t code,
                                        const std::string& space,
-                                       const ClientClassesPtr client_classes /* = ClientClassesPtr */) {
+                                       const ClientClassesPtr client_classes /* = ClientClassesPtr() */) {
     return (createUpdateDeleteProperty<uint64_t, const std::string&, uint16_t,
                                        const std::string&>
             (&ConfigBackendDHCPv4::deleteOption4, backend_selector, server_selector,
@@ -460,7 +459,7 @@ ConfigBackendPoolDHCPv4::deleteOption4(const BackendSelector& backend_selector,
                                        const SubnetID& subnet_id,
                                        const uint16_t code,
                                        const std::string& space,
-                                       const ClientClassesPtr client_classes /* = ClientClassesPtr */) {
+                                       const ClientClassesPtr client_classes /* = ClientClassesPtr() */) {
     return (createUpdateDeleteProperty<uint64_t, const SubnetID&, uint16_t, const std::string&>
             (&ConfigBackendDHCPv4::deleteOption4, backend_selector, server_selector,
              subnet_id, code, space, client_classes));
@@ -473,7 +472,7 @@ ConfigBackendPoolDHCPv4::deleteOption4(const BackendSelector& backend_selector,
                                        const asiolink::IOAddress& pool_end_address,
                                        const uint16_t code,
                                        const std::string& space,
-                                       const ClientClassesPtr client_classes /* = ClientClassesPtr */) {
+                                       const ClientClassesPtr client_classes /* = ClientClassesPtr() */) {
     return (createUpdateDeleteProperty<uint64_t, const IOAddress&, const IOAddress&,
                                        uint16_t, const std::string&>
             (&ConfigBackendDHCPv4::deleteOption4, backend_selector, server_selector,
index d52622e47ba7991cae0a8e2b99ecf7dbaf157f09..bf53a6ea1b6d8f86224113f6d286b604e5a7c6c3 100644 (file)
@@ -1509,16 +1509,16 @@ TEST_F(CfgOptionTest, optionsWithClientClasses) {
     OptionDescriptorList list_options = cfg.getList(DHCP6_OPTION_SPACE, 777);
     ASSERT_EQ(options->size(), reference_options.size());
     auto reference_rdesc = reference_options.rbegin();
-    for (auto &returned_desc : list_options ){
+    for (auto &returned_desc : list_options{
         ASSERT_EQ(*reference_rdesc, returned_desc);
         ++reference_rdesc;
     }
 
     // Verify that CfgOption::get() with client classes returns
     // each one correctly.
-    for (auto &reference_desc : reference_options ){
+    for (auto &reference_desc : reference_options{
         OptionDescriptor found_desc = cfg.get(DHCP6_OPTION_SPACE, 777,
-                                                reference_desc.client_classes_);
+                                              reference_desc.client_classes_);
         ASSERT_TRUE(found_desc.option_);
         ASSERT_EQ(found_desc, reference_desc);
     }
@@ -1561,7 +1561,7 @@ TEST_F(CfgOptionTest, replaceWithClientClasses) {
     (boost::dynamic_pointer_cast<OptionUint16>(replacement.option_))->setValue(100);
     ASSERT_NO_THROW(cfg.replace(replacement, DHCP6_OPTION_SPACE));
 
-    // Make sure we can the updated option.
+    // Make sure we can get the updated option.
     OptionDescriptor found_desc = cfg.get(DHCP6_OPTION_SPACE, 777,
                                           replacement.client_classes_);
     ASSERT_TRUE(found_desc.option_);
@@ -1572,7 +1572,7 @@ TEST_F(CfgOptionTest, replaceWithClientClasses) {
     (boost::dynamic_pointer_cast<OptionUint16>(replacement2.option_))->setValue(300);
     ASSERT_NO_THROW(cfg.replace(replacement2, DHCP6_OPTION_SPACE));
 
-    // Make sure we can the updated option.
+    // Make sure we can get the updated option.
     found_desc = cfg.get(DHCP6_OPTION_SPACE, 777, replacement2.client_classes_);
     ASSERT_TRUE(found_desc.option_);
     ASSERT_EQ(found_desc, replacement2);
@@ -1631,7 +1631,7 @@ TEST_F(CfgOptionTest, deleteWithClientClasses) {
     ASSERT_FALSE(found_desc.option_);
     reference_options.pop_back();
 
-    // Delete the secon reference option.
+    // Delete the second reference option.
     ASSERT_NO_THROW(cfg.del(DHCP6_OPTION_SPACE, 777, reference_options[1].client_classes_));
 
     // Make sure we can no longer find the deleted option.
index 5fff7a62f7a612fa9ad788ab8138d1dc396ffeaa..ca7f9d6dae4fab3f2ef9d8ae7290477b8730cd43 100644 (file)
@@ -150,7 +150,6 @@ GenericConfigBackendDHCPv4Test::initTestSubnets() {
     subnet->getCfgOption()->add(*test_options_[1], test_options_[1]->space_name_);
     subnet->getCfgOption()->add(*test_options_[2], test_options_[2]->space_name_);
 
-
     test_subnets_.push_back(subnet);
 
     // Adding another subnet with the same subnet id to test
@@ -3663,7 +3662,7 @@ GenericConfigBackendDHCPv4Test::makeClassTaggedOptions() {
     };
 
     std::list<OptionDescriptorPtr> tagged_options;
-    for ( auto const& opt_to_make : opts_to_make) {
+    for (auto const& opt_to_make : opts_to_make) {
         OptionDescriptor desc = createOption<OptionString>(Option::V4, opt_to_make.code_,
                                                            true, false, false, opt_to_make.value_);
         desc.space_name_ = DHCP4_OPTION_SPACE;
@@ -3680,7 +3679,7 @@ GenericConfigBackendDHCPv4Test::makeClassTaggedOptions() {
 void
 GenericConfigBackendDHCPv4Test::updateClassTaggedOptions(
     std::list<OptionDescriptorPtr>& options) {
-    for ( auto& desc : options) {
+    for (auto& desc : options) {
         OptionStringPtr opt = boost::dynamic_pointer_cast<OptionString>(desc->option_);
         ASSERT_TRUE(opt);
         std::string new_value(opt->getValue() + std::string(".") + opt->getValue());
@@ -3688,7 +3687,7 @@ GenericConfigBackendDHCPv4Test::updateClassTaggedOptions(
     }
 }
 
-// Macro the make SCOPED_TRACE around equivalance functon more compact and helpful.
+// Macro the make SCOPED_TRACE around equivalance function more compact and helpful.
 #define SCOPED_OPT_COMPARE(exp_opt,test_opt)\
 {\
     std::stringstream oss;\
@@ -3758,7 +3757,6 @@ GenericConfigBackendDHCPv4Test::globalOption4WithClientClassesTest() {
     }
 }
 
-
 void
 GenericConfigBackendDHCPv4Test::getAllOptions4WithClientClassesTest() {
     // Describes an option to create.
@@ -3789,9 +3787,9 @@ GenericConfigBackendDHCPv4Test::getAllOptions4WithClientClassesTest() {
 
     // Add all of the global options.
     std::vector<OptionDescriptorPtr> ref_options;
-    for ( auto const& opt_to_make : opts_to_make) {
+    for (auto const& opt_to_make : opts_to_make) {
         OptionDescriptor desc = createOption<OptionInt<uint32_t>>(Option::V4, opt_to_make.code_,
-                                                           true, false, false, opt_to_make.value_);
+                                                                  true, false, false, opt_to_make.value_);
         desc.space_name_ = DHCP4_OPTION_SPACE;
         if (!opt_to_make.cclass_.empty()) {
             desc.addClientClass(opt_to_make.cclass_);
@@ -3819,7 +3817,7 @@ GenericConfigBackendDHCPv4Test::getAllOptions4WithClientClassesTest() {
         ++exp_option;
     }
 
-    // Try to fetch the collection of global options for the server1.
+    // Try to fetch the collection of global options for the server2.
     // Build list of options we expect to get back.
     exp_options.clear();
     exp_options.push_back(ref_options[3]);
@@ -3849,7 +3847,6 @@ GenericConfigBackendDHCPv4Test::getAllOptions4WithClientClassesTest() {
     }
 }
 
-
 void
 GenericConfigBackendDHCPv4Test::createUpdateDeleteSubnetOption4Test() {
     // Insert new subnet.
@@ -4940,7 +4937,7 @@ GenericConfigBackendDHCPv4Test::sharedNetworkOption4WithClientClassesTest() {
     // Make a network with options.
     SharedNetwork4Ptr network(new SharedNetwork4("net1"));
     auto ref_options = makeClassTaggedOptions();
-    for ( auto const& ref_option : ref_options) {
+    for (auto const& ref_option : ref_options) {
         network->getCfgOption()->add(*ref_option, ref_option->space_name_);
     }
 
@@ -4979,7 +4976,6 @@ GenericConfigBackendDHCPv4Test::sharedNetworkOption4WithClientClassesTest() {
     }
 
     // Now make sure that we can delete the options individually.
-    updateClassTaggedOptions(ref_options);
     for (auto const& ref_option : ref_options) {
         ASSERT_EQ(1, cbptr_->deleteOption4(ServerSelector::ANY(),
                                            network->getName(),
@@ -5005,7 +5001,7 @@ GenericConfigBackendDHCPv4Test::subnetOption4WithClientClassesTest() {
     Subnet4Ptr subnet(new Subnet4(IOAddress("192.0.2.0"), 24,
                                   30, 40, 60, 1024));
     auto ref_options = makeClassTaggedOptions();
-    for ( auto const& ref_option : ref_options) {
+    for (auto const& ref_option : ref_options) {
         subnet->getCfgOption()->add(*ref_option, ref_option->space_name_);
     }
 
@@ -5043,7 +5039,6 @@ GenericConfigBackendDHCPv4Test::subnetOption4WithClientClassesTest() {
     }
 
     // Now make sure that we can delete the options individually.
-    updateClassTaggedOptions(ref_options);
     for (auto const& ref_option : ref_options) {
         ASSERT_EQ(1, cbptr_->deleteOption4(ServerSelector::ANY(),
                                            subnet->getID(),
@@ -5074,7 +5069,7 @@ GenericConfigBackendDHCPv4Test::poolOption4WithClientClassesTest() {
 
     // Add the options to the pool.
     auto ref_options = makeClassTaggedOptions();
-    for ( auto const& ref_option : ref_options) {
+    for (auto const& ref_option : ref_options) {
         pool->getCfgOption()->add(*ref_option, ref_option->space_name_);
     }
 
@@ -5116,8 +5111,8 @@ GenericConfigBackendDHCPv4Test::poolOption4WithClientClassesTest() {
     // Make sure that CfgOption->get() with client_classes finds each ref option.
     for (auto const& ref_option : ref_options) {
         auto cfg_option = returned_pool->getCfgOption()->get(DHCP4_OPTION_SPACE,
-                                                               ref_option->option_->getType(),
-                                                               ref_option->client_classes_);
+                                                             ref_option->option_->getType(),
+                                                             ref_option->client_classes_);
         SCOPED_OPT_COMPARE((*ref_option), cfg_option);
     }
 
index 4a5f97757b8179711debcddaffcbc5d1be03dcbe..b1c53ac44e7c06a9182888c95135c19685d2cd31 100644 (file)
@@ -469,7 +469,6 @@ TestConfigBackendDHCPv4::getGlobalParameter4(const db::ServerSelector& server_se
     return (candidate);
 }
 
-
 StampedValueCollection
 TestConfigBackendDHCPv4::getAllGlobalParameters4(const db::ServerSelector& server_selector) const {
     auto tags = server_selector.getTags();
@@ -1237,7 +1236,6 @@ TestConfigBackendDHCPv4::deleteOption4(const db::ServerSelector& server_selector
         }
     }
 
-
     if (!found) {
         isc_throw(BadValue, "attempted to delete option in a "
                   "shared network " << shared_network_name
index 656f9cecdd752487dc41e07ddaf5327d1844fc55..fe8dbf1dc927a5087192008be8c5e26bc459d962 100644 (file)
@@ -444,8 +444,7 @@ public:
     /// Defaults to an empty pointer.
     /// @return Number of deleted options.
     virtual uint64_t
-    deleteOption4(const db::ServerSelector& server_selector,
-                  const uint16_t code,
+    deleteOption4(const db::ServerSelector& server_selector, const uint16_t code,
                   const std::string& space,
                   ClientClassesPtr client_classes = ClientClassesPtr());
 
index d2e8a991b9a38b70ccd1598dc310a1b621a3fb38..90e1477456f9c749627d02338fb841aa54e47d26 100755 (executable)
@@ -1,6 +1,7 @@
 #!/bin/sh
 
 # Copyright (C) 2025 Internet Systems Consortium, Inc. ("ISC") #
+#
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.