From e3501acc02ed068e578122fc68739107a760cbd3 Mon Sep 17 00:00:00 2001
From: Shawn <506895667@qq.com>
Date: Mon, 26 Apr 2021 21:26:46 +0800
Subject: [PATCH] fix(rule_engine): delete resource crashes when dependent rule
 exists

---
 apps/emqx_rule_engine/src/emqx_rule_engine.erl     |  9 ++++++---
 apps/emqx_rule_engine/src/emqx_rule_engine_api.erl |  2 +-
 apps/emqx_rule_engine/src/emqx_rule_monitor.erl    |  2 +-
 apps/emqx_rule_engine/src/emqx_rule_registry.erl   | 11 +++++------
 4 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine.erl b/apps/emqx_rule_engine/src/emqx_rule_engine.erl
index c63891059..f4eb04d1f 100644
--- a/apps/emqx_rule_engine/src/emqx_rule_engine.erl
+++ b/apps/emqx_rule_engine/src/emqx_rule_engine.erl
@@ -356,9 +356,12 @@ delete_resource(ResId) ->
             {ok, #resource_type{on_destroy = {ModD, Destroy}}}
                 = emqx_rule_registry:find_resource_type(ResType),
             try
-                ok = emqx_rule_registry:remove_resource(ResId),
-                _ = ?CLUSTER_CALL(clear_resource, [ModD, Destroy, ResId]),
-                ok
+                case emqx_rule_registry:remove_resource(ResId) of
+                    ok ->
+                        _ = ?CLUSTER_CALL(clear_resource, [ModD, Destroy, ResId]),
+                        ok;
+                    {error, _} = R -> R
+                end
             catch
                 throw:Reason -> {error, Reason}
             end;
diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl
index 4a7e27ec1..4fa3b8aa3 100644
--- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl
+++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl
@@ -350,7 +350,7 @@ update_resource(#{id := Id}, NewParams) ->
             return({error, 400, ?ERR_DEP_RULES_EXISTS(RuleIds)});
         {error, Reason} ->
             ?LOG(error, "Resource update failed: ~0p", [Reason]),
-            return({error, 500, <<"Resource update failed!">>})
+            return({error, 400, ?ERR_BADARGS(Reason)})
     end.
 
 delete_resource(#{id := Id}, _Params) ->
diff --git a/apps/emqx_rule_engine/src/emqx_rule_monitor.erl b/apps/emqx_rule_engine/src/emqx_rule_monitor.erl
index 316ef5ff6..b24d98bce 100644
--- a/apps/emqx_rule_engine/src/emqx_rule_monitor.erl
+++ b/apps/emqx_rule_engine/src/emqx_rule_monitor.erl
@@ -120,6 +120,6 @@ retry_loop(resource, ResId, Interval) ->
 enable_rules_of_resource(ResId) ->
     lists:foreach(
         fun (#rule{enabled = false} = Rule) ->
-                emqx_rule_registry:add_rule(Rule#rule{enabled = false});
+                emqx_rule_registry:add_rule(Rule#rule{enabled = true});
             (_) -> ok
         end, emqx_rule_registry:find_rules_depends_on_resource(ResId)).
diff --git a/apps/emqx_rule_engine/src/emqx_rule_registry.erl b/apps/emqx_rule_engine/src/emqx_rule_registry.erl
index 58376a4cb..4ec5d1cb7 100644
--- a/apps/emqx_rule_engine/src/emqx_rule_registry.erl
+++ b/apps/emqx_rule_engine/src/emqx_rule_registry.erl
@@ -356,7 +356,7 @@ find_resource_params(Id) ->
         [] -> not_found
     end.
 
--spec(remove_resource(emqx_rule_engine:resource() | emqx_rule_engine:resource_id()) -> ok).
+-spec(remove_resource(emqx_rule_engine:resource() | emqx_rule_engine:resource_id()) -> ok | {error, term()}).
 remove_resource(Resource) when is_record(Resource, resource) ->
     trans(fun delete_resource/1, [Resource#resource.id]);
 
@@ -371,11 +371,10 @@ remove_resource_params(ResId) ->
 %% @private
 delete_resource(ResId) ->
     case find_enabled_rules_depends_on_resource(ResId) of
-        [] -> ok;
+        [] -> mnesia:delete(?RES_TAB, ResId, write);
         Rules ->
-            throw({dependent_rules_exists, [Id || #rule{id = Id} <- Rules]})
-    end,
-    mnesia:delete(?RES_TAB, ResId, write).
+            {error, {dependent_rules_exists, [Id || #rule{id = Id} <- Rules]}}
+    end.
 
 %% @private
 insert_resource(Resource) ->
@@ -490,6 +489,6 @@ get_all_records(Tab) ->
 trans(Fun) -> trans(Fun, []).
 trans(Fun, Args) ->
     case mnesia:transaction(Fun, Args) of
-        {atomic, ok} -> ok;
+        {atomic, Result} -> Result;
         {aborted, Reason} -> error(Reason)
     end.