Merge pull request #3763 from esl/hooks/ejabberd_sm

Refactored hook handlers in ejabberd_sm module

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 :: #{jid := jid:jid()},
+      Extra :: map().
+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}.
 
 %%====================================================================
 %% 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),

src/mongoose_hooks.erl

@@ -368,8 +368,12 @@ register_user(HostType, LServer, LUser) ->
     LUser :: jid:luser(),
     Result :: mongoose_acc:t().
 remove_user(Acc, LServer, LUser) ->
+    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),
-    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 +694,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 +707,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 +850,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.

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),