From 79a653e2b417ef991736cfd47984e1461fad883b Mon Sep 17 00:00:00 2001 From: JianBo He Date: Wed, 5 Jan 2022 16:32:16 +0800 Subject: [PATCH] refactor(gw): more readable CLI print --- apps/emqx_gateway/include/emqx_gateway.hrl | 1 + apps/emqx_gateway/src/emqx_gateway_cli.erl | 97 ++++++++++----- apps/emqx_gateway/src/emqx_gateway_conf.erl | 1 + apps/emqx_gateway/src/emqx_gateway_http.erl | 111 ++++++++++-------- .../test/emqx_gateway_cli_SUITE.erl | 74 +++++++++++- 5 files changed, 199 insertions(+), 85 deletions(-) diff --git a/apps/emqx_gateway/include/emqx_gateway.hrl b/apps/emqx_gateway/include/emqx_gateway.hrl index 8a89d237c..b6b176429 100644 --- a/apps/emqx_gateway/include/emqx_gateway.hrl +++ b/apps/emqx_gateway/include/emqx_gateway.hrl @@ -22,6 +22,7 @@ %% @doc The Gateway defination -type gateway() :: #{ name := gateway_name() + %% Description , descr => binary() | undefined %% Appears only in getting gateway info , status => stopped | running | unloaded diff --git a/apps/emqx_gateway/src/emqx_gateway_cli.erl b/apps/emqx_gateway/src/emqx_gateway_cli.erl index f8f4c5821..c9ed51d92 100644 --- a/apps/emqx_gateway/src/emqx_gateway_cli.erl +++ b/apps/emqx_gateway/src/emqx_gateway_cli.erl @@ -53,23 +53,16 @@ is_cmd(Fun) -> gateway(["list"]) -> lists:foreach( - fun (#{name := Name, status := unloaded}) -> - print("Gateway(name=~ts, status=unloaded)\n", [Name]); - (#{name := Name, status := stopped, stopped_at := StoppedAt}) -> - print("Gateway(name=~ts, status=stopped, stopped_at=~ts)\n", - [Name, StoppedAt]); - (#{name := Name, status := running, current_connections := ConnCnt, - started_at := StartedAt}) -> - print("Gateway(name=~ts, status=running, clients=~w, started_at=~ts)\n", - [Name, ConnCnt, StartedAt]) + fun (GwSummary) -> + print(format_gw_summary(GwSummary)) end, emqx_gateway_http:gateways(all)); gateway(["lookup", Name]) -> case emqx_gateway:lookup(atom(Name)) of undefined -> print("undefined\n"); - Info -> - print("~p\n", [Info]) + Gateway -> + print(format_gateway(Gateway)) end; gateway(["load", Name, Conf]) -> @@ -80,7 +73,7 @@ gateway(["load", Name, Conf]) -> {ok, _} -> print("ok\n"); {error, Reason} -> - print("Error: ~p\n", [Reason]) + print("Error: ~ts\n", [format_error(Reason)]) end; gateway(["unload", Name]) -> @@ -88,7 +81,7 @@ gateway(["unload", Name]) -> ok -> print("ok\n"); {error, Reason} -> - print("Error: ~p\n", [Reason]) + print("Error: ~ts\n", [format_error(Reason)]) end; gateway(["stop", Name]) -> @@ -99,7 +92,7 @@ gateway(["stop", Name]) -> {ok, _} -> print("ok\n"); {error, Reason} -> - print("Error: ~p\n", [Reason]) + print("Error: ~ts\n", [format_error(Reason)]) end; gateway(["start", Name]) -> @@ -110,23 +103,24 @@ gateway(["start", Name]) -> {ok, _} -> print("ok\n"); {error, Reason} -> - print("Error: ~p\n", [Reason]) + print("Error: ~ts\n", [format_error(Reason)]) end; gateway(_) -> - emqx_ctl:usage([ {"gateway list", - "List all gateway"} - , {"gateway lookup ", - "Lookup a gateway detailed informations"} - , {"gateway load ", - "Load a gateway with config"} - , {"gateway unload ", - "Unload the gateway"} - , {"gateway stop ", - "Stop the gateway"} - , {"gateway start ", - "Start the gateway"} - ]). + emqx_ctl:usage( + [ {"gateway list", + "List all gateway"} + , {"gateway lookup ", + "Lookup a gateway detailed informations"} + , {"gateway load ", + "Load a gateway with config"} + , {"gateway unload ", + "Unload the gateway"} + , {"gateway stop ", + "Stop the gateway"} + , {"gateway start ", + "Start the gateway"} + ]). 'gateway-registry'(["list"]) -> lists:foreach( @@ -255,3 +249,50 @@ format(peername, {IPAddr, Port}) -> format(_, Val) -> Val. + +format_gw_summary(#{name := Name, status := unloaded}) -> + io_lib:format("Gateway(name=~ts, status=unloaded)\n", [Name]); + +format_gw_summary(#{name := Name, status := stopped, + stopped_at := StoppedAt}) -> + io_lib:format("Gateway(name=~ts, status=stopped, stopped_at=~ts)\n", + [Name, StoppedAt]); +format_gw_summary(#{name := Name, status := running, + current_connections := ConnCnt, + started_at := StartedAt}) -> + io_lib:format("Gateway(name=~ts, status=running, clients=~w, " + "started_at=~ts)\n", [Name, ConnCnt, StartedAt]). + +format_gateway(#{name := Name, + status := unloaded}) -> + io_lib:format( + "name: ~ts\n" + "status: unloaded\n", [Name]); + +format_gateway(Gw = + #{name := Name, + status := Status, + created_at := CreatedAt, + config := Config + }) -> + {StopOrStart, Timestamp} = + case Status of + stopped -> {stopped_at, maps:get(stopped_at, Gw)}; + running -> {started_at, maps:get(started_at, Gw)} + end, + io_lib:format( + "name: ~ts\n" + "status: ~ts\n" + "created_at: ~ts\n" + "~ts: ~ts\n" + "config: ~p\n", + [Name, Status, + emqx_gateway_utils:unix_ts_to_rfc3339(CreatedAt), + StopOrStart, emqx_gateway_utils:unix_ts_to_rfc3339(Timestamp), + Config]). + +format_error(Reason) -> + case emqx_gateway_http:reason2msg(Reason) of + error -> io_lib:format("~p", [Reason]); + Msg -> Msg + end. diff --git a/apps/emqx_gateway/src/emqx_gateway_conf.erl b/apps/emqx_gateway/src/emqx_gateway_conf.erl index 0b7b3f099..52aa204a3 100644 --- a/apps/emqx_gateway/src/emqx_gateway_conf.erl +++ b/apps/emqx_gateway/src/emqx_gateway_conf.erl @@ -93,6 +93,7 @@ load_gateway(GwName, Conf) -> %% @doc convert listener array to map unconvert_listeners(Ls) when is_list(Ls) -> lists:foldl(fun(Lis, Acc) -> + %% FIXME: params apperence guard? {[Type, Name], Lis1} = maps_key_take([<<"type">>, <<"name">>], Lis), NLis1 = maps:without([<<"id">>], Lis1), emqx_map_lib:deep_merge(Acc, #{Type => #{Name => NLis1}}) diff --git a/apps/emqx_gateway/src/emqx_gateway_http.erl b/apps/emqx_gateway/src/emqx_gateway_http.erl index 1560a2126..7a1ac519d 100644 --- a/apps/emqx_gateway/src/emqx_gateway_http.erl +++ b/apps/emqx_gateway/src/emqx_gateway_http.erl @@ -62,12 +62,15 @@ , with_listener_authn/3 , checks/2 , reason2resp/1 + , reason2msg/1 ]). -type gateway_summary() :: #{ name := binary() , status := running | stopped | unloaded + , created_at => binary() , started_at => binary() + , stopped_at => binary() , max_connections => integer() , current_connections => integer() , listeners => [] @@ -317,57 +320,13 @@ with_channel(GwName, ClientId, Fun) -> %%-------------------------------------------------------------------- -spec reason2resp({atom(), map()} | any()) -> binary() | any(). -reason2resp({badconf, #{key := Key, value := Value, reason := Reason}}) -> - fmt400err("Bad config value '~s' for '~s', reason: ~s", - [Value, Key, Reason]); -reason2resp({badres, #{resource := gateway, - gateway := GwName, - reason := not_found}}) -> - fmt400err("The ~s gateway is unloaded", [GwName]); - -reason2resp({badres, #{resource := gateway, - gateway := GwName, - reason := already_exist}}) -> - fmt400err("The ~s gateway has loaded", [GwName]); - -reason2resp({badres, #{resource := listener, - listener := {GwName, LType, LName}, - reason := not_found}}) -> - fmt400err("Listener ~s not found", - [listener_id(GwName, LType, LName)]); - -reason2resp({badres, #{resource := listener, - listener := {GwName, LType, LName}, - reason := already_exist}}) -> - fmt400err("The listener ~s of ~s already exist", - [listener_id(GwName, LType, LName), GwName]); - -reason2resp({badres, #{resource := authn, - gateway := GwName, - reason := not_found}}) -> - fmt400err("The authentication not found on ~s", [GwName]); - -reason2resp({badres, #{resource := authn, - gateway := GwName, - reason := already_exist}}) -> - fmt400err("The authentication already exist on ~s", [GwName]); - -reason2resp({badres, #{resource := listener_authn, - listener := {GwName, LType, LName}, - reason := not_found}}) -> - fmt400err("The authentication not found on ~s", - [listener_id(GwName, LType, LName)]); - -reason2resp({badres, #{resource := listener_authn, - listener := {GwName, LType, LName}, - reason := already_exist}}) -> - fmt400err("The authentication already exist on ~s", - [listener_id(GwName, LType, LName)]); - -reason2resp(R) -> return_http_error(500, R). - -fmt400err(Fmt, Args) -> - return_http_error(400, io_lib:format(Fmt, Args)). +reason2resp(R) -> + case reason2msg(R) of + error -> + return_http_error(500, R); + Msg -> + return_http_error(400, Msg) + end. -spec return_http_error(integer(), any()) -> {integer(), binary()}. return_http_error(Code, Msg) -> @@ -377,6 +336,56 @@ return_http_error(Code, Msg) -> }) }. +-spec reason2msg({atom(), map()} | any()) -> error | io_lib:chars(). +reason2msg({badconf, #{key := Key, value := Value, reason := Reason}}) -> + io_lib:format("Bad config value '~s' for '~s', reason: ~s", + [Value, Key, Reason]); +reason2msg({badres, #{resource := gateway, + gateway := GwName, + reason := not_found}}) -> + io_lib:format("The ~s gateway is unloaded", [GwName]); + +reason2msg({badres, #{resource := gateway, + gateway := GwName, + reason := already_exist}}) -> + io_lib:format("The ~s gateway already loaded", [GwName]); + +reason2msg({badres, #{resource := listener, + listener := {GwName, LType, LName}, + reason := not_found}}) -> + io_lib:format("Listener ~s not found", + [listener_id(GwName, LType, LName)]); + +reason2msg({badres, #{resource := listener, + listener := {GwName, LType, LName}, + reason := already_exist}}) -> + io_lib:format("The listener ~s of ~s already exist", + [listener_id(GwName, LType, LName), GwName]); + +reason2msg({badres, #{resource := authn, + gateway := GwName, + reason := not_found}}) -> + io_lib:format("The authentication not found on ~s", [GwName]); + +reason2msg({badres, #{resource := authn, + gateway := GwName, + reason := already_exist}}) -> + io_lib:format("The authentication already exist on ~s", [GwName]); + +reason2msg({badres, #{resource := listener_authn, + listener := {GwName, LType, LName}, + reason := not_found}}) -> + io_lib:format("The authentication not found on ~s", + [listener_id(GwName, LType, LName)]); + +reason2msg({badres, #{resource := listener_authn, + listener := {GwName, LType, LName}, + reason := already_exist}}) -> + io_lib:format("The authentication already exist on ~s", + [listener_id(GwName, LType, LName)]); +reason2msg(_) -> + error. + codestr(400) -> 'BAD_REQUEST'; codestr(401) -> 'NOT_SUPPORTED_NOW'; codestr(404) -> 'RESOURCE_NOT_FOUND'; diff --git a/apps/emqx_gateway/test/emqx_gateway_cli_SUITE.erl b/apps/emqx_gateway/test/emqx_gateway_cli_SUITE.erl index cf27dc027..618908f92 100644 --- a/apps/emqx_gateway/test/emqx_gateway_cli_SUITE.erl +++ b/apps/emqx_gateway/test/emqx_gateway_cli_SUITE.erl @@ -29,6 +29,23 @@ gateway {} ">>). +%% The config with json format for mqtt-sn gateway +-define(CONF_MQTTSN, " +{\"idle_timeout\": \"30s\", + \"enable_stats\": true, + \"mountpoint\": \"mqttsn/\", + \"gateway_id\": 1, + \"broadcast\": true, + \"enable_qos3\": true, + \"predefined\": [{\"id\": 1001, \"topic\": \"pred/a\"}], + \"listeners\": + [{\"type\": \"udp\", + \"name\": \"ct\", + \"bind\": \"2885\" + }] +} +"). + %%-------------------------------------------------------------------- %% Setup %%-------------------------------------------------------------------- @@ -109,16 +126,61 @@ t_gateway_list(_) -> "Gateway(name=stomp, status=unloaded)\n" , acc_print()). -t_gateway_load(_) -> +t_gateway_load_unload_lookup(_) -> + emqx_gateway_cli:gateway(["lookup", "mqttsn"]), + ?assertEqual("undefined\n", acc_print()), + + emqx_gateway_cli:gateway(["load", "mqttsn", ?CONF_MQTTSN]), + ?assertEqual("ok\n", acc_print()), + + %% TODO: bad config name, format??? + + emqx_gateway_cli:gateway(["lookup", "mqttsn"]), + %% TODO: assert it. for example: + %% name: mqttsn + %% status: running + %% created_at: 2022-01-05T14:40:20.039+08:00 + %% started_at: 2022-01-05T14:42:37.894+08:00 + %% config: #{broadcast => false,enable => true,enable_qos3 => true, + %% enable_stats => true,gateway_id => 1,idle_timeout => 30000, + %% mountpoint => <<>>,predefined => []} + _ = acc_print(), + + emqx_gateway_cli:gateway(["load", "mqttsn", "{}"]), + ?assertEqual( + "Error: The mqttsn gateway already loaded\n" + , acc_print()), + + emqx_gateway_cli:gateway(["load", "bad-gw-name", "{}"]), + %% TODO: assert it. for example: + %% Error: Illegal gateway name + _ = acc_print(), + + emqx_gateway_cli:gateway(["unload", "mqttsn"]), + ?assertEqual("ok\n", acc_print()), + %% Always return ok, even the gateway has unloaded + emqx_gateway_cli:gateway(["unload", "mqttsn"]), + ?assertEqual("ok\n", acc_print()), + + emqx_gateway_cli:gateway(["lookup", "mqttsn"]), + ?assertEqual("undefined\n", acc_print()), ok. -t_gateway_unload(_) -> - ok. +t_gateway_start_stop(_) -> + emqx_gateway_cli:gateway(["load", "mqttsn", ?CONF_MQTTSN]), + ?assertEqual("ok\n", acc_print()), -t_gateway_start(_) -> - ok. + emqx_gateway_cli:gateway(["stop", "mqttsn"]), + ?assertEqual("ok\n", acc_print()), + %% dupliacted stop gateway, return ok + emqx_gateway_cli:gateway(["stop", "mqttsn"]), + ?assertEqual("ok\n", acc_print()), -t_gateway_stop(_) -> + emqx_gateway_cli:gateway(["start", "mqttsn"]), + ?assertEqual("ok\n", acc_print()), + %% dupliacted start gateway, return ok + emqx_gateway_cli:gateway(["start", "mqttsn"]), + ?assertEqual("ok\n", acc_print()), ok. t_gateway_clients_usage(_) ->