From d6b72f1c5dba891d6d1bb4a24bb297170b130a73 Mon Sep 17 00:00:00 2001 From: Linus Jahn Date: Sat, 16 Jul 2022 23:23:48 +0200 Subject: [PATCH] mod_roster: Change hook type from #roster{} to #roster_item{} The problem with #roster{} is that every new record entry is also stored in the mnesia roster table. Adding the mix_participant_id there makes no sense because the normal roster items are no MIX channels. Using \#roster_item{} for the hook and #roster{} for storing the normal items seems to be a better idea. --- include/mod_roster.hrl | 3 +- src/ejabberd_c2s.erl | 12 ++--- src/mod_admin_extra.erl | 13 +++-- src/mod_mix_pam.erl | 32 +++++------- src/mod_roster.erl | 89 +++++++++++++++------------------- src/mod_shared_roster.erl | 22 ++++----- src/mod_shared_roster_ldap.erl | 11 ++--- 7 files changed, 79 insertions(+), 103 deletions(-) diff --git a/include/mod_roster.hrl b/include/mod_roster.hrl index 3f9a951c4..6ec05b3da 100644 --- a/include/mod_roster.hrl +++ b/include/mod_roster.hrl @@ -28,8 +28,7 @@ ask = none :: ask() | '_', groups = [] :: [binary()] | '_', askmessage = <<"">> :: binary() | '_', - xs = [] :: [fxml:xmlel()] | '_', - mix_participant_id = <<>> :: binary() | '_' + xs = [] :: [fxml:xmlel()] | '_' }). -record(roster_version, diff --git a/src/ejabberd_c2s.erl b/src/ejabberd_c2s.erl index 6f1038947..ec5c610e8 100644 --- a/src/ejabberd_c2s.erl +++ b/src/ejabberd_c2s.erl @@ -777,9 +777,9 @@ broadcast_presence_unavailable(#{jid := JID, pres_a := PresA} = State, Pres, Roster = ejabberd_hooks:run_fold(roster_get, LServer, [], [{LUser, LServer}]), lists:foldl( - fun(#roster{jid = LJID, subscription = Sub}, Acc) + fun(#roster_item{jid = ItemJID, subscription = Sub}, Acc) when Sub == both; Sub == from -> - maps:put(LJID, 1, Acc); + maps:put(jid:tolower(ItemJID), 1, Acc); (_, Acc) -> Acc end, #{BareJID => 1}, Roster); @@ -813,8 +813,7 @@ broadcast_presence_available(#{jid := JID} = State, [], [{LUser, LServer}]), {FJIDs, TJIDs} = lists:foldl( - fun(#roster{jid = LJID, subscription = Sub}, {F, T}) -> - To = jid:make(LJID), + fun(#roster_item{jid = To, subscription = Sub}, {F, T}) -> F1 = if Sub == both orelse Sub == from -> Pres1 = xmpp:set_to(Pres, To), case privacy_check_packet(State, Pres1, out) of @@ -843,10 +842,9 @@ broadcast_presence_available(#{jid := JID} = State, Items = ejabberd_hooks:run_fold( roster_get, LServer, [], [{LUser, LServer}]), JIDs = lists:foldl( - fun(#roster{jid = LJID, subscription = Sub}, Tos) + fun(#roster_item{jid = To, subscription = Sub}, Tos) when Sub == both orelse Sub == from -> - To = jid:make(LJID), - P = xmpp:set_to(Pres, jid:make(LJID)), + P = xmpp:set_to(Pres, To), case privacy_check_packet(State, P, out) of allow -> [To|Tos]; deny -> Tos diff --git a/src/mod_admin_extra.erl b/src/mod_admin_extra.erl index 19aa983f5..b0dffd2d6 100644 --- a/src/mod_admin_extra.erl +++ b/src/mod_admin_extra.erl @@ -1333,16 +1333,15 @@ get_roster(User, Server) -> %% several times, each one in a different group. make_roster_xmlrpc(Roster) -> lists:foldl( - fun(Item, Res) -> - JIDS = jid:encode(Item#roster.jid), - Nick = Item#roster.name, - Subs = atom_to_list(Item#roster.subscription), - Ask = atom_to_list(Item#roster.ask), - Groups = case Item#roster.groups of + fun(#roster_item{jid = JID, name = Nick, subscription = Sub, ask = Ask} = Item, Res) -> + JIDS = jid:encode(JID), + Subs = atom_to_list(Sub), + Asks = atom_to_list(Ask), + Groups = case Item#roster_item.groups of [] -> [<<>>]; Gs -> Gs end, - ItemsX = [{JIDS, Nick, Subs, Ask, Group} || Group <- Groups], + ItemsX = [{JIDS, Nick, Subs, Asks, Group} || Group <- Groups], ItemsX ++ Res end, [], diff --git a/src/mod_mix_pam.erl b/src/mod_mix_pam.erl index 9491d217a..ca3b4e6ea 100644 --- a/src/mod_mix_pam.erl +++ b/src/mod_mix_pam.erl @@ -212,22 +212,20 @@ process_iq(#iq{type = set, process_iq(IQ) -> xmpp:make_error(IQ, unsupported_query_error(IQ)). --spec get_mix_roster_items([#roster{}], {binary(), binary()}) -> [#roster{}]. +-spec get_mix_roster_items([#roster_item{}], {binary(), binary()}) -> [#roster_item{}]. get_mix_roster_items(Acc, {LUser, LServer}) -> JID = jid:make(LUser, LServer), case get_channels(JID) of {ok, Channels} -> lists:map( - fun({#jid{luser=Channel, lserver=Service}, Id}) -> - #roster{ - jid = {Channel, Service, <<>>}, + fun({ItemJID, Id}) -> + #roster_item{ + jid = ItemJID, name = <<>>, subscription = both, - ask = none, + ask = undefined, groups = [<<"Channels">>], - askmessage = <<>>, - xs = [], - mix_participant_id = Id + mix_channel = #mix_roster_channel{'participant-id' = Id} } end, Channels); _ -> @@ -288,21 +286,19 @@ process_leave(#iq{from = From, end. -spec process_join_result(iq(), iq()) -> ok. -process_join_result(#iq{from = Channel, +process_join_result(#iq{from = #jid{} = Channel, type = result, sub_els = [#mix_join{id = ID} = Join]}, #iq{to = To} = IQ) -> case add_channel(To, Channel, ID) of ok -> % Do roster push - #jid{luser = ChannelName, lserver = Service} = Channel, - mod_roster:push_item(To, #roster{}, #roster{ - jid = {ChannelName, Service, <<>>}, + mod_roster:push_item(To, #roster_item{jid = #jid{}}, #roster_item{ + jid = Channel, name = <<>>, subscription = none, - ask = none, + ask = undefined, groups = [], - askmessage = <<>>, - mix_participant_id = ID + mix_channel = #mix_roster_channel{'participant-id' = ID} }), % send IQ result ChanID = make_channel_id(Channel, ID), @@ -319,11 +315,9 @@ process_join_result(Err, IQ) -> process_leave_result(#iq{from = Channel, type = result, sub_els = [#mix_leave{} = Leave]}, #iq{to = User} = IQ) -> % Do roster push - #jid{luser = ChannelName, lserver = Service} = Channel, mod_roster:push_item(User, - #roster{jid = {ChannelName, Service, <<>>}, subscription = none}, - #roster{jid = {ChannelName, Service, <<>>}, - subscription = remove}), + #roster_item{jid = Channel, subscription = none}, + #roster_item{jid = Channel, subscription = remove}), % send iq result ResIQ = xmpp:make_iq_result(IQ, #mix_client_leave{leave = Leave}), ejabberd_router:route(ResIQ); diff --git a/src/mod_roster.erl b/src/mod_roster.erl index 27fac3131..fdf9de9f4 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -41,7 +41,7 @@ -behaviour(gen_mod). -export([start/2, stop/1, reload/3, process_iq/1, export/1, - import_info/0, process_local_iq/1, get_user_roster/2, + import_info/0, process_local_iq/1, get_user_roster_items/2, import/5, get_roster/2, push_item/3, import_start/2, import_stop/2, is_subscribed/2, c2s_self_presence/1, in_subscription/2, @@ -92,7 +92,7 @@ start(Host, Opts) -> Mod:init(Host, Opts), init_cache(Mod, Host, Opts), ejabberd_hooks:add(roster_get, Host, ?MODULE, - get_user_roster, 50), + get_user_roster_items, 50), ejabberd_hooks:add(roster_in_subscription, Host, ?MODULE, in_subscription, 50), ejabberd_hooks:add(roster_out_subscription, Host, @@ -114,7 +114,7 @@ start(Host, Opts) -> stop(Host) -> ejabberd_hooks:delete(roster_get, Host, ?MODULE, - get_user_roster, 50), + get_user_roster_items, 50), ejabberd_hooks:delete(roster_in_subscription, Host, ?MODULE, in_subscription, 50), ejabberd_hooks:delete(roster_out_subscription, Host, @@ -205,9 +205,8 @@ process_local_iq(#iq{lang = Lang} = IQ) -> -spec roster_hash([#roster{}]) -> binary(). roster_hash(Items) -> - str:sha(term_to_binary(lists:sort([R#roster{groups = - lists:sort(Grs)} - || R = #roster{groups = Grs} + str:sha(term_to_binary(lists:sort([R#roster_item{groups = lists:sort(Grs)} + || R = #roster_item{groups = Grs} <- Items]))). %% Returns a list that may contain an xmlelement with the XEP-237 feature if it's enabled. @@ -227,7 +226,6 @@ get_versioning_feature(Acc, Host) -> -spec roster_version(binary(), binary()) -> undefined | binary(). roster_version(LServer, LUser) -> - US = {LUser, LServer}, case mod_roster_opt:store_current_id(LServer) of true -> case read_roster_version(LUser, LServer) of @@ -235,8 +233,7 @@ roster_version(LServer, LUser) -> {ok, V} -> V end; false -> - roster_hash(ejabberd_hooks:run_fold(roster_get, LServer, - [], [US])) + roster_hash(run_roster_get_hook(LUser, LServer)) end. -spec read_roster_version(binary(), binary()) -> {ok, binary()} | error. @@ -279,7 +276,6 @@ process_iq_get(#iq{to = To, from = From, sub_els = [#roster_query{ver = RequestedVersion, mix_annotate = MixAnnotate}]} = IQ) -> LUser = To#jid.luser, LServer = To#jid.lserver, - US = {LUser, LServer}, MixEnabled = MixAnnotate == #mix_roster_annotate{}, {ItemsToSend, VersionToSend} = case {mod_roster_opt:versioning(LServer), @@ -288,32 +284,22 @@ process_iq_get(#iq{to = To, from = From, case read_roster_version(LUser, LServer) of error -> RosterVersion = write_roster_version(LUser, LServer), - {lists:map(fun encode_item/1, - ejabberd_hooks:run_fold( - roster_get, To#jid.lserver, [], [US])), - RosterVersion}; + {run_roster_get_hook(LUser, LServer), RosterVersion}; {ok, RequestedVersion} -> {false, false}; {ok, NewVersion} -> - {lists:map(fun encode_item/1, - ejabberd_hooks:run_fold( - roster_get, To#jid.lserver, [], [US])), - NewVersion} + {run_roster_get_hook(LUser, LServer), NewVersion} end; {true, false} when RequestedVersion /= undefined -> - RosterItems = ejabberd_hooks:run_fold( - roster_get, To#jid.lserver, [], [US]), + RosterItems = run_roster_get_hook(LUser, LServer), case roster_hash(RosterItems) of RequestedVersion -> {false, false}; New -> - {lists:map(fun encode_item/1, RosterItems), New} + {RosterItems, New} end; _ -> - {lists:map(fun encode_item/1, - ejabberd_hooks:run_fold( - roster_get, To#jid.lserver, [], [US])), - false} + {run_roster_get_hook(LUser, LServer), false} end, % Store that MIX annotation is enabled (for roster pushes) set_mix_annotation_enabled(From, MixEnabled), @@ -334,16 +320,21 @@ process_iq_get(#iq{to = To, from = From, ver = Version} end). --spec get_user_roster([#roster{}], {binary(), binary()}) -> [#roster{}]. -get_user_roster(Acc, {LUser, LServer}) -> - Items = get_roster(LUser, LServer), - lists:filter(fun (#roster{subscription = none, - ask = in}) -> - false; - (_) -> true - end, - Items) - ++ Acc. +-spec run_roster_get_hook(binary(), binary()) -> [#roster_item{}]. +run_roster_get_hook(LUser, LServer) -> + ejabberd_hooks:run_fold(roster_get, LServer, [], [{LUser, LServer}]). + +-spec get_filtered_roster(binary(), binary()) -> [#roster{}]. +get_filtered_roster(LUser, LServer) -> + lists:filter( + fun (#roster{subscription = none, ask = in}) -> false; + (_) -> true + end, + get_roster(LUser, LServer)). + +-spec get_user_roster_items([#roster_item{}], {binary(), binary()}) -> [#roster_item{}]. +get_user_roster_items(Acc, {LUser, LServer}) -> + lists:map(fun encode_item/1, get_filtered_roster(LUser, LServer)) ++ Acc. -spec get_roster(binary(), binary()) -> [#roster{}]. get_roster(LUser, LServer) -> @@ -435,11 +426,7 @@ encode_item(Item) -> both -> subscribe; _ -> undefined end, - groups = Item#roster.groups, - mix_channel = case Item#roster.mix_participant_id of - <<>> -> undefined; - _ -> #mix_roster_channel{'participant-id' = Item#roster.mix_participant_id} - end}. + groups = Item#roster.groups}. -spec decode_item(roster_item(), #roster{}, boolean()) -> #roster{}. decode_item(#roster_item{subscription = remove} = Item, R, _) -> @@ -494,7 +481,7 @@ set_item_and_notify_clients(To, #roster_item{jid = PeerJID} = RosterItem, end, case transaction(LUser, LServer, [PeerLJID], F) of {atomic, {OldItem, NewItem}} -> - push_item(To, OldItem, NewItem), + push_item(To, encode_item(OldItem), encode_item(NewItem)), case NewItem#roster.subscription of remove -> send_unsubscribing_presence(To, OldItem); @@ -505,7 +492,7 @@ set_item_and_notify_clients(To, #roster_item{jid = PeerJID} = RosterItem, {error, Reason} end. --spec push_item(jid(), #roster{}, #roster{}) -> ok. +-spec push_item(jid(), #roster_item{}, #roster_item{}) -> ok. push_item(To, OldItem, NewItem) -> #jid{luser = LUser, lserver = LServer} = To, Ver = case mod_roster_opt:versioning(LServer) of @@ -518,10 +505,10 @@ push_item(To, OldItem, NewItem) -> push_item(To1, OldItem, NewItem, Ver) end, ejabberd_sm:get_user_resources(LUser, LServer)). --spec push_item(jid(), #roster{}, #roster{}, undefined | binary()) -> ok. +-spec push_item(jid(), #roster_item{}, #roster_item{}, undefined | binary()) -> ok. push_item(To, OldItem, NewItem, Ver) -> route_presence_change(To, OldItem, NewItem), - [Item] = process_items_mix([encode_item(NewItem)], To), + [Item] = process_items_mix([NewItem], To), IQ = #iq{type = set, to = To, from = jid:remove_resource(To), id = <<"push", (p1_rand:get_string())/binary>>, @@ -529,11 +516,11 @@ push_item(To, OldItem, NewItem, Ver) -> items = [Item]}]}, ejabberd_router:route(IQ). --spec route_presence_change(jid(), #roster{}, #roster{}) -> ok. +-spec route_presence_change(jid(), #roster_item{}, #roster_item{}) -> ok. route_presence_change(From, OldItem, NewItem) -> - OldSub = OldItem#roster.subscription, - NewSub = NewItem#roster.subscription, - To = jid:make(NewItem#roster.jid), + OldSub = OldItem#roster_item.subscription, + NewSub = NewItem#roster_item.subscription, + To = NewItem#roster_item.jid, NewIsFrom = NewSub == both orelse NewSub == from, OldIsFrom = OldSub == both orelse OldSub == from, if NewIsFrom andalso not OldIsFrom -> @@ -657,7 +644,9 @@ process_subscription(Direction, User, Server, JID1, NewItem#roster.ask == in -> ok; true -> - push_item(jid:make(User, Server), OldItem, NewItem) + push_item(jid:make(User, Server), + encode_item(OldItem), + encode_item(NewItem)) end, true; none -> @@ -835,7 +824,7 @@ in_auto_reply(_, _, _) -> none. remove_user(User, Server) -> LUser = jid:nodeprep(User), LServer = jid:nameprep(Server), - Items = get_user_roster([], {LUser, LServer}), + Items = get_filtered_roster(LUser, LServer), send_unsubscription_to_rosteritems(LUser, LServer, Items), Mod = gen_mod:db_mod(LServer, ?MODULE), Mod:remove_user(LUser, LServer), diff --git a/src/mod_shared_roster.erl b/src/mod_shared_roster.erl index 333a64a45..067d4504b 100644 --- a/src/mod_shared_roster.erl +++ b/src/mod_shared_roster.erl @@ -185,8 +185,8 @@ cache_nodes(Mod, Host) -> false -> ejabberd_cluster:get_nodes() end. --spec get_user_roster([#roster{}], {binary(), binary()}) -> [#roster{}]. -get_user_roster(Items, {U, S} = US) -> +-spec get_user_roster([#roster_item{}], {binary(), binary()}) -> [#roster_item{}]. +get_user_roster(Items, {_, S} = US) -> {DisplayedGroups, Cache} = get_user_displayed_groups(US), SRUsers = lists:foldl( fun(Group, Acc1) -> @@ -216,10 +216,10 @@ get_user_roster(Items, {U, S} = US) -> end end, SRUsers, Items), - SRItems = [#roster{usj = {U, S, {U1, S1, <<"">>}}, - us = US, jid = {U1, S1, <<"">>}, - name = get_rosteritem_name(U1, S1), - subscription = both, ask = none, groups = GroupLabels} + SRItems = [#roster_item{jid = jid:make(U1, S1), + name = get_rosteritem_name(U1, S1), + subscription = both, ask = undefined, + groups = GroupLabels} || {{U1, S1}, GroupLabels} <- dict:to_list(SRUsersRest)], SRItems ++ NewItems1. @@ -855,16 +855,14 @@ displayed_to_groups(GroupName, LServer) -> push_item(User, Server, Item) -> mod_roster:push_item(jid:make(User, Server), - Item#roster{subscription = none}, + Item#roster_item{subscription = none}, Item). push_roster_item(User, Server, ContactU, ContactS, ContactN, GroupLabel, Subscription) -> - Item = #roster{usj = - {User, Server, {ContactU, ContactS, <<"">>}}, - us = {User, Server}, jid = {ContactU, ContactS, <<"">>}, - name = ContactN, subscription = Subscription, ask = none, - groups = [GroupLabel]}, + Item = #roster_item{jid = jid:make(ContactU, ContactS), + name = ContactN, subscription = Subscription, ask = undefined, + groups = [GroupLabel]}, push_item(User, Server, Item). -spec c2s_self_presence({presence(), ejabberd_c2s:state()}) diff --git a/src/mod_shared_roster_ldap.erl b/src/mod_shared_roster_ldap.erl index 216dd4ce1..2b4ec1daf 100644 --- a/src/mod_shared_roster_ldap.erl +++ b/src/mod_shared_roster_ldap.erl @@ -109,8 +109,8 @@ depends(_Host, _Opts) -> %%-------------------------------------------------------------------- %% Hooks %%-------------------------------------------------------------------- --spec get_user_roster([#roster{}], {binary(), binary()}) -> [#roster{}]. -get_user_roster(Items, {U, S} = US) -> +-spec get_user_roster([#roster_item{}], {binary(), binary()}) -> [#roster_item{}]. +get_user_roster(Items, US) -> SRUsers = get_user_to_groups_map(US, true), {NewItems1, SRUsersRest} = lists:mapfoldl(fun (Item, SRUsers1) -> @@ -135,10 +135,9 @@ get_user_roster(Items, {U, S} = US) -> end end, SRUsers, Items), - SRItems = [#roster{usj = {U, S, {U1, S1, <<"">>}}, - us = US, jid = {U1, S1, <<"">>}, - name = get_user_name(U1, S1), subscription = both, - ask = none, groups = GroupNames} + SRItems = [#roster_item{jid = jid:make(U1, S1), + name = get_user_name(U1, S1), subscription = both, + ask = undefined, groups = GroupNames} || {{U1, S1}, GroupNames} <- dict:to_list(SRUsersRest)], SRItems ++ NewItems1.