Merge pull request #3829 from esl/hooks/mongoose_metrics_mam_hooks

Refactored hook handlers in mongoose_metrics_mam_hooks
esl · Nov 7, 2022 · a6771c0 · a6771c0
2 parents adfb9db + 4b6dc14
commit a6771c0
Show file tree

Hide file tree

Showing 5 changed files with 188 additions and 133 deletions.
diff --git a/src/gen_hook.erl b/src/gen_hook.erl
@@ -32,12 +32,16 @@
 -type hook_acc() :: any().
 -type hook_params() :: map().
 -type hook_extra() :: map().
+-type extra() :: #{hook_name := hook_name(),
+                   hook_tag := hook_tag(),
+                   host_type => mongooseim:host_type(),
+                   _ => _}.
 
 -type hook_fn_ret_value() :: {ok | stop, NewAccumulator :: hook_acc()}.
 -type hook_fn() :: %% see run_fold/4 documentation
     fun((Accumulator :: hook_acc(),
          ExecutionParameters :: hook_params(),
-         ExtraParameters :: hook_extra()) -> hook_fn_ret_value()).
+         ExtraParameters :: extra()) -> hook_fn_ret_value()).
 
 -type key() :: {HookName :: atom(),
                 Tag :: any()}.
@@ -54,7 +58,7 @@
 
 -record(hook_handler, {prio :: pos_integer(),
                        hook_fn :: hook_fn(),
-                       extra :: map()}).
+                       extra :: extra()}).
 
 -define(TABLE, ?MODULE).
 %%%----------------------------------------------------------------------

diff --git a/src/mam/mod_mam_muc.erl b/src/mam/mod_mam_muc.erl
@@ -119,14 +119,16 @@ archive_id(MucHost, RoomName) when is_binary(MucHost), is_binary(RoomName) ->
 start(HostType, Opts) ->
     ?LOG_DEBUG(#{what => mam_muc_starting}),
     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_DEBUG(#{what => mam_muc_stopping}),
-    ejabberd_hooks:delete(hooks(HostType)),
+    ejabberd_hooks:delete(legacy_hooks(HostType)),
+    gen_hook:delete_handlers(hooks(HostType)),
     remove_iq_handlers(HostType),
     ok.
 
@@ -616,13 +618,16 @@ 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(host_type()) -> [ejabberd_hooks:hook()].
-hooks(HostType) ->
+-spec legacy_hooks(host_type()) -> [ejabberd_hooks:hook()].
+legacy_hooks(HostType) ->
     [{disco_muc_features, HostType, ?MODULE, disco_muc_features, 99},
      {filter_room_packet, HostType, ?MODULE, filter_room_packet, 60},
      {forget_room, HostType, ?MODULE, forget_room, 90},
-     {get_personal_data, HostType, ?MODULE, get_personal_data, 50}
-     | mongoose_metrics_mam_hooks:get_mam_muc_hooks(HostType)].
+     {get_personal_data, HostType, ?MODULE, get_personal_data, 50}].
+
+-spec hooks(mongooseim:host_type()) -> gen_hook:hook_list().
+hooks(HostType) ->
+    mongoose_metrics_mam_hooks:get_mam_muc_hooks(HostType).
 
 add_iq_handlers(HostType, Opts) ->
     IQDisc = gen_mod:get_opt(iqdisc, Opts, parallel),

diff --git a/src/mam/mod_mam_pm.erl b/src/mam/mod_mam_pm.erl
@@ -112,15 +112,13 @@ archive_id(Server, User)
 start(HostType, Opts) ->
     ?LOG_INFO(#{what => mam_starting, host_type => HostType}),
     ensure_metrics(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(legacy_hooks(HostType)),
     gen_hook:delete_handlers(hooks(HostType)),
     remove_iq_handlers(HostType),
     ok.
@@ -664,10 +662,7 @@ 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 legacy_hooks(jid:lserver()) -> [ejabberd_hooks:hook()].
-legacy_hooks(HostType) ->
-    mongoose_metrics_mam_hooks:get_mam_hooks(HostType).
-
+-spec hooks(mongooseim:host_type()) -> gen_hook:hook_list().
 hooks(HostType) ->
     [
         {disco_local_features, HostType, fun ?MODULE:disco_local_features/3, #{}, 99},
@@ -678,6 +673,7 @@ hooks(HostType) ->
         {amp_determine_strategy, HostType, fun ?MODULE:determine_amp_strategy/3, #{}, 20},
         {sm_filter_offline_message, HostType, fun ?MODULE:sm_filter_offline_message/3, #{}, 50},
         {get_personal_data, HostType, fun ?MODULE:get_personal_data/3, #{}, 50}
+        | mongoose_metrics_mam_hooks:get_mam_hooks(HostType)
     ].
 
 add_iq_handlers(HostType, Opts) ->

diff --git a/src/metrics/mongoose_metrics_mam_hooks.erl b/src/metrics/mongoose_metrics_mam_hooks.erl
@@ -16,135 +16,158 @@
 %%-------------------
 %% Internal exports
 %%-------------------
--export([mam_get_prefs/4,
-         mam_set_prefs/7,
-         mam_remove_archive/4,
+-export([mam_get_prefs/3,
+         mam_set_prefs/3,
+         mam_remove_archive/3,
          mam_lookup_messages/3,
          mam_archive_message/3,
          mam_flush_messages/3,
-         mam_muc_get_prefs/4,
-         mam_muc_set_prefs/7,
-         mam_muc_remove_archive/4,
+         mam_muc_get_prefs/3,
+         mam_muc_set_prefs/3,
+         mam_muc_remove_archive/3,
          mam_muc_lookup_messages/3,
          mam_muc_archive_message/3,
          mam_muc_flush_messages/3]).
 
--ignore_xref([mam_archive_message/3, mam_get_prefs/4, mam_lookup_messages/3, mam_flush_messages/3,
-              mam_muc_archive_message/3, mam_muc_flush_messages/3, mam_muc_get_prefs/4,
-              mam_muc_lookup_messages/3, mam_muc_remove_archive/4, mam_muc_set_prefs/7,
-              mam_remove_archive/4, mam_set_prefs/7]).
-
--type metrics_notify_return() :: mongoose_metrics_hooks:metrics_notify_return().
-
 %%-------------------
 %% Implementation
 %%-------------------
 
 %% @doc Here will be declared which hooks should be registered when mod_mam_pm is enabled.
--spec get_mam_hooks(_) -> [ejabberd_hooks:hook(), ...].
-get_mam_hooks(Host) ->
+-spec get_mam_hooks(_) -> gen_hook:hook_list().
+get_mam_hooks(HostType) ->
     [
-        {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_flush_messages, Host, ?MODULE, mam_flush_messages, 50}
+        {mam_set_prefs, HostType, fun ?MODULE:mam_set_prefs/3, #{}, 50},
+        {mam_get_prefs, HostType, fun ?MODULE:mam_get_prefs/3, #{}, 50},
+        {mam_archive_message, HostType, fun ?MODULE:mam_archive_message/3, #{}, 50},
+        {mam_remove_archive, HostType, fun ?MODULE:mam_remove_archive/3, #{}, 50},
+        {mam_lookup_messages, HostType, fun ?MODULE:mam_lookup_messages/3, #{}, 100},
+        {mam_flush_messages, HostType, fun ?MODULE:mam_flush_messages/3, #{}, 50}
     ].
 
 %% @doc Here will be declared which hooks should be registered when mod_mam_muc is enabled.
--spec get_mam_muc_hooks(_) -> [ejabberd_hooks:hook(), ...].
-get_mam_muc_hooks(Host) ->
+-spec get_mam_muc_hooks(_) -> gen_hook:hook_list().
+get_mam_muc_hooks(HostType) ->
     [
-        {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_muc_set_prefs, HostType, fun ?MODULE:mam_muc_set_prefs/3, #{}, 50},
+        {mam_muc_get_prefs, HostType, fun ?MODULE:mam_muc_get_prefs/3, #{}, 50},
+        {mam_muc_archive_message, HostType, fun ?MODULE:mam_muc_archive_message/3, #{}, 50},
+        {mam_muc_remove_archive, HostType, fun ?MODULE:mam_muc_remove_archive/3, #{}, 50},
+        {mam_muc_lookup_messages, HostType, fun ?MODULE:mam_muc_lookup_messages/3, #{}, 100},
+        {mam_muc_flush_messages, HostType, fun ?MODULE:mam_muc_flush_messages/3, #{}, 50}
     ].
 
--spec mam_get_prefs(Result :: any(),
-                    Host :: jid:server(),
-                    _ArcID :: mod_mam:archive_id(),
-                    _ArcJID :: jid:jid()) -> any().
-mam_get_prefs(Result, Host, _ArcID, _ArcJID) ->
-    mongoose_metrics:update(Host, modMamPrefsGets, 1),
-    Result.
-
--spec mam_set_prefs(Result :: any(), Host :: jid:server(),
-    _ArcID :: mod_mam:archive_id(), _ArcJID :: jid:jid(),
-    _DefaultMode :: any(), _AlwaysJIDs :: [jid:literal_jid()],
-    _NeverJIDs :: [jid:literal_jid()]) -> any().
-mam_set_prefs(Result, Host, _ArcID, _ArcJID, _DefaultMode, _AlwaysJIDs, _NeverJIDs) ->
-    mongoose_metrics:update(Host, modMamPrefsSets, 1),
-    Result.
-
--spec mam_remove_archive(Acc :: map(),
-                         Host :: jid:server(),
-                         _ArcID :: mod_mam:archive_id(),
-                         _ArcJID :: jid:jid()) -> metrics_notify_return().
-mam_remove_archive(Acc, Host, _ArcID, _ArcJID) ->
-    mongoose_metrics:update(Host, modMamArchiveRemoved, 1),
-    Acc.
-
+-spec mam_get_prefs(Acc, Params, Extra) -> {ok, Acc} when
+    Acc :: any(),
+    Params :: map(),
+    Extra :: gen_hook:extra().
+mam_get_prefs(Acc, _, #{host_type := HostType}) ->
+    mongoose_metrics:update(HostType, modMamPrefsGets, 1),
+    {ok, Acc}.
+
+-spec mam_set_prefs(Acc, Params, Extra) -> {ok, Acc} when
+    Acc :: any(),
+    Params :: map(),
+    Extra :: gen_hook:extra().
+mam_set_prefs(Acc, _, #{host_type := HostType}) ->
+    mongoose_metrics:update(HostType, modMamPrefsSets, 1),
+    {ok, Acc}.
+
+-spec mam_remove_archive(Acc, Params, Extra) -> {ok, Acc} when
+    Acc :: any(),
+    Params :: map(),
+    Extra :: gen_hook:extra().
+mam_remove_archive(Acc, _, #{host_type := HostType}) ->
+    mongoose_metrics:update(HostType, modMamArchiveRemoved, 1),
+    {ok, Acc}.
+
+-spec mam_lookup_messages(Result, Params, Extra) -> {ok, Result} when
+    Result :: {ok | error, mod_mam:lookup_result()},
+    Params :: map(),
+    Extra :: gen_hook:extra().
 mam_lookup_messages(Result = {ok, {_TotalCount, _Offset, MessageRows}},
-                    Host, #{is_simple := IsSimple}) ->
-    mongoose_metrics:update(Host, modMamForwarded, length(MessageRows)),
-    mongoose_metrics:update(Host, modMamLookups, 1),
+                    #{is_simple := IsSimple}, #{host_type := HostType}) ->
+    mongoose_metrics:update(HostType, modMamForwarded, length(MessageRows)),
+    mongoose_metrics:update(HostType, modMamLookups, 1),
     case IsSimple of
         true ->
-            mongoose_metrics:update(Host, [modMamLookups, simple], 1);
+            mongoose_metrics:update(HostType, [modMamLookups, simple], 1);
         _ ->
             ok
     end,
-    Result;
-mam_lookup_messages(Result = {error, _}, _Host, _Params) ->
-    Result.
-
--spec mam_archive_message(Result :: any(), Host :: jid:server(),
-                          _Params :: mod_mam:archive_message_params()) -> any().
-mam_archive_message(Result, Host, _Params) ->
-    mongoose_metrics:update(Host, modMamArchived, 1),
-    Result.
-
-mam_flush_messages(Acc, Host, MessageCount) ->
-    mongoose_metrics:update(Host, modMamFlushed, MessageCount),
-    Acc.
+    {ok, Result};
+mam_lookup_messages(Result = {error, _}, _, _) ->
+    {ok, Result}.
+
+-spec mam_archive_message(Acc, Params, Extra) -> {ok, Acc} when
+    Acc :: any(),
+    Params :: mod_mam:archive_message_params(),
+    Extra :: gen_hook:extra().
+mam_archive_message(Acc, _, #{host_type := HostType}) ->
+    mongoose_metrics:update(HostType, modMamArchived, 1),
+    {ok, Acc}.
+
+-spec mam_flush_messages(Acc, Params, Extra) -> {ok, Acc} when
+    Acc :: any(),
+    Params :: #{message_count := integer()},
+    Extra :: gen_hook:extra().
+mam_flush_messages(Acc, #{message_count := MessageCount}, #{host_type := HostType}) ->
+    mongoose_metrics:update(HostType, modMamFlushed, MessageCount),
+    {ok, Acc}.
 
 %% ----------------------------------------------------------------------------
 %% mod_mam_muc
 
-mam_muc_get_prefs(Result, Host, _ArcID, _ArcJID) ->
-    mongoose_metrics:update(Host, modMucMamPrefsGets, 1),
-    Result.
-
-mam_muc_set_prefs(Result, Host, _ArcID, _ArcJID, _DefaultMode, _AlwaysJIDs, _NeverJIDs) ->
-    mongoose_metrics:update(Host, modMucMamPrefsSets, 1),
-    Result.
-
-mam_muc_remove_archive(Acc, Host, _ArcID, _ArcJID) ->
-    mongoose_metrics:update(Host, modMucMamArchiveRemoved, 1),
-    Acc.
-
+-spec mam_muc_get_prefs(Acc, Params, Extra) -> {ok, Acc} when
+    Acc :: any(),
+    Params :: map(),
+    Extra :: gen_hook:extra().
+mam_muc_get_prefs(Acc, _, #{host_type := HostType}) ->
+    mongoose_metrics:update(HostType, modMucMamPrefsGets, 1),
+    {ok, Acc}.
+
+-spec mam_muc_set_prefs(Acc, Params, Extra) -> {ok, Acc} when
+    Acc :: any(),
+    Params :: map(),
+    Extra :: gen_hook:extra().
+mam_muc_set_prefs(Acc, _, #{host_type := HostType}) ->
+    mongoose_metrics:update(HostType, modMucMamPrefsSets, 1),
+    {ok, Acc}.
+
+-spec mam_muc_remove_archive(Acc, Params, Extra) -> {ok, Acc} when
+    Acc :: any(),
+    Params :: map(),
+    Extra :: gen_hook:extra().
+mam_muc_remove_archive(Acc, _, #{host_type := HostType}) ->
+    mongoose_metrics:update(HostType, modMucMamArchiveRemoved, 1),
+    {ok, Acc}.
+
+-spec mam_muc_lookup_messages(Result, Params, Extra) -> {ok, Result} when
+    Result :: {ok | error, mod_mam:lookup_result()},
+    Params :: map(),
+    Extra :: gen_hook:extra().
 mam_muc_lookup_messages(Result = {ok, {_TotalCount, _Offset, MessageRows}},
-    Host, _Params) ->
-    mongoose_metrics:update(Host, modMucMamForwarded, length(MessageRows)),
-    mongoose_metrics:update(Host, modMucMamLookups, 1),
-    Result;
-mam_muc_lookup_messages(Result = {error, _},
-    _Host, _Params) ->
-    Result.
-
--spec mam_muc_archive_message(Result :: any(), Host :: jid:server(),
-                              _Params :: mod_mam:archive_message_params()) -> any().
-mam_muc_archive_message(Result, Host, _Params) ->
-    mongoose_metrics:update(Host, modMucMamArchived, 1),
-    Result.
-
-%% #rh
-mam_muc_flush_messages(Acc, Host, MessageCount) ->
-    mongoose_metrics:update(Host, modMucMamFlushed, MessageCount),
-    Acc.
+                        _, #{host_type := HostType}) ->
+    mongoose_metrics:update(HostType, modMucMamForwarded, length(MessageRows)),
+    mongoose_metrics:update(HostType, modMucMamLookups, 1),
+    {ok, Result};
+mam_muc_lookup_messages(Result = {error, _}, _, _) ->
+    {ok, Result}.
+
+-spec mam_muc_archive_message(Acc, Params, Extra) -> {ok, Acc} when
+    Acc :: any(),
+    Params :: mod_mam:archive_message_params(),
+    Extra :: gen_hook:extra().
+mam_muc_archive_message(Acc, _, #{host_type := HostType}) ->
+    mongoose_metrics:update(HostType, modMucMamArchived, 1),
+    {ok, Acc}.
+
+-spec mam_muc_flush_messages(Acc, Params, Extra) -> {ok, Acc} when
+    Acc :: any(),
+    Params :: #{message_count := integer()},
+    Extra :: gen_hook:extra().
+mam_muc_flush_messages(Acc, #{message_count := MessageCount}, #{host_type := HostType}) ->
+    mongoose_metrics:update(HostType, modMucMamFlushed, MessageCount),
+    {ok, Acc}.
 
 %%% vim: set sts=4 ts=4 sw=4 et filetype=erlang foldmarker=%%%',%%%. foldmethod=marker: