feat: add validation for value updating through http api

include/dsn/utility/flags.h

@@ -65,9 +65,8 @@ struct hash<flag_tag>
 // The program corrupts if the validation failed.
 #define DSN_DEFINE_validator(name, validator)                                                      \
     static auto FLAGS_VALIDATOR_FN_##name = validator;                                             \
-    static const dsn::flag_validator FLAGS_VALIDATOR_##name(#name, []() {                          \
-        dassert(FLAGS_VALIDATOR_FN_##name(FLAGS_##name), "validation failed: %s", #name);          \
-    })
+    static const dsn::flag_validator FLAGS_VALIDATOR_##name(                                       \
+        #name, []() -> bool { return FLAGS_VALIDATOR_FN_##name(FLAGS_##name); })
 
 #define DSN_TAG_VARIABLE(name, tag)                                                                \
     COMPILE_ASSERT(sizeof(decltype(FLAGS_##name)), exist_##name##_##tag);                          \
@@ -89,10 +88,11 @@ class flag_registerer
 };
 
 // An utility class that registers a validator upon initialization.
+using validator_fn = std::function<bool()>;
 class flag_validator
 {
 public:
-    flag_validator(const char *name, std::function<void()>);
+    flag_validator(const char *name, validator_fn);
 };
 
 class flag_tagger

src/utils/flags.cpp

@@ -9,7 +9,7 @@
 #include <dsn/utility/string_conv.h>
 #include <dsn/c/api_utilities.h>
 #include <boost/optional/optional.hpp>
-#include <fmt/format.h>
+#include <dsn/dist/fmt_logging.h>
 
 #include <map>
 
@@ -27,27 +27,29 @@ enum value_type
     FV_MAX_INDEX = 6,
 };
 
-using validator_fn = std::function<void()>;
-
 class flag_data
 {
 public:
 #define FLAG_DATA_LOAD_CASE(type, type_enum, suffix)                                               \
     case type_enum:                                                                                \
         value<type>() = dsn_config_get_value_##suffix(_section, _name, value<type>(), _desc);      \
         if (_validator) {                                                                          \
-            _validator();                                                                          \
+            dassert_f(_validator(), "validation failed: {}", _name);                               \
         }                                                                                          \
         break
 
 #define FLAG_DATA_UPDATE_CASE(type, type_enum, suffix)                                             \
-    case type_enum:                                                                                \
-        type tmpval_##type_enum;                                                                   \
+    case type_enum: {                                                                              \
+        type old_val = value<type>(), tmpval_##type_enum;                                          \
         if (!dsn::buf2##suffix(val, tmpval_##type_enum)) {                                         \
-            return error_s::make(ERR_INVALID_PARAMETERS, fmt::format("{} in invalid", val));       \
+            return error_s::make(ERR_INVALID_PARAMETERS, fmt::format("{} is invalid", val));       \
         }                                                                                          \
         value<type>() = tmpval_##type_enum;                                                        \
-        break
+        if (_validator && !_validator()) {                                                         \
+            value<type>() = old_val;                                                               \
+            return error_s::make(ERR_INVALID_PARAMETERS, "value validation failed");               \
+        }                                                                                          \
+    } break
 
 #define FLAG_DATA_UPDATE_STRING()                                                                  \
     case FV_STRING:                                                                                \

src/utils/test/flag_test.cpp

@@ -41,6 +41,15 @@ DSN_TAG_VARIABLE(test_bool, FT_MUTABLE);
 
 DSN_DEFINE_string("flag_test", test_string_immutable, "immutable_string", "");
 
+DSN_DEFINE_int32("flag_test", test_validator, 10, "");
+DSN_TAG_VARIABLE(test_validator, FT_MUTABLE);
+DSN_DEFINE_validator(test_validator, [](int32_t test_validator) -> bool {
+    if (test_validator < 0) {
+        return false;
+    }
+    return true;
+});
+
 TEST(flag_test, update_config)
 {
     auto res = update_flag("test_int32", "3");
@@ -80,6 +89,16 @@ TEST(flag_test, update_config)
     res = update_flag("test_int32", "3ab");
     ASSERT_EQ(res.code(), ERR_INVALID_PARAMETERS);
     ASSERT_EQ(FLAGS_test_int32, 3);
+
+    // validation succeed
+    res = update_flag("test_validator", "5");
+    ASSERT_TRUE(res.is_ok());
+    ASSERT_EQ(FLAGS_test_validator, 5);
+
+    // validation failed
+    res = update_flag("test_validator", "-1");
+    ASSERT_EQ(res.code(), ERR_INVALID_PARAMETERS);
+    ASSERT_EQ(FLAGS_test_validator, 5);
 }
 
 DSN_DEFINE_int32("flag_test", has_tag, 5, "");