Merge pull request #7516 from savonarola/emqx_mgmt_api_nodes-cov

chore(emqx_mangement): improve emqx_mgmt_api_nodes coverage

apps/emqx_dashboard/test/emqx_dashboard_api_test_helpers.erl

@@ -48,7 +48,9 @@ request(Method, Url, Body) ->
 
 request(Username, Method, Url, Body) ->
     Request = case Body of
-        [] -> {Url, [auth_header(Username)]};
+        [] when Method =:= get orelse Method =:= put orelse
+                Method =:= head orelse Method =:= delete orelse
+                Method =:= trace -> {Url, [auth_header(Username)]};
         _ -> {Url, [auth_header(Username)], "application/json", jsx:encode(Body)}
     end,
     ct:pal("Method: ~p, Request: ~p", [Method, Request]),

apps/emqx_management/src/emqx_mgmt_api_nodes.erl

@@ -237,9 +237,8 @@ log_path() ->
     Configs = logger:get_handler_config(),
     get_log_path(Configs).
 
-get_log_path([#{id := file} = LoggerConfig | _LoggerConfigs]) ->
+get_log_path([#{config := #{file := Path}} | _LoggerConfigs]) ->
-    Config = maps:get(config, LoggerConfig),
+    Path;
-    maps:get(file, Config);
 get_log_path([_LoggerConfig | LoggerConfigs]) ->
     get_log_path(LoggerConfigs);
 get_log_path([]) ->

apps/emqx_management/test/emqx_mgmt_api_nodes_SUITE.erl

@@ -24,11 +24,31 @@ all() ->
     emqx_common_test_helpers:all(?MODULE).
 
 init_per_suite(Config) ->
-    emqx_mgmt_api_test_util:init_suite(),
+    emqx_mgmt_api_test_util:init_suite([emqx_conf]),
     Config.
 
 end_per_suite(_) ->
-    emqx_mgmt_api_test_util:end_suite().
+    emqx_mgmt_api_test_util:end_suite([emqx_conf]).
+
+init_per_testcase(t_log_path, Config) ->
+    emqx_config_logger:add_handler(),
+    Log = emqx_conf:get_raw([log], #{}),
+    File = "log/emqx-test.log",
+    Log1 = emqx_map_lib:deep_put([<<"file_handlers">>, <<"default">>, <<"enable">>], Log, true),
+    Log2 = emqx_map_lib:deep_put([<<"file_handlers">>, <<"default">>, <<"file">>], Log1, File),
+    {ok, #{}} = emqx_conf:update([log], Log2, #{rawconf_with_defaults => true}),
+    Config;
+init_per_testcase(_, Config) ->
+    Config.
+
+end_per_testcase(t_log_path, Config) ->
+    Log = emqx_conf:get_raw([log], #{}),
+    Log1 = emqx_map_lib:deep_put([<<"file_handlers">>, <<"default">>, <<"enable">>], Log, false),
+    {ok, #{}} = emqx_conf:update([log], Log1, #{rawconf_with_defaults => true}),
+    emqx_config_logger:remove_handler(),
+    Config;
+end_per_testcase(_, Config) ->
+    Config.
 
 t_nodes_api(_) ->
     NodesPath = emqx_mgmt_api_test_util:api_path(["nodes"]),
@@ -42,9 +62,22 @@ t_nodes_api(_) ->
     {ok, NodeInfo} = emqx_mgmt_api_test_util:request_api(get, NodePath),
     NodeNameResponse =
         binary_to_atom(maps:get(<<"node">>, emqx_json:decode(NodeInfo, [return_maps])), utf8),
-    ?assertEqual(node(), NodeNameResponse).
+    ?assertEqual(node(), NodeNameResponse),
 
-t_node_stats_api() ->
+    BadNodePath = emqx_mgmt_api_test_util:api_path(["nodes", "badnode"]),
+    ?assertMatch(
+        {error, {_, 400, _}},
+        emqx_mgmt_api_test_util:request_api(get, BadNodePath)).
+
+t_log_path(_) ->
+    NodePath = emqx_mgmt_api_test_util:api_path(["nodes", atom_to_list(node())]),
+    {ok, NodeInfo} = emqx_mgmt_api_test_util:request_api(get, NodePath),
+    #{<<"log_path">> := Path} = emqx_json:decode(NodeInfo, [return_maps]),
+    ?assertEqual(
+       <<"emqx-test.log">>,
+       filename:basename(Path)).
+
+t_node_stats_api(_) ->
     StatsPath = emqx_mgmt_api_test_util:api_path(["nodes", atom_to_binary(node(), utf8), "stats"]),
     SystemStats= emqx_mgmt:get_stats(),
     {ok, StatsResponse} = emqx_mgmt_api_test_util:request_api(get, StatsPath),
@@ -53,9 +86,14 @@ t_node_stats_api() ->
         fun(Key) ->
             ?assertEqual(maps:get(Key, SystemStats), maps:get(atom_to_binary(Key, utf8), Stats))
         end,
-    lists:foreach(Fun, maps:keys(SystemStats)).
+    lists:foreach(Fun, maps:keys(SystemStats)),
 
-t_node_metrics_api() ->
+    BadNodePath = emqx_mgmt_api_test_util:api_path(["nodes", "badnode", "stats"]),
+    ?assertMatch(
+        {error, {_, 400, _}},
+        emqx_mgmt_api_test_util:request_api(get, BadNodePath)).
+
+t_node_metrics_api(_) ->
     MetricsPath =
         emqx_mgmt_api_test_util:api_path(["nodes", atom_to_binary(node(), utf8), "metrics"]),
     SystemMetrics= emqx_mgmt:get_metrics(),
@@ -65,4 +103,9 @@ t_node_metrics_api() ->
         fun(Key) ->
             ?assertEqual(maps:get(Key, SystemMetrics), maps:get(atom_to_binary(Key, utf8), Metrics))
         end,
-    lists:foreach(Fun, maps:keys(SystemMetrics)).
+    lists:foreach(Fun, maps:keys(SystemMetrics)),
+
+    BadNodePath = emqx_mgmt_api_test_util:api_path(["nodes", "badnode", "metrics"]),
+    ?assertMatch(
+        {error, {_, 400, _}},
+        emqx_mgmt_api_test_util:request_api(get, BadNodePath)).

apps/emqx_modules/src/emqx_modules_app.erl

@@ -46,7 +46,7 @@ maybe_enable_modules() ->
         }
     ),
     DelayedEnabled andalso emqx_delayed:enable(),
-    emqx_modules_conf:telemetry_status() andalso emqx_telemetry:enable(),
+    emqx_modules_conf:is_telemetry_enabled() andalso emqx_telemetry:enable(),
     emqx_conf:get([observer_cli, enable], true) andalso emqx_observer_cli:enable(),
     emqx_conf_cli:load(),
     ok = emqx_rewrite:enable(),
@@ -55,7 +55,7 @@ maybe_enable_modules() ->
 
 maybe_disable_modules() ->
     emqx_conf:get([delayed, enable], true) andalso emqx_delayed:disable(),
-    emqx_modules_conf:telemetry_status() andalso emqx_telemetry:disable(),
+    emqx_modules_conf:is_telemetry_enabled() andalso emqx_telemetry:disable(),
     emqx_conf:get([observer_cli, enable], true) andalso emqx_observer_cli:disable(),
     emqx_rewrite:disable(),
     emqx_conf_cli:unload(),

apps/emqx_modules/src/emqx_modules_conf.erl

@@ -29,7 +29,7 @@
     topic_metrics/0,
     add_topic_metrics/1,
     remove_topic_metrics/1,
-    telemetry_status/0,
+    is_telemetry_enabled/0,
     set_telemetry_status/1
 ]).
 
@@ -82,8 +82,8 @@ remove_topic_metrics(Topic) ->
         {error, Reason} -> {error, Reason}
     end.
 
--spec telemetry_status() -> boolean().
+-spec is_telemetry_enabled() -> boolean().
-telemetry_status() ->
+is_telemetry_enabled() ->
     emqx:get_config([telemetry, enable], true).
 
 -spec set_telemetry_status(boolean()) -> ok | {error, term()}.

apps/emqx_modules/src/emqx_telemetry_api.erl

@@ -210,7 +210,7 @@ status(get, _Params) ->
     {200, get_telemetry_status()};
 status(put, #{body := Body}) ->
     Enable = maps:get(<<"enable">>, Body),
-    case Enable =:= emqx_modules_conf:telemetry_status() of
+    case Enable =:= emqx_modules_conf:is_telemetry_enabled() of
         true ->
             Reason =
                 case Enable of
@@ -241,7 +241,7 @@ enable_telemetry(Enable) ->
     emqx_modules_conf:set_telemetry_status(Enable).
 
 get_telemetry_status() ->
-    #{enable => emqx_modules_conf:telemetry_status()}.
+    #{enable => emqx_modules_conf:is_telemetry_enabled()}.
 
 get_telemetry_data() ->
     {ok, TelemetryData} = emqx_telemetry:get_telemetry(),

apps/emqx_modules/test/emqx_telemetry_api_SUITE.erl

@@ -47,7 +47,9 @@ end_per_suite(_Config) ->
             <<"sources">> => []
         }
     ),
-    emqx_common_test_helpers:stop_apps([emqx_conf, emqx_authn, emqx_authz, emqx_modules]),
+    emqx_common_test_helpers:stop_apps([
+        emqx_dashboard, emqx_conf, emqx_authn, emqx_authz, emqx_modules
+    ]),
     ok.
 
 init_per_testcase(t_status_fail, Config) ->