diff --git a/src/ejabberd_app.erl b/src/ejabberd_app.erl index c1ede1526b..7d02bba4f2 100644 --- a/src/ejabberd_app.erl +++ b/src/ejabberd_app.erl @@ -80,6 +80,7 @@ do_start() -> mongoose_listener:start(), ejabberd_admin:start(), mongoose_metrics:init_mongooseim_metrics(), + gen_hook:reload_hooks(), update_status_file(started), ?LOG_NOTICE(#{what => mongooseim_node_started, version => ?MONGOOSE_VERSION, node => node()}), Sup. diff --git a/src/gen_hook.erl b/src/gen_hook.erl index 3465ce48d5..cf47027c0e 100644 --- a/src/gen_hook.erl +++ b/src/gen_hook.erl @@ -9,6 +9,7 @@ add_handlers/1, delete_handlers/1, run_fold/4]). +-export([reload_hooks/0]). %% gen_server callbacks -export([init/1, @@ -70,7 +71,6 @@ hook_fn :: hook_fn(), extra :: extra()}). --define(TABLE, ?MODULE). %%%---------------------------------------------------------------------- %%% API %%%---------------------------------------------------------------------- @@ -135,59 +135,66 @@ delete_handler({HookName, Tag, _, _, _} = HookTuple) -> Params :: hook_params()) -> hook_fn_ret(). run_fold(HookName, Tag, Acc, Params) -> Key = hook_key(HookName, Tag), - case ets:lookup(?TABLE, Key) of - [{_, Ls}] -> + case persistent_term:get(?MODULE, #{}) of + #{Key := Ls} -> mongoose_metrics:increment_generic_hook_metric(Tag, HookName), run_hook(Ls, Acc, Params, Key); - [] -> + _ -> {ok, Acc} end. +reload_hooks() -> + gen_server:call(?MODULE, reload_hooks). + %%%---------------------------------------------------------------------- %%% gen_server callback functions %%%---------------------------------------------------------------------- init([]) -> - ets:new(?TABLE, [named_table, {read_concurrency, true}]), - {ok, no_state}. + erlang:process_flag(trap_exit, true), %% We need to make sure that terminate is called in tests + {ok, #{}}. handle_call({add_handler, Key, #hook_handler{} = HookHandler}, _From, State) -> - Reply = case ets:lookup(?TABLE, Key) of - [{_, Ls}] -> - case lists:search(fun_is_handler_equal_to(HookHandler), Ls) of - {value, _} -> - ?LOG_WARNING(#{what => duplicated_handler, - key => Key, handler => HookHandler}), - ok; - false -> - %% NOTE: sort *only* on the priority, - %% order of other fields is not part of the contract - NewLs = lists:keymerge(#hook_handler.prio, Ls, [HookHandler]), - ets:insert(?TABLE, {Key, NewLs}), - ok - end; - [] -> - NewLs = [HookHandler], - ets:insert(?TABLE, {Key, NewLs}), - create_hook_metric(Key), - ok - end, - {reply, Reply, State}; + NewState = + case maps:get(Key, State, []) of + [] -> + NewLs = [HookHandler], + create_hook_metric(Key), + maps:put(Key, NewLs, State); + Ls -> + case lists:search(fun_is_handler_equal_to(HookHandler), Ls) of + {value, _} -> + ?LOG_WARNING(#{what => duplicated_handler, + key => Key, handler => HookHandler}), + State; + false -> + %% NOTE: sort *only* on the priority, + %% order of other fields is not part of the contract + NewLs = lists:keymerge(#hook_handler.prio, Ls, [HookHandler]), + maps:put(Key, NewLs, State) + end + end, + maybe_insert_immediately(NewState), + {reply, ok, NewState}; handle_call({delete_handler, Key, #hook_handler{} = HookHandler}, _From, State) -> - Reply = case ets:lookup(?TABLE, Key) of - [{_, Ls}] -> - %% NOTE: The straightforward handlers comparison would compare - %% the function objects, which is not well-defined in OTP. - %% So we do a manual comparison on the MFA of the funs, - %% by using `erlang:fun_info/2` - Pred = fun_is_handler_equal_to(HookHandler), - {_, NewLs} = lists:partition(Pred, Ls), - ets:insert(?TABLE, {Key, NewLs}), - ok; - [] -> - ok - end, - {reply, Reply, State}; + NewState = + case maps:get(Key, State, []) of + [] -> + State; + Ls -> + %% NOTE: The straightforward handlers comparison would compare + %% the function objects, which is not well-defined in OTP. + %% So we do a manual comparison on the MFA of the funs, + %% by using `erlang:fun_info/2` + Pred = fun_is_handler_equal_to(HookHandler), + {_, NewLs} = lists:partition(Pred, Ls), + maps:put(Key, NewLs, State) + end, + maybe_insert_immediately(NewState), + {reply, ok, NewState}; +handle_call(reload_hooks, _From, State) -> + persistent_term:put(?MODULE, State), + {reply, ok, State}; handle_call(Request, From, State) -> ?UNEXPECTED_CALL(Request, From), {reply, bad_request, State}. @@ -201,7 +208,7 @@ handle_info(Info, State) -> {noreply, State}. terminate(_Reason, _State) -> - ets:delete(?TABLE), + persistent_term:erase(?MODULE), ok. code_change(_OldVsn, State, _Extra) -> @@ -210,6 +217,19 @@ code_change(_OldVsn, State, _Extra) -> %%%---------------------------------------------------------------------- %%% Internal functions %%%---------------------------------------------------------------------- + +%% @doc This call inserts the new hooks map immediately only if an existing map was already present. +%% This simplifies tests: at startup we wait until all hooks have been accumulated +%% before inserting them all at once, while during tests we don't need to remember +%% to reload on every change +maybe_insert_immediately(State) -> + case persistent_term:get(?MODULE, hooks_not_set) of + hooks_not_set -> + ok; + _ -> + persistent_term:put(?MODULE, State) + end. + -spec run_hook([#hook_handler{}], hook_acc(), hook_params(), key()) -> hook_fn_ret(). run_hook([], Acc, _Params, _Key) -> {ok, Acc}; diff --git a/src/metrics/mongoose_metrics.erl b/src/metrics/mongoose_metrics.erl index 5af20978f0..4756afeac2 100644 --- a/src/metrics/mongoose_metrics.erl +++ b/src/metrics/mongoose_metrics.erl @@ -145,7 +145,7 @@ get_global_metric_names() -> get_aggregated_values(Metric) -> exometer:aggregate([{{['_', Metric], '_', '_'}, [], [true]}], [one, count, value]). --spec increment_generic_hook_metric(mongooseim:host_type(), atom()) -> ok | {error, any()}. +-spec increment_generic_hook_metric(mongooseim:host_type_or_global(), atom()) -> ok | {error, any()}. increment_generic_hook_metric(HostType, Hook) -> UseOrSkip = filter_hook(Hook), do_increment_generic_hook_metric(HostType, Hook, UseOrSkip). diff --git a/test/auth_tokens_SUITE.erl b/test/auth_tokens_SUITE.erl index 9c5be975e1..15c0d03af6 100644 --- a/test/auth_tokens_SUITE.erl +++ b/test/auth_tokens_SUITE.erl @@ -56,7 +56,7 @@ init_per_testcase(Test, Config) Test =:= validation_property; Test =:= choose_key_by_token_type -> mock_mongoose_metrics(), - Config1 = async_helper:start(Config, [{gen_hook, start_link, []}]), + Config1 = async_helper:start(Config, [{mongooseim_helper, start_link_loaded_hooks, []}]), mock_keystore(), mock_rdbms_backend(), Config1; @@ -66,12 +66,12 @@ init_per_testcase(validity_period_test, Config) -> mock_mongoose_metrics(), mock_gen_iq_handler(), mock_ejabberd_commands(), - async_helper:start(Config, [{gen_hook, start_link, []}]); + async_helper:start(Config, [{mongooseim_helper, start_link_loaded_hooks, []}]); init_per_testcase(revoked_token_is_not_valid, Config) -> mock_mongoose_metrics(), mock_tested_backend(), - Config1 = async_helper:start(Config, [{gen_hook, start_link, []}]), + Config1 = async_helper:start(Config, [{mongooseim_helper, start_link_loaded_hooks, []}]), mock_keystore(), Config1; diff --git a/test/component_reg_SUITE.erl b/test/component_reg_SUITE.erl index 3a68f99bb1..994e5d07df 100644 --- a/test/component_reg_SUITE.erl +++ b/test/component_reg_SUITE.erl @@ -18,13 +18,13 @@ init_per_suite(C) -> meck:expect(mongoose_domain_api, get_host_type, fun(_) -> {error, not_found} end), application:ensure_all_started(exometer_core), - gen_hook:start_link(), + mongooseim_helper:start_link_loaded_hooks(), ejabberd_router:start_link(), C. init_per_testcase(_, C) -> mongoose_router:start(), - gen_hook:start_link(), + mongooseim_helper:start_link_loaded_hooks(), C. end_per_suite(_C) -> @@ -48,7 +48,7 @@ registering(_C) -> ok. registering_with_local(_C) -> - gen_hook:start_link(), + mongooseim_helper:start_link_loaded_hooks(), Dom = <<"aaa.bbb.com">>, ThisNode = node(), AnotherNode = 'another@nohost', diff --git a/test/event_pusher_sns_SUITE.erl b/test/event_pusher_sns_SUITE.erl index 36b7d15219..bcbaef342a 100644 --- a/test/event_pusher_sns_SUITE.erl +++ b/test/event_pusher_sns_SUITE.erl @@ -112,7 +112,7 @@ end_per_suite(_) -> ok. init_per_testcase(CaseName, Config) -> - gen_hook:start_link(), + mongooseim_helper:start_link_loaded_hooks(), meck:new(erlcloud_sns, [non_strict, passthrough]), meck:new([mongoose_wpool, mongoose_metrics], [stub_all]), meck:expect(erlcloud_sns, new, fun(_, _, _) -> mod_aws_sns_SUITE_erlcloud_sns_new end), diff --git a/test/gen_hook_SUITE.erl b/test/gen_hook_SUITE.erl index 822362f8c8..cfd360c406 100644 --- a/test/gen_hook_SUITE.erl +++ b/test/gen_hook_SUITE.erl @@ -33,7 +33,7 @@ end_per_suite(Config) -> Config. init_per_testcase(_, Config) -> - gen_hook:start_link(), + mongooseim_helper:start_link_loaded_hooks(), Config. end_per_testcase(_, Config) -> @@ -338,4 +338,4 @@ get_hook_handler(ModName, FunName, Fun) when is_function(Fun, 3) -> fun ModName:FunName/3. get_handlers_for_all_hooks() -> - ets:tab2list(gen_hook). + maps:to_list(persistent_term:get(gen_hook, #{})). diff --git a/test/keystore_SUITE.erl b/test/keystore_SUITE.erl index 0f1de97040..055c02b868 100644 --- a/test/keystore_SUITE.erl +++ b/test/keystore_SUITE.erl @@ -27,7 +27,7 @@ end_per_suite(_C) -> init_per_testcase(_, Config) -> mock_mongoose_metrics(), - Config1 = async_helper:start(Config, gen_hook, start_link, []), + Config1 = async_helper:start(Config, mongooseim_helper, start_link_loaded_hooks, []), mongoose_config:set_opts(opts()), Config1. diff --git a/test/mod_global_distrib_SUITE.erl b/test/mod_global_distrib_SUITE.erl index d714899b16..8c42b327da 100644 --- a/test/mod_global_distrib_SUITE.erl +++ b/test/mod_global_distrib_SUITE.erl @@ -68,7 +68,7 @@ init_per_testcase(_CaseName, Config) -> mongoose_config:set_opts(opts()), mongoose_domain_sup:start_link(), mim_ct_sup:start_link(ejabberd_sup), - gen_hook:start_link(), + mongooseim_helper:start_link_loaded_hooks(), mongoose_modules:start(), Config. diff --git a/test/mongoose_cleanup_SUITE.erl b/test/mongoose_cleanup_SUITE.erl index f3543dc624..533c6a5123 100644 --- a/test/mongoose_cleanup_SUITE.erl +++ b/test/mongoose_cleanup_SUITE.erl @@ -82,7 +82,7 @@ end_per_group(_Group, Config) -> init_per_testcase(TestCase, Config) -> mim_ct_sup:start_link(ejabberd_sup), - {ok, _HooksServer} = gen_hook:start_link(), + {ok, _HooksServer} = mongooseim_helper:start_link_loaded_hooks(), {ok, _DomainSup} = mongoose_domain_sup:start_link(), setup_meck(meck_mods(TestCase)), start_component(TestCase), diff --git a/test/mongoose_subdomain_core_SUITE.erl b/test/mongoose_subdomain_core_SUITE.erl index 807c40b433..f2bc034f0b 100644 --- a/test/mongoose_subdomain_core_SUITE.erl +++ b/test/mongoose_subdomain_core_SUITE.erl @@ -39,7 +39,7 @@ init_per_testcase(TestCase, Config) -> %% - one "dynamic" host type with two configured domain names %% initial mongoose_subdomain_core conditions: %% - no subdomains configured for any host type - gen_hook:start_link(), + mongooseim_helper:start_link_loaded_hooks(), mongoose_domain_sup:start_link(?STATIC_PAIRS, ?ALLOWED_HOST_TYPES), [mongoose_domain_core:insert(Domain, ?DYNAMIC_HOST_TYPE2, dummy_source) || Domain <- ?DYNAMIC_DOMAINS], diff --git a/test/mongooseim_helper.erl b/test/mongooseim_helper.erl new file mode 100644 index 0000000000..e8d9a81a46 --- /dev/null +++ b/test/mongooseim_helper.erl @@ -0,0 +1,7 @@ +-module(mongooseim_helper). +-compile([export_all, nowarn_export_all]). + +start_link_loaded_hooks() -> + Ret = gen_hook:start_link(), + gen_hook:reload_hooks(), + Ret. diff --git a/test/mongooseim_metrics_SUITE.erl b/test/mongooseim_metrics_SUITE.erl index ce82601655..2c14bd1804 100644 --- a/test/mongooseim_metrics_SUITE.erl +++ b/test/mongooseim_metrics_SUITE.erl @@ -38,7 +38,7 @@ init_per_suite(C) -> Sup = spawn(fun() -> mim_ct_sup:start_link(ejabberd_sup), Hooks = {gen_hook, - {gen_hook, start_link, []}, + {mongooseim_helper, start_link_loaded_hooks, []}, permanent, brutal_kill, worker, diff --git a/test/muc_light_SUITE.erl b/test/muc_light_SUITE.erl index 1dfb784395..34fbb25ab6 100644 --- a/test/muc_light_SUITE.erl +++ b/test/muc_light_SUITE.erl @@ -56,7 +56,7 @@ init_per_testcase(codec_calls, Config) -> ok = mnesia:create_schema([node()]), ok = mnesia:start(), {ok, _} = application:ensure_all_started(exometer_core), - gen_hook:start_link(), + mongooseim_helper:start_link_loaded_hooks(), ejabberd_router:start_link(), mim_ct_sup:start_link(ejabberd_sup), mongoose_modules:start(), diff --git a/test/privacy_SUITE.erl b/test/privacy_SUITE.erl index 7f4ee78b45..bf240d0fd3 100644 --- a/test/privacy_SUITE.erl +++ b/test/privacy_SUITE.erl @@ -46,7 +46,7 @@ end_per_suite(_C) -> ok. init_per_testcase(_, C) -> - gen_hook:start_link(), + mongooseim_helper:start_link_loaded_hooks(), mongoose_config:set_opts(opts()), mongoose_modules:start(), C. diff --git a/test/roster_SUITE.erl b/test/roster_SUITE.erl index 4548e67e5c..8de669cc69 100644 --- a/test/roster_SUITE.erl +++ b/test/roster_SUITE.erl @@ -48,7 +48,7 @@ end_per_suite(_C) -> init_per_testcase(_TC, C) -> init_ets(), - gen_hook:start_link(), + mongooseim_helper:start_link_loaded_hooks(), mongoose_modules:start(), C. diff --git a/test/router_SUITE.erl b/test/router_SUITE.erl index 3e46a99dbf..d5fffdcc06 100644 --- a/test/router_SUITE.erl +++ b/test/router_SUITE.erl @@ -38,7 +38,7 @@ end_per_suite(_C) -> init_per_group(routing, Config) -> mongoose_config:set_opts(opts()), - gen_hook:start_link(), + mongooseim_helper:start_link_loaded_hooks(), ejabberd_router:start_link(), Config.