From fee388265827a18b8262bfcbce6aa59bf75b2da5 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 3 Dec 2020 19:03:42 +0800 Subject: [PATCH 01/23] fix --- include/dsn/utility/flags.h | 2 ++ src/utils/flags.cpp | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index c312494dfa..6c716fb466 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -52,6 +52,8 @@ dassert(FLAGS_VALIDATOR_FN_##name(FLAGS_##name), "validation failed: %s", #name); \ }) +#define DSN_UPDATE(name, val) flag_registry::instance().update_flag(name, val) + namespace dsn { // An utility class that registers a flag upon initialization. diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index 43c6d7b297..478e2a4e67 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -55,6 +55,21 @@ class flag_data { } + bool update(const char *val) + { + switch (_type) { + case FV_INT32: + case FV_INT64: + case FV_UINT32: + case FV_UINT64: + case FV_BOOL: + case FV_DOUBLE: + case FV_STRING: + strcpy(value(), val); + break; + } + } + void set_validator(validator_fn &validator) { _validator = std::move(validator); } const validator_fn &validator() const { return _validator; } @@ -79,6 +94,16 @@ class flag_registry : public utils::singleton public: void add_flag(const char *name, flag_data flag) { _flags.emplace(name, flag); } + bool update_flag(const char *name, const char *val) + { + auto it = _flags.find(name); + if (it == _flags.end()) { + return false; + } + + return it->second.update(val); + } + void add_validator(const char *name, validator_fn &validator) { auto it = _flags.find(name); From d3a07c09fcdc444559174522454ef6b90887066b Mon Sep 17 00:00:00 2001 From: levy Date: Fri, 4 Dec 2020 14:58:40 +0800 Subject: [PATCH 02/23] fix --- include/dsn/utility/flags.h | 61 ++++++++++++++++++++--------- include/dsn/utility/string_conv.h | 5 +++ src/utils/flags.cpp | 61 +++++++++++++++++++++++------ src/utils/test/string_conv_test.cpp | 46 ++++++++++++++++++++++ 4 files changed, 142 insertions(+), 31 deletions(-) diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index 6c716fb466..56f10882a2 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -23,24 +23,39 @@ #define DSN_DECLARE_bool(name) DSN_DECLARE_VARIABLE(bool, name) #define DSN_DECLARE_string(name) DSN_DECLARE_VARIABLE(const char *, name) -#define DSN_DEFINE_VARIABLE(type, section, name, default_value, desc) \ +#define DSN_DEFINE_VARIABLE(type, section, name, default_value, desc, value_mutable) \ type FLAGS_##name = default_value; \ - static dsn::flag_registerer FLAGS_REG_##name(section, #name, desc, &FLAGS_##name) + static dsn::flag_registerer FLAGS_REG_##name(section, #name, desc, &FLAGS_##name, value_mutable) #define DSN_DEFINE_int32(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(int32_t, section, name, val, desc) + DSN_DEFINE_VARIABLE(int32_t, section, name, val, desc, false) #define DSN_DEFINE_uint32(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(uint32_t, section, name, val, desc) + DSN_DEFINE_VARIABLE(uint32_t, section, name, val, desc, false) #define DSN_DEFINE_int64(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(int64_t, section, name, val, desc) + DSN_DEFINE_VARIABLE(int64_t, section, name, val, desc, false) #define DSN_DEFINE_uint64(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(uint64_t, section, name, val, desc) + DSN_DEFINE_VARIABLE(uint64_t, section, name, val, desc, false) #define DSN_DEFINE_double(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(double, section, name, val, desc) + DSN_DEFINE_VARIABLE(double, section, name, val, desc, false) #define DSN_DEFINE_bool(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(bool, section, name, val, desc) + DSN_DEFINE_VARIABLE(bool, section, name, val, desc, false) #define DSN_DEFINE_string(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(const char *, section, name, val, desc) + DSN_DEFINE_VARIABLE(const char *, section, name, val, desc, false) + +#define DSN_DEFINE_MUTABLE_int32(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(int32_t, section, name, val, desc, true) +#define DSN_DEFINE_MUTABLE_uint32(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(uint32_t, section, name, val, desc, true) +#define DSN_DEFINE_MUTABLE_int64(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(int64_t, section, name, val, desc, true) +#define DSN_DEFINE_MUTABLE_uint64(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(uint64_t, section, name, val, desc, true) +#define DSN_DEFINE_MUTABLE_double(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(double, section, name, val, desc, true) +#define DSN_DEFINE_MUTABLE_bool(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(bool, section, name, val, desc, true) +#define DSN_DEFINE_MUTABLE_string(section, name, val, desc) \ + DSN_DEFINE_VARIABLE(const char *, section, name, val, desc, true) // Convenience macro for the registration of a flag validator. // `validator` must be a std::function and receives the flag value as argument, @@ -52,21 +67,29 @@ dassert(FLAGS_VALIDATOR_FN_##name(FLAGS_##name), "validation failed: %s", #name); \ }) -#define DSN_UPDATE(name, val) flag_registry::instance().update_flag(name, val) - namespace dsn { // An utility class that registers a flag upon initialization. class flag_registerer { public: - flag_registerer(const char *section, const char *name, const char *desc, int32_t *val); - flag_registerer(const char *section, const char *name, const char *desc, uint32_t *val); - flag_registerer(const char *section, const char *name, const char *desc, int64_t *val); - flag_registerer(const char *section, const char *name, const char *desc, uint64_t *val); - flag_registerer(const char *section, const char *name, const char *desc, double *val); - flag_registerer(const char *section, const char *name, const char *desc, bool *val); - flag_registerer(const char *section, const char *name, const char *desc, const char **val); + flag_registerer( + const char *section, const char *name, const char *desc, int32_t *val, bool value_mutable); + flag_registerer( + const char *section, const char *name, const char *desc, uint32_t *val, bool value_mutable); + flag_registerer( + const char *section, const char *name, const char *desc, int64_t *val, bool value_mutable); + flag_registerer( + const char *section, const char *name, const char *desc, uint64_t *val, bool value_mutable); + flag_registerer( + const char *section, const char *name, const char *desc, double *val, bool value_mutable); + flag_registerer( + const char *section, const char *name, const char *desc, bool *val, bool value_mutable); + flag_registerer(const char *section, + const char *name, + const char *desc, + const char **val, + bool value_mutable); }; // An utility class that registers a validator upon initialization. @@ -79,4 +102,6 @@ class flag_validator // Loads all the flags from configuration. extern void flags_initialize(); +extern bool flags_update(const char *name, const char *val); + } // namespace dsn diff --git a/include/dsn/utility/string_conv.h b/include/dsn/utility/string_conv.h index b89c2ddff7..5d2d8b9fa8 100644 --- a/include/dsn/utility/string_conv.h +++ b/include/dsn/utility/string_conv.h @@ -116,6 +116,11 @@ inline bool buf2int64(string_view buf, int64_t &result) return internal::buf2signed(buf, result); } +inline bool buf2uint32(string_view buf, uint32_t &result) +{ + return internal::buf2unsigned(buf, result); +} + inline bool buf2uint64(string_view buf, uint64_t &result) { return internal::buf2unsigned(buf, result); diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index 478e2a4e67..cc258bd534 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -9,6 +9,7 @@ #include #include +#include namespace dsn { @@ -37,6 +38,20 @@ class flag_data } \ break +#define FLAG_DATA_UPDATE_CASE(type, type_enum, suffix) \ + case type_enum: \ + type tmpval_##type_enum; \ + if (!dsn::buf2##suffix(val, tmpval_##type_enum)) { \ + return false; \ + } \ + value() = tmpval_##type_enum; \ + break + +#define FLAG_DATA_UPDATE_STRING(type_enum) \ + case type_enum: \ + strcpy(value(), val); \ + break + void load() { switch (_type) { @@ -50,24 +65,37 @@ class flag_data } } - flag_data(const char *section, const char *name, const char *desc, value_type type, void *val) - : _type(type), _val(val), _section(section), _name(name), _desc(desc) + flag_data(const char *section, + const char *name, + const char *desc, + value_type type, + void *val, + bool value_mutable) + : _type(type), + _val(val), + _section(section), + _name(name), + _desc(desc), + _value_mutable(value_mutable) { } bool update(const char *val) { + if (!_value_mutable) { + return false; + } + switch (_type) { - case FV_INT32: - case FV_INT64: - case FV_UINT32: - case FV_UINT64: - case FV_BOOL: - case FV_DOUBLE: - case FV_STRING: - strcpy(value(), val); - break; + FLAG_DATA_UPDATE_CASE(int32_t, FV_INT32, int32); + FLAG_DATA_UPDATE_CASE(int64_t, FV_INT64, int64); + FLAG_DATA_UPDATE_CASE(uint32_t, FV_UINT32, uint32); + FLAG_DATA_UPDATE_CASE(uint64_t, FV_UINT64, uint64); + FLAG_DATA_UPDATE_CASE(bool, FV_BOOL, bool); + FLAG_DATA_UPDATE_CASE(double, FV_DOUBLE, double); + FLAG_DATA_UPDATE_STRING(FV_STRING); } + return true; } void set_validator(validator_fn &validator) { _validator = std::move(validator); } @@ -86,6 +114,7 @@ class flag_data const char *_section; const char *_name; const char *_desc; + const bool _value_mutable; validator_fn _validator; }; @@ -132,9 +161,10 @@ class flag_registry : public utils::singleton #define FLAG_REG_CONSTRUCTOR(type, type_enum) \ flag_registerer::flag_registerer( \ - const char *section, const char *name, const char *desc, type *val) \ + const char *section, const char *name, const char *desc, type *val, bool value_mutable) \ { \ - flag_registry::instance().add_flag(name, flag_data(section, name, desc, type_enum, val)); \ + flag_registry::instance().add_flag( \ + name, flag_data(section, name, desc, type_enum, val, value_mutable)); \ } FLAG_REG_CONSTRUCTOR(int32_t, FV_INT32); @@ -152,4 +182,9 @@ flag_validator::flag_validator(const char *name, validator_fn validator) /*extern*/ void flags_initialize() { flag_registry::instance().load_from_config(); } +/*extern*/ bool flags_update(const char *name, const char *val) +{ + return flag_registry::instance().update_flag(name, val); +} + } // namespace dsn diff --git a/src/utils/test/string_conv_test.cpp b/src/utils/test/string_conv_test.cpp index 5623de1cb9..01607591dd 100644 --- a/src/utils/test/string_conv_test.cpp +++ b/src/utils/test/string_conv_test.cpp @@ -189,6 +189,52 @@ TEST(string_conv, buf2uint64) ASSERT_FALSE(dsn::buf2uint64(dsn::string_view(str.data(), 5), result)); } +TEST(string_conv, buf2uint32) +{ + uint32_t result = 1; + + ASSERT_TRUE(dsn::buf2uint32(std::to_string(0), result)); + ASSERT_EQ(result, 0); + + ASSERT_TRUE(dsn::buf2uint32("-0", result)); + ASSERT_EQ(result, 0); + + ASSERT_FALSE(dsn::buf2uint32("-1", result)); + + ASSERT_TRUE(dsn::buf2uint32("0xdeadbeef", result)); + ASSERT_EQ(result, 0xdeadbeef); + + ASSERT_TRUE(dsn::buf2uint32("0xDEADBEEF", result)); + ASSERT_EQ(result, 0xdeadbeef); + + ASSERT_TRUE(dsn::buf2uint32(std::to_string(42), result)); + ASSERT_EQ(result, 42); + + ASSERT_TRUE(dsn::buf2uint32(std::to_string(std::numeric_limits::max()), result)); + ASSERT_EQ(result, std::numeric_limits::max()); + + ASSERT_TRUE(dsn::buf2uint32(std::to_string(std::numeric_limits::max()), result)); + ASSERT_EQ(result, std::numeric_limits::max()); + + ASSERT_TRUE(dsn::buf2uint32(std::to_string(std::numeric_limits::max()), result)); + ASSERT_EQ(result, std::numeric_limits::max()); + + ASSERT_TRUE(dsn::buf2uint32(std::to_string(std::numeric_limits::min()), result)); + ASSERT_EQ(result, std::numeric_limits::min()); + + ASSERT_FALSE(dsn::buf2uint32(std::to_string(std::numeric_limits::max()), result)); + + // "\045" is "%", so the string length=5, otherwise(2th argument > 5) it will be reported + // "global-buffer-overflow" error under AddressSanitizer check + std::string str("123\0456", 5); + ASSERT_TRUE(dsn::buf2uint32(dsn::string_view(str.data(), 2), result)); + ASSERT_EQ(result, 12); + ASSERT_TRUE(dsn::buf2uint32(dsn::string_view(str.data(), 3), result)); + ASSERT_EQ(result, 123); + ASSERT_FALSE(dsn::buf2uint32(dsn::string_view(str.data(), 4), result)); + ASSERT_FALSE(dsn::buf2uint32(dsn::string_view(str.data(), 5), result)); +} + TEST(string_conv, int64_partial) { int64_t result = 0; From 8df75be0675f34a4b164468d2aa8240893794e16 Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 7 Dec 2020 10:30:21 +0800 Subject: [PATCH 03/23] fix --- src/utils/flags.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index cc258bd534..d581870d67 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -47,8 +47,8 @@ class flag_data value() = tmpval_##type_enum; \ break -#define FLAG_DATA_UPDATE_STRING(type_enum) \ - case type_enum: \ +#define FLAG_DATA_UPDATE_STRING() \ + case FV_STRING: \ strcpy(value(), val); \ break @@ -93,7 +93,7 @@ class flag_data FLAG_DATA_UPDATE_CASE(uint64_t, FV_UINT64, uint64); FLAG_DATA_UPDATE_CASE(bool, FV_BOOL, bool); FLAG_DATA_UPDATE_CASE(double, FV_DOUBLE, double); - FLAG_DATA_UPDATE_STRING(FV_STRING); + FLAG_DATA_UPDATE_STRING(); } return true; } From 4a3a2c3f28fc74a83b67b9af48f9b701ff80db66 Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 7 Dec 2020 14:36:10 +0800 Subject: [PATCH 04/23] fix --- include/dsn/utility/error_code.h | 1 + include/dsn/utility/flags.h | 2 +- src/http/builtin_http_calls.cpp | 6 ++++++ src/http/builtin_http_calls.h | 2 ++ src/http/config_http_service.cpp | 27 +++++++++++++++++++++++++++ src/utils/flags.cpp | 14 ++++++++------ 6 files changed, 45 insertions(+), 7 deletions(-) create mode 100644 src/http/config_http_service.cpp diff --git a/include/dsn/utility/error_code.h b/include/dsn/utility/error_code.h index b6738db5b5..81a08433b0 100644 --- a/include/dsn/utility/error_code.h +++ b/include/dsn/utility/error_code.h @@ -126,4 +126,5 @@ DEFINE_ERR_CODE(ERR_KRB5_INTERNAL) DEFINE_ERR_CODE(ERR_SASL_INTERNAL) DEFINE_ERR_CODE(ERR_SASL_INCOMPLETE) DEFINE_ERR_CODE(ERR_ACL_DENY) +DEFINE_ERR_CODE(ERR_NO_PERMISSION) } // namespace dsn diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index 56f10882a2..bca2c98943 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -102,6 +102,6 @@ class flag_validator // Loads all the flags from configuration. extern void flags_initialize(); -extern bool flags_update(const char *name, const char *val); +extern bool update_flag(const char *name, const char *val); } // namespace dsn diff --git a/src/http/builtin_http_calls.cpp b/src/http/builtin_http_calls.cpp index 587536b484..b2c8b27893 100644 --- a/src/http/builtin_http_calls.cpp +++ b/src/http/builtin_http_calls.cpp @@ -72,6 +72,12 @@ namespace dsn { get_perf_counter_handler(req, resp); }) .with_help("Gets the value of a perf counter"); + + register_http_call("updateConfig") + .with_callback([](const http_request &req, http_response &resp) { + update_config(req, resp); + }) + .with_help("Updates the value of a config"); } } // namespace dsn diff --git a/src/http/builtin_http_calls.h b/src/http/builtin_http_calls.h index 2cb190a35f..a218569d19 100644 --- a/src/http/builtin_http_calls.h +++ b/src/http/builtin_http_calls.h @@ -32,4 +32,6 @@ extern void get_help_handler(const http_request &req, http_response &resp); extern void get_recent_start_time_handler(const http_request &req, http_response &resp); +extern void update_config(const http_request &req, http_response &resp); + } // namespace dsn diff --git a/src/http/config_http_service.cpp b/src/http/config_http_service.cpp new file mode 100644 index 0000000000..dbd05e7979 --- /dev/null +++ b/src/http/config_http_service.cpp @@ -0,0 +1,27 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include +#include + +namespace dsn { +void update_config(const http_request &req, http_response &resp) { + for (const auto &p : req.query_args) { + update_flag(p.first, p.second); + } +} +} // namespace dsn diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index d581870d67..50e2232c3b 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -5,8 +5,10 @@ #include #include #include +#include #include #include +#include #include #include @@ -42,7 +44,7 @@ class flag_data case type_enum: \ type tmpval_##type_enum; \ if (!dsn::buf2##suffix(val, tmpval_##type_enum)) { \ - return false; \ + return error_s::make(ERR_INVALID_PARAMETERS, fmt::format("{} in invalid", val)); \ } \ value() = tmpval_##type_enum; \ break @@ -80,10 +82,11 @@ class flag_data { } - bool update(const char *val) + error_with update(const char *val) { if (!_value_mutable) { - return false; + return error_s::make(ERR_NO_PERMISSION, + fmt::format("{} is not mutable", _name)); } switch (_type) { @@ -95,7 +98,7 @@ class flag_data FLAG_DATA_UPDATE_CASE(double, FV_DOUBLE, double); FLAG_DATA_UPDATE_STRING(); } - return true; + return fmt::format("{} = {}", _name, _val); } void set_validator(validator_fn &validator) { _validator = std::move(validator); } @@ -129,7 +132,6 @@ class flag_registry : public utils::singleton if (it == _flags.end()) { return false; } - return it->second.update(val); } @@ -182,7 +184,7 @@ flag_validator::flag_validator(const char *name, validator_fn validator) /*extern*/ void flags_initialize() { flag_registry::instance().load_from_config(); } -/*extern*/ bool flags_update(const char *name, const char *val) +/*extern*/ bool update_flag(const char *name, const char *val) { return flag_registry::instance().update_flag(name, val); } From 6b8f82b28efb488bc7e1059d932e0da9241dedc5 Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 7 Dec 2020 17:05:44 +0800 Subject: [PATCH 05/23] fix --- include/dsn/utility/flags.h | 3 ++- src/http/config_http_service.cpp | 19 ++++++++++++++++--- src/utils/flags.cpp | 13 ++++++------- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index bca2c98943..445d18f905 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -7,6 +7,7 @@ #include #include #include +#include "errors.h" // Example: // DSN_DEFINE_string("core", filename, "my_file.txt", "The file to read"); @@ -102,6 +103,6 @@ class flag_validator // Loads all the flags from configuration. extern void flags_initialize(); -extern bool update_flag(const char *name, const char *val); +extern error_s update_flag(const char *name, const char *val); } // namespace dsn diff --git a/src/http/config_http_service.cpp b/src/http/config_http_service.cpp index dbd05e7979..6765c65d40 100644 --- a/src/http/config_http_service.cpp +++ b/src/http/config_http_service.cpp @@ -17,11 +17,24 @@ #include #include +#include namespace dsn { -void update_config(const http_request &req, http_response &resp) { - for (const auto &p : req.query_args) { - update_flag(p.first, p.second); +void update_config(const http_request &req, http_response &resp) +{ + if (req.query_args.size() != 1) { + resp.status_code = http_status_code::bad_request; + return; } + + auto iter = req.query_args.begin(); + auto res = update_flag(iter->first.c_str(), iter->second.c_str()); + + utils::table_printer tp; + tp.add_row_name_and_data("update_status", res.description()); + std::ostringstream out; + tp.output(out, dsn::utils::table_printer::output_format::kJsonCompact); + resp.body = out.str(); + resp.status_code = http_status_code::ok; } } // namespace dsn diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index 50e2232c3b..0431c82ced 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -82,11 +82,10 @@ class flag_data { } - error_with update(const char *val) + error_s update(const char *val) { if (!_value_mutable) { - return error_s::make(ERR_NO_PERMISSION, - fmt::format("{} is not mutable", _name)); + return error_s::make(ERR_NO_PERMISSION, fmt::format("{} is not mutable", _name)); } switch (_type) { @@ -98,7 +97,7 @@ class flag_data FLAG_DATA_UPDATE_CASE(double, FV_DOUBLE, double); FLAG_DATA_UPDATE_STRING(); } - return fmt::format("{} = {}", _name, _val); + return error_s::make(ERR_OK); } void set_validator(validator_fn &validator) { _validator = std::move(validator); } @@ -126,11 +125,11 @@ class flag_registry : public utils::singleton public: void add_flag(const char *name, flag_data flag) { _flags.emplace(name, flag); } - bool update_flag(const char *name, const char *val) + error_s update_flag(const char *name, const char *val) { auto it = _flags.find(name); if (it == _flags.end()) { - return false; + return error_s::make(ERR_OBJECT_NOT_FOUND, fmt::format("{} is not found", name)); } return it->second.update(val); } @@ -184,7 +183,7 @@ flag_validator::flag_validator(const char *name, validator_fn validator) /*extern*/ void flags_initialize() { flag_registry::instance().load_from_config(); } -/*extern*/ bool update_flag(const char *name, const char *val) +/*extern*/ error_s update_flag(const char *name, const char *val) { return flag_registry::instance().update_flag(name, val); } From 1396ec83e833768a72e0d34922bc4b590aa81496 Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 7 Dec 2020 18:54:55 +0800 Subject: [PATCH 06/23] fix --- include/dsn/utility/flags.h | 2 -- src/utils/flags.cpp | 3 +- src/utils/test/flag_test.cpp | 70 ++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 src/utils/test/flag_test.cpp diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index 445d18f905..aa58b16fd0 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -55,8 +55,6 @@ DSN_DEFINE_VARIABLE(double, section, name, val, desc, true) #define DSN_DEFINE_MUTABLE_bool(section, name, val, desc) \ DSN_DEFINE_VARIABLE(bool, section, name, val, desc, true) -#define DSN_DEFINE_MUTABLE_string(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(const char *, section, name, val, desc, true) // Convenience macro for the registration of a flag validator. // `validator` must be a std::function and receives the flag value as argument, diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index 0431c82ced..548cc7e5ce 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -51,8 +51,7 @@ class flag_data #define FLAG_DATA_UPDATE_STRING() \ case FV_STRING: \ - strcpy(value(), val); \ - break + return error_s::make(ERR_NO_PERMISSION, "string modifications are not supported") void load() { diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp new file mode 100644 index 0000000000..70bcc4d393 --- /dev/null +++ b/src/utils/test/flag_test.cpp @@ -0,0 +1,70 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include +#include + +namespace dsn { +namespace utils { +class flag_test : public testing::Test +{ +public: + void SetUp() override { flags_initialize(); } +}; + +TEST_F(flag_test, update_config) +{ + DSN_DEFINE_MUTABLE_int32("flag_test", test_int32, 5, ""); + auto res = update_flag("test_int32", "3"); + ASSERT_EQ(res.is_ok(), true); + ASSERT_EQ(FLAGS_test_int32, 3); + + DSN_DEFINE_MUTABLE_uint32("flag_test", test_uint32, 5, ""); + res = update_flag("test_uint32", "3"); + ASSERT_EQ(res.is_ok(), true); + ASSERT_EQ(FLAGS_test_uint32, 3); + + DSN_DEFINE_MUTABLE_int64("flag_test", test_int64, 5, ""); + res = update_flag("test_int64", "3"); + ASSERT_EQ(res.is_ok(), true); + ASSERT_EQ(FLAGS_test_int64, 3); + + DSN_DEFINE_MUTABLE_uint64("flag_test", test_uint64, 5, ""); + res = update_flag("test_uint64", "3"); + ASSERT_EQ(res.is_ok(), true); + ASSERT_EQ(FLAGS_test_uint64, 3); + + DSN_DEFINE_MUTABLE_double("flag_test", test_double, 5.0, ""); + res = update_flag("test_double", "3.0"); + ASSERT_EQ(res.is_ok(), true); + ASSERT_EQ(FLAGS_test_double, 3.0); + + DSN_DEFINE_MUTABLE_bool("flag_test", test_bool, true, ""); + res = update_flag("test_bool", "false"); + ASSERT_EQ(res.is_ok(), true); + ASSERT_EQ(FLAGS_test_bool, false); + + DSN_DEFINE_string("flag_test", test_string_immutable, "immutable_string", ""); + res = update_flag("test_string_immutable", "update_string"); + ASSERT_EQ(res.code(), ERR_NO_PERMISSION); + ASSERT_EQ(FLAGS_test_string_immutable, "immutable_string"); + + res = update_flag("test_not_exist", "test_string"); + ASSERT_EQ(res.code(), ERR_OBJECT_NOT_FOUND); +} +} // namespace utils +} // namespace dsn From cc93bce9de43ed818c9199c8d4b43570b999daaa Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 7 Dec 2020 19:04:37 +0800 Subject: [PATCH 07/23] fix --- src/utils/test/flag_test.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp index 70bcc4d393..25d43b7575 100644 --- a/src/utils/test/flag_test.cpp +++ b/src/utils/test/flag_test.cpp @@ -58,13 +58,21 @@ TEST_F(flag_test, update_config) ASSERT_EQ(res.is_ok(), true); ASSERT_EQ(FLAGS_test_bool, false); + // string modifications is not supported DSN_DEFINE_string("flag_test", test_string_immutable, "immutable_string", ""); res = update_flag("test_string_immutable", "update_string"); ASSERT_EQ(res.code(), ERR_NO_PERMISSION); ASSERT_EQ(FLAGS_test_string_immutable, "immutable_string"); + // test config is not exist res = update_flag("test_not_exist", "test_string"); ASSERT_EQ(res.code(), ERR_OBJECT_NOT_FOUND); + + // test invalid value + DSN_DEFINE_MUTABLE_int32("flag_test", test_int32_invalid, 5, ""); + res = update_flag("test_int32_invalid", "3ab"); + ASSERT_EQ(res.code(), ERR_INVALID_PARAMETERS); + ASSERT_EQ(FLAGS_test_int32_invalid, 5); } } // namespace utils } // namespace dsn From 40c9c8ba6a76dd36ed6706b8704fe5a360b14907 Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 7 Dec 2020 19:08:37 +0800 Subject: [PATCH 08/23] fix --- src/http/builtin_http_calls.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/http/builtin_http_calls.cpp b/src/http/builtin_http_calls.cpp index b2c8b27893..b1649ffcf8 100644 --- a/src/http/builtin_http_calls.cpp +++ b/src/http/builtin_http_calls.cpp @@ -74,9 +74,8 @@ namespace dsn { .with_help("Gets the value of a perf counter"); register_http_call("updateConfig") - .with_callback([](const http_request &req, http_response &resp) { - update_config(req, resp); - }) + .with_callback( + [](const http_request &req, http_response &resp) { update_config(req, resp); }) .with_help("Updates the value of a config"); } From 02bac723c4f47180bc6d98cfdbf2a7c039fcbea4 Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 7 Dec 2020 19:17:00 +0800 Subject: [PATCH 09/23] fix --- src/utils/flags.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index 548cc7e5ce..1a6a5fe142 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -6,12 +6,12 @@ #include #include #include +#include #include #include #include #include -#include namespace dsn { From bfaa2fdbcb24300fdb456f18e8b71dd020f2e7fa Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 7 Dec 2020 19:17:56 +0800 Subject: [PATCH 10/23] fix --- src/utils/test/flag_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp index 25d43b7575..11d48effa9 100644 --- a/src/utils/test/flag_test.cpp +++ b/src/utils/test/flag_test.cpp @@ -58,7 +58,7 @@ TEST_F(flag_test, update_config) ASSERT_EQ(res.is_ok(), true); ASSERT_EQ(FLAGS_test_bool, false); - // string modifications is not supported + // string modifications are not supported DSN_DEFINE_string("flag_test", test_string_immutable, "immutable_string", ""); res = update_flag("test_string_immutable", "update_string"); ASSERT_EQ(res.code(), ERR_NO_PERMISSION); From 24cec31638afe7e32d0001c58e98801acad3307f Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 7 Dec 2020 19:20:34 +0800 Subject: [PATCH 11/23] fix --- src/utils/test/flag_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp index 11d48effa9..b85b3e8d6a 100644 --- a/src/utils/test/flag_test.cpp +++ b/src/utils/test/flag_test.cpp @@ -68,7 +68,7 @@ TEST_F(flag_test, update_config) res = update_flag("test_not_exist", "test_string"); ASSERT_EQ(res.code(), ERR_OBJECT_NOT_FOUND); - // test invalid value + // test to update invalid value DSN_DEFINE_MUTABLE_int32("flag_test", test_int32_invalid, 5, ""); res = update_flag("test_int32_invalid", "3ab"); ASSERT_EQ(res.code(), ERR_INVALID_PARAMETERS); From 1c6cbb6448f0188bf1a17eaa821a036f5a7681bf Mon Sep 17 00:00:00 2001 From: levy Date: Wed, 9 Dec 2020 19:39:55 +0800 Subject: [PATCH 12/23] fix --- include/dsn/utility/flags.h | 71 ++++++++++++++++-------------------- include/dsn/utility/utils.h | 24 +++++++++++- src/utils/flags.cpp | 42 ++++++++++++--------- src/utils/test/flag_test.cpp | 34 +++++++++++------ 4 files changed, 102 insertions(+), 69 deletions(-) diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index aa58b16fd0..b98694593c 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -8,6 +8,12 @@ #include #include #include "errors.h" +#include "utils.h" + +enum class flag_tag +{ + FT_MUTABLE = 0, +}; // Example: // DSN_DEFINE_string("core", filename, "my_file.txt", "The file to read"); @@ -24,37 +30,24 @@ #define DSN_DECLARE_bool(name) DSN_DECLARE_VARIABLE(bool, name) #define DSN_DECLARE_string(name) DSN_DECLARE_VARIABLE(const char *, name) -#define DSN_DEFINE_VARIABLE(type, section, name, default_value, desc, value_mutable) \ +#define DSN_DEFINE_VARIABLE(type, section, name, default_value, desc) \ type FLAGS_##name = default_value; \ - static dsn::flag_registerer FLAGS_REG_##name(section, #name, desc, &FLAGS_##name, value_mutable) + static dsn::flag_registerer FLAGS_REG_##name(section, #name, desc, &FLAGS_##name) #define DSN_DEFINE_int32(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(int32_t, section, name, val, desc, false) + DSN_DEFINE_VARIABLE(int32_t, section, name, val, desc) #define DSN_DEFINE_uint32(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(uint32_t, section, name, val, desc, false) + DSN_DEFINE_VARIABLE(uint32_t, section, name, val, desc) #define DSN_DEFINE_int64(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(int64_t, section, name, val, desc, false) + DSN_DEFINE_VARIABLE(int64_t, section, name, val, desc) #define DSN_DEFINE_uint64(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(uint64_t, section, name, val, desc, false) + DSN_DEFINE_VARIABLE(uint64_t, section, name, val, desc) #define DSN_DEFINE_double(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(double, section, name, val, desc, false) + DSN_DEFINE_VARIABLE(double, section, name, val, desc) #define DSN_DEFINE_bool(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(bool, section, name, val, desc, false) + DSN_DEFINE_VARIABLE(bool, section, name, val, desc) #define DSN_DEFINE_string(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(const char *, section, name, val, desc, false) - -#define DSN_DEFINE_MUTABLE_int32(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(int32_t, section, name, val, desc, true) -#define DSN_DEFINE_MUTABLE_uint32(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(uint32_t, section, name, val, desc, true) -#define DSN_DEFINE_MUTABLE_int64(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(int64_t, section, name, val, desc, true) -#define DSN_DEFINE_MUTABLE_uint64(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(uint64_t, section, name, val, desc, true) -#define DSN_DEFINE_MUTABLE_double(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(double, section, name, val, desc, true) -#define DSN_DEFINE_MUTABLE_bool(section, name, val, desc) \ - DSN_DEFINE_VARIABLE(bool, section, name, val, desc, true) + DSN_DEFINE_VARIABLE(const char *, section, name, val, desc) // Convenience macro for the registration of a flag validator. // `validator` must be a std::function and receives the flag value as argument, @@ -66,29 +59,23 @@ dassert(FLAGS_VALIDATOR_FN_##name(FLAGS_##name), "validation failed: %s", #name); \ }) +#define DSN_TAG_FLAG(name, tag) \ + COMPILE_ASSERT(sizeof(decltype(FLAGS_##name)), exist_##name##_##tag); \ + static dsn::flag_tagger FLAGS_TAGGER_##name##_##tag(#name, flag_tag::tag) + namespace dsn { // An utility class that registers a flag upon initialization. class flag_registerer { public: - flag_registerer( - const char *section, const char *name, const char *desc, int32_t *val, bool value_mutable); - flag_registerer( - const char *section, const char *name, const char *desc, uint32_t *val, bool value_mutable); - flag_registerer( - const char *section, const char *name, const char *desc, int64_t *val, bool value_mutable); - flag_registerer( - const char *section, const char *name, const char *desc, uint64_t *val, bool value_mutable); - flag_registerer( - const char *section, const char *name, const char *desc, double *val, bool value_mutable); - flag_registerer( - const char *section, const char *name, const char *desc, bool *val, bool value_mutable); - flag_registerer(const char *section, - const char *name, - const char *desc, - const char **val, - bool value_mutable); + flag_registerer(const char *section, const char *name, const char *desc, int32_t *val); + flag_registerer(const char *section, const char *name, const char *desc, uint32_t *val); + flag_registerer(const char *section, const char *name, const char *desc, int64_t *val); + flag_registerer(const char *section, const char *name, const char *desc, uint64_t *val); + flag_registerer(const char *section, const char *name, const char *desc, double *val); + flag_registerer(const char *section, const char *name, const char *desc, bool *val); + flag_registerer(const char *section, const char *name, const char *desc, const char **val); }; // An utility class that registers a validator upon initialization. @@ -98,6 +85,12 @@ class flag_validator flag_validator(const char *name, std::function); }; +class flag_tagger +{ +public: + flag_tagger(const char *name, const flag_tag &tag); +}; + // Loads all the flags from configuration. extern void flags_initialize(); diff --git a/include/dsn/utility/utils.h b/include/dsn/utility/utils.h index c9e414c93d..fa7085c124 100644 --- a/include/dsn/utility/utils.h +++ b/include/dsn/utility/utils.h @@ -43,6 +43,26 @@ #define TIME_MS_MAX 0xffffffff +// The COMPILE_ASSERT macro can be used to verify that a compile time +// expression is true. For example, you could use it to verify the +// size of a static array: +// +// COMPILE_ASSERT(ARRAYSIZE(content_type_names) == CONTENT_NUM_TYPES, +// content_type_names_incorrect_size); +// +// or to make sure a struct is smaller than a certain size: +// +// COMPILE_ASSERT(sizeof(foo) < 128, foo_too_large); +// +// The second argument to the macro is the name of the variable. If +// the expression is false, most compilers will issue a warning/error +// containing the name of the variable. +struct CompileAssert +{ +}; + +#define COMPILE_ASSERT(expr, msg) static const CompileAssert msg[bool(expr) ? 1 : -1] + namespace dsn { namespace utils { @@ -88,5 +108,5 @@ bool hostname(const dsn::rpc_address &address, std::string *hostname_result); // valid_ip_network_order -> return TRUE && hostname_result=hostname | // invalid_ip_network_order -> return FALSE bool hostname_from_ip(uint32_t ip, std::string *hostname_result); -} -} +} // namespace utils +} // namespace dsn diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index 1a6a5fe142..37727129af 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -12,6 +12,7 @@ #include #include +#include namespace dsn { @@ -66,24 +67,14 @@ class flag_data } } - flag_data(const char *section, - const char *name, - const char *desc, - value_type type, - void *val, - bool value_mutable) - : _type(type), - _val(val), - _section(section), - _name(name), - _desc(desc), - _value_mutable(value_mutable) + flag_data(const char *section, const char *name, const char *desc, value_type type, void *val) + : _type(type), _val(val), _section(section), _name(name), _desc(desc) { } error_s update(const char *val) { - if (!_value_mutable) { + if (!has_tag(flag_tag::FT_MUTABLE)) { return error_s::make(ERR_NO_PERMISSION, fmt::format("{} is not mutable", _name)); } @@ -102,6 +93,9 @@ class flag_data void set_validator(validator_fn &validator) { _validator = std::move(validator); } const validator_fn &validator() const { return _validator; } + bool has_tag(const flag_tag &tag) const { return _tags.find(tag) != _tags.end(); } + bool add_tag(const flag_tag &tag) { return _tags.insert(tag).second; } + private: template T &value() @@ -115,8 +109,8 @@ class flag_data const char *_section; const char *_name; const char *_desc; - const bool _value_mutable; validator_fn _validator; + std::unordered_set _tags; }; class flag_registry : public utils::singleton @@ -151,6 +145,16 @@ class flag_registry : public utils::singleton } } + void add_tag(const char *name, const flag_tag &tag) + { + auto it = _flags.find(name); + dassert(it != _flags.end(), "flag \"%s\" does not exist", name); + if (!it->second.add_tag(tag)) { + // TODO(zlw): enum to string + /// ddebug_f("{} was tagged more than once with the tag {}", name, tag); + } + } + private: friend class utils::singleton; flag_registry() = default; @@ -161,10 +165,9 @@ class flag_registry : public utils::singleton #define FLAG_REG_CONSTRUCTOR(type, type_enum) \ flag_registerer::flag_registerer( \ - const char *section, const char *name, const char *desc, type *val, bool value_mutable) \ + const char *section, const char *name, const char *desc, type *val) \ { \ - flag_registry::instance().add_flag( \ - name, flag_data(section, name, desc, type_enum, val, value_mutable)); \ + flag_registry::instance().add_flag(name, flag_data(section, name, desc, type_enum, val)); \ } FLAG_REG_CONSTRUCTOR(int32_t, FV_INT32); @@ -180,6 +183,11 @@ flag_validator::flag_validator(const char *name, validator_fn validator) flag_registry::instance().add_validator(name, validator); } +flag_tagger::flag_tagger(const char *name, const flag_tag &tag) +{ + flag_registry::instance().add_tag(name, tag); +} + /*extern*/ void flags_initialize() { flag_registry::instance().load_from_config(); } /*extern*/ error_s update_flag(const char *name, const char *val) diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp index b85b3e8d6a..1f0023775e 100644 --- a/src/utils/test/flag_test.cpp +++ b/src/utils/test/flag_test.cpp @@ -26,53 +26,65 @@ class flag_test : public testing::Test void SetUp() override { flags_initialize(); } }; +DSN_DEFINE_int32("flag_test", test_int32, 5, ""); +DSN_TAG_FLAG(test_int32, FT_MUTABLE); + +DSN_DEFINE_uint32("flag_test", test_uint32, 5, ""); +DSN_TAG_FLAG(test_uint32, FT_MUTABLE); + +DSN_DEFINE_int64("flag_test", test_int64, 5, ""); +DSN_TAG_FLAG(test_int64, FT_MUTABLE); + +DSN_DEFINE_uint64("flag_test", test_uint64, 5, ""); +DSN_TAG_FLAG(test_uint64, FT_MUTABLE); + +DSN_DEFINE_double("flag_test", test_double, 5.0, ""); +DSN_TAG_FLAG(test_double, FT_MUTABLE); + +DSN_DEFINE_bool("flag_test", test_bool, true, ""); +DSN_TAG_FLAG(test_bool, FT_MUTABLE); + +DSN_DEFINE_string("flag_test", test_string_immutable, "immutable_string", ""); + TEST_F(flag_test, update_config) { - DSN_DEFINE_MUTABLE_int32("flag_test", test_int32, 5, ""); auto res = update_flag("test_int32", "3"); ASSERT_EQ(res.is_ok(), true); ASSERT_EQ(FLAGS_test_int32, 3); - DSN_DEFINE_MUTABLE_uint32("flag_test", test_uint32, 5, ""); res = update_flag("test_uint32", "3"); ASSERT_EQ(res.is_ok(), true); ASSERT_EQ(FLAGS_test_uint32, 3); - DSN_DEFINE_MUTABLE_int64("flag_test", test_int64, 5, ""); res = update_flag("test_int64", "3"); ASSERT_EQ(res.is_ok(), true); ASSERT_EQ(FLAGS_test_int64, 3); - DSN_DEFINE_MUTABLE_uint64("flag_test", test_uint64, 5, ""); res = update_flag("test_uint64", "3"); ASSERT_EQ(res.is_ok(), true); ASSERT_EQ(FLAGS_test_uint64, 3); - DSN_DEFINE_MUTABLE_double("flag_test", test_double, 5.0, ""); res = update_flag("test_double", "3.0"); ASSERT_EQ(res.is_ok(), true); ASSERT_EQ(FLAGS_test_double, 3.0); - DSN_DEFINE_MUTABLE_bool("flag_test", test_bool, true, ""); res = update_flag("test_bool", "false"); ASSERT_EQ(res.is_ok(), true); ASSERT_EQ(FLAGS_test_bool, false); // string modifications are not supported - DSN_DEFINE_string("flag_test", test_string_immutable, "immutable_string", ""); res = update_flag("test_string_immutable", "update_string"); ASSERT_EQ(res.code(), ERR_NO_PERMISSION); - ASSERT_EQ(FLAGS_test_string_immutable, "immutable_string"); + ASSERT_EQ(strcmp(FLAGS_test_string_immutable, "immutable_string"), 0); // test config is not exist res = update_flag("test_not_exist", "test_string"); ASSERT_EQ(res.code(), ERR_OBJECT_NOT_FOUND); // test to update invalid value - DSN_DEFINE_MUTABLE_int32("flag_test", test_int32_invalid, 5, ""); - res = update_flag("test_int32_invalid", "3ab"); + res = update_flag("test_int32", "3ab"); ASSERT_EQ(res.code(), ERR_INVALID_PARAMETERS); - ASSERT_EQ(FLAGS_test_int32_invalid, 5); + ASSERT_EQ(FLAGS_test_int32, 3); } } // namespace utils } // namespace dsn From 101b734d4d53a4df3c3744a7f9fda2cecf4cec4d Mon Sep 17 00:00:00 2001 From: levy Date: Wed, 9 Dec 2020 19:42:00 +0800 Subject: [PATCH 13/23] fix --- src/utils/flags.cpp | 7 ++----- src/utils/test/flag_test.cpp | 5 +++++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index 37727129af..5be5e15c30 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -94,7 +94,7 @@ class flag_data const validator_fn &validator() const { return _validator; } bool has_tag(const flag_tag &tag) const { return _tags.find(tag) != _tags.end(); } - bool add_tag(const flag_tag &tag) { return _tags.insert(tag).second; } + void add_tag(const flag_tag &tag) { _tags.insert(tag); } private: template @@ -149,10 +149,7 @@ class flag_registry : public utils::singleton { auto it = _flags.find(name); dassert(it != _flags.end(), "flag \"%s\" does not exist", name); - if (!it->second.add_tag(tag)) { - // TODO(zlw): enum to string - /// ddebug_f("{} was tagged more than once with the tag {}", name, tag); - } + it->second.add_tag(tag); } private: diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp index 1f0023775e..a9e325d896 100644 --- a/src/utils/test/flag_test.cpp +++ b/src/utils/test/flag_test.cpp @@ -86,5 +86,10 @@ TEST_F(flag_test, update_config) ASSERT_EQ(res.code(), ERR_INVALID_PARAMETERS); ASSERT_EQ(FLAGS_test_int32, 3); } + +TEST_F(flag_test, tag_flag) +{ + // TBD(zlw) +} } // namespace utils } // namespace dsn From 406c65a349180f609aeaad864393dd663b68d189 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 10 Dec 2020 10:11:09 +0800 Subject: [PATCH 14/23] fix --- include/dsn/utility/flags.h | 5 +++- src/utils/flags.cpp | 17 +++++++++-- src/utils/test/flag_test.cpp | 55 ++++++++++++++++++++++-------------- src/utils/test/main.cpp | 3 ++ 4 files changed, 56 insertions(+), 24 deletions(-) diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index b98694593c..e9e18f4619 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -59,10 +59,12 @@ enum class flag_tag dassert(FLAGS_VALIDATOR_FN_##name(FLAGS_##name), "validation failed: %s", #name); \ }) -#define DSN_TAG_FLAG(name, tag) \ +#define DSN_TAG_VARIABLE(name, tag) \ COMPILE_ASSERT(sizeof(decltype(FLAGS_##name)), exist_##name##_##tag); \ static dsn::flag_tagger FLAGS_TAGGER_##name##_##tag(#name, flag_tag::tag) +#define DSN_HAS_TAG(name, tag) has_tag(#name, flag_tag::tag) + namespace dsn { // An utility class that registers a flag upon initialization. @@ -96,4 +98,5 @@ extern void flags_initialize(); extern error_s update_flag(const char *name, const char *val); +extern error_with has_tag(const char *name, const flag_tag &tag); } // namespace dsn diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index 5be5e15c30..3e5be78fc5 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -12,7 +12,6 @@ #include #include -#include namespace dsn { @@ -93,8 +92,8 @@ class flag_data void set_validator(validator_fn &validator) { _validator = std::move(validator); } const validator_fn &validator() const { return _validator; } - bool has_tag(const flag_tag &tag) const { return _tags.find(tag) != _tags.end(); } void add_tag(const flag_tag &tag) { _tags.insert(tag); } + bool has_tag(const flag_tag &tag) const { return _tags.find(tag) != _tags.end(); } private: template @@ -152,6 +151,15 @@ class flag_registry : public utils::singleton it->second.add_tag(tag); } + error_with has_tag(const char *name, const flag_tag &tag) const + { + auto it = _flags.find(name); + if (it == _flags.end()) { + return error_s::make(ERR_OBJECT_NOT_FOUND, fmt::format("{} is not found", name)); + } + return it->second.has_tag(tag); + } + private: friend class utils::singleton; flag_registry() = default; @@ -192,4 +200,9 @@ flag_tagger::flag_tagger(const char *name, const flag_tag &tag) return flag_registry::instance().update_flag(name, val); } +/*extern*/ error_with has_tag(const char *name, const flag_tag &tag) +{ + return flag_registry::instance().has_tag(name, tag); +} + } // namespace dsn diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp index a9e325d896..4201d8c5b0 100644 --- a/src/utils/test/flag_test.cpp +++ b/src/utils/test/flag_test.cpp @@ -20,57 +20,52 @@ namespace dsn { namespace utils { -class flag_test : public testing::Test -{ -public: - void SetUp() override { flags_initialize(); } -}; DSN_DEFINE_int32("flag_test", test_int32, 5, ""); -DSN_TAG_FLAG(test_int32, FT_MUTABLE); +DSN_TAG_VARIABLE(test_int32, FT_MUTABLE); DSN_DEFINE_uint32("flag_test", test_uint32, 5, ""); -DSN_TAG_FLAG(test_uint32, FT_MUTABLE); +DSN_TAG_VARIABLE(test_uint32, FT_MUTABLE); DSN_DEFINE_int64("flag_test", test_int64, 5, ""); -DSN_TAG_FLAG(test_int64, FT_MUTABLE); +DSN_TAG_VARIABLE(test_int64, FT_MUTABLE); DSN_DEFINE_uint64("flag_test", test_uint64, 5, ""); -DSN_TAG_FLAG(test_uint64, FT_MUTABLE); +DSN_TAG_VARIABLE(test_uint64, FT_MUTABLE); DSN_DEFINE_double("flag_test", test_double, 5.0, ""); -DSN_TAG_FLAG(test_double, FT_MUTABLE); +DSN_TAG_VARIABLE(test_double, FT_MUTABLE); DSN_DEFINE_bool("flag_test", test_bool, true, ""); -DSN_TAG_FLAG(test_bool, FT_MUTABLE); +DSN_TAG_VARIABLE(test_bool, FT_MUTABLE); DSN_DEFINE_string("flag_test", test_string_immutable, "immutable_string", ""); -TEST_F(flag_test, update_config) +TEST(flag_test, update_config) { auto res = update_flag("test_int32", "3"); - ASSERT_EQ(res.is_ok(), true); + ASSERT_TRUE(res.is_ok()); ASSERT_EQ(FLAGS_test_int32, 3); res = update_flag("test_uint32", "3"); - ASSERT_EQ(res.is_ok(), true); + ASSERT_TRUE(res.is_ok()); ASSERT_EQ(FLAGS_test_uint32, 3); res = update_flag("test_int64", "3"); - ASSERT_EQ(res.is_ok(), true); + ASSERT_TRUE(res.is_ok()); ASSERT_EQ(FLAGS_test_int64, 3); res = update_flag("test_uint64", "3"); - ASSERT_EQ(res.is_ok(), true); + ASSERT_TRUE(res.is_ok()); ASSERT_EQ(FLAGS_test_uint64, 3); res = update_flag("test_double", "3.0"); - ASSERT_EQ(res.is_ok(), true); + ASSERT_TRUE(res.is_ok()); ASSERT_EQ(FLAGS_test_double, 3.0); res = update_flag("test_bool", "false"); - ASSERT_EQ(res.is_ok(), true); - ASSERT_EQ(FLAGS_test_bool, false); + ASSERT_TRUE(res.is_ok()); + ASSERT_FALSE(FLAGS_test_bool); // string modifications are not supported res = update_flag("test_string_immutable", "update_string"); @@ -87,9 +82,27 @@ TEST_F(flag_test, update_config) ASSERT_EQ(FLAGS_test_int32, 3); } -TEST_F(flag_test, tag_flag) +DSN_DEFINE_int32("flag_test", has_tag, 5, ""); +DSN_TAG_VARIABLE(has_tag, FT_MUTABLE); + +DSN_DEFINE_int32("flag_test", no_tag, 5, ""); + +TEST(flag_test, tag_flag) { - // TBD(zlw) + // has tag + auto res = DSN_HAS_TAG(has_tag, FT_MUTABLE); + ASSERT_TRUE(res.is_ok()); + ASSERT_TRUE(res.get_value()); + + // doesn't has tag + res = DSN_HAS_TAG(no_tag, FT_MUTABLE); + ASSERT_TRUE(res.is_ok()); + ASSERT_FALSE(res.get_value()); + + // flag is not exist + res = DSN_HAS_TAG(no_flag, FT_MUTABLE); + ASSERT_FALSE(res.is_ok()); + ASSERT_EQ(res.get_error().code(), ERR_OBJECT_NOT_FOUND); } } // namespace utils } // namespace dsn diff --git a/src/utils/test/main.cpp b/src/utils/test/main.cpp index d64376a3a4..075b224a1c 100644 --- a/src/utils/test/main.cpp +++ b/src/utils/test/main.cpp @@ -1,6 +1,7 @@ #include #include #include +#include extern void command_manager_module_init(); @@ -12,5 +13,7 @@ GTEST_API_ int main(int argc, char **argv) // init logging dsn_log_init("dsn::tools::simple_logger", "./", nullptr); + dsn::flags_initialize(); + return RUN_ALL_TESTS(); } From af2319d6a72d9f2fbfa5ca50293397edb218b553 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 10 Dec 2020 11:27:04 +0800 Subject: [PATCH 15/23] fix --- include/dsn/utility/flags.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index e9e18f4619..7dfc7de347 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -12,7 +12,7 @@ enum class flag_tag { - FT_MUTABLE = 0, + FT_MUTABLE = 0, /** flag data is mutable */ }; // Example: From 39c015aa88de8afea6a8dea3ba242f4deb8fccba Mon Sep 17 00:00:00 2001 From: levy Date: Tue, 15 Dec 2020 17:00:04 +0800 Subject: [PATCH 16/23] fix --- include/dsn/utility/flags.h | 2 -- src/utils/test/flag_test.cpp | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index 7dfc7de347..dd30524f70 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -63,8 +63,6 @@ enum class flag_tag COMPILE_ASSERT(sizeof(decltype(FLAGS_##name)), exist_##name##_##tag); \ static dsn::flag_tagger FLAGS_TAGGER_##name##_##tag(#name, flag_tag::tag) -#define DSN_HAS_TAG(name, tag) has_tag(#name, flag_tag::tag) - namespace dsn { // An utility class that registers a flag upon initialization. diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp index 4201d8c5b0..5f2bb6d068 100644 --- a/src/utils/test/flag_test.cpp +++ b/src/utils/test/flag_test.cpp @@ -90,17 +90,17 @@ DSN_DEFINE_int32("flag_test", no_tag, 5, ""); TEST(flag_test, tag_flag) { // has tag - auto res = DSN_HAS_TAG(has_tag, FT_MUTABLE); + auto res = has_tag("has_tag", flag_tag::FT_MUTABLE); ASSERT_TRUE(res.is_ok()); ASSERT_TRUE(res.get_value()); // doesn't has tag - res = DSN_HAS_TAG(no_tag, FT_MUTABLE); + res = has_tag("no_tag", flag_tag::FT_MUTABLE); ASSERT_TRUE(res.is_ok()); ASSERT_FALSE(res.get_value()); // flag is not exist - res = DSN_HAS_TAG(no_flag, FT_MUTABLE); + res = has_tag("no_flag", flag_tag::FT_MUTABLE); ASSERT_FALSE(res.is_ok()); ASSERT_EQ(res.get_error().code(), ERR_OBJECT_NOT_FOUND); } From e5ab8237e7953aa8121c766cf8bd2c2ee04b3cce Mon Sep 17 00:00:00 2001 From: levy Date: Tue, 15 Dec 2020 17:02:07 +0800 Subject: [PATCH 17/23] fix --- src/utils/test/flag_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp index 5f2bb6d068..b1b59b1db5 100644 --- a/src/utils/test/flag_test.cpp +++ b/src/utils/test/flag_test.cpp @@ -72,7 +72,7 @@ TEST(flag_test, update_config) ASSERT_EQ(res.code(), ERR_NO_PERMISSION); ASSERT_EQ(strcmp(FLAGS_test_string_immutable, "immutable_string"), 0); - // test config is not exist + // test flag is not exist res = update_flag("test_not_exist", "test_string"); ASSERT_EQ(res.code(), ERR_OBJECT_NOT_FOUND); From f41964df408d87caa567e04fc974b2247bd33f01 Mon Sep 17 00:00:00 2001 From: levy Date: Wed, 16 Dec 2020 14:48:58 +0800 Subject: [PATCH 18/23] fix --- include/dsn/utility/flags.h | 6 ++++-- src/http/config_http_service.cpp | 2 +- src/utils/flags.cpp | 12 ++++++------ src/utils/test/flag_test.cpp | 9 +++------ 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index dd30524f70..037bc7e1dc 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -94,7 +94,9 @@ class flag_tagger // Loads all the flags from configuration. extern void flags_initialize(); -extern error_s update_flag(const char *name, const char *val); +// update the specified flag to val +extern error_s update_flag(const std::string &name, const std::string &val); -extern error_with has_tag(const char *name, const flag_tag &tag); +// determine if the tag is exist for the specified flag +extern bool has_tag(const std::string &name, const flag_tag &tag); } // namespace dsn diff --git a/src/http/config_http_service.cpp b/src/http/config_http_service.cpp index 6765c65d40..620b4b7a62 100644 --- a/src/http/config_http_service.cpp +++ b/src/http/config_http_service.cpp @@ -28,7 +28,7 @@ void update_config(const http_request &req, http_response &resp) } auto iter = req.query_args.begin(); - auto res = update_flag(iter->first.c_str(), iter->second.c_str()); + auto res = update_flag(iter->first, iter->second); utils::table_printer tp; tp.add_row_name_and_data("update_status", res.description()); diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index 3e5be78fc5..a6b47d4258 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -71,7 +71,7 @@ class flag_data { } - error_s update(const char *val) + error_s update(const std::string &val) { if (!has_tag(flag_tag::FT_MUTABLE)) { return error_s::make(ERR_NO_PERMISSION, fmt::format("{} is not mutable", _name)); @@ -117,7 +117,7 @@ class flag_registry : public utils::singleton public: void add_flag(const char *name, flag_data flag) { _flags.emplace(name, flag); } - error_s update_flag(const char *name, const char *val) + error_s update_flag(const std::string &name, const std::string &val) { auto it = _flags.find(name); if (it == _flags.end()) { @@ -151,11 +151,11 @@ class flag_registry : public utils::singleton it->second.add_tag(tag); } - error_with has_tag(const char *name, const flag_tag &tag) const + bool has_tag(const std::string &name, const flag_tag &tag) const { auto it = _flags.find(name); if (it == _flags.end()) { - return error_s::make(ERR_OBJECT_NOT_FOUND, fmt::format("{} is not found", name)); + return false; } return it->second.has_tag(tag); } @@ -195,12 +195,12 @@ flag_tagger::flag_tagger(const char *name, const flag_tag &tag) /*extern*/ void flags_initialize() { flag_registry::instance().load_from_config(); } -/*extern*/ error_s update_flag(const char *name, const char *val) +/*extern*/ error_s update_flag(const std::string &name, const std::string &val) { return flag_registry::instance().update_flag(name, val); } -/*extern*/ error_with has_tag(const char *name, const flag_tag &tag) +/*extern*/ bool has_tag(const std::string &name, const flag_tag &tag) { return flag_registry::instance().has_tag(name, tag); } diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp index b1b59b1db5..7b2f3a80f0 100644 --- a/src/utils/test/flag_test.cpp +++ b/src/utils/test/flag_test.cpp @@ -91,18 +91,15 @@ TEST(flag_test, tag_flag) { // has tag auto res = has_tag("has_tag", flag_tag::FT_MUTABLE); - ASSERT_TRUE(res.is_ok()); - ASSERT_TRUE(res.get_value()); + ASSERT_TRUE(res); // doesn't has tag res = has_tag("no_tag", flag_tag::FT_MUTABLE); - ASSERT_TRUE(res.is_ok()); - ASSERT_FALSE(res.get_value()); + ASSERT_FALSE(res); // flag is not exist res = has_tag("no_flag", flag_tag::FT_MUTABLE); - ASSERT_FALSE(res.is_ok()); - ASSERT_EQ(res.get_error().code(), ERR_OBJECT_NOT_FOUND); + ASSERT_FALSE(res); } } // namespace utils } // namespace dsn From 41d44b4150dc4a616731cc9969a49563c60283cb Mon Sep 17 00:00:00 2001 From: levy Date: Fri, 18 Dec 2020 13:23:46 +0800 Subject: [PATCH 19/23] fix --- include/dsn/utility/error_code.h | 1 - src/utils/flags.cpp | 4 ++-- src/utils/test/flag_test.cpp | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/include/dsn/utility/error_code.h b/include/dsn/utility/error_code.h index 81a08433b0..b6738db5b5 100644 --- a/include/dsn/utility/error_code.h +++ b/include/dsn/utility/error_code.h @@ -126,5 +126,4 @@ DEFINE_ERR_CODE(ERR_KRB5_INTERNAL) DEFINE_ERR_CODE(ERR_SASL_INTERNAL) DEFINE_ERR_CODE(ERR_SASL_INCOMPLETE) DEFINE_ERR_CODE(ERR_ACL_DENY) -DEFINE_ERR_CODE(ERR_NO_PERMISSION) } // namespace dsn diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index a6b47d4258..bf84e3ff0e 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -51,7 +51,7 @@ class flag_data #define FLAG_DATA_UPDATE_STRING() \ case FV_STRING: \ - return error_s::make(ERR_NO_PERMISSION, "string modifications are not supported") + return error_s::make(ERR_INVALID_PARAMETERS, "string modifications are not supported") void load() { @@ -74,7 +74,7 @@ class flag_data error_s update(const std::string &val) { if (!has_tag(flag_tag::FT_MUTABLE)) { - return error_s::make(ERR_NO_PERMISSION, fmt::format("{} is not mutable", _name)); + return error_s::make(ERR_INVALID_PARAMETERS, fmt::format("{} is not mutable", _name)); } switch (_type) { diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp index 7b2f3a80f0..f67c3adaea 100644 --- a/src/utils/test/flag_test.cpp +++ b/src/utils/test/flag_test.cpp @@ -69,7 +69,7 @@ TEST(flag_test, update_config) // string modifications are not supported res = update_flag("test_string_immutable", "update_string"); - ASSERT_EQ(res.code(), ERR_NO_PERMISSION); + ASSERT_EQ(res.code(), ERR_INVALID_PARAMETERS); ASSERT_EQ(strcmp(FLAGS_test_string_immutable, "immutable_string"), 0); // test flag is not exist From c87db041fb3820b75543cd4e98511a20eb46d72f Mon Sep 17 00:00:00 2001 From: levy Date: Fri, 18 Dec 2020 19:16:46 +0800 Subject: [PATCH 20/23] fix --- src/runtime/security/negotiation_manager.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/runtime/security/negotiation_manager.cpp b/src/runtime/security/negotiation_manager.cpp index 520bb8a022..30645d3b21 100644 --- a/src/runtime/security/negotiation_manager.cpp +++ b/src/runtime/security/negotiation_manager.cpp @@ -44,7 +44,6 @@ negotiation_map negotiation_manager::_negotiations; utils::rw_lock_nr negotiation_manager::_lock; negotiation_manager::negotiation_manager() : serverlet("negotiation_manager") {} - void negotiation_manager::open_service() { register_rpc_handler_with_rpc_holder( From b308b0bd1a78e196c54a3abc1b0406b8edd2b016 Mon Sep 17 00:00:00 2001 From: levy Date: Fri, 18 Dec 2020 19:16:50 +0800 Subject: [PATCH 21/23] fix --- src/runtime/security/negotiation_manager.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/runtime/security/negotiation_manager.cpp b/src/runtime/security/negotiation_manager.cpp index 30645d3b21..520bb8a022 100644 --- a/src/runtime/security/negotiation_manager.cpp +++ b/src/runtime/security/negotiation_manager.cpp @@ -44,6 +44,7 @@ negotiation_map negotiation_manager::_negotiations; utils::rw_lock_nr negotiation_manager::_lock; negotiation_manager::negotiation_manager() : serverlet("negotiation_manager") {} + void negotiation_manager::open_service() { register_rpc_handler_with_rpc_holder( From 8840fa2d7c3ec372bf1ed896da3a63959dc12cc8 Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 21 Dec 2020 11:39:26 +0800 Subject: [PATCH 22/23] fix --- src/runtime/security/negotiation_manager.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/runtime/security/negotiation_manager.cpp b/src/runtime/security/negotiation_manager.cpp index 520bb8a022..b25f1e56aa 100644 --- a/src/runtime/security/negotiation_manager.cpp +++ b/src/runtime/security/negotiation_manager.cpp @@ -81,6 +81,7 @@ void negotiation_manager::on_negotiation_response(error_code err, negotiation_rp } } + void negotiation_manager::on_rpc_connected(rpc_session *session) { std::shared_ptr nego = security::create_negotiation(session->is_client(), session); From a5fb19361a761c67835f5a15c7e90546c2332fec Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 21 Dec 2020 11:39:30 +0800 Subject: [PATCH 23/23] fix --- src/runtime/security/negotiation_manager.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/runtime/security/negotiation_manager.cpp b/src/runtime/security/negotiation_manager.cpp index b25f1e56aa..520bb8a022 100644 --- a/src/runtime/security/negotiation_manager.cpp +++ b/src/runtime/security/negotiation_manager.cpp @@ -81,7 +81,6 @@ void negotiation_manager::on_negotiation_response(error_code err, negotiation_rp } } - void negotiation_manager::on_rpc_connected(rpc_session *session) { std::shared_ptr nego = security::create_negotiation(session->is_client(), session);