From f3ced5d5eb02c01d21323f9a6f43e4c8a1063684 Mon Sep 17 00:00:00 2001
From: Stefan Strigler <stefan.strigler@emqx.io>
Date: Thu, 16 Feb 2023 17:04:15 +0100
Subject: [PATCH] refactor: kickout_client doesn't need a format fun

---
 apps/emqx_management/src/emqx_mgmt.erl              | 13 +++++++++----
 apps/emqx_management/src/emqx_mgmt_api_clients.erl  |  2 +-
 apps/emqx_management/test/emqx_mgmt_SUITE.erl       | 12 +++++++-----
 .../test/emqx_mgmt_api_clients_SUITE.erl            |  9 ++++++++-
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/apps/emqx_management/src/emqx_mgmt.erl b/apps/emqx_management/src/emqx_mgmt.erl
index 52c50fda6..04f7d9ad0 100644
--- a/apps/emqx_management/src/emqx_mgmt.erl
+++ b/apps/emqx_management/src/emqx_mgmt.erl
@@ -267,7 +267,7 @@ lookup_client({username, Username}, FormatFun) ->
      || Node <- mria_mnesia:running_nodes()
     ]).
 
-lookup_client(Node, Key, {M, F}) ->
+lookup_client(Node, Key, FormatFun) ->
     case unwrap_rpc(emqx_cm_proto_v1:lookup_client(Node, Key)) of
         {error, Err} ->
             {error, Err};
@@ -275,14 +275,19 @@ lookup_client(Node, Key, {M, F}) ->
             lists:map(
                 fun({Chan, Info0, Stats}) ->
                     Info = Info0#{node => Node},
-                    M:F({Chan, Info, Stats})
+                    maybe_format(FormatFun, {Chan, Info, Stats})
                 end,
                 L
             )
     end.
 
-kickout_client({ClientID, FormatFun}) ->
-    case lookup_client({clientid, ClientID}, FormatFun) of
+maybe_format(undefined, A) ->
+    A;
+maybe_format({M, F}, A) ->
+    M:F(A).
+
+kickout_client(ClientID) ->
+    case lookup_client({clientid, ClientID}, undefined) of
         [] ->
             {error, not_found};
         _ ->
diff --git a/apps/emqx_management/src/emqx_mgmt_api_clients.erl b/apps/emqx_management/src/emqx_mgmt_api_clients.erl
index 571f190f2..0a5488389 100644
--- a/apps/emqx_management/src/emqx_mgmt_api_clients.erl
+++ b/apps/emqx_management/src/emqx_mgmt_api_clients.erl
@@ -677,7 +677,7 @@ lookup(#{clientid := ClientID}) ->
     end.
 
 kickout(#{clientid := ClientID}) ->
-    case emqx_mgmt:kickout_client({ClientID, ?FORMAT_FUN}) of
+    case emqx_mgmt:kickout_client(ClientID) of
         {error, not_found} ->
             {404, ?CLIENTID_NOT_FOUND};
         _ ->
diff --git a/apps/emqx_management/test/emqx_mgmt_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_SUITE.erl
index 28533cac5..13acb9fc3 100644
--- a/apps/emqx_management/test/emqx_mgmt_SUITE.erl
+++ b/apps/emqx_management/test/emqx_mgmt_SUITE.erl
@@ -23,6 +23,8 @@
 
 -export([ident/1]).
 
+-define(FORMATFUN, {?MODULE, ident}).
+
 all() ->
     emqx_common_test_helpers:all(?MODULE).
 
@@ -117,12 +119,12 @@ t_lookup_client('end', Config) ->
     disconnect_clients(Config).
 
 t_lookup_client(_Config) ->
-    [{Chan, Info, Stats}] = emqx_mgmt:lookup_client({clientid, <<"client1">>}, {?MODULE, ident}),
+    [{Chan, Info, Stats}] = emqx_mgmt:lookup_client({clientid, <<"client1">>}, ?FORMATFUN),
     ?assertEqual(
         [{Chan, Info, Stats}],
-        emqx_mgmt:lookup_client({username, <<"user1">>}, {?MODULE, ident})
+        emqx_mgmt:lookup_client({username, <<"user1">>}, ?FORMATFUN)
     ),
-    ?assertEqual([], emqx_mgmt:lookup_client({clientid, <<"notfound">>}, {?MODULE, ident})).
+    ?assertEqual([], emqx_mgmt:lookup_client({clientid, <<"notfound">>}, ?FORMATFUN)).
 
 t_kickout_client(init, Config) ->
     process_flag(trap_exit, true),
@@ -132,7 +134,7 @@ t_kickout_client('end', _Config) ->
 
 t_kickout_client(Config) ->
     [C | _] = ?config(clients, Config),
-    ok = emqx_mgmt:kickout_client({<<"client1">>, {?MODULE, ident}}),
+    ok = emqx_mgmt:kickout_client(<<"client1">>),
     receive
         {'EXIT', C, Reason} ->
             ?assertEqual({shutdown, tcp_closed}, Reason);
@@ -141,7 +143,7 @@ t_kickout_client(Config) ->
     after 1000 ->
         error(timeout)
     end,
-    ?assertEqual({error, not_found}, emqx_mgmt:kickout_client({<<"notfound">>, {?MODULE, ident}})).
+    ?assertEqual({error, not_found}, emqx_mgmt:kickout_client(<<"notfound">>)).
 
 %%% helpers
 ident(Arg) ->
diff --git a/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl
index c7f4c9845..d843a2209 100644
--- a/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl
+++ b/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl
@@ -78,7 +78,14 @@ t_clients(_) ->
     %% delete /clients/:clientid kickout
     Client2Path = emqx_mgmt_api_test_util:api_path(["clients", binary_to_list(ClientId2)]),
     {ok, _} = emqx_mgmt_api_test_util:request_api(delete, Client2Path),
-    timer:sleep(300),
+    Kick =
+        receive
+            {'EXIT', C2, _} ->
+                ok
+        after 300 ->
+            timeout
+        end,
+    ?assertEqual(ok, Kick),
     AfterKickoutResponse2 = emqx_mgmt_api_test_util:request_api(get, Client2Path),
     ?assertEqual({error, {"HTTP/1.1", 404, "Not Found"}}, AfterKickoutResponse2),