From e841a6ec34e4b79adbeb070e56edec766adc9987 Mon Sep 17 00:00:00 2001 From: Evgeniy Khramtsov Date: Tue, 8 Nov 2016 15:15:19 +0300 Subject: Add more tests for offline storage --- src/ejabberd_sm.erl | 29 ++++++-------- src/mod_offline.erl | 98 +++++++++++++++++++++++++++------------------- src/mod_offline_mnesia.erl | 16 +++++--- src/xmpp_util.erl | 2 +- 4 files changed, 82 insertions(+), 63 deletions(-) (limited to 'src') diff --git a/src/ejabberd_sm.erl b/src/ejabberd_sm.erl index 6f6a196e5..0655bbcf3 100644 --- a/src/ejabberd_sm.erl +++ b/src/ejabberd_sm.erl @@ -498,7 +498,7 @@ do_route(From, #jid{lresource = <<"">>} = To, #presence{} = Packet) -> end, get_user_present_resources(LUser, LServer)); do_route(From, #jid{lresource = <<"">>} = To, #message{type = T} = Packet) -> ?DEBUG("processing message to bare JID:~n~s", [xmpp:pp(Packet)]), - if T == chat; T == headline; T == normal -> + if T == chat; T == headline; T == normal; T == groupchat -> route_message(From, To, Packet, T); true -> Lang = xmpp:get_lang(Packet), @@ -516,7 +516,8 @@ do_route(From, To, Packet) -> case online(Mod:get_sessions(LUser, LServer, LResource)) of [] -> case Packet of - #message{type = T} when T == chat; T == normal -> + #message{type = T} when T == chat; T == normal; + T == headline; T == groupchat -> route_message(From, To, Packet, T); #presence{} -> ?DEBUG("dropping presence to unavalable resource:~n~s", @@ -586,20 +587,16 @@ route_message(From, To, Packet, Type) -> end, PrioRes); _ -> - case Type of - headline -> ok; - _ -> - case ejabberd_auth:is_user_exists(LUser, LServer) andalso - is_privacy_allow(From, To, Packet) of - true -> - ejabberd_hooks:run(offline_message_hook, LServer, - [From, To, Packet]); - false -> - Err = xmpp:make_error(Packet, - xmpp:err_service_unavailable()), - ejabberd_router:route(To, From, Err) - end - end + case ejabberd_auth:is_user_exists(LUser, LServer) andalso + is_privacy_allow(From, To, Packet) of + true -> + ejabberd_hooks:run(offline_message_hook, LServer, + [From, To, Packet]); + false -> + Err = xmpp:make_error(Packet, + xmpp:err_service_unavailable()), + ejabberd_router:route(To, From, Err) + end end. %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% diff --git a/src/mod_offline.erl b/src/mod_offline.erl index 240650234..6134823c1 100644 --- a/src/mod_offline.erl +++ b/src/mod_offline.erl @@ -102,7 +102,7 @@ -callback read_message_headers(binary(), binary()) -> any(). -callback read_message(binary(), binary(), non_neg_integer()) -> {ok, #offline_msg{}} | error. --callback remove_message(binary(), binary(), non_neg_integer()) -> ok. +-callback remove_message(binary(), binary(), non_neg_integer()) -> ok | {error, any()}. -callback read_all_messages(binary(), binary()) -> [#offline_msg{}]. -callback remove_all_messages(binary(), binary()) -> {atomic, any()}. -callback count_messages(binary(), binary()) -> non_neg_integer(). @@ -315,32 +315,52 @@ get_info(Acc, _From, _To, _Node, _Lang) -> Acc. -spec handle_offline_query(iq()) -> iq(). +handle_offline_query(#iq{from = #jid{luser = U1, lserver = S1}, + to = #jid{luser = U2, lserver = S2}, + lang = Lang, + sub_els = [#offline{}]} = IQ) + when {U1, S1} /= {U2, S2} -> + Txt = <<"Query to another users is forbidden">>, + xmpp:make_error(IQ, xmpp:err_forbidden(Txt, Lang)); handle_offline_query(#iq{from = #jid{luser = U, lserver = S} = From, to = #jid{luser = U, lserver = S} = _To, - type = Type, - sub_els = [#offline{purge = Purge, - items = Items, - fetch = Fetch}]} = IQ) -> - case Type of - get -> - if Fetch -> handle_offline_fetch(From); - true -> handle_offline_items_view(From, Items) + type = Type, lang = Lang, + sub_els = [#offline{} = Offline]} = IQ) -> + case {Type, Offline} of + {get, #offline{fetch = true, items = [], purge = false}} -> + %% TODO: report database errors + handle_offline_fetch(From), + xmpp:make_iq_result(IQ); + {get, #offline{fetch = false, items = [_|_] = Items, purge = false}} -> + case handle_offline_items_view(From, Items) of + true -> xmpp:make_iq_result(IQ); + false -> xmpp:make_error(IQ, xmpp:err_item_not_found()) end; - set -> - if Purge -> delete_all_msgs(U, S); - true -> handle_offline_items_remove(From, Items) - end - end, - xmpp:make_iq_result(IQ); + {set, #offline{fetch = false, items = [], purge = true}} -> + case delete_all_msgs(U, S) of + {atomic, ok} -> + xmpp:make_iq_result(IQ); + _Err -> + Txt = <<"Database failure">>, + xmpp:make_error(IQ, xmpp:err_internal_server_error(Txt, Lang)) + end; + {set, #offline{fetch = false, items = [_|_] = Items, purge = false}} -> + case handle_offline_items_remove(From, Items) of + true -> xmpp:make_iq_result(IQ); + false -> xmpp:make_error(IQ, xmpp:err_item_not_found()) + end; + _ -> + xmpp:make_error(IQ, xmpp:err_bad_request()) + end; handle_offline_query(#iq{lang = Lang} = IQ) -> - Txt = <<"Query to another users is forbidden">>, - xmpp:make_error(IQ, xmpp:err_forbidden(Txt, Lang)). + Txt = <<"No module is handling this query">>, + xmpp:make_error(IQ, xmpp:err_service_unavailable(Txt, Lang)). --spec handle_offline_items_view(jid(), [offline_item()]) -> ok. +-spec handle_offline_items_view(jid(), [offline_item()]) -> boolean(). handle_offline_items_view(JID, Items) -> {U, S, R} = jid:tolower(JID), - lists:foreach( - fun(#offline_item{node = Node, action = view}) -> + lists:foldl( + fun(#offline_item{node = Node, action = view}, Acc) -> case fetch_msg_by_node(JID, Node) of {ok, OfflineMsg} -> case offline_msg_to_route(S, OfflineMsg) of @@ -351,25 +371,22 @@ handle_offline_items_view(JID, Items) -> Pid ! {route, From, To, NewEl}; none -> ok - end; + end, + Acc or true; error -> - ok + Acc or false end; error -> - ok - end; - (_) -> - ok - end, Items). + Acc or false + end + end, false, Items). --spec handle_offline_items_remove(jid(), [offline_item()]) -> ok. +-spec handle_offline_items_remove(jid(), [offline_item()]) -> boolean(). handle_offline_items_remove(JID, Items) -> - lists:foreach( - fun(#offline_item{node = Node, action = remove}) -> - remove_msg_by_node(JID, Node); - (_) -> - ok - end, Items). + lists:foldl( + fun(#offline_item{node = Node, action = remove}, Acc) -> + Acc or remove_msg_by_node(JID, Node) + end, false, Items). -spec set_offline_tag(message(), binary()) -> message(). set_offline_tag(Msg, Node) -> @@ -401,23 +418,22 @@ fetch_msg_by_node(To, Seq) -> error end. --spec remove_msg_by_node(jid(), binary()) -> ok. +-spec remove_msg_by_node(jid(), binary()) -> boolean(). remove_msg_by_node(To, Seq) -> case catch binary_to_integer(Seq) of I when is_integer(I), I>= 0 -> LUser = To#jid.luser, LServer = To#jid.lserver, Mod = gen_mod:db_mod(LServer, ?MODULE), - Mod:remove_message(LUser, LServer, I); + Mod:remove_message(LUser, LServer, I), + true; _ -> - ok + false end. -spec need_to_store(binary(), message()) -> boolean(). need_to_store(_LServer, #message{type = error}) -> false; -need_to_store(_LServer, #message{type = groupchat}) -> false; -need_to_store(_LServer, #message{type = headline}) -> false; -need_to_store(LServer, Packet) -> +need_to_store(LServer, #message{type = Type} = Packet) -> case xmpp:has_subtag(Packet, #offline{}) of false -> case check_store_hint(Packet) of @@ -425,6 +441,8 @@ need_to_store(LServer, Packet) -> true; no_store -> false; + none when Type == headline; Type == groupchat -> + false; none -> case gen_mod:get_module_opt( LServer, ?MODULE, store_empty_body, diff --git a/src/mod_offline_mnesia.erl b/src/mod_offline_mnesia.erl index e8db08ddf..c9f088fa4 100644 --- a/src/mod_offline_mnesia.erl +++ b/src/mod_offline_mnesia.erl @@ -127,12 +127,16 @@ read_message(LUser, LServer, I) -> remove_message(LUser, LServer, I) -> US = {LUser, LServer}, TS = integer_to_now(I), - Msgs = mnesia:dirty_match_object( - offline_msg, #offline_msg{us = US, timestamp = TS, _ = '_'}), - lists:foreach( - fun(Msg) -> - mnesia:dirty_delete_object(Msg) - end, Msgs). + case mnesia:dirty_match_object( + offline_msg, #offline_msg{us = US, timestamp = TS, _ = '_'}) of + [] -> + {error, notfound}; + Msgs -> + lists:foreach( + fun(Msg) -> + mnesia:dirty_delete_object(Msg) + end, Msgs) + end. read_all_messages(LUser, LServer) -> US = {LUser, LServer}, diff --git a/src/xmpp_util.erl b/src/xmpp_util.erl index 102d88412..fb3bbc7ab 100644 --- a/src/xmpp_util.erl +++ b/src/xmpp_util.erl @@ -70,7 +70,7 @@ unwrap_carbon(Stanza) -> Stanza. is_standalone_chat_state(Stanza) -> case unwrap_carbon(Stanza) of #message{body = [], subject = [], sub_els = Els} -> - IgnoreNS = [?NS_CHATSTATES, ?NS_DELAY], + IgnoreNS = [?NS_CHATSTATES, ?NS_DELAY, ?NS_EVENT], Stripped = [El || El <- Els, not lists:member(xmpp:get_ns(El), IgnoreNS)], Stripped == []; -- cgit v1.2.3