From 1b68fbff05c54689e576170b498ee1da3a7bdf7c Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Mon, 4 Dec 2023 16:44:01 -0300 Subject: [PATCH] fix(api): return list of dependent rule ids when trying to delete bridge/action Fixes https://emqx.atlassian.net/browse/EMQX-11523 --- apps/emqx_bridge/src/emqx_bridge_api.erl | 29 ++++++++++------- apps/emqx_bridge/src/emqx_bridge_v2_api.erl | 31 ++++++++++--------- .../test/emqx_bridge_api_SUITE.erl | 3 +- .../test/emqx_bridge_v2_api_SUITE.erl | 3 +- mix.exs | 2 +- rebar.config | 2 +- 6 files changed, 39 insertions(+), 31 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index b725eb740..2343e3ccf 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -87,12 +87,15 @@ paths() -> "/bridges_probe" ]. -error_schema(Code, Message) when is_atom(Code) -> - error_schema([Code], Message); -error_schema(Codes, Message) when is_list(Message) -> - error_schema(Codes, list_to_binary(Message)); -error_schema(Codes, Message) when is_list(Codes) andalso is_binary(Message) -> - emqx_dashboard_swagger:error_codes(Codes, Message). +error_schema(Code, Message) -> + error_schema(Code, Message, _ExtraFields = []). + +error_schema(Code, Message, ExtraFields) when is_atom(Code) -> + error_schema([Code], Message, ExtraFields); +error_schema(Codes, Message, ExtraFields) when is_list(Message) -> + error_schema(Codes, list_to_binary(Message), ExtraFields); +error_schema(Codes, Message, ExtraFields) when is_list(Codes) andalso is_binary(Message) -> + ExtraFields ++ emqx_dashboard_swagger:error_codes(Codes, Message). get_response_body_schema() -> emqx_dashboard_swagger:schema_with_examples( @@ -340,7 +343,8 @@ schema("/bridges/:id") -> 204 => <<"Bridge deleted">>, 400 => error_schema( 'BAD_REQUEST', - "Cannot delete bridge while active rules are defined for this bridge" + "Cannot delete bridge while active rules are defined for this bridge", + [{rules, mk(array(string()), #{desc => "Dependent Rule IDs"})}] ), 404 => error_schema('NOT_FOUND', "Bridge not found"), 503 => error_schema('SERVICE_UNAVAILABLE', "Service unavailable") @@ -517,11 +521,12 @@ schema("/bridges_probe") -> reason := rules_depending_on_this_bridge, rule_ids := RuleIds }} -> - RulesStr = [[" ", I] || I <- RuleIds], - Msg = bin([ - "Cannot delete bridge while active rules are depending on it:", RulesStr - ]), - ?BAD_REQUEST(Msg); + Msg0 = ?ERROR_MSG( + 'BAD_REQUEST', + bin("Cannot delete bridge while active rules are depending on it") + ), + Msg = Msg0#{rules => RuleIds}, + {400, Msg}; {error, timeout} -> ?SERVICE_UNAVAILABLE(<<"request timeout">>); {error, Reason} -> diff --git a/apps/emqx_bridge/src/emqx_bridge_v2_api.erl b/apps/emqx_bridge/src/emqx_bridge_v2_api.erl index e6fcca50a..4cb371103 100644 --- a/apps/emqx_bridge/src/emqx_bridge_v2_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_v2_api.erl @@ -91,12 +91,15 @@ paths() -> "/action_types" ]. -error_schema(Code, Message) when is_atom(Code) -> - error_schema([Code], Message); -error_schema(Codes, Message) when is_list(Message) -> - error_schema(Codes, list_to_binary(Message)); -error_schema(Codes, Message) when is_list(Codes) andalso is_binary(Message) -> - emqx_dashboard_swagger:error_codes(Codes, Message). +error_schema(Code, Message) -> + error_schema(Code, Message, _ExtraFields = []). + +error_schema(Code, Message, ExtraFields) when is_atom(Code) -> + error_schema([Code], Message, ExtraFields); +error_schema(Codes, Message, ExtraFields) when is_list(Message) -> + error_schema(Codes, list_to_binary(Message), ExtraFields); +error_schema(Codes, Message, ExtraFields) when is_list(Codes) andalso is_binary(Message) -> + ExtraFields ++ emqx_dashboard_swagger:error_codes(Codes, Message). get_response_body_schema() -> emqx_dashboard_swagger:schema_with_examples( @@ -247,7 +250,8 @@ schema("/actions/:id") -> 204 => <<"Bridge deleted">>, 400 => error_schema( 'BAD_REQUEST', - "Cannot delete bridge while active rules are defined for this bridge" + "Cannot delete bridge while active rules are defined for this bridge", + [{rules, mk(array(string()), #{desc => "Dependent Rule IDs"})}] ), 404 => error_schema('NOT_FOUND', "Bridge not found"), 503 => error_schema('SERVICE_UNAVAILABLE', "Service unavailable") @@ -445,15 +449,12 @@ schema("/action_types") -> reason := rules_depending_on_this_bridge, rule_ids := RuleIds }} -> - RuleIdLists = [binary_to_list(iolist_to_binary(X)) || X <- RuleIds], - RulesStr = string:join(RuleIdLists, ", "), - Msg = io_lib:format( - "Cannot delete bridge while active rules are depending on it: ~s\n" - "Append ?also_delete_dep_actions=true to the request URL to delete " - "rule actions that depend on this bridge as well.", - [RulesStr] + Msg0 = ?ERROR_MSG( + 'BAD_REQUEST', + bin("Cannot delete action while active rules are depending on it") ), - ?BAD_REQUEST(iolist_to_binary(Msg)); + Msg = Msg0#{rules => RuleIds}, + {400, Msg}; {error, timeout} -> ?SERVICE_UNAVAILABLE(<<"request timeout">>); {error, Reason} -> diff --git a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl index e88206ccd..d54330d54 100644 --- a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl @@ -621,9 +621,10 @@ t_check_dependent_actions_on_delete(Config) -> Config ), %% deleting the bridge should fail because there is a rule that depends on it - {ok, 400, _} = request( + {ok, 400, Body} = request( delete, uri(["bridges", BridgeID]) ++ "?also_delete_dep_actions=false", Config ), + ?assertMatch(#{<<"rules">> := [_ | _]}, emqx_utils_json:decode(Body, [return_maps])), %% delete the rule first {ok, 204, <<>>} = request(delete, uri(["rules", RuleId]), Config), %% then delete the bridge is OK diff --git a/apps/emqx_bridge/test/emqx_bridge_v2_api_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_v2_api_SUITE.erl index 83a857b47..16bbcf7a5 100644 --- a/apps/emqx_bridge/test/emqx_bridge_v2_api_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_v2_api_SUITE.erl @@ -958,11 +958,12 @@ t_cascade_delete_actions(Config) -> }, Config ), - {ok, 400, _} = request( + {ok, 400, Body} = request( delete, uri([?ROOT, BridgeID]), Config ), + ?assertMatch(#{<<"rules">> := [_ | _]}, emqx_utils_json:decode(Body, [return_maps])), {ok, 200, [_]} = request_json(get, uri([?ROOT]), Config), %% Cleanup {ok, 204, _} = request( diff --git a/mix.exs b/mix.exs index 159b105a5..7768d84d3 100644 --- a/mix.exs +++ b/mix.exs @@ -58,7 +58,7 @@ defmodule EMQXUmbrella.MixProject do {:ekka, github: "emqx/ekka", tag: "0.15.16", override: true}, {:gen_rpc, github: "emqx/gen_rpc", tag: "3.2.2", override: true}, {:grpc, github: "emqx/grpc-erl", tag: "0.6.12", override: true}, - {:minirest, github: "emqx/minirest", tag: "1.3.14", override: true}, + {:minirest, github: "emqx/minirest", tag: "1.3.15", override: true}, {:ecpool, github: "emqx/ecpool", tag: "0.5.4", override: true}, {:replayq, github: "emqx/replayq", tag: "0.3.7", override: true}, {:pbkdf2, github: "emqx/erlang-pbkdf2", tag: "2.0.4", override: true}, diff --git a/rebar.config b/rebar.config index eec2d505a..98a53ba3b 100644 --- a/rebar.config +++ b/rebar.config @@ -65,7 +65,7 @@ , {ekka, {git, "https://github.com/emqx/ekka", {tag, "0.15.16"}}} , {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "3.2.2"}}} , {grpc, {git, "https://github.com/emqx/grpc-erl", {tag, "0.6.12"}}} - , {minirest, {git, "https://github.com/emqx/minirest", {tag, "1.3.14"}}} + , {minirest, {git, "https://github.com/emqx/minirest", {tag, "1.3.15"}}} , {ecpool, {git, "https://github.com/emqx/ecpool", {tag, "0.5.4"}}} , {replayq, {git, "https://github.com/emqx/replayq.git", {tag, "0.3.7"}}} , {pbkdf2, {git, "https://github.com/emqx/erlang-pbkdf2.git", {tag, "2.0.4"}}}