From: Thomas Markwalder <tmark@isc.org>
Date: Thu, 10 Jul 2025 20:17:38 +0000 (-0400)
Subject: [#3770] Clean up
X-Git-Tag: Kea-3.1.0~32
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3124a9233913c2498197f285af1146250e107066;p=thirdparty%2Fkea.git

[#3770] Clean up

/src/hooks/dhcp/pgsql/pgsql_cb_impl.cc
    PgSqlConfigBackendImpl::addClientClassesBinding()
    - use ClientClasses::toElement()

/src/lib/dhcpsrv/cfg_option.*
    ClientClassesPtr OptionDescriptor::copyClientClasses()
    - new convenience function

/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc
    Use new convenience function
---

diff --git a/src/hooks/dhcp/pgsql/pgsql_cb_impl.cc b/src/hooks/dhcp/pgsql/pgsql_cb_impl.cc
index b386cf40e4..a80540914c 100644
--- a/src/hooks/dhcp/pgsql/pgsql_cb_impl.cc
+++ b/src/hooks/dhcp/pgsql/pgsql_cb_impl.cc
@@ -1159,12 +1159,7 @@ void
 PgSqlConfigBackendImpl::addClientClassesBinding(db::PsqlBindArray& bindings,
                                                 const ClientClasses& client_classes) {
     // Create JSON list of client classes.
-    data::ElementPtr client_classes_element = data::Element::createList();
-    for (auto const& client_class : client_classes) {
-        client_classes_element->add(data::Element::create(client_class));
-    }
-
-    bindings.add(client_classes_element);
+    bindings.add(client_classes.toElement());
 }
 
 void
diff --git a/src/lib/dhcpsrv/cfg_option.cc b/src/lib/dhcpsrv/cfg_option.cc
index 8f18c135e8..76bc6b01c7 100644
--- a/src/lib/dhcpsrv/cfg_option.cc
+++ b/src/lib/dhcpsrv/cfg_option.cc
@@ -61,6 +61,11 @@ OptionDescriptor::addClientClass(const std::string& class_name) {
     }
 }
 
+ClientClassesPtr
+OptionDescriptor::copyClientClasses() const {
+    return (ClientClassesPtr(new ClientClasses(client_classes_)));
+}
+
 bool
 OptionDescriptor::allowedForClientClasses(const ClientClasses& cclasses) const {
     if (client_classes_.empty()) {
diff --git a/src/lib/dhcpsrv/cfg_option.h b/src/lib/dhcpsrv/cfg_option.h
index e5d0930f94..9c8ef0f75a 100644
--- a/src/lib/dhcpsrv/cfg_option.h
+++ b/src/lib/dhcpsrv/cfg_option.h
@@ -219,6 +219,11 @@ public:
     /// @return True if descriptor's client-classes is empty or at least
     /// one of its members is found in the validation list.
     bool allowedForClientClasses(const ClientClasses& cclasses) const;
+
+    /// @brief Get a copy of client classes
+    ///
+    /// @return Pointer to a copy of descriptor's client classes
+    ClientClassesPtr copyClientClasses() const;
 };
 
 /// @brief Multi index container for DHCP option descriptors.
diff --git a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc
index 03ae354f1f..5fff7a62f7 100644
--- a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc
+++ b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc
@@ -3714,11 +3714,10 @@ GenericConfigBackendDHCPv4Test::globalOption4WithClientClassesTest() {
     OptionDescriptorPtr found_option;
     for (auto const& ref_option : ref_options) {
         // Find the option by code and client_classes.
-        ClientClassesPtr cclasses(new ClientClasses(ref_option->client_classes_));
         found_option = cbptr_->getOption4(ServerSelector::ALL(),
                                           ref_option->option_->getType(),
                                           DHCP4_OPTION_SPACE,
-                                          cclasses);
+                                          ref_option->copyClientClasses());
         ASSERT_TRUE(found_option);
         SCOPED_OPT_COMPARE((*ref_option), (*found_option));
     }
@@ -3728,8 +3727,6 @@ GenericConfigBackendDHCPv4Test::globalOption4WithClientClassesTest() {
 
     // Update each option in the backend.
     for (auto const& ref_option : ref_options) {
-        ClientClassesPtr cclasses(new ClientClasses(ref_option->client_classes_));
-
         // Update option in the config back end.
         cbptr_->createUpdateOption4(ServerSelector::ALL(), ref_option);
 
@@ -3737,14 +3734,14 @@ GenericConfigBackendDHCPv4Test::globalOption4WithClientClassesTest() {
         found_option = cbptr_->getOption4(ServerSelector::ALL(),
                                           ref_option->option_->getType(),
                                           DHCP4_OPTION_SPACE,
-                                          cclasses);
+                                          ref_option->copyClientClasses());
         ASSERT_TRUE(found_option);
         SCOPED_OPT_COMPARE((*ref_option), (*found_option));
     }
 
     // Delete each option from the backend.
     for (auto const& ref_option : ref_options) {
-        ClientClassesPtr cclasses(new ClientClasses(ref_option->client_classes_));
+        ClientClassesPtr cclasses = ref_option->copyClientClasses();
 
         // Delete the option by code and client_classes.
         ASSERT_EQ(1, cbptr_->deleteOption4(ServerSelector::ALL(),
@@ -4175,7 +4172,8 @@ GenericConfigBackendDHCPv4Test::createUpdateDeleteSharedNetworkOption4Test() {
     EXPECT_EQ(1, cbptr_->deleteOption4(ServerSelector::ANY(),
                                        shared_network->getName(),
                                        opt_boot_file_name->option_->getType(),
-                                       opt_boot_file_name->space_name_));
+                                       opt_boot_file_name->space_name_,
+                                       opt_boot_file_name->copyClientClasses()));
     returned_network = cbptr_->getSharedNetwork4(ServerSelector::ALL(),
                                                  shared_network->getName());
     ASSERT_TRUE(returned_network);
@@ -4983,13 +4981,11 @@ GenericConfigBackendDHCPv4Test::sharedNetworkOption4WithClientClassesTest() {
     // Now make sure that we can delete the options individually.
     updateClassTaggedOptions(ref_options);
     for (auto const& ref_option : ref_options) {
-        ClientClassesPtr cclasses(new ClientClasses(ref_option->client_classes_));
         ASSERT_EQ(1, cbptr_->deleteOption4(ServerSelector::ANY(),
                                            network->getName(),
                                            ref_option->option_->getType(),
                                            DHCP4_OPTION_SPACE,
-                                           cclasses)) << "code:" << ref_option->option_->getType() 
-                                                      <<  " classes: " << cclasses->toText();
+                                           ref_option->copyClientClasses()));
     }
 
     // Re-fetch the network.
@@ -5049,12 +5045,11 @@ GenericConfigBackendDHCPv4Test::subnetOption4WithClientClassesTest() {
     // Now make sure that we can delete the options individually.
     updateClassTaggedOptions(ref_options);
     for (auto const& ref_option : ref_options) {
-        ClientClassesPtr cclasses(new ClientClasses(ref_option->client_classes_));
         ASSERT_EQ(1, cbptr_->deleteOption4(ServerSelector::ANY(),
                                            subnet->getID(),
                                            ref_option->option_->getType(),
                                            DHCP4_OPTION_SPACE,
-                                           cclasses));
+                                           ref_option->copyClientClasses()));
     }
 
     // Re-fetch the subnet.
@@ -5128,13 +5123,12 @@ GenericConfigBackendDHCPv4Test::poolOption4WithClientClassesTest() {
 
     // Now make sure that we can delete the options individually.
     for (auto const& ref_option : ref_options) {
-        ClientClassesPtr cclasses(new ClientClasses(ref_option->client_classes_));
         ASSERT_EQ(1, cbptr_->deleteOption4(ServerSelector::ANY(),
                                            pool->getFirstAddress(),
                                            pool->getLastAddress(),
                                            ref_option->option_->getType(),
                                            DHCP4_OPTION_SPACE,
-                                           cclasses));
+                                           ref_option->copyClientClasses()));
     }
 
     // Re-fetch the subnet.