From 0ddaf5c6109f0eba6c886625db7c821080a956f3 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Sun, 5 Mar 2017 18:11:44 -0800 Subject: [PATCH] push: Pass reg_ids list into send_android_push_notification. This refactoring is preparation for being able to forward push notifications to users on behalf of another Zulip server. The goal is to remove access to the current server's database from the send_*_push_notification code paths. --- zerver/lib/push_notifications.py | 24 +++++++++++++++--------- zerver/tests/test_push_notifications.py | 14 +++++++------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 9058ef20c6..159c55f22e 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -199,15 +199,19 @@ if settings.ANDROID_GCM_API_KEY: else: gcm = None -@statsd_increment("android_push_notification") -def send_android_push_notification(user, data): +def send_android_push_notification_to_user(user_profile, data): # type: (UserProfile, Dict[str, Any]) -> None + devices = list(PushDeviceToken.objects.filter(user=user_profile, + kind=PushDeviceToken.GCM)) + send_android_push_notification(devices, data) + +@statsd_increment("android_push_notification") +def send_android_push_notification(devices, data): + # type: (List[PushDeviceToken], Dict[str, Any]) -> None if not gcm: logging.error("Attempting to send a GCM push notification, but no API key was configured") return - - reg_ids = [device.token for device in - PushDeviceToken.objects.filter(user=user, kind=PushDeviceToken.GCM)] + reg_ids = [device.token for device in devices] res = gcm.json_request(registration_ids=reg_ids, data=data) @@ -270,9 +274,11 @@ def handle_push_notification(user_profile_id, missed_message): sender_str = message.sender.full_name apple = num_push_devices_for_user(user_profile, kind=PushDeviceToken.APNS) - android = num_push_devices_for_user(user_profile, kind=PushDeviceToken.GCM) + android_devices = [device for device in + PushDeviceToken.objects.filter(user=user_profile, + kind=PushDeviceToken.GCM)] - if apple or android: + if apple or android_devices: # TODO: set badge count in a better way # Determine what alert string to display based on the missed messages if message.recipient.type == Recipient.HUDDLE: @@ -288,7 +294,7 @@ def handle_push_notification(user_profile_id, missed_message): apple_extra_data = {'message_ids': [message.id]} send_apple_push_notification(user_profile, alert, badge=1, zulip=apple_extra_data) - if android: + if android_devices: content = message.content content_truncated = (len(content) > 200) if content_truncated: @@ -314,7 +320,7 @@ def handle_push_notification(user_profile_id, missed_message): elif message.recipient.type in (Recipient.HUDDLE, Recipient.PERSONAL): android_data['recipient_type'] = "private" - send_android_push_notification(user_profile, android_data) + send_android_push_notification(android_devices, android_data) except UserMessage.DoesNotExist: logging.error("Could not find UserMessage with message_id %s" % (missed_message['message_id'],)) diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index da3808042c..cd06a21710 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -251,7 +251,7 @@ class GCMNotSetTest(GCMTest): def test_gcm_is_none(self, mock_error): # type: (mock.MagicMock) -> None apn.gcm = None - apn.send_android_push_notification(self.user_profile, {}) + apn.send_android_push_notification_to_user(self.user_profile, {}) mock_error.assert_called_with("Attempting to send a GCM push " "notification, but no API key was " "configured") @@ -267,7 +267,7 @@ class GCMSuccessTest(GCMTest): mock_send.return_value = res data = self.get_gcm_data() - apn.send_android_push_notification(self.user_profile, data) + apn.send_android_push_notification_to_user(self.user_profile, data) self.assertEqual(mock_info.call_count, 2) c1 = call("GCM: Sent 1111 as 0") c2 = call("GCM: Sent 2222 as 1") @@ -284,7 +284,7 @@ class GCMCanonicalTest(GCMTest): mock_send.return_value = res data = self.get_gcm_data() - apn.send_android_push_notification(self.user_profile, data) + apn.send_android_push_notification_to_user(self.user_profile, data) mock_warning.assert_called_once_with("GCM: Got canonical ref but it " "already matches our ID 1!") @@ -308,7 +308,7 @@ class GCMCanonicalTest(GCMTest): self.assertEqual(get_count(u'3333'), 0) data = self.get_gcm_data() - apn.send_android_push_notification(self.user_profile, data) + apn.send_android_push_notification_to_user(self.user_profile, data) msg = ("GCM: Got canonical ref %s " "replacing %s but new ID not " "registered! Updating.") @@ -337,7 +337,7 @@ class GCMCanonicalTest(GCMTest): self.assertEqual(get_count(u'2222'), 1) data = self.get_gcm_data() - apn.send_android_push_notification(self.user_profile, data) + apn.send_android_push_notification_to_user(self.user_profile, data) mock_info.assert_called_once_with( "GCM: Got canonical ref %s, dropping %s" % (new_token, old_token)) @@ -363,7 +363,7 @@ class GCMNotRegisteredTest(GCMTest): self.assertEqual(get_count(u'1111'), 1) data = self.get_gcm_data() - apn.send_android_push_notification(self.user_profile, data) + apn.send_android_push_notification_to_user(self.user_profile, data) mock_info.assert_called_once_with("GCM: Removing %s" % (token,)) self.assertEqual(get_count(u'1111'), 0) @@ -378,7 +378,7 @@ class GCMFailureTest(GCMTest): mock_send.return_value = res data = self.get_gcm_data() - apn.send_android_push_notification(self.user_profile, data) + apn.send_android_push_notification_to_user(self.user_profile, data) c1 = call("GCM: Delivery to %s failed: Failed" % (token,)) mock_warn.assert_has_calls([c1], any_order=True)