Skip to content

Commit

Permalink
ensuring that all the calls to ejabberd_hooks run/run_fold functions …
Browse files Browse the repository at this point in the history
…are wrapped in mongoose_hooks module.

hint: all inderect calls to ejabberd_hooks found using 'ejabberd_hooks[^:]' search regex
  • Loading branch information
DenysGonchar committed Apr 18, 2021
1 parent b3541f9 commit 49fe2cd
Show file tree
Hide file tree
Showing 12 changed files with 94 additions and 64 deletions.
2 changes: 1 addition & 1 deletion big_tests/tests/ejabberdctl_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions big_tests/tests/oauth_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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) ->
Expand Down
8 changes: 4 additions & 4 deletions src/metrics/mongoose_metrics.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
42 changes: 21 additions & 21 deletions src/metrics/mongoose_metrics_hooks.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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().
Expand Down
24 changes: 12 additions & 12 deletions src/metrics/mongoose_metrics_mam_hooks.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
18 changes: 11 additions & 7 deletions src/mod_adhoc.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) ->
Expand All @@ -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 ->
Expand All @@ -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(),
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/mod_amp.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
4 changes: 2 additions & 2 deletions src/mod_stream_management.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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]).
Expand Down Expand Up @@ -120,7 +120,7 @@ stale_h_config_spec() ->
}.

%%
%% `ejabberd_hooks' handlers
%% hooks handlers
%%

add_sm_feature(Acc, _Server) ->
Expand Down
2 changes: 1 addition & 1 deletion src/mongoose_cleaner.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
27 changes: 26 additions & 1 deletion src/mongoose_hooks.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand All @@ -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()]},
Expand All @@ -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(),
Expand Down Expand Up @@ -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()],
Expand Down
2 changes: 1 addition & 1 deletion test/keystore_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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"}},
Expand Down
23 changes: 12 additions & 11 deletions test/mongoose_cleanup_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -77,43 +77,44 @@ 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),
Acc = mongoose_acc:new(
#{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) ->
Expand All @@ -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) ->
Expand All @@ -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) ->
Expand All @@ -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.

Expand Down

0 comments on commit 49fe2cd

Please sign in to comment.