From 936c2b54cb3b2d62a8379108191f97bba1083c2d Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Fri, 12 Apr 2024 16:34:37 +0000 Subject: [PATCH] push_notifications: Use ignore_conflicts, over catching IntegrityError. The IntegrityError shows up in the database logs, which looks unnecessarily concerning. Use `ON CONFLICT IGNORE` to mark this as expected, especially since the return value is never used. --- zerver/lib/push_notifications.py | 59 +++++++++++++++----------------- zilencer/views.py | 13 +++---- 2 files changed, 35 insertions(+), 37 deletions(-) diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 85bf6cb06f..e32cd510b7 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -26,7 +26,7 @@ import gcm import lxml.html import orjson from django.conf import settings -from django.db import IntegrityError, transaction +from django.db import transaction from django.db.models import F, Q from django.utils.timezone import now as timezone_now from django.utils.translation import gettext as _ @@ -687,7 +687,7 @@ def send_notifications_to_bouncer( def add_push_device_token( user_profile: UserProfile, token_str: str, kind: int, ios_app_id: Optional[str] = None -) -> PushDeviceToken: +) -> None: logger.info( "Registering push device: %d %r %d %r", user_profile.id, token_str, kind, ios_app_id ) @@ -697,45 +697,42 @@ def add_push_device_token( # These can be used to discern whether the user has any mobile # devices configured, and is also where we will store encryption # keys for mobile push notifications. - try: - with transaction.atomic(): - token = PushDeviceToken.objects.create( + PushDeviceToken.objects.bulk_create( + [ + PushDeviceToken( user_id=user_profile.id, - kind=kind, token=token_str, + kind=kind, ios_app_id=ios_app_id, # last_updated is to be renamed to date_created. last_updated=timezone_now(), - ) - except IntegrityError: - token = PushDeviceToken.objects.get( - user_id=user_profile.id, - kind=kind, - token=token_str, - ) + ), + ], + ignore_conflicts=True, + ) + + if not uses_notification_bouncer(): + return # If we're sending things to the push notification bouncer # register this user with them here - if uses_notification_bouncer(): - post_data = { - "server_uuid": settings.ZULIP_ORG_ID, - "user_uuid": str(user_profile.uuid), - "realm_uuid": str(user_profile.realm.uuid), - # user_id is sent so that the bouncer can delete any pre-existing registrations - # for this user+device to avoid duplication upon adding the uuid registration. - "user_id": str(user_profile.id), - "token": token_str, - "token_kind": kind, - } + post_data = { + "server_uuid": settings.ZULIP_ORG_ID, + "user_uuid": str(user_profile.uuid), + "realm_uuid": str(user_profile.realm.uuid), + # user_id is sent so that the bouncer can delete any pre-existing registrations + # for this user+device to avoid duplication upon adding the uuid registration. + "user_id": str(user_profile.id), + "token": token_str, + "token_kind": kind, + } - if kind == PushDeviceToken.APNS: - post_data["ios_app_id"] = ios_app_id + if kind == PushDeviceToken.APNS: + post_data["ios_app_id"] = ios_app_id - logger.info("Sending new push device to bouncer: %r", post_data) - # Calls zilencer.views.register_remote_push_device - send_to_push_bouncer("POST", "push/register", post_data) - - return token + logger.info("Sending new push device to bouncer: %r", post_data) + # Calls zilencer.views.register_remote_push_device + send_to_push_bouncer("POST", "push/register", post_data) def remove_push_device_token(user_profile: UserProfile, token_str: str, kind: int) -> None: diff --git a/zilencer/views.py b/zilencer/views.py index 602df1ea23..3eea3a9cd7 100644 --- a/zilencer/views.py +++ b/zilencer/views.py @@ -260,9 +260,9 @@ def register_remote_push_device( remote_realm.last_request_datetime = timezone_now() remote_realm.save(update_fields=["last_request_datetime"]) - try: - with transaction.atomic(): - RemotePushDeviceToken.objects.create( + RemotePushDeviceToken.objects.bulk_create( + [ + RemotePushDeviceToken( server=server, kind=token_kind, token=token, @@ -270,9 +270,10 @@ def register_remote_push_device( # last_updated is to be renamed to date_created. last_updated=timezone_now(), **kwargs, - ) - except IntegrityError: - pass + ), + ], + ignore_conflicts=True, + ) return json_success(request)