push notifications: Clean up unregistered/bad APNS tokens.

We've had this sort of logic for GCM for a long time; it's worth
adding for APNS as well.

Writing this is a bit of a reminder that I'm not a fan of how our unit
tests for push notifications work.

zerver/lib/push_notifications.py

@@ -102,12 +102,18 @@ APNS_MAX_RETRIES = 3
 
 @statsd_increment("apple_push_notification")
 def send_apple_push_notification(user_id: int, devices: List[DeviceToken],
-                                 payload_data: Dict[str, Any]) -> None:
+                                 payload_data: Dict[str, Any], remote=False) -> None:
     client = get_apns_client()
     if client is None:
         logging.warning("APNs: Dropping a notification because nothing configured.  "
                         "Set PUSH_NOTIFICATION_BOUNCER_URL (or APNS_CERT_FILE).")
         return
+
+    if remote:
+        DeviceTokenClass = RemotePushDeviceToken
+    else:
+        DeviceTokenClass = PushDeviceToken
+
     logging.info("APNs: Sending notification for user %d to %d devices",
                  user_id, len(devices))
     payload = APNsPayload(**modernize_apns_payload(payload_data))
@@ -137,11 +143,14 @@ def send_apple_push_notification(user_id: int, devices: List[DeviceToken],
         if result == 'Success':
             logging.info("APNs: Success sending for user %d to device %s",
                          user_id, device.token)
+        elif result in ["Unregistered", "BadDeviceToken", "DeviceTokenNotForTopic"]:
+            logging.info("APNs: Removing invalid/expired token %s (%s)" % (device.token, result))
+            # We remove all entries for this token (There
+            # could be multiple for different Zulip servers).
+            DeviceTokenClass.objects.filter(token=device.token, kind=DeviceTokenClass.APNS).delete()
         else:
             logging.warning("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

zerver/tests/test_push_notifications.py

@@ -390,6 +390,15 @@ class HandlePushNotificationTest(PushNotificationTest):
                 mock_info.assert_any_call(
                     "GCM: Sent %s as %s" % (token, message.id))
 
+            # Now test the unregistered case
+            mock_apns.get_notification_result.return_value = 'Unregistered'
+            apn.handle_push_notification(self.user_profile.id, missed_message)
+            for _, _, token in apns_devices:
+                mock_info.assert_any_call(
+                    "APNs: Removing invalid/expired token %s (%s)" %
+                    (token, "Unregistered"))
+            self.assertEqual(RemotePushDeviceToken.objects.filter(kind=PushDeviceToken.APNS).count(), 0)
+
     def test_end_to_end_connection_error(self) -> None:
         remote_gcm_tokens = [u'dddd']
         for token in remote_gcm_tokens:

zilencer/views.py

@@ -147,7 +147,7 @@ def remote_server_notify_push(request: HttpRequest, entity: Union[UserProfile, R
         send_android_push_notification(android_devices, gcm_payload, remote=True)
 
     if apple_devices:
-        send_apple_push_notification(user_id, apple_devices, apns_payload)
+        send_apple_push_notification(user_id, apple_devices, apns_payload, remote=True)
 
     return json_success()