Jelajahi Sumber

chore: rm `move` API

Only `reorder` API will be supported, after discussions with product and frontend teams.
Thales Macedo Garitezi 1 tahun lalu
induk
melakukan
2eac54bf54

+ 0 - 39
apps/emqx_message_validation/src/emqx_message_validation.erl

@@ -16,7 +16,6 @@
     unload/0,
 
     list/0,
-    move/2,
     reorder/1,
     lookup/1,
     insert/1,
@@ -53,7 +52,6 @@
 
 -type validation_name() :: binary().
 -type validation() :: _TODO.
--type position() :: front | rear | {'after', validation_name()} | {before, validation_name()}.
 
 %%------------------------------------------------------------------------------
 %% API
@@ -79,15 +77,6 @@ unload() ->
 list() ->
     emqx:get_config(?VALIDATIONS_CONF_PATH, []).
 
--spec move(validation_name(), position()) ->
-    {ok, _} | {error, _}.
-move(Name, Position) ->
-    emqx:update_config(
-        ?VALIDATIONS_CONF_PATH,
-        {move, Name, Position},
-        #{override_to => cluster}
-    ).
-
 -spec reorder([validation_name()]) ->
     {ok, _} | {error, _}.
 reorder(Order) ->
@@ -174,8 +163,6 @@ pre_config_update(?VALIDATIONS_CONF_PATH, {update, Validation}, OldValidations)
     replace(OldValidations, Validation);
 pre_config_update(?VALIDATIONS_CONF_PATH, {delete, Validation}, OldValidations) ->
     delete(OldValidations, Validation);
-pre_config_update(?VALIDATIONS_CONF_PATH, {move, Name, Position}, OldValidations) ->
-    move(OldValidations, Name, Position);
 pre_config_update(?VALIDATIONS_CONF_PATH, {reorder, Order}, OldValidations) ->
     reorder(OldValidations, Order).
 
@@ -192,9 +179,6 @@ post_config_update(?VALIDATIONS_CONF_PATH, {delete, Name}, _New, Old, _AppEnvs)
     {_Pos, Validation} = fetch_with_index(Old, Name),
     ok = emqx_message_validation_registry:delete(Validation),
     ok;
-post_config_update(?VALIDATIONS_CONF_PATH, {move, _Name, _Position}, New, _Old, _AppEnvs) ->
-    ok = emqx_message_validation_registry:reindex_positions(New),
-    ok;
 post_config_update(?VALIDATIONS_CONF_PATH, {reorder, _Order}, New, _Old, _AppEnvs) ->
     ok = emqx_message_validation_registry:reindex_positions(New),
     ok.
@@ -348,21 +332,6 @@ delete(OldValidations, Name) ->
             {error, not_found}
     end.
 
-move(OldValidations, Name, front) ->
-    {Validation, Front, Rear} = take(Name, OldValidations),
-    {ok, [Validation | Front ++ Rear]};
-move(OldValidations, Name, rear) ->
-    {Validation, Front, Rear} = take(Name, OldValidations),
-    {ok, Front ++ Rear ++ [Validation]};
-move(OldValidations, Name, {'after', OtherName}) ->
-    {Validation, Front1, Rear1} = take(Name, OldValidations),
-    {OtherValidation, Front2, Rear2} = take(OtherName, Front1 ++ Rear1),
-    {ok, Front2 ++ [OtherValidation, Validation] ++ Rear2};
-move(OldValidations, Name, {before, OtherName}) ->
-    {Validation, Front1, Rear1} = take(Name, OldValidations),
-    {OtherValidation, Front2, Rear2} = take(OtherName, Front1 ++ Rear1),
-    {ok, Front2 ++ [Validation, OtherValidation] ++ Rear2}.
-
 reorder(Validations, Order) ->
     Context = #{
         not_found => sets:new([{version, 2}]),
@@ -416,14 +385,6 @@ fetch_with_index([{_, _} | Rest], Name) ->
 fetch_with_index(Validations, Name) ->
     fetch_with_index(lists:enumerate(Validations), Name).
 
-take(Name, Validations) ->
-    case safe_take(Name, Validations) of
-        error ->
-            throw({validation_not_found, Name});
-        {ok, {Found, Front, Rear}} ->
-            {Found, Front, Rear}
-    end.
-
 safe_take(Name, Validations) ->
     case lists:splitwith(fun(#{<<"name">> := N}) -> N =/= Name end, Validations) of
         {_Front, []} ->

+ 2 - 107
apps/emqx_message_validation/src/emqx_message_validation_http_api.erl

@@ -23,8 +23,7 @@
 -export([
     '/message_validations'/2,
     '/message_validations/reorder'/2,
-    '/message_validations/validation/:name'/2,
-    '/message_validations/validation/:name/move'/2
+    '/message_validations/validation/:name'/2
 ]).
 
 %%-------------------------------------------------------------------------------------------------
@@ -46,8 +45,7 @@ paths() ->
     [
         "/message_validations",
         "/message_validations/reorder",
-        "/message_validations/validation/:name",
-        "/message_validations/validation/:name/move"
+        "/message_validations/validation/:name"
     ].
 
 schema("/message_validations") ->
@@ -172,27 +170,6 @@ schema("/message_validations/validation/:name") ->
                     404 => error_schema('NOT_FOUND', "Validation not found")
                 }
         }
-    };
-schema("/message_validations/validation/:name/move") ->
-    #{
-        'operationId' => '/message_validations/validation/:name/move',
-        post => #{
-            tags => ?TAGS,
-            summary => <<"Change the order of a validation">>,
-            description => ?DESC("move_validation"),
-            parameters => [param_path_name()],
-            'requestBody' =>
-                emqx_dashboard_swagger:schema_with_examples(
-                    hoconsc:union(fun position_union_member_selector/1),
-                    example_position()
-                ),
-            responses =>
-                #{
-                    204 => <<"No Content">>,
-                    400 => error_schema('BAD_REQUEST', <<"Bad request">>),
-                    404 => error_schema('NOT_FOUND', "Validation not found")
-                }
-        }
     }.
 
 param_path_name() ->
@@ -281,15 +258,6 @@ fields(reorder) ->
         not_found()
     ).
 
-'/message_validations/validation/:name/move'(post, #{bindings := #{name := Name}, body := Body}) ->
-    with_validation(
-        Name,
-        fun() ->
-            do_move(Name, parse_position(Body))
-        end,
-        not_found(Name)
-    ).
-
 '/message_validations/reorder'(post, #{body := #{<<"order">> := Order}}) ->
     do_reorder(Order).
 
@@ -329,10 +297,6 @@ example_return_lookup() ->
     %% TODO
     #{}.
 
-example_position() ->
-    %% TODO
-    #{}.
-
 error_schema(Code, Message) ->
     error_schema(Code, Message, _ExtraFields = []).
 
@@ -343,68 +307,6 @@ error_schema(Codes, Message, ExtraFields) when is_list(Message) ->
 error_schema(Codes, Message, ExtraFields) when is_list(Codes) andalso is_binary(Message) ->
     ExtraFields ++ emqx_dashboard_swagger:error_codes(Codes, Message).
 
-position_union_member_selector(all_union_members) ->
-    position_refs();
-position_union_member_selector({value, V}) ->
-    position_refs(V).
-
-position_refs() ->
-    [].
-
-position_types() ->
-    [
-        front,
-        rear,
-        'after',
-        before
-    ].
-
-position_refs(#{<<"position">> := <<"front">>}) ->
-    [ref(front)];
-position_refs(#{<<"position">> := <<"rear">>}) ->
-    [ref(rear)];
-position_refs(#{<<"position">> := <<"after">>}) ->
-    [ref('after')];
-position_refs(#{<<"position">> := <<"before">>}) ->
-    [ref(before)];
-position_refs(_) ->
-    Expected = lists:join(" | ", [atom_to_list(T) || T <- position_types()]),
-    throw(#{
-        field_name => position,
-        expected => iolist_to_binary(Expected)
-    }).
-
-%% Schema is already checked, so we don't need to do further validation.
-parse_position(#{<<"position">> := <<"front">>}) ->
-    front;
-parse_position(#{<<"position">> := <<"rear">>}) ->
-    rear;
-parse_position(#{<<"position">> := <<"after">>, <<"validation">> := OtherValidationName}) ->
-    {'after', OtherValidationName};
-parse_position(#{<<"position">> := <<"before">>, <<"validation">> := OtherValidationName}) ->
-    {before, OtherValidationName}.
-
-do_move(ValidationName, {_, OtherValidationName} = Position) ->
-    with_validation(
-        OtherValidationName,
-        fun() ->
-            case emqx_message_validation:move(ValidationName, Position) of
-                {ok, _} ->
-                    ?NO_CONTENT;
-                {error, Error} ->
-                    ?BAD_REQUEST(Error)
-            end
-        end,
-        bad_request_not_found(OtherValidationName)
-    );
-do_move(ValidationName, Position) ->
-    case emqx_message_validation:move(ValidationName, Position) of
-        {ok, _} ->
-            ?NO_CONTENT;
-        {error, Error} ->
-            ?BAD_REQUEST(Error)
-    end.
-
 do_reorder(Order) ->
     case emqx_message_validation:reorder(Order) of
         {ok, _} ->
@@ -443,10 +345,3 @@ return(Response) ->
 
 not_found() ->
     return(?NOT_FOUND(<<"Validation not found">>)).
-
-not_found(Name) ->
-    return(?NOT_FOUND(<<"Validation not found: ", Name/binary>>)).
-
-%% After we found the base validation, but not the other one being referenced in a move.
-bad_request_not_found(Name) ->
-    return(?BAD_REQUEST(<<"Validation not found: ", Name/binary>>)).

+ 0 - 104
apps/emqx_message_validation/test/emqx_message_validation_http_api_SUITE.erl

@@ -174,12 +174,6 @@ delete(Name) ->
     ct:pal("delete result:\n  ~p", [Res]),
     simplify_result(Res).
 
-move(Name, Pos) ->
-    Path = emqx_mgmt_api_test_util:api_path([api_root(), "validation", Name, "move"]),
-    Res = request(post, Path, Pos),
-    ct:pal("move result:\n  ~p", [Res]),
-    simplify_result(Res).
-
 reorder(Order) ->
     Path = emqx_mgmt_api_test_util:api_path([api_root(), "reorder"]),
     Params = #{<<"order">> => Order},
@@ -417,104 +411,6 @@ t_crud(_Config) ->
 
     ok.
 
-%% test the "move" API
-t_move(_Config) ->
-    lists:foreach(
-        fun(Pos) ->
-            ?assertMatch({404, _}, move(<<"nonexistent_validation">>, Pos))
-        end,
-        [
-            #{<<"position">> => <<"front">>},
-            #{<<"position">> => <<"rear">>},
-            #{<<"position">> => <<"after">>, <<"validation">> => <<"also_non_existent">>},
-            #{<<"position">> => <<"before">>, <<"validation">> => <<"also_non_existent">>}
-        ]
-    ),
-
-    Topic = <<"t">>,
-
-    Name1 = <<"foo">>,
-    Validation1 = validation(Name1, [sql_check()], #{<<"topics">> => Topic}),
-    {201, _} = insert(Validation1),
-
-    %% bogus positions
-    lists:foreach(
-        fun(Pos) ->
-            ?assertMatch(
-                {400, #{<<"message">> := #{<<"kind">> := <<"validation_error">>}}},
-                move(Name1, Pos)
-            )
-        end,
-        [
-            #{<<"position">> => <<"foo">>},
-            #{<<"position">> => <<"bar">>, <<"validation">> => Name1}
-        ]
-    ),
-
-    lists:foreach(
-        fun(Pos) ->
-            ?assertMatch({204, _}, move(Name1, Pos)),
-            ?assertMatch({200, [#{<<"name">> := Name1}]}, list())
-        end,
-        [
-            #{<<"position">> => <<"front">>},
-            #{<<"position">> => <<"rear">>}
-        ]
-    ),
-
-    lists:foreach(
-        fun(Pos) ->
-            ?assertMatch({400, _}, move(Name1, Pos))
-        end,
-        [
-            #{<<"position">> => <<"after">>, <<"validation">> => <<"nonexistent">>},
-            #{<<"position">> => <<"before">>, <<"validation">> => <<"nonexistent">>}
-        ]
-    ),
-
-    Name2 = <<"bar">>,
-    Validation2 = validation(Name2, [sql_check()], #{<<"topics">> => Topic}),
-    {201, _} = insert(Validation2),
-    ?assertMatch({200, [#{<<"name">> := Name1}, #{<<"name">> := Name2}]}, list()),
-    ?assertIndexOrder([Name1, Name2], Topic),
-
-    ?assertMatch({204, _}, move(Name1, #{<<"position">> => <<"rear">>})),
-    ?assertMatch({200, [#{<<"name">> := Name2}, #{<<"name">> := Name1}]}, list()),
-    ?assertIndexOrder([Name2, Name1], Topic),
-
-    ?assertMatch({204, _}, move(Name1, #{<<"position">> => <<"front">>})),
-    ?assertMatch({200, [#{<<"name">> := Name1}, #{<<"name">> := Name2}]}, list()),
-    ?assertIndexOrder([Name1, Name2], Topic),
-
-    Name3 = <<"baz">>,
-    Validation3 = validation(Name3, [sql_check()], #{<<"topics">> => Topic}),
-    {201, _} = insert(Validation3),
-    ?assertMatch(
-        {200, [#{<<"name">> := Name1}, #{<<"name">> := Name2}, #{<<"name">> := Name3}]},
-        list()
-    ),
-    ?assertIndexOrder([Name1, Name2, Name3], Topic),
-
-    ?assertMatch(
-        {204, _}, move(Name3, #{<<"position">> => <<"before">>, <<"validation">> => Name2})
-    ),
-    ?assertMatch(
-        {200, [#{<<"name">> := Name1}, #{<<"name">> := Name3}, #{<<"name">> := Name2}]},
-        list()
-    ),
-    ?assertIndexOrder([Name1, Name3, Name2], Topic),
-
-    ?assertMatch(
-        {204, _}, move(Name1, #{<<"position">> => <<"after">>, <<"validation">> => Name2})
-    ),
-    ?assertMatch(
-        {200, [#{<<"name">> := Name3}, #{<<"name">> := Name2}, #{<<"name">> := Name1}]},
-        list()
-    ),
-    ?assertIndexOrder([Name3, Name2, Name1], Topic),
-
-    ok.
-
 %% test the "reorder" API
 t_reorder(_Config) ->
     %% no validations to reorder

+ 0 - 3
rel/i18n/emqx_message_validation_http_api.hocon

@@ -15,9 +15,6 @@ emqx_message_validation_http_api {
   append_validation.desc:
   """Append a new validation to the list of validations"""
 
-  move_validation.desc:
-  """Change the order of a validation in the list of validations"""
-
   reorder_validations.desc:
   """Reorder of all validations"""