diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 275f1355f5..072e2ab769 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -15,6 +15,7 @@ from typing import Any, Dict, List, Optional, SupportsInt, Tuple, Type, Union from apns2.client import APNsClient from apns2.payload import Payload as APNsPayload from django.conf import settings +from django.db import IntegrityError, transaction from django.utils.timezone import now as timezone_now from django.utils.translation import ugettext as _ from gcm import GCM @@ -345,8 +346,7 @@ def add_push_device_token(user_profile: UserProfile, token_str: bytes, kind: int, ios_app_id: Optional[str]=None) -> None: - - logging.info("New push device: %d %r %d %r", + logging.info("Registering push device: %d %r %d %r", user_profile.id, token_str, kind, ios_app_id) # If we're sending things to the push notification bouncer @@ -367,22 +367,17 @@ def add_push_device_token(user_profile: UserProfile, send_to_push_bouncer('POST', 'register', post_data) return - # If another user was previously logged in on the same device and didn't - # properly log out, the token will still be registered to the wrong account - PushDeviceToken.objects.filter(token=token_str).exclude(user=user_profile).delete() - - # Overwrite with the latest value - token, created = PushDeviceToken.objects.get_or_create(user=user_profile, - token=token_str, - defaults=dict( - kind=kind, - ios_app_id=ios_app_id)) - if not created: - logging.info("Existing push device updated.") - token.last_updated = timezone_now() - token.save(update_fields=['last_updated']) - else: - logging.info("New push device created.") + try: + with transaction.atomic(): + PushDeviceToken.objects.create( + user_id=user_profile.id, + kind=kind, + token=token_str, + ios_app_id=ios_app_id, + # last_updated is to be renamed to date_created. + last_updated=timezone_now()) + except IntegrityError: + pass def remove_push_device_token(user_profile: UserProfile, token_str: bytes, kind: int) -> None: diff --git a/zerver/migrations/0190_cleanup_pushdevicetoken.py b/zerver/migrations/0190_cleanup_pushdevicetoken.py new file mode 100644 index 0000000000..611bad8659 --- /dev/null +++ b/zerver/migrations/0190_cleanup_pushdevicetoken.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.14 on 2018-10-10 22:52 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('zerver', '0189_userprofile_add_some_emojisets'), + ] + + operations = [ + migrations.AlterField( + model_name='pushdevicetoken', + name='token', + field=models.CharField(db_index=True, max_length=4096), + ), + migrations.AlterUniqueTogether( + name='pushdevicetoken', + unique_together=set([('user', 'kind', 'token')]), + ), + ] diff --git a/zerver/models.py b/zerver/models.py index 89b10a1bb9..1edc756f77 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1039,6 +1039,9 @@ class AbstractPushDeviceToken(models.Model): # sent to us from each device: # - APNS token if kind == APNS # - GCM registration id if kind == GCM + + # TODO: last_updated should be renamed date_created, since it is + # no longer maintained as a last_updated value. last_updated = models.DateTimeField(auto_now=True) # type: datetime.datetime # [optional] Contains the app id of the device if it is an iOS device @@ -1050,7 +1053,10 @@ class AbstractPushDeviceToken(models.Model): class PushDeviceToken(AbstractPushDeviceToken): # The user who's device this is user = models.ForeignKey(UserProfile, db_index=True, on_delete=CASCADE) # type: UserProfile - token = models.CharField(max_length=4096, unique=True) # type: bytes + token = models.CharField(max_length=4096, db_index=True) # type: bytes + + class Meta: + unique_together = ("user", "kind", "token") def generate_email_token_for_stream() -> str: return generate_random_token(32) diff --git a/zilencer/migrations/0014_cleanup_pushdevicetoken.py b/zilencer/migrations/0014_cleanup_pushdevicetoken.py new file mode 100644 index 0000000000..0ea4a94b3d --- /dev/null +++ b/zilencer/migrations/0014_cleanup_pushdevicetoken.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.14 on 2018-10-10 22:52 +from __future__ import unicode_literals + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('zilencer', '0013_remove_customer_billing_user'), + ] + + operations = [ + migrations.AlterUniqueTogether( + name='remotepushdevicetoken', + unique_together=set([('server', 'user_id', 'kind', 'token')]), + ), + ] diff --git a/zilencer/models.py b/zilencer/models.py index 4cc4ea102c..cd414a1501 100644 --- a/zilencer/models.py +++ b/zilencer/models.py @@ -31,7 +31,7 @@ class RemotePushDeviceToken(AbstractPushDeviceToken): token = models.CharField(max_length=4096, db_index=True) # type: bytes class Meta: - unique_together = ("server", "token") + unique_together = ("server", "user_id", "kind", "token") def __str__(self) -> str: return "" % (self.server, self.user_id) diff --git a/zilencer/views.py b/zilencer/views.py index 8b5c5a3acc..d76eb30e21 100644 --- a/zilencer/views.py +++ b/zilencer/views.py @@ -3,6 +3,7 @@ import logging from django.core.exceptions import ValidationError from django.core.validators import validate_email, URLValidator +from django.db import IntegrityError, transaction from django.http import HttpRequest, HttpResponse from django.utils import timezone from django.utils.translation import ugettext as _, ugettext as err_ @@ -82,21 +83,18 @@ def register_remote_push_device(request: HttpRequest, entity: Union[UserProfile, validate_bouncer_token_request(entity, token, token_kind) server = cast(RemoteZulipServer, entity) - # If a user logged out on a device and failed to unregister, - # we should delete any other user associations for this token - # & RemoteServer pair - RemotePushDeviceToken.objects.filter( - token=token, kind=token_kind, server=server).exclude(user_id=user_id).delete() - - # Save or update - remote_token, created = RemotePushDeviceToken.objects.update_or_create( - user_id=user_id, - server=server, - kind=token_kind, - token=token, - defaults=dict( - ios_app_id=ios_app_id, - last_updated=timezone.now())) + try: + with transaction.atomic(): + RemotePushDeviceToken.objects.create( + user_id=user_id, + server=server, + kind=token_kind, + token=token, + ios_app_id=ios_app_id, + # last_updated is to be renamed to date_created. + last_updated=timezone.now()) + except IntegrityError: + pass return json_success()