From 49fe2cdf260c4ab3f9389e3b12bd41e7fcb918a0 Mon Sep 17 00:00:00 2001 From: DenysGonchar Date: Thu, 15 Apr 2021 13:42:58 +0200 Subject: [PATCH] ensuring that all the calls to ejabberd_hooks run/run_fold functions are wrapped in mongoose_hooks module. hint: all inderect calls to ejabberd_hooks found using 'ejabberd_hooks[^:]' search regex --- big_tests/tests/ejabberdctl_SUITE.erl | 2 +- big_tests/tests/oauth_SUITE.erl | 4 +-- src/metrics/mongoose_metrics.erl | 8 ++--- src/metrics/mongoose_metrics_hooks.erl | 42 +++++++++++----------- src/metrics/mongoose_metrics_mam_hooks.erl | 24 ++++++------- src/mod_adhoc.erl | 18 ++++++---- src/mod_amp.erl | 2 +- src/mod_stream_management.erl | 4 +-- src/mongoose_cleaner.erl | 2 +- src/mongoose_hooks.erl | 27 +++++++++++++- test/keystore_SUITE.erl | 2 +- test/mongoose_cleanup_SUITE.erl | 23 ++++++------ 12 files changed, 94 insertions(+), 64 deletions(-) diff --git a/big_tests/tests/ejabberdctl_SUITE.erl b/big_tests/tests/ejabberdctl_SUITE.erl index fe5476fefbb..b25fbcafc3a 100644 --- a/big_tests/tests/ejabberdctl_SUITE.erl +++ b/big_tests/tests/ejabberdctl_SUITE.erl @@ -246,7 +246,7 @@ end_per_group(Rosters, Config) when (Rosters == roster) or (Rosters == roster_ad [#{ location => {?MODULE, ?FUNCTION_NAME, ?LINE}, lserver => SB, element => undefined }]), - rpc(mim(), ejabberd_hooks, run_fold, [remove_user, SB, Acc, [UB, SB]]); + rpc(mim(), mongoose_hooks, remove_user, [SB, Acc, UB]); _ -> ok end diff --git a/big_tests/tests/oauth_SUITE.erl b/big_tests/tests/oauth_SUITE.erl index 8b854959bc8..3ca0ce52d6d 100644 --- a/big_tests/tests/oauth_SUITE.erl +++ b/big_tests/tests/oauth_SUITE.erl @@ -422,8 +422,8 @@ extract_bound_jid(BindReply) -> cdata]). get_provision_key(Domain) -> - RPCArgs = [get_key, Domain, [], [{provision_pre_shared, Domain}]], - [{_, RawKey}] = rpc(mim(), ejabberd_hooks, run_fold, RPCArgs), + RPCArgs = [Domain, [], provision_pre_shared], + [{_, RawKey}] = rpc(mim(), mongoose_hooks, get_key, RPCArgs), RawKey. make_vcard(Config, User) -> diff --git a/src/metrics/mongoose_metrics.erl b/src/metrics/mongoose_metrics.erl index 0e92a5a6e6f..09ed636f3ea 100644 --- a/src/metrics/mongoose_metrics.erl +++ b/src/metrics/mongoose_metrics.erl @@ -394,10 +394,10 @@ do_create_metric(PrefixedMetric, ExometerType, ExometerOpts) -> end. -spec metrics_hooks('add' | 'delete', jid:server()) -> 'ok'. -metrics_hooks(Op, Host) -> - lists:foreach(fun(Hook) -> - apply(ejabberd_hooks, Op, Hook) - end, mongoose_metrics_hooks:get_hooks(Host)). +metrics_hooks(add, Host) -> + ejabberd_hooks:add(mongoose_metrics_hooks:get_hooks(Host)); +metrics_hooks(delete, Host) -> + ejabberd_hooks:delete(mongoose_metrics_hooks:get_hooks(Host)). create_data_metrics() -> lists:foreach(fun(Metric) -> ensure_metric(global, Metric, histogram) end, diff --git a/src/metrics/mongoose_metrics_hooks.erl b/src/metrics/mongoose_metrics_hooks.erl index 9cfac70dc00..f7e66a1ab30 100644 --- a/src/metrics/mongoose_metrics_hooks.erl +++ b/src/metrics/mongoose_metrics_hooks.erl @@ -36,7 +36,7 @@ privacy_list_push/5 ]). --type hook() :: [atom() | jid:server() | integer(), ...]. +-type hook() :: {HookName::atom(), HostType :: binary(), Module::atom , integer(), ...]. -type t() :: hook(). -type metrics_notify_return() :: map() @@ -49,27 +49,27 @@ %%------------------- %% @doc Here will be declared which hooks should be registered --spec get_hooks(_) -> [hook(), ...]. +-spec get_hooks(_) -> [ejabberd_hooks:hook(), ...]. get_hooks(Host) -> - [[sm_register_connection_hook, Host, ?MODULE, sm_register_connection_hook, 50], - [sm_remove_connection_hook, Host, ?MODULE, sm_remove_connection_hook, 50], - [auth_failed, Host, ?MODULE, auth_failed, 50], - [user_send_packet, Host, ?MODULE, user_send_packet, 50], - [user_receive_packet, Host, ?MODULE, user_receive_packet, 50], - [xmpp_stanza_dropped, Host, ?MODULE, xmpp_stanza_dropped, 50], - [xmpp_bounce_message, Host, ?MODULE, xmpp_bounce_message, 50], - [xmpp_send_element, Host, ?MODULE, xmpp_send_element, 50], - [roster_get, Host, ?MODULE, roster_get, 55], - [roster_set, Host, ?MODULE, roster_set, 50], - [roster_push, Host, ?MODULE, roster_push, 50], - [roster_in_subscription, Host, ?MODULE, roster_in_subscription, 55], - [register_user, Host, ?MODULE, register_user, 50], - [remove_user, Host, ?MODULE, remove_user, 50], - [privacy_iq_get, Host, ?MODULE, privacy_iq_get, 1], - [privacy_iq_set, Host, ?MODULE, privacy_iq_set, 1], - [privacy_check_packet, Host, ?MODULE, privacy_check_packet, 55], - [sm_broadcast, Host, ?MODULE, privacy_list_push, 1] - | mongoose_metrics_mam_hooks:get_hooks(Host)]. + [ {sm_register_connection_hook, Host, ?MODULE, sm_register_connection_hook, 50}, + {sm_remove_connection_hook, Host, ?MODULE, sm_remove_connection_hook, 50}, + {auth_failed, Host, ?MODULE, auth_failed, 50}, + {user_send_packet, Host, ?MODULE, user_send_packet, 50}, + {user_receive_packet, Host, ?MODULE, user_receive_packet, 50}, + {xmpp_stanza_dropped, Host, ?MODULE, xmpp_stanza_dropped, 50}, + {xmpp_bounce_message, Host, ?MODULE, xmpp_bounce_message, 50}, + {xmpp_send_element, Host, ?MODULE, xmpp_send_element, 50}, + {roster_get, Host, ?MODULE, roster_get, 55}, + {roster_set, Host, ?MODULE, roster_set, 50}, + {roster_push, Host, ?MODULE, roster_push, 50}, + {roster_in_subscription, Host, ?MODULE, roster_in_subscription, 55}, + {register_user, Host, ?MODULE, register_user, 50}, + {remove_user, Host, ?MODULE, remove_user, 50}, + {privacy_iq_get, Host, ?MODULE, privacy_iq_get, 1}, + {privacy_iq_set, Host, ?MODULE, privacy_iq_set, 1}, + {privacy_check_packet, Host, ?MODULE, privacy_check_packet, 55}, + {sm_broadcast, Host, ?MODULE, privacy_list_push, 1} + | mongoose_metrics_mam_hooks:get_hooks(Host)]. -spec sm_register_connection_hook(map(), tuple(), jid:jid(), term() ) -> metrics_notify_return(). diff --git a/src/metrics/mongoose_metrics_mam_hooks.erl b/src/metrics/mongoose_metrics_mam_hooks.erl index 3c66c5dae76..b85ce0754f9 100644 --- a/src/metrics/mongoose_metrics_mam_hooks.erl +++ b/src/metrics/mongoose_metrics_mam_hooks.erl @@ -34,20 +34,20 @@ %%------------------- %% @doc Here will be declared which hooks should be registered --spec get_hooks(_) -> [mongoose_metrics_hooks:t(), ...]. +-spec get_hooks(_) -> [ejabberd_hooks:hook(), ...]. get_hooks(Host) -> [ - [mam_set_prefs, Host, ?MODULE, mam_set_prefs, 50], - [mam_get_prefs, Host, ?MODULE, mam_get_prefs, 50], - [mam_archive_message, Host, ?MODULE, mam_archive_message, 50], - [mam_remove_archive, Host, ?MODULE, mam_remove_archive, 50], - [mam_lookup_messages, Host, ?MODULE, mam_lookup_messages, 100], - [mam_muc_set_prefs, Host, ?MODULE, mam_muc_set_prefs, 50], - [mam_muc_get_prefs, Host, ?MODULE, mam_muc_get_prefs, 50], - [mam_muc_archive_message, Host, ?MODULE, mam_muc_archive_message, 50], - [mam_muc_remove_archive, Host, ?MODULE, mam_muc_remove_archive, 50], - [mam_muc_lookup_messages, Host, ?MODULE, mam_muc_lookup_messages, 100], - [mam_muc_flush_messages, Host, ?MODULE, mam_muc_flush_messages, 50] + {mam_set_prefs, Host, ?MODULE, mam_set_prefs, 50}, + {mam_get_prefs, Host, ?MODULE, mam_get_prefs, 50}, + {mam_archive_message, Host, ?MODULE, mam_archive_message, 50}, + {mam_remove_archive, Host, ?MODULE, mam_remove_archive, 50}, + {mam_lookup_messages, Host, ?MODULE, mam_lookup_messages, 100}, + {mam_muc_set_prefs, Host, ?MODULE, mam_muc_set_prefs, 50}, + {mam_muc_get_prefs, Host, ?MODULE, mam_muc_get_prefs, 50}, + {mam_muc_archive_message, Host, ?MODULE, mam_muc_archive_message, 50}, + {mam_muc_remove_archive, Host, ?MODULE, mam_muc_remove_archive, 50}, + {mam_muc_lookup_messages, Host, ?MODULE, mam_muc_lookup_messages, 100}, + {mam_muc_flush_messages, Host, ?MODULE, mam_muc_flush_messages, 50} ]. -spec mam_get_prefs(Result :: any(), diff --git a/src/mod_adhoc.erl b/src/mod_adhoc.erl index 5b08b71b5f4..b20c3593c64 100644 --- a/src/mod_adhoc.erl +++ b/src/mod_adhoc.erl @@ -28,6 +28,9 @@ -behaviour(gen_mod). -behaviour(mongoose_module_metrics). +-type command_hook_acc() :: {error, exml:element()} | exml:element() | ignore | empty. +-export_type([command_hook_acc/0]). + -export([start/2, stop/1, config_spec/0, @@ -224,13 +227,11 @@ get_sm_features(Acc, _From, _To, _Node, _Lang) -> process_local_iq(From, To, Acc, IQ) -> {Acc, process_adhoc_request(From, To, IQ, adhoc_local_commands)}. - -spec process_sm_iq(jid:jid(), jid:jid(), mongoose_acc:t(), jlib:iq()) -> {mongoose_acc:t(), ignore | jlib:iq()}. process_sm_iq(From, To, Acc, IQ) -> {Acc, process_adhoc_request(From, To, IQ, adhoc_sm_commands)}. - -spec process_adhoc_request(jid:jid(), jid:jid(), jlib:iq(), Hook :: atom()) -> ignore | jlib:iq(). process_adhoc_request(From, To, #iq{sub_el = SubEl} = IQ, Hook) -> @@ -240,8 +241,7 @@ process_adhoc_request(From, To, #iq{sub_el = SubEl} = IQ, Hook) -> IQ#iq{type = error, sub_el = [SubEl, Error]}; #adhoc_request{} = AdhocRequest -> Host = To#jid.lserver, - case ejabberd_hooks:run_fold(Hook, Host, empty, - [From, To, AdhocRequest]) of + case run_request_hook(Hook, Host, From, To, AdhocRequest) of ignore -> ignore; empty -> @@ -253,6 +253,10 @@ process_adhoc_request(From, To, #iq{sub_el = SubEl} = IQ, Hook) -> end end. +run_request_hook(adhoc_local_commands, Host, From, To, AdhocRequest) -> + mongoose_hooks:adhoc_local_commands(Host, From, To, AdhocRequest); +run_request_hook(adhoc_sm_commands, Host, From, To, AdhocRequest) -> + mongoose_hooks:adhoc_sm_commands(Host, From, To, AdhocRequest). -spec ping_item(Acc :: {result, [exml:element()]}, From :: jid:jid(), @@ -272,11 +276,11 @@ ping_item(Acc, _From, #jid{lserver = Server} = _To, Lang) -> {result, Items ++ Nodes}. --spec ping_command(Acc :: _, +-spec ping_command(Acc :: command_hook_acc(), From :: jid:jid(), To :: jid:jid(), - adhoc:request()) -> {error, _} | adhoc:response(). -ping_command(_Acc, _From, _To, + adhoc:request()) -> command_hook_acc(). +ping_command(empty, _From, _To, #adhoc_request{lang = Lang, node = <<"ping">> = Node, session_id = SessionID, diff --git a/src/mod_amp.erl b/src/mod_amp.erl index c6f52b9c308..90d1d882652 100644 --- a/src/mod_amp.erl +++ b/src/mod_amp.erl @@ -134,7 +134,7 @@ process_rules(Packet, From, Event, Rules) -> PacketResult = take_action(Packet, From, RulesWithResults), return_result(PacketResult, Event, RulesWithResults). -%% @doc ejabberd_hooks helpers +%% @doc hooks helpers -spec verify_support(binary(), amp_rules()) -> [amp_rule_support()]. verify_support(Host, Rules) -> mongoose_hooks:amp_verify_support(Host, [], Rules). diff --git a/src/mod_stream_management.erl b/src/mod_stream_management.erl index 50ffda4b078..ed2e9773337 100644 --- a/src/mod_stream_management.erl +++ b/src/mod_stream_management.erl @@ -9,7 +9,7 @@ config_spec/0, process_buffer_and_ack/1]). -%% `ejabberd_hooks' handlers +%% hooks handlers -export([add_sm_feature/2, remove_smid/5, session_cleanup/5]). @@ -120,7 +120,7 @@ stale_h_config_spec() -> }. %% -%% `ejabberd_hooks' handlers +%% hooks handlers %% add_sm_feature(Acc, _Server) -> diff --git a/src/mongoose_cleaner.erl b/src/mongoose_cleaner.erl index 94449e98291..c5dd59249dd 100644 --- a/src/mongoose_cleaner.erl +++ b/src/mongoose_cleaner.erl @@ -85,7 +85,7 @@ cleanup_modules(Node) -> end. run_node_cleanup(Node) -> - {Elapsed, RetVal} = timer:tc(ejabberd_hooks, run_fold, [node_cleanup, #{}, [Node]]), + {Elapsed, RetVal} = timer:tc(mongoose_hooks, node_cleanup, [Node]), ?LOG_NOTICE(#{what => cleaner_done, text => <<"Finished cleaning after dead node">>, duration => erlang:round(Elapsed / 1000), diff --git a/src/mongoose_hooks.erl b/src/mongoose_hooks.erl index 23129de9da5..a4d114dc51d 100644 --- a/src/mongoose_hooks.erl +++ b/src/mongoose_hooks.erl @@ -6,7 +6,9 @@ -module(mongoose_hooks). -export([adhoc_local_items/5, + adhock_local_commands/4, adhoc_sm_items/5, + adhock_sm_commands/4, anonymous_purge_hook/3, auth_failed/2, ejabberd_ctl_process/2, @@ -152,7 +154,8 @@ -export([c2s_remote_hook/5]). -export([disable_domain/2, - remove_domain/2]). + remove_domain/2, + node_cleanup/1]). -spec c2s_remote_hook(LServer, Tag, Args, HandlerState, C2SState) -> Result when LServer :: jid:lserver(), @@ -174,6 +177,15 @@ c2s_remote_hook(LServer, Tag, Args, HandlerState, C2SState) -> adhoc_local_items(LServer, Acc, From, To, Lang) -> ejabberd_hooks:run_fold(adhoc_local_items, LServer, Acc, [From, To, Lang]). +-spec adhock_local_commands(LServer, From, To, AdhocRequest) -> Result when + LServer :: jid:lserver(), + From :: jid:jid(), + To :: jid:jid(), + AdhocRequest :: adhock:request(), + Result :: mod_adhoc:command_hook_acc(). +adhock_local_commands(LServer, From, To, AdhocRequest) -> + ejabberd_hooks:run_fold(adhock_local_commands, LServer, empty, [From, To, AdhocRequest]). + -spec adhoc_sm_items(LServer, Acc, From, To, Lang) -> Result when LServer :: jid:lserver(), Acc :: {result, [exml:element()]}, @@ -184,6 +196,15 @@ adhoc_local_items(LServer, Acc, From, To, Lang) -> adhoc_sm_items(LServer, Acc, From, To, Lang) -> ejabberd_hooks:run_fold(adhoc_sm_items, LServer, Acc, [From, To, Lang]). +-spec adhock_sm_commands(LServer, From, To, AdhocRequest) -> Result when + LServer :: jid:lserver(), + From :: jid:jid(), + To :: jid:jid(), + AdhocRequest :: adhock:request(), + Result :: ignore | empty | {error, exml:element()} | exml:element(). +adhock_sm_commands(LServer, From, To, AdhocRequest) -> + ejabberd_hooks:run_fold(adhock_sm_commands, LServer, empty, [From, To, AdhocRequest]). + %%% @doc The `anonymous_purge_hook' hook is called when anonymous user's data is removed. -spec anonymous_purge_hook(LServer, Acc, LUser) -> Result when LServer :: jid:lserver(), @@ -214,6 +235,10 @@ disable_domain(HostType, Domain) -> remove_domain(HostType, Domain) -> ejabberd_hooks:run(remove_domain, [HostType, Domain]). +-spec node_cleanup(Node :: node()) -> Acc :: map(). +node_cleanup(Node) -> + ejabberd_hooks:run_fold(node_cleanup, #{}, [Node]). + -spec ejabberd_ctl_process(Acc, Args) -> Result when Acc :: any(), Args :: [string()], diff --git a/test/keystore_SUITE.erl b/test/keystore_SUITE.erl index b5c8fa0e617..fc135860f7f 100644 --- a/test/keystore_SUITE.erl +++ b/test/keystore_SUITE.erl @@ -182,7 +182,7 @@ mock_mongoose_metrics() -> KeyName :: mod_keystore:key_name(), Result :: mod_keystore:key_list(). get_key(Domain, KeyName) -> - ejabberd_hooks:run_fold(get_key, Domain, [], [{KeyName, Domain}]). + mongoose_hooks:get_key(Domain,[],KeyName). %%{mod_keystore, [{keys, [{asdqwe_access_secret, ram}, %% {asdqwe_access_psk, {file, "priv/asdqwe_access_psk"}}, diff --git a/test/mongoose_cleanup_SUITE.erl b/test/mongoose_cleanup_SUITE.erl index 8325ec914c0..67dae7bbb2f 100644 --- a/test/mongoose_cleanup_SUITE.erl +++ b/test/mongoose_cleanup_SUITE.erl @@ -66,7 +66,7 @@ cleaner_runs_hook_on_nodedown(_Config) -> meck:expect(ejabberd_hooks, error_running_hook, fun(_, _, _) -> ok end), {ok, Cleaner} = mongoose_cleaner:start_link(), Self = self(), - NotifySelf = fun (Acc, Node) -> Self ! {got_nodedown, Node}, Acc end, + NotifySelf = fun(Acc, Node) -> Self ! {got_nodedown, Node}, Acc end, ejabberd_hooks:add(node_cleanup, global, undefined, NotifySelf, 50), FakeNode = fakename@fakehost, @@ -77,35 +77,36 @@ cleaner_runs_hook_on_nodedown(_Config) -> after timer:seconds(1) -> ct:fail({timeout, got_nodedown}) end, - ?assertEqual(false, meck:called(ejabberd_hooks, error_running_hook, ['_','_','_'])). + ?assertEqual(false, meck:called(ejabberd_hooks, error_running_hook, + ['_', {node_cleanup, global}, '_'])). auth_anonymous(_Config) -> - ejabberd_auth_anonymous:start(?HOST), {U, S, R, JID, SID} = get_fake_session(), + ejabberd_auth_anonymous:start(S), Info = [{auth_module, ejabberd_auth_anonymous}], ejabberd_auth_anonymous:register_connection(#{}, SID, JID, Info), true = ejabberd_auth_anonymous:anonymous_user_exist(U, S), - ejabberd_hooks:run(session_cleanup, ?HOST, [U, S, R, SID]), + mongoose_hooks:session_cleanup(S, ok, U, R, SID), false = ejabberd_auth_anonymous:anonymous_user_exist(U, S). last(_Config) -> - mod_last:start(?HOST, [{backend, mnesia}, {iqdisc, no_queue}]), {U, S, R, _JID, SID} = get_fake_session(), + mod_last:start(S, [{backend, mnesia}, {iqdisc, no_queue}]), not_found = mod_last:get_last_info(U, S), Status1 = <<"status1">>, #{} = mod_last:on_presence_update(#{}, U, S, R, Status1), {ok, TS1, Status1} = mod_last:get_last_info(U, S), async_helper:wait_until( fun() -> - ejabberd_hooks:run(session_cleanup, ?HOST, [U, S, R, SID]), + mongoose_hooks:session_cleanup(S, ok, U, R, SID), {ok, TS2, <<>>} = mod_last:get_last_info(U, S), TS2 - TS1 > 0 end, true). stream_management(_Config) -> - mod_stream_management:start(?HOST, []), {U, S, R, _JID, SID} = get_fake_session(), + mod_stream_management:start(S, []), SMID = <<"123">>, mod_stream_management:register_smid(SMID, SID), {sid, SID} = mod_stream_management:get_sid(SMID), @@ -113,7 +114,7 @@ stream_management(_Config) -> #{location => ?LOCATION, lserver => S, element => undefined}), - ejabberd_hooks:run_fold(session_cleanup, ?HOST, Acc, [U, S, R, SID]), + mongoose_hooks:session_cleanup(S, Acc, U, R, SID), {error, smid_not_found} = mod_stream_management:get_sid(SMID). local(_Config) -> @@ -131,7 +132,7 @@ local(_Config) -> ejabberd_local:register_iq_response_handler(?HOST, ID, undefined, SelfNotify, 2000), {ok, undefined, _F} = ejabberd_local:get_iq_callback(ID), - ejabberd_hooks:run(node_cleanup, [node()]), + mongoose_hooks:node_cleanup(node()), error = ejabberd_local:get_iq_callback(ID). s2s(_Config) -> @@ -140,7 +141,7 @@ s2s(_Config) -> ejabberd_s2s:try_register(FromTo), Self = self(), [Self] = ejabberd_s2s:get_connections_pids(FromTo), - ejabberd_hooks:run(node_cleanup, [node()]), + mongoose_hooks:node_cleanup(node()), [] = ejabberd_s2s:get_connections_pids(FromTo). bosh(_Config) -> @@ -150,7 +151,7 @@ bosh(_Config) -> {error, _} = mod_bosh:get_session_socket(SID), mod_bosh:store_session(SID, Self), {ok, Self} = mod_bosh:get_session_socket(SID), - ejabberd_hooks:run(node_cleanup, [node()]), + mongoose_hooks:node_cleanup(node()), {error, _} = mod_bosh:get_session_socket(SID), ok.