Implement cache for roster

include/pubsub.hrl

@@ -104,13 +104,6 @@
 ).
 %% @type affiliation() = 'none' | 'owner' | 'publisher' | 'publish-only' | 'member' | 'outcast'.
 
--type(subscription() :: 'none'
-                      | 'pending'
-                      | 'unconfigured'
-                      | 'subscribed'
-).
-%% @type subscription() = 'none' | 'pending' | 'unconfigured' | 'subscribed'.
-
 -type(accessModel() :: 'open'
                      | 'presence'
                      | 'roster'

rebar.config

@@ -20,7 +20,7 @@
 
 {deps, [{lager, ".*", {git, "https://github.com/basho/lager", {tag, "3.2.1"}}},
         {p1_utils, ".*", {git, "https://github.com/processone/p1_utils", "470539a"}},
-        {cache_tab, ".*", {git, "https://github.com/processone/cache_tab", "51eee22"}},
+        {cache_tab, ".*", {git, "https://github.com/processone/cache_tab", "6f762a59"}},
         {fast_tls, ".*", {git, "https://github.com/processone/fast_tls", {tag, "1.0.11"}}},
         {stringprep, ".*", {git, "https://github.com/processone/stringprep", {tag, "1.0.8"}}},
         {fast_xml, ".*", {git, "https://github.com/processone/fast_xml", {tag, "1.1.21"}}},

src/ejabberd_c2s.erl

@@ -46,7 +46,7 @@
 	 reject_unauthenticated_packet/2, process_closed/2,
 	 process_terminated/2, process_info/2]).
 %% API
--export([get_presence/1, get_subscription/2, get_subscribed/1,
+-export([get_presence/1, resend_presence/1, resend_presence/2,
 	 open_session/1, call/3, send/2, close/1, close/2, stop/1,
 	 reply/2, copy_state/2, set_timeout/2, route/2,
 	 host_up/1, host_down/1]).
@@ -54,6 +54,7 @@
 -include("ejabberd.hrl").
 -include("xmpp.hrl").
 -include("logger.hrl").
+-include("mod_roster.hrl").
 
 -define(SETS, gb_sets).
 
@@ -93,23 +94,13 @@ reply(Ref, Reply) ->
 get_presence(Ref) ->
     call(Ref, get_presence, 1000).
 
--spec get_subscription(jid() | ljid(), state()) -> both | from | to | none.
+-spec resend_presence(pid()) -> ok.
-get_subscription(#jid{} = From, State) ->
+resend_presence(Pid) ->
-    get_subscription(jid:tolower(From), State);
+    resend_presence(Pid, undefined).
-get_subscription(LFrom, #{pres_f := PresF, pres_t := PresT}) ->
-    LBFrom = jid:remove_resource(LFrom),
-    F = ?SETS:is_element(LFrom, PresF) orelse ?SETS:is_element(LBFrom, PresF),
-    T = ?SETS:is_element(LFrom, PresT) orelse ?SETS:is_element(LBFrom, PresT),
-    if F and T -> both;
-       F -> from;
-       T -> to;
-       true -> none
-    end.
 
--spec get_subscribed(pid()) -> [ljid()].
+-spec resend_presence(pid(), jid() | undefined) -> ok.
-%% Return list of all available resources of contacts
+resend_presence(Pid, To) ->
-get_subscribed(Ref) ->
+    route(Pid, {resend_presence, To}).
-    call(Ref, get_subscribed, 1000).
 
 -spec close(pid()) -> ok;
 	   (state()) -> state().
@@ -183,8 +174,7 @@ host_down(Host) ->
 copy_state(#{owner := Owner} = NewState,
 	   #{jid := JID, resource := Resource, sid := {Time, _},
 	     auth_module := AuthModule, lserver := LServer,
-	     pres_t := PresT, pres_a := PresA,
+	     pres_a := PresA} = OldState) ->
-	     pres_f := PresF} = OldState) ->
     State1 = case OldState of
 		 #{pres_last := Pres, pres_timestamp := PresTS} ->
 		     NewState#{pres_last => Pres, pres_timestamp => PresTS};
@@ -196,8 +186,7 @@ copy_state(#{owner := Owner} = NewState,
 		     conn => Conn,
 		     sid => {Time, Owner},
 		     auth_module => AuthModule,
-		     pres_t => PresT, pres_a => PresA,
+		     pres_a => PresA},
-		     pres_f => PresF},
     ejabberd_hooks:run_fold(c2s_copy_session, LServer, State2, [OldState]).
 
 -spec open_session(state()) -> {ok, state()} | state().
@@ -238,10 +227,17 @@ process_info(#{lserver := LServer} = State, {route, Packet}) ->
        true ->
 	    State1
     end;
-process_info(State, force_update_presence) ->
+process_info(#{jid := JID} = State, {resend_presence, To}) ->
     case maps:get(pres_last, State, error) of
 	error -> State;
-	Pres -> process_self_presence(State, Pres)
+	Pres when To == undefined ->
+	    process_self_presence(State, Pres);
+	Pres when To#jid.luser == JID#jid.luser andalso
+		  To#jid.lserver == JID#jid.lserver andalso
+		  To#jid.lresource == <<"">> ->
+	    process_self_presence(State, Pres);
+	Pres ->
+	    process_presence_out(State, xmpp:set_to(Pres, To))
     end;
 process_info(State, Info) ->
     ?WARNING_MSG("got unexpected info: ~p", [Info]),
@@ -390,15 +386,11 @@ bind(R, #{user := U, server := S, access := Access, lang := Lang,
 		allow ->
 		    State1 = open_session(State#{resource => Resource,
 						 sid => ejabberd_sm:make_sid()}),
-		    LBJID = jid:remove_resource(jid:tolower(JID)),
+		    State2 = ejabberd_hooks:run_fold(
-		    PresF = ?SETS:add_element(LBJID, maps:get(pres_f, State1)),
+			       c2s_session_opened, LServer, State1, []),
-		    PresT = ?SETS:add_element(LBJID, maps:get(pres_t, State1)),
-		    State2 = State1#{pres_f => PresF, pres_t => PresT},
-		    State3 = ejabberd_hooks:run_fold(
-			       c2s_session_opened, LServer, State2, []),
 		    ?INFO_MSG("(~s) Opened c2s session for ~s",
 			      [SockMod:pp(Socket), jid:encode(JID)]),
-		    {ok, State3};
+		    {ok, State2};
 		deny ->
 		    ejabberd_hooks:run(forbidden_session_hook, LServer, [JID]),
 		    ?INFO_MSG("(~s) Forbidden c2s session for ~s",
@@ -513,8 +505,6 @@ init([State, Opts]) ->
 		    tls_enabled => TLSEnabled,
 		    tls_verify => TLSVerify,
 		    pres_a => ?SETS:new(),
-		    pres_f => ?SETS:new(),
-		    pres_t => ?SETS:new(),
 		    zlib => Zlib,
 		    lang => ?MYLANG,
 		    server => ?MYNAME,
@@ -532,9 +522,6 @@ handle_call(get_presence, From, #{jid := JID} = State) ->
 	   end,
     reply(From, Pres),
     State;
-handle_call(get_subscribed, From, #{pres_f := PresF} = State) ->
-    reply(From, ?SETS:to_list(PresF)),
-    State;
 handle_call(Request, From, #{lserver := LServer} = State) ->
     ejabberd_hooks:run_fold(
       c2s_handle_call, LServer, State, [Request, From]).
@@ -589,36 +576,36 @@ process_message_in(State, #message{type = T} = Msg) ->
 
 -spec process_presence_in(state(), presence()) -> {boolean(), state()}.
 process_presence_in(#{lserver := LServer, pres_a := PresA} = State0,
-		    #presence{from = From, to = To, type = T} = Pres) ->
+		    #presence{from = From, type = T} = Pres) ->
     State = ejabberd_hooks:run_fold(c2s_presence_in, LServer, State0, [Pres]),
     case T of
 	probe ->
-	    NewState = add_to_pres_a(State, From),
+	    route_probe_reply(From, State),
-	    route_probe_reply(From, To, NewState),
+	    {false, State};
-	    {false, NewState};
 	error ->
 	    A = ?SETS:del_element(jid:tolower(From), PresA),
 	    {true, State#{pres_a => A}};
 	_ ->
 	    case privacy_check_packet(State, Pres, in) of
 		allow ->
-		    NewState = add_to_pres_a(State, From),
+		    {true, State};
-		    {true, NewState};
 		deny ->
 		    {false, State}
 	    end
     end.
 
--spec route_probe_reply(jid(), jid(), state()) -> ok.
+-spec route_probe_reply(jid(), state()) -> ok.
-route_probe_reply(From, To, #{lserver := LServer, pres_f := PresF,
+route_probe_reply(From, #{jid := To,
 			  pres_last := LastPres,
 			  pres_timestamp := TS} = State) ->
-    LFrom = jid:tolower(From),
+    {LUser, LServer, LResource} = jid:tolower(To),
-    LBFrom = jid:remove_resource(LFrom),
+    IsAnotherResource = case jid:tolower(From) of
-    case ?SETS:is_element(LFrom, PresF)
+			    {LUser, LServer, R} when R /= LResource -> true;
-	orelse ?SETS:is_element(LBFrom, PresF) of
+			    _ -> false
-	true ->
+			end,
-	    %% To is my JID
+    Subscription = get_subscription(To, From),
+    if IsAnotherResource orelse
+       Subscription == both orelse Subscription == from ->
 	    Packet = xmpp_util:add_delay_info(LastPres, To, TS),
 	    case privacy_check_packet(State, Packet, out) of
 		deny ->
@@ -627,19 +614,12 @@ route_probe_reply(From, To, #{lserver := LServer, pres_f := PresF,
 		    ejabberd_hooks:run(presence_probe_hook,
 				       LServer,
 				       [From, To, self()]),
-		    %% Don't route a presence probe to oneself
+		    ejabberd_router:route(xmpp:set_from_to(Packet, To, From))
-		    case From == To of
+	    end;
-			false ->
-			    ejabberd_router:route(
-			      xmpp:set_from_to(Packet, To, From));
        true ->
 	    ok
-		    end
     end;
-	false ->
+route_probe_reply(_, _) ->
-	    ok
-    end;
-route_probe_reply(_, _, _) ->
     ok.
 
 -spec process_presence_out(state(), presence()) -> state().
@@ -675,11 +655,22 @@ process_presence_out(#{user := User, server := Server, lserver := LServer,
 	    State;
 	allow ->
 	    ejabberd_router:route(Pres),
+	    LBareTo = jid:remove_resource(LTo),
+	    LBareFrom = jid:remove_resource(jid:tolower(From)),
+	    if LBareTo /= LBareFrom ->
+		    Subscription = get_subscription(From, To),
+		    if Subscription /= both andalso Subscription /= from ->
 			    A = case Type of
 				    available -> ?SETS:add_element(LTo, PresA);
 				    unavailable -> ?SETS:del_element(LTo, PresA)
 				end,
-	    State#{pres_a => A}
+			    State#{pres_a => A};
+		       true ->
+			    State
+		    end;
+	       true ->
+		    State
+	    end
     end.
 
 -spec process_self_presence(state(), presence()) -> state().
@@ -716,24 +707,81 @@ update_priority(#{ip := IP, conn := Conn, auth_module := AuthMod,
     ejabberd_sm:set_presence(SID, U, S, R, Priority, Pres, Info).
 
 -spec broadcast_presence_unavailable(state(), presence()) -> state().
-broadcast_presence_unavailable(#{pres_a := PresA} = State, Pres) ->
+broadcast_presence_unavailable(#{jid := JID, pres_a := PresA} = State, Pres) ->
-    JIDs = filter_blocked(State, Pres, PresA),
+    #jid{luser = LUser, lserver = LServer} = JID,
+    BareJID = jid:remove_resource(JID),
+    Items1 = ejabberd_hooks:run_fold(roster_get, LServer,
+				     [], [{LUser, LServer}]),
+    Items2 = ?SETS:fold(
+		fun(LJID, Items) ->
+			[#roster{jid = LJID, subscription = from}|Items]
+		end, Items1, PresA),
+    JIDs = lists:foldl(
+	     fun(#roster{jid = LJID, subscription = Sub}, Tos)
+		   when Sub == both orelse Sub == from ->
+		     To = jid:make(LJID),
+		     P = xmpp:set_to(Pres, jid:make(LJID)),
+		     case privacy_check_packet(State, P, out) of
+			 allow -> [To|Tos];
+			 deny -> Tos
+		     end;
+		(_, Tos) ->
+		     Tos
+	     end, [BareJID], Items2),
     route_multiple(State, JIDs, Pres),
     State#{pres_a => ?SETS:new()}.
 
 -spec broadcast_presence_available(state(), presence(), boolean()) -> state().
-broadcast_presence_available(#{pres_a := PresA, pres_f := PresF,
+broadcast_presence_available(#{jid := JID} = State,
-			       pres_t := PresT, jid := JID} = State,
 			     Pres, _FromUnavailable = true) ->
     Probe = #presence{from = JID, type = probe},
-    TJIDs = filter_blocked(State, Probe, PresT),
+    #jid{luser = LUser, lserver = LServer} = JID,
-    FJIDs = filter_blocked(State, Pres, PresF),
+    BareJID = jid:remove_resource(JID),
+    Items = ejabberd_hooks:run_fold(roster_get, LServer,
+				    [], [{LUser, LServer}]),
+    {FJIDs, TJIDs} =
+	lists:foldl(
+	  fun(#roster{jid = LJID, subscription = Sub}, {F, T}) ->
+		  To = jid:make(LJID),
+		  F1 = if Sub == both orelse Sub == from ->
+			       Pres1 = xmpp:set_to(Pres, To),
+			       case privacy_check_packet(State, Pres1, out) of
+				   allow -> [To|F];
+				   deny -> F
+			       end;
+			  true -> F
+		       end,
+		  T1 = if Sub == both orelse Sub == to ->
+			       Probe1 = xmpp:set_to(Probe, To),
+			       case privacy_check_packet(State, Probe1, out) of
+				   allow -> [To|T];
+				   deny -> T
+			       end;
+			  true -> T
+		       end,
+		  {F1, T1}
+	  end, {[BareJID], [BareJID]}, Items),
     route_multiple(State, TJIDs, Probe),
     route_multiple(State, FJIDs, Pres),
-    State#{pres_a => ?SETS:union(PresA, PresF)};
+    State;
-broadcast_presence_available(#{pres_a := PresA, pres_f := PresF} = State,
+broadcast_presence_available(#{jid := JID} = State,
 			     Pres, _FromUnavailable = false) ->
-    JIDs = filter_blocked(State, Pres, ?SETS:intersection(PresA, PresF)),
+    #jid{luser = LUser, lserver = LServer} = JID,
+    BareJID = jid:remove_resource(JID),
+    Items = ejabberd_hooks:run_fold(
+	      roster_get, LServer, [], [{LUser, LServer}]),
+    JIDs = lists:foldl(
+	     fun(#roster{jid = LJID, subscription = Sub}, Tos)
+		   when Sub == both orelse Sub == from ->
+		     To = jid:make(LJID),
+		     P = xmpp:set_to(Pres, jid:make(LJID)),
+		     case privacy_check_packet(State, P, out) of
+			 allow -> [To|Tos];
+			 deny -> Tos
+		     end;
+		(_, Tos) ->
+		     Tos
+	     end, [BareJID], Items),
     route_multiple(State, JIDs, Pres),
     State.
 
@@ -761,23 +809,17 @@ get_priority_from_presence(#presence{priority = Prio}) ->
 	_ -> Prio
     end.
 
--spec filter_blocked(state(), presence(), ?SETS:set()) -> [jid()].
-filter_blocked(#{jid := From} = State, Pres, LJIDSet) ->
-    ?SETS:fold(
-       fun(LJID, Acc) ->
-	       To = jid:make(LJID),
-	       Pkt = xmpp:set_from_to(Pres, From, To),
-	       case privacy_check_packet(State, Pkt, out) of
-		   allow -> [To|Acc];
-		   deny -> Acc
-	       end
-       end, [], LJIDSet).
-
 -spec route_multiple(state(), [jid()], stanza()) -> ok.
 route_multiple(#{lserver := LServer}, JIDs, Pkt) ->
     From = xmpp:get_from(Pkt),
     ejabberd_router_multicast:route_multicast(From, LServer, JIDs, Pkt).
 
+get_subscription(#jid{luser = LUser, lserver = LServer}, JID) ->
+    {Subscription, _} = ejabberd_hooks:run_fold(
+			  roster_get_jid_info, LServer, {none, []},
+			  [LUser, LServer, JID]),
+    Subscription.
+
 -spec resource_conflict_action(binary(), binary(), binary()) ->
 				      {accept_resource, binary()} | closenew.
 resource_conflict_action(U, S, R) ->
@@ -855,30 +897,6 @@ change_shaper(#{shaper := ShaperName, ip := IP, lserver := LServer,
 				LServer),
     xmpp_stream_in:change_shaper(State, Shaper).
 
--spec add_to_pres_a(state(), jid()) -> state().
-add_to_pres_a(#{pres_a := PresA, pres_f := PresF} = State, From) ->
-    LFrom = jid:tolower(From),
-    LBFrom = jid:remove_resource(LFrom),
-    case (?SETS):is_element(LFrom, PresA) orelse
-	 (?SETS):is_element(LBFrom, PresA) of
-	true ->
-	    State;
-	false ->
-	    case (?SETS):is_element(LFrom, PresF) of
-		true ->
-		    A = (?SETS):add_element(LFrom, PresA),
-		    State#{pres_a => A};
-		false ->
-		    case (?SETS):is_element(LBFrom, PresF) of
-			true ->
-			    A = (?SETS):add_element(LBFrom, PresA),
-			    State#{pres_a => A};
-			false ->
-			    State
-		    end
-	    end
-    end.
-
 -spec format_reason(state(), term()) -> binary().
 format_reason(#{stop_reason := Reason}, _) ->
     xmpp_stream_in:format_error(Reason);

src/ejabberd_sm.erl

@@ -858,7 +858,7 @@ force_update_presence({LUser, LServer}) ->
     Mod = get_sm_backend(LServer),
     Ss = online(get_sessions(Mod, LUser, LServer)),
     lists:foreach(fun (#session{sid = {_, Pid}}) ->
-			  ejabberd_c2s:route(Pid, force_update_presence)
+			  ejabberd_c2s:resend_presence(Pid)
 		  end,
 		  Ss).
 

src/mod_block_strangers.erl

@@ -57,23 +57,19 @@ filter_packet({#message{} = Msg, State} = Acc) ->
     From = xmpp:get_from(Msg),
     LFrom = jid:tolower(From),
     LBFrom = jid:remove_resource(LFrom),
-    #{pres_a := PresA,
+    #{pres_a := PresA, jid := JID, lserver := LServer} = State,
-      pres_t := PresT,
-      pres_f := PresF} = State,
     case (Msg#message.body == [] andalso
           Msg#message.subject == [])
         orelse ejabberd_router:is_my_route(From#jid.lserver)
         orelse (?SETS):is_element(LFrom, PresA)
         orelse (?SETS):is_element(LBFrom, PresA)
-        orelse sets_bare_member(LBFrom, PresA)
+        orelse sets_bare_member(LBFrom, PresA) of
-        orelse (?SETS):is_element(LFrom, PresT)
-        orelse (?SETS):is_element(LBFrom, PresT)
-        orelse (?SETS):is_element(LFrom, PresF)
-        orelse (?SETS):is_element(LBFrom, PresF) of
-	true ->
-	    Acc;
 	false ->
-            #{lserver := LServer} = State,
+	    {Sub, _} = ejabberd_hooks:run_fold(
+			 roster_get_jid_info, LServer,
+			 {none, []}, [JID#jid.luser, LServer, From]),
+	    case Sub of
+		none ->
 		    Drop = gen_mod:get_module_opt(LServer, ?MODULE, drop, true),
 		    Log = gen_mod:get_module_opt(LServer, ?MODULE, log, false),
 		    if
@@ -89,7 +85,12 @@ filter_packet({#message{} = Msg, State} = Acc) ->
 			    {stop, {drop, State}};
 			true ->
 			    Acc
-            end
+		    end;
+		_ ->
+		    Acc
+	    end;
+	true ->
+	    Acc
     end;
 filter_packet(Acc) ->
     Acc.

src/mod_caps.erl

@@ -203,7 +203,9 @@ disco_info(Acc, _, _, _Node, _Lang) ->
 -spec c2s_presence_in(ejabberd_c2s:state(), presence()) -> ejabberd_c2s:state().
 c2s_presence_in(C2SState,
 		#presence{from = From, to = To, type = Type} = Presence) ->
-    Subscription = ejabberd_c2s:get_subscription(From, C2SState),
+    {Subscription, _} = ejabberd_hooks:run_fold(
+			  roster_get_jid_info, To#jid.lserver,
+			  {none, []}, [To#jid.luser, To#jid.lserver, From]),
     Insert = (Type == available)
 	       and ((Subscription == both) or (Subscription == to)),
     Delete = (Type == unavailable) or (Type == error),

src/mod_last.erl

@@ -157,7 +157,10 @@ privacy_check_packet(allow, C2SState,
   when T == get; T == set ->
     case xmpp:has_subtag(IQ, #last{}) of
 	true ->
-	    Sub = ejabberd_c2s:get_subscription(From, C2SState),
+	    #jid{luser = LUser, lserver = LServer} = To,
+	    {Sub, _} = ejabberd_hooks:run_fold(
+			 roster_get_jid_info, LServer,
+			 {none, []}, [LUser, LServer, From]),
 	    if Sub == from; Sub == both ->
 		    Pres = #presence{from = To, to = From},
 		    case ejabberd_hooks:run_fold(

src/mod_mam.erl

@@ -255,7 +255,7 @@ set_room_option(_Acc, {mam, Val}, _Lang) ->
 set_room_option(Acc, _Property, _Lang) ->
     Acc.
 
--spec user_receive_packet({stanza(), ejabberd_c2s:state()}) -> {stanza(), ejabberd_c2s:state()}.
+-spec user_receive_packet({stanza(), c2s_state()}) -> {stanza(), c2s_state()}.
 user_receive_packet({Pkt, #{jid := JID} = C2SState}) ->
     Peer = xmpp:get_from(Pkt),
     LUser = JID#jid.luser,
@@ -263,7 +263,7 @@ user_receive_packet({Pkt, #{jid := JID} = C2SState}) ->
     Pkt2 = case should_archive(Pkt, LServer) of
 	true ->
 		   Pkt1 = strip_my_archived_tag(Pkt, LServer),
-		   case store_msg(C2SState, Pkt1, LUser, LServer, Peer, recv) of
+		   case store_msg(Pkt1, LUser, LServer, Peer, recv) of
 		{ok, ID} ->
 			   set_stanza_id(Pkt1, JID, ID);
 		_ ->
@@ -274,7 +274,7 @@ user_receive_packet({Pkt, #{jid := JID} = C2SState}) ->
 	   end,
     {Pkt2, C2SState}.
 
--spec user_send_packet({stanza(), ejabberd_c2s:state()}) -> {stanza(), ejabberd_c2s:state()}.
+-spec user_send_packet({stanza(), c2s_state()}) -> {stanza(), c2s_state()}.
 user_send_packet({Pkt, #{jid := JID} = C2SState}) ->
     Peer = xmpp:get_to(Pkt),
     LUser = JID#jid.luser,
@@ -282,7 +282,7 @@ user_send_packet({Pkt, #{jid := JID} = C2SState}) ->
     Pkt2 = case should_archive(Pkt, LServer) of
 	true ->
 		   Pkt1 = strip_my_archived_tag(Pkt, LServer),
-		   case store_msg(C2SState, xmpp:set_from_to(Pkt1, JID, Peer),
+		   case store_msg(xmpp:set_from_to(Pkt1, JID, Peer),
 		      LUser, LServer, Peer, send) of
               {ok, ID} ->
 			   set_stanza_id(Pkt1, JID, ID);
@@ -301,7 +301,7 @@ offline_message({_Action, #message{from = Peer, to = To} = Pkt} = Acc) ->
     case should_archive(Pkt, LServer) of
 	true ->
 	    Pkt1 = strip_my_archived_tag(Pkt, LServer),
-	    case store_msg(undefined, Pkt1, LUser, LServer, Peer, recv) of
+	    case store_msg(Pkt1, LUser, LServer, Peer, recv) of
 		{ok, ID} ->
 		    {archived, set_stanza_id(Pkt1, To, ID)};
 		_ ->
@@ -311,8 +311,8 @@ offline_message({_Action, #message{from = Peer, to = To} = Pkt} = Acc) ->
 	    Acc
     end.
 
--spec user_send_packet_strip_tag({stanza(), ejabberd_c2s:state()}) ->
+-spec user_send_packet_strip_tag({stanza(), c2s_state()}) ->
-					{stanza(), ejabberd_c2s:state()}.
+					{stanza(), c2s_state()}.
 user_send_packet_strip_tag({Pkt, #{jid := JID} = C2SState}) ->
     LServer = JID#jid.lserver,
     {strip_my_archived_tag(Pkt, LServer), C2SState}.
@@ -415,16 +415,16 @@ disco_sm_features({result, OtherFeatures},
 disco_sm_features(Acc, _From, _To, _Node, _Lang) ->
     Acc.
 
--spec message_is_archived(boolean(), ejabberd_c2s:state(), message()) -> boolean().
+-spec message_is_archived(boolean(), c2s_state(), message()) -> boolean().
 message_is_archived(true, _C2SState, _Pkt) ->
     true;
-message_is_archived(false, #{jid := JID} = C2SState, Pkt) ->
+message_is_archived(false, #{jid := JID}, Pkt) ->
     #jid{luser = LUser, lserver = LServer} = JID,
     Peer = xmpp:get_from(Pkt),
     case gen_mod:get_module_opt(LServer, ?MODULE, assume_mam_usage, false) of
 	true ->
 	    should_archive(strip_my_archived_tag(Pkt, LServer), LServer)
-		andalso should_archive_peer(C2SState, LUser, LServer,
+		andalso should_archive_peer(LUser, LServer,
 					    get_prefs(LUser, LServer),
 					    Peer);
 	false ->
@@ -615,9 +615,9 @@ strip_x_jid_tags(Pkt) ->
 	       end, Els),
     xmpp:set_els(Pkt, NewEls).
 
--spec should_archive_peer(c2s_state() | undefined, binary(), binary(),
+-spec should_archive_peer(binary(), binary(),
 			  #archive_prefs{}, jid()) -> boolean().
-should_archive_peer(C2SState, LUser, LServer,
+should_archive_peer(LUser, LServer,
 		    #archive_prefs{default = Default,
 				   always = Always,
 				   never = Never},
@@ -635,23 +635,11 @@ should_archive_peer(C2SState, LUser, LServer,
 			always -> true;
 			never -> false;
 			roster ->
-			    Sub = case C2SState of
+			    {Sub, _} = ejabberd_hooks:run_fold(
-				      undefined ->
-					  {S, _} = ejabberd_hooks:run_fold(
 					 roster_get_jid_info,
 					 LServer, {none, []},
 					 [LUser, LServer, Peer]),
-					  S;
+			    Sub == both orelse Sub == from orelse Sub == to
-				      _ ->
-					  ejabberd_c2s:get_subscription(
-					    LPeer, C2SState)
-				  end,
-			    case Sub of
-				both -> true;
-				from -> true;
-				to -> true;
-				_ -> false
-			    end
 		    end
 	    end
     end.
@@ -719,12 +707,12 @@ may_enter_room(From,
 may_enter_room(From, MUCState) ->
     mod_muc_room:is_occupant_or_admin(From, MUCState).
 
--spec store_msg(c2s_state() | undefined, stanza(),
+-spec store_msg(stanza(),
 		binary(), binary(), jid(), send | recv) ->
 		       {ok, binary()} | pass.
-store_msg(C2SState, Pkt, LUser, LServer, Peer, Dir) ->
+store_msg(Pkt, LUser, LServer, Peer, Dir) ->
     Prefs = get_prefs(LUser, LServer),
-    case should_archive_peer(C2SState, LUser, LServer, Prefs, Peer) of
+    case should_archive_peer(LUser, LServer, Prefs, Peer) of
 	true ->
 	    US = {LUser, LServer},
 	    case ejabberd_hooks:run_fold(store_mam_message, LServer, Pkt,

src/mod_pubsub.erl

@@ -43,6 +43,7 @@
 -include("logger.hrl").
 -include("xmpp.hrl").
 -include("pubsub.hrl").
+-include("mod_roster.hrl").
 
 -define(STDTREE, <<"tree">>).
 -define(STDNODE, <<"flat">>).
@@ -405,9 +406,19 @@ terminate_plugins(Host, ServerHost, Plugins, TreePlugin) ->
     TreePlugin:terminate(Host, ServerHost),
     ok.
 
+get_subscribed(User, Server) ->
+    Items = ejabberd_hooks:run_fold(roster_get, Server, [], [{User, Server}]),
+    lists:filtermap(
+      fun(#roster{jid = LJID, subscription = Sub})
+	    when Sub == both orelse Sub == from ->
+	      {true, LJID};
+	 (_) ->
+	      false
+      end, Items).
+
 send_loop(State) ->
     receive
-	{presence, JID, Pid} ->
+	{presence, JID, _Pid} ->
 	    Host = State#state.host,
 	    ServerHost = State#state.server_host,
 	    DBType = State#state.db_type,
@@ -429,8 +440,7 @@ send_loop(State) ->
 		State#state.plugins),
 	    if not State#state.ignore_pep_from_offline ->
 		    {User, Server, Resource} = LJID,
-		    case catch ejabberd_c2s:get_subscribed(Pid) of
+		    Contacts = get_subscribed(User, Server),
-			Contacts when is_list(Contacts) ->
 		    lists:foreach(
 		      fun({U, S, R}) when S == ServerHost ->
 			      case user_resources(U, S) of
@@ -438,17 +448,13 @@ send_loop(State) ->
 				      PeerJID = jid:make(U, S, R),
 				      self() !  {presence, User, Server, [Resource], PeerJID};
 				  _ -> %% online
-						% this is already handled by presence probe
+				      %% this is already handled by presence probe
 				      ok
 			      end;
 			 (_) ->
-					% we can not do anything in any cases
+			      %% we can not do anything in any cases
 			      ok
-				end,
+		      end, Contacts);
-				Contacts);
-			_ ->
-			    ok
-		    end;
 		true ->
 		    ok
 	    end,

src/mod_roster.erl

@@ -43,8 +43,8 @@
 
 -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, c2s_session_opened/1, get_roster/2,
+	 import/5, get_roster/2,
-	 import_start/2, import_stop/2, user_receive_packet/1,
+	 import_start/2, import_stop/2,
 	 c2s_self_presence/1, in_subscription/6,
 	 out_subscription/4, set_items/3, remove_user/2,
 	 get_jid_info/4, encode_item/1, webadmin_page/3,
@@ -63,38 +63,41 @@
 
 -include("ejabberd_web_admin.hrl").
 
--define(SETS, gb_sets).
+-define(ROSTER_CACHE, roster_cache).
+-define(ROSTER_ITEM_CACHE, roster_item_cache).
+-define(ROSTER_VERSION_CACHE, roster_version_cache).
 
 -export_type([subscription/0]).
 
 -callback init(binary(), gen_mod:opts()) -> any().
 -callback import(binary(), binary(), #roster{} | [binary()]) -> ok.
--callback read_roster_version(binary(), binary()) -> binary() | error.
+-callback read_roster_version(binary(), binary()) -> {ok, binary()} | error.
 -callback write_roster_version(binary(), binary(), boolean(), binary()) -> any().
--callback get_roster(binary(), binary()) -> [#roster{}].
+-callback get_roster(binary(), binary()) -> {ok, [#roster{}]} | error.
--callback get_roster_by_jid(binary(), binary(), ljid()) -> #roster{}.
+-callback get_roster_item(binary(), binary(), ljid()) -> {ok, #roster{}} | error.
--callback get_only_items(binary(), binary()) -> [#roster{}].
+-callback read_subscription_and_groups(binary(), binary(), ljid())
+          -> {ok, {subscription(), [binary()]}} | error.
 -callback roster_subscribe(binary(), binary(), ljid(), #roster{}) -> any().
 -callback transaction(binary(), function()) -> {atomic, any()} | {aborted, any()}.
--callback get_roster_by_jid_with_groups(binary(), binary(), ljid()) -> #roster{}.
+-callback remove_user(binary(), binary()) -> any().
--callback remove_user(binary(), binary()) -> {atomic, any()}.
 -callback update_roster(binary(), binary(), ljid(), #roster{}) -> any().
 -callback del_roster(binary(), binary(), ljid()) -> any().
--callback read_subscription_and_groups(binary(), binary(), ljid()) ->
+-callback use_cache(binary(), roster | roster_version) -> boolean().
-    {subscription(), [binary()]}.
+-callback cache_nodes(binary()) -> [node()].
+
+-optional_callbacks([use_cache/2, cache_nodes/1]).
 
 start(Host, Opts) ->
     IQDisc = gen_mod:get_opt(iqdisc, Opts, gen_iq_handler:iqdisc(Host)),
     Mod = gen_mod:db_mod(Host, Opts, ?MODULE),
     Mod:init(Host, Opts),
+    init_cache(Mod, Host, Opts),
     ejabberd_hooks:add(roster_get, Host, ?MODULE,
 		       get_user_roster, 50),
     ejabberd_hooks:add(roster_in_subscription, Host,
 		       ?MODULE, in_subscription, 50),
     ejabberd_hooks:add(roster_out_subscription, Host,
 		       ?MODULE, out_subscription, 50),
-    ejabberd_hooks:add(c2s_session_opened, Host, ?MODULE,
-		       c2s_session_opened, 50),
     ejabberd_hooks:add(roster_get_jid_info, Host, ?MODULE,
 		       get_jid_info, 50),
     ejabberd_hooks:add(remove_user, Host, ?MODULE,
@@ -107,8 +110,6 @@ start(Host, Opts) ->
 		       webadmin_page, 50),
     ejabberd_hooks:add(webadmin_user, Host, ?MODULE,
 		       webadmin_user, 50),
-    ejabberd_hooks:add(user_receive_packet, Host, ?MODULE,
-		       user_receive_packet, 50),
     gen_iq_handler:add_iq_handler(ejabberd_sm, Host,
 				  ?NS_ROSTER, ?MODULE, process_iq, IQDisc).
 
@@ -119,8 +120,6 @@ stop(Host) ->
 			  ?MODULE, in_subscription, 50),
     ejabberd_hooks:delete(roster_out_subscription, Host,
 			  ?MODULE, out_subscription, 50),
-    ejabberd_hooks:delete(c2s_session_opened, Host, ?MODULE,
-			  c2s_session_opened, 50),
     ejabberd_hooks:delete(roster_get_jid_info, Host,
 			  ?MODULE, get_jid_info, 50),
     ejabberd_hooks:delete(remove_user, Host, ?MODULE,
@@ -133,8 +132,6 @@ stop(Host) ->
 			  webadmin_page, 50),
     ejabberd_hooks:delete(webadmin_user, Host, ?MODULE,
 			  webadmin_user, 50),
-    ejabberd_hooks:delete(user_receive_packet, Host, ?MODULE,
-			  user_receive_packet, 50),
     gen_iq_handler:remove_iq_handler(ejabberd_sm, Host,
 				     ?NS_ROSTER).
 
@@ -244,7 +241,7 @@ roster_version(LServer, LUser) ->
       true ->
 	  case read_roster_version(LUser, LServer) of
 	    error -> not_found;
-	    V -> V
+	    {ok, V} -> V
 	  end;
       false ->
 	  roster_hash(ejabberd_hooks:run_fold(roster_get, LServer,
@@ -252,8 +249,12 @@ roster_version(LServer, LUser) ->
     end.
 
 read_roster_version(LUser, LServer) ->
+    ets_cache:lookup(
+      ?ROSTER_VERSION_CACHE, {LUser, LServer},
+      fun() ->
 	      Mod = gen_mod:db_mod(LServer, ?MODULE),
-    Mod:read_roster_version(LUser, LServer).
+	      Mod:read_roster_version(LUser, LServer)
+      end).
 
 write_roster_version(LUser, LServer) ->
     write_roster_version(LUser, LServer, false).
@@ -265,6 +266,11 @@ write_roster_version(LUser, LServer, InTransaction) ->
     Ver = str:sha(term_to_binary(p1_time_compat:unique_integer())),
     Mod = gen_mod:db_mod(LServer, ?MODULE),
     Mod:write_roster_version(LUser, LServer, InTransaction, Ver),
+    if InTransaction -> ok;
+       true ->
+	    ets_cache:delete(?ROSTER_VERSION_CACHE, {LUser, LServer},
+			     cache_nodes(Mod, LServer))
+    end,
     Ver.
 
 %% Load roster from DB only if neccesary.
@@ -289,9 +295,9 @@ process_iq_get(#iq{to = To, lang = Lang,
 					ejabberd_hooks:run_fold(
 					  roster_get, To#jid.lserver, [], [US])),
 			      RosterVersion};
-			 RequestedVersion ->
+			 {ok, RequestedVersion} ->
 			     {false, false};
-			 NewVersion ->
+			 {ok, NewVersion} ->
 			     {lists:map(fun encode_item/1,
 					ejabberd_hooks:run_fold(
 					  roster_get, To#jid.lserver, [], [US])),
@@ -343,18 +349,72 @@ get_user_roster(Acc, {LUser, LServer}) ->
 
 get_roster(LUser, LServer) ->
     Mod = gen_mod:db_mod(LServer, ?MODULE),
-    Mod:get_roster(LUser, LServer).
+    R = case use_cache(Mod, LServer, roster) of
+	    true ->
+		ets_cache:lookup(
+		  ?ROSTER_CACHE, {LUser, LServer},
+		  fun() -> Mod:get_roster(LUser, LServer) end);
+	    false ->
+		Mod:get_roster(LUser, LServer)
+	end,
+    case R of
+	{ok, Items} -> Items;
+	error -> []
+    end.
+
+get_roster_item(LUser, LServer, LJID) ->
+    Mod = gen_mod:db_mod(LServer, ?MODULE),
+    case Mod:get_roster_item(LUser, LServer, LJID) of
+	{ok, Item} ->
+	    Item;
+	error ->
+	    #roster{usj = {LUser, LServer, LJID},
+		    us = {LUser, LServer}, jid = LJID}
+    end.
+
+get_subscription_and_groups(LUser, LServer, LJID) ->
+    Mod = gen_mod:db_mod(LServer, ?MODULE),
+    Res = case use_cache(Mod, LServer, roster) of
+	      true ->
+		  LBJID = jid:remove_resource(LJID),
+		  ets_cache:lookup(
+		    ?ROSTER_ITEM_CACHE, {LUser, LServer, LBJID},
+		    fun() ->
+			    Items = get_roster(LUser, LServer),
+			    case lists:keyfind(LBJID, #roster.jid, Items) of
+				#roster{subscription = Sub, groups = Groups} ->
+				    {ok, {Sub, Groups}};
+				false when element(3, LJID) == <<"">> ->
+				    error;
+				false ->
+				    case lists:keyfind(LJID, #roster.jid, Items) of
+					{Sub, Groups} ->
+					    {ok, {Sub, Groups}};
+					false ->
+					    error
+				    end
+			    end
+		    end);
+	      false ->
+		  Mod:read_subscription_and_groups(LUser, LServer, LJID)
+	  end,
+    case Res of
+	{ok, SubAndGroups} ->
+	    SubAndGroups;
+	error ->
+	    {none, []}
+    end.
 
 set_roster(#roster{us = {LUser, LServer}, jid = LJID} = Item) ->
     transaction(
-      LServer,
+      LUser, LServer, [LJID],
       fun() ->
 	      update_roster_t(LUser, LServer, LJID, Item)
       end).
 
 del_roster(LUser, LServer, LJID) ->
     transaction(
-      LServer,
+      LUser, LServer, [LJID],
       fun() ->
 	      del_roster_t(LUser, LServer, LJID)
       end).
@@ -387,19 +447,13 @@ decode_item(Item, R, Managed) ->
 			    end,
 	     groups = Item#roster_item.groups}.
 
-get_roster_by_jid_t(LUser, LServer, LJID) ->
-    Mod = gen_mod:db_mod(LServer, ?MODULE),
-    Mod:get_roster_by_jid(LUser, LServer, LJID).
-
 process_iq_set(#iq{from = From, to = To,
-		   sub_els = [#roster_query{items = QueryItems}]} = IQ) ->
+		   sub_els = [#roster_query{items = [QueryItem]}]} = IQ) ->
     #jid{user = User, luser = LUser, lserver = LServer} = To,
     Managed = {From#jid.luser, From#jid.lserver} /= {LUser, LServer},
+    LJID = jid:tolower(QueryItem#roster_item.jid),
     F = fun () ->
-		lists:map(
+		Item = get_roster_item(LUser, LServer, LJID),
-		  fun(#roster_item{jid = JID1} = QueryItem) ->
-			  LJID = jid:tolower(JID1),
-			  Item = get_roster_by_jid_t(LUser, LServer, LJID),
 		Item2 = decode_item(QueryItem, Item, Managed),
 		Item3 = ejabberd_hooks:run_fold(roster_process_item,
 						LServer, Item2,
@@ -413,20 +467,16 @@ process_iq_set(#iq{from = From, to = To,
 		    false -> ok
 		end,
 		{Item, Item3}
-		  end, QueryItems)
 	end,
-    case transaction(LServer, F) of
+    case transaction(LUser, LServer, [LJID], F) of
-	{atomic, ItemPairs} ->
+	{atomic, {OldItem, Item}} ->
-	    lists:foreach(
+	    push_item(User, LServer, To, OldItem, Item),
-	      fun({OldItem, Item}) ->
-		      push_item(User, LServer, To, Item),
 	    case Item#roster.subscription of
 		remove ->
 		    send_unsubscribing_presence(To, OldItem);
 		_ ->
 		    ok
-		      end
+	    end,
-	      end, ItemPairs),
 	    xmpp:make_iq_result(IQ);
 	E ->
 	    ?ERROR_MSG("roster set failed:~nIQ = ~s~nError = ~p",
@@ -434,126 +484,66 @@ process_iq_set(#iq{from = From, to = To,
 	    xmpp:make_error(IQ, xmpp:err_internal_server_error())
     end.
 
-push_item(User, Server, From, Item) ->
+push_item(User, Server, From, OldItem, NewItem) ->
     case roster_versioning_enabled(Server) of
 	true ->
-	  push_item_version(Server, User, From, Item,
+	    push_item_version(Server, User, From, OldItem, NewItem,
 			      roster_version(Server, User));
 	false ->
-	  lists:foreach(fun (Resource) ->
+	    lists:foreach(
-				push_item(User, Server, Resource, From, Item)
+	      fun(Resource) ->
-			end,
+		      push_item(User, Server, Resource, From, OldItem, NewItem)
-			ejabberd_sm:get_user_resources(User, Server))
+	      end, ejabberd_sm:get_user_resources(User, Server))
     end.
 
-push_item(User, Server, Resource, From, Item) ->
+push_item(User, Server, Resource, From, OldItem, NewItem) ->
-    push_item(User, Server, Resource, From, Item,
+    push_item(User, Server, Resource, From, OldItem, NewItem, undefined).
-	      not_found).
 
-push_item(User, Server, Resource, From, Item,
+push_item(User, Server, Resource, From, OldItem, NewItem, Ver) ->
-	  RosterVersion) ->
-    Ver = case RosterVersion of
-	      not_found -> undefined;
-	      _ -> RosterVersion
-	  end,
     To = jid:make(User, Server, Resource),
+    route_presence_change(To, OldItem, NewItem),
     ResIQ = #iq{type = set, from = From, to = To,
 		id = <<"push", (randoms:get_string())/binary>>,
 		sub_els = [#roster_query{ver = Ver,
-					 items = [encode_item(Item)]}]},
+					 items = [encode_item(NewItem)]}]},
-    ejabberd_router:route(xmpp:put_meta(ResIQ, roster_item, Item)).
+    ejabberd_router:route(ResIQ).
 
-push_item_version(Server, User, From, Item,
+push_item_version(Server, User, From, OldItem, NewItem, RosterVersion) ->
-		  RosterVersion) ->
+    lists:foreach(
-    lists:foreach(fun (Resource) ->
+      fun(Resource) ->
-			  push_item(User, Server, Resource, From, Item,
+	      push_item(User, Server, Resource, From,
-				    RosterVersion)
+			OldItem, NewItem, RosterVersion)
-		  end,
+      end, ejabberd_sm:get_user_resources(User, Server)).
-		  ejabberd_sm:get_user_resources(User, Server)).
 
--spec user_receive_packet({stanza(), ejabberd_c2s:state()}) -> {stanza(), ejabberd_c2s:state()}.
+-spec route_presence_change(jid(), #roster{}, #roster{}) -> ok.
-user_receive_packet({#iq{type = set, meta = #{roster_item := Item}} = IQ, State}) ->
+route_presence_change(From, OldItem, NewItem) ->
-    {IQ, roster_change(State, Item)};
+    OldSub = OldItem#roster.subscription,
-user_receive_packet(Acc) ->
+    NewSub = NewItem#roster.subscription,
-    Acc.
+    To = jid:make(NewItem#roster.jid),
-
+    NewIsFrom = NewSub == both orelse NewSub == from,
--spec roster_change(ejabberd_c2s:state(), #roster{}) -> ejabberd_c2s:state().
+    OldIsFrom = OldSub == both orelse OldSub == from,
-roster_change(#{user := U, server := S, resource := R,
+    if NewIsFrom andalso not OldIsFrom ->
-		pres_a := PresA, pres_f := PresF, pres_t := PresT} = State,
+	    case ejabberd_sm:get_session_pid(
-	      #roster{jid = IJID, subscription = ISubscription}) ->
+		   From#jid.luser, From#jid.lserver, From#jid.lresource) of
-    LIJID = jid:tolower(IJID),
+		none ->
-    IsFrom = (ISubscription == both) or (ISubscription == from),
-    IsTo = (ISubscription == both) or (ISubscription == to),
-    OldIsFrom = ?SETS:is_element(LIJID, PresF),
-    FSet = if IsFrom -> ?SETS:add_element(LIJID, PresF);
-	      true -> ?SETS:del_element(LIJID, PresF)
-	   end,
-    TSet = if IsTo -> ?SETS:add_element(LIJID, PresT);
-	      true -> ?SETS:del_element(LIJID, PresT)
-	   end,
-    State1 = State#{pres_f => FSet, pres_t => TSet},
-    case maps:get(pres_last, State, undefined) of
-	undefined ->
-	    State1;
-	LastPres ->
-	    From = jid:make(U, S, R),
-	    To = jid:make(IJID),
-	    Cond1 = IsFrom andalso not OldIsFrom,
-	    Cond2 = not IsFrom andalso OldIsFrom andalso
-		?SETS:is_element(LIJID, PresA),
-	    if Cond1 ->
-		    case ejabberd_hooks:run_fold(
-			   privacy_check_packet, allow,
-			   [State1, LastPres, out]) of
-			deny ->
 		    ok;
-			allow ->
+		Pid ->
-			    Pres = xmpp:set_from_to(LastPres, From, To),
+		    ejabberd_c2s:resend_presence(Pid, To)
-			    ejabberd_router:route(Pres)
+	    end;
-		    end,
+       OldIsFrom andalso not NewIsFrom ->
-		    A = ?SETS:add_element(LIJID, PresA),
-		    State1#{pres_a => A};
-	       Cond2 ->
 	    PU = #presence{from = From, to = To, type = unavailable},
 	    case ejabberd_hooks:run_fold(
 		   privacy_check_packet, allow,
-			   [State1, PU, out]) of
+		   [From, PU, out]) of
 		deny ->
 		    ok;
 		allow ->
 		    ejabberd_router:route(PU)
-		    end,
+	    end;
-		    A = ?SETS:del_element(LIJID, PresA),
-		    State1#{pres_a => A};
        true ->
-		    State1
+	    ok
-	    end
     end.
 
--spec c2s_session_opened(ejabberd_c2s:state()) -> ejabberd_c2s:state().
-c2s_session_opened(#{jid := #jid{luser = LUser, lserver = LServer},
-		     pres_f := PresF, pres_t := PresT} = State) ->
-    Mod = gen_mod:db_mod(LServer, ?MODULE),
-    Items = Mod:get_only_items(LUser, LServer),
-    {F, T} = fill_subscription_lists(Items, PresF, PresT),
-    State#{pres_f => F, pres_t => T}.
-
-fill_subscription_lists([I | Is], F, T) ->
-    J = element(3, I#roster.usj),
-    {F1, T1} = case I#roster.subscription of
-	both ->
-		       {?SETS:add_element(J, F), ?SETS:add_element(J, T)};
-	from ->
-		       {?SETS:add_element(J, F), T};
-		   to ->
-		       {F, ?SETS:add_element(J, T)};
-		   _ ->
-		       {F, T}
-	       end,
-    fill_subscription_lists(Is, F1, T1);
-fill_subscription_lists([], F, T) ->
-    {F, T}.
-
 ask_to_pending(subscribe) -> out;
 ask_to_pending(unsubscribe) -> none;
 ask_to_pending(Ask) -> Ask.
@@ -562,9 +552,15 @@ roster_subscribe_t(LUser, LServer, LJID, Item) ->
     Mod = gen_mod:db_mod(LServer, ?MODULE),
     Mod:roster_subscribe(LUser, LServer, LJID, Item).
 
-transaction(LServer, F) ->
+transaction(LUser, LServer, LJIDs, F) ->
     Mod = gen_mod:db_mod(LServer, ?MODULE),
-    Mod:transaction(LServer, F).
+    case Mod:transaction(LServer, F) of
+	{atomic, _} = Result ->
+	    delete_cache(Mod, LUser, LServer, LJIDs),
+	    Result;
+	Err ->
+	    Err
+    end.
 
 -spec in_subscription(boolean(), binary(), binary(), jid(),
 		      subscribe | subscribed | unsubscribe | unsubscribed,
@@ -579,18 +575,13 @@ in_subscription(_, User, Server, JID, Type, Reason) ->
 out_subscription(User, Server, JID, Type) ->
     process_subscription(out, User, Server, JID, Type, <<"">>).
 
-get_roster_by_jid_with_groups_t(LUser, LServer, LJID) ->
-    Mod = gen_mod:db_mod(LServer, ?MODULE),
-    Mod:get_roster_by_jid_with_groups(LUser, LServer, LJID).
-
 process_subscription(Direction, User, Server, JID1,
 		     Type, Reason) ->
     LUser = jid:nodeprep(User),
     LServer = jid:nameprep(Server),
     LJID = jid:tolower(JID1),
     F = fun () ->
-		Item = get_roster_by_jid_with_groups_t(LUser, LServer,
+		Item = get_roster_item(LUser, LServer, LJID),
-						       LJID),
 		NewState = case Direction of
 			     out ->
 				 out_state_change(Item#roster.subscription,
@@ -611,9 +602,9 @@ process_subscription(Direction, User, Server, JID1,
 			       _ -> <<"">>
 			     end,
 		case NewState of
-		  none -> {none, AutoReply};
+		    none ->
-		  {none, none}
+			{none, AutoReply};
-		      when Item#roster.subscription == none,
+		    {none, none} when Item#roster.subscription == none,
 				      Item#roster.ask == in ->
 			del_roster_t(LUser, LServer, LJID), {none, AutoReply};
 		    {Subscription, Pending} ->
@@ -625,10 +616,10 @@ process_subscription(Direction, User, Server, JID1,
 			    true -> write_roster_version_t(LUser, LServer);
 			    false -> ok
 			end,
-		      {{push, NewItem}, AutoReply}
+			{{push, Item, NewItem}, AutoReply}
 		end
 	end,
-    case transaction(LServer, F) of
+    case transaction(LUser, LServer, [LJID], F) of
 	{atomic, {Push, AutoReply}} ->
 	    case AutoReply of
 		none -> ok;
@@ -639,18 +630,20 @@ process_subscription(Direction, User, Server, JID1,
 				to = JID1})
 	    end,
 	    case Push of
-	    {push, Item} ->
+		{push, OldItem, NewItem} ->
-		if Item#roster.subscription == none,
+		    if NewItem#roster.subscription == none,
-		   Item#roster.ask == in ->
+		       NewItem#roster.ask == in ->
 			    ok;
 		       true ->
 			    push_item(User, Server,
-				 jid:make(User, Server), Item)
+				      jid:make(User, Server), OldItem, NewItem)
 		    end,
 		    true;
-	    none -> false
+		none ->
+		    false
 	    end;
-      _ -> false
+	_ ->
+	    false
     end.
 
 %% in_state_change(Subscription, Pending, Type) -> NewState
@@ -772,16 +765,16 @@ in_auto_reply(_, _, _) -> none.
 remove_user(User, Server) ->
     LUser = jid:nodeprep(User),
     LServer = jid:nameprep(Server),
-    send_unsubscription_to_rosteritems(LUser, LServer),
+    Items = get_user_roster([], {LUser, LServer}),
+    send_unsubscription_to_rosteritems(LUser, LServer, Items),
     Mod = gen_mod:db_mod(LServer, ?MODULE),
     Mod:remove_user(LUser, LServer),
-    ok.
+    delete_cache(Mod, LUser, LServer, [Item#roster.jid || Item <- Items]).
 
 %% For each contact with Subscription:
 %% Both or From, send a "unsubscribed" presence stanza;
 %% Both or To, send a "unsubscribe" presence stanza.
-send_unsubscription_to_rosteritems(LUser, LServer) ->
+send_unsubscription_to_rosteritems(LUser, LServer, RosterItems) ->
-    RosterItems = get_user_roster([], {LUser, LServer}),
     From = jid:make({LUser, LServer, <<"">>}),
     lists:foreach(fun (RosterItem) ->
 			  send_unsubscribing_presence(From, RosterItem)
@@ -821,12 +814,14 @@ send_unsubscribing_presence(From, Item) ->
 set_items(User, Server, #roster_query{items = Items}) ->
     LUser = jid:nodeprep(User),
     LServer = jid:nameprep(Server),
+    LJIDs = [jid:tolower(Item#roster_item.jid) || Item <- Items],
     F = fun () ->
-		lists:foreach(fun (Item) ->
+		lists:foreach(
+		  fun(Item) ->
 			  process_item_set_t(LUser, LServer, Item)
 		  end, Items)
 	end,
-    transaction(LServer, F).
+    transaction(LUser, LServer, LJIDs, F).
 
 update_roster_t(LUser, LServer, LJID, Item) ->
     Mod = gen_mod:db_mod(LServer, ?MODULE),
@@ -856,8 +851,7 @@ c2s_self_presence({#presence{type = available} = Pkt,
 		   #{lserver := LServer} = State}) ->
     Prio = get_priority_from_presence(Pkt),
     if Prio >= 0 ->
-    Mod = gen_mod:db_mod(LServer, ?MODULE),
+	    State1 = resend_pending_subscriptions(State),
-	    State1 = resend_pending_subscriptions(State, Mod),
 	    {Pkt, State1};
        true ->
 	    {Pkt, State}
@@ -865,10 +859,10 @@ c2s_self_presence({#presence{type = available} = Pkt,
 c2s_self_presence(Acc) ->
     Acc.
 
--spec resend_pending_subscriptions(ejabberd_c2s:state(), module()) -> ejabberd_c2s:state().
+-spec resend_pending_subscriptions(ejabberd_c2s:state()) -> ejabberd_c2s:state().
-resend_pending_subscriptions(#{jid := JID} = State, Mod) ->
+resend_pending_subscriptions(#{jid := JID} = State) ->
     BareJID = jid:remove_resource(JID),
-    Result = Mod:get_only_items(JID#jid.luser, JID#jid.lserver),
+    Result = get_roster(JID#jid.luser, JID#jid.lserver),
     lists:foldl(
       fun(#roster{ask = Ask} = R, AccState) when Ask == in; Ask == both ->
 		    Message = R#roster.askmessage,
@@ -892,30 +886,13 @@ get_priority_from_presence(#presence{priority = Prio}) ->
     end.
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
-
-read_subscription_and_groups(User, Server, LJID) ->
-    LUser = jid:nodeprep(User),
-    LServer = jid:nameprep(Server),
-    Mod = gen_mod:db_mod(LServer, ?MODULE),
-    Mod:read_subscription_and_groups(LUser, LServer, LJID).
-
 -spec get_jid_info({subscription(), [binary()]}, binary(), binary(), jid())
       -> {subscription(), [binary()]}.
 get_jid_info(_, User, Server, JID) ->
+    LUser = jid:nodeprep(User),
+    LServer = jid:nameprep(Server),
     LJID = jid:tolower(JID),
-    case read_subscription_and_groups(User, Server, LJID) of
+    get_subscription_and_groups(LUser, LServer, LJID).
-      {Subscription, Groups} -> {Subscription, Groups};
-      error ->
-	  LRJID = jid:tolower(jid:remove_resource(JID)),
-	  if LRJID == LJID -> {none, []};
-	     true ->
-		 case read_subscription_and_groups(User, Server, LRJID)
-		     of
-		   {Subscription, Groups} -> {Subscription, Groups};
-		   error -> {none, []}
-		 end
-	  end
-    end.
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 
@@ -1118,6 +1095,81 @@ has_duplicated_groups(Groups) ->
     GroupsPrep = lists:usort([jid:resourceprep(G) || G <- Groups]),
     not (length(GroupsPrep) == length(Groups)).
 
+-spec init_cache(module(), binary(), gen_mod:opts()) -> ok.
+init_cache(Mod, Host, Opts) ->
+    CacheOpts = cache_opts(Host, Opts),
+    case use_cache(Mod, Host, roster_version) of
+	true ->
+	    ets_cache:new(?ROSTER_VERSION_CACHE, CacheOpts);
+	false ->
+	    ets_cache:delete(?ROSTER_VERSION_CACHE)
+    end,
+    case use_cache(Mod, Host, roster) of
+	true ->
+	    ets_cache:new(?ROSTER_CACHE, CacheOpts),
+	    ets_cache:new(?ROSTER_ITEM_CACHE, CacheOpts);
+	false ->
+	    ets_cache:delete(?ROSTER_CACHE),
+	    ets_cache:delete(?ROSTER_ITEM_CACHE)
+    end.
+
+-spec cache_opts(binary(), gen_mod:opts()) -> [proplists:property()].
+cache_opts(Host, Opts) ->
+    MaxSize = gen_mod:get_opt(
+		cache_size, Opts,
+		ejabberd_config:cache_size(Host)),
+    CacheMissed = gen_mod:get_opt(
+		    cache_missed, Opts,
+		    ejabberd_config:cache_missed(Host)),
+    LifeTime = case gen_mod:get_opt(
+		      cache_life_time, Opts,
+		      ejabberd_config:cache_life_time(Host)) of
+		   infinity -> infinity;
+		   I -> timer:seconds(I)
+	       end,
+    [{max_size, MaxSize}, {cache_missed, CacheMissed}, {life_time, LifeTime}].
+
+-spec use_cache(module(), binary(), roster | roster_version) -> boolean().
+use_cache(Mod, Host, Table) ->
+    case erlang:function_exported(Mod, use_cache, 2) of
+	true -> Mod:use_cache(Host, Table);
+	false ->
+	    gen_mod:get_module_opt(
+	      Host, ?MODULE, use_cache,
+	      ejabberd_config:use_cache(Host))
+    end.
+
+-spec cache_nodes(module(), binary()) -> [node()].
+cache_nodes(Mod, Host) ->
+    case erlang:function_exported(Mod, cache_nodes, 1) of
+	true -> Mod:cache_nodes(Host);
+	false -> ejabberd_cluster:get_nodes()
+    end.
+
+-spec delete_cache(module(), binary(), binary(), [ljid()]) -> ok.
+delete_cache(Mod, LUser, LServer, LJIDs) ->
+    case use_cache(Mod, LServer, roster_version) of
+	true ->
+	    ets_cache:delete(?ROSTER_VERSION_CACHE, {LUser, LServer},
+			     cache_nodes(Mod, LServer));
+	false ->
+	    ok
+    end,
+    case use_cache(Mod, LServer, roster) of
+	true ->
+	    Nodes = cache_nodes(Mod, LServer),
+	    ets_cache:delete(?ROSTER_CACHE, {LUser, LServer}, Nodes),
+	    lists:foreach(
+	      fun(LJID) ->
+		      ets_cache:delete(
+			?ROSTER_ITEM_CACHE,
+			{LUser, LServer, jid:remove_resource(LJID)},
+			Nodes)
+	      end, LJIDs);
+	false ->
+	    ok
+    end.
+
 export(LServer) ->
     Mod = gen_mod:db_mod(LServer, ?MODULE),
     Mod:export(LServer).

src/mod_roster_mnesia.erl

@@ -28,10 +28,10 @@
 
 %% API
 -export([init/2, read_roster_version/2, write_roster_version/4,
-	 get_roster/2, get_roster_by_jid/3, get_only_items/2,
+	 get_roster/2, get_roster_item/3, roster_subscribe/4,
-	 roster_subscribe/4, get_roster_by_jid_with_groups/3,
 	 remove_user/2, update_roster/4, del_roster/3, transaction/2,
-	 read_subscription_and_groups/3, import/3, create_roster/1]).
+	 read_subscription_and_groups/3, import/3, create_roster/1,
+	 use_cache/2]).
 -export([need_transform/1, transform/1]).
 
 -include("mod_roster.hrl").
@@ -42,18 +42,28 @@
 %%%===================================================================
 init(_Host, _Opts) ->
     ejabberd_mnesia:create(?MODULE, roster,
-			[{disc_copies, [node()]},
+			[{disc_only_copies, [node()]},
 			 {attributes, record_info(fields, roster)},
 			 {index, [us]}]),
     ejabberd_mnesia:create(?MODULE, roster_version,
-			[{disc_copies, [node()]},
+			[{disc_only_copies, [node()]},
 			 {attributes,
 			  record_info(fields, roster_version)}]).
 
+use_cache(Host, Table) ->
+    case mnesia:table_info(Table, storage_type) of
+	disc_only_copies ->
+	    gen_mod:get_module_opt(
+	      Host, ?MODULE, use_cache,
+	      ejabberd_config:use_cache(Host));
+	_ ->
+	    false
+    end.
+
 read_roster_version(LUser, LServer) ->
     US = {LUser, LServer},
     case mnesia:dirty_read(roster_version, US) of
-	[#roster_version{version = V}] -> V;
+	[#roster_version{version = V}] -> {ok, V};
 	[] -> error
     end.
 
@@ -66,32 +76,17 @@ write_roster_version(LUser, LServer, InTransaction, Ver) ->
     end.
 
 get_roster(LUser, LServer) ->
-    mnesia:dirty_index_read(roster, {LUser, LServer}, #roster.us).
+    {ok, mnesia:dirty_index_read(roster, {LUser, LServer}, #roster.us)}.
 
-get_roster_by_jid(LUser, LServer, LJID) ->
+get_roster_item(LUser, LServer, LJID) ->
     case mnesia:read({roster, {LUser, LServer, LJID}}) of
-	[] ->
+	[I] -> {ok, I};
-	    #roster{usj = {LUser, LServer, LJID},
+	[] -> error
-		    us = {LUser, LServer}, jid = LJID};
-	[I] ->
-	    I#roster{jid = LJID, name = <<"">>, groups = [],
-		     xs = []}
     end.
 
-get_only_items(LUser, LServer) ->
-    get_roster(LUser, LServer).
-
 roster_subscribe(_LUser, _LServer, _LJID, Item) ->
     mnesia:write(Item).
 
-get_roster_by_jid_with_groups(LUser, LServer, LJID) ->
-    case mnesia:read({roster, {LUser, LServer, LJID}}) of
-	[] ->
-	    #roster{usj = {LUser, LServer, LJID},
-		    us = {LUser, LServer}, jid = LJID};
-	[I] -> I
-    end.
-
 remove_user(LUser, LServer) ->
     US = {LUser, LServer},
     F = fun () ->
@@ -110,7 +105,7 @@ del_roster(LUser, LServer, LJID) ->
 read_subscription_and_groups(LUser, LServer, LJID) ->
     case mnesia:dirty_read(roster, {LUser, LServer, LJID}) of
 	[#roster{subscription = Subscription, groups = Groups}] ->
-	    {Subscription, Groups};
+	    {ok, {Subscription, Groups}};
 	_ ->
 	    error
     end.

src/mod_roster_riak.erl

@@ -28,10 +28,10 @@
 
 %% API
 -export([init/2, read_roster_version/2, write_roster_version/4,
-	 get_roster/2, get_roster_by_jid/3, create_roster/1,
+	 get_roster/2, get_roster_item/3, create_roster/1,
-	 roster_subscribe/4, get_roster_by_jid_with_groups/3,
+	 roster_subscribe/4, remove_user/2, update_roster/4,
-	 remove_user/2, update_roster/4, del_roster/3, transaction/2,
+	 del_roster/3, read_subscription_and_groups/3, transaction/2,
-	 read_subscription_and_groups/3, get_only_items/2, import/3]).
+	 import/3]).
 
 -include("mod_roster.hrl").
 
@@ -44,7 +44,7 @@ init(_Host, _Opts) ->
 read_roster_version(LUser, LServer) ->
     case ejabberd_riak:get(roster_version, roster_version_schema(),
 			   {LUser, LServer}) of
-        {ok, #roster_version{version = V}} -> V;
+        {ok, #roster_version{version = V}} -> {ok, V};
         _Err -> error
     end.
 
@@ -56,24 +56,10 @@ write_roster_version(LUser, LServer, _InTransaction, Ver) ->
 get_roster(LUser, LServer) ->
     case ejabberd_riak:get_by_index(roster, roster_schema(),
 				    <<"us">>, {LUser, LServer}) of
-        {ok, Items} -> Items;
+        {ok, Items} -> {ok, Items};
-        _Err -> []
+        _Err -> error
     end.
 
-get_roster_by_jid(LUser, LServer, LJID) ->
-    case ejabberd_riak:get(roster, roster_schema(), {LUser, LServer, LJID}) of
-        {ok, I} ->
-            I#roster{jid = LJID, name = <<"">>, groups = [], xs = []};
-        {error, notfound} ->
-            #roster{usj = {LUser, LServer, LJID},
-                    us = {LUser, LServer}, jid = LJID};
-        Err ->
-            exit(Err)
-    end.
-
-get_only_items(LUser, LServer) ->
-    get_roster(LUser, LServer).
-
 roster_subscribe(LUser, LServer, _LJID, Item) ->
     ejabberd_riak:put(Item, roster_schema(),
                       [{'2i', [{<<"us">>, {LUser, LServer}}]}]).
@@ -81,19 +67,16 @@ roster_subscribe(LUser, LServer, _LJID, Item) ->
 transaction(_LServer, F) ->
     {atomic, F()}.
 
-get_roster_by_jid_with_groups(LUser, LServer, LJID) ->
+get_roster_item(LUser, LServer, LJID) ->
     case ejabberd_riak:get(roster, roster_schema(), {LUser, LServer, LJID}) of
         {ok, I} ->
-            I;
+            {ok, I};
-        {error, notfound} ->
+        {error, _} ->
-            #roster{usj = {LUser, LServer, LJID},
+	    error
-                    us = {LUser, LServer}, jid = LJID};
-        Err ->
-            exit(Err)
     end.
 
 remove_user(LUser, LServer) ->
-    {atomic, ejabberd_riak:delete_by_index(roster, <<"us">>, {LUser, LServer})}.
+    ejabberd_riak:delete_by_index(roster, <<"us">>, {LUser, LServer}).
 
 update_roster(LUser, LServer, _LJID, Item) ->
     ejabberd_riak:put(Item, roster_schema(),
@@ -106,7 +89,7 @@ read_subscription_and_groups(LUser, LServer, LJID) ->
     case ejabberd_riak:get(roster, roster_schema(), {LUser, LServer, LJID}) of
 	{ok, #roster{subscription = Subscription,
 		     groups = Groups}} ->
-            {Subscription, Groups};
+	    {ok, {Subscription, Groups}};
 	_ ->
 	    error
     end.

src/mod_roster_sql.erl

@@ -30,10 +30,9 @@
 
 %% API
 -export([init/2, read_roster_version/2, write_roster_version/4,
-	 get_roster/2, get_roster_by_jid/3,
+	 get_roster/2, get_roster_item/3, roster_subscribe/4,
-	 roster_subscribe/4, get_roster_by_jid_with_groups/3,
+	 read_subscription_and_groups/3, remove_user/2,
-	 remove_user/2, update_roster/4, del_roster/3, transaction/2,
+	 update_roster/4, del_roster/3, transaction/2,
-	 read_subscription_and_groups/3, get_only_items/2,
 	 import/3, export/1, raw_to_record/2]).
 
 -include("mod_roster.hrl").
@@ -48,7 +47,7 @@ init(_Host, _Opts) ->
 
 read_roster_version(LUser, LServer) ->
     case sql_queries:get_roster_version(LServer, LUser) of
-	{selected, [{Version}]} -> Version;
+	{selected, [{Version}]} -> {ok, Version};
 	{selected, []} -> error
     end.
 
@@ -77,7 +76,7 @@ get_roster(LUser, LServer) ->
                                              dict:append(J, G, Acc)
                                      end,
                                      dict:new(), JIDGroups),
-	    lists:flatmap(
+	    {ok, lists:flatmap(
 		   fun(I) ->
 			   case raw_to_record(LServer, I) of
 			       %% Bad JID in database:
@@ -90,42 +89,9 @@ get_roster(LUser, LServer) ->
 					    end,
 				   [R#roster{groups = Groups}]
 			   end
-	      end, Items);
+		   end, Items)};
         _ ->
-	    []
+	    error
-    end.
-
-get_roster_by_jid(LUser, LServer, LJID) ->
-    {selected, Res} =
-	sql_queries:get_roster_by_jid(LServer, LUser, jid:encode(LJID)),
-    case Res of
-	[] ->
-	    #roster{usj = {LUser, LServer, LJID},
-		    us = {LUser, LServer}, jid = LJID};
-	[I] ->
-	    R = raw_to_record(LServer, I),
-	    case R of
-		%% Bad JID in database:
-		error ->
-		    #roster{usj = {LUser, LServer, LJID},
-			    us = {LUser, LServer}, jid = LJID};
-		_ ->
-		    R#roster{usj = {LUser, LServer, LJID},
-			     us = {LUser, LServer}, jid = LJID, name = <<"">>}
-	    end
-    end.
-
-get_only_items(LUser, LServer) ->
-    case catch sql_queries:get_roster(LServer, LUser) of
-	{selected, Is} when is_list(Is) ->
-	    lists:flatmap(
-	      fun(I) ->
-		      case raw_to_record(LServer, I) of
-			  error -> [];
-			  R -> [R]
-		      end
-	      end, Is);
-	_ -> []
     end.
 
 roster_subscribe(_LUser, _LServer, _LJID, Item) ->
@@ -135,14 +101,13 @@ roster_subscribe(_LUser, _LServer, _LJID, Item) ->
 transaction(LServer, F) ->
     ejabberd_sql:sql_transaction(LServer, F).
 
-get_roster_by_jid_with_groups(LUser, LServer, LJID) ->
+get_roster_item(LUser, LServer, LJID) ->
     SJID = jid:encode(LJID),
     case sql_queries:get_roster_by_jid(LServer, LUser, SJID) of
 	{selected, [I]} ->
             case raw_to_record(LServer, I) of
 		error ->
-		    #roster{usj = {LUser, LServer, LJID},
+		    error;
-			    us = {LUser, LServer}, jid = LJID};
 		R ->
 		    Groups =
 			case sql_queries:get_roster_groups(LServer, LUser, SJID) of
@@ -150,16 +115,15 @@ get_roster_by_jid_with_groups(LUser, LServer, LJID) ->
 				[JGrp || {JGrp} <- JGrps];
 			    _ -> []
 			end,
-		    R#roster{groups = Groups}
+		    {ok, R#roster{groups = Groups}}
 	    end;
 	{selected, []} ->
-	    #roster{usj = {LUser, LServer, LJID},
+	    error
-		    us = {LUser, LServer}, jid = LJID}
     end.
 
 remove_user(LUser, LServer) ->
     sql_queries:del_user_roster_t(LServer, LUser),
-    {atomic, ok}.
+    ok.
 
 update_roster(LUser, LServer, LJID, Item) ->
     SJID = jid:encode(LJID),
@@ -194,7 +158,7 @@ read_subscription_and_groups(LUser, LServer, LJID) ->
 			     [JGrp || {JGrp} <- JGrps];
 			 _ -> []
 		     end,
-	    {Subscription, Groups};
+	    {ok, {Subscription, Groups}};
 	_ ->
 	    error
     end.

src/mod_shared_roster.erl

@@ -31,7 +31,7 @@
 
 -export([start/2, stop/1, reload/3, export/1,
 	 import_info/0, webadmin_menu/3, webadmin_page/3,
-	 get_user_roster/2, c2s_session_opened/1,
+	 get_user_roster/2,
 	 get_jid_info/4, import/5, process_item/2, import_start/2,
 	 in_subscription/6, out_subscription/4, c2s_self_presence/1,
 	 unset_presence/4, register_user/2, remove_user/2,
@@ -54,8 +54,6 @@
 
 -include("mod_shared_roster.hrl").
 
--define(SETS, gb_sets).
-
 -type group_options() :: [{atom(), any()}].
 -callback init(binary(), gen_mod:opts()) -> any().
 -callback import(binary(), binary(), [binary()]) -> ok.
@@ -86,8 +84,6 @@ start(Host, Opts) ->
 		       ?MODULE, in_subscription, 30),
     ejabberd_hooks:add(roster_out_subscription, Host,
 		       ?MODULE, out_subscription, 30),
-    ejabberd_hooks:add(c2s_session_opened, Host,
-		       ?MODULE, c2s_session_opened, 70),
     ejabberd_hooks:add(roster_get_jid_info, Host, ?MODULE,
 		       get_jid_info, 70),
     ejabberd_hooks:add(roster_process_item, Host, ?MODULE,
@@ -112,8 +108,6 @@ stop(Host) ->
 			  ?MODULE, in_subscription, 30),
     ejabberd_hooks:delete(roster_out_subscription, Host,
 			  ?MODULE, out_subscription, 30),
-    ejabberd_hooks:delete(c2s_session_opened,
-			  Host, ?MODULE, c2s_session_opened, 70),
     ejabberd_hooks:delete(roster_get_jid_info, Host,
 			  ?MODULE, get_jid_info, 70),
     ejabberd_hooks:delete(roster_process_item, Host,
@@ -300,23 +294,6 @@ set_item(User, Server, Resource, Item) ->
 			      items = [mod_roster:encode_item(Item)]}]},
     ejabberd_router:route(ResIQ).
 
-c2s_session_opened(#{jid := #jid{luser = LUser, lserver = LServer},
-		     pres_f := PresF, pres_t := PresT} = State) ->
-    US = {LUser, LServer},
-    DisplayedGroups = get_user_displayed_groups(US),
-    SRUsers = lists:flatmap(fun(Group) ->
-						get_group_users(LServer, Group)
-					end,
-			    DisplayedGroups),
-    PresBoth = lists:foldl(
-		 fun({U, S, _}, Acc) ->
-			 ?SETS:add_element({U, S, <<"">>}, Acc);
-		    ({U, S}, Acc) ->
-			 ?SETS:add_element({U, S, <<"">>}, Acc)
-		 end, ?SETS:new(), SRUsers),
-    State#{pres_f => ?SETS:union(PresBoth, PresF),
-	   pres_t => ?SETS:union(PresBoth, PresT)}.
-
 -spec get_jid_info({subscription(), [binary()]}, binary(), binary(), jid())
       -> {subscription(), [binary()]}.
 get_jid_info({Subscription, Groups}, User, Server,

src/mod_shared_roster_ldap.erl

@@ -39,7 +39,7 @@
 -export([init/1, handle_call/3, handle_cast/2,
 	 handle_info/2, terminate/2, code_change/3]).
 
--export([get_user_roster/2, c2s_session_opened/1,
+-export([get_user_roster/2,
 	 get_jid_info/4, process_item/2, in_subscription/6,
 	 out_subscription/4, mod_opt_type/1, opt_type/1, depends/2,
 	 transform_module_options/1]).
@@ -50,7 +50,6 @@
 -include("mod_roster.hrl").
 -include("eldap.hrl").
 
--define(SETS, gb_sets).
 -define(USER_CACHE, shared_roster_ldap_user_cache).
 -define(GROUP_CACHE, shared_roster_ldap_group_cache).
 -define(LDAP_SEARCH_TIMEOUT, 5).    %% Timeout for LDAP search queries in seconds
@@ -160,23 +159,6 @@ process_item(RosterItem, _Host) ->
       _ -> RosterItem#roster{subscription = both, ask = none}
     end.
 
-c2s_session_opened(#{jid := #jid{luser = LUser, lserver = LServer},
-		     pres_f := PresF, pres_t := PresT} = State) ->
-    US = {LUser, LServer},
-    DisplayedGroups = get_user_displayed_groups(US),
-    SRUsers = lists:flatmap(fun(Group) ->
-						get_group_users(LServer, Group)
-					end,
-			    DisplayedGroups),
-    PresBoth = lists:foldl(
-		 fun({U, S, _}, Acc) ->
-			 ?SETS:add_element({U, S, <<"">>}, Acc);
-		    ({U, S}, Acc) ->
-			 ?SETS:add_element({U, S, <<"">>}, Acc)
-		 end, ?SETS:new(), SRUsers),
-    State#{pres_f => ?SETS:union(PresBoth, PresF),
-	   pres_t => ?SETS:union(PresBoth, PresT)}.
-
 -spec get_jid_info({subscription(), [binary()]}, binary(), binary(), jid())
       -> {subscription(), [binary()]}.
 get_jid_info({Subscription, Groups}, User, Server,
@@ -245,8 +227,6 @@ init([Host, Opts]) ->
 		       ?MODULE, in_subscription, 30),
     ejabberd_hooks:add(roster_out_subscription, Host,
 		       ?MODULE, out_subscription, 30),
-    ejabberd_hooks:add(c2s_session_opened, Host,
-		       ?MODULE, c2s_session_opened, 70),
     ejabberd_hooks:add(roster_get_jid_info, Host, ?MODULE,
 		       get_jid_info, 70),
     ejabberd_hooks:add(roster_process_item, Host, ?MODULE,
@@ -276,8 +256,6 @@ terminate(_Reason, State) ->
 			  ?MODULE, in_subscription, 30),
     ejabberd_hooks:delete(roster_out_subscription, Host,
 			  ?MODULE, out_subscription, 30),
-    ejabberd_hooks:delete(c2s_session_opened,
-			  Host, ?MODULE, c2s_session_opened, 70),
     ejabberd_hooks:delete(roster_get_jid_info, Host,
 			  ?MODULE, get_jid_info, 70),
     ejabberd_hooks:delete(roster_process_item, Host,

test/roster_tests.erl

@@ -161,18 +161,25 @@ subscribe_slave(Config) ->
 process_subscriptions_master(Config, Actions) ->
     EnumeratedActions = lists:zip(lists:seq(1, length(Actions)), Actions),
     self_presence(Config, available),
+    Peer = ?config(peer, Config),
     lists:foldl(
       fun({N, {Dir, Type}}, State) ->
-	      timer:sleep(100),
 	      if Dir == out -> put_event(Config, {N, in, Type});
 		 Dir == in -> put_event(Config, {N, out, Type})
 	      end,
-	      wait_for_slave(Config),
+	      Roster = get_roster(Config),
 	      ct:pal("Performing ~s-~s (#~p) "
 		     "in state:~n~s~nwith roster:~n~s",
-		     [Dir, Type, N, pp(State),
+		     [Dir, Type, N, pp(State), pp(Roster)]),
-		      pp(get_roster(Config))]),
+	      check_roster(Roster, Config, State),
-	      transition(Config, Dir, Type, State)
+	      wait_for_slave(Config),
+	      Id = mk_id(N, Dir, Type),
+	      NewState = transition(Id, Config, Dir, Type, State),
+	      wait_for_slave(Config),
+	      send_recv(Config, #iq{type = get, to = Peer, id = Id,
+				    sub_els = [#ping{}]}),
+	      check_roster_item(Config, NewState),
+	      NewState
       end, #state{}, EnumeratedActions),
     put_event(Config, done),
     wait_for_slave(Config),
@@ -186,11 +193,16 @@ process_subscriptions_slave(Config, done, _State) ->
     wait_for_master(Config),
     Config;
 process_subscriptions_slave(Config, {N, Dir, Type}, State) ->
-    wait_for_master(Config),
+    Roster = get_roster(Config),
     ct:pal("Performing ~s-~s (#~p) "
 	   "in state:~n~s~nwith roster:~n~s",
-	   [Dir, Type, N, pp(State), pp(get_roster(Config))]),
+	   [Dir, Type, N, pp(State), pp(Roster)]),
-    NewState = transition(Config, Dir, Type, State),
+    check_roster(Roster, Config, State),
+    wait_for_master(Config),
+    NewState = transition(mk_id(N, Dir, Type), Config, Dir, Type, State),
+    wait_for_master(Config),
+    send(Config, xmpp:make_iq_result(recv_iq(Config))),
+    check_roster_item(Config, NewState),
     process_subscriptions_slave(Config, get_event(Config), NewState).
 
 %%%===================================================================
@@ -288,12 +300,42 @@ pp(roster, N) ->
     catch _:_ -> no end;
 pp(_, _) -> no.
 
+mk_id(N, Dir, Type) ->
+    list_to_binary([integer_to_list(N), $-, atom_to_list(Dir),
+		    $-, atom_to_list(Type)]).
+
+check_roster([], _Config, _State) ->
+    ok;
+check_roster([Roster], _Config, State) ->
+    case {Roster#roster.subscription == State#state.subscription,
+	  Roster#roster.ask, State#state.pending_in, State#state.pending_out} of
+	{true, both, true, true} -> ok;
+	{true, in, true, false} -> ok;
+	{true, out, false, true} -> ok;
+	{true, none, false, false} -> ok;
+	_ ->
+	    ct:fail({roster_mismatch, State, Roster})
+    end.
+
+check_roster_item(Config, State) ->
+    Peer = jid:remove_resource(?config(peer, Config)),
+    RosterItem = case get_item(Config, Peer) of
+		     false -> #roster_item{};
+		     Item -> Item
+		 end,
+    case {RosterItem#roster_item.subscription == State#state.subscription,
+	  RosterItem#roster_item.ask, State#state.pending_out} of
+	{true, subscribe, true} -> ok;
+	{true, undefined, false} -> ok;
+	_ -> ct:fail({roster_item_mismatch, State, RosterItem})
+    end.
+
 %% RFC6121, A.2.1
-transition(Config, out, subscribe,
+transition(Id, Config, out, subscribe,
 	   #state{subscription = Sub, pending_in = In, pending_out = Out} = State) ->
     PeerJID = ?config(peer, Config),
     PeerBareJID = jid:remove_resource(PeerJID),
-    send(Config, #presence{to = PeerBareJID, type = subscribe}),
+    send(Config, #presence{id = Id, to = PeerBareJID, type = subscribe}),
     case {Sub, Out, In} of
 	{none, false, _} ->
 	    recv_push(Config, none, subscribe),
@@ -309,11 +351,11 @@ transition(Config, out, subscribe,
 	    State
     end;
 %% RFC6121, A.2.2
-transition(Config, out, unsubscribe,
+transition(Id, Config, out, unsubscribe,
 	   #state{subscription = Sub, pending_in = In, pending_out = Out} = State) ->
     PeerJID = ?config(peer, Config),
     PeerBareJID = jid:remove_resource(PeerJID),
-    send(Config, #presence{to = PeerBareJID, type = unsubscribe}),
+    send(Config, #presence{id = Id, to = PeerBareJID, type = unsubscribe}),
     case {Sub, Out, In} of
 	{none, true, _} ->
 	    recv_push(Config, none, undefined),
@@ -333,11 +375,11 @@ transition(Config, out, unsubscribe,
 	    State
     end;
 %% RFC6121, A.2.3
-transition(Config, out, subscribed,
+transition(Id, Config, out, subscribed,
 	    #state{subscription = Sub, pending_in = In, pending_out = Out} = State) ->
     PeerJID = ?config(peer, Config),
     PeerBareJID = jid:remove_resource(PeerJID),
-    send(Config, #presence{to = PeerBareJID, type = subscribed}),
+    send(Config, #presence{id = Id, to = PeerBareJID, type = subscribed}),
     case {Sub, Out, In} of
 	{none, false, true} ->
 	    recv_push(Config, from, undefined),
@@ -356,11 +398,11 @@ transition(Config, out, subscribed,
 	    State
     end;
 %% RFC6121, A.2.4
-transition(Config, out, unsubscribed,
+transition(Id, Config, out, unsubscribed,
 	   #state{subscription = Sub, pending_in = In, pending_out = Out} = State) ->
     PeerJID = ?config(peer, Config),
     PeerBareJID = jid:remove_resource(PeerJID),
-    send(Config, #presence{to = PeerBareJID, type = unsubscribed}),
+    send(Config, #presence{id = Id, to = PeerBareJID, type = unsubscribed}),
     case {Sub, Out, In} of
 	{none, false, true} ->
 	    State#state{subscription = none, pending_in = false};
@@ -382,7 +424,7 @@ transition(Config, out, unsubscribed,
 	    State
     end;
 %% RFC6121, A.3.1
-transition(Config, in, subscribe = Type,
+transition(_, Config, in, subscribe = Type,
 	   #state{subscription = Sub, pending_in = In, pending_out = Out} = State) ->
     case {Sub, Out, In} of
 	{none, false, false} ->
@@ -401,7 +443,7 @@ transition(Config, in, subscribe = Type,
 	    State
     end;
 %% RFC6121, A.3.2
-transition(Config, in, unsubscribe = Type,
+transition(_, Config, in, unsubscribe = Type,
 	   #state{subscription = Sub, pending_in = In, pending_out = Out} = State) ->
     case {Sub, Out, In} of
 	{none, _, true} ->
@@ -426,7 +468,7 @@ transition(Config, in, unsubscribe = Type,
 	    State
     end;
 %% RFC6121, A.3.3
-transition(Config, in, subscribed = Type,
+transition(_, Config, in, subscribed = Type,
 	   #state{subscription = Sub, pending_in = In, pending_out = Out} = State) ->
     case {Sub, Out, In} of
 	{none, true, _} ->
@@ -449,7 +491,7 @@ transition(Config, in, subscribed = Type,
 	    State
     end;
 %% RFC6121, A.3.4
-transition(Config, in, unsubscribed = Type,
+transition(_, Config, in, unsubscribed = Type,
 	   #state{subscription = Sub, pending_in = In, pending_out = Out} = State) ->
     case {Sub, Out, In} of
 	{none, true, true} ->
@@ -464,8 +506,8 @@ transition(Config, in, unsubscribed = Type,
 	    State;
 	{to, false, _} ->
 	    recv_push(Config, none, undefined),
-	    recv_subscription(Config, Type),
 	    recv_presence(Config, unavailable),
+	    recv_subscription(Config, Type),
 	    State#state{subscription = none, peer_available = false};
 	{from, true, false} ->
 	    recv_push(Config, from, undefined),
@@ -473,20 +515,20 @@ transition(Config, in, unsubscribed = Type,
 	    State#state{subscription = from, pending_out = false};
 	{both, _, _} ->
 	    recv_push(Config, from, undefined),
-	    recv_subscription(Config, Type),
 	    recv_presence(Config, unavailable),
+	    recv_subscription(Config, Type),
 	    State#state{subscription = from, peer_available = false};
 	_ ->
 	    State
     end;
 %% Outgoing roster remove
-transition(Config, out, remove,
+transition(Id, Config, out, remove,
 	   #state{subscription = Sub, pending_in = In, pending_out = Out}) ->
     PeerJID = ?config(peer, Config),
     PeerBareJID = jid:remove_resource(PeerJID),
     Item = #roster_item{jid = PeerBareJID, subscription = remove},
     #iq{type = result, sub_els = []} =
-	send_recv(Config, #iq{type = set,
+	send_recv(Config, #iq{type = set, id = Id,
 			      sub_els = [#roster_query{items = [Item]}]}),
     recv_push(Config, remove, undefined),
     case {Sub, Out, In} of
@@ -499,7 +541,7 @@ transition(Config, out, remove,
     end,
     #state{};
 %% Incoming roster remove
-transition(Config, in, remove,
+transition(_, Config, in, remove,
 	   #state{subscription = Sub, pending_in = In, pending_out = Out} = State) ->
     case {Sub, Out, In} of
 	{none, true, _} ->