push notif: Add GCM options to bouncer API; empty for now.

The first use case for this will be setting `priority`,
coming up shortly.
This commit is contained in:
Greg Price 2018-11-29 12:37:40 -08:00 committed by Tim Abbott
parent 3470e541c8
commit 49fd2e65de
3 changed files with 71 additions and 24 deletions

View File

@ -15,6 +15,7 @@ from django.utils.timezone import now as timezone_now
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
from gcm import GCM from gcm import GCM
import requests import requests
import ujson
from zerver.decorator import statsd_increment from zerver.decorator import statsd_increment
from zerver.lib.avatar import absolute_avatar_url from zerver.lib.avatar import absolute_avatar_url
@ -182,20 +183,42 @@ else:
def gcm_enabled() -> bool: # nocoverage def gcm_enabled() -> bool: # nocoverage
return gcm is not None return gcm is not None
def send_android_push_notification_to_user(user_profile: UserProfile, data: Dict[str, Any]) -> None: def send_android_push_notification_to_user(user_profile: UserProfile, data: Dict[str, Any],
options: Dict[str, Any]) -> None:
devices = list(PushDeviceToken.objects.filter(user=user_profile, devices = list(PushDeviceToken.objects.filter(user=user_profile,
kind=PushDeviceToken.GCM)) kind=PushDeviceToken.GCM))
send_android_push_notification(devices, data) send_android_push_notification(devices, data, options)
@statsd_increment("android_push_notification") @statsd_increment("android_push_notification")
def send_android_push_notification(devices: List[DeviceToken], data: Dict[str, Any], def send_android_push_notification(devices: List[DeviceToken], data: Dict[str, Any],
remote: bool=False) -> None: options: Dict[str, Any], remote: bool=False) -> None:
"""
Send a GCM message to the given devices.
See https://developers.google.com/cloud-messaging/http-server-ref
for the GCM upstream API which this talks to.
data: The JSON object (decoded) to send as the 'data' parameter of
the GCM message.
options: Additional options to control the GCM message sent, defined as
part of the Zulip notification bouncer's API. Including unrecognized
options is an error. Currently no options are recognized, so this
parameter must be `{}`.
"""
if not gcm: if not gcm:
logger.debug("Skipping sending a GCM push notification since " logger.debug("Skipping sending a GCM push notification since "
"PUSH_NOTIFICATION_BOUNCER_URL and ANDROID_GCM_API_KEY are both unset") "PUSH_NOTIFICATION_BOUNCER_URL and ANDROID_GCM_API_KEY are both unset")
return return
reg_ids = [device.token for device in devices] reg_ids = [device.token for device in devices]
if options:
# We're strict about the API; there is no use case for a newer Zulip
# server talking to an older bouncer, so we only need to provide
# one-way compatibility.
raise JsonableError(_("Invalid GCM options to bouncer: %s")
% (ujson.dumps(options),))
if remote: if remote:
DeviceTokenClass = RemotePushDeviceToken DeviceTokenClass = RemotePushDeviceToken
else: else:
@ -260,11 +283,13 @@ def uses_notification_bouncer() -> bool:
def send_notifications_to_bouncer(user_profile_id: int, def send_notifications_to_bouncer(user_profile_id: int,
apns_payload: Dict[str, Any], apns_payload: Dict[str, Any],
gcm_payload: Dict[str, Any]) -> None: gcm_payload: Dict[str, Any],
gcm_options: Dict[str, Any]) -> None:
post_data = { post_data = {
'user_id': user_profile_id, 'user_id': user_profile_id,
'apns_payload': apns_payload, 'apns_payload': apns_payload,
'gcm_payload': gcm_payload, 'gcm_payload': gcm_payload,
'gcm_options': gcm_options,
} }
# Calls zilencer.views.remote_server_notify_push # Calls zilencer.views.remote_server_notify_push
send_json_to_push_bouncer('POST', 'push/notify', post_data) send_json_to_push_bouncer('POST', 'push/notify', post_data)
@ -539,12 +564,14 @@ def handle_remove_push_notification(user_profile_id: int, message_id: int) -> No
'event': 'remove', 'event': 'remove',
'zulip_message_id': message_id, # message_id is reserved for CCS 'zulip_message_id': message_id, # message_id is reserved for CCS
}) })
gcm_options = {} # type: Dict[str, Any]
if uses_notification_bouncer(): if uses_notification_bouncer():
try: try:
send_notifications_to_bouncer(user_profile_id, send_notifications_to_bouncer(user_profile_id,
{}, {},
gcm_payload) gcm_payload,
gcm_options)
except requests.ConnectionError: # nocoverage except requests.ConnectionError: # nocoverage
def failure_processor(event: Dict[str, Any]) -> None: def failure_processor(event: Dict[str, Any]) -> None:
logger.warning( logger.warning(
@ -556,7 +583,7 @@ def handle_remove_push_notification(user_profile_id: int, message_id: int) -> No
kind=PushDeviceToken.GCM)) kind=PushDeviceToken.GCM))
if android_devices: if android_devices:
send_android_push_notification(android_devices, gcm_payload) send_android_push_notification(android_devices, gcm_payload, gcm_options)
user_message.flags.active_mobile_push_notification = False user_message.flags.active_mobile_push_notification = False
user_message.save(update_fields=["flags"]) user_message.save(update_fields=["flags"])
@ -613,13 +640,15 @@ def handle_push_notification(user_profile_id: int, missed_message: Dict[str, Any
apns_payload = get_apns_payload(user_profile, message) apns_payload = get_apns_payload(user_profile, message)
gcm_payload = get_gcm_payload(user_profile, message) gcm_payload = get_gcm_payload(user_profile, message)
gcm_options = {} # type: Dict[str, Any]
logger.info("Sending push notifications to mobile clients for user %s" % (user_profile_id,)) logger.info("Sending push notifications to mobile clients for user %s" % (user_profile_id,))
if uses_notification_bouncer(): if uses_notification_bouncer():
try: try:
send_notifications_to_bouncer(user_profile_id, send_notifications_to_bouncer(user_profile_id,
apns_payload, apns_payload,
gcm_payload) gcm_payload,
gcm_options)
except requests.ConnectionError: except requests.ConnectionError:
def failure_processor(event: Dict[str, Any]) -> None: def failure_processor(event: Dict[str, Any]) -> None:
logger.warning( logger.warning(
@ -640,4 +669,4 @@ def handle_push_notification(user_profile_id: int, missed_message: Dict[str, Any
apns_payload) apns_payload)
if android_devices: if android_devices:
send_android_push_notification(android_devices, gcm_payload) send_android_push_notification(android_devices, gcm_payload, gcm_options)

View File

@ -646,6 +646,7 @@ class HandlePushNotificationTest(PushNotificationTest):
mock_send.assert_called_with(user_profile.id, mock_send.assert_called_with(user_profile.id,
{'apns': True}, {'apns': True},
{'gcm': True}, {'gcm': True},
{},
) )
def test_non_bouncer_push(self) -> None: def test_non_bouncer_push(self) -> None:
@ -687,8 +688,7 @@ class HandlePushNotificationTest(PushNotificationTest):
mock_send_apple.assert_called_with(self.user_profile.id, mock_send_apple.assert_called_with(self.user_profile.id,
apple_devices, apple_devices,
{'apns': True}) {'apns': True})
mock_send_android.assert_called_with(android_devices, mock_send_android.assert_called_with(android_devices, {'gcm': True}, {})
{'gcm': True})
mock_push_notifications.assert_called_once() mock_push_notifications.assert_called_once()
@override_settings(SEND_REMOVE_PUSH_NOTIFICATIONS=True) @override_settings(SEND_REMOVE_PUSH_NOTIFICATIONS=True)
@ -709,7 +709,8 @@ class HandlePushNotificationTest(PushNotificationTest):
mock_send_android.assert_called_with(user_profile.id, {}, mock_send_android.assert_called_with(user_profile.id, {},
{'gcm': True, {'gcm': True,
'event': 'remove', 'event': 'remove',
'zulip_message_id': message.id}) 'zulip_message_id': message.id},
{})
@override_settings(SEND_REMOVE_PUSH_NOTIFICATIONS=True) @override_settings(SEND_REMOVE_PUSH_NOTIFICATIONS=True)
def test_non_bouncer_push_remove(self) -> None: def test_non_bouncer_push_remove(self) -> None:
@ -737,7 +738,8 @@ class HandlePushNotificationTest(PushNotificationTest):
mock_send_android.assert_called_with(android_devices, mock_send_android.assert_called_with(android_devices,
{'gcm': True, {'gcm': True,
'event': 'remove', 'event': 'remove',
'zulip_message_id': message.id}) 'zulip_message_id': message.id},
{})
def test_user_message_does_not_exist(self) -> None: def test_user_message_does_not_exist(self) -> None:
"""This simulates a condition that should only be an error if the user is """This simulates a condition that should only be an error if the user is
@ -797,8 +799,7 @@ class HandlePushNotificationTest(PushNotificationTest):
mock_send_apple.assert_called_with(self.user_profile.id, mock_send_apple.assert_called_with(self.user_profile.id,
apple_devices, apple_devices,
{'apns': True}) {'apns': True})
mock_send_android.assert_called_with(android_devices, mock_send_android.assert_called_with(android_devices, {'gcm': True}, {})
{'gcm': True})
mock_push_notifications.assert_called_once() mock_push_notifications.assert_called_once()
class TestAPNs(PushNotificationTest): class TestAPNs(PushNotificationTest):
@ -1201,11 +1202,12 @@ class TestGetGCMPayload(PushNotificationTest):
class TestSendNotificationsToBouncer(ZulipTestCase): class TestSendNotificationsToBouncer(ZulipTestCase):
@mock.patch('zerver.lib.remote_server.send_to_push_bouncer') @mock.patch('zerver.lib.remote_server.send_to_push_bouncer')
def test_send_notifications_to_bouncer(self, mock_send: mock.MagicMock) -> None: def test_send_notifications_to_bouncer(self, mock_send: mock.MagicMock) -> None:
apn.send_notifications_to_bouncer(1, {'apns': True}, {'gcm': True}) apn.send_notifications_to_bouncer(1, {'apns': True}, {'gcm': True}, {})
post_data = { post_data = {
'user_id': 1, 'user_id': 1,
'apns_payload': {'apns': True}, 'apns_payload': {'apns': True},
'gcm_payload': {'gcm': True}, 'gcm_payload': {'gcm': True},
'gcm_options': {},
} }
mock_send.assert_called_with('POST', mock_send.assert_called_with('POST',
'push/notify', 'push/notify',
@ -1345,7 +1347,7 @@ class GCMNotSetTest(GCMTest):
@mock.patch('zerver.lib.push_notifications.logger.debug') @mock.patch('zerver.lib.push_notifications.logger.debug')
def test_gcm_is_none(self, mock_debug: mock.MagicMock) -> None: def test_gcm_is_none(self, mock_debug: mock.MagicMock) -> None:
apn.gcm = None apn.gcm = None
apn.send_android_push_notification_to_user(self.user_profile, {}) apn.send_android_push_notification_to_user(self.user_profile, {}, {})
mock_debug.assert_called_with( mock_debug.assert_called_with(
"Skipping sending a GCM push notification since PUSH_NOTIFICATION_BOUNCER_URL " "Skipping sending a GCM push notification since PUSH_NOTIFICATION_BOUNCER_URL "
"and ANDROID_GCM_API_KEY are both unset") "and ANDROID_GCM_API_KEY are both unset")
@ -1356,7 +1358,7 @@ class GCMIOErrorTest(GCMTest):
def test_json_request_raises_ioerror(self, mock_warn: mock.MagicMock, def test_json_request_raises_ioerror(self, mock_warn: mock.MagicMock,
mock_json_request: mock.MagicMock) -> None: mock_json_request: mock.MagicMock) -> None:
mock_json_request.side_effect = IOError('error') mock_json_request.side_effect = IOError('error')
apn.send_android_push_notification_to_user(self.user_profile, {}) apn.send_android_push_notification_to_user(self.user_profile, {}, {})
mock_warn.assert_called_with('error') mock_warn.assert_called_with('error')
class GCMSuccessTest(GCMTest): class GCMSuccessTest(GCMTest):
@ -1370,13 +1372,28 @@ class GCMSuccessTest(GCMTest):
mock_send.return_value = res mock_send.return_value = res
data = self.get_gcm_data() data = self.get_gcm_data()
apn.send_android_push_notification_to_user(self.user_profile, data) apn.send_android_push_notification_to_user(self.user_profile, data, {})
self.assertEqual(mock_info.call_count, 2) self.assertEqual(mock_info.call_count, 2)
c1 = call("GCM: Sent 1111 as 0") c1 = call("GCM: Sent 1111 as 0")
c2 = call("GCM: Sent 2222 as 1") c2 = call("GCM: Sent 2222 as 1")
mock_info.assert_has_calls([c1, c2], any_order=True) mock_info.assert_has_calls([c1, c2], any_order=True)
mock_warning.assert_not_called() mock_warning.assert_not_called()
@mock.patch('zerver.lib.push_notifications.logger.warning')
@mock.patch('zerver.lib.push_notifications.logger.info')
@mock.patch('gcm.GCM.json_request')
def test_invalid_options(self, mock_send: mock.MagicMock, mock_info: mock.MagicMock,
mock_warning: mock.MagicMock) -> None:
res = {}
res['success'] = {token: ind for ind, token in enumerate(self.gcm_tokens)}
mock_send.return_value = res
data = self.get_gcm_data()
with self.assertRaises(JsonableError):
apn.send_android_push_notification_to_user(self.user_profile, data,
{"invalid": True})
mock_send.assert_not_called()
class GCMCanonicalTest(GCMTest): class GCMCanonicalTest(GCMTest):
@mock.patch('zerver.lib.push_notifications.logger.warning') @mock.patch('zerver.lib.push_notifications.logger.warning')
@mock.patch('gcm.GCM.json_request') @mock.patch('gcm.GCM.json_request')
@ -1386,7 +1403,7 @@ class GCMCanonicalTest(GCMTest):
mock_send.return_value = res mock_send.return_value = res
data = self.get_gcm_data() data = self.get_gcm_data()
apn.send_android_push_notification_to_user(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 " mock_warning.assert_called_once_with("GCM: Got canonical ref but it "
"already matches our ID 1!") "already matches our ID 1!")
@ -1409,7 +1426,7 @@ class GCMCanonicalTest(GCMTest):
self.assertEqual(get_count(u'3333'), 0) self.assertEqual(get_count(u'3333'), 0)
data = self.get_gcm_data() data = self.get_gcm_data()
apn.send_android_push_notification_to_user(self.user_profile, data) apn.send_android_push_notification_to_user(self.user_profile, data, {})
msg = ("GCM: Got canonical ref %s " msg = ("GCM: Got canonical ref %s "
"replacing %s but new ID not " "replacing %s but new ID not "
"registered! Updating.") "registered! Updating.")
@ -1437,7 +1454,7 @@ class GCMCanonicalTest(GCMTest):
self.assertEqual(get_count(u'2222'), 1) self.assertEqual(get_count(u'2222'), 1)
data = self.get_gcm_data() data = self.get_gcm_data()
apn.send_android_push_notification_to_user(self.user_profile, data) apn.send_android_push_notification_to_user(self.user_profile, data, {})
mock_info.assert_called_once_with( mock_info.assert_called_once_with(
"GCM: Got canonical ref %s, dropping %s" % (new_token, old_token)) "GCM: Got canonical ref %s, dropping %s" % (new_token, old_token))
@ -1461,7 +1478,7 @@ class GCMNotRegisteredTest(GCMTest):
self.assertEqual(get_count(u'1111'), 1) self.assertEqual(get_count(u'1111'), 1)
data = self.get_gcm_data() data = self.get_gcm_data()
apn.send_android_push_notification_to_user(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,)) mock_info.assert_called_once_with("GCM: Removing %s" % (token,))
self.assertEqual(get_count(u'1111'), 0) self.assertEqual(get_count(u'1111'), 0)
@ -1475,7 +1492,7 @@ class GCMFailureTest(GCMTest):
mock_send.return_value = res mock_send.return_value = res
data = self.get_gcm_data() data = self.get_gcm_data()
apn.send_android_push_notification_to_user(self.user_profile, data) apn.send_android_push_notification_to_user(self.user_profile, data, {})
c1 = call("GCM: Delivery to %s failed: Failed" % (token,)) c1 = call("GCM: Delivery to %s failed: Failed" % (token,))
mock_warn.assert_has_calls([c1], any_order=True) mock_warn.assert_has_calls([c1], any_order=True)

View File

@ -128,6 +128,7 @@ def remote_server_notify_push(request: HttpRequest, entity: Union[UserProfile, R
user_id = payload['user_id'] user_id = payload['user_id']
gcm_payload = payload['gcm_payload'] gcm_payload = payload['gcm_payload']
apns_payload = payload['apns_payload'] apns_payload = payload['apns_payload']
gcm_options = payload.get('gcm_options', {})
android_devices = list(RemotePushDeviceToken.objects.filter( android_devices = list(RemotePushDeviceToken.objects.filter(
user_id=user_id, user_id=user_id,
@ -142,7 +143,7 @@ def remote_server_notify_push(request: HttpRequest, entity: Union[UserProfile, R
)) ))
if android_devices: if android_devices:
send_android_push_notification(android_devices, gcm_payload, remote=True) send_android_push_notification(android_devices, gcm_payload, gcm_options, remote=True)
if apple_devices: if apple_devices:
send_apple_push_notification(user_id, apple_devices, apns_payload, remote=True) send_apple_push_notification(user_id, apple_devices, apns_payload, remote=True)