Add more tests for offline storage

4 files changed, 82 insertions, 63 deletions

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 == [];