From c3b044a37b1b177423d822af259f145af3daf71a Mon Sep 17 00:00:00 2001
From: Andrew Mayorov <encube.ul@gmail.com>
Date: Wed, 28 Feb 2024 11:28:33 +0100
Subject: [PATCH] fix(api-topics): respond with `count` when it's cheap to
 compute

---
 .../src/emqx_mgmt_api_topics.erl              | 13 +++++--
 .../test/emqx_mgmt_api_topics_SUITE.erl       | 38 ++++++++++---------
 2 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/apps/emqx_management/src/emqx_mgmt_api_topics.erl b/apps/emqx_management/src/emqx_mgmt_api_topics.erl
index ed2de351b..9ab111fed 100644
--- a/apps/emqx_management/src/emqx_mgmt_api_topics.erl
+++ b/apps/emqx_management/src/emqx_mgmt_api_topics.erl
@@ -115,7 +115,7 @@ do_list(Params) ->
         {_, Query} = emqx_mgmt_api:parse_qstring(Params, ?TOPICS_QUERY_SCHEMA),
         QState = Pager#{continuation => undefined},
         QResult = eval_topic_query(qs2ms(Query), QState),
-        {200, format_list_response(Pager, QResult)}
+        {200, format_list_response(Pager, Query, QResult)}
     catch
         throw:{error, page_limit_invalid} ->
             {400, #{code => <<"INVALID_PARAMETER">>, message => <<"page_limit_invalid">>}};
@@ -186,15 +186,22 @@ finalize_query(QResult = #{overflow := Overflow, complete := Complete}) ->
     HasNext = Overflow orelse not Complete,
     QResult#{hasnext => HasNext}.
 
-format_list_response(Meta, _QResult = #{hasnext := HasNext, rows := RowsAcc, cursor := Cursor}) ->
+format_list_response(Meta, Query, QResult = #{rows := RowsAcc}) ->
     #{
-        meta => Meta#{hasnext => HasNext, count => Cursor},
+        meta => format_response_meta(Meta, Query, QResult),
         data => lists:flatmap(
             fun({_Node, Rows}) -> [format(R) || R <- Rows] end,
             RowsAcc
         )
     }.
 
+format_response_meta(Meta, _Query, #{hasnext := HasNext, complete := true, cursor := Cursor}) ->
+    Meta#{hasnext => HasNext, count => Cursor};
+format_response_meta(Meta, _Query = {[], []}, #{hasnext := HasNext}) ->
+    Meta#{hasnext => HasNext, count => emqx_router:stats(n_routes)};
+format_response_meta(Meta, _Query, #{hasnext := HasNext}) ->
+    Meta#{hasnext => HasNext}.
+
 format(#route{topic = Topic, dest = {Group, Node}}) ->
     #{topic => ?SHARE(Group, Topic), node => Node};
 format(#route{topic = Topic, dest = Node}) when is_atom(Node) ->
diff --git a/apps/emqx_management/test/emqx_mgmt_api_topics_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_topics_SUITE.erl
index bbddde749..8b362af1b 100644
--- a/apps/emqx_management/test/emqx_mgmt_api_topics_SUITE.erl
+++ b/apps/emqx_management/test/emqx_mgmt_api_topics_SUITE.erl
@@ -64,12 +64,11 @@ t_nodes_api(Config) ->
         {"node", atom_to_list(node())}
     ]),
     ?assertMatch(
-        #{<<"count">> := 1, <<"page">> := 1, <<"limit">> := 100},
-        maps:get(<<"meta">>, MatchData)
-    ),
-    ?assertMatch(
-        [#{<<"topic">> := Topic2, <<"node">> := Node}],
-        maps:get(<<"data">>, MatchData)
+        #{
+            <<"data">> := [#{<<"topic">> := Topic2, <<"node">> := Node}],
+            <<"meta">> := #{<<"page">> := 1, <<"limit">> := 100, <<"count">> := 1}
+        },
+        MatchData
     ),
 
     %% get topics/:topic
@@ -114,6 +113,11 @@ t_paging(_Config) ->
         Matched,
         request_json(get, ["topics"], [{"node", Node}])
     ),
+    ?assertEqual(
+        %% NOTE: No `count` in this case.
+        #{<<"hasnext">> => true, <<"page">> => 1, <<"limit">> => 3},
+        maps:get(<<"meta">>, request_json(get, ["topics"], [{"node", Node}, {"limit", "3"}]))
+    ),
     R1 = #{<<"data">> := Data1} = request_json(get, ["topics"], [{"page", "1"}, {"limit", "5"}]),
     R2 = #{<<"data">> := Data2} = request_json(get, ["topics"], [{"page", "2"}, {"limit", "5"}]),
     ?assertMatch(
@@ -150,12 +154,11 @@ t_percent_topics(_Config) ->
         {"node", atom_to_list(node())}
     ]),
     ?assertMatch(
-        #{<<"count">> := 1, <<"page">> := 1, <<"limit">> := 100},
-        maps:get(<<"meta">>, MatchData)
-    ),
-    ?assertMatch(
-        [#{<<"topic">> := Topic, <<"node">> := Node}],
-        maps:get(<<"data">>, MatchData)
+        #{
+            <<"data">> := [#{<<"topic">> := Topic, <<"node">> := Node}],
+            <<"meta">> := #{<<"page">> := 1, <<"limit">> := 100, <<"count">> := 1}
+        },
+        MatchData
     ),
 
     ok = emqtt:stop(Client).
@@ -175,12 +178,11 @@ t_shared_topics(_Configs) ->
         {"node", atom_to_list(node())}
     ]),
     ?assertMatch(
-        #{<<"count">> := 1, <<"page">> := 1, <<"limit">> := 100},
-        maps:get(<<"meta">>, MatchData)
-    ),
-    ?assertMatch(
-        [#{<<"topic">> := Topic, <<"node">> := Node}],
-        maps:get(<<"data">>, MatchData)
+        #{
+            <<"data">> := [#{<<"topic">> := Topic, <<"node">> := Node}],
+            <<"meta">> := #{<<"page">> := 1, <<"limit">> := 100, <<"count">> := 1}
+        },
+        MatchData
     ),
 
     ok = emqtt:stop(Client).