[#1928] Do not re-position DHCPv6 class on update

diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc
index fb85fefac5928b98937fc3752fffe43e8229a7e6..703a40439ef84744446126d9c819bb304dcade72 100644 (file)
--- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc
+++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc
@@ -136,6 +136,7 @@ public:
         UPDATE_OPTION6_SHARED_NETWORK,
         UPDATE_OPTION6_CLIENT_CLASS,
         UPDATE_CLIENT_CLASS6,
+        UPDATE_CLIENT_CLASS6_SAME_POSITION,
         UPDATE_SERVER6,
         DELETE_GLOBAL_PARAMETER6,
         DELETE_ALL_GLOBAL_PARAMETERS6,
@@ -3016,13 +3017,13 @@ public:
     ///
     /// @param server_selector Server selector.
     /// @param client_class Pointer to the upserted client class.
-    /// @param follow_client_class name of the class after which the
+    /// @param follow_class_name name of the class after which the
     /// new or updated class should be positioned. An empty value
     /// causes the class to be appended at the end of the class
     /// hierarchy.
     void createUpdateClientClass6(const ServerSelector& server_selector,
                                   const ClientClassDefPtr& client_class,
-                                  const std::string& follow_client_class) {
+                                  const std::string& follow_class_name) {
         // We need to evaluate class expression to see if it references any
         // other classes (dependencies). As part of this evaluation we will
         // also check if the client class depends on KNOWN/UNKNOWN built-in
@@ -3062,8 +3063,8 @@ public:
             MySqlBinding::createInteger<uint32_t>(client_class->getValid().getMin()),
             MySqlBinding::createInteger<uint32_t>(client_class->getValid().getMax()),
             MySqlBinding::createBool(depend_on_known),
-            (follow_client_class.empty() ? MySqlBinding::createNull() :
-             MySqlBinding::createString(follow_client_class)),
+            (follow_class_name.empty() ? MySqlBinding::createNull() :
+             MySqlBinding::createString(follow_class_name)),
             MySqlBinding::createTimestamp(client_class->getModificationTime()),
         };
 
@@ -3086,7 +3087,18 @@ public:
 
             // Try to update the class.
             in_bindings.push_back(MySqlBinding::createString(client_class->getName()));
-            conn_.updateDeleteQuery(MySqlConfigBackendDHCPv6Impl::UPDATE_CLIENT_CLASS6, in_bindings);
+            if (follow_class_name.empty()) {
+                // If position is not specified, leave the class at the same position.
+                // Remove the binding which specifies the position and use different
+                // query.
+                in_bindings.erase(in_bindings.begin() + 7, in_bindings.begin() + 8);
+                conn_.updateDeleteQuery(MySqlConfigBackendDHCPv6Impl::UPDATE_CLIENT_CLASS6_SAME_POSITION,
+                                        in_bindings);
+            } else {
+                // Update with specifying the position.
+                conn_.updateDeleteQuery(MySqlConfigBackendDHCPv6Impl::UPDATE_CLIENT_CLASS6,
+                                        in_bindings);
+            }
 
             // Delete class associations with the servers and dependencies. We will re-create
             // them according to the new class specification.
@@ -3877,18 +3889,14 @@ TaggedStatementArray tagged_statements = { {
       MYSQL_UPDATE_OPTION6_NO_TAG(o.scope_id = 2 AND o.dhcp_client_class = ? AND o.code = ? AND o.space = ?)
     },
 
+    // Update existing client class with specifying its position.
     { MySqlConfigBackendDHCPv6Impl::UPDATE_CLIENT_CLASS6,
-      "UPDATE dhcp6_client_class SET"
-      "  name = ?,"
-      "  test = ?,"
-      "  only_if_required = ?,"
-      "  valid_lifetime = ?,"
-      "  min_valid_lifetime = ?,"
-      "  max_valid_lifetime = ?,"
-      "  depend_on_known_directly = ?,"
-      "  follow_class_name = ?,"
-      "  modification_ts = ? "
-      "WHERE name = ?"
+      MYSQL_UPDATE_CLIENT_CLASS6("follow_class_name = ?,")
+    },
+
+    // Update existing client class without specifying its position.
+    { MySqlConfigBackendDHCPv6Impl::UPDATE_CLIENT_CLASS6_SAME_POSITION,
+      MYSQL_UPDATE_CLIENT_CLASS6("")
     },
 
     // Update existing server, e.g. server description.
@@ -4459,10 +4467,10 @@ MySqlConfigBackendDHCPv6::createUpdateGlobalParameter6(const ServerSelector& ser
 void
 MySqlConfigBackendDHCPv6::createUpdateClientClass6(const db::ServerSelector& server_selector,
                                                    const ClientClassDefPtr& client_class,
-                                                   const std::string& follow_client_class) {
+                                                   const std::string& follow_class_name) {
     LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_CREATE_UPDATE_CLIENT_CLASS6)
         .arg(client_class->getName());
-    impl_->createUpdateClientClass6(server_selector, client_class, follow_client_class);
+    impl_->createUpdateClientClass6(server_selector, client_class, follow_class_name);
 }
 
 void

diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.h b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.h
index 8c80741e3b80f220a5814dc2ea62019d4f1a725d..4ab608c9f4c23f153d956681b285e96642f45b8b 100644 (file)
--- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.h
+++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.h
@@ -383,14 +383,14 @@ public:
     ///
     /// @param server_selector Server selector.
     /// @param client_class Client class to be added or updated.
-    /// @param follow_client_class name of the class after which the
+    /// @param follow_class_name name of the class after which the
     /// new or updated class should be positioned. An empty value
     /// causes the class to be appended at the end of the class
     /// hierarchy.
     virtual void
     createUpdateClientClass6(const db::ServerSelector& server_selector,
                              const ClientClassDefPtr& client_class,
-                             const std::string& follow_client_class);
+                             const std::string& follow_class_name);
 
     /// @brief Creates or updates a server.
     ///

diff --git a/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h b/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h
index 7808fd5be973f72bb3a19380f5b0182dd7e7af7c..4754869ef36a0d345daaf0e4caab91a59b4c6045 100644 (file)
--- a/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h
+++ b/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h
@@ -1077,6 +1077,21 @@ namespace {
     "WHERE name = ?"
 #endif
 
+#ifndef MYSQL_UPDATE_CLIENT_CLASS6
+#define MYSQL_UPDATE_CLIENT_CLASS6(follow_class_name_set) \
+    "UPDATE dhcp6_client_class SET" \
+    "  name = ?," \
+    "  test = ?," \
+    "  only_if_required = ?," \
+    "  valid_lifetime = ?," \
+    "  min_valid_lifetime = ?," \
+    "  max_valid_lifetime = ?," \
+    "  depend_on_known_directly = ?," \
+    follow_class_name_set \
+    "  modification_ts = ? " \
+    "WHERE name = ?"
+#endif
+
 #ifndef MYSQL_UPDATE_SERVER
 #define MYSQL_UPDATE_SERVER(table_prefix) \
     "UPDATE " #table_prefix "_server " \

diff --git a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc
index a9bb548557f0e4d09cbee12911d6cc03e9cdc803..035ce5d8a45dd2eada56c135b5bf88caefff5f54 100644 (file)
--- a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc
+++ b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc
@@ -4385,7 +4385,6 @@ TEST_F(MySqlConfigBackendDHCPv6Test, setAndGetAllClientClasses6) {
     EXPECT_EQ("bar", (*(classes_list->begin() + 1))->getName());
     EXPECT_EQ("foobar", (*(classes_list->begin() + 2))->getName());
 
-
     // Move the third class between the first and second class.
     ASSERT_NO_THROW(cbptr_->createUpdateClientClass6(ServerSelector::ONE("server1"), class3, "foo"));
 
@@ -4396,6 +4395,17 @@ TEST_F(MySqlConfigBackendDHCPv6Test, setAndGetAllClientClasses6) {
     EXPECT_EQ("foo", (*classes_list->begin())->getName());
     EXPECT_EQ("foobar", (*(classes_list->begin() + 1))->getName());
     EXPECT_EQ("bar", (*(classes_list->begin() + 2))->getName());
+
+    // Update the foobar class without specifying its position. It should not
+    // be moved.
+    ASSERT_NO_THROW(cbptr_->createUpdateClientClass6(ServerSelector::ONE("server1"), class3, ""));
+
+    client_classes = cbptr_->getAllClientClasses6(ServerSelector::ONE("server1"));
+    classes_list = client_classes.getClasses();
+    ASSERT_EQ(3, classes_list->size());
+    EXPECT_EQ("foo", (*classes_list->begin())->getName());
+    EXPECT_EQ("foobar", (*(classes_list->begin() + 1))->getName());
+    EXPECT_EQ("bar", (*(classes_list->begin() + 2))->getName());
 }
 
 // This test verifies that a single class can be retrieved from the database.
@@ -4871,6 +4881,11 @@ TEST_F(MySqlConfigBackendDHCPv6Test, clientClassDependencies6) {
     class3->setTest("member('bar')");
     EXPECT_NO_THROW(cbptr_->createUpdateClientClass6(ServerSelector::ALL(), class3, ""));
 
+    // An attempt to move the first class to the end of the class hierarchy should
+    // fail because other classes depend on it.
+    EXPECT_THROW(cbptr_->createUpdateClientClass6(ServerSelector::ALL(), class1, "bar"),
+                 DbOperationError);
+
     // Try to change the dependency of the first class. There are other classes
     // having indirect dependency on KNOWN class via this class. Therefore, the
     // update should be unsuccessful.

diff --git a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.cc b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.cc
index db87fac7d7201722820baad6d7dd91e7f846cb9e..8a4b2623af96683612e411a98d63855f04aa3e27 100644 (file)
--- a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.cc
+++ b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.cc
@@ -591,7 +591,7 @@ TestConfigBackendDHCPv6::getModifiedClientClasses6(const db::ServerSelector& ser
 void
 TestConfigBackendDHCPv6::createUpdateClientClass6(const db::ServerSelector& server_selector,
                                                   const ClientClassDefPtr& client_class,
-                                                  const std::string& follow_client_class) {
+                                                  const std::string&) {
     mergeServerTags(client_class, server_selector);
 
     auto existing_class = classes_.findClass(client_class->getName());

diff --git a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.h b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.h
index 012120194bd2cd8ed316b6d2fbeab8eda2f47223..1c6b1d6abd860ec2443b952e95c41790dcb20ff8 100644 (file)
--- a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.h
+++ b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.h
@@ -360,14 +360,14 @@ public:
     ///
     /// @param server_selector Server selector.
     /// @param client_class Client class to be added or updated.
-    /// @param follow_client_class name of the class after which the
+    /// @param follow_class_name name of the class after which the
     /// new or updated class should be positioned. An empty value
     /// causes the class to be appended at the end of the class
     /// hierarchy.
     virtual void
     createUpdateClientClass6(const db::ServerSelector& server_selector,
                              const ClientClassDefPtr& client_class,
-                             const std::string& follow_client_class);
+                             const std::string& follow_class_name);
 
     /// @brief Creates or updates a server.
     ///