From 878322979861def1deb08c81de02a1fc1fdc6782 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20D=C5=82ugosz?= Date: Thu, 27 Oct 2022 10:56:04 +0200 Subject: [PATCH 1/3] Refactored hook handlers in mongoose_metrics_mam_hooks --- src/mam/mod_mam_muc.erl | 17 +- src/mam/mod_mam_pm.erl | 8 +- src/metrics/mongoose_metrics_mam_hooks.erl | 173 ++++++++++++--------- src/mongoose_hooks.erl | 65 +++++--- 4 files changed, 157 insertions(+), 106 deletions(-) diff --git a/src/mam/mod_mam_muc.erl b/src/mam/mod_mam_muc.erl index a65af61257a..49901a5f454 100644 --- 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 index c558484afde..6610236b8d3 100644 --- a/src/mam/mod_mam_pm.erl +++ b/src/mam/mod_mam_pm.erl @@ -112,7 +112,6 @@ 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. @@ -120,7 +119,6 @@ start(HostType, Opts) -> -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 index 870910db12f..c9419c435c5 100644 --- a/src/metrics/mongoose_metrics_mam_hooks.erl +++ b/src/metrics/mongoose_metrics_mam_hooks.erl @@ -16,80 +16,77 @@ %%------------------- %% 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(), ...]. +-spec get_mam_hooks(_) -> gen_hook:hook_list(). get_mam_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_flush_messages, Host, ?MODULE, mam_flush_messages, 50} + {mam_set_prefs, Host, fun ?MODULE:mam_set_prefs/3, #{}, 50}, + {mam_get_prefs, Host, fun ?MODULE:mam_get_prefs/3, #{}, 50}, + {mam_archive_message, Host, fun ?MODULE:mam_archive_message/3, #{}, 50}, + {mam_remove_archive, Host, fun ?MODULE:mam_remove_archive/3, #{}, 50}, + {mam_lookup_messages, Host, fun ?MODULE:mam_lookup_messages/3, #{}, 100}, + {mam_flush_messages, Host, 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(), ...]. +-spec get_mam_muc_hooks(_) -> gen_hook:hook_list(). get_mam_muc_hooks(Host) -> [ - {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, Host, fun ?MODULE:mam_muc_set_prefs/3, #{}, 50}, + {mam_muc_get_prefs, Host, fun ?MODULE:mam_muc_get_prefs/3, #{}, 50}, + {mam_muc_archive_message, Host, fun ?MODULE:mam_muc_archive_message/3, #{}, 50}, + {mam_muc_remove_archive, Host, fun ?MODULE:mam_muc_remove_archive/3, #{}, 50}, + {mam_muc_lookup_messages, Host, fun ?MODULE:mam_muc_lookup_messages/3, #{}, 100}, + {mam_muc_flush_messages, Host, 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) -> +-spec mam_get_prefs(Acc, Params, Extra) -> {ok, Acc} when + Acc :: any(), + Params :: map(), + Extra :: #{host_type := mongooseim:host_type()}. +mam_get_prefs(Acc, _, #{host_type := Host}) -> mongoose_metrics:update(Host, modMamPrefsGets, 1), - Result. + {ok, Acc}. --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) -> +-spec mam_set_prefs(Acc, Params, Extra) -> {ok, Acc} when + Acc :: any(), + Params :: map(), + Extra :: #{host_type := mongooseim:host_type()}. +mam_set_prefs(Acc, _, #{host_type := Host}) -> mongoose_metrics:update(Host, modMamPrefsSets, 1), - Result. + {ok, Acc}. --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) -> +-spec mam_remove_archive(Acc, Params, Extra) -> {ok, Acc} when + Acc :: any(), + Params :: map(), + Extra :: #{host_type := mongooseim:host_type()}. +mam_remove_archive(Acc, _, #{host_type := Host}) -> mongoose_metrics:update(Host, modMamArchiveRemoved, 1), - Acc. + {ok, Acc}. +-spec mam_lookup_messages(Result, Params, Extra) -> {ok, Result} when + Result :: {ok | error, mod_mam:lookup_result()}, + Params :: map(), + Extra :: #{host_type := mongooseim:host_type()}. mam_lookup_messages(Result = {ok, {_TotalCount, _Offset, MessageRows}}, - Host, #{is_simple := IsSimple}) -> + #{is_simple := IsSimple}, #{host_type := Host}) -> mongoose_metrics:update(Host, modMamForwarded, length(MessageRows)), mongoose_metrics:update(Host, modMamLookups, 1), case IsSimple of @@ -98,53 +95,79 @@ mam_lookup_messages(Result = {ok, {_TotalCount, _Offset, MessageRows}}, _ -> 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) -> + {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 :: #{host_type := mongooseim:host_type()}. +mam_archive_message(Acc, _, #{host_type := Host}) -> mongoose_metrics:update(Host, modMamArchived, 1), - Result. + {ok, Acc}. -mam_flush_messages(Acc, Host, MessageCount) -> +-spec mam_flush_messages(Acc, Params, Extra) -> {ok, Acc} when + Acc :: any(), + Params :: #{message_count := integer()}, + Extra :: #{host_type := mongooseim:host_type()}. +mam_flush_messages(Acc, #{message_count := MessageCount}, #{host_type := Host}) -> mongoose_metrics:update(Host, modMamFlushed, MessageCount), - Acc. + {ok, Acc}. %% ---------------------------------------------------------------------------- %% mod_mam_muc -mam_muc_get_prefs(Result, Host, _ArcID, _ArcJID) -> +-spec mam_muc_get_prefs(Acc, Params, Extra) -> {ok, Acc} when + Acc :: any(), + Params :: map(), + Extra :: #{host_type := mongooseim:host_type()}. +mam_muc_get_prefs(Acc, _, #{host_type := Host}) -> mongoose_metrics:update(Host, modMucMamPrefsGets, 1), - Result. + {ok, Acc}. -mam_muc_set_prefs(Result, Host, _ArcID, _ArcJID, _DefaultMode, _AlwaysJIDs, _NeverJIDs) -> +-spec mam_muc_set_prefs(Acc, Params, Extra) -> {ok, Acc} when + Acc :: any(), + Params :: map(), + Extra :: #{host_type := mongooseim:host_type()}. +mam_muc_set_prefs(Acc, _, #{host_type := Host}) -> mongoose_metrics:update(Host, modMucMamPrefsSets, 1), - Result. + {ok, Acc}. -mam_muc_remove_archive(Acc, Host, _ArcID, _ArcJID) -> +-spec mam_muc_remove_archive(Acc, Params, Extra) -> {ok, Acc} when + Acc :: any(), + Params :: map(), + Extra :: #{host_type := mongooseim:host_type()}. +mam_muc_remove_archive(Acc, _, #{host_type := Host}) -> mongoose_metrics:update(Host, modMucMamArchiveRemoved, 1), - Acc. + {ok, Acc}. +-spec mam_muc_lookup_messages(Result, Params, Extra) -> {ok, Result} when + Result :: {ok | error, mod_mam:lookup_result()}, + Params :: map(), + Extra :: #{host_type := mongooseim:host_type()}. mam_muc_lookup_messages(Result = {ok, {_TotalCount, _Offset, MessageRows}}, - Host, _Params) -> + _, #{host_type := Host}) -> 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) -> + {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 :: #{host_type := mongooseim:host_type()}. +mam_muc_archive_message(Acc, _, #{host_type := Host}) -> mongoose_metrics:update(Host, modMucMamArchived, 1), - Result. + {ok, Acc}. -%% #rh -mam_muc_flush_messages(Acc, Host, MessageCount) -> +-spec mam_muc_flush_messages(Acc, Params, Extra) -> {ok, Acc} when + Acc :: any(), + Params :: #{message_count := integer()}, + Extra :: #{host_type := mongooseim:host_type()}. +mam_muc_flush_messages(Acc, #{message_count := MessageCount}, #{host_type := Host}) -> mongoose_metrics:update(Host, modMucMamFlushed, MessageCount), - Acc. + {ok, Acc}. %%% vim: set sts=4 ts=4 sw=4 et filetype=erlang foldmarker=%%%',%%%. foldmethod=marker: diff --git a/src/mongoose_hooks.erl b/src/mongoose_hooks.erl index 4113df35379..4f20076214f 100644 --- a/src/mongoose_hooks.erl +++ b/src/mongoose_hooks.erl @@ -1107,9 +1107,12 @@ mam_get_behaviour(HookServer, ArchiveID, OwnerJID, RemoteJID) -> NeverJIDs :: [jid:literel_jid()], Result :: any(). mam_set_prefs(HookServer, ArchiveID, OwnerJID, DefaultMode, AlwaysJIDs, NeverJIDs) -> + Params = #{archive_id => ArchiveID, owner_jid => OwnerJID, default_mode => DefaultMode, + always_jids => AlwaysJIDs, never_jids => NeverJIDs}, + Args = [HookServer, ArchiveID, OwnerJID, DefaultMode, AlwaysJIDs, NeverJIDs], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), run_hook_for_host_type(mam_set_prefs, HookServer, {error, not_implemented}, - [HookServer, ArchiveID, OwnerJID, - DefaultMode, AlwaysJIDs, NeverJIDs]). + ParamsWithLegacyArgs). %%% @doc The `mam_get_prefs' hook is called to read %%% the archive settings for a given user. @@ -1120,9 +1123,12 @@ mam_set_prefs(HookServer, ArchiveID, OwnerJID, DefaultMode, AlwaysJIDs, NeverJI OwnerJID :: jid:jid(), Result :: mod_mam:preference() | {error, Reason :: term()}. mam_get_prefs(HookServer, DefaultMode, ArchiveID, OwnerJID) -> + Params = #{archive_id => ArchiveID, owner_jid => OwnerJID}, + Args = [HookServer, ArchiveID, OwnerJID], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), InitialAccValue = {DefaultMode, [], []}, %% mod_mam:preference() type run_hook_for_host_type(mam_get_prefs, HookServer, InitialAccValue, - [HookServer, ArchiveID, OwnerJID]). + ParamsWithLegacyArgs). %%% @doc The `mam_remove_archive' hook is called in order to %%% remove the entire archive for a particular user. @@ -1131,8 +1137,11 @@ mam_get_prefs(HookServer, DefaultMode, ArchiveID, OwnerJID) -> ArchiveID :: undefined | mod_mam:archive_id(), OwnerJID :: jid:jid(). mam_remove_archive(HookServer, ArchiveID, OwnerJID) -> + Params = #{archive_id => ArchiveID, owner_jid => OwnerJID}, + Args = [HookServer, ArchiveID, OwnerJID], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), run_hook_for_host_type(mam_remove_archive, HookServer, ok, - [HookServer, ArchiveID, OwnerJID]). + ParamsWithLegacyArgs). %%% @doc The `mam_lookup_messages' hook is to retrieve %%% archived messages for given search parameters. @@ -1141,9 +1150,11 @@ mam_remove_archive(HookServer, ArchiveID, OwnerJID) -> Params :: map(), Result :: {ok, mod_mam:lookup_result()}. mam_lookup_messages(HookServer, Params) -> + Args = [HookServer, Params], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), InitialLookupValue = {0, 0, []}, %% mod_mam:lookup_result() type run_hook_for_host_type(mam_lookup_messages, HookServer, {ok, InitialLookupValue}, - [HookServer, Params]). + ParamsWithLegacyArgs). %%% @doc The `mam_archive_message' hook is called in order %%% to store the message in the archive. @@ -1153,15 +1164,19 @@ mam_lookup_messages(HookServer, Params) -> Params :: mod_mam:archive_message_params(), Result :: ok | {error, timeout}. mam_archive_message(HookServer, Params) -> - run_hook_for_host_type(mam_archive_message, HookServer, ok, [HookServer, Params]). + Args = [HookServer, Params], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), + run_hook_for_host_type(mam_archive_message, HookServer, ok, ParamsWithLegacyArgs). %%% @doc The `mam_flush_messages' hook is run after the async bulk write %%% happens for messages despite the result of the write. -spec mam_flush_messages(HookServer :: jid:lserver(), MessageCount :: integer()) -> ok. mam_flush_messages(HookServer, MessageCount) -> - run_hook_for_host_type(mam_flush_messages, HookServer, ok, - [HookServer, MessageCount]). + Params = #{message_count => MessageCount}, + Args = [HookServer, MessageCount], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), + run_hook_for_host_type(mam_flush_messages, HookServer, ok, ParamsWithLegacyArgs). %% @doc Waits until all pending messages are written -spec mam_archive_sync(HostType :: mongooseim:host_type()) -> ok. @@ -1235,10 +1250,12 @@ mam_muc_get_behaviour(HostType, ArchiveID, RoomJID, RemoteJID) -> NeverJIDs :: [jid:literel_jid()], Result :: any(). mam_muc_set_prefs(HostType, ArchiveID, RoomJID, DefaultMode, AlwaysJIDs, NeverJIDs) -> + Params = #{archive_id => ArchiveID, room_jid => RoomJID, default_mode => DefaultMode, + always_jids => AlwaysJIDs, never_jids => NeverJIDs}, + Args = [HostType, ArchiveID, RoomJID, DefaultMode, AlwaysJIDs, NeverJIDs], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), InitialAcc = {error, not_implemented}, - run_hook_for_host_type(mam_muc_set_prefs, HostType, InitialAcc, - [HostType, ArchiveID, RoomJID, DefaultMode, - AlwaysJIDs, NeverJIDs]). + run_hook_for_host_type(mam_muc_set_prefs, HostType, InitialAcc, ParamsWithLegacyArgs). %%% @doc The `mam_muc_get_prefs' hook is called to read %%% the archive settings for a given room. @@ -1249,9 +1266,11 @@ mam_muc_set_prefs(HostType, ArchiveID, RoomJID, DefaultMode, AlwaysJIDs, NeverJI RoomJID :: jid:jid(), Result :: mod_mam:preference() | {error, Reason :: term()}. mam_muc_get_prefs(HostType, DefaultMode, ArchiveID, RoomJID) -> + Params = #{archive_id => ArchiveID, room_jid => RoomJID}, + Args = [HostType, ArchiveID, RoomJID], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), InitialAcc = {DefaultMode, [], []}, %% mod_mam:preference() type - run_hook_for_host_type(mam_muc_get_prefs, HostType, InitialAcc, - [HostType, ArchiveID, RoomJID]). + run_hook_for_host_type(mam_muc_get_prefs, HostType, InitialAcc, ParamsWithLegacyArgs). %%% @doc The `mam_muc_remove_archive' hook is called in order to remove the entire %%% archive for a particular user. @@ -1260,8 +1279,10 @@ mam_muc_get_prefs(HostType, DefaultMode, ArchiveID, RoomJID) -> ArchiveID :: undefined | mod_mam:archive_id(), RoomJID :: jid:jid(). mam_muc_remove_archive(HostType, ArchiveID, RoomJID) -> - run_hook_for_host_type(mam_muc_remove_archive, HostType, ok, - [HostType, ArchiveID, RoomJID]). + Params = #{archive_id => ArchiveID, room_jid => RoomJID}, + Args = [HostType, ArchiveID, RoomJID], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), + run_hook_for_host_type(mam_muc_remove_archive, HostType, ok, ParamsWithLegacyArgs). %%% @doc The `mam_muc_lookup_messages' hook is to retrieve archived %%% MUC messages for any given search parameters. @@ -1270,9 +1291,11 @@ mam_muc_remove_archive(HostType, ArchiveID, RoomJID) -> Params :: map(), Result :: {ok, mod_mam:lookup_result()}. mam_muc_lookup_messages(HostType, Params) -> + Args = [HostType, Params], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), InitialLookupValue = {0, 0, []}, %% mod_mam:lookup_result() type run_hook_for_host_type(mam_muc_lookup_messages, HostType, {ok, InitialLookupValue}, - [HostType, Params]). + ParamsWithLegacyArgs). %%% @doc The `mam_muc_archive_message' hook is called in order %%% to store the MUC message in the archive. @@ -1281,15 +1304,19 @@ mam_muc_lookup_messages(HostType, Params) -> Params :: mod_mam:archive_message_params(), Result :: ok | {error, timeout}. mam_muc_archive_message(HostType, Params) -> - run_hook_for_host_type(mam_muc_archive_message, HostType, ok, [HostType, Params]). + Args = [HostType, Params], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), + run_hook_for_host_type(mam_muc_archive_message, HostType, ok, ParamsWithLegacyArgs). %%% @doc The `mam_muc_flush_messages' hook is run after the async bulk write %%% happens for MUC messages despite the result of the write. -spec mam_muc_flush_messages(HookServer :: jid:lserver(), MessageCount :: integer()) -> ok. mam_muc_flush_messages(HookServer, MessageCount) -> - run_hook_for_host_type(mam_muc_flush_messages, HookServer, ok, - [HookServer, MessageCount]). + Params = #{message_count => MessageCount}, + Args = [HookServer, MessageCount], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), + run_hook_for_host_type(mam_muc_flush_messages, HookServer, ok, ParamsWithLegacyArgs). %% @doc Waits until all pending messages are written -spec mam_muc_archive_sync(HostType :: mongooseim:host_type()) -> ok. From 2cf352ff4d7a795239c75a0126b531d19ead891c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20D=C5=82ugosz?= Date: Wed, 2 Nov 2022 10:15:02 +0100 Subject: [PATCH 2/3] Code review follow-up --- src/metrics/mongoose_metrics_mam_hooks.erl | 82 +++++++++++----------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/src/metrics/mongoose_metrics_mam_hooks.erl b/src/metrics/mongoose_metrics_mam_hooks.erl index c9419c435c5..6d0f2999983 100644 --- a/src/metrics/mongoose_metrics_mam_hooks.erl +++ b/src/metrics/mongoose_metrics_mam_hooks.erl @@ -35,50 +35,50 @@ %% @doc Here will be declared which hooks should be registered when mod_mam_pm is enabled. -spec get_mam_hooks(_) -> gen_hook:hook_list(). -get_mam_hooks(Host) -> +get_mam_hooks(HostType) -> [ - {mam_set_prefs, Host, fun ?MODULE:mam_set_prefs/3, #{}, 50}, - {mam_get_prefs, Host, fun ?MODULE:mam_get_prefs/3, #{}, 50}, - {mam_archive_message, Host, fun ?MODULE:mam_archive_message/3, #{}, 50}, - {mam_remove_archive, Host, fun ?MODULE:mam_remove_archive/3, #{}, 50}, - {mam_lookup_messages, Host, fun ?MODULE:mam_lookup_messages/3, #{}, 100}, - {mam_flush_messages, Host, fun ?MODULE:mam_flush_messages/3, #{}, 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(_) -> gen_hook:hook_list(). -get_mam_muc_hooks(Host) -> +get_mam_muc_hooks(HostType) -> [ - {mam_muc_set_prefs, Host, fun ?MODULE:mam_muc_set_prefs/3, #{}, 50}, - {mam_muc_get_prefs, Host, fun ?MODULE:mam_muc_get_prefs/3, #{}, 50}, - {mam_muc_archive_message, Host, fun ?MODULE:mam_muc_archive_message/3, #{}, 50}, - {mam_muc_remove_archive, Host, fun ?MODULE:mam_muc_remove_archive/3, #{}, 50}, - {mam_muc_lookup_messages, Host, fun ?MODULE:mam_muc_lookup_messages/3, #{}, 100}, - {mam_muc_flush_messages, Host, fun ?MODULE:mam_muc_flush_messages/3, #{}, 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(Acc, Params, Extra) -> {ok, Acc} when Acc :: any(), Params :: map(), Extra :: #{host_type := mongooseim:host_type()}. -mam_get_prefs(Acc, _, #{host_type := Host}) -> - mongoose_metrics:update(Host, modMamPrefsGets, 1), +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 :: #{host_type := mongooseim:host_type()}. -mam_set_prefs(Acc, _, #{host_type := Host}) -> - mongoose_metrics:update(Host, modMamPrefsSets, 1), +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 :: #{host_type := mongooseim:host_type()}. -mam_remove_archive(Acc, _, #{host_type := Host}) -> - mongoose_metrics:update(Host, modMamArchiveRemoved, 1), +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 @@ -86,12 +86,12 @@ mam_remove_archive(Acc, _, #{host_type := Host}) -> Params :: map(), Extra :: #{host_type := mongooseim:host_type()}. mam_lookup_messages(Result = {ok, {_TotalCount, _Offset, MessageRows}}, - #{is_simple := IsSimple}, #{host_type := Host}) -> - 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, @@ -103,16 +103,16 @@ mam_lookup_messages(Result = {error, _}, _, _) -> Acc :: any(), Params :: mod_mam:archive_message_params(), Extra :: #{host_type := mongooseim:host_type()}. -mam_archive_message(Acc, _, #{host_type := Host}) -> - mongoose_metrics:update(Host, modMamArchived, 1), +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 :: #{host_type := mongooseim:host_type()}. -mam_flush_messages(Acc, #{message_count := MessageCount}, #{host_type := Host}) -> - mongoose_metrics:update(Host, modMamFlushed, MessageCount), +mam_flush_messages(Acc, #{message_count := MessageCount}, #{host_type := HostType}) -> + mongoose_metrics:update(HostType, modMamFlushed, MessageCount), {ok, Acc}. %% ---------------------------------------------------------------------------- @@ -122,24 +122,24 @@ mam_flush_messages(Acc, #{message_count := MessageCount}, #{host_type := Host}) Acc :: any(), Params :: map(), Extra :: #{host_type := mongooseim:host_type()}. -mam_muc_get_prefs(Acc, _, #{host_type := Host}) -> - mongoose_metrics:update(Host, modMucMamPrefsGets, 1), +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 :: #{host_type := mongooseim:host_type()}. -mam_muc_set_prefs(Acc, _, #{host_type := Host}) -> - mongoose_metrics:update(Host, modMucMamPrefsSets, 1), +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 :: #{host_type := mongooseim:host_type()}. -mam_muc_remove_archive(Acc, _, #{host_type := Host}) -> - mongoose_metrics:update(Host, modMucMamArchiveRemoved, 1), +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 @@ -147,9 +147,9 @@ mam_muc_remove_archive(Acc, _, #{host_type := Host}) -> Params :: map(), Extra :: #{host_type := mongooseim:host_type()}. mam_muc_lookup_messages(Result = {ok, {_TotalCount, _Offset, MessageRows}}, - _, #{host_type := Host}) -> - mongoose_metrics:update(Host, modMucMamForwarded, length(MessageRows)), - mongoose_metrics:update(Host, modMucMamLookups, 1), + _, #{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}. @@ -158,16 +158,16 @@ mam_muc_lookup_messages(Result = {error, _}, _, _) -> Acc :: any(), Params :: mod_mam:archive_message_params(), Extra :: #{host_type := mongooseim:host_type()}. -mam_muc_archive_message(Acc, _, #{host_type := Host}) -> - mongoose_metrics:update(Host, modMucMamArchived, 1), +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 :: #{host_type := mongooseim:host_type()}. -mam_muc_flush_messages(Acc, #{message_count := MessageCount}, #{host_type := Host}) -> - mongoose_metrics:update(Host, modMucMamFlushed, MessageCount), +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: From 4b6dc147f8f8c9347d9df7fb94a7157245e96c37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20D=C5=82ugosz?= Date: Mon, 7 Nov 2022 12:25:05 +0100 Subject: [PATCH 3/3] Code review follow-up --- src/gen_hook.erl | 8 ++++++-- src/metrics/mongoose_metrics_mam_hooks.erl | 24 +++++++++++----------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/gen_hook.erl b/src/gen_hook.erl index 5125f3e2340..f157e189291 100644 --- 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/metrics/mongoose_metrics_mam_hooks.erl b/src/metrics/mongoose_metrics_mam_hooks.erl index 6d0f2999983..cc1fd2611ee 100644 --- a/src/metrics/mongoose_metrics_mam_hooks.erl +++ b/src/metrics/mongoose_metrics_mam_hooks.erl @@ -60,7 +60,7 @@ get_mam_muc_hooks(HostType) -> -spec mam_get_prefs(Acc, Params, Extra) -> {ok, Acc} when Acc :: any(), Params :: map(), - Extra :: #{host_type := mongooseim:host_type()}. + Extra :: gen_hook:extra(). mam_get_prefs(Acc, _, #{host_type := HostType}) -> mongoose_metrics:update(HostType, modMamPrefsGets, 1), {ok, Acc}. @@ -68,7 +68,7 @@ mam_get_prefs(Acc, _, #{host_type := HostType}) -> -spec mam_set_prefs(Acc, Params, Extra) -> {ok, Acc} when Acc :: any(), Params :: map(), - Extra :: #{host_type := mongooseim:host_type()}. + Extra :: gen_hook:extra(). mam_set_prefs(Acc, _, #{host_type := HostType}) -> mongoose_metrics:update(HostType, modMamPrefsSets, 1), {ok, Acc}. @@ -76,7 +76,7 @@ mam_set_prefs(Acc, _, #{host_type := HostType}) -> -spec mam_remove_archive(Acc, Params, Extra) -> {ok, Acc} when Acc :: any(), Params :: map(), - Extra :: #{host_type := mongooseim:host_type()}. + Extra :: gen_hook:extra(). mam_remove_archive(Acc, _, #{host_type := HostType}) -> mongoose_metrics:update(HostType, modMamArchiveRemoved, 1), {ok, Acc}. @@ -84,7 +84,7 @@ mam_remove_archive(Acc, _, #{host_type := HostType}) -> -spec mam_lookup_messages(Result, Params, Extra) -> {ok, Result} when Result :: {ok | error, mod_mam:lookup_result()}, Params :: map(), - Extra :: #{host_type := mongooseim:host_type()}. + Extra :: gen_hook:extra(). mam_lookup_messages(Result = {ok, {_TotalCount, _Offset, MessageRows}}, #{is_simple := IsSimple}, #{host_type := HostType}) -> mongoose_metrics:update(HostType, modMamForwarded, length(MessageRows)), @@ -102,7 +102,7 @@ mam_lookup_messages(Result = {error, _}, _, _) -> -spec mam_archive_message(Acc, Params, Extra) -> {ok, Acc} when Acc :: any(), Params :: mod_mam:archive_message_params(), - Extra :: #{host_type := mongooseim:host_type()}. + Extra :: gen_hook:extra(). mam_archive_message(Acc, _, #{host_type := HostType}) -> mongoose_metrics:update(HostType, modMamArchived, 1), {ok, Acc}. @@ -110,7 +110,7 @@ mam_archive_message(Acc, _, #{host_type := HostType}) -> -spec mam_flush_messages(Acc, Params, Extra) -> {ok, Acc} when Acc :: any(), Params :: #{message_count := integer()}, - Extra :: #{host_type := mongooseim:host_type()}. + Extra :: gen_hook:extra(). mam_flush_messages(Acc, #{message_count := MessageCount}, #{host_type := HostType}) -> mongoose_metrics:update(HostType, modMamFlushed, MessageCount), {ok, Acc}. @@ -121,7 +121,7 @@ mam_flush_messages(Acc, #{message_count := MessageCount}, #{host_type := HostTyp -spec mam_muc_get_prefs(Acc, Params, Extra) -> {ok, Acc} when Acc :: any(), Params :: map(), - Extra :: #{host_type := mongooseim:host_type()}. + Extra :: gen_hook:extra(). mam_muc_get_prefs(Acc, _, #{host_type := HostType}) -> mongoose_metrics:update(HostType, modMucMamPrefsGets, 1), {ok, Acc}. @@ -129,7 +129,7 @@ mam_muc_get_prefs(Acc, _, #{host_type := HostType}) -> -spec mam_muc_set_prefs(Acc, Params, Extra) -> {ok, Acc} when Acc :: any(), Params :: map(), - Extra :: #{host_type := mongooseim:host_type()}. + Extra :: gen_hook:extra(). mam_muc_set_prefs(Acc, _, #{host_type := HostType}) -> mongoose_metrics:update(HostType, modMucMamPrefsSets, 1), {ok, Acc}. @@ -137,7 +137,7 @@ mam_muc_set_prefs(Acc, _, #{host_type := HostType}) -> -spec mam_muc_remove_archive(Acc, Params, Extra) -> {ok, Acc} when Acc :: any(), Params :: map(), - Extra :: #{host_type := mongooseim:host_type()}. + Extra :: gen_hook:extra(). mam_muc_remove_archive(Acc, _, #{host_type := HostType}) -> mongoose_metrics:update(HostType, modMucMamArchiveRemoved, 1), {ok, Acc}. @@ -145,7 +145,7 @@ mam_muc_remove_archive(Acc, _, #{host_type := HostType}) -> -spec mam_muc_lookup_messages(Result, Params, Extra) -> {ok, Result} when Result :: {ok | error, mod_mam:lookup_result()}, Params :: map(), - Extra :: #{host_type := mongooseim:host_type()}. + Extra :: gen_hook:extra(). mam_muc_lookup_messages(Result = {ok, {_TotalCount, _Offset, MessageRows}}, _, #{host_type := HostType}) -> mongoose_metrics:update(HostType, modMucMamForwarded, length(MessageRows)), @@ -157,7 +157,7 @@ mam_muc_lookup_messages(Result = {error, _}, _, _) -> -spec mam_muc_archive_message(Acc, Params, Extra) -> {ok, Acc} when Acc :: any(), Params :: mod_mam:archive_message_params(), - Extra :: #{host_type := mongooseim:host_type()}. + Extra :: gen_hook:extra(). mam_muc_archive_message(Acc, _, #{host_type := HostType}) -> mongoose_metrics:update(HostType, modMucMamArchived, 1), {ok, Acc}. @@ -165,7 +165,7 @@ mam_muc_archive_message(Acc, _, #{host_type := HostType}) -> -spec mam_muc_flush_messages(Acc, Params, Extra) -> {ok, Acc} when Acc :: any(), Params :: #{message_count := integer()}, - Extra :: #{host_type := mongooseim:host_type()}. + Extra :: gen_hook:extra(). mam_muc_flush_messages(Acc, #{message_count := MessageCount}, #{host_type := HostType}) -> mongoose_metrics:update(HostType, modMucMamFlushed, MessageCount), {ok, Acc}.