Merge pull request #3758 from esl/hooks/disco_local_features

Refactored disco_local_features hook to a gen_hook format

src/ejabberd_local.erl

@@ -54,15 +54,15 @@
 
 %% Hooks callbacks
 
--export([disco_local_features/1]).
+-export([disco_local_features/3]).
 
 %% gen_server callbacks
 -export([init/1, handle_call/3, handle_cast/2, handle_info/2,
          terminate/2, code_change/3]).
 
 -export([do_route/4]).
 
--ignore_xref([disco_local_features/1, do_route/4, get_iq_callback/1,
+-ignore_xref([do_route/4, get_iq_callback/1,
               process_iq_reply/4, start_link/0]).
 
 -include("mongoose.hrl").
@@ -241,12 +241,14 @@ register_host(Host) ->
 unregister_host(Host) ->
     gen_server:call(?MODULE, {unregister_host, Host}).
 
--spec disco_local_features(mongoose_disco:feature_acc()) -> mongoose_disco:feature_acc().
-disco_local_features(Acc = #{to_jid := #jid{lserver = LServer}, node := <<>>}) ->
+-spec disco_local_features(mongoose_disco:feature_acc(),
+                           map(),
+                           map()) -> {ok, mongoose_disco:feature_acc()}.
+disco_local_features(Acc = #{to_jid := #jid{lserver = LServer}, node := <<>>}, _, _) ->
     Features = [Feature || {_, Feature} <- ets:lookup(?NSTABLE, LServer)],
-    mongoose_disco:add_features(Features, Acc);
-disco_local_features(Acc) ->
-    Acc.
+    {ok, mongoose_disco:add_features(Features, Acc)};
+disco_local_features(Acc, _, _) ->
+    {ok, Acc}.
 
 %%====================================================================
 %% gen_server callbacks
@@ -263,7 +265,7 @@ init([]) ->
     catch ets:new(?IQTABLE, [named_table, protected, {read_concurrency, true}]),
     catch ets:new(?NSTABLE, [named_table, bag, protected, {read_concurrency, true}]),
     catch ets:new(?IQRESPONSE, [named_table, public]),
-    ejabberd_hooks:add(hooks()),
+    gen_hook:add_handlers(hooks()),
     {ok, #state{}}.
 
 %%--------------------------------------------------------------------
@@ -337,7 +339,7 @@ handle_info(_Info, State) ->
 %% The return value is ignored.
 %%--------------------------------------------------------------------
 terminate(_Reason, _State) ->
-    ejabberd_hooks:delete(hooks()).
+    gen_hook:delete_handlers(hooks()).
 
 %%--------------------------------------------------------------------
 %% Func: code_change(OldVsn, State, Extra) -> {ok, NewState}
@@ -351,7 +353,7 @@ code_change(_OldVsn, State, _Extra) ->
 %%--------------------------------------------------------------------
 
 hooks() ->
-    [{disco_local_features, HostType, ?MODULE, disco_local_features, 99}
+    [{disco_local_features, HostType, fun ?MODULE:disco_local_features/3, #{}, 99}
      || HostType <- ?ALL_HOST_TYPES].
 
 -spec do_route(Acc :: mongoose_acc:t(),

src/inbox/mod_inbox.erl

@@ -27,12 +27,12 @@
          inbox_unread_count/2,
          remove_user/3,
          remove_domain/3,
-         disco_local_features/1
+         disco_local_features/3
         ]).
 
 -ignore_xref([
-    disco_local_features/1, filter_local_packet/1, get_personal_data/3,
-    inbox_unread_count/2, remove_domain/3, remove_user/3, user_send_packet/4
+    filter_local_packet/1, get_personal_data/3, inbox_unread_count/2, 
+    remove_domain/3, remove_user/3, user_send_packet/4
 ]).
 
 -export([process_inbox_boxes/1]).
@@ -89,7 +89,8 @@ process_entry(#{remote_jid := RemJID,
 start(HostType, #{iqdisc := IQDisc, groupchat := MucTypes} = Opts) ->
     mod_inbox_backend:init(HostType, Opts),
     lists:member(muc, MucTypes) andalso mod_inbox_muc:start(HostType),
-    ejabberd_hooks:add(hooks(HostType)),
+    ejabberd_hooks:add(legacy_hooks(HostType)),
+    gen_hook:add_handlers(hooks(HostType)),
     gen_iq_handler:add_iq_handler_for_domain(HostType, ?NS_ESL_INBOX, ejabberd_sm,
                                              fun ?MODULE:process_iq/5, #{}, IQDisc),
     gen_iq_handler:add_iq_handler_for_domain(HostType, ?NS_ESL_INBOX_CONVERSATION, ejabberd_sm,
@@ -101,7 +102,8 @@ start(HostType, #{iqdisc := IQDisc, groupchat := MucTypes} = Opts) ->
 stop(HostType) ->
     gen_iq_handler:remove_iq_handler_for_domain(HostType, ?NS_ESL_INBOX, ejabberd_sm),
     gen_iq_handler:remove_iq_handler_for_domain(HostType, ?NS_ESL_INBOX_CONVERSATION, ejabberd_sm),
-    ejabberd_hooks:delete(hooks(HostType)),
+    ejabberd_hooks:delete(legacy_hooks(HostType)),
+    gen_hook:delete_handlers(hooks(HostType)),
     stop_cleaner(HostType),
     mod_inbox_muc:stop(HostType),
     case mongoose_config:get_opt([{modules, HostType}, ?MODULE, backend]) of
@@ -265,11 +267,13 @@ remove_domain(Acc, HostType, Domain) ->
     mod_inbox_backend:remove_domain(HostType, Domain),
     Acc.
 
--spec disco_local_features(mongoose_disco:feature_acc()) -> mongoose_disco:feature_acc().
-disco_local_features(Acc = #{node := <<>>}) ->
-    mongoose_disco:add_features([?NS_ESL_INBOX], Acc);
-disco_local_features(Acc) ->
-    Acc.
+-spec disco_local_features(mongoose_disco:feature_acc(),
+                           map(),
+                           map()) -> {ok, mongoose_disco:feature_acc()}.
+disco_local_features(Acc = #{node := <<>>}, _, _) ->
+    {ok, mongoose_disco:add_features([?NS_ESL_INBOX], Acc)};
+disco_local_features(Acc, _, _) ->
+    {ok, Acc}   .
 
 -spec maybe_process_message(Acc :: mongoose_acc:t(),
                             From :: jid:jid(),
@@ -546,17 +550,19 @@ get_inbox_unread(undefined, Acc, To) ->
     {ok, Count} = mod_inbox_backend:get_inbox_unread(HostType, InboxEntryKey),
     mongoose_acc:set(inbox, unread_count, Count, Acc).
 
-hooks(HostType) ->
+legacy_hooks(HostType) ->
     [
      {remove_user, HostType, ?MODULE, remove_user, 50},
      {remove_domain, HostType, ?MODULE, remove_domain, 50},
      {user_send_packet, HostType, ?MODULE, user_send_packet, 70},
      {filter_local_packet, HostType, ?MODULE, filter_local_packet, 90},
      {inbox_unread_count, HostType, ?MODULE, inbox_unread_count, 80},
-     {get_personal_data, HostType, ?MODULE, get_personal_data, 50},
-     {disco_local_features, HostType, ?MODULE, disco_local_features, 99}
+     {get_personal_data, HostType, ?MODULE, get_personal_data, 50}
     ].
 
+hooks(HostType) ->
+    [{disco_local_features, HostType, fun ?MODULE:disco_local_features/3, #{}, 99}].
+
 get_groupchat_types(HostType) ->
     gen_mod:get_module_opt(HostType, ?MODULE, groupchat).
 

src/mam/mod_mam_pm.erl

@@ -46,7 +46,7 @@
 -export([start/2, stop/1, supported_features/0]).
 
 %% ejabberd handlers
--export([disco_local_features/1,
+-export([disco_local_features/3,
          process_mam_iq/5,
          user_send_packet/4,
          remove_user/3,
@@ -64,9 +64,8 @@
 
 -ignore_xref([archive_message_from_ct/1,
               archive_size/2, archive_size_with_host_type/3, delete_archive/2,
-              determine_amp_strategy/5, disco_local_features/1, filter_packet/1,
-              get_personal_data/3, remove_user/3, sm_filter_offline_message/4,
-              user_send_packet/4]).
+              determine_amp_strategy/5, filter_packet/1, get_personal_data/3, 
+              remove_user/3, sm_filter_offline_message/4, user_send_packet/4]).
 
 -type host_type() :: mongooseim:host_type().
 
@@ -154,14 +153,16 @@ archive_id(Server, User)
 start(HostType, Opts) ->
     ?LOG_INFO(#{what => mam_starting, host_type => HostType}),
     ensure_metrics(HostType),
-    ejabberd_hooks:add(hooks(HostType)),
+    ejabberd_hooks:add(legacy_hooks(HostType)),
+    gen_hook:add_handlers(hooks(HostType)),
     add_iq_handlers(HostType, Opts),
     ok.
 
 -spec stop(host_type()) -> any().
 stop(HostType) ->
     ?LOG_INFO(#{what => mam_stopping, host_type => HostType}),
-    ejabberd_hooks:delete(hooks(HostType)),
+    ejabberd_hooks:delete(legacy_hooks(HostType)),
+    gen_hook:delete_handlers(hooks(HostType)),
     remove_iq_handlers(HostType),
     ok.
 
@@ -202,11 +203,13 @@ process_mam_iq(Acc, From, To, IQ, _Extra) ->
             {Acc, return_action_not_allowed_error_iq(IQ)}
     end.
 
--spec disco_local_features(mongoose_disco:feature_acc()) -> mongoose_disco:feature_acc().
-disco_local_features(Acc = #{host_type := HostType, node := <<>>}) ->
-    mongoose_disco:add_features(features(?MODULE, HostType), Acc);
-disco_local_features(Acc) ->
-    Acc.
+-spec disco_local_features(mongoose_disco:feature_acc(),
+                           map(),
+                           map()) -> {ok, mongoose_disco:feature_acc()}.
+disco_local_features(Acc = #{host_type := HostType, node := <<>>}, _, _) ->
+    {ok, mongoose_disco:add_features(features(?MODULE, HostType), Acc)};
+disco_local_features(Acc, _, _) ->
+    {ok, Acc}.
 
 %% @doc Handle an outgoing message.
 %%
@@ -679,10 +682,9 @@ is_archivable_message(HostType, Dir, Packet) ->
     ArchiveChatMarkers = mod_mam_params:archive_chat_markers(?MODULE, HostType),
     erlang:apply(M, is_archivable_message, [?MODULE, Dir, Packet, ArchiveChatMarkers]).
 
--spec hooks(jid:lserver()) -> [ejabberd_hooks:hook()].
-hooks(HostType) ->
-    [{disco_local_features, HostType, ?MODULE, disco_local_features, 99},
-     {user_send_packet, HostType, ?MODULE, user_send_packet, 60},
+-spec legacy_hooks(jid:lserver()) -> [ejabberd_hooks:hook()].
+legacy_hooks(HostType) ->
+    [{user_send_packet, HostType, ?MODULE, user_send_packet, 60},
      {rest_user_send_packet, HostType, ?MODULE, user_send_packet, 60},
      {filter_local_packet, HostType, ?MODULE, filter_packet, 60},
      {remove_user, HostType, ?MODULE, remove_user, 50},
@@ -692,6 +694,9 @@ hooks(HostType) ->
      {get_personal_data, HostType, ?MODULE, get_personal_data, 50}
      | mongoose_metrics_mam_hooks:get_mam_hooks(HostType)].
 
+hooks(HostType) ->
+    [{disco_local_features, HostType, fun ?MODULE:disco_local_features/3, #{}, 99}].
+
 add_iq_handlers(HostType, Opts) ->
     Component = ejabberd_sm,
     %% `parallel' is the only one recommended here.

src/mod_adhoc.erl

@@ -42,13 +42,13 @@
          process_sm_iq/5,
          disco_local_items/1,
          disco_local_identity/1,
-         disco_local_features/1,
+         disco_local_features/3,
          disco_sm_items/1,
          disco_sm_identity/1,
          disco_sm_features/1,
          ping_command/4]).
 
--ignore_xref([disco_local_features/1, disco_local_identity/1, disco_local_items/1,
+-ignore_xref([disco_local_identity/1, disco_local_items/1,
               disco_sm_features/1, disco_sm_identity/1, disco_sm_items/1,
               ping_command/4, process_local_iq/5, process_sm_iq/5]).
 
@@ -61,11 +61,13 @@
 start(HostType, #{iqdisc := IQDisc}) ->
     [gen_iq_handler:add_iq_handler_for_domain(HostType, ?NS_COMMANDS, Component, Fn, #{}, IQDisc) ||
         {Component, Fn} <- iq_handlers()],
-    ejabberd_hooks:add(hooks(HostType)).
+    ejabberd_hooks:add(legacy_hooks(HostType)),
+    gen_hook:add_handlers(hooks(HostType)).
 
 -spec stop(mongooseim:host_type()) -> ok.
 stop(HostType) ->
-    ejabberd_hooks:delete(hooks(HostType)),
+    ejabberd_hooks:delete(legacy_hooks(HostType)),
+    gen_hook:delete_handlers(hooks(HostType)),
     [gen_iq_handler:remove_iq_handler_for_domain(HostType, ?NS_COMMANDS, Component) ||
         {Component, _Fn} <- iq_handlers()],
     ok.
@@ -74,15 +76,17 @@ iq_handlers() ->
     [{ejabberd_local, fun ?MODULE:process_local_iq/5},
      {ejabberd_sm, fun ?MODULE:process_sm_iq/5}].
 
-hooks(HostType) ->
+legacy_hooks(HostType) ->
     [{disco_local_identity, HostType, ?MODULE, disco_local_identity, 99},
-     {disco_local_features, HostType, ?MODULE, disco_local_features, 99},
      {disco_local_items, HostType, ?MODULE, disco_local_items, 99},
      {disco_sm_identity, HostType, ?MODULE, disco_sm_identity, 99},
      {disco_sm_features, HostType, ?MODULE, disco_sm_features, 99},
      {disco_sm_items, HostType, ?MODULE, disco_sm_items, 99},
      {adhoc_local_commands, HostType, ?MODULE, ping_command, 100}].
 
+hooks(HostType) ->
+    [{disco_local_features, HostType, fun ?MODULE:disco_local_features/3, #{}, 99}].
+
 %%%
 %%% config_spec
 %%%
@@ -212,17 +216,19 @@ command_list_identity(Lang) ->
 
 %%-------------------------------------------------------------------------
 
--spec disco_local_features(mongoose_disco:feature_acc()) -> mongoose_disco:feature_acc().
-disco_local_features(Acc = #{node := <<>>}) ->
-    mongoose_disco:add_features([?NS_COMMANDS], Acc);
-disco_local_features(Acc = #{node := ?NS_COMMANDS}) ->
+-spec disco_local_features(mongoose_disco:feature_acc(),
+                           map(),
+                           map()) -> {ok, mongoose_disco:feature_acc()}.
+disco_local_features(Acc = #{node := <<>>}, _, _) ->
+    {ok, mongoose_disco:add_features([?NS_COMMANDS], Acc)};
+disco_local_features(Acc = #{node := ?NS_COMMANDS}, _, _) ->
     %% override all lesser features...
-    Acc#{result := []};
-disco_local_features(Acc = #{node := <<"ping">>}) ->
+    {ok, Acc#{result := []}};
+disco_local_features(Acc = #{node := <<"ping">>}, _, _) ->
     %% override all lesser features...
-    Acc#{result := [?NS_COMMANDS]};
-disco_local_features(Acc) ->
-    Acc.
+    {ok, Acc#{result := [?NS_COMMANDS]}};
+disco_local_features(Acc, _, _) ->
+    {ok, Acc}.
 
 %%-------------------------------------------------------------------------
 

src/mod_amp.erl

@@ -11,11 +11,11 @@
 -export([start/2, stop/1, supported_features/0]).
 -export([run_initial_check/2,
          check_packet/2,
-         disco_local_features/1,
+         disco_local_features/3,
          c2s_stream_features/3
         ]).
 
--ignore_xref([c2s_stream_features/3, disco_local_features/1, run_initial_check/2]).
+-ignore_xref([c2s_stream_features/3, run_initial_check/2]).
 
 -include("amp.hrl").
 -include("mongoose.hrl").
@@ -29,23 +29,27 @@
 
 -spec start(mongooseim:host_type(), gen_mod:module_opts()) -> ok.
 start(HostType, _Opts) ->
-    ejabberd_hooks:add(hooks(HostType)).
+    ejabberd_hooks:add(legacy_hooks(HostType)),
+    gen_hook:add_handlers(hooks(HostType)).
 
 -spec stop(mongooseim:host_type()) -> ok.
 stop(HostType) ->
-    ejabberd_hooks:delete(hooks(HostType)).
+    ejabberd_hooks:delete(legacy_hooks(HostType)),
+    gen_hook:delete_handlers(hooks(HostType)).
 
 -spec supported_features() -> [atom()].
 supported_features() -> [dynamic_domains].
 
-hooks(HostType) ->
+legacy_hooks(HostType) ->
     [{c2s_stream_features, HostType, ?MODULE, c2s_stream_features, 50},
-     {disco_local_features, HostType, ?MODULE, disco_local_features, 99},
      {c2s_preprocessing_hook, HostType, ?MODULE, run_initial_check, 10},
      {amp_verify_support, HostType, ?AMP_RESOLVER, verify_support, 10},
      {amp_check_condition, HostType, ?AMP_RESOLVER, check_condition, 10},
      {amp_determine_strategy, HostType, ?AMP_STRATEGY, determine_strategy, 10}].
 
+hooks(HostType) ->
+    [{disco_local_features, HostType, fun ?MODULE:disco_local_features/3, #{}, 99}].
+
 %% API
 
 -spec run_initial_check(mongoose_acc:t(), ejabberd_c2s:state()) -> mongoose_acc:t().
@@ -64,12 +68,15 @@ check_packet(Acc, Event) ->
         Rules -> process_event(Acc, Rules, Event)
     end.
 
--spec disco_local_features(mongoose_disco:feature_acc()) -> mongoose_disco:feature_acc().
-disco_local_features(Acc = #{node := Node}) ->
-    case amp_features(Node) of
+-spec disco_local_features(mongoose_disco:feature_acc(),
+                           map(),
+                           map()) -> {ok, mongoose_disco:feature_acc()}.
+disco_local_features(Acc = #{node := Node}, _, _) ->
+    NewAcc = case amp_features(Node) of
         [] -> Acc;
         Features -> mongoose_disco:add_features(Features, Acc)
-    end.
+    end,
+    {ok, NewAcc}.
 
 -spec c2s_stream_features([exml:element()], mongooseim:host_type(), jid:lserver()) ->
           [exml:element()].