Merge pull request #12235 from zhongwencool/ft-config-update-api

fix: ft config update api return raw config

apps/emqx_ft/src/emqx_ft.app.src

@@ -1,6 +1,6 @@
 {application, emqx_ft, [
     {description, "EMQX file transfer over MQTT"},
-    {vsn, "0.1.11"},
+    {vsn, "0.1.12"},
     {registered, []},
     {mod, {emqx_ft_app, []}},
     {applications, [

apps/emqx_ft/src/emqx_ft_api.erl

@@ -125,7 +125,8 @@ schema("/file_transfer") ->
             responses => #{
                 200 => ?SCHEMA_CONFIG,
                 400 => emqx_dashboard_swagger:error_codes(
-                    ['INVALID_CONFIG'], error_desc('INVALID_CONFIG')
+                    ['UPDATE_FAILED', 'INVALID_CONFIG'],
+                    error_desc('INVALID_CONFIG')
                 )
             }
         }
@@ -175,19 +176,9 @@ check_ft_enabled(Params, _Meta) ->
             {503, error_msg('SERVICE_UNAVAILABLE')}
     end.
 
-'/file_transfer'(get, _Meta) ->
+%% Forward /file_transfer to /configs/file_transfer
-    {200, format_config(emqx_ft_conf:get())};
+'/file_transfer'(Method, Data) ->
-'/file_transfer'(put, #{body := ConfigIn}) ->
+    emqx_mgmt_api_configs:request_config([<<"file_transfer">>], Method, Data).
-    OldConf = emqx_ft_conf:get_raw(),
-    UpdateConf = emqx_utils:deobfuscate(ConfigIn, OldConf),
-    case emqx_ft_conf:update(UpdateConf) of
-        {ok, #{config := Config}} ->
-            {200, format_config(Config)};
-        {error, Error = #{kind := validation_error}} ->
-            {400, error_msg('INVALID_CONFIG', format_validation_error(Error))};
-        {error, Error} ->
-            {400, error_msg('INVALID_CONFIG', emqx_utils:format(Error))}
-    end.
 
 format_page(#{items := Files, cursor := Cursor}) ->
     #{
@@ -199,17 +190,6 @@ format_page(#{items := Files}) ->
         <<"files">> => lists:map(fun format_file_info/1, Files)
     }.
 
-format_config(Config) ->
-    Schema = emqx_hocon:make_schema(emqx_ft_schema:fields(file_transfer)),
-    hocon_tconf:make_serializable(
-        Schema,
-        emqx_utils_maps:binary_key_map(Config),
-        #{obfuscate_sensitive_values => true}
-    ).
-
-format_validation_error(Error) ->
-    emqx_logger_jsonfmt:best_effort_json(Error).
-
 error_msg(Code) ->
     #{code => Code, message => error_desc(Code)}.
 

apps/emqx_ft/src/emqx_ft_conf.erl

@@ -36,7 +36,6 @@
 -export([
     load/0,
     unload/0,
-    get/0,
     update/1,
     get_raw/0
 ]).
@@ -109,10 +108,6 @@ unload() ->
     ok = emqx_conf:remove_handler([file_transfer]),
     maybe_stop().
 
--spec get() -> emqx_config:config().
-get() ->
-    emqx_config:get([file_transfer]).
-
 get_raw() ->
     emqx:get_raw_config([file_transfer], #{}).
 

apps/emqx_ft/test/emqx_ft_api_SUITE.erl

@@ -277,17 +277,37 @@ t_ft_disabled(Config) ->
         )
     ).
 
-t_configure_1(Config) ->
+t_configure_file_transfer(Config) ->
     Uri = uri(["file_transfer"]),
     test_configure(Uri, Config).
 
-t_configure_2(Config) ->
+t_configure_config_file_transfer(Config) ->
     Uri = uri(["configs/file_transfer"]),
     test_configure(Uri, Config).
 
 test_configure(Uri, Config) ->
     ?assertMatch(
-        {ok, 200, #{<<"enable">> := true, <<"storage">> := #{}}},
+        {ok, 200, #{
+            <<"enable">> := true,
+            <<"storage">> :=
+                #{
+                    <<"local">> :=
+                        #{
+                            <<"enable">> := true,
+                            <<"segments">> :=
+                                #{
+                                    <<"gc">> :=
+                                        #{
+                                            %% Match keep the raw conf
+                                            %% 1h is not change to 3600000
+                                            <<"interval">> := <<"1h">>,
+                                            <<"maximum_segments_ttl">> := <<"24h">>,
+                                            <<"minimum_segments_ttl">> := <<"5m">>
+                                        }
+                                }
+                        }
+                }
+        }},
         request_json(get, Uri, Config)
     ),
     ?assertMatch(
@@ -298,14 +318,43 @@ test_configure(Uri, Config) ->
         {ok, 200, #{<<"enable">> := false}},
         request_json(get, Uri, Config)
     ),
+    Storage0 = emqx_ft_test_helpers:local_storage(Config),
+    Storage = emqx_utils_maps:deep_put(
+        [
+            <<"local">>,
+            <<"segments">>,
+            <<"gc">>,
+            <<"maximum_segments_ttl">>
+        ],
+        Storage0,
+        <<"10m">>
+    ),
     ?assertMatch(
-        {ok, 200, #{}},
+        {ok, 200, #{
+            <<"storage">> :=
+                #{
+                    <<"local">> :=
+                        #{
+                            <<"segments">> :=
+                                #{
+                                    <<"gc">> :=
+                                        #{
+                                            <<"interval">> := <<"1h">>,
+                                            %% Match keep the raw conf
+                                            %% 10m is not change to 600,000
+                                            <<"maximum_segments_ttl">> := <<"10m">>,
+                                            <<"minimum_segments_ttl">> := <<"5m">>
+                                        }
+                                }
+                        }
+                }
+        }},
         request_json(
             put,
             Uri,
             #{
                 <<"enable">> => true,
-                <<"storage">> => emqx_ft_test_helpers:local_storage(Config)
+                <<"storage">> => Storage
             },
             Config
         )
@@ -537,7 +586,14 @@ reset_ft_config(Config, Enable) ->
             <<"enable">> => Enable,
             <<"storage">> => #{
                 <<"local">> => #{
-                    <<"enable">> => true
+                    <<"enable">> => true,
+                    <<"segments">> => #{
+                        <<"gc">> => #{
+                            <<"interval">> => <<"1h">>,
+                            <<"maximum_segments_ttl">> => "24h",
+                            <<"minimum_segments_ttl">> => "5m"
+                        }
+                    }
                 }
             }
         },

apps/emqx_management/src/emqx_mgmt_api_configs.erl

@@ -29,8 +29,10 @@
     configs/3,
     get_full_config/0,
     global_zone_configs/3,
-    limiter/3
+    limiter/3,
+    get_raw_config/1
 ]).
+-export([request_config/3]).
 
 -define(PREFIX, "/configs/").
 -define(PREFIX_RESET, "/configs_reset/").
@@ -289,35 +291,22 @@ fields(Field) ->
 %% HTTP API Callbacks
 config(Method, Data, Req) ->
     Path = conf_path(Req),
-    do_config(Path, Method, Data).
+    request_config(Path, Method, Data).
 
-do_config([<<"file_transfer">> | Path], Method, Data) ->
+request_config([ConfigRoot | Path], get, _Params) ->
-    [] =/= Path andalso throw("file_transfer does no support deep config get/put"),
-    forward_file_transfer(Method, Data);
-do_config([ConfigRoot | Path], get, _Params) ->
     [] =/= Path andalso throw("deep config get is not supported"),
     {200, get_raw_config(ConfigRoot)};
-do_config(Path, put, #{body := NewConf}) ->
+request_config(Path, put, #{body := NewConf}) ->
-    case emqx_conf:update(Path, NewConf, ?OPTS) of
+    OldConf = emqx:get_raw_config(Path, #{}),
+    UpdateConf = emqx_utils:deobfuscate(NewConf, OldConf),
+    case emqx_conf:update(Path, UpdateConf, ?OPTS) of
         {ok, #{raw_config := RawConf}} ->
-            {200, RawConf};
+            [ConfigRoot] = Path,
+            {200, obfuscate_raw_config(ConfigRoot, RawConf)};
         {error, Reason} ->
             {400, #{code => 'UPDATE_FAILED', message => ?ERR_MSG(Reason)}}
     end.
 
-%% @private file_transfer config update reside in this module
-%% because it's needed to generate hotconf schema for dashboard UI rendering.
-%% As a result of adding "file_transfer" root key to the root keys list
-%% there is also a configs/file_transfer http path handler to be implemented.
-%% Here we simply forward the call to the file_transfer API module.
--if(?EMQX_RELEASE_EDITION == ee).
-forward_file_transfer(Method, Data) ->
-    emqx_ft_api:'/file_transfer'(Method, Data).
--else.
-forward_file_transfer(_Method, _Data) ->
-    {400, #{code => 'BAD_REQUEST', message => <<"not supported">>}}.
--endif.
-
 global_zone_configs(get, _Params, _Req) ->
     {200, get_zones()};
 global_zone_configs(put, #{body := Body}, _Req) ->
@@ -474,9 +463,12 @@ get_full_config() ->
     ).
 
 get_raw_config(Path) ->
+    obfuscate_raw_config(Path, emqx:get_raw_config([Path])).
+
+obfuscate_raw_config(Path, Raw) ->
     #{Path := Conf} =
         emqx_config:fill_defaults(
-            #{Path => emqx:get_raw_config([Path])},
+            #{Path => Raw},
             #{obfuscate_sensitive_values => true}
         ),
     Conf.