From 8879d1d533cdea83efebad54b37e22f849092ac1 Mon Sep 17 00:00:00 2001 From: Evgeniy Khramtsov Date: Thu, 28 Jun 2018 10:37:20 +0300 Subject: [PATCH] Avoid code duplication when checking presence subscription --- src/mod_block_strangers.erl | 16 +++------------- src/mod_disco.erl | 21 +++------------------ src/mod_roster.erl | 19 ++++++++++++++++++- 3 files changed, 24 insertions(+), 32 deletions(-) diff --git a/src/mod_block_strangers.erl b/src/mod_block_strangers.erl index fbbef1899..6e9d1097a 100644 --- a/src/mod_block_strangers.erl +++ b/src/mod_block_strangers.erl @@ -208,29 +208,19 @@ need_check(Pkt) -> -spec check_subscription(jid(), jid()) -> boolean(). check_subscription(From, To) -> - {LocalUser, LocalServer, _} = jid:tolower(To), + LocalServer = To#jid.lserver, {RemoteUser, RemoteServer, _} = jid:tolower(From), - case has_subscription(LocalUser, LocalServer, From) of + case mod_roster:is_subscribed(From, To) of false when RemoteUser == <<"">> -> false; false -> %% Check if the contact's server is in the roster gen_mod:get_module_opt(LocalServer, ?MODULE, allow_transports) - andalso has_subscription(LocalUser, LocalServer, - jid:make(RemoteServer)); + andalso mod_roster:is_subscribed(jid:make(RemoteServer), To); true -> true end. --spec has_subscription(binary(), binary(), jid()) -> boolean(). -has_subscription(User, Server, JID) -> - {Sub, Ask, _} = ejabberd_hooks:run_fold( - roster_get_jid_info, Server, - {none, none, []}, - [User, Server, JID]), - (Sub /= none) orelse (Ask == subscribe) - orelse (Ask == out) orelse (Ask == both). - sets_bare_member({U, S, <<"">>} = LBJID, Set) -> case ?SETS:next(sets_iterator_from(LBJID, Set)) of {{U, S, _}, _} -> true; diff --git a/src/mod_disco.erl b/src/mod_disco.erl index 170134559..08299ae2d 100644 --- a/src/mod_disco.erl +++ b/src/mod_disco.erl @@ -263,7 +263,7 @@ process_sm_iq_items(#iq{type = set, lang = Lang} = IQ) -> process_sm_iq_items(#iq{type = get, lang = Lang, from = From, to = To, sub_els = [#disco_items{node = Node}]} = IQ) -> - case is_presence_subscribed(From, To) of + case mod_roster:is_subscribed(From, To) of true -> Host = To#jid.lserver, case ejabberd_hooks:run_fold(disco_sm_items, Host, @@ -290,7 +290,7 @@ get_sm_items(Acc, From, {result, Its} -> Its; empty -> [] end, - Items1 = case is_presence_subscribed(From, To) of + Items1 = case mod_roster:is_subscribed(From, To) of true -> get_user_resources(User, Server); _ -> [] end, @@ -308,21 +308,6 @@ get_sm_items(empty, From, To, _Node, Lang) -> {error, xmpp:err_not_allowed(Txt, Lang)} end. --spec is_presence_subscribed(jid(), jid()) -> boolean(). -is_presence_subscribed(#jid{luser = User, lserver = Server}, - #jid{luser = User, lserver = Server}) -> true; -is_presence_subscribed(#jid{luser = FromUser, lserver = FromServer}, - #jid{luser = ToUser, lserver = ToServer}) -> - lists:any(fun (#roster{jid = {SubUser, SubServer, _}, subscription = Sub}) - when FromUser == SubUser, FromServer == SubServer, - Sub /= none -> - true; - (_RosterEntry) -> - false - end, - ejabberd_hooks:run_fold(roster_get, ToServer, [], - [{ToUser, ToServer}])). - -spec process_sm_iq_info(iq()) -> iq(). process_sm_iq_info(#iq{type = set, lang = Lang} = IQ) -> Txt = <<"Value 'set' of 'type' attribute is not allowed">>, @@ -330,7 +315,7 @@ process_sm_iq_info(#iq{type = set, lang = Lang} = IQ) -> process_sm_iq_info(#iq{type = get, lang = Lang, from = From, to = To, sub_els = [#disco_info{node = Node}]} = IQ) -> - case is_presence_subscribed(From, To) of + case mod_roster:is_subscribed(From, To) of true -> Host = To#jid.lserver, Identity = ejabberd_hooks:run_fold(disco_sm_identity, diff --git a/src/mod_roster.erl b/src/mod_roster.erl index 3375220d1..38c3a78b0 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -43,7 +43,7 @@ -export([start/2, stop/1, reload/3, process_iq/1, export/1, import_info/0, process_local_iq/1, get_user_roster/2, import/5, get_roster/2, push_item/3, - import_start/2, import_stop/2, + import_start/2, import_stop/2, is_subscribed/2, c2s_self_presence/1, in_subscription/2, out_subscription/1, set_items/3, remove_user/2, get_jid_info/4, encode_item/1, webadmin_page/3, @@ -876,6 +876,23 @@ get_jid_info(_, User, Server, JID) -> LJID = jid:tolower(JID), get_subscription_and_groups(LUser, LServer, LJID). +%% Check if `From` is subscriberd to `To`s presence +%% note 1: partial subscriptions are also considered, i.e. +%% `To` has already sent a subscription request to `From` +%% note 2: it's assumed a user is subscribed to self +%% note 3: `To` MUST be a local user, `From` can be any user +-spec is_subscribed(jid(), jid()) -> boolean(). +is_subscribed(#jid{luser = LUser, lserver = LServer}, + #jid{luser = LUser, lserver = LServer}) -> + true; +is_subscribed(From, #jid{luser = LUser, lserver = LServer}) -> + {Sub, Ask, _} = ejabberd_hooks:run_fold( + roster_get_jid_info, LServer, + {none, none, []}, + [LUser, LServer, From]), + (Sub /= none) orelse (Ask == subscribe) + orelse (Ask == out) orelse (Ask == both). + %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% webadmin_page(_, Host,