refactor(gw): more readable CLI print

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

apps/emqx_gateway/src/emqx_gateway_cli.erl

@@ -53,23 +53,16 @@ is_cmd(Fun) ->
 
 gateway(["list"]) ->
     lists:foreach(
-      fun (#{name := Name, status := unloaded}) ->
+    fun (GwSummary) ->
-            print("Gateway(name=~ts, status=unloaded)\n", [Name]);
+        print(format_gw_summary(GwSummary))
-          (#{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])
     end, emqx_gateway_http:gateways(all));
 
 gateway(["lookup", Name]) ->
     case emqx_gateway:lookup(atom(Name)) of
         undefined ->
             print("undefined\n");
-        Info ->
+        Gateway ->
-            print("~p\n", [Info])
+            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",
+    emqx_ctl:usage(
-                        "List all gateway"}
+      [ {"gateway list",
-                   , {"gateway lookup <Name>",
+           "List all gateway"}
-                        "Lookup a gateway detailed informations"}
+      , {"gateway lookup <Name>",
-                   , {"gateway load   <Name> <JsonConf>",
+           "Lookup a gateway detailed informations"}
-                        "Load a gateway with config"}
+      , {"gateway load   <Name> <JsonConf>",
-                   , {"gateway unload <Name>",
+           "Load a gateway with config"}
-                        "Unload the gateway"}
+      , {"gateway unload <Name>",
-                   , {"gateway stop   <Name>",
+           "Unload the gateway"}
-                        "Stop the gateway"}
+      , {"gateway stop   <Name>",
-                   , {"gateway start  <Name>",
+           "Stop the gateway"}
-                        "Start the gateway"}
+      , {"gateway start  <Name>",
-                   ]).
+           "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.

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}})

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}}) ->
+reason2resp(R) ->
-    fmt400err("Bad config value '~s' for '~s', reason: ~s",
+    case reason2msg(R) of
-              [Value, Key, Reason]);
+        error ->
-reason2resp({badres, #{resource := gateway,
+            return_http_error(500, R);
-                       gateway := GwName,
+        Msg ->
-                       reason := not_found}}) ->
+            return_http_error(400, Msg)
-    fmt400err("The ~s gateway is unloaded", [GwName]);
+    end.
-
-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)).
 
 -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';

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(_) ->
+t_gateway_start_stop(_) ->
-    ok.
+    emqx_gateway_cli:gateway(["load", "mqttsn", ?CONF_MQTTSN]),
+    ?assertEqual("ok\n", acc_print()),
 
-t_gateway_start(_) ->
+    emqx_gateway_cli:gateway(["stop", "mqttsn"]),
-    ok.
+    ?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(_) ->