fix: create consistent interface 'with_node' for API access

apps/emqx/include/emqx_api_lib.hrl

@@ -0,0 +1,36 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2021-2023 EMQ Technologies Co., Ltd. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%--------------------------------------------------------------------
+
+-ifndef(EMQX_API_LIB_HRL).
+-define(EMQX_API_LIB_HRL, true).
+
+-define(ERROR_MSG(CODE, REASON), #{code => CODE, message => emqx_misc:readable_error_msg(REASON)}).
+
+-define(OK(CONTENT), {200, CONTENT}).
+
+-define(NO_CONTENT, 204).
+
+-define(BAD_REQUEST(CODE, REASON), {400, ?ERROR_MSG(CODE, REASON)}).
+-define(BAD_REQUEST(REASON), ?BAD_REQUEST('BAD_REQUEST', REASON)).
+
+-define(NOT_FOUND(REASON), {404, ?ERROR_MSG('NOT_FOUND', REASON)}).
+
+-define(INTERNAL_ERROR(REASON), {500, ?ERROR_MSG('INTERNAL_ERROR', REASON)}).
+
+-define(NOT_IMPLEMENTED, 501).
+
+-define(SERVICE_UNAVAILABLE(REASON), {503, ?ERROR_MSG('SERVICE_UNAVAILABLE', REASON)}).
+-endif.

apps/emqx/src/emqx_api_lib.erl

@@ -0,0 +1,69 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2020-2023 EMQ Technologies Co., Ltd. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%--------------------------------------------------------------------
+
+-module(emqx_api_lib).
+
+-export([
+    with_node/2,
+    with_node_or_cluster/2
+]).
+
+-include("emqx_api_lib.hrl").
+
+-define(NODE_NOT_FOUND(NODE), ?NOT_FOUND(<<"Node not found: ", NODE/binary>>)).
+
+%%--------------------------------------------------------------------
+%% exported API
+%%--------------------------------------------------------------------
+-spec with_node(binary(), fun((atom()) -> {ok, term()} | {error, term()})) ->
+    ?OK(term()) | ?NOT_FOUND(binary()) | ?BAD_REQUEST(term()).
+with_node(BinNode, Fun) ->
+    case lookup_node(BinNode) of
+        {ok, Node} ->
+            handle_result(Fun(Node));
+        not_found ->
+            ?NODE_NOT_FOUND(BinNode)
+    end.
+
+-spec with_node_or_cluster(binary(), fun((atom()) -> {ok, term()} | {error, term()})) ->
+    ?OK(term()) | ?NOT_FOUND(iolist()) | ?BAD_REQUEST(term()).
+with_node_or_cluster(<<"all">>, Fun) ->
+    handle_result(Fun(all));
+with_node_or_cluster(Node, Fun) ->
+    with_node(Node, Fun).
+
+%%--------------------------------------------------------------------
+%% Internal
+%%--------------------------------------------------------------------
+
+-spec lookup_node(binary()) -> {ok, atom()} | not_found.
+lookup_node(BinNode) ->
+    case emqx_misc:safe_to_existing_atom(BinNode, utf8) of
+        {ok, Node} ->
+            case lists:member(Node, mria:running_nodes()) of
+                true ->
+                    {ok, Node};
+                false ->
+                    not_found
+            end;
+        _Error ->
+            not_found
+    end.
+
+handle_result({ok, Result}) ->
+    ?OK(Result);
+handle_result({error, Reason}) ->
+    ?BAD_REQUEST(Reason).

apps/emqx/test/emqx_api_lib_SUITE.erl

@@ -0,0 +1,101 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2020-2023 EMQ Technologies Co., Ltd. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%--------------------------------------------------------------------
+
+-module(emqx_api_lib_SUITE).
+
+-compile(export_all).
+-compile(nowarn_export_all).
+
+-include("emqx_api_lib.hrl").
+-include_lib("eunit/include/eunit.hrl").
+
+-define(DUMMY, dummy_module).
+
+all() -> emqx_common_test_helpers:all(?MODULE).
+
+init_per_suite(Config) ->
+    emqx_common_test_helpers:boot_modules(all),
+    emqx_common_test_helpers:start_apps([]),
+    Config.
+
+end_per_suite(_Config) ->
+    emqx_common_test_helpers:stop_apps([]).
+
+init_per_testcase(_Case, Config) ->
+    meck:new(?DUMMY, [non_strict]),
+    meck:expect(?DUMMY, expect_not_called, 1, fun(Node) -> throw({blow_this_up, Node}) end),
+    meck:expect(?DUMMY, expect_success, 1, {ok, success}),
+    meck:expect(?DUMMY, expect_error, 1, {error, error}),
+    Config.
+
+end_per_testcase(_Case, _Config) ->
+    meck:unload(?DUMMY).
+
+t_with_node(_) ->
+    test_with(fun emqx_api_lib:with_node/2, [<<"all">>]).
+
+t_with_node_or_cluster(_) ->
+    test_with(fun emqx_api_lib:with_node_or_cluster/2, []),
+    meck:reset(?DUMMY),
+    ?assertEqual(
+        ?OK(success),
+        emqx_api_lib:with_node_or_cluster(
+            <<"all">>,
+            fun ?DUMMY:expect_success/1
+        )
+    ),
+    ?assertMatch([{_, {?DUMMY, expect_success, [all]}, {ok, success}}], meck:history(?DUMMY)).
+
+%% helpers
+test_with(TestFun, ExtraBadNodes) ->
+    % make sure this is an atom
+    'unknownnode@unknownnohost',
+    BadNodes =
+        [
+            <<"undefined">>,
+            <<"this_should_not_be_an_atom">>,
+            <<"unknownnode@unknownnohost">>
+        ] ++ ExtraBadNodes,
+    [ensure_not_found(TestFun(N, fun ?DUMMY:expect_not_called/1)) || N <- BadNodes],
+    ensure_not_called(?DUMMY, expect_not_called),
+    ensure_not_existing_atom(<<"this_should_not_be_an_atom">>),
+
+    GoodNode = node(),
+
+    ?assertEqual(
+        ?OK(success),
+        TestFun(GoodNode, fun ?DUMMY:expect_success/1)
+    ),
+
+    ?assertEqual(
+        ?BAD_REQUEST(error),
+        TestFun(GoodNode, fun ?DUMMY:expect_error/1)
+    ),
+    ok.
+
+ensure_not_found(Result) ->
+    ?assertMatch({404, _}, Result).
+
+ensure_not_called(Mod, Fun) ->
+    ?assert(not meck:called(Mod, Fun, '_')).
+
+ensure_not_existing_atom(Bin) ->
+    try binary_to_existing_atom(Bin) of
+        _ -> throw(is_atom)
+    catch
+        error:badarg ->
+            ok
+    end.

apps/emqx_bridge/src/emqx_bridge_api.erl

@@ -20,6 +20,7 @@
 -include_lib("typerefl/include/types.hrl").
 -include_lib("hocon/include/hoconsc.hrl").
 -include_lib("emqx/include/logger.hrl").
+-include_lib("emqx/include/emqx_api_lib.hrl").
 -include_lib("emqx_bridge/include/emqx_bridge.hrl").
 
 -import(hoconsc, [mk/2, array/1, enum/1]).
@@ -46,25 +47,6 @@
 
 -export([lookup_from_local_node/2]).
 
-%% [TODO] Move those to a commonly shared header file
--define(ERROR_MSG(CODE, REASON), #{code => CODE, message => emqx_misc:readable_error_msg(REASON)}).
-
--define(OK(CONTENT), {200, CONTENT}).
-
--define(NO_CONTENT, 204).
-
--define(BAD_REQUEST(CODE, REASON), {400, ?ERROR_MSG(CODE, REASON)}).
--define(BAD_REQUEST(REASON), ?BAD_REQUEST('BAD_REQUEST', REASON)).
-
--define(NOT_FOUND(REASON), {404, ?ERROR_MSG('NOT_FOUND', REASON)}).
-
--define(INTERNAL_ERROR(REASON), {500, ?ERROR_MSG('INTERNAL_ERROR', REASON)}).
-
--define(NOT_IMPLEMENTED, 501).
-
--define(SERVICE_UNAVAILABLE(REASON), {503, ?ERROR_MSG('SERVICE_UNAVAILABLE', REASON)}).
-%% End TODO
-
 -define(BRIDGE_NOT_ENABLED,
     ?BAD_REQUEST(<<"Forbidden operation, bridge not enabled">>)
 ).

apps/emqx_dashboard/src/emqx_dashboard_monitor_api.erl

@@ -121,32 +121,27 @@ fields(sampler_current) ->
 
 monitor(get, #{query_string := QS, bindings := Bindings}) ->
     Latest = maps:get(<<"latest">>, QS, infinity),
-    RawNode = maps:get(node, Bindings, all),
-    with_node(RawNode, dashboard_samplers_fun(Latest)).
+    RawNode = maps:get(node, Bindings, <<"all">>),
+    emqx_api_lib:with_node_or_cluster(RawNode, dashboard_samplers_fun(Latest)).
 
 dashboard_samplers_fun(Latest) ->
     fun(NodeOrCluster) ->
         case emqx_dashboard_monitor:samplers(NodeOrCluster, Latest) of
-            {badrpc, _} = Error -> Error;
+            {badrpc, _} = Error -> {error, Error};
             Samplers -> {ok, Samplers}
         end
     end.
 
 monitor_current(get, #{bindings := Bindings}) ->
-    RawNode = maps:get(node, Bindings, all),
-    with_node(RawNode, fun emqx_dashboard_monitor:current_rate/1).
-
-with_node(RawNode, Fun) ->
-    case emqx_misc:safe_to_existing_atom(RawNode, utf8) of
-        {ok, NodeOrCluster} ->
-            case Fun(NodeOrCluster) of
-                {badrpc, {Node, Reason}} ->
-                    {404, 'NOT_FOUND', io_lib:format("Node not found: ~p (~p)", [Node, Reason])};
-                {ok, Result} ->
-                    {200, Result}
-            end;
-        _Error ->
-            {404, 'NOT_FOUND', io_lib:format("Node not found: ~p", [RawNode])}
+    RawNode = maps:get(node, Bindings, <<"all">>),
+    emqx_api_lib:with_node_or_cluster(RawNode, fun current_rate/1).
+
+current_rate(Node) ->
+    case emqx_dashboard_monitor:current_rate(Node) of
+        {badrpc, _} = BadRpc ->
+            {error, BadRpc};
+        {ok, _} = OkResult ->
+            OkResult
     end.
 
 %% -------------------------------------------------------------------------------------------------