From ae8f03a1884c5a6189b8b204a0a165059d31e856 Mon Sep 17 00:00:00 2001
From: Shawn <506895667@qq.com>
Date: Wed, 13 Apr 2022 23:16:29 +0800
Subject: [PATCH] fix(rules): deny POST /rules with an existing Id

---
 .../src/emqx_rule_engine_api.erl              | 24 +++++++++++++------
 .../test/emqx_rule_engine_SUITE.erl           |  8 +++++++
 2 files changed, 25 insertions(+), 7 deletions(-)

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 6a50c553d..baa47d71d 100644
--- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl
+++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl
@@ -222,19 +222,29 @@ test_rule_sql(Params) ->
 do_create_rule(Params) ->
     case parse_rule_params(Params) of
         {ok, ParsedParams} ->
-            case emqx_rule_engine:create_rule(ParsedParams) of
-                {ok, Rule} -> return({ok, record_to_map(Rule)});
-                {error, {action_not_found, ActionName}} ->
-                    return({error, 400, ?ERR_NO_ACTION(ActionName)});
-                {error, Reason} ->
-                    ?LOG(error, "~p failed: ~0p", [?FUNCTION_NAME, Reason]),
-                    return({error, 400, ?ERR_BADARGS(Reason)})
+            case maps:find(id, ParsedParams) of
+                {ok, RuleId} ->
+                    case emqx_rule_registry:get_rule(RuleId) of
+                        {ok, _} -> return({error, 400, <<"Already Exists">>});
+                        not_found -> do_create_rule2(ParsedParams)
+                    end;
+                error -> do_create_rule2(ParsedParams)
             end;
         {error, Reason} ->
             ?LOG(error, "~p failed: ~0p", [?FUNCTION_NAME, Reason]),
             return({error, 400, ?ERR_BADARGS(Reason)})
     end.
 
+do_create_rule2(ParsedParams) ->
+    case emqx_rule_engine:create_rule(ParsedParams) of
+        {ok, Rule} -> return({ok, record_to_map(Rule)});
+        {error, {action_not_found, ActionName}} ->
+            return({error, 400, ?ERR_NO_ACTION(ActionName)});
+        {error, Reason} ->
+            ?LOG(error, "~p failed: ~0p", [?FUNCTION_NAME, Reason]),
+            return({error, 400, ?ERR_BADARGS(Reason)})
+    end.
+
 update_rule(#{id := Id}, Params) ->
     case parse_rule_params(Params, #{id => Id}) of
         {ok, ParsedParams} ->
diff --git a/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl b/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl
index 240438134..20a5345f7 100644
--- a/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl
+++ b/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl
@@ -428,6 +428,14 @@ t_crud_rule_api(_Config) ->
                                    {<<"params">>,[{<<"arg1">>,1}]}]]},
                  {<<"description">>, <<"debug rule">>}]),
     RuleID = maps:get(id, Rule),
+    {ok, #{code := 400, message := <<"Already Exists">>}} =
+        emqx_rule_engine_api:create_rule(#{},
+                [{<<"name">>, <<"debug-rule">>},
+                 {<<"id">>, RuleID},
+                 {<<"rawsql">>, <<"select * from \"t/a\"">>},
+                 {<<"actions">>, [[{<<"name">>,<<"inspect">>},
+                                   {<<"params">>,[{<<"arg1">>,1}]}]]},
+                 {<<"description">>, <<"debug rule">>}]),
     %ct:pal("RCreated : ~p", [Rule]),
 
     {ok, #{code := 0, data := Rules}} = emqx_rule_engine_api:list_rules(#{}, []),