Merge pull request #5263 from DDDHuang/apps_api

refactor: apps api; rename: error response util function

apps/emqx_management/src/emqx_mgmt_api_apps.erl

@@ -16,88 +16,227 @@
 
 -module(emqx_mgmt_api_apps).
 
--include("emqx_mgmt.hrl").
+-behavior(minirest_api).
 
--rest_api(#{name   => add_app,
-            method => 'POST',
-            path   => "/apps/",
-            func   => add_app,
-            descr  => "Add Application"}).
+-export([api_spec/0]).
 
--rest_api(#{name   => del_app,
-            method => 'DELETE',
-            path   => "/apps/:bin:appid",
-            func   => del_app,
-            descr  => "Delete Application"}).
 
--rest_api(#{name   => list_apps,
-            method => 'GET',
-            path   => "/apps/",
-            func   => list_apps,
-            descr  => "List Applications"}).
+-export([ apps/2
+        , app/2]).
 
--rest_api(#{name   => lookup_app,
-            method => 'GET',
-            path   => "/apps/:bin:appid",
-            func   => lookup_app,
-            descr  => "Lookup Application"}).
+-define(BAD_APP_ID, 'BAD_APP_ID').
+-define(APP_ID_NOT_FOUND, <<"{\"code\": \"BAD_APP_ID\", \"reason\": \"App id not found\"}">>).
 
--rest_api(#{name   => update_app,
-            method => 'PUT',
-            path   => "/apps/:bin:appid",
-            func   => update_app,
-            descr  => "Update Application"}).
+api_spec() ->
+    {
+        [apps_api(), app_api()],
+        [app_schema(), app_secret_schema()]
+    }.
 
--export([ add_app/2
-        , del_app/2
-        , list_apps/2
-        , lookup_app/2
-        , update_app/2
-        ]).
+app_schema() ->
+     #{app => #{
+         type => object,
+         properties => app_properties()}}.
 
-add_app(_Bindings, Params) ->
-    AppId = proplists:get_value(<<"app_id">>, Params),
-    Name = proplists:get_value(<<"name">>, Params),
-    Secret = proplists:get_value(<<"secret">>, Params),
-    Desc = proplists:get_value(<<"desc">>, Params),
-    Status = proplists:get_value(<<"status">>, Params),
-    Expired = proplists:get_value(<<"expired">>, Params),
-    case emqx_mgmt_auth:add_app(AppId, Name, Secret, Desc, Status, Expired) of
-        {ok, AppSecret} -> emqx_mgmt:return({ok, #{secret => AppSecret}});
-        {error, Reason} -> emqx_mgmt:return({error, Reason})
+app_properties() ->
+    #{
+        app_id => #{
+            type => string,
+            description => <<"App ID">>},
+        secret => #{
+            type => string,
+            description => <<"App Secret">>},
+        name => #{
+            type => string,
+            description => <<"Dsiplay name">>},
+        desc => #{
+            type => string,
+            description => <<"App description">>},
+        status => #{
+            type => boolean,
+            description => <<"Enable or disable">>},
+        expired => #{
+            type => integer,
+            description => <<"Expired time">>}
+    }.
+
+app_secret_schema() ->
+    #{app_secret => #{
+        type => object,
+        properties => #{
+            secret => #{type => string}}}}.
+
+%% not export schema
+app_without_secret_schema() ->
+    #{
+        type => object,
+        properties => maps:without([secret], app_properties())
+    }.
+
+apps_api() ->
+    Metadata = #{
+        get => #{
+            description => "List EMQ X apps",
+            responses => #{
+                <<"200">> =>
+                    emqx_mgmt_util:response_array_schema(<<"All apps">>,
+                        app_without_secret_schema())}},
+        post => #{
+            description => "EMQ X create apps",
+            'requestBody' => emqx_mgmt_util:request_body_schema(<<"app">>),
+            responses => #{
+                <<"200">> =>
+                    emqx_mgmt_util:response_schema(<<"Create apps">>, <<"app_secret">>),
+                <<"400">> =>
+                    emqx_mgmt_util:response_error_schema(<<"App ID already exist">>, [?BAD_APP_ID])}}},
+    {"/apps", Metadata, apps}.
+
+app_api() ->
+    Metadata = #{
+        get => #{
+            description => "EMQ X apps",
+            parameters => [#{
+                name => app_id,
+                in => path,
+                required => true,
+                schema => #{type => string},
+                example => <<"admin">>}],
+            responses => #{
+                <<"404">> =>
+                    emqx_mgmt_util:response_error_schema(<<"App id not found">>),
+                <<"200">> =>
+                    emqx_mgmt_util:response_schema("Get App", app_without_secret_schema())}},
+        delete => #{
+            description => "EMQ X apps",
+            parameters => [#{
+                name => app_id,
+                in => path,
+                required => true,
+                schema => #{type => string},
+                example => <<"admin">>}],
+            responses => #{
+                <<"200">> => emqx_mgmt_util:response_schema("Remove app ok")}},
+        put => #{
+            description => "EMQ X update apps",
+            parameters => [#{
+                name => app_id,
+                in => path,
+                required => true,
+                schema => #{type => string},
+                default => <<"admin">>
+            }],
+            'requestBody' => emqx_mgmt_util:request_body_schema(app_without_secret_schema()),
+            responses => #{
+                <<"404">> =>
+                    emqx_mgmt_util:response_error_schema(<<"App id not found">>, [?BAD_APP_ID]),
+                <<"200">> =>
+                    emqx_mgmt_util:response_schema(<<"Update ok">>, app_without_secret_schema())}}},
+    {"/apps/:app_id", Metadata, app}.
+
+%%%==============================================================================================
+%% parameters trans
+apps(get, _Request) ->
+    list(#{});
+
+apps(post, Request) ->
+    {ok, Body, _} = cowboy_req:read_body(Request),
+    Data = emqx_json:decode(Body, [return_maps]),
+    Parameters = #{
+        app_id  => maps:get(<<"app_id">>, Data),
+        name    => maps:get(<<"name">>, Data),
+        secret  => maps:get(<<"secret">>, Data),
+        desc    => maps:get(<<"desc">>, Data),
+        status  => maps:get(<<"status">>, Data),
+        expired => maps:get(<<"expired">>, Data, undefined)
+    },
+    create(Parameters).
+
+app(get, Request) ->
+    AppID = cowboy_req:binding(app_id, Request),
+    lookup(#{app_id => AppID});
+
+app(delete, Request) ->
+    AppID = cowboy_req:binding(app_id, Request),
+    delete(#{app_id => AppID});
+
+app(put, Request) ->
+    AppID = cowboy_req:binding(app_id, Request),
+    {ok, Body, _} = cowboy_req:read_body(Request),
+    Data = emqx_json:decode(Body, [return_maps]),
+    Parameters = #{
+        app_id  => AppID,
+        name    => maps:get(<<"name">>, Data),
+        desc    => maps:get(<<"desc">>, Data),
+        status  => maps:get(<<"status">>, Data),
+        expired => maps:get(<<"expired">>, Data, undefined)
+    },
+    update(Parameters).
+
+
+%%%==============================================================================================
+%% api apply
+list(_) ->
+    Data = [format_without_app_secret(Apps) || Apps <- emqx_mgmt_auth:list_apps()],
+    Response = emqx_json:encode(Data),
+    {200, Response}.
+
+create(#{app_id := AppID, name := Name, secret := Secret,
+    desc := Desc, status := Status, expired := Expired}) ->
+    case emqx_mgmt_auth:add_app(AppID, Name, Secret, Desc, Status, Expired) of
+        {ok, AppSecret} ->
+            Response = emqx_json:encode(#{secret => AppSecret}),
+            {200, Response};
+        {error, alread_existed} ->
+            Message = list_to_binary(io_lib:format("appid ~p already existed", [AppID])),
+            {400, #{code => 'BAD_APP_ID', reason => Message}};
+        {error, Reason} ->
+            Data = #{code => 'UNKNOW_ERROR',
+                reason => list_to_binary(io_lib:format("~p", [Reason]))},
+            Response = emqx_json:encode(Data),
+            {500, Response}
     end.
 
-del_app(#{appid := AppId}, _Params) ->
-    case emqx_mgmt_auth:del_app(AppId) of
-        ok -> emqx_mgmt:return();
-        {error, Reason} -> emqx_mgmt:return({error, Reason})
-    end.
-
-list_apps(_Bindings, _Params) ->
-    emqx_mgmt:return({ok, [format(Apps)|| Apps <- emqx_mgmt_auth:list_apps()]}).
-
-lookup_app(#{appid := AppId}, _Params) ->
-    case emqx_mgmt_auth:lookup_app(AppId) of
-        {AppId, AppSecret, Name, Desc, Status, Expired} ->
-            emqx_mgmt:return({ok, #{app_id => AppId,
-                          secret => AppSecret,
-                          name => Name,
-                          desc => Desc,
-                          status => Status,
-                          expired => Expired}});
+lookup(#{app_id := AppID}) ->
+    case emqx_mgmt_auth:lookup_app(AppID) of
         undefined ->
-            emqx_mgmt:return({ok, #{}})
+            {404, ?APP_ID_NOT_FOUND};
+        App ->
+            Data = format_with_app_secret(App),
+            Response = emqx_json:encode(Data),
+            {200, Response}
     end.
 
-update_app(#{appid := AppId}, Params) ->
-    Name = proplists:get_value(<<"name">>, Params),
-    Desc = proplists:get_value(<<"desc">>, Params),
-    Status = proplists:get_value(<<"status">>, Params),
-    Expired = proplists:get_value(<<"expired">>, Params),
-    case emqx_mgmt_auth:update_app(AppId, Name, Desc, Status, Expired) of
-        ok -> emqx_mgmt:return();
-        {error, Reason} -> emqx_mgmt:return({error, Reason})
+delete(#{app_id := AppID}) ->
+    _ = emqx_mgmt_auth:del_app(AppID),
+    {200}.
+
+update(App = #{app_id := AppID, name := Name, desc := Desc, status := Status, expired := Expired}) ->
+    case emqx_mgmt_auth:update_app(AppID, Name, Desc, Status, Expired) of
+        ok ->
+            {200, App};
+        {error, not_found} ->
+            {404, ?APP_ID_NOT_FOUND};
+        {error, Reason} ->
+            Data = #{code => 'UNKNOW_ERROR', reason => list_to_binary(io_lib:format("~p", [Reason]))},
+            Response = emqx_json:encode(Data),
+            {500, Response}
     end.
 
-format({AppId, _AppSecret, Name, Desc, Status, Expired}) ->
-    [{app_id, AppId}, {name, Name}, {desc, Desc}, {status, Status}, {expired, Expired}].
+%%%==============================================================================================
+%% format
+format_without_app_secret(App) ->
+    format_without([secret], App).
+
+format_with_app_secret(App) ->
+    format_without([], App).
+
+format_without(List, {AppID, AppSecret, Name, Desc, Status, Expired}) ->
+    Data = #{
+        app_id => AppID,
+        secret => AppSecret,
+        name => Name,
+        desc => Desc,
+        status => Status,
+        expired => Expired
+    },
+    maps:without(List, Data).

apps/emqx_management/src/emqx_mgmt_api_clients.erl

@@ -230,7 +230,7 @@ client_api() ->
                 required => true,
                 example => 123456}],
             responses => #{
-                <<"404">> => emqx_mgmt_util:not_found_schema(<<"Client id not found">>),
+                <<"404">> => emqx_mgmt_util:response_error_schema(<<"Client id not found">>),
                 <<"200">> => emqx_mgmt_util:response_schema(<<"List clients 200 OK">>, <<"client">>)}},
         delete => #{
             description => "Kick out client by client ID",
@@ -241,7 +241,7 @@ client_api() ->
                 required => true,
                 example => 123456}],
             responses => #{
-                <<"404">> => emqx_mgmt_util:not_found_schema(<<"Client id not found">>),
+                <<"404">> => emqx_mgmt_util:response_error_schema(<<"Client id not found">>),
                 <<"200">> => emqx_mgmt_util:response_schema(<<"List clients 200 OK">>, <<"client">>)}}},
     {"/clients/:clientid", Metadata, client}.
 
@@ -256,7 +256,7 @@ clients_acl_cache_api() ->
                 required => true,
                 example => 123456}],
             responses => #{
-                <<"404">> => emqx_mgmt_util:not_found_schema(<<"Client id not found">>),
+                <<"404">> => emqx_mgmt_util:response_error_schema(<<"Client id not found">>),
                 <<"200">> => emqx_mgmt_util:response_schema(<<"List clients 200 OK">>, <<"acl_cache">>)}},
         delete => #{
             description => "Clean client acl cache",
@@ -267,7 +267,7 @@ clients_acl_cache_api() ->
                 required => true,
                 example => 123456}],
             responses => #{
-                <<"404">> => emqx_mgmt_util:not_found_schema(<<"Client id not found">>),
+                <<"404">> => emqx_mgmt_util:response_error_schema(<<"Client id not found">>),
                 <<"200">> => emqx_mgmt_util:response_schema(<<"Delete clients 200 OK">>)}}},
     {"/clients/:clientid/acl_cache", Metadata, acl_cache}.
 
@@ -297,7 +297,7 @@ subscribe_api() ->
                         example => 0,
                         description => <<"QoS">>}}}),
             responses => #{
-                <<"404">> => emqx_mgmt_util:not_found_schema(<<"Client id not found">>),
+                <<"404">> => emqx_mgmt_util:response_error_schema(<<"Client id not found">>),
                 <<"200">> => emqx_mgmt_util:response_schema(<<"subscribe ok">>)}},
         delete => #{
             description => "unsubscribe",
@@ -318,7 +318,7 @@ subscribe_api() ->
                 }
             ],
             responses => #{
-                <<"404">> => emqx_mgmt_util:not_found_schema(<<"Client id not found">>),
+                <<"404">> => emqx_mgmt_util:response_error_schema(<<"Client id not found">>),
                 <<"200">> => emqx_mgmt_util:response_schema(<<"unsubscribe ok">>)}}},
     {"/clients/:clientid/subscribe", Metadata, subscribe}.
 

apps/emqx_management/src/emqx_mgmt_api_nodes.erl

@@ -120,7 +120,7 @@ node_api() ->
                 required => true,
                 example => node()}],
             responses => #{
-                <<"400">> => emqx_mgmt_util:not_found_schema(<<"Node error">>, [<<"SOURCE_ERROR">>]),
+                <<"400">> => emqx_mgmt_util:response_error_schema(<<"Node error">>, ['SOURCE_ERROR']),
                 <<"200">> => emqx_mgmt_util:response_schema(<<"Get EMQ X Nodes info by name">>, <<"node">>)}}},
     {"/nodes/:node_name", Metadata, node}.
 
@@ -136,7 +136,7 @@ node_metrics_api() ->
                 required => true,
                 example => node()}],
             responses => #{
-                <<"400">> => emqx_mgmt_util:not_found_schema(<<"Node error">>, [<<"SOURCE_ERROR">>]),
+                <<"400">> => emqx_mgmt_util:response_error_schema(<<"Node error">>, ['SOURCE_ERROR']),
                 <<"200">> => emqx_mgmt_util:response_schema(<<"Get EMQ X Node Metrics">>, <<"metrics">>)}}},
     {"/nodes/:node_name/metrics", Metadata, node_metrics}.
 
@@ -152,7 +152,7 @@ node_stats_api() ->
                 required => true,
                 example => node()}],
             responses => #{
-                <<"400">> => emqx_mgmt_util:not_found_schema(<<"Node error">>, [<<"SOURCE_ERROR">>]),
+                <<"400">> => emqx_mgmt_util:response_error_schema(<<"Node error">>, ['SOURCE_ERROR']),
                 <<"200">> => emqx_mgmt_util:response_schema(<<"Get EMQ X Node Stats">>, <<"stats">>)}}},
     {"/nodes/:node_name/stats", Metadata, node_metrics}.
 

apps/emqx_management/src/emqx_mgmt_util.erl

@@ -29,8 +29,8 @@
         , response_schema/1
         , response_schema/2
         , response_array_schema/2
-        , not_found_schema/1
-        , not_found_schema/2
+        , response_error_schema/1
+        , response_error_schema/2
         , batch_response_schema/1]).
 
 -export([urldecode/1]).
@@ -113,10 +113,11 @@ response_schema(Description, Schema) when is_map(Schema) ->
 response_schema(Description, Ref) when is_binary(Ref) ->
     json_content_schema(Description, minirest:ref(Ref)).
 
-not_found_schema(Description) ->
-    not_found_schema(Description, ["RESOURCE_NOT_FOUND"]).
+%% @doc default code is RESOURCE_NOT_FOUND
+response_error_schema(Description) ->
+    response_error_schema(Description, ['RESOURCE_NOT_FOUND']).
 
-not_found_schema(Description, Enum) ->
+response_error_schema(Description, Enum) ->
     Schema = #{
         type => object,
         properties => #{

apps/emqx_management/test/emqx_mgmt_apps_api_SUITE.erl

@@ -0,0 +1,110 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2020-2021 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_mgmt_apps_api_SUITE).
+
+-compile(export_all).
+-compile(nowarn_export_all).
+
+-include_lib("eunit/include/eunit.hrl").
+
+all() ->
+    emqx_ct:all(?MODULE).
+
+init_per_suite(Config) ->
+    ekka_mnesia:start(),
+    emqx_mgmt_auth:mnesia(boot),
+    emqx_ct_helpers:start_apps([emqx_management], fun set_special_configs/1),
+    Config.
+
+end_per_suite(_) ->
+    emqx_ct_helpers:stop_apps([emqx_management]).
+
+set_special_configs(emqx_management) ->
+    emqx_config:put([emqx_management], #{listeners => [#{protocol => http, port => 8081}],
+        applications =>[#{id => "admin", secret => "public"}]}),
+    ok;
+set_special_configs(_App) ->
+    ok.
+
+t_list_app(_) ->
+    Path = emqx_mgmt_api_test_util:api_path(["apps"]),
+    {ok, Body} = emqx_mgmt_api_test_util:request_api(get, Path),
+    Data = emqx_json:decode(Body, [return_maps]),
+    AdminApp = hd(Data),
+    Admin = maps:get(<<"app_id">>, AdminApp),
+    ?assertEqual(<<"admin">>, Admin).
+
+t_get_app(_) ->
+    Path = emqx_mgmt_api_test_util:api_path(["apps/admin"]),
+    {ok, Body} = emqx_mgmt_api_test_util:request_api(get, Path),
+    AdminApp = emqx_json:decode(Body, [return_maps]),
+    ?assertEqual(<<"admin">>, maps:get(<<"app_id">>, AdminApp)),
+    ?assertEqual(<<"public">>, maps:get(<<"secret">>, AdminApp)).
+
+t_add_app(_) ->
+    AuthHeader = emqx_mgmt_api_test_util:auth_header_(),
+    AppId = <<"test_app_id">>,
+    TestAppPath = emqx_mgmt_api_test_util:api_path(["apps", AppId]),
+    AppSecret = <<"test_app_secret">>,
+
+    %% new test app
+    Path = emqx_mgmt_api_test_util:api_path(["apps"]),
+    RequestBody = #{
+        app_id => AppId,
+        secret => AppSecret,
+        desc => <<"test desc">>,
+        name => <<"test_app_name">>,
+        expired => erlang:system_time(second) + 3000,
+        status => true
+    },
+    {ok, Body} = emqx_mgmt_api_test_util:request_api(post, Path, "", AuthHeader, RequestBody),
+    TestAppSecret = emqx_json:decode(Body, [return_maps]),
+    ?assertEqual(AppSecret, maps:get(<<"secret">>, TestAppSecret)),
+
+    %% get new test app
+    {ok, GetApp} = emqx_mgmt_api_test_util:request_api(get, TestAppPath),
+    TestApp = emqx_json:decode(GetApp, [return_maps]),
+    ?assertEqual(AppId, maps:get(<<"app_id">>, TestApp)),
+    ?assertEqual(AppSecret, maps:get(<<"secret">>, TestApp)),
+
+    %% update app
+    Desc2 = <<"test desc 2">>,
+    Name2 = <<"test_app_name_2">>,
+    PutBody = #{
+        desc => Desc2,
+        name => Name2,
+        expired => erlang:system_time(second) + 3000,
+        status => false
+    },
+    {ok, PutApp} = emqx_mgmt_api_test_util:request_api(put, TestAppPath, "", AuthHeader, PutBody),
+    TestApp1 = emqx_json:decode(PutApp, [return_maps]),
+    ?assertEqual(Desc2, maps:get(<<"desc">>, TestApp1)),
+    ?assertEqual(Name2, maps:get(<<"name">>, TestApp1)),
+    ?assertEqual(false, maps:get(<<"status">>, TestApp1)),
+
+    %% after update
+    {ok, GetApp2} = emqx_mgmt_api_test_util:request_api(get, TestAppPath),
+    TestApp2 = emqx_json:decode(GetApp2, [return_maps]),
+    ?assertEqual(Desc2, maps:get(<<"desc">>, TestApp2)),
+    ?assertEqual(Name2, maps:get(<<"name">>, TestApp2)),
+    ?assertEqual(false, maps:get(<<"status">>, TestApp2)),
+
+    %% delete new app
+    {ok, _} = emqx_mgmt_api_test_util:request_api(delete, TestAppPath),
+
+    %% after delete
+    ?assertEqual({error,{"HTTP/1.1",404,"Not Found"}},
+        emqx_mgmt_api_test_util:request_api(get, TestAppPath)).