diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 09e683fa82..27d7033ba2 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -15,6 +15,7 @@ import orjson from django.conf import settings from django.db import transaction from django.db.models import F, Q +from django.db.models.functions import Lower from django.utils.timezone import now as timezone_now from django.utils.translation import gettext as _ from django.utils.translation import override as override_language @@ -360,8 +361,8 @@ def send_apple_push_notification( ) # We remove all entries for this token (There # could be multiple for different Zulip servers). - DeviceTokenClass._default_manager.filter( - token=device.token, kind=DeviceTokenClass.APNS + DeviceTokenClass._default_manager.alias(lower_token=Lower("token")).filter( + lower_token=device.token.lower(), kind=DeviceTokenClass.APNS ).delete() else: logger.warning( @@ -624,8 +625,9 @@ def send_notifications_to_bouncer( PushDeviceToken.objects.filter( kind=PushDeviceToken.FCM, token__in=android_deleted_devices ).delete() - PushDeviceToken.objects.filter( - kind=PushDeviceToken.APNS, token__in=apple_deleted_devices + PushDeviceToken.objects.alias(lower_token=Lower("token")).filter( + kind=PushDeviceToken.APNS, + lower_token__in=[token.lower() for token in apple_deleted_devices], ).delete() total_android_devices, total_apple_devices = ( @@ -723,7 +725,13 @@ def add_push_device_token( def remove_push_device_token(user_profile: UserProfile, token_str: str, kind: int) -> None: try: - token = PushDeviceToken.objects.get(token=token_str, kind=kind, user=user_profile) + if kind == PushDeviceToken.APNS: + token_str = token_str.lower() + token: PushDeviceToken = PushDeviceToken.objects.alias(lower_token=Lower("token")).get( + lower_token=token_str, kind=kind, user=user_profile + ) + else: + token = PushDeviceToken.objects.get(token=token_str, kind=kind, user=user_profile) token.delete() except PushDeviceToken.DoesNotExist: # If we are using bouncer, don't raise the exception. It will diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index 10a43b03f4..ca0348b221 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -2757,7 +2757,7 @@ class PushNotificationTestCase(BouncerTestCase): apns_context.loop.close() def setup_apns_tokens(self) -> None: - self.tokens = [("aaaa", "org.zulip.Zulip"), ("bbbb", "com.zulip.flutter")] + self.tokens = [("aAAa", "org.zulip.Zulip"), ("bBBb", "com.zulip.flutter")] for token, appid in self.tokens: PushDeviceToken.objects.create( kind=PushDeviceToken.APNS, @@ -2767,8 +2767,8 @@ class PushNotificationTestCase(BouncerTestCase): ) self.remote_tokens = [ - ("cccc", "dddd", "org.zulip.Zulip"), - ("eeee", "ffff", "com.zulip.flutter"), + ("cCCc", "dDDd", "org.zulip.Zulip"), + ("eEEe", "fFFf", "com.zulip.flutter"), ] for id_token, uuid_token, appid in self.remote_tokens: # We want to set up both types of RemotePushDeviceToken here: diff --git a/zerver/migrations/0740_pushdevicetoken_apns_case_insensitive.py b/zerver/migrations/0740_pushdevicetoken_apns_case_insensitive.py new file mode 100644 index 0000000000..31bf018e33 --- /dev/null +++ b/zerver/migrations/0740_pushdevicetoken_apns_case_insensitive.py @@ -0,0 +1,71 @@ +import django.db.models.functions.text +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0739_alter_realm_can_set_delete_message_policy_group"), + ] + + operations = [ + # Update the last_updated to the max for any set of (user_id, kind=1, lower(token)) + migrations.RunSQL( + """ + WITH dups AS ( + SELECT user_id, kind, LOWER(token) AS token, MAX(last_updated) AS max_last_updated + FROM zerver_pushdevicetoken + WHERE kind = 1 + GROUP BY user_id, kind, LOWER(token) + HAVING COUNT(*) > 1 + ) + UPDATE zerver_pushdevicetoken + SET last_updated = dups.max_last_updated + FROM dups + WHERE zerver_pushdevicetoken.user_id = dups.user_id + AND zerver_pushdevicetoken.kind = dups.kind + AND LOWER(zerver_pushdevicetoken.token) = dups.token + """ + ), + # And then delete all but the first of each of those sets + migrations.RunSQL( + """ + WITH dups AS ( + SELECT user_id, kind, LOWER(token) AS token, MIN(id) AS min_id + FROM zerver_pushdevicetoken + WHERE kind = 1 + GROUP BY user_id, kind, LOWER(token) + HAVING COUNT(*) > 1 + ) + DELETE FROM zerver_pushdevicetoken + USING dups + WHERE zerver_pushdevicetoken.user_id = dups.user_id + AND zerver_pushdevicetoken.kind = dups.kind + AND LOWER(zerver_pushdevicetoken.token) = dups.token + AND zerver_pushdevicetoken.id != dups.min_id + """ + ), + migrations.AddConstraint( + model_name="pushdevicetoken", + constraint=models.UniqueConstraint( + models.F("user_id"), + models.F("kind"), + django.db.models.functions.text.Lower(models.F("token")), + condition=models.Q(("kind", 1)), + name="zerver_pushdevicetoken_apns_user_kind_token", + ), + ), + migrations.AddConstraint( + model_name="pushdevicetoken", + constraint=models.UniqueConstraint( + models.F("user_id"), + models.F("kind"), + models.F("token"), + condition=models.Q(("kind", 2)), + name="zerver_pushdevicetoken_fcm_user_kind_token", + ), + ), + migrations.AlterUniqueTogether( + name="pushdevicetoken", + unique_together=set(), + ), + ] diff --git a/zerver/models/push_notifications.py b/zerver/models/push_notifications.py index eaf950d29f..ca504ee136 100644 --- a/zerver/models/push_notifications.py +++ b/zerver/models/push_notifications.py @@ -1,7 +1,8 @@ from typing import Literal from django.db import models -from django.db.models import CASCADE, UniqueConstraint +from django.db.models import CASCADE, F, Q, UniqueConstraint +from django.db.models.functions import Lower from zerver.lib.exceptions import ( InvalidBouncerPublicKeyError, @@ -46,7 +47,22 @@ class PushDeviceToken(AbstractPushDeviceToken): user = models.ForeignKey(UserProfile, db_index=True, on_delete=CASCADE) class Meta: - unique_together = ("user", "kind", "token") + constraints = [ + models.UniqueConstraint( + "user_id", + "kind", + Lower(F("token")), + name="zerver_pushdevicetoken_apns_user_kind_token", + condition=Q(kind=AbstractPushDeviceToken.APNS), + ), + models.UniqueConstraint( + "user_id", + "kind", + "token", + name="zerver_pushdevicetoken_fcm_user_kind_token", + condition=Q(kind=AbstractPushDeviceToken.FCM), + ), + ] class AbstractPushDevice(models.Model): diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index 8a78032e9a..8c63f53789 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -82,7 +82,7 @@ from zilencer.auth import ( generate_registration_transfer_verification_secret, ) from zilencer.models import RemoteZulipServerAuditLog -from zilencer.views import DevicesToCleanUpDict +from zilencer.views import DevicesToCleanUpDict, get_deleted_devices if settings.ZILENCER_ENABLED: from zilencer.models import ( @@ -1157,7 +1157,7 @@ class PushBouncerNotificationTest(BouncerTestCase): endpoints: list[tuple[str, str, int, Mapping[str, str]]] = [ ( "/json/users/me/apns_device_token", - "c0ffee", + "c0fFeE", RemotePushDeviceToken.APNS, {"appid": "org.zulip.Zulip"}, ), @@ -3224,3 +3224,159 @@ class TestUserPushIdentityCompat(ZulipTestCase): # An integer can't be equal to an instance of the class. self.assertNotEqual(user_identity_a, 1) + + +class TestDeletedDevices(BouncerTestCase): + def test_delete_android(self) -> None: + hamlet = self.example_user("hamlet") + server = self.server + + # Android tokens are case-sensitive, so this is just 4 different tokens. + for token in ["aaaa", "aaAA", "bbbb", "BBBB"]: + RemotePushDeviceToken.objects.create( + kind=RemotePushDeviceToken.FCM, + server=server, + user_id=hamlet.id, + token=token, + ) + + self.assertEqual( + DevicesToCleanUpDict(android_devices=[], apple_devices=[]), + get_deleted_devices( + UserPushIdentityCompat(user_id=hamlet.id), + server, + android_devices=["aaaa", "aaAA", "bbbb", "BBBB"], + apple_devices=[], + ), + ) + + self.assertEqual( + DevicesToCleanUpDict(android_devices=[], apple_devices=[]), + get_deleted_devices( + UserPushIdentityCompat(user_id=hamlet.id), + server, + android_devices=["aaAA", "bbbb"], + apple_devices=[], + ), + ) + + self.assertEqual( + DevicesToCleanUpDict(android_devices=["more", "other"], apple_devices=[]), + get_deleted_devices( + UserPushIdentityCompat(user_id=hamlet.id), + server, + android_devices=["aaAA", "bbbb", "other", "more"], + apple_devices=[], + ), + ) + + # Add some tokens which have both user-id and user-UUIDs. + for token in ["cccc", "dddd"]: + RemotePushDeviceToken.objects.create( + kind=RemotePushDeviceToken.FCM, + server=server, + user_id=hamlet.id, + user_uuid=hamlet.uuid, + token=token, + ) + self.assertEqual( + DevicesToCleanUpDict(android_devices=["more", "other"], apple_devices=[]), + get_deleted_devices( + UserPushIdentityCompat(user_id=hamlet.id, user_uuid=str(hamlet.uuid)), + server, + android_devices=["aaAA", "bbbb", "cccc", "other", "more"], + apple_devices=[], + ), + ) + + def test_delete_apple(self) -> None: + hamlet = self.example_user("hamlet") + server = self.server + + # APNs tokens are case-preserving but case-insensitive -- but + # old versions of the server did not know that. We therefore + # must be able to correctly handle getting multiple cases of + # the same token, and always responding with the case that the + # caller provided. + for token in ["aaaa", "bBBb", "CCCC"]: + RemotePushDeviceToken.objects.create( + kind=RemotePushDeviceToken.APNS, + server=server, + user_id=hamlet.id, + token=token, + ) + + # Simple case -- remote server and bouncer agree on tokens and + # their case. + self.assertEqual( + DevicesToCleanUpDict(android_devices=[], apple_devices=[]), + get_deleted_devices( + UserPushIdentityCompat(user_id=hamlet.id), + server, + android_devices=[], + apple_devices=["aaaa", "bBBb", "CCCC"], + ), + ) + + # Same, but with extra tokens present + self.assertEqual( + DevicesToCleanUpDict(android_devices=[], apple_devices=["cafe", "ffff"]), + get_deleted_devices( + UserPushIdentityCompat(user_id=hamlet.id), + server, + android_devices=[], + apple_devices=["aaaa", "bBBb", "CCCC", "ffff", "cafe"], + ), + ) + + # The remote server has a token in multiple cases, none of + # which potentially agree with our case. It will tell the + # remote server to remove all but the first case it + # encountered. + self.assertEqual( + DevicesToCleanUpDict(android_devices=[], apple_devices=["AAaa"]), + get_deleted_devices( + UserPushIdentityCompat(user_id=hamlet.id), + server, + android_devices=[], + apple_devices=["AAAA", "AAaa", "BBBB"], + ), + ) + + # Add some tokens which have both user-id and user-UUIDs. + for token in ["dddd", "EeeE"]: + RemotePushDeviceToken.objects.create( + kind=RemotePushDeviceToken.APNS, + server=server, + user_id=hamlet.id, + user_uuid=hamlet.uuid, + token=token, + ) + self.assertEqual( + DevicesToCleanUpDict( + android_devices=[], + apple_devices=["AAaa", "EEEE", "more", "other"], + ), + get_deleted_devices( + UserPushIdentityCompat(user_id=hamlet.id, user_uuid=str(hamlet.uuid)), + server, + android_devices=[], + apple_devices=["AAAA", "AAaa", "BBBB", "DDDD", "eeEE", "EEEE", "other", "more"], + ), + ) + + # It should not be possible to have a token be passed in more + # than once with the same case, but in such cases we should + # not return it in the to-clean-up list. + self.assertEqual( + DevicesToCleanUpDict( + android_devices=[], + apple_devices=["MORE"], + ), + get_deleted_devices( + UserPushIdentityCompat(user_id=hamlet.id, user_uuid=str(hamlet.uuid)), + server, + android_devices=[], + apple_devices=["AAAA", "AAAA", "MORE", "MORE"], + ), + ) diff --git a/zilencer/migrations/0067_remotepushdevicetoken_apns_case_insensitive.py b/zilencer/migrations/0067_remotepushdevicetoken_apns_case_insensitive.py new file mode 100644 index 0000000000..00c0650517 --- /dev/null +++ b/zilencer/migrations/0067_remotepushdevicetoken_apns_case_insensitive.py @@ -0,0 +1,141 @@ +import django.db.models.functions.text +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zilencer", "0066_alter_remotepushdevice_token_kind"), + ] + + operations = [ + # This parallels zerver/migrations/0740 but must account for + # the user_id / user_uuid split. + migrations.RunSQL( + """ + WITH dups AS ( + SELECT server_id, user_id, kind, LOWER(token) AS token, MAX(last_updated) AS max_last_updated + FROM zilencer_remotepushdevicetoken + WHERE kind = 1 + AND user_uuid IS NULL + GROUP BY server_id, user_id, kind, LOWER(token) + HAVING COUNT(*) > 1 + ) + UPDATE zilencer_remotepushdevicetoken + SET last_updated = dups.max_last_updated + FROM dups + WHERE zilencer_remotepushdevicetoken.server_id = dups.server_id + AND zilencer_remotepushdevicetoken.user_id = dups.user_id + AND zilencer_remotepushdevicetoken.user_uuid IS NULL + AND zilencer_remotepushdevicetoken.kind = dups.kind + AND LOWER(zilencer_remotepushdevicetoken.token) = dups.token + """ + ), + migrations.RunSQL( + """ + WITH dups AS ( + SELECT server_id, user_id, kind, LOWER(token) AS token, MIN(id) AS min_id + FROM zilencer_remotepushdevicetoken + WHERE kind = 1 + AND user_uuid IS NULL + GROUP BY server_id, user_id, kind, LOWER(token) + HAVING COUNT(*) > 1 + ) + DELETE FROM zilencer_remotepushdevicetoken + USING dups + WHERE zilencer_remotepushdevicetoken.server_id = dups.server_id + AND zilencer_remotepushdevicetoken.user_id = dups.user_id + AND zilencer_remotepushdevicetoken.user_uuid IS NULL + AND zilencer_remotepushdevicetoken.kind = dups.kind + AND LOWER(zilencer_remotepushdevicetoken.token) = dups.token + AND zilencer_remotepushdevicetoken.id != dups.min_id + """ + ), + migrations.AddConstraint( + model_name="remotepushdevicetoken", + constraint=models.UniqueConstraint( + models.F("server_id"), + models.F("user_id"), + models.F("kind"), + django.db.models.functions.text.Lower(models.F("token")), + condition=models.Q(("kind", 1)), + name="zilencer_remotepushdevicetoken_apns_server_user_id_kind_token", + ), + ), + migrations.AddConstraint( + model_name="remotepushdevicetoken", + constraint=models.UniqueConstraint( + models.F("server_id"), + models.F("user_id"), + models.F("kind"), + models.F("token"), + condition=models.Q(("kind", 2)), + name="zilencer_remotepushdevicetoken_fcm_server_user_id_kind_token", + ), + ), + migrations.RunSQL( + """ + WITH dups AS ( + SELECT server_id, user_uuid, kind, LOWER(token) AS token, MAX(last_updated) AS max_last_updated + FROM zilencer_remotepushdevicetoken + WHERE kind = 1 + AND user_id IS NULL + GROUP BY server_id, user_uuid, kind, LOWER(token) + HAVING COUNT(*) > 1 + ) + UPDATE zilencer_remotepushdevicetoken + SET last_updated = dups.max_last_updated + FROM dups + WHERE zilencer_remotepushdevicetoken.server_id = dups.server_id + AND zilencer_remotepushdevicetoken.user_uuid = dups.user_uuid + AND zilencer_remotepushdevicetoken.user_id IS NULL + AND zilencer_remotepushdevicetoken.kind = dups.kind + AND LOWER(zilencer_remotepushdevicetoken.token) = dups.token + """ + ), + migrations.RunSQL( + """ + WITH dups AS ( + SELECT server_id, user_uuid, kind, LOWER(token) AS token, MIN(id) AS min_id + FROM zilencer_remotepushdevicetoken + WHERE kind = 1 + AND user_id IS NULL + GROUP BY server_id, user_uuid, kind, LOWER(token) + HAVING COUNT(*) > 1 + ) + DELETE FROM zilencer_remotepushdevicetoken + USING dups + WHERE zilencer_remotepushdevicetoken.server_id = dups.server_id + AND zilencer_remotepushdevicetoken.user_uuid = dups.user_uuid + AND zilencer_remotepushdevicetoken.user_id IS NULL + AND zilencer_remotepushdevicetoken.kind = dups.kind + AND LOWER(zilencer_remotepushdevicetoken.token) = dups.token + AND zilencer_remotepushdevicetoken.id != dups.min_id + """ + ), + migrations.AddConstraint( + model_name="remotepushdevicetoken", + constraint=models.UniqueConstraint( + models.F("server_id"), + models.F("user_uuid"), + models.F("kind"), + django.db.models.functions.text.Lower(models.F("token")), + condition=models.Q(("kind", 1)), + name="zilencer_remotepushdevicetoken_apns_server_uuid_kind_token", + ), + ), + migrations.AddConstraint( + model_name="remotepushdevicetoken", + constraint=models.UniqueConstraint( + models.F("server_id"), + models.F("user_uuid"), + models.F("kind"), + models.F("token"), + condition=models.Q(("kind", 2)), + name="zilencer_remotepushdevicetoken_fcm_server_uuid_kind_token", + ), + ), + migrations.AlterUniqueTogether( + name="remotepushdevicetoken", + unique_together=set(), + ), + ] diff --git a/zilencer/models.py b/zilencer/models.py index a3dd5088c7..31b86e7d9b 100644 --- a/zilencer/models.py +++ b/zilencer/models.py @@ -4,7 +4,8 @@ from datetime import datetime, timedelta from django.conf import settings from django.core.exceptions import ValidationError from django.db import models -from django.db.models import Max, Q, QuerySet, UniqueConstraint +from django.db.models import F, Max, Q, QuerySet, UniqueConstraint +from django.db.models.functions import Lower from django.utils.timezone import now as timezone_now from typing_extensions import override @@ -116,13 +117,43 @@ class RemotePushDeviceToken(AbstractPushDeviceToken): remote_realm = models.ForeignKey("RemoteRealm", on_delete=models.SET_NULL, null=True) class Meta: - unique_together = [ + constraints = [ # These indexes rely on the property that in Postgres, # NULL != NULL in the context of unique indexes, so multiple # rows with the same values in these columns can exist # if one of them is NULL. - ("server", "user_id", "kind", "token"), - ("server", "user_uuid", "kind", "token"), + models.UniqueConstraint( + "server_id", + "user_id", + "kind", + Lower(F("token")), + name="zilencer_remotepushdevicetoken_apns_server_user_id_kind_token", + condition=Q(kind=1), + ), + models.UniqueConstraint( + "server_id", + "user_id", + "kind", + "token", + name="zilencer_remotepushdevicetoken_fcm_server_user_id_kind_token", + condition=Q(kind=2), + ), + models.UniqueConstraint( + "server_id", + "user_uuid", + "kind", + Lower(F("token")), + name="zilencer_remotepushdevicetoken_apns_server_uuid_kind_token", + condition=Q(kind=1), + ), + models.UniqueConstraint( + "server_id", + "user_uuid", + "kind", + "token", + name="zilencer_remotepushdevicetoken_fcm_server_uuid_kind_token", + condition=Q(kind=2), + ), ] @override diff --git a/zilencer/views.py b/zilencer/views.py index 7fd3a3424b..2b36bbf0ec 100644 --- a/zilencer/views.py +++ b/zilencer/views.py @@ -12,8 +12,9 @@ from django.conf import settings from django.core.exceptions import ValidationError from django.core.validators import URLValidator, validate_email from django.db import IntegrityError, transaction -from django.db.models import Model +from django.db.models import Model, QuerySet from django.db.models.constants import OnConflict +from django.db.models.functions import Lower from django.http import HttpRequest, HttpResponse from django.utils.crypto import constant_time_compare, get_random_string from django.utils.timezone import now as timezone_now @@ -424,6 +425,26 @@ def check_transfer_challenge_response_secret_not_prepared(response: requests.Res return secret_not_prepared +def get_remote_push_device_token( + *, + server: RemoteZulipServer, + token: str, + kind: int, +) -> QuerySet[RemotePushDeviceToken]: + if kind == RemotePushDeviceToken.APNS: + return RemotePushDeviceToken.objects.alias(lower_token=Lower("token")).filter( + server=server, + lower_token=token.lower(), + kind=kind, + ) + else: + return RemotePushDeviceToken.objects.filter( + server=server, + token=token, + kind=kind, + ) + + @typed_endpoint def register_remote_push_device( request: HttpRequest, @@ -446,9 +467,11 @@ def register_remote_push_device( kwargs: dict[str, object] = {"user_uuid": user_uuid, "user_id": None} # Delete pre-existing user_id registration for this user+device to avoid # duplication. Further down, uuid registration will be created. - RemotePushDeviceToken.objects.filter( - server=server, token=token, kind=token_kind, user_id=user_id - ).delete() + get_remote_push_device_token( + server=server, + token=token, + kind=token_kind, + ).filter(user_id=user_id).delete() else: # One of these is None, so these kwargs will lead to a proper registration # of either user_id or user_uuid type @@ -622,9 +645,11 @@ def unregister_remote_push_device( update_remote_realm_last_request_datetime_helper(request, server, realm_uuid, user_uuid) - (num_deleted, ignored) = RemotePushDeviceToken.objects.filter( - user_identity.filter_q(), token=token, kind=token_kind, server=server - ).delete() + (num_deleted, ignored) = ( + get_remote_push_device_token(token=token, kind=token_kind, server=server) + .filter(user_identity.filter_q()) + .delete() + ) if num_deleted == 0: raise JsonableError(err_("Token does not exist")) @@ -683,7 +708,10 @@ def delete_duplicate_registrations( assert len({registration.kind for registration in registrations}) == 1 kind = registrations[0].kind - tokens_counter = Counter(device.token for device in registrations) + if kind == RemotePushDeviceToken.APNS: + tokens_counter = Counter(device.token.lower() for device in registrations) + else: + tokens_counter = Counter(device.token for device in registrations) tokens_to_deduplicate = [] for key in tokens_counter: @@ -757,11 +785,12 @@ def remote_server_send_test_notification( update_remote_realm_last_request_datetime_helper(request, server, realm_uuid, user_uuid) - try: - device = RemotePushDeviceToken.objects.get( - user_identity.filter_q(), token=token, kind=token_kind, server=server - ) - except RemotePushDeviceToken.DoesNotExist: + device = ( + get_remote_push_device_token(token=token, kind=token_kind, server=server) + .filter(user_identity.filter_q()) + .first() + ) + if device is None: raise InvalidRemotePushDeviceTokenError send_test_push_notification_directly_to_devices( @@ -1046,16 +1075,38 @@ def get_deleted_devices( kind=RemotePushDeviceToken.FCM, server=server, ).values_list("token", flat=True) - apple_devices_we_have = RemotePushDeviceToken.objects.filter( - user_identity.filter_q(), - token__in=apple_devices, - kind=RemotePushDeviceToken.APNS, - server=server, - ).values_list("token", flat=True) + + # APNS tokens are case-insensitive -- but the remote server may + # not know that yet. As such, we perform our local lookups + # case-insensitively, returning the exact case the remote server + # used, and also return all-but-one of any case duplicates that + # the remote server passed us. + canonical_case = {} + apns_token_to_remove = set() + for token in apple_devices: + if token.lower() not in canonical_case: + canonical_case[token.lower()] = token + elif canonical_case[token.lower()] == token: + # Be careful to skip if identical-case tokens somehow show up more than once + pass + else: + apns_token_to_remove.add(token) + apple_devices_we_have = ( + RemotePushDeviceToken.objects.annotate(lower_token=Lower("token")) + .filter( + user_identity.filter_q(), + lower_token__in=canonical_case.keys(), + kind=RemotePushDeviceToken.APNS, + server=server, + ) + .values_list("lower_token", flat=True) + ) + for token_to_remove in set(canonical_case.keys()) - set(apple_devices_we_have): + apns_token_to_remove.add(canonical_case[token_to_remove]) return DevicesToCleanUpDict( - android_devices=list(set(android_devices) - set(android_devices_we_have)), - apple_devices=list(set(apple_devices) - set(apple_devices_we_have)), + android_devices=sorted(set(android_devices) - set(android_devices_we_have)), + apple_devices=sorted(apns_token_to_remove), )