From bbbbbec7543ca0e167a2250e73287b72467465b8 Mon Sep 17 00:00:00 2001 From: Hans Svensson Date: Mon, 23 Mar 2020 08:08:28 +0100 Subject: [PATCH 1/3] Trivial simplification --- apps/roster/src/roster.erl | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/apps/roster/src/roster.erl b/apps/roster/src/roster.erl index d403dee9d..1b3c48036 100644 --- a/apps/roster/src/roster.erl +++ b/apps/roster/src/roster.erl @@ -1587,11 +1587,9 @@ patch_members(Members) -> case muc_member(PhoneId, Room) of #'Member'{} = M2 -> [patch_member(M2, M) | Acc];_ -> Acc end end, [], Members), split_members(Mmbrs). + split_members(Members) -> - lists:foldr( - fun(#'Member'{status = admin} = M, {As, Ms}) -> {[M | As], Ms}; - (#'Member'{} = M, {As, Ms}) -> {As, [M | Ms]} - end, {[], []}, Members). + lists:partition(fun(#'Member'{ status = S }) -> S == admin end, Members). reader_cache(ReaderId) when is_integer(ReaderId)-> reader_cache(kvs_stream:load_reader(ReaderId)); reader_cache(#reader{pos = 0}) -> 0; -- GitLab From 7c1cb341611de569f36e248f5388eae2d0920ab8 Mon Sep 17 00:00:00 2001 From: Hans Svensson Date: Mon, 23 Mar 2020 08:20:57 +0100 Subject: [PATCH 2/3] Removing redundant work in subscription/sub_muc + fixing is_shared_rooms --- apps/roster/src/roster.erl | 85 +++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 33 deletions(-) diff --git a/apps/roster/src/roster.erl b/apps/roster/src/roster.erl index 1b3c48036..b92492670 100644 --- a/apps/roster/src/roster.erl +++ b/apps/roster/src/roster.erl @@ -849,9 +849,10 @@ sub_client(Subscr, ClientId, PhoneId) when Subscr == subscribe; Subscr == unsubs || #'Contact'{phone_id = Friend} <- get_on_status(Id, friend), Friend =/= PhoneId], [n2o_vnode:Subscr(ClientId, Topic) || Topic <- [ses_topic(Phone), p2p_topic(PhoneId, PhoneId) | lists:flatten(Topics)]], - sub_room(Subscr, PhoneId), - [sub_muc(Subscr, PhoneId, #muc{name = Room}, [ClientId]) || - #'Room'{id = Room} <- get_rooms(PhoneId)], ok. + Rooms = get_rooms(PhoneId), + [sub_room(Subscr, muc_member(PhoneId, Room)) || #'Room'{id = Room} <- Rooms], + [sub_muc(Subscr, PhoneId, #muc{name = Room}, [ClientId]) || #'Room'{id = Room} <- Rooms], + ok. feed_key(Type, From, To) -> case From < To of true -> {Type, From, To}; _ -> {Type, To, From} end. feed_key(#p2p{from = From, to = To}) -> feed_key(p2p, From, To); @@ -1148,13 +1149,14 @@ remove_rooms(RosterId, Rooms) when is_integer(RosterId) -> {ok, #'Roster'{} = Roster} -> remove_rooms(Roster, Rooms) end; remove_rooms(PhoneId, Rooms) -> remove_rooms(roster_id(PhoneId), Rooms). -get_rooms(RosterId, Fun) when is_integer(RosterId) -> +get_rooms(RosterId) when is_integer(RosterId) -> case kvs:get('Roster', RosterId) of {error, _} -> {error, roster_not_found}; - {ok, #'Roster'{roomlist = List}} -> [R || #'Room'{} = R <- List, Fun(R)] end; -get_rooms(PhoneId, Fun) -> [_, RosterId] = parts_phone_id(PhoneId), get_rooms(RosterId, Fun). -get_rooms(RosterId) when is_integer(RosterId) -> get_rooms(RosterId, fun(#'Room'{}) -> true end); -get_rooms(PhoneId) -> get_rooms(PhoneId, fun(#'Room'{}) -> true end). + {ok, #'Roster'{roomlist = List}} -> [R || R = #'Room'{} <- List] + end; +get_rooms(PhoneId) -> + [_, RosterId] = parts_phone_id(PhoneId), + get_rooms(RosterId). last_rooms([], _) -> []; last_rooms(Rooms, N) -> @@ -1510,7 +1512,8 @@ roster(Id, N, LastSync) -> sub_room(subscribe, #'Member'{status = removed}) -> []; sub_room(Subscr, #'Member'{phone_id = PhoneId, feed_id = #muc{name = Room}}) -> - [n2o_vnode:Subscr(ClientId, room_topic(Room)) || #'Auth'{client_id = ClientId} <- kvs:index('Auth', user_id, PhoneId)]; + Clients = kvs:index('Auth', user_id, PhoneId), + [n2o_vnode:Subscr(ClientId, room_topic(Room)) || #'Auth'{client_id = ClientId} <- Clients]; sub_room(Subscr, Members) when is_list(Members) -> [sub_room(Subscr, M) || M <- Members]; sub_room(Subscr, <<"emqttd_", _/binary>> = ClientId) -> {ok, #'Auth'{user_id = PhoneId}} = kvs:get('Auth', ClientId), @@ -1527,21 +1530,34 @@ sub_muc(Subscr, #'Roster'{id = RosterId, phone = Phone} = Roster, #muc{} = Feed) sub_muc(Subscr, PhoneId, #muc{} = Feed) -> {ok, Roster} = kvs:get('Roster', roster_id(PhoneId)), sub_muc(Subscr, Roster, Feed). + sub_muc(Subscr, PhoneId, #muc{} = Feed, Clients) -> sub_muc(Subscr, PhoneId, members(Feed, active), Clients); -sub_muc(Subscr, #'Roster'{id = RosterId, phone = Phone, userlist = Cntcts, roomlist = Rooms}, Members, ClientIds) -> - PhoneId = phone_id(Phone, RosterId), Contacts = lists:droplast(Cntcts), - [begin n2o_vnode:Subscr(ClientId, muc_topic(PId)), n2o_vnode:Subscr(CId, muc_topic(PhoneId)) end || - #'Member'{phone_id = PId} <- Members, ClientId <- ClientIds, - #'Auth'{client_id = CId} <- kvs:index('Auth', user_id, PId), - PhoneId /= PId, - case {Subscr, lists:keyfind(PId, #'Contact'.phone_id, Contacts), - is_shared_rooms(Rooms, PId)} of - {subscribe, #'Contact'{status = friend}, true} -> false; - {subscribe, _, IsShared} -> IsShared; - {unsubscribe, true, _} -> true; - {unsubscribe, false, false} -> true; - _ -> false end]; +sub_muc(Subscr, #'Roster'{id = RosterId, phone = Phone, userlist = Contacts0, roomlist = Rooms}, Members, ClientIds) -> + PhoneId = phone_id(Phone, RosterId), + Contacts = lists:droplast(Contacts0), %% Why?? + + Sub = fun(C, P) -> n2o_vnode:Subscr(C, muc_topic(P)) end, + FriendOrShared = + fun(P) -> + case lists:keyfind(P, #'Contact'.phone_id, Contacts) of + #'Contact'{status = friend} when Subscr == subscribe -> false; + _ when Subscr == subscribe -> is_shared_rooms(Rooms, P); + #'Contact'{} when Subscr == unsubscribe -> true; + _ when Subscr == unsubscribe -> not is_shared_rooms(Rooms, P); + _ -> false + end + end, + + PIds = [ PId || #'Member'{phone_id = PId} <- Members, PhoneId /= PId, FriendOrShared(PId) ], + + lists:foreach( + fun(PId) -> + lists:foreach(fun(C) -> Sub(C, PId) end, ClientIds), + lists:foreach(fun(#'Auth'{ client_id = C }) -> Sub(C, PId) end, + kvs:index('Auth', user_id, PId)) + end, PIds), + []; sub_muc(Subscr, PhoneId, Members, ClientIds) -> {ok, Roster} = kvs:get('Roster', roster_id(PhoneId)), sub_muc(Subscr, Roster, Members, ClientIds). @@ -1552,17 +1568,20 @@ unsubscribe_muc(PhoneId, Ms, Clients) -> sub_muc(unsubscribe, PhoneId, Ms, Clien unsubscribe_muc(PhoneId) -> [n2o_vnode:unsubscribe(ClientId, muc_topic(PhoneId)) || ClientId <- emqttd_pubsub:subscribers(muc_topic(PhoneId))]. -is_shared_rooms(Rooms, PhoneId2) when is_binary(PhoneId2), is_list(Rooms) -> - is_shared_rooms(Rooms, get_rooms(PhoneId2)); -is_shared_rooms(Rooms, Rooms2) when Rooms == [];Rooms2 == [] -> false; -is_shared_rooms([#'Room'{status = removed} | TRooms], Rooms) -> is_shared_rooms(TRooms, Rooms); -is_shared_rooms(Rooms, [#'Room'{status = removed} | TRooms2]) -> is_shared_rooms(Rooms, TRooms2); -is_shared_rooms(Rooms, [#'Room'{id = Id2} | TRooms2] = Rooms2) -> - case lists:last(Rooms) of - #'Room'{id = Id2} -> true; - #'Room'{id = Id} when Id < Id2 -> is_shared_rooms(lists:droplast(Rooms), Rooms2); - _ -> is_shared_rooms(Rooms, TRooms2) end; -is_shared_rooms(PhoneId, PhoneId2) -> is_shared_rooms(get_rooms(PhoneId), PhoneId2). + +is_shared_rooms(Rooms, PhoneId) when is_list(Rooms), is_binary(PhoneId) -> + is_shared_rooms_(Rooms, get_rooms(PhoneId)); +is_shared_rooms(Rooms1, Rooms2) when is_list(Rooms1), is_list(Rooms2) -> + is_shared_rooms_(Rooms1, Rooms2). + +is_shared_rooms_([], _Rooms2) -> false; +is_shared_rooms_(_Rooms1, []) -> false; +is_shared_rooms_([#'Room'{id = Id, status = S1} | _], [#'Room'{id = Id, status = S2} | _]) -> + S1 /= removed andalso S2 /= removed; +is_shared_rooms_([#'Room'{id = Id1} | Rooms1], [#'Room'{id = Id2} | _] = Rooms2) when Id1 < Id2 -> + is_shared_rooms_(Rooms1, Rooms2); +is_shared_rooms_([#'Room'{id = Id1} | _] = Rooms1, [#'Room'{id = Id2} | Rooms2]) -> + is_shared_rooms_(Rooms1, Rooms2). patch_room(#'Room'{update = Upd} = OldR, #'Room'{settings = Settings} = NewR) when Settings /= [] -> case now_msec() - Upd - application:get_env(roster, freeze_time, 5 * 60 * 1000) < 0 of -- GitLab From 68b9ef7ecd541c9ae9d7b23c89b657ae6c05b482 Mon Sep 17 00:00:00 2001 From: Hans Svensson Date: Mon, 23 Mar 2020 12:44:28 +0100 Subject: [PATCH 3/3] Don't abort is_shared_rooms on removed shared room --- apps/roster/src/roster.erl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/roster/src/roster.erl b/apps/roster/src/roster.erl index b92492670..86670d30a 100644 --- a/apps/roster/src/roster.erl +++ b/apps/roster/src/roster.erl @@ -1576,8 +1576,10 @@ is_shared_rooms(Rooms1, Rooms2) when is_list(Rooms1), is_list(Rooms2) -> is_shared_rooms_([], _Rooms2) -> false; is_shared_rooms_(_Rooms1, []) -> false; -is_shared_rooms_([#'Room'{id = Id, status = S1} | _], [#'Room'{id = Id, status = S2} | _]) -> - S1 /= removed andalso S2 /= removed; +is_shared_rooms_([#'Room'{id = Id, status = S1} | _], + [#'Room'{id = Id, status = S2} | _]) + when S1 /= removed andalso S2 /= removed -> + true; is_shared_rooms_([#'Room'{id = Id1} | Rooms1], [#'Room'{id = Id2} | _] = Rooms2) when Id1 < Id2 -> is_shared_rooms_(Rooms1, Rooms2); is_shared_rooms_([#'Room'{id = Id1} | _] = Rooms1, [#'Room'{id = Id2} | Rooms2]) -> -- GitLab