From 54a1d188184e8da21469c72a3d109e36b0f03935 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20D=C5=82ugosz?= Date: Tue, 4 Oct 2022 13:34:55 +0200 Subject: [PATCH] Refactored hook handlers in mod_global_distrib_bounce module --- src/global_distrib/mod_global_distrib.erl | 4 +- .../mod_global_distrib_bounce.erl | 43 ++++++++++--------- src/mongoose_hooks.erl | 9 +++- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/global_distrib/mod_global_distrib.erl b/src/global_distrib/mod_global_distrib.erl index f7f0f4f5a4..0c188baa75 100644 --- a/src/global_distrib/mod_global_distrib.erl +++ b/src/global_distrib/mod_global_distrib.erl @@ -224,9 +224,9 @@ remove_metadata(Acc, Key) -> %% Hooks implementation %%-------------------------------------------------------------------- --spec maybe_reroute(drop, _, _) -> drop; +-spec maybe_reroute(drop, _, _) -> {ok, drop}; (FPacket, Params, Extra) -> {ok, drop} | {ok, FPacket} when - FPacket :: {jid:jid(), jid:jid(), mongoose_acc:t(), exml:element()}, + FPacket :: {jid:jid(), jid:jid(), mongoose_acc:t(), exml:element()}, Params :: map(), Extra :: map(). maybe_reroute(drop, _, _) -> {ok, drop}; diff --git a/src/global_distrib/mod_global_distrib_bounce.erl b/src/global_distrib/mod_global_distrib_bounce.erl index c638c07f9b..7a65a4c89a 100644 --- a/src/global_distrib/mod_global_distrib_bounce.erl +++ b/src/global_distrib/mod_global_distrib_bounce.erl @@ -30,10 +30,10 @@ -export([start_link/0, start/2, stop/1, deps/2]). -export([init/1, handle_info/2, handle_cast/2, handle_call/3, code_change/3, terminate/2]). --export([maybe_store_message/1, reroute_messages/4]). +-export([maybe_store_message/3, reroute_messages/3]). -export([bounce_queue_size/0]). --ignore_xref([bounce_queue_size/0, maybe_store_message/1, reroute_messages/4, start_link/0]). +-ignore_xref([bounce_queue_size/0, start_link/0]). %%-------------------------------------------------------------------- %% gen_mod API @@ -46,14 +46,14 @@ start(HostType, _Opts) -> EvalDef = {[{l, [{t, [value, {v, 'Value'}]}]}], [value]}, QueueSizeDef = {function, ?MODULE, bounce_queue_size, [], eval, EvalDef}, mongoose_metrics:ensure_metric(global, ?GLOBAL_DISTRIB_BOUNCE_QUEUE_SIZE, QueueSizeDef), - ejabberd_hooks:add(hooks(HostType)), + gen_hook:add_handlers(hooks(HostType)), ChildSpec = {?MODULE, {?MODULE, start_link, []}, permanent, 1000, worker, [?MODULE]}, ejabberd_sup:start_child(ChildSpec). -spec stop(mongooseim:host_type()) -> any(). stop(HostType) -> ejabberd_sup:stop_child(?MODULE), - ejabberd_hooks:delete(hooks(HostType)), + gen_hook:add_handlers(hooks(HostType)), ets:delete(?MS_BY_TARGET), ets:delete(?MESSAGE_STORE). @@ -62,8 +62,8 @@ deps(_HostType, Opts) -> [{mod_global_distrib_utils, Opts, hard}]. hooks(HostType) -> - [{mod_global_distrib_unknown_recipient, HostType, ?MODULE, maybe_store_message, 80}, - {mod_global_distrib_known_recipient, HostType, ?MODULE, reroute_messages, 80}]. + [{mod_global_distrib_unknown_recipient, HostType, fun ?MODULE:maybe_store_message/3, #{}, 80}, + {mod_global_distrib_known_recipient, HostType, fun ?MODULE:reroute_messages/3, #{}, 80}]. -spec start_link() -> {ok, pid()} | {error, any()}. start_link() -> @@ -99,14 +99,16 @@ terminate(_Reason, _State) -> %% Hooks implementation %%-------------------------------------------------------------------- --spec maybe_store_message(drop) -> drop; - ({jid:jid(), jid:jid(), mongoose_acc:t(), exml:packet()}) -> - drop | {jid:jid(), jid:jid(), mongoose_acc:t(), exml:packet()}. -maybe_store_message(drop) -> drop; -maybe_store_message({From, To, Acc0, Packet} = FPacket) -> +-spec maybe_store_message(drop, _, _) -> {ok, drop}; + (FPacket, Params, Extra) -> {ok, drop} | {ok, FPacket} when + FPacket :: {jid:jid(), jid:jid(), mongoose_acc:t(), exml:element()}, + Params :: map(), + Extra :: map(). +maybe_store_message(drop, _, _) -> {ok, drop}; +maybe_store_message({From, To, Acc0, Packet} = FPacket, _, _) -> LocalHost = opt(local_host), {ok, ID} = mod_global_distrib:find_metadata(Acc0, id), - case mod_global_distrib:get_metadata(Acc0, {bounce_ttl, LocalHost}, + ResultAcc = case mod_global_distrib:get_metadata(Acc0, {bounce_ttl, LocalHost}, opt([bounce, max_retries])) of 0 -> ?LOG_DEBUG(#{what => gd_skip_store_message, @@ -128,13 +130,14 @@ maybe_store_message({From, To, Acc0, Packet} = FPacket) -> ResendAt = erlang:monotonic_time() + ResendAfter, do_insert_in_store(ResendAt, {From, To, Acc, Packet}), drop - end. - --spec reroute_messages(SomeAcc :: mongoose_acc:t(), - From :: jid:jid(), - To :: jid:jid(), - TargetHost :: binary()) -> mongoose_acc:t(). -reroute_messages(Acc, From, To, TargetHost) -> + end, + {ok, ResultAcc}. + +-spec reroute_messages(Acc, Params, Extra) -> {ok, Acc} when + Acc :: mongoose_acc:t(), + Params :: #{from := jid:jid(), to := jid:jid(), target_host := binary()}, + Extra :: map(). +reroute_messages(Acc, #{from := From, to := To, target_host := TargetHost}, _) -> Key = get_index_key(From, To), StoredMessages = lists:filtermap( @@ -150,7 +153,7 @@ reroute_messages(Acc, From, To, TargetHost) -> text => <<"Routing multiple previously stored messages">>, stored_messages_length => length(StoredMessages), acc => Acc}), lists:foreach(pa:bind(fun reroute_message/2, TargetHost), StoredMessages), - Acc. + {ok, Acc}. %%-------------------------------------------------------------------- %% API for metrics diff --git a/src/mongoose_hooks.erl b/src/mongoose_hooks.erl index 006bb641aa..abd4b2d414 100644 --- a/src/mongoose_hooks.erl +++ b/src/mongoose_hooks.erl @@ -1515,8 +1515,11 @@ pubsub_publish_item(Server, NodeId, Publisher, ServiceJID, ItemId, BrPayload) -> LocalHost :: jid:server(), Result :: any(). mod_global_distrib_known_recipient(GlobalHost, From, To, LocalHost) -> + Params = #{from => From, to => To, target_host => LocalHost}, + Args = [From, To, LocalHost], + ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args), run_hook_for_host_type(mod_global_distrib_known_recipient, GlobalHost, ok, - [From, To, LocalHost]). + ParamsWithLegacyArgs). %%% @doc The `mod_global_distrib_unknown_recipient' hook is called when %%% the recipient is unknown to `global_distrib'. @@ -1525,7 +1528,9 @@ mod_global_distrib_known_recipient(GlobalHost, From, To, LocalHost) -> Info :: filter_packet_acc(), Result :: any(). mod_global_distrib_unknown_recipient(GlobalHost, Info) -> - run_hook_for_host_type(mod_global_distrib_unknown_recipient, GlobalHost, Info, []). + ParamsWithLegacyArgs = ejabberd_hooks:add_args(#{}, []), + run_hook_for_host_type(mod_global_distrib_unknown_recipient, GlobalHost, Info, + ParamsWithLegacyArgs). %%%----------------------------------------------------------------------