From 0af7c84c99b7cf139494d737d9619875b9fbaf5a Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Tue, 28 Sep 2021 14:17:16 +0200 Subject: [PATCH] push_notifs: Log the number of devices notification was sent to. --- zerver/lib/push_notifications.py | 25 +++++++++-- zerver/lib/remote_server.py | 6 ++- zerver/tests/test_push_notifications.py | 59 +++++++++++++++++++++++-- zilencer/views.py | 4 +- 4 files changed, 84 insertions(+), 10 deletions(-) diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 79ed28761a..b8c144e011 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -391,7 +391,7 @@ def send_notifications_to_bouncer( apns_payload: Dict[str, Any], gcm_payload: Dict[str, Any], gcm_options: Dict[str, Any], -) -> None: +) -> Tuple[int, int]: post_data = { "user_id": user_profile_id, "apns_payload": apns_payload, @@ -399,7 +399,11 @@ def send_notifications_to_bouncer( "gcm_options": gcm_options, } # Calls zilencer.views.remote_server_notify_push - send_json_to_push_bouncer("POST", "push/notify", post_data) + response_data = send_json_to_push_bouncer("POST", "push/notify", post_data) + assert isinstance(response_data["total_android_devices"], int) + assert isinstance(response_data["total_apple_devices"], int) + + return response_data["total_android_devices"], response_data["total_apple_devices"] # @@ -959,7 +963,15 @@ def handle_push_notification(user_profile_id: int, missed_message: Dict[str, Any logger.info("Sending push notifications to mobile clients for user %s", user_profile_id) if uses_notification_bouncer(): - send_notifications_to_bouncer(user_profile_id, apns_payload, gcm_payload, gcm_options) + total_android_devices, total_apple_devices = send_notifications_to_bouncer( + user_profile_id, apns_payload, gcm_payload, gcm_options + ) + logger.info( + "Sent mobile push notifications for user %s through bouncer: %s via FCM devices, %s via APNs devices", + user_profile_id, + total_android_devices, + total_apple_devices, + ) return android_devices = list( @@ -970,6 +982,11 @@ def handle_push_notification(user_profile_id: int, missed_message: Dict[str, Any PushDeviceToken.objects.filter(user=user_profile, kind=PushDeviceToken.APNS) ) + logger.info( + "Sending mobile push notifications for user %s: %s via FCM devices, %s via APNs devices", + user_profile_id, + len(android_devices), + len(apple_devices), + ) send_apple_push_notification(user_profile.id, apple_devices, apns_payload) - send_android_push_notification(android_devices, gcm_payload, gcm_options) diff --git a/zerver/lib/remote_server.py b/zerver/lib/remote_server.py index 48929ecbd1..7877e663f8 100644 --- a/zerver/lib/remote_server.py +++ b/zerver/lib/remote_server.py @@ -107,8 +107,10 @@ def send_to_push_bouncer( return orjson.loads(res.content) -def send_json_to_push_bouncer(method: str, endpoint: str, post_data: Mapping[str, object]) -> None: - send_to_push_bouncer( +def send_json_to_push_bouncer( + method: str, endpoint: str, post_data: Mapping[str, object] +) -> Dict[str, object]: + return send_to_push_bouncer( method, endpoint, orjson.dumps(post_data), diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index 8639dceebe..f20c6d53e0 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -273,6 +273,45 @@ class PushBouncerNotificationTest(BouncerTestCase): result = self.uuid_post(self.server_uuid, endpoint, payload) self.assert_json_error(result, "Empty or invalid length token") + def test_send_notification_endpoint(self) -> None: + hamlet = self.example_user("hamlet") + server = RemoteZulipServer.objects.get(uuid=self.server_uuid) + token = "aaaa" + for i in ["aa", "bb"]: + RemotePushDeviceToken.objects.create( + kind=RemotePushDeviceToken.GCM, + token=hex_to_b64(token + i), + user_id=hamlet.id, + server=server, + ) + RemotePushDeviceToken.objects.create( + kind=RemotePushDeviceToken.APNS, + token=hex_to_b64(token), + user_id=hamlet.id, + server=server, + ) + payload = { + "user_id": hamlet.id, + "gcm_payload": "test", + "apns_payload": "test", + "gcm_options": {}, + } + with mock.patch("zilencer.views.send_android_push_notification"), mock.patch( + "zilencer.views.send_apple_push_notification" + ): + result = self.uuid_post( + self.server_uuid, + "/api/v1/remotes/push/notify", + payload, + content_type="application/json", + ) + self.assert_json_success(result) + data = result.json() + self.assertEqual( + {"result": "success", "msg": "", "total_android_devices": 2, "total_apple_devices": 1}, + data, + ) + def test_remote_push_unregister_all(self) -> None: payload = self.get_generic_payload("register") @@ -1051,8 +1090,10 @@ class HandlePushNotificationTest(PushNotificationTest): "zerver.lib.push_notifications.get_message_payload_gcm", return_value=({"gcm": True}, {}), ), mock.patch( - "zerver.lib.push_notifications.send_notifications_to_bouncer" - ) as mock_send: + "zerver.lib.push_notifications.send_notifications_to_bouncer", return_value=(3, 5) + ) as mock_send, self.assertLogs( + "zerver.lib.push_notifications", level="INFO" + ) as mock_logging_info: handle_push_notification(user_profile.id, missed_message) mock_send.assert_called_with( user_profile.id, @@ -1060,6 +1101,13 @@ class HandlePushNotificationTest(PushNotificationTest): {"gcm": True}, {}, ) + self.assertEqual( + mock_logging_info.output, + [ + f"INFO:zerver.lib.push_notifications:Sending push notifications to mobile clients for user {user_profile.id}", + f"INFO:zerver.lib.push_notifications:Sent mobile push notifications for user {user_profile.id} through bouncer: 3 via FCM devices, 5 via APNs devices", + ], + ) def test_non_bouncer_push(self) -> None: self.setup_apns_tokens() @@ -1877,7 +1925,10 @@ class TestGetGCMPayload(PushNotificationTest): class TestSendNotificationsToBouncer(ZulipTestCase): @mock.patch("zerver.lib.remote_server.send_to_push_bouncer") def test_send_notifications_to_bouncer(self, mock_send: mock.MagicMock) -> None: - send_notifications_to_bouncer(1, {"apns": True}, {"gcm": True}, {}) + mock_send.return_value = {"total_android_devices": 1, "total_apple_devices": 3} + total_android_devices, total_apple_devices = send_notifications_to_bouncer( + 1, {"apns": True}, {"gcm": True}, {} + ) post_data = { "user_id": 1, "apns_payload": {"apns": True}, @@ -1890,6 +1941,8 @@ class TestSendNotificationsToBouncer(ZulipTestCase): orjson.dumps(post_data), extra_headers={"Content-type": "application/json"}, ) + self.assertEqual(total_android_devices, 1) + self.assertEqual(total_apple_devices, 3) @override_settings(PUSH_NOTIFICATION_BOUNCER_URL="https://push.zulip.org.example.com") diff --git a/zilencer/views.py b/zilencer/views.py index ea1ef1b84b..7851f8b7cd 100644 --- a/zilencer/views.py +++ b/zilencer/views.py @@ -196,7 +196,9 @@ def remote_server_notify_push( send_apple_push_notification(user_id, apple_devices, apns_payload, remote=True) - return json_success() + return json_success( + {"total_android_devices": len(android_devices), "total_apple_devices": len(apple_devices)} + ) def validate_incoming_table_data(