Merge pull request #7291 from JimMoen/unified-move-api

Unified move api

apps/emqx/include/emqx_authentication.hrl

@@ -28,4 +28,10 @@
 %% and emqx_conf_schema for an examples
 -define(EMQX_AUTHENTICATION_SCHEMA_MODULE_PT_KEY, emqx_authentication_schema_module).
 
+%% authentication move cmd
+-define(CMD_MOVE_FRONT, front).
+-define(CMD_MOVE_REAR, rear).
+-define(CMD_MOVE_BEFORE(Before), {before, Before}).
+-define(CMD_MOVE_AFTER(After), {'after', After}).
+
 -endif.

apps/emqx/src/emqx_authentication.erl

@@ -100,7 +100,7 @@
 
 -type chain_name() :: atom().
 -type authenticator_id() :: binary().
--type position() :: top | bottom | {before, authenticator_id()}.
+-type position() :: front | rear | {before, authenticator_id()} | {'after', authenticator_id()}.
 -type authn_type() :: atom() | {atom(), atom()}.
 -type provider() :: module().
 
@@ -695,21 +695,29 @@ do_move_authenticator(ID, Authenticators, Position) ->
             {error, {not_found, {authenticator, ID}}};
         {value, Authenticator, NAuthenticators} ->
             case Position of
-                top ->
+                ?CMD_MOVE_FRONT ->
                     {ok, [Authenticator | NAuthenticators]};
-                bottom ->
+                ?CMD_MOVE_REAR ->
                     {ok, NAuthenticators ++ [Authenticator]};
-                {before, ID0} ->
-                    insert(Authenticator, NAuthenticators, ID0, [])
+                ?CMD_MOVE_BEFORE(RelatedID) ->
+                    insert(Authenticator, NAuthenticators, ?CMD_MOVE_BEFORE(RelatedID), []);
+                ?CMD_MOVE_AFTER(RelatedID) ->
+                    insert(Authenticator, NAuthenticators, ?CMD_MOVE_AFTER(RelatedID), [])
             end
     end.
 
-insert(_, [], ID, _) ->
-    {error, {not_found, {authenticator, ID}}};
-insert(Authenticator, [#authenticator{id = ID} | _] = Authenticators, ID, Acc) ->
-    {ok, lists:reverse(Acc) ++ [Authenticator | Authenticators]};
-insert(Authenticator, [Authenticator0 | More], ID, Acc) ->
-    insert(Authenticator, More, ID, [Authenticator0 | Acc]).
+insert(_, [], {_, RelatedID}, _) ->
+    {error, {not_found, {authenticator, RelatedID}}};
+insert(Authenticator, [#authenticator{id = RelatedID} = Related | Rest],
+       {Relative, RelatedID}, Acc) ->
+    case Relative of
+        before ->
+            {ok, lists:reverse(Acc) ++ [Authenticator, Related | Rest]};
+        'after' ->
+            {ok, lists:reverse(Acc) ++ [Related, Authenticator | Rest]}
+    end;
+insert(Authenticator, [Authenticator0 | More], {Relative, RelatedID}, Acc) ->
+    insert(Authenticator, More, {Relative, RelatedID}, [Authenticator0 | Acc]).
 
 update_chain(ChainName, UpdateFun) ->
     case ets:lookup(?CHAINS_TAB, ChainName) of

apps/emqx/src/emqx_authentication_config.erl

@@ -87,24 +87,36 @@ do_pre_config_update({update_authenticator, ChainName, AuthenticatorID, Config},
 do_pre_config_update({move_authenticator, _ChainName, AuthenticatorID, Position}, OldConfig) ->
     case split_by_id(AuthenticatorID, OldConfig) of
         {error, Reason} -> {error, Reason};
-        {ok, Part1, [Found | Part2]} ->
+        {ok, BeforeFound, [Found | AfterFound]} ->
             case Position of
-                top ->
-                    {ok, [Found | Part1] ++ Part2};
-                bottom ->
-                    {ok, Part1 ++ Part2 ++ [Found]};
-                {before, Before} ->
-                    case split_by_id(Before, Part1 ++ Part2) of
+                ?CMD_MOVE_FRONT ->
+                    {ok, [Found | BeforeFound] ++ AfterFound};
+                ?CMD_MOVE_REAR ->
+                    {ok, BeforeFound ++ AfterFound ++ [Found]};
+                ?CMD_MOVE_BEFORE(BeforeRelatedID) ->
+                    case split_by_id(BeforeRelatedID, BeforeFound ++ AfterFound) of
                         {error, Reason} ->
                             {error, Reason};
-                        {ok, NPart1, [NFound | NPart2]} ->
-                            {ok, NPart1 ++ [Found, NFound | NPart2]}
+                        {ok, BeforeNFound, [FoundRelated | AfterNFound]} ->
+                            {ok, BeforeNFound ++ [Found, FoundRelated | AfterNFound]}
+                    end;
+                ?CMD_MOVE_AFTER(AfterRelatedID) ->
+                    case split_by_id(AfterRelatedID, BeforeFound ++ AfterFound) of
+                        {error, Reason} ->
+                            {error, Reason};
+                        {ok, BeforeNFound, [FoundRelated | AfterNFound]} ->
+                            {ok, BeforeNFound ++ [FoundRelated, Found | AfterNFound]}
                     end
             end
     end.
 
--spec post_config_update(list(atom()), update_request(), map() | list(), emqx_config:raw_config(), emqx_config:app_envs())
-    -> ok | {ok, map()} | {error, term()}.
+-spec post_config_update(list(atom()),
+                         update_request(),
+                         map() | list(),
+                         emqx_config:raw_config(),
+                         emqx_config:app_envs()
+                        )
+                        -> ok | {ok, map()} | {error, term()}.
 post_config_update(_, UpdateReq, NewConfig, OldConfig, AppEnvs) ->
     do_post_config_update(UpdateReq, check_configs(to_list(NewConfig)), OldConfig, AppEnvs).
 
@@ -112,7 +124,8 @@ do_post_config_update({create_authenticator, ChainName, Config}, NewConfig, _Old
     NConfig = get_authenticator_config(authenticator_id(Config), NewConfig),
     _ = emqx_authentication:create_chain(ChainName),
     emqx_authentication:create_authenticator(ChainName, NConfig);
-do_post_config_update({delete_authenticator, ChainName, AuthenticatorID}, _NewConfig, OldConfig, _AppEnvs) ->
+do_post_config_update({delete_authenticator, ChainName, AuthenticatorID},
+                      _NewConfig, OldConfig, _AppEnvs) ->
     case emqx_authentication:delete_authenticator(ChainName, AuthenticatorID) of
         ok ->
             Config = get_authenticator_config(AuthenticatorID, to_list(OldConfig)),
@@ -121,14 +134,16 @@ do_post_config_update({delete_authenticator, ChainName, AuthenticatorID}, _NewCo
         {error, Reason} ->
             {error, Reason}
     end;
-do_post_config_update({update_authenticator, ChainName, AuthenticatorID, Config}, NewConfig, _OldConfig, _AppEnvs) ->
+do_post_config_update({update_authenticator, ChainName, AuthenticatorID, Config},
+                      NewConfig, _OldConfig, _AppEnvs) ->
     case get_authenticator_config(authenticator_id(Config), NewConfig) of
         {error, not_found} ->
             {error, {not_found, {authenticator, AuthenticatorID}}};
         NConfig ->
             emqx_authentication:update_authenticator(ChainName, AuthenticatorID, NConfig)
     end;
-do_post_config_update({move_authenticator, ChainName, AuthenticatorID, Position}, _NewConfig, _OldConfig, _AppEnvs) ->
+do_post_config_update({move_authenticator, ChainName, AuthenticatorID, Position},
+                      _NewConfig, _OldConfig, _AppEnvs) ->
     emqx_authentication:move_authenticator(ChainName, AuthenticatorID, Position).
 
 check_configs(Configs) ->

apps/emqx/test/emqx_authentication_SUITE.erl

@@ -185,14 +185,17 @@ t_authenticator(Config) when is_list(Config) ->
     % Move authenticator
     ?assertMatch({ok, [#{id := ID1}, #{id := ID2}]}, ?AUTHN:list_authenticators(ChainName)),
 
-    ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, top)),
+    ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, ?CMD_MOVE_FRONT)),
     ?assertMatch({ok, [#{id := ID2}, #{id := ID1}]}, ?AUTHN:list_authenticators(ChainName)),
 
-    ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, bottom)),
+    ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, ?CMD_MOVE_REAR)),
     ?assertMatch({ok, [#{id := ID1}, #{id := ID2}]}, ?AUTHN:list_authenticators(ChainName)),
 
-    ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, {before, ID1})),
-    ?assertMatch({ok, [#{id := ID2}, #{id := ID1}]}, ?AUTHN:list_authenticators(ChainName));
+    ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, ?CMD_MOVE_BEFORE(ID1))),
+    ?assertMatch({ok, [#{id := ID2}, #{id := ID1}]}, ?AUTHN:list_authenticators(ChainName)),
+
+    ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, ?CMD_MOVE_AFTER(ID1))),
+    ?assertMatch({ok, [#{id := ID1}, #{id := ID2}]}, ?AUTHN:list_authenticators(ChainName));
 
 t_authenticator({'end', Config}) ->
     ?AUTHN:delete_chain(test),
@@ -211,7 +214,7 @@ t_authenticate(Config) when is_list(Config) ->
                    listener => ListenerID,
                    protocol => mqtt,
                    username => <<"good">>,
-			       password => <<"any">>},
+                   password => <<"any">>},
     ?assertEqual({ok, #{is_superuser => false}}, emqx_access_control:authenticate(ClientInfo)),
 
     register_provider(AuthNType, ?MODULE),
@@ -291,7 +294,7 @@ t_update_config(Config) when is_list(Config) ->
 
     ?assertMatch(
        {ok, _},
-       update_config([?CONF_ROOT], {move_authenticator, Global, ID2, top})),
+       update_config([?CONF_ROOT], {move_authenticator, Global, ID2, ?CMD_MOVE_FRONT})),
 
     ?assertMatch({ok, [#{id := ID2}, #{id := ID1}]}, ?AUTHN:list_authenticators(Global)),
 
@@ -344,7 +347,7 @@ t_update_config(Config) when is_list(Config) ->
 
     ?assertMatch(
        {ok, _},
-       update_config(ConfKeyPath, {move_authenticator, ListenerID, ID2, top})),
+       update_config(ConfKeyPath, {move_authenticator, ListenerID, ID2, ?CMD_MOVE_FRONT})),
 
     ?assertMatch(
        {ok, [#{id := ID2}, #{id := ID1}]},

apps/emqx_authn/src/emqx_authn_api.erl

@@ -1059,12 +1059,18 @@ serialize_error(Reason) ->
     {400, #{code => <<"BAD_REQUEST">>,
             message => binfmt("~p", [Reason])}}.
 
-parse_position(<<"top">>) ->
-    {ok, top};
-parse_position(<<"bottom">>) ->
-    {ok, bottom};
+parse_position(<<"front">>) ->
+    {ok, ?CMD_MOVE_FRONT};
+parse_position(<<"rear">>) ->
+    {ok, ?CMD_MOVE_REAR};
+parse_position(<<"before:">>) ->
+    {error, {invalid_parameter, position}};
+parse_position(<<"after:">>) ->
+    {error, {invalid_parameter, position}};
 parse_position(<<"before:", Before/binary>>) ->
-    {ok, {before, Before}};
+    {ok, ?CMD_MOVE_BEFORE(Before)};
+parse_position(<<"after:", After/binary>>) ->
+    {ok, ?CMD_MOVE_AFTER(After)};
 parse_position(_) ->
     {error, {invalid_parameter, position}}.
 
@@ -1199,16 +1205,16 @@ request_user_update_examples() ->
 
 request_move_examples() ->
     #{
-        move_to_top => #{
+        move_to_front => #{
             summary => <<"Move authenticator to the beginning of the chain">>,
             value => #{
-                position => <<"top">>
+                position => <<"front">>
             }
         },
-        move_to_bottom => #{
+        move_to_rear => #{
             summary => <<"Move authenticator to the end of the chain">>,
             value => #{
-                position => <<"bottom">>
+                position => <<"rear">>
             }
         },
         'move_before_password_based:built_in_database' => #{

apps/emqx_authn/test/emqx_authn_api_SUITE.erl

@@ -347,7 +347,7 @@ test_authenticator_move(PathPrefix) ->
        ],
        PathPrefix ++ [?CONF_NS]),
 
-    % Invalid moves
+    %% Invalid moves
 
     {ok, 400, _} = request(
                      post,
@@ -374,12 +374,13 @@ test_authenticator_move(PathPrefix) ->
                      uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]),
                      #{position => <<"before:password_based:redis">>}),
 
-    % Valid moves
+    %% Valid moves
 
+    %% test front
     {ok, 204, _} = request(
                      post,
                      uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]),
-                     #{position => <<"top">>}),
+                     #{position => <<"front">>}),
 
     ?assertAuthenticatorsMatch(
        [
@@ -389,10 +390,11 @@ test_authenticator_move(PathPrefix) ->
        ],
        PathPrefix ++ [?CONF_NS]),
 
+    %% test rear
     {ok, 204, _} = request(
                      post,
                      uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]),
-                     #{position => <<"bottom">>}),
+                     #{position => <<"rear">>}),
 
     ?assertAuthenticatorsMatch(
        [
@@ -402,6 +404,7 @@ test_authenticator_move(PathPrefix) ->
        ],
        PathPrefix ++ [?CONF_NS]),
 
+    %% test before
     {ok, 204, _} = request(
                      post,
                      uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]),
@@ -413,6 +416,20 @@ test_authenticator_move(PathPrefix) ->
         #{<<"mechanism">> := <<"jwt">>},
         #{<<"mechanism">> := <<"password_based">>, <<"backend">> := <<"built_in_database">>}
        ],
+       PathPrefix ++ [?CONF_NS]),
+
+    %% test after
+    {ok, 204, _} = request(
+                     post,
+                     uri(PathPrefix ++ [?CONF_NS, "password_based%3Abuilt_in_database", "move"]),
+                     #{position => <<"after:password_based:http">>}),
+
+    ?assertAuthenticatorsMatch(
+       [
+         #{<<"mechanism">> := <<"password_based">>, <<"backend">> := <<"http">>},
+         #{<<"mechanism">> := <<"password_based">>, <<"backend">> := <<"built_in_database">>},
+         #{<<"mechanism">> := <<"jwt">>}
+       ],
        PathPrefix ++ [?CONF_NS]).
 
 test_authenticator_import_users(PathPrefix) ->

apps/emqx_authz/include/emqx_authz.hrl

@@ -34,10 +34,10 @@
 -define(CMD_APPEND, append).
 -define(CMD_MOVE, move).
 
--define(CMD_MOVE_TOP, <<"top">>).
--define(CMD_MOVE_BOTTOM, <<"bottom">>).
--define(CMD_MOVE_BEFORE(Before), {<<"before">>, Before}).
--define(CMD_MOVE_AFTER(After), {<<"after">>, After}).
+-define(CMD_MOVE_FRONT, front).
+-define(CMD_MOVE_REAR, rear).
+-define(CMD_MOVE_BEFORE(Before), {before, Before}).
+-define(CMD_MOVE_AFTER(After), {'after', After}).
 
 -define(CONF_KEY_PATH, [authorization, sources]).
 

apps/emqx_authz/src/emqx_authz.erl

@@ -110,10 +110,10 @@ lookup(Type) ->
     {Source, _Front, _Rear} = take(Type),
     Source.
 
-move(Type, #{<<"before">> := Before}) ->
+move(Type, {before, Before}) ->
     emqx_authz_utils:update_config(
       ?CONF_KEY_PATH, {?CMD_MOVE, type(Type), ?CMD_MOVE_BEFORE(type(Before))});
-move(Type, #{<<"after">> := After}) ->
+move(Type, {'after', After}) ->
     emqx_authz_utils:update_config(
       ?CONF_KEY_PATH, {?CMD_MOVE, type(Type), ?CMD_MOVE_AFTER(type(After))});
 move(Type, Position) ->
@@ -127,10 +127,10 @@ update({?CMD_DELETE, Type}, Sources) ->
 update(Cmd, Sources) ->
     emqx_authz_utils:update_config(?CONF_KEY_PATH, {Cmd, Sources}).
 
-do_update({?CMD_MOVE, Type, ?CMD_MOVE_TOP}, Conf) when is_list(Conf) ->
+do_update({?CMD_MOVE, Type, ?CMD_MOVE_FRONT}, Conf) when is_list(Conf) ->
     {Source, Front, Rear} = take(Type, Conf),
     [Source | Front] ++ Rear;
-do_update({?CMD_MOVE, Type, ?CMD_MOVE_BOTTOM}, Conf) when is_list(Conf) ->
+do_update({?CMD_MOVE, Type, ?CMD_MOVE_REAR}, Conf) when is_list(Conf) ->
     {Source, Front, Rear} = take(Type, Conf),
     Front ++ Rear ++ [Source];
 do_update({?CMD_MOVE, Type, ?CMD_MOVE_BEFORE(Before)}, Conf) when is_list(Conf) ->
@@ -334,7 +334,7 @@ take(Type, Sources) ->
     {Front, Rear} =  lists:splitwith(fun(T) -> type(T) =/= type(Type) end, Sources),
     case Rear =:= [] of
         true ->
-            error({authz_source_of_type_not_found, Type});
+            error({not_found_source, Type});
         _ ->
             {hd(Rear), Front, tl(Rear)}
     end.

apps/emqx_authz/src/emqx_authz_api_schema.erl

@@ -74,11 +74,10 @@ fields(file) ->
                  , example => <<"acl.conf">>}}];
 fields(position) ->
     [ { position
-      , mk( hoconsc:union([binary(), map()])
+      , mk( string()
           , #{ desc => <<"Where to place the source">>
              , required => true
-             , in => body
-             , example => #{<<"before">> => <<"file">>}})}].
+             , in => body})}].
 
 %%------------------------------------------------------------------------------
 %% http type funcs

apps/emqx_authz/src/emqx_authz_api_sources.erl

@@ -262,14 +262,25 @@ move_source(Method, #{bindings := #{type := Type} = Bindings } = Req)
   when is_atom(Type) ->
     move_source(Method, Req#{bindings => Bindings#{type => atom_to_binary(Type, utf8)}});
 move_source(post, #{bindings := #{type := Type}, body := #{<<"position">> := Position}}) ->
-    case emqx_authz:move(Type, Position) of
-        {ok, _} -> {204};
-        {error, not_found_source} ->
-            {404, #{code => <<"NOT_FOUND">>,
-                    message => <<"source ", Type/binary, " not found">>}};
-        {error, {emqx_conf_schema, _}} ->
-            {400, #{code => <<"BAD_REQUEST">>,
-                    message => <<"BAD_SCHEMA">>}};
+    case parse_position(Position)  of
+        {ok, NPosition} ->
+            try emqx_authz:move(Type, NPosition) of
+                {ok, _} -> {204};
+                {error, {not_found_source, _Type}} ->
+                    {404, #{code => <<"NOT_FOUND">>,
+                            message => <<"source ", Type/binary, " not found">>}};
+                {error, {emqx_conf_schema, _}} ->
+                    {400, #{code => <<"BAD_REQUEST">>,
+                            message => <<"BAD_SCHEMA">>}};
+                {error, Reason} ->
+                    {400, #{code => <<"BAD_REQUEST">>,
+                            message => bin(Reason)}}
+            catch
+                error : {unknown_authz_source_type, Unknown} ->
+                    NUnknown = bin(Unknown),
+                    {400, #{code => <<"BAD_REQUEST">>,
+                            message => <<"Unknown authz Source Type: ", NUnknown/binary>>}}
+            end;
         {error, Reason} ->
             {400, #{code => <<"BAD_REQUEST">>,
                     message => bin(Reason)}}
@@ -457,8 +468,6 @@ do_write_file(Filename, Bytes) ->
            error(Reason)
     end.
 
-bin(Term) -> erlang:iolist_to_binary(io_lib:format("~p", [Term])).
-
 acl_conf_file() ->
     emqx_authz:acl_conf_file().
 
@@ -468,8 +477,37 @@ parameters_field() ->
       }
     ].
 
+parse_position(<<"front">>) ->
+    {ok, ?CMD_MOVE_FRONT};
+parse_position(<<"rear">>) ->
+    {ok, ?CMD_MOVE_REAR};
+parse_position(<<"before:", Before/binary>>) ->
+    {ok, ?CMD_MOVE_BEFORE(Before)};
+parse_position(<<"after:", After/binary>>) ->
+    {ok, ?CMD_MOVE_AFTER(After)};
+parse_position(<<"before:">>) ->
+    {error, {invalid_parameter, position}};
+parse_position(<<"after:">>) ->
+    {error, {invalid_parameter, position}};
+parse_position(_) ->
+    {error, {invalid_parameter, position}}.
+
 position_example() ->
-    #{<<"position">> => #{<<"before">> => <<"file">>}}.
+    #{ front =>
+          #{ summary => <<"front example">>
+           , value => #{<<"position">> => <<"front">>}}
+     , rear =>
+          #{ summary => <<"rear example">>
+           , value => #{<<"position">> => <<"rear">>}}
+     , relative_before =>
+          #{ summary => <<"relative example">>
+           , value => #{<<"position">> => <<"before:file">>}}
+     , relative_after =>
+          #{ summary => <<"relative example">>
+           , value => #{<<"position">> => <<"after:file">>}}
+     }.
 
 authz_sources_types(Type) ->
     emqx_authz_api_schema:authz_sources_types(Type).
+
+bin(Term) -> erlang:iolist_to_binary(io_lib:format("~p", [Term])).

apps/emqx_authz/test/emqx_authz_SUITE.erl

@@ -186,7 +186,7 @@ t_move_source(_) ->
                  , #{type := file}
                  ], emqx_authz:lookup()),
 
-    {ok, _} = emqx_authz:move(postgresql, <<"top">>),
+    {ok, _} = emqx_authz:move(postgresql, ?CMD_MOVE_FRONT),
     ?assertMatch([ #{type := postgresql}
                  , #{type := http}
                  , #{type := mongodb}
@@ -195,7 +195,7 @@ t_move_source(_) ->
                  , #{type := file}
                  ], emqx_authz:lookup()),
 
-    {ok, _} = emqx_authz:move(http, <<"bottom">>),
+    {ok, _} = emqx_authz:move(http, ?CMD_MOVE_REAR),
     ?assertMatch([ #{type := postgresql}
                  , #{type := mongodb}
                  , #{type := mysql}
@@ -204,7 +204,7 @@ t_move_source(_) ->
                  , #{type := http}
                  ], emqx_authz:lookup()),
 
-    {ok, _} = emqx_authz:move(mysql, #{<<"before">> => postgresql}),
+    {ok, _} = emqx_authz:move(mysql, ?CMD_MOVE_BEFORE(postgresql)),
     ?assertMatch([ #{type := mysql}
                  , #{type := postgresql}
                  , #{type := mongodb}
@@ -213,7 +213,7 @@ t_move_source(_) ->
                  , #{type := http}
                  ], emqx_authz:lookup()),
 
-    {ok, _} = emqx_authz:move(mongodb, #{<<"after">> => http}),
+    {ok, _} = emqx_authz:move(mongodb, ?CMD_MOVE_AFTER(http)),
     ?assertMatch([ #{type := mysql}
                  , #{type := postgresql}
                  , #{type := redis}

apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl

@@ -318,7 +318,7 @@ t_move_source(_) ->
                  ], emqx_authz:lookup()),
 
     {ok, 204, _} = request(post, uri(["authorization", "sources", "postgresql", "move"]),
-                           #{<<"position">> => <<"top">>}),
+                           #{<<"position">> => <<"front">>}),
     ?assertMatch([ #{type := postgresql}
                  , #{type := http}
                  , #{type := mongodb}
@@ -327,7 +327,7 @@ t_move_source(_) ->
                  ], emqx_authz:lookup()),
 
     {ok, 204, _} = request(post, uri(["authorization", "sources", "http", "move"]),
-                           #{<<"position">> => <<"bottom">>}),
+                           #{<<"position">> => <<"rear">>}),
     ?assertMatch([ #{type := postgresql}
                  , #{type := mongodb}
                  , #{type := mysql}
@@ -336,7 +336,7 @@ t_move_source(_) ->
                  ], emqx_authz:lookup()),
 
     {ok, 204, _} = request(post, uri(["authorization", "sources", "mysql", "move"]),
-                           #{<<"position">> => #{<<"before">> => <<"postgresql">>}}),
+                           #{<<"position">> => <<"before:postgresql">>}),
     ?assertMatch([ #{type := mysql}
                  , #{type := postgresql}
                  , #{type := mongodb}
@@ -345,7 +345,7 @@ t_move_source(_) ->
                  ], emqx_authz:lookup()),
 
     {ok, 204, _} = request(post, uri(["authorization", "sources", "mongodb", "move"]),
-                           #{<<"position">> => #{<<"after">> => <<"http">>}}),
+                           #{<<"position">> => <<"after:http">>}),
     ?assertMatch([ #{type := mysql}
                  , #{type := postgresql}
                  , #{type := redis}

apps/emqx_exhook/include/emqx_exhook.hrl

@@ -45,3 +45,8 @@
       ]).
 
 -endif.
+
+-define(CMD_MOVE_FRONT, front).
+-define(CMD_MOVE_REAR, rear).
+-define(CMD_MOVE_BEFORE(Before), {before, Before}).
+-define(CMD_MOVE_AFTER(After), {'after', After}).

apps/emqx_exhook/src/emqx_exhook_api.erl

@@ -18,6 +18,7 @@
 
 -behaviour(minirest_api).
 
+-include("emqx_exhook.hrl").
 -include_lib("typerefl/include/types.hrl").
 -include_lib("emqx/include/logger.hrl").
 
@@ -104,10 +105,14 @@ schema("/exhooks/:name/hooks") ->
 schema("/exhooks/:name/move") ->
     #{'operationId' => move,
       post => #{tags => ?TAGS,
-                description => <<"Move the server">>,
+                description =>
+                    <<"Move the server.\n",
+                      "NOTE: The position should be \"front|rear|before:{name}|after:{name}\"\n">>,
                 parameters => params_server_name_in_path(),
-                'requestBody' => mk(ref(move_req), #{}),
-                responses => #{200 => <<>>,
+                'requestBody' => emqx_dashboard_swagger:schema_with_examples(
+                                   ref(move_req),
+                                   position_example()),
+                responses => #{204 => <<"No Content">>,
                                400 => error_codes([?BAD_REQUEST], <<"Bad Request">>),
                                500 => error_codes([?BAD_RPC], <<"Bad RPC">>)
                               }
@@ -115,12 +120,8 @@ schema("/exhooks/:name/move") ->
      }.
 
 fields(move_req) ->
-    [ {position, mk(enum([top, bottom, before, 'after']), #{})}
-    , {related, mk(string(), #{desc => <<"Relative position of movement">>,
-                               default => <<>>,
-                               example => <<>>
-                              })}
-    ];
+    [{position, mk(string(), #{ desc => <<"The target position to be moved.">>
+                              , example => <<"front">>})}];
 
 fields(detail_server_info) ->
     [ {metrics, mk(ref(metrics), #{})}
@@ -210,7 +211,8 @@ action_with_name(put, #{bindings := #{name := Name}, body := Body}) ->
                    }};
         {ok, {error, Reason}} ->
             {400, #{code => <<"BAD_REQUEST">>,
-                    message => unicode:characters_to_binary(io_lib:format("Error Reason:~p~n", [Reason]))
+                    message => unicode:characters_to_binary(
+                                 io_lib:format("Error Reason:~p~n", [Reason]))
                    }};
         {ok, _} ->
             {200};
@@ -231,20 +233,26 @@ action_with_name(delete, #{bindings := #{name := Name}}) ->
                    }}
     end.
 
-move(post, #{bindings := #{name := Name}, body := Body}) ->
-    #{<<"position">> := PositionT, <<"related">> := Related} = Body,
-    Position = erlang:binary_to_atom(PositionT),
-    case emqx_exhook_mgr:update_config([exhook, servers],
-                                       {move, Name, Position, Related}) of
-        {ok, ok} ->
-            {200};
-        {ok, not_found} ->
+move(post, #{bindings := #{name := Name}, body := #{<<"position">> := RawPosition}}) ->
+    case parse_position(RawPosition) of
+        {ok, Position} ->
+            case emqx_exhook_mgr:update_config([exhook, servers],
+                                               {move, Name, Position}) of
+                {ok, ok} ->
+                    {204};
+                {ok, not_found} ->
+                    %% TODO: unify status code
+                    {400, #{code => <<"BAD_REQUEST">>,
+                            message => <<"Server not found">>
+                           }};
+                {error, Error} ->
+                    {500, #{code => <<"BAD_RPC">>,
+                            message => Error
+                           }}
+            end;
+        {error, invalid_position} ->
             {400, #{code => <<"BAD_REQUEST">>,
-                    message => <<"Server not found">>
-                   }};
-        {error, Error} ->
-            {500, #{code => <<"BAD_RPC">>,
-                    message => Error
+                    message => <<"Invalid Position">>
                    }}
     end.
 
@@ -297,11 +305,12 @@ fill_cluster_server_info([{Node, {error, _}} | T], StatusL, MetricsL, ServerName
 
 fill_cluster_server_info([{Node, Result} | T], StatusL, MetricsL, ServerName, Default) ->
     #{status := Status, metrics := Metrics} = Result,
-    fill_cluster_server_info(T,
-                             [#{node => Node, status => maps:get(ServerName, Status, error)} | StatusL],
-                             [#{node => Node, metrics => maps:get(ServerName, Metrics, Default)} | MetricsL],
-                             ServerName,
-                             Default);
+    fill_cluster_server_info(
+      T,
+      [#{node => Node, status => maps:get(ServerName, Status, error)} | StatusL],
+      [#{node => Node, metrics => maps:get(ServerName, Metrics, Default)} | MetricsL],
+      ServerName,
+      Default);
 
 fill_cluster_server_info([], StatusL, MetricsL, ServerName, _) ->
     Metrics = emqx_exhook_metrics:metrics_aggregate_by_key(metrics, MetricsL),
@@ -350,7 +359,9 @@ get_nodes_server_hooks_info(Name) ->
     case emqx_exhook_mgr:hooks(Name) of
         [] -> [];
         Hooks ->
-            AllInfos = call_cluster(fun(Nodes) -> emqx_exhook_proto_v1:server_hooks_metrics(Nodes, Name) end),
+            AllInfos = call_cluster(fun(Nodes) ->
+                                            emqx_exhook_proto_v1:server_hooks_metrics(Nodes, Name)
+                                    end),
             Default = emqx_exhook_metrics:new_metrics_info(),
             get_nodes_server_hooks_info(Hooks, AllInfos, Default, [])
     end.
@@ -385,3 +396,38 @@ call_cluster(Fun) ->
     Nodes = mria_mnesia:running_nodes(),
     Ret = Fun(Nodes),
     lists:zip(Nodes, lists:map(fun emqx_rpc:unwrap_erpc/1, Ret)).
+
+
+%%--------------------------------------------------------------------
+%% Internal Funcs
+%%--------------------------------------------------------------------
+
+position_example() ->
+    #{ front =>
+           #{ summary => <<"absolute position 'front'">>
+            , value => #{<<"position">> => <<"front">>}}
+     , rear =>
+           #{ summary => <<"absolute position 'rear'">>
+            , value => #{<<"position">> => <<"rear">>}}
+     , related_before =>
+           #{ summary => <<"relative position 'before'">>
+            , value => #{<<"position">> => <<"before:default">>}}
+     , related_after =>
+           #{ summary => <<"relative position 'after'">>
+            , value => #{<<"position">> => <<"after:default">>}}
+     }.
+
+parse_position(<<"front">>) ->
+    {ok, ?CMD_MOVE_FRONT};
+parse_position(<<"rear">>) ->
+    {ok, ?CMD_MOVE_REAR};
+parse_position(<<"before:">>) ->
+    {error, invalid_position};
+parse_position(<<"after:">>) ->
+    {error, invalid_position};
+parse_position(<<"before:", Related/binary>>) ->
+    {ok, ?CMD_MOVE_BEFORE(Related)};
+parse_position(<<"after:", Related/binary>>) ->
+    {ok, ?CMD_MOVE_AFTER(Related)};
+parse_position(_) ->
+    {error, invalid_position}.

apps/emqx_exhook/src/emqx_exhook_mgr.erl

@@ -74,10 +74,10 @@
 -type server() :: server_options().
 -type server_options() :: map().
 
--type move_direct() :: top
-                     | bottom
-                     | before
-                     | 'after'.
+-type position() :: front
+                  | rear
+                  | {before, binary()}
+                  | {'after', binary()}.
 
 -type orders() :: #{server_name() => integer()}.
 
@@ -157,8 +157,8 @@ pre_config_update(_, {delete, ToDelete}, OldConf) ->
     {ok, lists:dropwhile(fun(#{<<"name">> := Name}) -> Name =:= ToDelete end,
                          OldConf)};
 
-pre_config_update(_, {move, Name, Position, Relate}, OldConf) ->
-    case do_move(Name, Position, Relate, OldConf) of
+pre_config_update(_, {move, Name, Position}, OldConf) ->
+    case do_move(Name, Position, OldConf) of
         not_found -> {error, not_found};
         NewConf -> {ok, NewConf}
     end;
@@ -226,7 +226,7 @@ handle_call(list, _From, State = #{running := Running,
 
     {reply, OrderServers, State};
 
-handle_call({update_config, {move, _Name, _Direct, _Related}, NewConfL},
+handle_call({update_config, {move, _Name, _Position}, NewConfL},
             _From,
             State) ->
     Orders = reorder(NewConfL),
@@ -461,39 +461,39 @@ clean_reload_timer(Name, State = #{trefs := TRefs}) ->
             State#{trefs := NTRefs}
     end.
 
--spec do_move(binary(), move_direct(), binary(), list(server_options())) ->
+-spec do_move(binary(), position(), list(server_options())) ->
           not_found | list(server_options()).
-do_move(Name, Direct, ToName, ConfL) ->
-    move(ConfL, Name, Direct, ToName, []).
+do_move(Name, Position, ConfL) ->
+    move(ConfL, Name, Position, []).
 
-move([#{<<"name">> := Name} = Server | T], Name, Direct, ToName, HeadL) ->
-    move_to(Direct, ToName, Server, lists:reverse(HeadL) ++ T);
+move([#{<<"name">> := Name} = Server | T], Name, Position, HeadL) ->
+    move_to(Position, Server, lists:reverse(HeadL) ++ T);
 
-move([Server | T], Name, Direct, ToName, HeadL) ->
-    move(T, Name, Direct, ToName, [Server | HeadL]);
+move([Server | T], Name, Position, HeadL) ->
+    move(T, Name, Position, [Server | HeadL]);
 
-move([], _Name, _Direct, _ToName, _HeadL) ->
+move([], _Name, _Position, _HeadL) ->
     not_found.
 
-move_to(top, _, Server, ServerL) ->
+move_to(?CMD_MOVE_FRONT, Server, ServerL) ->
     [Server | ServerL];
 
-move_to(bottom, _, Server, ServerL) ->
+move_to(?CMD_MOVE_REAR, Server, ServerL) ->
     ServerL ++ [Server];
 
-move_to(Direct, ToName, Server, ServerL) ->
-    move_to(ServerL, Direct, ToName, Server, []).
+move_to(Position, Server, ServerL) ->
+    move_to(ServerL, Position, Server, []).
 
-move_to([#{<<"name">> := Name} | _] = T, before, Name, Server, HeadL) ->
+move_to([#{<<"name">> := Name} | _] = T, ?CMD_MOVE_BEFORE(Name), Server, HeadL) ->
     lists:reverse(HeadL) ++ [Server | T];
 
-move_to([#{<<"name">> := Name} = H | T], 'after', Name, Server, HeadL) ->
+move_to([#{<<"name">> := Name} = H | T], ?CMD_MOVE_AFTER(Name), Server, HeadL) ->
     lists:reverse(HeadL) ++ [H, Server | T];
 
-move_to([H | T], Direct, Name, Server, HeadL) ->
-    move_to(T, Direct, Name, Server, [H | HeadL]);
+move_to([H | T], Position, Server, HeadL) ->
+    move_to(T, Position, Server, [H | HeadL]);
 
-move_to([], _Direct, _Name, _Server, _HeadL) ->
+move_to([], _Position, _Server, _HeadL) ->
     not_found.
 
 -spec reorder(list(server_options())) -> orders().

apps/emqx_exhook/test/emqx_exhook_api_SUITE.erl

@@ -37,7 +37,7 @@ exhook {
 ">>).
 
 all() ->
-    [ t_list, t_get, t_add, t_move_top, t_move_bottom
+    [ t_list, t_get, t_add, t_move_front, t_move_rear
     , t_move_before, t_move_after, t_delete, t_hooks, t_update
     ].
 
@@ -133,18 +133,18 @@ t_add(Cfg) ->
 
     ?assertMatch([<<"default">>, <<"test1">>], emqx_exhook_mgr:running()).
 
-t_move_top(_) ->
+t_move_front(_) ->
     Result = request_api(post, api_path(["exhooks", "default", "move"]), "",
                          auth_header_(),
-                         #{position => top, related => <<>>}),
+                         #{position => <<"front">>}),
 
     ?assertMatch({ok, <<>>}, Result),
     ?assertMatch([<<"default">>, <<"test1">>], emqx_exhook_mgr:running()).
 
-t_move_bottom(_) ->
+t_move_rear(_) ->
     Result = request_api(post, api_path(["exhooks", "default", "move"]), "",
                          auth_header_(),
-                         #{position => bottom, related => <<>>}),
+                         #{position => <<"rear">>}),
 
     ?assertMatch({ok, <<>>}, Result),
     ?assertMatch([<<"test1">>, <<"default">>], emqx_exhook_mgr:running()).
@@ -152,7 +152,7 @@ t_move_bottom(_) ->
 t_move_before(_) ->
     Result = request_api(post, api_path(["exhooks", "default", "move"]), "",
                          auth_header_(),
-                         #{position => before, related => <<"test1">>}),
+                         #{position => <<"before:test1">>}),
 
     ?assertMatch({ok, <<>>}, Result),
     ?assertMatch([<<"default">>, <<"test1">>], emqx_exhook_mgr:running()).
@@ -160,7 +160,7 @@ t_move_before(_) ->
 t_move_after(_) ->
     Result = request_api(post, api_path(["exhooks", "default", "move"]), "",
                          auth_header_(),
-                         #{position => 'after', related => <<"test1">>}),
+                         #{position => <<"after:test1">>}),
 
     ?assertMatch({ok, <<>>}, Result),
     ?assertMatch([<<"test1">>, <<"default">>], emqx_exhook_mgr:running()).

apps/emqx_management/src/emqx_mgmt_api_plugins.erl

@@ -199,11 +199,11 @@ fields(builder) ->
         {website, hoconsc:mk(string(), #{example => "www.emqx.com"})}
     ];
 fields(position) ->
-    [{position, hoconsc:mk(hoconsc:union([top, bottom, binary()]),
+    [{position, hoconsc:mk(hoconsc:union([front, rear, binary()]),
         #{
             desc => """
              Enable auto-boot at position in the boot list, where Position could be
-             'top', 'bottom', or 'before:other-vsn', 'after:other-vsn'
+             'front', 'rear', or 'before:other-vsn', 'after:other-vsn'
              to specify a relative position.
             """,
             required => false
@@ -221,13 +221,13 @@ fields(running_status) ->
 move_request_body() ->
     emqx_dashboard_swagger:schema_with_examples(hoconsc:ref(?MODULE, position),
         #{
-            move_to_top => #{
-                summary => <<"move plugin on the top">>,
-                value => #{position => <<"top">>}
+            move_to_front => #{
+                summary => <<"move plugin on the front">>,
+                value => #{position => <<"front">>}
             },
-            move_to_bottom => #{
-                summary => <<"move plugin on the bottom">>,
-                value => #{position => <<"bottom">>}
+            move_to_rear => #{
+                summary => <<"move plugin on the rear">>,
+                value => #{position => <<"rear">>}
             },
             move_to_before => #{
                 summary => <<"move plugin before other plugins">>,
@@ -350,8 +350,8 @@ return(_, {error, #{error := "bad_info_file", return := {enoent, _}, path := Pat
 return(_, {error, Reason}) ->
     {400, #{code => 'PARAM_ERROR', message => iolist_to_binary(io_lib:format("~p", [Reason]))}}.
 
-parse_position(#{<<"position">> := <<"top">>}, _) -> front;
-parse_position(#{<<"position">> := <<"bottom">>}, _) -> rear;
+parse_position(#{<<"position">> := <<"front">>}, _) -> front;
+parse_position(#{<<"position">> := <<"rear">>}, _) -> rear;
 parse_position(#{<<"position">> := <<"before:", Name/binary>>}, Name) ->
     {error, <<"Can't before:self">>};
 parse_position(#{<<"position">> := <<"after:", Name/binary>>}, Name) ->