From cdcd4d6d21e4fe0d91fea97c6ada405d137f5878 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 3 Nov 2020 17:19:43 +0100 Subject: [PATCH 1/8] Refactor all exported handlers in mod_roster --- src/mod_roster.erl | 130 ++++++++++++++++++++++++++++++--------------- 1 file changed, 86 insertions(+), 44 deletions(-) diff --git a/src/mod_roster.erl b/src/mod_roster.erl index ff8a9f9f91..b49ba872a9 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -44,32 +44,45 @@ stop/1, process_iq/4, process_local_iq/4, - get_user_roster/2, - get_subscription_lists/3, get_roster_entry/2, get_roster_entry/3, get_roster/2, item_to_map/1, - in_subscription/6, - out_subscription/5, set_items/2, set_roster_entry/4, - remove_user/2, % for tests - remove_user/3, remove_from_roster/2, - get_jid_info/4, item_to_xml/1, - get_versioning_feature/2, roster_version/2 ]). +% Main hooks +-export([ + get_user_roster/2, + in_subscription/5, + out_subscription/4, + get_subscription_lists/2, + get_jid_info/3, + remove_user/2, % for tests + remove_user/3, + get_versioning_feature/2, + get_personal_data/2 + ]). + +% Deprecated Hooks +-export([ + % get_user_roster/2, + in_subscription/6, + out_subscription/5, + get_subscription_lists/3, + get_jid_info/4, + remove_user/3 + ]). + -export([remove_test_user/2, transaction/2, process_subscription_transaction/5, get_user_rosters_length/2]). % for testing --export([get_personal_data/2]). - -export([config_metrics/1]). -include("mongoose.hrl"). @@ -160,6 +173,39 @@ Item :: term(), Result :: error | roster(). +%==================================================================== +% Deprecated API +%==================================================================== + -define(FUNCTION_STRING, + ?MODULE_STRING ++ ":" ++ + atom_to_list(?FUNCTION_NAME) ++ "/" ++ + integer_to_list(?FUNCTION_ARITY)). + -define(DEPRECATE_FUNCTION, + mongoose_deprecations:log( + {?MODULE, ?FUNCTION_NAME, ?FUNCTION_ARITY}, + #{what => roster_function_deprecated, + text => <<"The function ", (list_to_binary(?FUNCTION_STRING))/binary, + " is deprecated, please use the #jid{} equivalent instead">>}, + [{log_level, warning}])). + in_subscription(Acc, U, S, JID, Type, Reason) -> + ?DEPRECATE_FUNCTION, + in_subscription(Acc, jid:make(U, S, <<>>), JID, Type, Reason). + out_subscription(Acc, U, S, JID, Type) -> + ?DEPRECATE_FUNCTION, + out_subscription(Acc, jid:make(U, S, <<>>), JID, Type). + get_subscription_lists(Acc, U, S) -> + ?DEPRECATE_FUNCTION, + get_subscription_lists(Acc, jid:make(U, S, <<>>)). + get_jid_info(Acc, U, S, JID) -> + ?DEPRECATE_FUNCTION, + get_jid_info(Acc, jid:make(U, S, <<>>), JID). + remove_user(Acc, U, S) -> + ?DEPRECATE_FUNCTION, + remove_user(Acc, jid:make(U, S, <<>>)). +%==================================================================== +% Deprecated API +%==================================================================== + %%-------------------------------------------------------------------- %% gdpr callback %%-------------------------------------------------------------------- @@ -393,8 +439,10 @@ create_sub_el(Items, Version) -> {<<"ver">>, Version}], children = Items}]. --spec get_user_roster(mongoose_acc:t(), {jid:luser(), jid:lserver()}) -> mongoose_acc:t(). -get_user_roster(Acc, {LUser, LServer}) -> +-spec get_user_roster(mongoose_acc:t(), + jid:jid() | {jid:luser(), jid:lserver()}) -> + mongoose_acc:t(). +get_user_roster(Acc, #jid{luser = LUser, lserver = LServer}) -> case mongoose_acc:get(roster, show_full_roster, false, Acc) of true -> Roster = get_roster(LUser, LServer), @@ -406,10 +454,14 @@ get_user_roster(Acc, {LUser, LServer}) -> true end, get_roster(LUser, LServer)), mongoose_acc:append(roster, items, Roster, Acc) - end. + end; +get_user_roster(Acc, {U, S}) when is_binary(U), is_binary(S) -> + ?DEPRECATE_FUNCTION, + get_user_roster(Acc, jid:make(U, S, <<>>)). +-spec get_roster(jid:luser(), jid:lserver()) -> [roster()]. get_roster(LUser, LServer) -> - mod_roster_backend:get_roster(jid:nameprep(LUser), LServer). + mod_roster_backend:get_roster(LUser, LServer). item_to_xml(Item) -> Attrs1 = [{<<"jid">>, @@ -578,11 +630,9 @@ push_item_final(JID, From, Item, RosterVersion) -> children = [item_to_xml(Item)]}]}, ejabberd_router:route(From, JID, jlib:iq_to_xml(ResIQ)). --spec get_subscription_lists(Acc :: mongoose_acc:t(), - User :: binary(), - Server :: binary()) -> mongoose_acc:t(). -get_subscription_lists(Acc, User, Server) -> - #jid{luser = LUser, lserver = LServer} = JID = jid:make(User, Server, <<>>), +-spec get_subscription_lists(Acc :: mongoose_acc:t(), JID :: jid:jid()) -> + mongoose_acc:t(). +get_subscription_lists(Acc, #jid{luser = LUser, lserver = LServer} = JID) -> Items = mod_roster_backend:get_subscription_lists(Acc, LUser, LServer), SubLists = fill_subscription_lists(JID, LServer, Items, [], [], []), mongoose_acc:set(roster, subscription_lists, SubLists, Acc). @@ -590,9 +640,7 @@ get_subscription_lists(Acc, User, Server) -> fill_subscription_lists(JID, LServer, [#roster{} = I | Is], F, T, P) -> J = element(3, I#roster.usj), - NewP = build_pending(I, JID, P), - case I#roster.subscription of both -> fill_subscription_lists(JID, LServer, Is, [J | F], [J | T], NewP); @@ -641,34 +689,31 @@ transaction(LServer, F) -> mod_roster_backend:transaction(LServer, F). -spec in_subscription(Acc:: mongoose_acc:t(), - User :: binary(), - Server :: binary(), - JID :: jid:jid(), + ToJID :: jid:jid(), + FromJID :: jid:jid(), Type :: sub_presence(), Reason :: any()) -> mongoose_acc:t(). -in_subscription(Acc, User, Server, JID, Type, Reason) -> - Res = process_subscription(in, User, Server, JID, Type, Reason), +in_subscription(Acc, ToJID, FromJID, Type, Reason) -> + Res = process_subscription(in, ToJID, FromJID, Type, Reason), mongoose_acc:set(hook, result, Res, Acc). -spec out_subscription(Acc:: mongoose_acc:t(), - User :: binary(), - Server :: binary(), - JID :: jid:jid(), + FromJID :: jid:jid(), + ToJID :: jid:jid(), Type :: sub_presence()) -> mongoose_acc:t(). -out_subscription(Acc, User, Server, JID, Type) -> - Res = process_subscription(out, User, Server, JID, Type, <<>>), +out_subscription(Acc, FromJID, ToJID, Type) -> + Res = process_subscription(out, FromJID, ToJID, Type, <<>>), mongoose_acc:set(hook, result, Res, Acc). -process_subscription(Direction, User, Server, JID1, Type, Reason) -> - JID = jid:make(User, Server, <<>>), - LServer = case JID of +process_subscription(Direction, FromJID, ToJID, Type, Reason) -> + LServer = case FromJID of #jid{lserver = LS} -> LS; error -> error end, TransactionFun = - fun() -> process_subscription_transaction(Direction, JID, JID1, Type, Reason) end, + fun() -> process_subscription_transaction(Direction, FromJID, ToJID, Type, Reason) end, case transaction(LServer, TransactionFun) of {atomic, {Push, AutoReply}} -> case AutoReply of @@ -677,13 +722,13 @@ process_subscription(Direction, User, Server, JID1, Type, Reason) -> PresenceStanza = #xmlel{name = <<"presence">>, attrs = [{<<"type">>, autoreply_to_type(AutoReply)}], children = []}, - ejabberd_router:route(JID, JID1, PresenceStanza) + ejabberd_router:route(FromJID, ToJID, PresenceStanza) end, case Push of - {push, #roster{ subscription = none, ask = in }} -> + {push, #roster{subscription = none, ask = in}} -> true; {push, Item} -> - push_item(JID, JID, Item), + push_item(FromJID, FromJID, Item), true; none -> false end; @@ -880,8 +925,7 @@ try_send_unsubscription_to_rosteritems(Acc, JID) -> %% Both or From, send a "unsubscribed" presence stanza; %% Both or To, send a "unsubscribe" presence stanza. send_unsubscription_to_rosteritems(Acc, JID) -> - #jid{luser = LUser, lserver = LServer} = JID, - Acc1 = get_user_roster(Acc, {LUser, LServer}), + Acc1 = get_user_roster(Acc, JID), RosterItems = mongoose_acc:get(roster, items, [], Acc1), lists:foreach(fun(RosterItem) -> send_unsubscribing_presence(JID, RosterItem) @@ -1010,11 +1054,9 @@ process_item_attrs_ws(Item, []) -> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% -spec get_jid_info(_ :: term(), - User :: jid:luser(), - Server :: jid:lserver(), + ToJID :: jid:jid(), JID ::jid:jid() | jid:ljid()) -> {subscription_state(), [binary()]}. -get_jid_info(_, User, Server, JID) -> - ToJID = jid:make(User, Server, <<>>), +get_jid_info(_, ToJID, JID) -> case get_roster_entry(ToJID, JID, full) of error -> {none, []}; does_not_exist -> From 8d646e006eaa07a94d5baeee05ffe4aca0ee80e6 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 30 Oct 2020 13:45:57 +0100 Subject: [PATCH 2/8] Update roster_get hook --- big_tests/tests/muc_light_SUITE.erl | 2 +- .../service_admin_extra_roster.erl | 8 +++--- src/metrics/mongoose_metrics_hooks.erl | 6 ++--- src/mod_commands.erl | 7 +++--- src/mod_disco.erl | 4 +-- src/mod_roster.erl | 25 +++++++++---------- src/mod_shared_roster_ldap.erl | 3 ++- src/mongoose_hooks.erl | 11 ++++---- src/muc_light/mod_muc_light.erl | 6 ++--- test/roster_SUITE.erl | 4 +-- 10 files changed, 37 insertions(+), 39 deletions(-) diff --git a/big_tests/tests/muc_light_SUITE.erl b/big_tests/tests/muc_light_SUITE.erl index 6fb14bf327..c947063d25 100644 --- a/big_tests/tests/muc_light_SUITE.erl +++ b/big_tests/tests/muc_light_SUITE.erl @@ -420,7 +420,7 @@ rooms_in_rosters(Config) -> distributed_helper:mim(), mod_roster, get_user_rosters_length, - [AliceU, AliceS]) + [jid:make(AliceU, AliceS, <<>>)]) end, 1, #{time_left => timer:seconds(10)}), RosterResult = escalus:wait_for_stanza(Alice), escalus_assert:is_roster_result(RosterResult), diff --git a/src/admin_extra/service_admin_extra_roster.erl b/src/admin_extra/service_admin_extra_roster.erl index 29aae1a86e..3efd7fbf45 100644 --- a/src/admin_extra/service_admin_extra_roster.erl +++ b/src/admin_extra/service_admin_extra_roster.erl @@ -248,10 +248,10 @@ unsubscribe(LocalJID, RemoteJID) -> [jids_nick_subs_ask_grp()]. get_roster(User, Server) -> UserJID = jid:make(User, Server, <<>>), - Acc = mongoose_acc:new(#{ location => ?LOCATION, - lserver => UserJID#jid.lserver, - element => undefined }), - Acc2 = mongoose_hooks:roster_get(Server, Acc, UserJID#jid.luser, Server), + Acc = mongoose_acc:new(#{location => ?LOCATION, + lserver => UserJID#jid.lserver, + element => undefined}), + Acc2 = mongoose_hooks:roster_get(Server, Acc, UserJID), Items = mongoose_acc:get(roster, items, [], Acc2), make_roster(Items). diff --git a/src/metrics/mongoose_metrics_hooks.erl b/src/metrics/mongoose_metrics_hooks.erl index 69fec0e16b..1635d84a15 100644 --- a/src/metrics/mongoose_metrics_hooks.erl +++ b/src/metrics/mongoose_metrics_hooks.erl @@ -156,9 +156,9 @@ xmpp_send_element(Acc, _El) -> %% Roster --spec roster_get(list(), {_, jid:server()}) -> list(). -roster_get(Acc, {_, Server}) -> - mongoose_metrics:update(Server, modRosterGets, 1), +-spec roster_get(list(), jid:jid()) -> list(). +roster_get(Acc, #jid{lserver = LServer}) -> + mongoose_metrics:update(LServer, modRosterGets, 1), Acc. -spec roster_set(Acc :: map(), JID :: jid:jid(), tuple(), tuple()) -> diff --git a/src/mod_commands.erl b/src/mod_commands.erl index 1e07167d88..450e18c061 100644 --- a/src/mod_commands.erl +++ b/src/mod_commands.erl @@ -337,13 +337,12 @@ parse_from_to(F, T) -> end. list_contacts(Caller) -> - CallerJID = jid:from_binary(Caller), + CallerJID = #jid{lserver = LServer} = jid:from_binary(Caller), Acc0 = mongoose_acc:new(#{ location => ?LOCATION, - lserver => CallerJID#jid.lserver, + lserver => LServer, element => undefined }), Acc1 = mongoose_acc:set(roster, show_full_roster, true, Acc0), - {User, Host} = jid:to_lus(CallerJID), - Acc2 = mongoose_hooks:roster_get(Host, Acc1, User, Host), + Acc2 = mongoose_hooks:roster_get(LServer, Acc1, CallerJID), Res = mongoose_acc:get(roster, items, Acc2), [roster_info(mod_roster:item_to_map(I)) || I <- Res]. diff --git a/src/mod_disco.erl b/src/mod_disco.erl index 119eed1f01..62cc612410 100644 --- a/src/mod_disco.erl +++ b/src/mod_disco.erl @@ -380,12 +380,12 @@ get_sm_items(empty, From, To, _Node, _Lang) -> -spec is_presence_subscribed(jid:jid(), jid:jid()) -> boolean(). -is_presence_subscribed(#jid{luser = LFromUser, lserver = LFromServer} = _From, +is_presence_subscribed(#jid{luser = LFromUser, lserver = LFromServer} = FromJID, #jid{luser = LToUser, lserver = LToServer} = _To) -> A = mongoose_acc:new(#{ location => ?LOCATION, lserver => LFromServer, element => undefined }), - A2 = mongoose_hooks:roster_get(LFromServer, A, LFromUser, LFromServer), + A2 = mongoose_hooks:roster_get(LFromServer, A, FromJID), Roster = mongoose_acc:get(roster, items, [], A2), lists:any(fun({roster, _, _, JID, _, S, _, _, _, _}) -> {TUser, TServer} = jid:to_lus(JID), diff --git a/src/mod_roster.erl b/src/mod_roster.erl index b49ba872a9..f02ced8d89 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -81,7 +81,7 @@ -export([remove_test_user/2, transaction/2, process_subscription_transaction/5, - get_user_rosters_length/2]). % for testing + get_user_rosters_length/1]). % for testing -export([config_metrics/1]). @@ -339,7 +339,7 @@ roster_version(LServer, LUser) -> V -> V end; false -> - R = get_roster_old(LUser, LServer), + R = get_roster_old(jid:make(LUser, LServer, <<>>)), roster_hash(R) end. @@ -401,19 +401,19 @@ get_user_roster_db_versioning(RequestedVersion, From, To) error -> RosterVersion = write_roster_version(LUser, LServer), {lists:map(fun item_to_xml/1, - get_roster_old(To#jid.server, LUser, LServer)), + get_roster_old(To#jid.server, From)), RosterVersion}; RequestedVersion -> {false, false}; NewVersion -> {lists:map(fun item_to_xml/1, - get_roster_old(To#jid.server, LUser, LServer)), + get_roster_old(To#jid.server, From)), NewVersion} end. get_user_roster_hash_versioning(RequestedVersion, From, To) when is_binary(RequestedVersion) -> - RosterItems = get_roster_old(To#jid.lserver, From#jid.luser, From#jid.lserver), + RosterItems = get_roster_old(To#jid.lserver, From), case roster_hash(RosterItems) of RequestedVersion -> {false, false}; @@ -423,8 +423,7 @@ get_user_roster_hash_versioning(RequestedVersion, From, To) get_user_roster_no_versioning(From, To) -> {lists:map(fun item_to_xml/1, - get_roster_old(To#jid.lserver, - From#jid.luser, From#jid.lserver)), + get_roster_old(To#jid.lserver, From)), false}. create_sub_el(false, false) -> @@ -891,8 +890,8 @@ in_auto_reply(_, _, _) -> none. remove_test_user(User, Server) -> mod_roster_backend:remove_user(User, Server). -get_user_rosters_length(User, Server) -> - length(get_roster_old(User, Server)). +get_user_rosters_length(JID) -> + length(get_roster_old(JID)). %% Used only by tests remove_user(User, Server) -> @@ -1069,14 +1068,14 @@ get_jid_info(_, ToJID, JID) -> Re -> {Re#roster.subscription, Re#roster.groups} end. -get_roster_old(LUser, LServer) -> - get_roster_old(LServer, LUser, LServer). +get_roster_old(#jid{lserver = LServer} = JID) -> + get_roster_old(LServer, JID). -get_roster_old(DestServer, LUser, LServer) -> +get_roster_old(DestServer, JID) -> A = mongoose_acc:new(#{ location => ?LOCATION, lserver => DestServer, element => undefined }), - A2 = mongoose_hooks:roster_get(DestServer, A, LUser, LServer), + A2 = mongoose_hooks:roster_get(DestServer, A, JID), mongoose_acc:get(roster, items, [], A2). -spec item_to_map(roster()) -> map(). diff --git a/src/mod_shared_roster_ldap.erl b/src/mod_shared_roster_ldap.erl index 60a3fa2936..ccac3dffc7 100644 --- a/src/mod_shared_roster_ldap.erl +++ b/src/mod_shared_roster_ldap.erl @@ -107,7 +107,8 @@ stop(Host) -> %%-------------------------------------------------------------------- %% Hooks %%-------------------------------------------------------------------- -get_user_roster(Acc, {U, S} = US) -> +get_user_roster(Acc, #jid{luser = U, lserver = S} = JID) -> + US = jid:to_lus(JID), Items = mongoose_acc:get(roster, items, [], Acc), SRUsers = get_user_to_groups_map(US, true), {NewItems1, SRUsersRest} = diff --git a/src/mongoose_hooks.erl b/src/mongoose_hooks.erl index 2fbb123c56..0bfc836a35 100644 --- a/src/mongoose_hooks.erl +++ b/src/mongoose_hooks.erl @@ -68,7 +68,7 @@ unset_presence_hook/5, xmpp_bounce_message/2]). --export([roster_get/4, +-export([roster_get/3, roster_get_jid_info/4, roster_get_subscription_lists/3, roster_get_versioning_feature/2, @@ -781,14 +781,13 @@ xmpp_bounce_message(Server, Acc) -> %% Roster related hooks %%% @doc The `roster_get' hook is called to extract a user's roster. --spec roster_get(LServer, Acc, User, UserServer) -> Result when +-spec roster_get(LServer, Acc, JID) -> Result when LServer :: jid:lserver(), Acc :: mongoose_acc:t(), - User :: jid:luser(), - UserServer :: jid:lserver(), + JID :: jid:jid(), Result :: mongoose_acc:t(). -roster_get(LServer, Acc, User, UserServer) -> - ejabberd_hooks:run_fold(roster_get, LServer, Acc, [{User, UserServer}]). +roster_get(LServer, Acc, JID) -> + ejabberd_hooks:run_fold(roster_get, LServer, Acc, [JID]). %%% @doc The `roster_groups' hook is called to extract roster groups. -spec roster_groups(LServer, Acc) -> Result when diff --git a/src/muc_light/mod_muc_light.erl b/src/muc_light/mod_muc_light.erl index 59a9fdd23c..2f5211f6d9 100644 --- a/src/muc_light/mod_muc_light.erl +++ b/src/muc_light/mod_muc_light.erl @@ -311,9 +311,9 @@ remove_user(Acc, User, Server) -> Acc end. --spec add_rooms_to_roster(Acc :: mongoose_acc:t(), UserUS :: jid:simple_bare_jid()) -> - mongoose_acc:t(). -add_rooms_to_roster(Acc, UserUS) -> +-spec add_rooms_to_roster(Acc :: mongoose_acc:t(), UserJID :: jid:jid()) -> mongoose_acc:t(). +add_rooms_to_roster(Acc, UserJID) -> + UserUS = jid:to_lus(UserJID), Items = mongoose_acc:get(roster, items, [], Acc), RoomList = mod_muc_light_db_backend:get_user_rooms(UserUS, undefined), Info = get_rooms_info(lists:sort(RoomList)), diff --git a/test/roster_SUITE.erl b/test/roster_SUITE.erl index 28e8e6b451..ff9ace7f91 100644 --- a/test/roster_SUITE.erl +++ b/test/roster_SUITE.erl @@ -139,7 +139,7 @@ get_roster_old(User) -> Acc = mongoose_acc:new(#{ location => ?LOCATION, lserver => <<"localhost">>, element => undefined }), - Acc1 = mod_roster:get_user_roster(Acc, {User, host()}), + Acc1 = mod_roster:get_user_roster(Acc, jid:make(User, host(), <<>>)), mongoose_acc:get(roster, items, Acc1). get_full_roster() -> @@ -147,7 +147,7 @@ get_full_roster() -> lserver => <<"localhost">>, element => undefined }), Acc1 = mongoose_acc:set(roster, show_full_roster, true, Acc0), - Acc2 = mod_roster:get_user_roster(Acc1, {a(), host()}), + Acc2 = mod_roster:get_user_roster(Acc1, alice_jid()), mongoose_acc:get(roster, items, Acc2). assert_state_old(Subscription, Ask) -> From 9d9ae9120ce7c2a9727da65ab0a79f48bf467f72 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 3 Nov 2020 17:15:04 +0100 Subject: [PATCH 3/8] Update roster_in_subscription hook --- src/ejabberd_sm.erl | 17 +++++++---------- src/metrics/mongoose_metrics_hooks.erl | 14 +++++++------- src/mod_shared_roster_ldap.erl | 19 ++++++++----------- src/mongoose_hooks.erl | 11 +++++------ src/pubsub/mod_pubsub.erl | 13 ++++++------- 5 files changed, 33 insertions(+), 41 deletions(-) diff --git a/src/ejabberd_sm.erl b/src/ejabberd_sm.erl index 5f7a4b9731..1283e8f840 100644 --- a/src/ejabberd_sm.erl +++ b/src/ejabberd_sm.erl @@ -38,7 +38,7 @@ store_info/2, get_info/2, remove_info/2, - check_in_subscription/6, + check_in_subscription/5, bounce_offline_message/4, disconnect_removed_user/3, get_user_resources/1, @@ -315,16 +315,14 @@ remove_info(JID, Key) -> end end. --spec check_in_subscription(Acc, User, Server, JID, Type, Reason) -> any() | {stop, false} when +-spec check_in_subscription(Acc, ToJID, FromJID, Type, Reason) -> any() | {stop, false} when Acc :: any(), - User :: jid:user(), - Server :: jid:server(), - JID :: jid:jid(), + ToJID :: jid:jid(), + FromJID :: jid:jid(), Type :: any(), Reason :: any(). -check_in_subscription(Acc, User, Server, _JID, _Type, _Reason) -> - JID = jid:make(User, Server, <<>>), - case ejabberd_auth:does_user_exist(JID) of +check_in_subscription(Acc, ToJID, _FromJID, _Type, _Reason) -> + case ejabberd_auth:does_user_exist(ToJID) of true -> Acc; false -> @@ -714,8 +712,7 @@ do_route_no_resource_presence_prv(From, To, Acc, Packet, Type, Reason) -> case is_privacy_allow(From, To, Acc, Packet) of true -> Res = mongoose_hooks:roster_in_subscription(To#jid.lserver, - Acc, - To#jid.user, To#jid.server, From, Type, Reason), + Acc, To, From, Type, Reason), mongoose_acc:get(hook, result, false, Res); false -> false diff --git a/src/metrics/mongoose_metrics_hooks.erl b/src/metrics/mongoose_metrics_hooks.erl index 1635d84a15..f9e117bf77 100644 --- a/src/metrics/mongoose_metrics_hooks.erl +++ b/src/metrics/mongoose_metrics_hooks.erl @@ -27,7 +27,7 @@ roster_get/2, roster_set/4, roster_push/3, - roster_in_subscription/6, + roster_in_subscription/5, register_user/3, remove_user/3, privacy_iq_get/5, @@ -167,14 +167,14 @@ roster_set(Acc, #jid{server = Server}, _, _) -> mongoose_metrics:update(Server, modRosterSets, 1), Acc. --spec roster_in_subscription(term(), binary(), binary(), tuple(), atom(), term()) -> term(). -roster_in_subscription(Acc, _, Server, _, subscribed, _) -> - mongoose_metrics:update(Server, modPresenceSubscriptions, 1), +-spec roster_in_subscription(term(), jid:jid(), jid:jid(), atom(), term()) -> term(). +roster_in_subscription(Acc, #jid{lserver = LServer}, _, subscribed, _) -> + mongoose_metrics:update(LServer, modPresenceSubscriptions, 1), Acc; -roster_in_subscription(Acc, _, Server, _, unsubscribed, _) -> - mongoose_metrics:update(Server, modPresenceUnsubscriptions, 1), +roster_in_subscription(Acc, #jid{lserver = LServer}, _, unsubscribed, _) -> + mongoose_metrics:update(LServer, modPresenceUnsubscriptions, 1), Acc; -roster_in_subscription(Acc, _, _, _, _, _) -> +roster_in_subscription(Acc, _, _, _, _) -> Acc. -spec roster_push(map(), jid:jid(), term()) -> metrics_notify_return(). diff --git a/src/mod_shared_roster_ldap.erl b/src/mod_shared_roster_ldap.erl index ccac3dffc7..9682f3c678 100644 --- a/src/mod_shared_roster_ldap.erl +++ b/src/mod_shared_roster_ldap.erl @@ -40,7 +40,7 @@ handle_info/2, terminate/2, code_change/3]). -export([get_user_roster/2, get_subscription_lists/3, - get_jid_info/4, process_item/2, in_subscription/6, + get_jid_info/4, process_item/2, in_subscription/5, out_subscription/5]). -export([config_change/4]). @@ -181,14 +181,13 @@ get_jid_info({Subscription, Groups}, User, Server, JID) -> end. -spec in_subscription(Acc:: mongoose_acc:t(), - User :: binary(), - Server :: binary(), - JID ::jid:jid(), + ToJID :: jid:jid(), + FromJID :: jid:jid(), Type :: mod_roster:sub_presence(), _Reason :: any()) -> mongoose_acc:t() | {stop, mongoose_acc:t()}. -in_subscription(Acc, User, Server, JID, Type, _Reason) -> - case process_subscription(in, User, Server, JID, Type) of +in_subscription(Acc, ToJID, FromJID, Type, _Reason) -> + case process_subscription(in, ToJID, FromJID, Type) of stop -> {stop, Acc}; {stop, false} -> @@ -203,7 +202,7 @@ in_subscription(Acc, User, Server, JID, Type, _Reason) -> Type :: mod_roster:sub_presence()) -> mongoose_acc:t() | {stop, mongoose_acc:t()}. out_subscription(Acc, User, Server, JID, Type) -> - case process_subscription(out, User, Server, JID, Type) of + case process_subscription(out, jid:make(User, Server, <<>>), JID, Type) of stop -> {stop, Acc}; {stop, false} -> @@ -211,11 +210,9 @@ out_subscription(Acc, User, Server, JID, Type) -> false -> Acc end. -process_subscription(Direction, User, Server, JID, _Type) -> - LUser = jid:nodeprep(User), - LServer = jid:nameprep(Server), +process_subscription(Direction, #jid{luser = LUser, lserver = LServer}, ToJID, _Type) -> US = {LUser, LServer}, - {U1, S1, _} = jid:to_lower(jid:to_bare(JID)), + {U1, S1, _} = jid:to_lower(jid:to_bare(ToJID)), US1 = {U1, S1}, DisplayedGroups = get_user_displayed_groups(US), SRUsers = lists:usort(lists:flatmap( diff --git a/src/mongoose_hooks.erl b/src/mongoose_hooks.erl index 0bfc836a35..6aec3d3c79 100644 --- a/src/mongoose_hooks.erl +++ b/src/mongoose_hooks.erl @@ -73,7 +73,7 @@ roster_get_subscription_lists/3, roster_get_versioning_feature/2, roster_groups/2, - roster_in_subscription/7, + roster_in_subscription/6, roster_out_subscription/5, roster_process_item/2, roster_push/4, @@ -834,21 +834,20 @@ roster_get_versioning_feature(Server, Acc) -> Server, Acc, [Server]). %%% @doc The `roster_in_subscription' hook is called to determine if a subscription presence is routed to a user. --spec roster_in_subscription(LServer, Acc, User, Server, From, Type, Reason) -> Result when +-spec roster_in_subscription(LServer, Acc, To, From, Type, Reason) -> Result when LServer :: jid:lserver(), Acc :: mongoose_acc:t(), - User :: jid:user(), - Server :: jid:server(), + To :: jid:jid(), From :: jid:jid(), Type :: mod_roster:sub_presence(), Reason :: any(), Result :: mongoose_acc:t(). -roster_in_subscription(LServer, Acc, User, Server, From, Type, Reason) -> +roster_in_subscription(LServer, Acc, To, From, Type, Reason) -> ejabberd_hooks:run_fold( roster_in_subscription, LServer, Acc, - [User, Server, From, Type, Reason]). + [To, From, Type, Reason]). %%% @doc The `roster_out_subscription' hook is called when a user sends out subscription. -spec roster_out_subscription(Server, Acc, User, To, Type) -> Result when diff --git a/src/pubsub/mod_pubsub.erl b/src/pubsub/mod_pubsub.erl index 476ea8aeec..3546c6d09f 100644 --- a/src/pubsub/mod_pubsub.erl +++ b/src/pubsub/mod_pubsub.erl @@ -65,7 +65,7 @@ %% exports for hooks -export([presence_probe/4, caps_recognised/4, - in_subscription/6, out_subscription/5, + in_subscription/5, out_subscription/5, on_user_offline/5, remove_user/3, disco_local_identity/5, disco_local_features/5, disco_sm_identity/5, @@ -818,16 +818,15 @@ out_subscription(Acc, _, _, _, _) -> Acc. -spec in_subscription(Acc:: mongoose_acc:t(), - User :: binary(), - Server :: binary(), - JID ::jid:jid(), + ToJID :: jid:jid(), + OwnerJID ::jid:jid(), Type :: mod_roster:sub_presence(), _:: any()) -> mongoose_acc:t(). -in_subscription(Acc, User, Server, Owner, unsubscribed, _) -> - unsubscribe_user(jid:make(User, Server, <<>>), Owner), +in_subscription(Acc, ToJID, OwnerJID, unsubscribed, _) -> + unsubscribe_user(ToJID, OwnerJID), Acc; -in_subscription(Acc, _, _, _, _, _) -> +in_subscription(Acc, _, _, _, _) -> Acc. unsubscribe_user(Entity, Owner) -> From 344b02f169a19c360dc58070d7ba0db9ad87f164 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 3 Nov 2020 14:25:25 +0100 Subject: [PATCH 4/8] Update roster_out_subscription hook --- src/ejabberd_c2s.erl | 6 ++---- src/mod_commands.erl | 10 ++++------ src/mod_shared_roster_ldap.erl | 13 ++++++------- src/mongoose_hooks.erl | 8 ++++---- src/pubsub/mod_pubsub.erl | 18 ++++++++---------- 5 files changed, 24 insertions(+), 31 deletions(-) diff --git a/src/ejabberd_c2s.erl b/src/ejabberd_c2s.erl index 1666711580..e6ba717f7e 100644 --- a/src/ejabberd_c2s.erl +++ b/src/ejabberd_c2s.erl @@ -2054,12 +2054,10 @@ presence_track(Acc, StateData) -> StateData :: state()) -> mongoose_acc:t(). process_presence_subscription_and_route(Acc, Type, StateData) -> From = mongoose_acc:from_jid(Acc), - User = StateData#state.user, Server = StateData#state.server, To = mongoose_acc:to_jid(Acc), - Acc1 = mongoose_hooks:roster_out_subscription(Server, - Acc, - User, To, Type), + Acc1 = mongoose_hooks:roster_out_subscription( + Server, Acc, From, To, Type), check_privacy_and_route(Acc1, jid:to_bare(From), StateData). -spec check_privacy_and_route(Acc :: mongoose_acc:t(), diff --git a/src/mod_commands.erl b/src/mod_commands.erl index 450e18c061..a961e2f609 100644 --- a/src/mod_commands.erl +++ b/src/mod_commands.erl @@ -494,17 +494,15 @@ decode_action(_) -> error. run_subscription(Type, CallerJid, OtherJid) -> StanzaType = atom_to_binary(Type, latin1), El = #xmlel{name = <<"presence">>, attrs = [{<<"type">>, StanzaType}]}, + LServer = CallerJid#jid.lserver, Acc1 = mongoose_acc:new(#{ location => ?LOCATION, from_jid => CallerJid, to_jid => OtherJid, - lserver => CallerJid#jid.lserver, + lserver => LServer, element => El }), % set subscription to - Server = CallerJid#jid.server, - LUser = CallerJid#jid.luser, - Acc2 = mongoose_hooks:roster_out_subscription(Server, - Acc1, - LUser, OtherJid, Type), + Acc2 = mongoose_hooks:roster_out_subscription( + LServer, Acc1, CallerJid, OtherJid, Type), ejabberd_router:route(CallerJid, OtherJid, Acc2), ok. diff --git a/src/mod_shared_roster_ldap.erl b/src/mod_shared_roster_ldap.erl index 9682f3c678..0fcf033df2 100644 --- a/src/mod_shared_roster_ldap.erl +++ b/src/mod_shared_roster_ldap.erl @@ -41,7 +41,7 @@ -export([get_user_roster/2, get_subscription_lists/3, get_jid_info/4, process_item/2, in_subscription/5, - out_subscription/5]). + out_subscription/4]). -export([config_change/4]). @@ -196,13 +196,12 @@ in_subscription(Acc, ToJID, FromJID, Type, _Reason) -> end. -spec out_subscription(Acc:: mongoose_acc:t(), - User :: binary(), - Server :: binary(), - JID ::jid:jid(), - Type :: mod_roster:sub_presence()) -> + FromJID :: jid:jid(), + ToJID ::jid:jid(), + Type :: mod_roster:sub_presence()) -> mongoose_acc:t() | {stop, mongoose_acc:t()}. -out_subscription(Acc, User, Server, JID, Type) -> - case process_subscription(out, jid:make(User, Server, <<>>), JID, Type) of +out_subscription(Acc, FromJID, ToJID, Type) -> + case process_subscription(out, FromJID, ToJID, Type) of stop -> {stop, Acc}; {stop, false} -> diff --git a/src/mongoose_hooks.erl b/src/mongoose_hooks.erl index 6aec3d3c79..c6d57b53f1 100644 --- a/src/mongoose_hooks.erl +++ b/src/mongoose_hooks.erl @@ -850,18 +850,18 @@ roster_in_subscription(LServer, Acc, To, From, Type, Reason) -> [To, From, Type, Reason]). %%% @doc The `roster_out_subscription' hook is called when a user sends out subscription. --spec roster_out_subscription(Server, Acc, User, To, Type) -> Result when +-spec roster_out_subscription(Server, Acc, From, To, Type) -> Result when Server :: jid:server(), Acc :: mongoose_acc:t(), - User :: jid:user(), + From :: jid:jid(), To :: jid:jid(), Type :: mod_roster:sub_presence(), Result :: mongoose_acc:t(). -roster_out_subscription(Server, Acc, User, To, Type) -> +roster_out_subscription(Server, Acc, From, To, Type) -> ejabberd_hooks:run_fold(roster_out_subscription, Server, Acc, - [User, Server, To, Type]). + [From, To, Type]). %%% @doc The `roster_process_item' hook is called when a user's roster is set. -spec roster_process_item(LServer, Item) -> Result when diff --git a/src/pubsub/mod_pubsub.erl b/src/pubsub/mod_pubsub.erl index 3546c6d09f..cb1de9d4e0 100644 --- a/src/pubsub/mod_pubsub.erl +++ b/src/pubsub/mod_pubsub.erl @@ -65,7 +65,7 @@ %% exports for hooks -export([presence_probe/4, caps_recognised/4, - in_subscription/5, out_subscription/5, + in_subscription/5, out_subscription/4, on_user_offline/5, remove_user/3, disco_local_identity/5, disco_local_features/5, disco_sm_identity/5, @@ -799,22 +799,20 @@ notify_send_loop(ServerHost, Action) -> %% subscription hooks handling functions %% --spec out_subscription(Acc:: mongoose_acc:t(), - User :: binary(), - Server :: binary(), - JID ::jid:jid(), +-spec out_subscription(Acc :: mongoose_acc:t(), + FromJID :: jid:jid(), + ToJID :: jid:jid(), Type :: mod_roster:sub_presence()) -> mongoose_acc:t(). -out_subscription(Acc, User, Server, JID, subscribed) -> - Owner = jid:make(User, Server, <<>>), - {PUser, PServer, PResource} = jid:to_lower(JID), +out_subscription(Acc, #jid{lserver = LServer} = FromJID, ToJID, subscribed) -> + {PUser, PServer, PResource} = jid:to_lower(ToJID), PResources = case PResource of <<>> -> user_resources(PUser, PServer); _ -> [PResource] end, - notify_send_loop(Server, {send_last_items_from_owner, Owner, {PUser, PServer, PResources}}), + notify_send_loop(LServer, {send_last_items_from_owner, FromJID, {PUser, PServer, PResources}}), Acc; -out_subscription(Acc, _, _, _, _) -> +out_subscription(Acc, _, _, _) -> Acc. -spec in_subscription(Acc:: mongoose_acc:t(), From 0210be5d92dd3be0356309291418506f05bc5ece Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 3 Nov 2020 16:20:45 +0100 Subject: [PATCH 5/8] Update roster_get_subscription_lists hook --- src/ejabberd_c2s.erl | 6 +++--- src/mod_shared_roster_ldap.erl | 8 +++----- src/mongoose_hooks.erl | 8 ++++---- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/ejabberd_c2s.erl b/src/ejabberd_c2s.erl index e6ba717f7e..3cd1a11469 100644 --- a/src/ejabberd_c2s.erl +++ b/src/ejabberd_c2s.erl @@ -772,9 +772,9 @@ do_open_session(Acc, JID, StateData) -> end end. -do_open_session_common(Acc, JID, #state{user = U, server = S} = NewStateData0) -> +do_open_session_common(Acc, JID, #state{jid = JID, user = U, server = S} = NewStateData0) -> change_shaper(NewStateData0, JID), - Acc1 = mongoose_hooks:roster_get_subscription_lists(S, Acc, U), + Acc1 = mongoose_hooks:roster_get_subscription_lists(S, Acc, JID), {Fs, Ts, Pending} = mongoose_acc:get(roster, subscription_lists, {[], [], []}, Acc1), LJID = jid:to_lower(jid:to_bare(JID)), Fs1 = [LJID | Fs], @@ -1978,7 +1978,7 @@ presence_update_to_available(true, Acc, _, NewPriority, From, Packet, StateData) Acc3 = mongoose_hooks:roster_get_subscription_lists( StateData#state.server, Acc2, - StateData#state.user), + StateData#state.jid), {_, _, Pending} = mongoose_acc:get(roster, subscription_lists, {[], [], []}, Acc3), Acc4 = resend_offline_messages(Acc3, StateData), diff --git a/src/mod_shared_roster_ldap.erl b/src/mod_shared_roster_ldap.erl index 0fcf033df2..55562c72ce 100644 --- a/src/mod_shared_roster_ldap.erl +++ b/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, get_subscription_lists/3, +-export([get_user_roster/2, get_subscription_lists/2, get_jid_info/4, process_item/2, in_subscription/5, out_subscription/4]). @@ -149,11 +149,9 @@ process_item(RosterItem, _Host) -> _ -> RosterItem#roster{subscription = both, ask = none} end. -get_subscription_lists(Acc, User, Server) -> +get_subscription_lists(Acc, #jid{lserver = LServer} = JID) -> {F, T, P} = mongoose_acc:get(roster, subscription_lists, {[], [], []}, Acc), - LUser = jid:nodeprep(User), - LServer = jid:nameprep(Server), - US = {LUser, LServer}, + US = jid:to_lus(JID), DisplayedGroups = get_user_displayed_groups(US), SRUsers = lists:usort(lists:flatmap(fun (Group) -> get_group_users(LServer, Group) diff --git a/src/mongoose_hooks.erl b/src/mongoose_hooks.erl index c6d57b53f1..b49d0cdde7 100644 --- a/src/mongoose_hooks.erl +++ b/src/mongoose_hooks.erl @@ -816,13 +816,13 @@ roster_get_jid_info(LServer, InitialValue, LUser, RemBareJID) -> [LUser, LServer, RemBareJID]). %%% @doc The `roster_get_subscription_lists' hook is called to extract user's subscription list. --spec roster_get_subscription_lists(Server, Acc, User) -> Result when +-spec roster_get_subscription_lists(Server, Acc, JID) -> Result when Server :: jid:server(), Acc ::mongoose_acc:t(), - User :: jid:user(), + JID :: jid:jid(), Result :: mongoose_acc:t(). -roster_get_subscription_lists(Server, Acc, User) -> - ejabberd_hooks:run_fold(roster_get_subscription_lists, Server, Acc, [User, Server]). +roster_get_subscription_lists(Server, Acc, JID) -> + ejabberd_hooks:run_fold(roster_get_subscription_lists, Server, Acc, [JID]). %%% @doc The `roster_get_versioning_feature' hook is called to determine if roster versioning is enabled. -spec roster_get_versioning_feature(Server, Acc) -> Result when From 86eac7e3bb7c78763141a1dac1a7224ae5dda748 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 3 Nov 2020 16:40:16 +0100 Subject: [PATCH 6/8] Update roster_get_jid_info hook --- src/mam/mod_mam_utils.erl | 4 ++-- src/mod_last.erl | 4 +--- src/mod_privacy.erl | 6 +++--- src/mod_shared_roster_ldap.erl | 13 +++++-------- src/mongoose_hooks.erl | 10 +++++----- src/pubsub/mod_pubsub.erl | 7 ++++--- 6 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/mam/mod_mam_utils.erl b/src/mam/mod_mam_utils.erl index 4c9274d77c..c74f362722 100644 --- a/src/mam/mod_mam_utils.erl +++ b/src/mam/mod_mam_utils.erl @@ -1083,10 +1083,10 @@ send_message(From, To, Mess) -> -spec is_jid_in_user_roster(jid:jid(), jid:jid()) -> boolean(). -is_jid_in_user_roster(#jid{lserver=LServer, luser=LUser}, +is_jid_in_user_roster(#jid{lserver = LServer} = ToJID, #jid{} = RemJID) -> RemBareJID = jid:to_bare(RemJID), - {Subscription, _G} = mongoose_hooks:roster_get_jid_info(LServer, {none, []}, LUser, RemBareJID), + {Subscription, _G} = mongoose_hooks:roster_get_jid_info(LServer, {none, []}, ToJID, RemBareJID), Subscription == from orelse Subscription == both. diff --git a/src/mod_last.erl b/src/mod_last.erl index 4fde5f748a..5edc5b4519 100644 --- a/src/mod_last.erl +++ b/src/mod_last.erl @@ -150,9 +150,7 @@ process_sm_iq(From, To, Acc, #iq{type = get, sub_el = SubEl} = IQ) -> User = To#jid.luser, Server = To#jid.lserver, {Subscription, _Groups} = - mongoose_hooks:roster_get_jid_info(Server, - {none, []}, - User, From), + mongoose_hooks:roster_get_jid_info(Server, {none, []}, To, From), MutualSubscription = Subscription == both, RequesterSubscribedToTarget = Subscription == from, QueryingSameUsersLast = (From#jid.luser == To#jid.luser) and diff --git a/src/mod_privacy.erl b/src/mod_privacy.erl index 33f6a5a0c5..7e53c5b0a8 100644 --- a/src/mod_privacy.erl +++ b/src/mod_privacy.erl @@ -323,7 +323,7 @@ check_packet(Acc, User, Server, {Subscription, Groups} = case NeedDb of true -> - roster_get_jid_info(Server, User, LJID); + roster_get_jid_info(Server, jid:make(User, Server, <<>>), LJID); false -> {[], []} end, @@ -666,10 +666,10 @@ broadcast_privacy_list(LUser, LServer, Name, UserList) -> broadcast_privacy_list_packet(Name, UserList) -> {broadcast, {privacy_list, UserList, Name}}. -roster_get_jid_info(Host, User, LJID) -> +roster_get_jid_info(Host, ToJID, LJID) -> mongoose_hooks:roster_get_jid_info(Host, {none, []}, - User, LJID). + ToJID, LJID). config_metrics(Host) -> OptsToReport = [{backend, mnesia}], %list of tuples {option, defualt_value} diff --git a/src/mod_shared_roster_ldap.erl b/src/mod_shared_roster_ldap.erl index 55562c72ce..e99f608f3a 100644 --- a/src/mod_shared_roster_ldap.erl +++ b/src/mod_shared_roster_ldap.erl @@ -40,7 +40,7 @@ handle_info/2, terminate/2, code_change/3]). -export([get_user_roster/2, get_subscription_lists/2, - get_jid_info/4, process_item/2, in_subscription/5, + get_jid_info/3, process_item/2, in_subscription/5, out_subscription/4]). -export([config_change/4]). @@ -161,13 +161,10 @@ get_subscription_lists(Acc, #jid{lserver = LServer} = JID) -> NewLists = {lists:usort(SRJIDs ++ F), lists:usort(SRJIDs ++ T), P}, mongoose_acc:set(roster, subscription_lists, NewLists, Acc). -get_jid_info({Subscription, Groups}, User, Server, JID) -> - LUser = jid:nodeprep(User), - LServer = jid:nameprep(Server), - US = {LUser, LServer}, - {U1, S1, _} = jid:to_lower(JID), - US1 = {U1, S1}, - SRUsers = get_user_to_groups_map(US, false), +get_jid_info({Subscription, Groups}, ToJID, JID) -> + ToUS = jid:to_lus(ToJID), + US1 = jid:to_lus(JID), + SRUsers = get_user_to_groups_map(ToUS, false), case dict:find(US1, SRUsers) of {ok, GroupNames} -> NewGroups = case Groups of diff --git a/src/mongoose_hooks.erl b/src/mongoose_hooks.erl index b49d0cdde7..b832e00c39 100644 --- a/src/mongoose_hooks.erl +++ b/src/mongoose_hooks.erl @@ -805,15 +805,15 @@ roster_groups(LServer, Acc) -> %%% * RemoteBareJID, a bare JID of the other user. %%% %%% The arguments and the return value types correspond to the following spec. --spec roster_get_jid_info(LServer, InitialValue, LUser, RemoteJID) -> Result when - LServer :: jid:lserver(), +-spec roster_get_jid_info(LServer, InitialValue, ToJID, RemoteJID) -> Result when + LServer :: jid:lserver(), InitialValue :: {mod_roster:subscription_state(), []}, - LUser :: jid:luser(), + ToJID :: jid:jid(), RemoteJID :: jid:jid() | jid:simple_jid(), Result :: {mod_roster:subscription_state(), [binary()]}. -roster_get_jid_info(LServer, InitialValue, LUser, RemBareJID) -> +roster_get_jid_info(LServer, InitialValue, ToJID, RemBareJID) -> ejabberd_hooks:run_fold(roster_get_jid_info, LServer, InitialValue, - [LUser, LServer, RemBareJID]). + [ToJID, RemBareJID]). %%% @doc The `roster_get_subscription_lists' hook is called to extract user's subscription list. -spec roster_get_subscription_lists(Server, Acc, JID) -> Result when diff --git a/src/pubsub/mod_pubsub.erl b/src/pubsub/mod_pubsub.erl index cb1de9d4e0..8d69b7eec0 100644 --- a/src/pubsub/mod_pubsub.erl +++ b/src/pubsub/mod_pubsub.erl @@ -3242,9 +3242,10 @@ get_roster_info(_, _, {<<>>, <<>>, _}, _) -> {false, false}; get_roster_info(OwnerUser, OwnerServer, {SubscriberUser, SubscriberServer, _}, AllowedGroups) -> LJID = {SubscriberUser, SubscriberServer, <<>>}, - {Subscription, Groups} = mongoose_hooks:roster_get_jid_info(OwnerServer, - {none, []}, - OwnerUser, LJID), + {Subscription, Groups} = mongoose_hooks:roster_get_jid_info( + OwnerServer, + {none, []}, + jid:make(OwnerUser, OwnerServer, <<>>), LJID), PresenceSubscription = Subscription == both orelse Subscription == from orelse {OwnerUser, OwnerServer} == {SubscriberUser, SubscriberServer}, From b32f418e4ad14662f8cf4eb8f2d05802bf461832 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Mon, 4 Jan 2021 22:20:03 +0100 Subject: [PATCH 7/8] Fnal fixes and improvements --- src/mod_roster.erl | 15 +++++++-------- src/mongoose_hooks.erl | 9 ++++----- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/mod_roster.erl b/src/mod_roster.erl index f02ced8d89..887edd3bd1 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -51,8 +51,7 @@ set_items/2, set_roster_entry/4, remove_from_roster/2, - item_to_xml/1, - roster_version/2 + item_to_xml/1 ]). % Main hooks @@ -331,7 +330,7 @@ get_versioning_feature(Acc, Host) -> false -> [] end. -roster_version(LServer, LUser) -> +roster_version(#jid{luser = LUser, lserver = LServer} = JID) -> case roster_version_on_db(LServer) of true -> case read_roster_version(LUser, LServer) of @@ -339,7 +338,7 @@ roster_version(LServer, LUser) -> V -> V end; false -> - R = get_roster_old(jid:make(LUser, LServer, <<>>)), + R = get_roster_old(JID), roster_hash(R) end. @@ -592,12 +591,12 @@ process_item_els(Item, [{xmlcdata, _} | Els]) -> process_item_els(Item, Els); process_item_els(Item, []) -> Item. -push_item(#jid{luser = LUser, lserver = LServer} = JID, From, Item) -> +push_item(#jid{lserver = LServer} = JID, From, Item) -> ejabberd_sm:route(jid:make_noprep(<<>>, <<>>, <<>>), JID, {broadcast, {item, Item#roster.jid, Item#roster.subscription}}), case roster_versioning_enabled(LServer) of true -> - push_item_version(JID, From, Item, roster_version(LServer, LUser)); + push_item_version(JID, From, Item, roster_version(JID)); false -> lists:foreach(fun(Resource) -> push_item_without_version(JID, Resource, From, Item) @@ -934,7 +933,7 @@ send_unsubscription_to_rosteritems(Acc, JID) -> %% @spec (From::jid(), Item::roster()) -> any() send_unsubscribing_presence(From, #roster{ subscription = Subscription } = Item) -> BareFrom = jid:to_bare(From), - ContactJID = jid:make(Item#roster.jid), + ContactJID = jid:make_noprep(Item#roster.jid), IsTo = Subscription == both orelse Subscription == to, IsFrom = Subscription == both orelse Subscription == from, case IsTo of @@ -1081,7 +1080,7 @@ get_roster_old(DestServer, JID) -> -spec item_to_map(roster()) -> map(). item_to_map(#roster{} = Roster) -> {Name, Host, _} = Roster#roster.jid, - ContactJid = jid:make(Name, Host, <<"">>), + ContactJid = jid:make_noprep(Name, Host, <<>>), ContactName = Roster#roster.name, Subs = Roster#roster.subscription, Groups = Roster#roster.groups, diff --git a/src/mongoose_hooks.erl b/src/mongoose_hooks.erl index b832e00c39..4d05a27178 100644 --- a/src/mongoose_hooks.erl +++ b/src/mongoose_hooks.erl @@ -800,8 +800,7 @@ roster_groups(LServer, Acc) -> %%% @doc The `roster_get_jid_info' hook is called to determine the subscription state between a given pair of users. %%% The hook handlers need to expect following arguments: %%% * Acc with an initial value of {none, []}, -%%% * LUser, a stringprepd username part of the roster's owner, -%%% * LServer, a stringprepd server part of the roster's owner (same value as HookServer), +%%% * ToJID, a stringprepped roster's owner's jid %%% * RemoteBareJID, a bare JID of the other user. %%% %%% The arguments and the return value types correspond to the following spec. @@ -822,7 +821,7 @@ roster_get_jid_info(LServer, InitialValue, ToJID, RemBareJID) -> JID :: jid:jid(), Result :: mongoose_acc:t(). roster_get_subscription_lists(Server, Acc, JID) -> - ejabberd_hooks:run_fold(roster_get_subscription_lists, Server, Acc, [JID]). + ejabberd_hooks:run_fold(roster_get_subscription_lists, Server, Acc, [jid:to_bare(JID)]). %%% @doc The `roster_get_versioning_feature' hook is called to determine if roster versioning is enabled. -spec roster_get_versioning_feature(Server, Acc) -> Result when @@ -847,7 +846,7 @@ roster_in_subscription(LServer, Acc, To, From, Type, Reason) -> roster_in_subscription, LServer, Acc, - [To, From, Type, Reason]). + [jid:to_bare(To), From, Type, Reason]). %%% @doc The `roster_out_subscription' hook is called when a user sends out subscription. -spec roster_out_subscription(Server, Acc, From, To, Type) -> Result when @@ -861,7 +860,7 @@ roster_out_subscription(Server, Acc, From, To, Type) -> ejabberd_hooks:run_fold(roster_out_subscription, Server, Acc, - [From, To, Type]). + [jid:to_bare(From), To, Type]). %%% @doc The `roster_process_item' hook is called when a user's roster is set. -spec roster_process_item(LServer, Item) -> Result when From c476848c490732062aae4589aaf4253cf22e8013 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 30 Oct 2020 13:52:28 +0100 Subject: [PATCH 8/8] DROP: temporarily remove handlers to be deprecated --- src/mod_roster.erl | 92 +++++++++++++++++++++++----------------------- 1 file changed, 47 insertions(+), 45 deletions(-) diff --git a/src/mod_roster.erl b/src/mod_roster.erl index 887edd3bd1..ef622f4fc7 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -67,15 +67,15 @@ get_personal_data/2 ]). -% Deprecated Hooks --export([ - % get_user_roster/2, - in_subscription/6, - out_subscription/5, - get_subscription_lists/3, - get_jid_info/4, - remove_user/3 - ]). +% % Deprecated Hooks +% -export([ +% % get_user_roster/2, +% in_subscription/6, +% out_subscription/5, +% get_subscription_lists/3, +% get_jid_info/4, +% remove_user/3 +% ]). -export([remove_test_user/2, transaction/2, @@ -172,38 +172,38 @@ Item :: term(), Result :: error | roster(). -%==================================================================== -% Deprecated API -%==================================================================== - -define(FUNCTION_STRING, - ?MODULE_STRING ++ ":" ++ - atom_to_list(?FUNCTION_NAME) ++ "/" ++ - integer_to_list(?FUNCTION_ARITY)). - -define(DEPRECATE_FUNCTION, - mongoose_deprecations:log( - {?MODULE, ?FUNCTION_NAME, ?FUNCTION_ARITY}, - #{what => roster_function_deprecated, - text => <<"The function ", (list_to_binary(?FUNCTION_STRING))/binary, - " is deprecated, please use the #jid{} equivalent instead">>}, - [{log_level, warning}])). - in_subscription(Acc, U, S, JID, Type, Reason) -> - ?DEPRECATE_FUNCTION, - in_subscription(Acc, jid:make(U, S, <<>>), JID, Type, Reason). - out_subscription(Acc, U, S, JID, Type) -> - ?DEPRECATE_FUNCTION, - out_subscription(Acc, jid:make(U, S, <<>>), JID, Type). - get_subscription_lists(Acc, U, S) -> - ?DEPRECATE_FUNCTION, - get_subscription_lists(Acc, jid:make(U, S, <<>>)). - get_jid_info(Acc, U, S, JID) -> - ?DEPRECATE_FUNCTION, - get_jid_info(Acc, jid:make(U, S, <<>>), JID). - remove_user(Acc, U, S) -> - ?DEPRECATE_FUNCTION, - remove_user(Acc, jid:make(U, S, <<>>)). -%==================================================================== -% Deprecated API -%==================================================================== +%%==================================================================== +%% Deprecated API +%%==================================================================== +% -define(FUNCTION_STRING, +% ?MODULE_STRING ++ ":" ++ +% atom_to_list(?FUNCTION_NAME) ++ "/" ++ +% integer_to_list(?FUNCTION_ARITY)). +% -define(DEPRECATE_FUNCTION, +% mongoose_deprecations:log( +% {?MODULE, ?FUNCTION_NAME, ?FUNCTION_ARITY}, +% #{what => roster_function_deprecated, +% text => <<"The function ", (list_to_binary(?FUNCTION_STRING))/binary, +% " is deprecated, please use the #jid{} equivalent instead">>}, +% [{log_level, warning}])). +% in_subscription(Acc, U, S, JID, Type, Reason) -> +% ?DEPRECATE_FUNCTION, +% in_subscription(Acc, jid:make(U, S, <<>>), JID, Type, Reason). +% out_subscription(Acc, U, S, JID, Type) -> +% ?DEPRECATE_FUNCTION, +% out_subscription(Acc, jid:make(U, S, <<>>), JID, Type). +% get_subscription_lists(Acc, U, S) -> +% ?DEPRECATE_FUNCTION, +% get_subscription_lists(Acc, jid:make(U, S, <<>>)). +% get_jid_info(Acc, U, S, JID) -> +% ?DEPRECATE_FUNCTION, +% get_jid_info(Acc, jid:make(U, S, <<>>), JID). +% remove_user(Acc, U, S) -> +% ?DEPRECATE_FUNCTION, +% remove_user(Acc, jid:make(U, S, <<>>)). +%%==================================================================== +%% Deprecated API +%%==================================================================== %%-------------------------------------------------------------------- %% gdpr callback @@ -452,10 +452,12 @@ get_user_roster(Acc, #jid{luser = LUser, lserver = LServer}) -> true end, get_roster(LUser, LServer)), mongoose_acc:append(roster, items, Roster, Acc) - end; -get_user_roster(Acc, {U, S}) when is_binary(U), is_binary(S) -> - ?DEPRECATE_FUNCTION, - get_user_roster(Acc, jid:make(U, S, <<>>)). + end + . + % ; +% get_user_roster(Acc, {U, S}) when is_binary(U), is_binary(S) -> + % ?DEPRECATE_FUNCTION, + % get_user_roster(Acc, jid:make(U, S, <<>>)). -spec get_roster(jid:luser(), jid:lserver()) -> [roster()]. get_roster(LUser, LServer) ->