fix: return 'not found' for subscriptions of unknown client

apps/emqx_management/src/emqx_mgmt.erl

@@ -286,12 +286,12 @@ maybe_format(undefined, A) ->
 maybe_format({M, F}, A) ->
     M:F(A).
 
-kickout_client(ClientID) ->
-    case lookup_client({clientid, ClientID}, undefined) of
+kickout_client(ClientId) ->
+    case lookup_client({clientid, ClientId}, undefined) of
         [] ->
             {error, not_found};
         _ ->
-            Results = [kickout_client(Node, ClientID) || Node <- mria_mnesia:running_nodes()],
+            Results = [kickout_client(Node, ClientId) || Node <- mria_mnesia:running_nodes()],
             check_results(Results)
     end.
 
@@ -302,17 +302,22 @@ list_authz_cache(ClientId) ->
     call_client(ClientId, list_authz_cache).
 
 list_client_subscriptions(ClientId) ->
-    Results = [client_subscriptions(Node, ClientId) || Node <- mria_mnesia:running_nodes()],
-    Filter =
-        fun
-            ({error, _}) ->
-                false;
-            ({_Node, List}) ->
-                erlang:is_list(List) andalso 0 < erlang:length(List)
-        end,
-    case lists:filter(Filter, Results) of
-        [] -> [];
-        [Result | _] -> Result
+    case lookup_client({clientid, ClientId}, undefined) of
+        [] ->
+            {error, not_found};
+        _ ->
+            Results = [client_subscriptions(Node, ClientId) || Node <- mria_mnesia:running_nodes()],
+            Filter =
+                fun
+                    ({error, _}) ->
+                        false;
+                    ({_Node, List}) ->
+                        erlang:is_list(List) andalso 0 < erlang:length(List)
+                end,
+            case lists:filter(Filter, Results) of
+                [] -> [];
+                [Result | _] -> Result
+            end
     end.
 
 client_subscriptions(Node, ClientId) ->

apps/emqx_management/src/emqx_mgmt_api_clients.erl

@@ -274,11 +274,10 @@ schema("/clients/:clientid/subscriptions") ->
             responses => #{
                 200 => hoconsc:mk(
                     hoconsc:array(hoconsc:ref(emqx_mgmt_api_subscriptions, subscription)), #{}
+                ),
+                404 => emqx_dashboard_swagger:error_codes(
+                    ['CLIENTID_NOT_FOUND'], <<"Client ID not found">>
                 )
-                %% returns [] if client not existed in cluster
-                %404 => emqx_dashboard_swagger:error_codes(
-                %    ['CLIENTID_NOT_FOUND'], <<"Client ID not found">>
-                %)
             }
         }
     };
@@ -599,6 +598,8 @@ unsubscribe_batch(post, #{bindings := #{clientid := ClientID}, body := TopicInfo
 
 subscriptions(get, #{bindings := #{clientid := ClientID}}) ->
     case emqx_mgmt:list_client_subscriptions(ClientID) of
+        {error, not_found} ->
+            {404, ?CLIENTID_NOT_FOUND};
         [] ->
             {200, []};
         {Node, Subs} ->

apps/emqx_management/test/emqx_mgmt_SUITE.erl

@@ -145,6 +145,27 @@ t_kickout_client(Config) ->
     end,
     ?assertEqual({error, not_found}, emqx_mgmt:kickout_client(<<"notfound">>)).
 
+t_list_authz_cache(init, Config) ->
+    setup_clients(Config);
+t_list_authz_cache('end', Config) ->
+    disconnect_clients(Config).
+
+t_list_authz_cache(_) ->
+    ?assertNotMatch({error, _}, emqx_mgmt:list_authz_cache(<<"client1">>)),
+    ?assertMatch({error, not_found}, emqx_mgmt:list_authz_cache(<<"notfound">>)).
+
+t_list_client_subscriptions(init, Config) ->
+    setup_clients(Config);
+t_list_client_subscriptions('end', Config) ->
+    disconnect_clients(Config).
+
+t_list_client_subscriptions(Config) ->
+    [Client | _] = ?config(clients, Config),
+    ?assertEqual([], emqx_mgmt:list_client_subscriptions(<<"client1">>)),
+    emqtt:subscribe(Client, <<"t/#">>),
+    ?assertMatch({_, [{<<"t/#">>, _Opts}]}, emqx_mgmt:list_client_subscriptions(<<"client1">>)),
+    ?assertEqual({error, not_found}, emqx_mgmt:list_client_subscriptions(<<"notfound">>)).
+
 %%% helpers
 ident(Arg) ->
     Arg.

apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl

@@ -279,7 +279,7 @@ t_client_id_not_found(_Config) ->
     %% Client kickout
     ?assertMatch({error, {Http, _, Body}}, ReqFun(delete, PathFun([]))),
     %% Client Subscription list
-    ?assertMatch({ok, {{"HTTP/1.1", 200, "OK"}, _, "[]"}}, ReqFun(get, PathFun(["subscriptions"]))),
+    ?assertMatch({error, {Http, _, Body}}, ReqFun(get, PathFun(["subscriptions"]))),
     %% AuthZ Cache lookup
     ?assertMatch({error, {Http, _, Body}}, ReqFun(get, PathFun(["authorization", "cache"]))),
     %% AuthZ Cache clean