refactor(emqx_config): call MOD:post_config_update/3 after checking hocon schema

apps/emqx/src/emqx_config.erl

@@ -19,9 +19,10 @@
 
 -export([ init_load/2
         , read_override_conf/0
-        , save_configs/3
+        , check_config/2
+        , save_configs/4
         , save_to_app_env/1
-        , save_to_emqx_config/2
+        , save_to_config_map/2
         , save_to_override_conf/1
         ]).
 
@@ -53,7 +54,6 @@
         , remove/2
         ]).
 
-%% raw configs is the config that is now parsed and tranlated by hocon schema
 -export([ get_raw/1
         , get_raw/2
         , put_raw/1
@@ -67,8 +67,11 @@
 
 -export_type([update_request/0, raw_config/0, config/0]).
 -type update_request() :: term().
+%% raw_config() is the config that is NOT parsed and tranlated by hocon schema
 -type raw_config() :: #{binary() => term()} | undefined.
+%% config() is the config that is parsed and tranlated by hocon schema
 -type config() :: #{atom() => term()} | undefined.
+-type app_envs() :: [proplists:property()].
 
 %% @doc For the given path, get root value enclosed in a single-key map.
 -spec get_root(emqx_map_lib:config_key_path()) -> map().
@@ -198,27 +201,36 @@ init_load(SchemaModule, RawRichConf) when is_map(RawRichConf) ->
              nullable => true
             },
     %% this call throws exception in case of check failure
-    {_MappedEnvs, CheckedConf} =
+    {_AppEnvs, CheckedConf} = check_config(SchemaModule, RawConf, Opts),
-        hocon_schema:map_translate(SchemaModule, RawRichConf, Opts),
+    ok = save_to_config_map(CheckedConf, RawConf).
-    ok = save_to_emqx_config(CheckedConf, RawConf).
+
+-spec check_config(module(), raw_config()) -> {AppEnvs, CheckedConf}
+    when AppEnvs :: app_envs(), CheckedConf :: config().
+check_config(SchemaModule, RawConf) ->
+    Opts = #{return_plain => true,
+             nullable => true,
+             is_richmap => false
+            },
+    check_config(SchemaModule, RawConf, Opts).
+
+-spec check_config(module(), raw_config(), map()) -> {AppEnvs, CheckedConf}
+    when AppEnvs :: app_envs(), CheckedConf :: config().
+check_config(SchemaModule, RawConf, Opts) ->
+    {AppEnvs, CheckedConf} =
+        hocon_schema:map_translate(SchemaModule, RawConf, Opts),
+    Conf = maps:with(maps:keys(RawConf), CheckedConf),
+    {AppEnvs, emqx_map_lib:unsafe_atom_key_map(Conf)}.
 
 -spec read_override_conf() -> raw_config().
 read_override_conf() ->
     load_hocon_file(emqx_override_conf_name(), map).
 
--spec save_configs(module(), raw_config(), raw_config()) -> ok | {error, term()}.
+-spec save_configs(app_envs(), raw_config(), config(), raw_config()) -> ok | {error, term()}.
-save_configs(SchemaModule, RawConf, OverrideConf) ->
+save_configs(_AppEnvs, RawConf, Conf, OverrideConf) ->
-    Opts = #{return_plain => true,
-             nullable => true,
-             is_richmap => false
-            },
-    {_MappedEnvs, CheckedConf} =
-        hocon_schema:map_translate(SchemaModule, RawConf, Opts),
     %% We may need also support hot config update for the apps that use application envs.
-    %% If that is the case uncomment the following line to update the configs to application env
+    %% If that is the case uncomment the following line to update the configs to app env
-    %save_to_app_env(_MappedEnvs),
+    %save_to_app_env(AppEnvs),
-    Conf = maps:with(maps:keys(RawConf), CheckedConf),
+    save_to_config_map(Conf, RawConf),
-    save_to_emqx_config(Conf, RawConf),
     %% TODO: merge RawConf to OverrideConf can be done here
     save_to_override_conf(OverrideConf).
 
@@ -228,9 +240,9 @@ save_to_app_env(AppEnvs) ->
             [application:set_env(AppName, Par, Val) || {Par, Val} <- Envs]
         end, AppEnvs).
 
--spec save_to_emqx_config(config(), raw_config()) -> ok.
+-spec save_to_config_map(config(), raw_config()) -> ok.
-save_to_emqx_config(Conf, RawConf) ->
+save_to_config_map(Conf, RawConf) ->
-    ?MODULE:put(emqx_map_lib:unsafe_atom_key_map(Conf)),
+    ?MODULE:put(Conf),
     ?MODULE:put_raw(RawConf).
 
 -spec save_to_override_conf(raw_config()) -> ok | {error, term()}.

apps/emqx/src/emqx_config_handler.erl

@@ -38,18 +38,20 @@
          code_change/3]).
 
 -define(MOD, {mod}).
+-define(REMOVE_CONF, '$remove_config').
 
 -type handler_name() :: module().
 -type handlers() :: #{emqx_config:config_key() => handlers(), ?MOD => handler_name()}.
 
 -optional_callbacks([ pre_config_update/2
-                    , post_config_update/2
+                    , post_config_update/3
                     ]).
 
 -callback pre_config_update(emqx_config:update_request(), emqx_config:raw_config()) ->
     emqx_config:update_request().
 
--callback post_config_update(emqx_config:config(), emqx_config:config()) -> any().
+-callback post_config_update(emqx_config:update_request(), emqx_config:config(),
+    emqx_config:config()) -> ok | {error, term()}.
 
 -type state() :: #{
     handlers := handlers(),
@@ -61,13 +63,13 @@ start_link() ->
 
 -spec update_config(module(), emqx_config:config_key_path(), emqx_config:update_request()) ->
     ok | {error, term()}.
-update_config(SchemaModule, ConfKeyPath, UpdateReq) ->
+update_config(SchemaModule, ConfKeyPath, UpdateReq) when UpdateReq =/= ?REMOVE_CONF ->
-    gen_server:call(?MODULE, {update_config, SchemaModule, ConfKeyPath, UpdateReq}).
+    gen_server:call(?MODULE, {change_config, SchemaModule, ConfKeyPath, UpdateReq}).
 
 -spec remove_config(module(), emqx_config:config_key_path()) ->
     ok | {error, term()}.
 remove_config(SchemaModule, ConfKeyPath) ->
-    gen_server:call(?MODULE, {remove_config, SchemaModule, ConfKeyPath}).
+    gen_server:call(?MODULE, {change_config, SchemaModule, ConfKeyPath, ?REMOVE_CONF}).
 
 -spec add_handler(emqx_config:config_key_path(), handler_name()) -> ok.
 add_handler(ConfKeyPath, HandlerName) ->
@@ -84,35 +86,21 @@ handle_call({add_child, ConfKeyPath, HandlerName}, _From,
     {reply, ok, State#{handlers =>
         emqx_map_lib:deep_put(ConfKeyPath, Handlers, #{?MOD => HandlerName})}};
 
-handle_call({update_config, SchemaModule, ConfKeyPath, UpdateReq}, _From,
+handle_call({change_config, SchemaModule, ConfKeyPath, UpdateReq}, _From,
             #{handlers := Handlers} = State) ->
     OldConf = emqx_config:get_root(ConfKeyPath),
     OldRawConf = emqx_config:get_root_raw(ConfKeyPath),
-    try NewRawConf = do_update_config(ConfKeyPath, Handlers, OldRawConf, UpdateReq),
+    Result = try
-        OverrideConf = update_override_config(NewRawConf),
+        {NewRawConf, OverrideConf} = process_upadate_request(ConfKeyPath, OldRawConf,
-        Result = emqx_config:save_configs(SchemaModule, NewRawConf, OverrideConf),
+            Handlers, UpdateReq),
-        do_post_config_update(ConfKeyPath, Handlers, OldConf, emqx_config:get_root(ConfKeyPath)),
+        {AppEnvs, CheckedConf} = emqx_config:check_config(SchemaModule, NewRawConf),
-        {reply, Result, State}
+        do_post_config_update(ConfKeyPath, Handlers, OldConf, CheckedConf, UpdateReq),
-    catch
+        emqx_config:save_configs(AppEnvs, NewRawConf, CheckedConf, OverrideConf)
-        Error : Reason : ST ->
+    catch Error:Reason:ST ->
-            ?LOG(error, "update config failed: ~p", [{Error, Reason, ST}]),
+        ?LOG(error, "change_config failed: ~p", [{Error, Reason, ST}]),
-            {reply, {error, Reason}, State}
+        {error, Reason}
-    end;
+    end,
-
+    {reply, Result, State};
-handle_call({remove_config, SchemaModule, ConfKeyPath}, _From, #{handlers := Handlers} = State) ->
-    OldConf = emqx_config:get_root(ConfKeyPath),
-    OldRawConf = emqx_config:get_root_raw(ConfKeyPath),
-    BinKeyPath = bin_path(ConfKeyPath),
-    try NewRawConf = emqx_map_lib:deep_remove(BinKeyPath, OldRawConf),
-        OverrideConf = emqx_map_lib:deep_remove(BinKeyPath, emqx_config:read_override_conf()),
-        Result = emqx_config:save_configs(SchemaModule, NewRawConf, OverrideConf),
-        do_post_config_update(ConfKeyPath, Handlers, OldConf, emqx_config:get_root(ConfKeyPath)),
-        {reply, Result, State}
-    catch
-        Error : Reason : ST ->
-            ?LOG(error, "update config failed: ~p", [{Error, Reason, ST}]),
-            {reply, {error, Reason}, State}
-    end;
 
 handle_call(_Request, _From, State) ->
     Reply = ok,
@@ -130,6 +118,16 @@ terminate(_Reason, _State) ->
 code_change(_OldVsn, State, _Extra) ->
     {ok, State}.
 
+process_upadate_request(ConfKeyPath, OldRawConf, _Handlers, ?REMOVE_CONF) ->
+    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, UpdateReq) ->
+    NewRawConf = do_update_config(ConfKeyPath, Handlers, OldRawConf, UpdateReq),
+    OverrideConf = update_override_config(NewRawConf),
+    {NewRawConf, OverrideConf}.
+
 do_update_config([], Handlers, OldRawConf, UpdateReq) ->
     call_pre_config_update(Handlers, OldRawConf, UpdateReq);
 do_update_config([ConfKey | ConfKeyPath], Handlers, OldRawConf, UpdateReq) ->
@@ -138,14 +136,14 @@ do_update_config([ConfKey | ConfKeyPath], Handlers, OldRawConf, UpdateReq) ->
     NewUpdateReq = do_update_config(ConfKeyPath, SubHandlers, SubOldRawConf, UpdateReq),
     call_pre_config_update(Handlers, OldRawConf, #{bin(ConfKey) => NewUpdateReq}).
 
-do_post_config_update([], Handlers, OldConf, NewConf) ->
+do_post_config_update([], Handlers, OldConf, NewConf, UpdateReq) ->
-    call_post_config_update(Handlers, OldConf, NewConf);
+    call_post_config_update(Handlers, OldConf, NewConf, UpdateReq);
-do_post_config_update([ConfKey | ConfKeyPath], Handlers, OldConf, NewConf) ->
+do_post_config_update([ConfKey | ConfKeyPath], Handlers, OldConf, NewConf, UpdateReq) ->
     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),
+    _ = do_post_config_update(ConfKeyPath, SubHandlers, SubOldConf, SubNewConf, UpdateReq),
-    call_post_config_update(Handlers, OldConf, NewConf).
+    call_post_config_update(Handlers, OldConf, NewConf, UpdateReq).
 
 get_sub_config(ConfKey, Conf) when is_map(Conf) ->
     maps:get(ConfKey, Conf, undefined);
@@ -159,10 +157,10 @@ call_pre_config_update(Handlers, OldRawConf, UpdateReq) ->
         false -> merge_to_old_config(UpdateReq, OldRawConf)
     end.
 
-call_post_config_update(Handlers, OldConf, NewConf) ->
+call_post_config_update(Handlers, OldConf, NewConf, UpdateReq) ->
     HandlerName = maps:get(?MOD, Handlers, undefined),
-    case erlang:function_exported(HandlerName, post_config_update, 2) of
+    case erlang:function_exported(HandlerName, post_config_update, 3) of
-        true -> _ = HandlerName:post_config_update(NewConf, OldConf);
+        true -> HandlerName:post_config_update(UpdateReq, NewConf, OldConf);
         false -> ok
     end.