From 9fd43ef8823387a423c0dc8f7ffdb42e7d3d1f5d Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Mon, 26 Apr 2021 18:10:53 +0800 Subject: [PATCH] fix(rule_engine): reformat some code for dependent rules discovery --- .../emqx_rule_engine/src/emqx_rule_engine.erl | 39 +++++++------------ .../src/emqx_rule_engine_api.erl | 10 ++--- .../src/emqx_rule_registry.erl | 31 +++++++++++---- 3 files changed, 43 insertions(+), 37 deletions(-) diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine.erl b/apps/emqx_rule_engine/src/emqx_rule_engine.erl index 89b7dfe27..c63891059 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine.erl @@ -249,40 +249,30 @@ create_resource(#{type := Type, config := Config0} = Params) -> -spec(update_resource(resource_id(), map()) -> ok | {error, Reason :: term()}). update_resource(ResId, NewParams) -> - try - lists:foreach(fun(#rule{id = RuleId, enabled = Enabled, actions = Actions}) -> - lists:foreach( - fun (#action_instance{args = #{<<"$resource">> := ResId1}}) - when ResId =:= ResId1, Enabled =:= true -> - throw({dependency_exists, RuleId}); - (_) -> ok - end, Actions) - end, ets:tab2list(?RULE_TAB)), - do_update_resource_check(ResId, NewParams) - catch _ : Reason -> - {error, Reason} + case emqx_rule_registry:find_enabled_rules_depends_on_resource(ResId) of + [] -> check_and_update_resource(ResId, NewParams); + Rules -> + {error, {dependent_rules_exists, [Id || #rule{id = Id} <- Rules]}} end. -do_update_resource_check(Id, NewParams) -> +check_and_update_resource(Id, NewParams) -> case emqx_rule_registry:find_resource(Id) of - {ok, #resource{id = Id, - type = Type, - config = OldConfig, - description = OldDescription} = _OldResource} -> + {ok, #resource{id = Id, type = Type, config = OldConfig, description = OldDescr}} -> try Conifg = maps:get(<<"config">>, NewParams, OldConfig), - Descr = maps:get(<<"description">>, NewParams, OldDescription), - do_update_resource(#{id => Id, config => Conifg, type => Type, - description => Descr}), - ok - catch _ : Reason -> + Descr = maps:get(<<"description">>, NewParams, OldDescr), + do_check_and_update_resource(#{id => Id, config => Conifg, type => Type, + description => Descr}) + catch Error:Reason:ST -> + ?LOG(error, "check_and_update_resource failed: ~0p", [{Error, Reason, ST}]), {error, Reason} end; _Other -> {error, not_found} end. -do_update_resource(#{id := Id, type := Type, description := NewDescription, config := NewConfig}) -> +do_check_and_update_resource(#{id := Id, type := Type, description := NewDescription, + config := NewConfig}) -> case emqx_rule_registry:find_resource_type(Type) of {ok, #resource_type{on_create = {Module, Create}, params_spec = ParamSpec}} -> @@ -296,7 +286,8 @@ do_update_resource(#{id := Id, type := Type, description := NewDescription, conf config = Config, description = NewDescription, created_at = erlang:system_time(millisecond) - }); + }), + ok; {error, Reason} -> error({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 3b99db549..4a7e27ec1 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -180,6 +180,7 @@ -define(ERR_NO_ACTION(NAME), list_to_binary(io_lib:format("Action ~s Not Found", [(NAME)]))). -define(ERR_NO_RESOURCE(RESID), list_to_binary(io_lib:format("Resource ~s Not Found", [(RESID)]))). -define(ERR_NO_RESOURCE_TYPE(TYPE), list_to_binary(io_lib:format("Resource Type ~s Not Found", [(TYPE)]))). +-define(ERR_DEP_RULES_EXISTS(RULEIDS), list_to_binary(io_lib:format("Found rules ~0p depends on this resource, disable them first", [(RULEIDS)]))). -define(ERR_BADARGS(REASON), begin R0 = list_to_binary(io_lib:format("~0p", [REASON])), @@ -342,14 +343,11 @@ update_resource(#{id := Id}, NewParams) -> ok -> return(ok); {error, not_found} -> - ?LOG(error, "Resource not found: ~0p", [Id]), return({error, 400, <<"Resource not found:", Id/binary>>}); {error, {init_resource, _}} -> - ?LOG(error, "Init resource failure: ~0p", [Id]), return({error, 500, <<"Init resource failure:", Id/binary>>}); - {error, {dependency_exists, RuleId}} -> - ?LOG(error, "Dependency exists: ~0p", [RuleId]), - return({error, 500, <<"Dependency exists:", RuleId/binary>>}); + {error, {dependent_rules_exists, RuleIds}} -> + return({error, 400, ?ERR_DEP_RULES_EXISTS(RuleIds)}); {error, Reason} -> ?LOG(error, "Resource update failed: ~0p", [Reason]), return({error, 500, <<"Resource update failed!">>}) @@ -359,6 +357,8 @@ delete_resource(#{id := Id}, _Params) -> case emqx_rule_engine:delete_resource(Id) of ok -> return(ok); {error, not_found} -> return(ok); + {error, {dependent_rules_exists, RuleIds}} -> + return({error, 400, ?ERR_DEP_RULES_EXISTS(RuleIds)}); {error, Reason} -> return({error, 400, ?ERR_BADARGS(Reason)}) end. diff --git a/apps/emqx_rule_engine/src/emqx_rule_registry.erl b/apps/emqx_rule_engine/src/emqx_rule_registry.erl index 62893450c..58376a4cb 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_registry.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_registry.erl @@ -63,6 +63,8 @@ %% Resource Types -export([ get_resource_types/0 , find_resource_type/1 + , find_rules_depends_on_resource/1 + , find_enabled_rules_depends_on_resource/1 , register_resource_types/1 , unregister_resource_types_of/1 ]). @@ -368,20 +370,33 @@ remove_resource_params(ResId) -> %% @private delete_resource(ResId) -> - lists:foreach(fun(#rule{id = Id, actions = Actions}) -> - lists:foreach( - fun (#action_instance{args = #{<<"$resource">> := ResId1}}) - when ResId =:= ResId1 -> - throw({dependency_exists, {rule, Id}}); - (_) -> ok - end, Actions) - end, get_rules()), + case find_enabled_rules_depends_on_resource(ResId) of + [] -> ok; + Rules -> + throw({dependent_rules_exists, [Id || #rule{id = Id} <- Rules]}) + end, mnesia:delete(?RES_TAB, ResId, write). %% @private insert_resource(Resource) -> mnesia:write(?RES_TAB, Resource, write). +find_enabled_rules_depends_on_resource(ResId) -> + [R || #rule{enabled = true} = R <- find_rules_depends_on_resource(ResId)]. + +find_rules_depends_on_resource(ResId) -> + lists:foldl(fun(#rule{actions = Actions} = R, Rules) -> + case search_action_despends_on_resource(ResId, Actions) of + false -> Rules; + {value, _} -> [R | Rules] + end + end, [], get_rules()). + +search_action_despends_on_resource(ResId, Actions) -> + lists:search(fun(#action_instance{args = #{<<"$resource">> := ResId0}}) -> + ResId0 =:= ResId + end, Actions). + %%------------------------------------------------------------------------------ %% Resource Type Management %%------------------------------------------------------------------------------