events: Fix bug in get_recent_conversations_recipient_id.

user_profile.id was confused for user_profile.recipient_id. These bugs
are particularly sneaky as they can go undetected by tests due to ids of
objects accidentally coinciding. We add a mitigation for this class of
mistakes by shifting the Recipient.id sequence in test db.

This was introduced in dda3ff41e1.
On the rare occasion where user_profile.id would coincide with
recipient_id passed to the function, we would return the wrong value.
That is, instead of correctly returning recipient_id, we would return
sender.recipient_id - recipient id of the sender of the message, thus
possibly returning user_profile.recipient_id (if user_profile is the
sender) - exactly the situation the function wanted to avoid
with the `if recipient_id == my_recipient_id:` if. Ultimately resulting
in incorrect/malformed data in
state['raw_recent_private_conversations'].
This commit is contained in:
Mateusz Mandera 2021-02-04 18:15:38 +01:00 committed by Tim Abbott
parent b8c8ea5262
commit 90636d5e81
2 changed files with 11 additions and 1 deletions

View File

@ -1160,7 +1160,7 @@ def get_recent_conversations_recipient_id(user_profile: UserProfile,
get_recent_private_conversations would have used to record that get_recent_private_conversations would have used to record that
message in its data structure. message in its data structure.
""" """
my_recipient_id = user_profile.id my_recipient_id = user_profile.recipient_id
if recipient_id == my_recipient_id: if recipient_id == my_recipient_id:
return UserProfile.objects.values_list('recipient_id', flat=True).get(id=sender_id) return UserProfile.objects.values_list('recipient_id', flat=True).get(id=sender_id)
return recipient_id return recipient_id

View File

@ -11,6 +11,7 @@ from django.conf import settings
from django.contrib.sessions.models import Session from django.contrib.sessions.models import Session
from django.core.management import call_command from django.core.management import call_command
from django.core.management.base import BaseCommand, CommandParser from django.core.management.base import BaseCommand, CommandParser
from django.db import connection
from django.db.models import F, Max from django.db.models import F, Max
from django.utils.timezone import now as timezone_now from django.utils.timezone import now as timezone_now
from django.utils.timezone import timedelta as timezone_timedelta from django.utils.timezone import timedelta as timezone_timedelta
@ -252,6 +253,15 @@ class Command(BaseCommand):
if options["test_suite"]: if options["test_suite"]:
random.seed(0) random.seed(0)
with connection.cursor() as cursor:
# Sometimes bugs relating to confusing recipient.id for recipient.type_id
# or <object>.id for <object>.recipient_id remain undiscovered by the test suite
# due to these numbers happening to coincide in such a way that it makes tests
# accidentally pass. By bumping the Recipient.id sequence by a large enough number,
# we can have those ids in a completely different range of values than object ids,
# eliminatng the possibility of such coincidences.
cursor.execute("SELECT setval('zerver_recipient_id_seq', 100)")
# If max_topics is not set, we set it proportional to the # If max_topics is not set, we set it proportional to the
# number of messages. # number of messages.
if options["max_topics"] is None: if options["max_topics"] is None: