From 29c76d16d79eff27fc504e8dc8ca6189e2970fc3 Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Thu, 29 Sep 2022 13:40:01 +0800 Subject: [PATCH 1/5] fix: reset rule metrics crash if it has not initialized --- apps/emqx_rule_engine/src/emqx_rule_metrics.erl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/emqx_rule_engine/src/emqx_rule_metrics.erl b/apps/emqx_rule_engine/src/emqx_rule_metrics.erl index 624032056..e8a77e0db 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_metrics.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_metrics.erl @@ -328,6 +328,8 @@ handle_call({create_rule_metrics, Id}, _From, _ -> RuleSpeeds#{Id => #rule_speed{}} end}}; +handle_call({reset_speeds, _Id}, _From, State = #state{rule_speeds = undefined}) -> + {reply, ok, State}; handle_call({reset_speeds, Id}, _From, State = #state{rule_speeds = RuleSpeedMap}) -> {reply, ok, State#state{rule_speeds = maps:put(Id, #rule_speed{}, RuleSpeedMap)}}; From 7d2dd3d37daddde0f7fb2a60aa4617ff07480bab Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Thu, 29 Sep 2022 13:41:02 +0800 Subject: [PATCH 2/5] fix: deny POST an existing resource --- .../emqx_rule_engine/src/emqx_rule_engine_api.erl | 10 ++++++++++ .../test/emqx_rule_engine_SUITE.erl | 15 ++++++++++----- 2 files changed, 20 insertions(+), 5 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 50fdcfe22..eec67e914 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -315,6 +315,16 @@ create_resource(#{}, Params) -> end. do_create_resource(Create, ParsedParams) -> + case maps:find(id, ParsedParams) of + {ok, ResId} -> + case emqx_rule_registry:find_resource(ResId) of + {ok, _} -> return({error, 400, <<"Already Exists">>}); + not_found -> do_create_resource2(Create, ParsedParams) + end; + error -> do_create_resource2(Create, ParsedParams) + end. + +do_create_resource2(Create, ParsedParams) -> case emqx_rule_engine:Create(ParsedParams) of ok -> return(ok); 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 c8f66ea00..dc95c0368 100644 --- a/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl +++ b/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl @@ -600,13 +600,18 @@ t_show_action_api(_Config) -> ok. t_crud_resources_api(_Config) -> + ResParams = [ + {<<"name">>, <<"Simple Resource">>}, + {<<"type">>, <<"built_in">>}, + {<<"config">>, [{<<"a">>, 1}]}, + {<<"description">>, <<"Simple Resource">>} + ], {ok, #{code := 0, data := Resources1}} = - emqx_rule_engine_api:create_resource(#{}, - [{<<"name">>, <<"Simple Resource">>}, - {<<"type">>, <<"built_in">>}, - {<<"config">>, [{<<"a">>, 1}]}, - {<<"description">>, <<"Simple Resource">>}]), + emqx_rule_engine_api:create_resource(#{}, ResParams), ResId = maps:get(id, Resources1), + %% create again using given resource id returns error + {ok, #{code := 400, message := <<"Already Exists">>}} = + emqx_rule_engine_api:create_resource(#{}, [{<<"id">>, ResId} | ResParams]), {ok, #{code := 0, data := Resources}} = emqx_rule_engine_api:list_resources(#{}, []), ?assert(length(Resources) > 0), {ok, #{code := 0, data := Resources2}} = emqx_rule_engine_api:show_resource(#{id => ResId}, []), From 33e68c9d165ef09224c3200df6bb3069841340fc Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Fri, 30 Sep 2022 15:55:17 +0800 Subject: [PATCH 3/5] chore: update the CHANGES-4.3.md --- CHANGES-4.3.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGES-4.3.md b/CHANGES-4.3.md index 2e2f5157c..b1505844d 100644 --- a/CHANGES-4.3.md +++ b/CHANGES-4.3.md @@ -14,6 +14,11 @@ File format: ## v4.3.21 +### Bug fixes + +- Deny POST an existing resource id using HTTP API with error 400 "Already Exists". [#9079](https://github.com/emqx/emqx/pull/9079) +- Fix the issue that reseting rule metrics crashed under certain conditions. [#9079](https://github.com/emqx/emqx/pull/9079) + ### Enhancements - TLS listener memory usage optimization [#9005](https://github.com/emqx/emqx/pull/9005). From ab3ec9c176a60414cba794a30d15ce929fc76eba Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Fri, 30 Sep 2022 16:32:52 +0800 Subject: [PATCH 4/5] chore: update the appup.src --- .../src/emqx_rule_engine.appup.src | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src b/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src index 6aa35d2d6..b46be85c7 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src +++ b/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src @@ -3,17 +3,20 @@ {VSN, [{"4.3.15", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]}, {"4.3.14", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]}, {"4.3.13", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, @@ -22,6 +25,7 @@ {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]}, {"4.3.12", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, @@ -30,6 +34,7 @@ {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]}, {"4.3.11", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]}, @@ -39,6 +44,7 @@ {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]}, {"4.3.10", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_validator,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, @@ -48,6 +54,7 @@ {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]}, {"4.3.9", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_validator,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]}, @@ -200,17 +207,20 @@ {<<".*">>,[]}], [{"4.3.15", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]}, {"4.3.14", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]}, {"4.3.13", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, @@ -219,6 +229,7 @@ {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]}, {"4.3.12", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, @@ -227,6 +238,7 @@ {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]}, {"4.3.11", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]}, @@ -236,6 +248,7 @@ {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]}, {"4.3.10", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_validator,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}, @@ -245,6 +258,7 @@ {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]}, {"4.3.9", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_validator,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]}, From 1bb7c23db13a629d4c5bf37ef7d29eb8f3590a77 Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Sat, 8 Oct 2022 10:50:42 +0800 Subject: [PATCH 5/5] fix: discard the 'id' field when testing a resource --- apps/emqx_rule_engine/src/emqx_rule_engine_api.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 eec67e914..ea0a13824 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -306,7 +306,7 @@ show_action(#{name := Name}, _Params) -> create_resource(#{}, Params) -> case parse_resource_params(Params) of {ok, ParsedParams} -> - if_test(fun() -> do_create_resource(test_resource, ParsedParams) end, + if_test(fun() -> do_create_resource(test_resource, maps:without([id], ParsedParams)) end, fun() -> do_create_resource(create_resource, ParsedParams) end, Params); {error, Reason} ->