fix: pre_update_config is executed one more time than expected

apps/emqx/src/emqx_config_handler.erl

@@ -194,11 +194,8 @@ do_update_config([ConfKey | SubConfKeyPath], Handlers, OldRawConf,
     SubOldRawConf = get_sub_config(ConfKeyBin, OldRawConf),
     SubHandlers = get_sub_handlers(ConfKey, Handlers),
     case do_update_config(SubConfKeyPath, SubHandlers, SubOldRawConf, UpdateReq, ConfKeyPath) of
-        {ok, NewUpdateReq} ->
+        {ok, NewUpdateReq} -> merge_to_old_config(#{ConfKeyBin => NewUpdateReq}, OldRawConf);
-            call_pre_config_update(Handlers, OldRawConf, #{ConfKeyBin => NewUpdateReq},
+        Error -> Error
-                ConfKeyPath);
-        Error ->
-            Error
     end.
 
 check_and_save_configs(SchemaModule, ConfKeyPath, Handlers, NewRawConf, OverrideConf,
@@ -287,6 +284,7 @@ save_configs(ConfKeyPath, AppEnvs, CheckedConf, NewRawConf, OverrideConf, Update
 %%   1. the old config is undefined
 %%   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) ->
     {ok, maps:merge(RawConf, UpdateReq)};
 merge_to_old_config(UpdateReq, _RawConf) ->

apps/emqx/src/emqx_map_lib.erl

@@ -152,7 +152,7 @@ diff_maps(NewMap, OldMap) ->
 binary_string_kv(K, V, JsonableFun) ->
     case JsonableFun(K, V) of
         drop -> drop;
-        {K1, V1} -> {binary_string(K1), binary_string(V1)}
+        {K1, V1} -> {binary_string(K1), V1}
     end.
 
 binary_string([]) -> [];

apps/emqx/test/emqx_config_handler_SUITE.erl

@@ -129,8 +129,18 @@ t_sub_key_update_remove(_Config) ->
     %% remove
     ?assertEqual({ok,#{post_config_update => #{emqx_config_handler_SUITE => ok}}},
         emqx:remove_config(KeyPath)),
-    ?assertError({config_not_found, [sysmon,os, cpu_check_interval]},
+    ?assertError({config_not_found, KeyPath}, emqx:get_raw_config(KeyPath)),
-        emqx:get_raw_config(KeyPath)),
+    OSKey = maps:keys(emqx:get_raw_config([sysmon, os])),
+    ?assertEqual(false, lists:member(<<"cpu_check_interval">>, OSKey)),
+    ?assert(length(OSKey) > 0),
+
+    ?assertEqual({ok,#{config => 60000,
+        post_config_update => #{?MODULE => ok},
+        raw_config => <<"60s">>}}, emqx:reset_config(KeyPath, Opts)),
+    OSKey1 = maps:keys(emqx:get_raw_config([sysmon, os])),
+    ?assertEqual(true, lists:member(<<"cpu_check_interval">>, OSKey1)),
+    ?assert(length(OSKey1) > 1),
+
     ok = emqx_config_handler:remove_handler(KeyPath),
     ok = emqx_config_handler:remove_handler(KeyPath2),
     ok.
@@ -193,6 +203,7 @@ t_handler_root() ->
         raw_config := #{<<"os">> := #{<<"cpu_check_interval">> := <<"12s">>}}},
         Res),
     ?assertMatch(#{sysmon := #{os := #{cpu_check_interval := 12000}}}, emqx:get_config(RootKey)),
+    ok = emqx_config_handler:remove_handler(RootKey),
     ok.
 
 t_get_raw_cluster_override_conf(_Config) ->
@@ -215,9 +226,36 @@ t_get_raw_cluster_override_conf(_Config) ->
 t_save_config_failed(_Config) ->
     ok.
 
+t_update_sub(_Config) ->
+    PathKey = [sysmon],
+    Opts = #{rawconf_with_defaults => true},
+    ok = emqx_config_handler:add_handler(PathKey, ?MODULE),
+    %% update sub key
+    #{<<"os">> := OS1} = emqx:get_raw_config(PathKey),
+    {ok, Res} = emqx:update_config(PathKey ++ [os, cpu_check_interval], <<"120s">>, Opts),
+    ?assertMatch(#{config := 120000,
+        post_config_update := #{?MODULE := ok},
+        raw_config := <<"120s">>},
+        Res),
+    ?assertMatch(#{os := #{cpu_check_interval := 120000}}, emqx:get_config(PathKey)),
+    #{<<"os">> := OS2} = emqx:get_raw_config(PathKey),
+    ?assertEqual(lists:sort(maps:keys(OS1)), lists:sort(maps:keys(OS2))),
 
-pre_config_update([sysmon], UpdateReq, RawConf) ->
+    %% update sub key
-    {ok, emqx_map_lib:deep_merge(RawConf, UpdateReq)};
+    SubKey = PathKey ++ [os, cpu_high_watermark],
+    ?assertEqual({ok,#{config => 0.81,
+        post_config_update => #{?MODULE => ok},
+        raw_config => <<"81%">>}},
+        emqx:update_config(SubKey, "81%", Opts#{merge => shallow})),
+    ?assertEqual(0.81, emqx:get_config(SubKey)),
+    ?assertEqual("81%", emqx:get_raw_config(SubKey)),
+
+    ok = emqx_config_handler:remove_handler(PathKey),
+    ok.
+
+
+pre_config_update([sysmon], UpdateReq, _RawConf) ->
+    {ok, UpdateReq};
 pre_config_update([sysmon, os], UpdateReq, _RawConf) ->
     {ok, UpdateReq};
 pre_config_update([sysmon, os, cpu_check_interval], UpdateReq, _RawConf) ->