Przeglądaj źródła

Merge pull request #9824 from olcai/internal-error-api-get-topic

fix(emqx_management): handle multiple routes in topics/{topic} API
Erik Timan 3 lat temu
rodzic
commit
3786bb8086

+ 1 - 1
apps/emqx_management/src/emqx_management.app.src

@@ -2,7 +2,7 @@
 {application, emqx_management, [
 {application, emqx_management, [
     {description, "EMQX Management API and CLI"},
     {description, "EMQX Management API and CLI"},
     % strict semver, bump manually!
     % strict semver, bump manually!
-    {vsn, "5.0.12"},
+    {vsn, "5.0.13"},
     {modules, []},
     {modules, []},
     {registered, [emqx_management_sup]},
     {registered, [emqx_management_sup]},
     {applications, [kernel, stdlib, emqx_plugins, minirest, emqx]},
     {applications, [kernel, stdlib, emqx_plugins, minirest, emqx]},

+ 4 - 3
apps/emqx_management/src/emqx_mgmt_api_topics.erl

@@ -75,7 +75,7 @@ schema("/topics/:topic") ->
             tags => ?TAGS,
             tags => ?TAGS,
             parameters => [topic_param(path)],
             parameters => [topic_param(path)],
             responses => #{
             responses => #{
-                200 => hoconsc:mk(hoconsc:ref(topic), #{}),
+                200 => hoconsc:mk(hoconsc:array(hoconsc:ref(topic)), #{}),
                 404 =>
                 404 =>
                     emqx_dashboard_swagger:error_codes(['TOPIC_NOT_FOUND'], <<"Topic not found">>)
                     emqx_dashboard_swagger:error_codes(['TOPIC_NOT_FOUND'], <<"Topic not found">>)
             }
             }
@@ -130,8 +130,9 @@ lookup(#{topic := Topic}) ->
     case emqx_router:lookup_routes(Topic) of
     case emqx_router:lookup_routes(Topic) of
         [] ->
         [] ->
             {404, #{code => ?TOPIC_NOT_FOUND, message => <<"Topic not found">>}};
             {404, #{code => ?TOPIC_NOT_FOUND, message => <<"Topic not found">>}};
-        [Route] ->
-            {200, format(Route)}
+        Routes when is_list(Routes) ->
+            Formatted = [format(Route) || Route <- Routes],
+            {200, Formatted}
     end.
     end.
 
 
 %%%==============================================================================================
 %%%==============================================================================================

+ 22 - 6
apps/emqx_management/test/emqx_mgmt_api_topics_SUITE.erl

@@ -19,18 +19,25 @@
 -compile(nowarn_export_all).
 -compile(nowarn_export_all).
 
 
 -include_lib("eunit/include/eunit.hrl").
 -include_lib("eunit/include/eunit.hrl").
+-include_lib("common_test/include/ct.hrl").
+
+-define(ROUTE_TAB, emqx_route).
 
 
 all() ->
 all() ->
     emqx_common_test_helpers:all(?MODULE).
     emqx_common_test_helpers:all(?MODULE).
 
 
 init_per_suite(Config) ->
 init_per_suite(Config) ->
     emqx_mgmt_api_test_util:init_suite(),
     emqx_mgmt_api_test_util:init_suite(),
-    Config.
+    Slave = emqx_common_test_helpers:start_slave(some_node, []),
+    [{slave, Slave} | Config].
 
 
-end_per_suite(_) ->
+end_per_suite(Config) ->
+    Slave = ?config(slave, Config),
+    emqx_common_test_helpers:stop_slave(Slave),
+    mria:clear_table(?ROUTE_TAB),
     emqx_mgmt_api_test_util:end_suite().
     emqx_mgmt_api_test_util:end_suite().
 
 
-t_nodes_api(_) ->
+t_nodes_api(Config) ->
     Node = atom_to_binary(node(), utf8),
     Node = atom_to_binary(node(), utf8),
     Topic = <<"test_topic">>,
     Topic = <<"test_topic">>,
     {ok, Client} = emqtt:start_link(#{
     {ok, Client} = emqtt:start_link(#{
@@ -72,8 +79,17 @@ t_nodes_api(_) ->
     ),
     ),
 
 
     %% get topics/:topic
     %% get topics/:topic
+    %% We add another route here to ensure that the response handles
+    %% multiple routes for a single topic
+    Slave = ?config(slave, Config),
+    ok = emqx_router:add_route(Topic, Slave),
     RoutePath = emqx_mgmt_api_test_util:api_path(["topics", Topic]),
     RoutePath = emqx_mgmt_api_test_util:api_path(["topics", Topic]),
     {ok, RouteResponse} = emqx_mgmt_api_test_util:request_api(get, RoutePath),
     {ok, RouteResponse} = emqx_mgmt_api_test_util:request_api(get, RoutePath),
-    RouteData = emqx_json:decode(RouteResponse, [return_maps]),
-    ?assertEqual(Topic, maps:get(<<"topic">>, RouteData)),
-    ?assertEqual(Node, maps:get(<<"node">>, RouteData)).
+    ok = emqx_router:delete_route(Topic, Slave),
+
+    [
+        #{<<"topic">> := Topic, <<"node">> := Node1},
+        #{<<"topic">> := Topic, <<"node">> := Node2}
+    ] = emqx_json:decode(RouteResponse, [return_maps]),
+
+    ?assertEqual(lists:usort([Node, atom_to_binary(Slave)]), lists:usort([Node1, Node2])).

+ 1 - 0
changes/v5.0.16/fix-9824.en.md

@@ -0,0 +1 @@
+The `topics/{topic}` API endpoint would return `500 - Internal Error` if a topic had multiple routes. This is fixed by returning a list of routes.

+ 1 - 0
changes/v5.0.16/fix-9824.zh.md

@@ -0,0 +1 @@
+修复:当存在多个路由信息时,topics/{topic} 将会返回 500 - Internal Error 的问题,现在将会正确的返回路由信息列表。