Avoid code duplication when checking presence subscription

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,