From 24207b80cb59ac2b7659f63b9a2eb410cc39d953 Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Tue, 17 Aug 2021 17:25:19 +0800 Subject: [PATCH] refactor(config): improve the return value of emqx:update_config/2,3 --- apps/emqx/src/emqx_config.erl | 10 ++- apps/emqx/src/emqx_config_handler.erl | 102 ++++++++++++++++++-------- 2 files changed, 79 insertions(+), 33 deletions(-) diff --git a/apps/emqx/src/emqx_config.erl b/apps/emqx/src/emqx_config.erl index 1815ad66d..d5bf1a61e 100644 --- a/apps/emqx/src/emqx_config.erl +++ b/apps/emqx/src/emqx_config.erl @@ -82,7 +82,8 @@ end). -export_type([update_request/0, raw_config/0, config/0, - update_opts/0, update_cmd/0, update_args/0]). + update_opts/0, update_cmd/0, update_args/0, + update_error/0, update_result/0]). -type update_request() :: term(). -type update_cmd() :: {update, update_request()} | remove. @@ -91,6 +92,13 @@ rawconf_with_defaults => boolean() }. -type update_args() :: {update_cmd(), Opts :: update_opts()}. +-type update_stage() :: pre_config_update | post_config_update. +-type update_error() :: {update_stage(), module(), term()} | {save_configs, term()} | term(). +-type update_result() :: #{ + config := emqx_config:config(), + raw_config := emqx_config:raw_config(), + post_config_update => #{module() => any()} +}. %% raw_config() is the config that is NOT parsed and tranlated by hocon schema -type raw_config() :: #{binary() => term()} | undefined. diff --git a/apps/emqx/src/emqx_config_handler.erl b/apps/emqx/src/emqx_config_handler.erl index ee03ff4ef..ac21afaa1 100644 --- a/apps/emqx/src/emqx_config_handler.erl +++ b/apps/emqx/src/emqx_config_handler.erl @@ -46,10 +46,10 @@ ]). -callback pre_config_update(emqx_config:update_request(), emqx_config:raw_config()) -> - emqx_config:update_request(). + {ok, emqx_config:update_request()} | {error, term()}. -callback post_config_update(emqx_config:update_request(), emqx_config:config(), - emqx_config:config()) -> any(). + emqx_config:config()) -> ok | {ok, Result::any()} | {error, Reason::term()}. -type state() :: #{ handlers := handlers(), @@ -60,7 +60,7 @@ start_link() -> gen_server:start_link({local, ?MODULE}, ?MODULE, {}, []). -spec update_config(module(), emqx_config:config_key_path(), emqx_config:update_args()) -> - {ok, emqx_config:config(), emqx_config:raw_config()} | {error, term()}. + {ok, emqx_config:update_result()} | {error, emqx_config:update_error()}. update_config(SchemaModule, ConfKeyPath, UpdateArgs) -> gen_server:call(?MODULE, {change_config, SchemaModule, ConfKeyPath, UpdateArgs}). @@ -79,24 +79,23 @@ handle_call({add_child, ConfKeyPath, HandlerName}, _From, {reply, ok, State#{handlers => emqx_map_lib:deep_put(ConfKeyPath, Handlers, #{?MOD => HandlerName})}}; -handle_call({change_config, SchemaModule, ConfKeyPath, {_Cmd, Opts} = UpdateArgs}, _From, +handle_call({change_config, SchemaModule, ConfKeyPath, UpdateArgs}, _From, #{handlers := Handlers} = State) -> OldConf = emqx_config:get([]), OldRawConf = emqx_config:get_raw([]), - Result = try - {NewRawConf, OverrideConf} = process_upadate_request(ConfKeyPath, OldRawConf, - Handlers, UpdateArgs), - {AppEnvs, CheckedConf} = emqx_config:check_config(SchemaModule, NewRawConf), - _ = do_post_config_update(ConfKeyPath, Handlers, OldConf, CheckedConf, UpdateArgs), - case emqx_config:save_configs(AppEnvs, CheckedConf, NewRawConf, OverrideConf) of - ok -> {ok, emqx_config:get([]), return_rawconf(Opts)}; - Err -> Err + Reply = try + case process_update_request(ConfKeyPath, OldRawConf, Handlers, UpdateArgs) of + {ok, NewRawConf, OverrideConf} -> + check_and_save_configs(SchemaModule, ConfKeyPath, Handlers, NewRawConf, OldConf, + OverrideConf, UpdateArgs); + {error, Result} -> + {error, Result} end catch Error:Reason:ST -> ?LOG(error, "change_config failed: ~p", [{Error, Reason, ST}]), {error, Reason} end, - {reply, Result, State}; + {reply, Reply, State}; handle_call(_Request, _From, State) -> Reply = ok, @@ -114,32 +113,56 @@ terminate(_Reason, _State) -> code_change(_OldVsn, State, _Extra) -> {ok, State}. -process_upadate_request(ConfKeyPath, OldRawConf, _Handlers, {remove, _Opts}) -> +process_update_request(ConfKeyPath, OldRawConf, _Handlers, {remove, _Opts}) -> BinKeyPath = bin_path(ConfKeyPath), NewRawConf = emqx_map_lib:deep_remove(BinKeyPath, OldRawConf), OverrideConf = emqx_map_lib:deep_remove(BinKeyPath, emqx_config:read_override_conf()), - {NewRawConf, OverrideConf}; -process_upadate_request(ConfKeyPath, OldRawConf, Handlers, {{update, UpdateReq}, _Opts}) -> - NewRawConf = do_update_config(ConfKeyPath, Handlers, OldRawConf, UpdateReq), - OverrideConf = update_override_config(NewRawConf), - {NewRawConf, OverrideConf}. + {ok, NewRawConf, OverrideConf}; +process_update_request(ConfKeyPath, OldRawConf, Handlers, {{update, UpdateReq}, _Opts}) -> + case do_update_config(ConfKeyPath, Handlers, OldRawConf, UpdateReq) of + {ok, NewRawConf} -> + OverrideConf = update_override_config(NewRawConf), + {ok, NewRawConf, OverrideConf}; + Error -> Error + end. do_update_config([], Handlers, OldRawConf, UpdateReq) -> call_pre_config_update(Handlers, OldRawConf, UpdateReq); do_update_config([ConfKey | ConfKeyPath], Handlers, OldRawConf, UpdateReq) -> SubOldRawConf = get_sub_config(bin(ConfKey), OldRawConf), SubHandlers = maps:get(ConfKey, Handlers, #{}), - NewUpdateReq = do_update_config(ConfKeyPath, SubHandlers, SubOldRawConf, UpdateReq), - call_pre_config_update(Handlers, OldRawConf, #{bin(ConfKey) => NewUpdateReq}). + case do_update_config(ConfKeyPath, SubHandlers, SubOldRawConf, UpdateReq) of + {ok, NewUpdateReq} -> + call_pre_config_update(Handlers, OldRawConf, #{bin(ConfKey) => NewUpdateReq}); + Error -> + Error + end. -do_post_config_update([], Handlers, OldConf, NewConf, UpdateArgs) -> - call_post_config_update(Handlers, OldConf, NewConf, up_req(UpdateArgs)); -do_post_config_update([ConfKey | ConfKeyPath], Handlers, OldConf, NewConf, UpdateArgs) -> +check_and_save_configs(SchemaModule, ConfKeyPath, Handlers, NewRawConf, OldConf, OverrideConf, + UpdateArgs) -> + {AppEnvs, CheckedConf} = emqx_config:check_config(SchemaModule, NewRawConf), + case do_post_config_update(ConfKeyPath, Handlers, OldConf, CheckedConf, UpdateArgs, #{}) of + {ok, Result0} -> + case save_configs(AppEnvs, CheckedConf, NewRawConf, OverrideConf, UpdateArgs) of + {ok, Result1} -> + {ok, Result1#{post_config_update => Result0}}; + Error -> Error + end; + Error -> Error + end. + +do_post_config_update([], Handlers, OldConf, NewConf, UpdateArgs, Result) -> + call_post_config_update(Handlers, OldConf, NewConf, up_req(UpdateArgs), Result); +do_post_config_update([ConfKey | ConfKeyPath], Handlers, OldConf, NewConf, UpdateArgs, Result) -> SubOldConf = get_sub_config(ConfKey, OldConf), SubNewConf = get_sub_config(ConfKey, NewConf), SubHandlers = maps:get(ConfKey, Handlers, #{}), - _ = do_post_config_update(ConfKeyPath, SubHandlers, SubOldConf, SubNewConf, UpdateArgs), - call_post_config_update(Handlers, OldConf, NewConf, up_req(UpdateArgs)). + case do_post_config_update(ConfKeyPath, SubHandlers, SubOldConf, SubNewConf, UpdateArgs, + Result) of + {ok, Result1} -> + call_post_config_update(Handlers, OldConf, NewConf, up_req(UpdateArgs), Result1); + Error -> Error + end. get_sub_config(ConfKey, Conf) when is_map(Conf) -> maps:get(ConfKey, Conf, undefined); @@ -149,15 +172,30 @@ get_sub_config(_, _Conf) -> %% the Conf is a primitive call_pre_config_update(Handlers, OldRawConf, UpdateReq) -> HandlerName = maps:get(?MOD, Handlers, undefined), case erlang:function_exported(HandlerName, pre_config_update, 2) of - true -> HandlerName:pre_config_update(UpdateReq, OldRawConf); + true -> + case HandlerName:pre_config_update(UpdateReq, OldRawConf) of + {ok, NewUpdateReq} -> {ok, NewUpdateReq}; + {error, Reason} -> {error, {pre_config_update, HandlerName, Reason}} + end; false -> merge_to_old_config(UpdateReq, OldRawConf) end. -call_post_config_update(Handlers, OldConf, NewConf, UpdateReq) -> +call_post_config_update(Handlers, OldConf, NewConf, UpdateReq, Result) -> HandlerName = maps:get(?MOD, Handlers, undefined), case erlang:function_exported(HandlerName, post_config_update, 3) of - true -> HandlerName:post_config_update(UpdateReq, NewConf, OldConf); - false -> ok + true -> + case HandlerName:post_config_update(UpdateReq, NewConf, OldConf) of + ok -> {ok, Result}; + {ok, Result1} -> {ok, Result#{HandlerName => Result1}}; + {error, Reason} -> {error, {post_config_update, HandlerName, Reason}} + end; + false -> {ok, Result} + end. + +save_configs(AppEnvs, CheckedConf, NewRawConf, OverrideConf, {_Cmd, Opts}) -> + case emqx_config:save_configs(AppEnvs, CheckedConf, NewRawConf, OverrideConf) of + ok -> {ok, #{config => emqx_config:get([]), raw_config => return_rawconf(Opts)}}; + {error, Reason} -> {error, {save_configs, Reason}} end. %% The default callback of config handlers @@ -166,9 +204,9 @@ call_post_config_update(Handlers, OldConf, NewConf, UpdateReq) -> %% 2. either the old or the new config is not of map type %% the behaviour is merging the new the config to the old config if they are maps. merge_to_old_config(UpdateReq, RawConf) when is_map(UpdateReq), is_map(RawConf) -> - maps:merge(RawConf, UpdateReq); + {ok, maps:merge(RawConf, UpdateReq)}; merge_to_old_config(UpdateReq, _RawConf) -> - UpdateReq. + {ok, UpdateReq}. update_override_config(RawConf) -> OldConf = emqx_config:read_override_conf(),