Merge pull request #12096 from thalesmg/fix-delete-deps-rules-m-20231204

fix(api): return list of dependent rule ids when trying to delete bridge/action

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} ->

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} ->

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

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(

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.3.0", 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},

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.3.0"}}}
     , {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"}}}