From 49e9ace1c148f623f38c24955b78754aa595e477 Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Thu, 11 May 2023 22:37:50 +0500 Subject: [PATCH] fix(api): respond 404 on the deletion of nonexistent rule --- .../src/emqx_rule_engine.app.src | 2 +- .../src/emqx_rule_engine_api.erl | 27 +++++++++++-------- .../test/emqx_rule_engine_api_SUITE.erl | 9 ++++++- changes/ce/fix-10677.en.md | 1 + 4 files changed, 26 insertions(+), 13 deletions(-) create mode 100644 changes/ce/fix-10677.en.md diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine.app.src b/apps/emqx_rule_engine/src/emqx_rule_engine.app.src index 932ebc5ed..94a48fb35 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine.app.src +++ b/apps/emqx_rule_engine/src/emqx_rule_engine.app.src @@ -2,7 +2,7 @@ {application, emqx_rule_engine, [ {description, "EMQX Rule Engine"}, % strict semver, bump manually! - {vsn, "5.0.15"}, + {vsn, "5.0.16"}, {modules, []}, {registered, [emqx_rule_engine_sup, emqx_rule_engine]}, {applications, [kernel, stdlib, rulesql, getopt, emqx_ctl]}, 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 d66f2c1c9..fdd19bf41 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -407,17 +407,22 @@ param_path_id() -> {400, #{code => 'BAD_REQUEST', message => ?ERR_BADARGS(Reason)}} end; '/rules/:id'(delete, #{bindings := #{id := Id}}) -> - ConfPath = emqx_rule_engine:config_key_path() ++ [Id], - case emqx_conf:remove(ConfPath, #{override_to => cluster}) of - {ok, _} -> - {204}; - {error, Reason} -> - ?SLOG(error, #{ - msg => "delete_rule_failed", - id => Id, - reason => Reason - }), - {500, #{code => 'INTERNAL_ERROR', message => ?ERR_BADARGS(Reason)}} + case emqx_rule_engine:get_rule(Id) of + {ok, _Rule} -> + ConfPath = emqx_rule_engine:config_key_path() ++ [Id], + case emqx_conf:remove(ConfPath, #{override_to => cluster}) of + {ok, _} -> + {204}; + {error, Reason} -> + ?SLOG(error, #{ + msg => "delete_rule_failed", + id => Id, + reason => Reason + }), + {500, #{code => 'INTERNAL_ERROR', message => ?ERR_BADARGS(Reason)}} + end; + not_found -> + {404, #{code => 'NOT_FOUND', message => <<"Rule Id Not Found">>}} end. '/rules/:id/metrics'(get, #{bindings := #{id := Id}}) -> diff --git a/apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl b/apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl index 8d7546fca..ccee05604 100644 --- a/apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl +++ b/apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl @@ -120,7 +120,14 @@ t_crud_rule_api(_Config) -> ) ), - %ct:pal("Show After Deleted: ~p", [NotFound]), + ?assertMatch( + {404, #{code := 'NOT_FOUND'}}, + emqx_rule_engine_api:'/rules/:id'( + delete, + #{bindings => #{id => RuleId}} + ) + ), + ?assertMatch( {404, #{code := _, message := _Message}}, emqx_rule_engine_api:'/rules/:id'(get, #{bindings => #{id => RuleId}}) diff --git a/changes/ce/fix-10677.en.md b/changes/ce/fix-10677.en.md new file mode 100644 index 000000000..c669606e7 --- /dev/null +++ b/changes/ce/fix-10677.en.md @@ -0,0 +1 @@ +In Rule API, reapond with 404 HTTP error code when trying to delete a rule that does not exist.