From 6e1df490125c670fd12f7b891358df6987c1a683 Mon Sep 17 00:00:00 2001 From: Tobias Lindahl Date: Mon, 8 Jun 2020 17:39:26 +0200 Subject: [PATCH 1/5] Find out if device token is valid on fcm and align return values for push --- apps/roster/src/api/push/roster_apns_api.erl | 2 +- apps/roster/src/api/push/roster_fcm_api.erl | 77 +++++++++++--------- apps/roster/src/protocol/roster_push.erl | 26 +++---- sys.config | 5 +- 4 files changed, 61 insertions(+), 49 deletions(-) diff --git a/apps/roster/src/api/push/roster_apns_api.erl b/apps/roster/src/api/push/roster_apns_api.erl index 18287f5b5..1e571ff7e 100644 --- a/apps/roster/src/api/push/roster_apns_api.erl +++ b/apps/roster/src/api/push/roster_apns_api.erl @@ -165,7 +165,7 @@ binary_push(Config, Packet, Token, Attempt) -> {error, Reason} when Attempt > 10 -> ?LOG_INFO("Socket error, giving up: ~p, Token ~P", [Reason, Token, 5]), - ok; + {error, not_delivered}; {error, Reason} -> ?LOG_INFO("Socket error: ~p, Retrying on ~P", [Reason, Token, 5]), diff --git a/apps/roster/src/api/push/roster_fcm_api.erl b/apps/roster/src/api/push/roster_fcm_api.erl index fb07f73a1..eafd8cf8f 100644 --- a/apps/roster/src/api/push/roster_fcm_api.erl +++ b/apps/roster/src/api/push/roster_fcm_api.erl @@ -2,44 +2,53 @@ -include_lib("kernel/include/logger.hrl"). -include("roster.hrl"). --export([description/0, notify/3, test_push_notification/0]). +-export([ description/0 + , notify/2 + ]). description() -> "Android Push Notifications Module". --define(FCM_SERVER_KEY, proplists:get_value(fcm_server_key, application:get_env(roster, push_api, []))). --define(FCM_SEND_ENDPOINT, "https://fcm.googleapis.com/fcm/send"). --define(FCM_CONTENT_TYPE, "application/x-www-form-urlencoded;charset=UTF-8"). - %% ------------------------------------------------------------------ %% Android Push Notifications %% ------------------------------------------------------------------ -fcm_headers() -> - ContentType = "application/json", - Headers = [{"Authorization", binary_to_list(iolist_to_binary(["key=", ?FCM_SERVER_KEY]))}, {"Content-Type", ContentType}], - Headers. - -notify(MessageTitle, MessageBody, DeviceId) when is_binary(MessageTitle) -> - notify(binary_to_list(MessageTitle), MessageBody, DeviceId); -notify(MessageTitle, MessageBody, DeviceId) when is_binary(MessageBody) -> - notify(MessageTitle, binary_to_list(MessageBody), DeviceId); -notify(MessageTitle, MessageBody, DeviceId) when is_binary(DeviceId) -> - notify(MessageTitle, MessageBody, binary_to_list(DeviceId)); -notify(_, MessageBody, DeviceId) -> -%% create payload - Payload = binary_to_list(iolist_to_binary(["registration_id=", DeviceId, "&data=", MessageBody, "&priority=high"])), - ?LOG_INFO("Payload ~p~n~n", [Payload]), - Request = {?FCM_SEND_ENDPOINT, fcm_headers(), ?FCM_CONTENT_TYPE, Payload}, - {_, _} = roster_rest:send_request(post, Request, [], []), - ok. - -%% ------------------------------------------------------------------ -%% Tests -%% ------------------------------------------------------------------ - -%% Dima's phone --define(FCM_TEST_DEVICE_ID, <<"eBYJ3rgLdsc:APA91bHs_wR_SBrJxU7CjY0W4vwFN2LJugLwBefc1YLS2RWDVVUYLuT6g8x7TG640T7cmYmrxH_RvUv30bEZOAlZFLSwdLxy793sWC7L9YtRfow4Y9ro8Zwnz179YDnIzG5b8pky7QZV">>). - -test_push_notification() -> - MessageBody = "Notify Liubov about this push", - notify(MessageBody, MessageBody, ?FCM_TEST_DEVICE_ID). +notify(MessageBody, DeviceId) -> + Payload = binary_to_list( + iolist_to_binary(["registration_id=", DeviceId, + "&data=", MessageBody, + "&priority=high"])), + {ok, Config} = application:get_env(roster, push_api), + ServerKey = proplists:get_value(fcm_server_key, Config), + Headers = [{"Authorization", "key=" ++ ServerKey}, + {"Content-Type", "application/json"}], + Request = {"https://fcm.googleapis.com/fcm/send", + Headers, + "application/x-www-form-urlencoded;charset=UTF-8", + Payload}, + case httpc:request(post, Request, [], []) of + {ok, {{_, HttpCode, _}, _, Body}} when HttpCode =:= 200 -> + case re:split(Body, "=") of + [<<"id">> | _] -> + ?LOG_INFO("FCM push sent for ~P", [DeviceId, 5]), + ok; + [<<"Error">>, <<"InvalidRegistration">>] -> + ?LOG_INFO("FCM push to bad token ~P", + [DeviceId, 5]), + {error, bad_token}; + [<<"Error">> | _] -> + ?LOG_INFO("FCM push error for ~P, ~p", + [DeviceId, 5, Body]), + {error, not_delivered}; + _Other -> + ?LOG_INFO("FCM push unexpected reply ~P, ~p", + [DeviceId, 5, Body]), + {error, not_delivered} + end; + {ok, {{_, HttpCode, _}, _, Body}} -> + ?LOG_ERROR("FCM push error ~p:~p", [HttpCode, Body]), + {error, not_delivered}; + {error, What} -> + ?LOG_INFO("FCM push error for ~P: ~p", + [DeviceId, 5, What]), + {error, not_delivered} + end. diff --git a/apps/roster/src/protocol/roster_push.erl b/apps/roster/src/protocol/roster_push.erl index 5b675aca6..a4d34bde0 100644 --- a/apps/roster/src/protocol/roster_push.erl +++ b/apps/roster/src/protocol/roster_push.erl @@ -49,26 +49,26 @@ send_push_notification(#'Auth'{ os = OS case PushToken of [] -> skip; _ -> - ?LOG_INFO("~p:~p:~PPushAlert:~p", + ?LOG_INFO("~p:~p:~P PushAlert:~p", [PhoneId, OS, PushToken, 5, PushAlert]), - send_push_notification(OS, PushToken, Payload, PushAlert, PushType, AuthSettings, HS) + case send_push_notification(OS, PushToken, Payload, PushAlert, + PushType, AuthSettings, HS) of + skip -> skip; + ok -> ok; + {error, bad_token} -> + %% TODO: Remove bad token. + ok; + {error, not_delivered} -> + ok + end end. send_push_notification(ios, Push, Payload, PushAlert, PushType, AuthSettings, HS) -> EncodedPayload = base64:encode(term_to_binary(Payload)), IOS = maps:get(ios, HS), - case roster_apns_api:notify(PushAlert, EncodedPayload, - PushType, Push, AuthSettings, IOS) of - ok -> - ok; - {error, bad_token} -> - %% TODO: Remove auth. - ok; - {error,_What} -> - ok - end; + roster_apns_api:notify(PushAlert, EncodedPayload, PushType, Push, AuthSettings, IOS); send_push_notification(android, Push, Payload, PushAlert, PushType, _AuthSettings,_HS) -> PushModel = #push{model = Payload, type = PushType, alert = PushAlert, title = PushAlert, badge = 1}, AndroidPush = http_uri:encode(binary_to_list(base64:encode(term_to_binary(PushModel)))), - roster_fcm_api:notify(PushAlert, AndroidPush, Push); + roster_fcm_api:notify(AndroidPush, Push); send_push_notification(_, _, _, _, _, _, _) -> skip. diff --git a/sys.config b/sys.config index 144dd9d75..caf1ae47b 100644 --- a/sys.config +++ b/sys.config @@ -96,7 +96,10 @@ ]}, {push_api,[ {apns_force_http, false}, - {fcm_server_key,<<"AAAAAzb6_Zg:APA91bGN0jYv_4iqyk8IC4xUdPYXh0yPsTF9YYj_gd9oebRr_ZEoLuC5hCD9RfdqA3Y3AF_P_WbelqvzvgR3RsX_mHBLynV14Q6HakXAtrY_eWLK2xqamF2OC9uBXfKgxTFFqmyr1Kbw">>}, + {fcm_server_key, + "AAAAAzb6_Zg:APA91bGN0jYv_4iqyk8IC4xUdPYXh0yPsTF9YY" + "j_gd9oebRr_ZEoLuC5hCD9RfdqA3Y3AF_P_WbelqvzvgR3RsX_" + "mHBLynV14Q6HakXAtrY_eWLK2xqamF2OC9uBXfKgxTFFqmyr1Kbw"}, {apns_cert_dir, "etc/certs/apns_certificates"}, {apns_binary_port, 2195}, {apns_http_port, 443}]}, -- GitLab From 0f9437aa3db6ce7117fda20d5bc3664aa3370e11 Mon Sep 17 00:00:00 2001 From: Tobias Lindahl Date: Tue, 9 Jun 2020 08:57:44 +0200 Subject: [PATCH 2/5] Avoid sending useless Auth records to push server --- apps/roster/src/protocol/roster_push.erl | 63 ++++++++++++++---------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/apps/roster/src/protocol/roster_push.erl b/apps/roster/src/protocol/roster_push.erl index a4d34bde0..a2c0786ac 100644 --- a/apps/roster/src/protocol/roster_push.erl +++ b/apps/roster/src/protocol/roster_push.erl @@ -16,10 +16,13 @@ start() -> , state = init}). send_push_notification(Session, Payload, Alert, Type) -> - n2o_async:pid(system, ?MODULE) - ! {async_push, Session, Payload, Alert, Type}. - -%% TODO: Handle apns connection messages in proc/2 + case is_push_session(Session) of + true -> + n2o_async:pid(system, ?MODULE) + ! {async_push, Session, Payload, Alert, Type}; + false -> + ok + end. proc(init, #handler{name = ?MODULE} = Async) -> ConnState = #{ android => [] @@ -38,29 +41,36 @@ proc({reconnecting, _Pid}, #handler{} = H) -> %% APNS connection. Safe to ignore {noreply, H}; -proc({async_push, Session, Payload, PushAlert, PushType}, #handler{state = HS} = H) -> +proc({async_push, Session, Payload, PushAlert, PushType}, Handler) -> + HS = Handler#handler.state, send_push_notification(Session, Payload, PushAlert, PushType, HS), - {reply, [], H}. + {reply, [], Handler}. + + +is_push_session(#'Auth'{os = ios} ) -> true; +is_push_session(#'Auth'{os = android}) -> true; +is_push_session(#'Auth'{}) -> false. -send_push_notification(#'Auth'{ os = OS - , push = PushToken - , user_id = PhoneId - , settings = AuthSettings}, Payload, PushAlert, PushType, HS) -> - case PushToken of - [] -> skip; - _ -> - ?LOG_INFO("~p:~p:~P PushAlert:~p", - [PhoneId, OS, PushToken, 5, PushAlert]), - case send_push_notification(OS, PushToken, Payload, PushAlert, - PushType, AuthSettings, HS) of - skip -> skip; - ok -> ok; - {error, bad_token} -> - %% TODO: Remove bad token. - ok; - {error, not_delivered} -> - ok - end +send_push_notification(#'Auth'{ push = [] }, _, _, _, _) -> + skip; +send_push_notification(#'Auth'{ os = OS }, _, _, _, _) when OS /= android; + OS /= ios -> + skip; +send_push_notification(Auth, Payload, PushAlert, PushType, HS) -> + #'Auth'{ os = OS + , push = PushToken + , user_id = PhoneId + , settings = AuthSettings} = Auth, + ?LOG_INFO("~p:~p:~P PushAlert:~p", [PhoneId, OS, PushToken, 5, PushAlert]), + case send_push_notification(OS, PushToken, Payload, PushAlert, + PushType, AuthSettings, HS) of + ok -> + ok; + {error, bad_token} -> + %% TODO: Remove bad token. + ok; + {error, not_delivered} -> + ok end. send_push_notification(ios, Push, Payload, PushAlert, PushType, AuthSettings, HS) -> @@ -70,5 +80,4 @@ send_push_notification(ios, Push, Payload, PushAlert, PushType, AuthSettings, HS send_push_notification(android, Push, Payload, PushAlert, PushType, _AuthSettings,_HS) -> PushModel = #push{model = Payload, type = PushType, alert = PushAlert, title = PushAlert, badge = 1}, AndroidPush = http_uri:encode(binary_to_list(base64:encode(term_to_binary(PushModel)))), - roster_fcm_api:notify(AndroidPush, Push); -send_push_notification(_, _, _, _, _, _, _) -> skip. + roster_fcm_api:notify(AndroidPush, Push). -- GitLab From 74e60558a0d9e1fb6cd6bf12700b8966b2cefcd4 Mon Sep 17 00:00:00 2001 From: Tobias Lindahl Date: Tue, 9 Jun 2020 13:34:37 +0200 Subject: [PATCH 3/5] Remove the whole Auth record when push tokens are invalid --- apps/roster/src/protocol/roster_push.erl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/roster/src/protocol/roster_push.erl b/apps/roster/src/protocol/roster_push.erl index a2c0786ac..346ded29b 100644 --- a/apps/roster/src/protocol/roster_push.erl +++ b/apps/roster/src/protocol/roster_push.erl @@ -67,7 +67,10 @@ send_push_notification(Auth, Payload, PushAlert, PushType, HS) -> ok -> ok; {error, bad_token} -> - %% TODO: Remove bad token. + ClientId = Auth#'Auth'.client_id, + ?LOG_INFO("Deleting Auth for ~p because of bad push token", + [ClientId]), + kvs:delete('Auth', ClientId), ok; {error, not_delivered} -> ok -- GitLab From 0055a29aa9e34bc717faa820a8859b681fea1fab Mon Sep 17 00:00:00 2001 From: Tobias Lindahl Date: Tue, 9 Jun 2020 15:08:17 +0200 Subject: [PATCH 4/5] Refactor for silky smoothness --- apps/roster/src/protocol/roster_push.erl | 78 +++++++++++------------- 1 file changed, 36 insertions(+), 42 deletions(-) diff --git a/apps/roster/src/protocol/roster_push.erl b/apps/roster/src/protocol/roster_push.erl index 346ded29b..ca6e13e5f 100644 --- a/apps/roster/src/protocol/roster_push.erl +++ b/apps/roster/src/protocol/roster_push.erl @@ -2,7 +2,6 @@ -include_lib("kernel/include/logger.hrl"). -include("roster.hrl"). -include_lib("n2o/include/n2o.hrl"). --include_lib("kvs/include/kvs.hrl"). -export([ start/0 , send_push_notification/4 , proc/2 @@ -18,12 +17,18 @@ start() -> send_push_notification(Session, Payload, Alert, Type) -> case is_push_session(Session) of true -> - n2o_async:pid(system, ?MODULE) - ! {async_push, Session, Payload, Alert, Type}; + Pid = n2o_async:pid(system, ?MODULE), + Pid ! {async_push, Session, Payload, Alert, Type}, + ok; false -> - ok + skip end. +is_push_session(#'Auth'{push = []}) -> false; +is_push_session(#'Auth'{os = ios}) -> true; +is_push_session(#'Auth'{os = android}) -> true; +is_push_session(#'Auth'{}) -> false. + proc(init, #handler{name = ?MODULE} = Async) -> ConnState = #{ android => [] , ios => roster_apns_api:start()}, @@ -41,46 +46,35 @@ proc({reconnecting, _Pid}, #handler{} = H) -> %% APNS connection. Safe to ignore {noreply, H}; -proc({async_push, Session, Payload, PushAlert, PushType}, Handler) -> - HS = Handler#handler.state, - send_push_notification(Session, Payload, PushAlert, PushType, HS), - {reply, [], Handler}. - - -is_push_session(#'Auth'{os = ios} ) -> true; -is_push_session(#'Auth'{os = android}) -> true; -is_push_session(#'Auth'{}) -> false. - -send_push_notification(#'Auth'{ push = [] }, _, _, _, _) -> - skip; -send_push_notification(#'Auth'{ os = OS }, _, _, _, _) when OS /= android; - OS /= ios -> - skip; -send_push_notification(Auth, Payload, PushAlert, PushType, HS) -> - #'Auth'{ os = OS - , push = PushToken - , user_id = PhoneId - , settings = AuthSettings} = Auth, - ?LOG_INFO("~p:~p:~P PushAlert:~p", [PhoneId, OS, PushToken, 5, PushAlert]), - case send_push_notification(OS, PushToken, Payload, PushAlert, - PushType, AuthSettings, HS) of - ok -> - ok; +proc({async_push, Session, Payload, Alert, Type}, Handler) -> + case do_push(Session, Payload, Alert, Type, Handler#handler.state) of + ok -> + {noreply, Handler}; {error, bad_token} -> - ClientId = Auth#'Auth'.client_id, - ?LOG_INFO("Deleting Auth for ~p because of bad push token", - [ClientId]), + ClientId = Session#'Auth'.client_id, + ?LOG_INFO("Deleting Auth for ~p: bad push token", [ClientId]), kvs:delete('Auth', ClientId), - ok; + {noreply, Handler}; {error, not_delivered} -> - ok + {noreply, Handler} + end. + + +do_push(#'Auth'{ os = OS, push = Token, user_id = PhoneId, settings = Settings}, + Payload, Alert, Type, State) -> + ?LOG_INFO("~p:~p:~P PushAlert:~p", [PhoneId, OS, Token, 5, Alert]), + case OS of + ios -> + IOSPayload = base64:encode(term_to_binary(Payload)), + ConnectionState = maps:get(ios, State), + roster_apns_api:notify(Alert, IOSPayload, Type, Token, + Settings, ConnectionState); + android -> + AndroidPayload = android_push_payload(Payload, Alert, Type), + roster_fcm_api:notify(AndroidPayload, Token) end. -send_push_notification(ios, Push, Payload, PushAlert, PushType, AuthSettings, HS) -> - EncodedPayload = base64:encode(term_to_binary(Payload)), - IOS = maps:get(ios, HS), - roster_apns_api:notify(PushAlert, EncodedPayload, PushType, Push, AuthSettings, IOS); -send_push_notification(android, Push, Payload, PushAlert, PushType, _AuthSettings,_HS) -> - PushModel = #push{model = Payload, type = PushType, alert = PushAlert, title = PushAlert, badge = 1}, - AndroidPush = http_uri:encode(binary_to_list(base64:encode(term_to_binary(PushModel)))), - roster_fcm_api:notify(AndroidPush, Push). +android_push_payload(Payload, Alert, Type) -> + Model = #push{model = Payload, type = Type, alert = Alert, + title = Alert, badge = 1}, + http_uri:encode(binary_to_list(base64:encode(term_to_binary(Model)))). -- GitLab From 3d1c296822861b5d451aa3bfd9e1a1495b26ea57 Mon Sep 17 00:00:00 2001 From: Tobias Lindahl Date: Tue, 9 Jun 2020 16:18:24 +0200 Subject: [PATCH 5/5] Adjust for correct FCM error message signalling a bad token --- apps/roster/src/api/push/roster_fcm_api.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/roster/src/api/push/roster_fcm_api.erl b/apps/roster/src/api/push/roster_fcm_api.erl index eafd8cf8f..5ac1ba18d 100644 --- a/apps/roster/src/api/push/roster_fcm_api.erl +++ b/apps/roster/src/api/push/roster_fcm_api.erl @@ -31,7 +31,7 @@ notify(MessageBody, DeviceId) -> [<<"id">> | _] -> ?LOG_INFO("FCM push sent for ~P", [DeviceId, 5]), ok; - [<<"Error">>, <<"InvalidRegistration">>] -> + [<<"Error">>, <<"NotRegistered">>] -> ?LOG_INFO("FCM push to bad token ~P", [DeviceId, 5]), {error, bad_token}; -- GitLab