From 0911d0659af710a79981c875fb446fb29a71d621 Mon Sep 17 00:00:00 2001 From: Tobias Lindahl Date: Tue, 26 May 2020 16:24:48 +0200 Subject: [PATCH 1/5] Refactor push notification code for readability and efficiency --- apps/roster/src/protocol/roster_room.erl | 107 ++++++++++++----------- 1 file changed, 57 insertions(+), 50 deletions(-) diff --git a/apps/roster/src/protocol/roster_room.erl b/apps/roster/src/protocol/roster_room.erl index ebe9436ce..ca0b3a7bd 100644 --- a/apps/roster/src/protocol/roster_room.erl +++ b/apps/roster/src/protocol/roster_room.erl @@ -425,56 +425,63 @@ info(#'Member'{status = Status, phone_id = Id}, Req, #cx{params = ClientId} = St proc(init, #handler{name = roster_room} = Async) -> ?LOG_INFO("ASYNC ROOM PROC started", []), {ok, Async}; -%% TODO use roster_push instead proc({send_push, NewMembers, Msg, Type}, #handler{} = H) -> - PushType = <<"message">>, #'Message'{from = FromPhoneId, to = RoomId} = Msg, - {ok, #'Room'{name = RoomName}} = kvs:get('Room', RoomId), - NewMembersData = lists:flatten([begin #'Member'{alias = Al, phone_id = Ph} = M, {Ph, Al} end || M <- NewMembers]), #'Member'{alias = FromAlias} = roster:muc_member(FromPhoneId, RoomId), -%% delete acted admin member and previously removed members - ToRoomMembers = lists:filter(fun({S, _}) -> - S /= to_be_deleted andalso S /= FromPhoneId end, - lists:flatten([begin - case ToStatus of - removed -> case lists:keyfind(ToPhoneId, #'Member'.phone_id, NewMembers) of - false -> {to_be_deleted, []}; - _ -> {ToPhoneId, ToAlias} - end; - _ -> {ToPhoneId, ToAlias} - end - end || #'Member'{phone_id = ToPhoneId, alias = ToAlias, status = ToStatus} = _ <- roster:members(#muc{name = RoomId})])), - [begin - Payload = roster:room(roster:roster_id(MemPhoneId), RoomId), - PushAlert = - case Type of - T when T == add; T == remove;T == join -> - MemberPayload = case lists:member(Mem, NewMembersData) of - true -> - MemPay = lists:foldl(fun({_, Al}, Ac) -> - Ac ++ [Al, <<", ">>] end, [], lists:delete(Mem, NewMembersData)), - case MemPay of - [] -> "you"; - _ -> iolist_to_binary(["you, ", lists:droplast(MemPay)]) - end; - _ -> - lists:droplast(lists:foldl(fun({_, Al}, Ac) -> - Ac ++ [Al, <<", ">>] end, [], NewMembersData)) - end, - iolist_to_binary([FromAlias | case T of - add -> - [<<" invited ">>, MemberPayload, <<" to the group ">>, RoomName]; - join -> - [<<" joined ">>, MemberPayload, <<" to the group ">>, RoomName]; - remove -> - [<<" removed ">>, MemberPayload, <<" from the group ">>, RoomName] end]); - create -> - iolist_to_binary([FromAlias, <<" invited you to the group ">>, RoomName]); - leave -> - iolist_to_binary([FromAlias, <<" left the group ">>, RoomName]); - T when T == photo; T == name -> - iolist_to_binary([FromAlias, <<" edited the group's ">>, RoomName, <<" ">>, atom_to_list(T)]) - end, - roster_push:send_push_notification(Ses, Payload, PushAlert, PushType) - end || {MemPhoneId, _} = Mem <- ToRoomMembers, Ses <- kvs:index('Auth', user_id, MemPhoneId) - ], {reply, [], H}. + {ok, #'Room'{name = RoomName}} = kvs:get('Room', RoomId), + NewMembersData = + [{MPhoneId, MAlias} + || #'Member'{phone_id = MPhoneId, alias = MAlias} <- NewMembers], + ToRoomMembers = + [ {ToPhoneId, ToAlias} || + #'Member'{phone_id = ToPhoneId, alias = ToAlias, status = ToStatus} + <- roster:members(#muc{name = RoomId}), + ToPhoneId /= FromPhoneId, + ToStatus /= removed orelse lists:keymember(ToPhoneId, 1, NewMembersData) + ], + lists:foreach(fun(MemberData) -> + send_push(MemberData, RoomId, NewMembersData, + RoomName, FromAlias, Type) + end, ToRoomMembers), + {reply, [], H}. + +send_push(MemberData, RoomId, NewMembersData, RoomName, FromAlias, Type) -> + {MemPhoneId, _} = MemberData, + PushType = <<"message">>, + PushAlert = push_alert(Type, MemberData, NewMembersData, RoomName, FromAlias), + PushAlertBin = iolist_to_binary(PushAlert), + Payload = roster:room(roster:roster_id(MemPhoneId), RoomId), + lists:foreach( + fun(Auth) -> + roster_push:send_push_notification(Auth, Payload, PushAlertBin, PushType) + end, kvs:index('Auth', user_id, MemPhoneId)). + +push_alert(create, _MemberData,_NewMembersData, RoomName, FromAlias) -> + [FromAlias, <<" invited you to the group ">>, RoomName]; +push_alert(leave,_MemberData,_NewMembersData, RoomName, FromAlias) -> + [FromAlias, <<" left the group ">>, RoomName]; +push_alert(photo,_MemberData,_NewMembersData, RoomName, FromAlias) -> + [FromAlias, <<" edited the group's ">>, RoomName, <<" photo">>]; +push_alert(name,_MemberData,_NewMembersData, RoomName, FromAlias) -> + [FromAlias, <<" edited the group's ">>, RoomName, <<" name">>]; +push_alert(Type, MemberData, NewMembersData, RoomName, FromAlias) + when Type == add; Type == remove; Type == join -> + Aliases = [Alias || {_, Alias} = M <- NewMembersData, M =/= MemberData], + MemberPayload = + case lists:member(MemberData, NewMembersData) of + true -> + lists:join(<<", ">>, [<<"you">>|Aliases]); + false -> + lists:join(<<", ">>, Aliases) + end, + case Type of + add -> + [FromAlias, <<" invited ">>, MemberPayload, + <<" to the group ">>, RoomName]; + join -> + [FromAlias, <<" joined ">>, MemberPayload, + <<" to the group ">>, RoomName]; + remove -> + [FromAlias, <<" removed ">>, MemberPayload, + <<" from the group ">>, RoomName] + end. -- GitLab From 0527a348649d9bcec5e03e2f2b8ccba09ef953d2 Mon Sep 17 00:00:00 2001 From: Tobias Lindahl Date: Tue, 26 May 2020 16:56:26 +0200 Subject: [PATCH 2/5] Address review comment Co-authored-by: Hans Svensson --- apps/roster/src/protocol/roster_room.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/roster/src/protocol/roster_room.erl b/apps/roster/src/protocol/roster_room.erl index ca0b3a7bd..7edaf5805 100644 --- a/apps/roster/src/protocol/roster_room.erl +++ b/apps/roster/src/protocol/roster_room.erl @@ -470,7 +470,7 @@ push_alert(Type, MemberData, NewMembersData, RoomName, FromAlias) MemberPayload = case lists:member(MemberData, NewMembersData) of true -> - lists:join(<<", ">>, [<<"you">>|Aliases]); + lists:join(<<", ">>, [<<"you">> | Aliases]); false -> lists:join(<<", ">>, Aliases) end, -- GitLab From 9408b000dd56dcdcf5a456f63c53609cd300366c Mon Sep 17 00:00:00 2001 From: Tobias Lindahl Date: Wed, 27 May 2020 10:37:36 +0200 Subject: [PATCH 3/5] Refactor roster_message notification implementation --- apps/roster/src/protocol/roster_message.erl | 130 ++++++++++---------- 1 file changed, 67 insertions(+), 63 deletions(-) diff --git a/apps/roster/src/protocol/roster_message.erl b/apps/roster/src/protocol/roster_message.erl index 7ea6ac801..cc92943eb 100644 --- a/apps/roster/src/protocol/roster_message.erl +++ b/apps/roster/src/protocol/roster_message.erl @@ -457,15 +457,11 @@ proc(init, #handler{name = roster_message} = Async) -> proc({send_push, From, To, #'Message'{type = TypeList} = Msg, Action}, #handler{} = H) -> %% Check should server proceed push %% TODO add push for message/delete - case From of - To -> - case lists:member(cursor, TypeList) of - true -> - notify(From, To, Msg, Action); - _ -> - ?LOG_INFO("ExcessivePush:~p", [From]) - end; - _ -> notify(From, To, Msg, Action) + case From =/= To orelse lists:member(cursor, TypeList) of + true -> + notify(From, To, Msg, Action); + false -> + ?LOG_INFO("ExcessivePush:~p", [From]) end, {reply, [], H}; proc({#io{} = IO, ClientId}, #handler{state = C} = H) -> @@ -486,61 +482,69 @@ proc(Unknown, #handler{} = H) -> ?LOG_INFO("invalid message data :~p", [Unknown]), {reply, [], H}. - notify(From, To, Msg, Action) -> -%% Prepare push Payload and receivers list - PushType = <<"message">>, - #'Message'{files = Attachments, feed_id = Feed} = Msg, - LastAttachment = lists:last(Attachments), - #'Desc'{payload = MsgPayload, mime = Mime} = LastAttachment, - - MsgPreview = case Mime of -%% extra handling for posttranslate - M when M == <<"text">>; M == <<"posttranslate">> -> - Txt = case M of - <<"posttranslate">> -> - #'Desc'{payload = OriginText} = lists:keyfind(<<"text">>, #'Desc'.mime, Attachments), - OriginText; - _ -> MsgPayload end, -%% decode text payload because of utf8 text - unicode:characters_to_binary(lists:sublist(unicode:characters_to_list(Txt), 50)); - <<"audio">> -> <<"Voice Message">>; - _ -> -%% capitilize first letter - [FirstLetter | LastText] = binary_to_list(Mime), - iolist_to_binary([string:to_upper(FirstLetter), LastText]) - end, -%% prepare Receivers list. find Sender contact - {ToList, PushAlert} = - case Feed of - {muc, RoomId} -> - #'Member'{alias = FromAlias} = roster:muc_member(From, RoomId), - {ok, #'Room'{name = RoomName, type = RoomType}} = kvs:get('Room', RoomId), - RoomMsgPreview = iolist_to_binary([RoomName, <<": ">>, MsgPreview]), - PushAlertMsg = case RoomType of - channel -> RoomMsgPreview; - _ -> iolist_to_binary([FromAlias, <<"@">>, RoomMsgPreview]) - end, - {lists:foldl( - fun(#'Member'{ phone_id = ToPId }, Acc) when ToPId /= From -> - case roster:room(roster:roster_id(ToPId), RoomId) of - R = #'Room'{} -> [{ToPId, R#'Room'{last_msg = Msg}} | Acc]; - _ -> Acc - end; - (_, Acc) -> Acc - end, [], roster:members(#muc{name = To}, active)), - PushAlertMsg}; - _ -> - #'Contact'{names = FromName} = Contact = roster:user(roster:roster_id(To), From), - {[{To, Contact#'Contact'{last_msg = Msg}}], iolist_to_binary([FromName, ": ", MsgPreview])} - end, - [begin - PushAlertSync = case Action of - Act when Act == ?MSG_EDIT_ACTION orelse Act == ?MSG_DELETE_ACTION -> iolist_to_binary(["SyncPush:", Action]); - _ -> PushAlert - end, - roster_push:send_push_notification(Ses, Payload, PushAlertSync, PushType) - end || {ToUserId, Payload} <- ToList, #'Auth'{} = Ses <- kvs:index('Auth', user_id, ToUserId)]. + Alert = get_notify_alert(Msg, From, To, Action), + lists:foreach( + fun({ToUserId, Payload}) -> + lists:foreach( + fun(Ses) -> + roster_push:send_push_notification(Ses, Payload, Alert, <<"message">>) + end, kvs:index('Auth', user_id, ToUserId)) + end, get_notify_receivers(Msg, From, To)). + +get_notify_receivers(#'Message'{ feed_id = #muc{ name = RoomId }} = Msg, + From, To) -> + lists:foldl( + fun(#'Member'{ phone_id = ToPId }, Acc) when ToPId /= From -> + case roster:room(roster:roster_id(ToPId), RoomId) of + R = #'Room'{} -> [{ToPId, R#'Room'{last_msg = Msg}} + | Acc]; + _ -> Acc + end; + (_, Acc) -> Acc + end, [], roster:members(#muc{name = To}, active)); +get_notify_receivers(#'Message'{} = Msg, From, To) -> + Contact = roster:user(roster:roster_id(To), From), + [{To, Contact#'Contact'{last_msg = Msg}}]. + +get_notify_alert(#'Message'{}, _From, _To, ?MSG_EDIT_ACTION = Action) -> + iolist_to_binary(["SyncPush:", Action]); +get_notify_alert(#'Message'{}, _From, _To, ?MSG_DELETE_ACTION = Action) -> + iolist_to_binary(["SyncPush:", Action]); +get_notify_alert(#'Message'{ feed_id = #muc{ name = RoomId }} = Msg, + From,_To, _Action) -> + #'Member'{alias = FromAlias} = roster:muc_member(From, RoomId), + {ok, #'Room'{name = RoomName, type = RoomType}} = kvs:get('Room', RoomId), + case RoomType =:= channel of + true -> + iolist_to_binary([RoomName, <<": ">>, msg_preview(Msg)]); + false -> + iolist_to_binary([FromAlias, <<"@">>, RoomName, <<": ">>, + msg_preview(Msg)]) + end; +get_notify_alert(#'Message'{} = Msg, From, To, _Action) -> + #'Contact'{names = FromName} = roster:user(roster:roster_id(To), From), + iolist_to_binary([FromName, ": ", msg_preview(Msg)]). + +msg_preview(#'Message'{files = Attachments}) -> + #'Desc'{payload = MsgPayload, mime = Mime} = lists:last(Attachments), + case Mime of + <<"posttranslate">> -> + Desc = lists:keyfind(<<"text">>, #'Desc'.mime, Attachments), + #'Desc'{payload = OriginText} = Desc, + unicode:characters_to_binary( + lists:sublist( + unicode:characters_to_list(OriginText), 50)); + <<"text">> -> + unicode:characters_to_binary( + lists:sublist( + unicode:characters_to_list(MsgPayload), 50)); + <<"audio">> -> + <<"Voice Message">>; + _ -> + [FirstLetter | LastText] = binary_to_list(Mime), + iolist_to_binary([string:to_upper(FirstLetter), LastText]) + end. binary_delpart([], _Part) -> []; binary_delpart(Bin, Part) -> -- GitLab From 30f65df5b93a41bd3f4b38b86ae1af1b9d6bdf01 Mon Sep 17 00:00:00 2001 From: Tobias Lindahl Date: Wed, 27 May 2020 11:13:05 +0200 Subject: [PATCH 4/5] Refactor roster_friend notification implementation --- apps/roster/src/protocol/roster_friend.erl | 28 +++++++++++++++++----- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/apps/roster/src/protocol/roster_friend.erl b/apps/roster/src/protocol/roster_friend.erl index 7e3587fea..1425ed561 100644 --- a/apps/roster/src/protocol/roster_friend.erl +++ b/apps/roster/src/protocol/roster_friend.erl @@ -173,13 +173,29 @@ proc(init, #handler{name = ?MODULE} = Async) -> ?LOG_INFO("ASYNC", []), {ok, Async}; -proc({send_push, #'Contact'{names = FromName} = FromContactPayload, To, PushType}, #handler{} = H) -> - ActionText = case PushType of <<"request">> -> " wants to add you on NYNJA!"; _ -> " accepted your contact request! Send a message!" end, - PushAlert = iolist_to_binary([FromName, ActionText]), - [roster_push:send_push_notification(Ses, FromContactPayload, PushAlert, PushType) || #'Auth'{} = Ses <- kvs:index('Auth', user_id, To)], +proc({send_push, #'Contact'{} = C, To, PushType}, #handler{} = H) -> + send_push(C, To, PushType), {reply, [], H}; proc({send_push, From, To, PushType}, #handler{} = H) -> case roster:user(roster:roster_id(To), From) of - C = #'Contact'{} -> proc({send_push, C, To, PushType}, H); - [] -> {reply, [], H} + C = #'Contact'{} -> + send_push(C, To, PushType), + {reply, [], H}; + [] -> + {reply, [], H} end. + +send_push(#'Contact'{names = FromName} = FromContactPayload, To, PushType) -> + PushAlert = + case PushType =:= <<"request">> of + true -> + [FromName, " wants to add you on NYNJA!"]; + false -> + [FromName, " accepted your contact request! Send a message!"] + end, + PushAlertBin = iolist_to_binary(PushAlert), + lists:foreach( + fun(Ses) -> + roster_push:send_push_notification( + Ses, FromContactPayload, PushAlertBin, PushType) + end, kvs:index('Auth', user_id, To)). -- GitLab From afbce018eba9f92a56a89718061c1ab0107295ab Mon Sep 17 00:00:00 2001 From: Tobias Lindahl Date: Wed, 27 May 2020 11:17:08 +0200 Subject: [PATCH 5/5] Refactor roster_history notification implementation --- apps/roster/src/protocol/roster_history.erl | 48 ++++++++++++++------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/apps/roster/src/protocol/roster_history.erl b/apps/roster/src/protocol/roster_history.erl index ba1af15f8..8501c65b2 100644 --- a/apps/roster/src/protocol/roster_history.erl +++ b/apps/roster/src/protocol/roster_history.erl @@ -343,20 +343,38 @@ proc(init, #handler{name = roster_history} = Async) -> ?LOG_INFO("ASYNC:~p", [?MODULE]), {ok, Async}; -proc({send_push, ToPhoneId, Feed, Action}, #handler{} = H) -> - PushType = <<"message">>, - Payload = case Feed of - {muc, RoomId} -> - try Room = #'Room'{} = roster:room(roster:roster_id(ToPhoneId), RoomId), - case Action of ?HISTORY_UPDATE_ACTION -> Room#'Room'{last_msg = []}; _ -> Room end - catch Err:Rea -> - ?LOG_ERROR(":~p~n", [n2o:stack_trace(Err, Rea)]) - end; - #p2p{} -> - Contact = roster:user(roster:roster_id(ToPhoneId), roster:friend(ToPhoneId, Feed)), - case Action of ?HISTORY_UPDATE_ACTION -> Contact#'Contact'{last_msg = []}; _ -> Contact end +proc({send_push, ToPhoneId, #muc{name = RoomId}, Action}, #handler{} = H) -> + try roster:room(roster:roster_id(ToPhoneId), RoomId) of + #'Room'{} = Room -> + case Action of + ?HISTORY_UPDATE_ACTION -> + Payload = Room#'Room'{last_msg = []}, + send_push_and_reply(ToPhoneId, Payload, Action, H); + _ -> + send_push_and_reply(ToPhoneId, Room, Action, H) + end; + _ -> + {reply, [], H} + catch Err:Rea -> + ?LOG_ERROR(":~p~n", [n2o:stack_trace(Err, Rea)]), + {reply, [], H} + end; +proc({send_push, ToPhoneId, #p2p{} = Feed, Action}, #handler{} = H) -> + Contact = roster:user(roster:roster_id(ToPhoneId), + roster:friend(ToPhoneId, Feed)), + case Action of + ?HISTORY_UPDATE_ACTION -> + Payload = Contact#'Contact'{last_msg = []}, + send_push_and_reply(ToPhoneId, Payload, Action, H); + _ -> + send_push_and_reply(ToPhoneId, Contact, Action, H) + end. - end, +send_push_and_reply(ToPhoneId, Payload, Action, Handler) -> + PushType = <<"message">>, PushAlert = iolist_to_binary(["SyncPush:", Action]), - [roster_push:send_push_notification(Ses, Payload, PushAlert, PushType) || #'Auth'{} = Ses <- kvs:index('Auth', user_id, ToPhoneId)], - {reply, [], H}. + lists:foreach(fun(Ses) -> + roster_push:send_push_notification( + Ses, Payload, PushAlert, PushType) + end, kvs:index('Auth', user_id, ToPhoneId)), + {reply, [], Handler}. -- GitLab