From 0e96d64e6031b372680e681a4b3602dcad7d801d Mon Sep 17 00:00:00 2001
From: Holger Weiss <holger@zedat.fu-berlin.de>
Date: Sun, 5 Jan 2020 20:08:54 +0100
Subject: [PATCH] Omit push notifications if offline storage failed

This commit removes the 'store_offline_message' hook which didn't allow
mod_push to suppress notifications when storing an offline message
failed (due to the offline spool size limit being exceeded or due to
database issues).

Fixes #3120.
---
 src/mod_offline.erl | 34 ++++++++++++++--------------------
 src/mod_push.erl    | 22 +++++++++++++---------
 2 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/src/mod_offline.erl b/src/mod_offline.erl
index c3fca8868..6a3f3f2d7 100644
--- a/src/mod_offline.erl
+++ b/src/mod_offline.erl
@@ -493,26 +493,20 @@ store_packet({_Action, #message{from = From, to = To} = Packet} = Acc) ->
 	    case check_event(Packet) of
 		true ->
 		    #jid{luser = LUser, lserver = LServer} = To,
-		    case ejabberd_hooks:run_fold(store_offline_message, LServer,
-						 Packet, []) of
-			drop ->
-			    Acc;
-			NewPacket ->
-			    TimeStamp = erlang:timestamp(),
-			    Expire = find_x_expire(TimeStamp, NewPacket),
-			    OffMsg = #offline_msg{us = {LUser, LServer},
-						  timestamp = TimeStamp,
-						  expire = Expire,
-						  from = From,
-						  to = To,
-						  packet = NewPacket},
-			    case store_offline_msg(OffMsg) of
-				ok ->
-				    {offlined, NewPacket};
-				{error, Reason} ->
-				    discard_warn_sender(Packet, Reason),
-				    stop
-			    end
+		    TimeStamp = erlang:timestamp(),
+		    Expire = find_x_expire(TimeStamp, Packet),
+		    OffMsg = #offline_msg{us = {LUser, LServer},
+					  timestamp = TimeStamp,
+					  expire = Expire,
+					  from = From,
+					  to = To,
+					  packet = Packet},
+		    case store_offline_msg(OffMsg) of
+			ok ->
+			    {offlined, Packet};
+			{error, Reason} ->
+			    discard_warn_sender(Packet, Reason),
+			    stop
 		    end;
 		_ ->
 		    maybe_update_cache(To, Packet),
diff --git a/src/mod_push.erl b/src/mod_push.erl
index 409ded8cd..c9804a7e5 100644
--- a/src/mod_push.erl
+++ b/src/mod_push.erl
@@ -209,8 +209,8 @@ register_hooks(Host) ->
 		       c2s_stanza, 50),
     ejabberd_hooks:add(store_mam_message, Host, ?MODULE,
 		       mam_message, 50),
-    ejabberd_hooks:add(store_offline_message, Host, ?MODULE,
-		       offline_message, 50),
+    ejabberd_hooks:add(offline_message_hook, Host, ?MODULE,
+		       offline_message, 55),
     ejabberd_hooks:add(remove_user, Host, ?MODULE,
 		       remove_user, 50).
 
@@ -228,8 +228,8 @@ unregister_hooks(Host) ->
 			  c2s_stanza, 50),
     ejabberd_hooks:delete(store_mam_message, Host, ?MODULE,
 			  mam_message, 50),
-    ejabberd_hooks:delete(store_offline_message, Host, ?MODULE,
-			  offline_message, 50),
+    ejabberd_hooks:delete(offline_message_hook, Host, ?MODULE,
+			  offline_message, 55),
     ejabberd_hooks:delete(remove_user, Host, ?MODULE,
 			  remove_user, 50).
 
@@ -373,10 +373,12 @@ mam_message(#message{} = Pkt, LUser, LServer, _Peer, _Nick, chat, Dir) ->
 mam_message(Pkt, _LUser, _LServer, _Peer, _Nick, _Type, _Dir) ->
     Pkt.
 
--spec offline_message(message()) -> message().
-offline_message(#message{meta = #{mam_archived := true}} = Pkt) ->
-    Pkt; % Push notification was triggered via MAM.
-offline_message(#message{to = #jid{luser = LUser, lserver = LServer}} = Pkt) ->
+-spec offline_message({any(), message()}) -> {any(), message()}.
+offline_message({offlined, #message{meta = #{mam_archived := true}}} = Acc) ->
+    Acc; % Push notification was triggered via MAM.
+offline_message({offlined,
+		 #message{to = #jid{luser = LUser,
+				    lserver = LServer}} = Pkt} = Acc) ->
     case lookup_sessions(LUser, LServer) of
 	{ok, [_|_] = Clients} ->
 	    ?DEBUG("Notifying ~ts@~ts of offline message", [LUser, LServer]),
@@ -384,7 +386,9 @@ offline_message(#message{to = #jid{luser = LUser, lserver = LServer}} = Pkt) ->
 	_ ->
 	    ok
     end,
-    Pkt.
+    Acc;
+offline_message(Acc) ->
+    Acc.
 
 -spec c2s_session_pending(c2s_state()) -> c2s_state().
 c2s_session_pending(#{push_enabled := true, mgmt_queue := Queue} = State) ->