Merge pull request #3366 from esl/inbox/fixes

Inbox - log errors when they happen

include/mod_inbox.hrl

@@ -1,9 +1,3 @@
--type username() :: jid:luser().
-
--type host() :: jid:lserver().
-
--type sender() :: binary().
-
 -type content() :: binary().
 
 -type count_bin() :: binary().
@@ -25,6 +19,4 @@
                               archive := boolean(),
                               muted_until := integer()}.
 
--type inbox_write_res() :: ok | {error, any()}.
-
 -type marker() :: binary().

src/inbox/mod_inbox.erl

@@ -1,20 +1,17 @@
 %%%-------------------------------------------------------------------
-%%% @author ludwikbukowski
 %%% @copyright (C) 2018, Erlang-Solutions
 %%% @doc
 %%%
 %%% @end
 %%% Created : 30. Jan 2018 13:22
 %%%-------------------------------------------------------------------
 -module(mod_inbox).
--author("ludwikbukowski").
 
 -behaviour(gen_mod).
 -behaviour(mongoose_module_metrics).
 
 -include("jlib.hrl").
 -include("mod_inbox.hrl").
--include("mongoose.hrl").
 -include("mongoose_config_spec.hrl").
 -include("mongoose_logger.hrl").
 -include("mongoose_ns.hrl").
@@ -61,7 +58,11 @@
         archive => boolean()
        }.
 
+-type count_res() :: ok | {ok, non_neg_integer()} | {error, term()}.
+-type write_res() :: ok | {error, any()}.
+
 -export_type([entry_key/0, get_inbox_params/0]).
+-export_type([count_res/0, write_res/0]).
 
 -callback init(Host, Opts) -> ok when
       Host :: mongooseim:host_type(),
@@ -73,7 +74,7 @@
       LServer :: jid:lserver(),
       Params :: get_inbox_params().
 
--callback clear_inbox(HostType, LUser, LServer) -> inbox_write_res() when
+-callback clear_inbox(HostType, LUser, LServer) -> write_res() when
       HostType :: mongooseim:host_type(),
       LUser :: jid:luser(),
       LServer :: jid:lserver().
@@ -83,27 +84,27 @@
       LServer :: jid:lserver().
 
 -callback set_inbox(HostType, InboxEntryKey, Content, Count, MsgId, Timestamp) ->
-    inbox_write_res() when
+    write_res() when
       HostType :: mongooseim:host_type(),
       InboxEntryKey :: entry_key(),
       Content :: binary(),
       Count :: integer(),
       MsgId :: binary(),
       Timestamp :: integer().
 
--callback remove_inbox_row(HostType, InboxEntryKey) -> inbox_write_res() when
+-callback remove_inbox_row(HostType, InboxEntryKey) -> write_res() when
       HostType :: mongooseim:host_type(),
       InboxEntryKey :: entry_key().
 
 -callback set_inbox_incr_unread(HostType, InboxEntryKey, Content, MsgId, Timestamp) ->
-    {ok, integer()} | ok when
+    count_res() when
       HostType :: mongooseim:host_type(),
       InboxEntryKey :: entry_key(),
       Content :: binary(),
       MsgId :: binary(),
       Timestamp :: integer().
 
--callback reset_unread(HostType, InboxEntryKey, MsgId) -> inbox_write_res() when
+-callback reset_unread(HostType, InboxEntryKey, MsgId) -> write_res() when
       HostType :: mongooseim:host_type(),
       InboxEntryKey :: entry_key(),
       MsgId :: binary().
@@ -248,40 +249,26 @@ send_message(Acc, To = #jid{lserver = LServer}, Msg) ->
 
 %%%%%%%%%%%%%%%%%%%
 %% Handlers
--spec user_send_packet(Acc :: map(), From :: jid:jid(),
-                       To :: jid:jid(),
-                       Packet :: exml:element()) -> map().
+-spec user_send_packet(Acc :: mongoose_acc:t(), From :: jid:jid(),
+                       To :: jid:jid(), Packet :: exml:element()) ->
+    mongoose_acc:t().
 user_send_packet(Acc, From, To, #xmlel{name = <<"message">>} = Msg) ->
-    maybe_process_message(Acc, From, To, Msg, outgoing),
-    Acc;
+    maybe_process_message(Acc, From, To, Msg, outgoing);
 user_send_packet(Acc, _From, _To, _Packet) ->
     Acc.
 
--spec inbox_unread_count(Acc :: mongooseim_acc:t(), To :: jid:jid()) -> mongooseim_acc:t().
+-spec inbox_unread_count(Acc :: mongoose_acc:t(), To :: jid:jid()) -> mongoose_acc:t().
 inbox_unread_count(Acc, To) ->
     Res = mongoose_acc:get(inbox, unread_count, undefined, Acc),
     get_inbox_unread(Res, Acc, To).
 
--type fpacket() :: {From :: jid:jid(),
-                    To :: jid:jid(),
-                    Acc :: mongoose_acc:t(),
-                    Packet :: exml:element()}.
--spec filter_local_packet(Value :: fpacket() | drop) -> fpacket() | drop.
+-spec filter_local_packet(mongoose_hooks:filter_packet_acc() | drop) ->
+    mongoose_hooks:filter_packet_acc() | drop.
 filter_local_packet(drop) ->
     drop;
 filter_local_packet({From, To, Acc, Msg = #xmlel{name = <<"message">>}}) ->
-    %% In case of PgSQL we can we can update inbox and obtain unread_count in one query,
-    %% so we put it in accumulator here.
-    %% In case of MySQL/MsSQL it costs an extra query, so we fetch it only if necessary
-    %% (when push notification is created)
-    Acc0 = case maybe_process_message(Acc, From, To, Msg, incoming) of
-               {ok, UnreadCount} ->
-                   mongoose_acc:set(inbox, unread_count, UnreadCount, Acc);
-               _ ->
-                   Acc
-           end,
+    Acc0 = maybe_process_message(Acc, From, To, Msg, incoming),
     {From, To, Acc0, Msg};
-
 filter_local_packet({From, To, Acc, Packet}) ->
     {From, To, Acc, Packet}.
 
@@ -307,15 +294,32 @@ disco_local_features(Acc) ->
                             From :: jid:jid(),
                             To :: jid:jid(),
                             Msg :: exml:element(),
-                            Dir :: outgoing | incoming) -> ok | {ok, integer()}.
+                            Dir :: outgoing | incoming) -> mongoose_acc:t().
 maybe_process_message(Acc, From, To, Msg, Dir) ->
-    HostType = mongoose_acc:host_type(Acc),
     case should_be_stored_in_inbox(Msg, Dir) andalso inbox_owner_exists(Acc, From, To, Dir) of
         true ->
-            Type = get_message_type(Msg),
-            maybe_process_acceptable_message(HostType, From, To, Msg, Acc, Dir, Type);
+            do_maybe_process_message(Acc, From, To, Msg, Dir);
         false ->
-            ok
+            Acc
+    end.
+
+do_maybe_process_message(Acc, From, To, Msg, Dir) ->
+    %% In case of PgSQL we can update inbox and obtain unread_count in one query,
+    %% so we put it in accumulator here.
+    %% In case of MySQL/MsSQL it costs an extra query, so we fetch it only if necessary
+    %% (when push notification is created)
+    Type = get_message_type(Acc),
+    HostType = mongoose_acc:host_type(Acc),
+    case maybe_process_acceptable_message(HostType, From, To, Msg, Acc, Dir, Type) of
+        ok -> Acc;
+        {ok, UnreadCount} ->
+            mongoose_acc:set(inbox, unread_count, UnreadCount, Acc);
+        {error, Error} ->
+            HostType = mongoose_acc:host_type(Acc),
+            ?LOG_WARNING(#{what => inbox_process_message_failed,
+                           from_jid => jid:to_binary(From), to_jid => jid:to_binary(To),
+                           host_type => HostType, dir => incoming, reason => Error}),
+            Acc
     end.
 
 -spec inbox_owner_exists(Acc :: mongoose_acc:t(),
@@ -329,19 +333,25 @@ inbox_owner_exists(Acc, _From, To, incoming) ->
     HostType = mongoose_acc:host_type(Acc),
     ejabberd_auth:does_user_exist(HostType, To, stored).
 
+-spec maybe_process_acceptable_message(
+        mongooseim:host_type(), jid:jid(), jid:jid(), exml:element(),
+        mongoose_acc:t(), outgoing | incoming, one2one | groupchat) ->
+    count_res().
 maybe_process_acceptable_message(HostType, From, To, Msg, Acc, Dir, one2one) ->
-            process_message(HostType, From, To, Msg, Acc, Dir, one2one);
+    process_message(HostType, From, To, Msg, Acc, Dir, one2one);
 maybe_process_acceptable_message(HostType, From, To, Msg, Acc, Dir, groupchat) ->
-            muclight_enabled(HostType) andalso
-            process_message(HostType, From, To, Msg, Acc, Dir, groupchat).
+    case muclight_enabled(HostType) of
+        true -> process_message(HostType, From, To, Msg, Acc, Dir, groupchat);
+        false -> ok
+    end.
 
--spec process_message(HostType :: host(),
+-spec process_message(HostType :: mongooseim:host_type(),
                       From :: jid:jid(),
                       To :: jid:jid(),
                       Message :: exml:element(),
                       Acc :: mongoose_acc:t(),
                       Dir :: outgoing | incoming,
-                      Type :: one2one | groupchat) -> ok | {ok, integer()}.
+                      Type :: one2one | groupchat) -> count_res().
 process_message(HostType, From, To, Message, Acc, outgoing, one2one) ->
     mod_inbox_one2one:handle_outgoing_message(HostType, From, To, Message, Acc);
 process_message(HostType, From, To, Message, Acc, incoming, one2one) ->
@@ -642,13 +652,11 @@ muclight_enabled(HostType) ->
     Groupchats = get_groupchat_types(HostType),
     lists:member(muclight, Groupchats).
 
--spec get_message_type(Msg :: exml:element()) -> groupchat | one2one.
-get_message_type(Msg) ->
-    case exml_query:attr(Msg, <<"type">>, undefined) of
-        <<"groupchat">> ->
-            groupchat;
-        _ ->
-            one2one
+-spec get_message_type(mongoose_acc:t()) -> groupchat | one2one.
+get_message_type(Acc) ->
+    case mongoose_acc:stanza_type(Acc) of
+        <<"groupchat">> -> groupchat;
+        _ -> one2one
     end.
 
 %%%%%%%%%%%%%%%%%%%

src/inbox/mod_inbox_muclight.erl

@@ -1,24 +1,20 @@
 %%%-------------------------------------------------------------------
-%%% @author ludwikbukowski
 %%% @copyright (C) 2018, Erlang-Solutions
 %%% @doc
 %%%
 %%% @end
 %%% Created : 30. Jan 2018 13:22
 %%%-------------------------------------------------------------------
 -module(mod_inbox_muclight).
--author("ludwikbukowski").
+
 -include("mod_muc_light.hrl").
--include("mod_inbox.hrl").
 -include("jlib.hrl").
--include("mongoose_ns.hrl").
 -include("mongoose.hrl").
 
 -export([handle_outgoing_message/5, handle_incoming_message/5]).
 
 -ignore_xref([{mod_inbox_backend, remove_inbox_row, 2}]).
 
--type packet() :: exml:element().
 -type role() :: r_member() | r_owner() | r_none().
 -type r_member() :: binary().
 -type r_owner() :: binary().
@@ -27,16 +23,18 @@
 -spec handle_outgoing_message(HostType :: mongooseim:host_type(),
                               User :: jid:jid(),
                               Room :: jid:jid(),
-                              Packet :: packet(),
-                              Acc :: mongoose_acc:t()) -> any().
+                              Packet :: exml:element(),
+                              Acc :: mongoose_acc:t()) ->
+    mod_inbox:count_res().
 handle_outgoing_message(HostType, User, Room, Packet, _TS) ->
-    maybe_reset_unread_count(HostType, User, Room, Packet).
+    mod_inbox_utils:maybe_reset_unread_count(HostType, User, Room, Packet).
 
 -spec handle_incoming_message(HostType :: mongooseim:host_type(),
                               RoomUser :: jid:jid(),
                               Remote :: jid:jid(),
-                              Packet :: packet(),
-                              Acc :: mongoose_acc:t()) -> any().
+                              Packet :: exml:element(),
+                              Acc :: mongoose_acc:t()) ->
+    mod_inbox:count_res().
 handle_incoming_message(HostType, RoomUser, Remote, Packet, Acc) ->
     case mod_inbox_utils:has_chat_marker(Packet) of
         true ->
@@ -46,14 +44,12 @@ handle_incoming_message(HostType, RoomUser, Remote, Packet, Acc) ->
             maybe_handle_system_message(HostType, RoomUser, Remote, Packet, Acc)
     end.
 
-maybe_reset_unread_count(HostType, User, Room, Packet) ->
-    mod_inbox_utils:maybe_reset_unread_count(HostType, User, Room, Packet).
-
 -spec maybe_handle_system_message(HostType :: mongooseim:host_type(),
                                   RoomOrUser :: jid:jid(),
                                   Receiver :: jid:jid(),
                                   Packet :: exml:element(),
-                                  Acc :: mongoose_acc:t()) -> ok.
+                                  Acc :: mongoose_acc:t()) ->
+    mod_inbox:count_res().
 maybe_handle_system_message(HostType, RoomOrUser, Receiver, Packet, Acc) ->
     case is_system_message(HostType, RoomOrUser, Receiver, Packet) of
         true ->
@@ -127,7 +123,8 @@ maybe_remove_inbox_row(HostType, Room, Remote, true) ->
                      Remote :: jid:jid(),
                      Sender :: jid:jid(),
                      Packet :: exml:element(),
-                     Acc :: mongoose_acc:t()) -> ok.
+                     Acc :: mongoose_acc:t()) ->
+    mod_inbox:count_res().
 write_to_inbox(HostType, RoomUser, Remote, Sender, Packet, Acc) ->
     case jid:are_equal(Remote, Sender) of
         true -> mod_inbox_utils:write_to_sender_inbox(HostType, Remote, RoomUser, Packet, Acc);

src/inbox/mod_inbox_one2one.erl

@@ -1,46 +1,31 @@
 %%%-------------------------------------------------------------------
-%%% @author ludwikbukowski
 %%% @copyright (C) 2018, Erlang-Solutions
 %%% @doc
 %%%
 %%% @end
 %%% Created : 30. Jan 2018 13:22
 %%%-------------------------------------------------------------------
 -module(mod_inbox_one2one).
--author("ludwikbukowski").
--include("mod_inbox.hrl").
--include("jlib.hrl").
--include("mongoose_ns.hrl").
 
 -export([handle_outgoing_message/5, handle_incoming_message/5]).
 
--type packet() :: exml:element().
-
 -spec handle_outgoing_message(HostType :: mongooseim:host_type(),
                               User :: jid:jid(),
                               Remote :: jid:jid(),
-                              Packet :: packet(),
-                              Acc :: mongoose_acc:t()) -> ok.
+                              Packet :: exml:element(),
+                              Acc :: mongoose_acc:t()) ->
+    mod_inbox:count_res().
 handle_outgoing_message(HostType, User, Remote, Packet, Acc) ->
-    maybe_reset_unread_count(HostType, User, Remote, Packet),
-    maybe_write_to_inbox(HostType, User, Remote, Packet, Acc, fun write_to_sender_inbox/5).
+    mod_inbox_utils:maybe_reset_unread_count(HostType, User, Remote, Packet),
+    mod_inbox_utils:maybe_write_to_inbox(
+      HostType, User, Remote, Packet, Acc, fun mod_inbox_utils:write_to_sender_inbox/5).
 
 -spec handle_incoming_message(HostType :: mongooseim:host_type(),
                               User :: jid:jid(),
                               Remote :: jid:jid(),
-                              Packet :: packet(),
-                              Acc :: mongoose_acc:t()) -> ok | {ok, integer()}.
+                              Packet :: exml:element(),
+                              Acc :: mongoose_acc:t()) ->
+    mod_inbox:count_res().
 handle_incoming_message(HostType, User, Remote, Packet, Acc) ->
-    maybe_write_to_inbox(HostType, User, Remote, Packet, Acc, fun write_to_receiver_inbox/5).
-
-maybe_reset_unread_count(HostType, User, Remote, Packet) ->
-    mod_inbox_utils:maybe_reset_unread_count(HostType, User, Remote, Packet).
-
-maybe_write_to_inbox(HostType, User, Remote, Packet, Acc, WriteF) ->
-    mod_inbox_utils:maybe_write_to_inbox(HostType, User, Remote, Packet, Acc, WriteF).
-
-write_to_sender_inbox(HostType, User, Remote, Packet, Acc) ->
-    mod_inbox_utils:write_to_sender_inbox(HostType, User, Remote, Packet, Acc).
-
-write_to_receiver_inbox(HostType, User, Remote, Packet, Acc) ->
-    mod_inbox_utils:write_to_receiver_inbox(HostType, User, Remote, Packet, Acc).
+    mod_inbox_utils:maybe_write_to_inbox(
+      HostType, User, Remote, Packet, Acc, fun mod_inbox_utils:write_to_receiver_inbox/5).