From 2eac54bf54b4054d23ae4b1cb27a230942f8ae29 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Tue, 19 Mar 2024 09:06:41 -0300 Subject: [PATCH] chore: rm `move` API Only `reorder` API will be supported, after discussions with product and frontend teams. --- .../src/emqx_message_validation.erl | 39 ------- .../src/emqx_message_validation_http_api.erl | 109 +----------------- ...emqx_message_validation_http_api_SUITE.erl | 104 ----------------- .../emqx_message_validation_http_api.hocon | 3 - 4 files changed, 2 insertions(+), 253 deletions(-) diff --git a/apps/emqx_message_validation/src/emqx_message_validation.erl b/apps/emqx_message_validation/src/emqx_message_validation.erl index 72861f3c5..86ace1fdf 100644 --- a/apps/emqx_message_validation/src/emqx_message_validation.erl +++ b/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, []} -> diff --git a/apps/emqx_message_validation/src/emqx_message_validation_http_api.erl b/apps/emqx_message_validation/src/emqx_message_validation_http_api.erl index 34ed584fa..b024ca09e 100644 --- a/apps/emqx_message_validation/src/emqx_message_validation_http_api.erl +++ b/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>>)). diff --git a/apps/emqx_message_validation/test/emqx_message_validation_http_api_SUITE.erl b/apps/emqx_message_validation/test/emqx_message_validation_http_api_SUITE.erl index 67910d1aa..d8742ec23 100644 --- a/apps/emqx_message_validation/test/emqx_message_validation_http_api_SUITE.erl +++ b/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 diff --git a/rel/i18n/emqx_message_validation_http_api.hocon b/rel/i18n/emqx_message_validation_http_api.hocon index bb26ae146..6e549fe31 100644 --- a/rel/i18n/emqx_message_validation_http_api.hocon +++ b/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"""