Merge pull request #4070 from wwhai/fix-update-problem

feat(rule-engine): change init resource to test resource

apps/emqx_rule_engine/src/emqx_rule_engine.erl

@@ -249,13 +249,13 @@ create_resource(#{type := Type, config := Config0} = Params) ->
 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)),
+            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}
@@ -267,21 +267,21 @@ do_update_resource_check(Id, NewParams) ->
                        type = Type,
                        config = OldConfig,
                        description = OldDescription} = _OldResource} ->
-                try
-                    do_update_resource(#{id => Id,
-                                        config => case maps:find(<<"config">>, NewParams) of
-                                                       {ok, NewConfig} -> NewConfig;
-                                                       error -> OldConfig
-                                                  end,
-                                        type => Type,
-                                        description => case maps:find(<<"description">>, NewParams) of
-                                                            {ok, NewDescription} -> NewDescription;
-                                                            error -> OldDescription
-                                                       end}),
-                    ok
-                catch _ : Reason ->
-                    {error, Reason}
-                end;
+            try
+                do_update_resource(#{id => Id,
+                                     config => case maps:find(<<"config">>, NewParams) of
+                                                    {ok, NewConfig} -> NewConfig;
+                                                    error -> OldConfig
+                                               end,
+                                     type => Type,
+                                     description => case maps:find(<<"description">>, NewParams) of
+                                                         {ok, NewDescription} -> NewDescription;
+                                                         error -> OldDescription
+                                                    end}),
+                ok
+            catch _ : Reason ->
+                {error, Reason}
+            end;
         _Other ->
             {error, not_found}
     end.
@@ -289,15 +289,20 @@ do_update_resource_check(Id, NewParams) ->
 do_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},
-                            on_destroy = {Module, Destroy},
                             params_spec = ParamSpec}} ->
             Config = emqx_rule_validator:validate_params(NewConfig, ParamSpec),
-            cluster_call(init_resource, [Module, Create, Id, Config]),
-            emqx_rule_registry:add_resource(#resource{id = Id,
-                                                      type = Type,
-                                                      config = Config,
-                                                      description = NewDescription,
-                                                      created_at = erlang:system_time(millisecond)})
+            case test_resource(#{type => Type, config => NewConfig}) of
+                ok ->
+                    Resource = #resource{id = Id,
+                                         type = Type,
+                                         config = Config,
+                                         description = NewDescription,
+                                         created_at = erlang:system_time(millisecond)},
+                    cluster_call(init_resource, [Module, Create, Id, Config]),
+                    emqx_rule_registry:add_resource(Resource);
+               {error, Reason} ->
+                    {error, Reason}
+            end
     end.
 
 -spec(start_resource(resource_id()) -> ok | {error, Reason :: term()}).

apps/emqx_rule_engine/src/emqx_rule_engine_api.erl

@@ -329,13 +329,13 @@ start_resource(#{id := Id}, _Params) ->
 
 update_resource(#{id := Id}, NewParams) ->
     P1 = case proplists:get_value(<<"description">>, NewParams) of
-            undefined -> #{};
-            Value -> #{<<"description">> => Value}
+        undefined -> #{};
+        Value -> #{<<"description">> => Value}
     end,
     P2 = case proplists:get_value(<<"config">>, NewParams) of
-            undefined -> #{};
-            <<"{}">> -> #{};
-            Map -> #{<<"config">> => ?RAISE(maps:from_list(Map), {invalid_config, Map})}
+        undefined -> #{};
+        <<"{}">> -> #{};
+        Map -> #{<<"config">> => ?RAISE(maps:from_list(Map), {invalid_config, Map})}
     end,
     case emqx_rule_engine:update_resource(Id, maps:merge(P1, P2)) of
         ok ->

apps/emqx_rule_engine/src/emqx_rule_engine_cli.erl

@@ -167,7 +167,7 @@ resources(["create" | Params]) ->
 
 resources(["update" | Params]) ->
     with_opts(fun({Opts, _}) ->
-        Id = maps:get(id, maps:from_list(Opts)),
+        Id = proplists:get_value(id, Opts),
         Maps = make_updated_resource(Opts),
         case emqx_rule_engine:update_resource(Id, Maps) of
             ok ->
@@ -324,12 +324,12 @@ make_resource(Opts) ->
 
 make_updated_resource(Opts) ->
     P1 = case proplists:get_value(description, Opts) of
-            undefined -> #{};
-            Value -> #{<<"description">> => Value}
+        undefined -> #{};
+        Value -> #{<<"description">> => Value}
     end,
     P2 = case proplists:get_value(config, Opts) of
-            undefined -> #{};
-            Map -> #{<<"config">> => ?RAISE((emqx_json:decode(Map, [return_maps])), {invalid_config, Map})}
+        undefined -> #{};
+        Map -> #{<<"config">> => ?RAISE((emqx_json:decode(Map, [return_maps])), {invalid_config, Map})}
     end,
     maps:merge(P1, P2).