From 11fa02cd6c825ac59813c36aa947b83d7d6b9af5 Mon Sep 17 00:00:00 2001 From: Evgeny Khramtsov Date: Tue, 9 Jul 2019 14:30:59 +0300 Subject: [PATCH] Improve type specs of mod_roster --- src/mod_roster.erl | 118 +++++++++++++++++++++++++---------------- src/mod_roster_sql.erl | 2 +- 2 files changed, 72 insertions(+), 48 deletions(-) diff --git a/src/mod_roster.erl b/src/mod_roster.erl index 5c880019f..426589319 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -48,7 +48,7 @@ out_subscription/1, set_items/3, remove_user/2, get_jid_info/4, encode_item/1, webadmin_page/3, webadmin_user/4, get_versioning_feature/2, - roster_versioning_enabled/1, roster_version/2, + roster_version/2, mod_opt_type/1, mod_options/1, set_roster/1, del_roster/3, process_rosteritems/5, depends/2, set_item_and_notify_clients/3]). @@ -65,6 +65,7 @@ -define(ROSTER_ITEM_CACHE, roster_item_cache). -define(ROSTER_VERSION_CACHE, roster_version_cache). +-type c2s_state() :: ejabberd_c2s:state(). -export_type([subscription/0]). -callback init(binary(), gen_mod:opts()) -> any(). @@ -76,7 +77,7 @@ -callback read_subscription_and_groups(binary(), binary(), ljid()) -> {ok, {subscription(), ask(), [binary()]}} | error. -callback roster_subscribe(binary(), binary(), ljid(), #roster{}) -> any(). --callback transaction(binary(), function()) -> {atomic, any()} | {aborted, any()}. +-callback transaction(binary(), fun(() -> T)) -> {atomic, T} | {aborted, any()}. -callback remove_user(binary(), binary()) -> any(). -callback update_roster(binary(), binary(), ljid(), #roster{}) -> any(). -callback del_roster(binary(), binary(), ljid()) -> any(). @@ -145,6 +146,7 @@ reload(Host, NewOpts, OldOpts) -> depends(_Host, _Opts) -> []. +-spec process_iq(iq()) -> iq(). process_iq(#iq{from = #jid{luser = U, lserver = S}, to = #jid{luser = U, lserver = S}} = IQ) -> process_local_iq(IQ); @@ -158,6 +160,7 @@ process_iq(#iq{lang = Lang, to = To} = IQ) -> process_local_iq(IQ) end. +-spec process_local_iq(iq()) -> iq(). process_local_iq(#iq{type = set,lang = Lang, sub_els = [#roster_query{ items = [#roster_item{ask = Ask}]}]} = IQ) @@ -199,24 +202,19 @@ process_local_iq(#iq{lang = Lang} = IQ) -> Txt = ?T("No module is handling this query"), xmpp:make_error(IQ, xmpp:err_service_unavailable(Txt, Lang)). +-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} <- Items]))). -roster_versioning_enabled(Host) -> - mod_roster_opt:versioning(Host). - -roster_version_on_db(Host) -> - mod_roster_opt:store_current_id(Host). - %% Returns a list that may contain an xmlelement with the XEP-237 feature if it's enabled. -spec get_versioning_feature([xmpp_element()], binary()) -> [xmpp_element()]. get_versioning_feature(Acc, Host) -> case gen_mod:is_loaded(Host, ?MODULE) of true -> - case roster_versioning_enabled(Host) of + case mod_roster_opt:versioning(Host) of true -> [#rosterver_feature{}|Acc]; false -> @@ -226,9 +224,10 @@ get_versioning_feature(Acc, Host) -> Acc end. +-spec roster_version(binary(), binary()) -> undefined | binary(). roster_version(LServer, LUser) -> US = {LUser, LServer}, - case roster_version_on_db(LServer) of + case mod_roster_opt:store_current_id(LServer) of true -> case read_roster_version(LUser, LServer) of error -> undefined; @@ -239,6 +238,7 @@ roster_version(LServer, LUser) -> [], [US])) end. +-spec read_roster_version(binary(), binary()) -> {ok, binary()} | error. read_roster_version(LUser, LServer) -> ets_cache:lookup( ?ROSTER_VERSION_CACHE, {LUser, LServer}, @@ -247,12 +247,15 @@ read_roster_version(LUser, LServer) -> Mod:read_roster_version(LUser, LServer) end). +-spec write_roster_version(binary(), binary()) -> binary(). write_roster_version(LUser, LServer) -> write_roster_version(LUser, LServer, false). +-spec write_roster_version_t(binary(), binary()) -> binary(). write_roster_version_t(LUser, LServer) -> write_roster_version(LUser, LServer, true). +-spec write_roster_version(binary(), binary(), boolean()) -> binary(). write_roster_version(LUser, LServer, InTransaction) -> Ver = str:sha(term_to_binary(erlang:unique_integer())), Mod = gen_mod:db_mod(LServer, ?MODULE), @@ -270,14 +273,15 @@ write_roster_version(LUser, LServer, InTransaction) -> %% - roster versioning is not used by the client OR %% - roster versioning is used by server and client, BUT the server isn't storing versions on db OR %% - the roster version from client don't match current version. +-spec process_iq_get(iq()) -> iq(). process_iq_get(#iq{to = To, sub_els = [#roster_query{ver = RequestedVersion}]} = IQ) -> LUser = To#jid.luser, LServer = To#jid.lserver, US = {LUser, LServer}, {ItemsToSend, VersionToSend} = - case {roster_versioning_enabled(LServer), - roster_version_on_db(LServer)} of + case {mod_roster_opt:versioning(LServer), + mod_roster_opt:store_current_id(LServer)} of {true, true} when RequestedVersion /= undefined -> case read_roster_version(LUser, LServer) of error -> @@ -332,6 +336,7 @@ get_user_roster(Acc, {LUser, LServer}) -> Items) ++ Acc. +-spec get_roster(binary(), binary()) -> [#roster{}]. get_roster(LUser, LServer) -> Mod = gen_mod:db_mod(LServer, ?MODULE), R = case use_cache(Mod, LServer, roster) of @@ -347,6 +352,7 @@ get_roster(LUser, LServer) -> error -> [] end. +-spec get_roster_item(binary(), binary(), ljid()) -> #roster{}. get_roster_item(LUser, LServer, LJID) -> Mod = gen_mod:db_mod(LServer, ?MODULE), case Mod:get_roster_item(LUser, LServer, LJID) of @@ -358,6 +364,8 @@ get_roster_item(LUser, LServer, LJID) -> us = {LUser, LServer}, jid = LBJID} end. +-spec get_subscription_and_groups(binary(), binary(), ljid()) -> + {subscription(), ask(), [binary()]}. get_subscription_and_groups(LUser, LServer, LJID) -> LBJID = jid:remove_resource(LJID), Mod = gen_mod:db_mod(LServer, ?MODULE), @@ -392,6 +400,7 @@ get_subscription_and_groups(LUser, LServer, LJID) -> {none, none, []} end. +-spec set_roster(#roster{}) -> {atomic | aborted, any()}. set_roster(#roster{us = {LUser, LServer}, jid = LJID} = Item) -> transaction( LUser, LServer, [LJID], @@ -399,6 +408,7 @@ set_roster(#roster{us = {LUser, LServer}, jid = LJID} = Item) -> update_roster_t(LUser, LServer, LJID, Item) end). +-spec del_roster(binary(), binary(), ljid()) -> {atomic | aborted, any()}. del_roster(LUser, LServer, LJID) -> transaction( LUser, LServer, [LJID], @@ -406,6 +416,7 @@ del_roster(LUser, LServer, LJID) -> del_roster_t(LUser, LServer, LJID) end). +-spec encode_item(#roster{}) -> roster_item(). encode_item(Item) -> #roster_item{jid = jid:make(Item#roster.jid), name = Item#roster.name, @@ -417,6 +428,7 @@ encode_item(Item) -> end, groups = Item#roster.groups}. +-spec decode_item(roster_item(), #roster{}, boolean()) -> #roster{}. decode_item(#roster_item{subscription = remove} = Item, R, _) -> R#roster{jid = jid:tolower(Item#roster_item.jid), name = <<"">>, @@ -434,55 +446,56 @@ decode_item(Item, R, Managed) -> end, groups = Item#roster_item.groups}. -process_iq_set(#iq{from = _From, to = To, +-spec process_iq_set(iq()) -> iq(). +process_iq_set(#iq{from = _From, to = To, lang = Lang, sub_els = [#roster_query{items = [QueryItem]}]} = IQ) -> case set_item_and_notify_clients(To, QueryItem, false) of ok -> xmpp:make_iq_result(IQ); - E -> - ?ERROR_MSG("Roster set failed:~nIQ = ~s~nError = ~p", - [xmpp:pp(IQ), E]), - xmpp:make_error(IQ, xmpp:err_internal_server_error()) + {error, _} -> + Txt = ?T("Database failure"), + Err = xmpp:err_internal_server_error(Txt, Lang), + xmpp:make_error(IQ, Err) end. --spec set_item_and_notify_clients(jid(), #roster_item{}, boolean()) -> ok | error. +-spec set_item_and_notify_clients(jid(), #roster_item{}, boolean()) -> ok | {error, any()}. set_item_and_notify_clients(To, #roster_item{jid = PeerJID} = RosterItem, OverrideSubscription) -> #jid{luser = LUser, lserver = LServer} = To, PeerLJID = jid:tolower(PeerJID), F = fun () -> - Item = get_roster_item(LUser, LServer, PeerLJID), - Item2 = decode_item(RosterItem, Item, OverrideSubscription), - Item3 = ejabberd_hooks:run_fold(roster_process_item, - LServer, Item2, - [LServer]), - case Item3#roster.subscription of - remove -> del_roster_t(LUser, LServer, PeerLJID); - _ -> update_roster_t(LUser, LServer, PeerLJID, Item3) - end, - case roster_version_on_db(LServer) of - true -> write_roster_version_t(LUser, LServer); - false -> ok - end, - {Item, Item3} + Item1 = get_roster_item(LUser, LServer, PeerLJID), + Item2 = decode_item(RosterItem, Item1, OverrideSubscription), + Item3 = ejabberd_hooks:run_fold(roster_process_item, + LServer, Item2, + [LServer]), + case Item3#roster.subscription of + remove -> del_roster_t(LUser, LServer, PeerLJID); + _ -> update_roster_t(LUser, LServer, PeerLJID, Item3) + end, + case mod_roster_opt:store_current_id(LServer) of + true -> write_roster_version_t(LUser, LServer); + false -> ok + end, + {Item1, Item3} end, case transaction(LUser, LServer, [PeerLJID], F) of - {atomic, {OldItem, Item}} -> - push_item(To, OldItem, Item), - case Item#roster.subscription of + {atomic, {OldItem, NewItem}} -> + push_item(To, OldItem, NewItem), + case NewItem#roster.subscription of remove -> send_unsubscribing_presence(To, OldItem); _ -> ok - end, - ok; - E -> - E + end; + {aborted, Reason} -> + {error, Reason} end. +-spec push_item(jid(), #roster{}, #roster{}) -> ok. push_item(To, OldItem, NewItem) -> #jid{luser = LUser, lserver = LServer} = To, - Ver = case roster_versioning_enabled(LServer) of + Ver = case mod_roster_opt:versioning(LServer) of true -> roster_version(LServer, LUser); false -> undefined end, @@ -492,6 +505,7 @@ 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. push_item(To, OldItem, NewItem, Ver) -> route_presence_change(To, OldItem, NewItem), IQ = #iq{type = set, to = To, @@ -530,14 +544,17 @@ route_presence_change(From, OldItem, NewItem) -> ok end. +-spec ask_to_pending(ask()) -> none | in | out | both. ask_to_pending(subscribe) -> out; ask_to_pending(unsubscribe) -> none; ask_to_pending(Ask) -> Ask. +-spec roster_subscribe_t(binary(), binary(), ljid(), #roster{}) -> any(). roster_subscribe_t(LUser, LServer, LJID, Item) -> Mod = gen_mod:db_mod(LServer, ?MODULE), Mod:roster_subscribe(LUser, LServer, LJID, Item). +-spec transaction(binary(), binary(), [ljid()], fun(() -> T)) -> {atomic, T} | {aborted, any()}. transaction(LUser, LServer, LJIDs, F) -> Mod = gen_mod:db_mod(LServer, ?MODULE), case Mod:transaction(LServer, F) of @@ -563,6 +580,9 @@ out_subscription(#presence{from = From, to = JID, type = Type}) -> #jid{user = User, server = Server} = From, process_subscription(out, User, Server, JID, Type, <<"">>). +-spec process_subscription(in | out, binary(), binary(), jid(), + subscribe | subscribed | unsubscribe | unsubscribed, + binary()) -> boolean(). process_subscription(Direction, User, Server, JID1, Type, Reason) -> LUser = jid:nodeprep(User), @@ -600,7 +620,7 @@ process_subscription(Direction, User, Server, JID1, ask = Pending, askmessage = AskMessage}, roster_subscribe_t(LUser, LServer, LJID, NewItem), - case roster_version_on_db(LServer) of + case mod_roster_opt:store_current_id(LServer) of true -> write_roster_version_t(LUser, LServer); false -> ok end, @@ -761,6 +781,7 @@ remove_user(User, Server) -> %% For each contact with Subscription: %% Both or From, send a "unsubscribed" presence stanza; %% Both or To, send a "unsubscribe" presence stanza. +-spec send_unsubscription_to_rosteritems(binary(), binary(), [#roster{}]) -> ok. send_unsubscription_to_rosteritems(LUser, LServer, RosterItems) -> From = jid:make({LUser, LServer, <<"">>}), lists:foreach(fun (RosterItem) -> @@ -768,6 +789,7 @@ send_unsubscription_to_rosteritems(LUser, LServer, RosterItems) -> end, RosterItems). +-spec send_unsubscribing_presence(jid(), #roster{}) -> ok. send_unsubscribing_presence(From, Item) -> IsTo = case Item#roster.subscription of both -> true; @@ -792,12 +814,11 @@ send_unsubscribing_presence(From, Item) -> from = jid:remove_resource(From), to = jid:make(Item#roster.jid)}); true -> ok - end, - ok. + end. %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% --spec set_items(binary(), binary(), roster_query()) -> any(). +-spec set_items(binary(), binary(), roster_query()) -> {atomic, ok} | {aborted, any()}. set_items(User, Server, #roster_query{items = Items}) -> LUser = jid:nodeprep(User), LServer = jid:nameprep(Server), @@ -810,14 +831,17 @@ set_items(User, Server, #roster_query{items = Items}) -> end, transaction(LUser, LServer, LJIDs, F). +-spec update_roster_t(binary(), binary(), ljid(), #roster{}) -> any(). update_roster_t(LUser, LServer, LJID, Item) -> Mod = gen_mod:db_mod(LServer, ?MODULE), Mod:update_roster(LUser, LServer, LJID, Item). +-spec del_roster_t(binary(), binary(), ljid()) -> any(). del_roster_t(LUser, LServer, LJID) -> Mod = gen_mod:db_mod(LServer, ?MODULE), Mod:del_roster(LUser, LServer, LJID). +-spec process_item_set_t(binary(), binary(), roster_item()) -> any(). process_item_set_t(LUser, LServer, #roster_item{jid = JID1} = QueryItem) -> JID = {JID1#jid.user, JID1#jid.server, <<>>}, LJID = {JID1#jid.luser, JID1#jid.lserver, <<>>}, @@ -830,8 +854,7 @@ process_item_set_t(LUser, LServer, #roster_item{jid = JID1} = QueryItem) -> end; process_item_set_t(_LUser, _LServer, _) -> ok. --spec c2s_self_presence({presence(), ejabberd_c2s:state()}) - -> {presence(), ejabberd_c2s:state()}. +-spec c2s_self_presence({presence(), c2s_state()}) -> {presence(), c2s_state()}. c2s_self_presence({_, #{pres_last := _}} = Acc) -> Acc; c2s_self_presence({#presence{type = available} = Pkt, State}) -> @@ -845,7 +868,7 @@ c2s_self_presence({#presence{type = available} = Pkt, State}) -> c2s_self_presence(Acc) -> Acc. --spec resend_pending_subscriptions(ejabberd_c2s:state()) -> ejabberd_c2s:state(). +-spec resend_pending_subscriptions(c2s_state()) -> c2s_state(). resend_pending_subscriptions(#{jid := JID} = State) -> BareJID = jid:remove_resource(JID), Result = get_roster(JID#jid.luser, JID#jid.lserver), @@ -1099,6 +1122,7 @@ webadmin_user(Acc, _User, _Server, Lang) -> [?XE(<<"h3">>, [?ACT(<<"roster/">>, ?T("Roster"))])]. %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% +-spec has_duplicated_groups([binary()]) -> boolean(). has_duplicated_groups(Groups) -> GroupsPrep = lists:usort([jid:resourceprep(G) || G <- Groups]), not (length(GroupsPrep) == length(Groups)). diff --git a/src/mod_roster_sql.erl b/src/mod_roster_sql.erl index f62695a89..e2deb17b3 100644 --- a/src/mod_roster_sql.erl +++ b/src/mod_roster_sql.erl @@ -408,7 +408,7 @@ process_rosteritems_sql(ActionS, Subscription, Ask, SLocalJID, SJID) -> " and subscription LIKE %(SSubscription)s" " and ask LIKE %(SAsk)s")), case ActionS of - "delete" -> [mod_roster:del_roster(User, LServer, jid:decode(Contact)) || {User, Contact} <- List]; + "delete" -> [mod_roster:del_roster(User, LServer, jid:tolower(jid:decode(Contact))) || {User, Contact} <- List]; "list" -> ok end, List.