From efcb40d4f15b7b99cc6cb8125ae272c30ed92344 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20D=C5=82ugosz?= Date: Fri, 16 Sep 2022 10:51:26 +0200 Subject: [PATCH 1/2] Refactored hook handlers in ejabberd_sm module --- src/ejabberd_sm.erl | 104 +++++++++++++++++++------------------ src/mongoose_hooks.erl | 23 +++++--- test/ejabberd_sm_SUITE.erl | 1 + 3 files changed, 71 insertions(+), 57 deletions(-) diff --git a/src/ejabberd_sm.erl b/src/ejabberd_sm.erl index a27087d8c3d..f29cb910090 100644 --- a/src/ejabberd_sm.erl +++ b/src/ejabberd_sm.erl @@ -40,9 +40,6 @@ store_info/3, get_info/2, remove_info/2, - check_in_subscription/5, - bounce_offline_message/4, - disconnect_removed_user/3, get_user_resources/1, set_presence/6, unset_presence/5, @@ -70,7 +67,11 @@ ]). %% Hook handlers --export([node_cleanup/2]). +-export([node_cleanup/3, + check_in_subscription/3, + bounce_offline_message/3, + disconnect_removed_user/3 + ]). %% gen_server callbacks -export([init/1, handle_call/3, handle_cast/2, handle_info/2, @@ -80,10 +81,8 @@ -export([do_filter/3]). -export([do_route/4]). --ignore_xref([bounce_offline_message/4, check_in_subscription/5, disconnect_removed_user/3, - do_filter/3, do_route/4, force_update_presence/2, get_unique_sessions_number/0, - get_user_present_pids/2, node_cleanup/2, start_link/0, user_resources/2, - sm_backend/0]). +-ignore_xref([do_filter/3, do_route/4, force_update_presence/2, get_unique_sessions_number/0, + get_user_present_pids/2, start_link/0, user_resources/2, sm_backend/0]). -include("mongoose.hrl"). -include("jlib.hrl"). @@ -294,40 +293,6 @@ remove_info(JID, Key) -> end end. --spec check_in_subscription(Acc, ToJID, FromJID, Type, Reason) -> any() | {stop, false} when - Acc :: any(), - ToJID :: jid:jid(), - FromJID :: jid:jid(), - Type :: any(), - Reason :: any(). -check_in_subscription(Acc, ToJID, _FromJID, _Type, _Reason) -> - case ejabberd_auth:does_user_exist(ToJID) of - true -> - Acc; - false -> - {stop, mongoose_acc:set(hook, result, false, Acc)} - end. - --spec bounce_offline_message(Acc, From, To, Packet) -> {stop, Acc} when - Acc :: map(), - From :: jid:jid(), - To :: jid:jid(), - Packet :: exml:element(). -bounce_offline_message(Acc, From, To, Packet) -> - Acc1 = mongoose_hooks:xmpp_bounce_message(Acc), - E = mongoose_xmpp_errors:service_unavailable(<<"en">>, <<"Bounce offline message">>), - {Acc2, Err} = jlib:make_error_reply(Acc1, Packet, E), - Acc3 = ejabberd_router:route(To, From, Acc2, Err), - {stop, Acc3}. - --spec disconnect_removed_user(mongoose_acc:t(), User :: jid:user(), - Server :: jid:server()) -> mongoose_acc:t(). -disconnect_removed_user(Acc, User, Server) -> - lists:map(fun({_, Pid}) -> ejabberd_c2s:terminate_session(Pid, <<"User removed">>) end, - get_user_present_pids(User, Server)), - Acc. - - -spec get_user_resources(JID :: jid:jid()) -> [binary()]. get_user_resources(JID) -> #jid{luser = LUser, lserver = LServer} = JID, @@ -473,10 +438,46 @@ run_session_cleanup_hook(#session{usr = {U, S, R}, sid = SID}) -> %% Hook handlers %%==================================================================== -node_cleanup(Acc, Node) -> +-spec node_cleanup(Acc, Args, Extra) -> {ok, Acc} when + Acc :: any(), + Args :: #{node := node()}, + Extra :: map(). +node_cleanup(Acc, #{node := Node}, _) -> Timeout = timer:minutes(1), Res = gen_server:call(?MODULE, {node_cleanup, Node}, Timeout), - maps:put(?MODULE, Res, Acc). + {ok, maps:put(?MODULE, Res, Acc)}. + +-spec check_in_subscription(Acc, Args, Extra)-> {ok, Acc} | {stop, false} when + Acc :: any(), + Args :: #{to_jid := jid:jid()}, + Extra :: map(). +check_in_subscription(Acc, #{to_jid := ToJID}, _) -> + case ejabberd_auth:does_user_exist(ToJID) of + true -> + {ok, Acc}; + false -> + {stop, mongoose_acc:set(hook, result, false, Acc)} + end. + +-spec bounce_offline_message(Acc, Args, Extra) -> {stop, Acc} when + Acc :: map(), + Args :: #{from := jid:jid(), to := jid:jid(), packet := exml:element()}, + Extra :: map(). +bounce_offline_message(Acc, #{from := From, to := To, packet := Packet}, _) -> + Acc1 = mongoose_hooks:xmpp_bounce_message(Acc), + E = mongoose_xmpp_errors:service_unavailable(<<"en">>, <<"Bounce offline message">>), + {Acc2, Err} = jlib:make_error_reply(Acc1, Packet, E), + Acc3 = ejabberd_router:route(To, From, Acc2, Err), + {stop, Acc3}. + +-spec disconnect_removed_user(Acc, Args, Extra) -> {ok, Acc} when + Acc :: mongoose_acc:t(), + Args :: #{user := jid:user(), server := jid:server()}, + Extra :: map(). +disconnect_removed_user(Acc, #{user := User, server := Server}, _) -> + lists:map(fun({_, Pid}) -> ejabberd_c2s:terminate_session(Pid, <<"User removed">>) end, + get_user_present_pids(User, Server)), + {ok, Acc}. %%==================================================================== %% gen_server callbacks @@ -495,18 +496,19 @@ init([]) -> ejabberd_sm_backend:init(#{backend => Backend}), ets:new(sm_iqtable, [named_table, protected, {read_concurrency, true}]), - ejabberd_hooks:add(node_cleanup, global, ?MODULE, node_cleanup, 50), - lists:foreach(fun(HostType) -> ejabberd_hooks:add(hooks(HostType)) end, + gen_hook:add_handler(node_cleanup, global, fun ?MODULE:node_cleanup/3, #{}, 50), + lists:foreach(fun(HostType) -> gen_hook:add_handlers(hooks(HostType)) end, ?ALL_HOST_TYPES), ejabberd_commands:register_commands(commands()), {ok, #state{}}. +-spec hooks(binary()) -> [gen_hook:hook_tuple()]. hooks(HostType) -> [ - {roster_in_subscription, HostType, ejabberd_sm, check_in_subscription, 20}, - {offline_message_hook, HostType, ejabberd_sm, bounce_offline_message, 100}, - {offline_groupchat_message_hook, HostType, ejabberd_sm, bounce_offline_message, 100}, - {remove_user, HostType, ejabberd_sm, disconnect_removed_user, 100} + {roster_in_subscription, HostType, fun ?MODULE:check_in_subscription/3, #{}, 20}, + {offline_message_hook, HostType, fun ?MODULE:bounce_offline_message/3, #{}, 100}, + {offline_groupchat_message_hook, HostType, fun ?MODULE:bounce_offline_message/3, #{}, 100}, + {remove_user, HostType, fun ?MODULE:disconnect_removed_user/3, #{}, 100} ]. %%-------------------------------------------------------------------- @@ -835,7 +837,7 @@ route_message_by_type(<<"error">>, _From, _To, Acc, _Packet) -> route_message_by_type(<<"groupchat">>, From, To, Acc, Packet) -> mongoose_hooks:offline_groupchat_message_hook(Acc, From, To, Packet); route_message_by_type(<<"headline">>, From, To, Acc, Packet) -> - {stop, Acc1} = bounce_offline_message(Acc, From, To, Packet), + {stop, Acc1} = bounce_offline_message(Acc, #{from => From, to => To, packet => Packet}, #{}), Acc1; route_message_by_type(_, From, To, Acc, Packet) -> HostType = mongoose_acc:host_type(Acc), diff --git a/src/mongoose_hooks.erl b/src/mongoose_hooks.erl index 959588183ec..e0e903ac52d 100644 --- a/src/mongoose_hooks.erl +++ b/src/mongoose_hooks.erl @@ -368,8 +368,11 @@ register_user(HostType, LServer, LUser) -> LUser :: jid:luser(), Result :: mongoose_acc:t(). remove_user(Acc, LServer, LUser) -> + Params = #{user => LUser, server => LServer}, + Args = [LUser, LServer], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), HostType = mongoose_acc:host_type(Acc), - run_hook_for_host_type(remove_user, HostType, Acc, [LUser, LServer]). + run_hook_for_host_type(remove_user, HostType, Acc, ParamsWithLegacyArgs). -spec resend_offline_messages_hook(Acc, JID) -> Result when Acc :: mongoose_acc:t(), @@ -690,9 +693,11 @@ privacy_updated_list(HostType, OldList, NewList) -> Packet :: exml:element(), Result :: mongoose_acc:t(). offline_groupchat_message_hook(Acc, From, To, Packet) -> + Params = #{from => From, to => To, packet => Packet}, + Args = [From, To, Packet], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), HostType = mongoose_acc:host_type(Acc), - run_hook_for_host_type(offline_groupchat_message_hook, HostType, Acc, - [From, To, Packet]). + run_hook_for_host_type(offline_groupchat_message_hook, HostType, Acc, ParamsWithLegacyArgs). -spec offline_message_hook(Acc, From, To, Packet) -> Result when Acc :: mongoose_acc:t(), @@ -701,8 +706,11 @@ offline_groupchat_message_hook(Acc, From, To, Packet) -> Packet :: exml:element(), Result :: mongoose_acc:t(). offline_message_hook(Acc, From, To, Packet) -> + Params = #{from => From, to => To, packet => Packet}, + Args = [From, To, Packet], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), HostType = mongoose_acc:host_type(Acc), - run_hook_for_host_type(offline_message_hook, HostType, Acc, [From, To, Packet]). + run_hook_for_host_type(offline_message_hook, HostType, Acc, ParamsWithLegacyArgs). -spec set_presence_hook(Acc, JID, Presence) -> Result when Acc :: mongoose_acc:t(), @@ -841,9 +849,12 @@ roster_get_versioning_feature(HostType) -> Reason :: any(), Result :: mongoose_acc:t(). roster_in_subscription(Acc, To, From, Type, Reason) -> + ToJID = jid:to_bare(To), + Params = #{to_jid => ToJID, from => From, type => Type, reason => Reason}, + Args = [ToJID, From, Type, Reason], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), HostType = mongoose_acc:host_type(Acc), - run_hook_for_host_type(roster_in_subscription, HostType, Acc, - [jid:to_bare(To), From, Type, Reason]). + run_hook_for_host_type(roster_in_subscription, HostType, Acc, ParamsWithLegacyArgs). %%% @doc The `roster_out_subscription' hook is called %%% when a user sends out subscription. diff --git a/test/ejabberd_sm_SUITE.erl b/test/ejabberd_sm_SUITE.erl index fd09010654d..aa85adcfa87 100644 --- a/test/ejabberd_sm_SUITE.erl +++ b/test/ejabberd_sm_SUITE.erl @@ -608,6 +608,7 @@ sm_backend(ejabberd_sm_mnesia) -> mnesia. set_meck() -> meck:expect(gen_hook, add_handler, fun(_, _, _, _, _) -> ok end), + meck:expect(gen_hook, add_handlers, fun(_) -> ok end), meck:new(ejabberd_commands, []), meck:expect(ejabberd_commands, register_commands, fun(_) -> ok end), meck:expect(ejabberd_commands, unregister_commands, fun(_) -> ok end), From 0ba7a50a106e63a42468e71e8b145d2bfc7c9a2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20D=C5=82ugosz?= Date: Tue, 20 Sep 2022 14:35:18 +0200 Subject: [PATCH 2/2] Changed params in remove_user hook from luser, lserver to jid --- src/ejabberd_sm.erl | 4 ++-- src/mongoose_hooks.erl | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ejabberd_sm.erl b/src/ejabberd_sm.erl index f29cb910090..3a3f92a27ea 100644 --- a/src/ejabberd_sm.erl +++ b/src/ejabberd_sm.erl @@ -472,9 +472,9 @@ bounce_offline_message(Acc, #{from := From, to := To, packet := Packet}, _) -> -spec disconnect_removed_user(Acc, Args, Extra) -> {ok, Acc} when Acc :: mongoose_acc:t(), - Args :: #{user := jid:user(), server := jid:server()}, + Args :: #{jid := jid:jid()}, Extra :: map(). -disconnect_removed_user(Acc, #{user := User, server := Server}, _) -> +disconnect_removed_user(Acc, #{jid := #jid{luser = User, lserver = Server}}, _) -> lists:map(fun({_, Pid}) -> ejabberd_c2s:terminate_session(Pid, <<"User removed">>) end, get_user_present_pids(User, Server)), {ok, Acc}. diff --git a/src/mongoose_hooks.erl b/src/mongoose_hooks.erl index e0e903ac52d..4c876d4aba5 100644 --- a/src/mongoose_hooks.erl +++ b/src/mongoose_hooks.erl @@ -368,7 +368,8 @@ register_user(HostType, LServer, LUser) -> LUser :: jid:luser(), Result :: mongoose_acc:t(). remove_user(Acc, LServer, LUser) -> - Params = #{user => LUser, server => LServer}, + Jid = jid:make_bare(LUser, LServer), + Params = #{jid => Jid}, Args = [LUser, LServer], ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), HostType = mongoose_acc:host_type(Acc),