From 613d093d7ddf49edb7a0ee7a3e2a2cf429f9d7c4 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Aug 2017 16:38:11 -0700 Subject: [PATCH] push notifs: Implement APNs with new API. And it works! A couple of things still to do: * When a device token is no longer active, we'll get HTTP status 410. We should then remove the token from the database so we don't keep trying to push to it. This is fairly urgent. * The library we're using has a nice asynchronous API, but this version doesn't use it. This is OK now, but async will be essential at scale. --- zerver/lib/push_notifications.py | 49 +++++++++++++++++++++---- zerver/tests/test_push_notifications.py | 21 +++++++---- zilencer/views.py | 4 +- 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index b7cb1d4d15..18dd63d700 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -9,6 +9,8 @@ import time import random from typing import Any, Dict, List, Optional, SupportsInt, Text, Union, Type +from apns2.client import APNsClient +from apns2.payload import Payload as APNsPayload from django.conf import settings from django.utils.timezone import now as timezone_now from django.utils.translation import ugettext as _ @@ -49,16 +51,42 @@ def hex_to_b64(data): # Sending to APNs, for iOS # -# `APNS_SANDBOX` should be a bool -assert isinstance(settings.APNS_SANDBOX, bool) +_apns_client = None # type: APNsClient + +def get_apns_client(): + global _apns_client + if _apns_client is None: + # NB if called concurrently, this will make excess connections. + # That's a little sloppy, but harmless unless a server gets + # hammered with a ton of these all at once after startup. + _apns_client = APNsClient(credentials=settings.APNS_CERT_FILE, + use_sandbox=settings.APNS_SANDBOX) + return _apns_client @statsd_increment("apple_push_notification") -def send_apple_push_notification(user_id, devices, **extra_data): - # type: (int, List[DeviceToken], **Any) -> None +def send_apple_push_notification(user_id, devices, payload_data): + # type: (int, List[DeviceToken], Dict[str, Any]) -> None if not devices: return - logging.warn("APNs unimplemented. Dropping notification for user %d with %d devices.", + logging.info("APNs: Sending notification for user %d to %d devices", user_id, len(devices)) + payload = APNsPayload(**payload_data) + expiration = int(time.time() + 24 * 3600) + client = get_apns_client() + for device in devices: + # TODO obviously this should be made to actually use the async + stream_id = client.send_notification_async( + device.token, payload, topic='org.zulip.Zulip', + expiration=expiration) + result = client.get_notification_result(stream_id) + if result == 'Success': + logging.info("APNs: Success sending for user %d to device %s", + user_id, device.token) + else: + logging.warn("APNs: Failed to send for user %d to device %s: %s", + user_id, device.token, result) + # TODO delete token if status 410 (and timestamp isn't before + # the token we have) # # Sending to GCM, for Android @@ -293,7 +321,13 @@ def get_apns_payload(message): # type: (Message) -> Dict[str, Any] return { 'alert': get_alert_from_message(message), - 'message_ids': [message.id], + # TODO: set badge count in a better way + 'badge': 1, + 'custom': { + 'zulip': { + 'message_ids': [message.id], + } + } } def get_gcm_payload(user_profile, message): @@ -369,10 +403,9 @@ def handle_push_notification(user_profile_id, missed_message): apple_devices = list(PushDeviceToken.objects.filter(user=user_profile, kind=PushDeviceToken.APNS)) - # TODO: set badge count in a better way if apple_devices: send_apple_push_notification(user_profile.id, apple_devices, - badge=1, zulip=apns_payload) + apns_payload) if android_devices: send_android_push_notification(android_devices, gcm_payload) diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index 9d451f1160..119f9341ec 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -306,6 +306,7 @@ class HandlePushNotificationTest(PushNotificationTest): mock.patch('zerver.lib.push_notifications.requests.request', side_effect=self.bounce_request), \ mock.patch('zerver.lib.push_notifications.gcm') as mock_gcm, \ + mock.patch('zerver.lib.push_notifications._apns_client') as mock_apns, \ mock.patch('logging.info') as mock_info, \ mock.patch('logging.warn') as mock_warn: apns_devices = [ @@ -320,10 +321,12 @@ class HandlePushNotificationTest(PushNotificationTest): ] mock_gcm.json_request.return_value = { 'success': {gcm_devices[0][2]: message.id}} + mock_apns.get_notification_result.return_value = 'Success' apn.handle_push_notification(self.user_profile.id, missed_message) - mock_warn.assert_called_with( - "APNs unimplemented. Dropping notification for user %d with %d devices.", - self.user_profile.id, len(apns_devices)) + for _, _, token in apns_devices: + mock_info.assert_any_call( + "APNs: Success sending for user %d to device %s", + self.user_profile.id, token) for _, _, token in gcm_devices: mock_info.assert_any_call( "GCM: Sent %s as %s" % (token, message.id)) @@ -455,8 +458,7 @@ class HandlePushNotificationTest(PushNotificationTest): apn.handle_push_notification(self.user_profile.id, missed_message) mock_send_apple.assert_called_with(self.user_profile.id, apple_devices, - badge=1, - zulip={'apns': True}) + {'apns': True}) mock_send_android.assert_called_with(android_devices, {'gcm': True}) @@ -488,8 +490,13 @@ class TestGetAPNsPayload(PushNotificationTest): message = self.get_message(Recipient.HUDDLE) payload = apn.get_apns_payload(message) expected = { - "alert": "New private group message from King Hamlet", - "message_ids": [message.id], + 'alert': "New private group message from King Hamlet", + 'badge': 1, + 'custom': { + 'zulip': { + 'message_ids': [message.id], + } + } } self.assertDictEqual(payload, expected) diff --git a/zilencer/views.py b/zilencer/views.py index bb1a1b4000..9446b871d1 100644 --- a/zilencer/views.py +++ b/zilencer/views.py @@ -102,9 +102,7 @@ def remote_server_notify_push(request, # type: HttpRequest if android_devices: send_android_push_notification(android_devices, gcm_payload, remote=True) - # TODO: set badge count in a better way if apple_devices: - send_apple_push_notification(user_id, apple_devices, - badge=1, zulip=apns_payload) + send_apple_push_notification(user_id, apple_devices, apns_payload) return json_success()