paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-10-25 09:03:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

presence: Support null values in UserPresence.

Browse Source
This commit is contained in:
Mateusz Mandera
2023-04-08 15:52:48 +02:00
committed by Tim Abbott
parent 0d79f6dd27
commit a9f40a64fd
6 changed files with 151 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
zerver/actions/presence.py
View File

@@ -5,7 +5,10 @@ from django.conf import settings
from django.db import transaction
from zerver.actions.user_activity import update_user_activity_interval
from zerver.lib.presence import format_legacy_presence_dict
from zerver.lib.presence import (
format_legacy_presence_dict,
user_presence_datetime_with_date_joined_default,
)
from zerver.lib.queue import queue_json_publish
from zerver.lib.timestamp import datetime_to_timestamp
from zerver.models import Client, UserPresence, UserProfile, active_user_ids, get_client
@@ -42,12 +45,17 @@ def send_presence_changed(
# stop sending them at all.
return
last_active_time = user_presence_datetime_with_date_joined_default(
presence.last_active_time, user_profile.date_joined
)
last_connected_time = user_presence_datetime_with_date_joined_default(
presence.last_connected_time, user_profile.date_joined
)
# The mobile app handles these events so we need to use the old format.
# The format of the event should also account for the slim_presence
# API parameter when this becomes possible in the future.
presence_dict = format_legacy_presence_dict(
presence.last_active_time, presence.last_connected_time
)
presence_dict = format_legacy_presence_dict(last_active_time, last_connected_time)
event = dict(
type="presence",
email=user_profile.email,
@@ -81,37 +89,34 @@ def do_update_user_presence(
) -> None:
client = consolidate_client(client)
# TODO: While we probably DO want creating an account to
# automatically create a first `UserPresence` object with
# last_connected_time and last_active_time as the current time,
# our presence tests don't understand this, and it'd be perhaps
# wrong for some cases of account creation via the API. So we may
# want a "never" value here as the default.
# If the user doesn't have a UserPresence row yet, we create one with
# sensible defaults. If we're getting a presence update, clearly the user
# at least connected, so last_connected_time should be set. last_active_time
# will depend on whether the status sent is idle or active.
defaults = dict(
# Given that these are defaults for creation of a UserPresence row
# if one doesn't yet exist, the most sensible way to do this
# is to set both last_active_time and last_connected_time
# to log_time.
last_active_time=log_time,
last_active_time=None,
last_connected_time=log_time,
realm_id=user_profile.realm_id,
)
if status == UserPresence.LEGACY_STATUS_IDLE_INT:
# If the presence entry for the user is just to be created, and
# we want it to be created as idle, then there needs to be an appropriate
# offset between last_active_time and last_connected_time, since that's
# what the event-sending system calculates the status based on, via
# format_legacy_presence_dict.
defaults["last_active_time"] = log_time - datetime.timedelta(
seconds=settings.PRESENCE_LEGACY_EVENT_OFFSET_FOR_ACTIVITY_SECONDS + 1
)
if status == UserPresence.LEGACY_STATUS_ACTIVE_INT:
defaults["last_active_time"] = log_time
(presence, created) = UserPresence.objects.get_or_create(
user_profile=user_profile,
defaults=defaults,
)
time_since_last_active = log_time - presence.last_active_time
if presence.last_active_time is not None:
time_since_last_active = log_time - presence.last_active_time
else:
# The user was never active, so let's consider this large to go over any thresholds
# we may have.
time_since_last_active = datetime.timedelta(days=1)
if presence.last_connected_time is not None:
time_since_last_connected = log_time - presence.last_connected_time
else:
# Same approach as above.
time_since_last_connected = datetime.timedelta(days=1)
assert (3 * settings.PRESENCE_PING_INTERVAL_SECS + 20) <= settings.OFFLINE_THRESHOLD_SECS
now_online = time_since_last_active > datetime.timedelta(
@@ -136,7 +141,7 @@ def do_update_user_presence(
# times per minute with multiple connected browser windows.
# We also need to be careful not to wrongly "update" the timestamp if we actually already
# have newer presence than the reported log_time.
if not created and log_time - presence.last_connected_time > datetime.timedelta(
if not created and time_since_last_connected > datetime.timedelta(
seconds=settings.PRESENCE_UPDATE_MIN_FREQ_SECONDS
):
presence.last_connected_time = log_time
@@ -149,7 +154,7 @@ def do_update_user_presence(
):
presence.last_active_time = log_time
update_fields.append("last_active_time")
if log_time > presence.last_connected_time:
if presence.last_connected_time is None or log_time > presence.last_connected_time:
# Update last_connected_time as well to ensure
# last_connected_time >= last_active_time.
presence.last_connected_time = log_time

36
zerver/lib/presence.py
View File

@@ -1,7 +1,7 @@
import datetime
import time
from collections import defaultdict
from typing import Any, Dict, Mapping, Sequence, Set
from typing import Any, Dict, Mapping, Optional, Sequence, Set
from django.conf import settings
from django.utils.timezone import now as timezone_now
@@ -26,15 +26,43 @@ def get_presence_dicts_for_rows(
for presence_row in all_rows:
user_key = get_user_key(presence_row)
last_active_time = user_presence_datetime_with_date_joined_default(
presence_row["last_active_time"], presence_row["user_profile__date_joined"]
)
last_connected_time = user_presence_datetime_with_date_joined_default(
presence_row["last_connected_time"], presence_row["user_profile__date_joined"]
)
info = get_user_presence_info(
presence_row["last_active_time"],
presence_row["last_connected_time"],
last_active_time,
last_connected_time,
)
user_statuses[user_key] = info
return user_statuses
def user_presence_datetime_with_date_joined_default(
dt: Optional[datetime.datetime], date_joined: datetime.datetime
) -> datetime.datetime:
"""
Our data models support UserPresence objects not having None
values for last_active_time/last_connected_time. The legacy API
however has always sent timestamps, so for backward
compatibility we cannot send such values through the API and need
to default to a sane datetime.
This helper functions expects to take a last_active_time or
last_connected_time value and the date_joined of the user, which
will serve as the default value if the first argument is None.
"""
if dt is None:
return date_joined
return dt
def get_modern_user_presence_info(
last_active_time: datetime.datetime, last_connected_time: datetime.datetime
) -> Dict[str, Any]:
@@ -110,6 +138,7 @@ def get_presence_for_user(
"user_profile__email",
"user_profile_id",
"user_profile__enable_offline_push_notifications",
"user_profile__date_joined",
)
presence_rows = list(query)
@@ -136,6 +165,7 @@ def get_presence_dict_by_realm(
"user_profile__email",
"user_profile_id",
"user_profile__enable_offline_push_notifications",
"user_profile__date_joined",
)
presence_rows = list(query)

8
zerver/migrations/0443_userpresence_new_table_schema.py
View File

@@ -113,11 +113,15 @@ class Migration(migrations.Migration):
),
(
"last_connected_time",
models.DateTimeField(db_index=True, default=django.utils.timezone.now),
models.DateTimeField(
db_index=True, default=django.utils.timezone.now, null=True
),
),
(
"last_active_time",
models.DateTimeField(db_index=True, default=django.utils.timezone.now),
models.DateTimeField(
db_index=True, default=django.utils.timezone.now, null=True
),
),
(
"realm",

4
zerver/models.py
View File

@@ -4120,11 +4120,11 @@ class UserPresence(models.Model):
# The last time the user had a client connected to Zulip,
# including idle clients where the user hasn't interacted with the
# system recently (and thus might be AFK).
last_connected_time = models.DateTimeField(default=timezone_now, db_index=True)
last_connected_time = models.DateTimeField(default=timezone_now, db_index=True, null=True)
# The last time a client connected to Zulip reported that the user
# was actually present (E.g. via focusing a browser window or
# interacting with a computer running the desktop app)
last_active_time = models.DateTimeField(default=timezone_now, db_index=True)
last_active_time = models.DateTimeField(default=timezone_now, db_index=True, null=True)
# The following constants are used in the presence API for
# communicating whether a user is active (last_active_time recent)

17
zerver/tests/test_events.py
View File

@@ -1216,6 +1216,14 @@ class NormalActionsTest(BaseAction):
def test_away_events(self) -> None:
client = get_client("website")
# Updating user status to away activates the codepath of disabling
# the presence_enabled user setting. Correctly simulating the presence
# event status for a typical user requires settings the user's date_joined
# further into the past. See test_change_presence_enabled for more details,
# since it tests that codepath directly.
self.user_profile.date_joined = timezone_now() - datetime.timedelta(days=15)
self.user_profile.save()
# Set all
away_val = True
events = self.verify_action(
@@ -1920,6 +1928,15 @@ class NormalActionsTest(BaseAction):
def test_change_presence_enabled(self) -> None:
presence_enabled_setting = "presence_enabled"
# Disabling presence will lead to the creation of a UserPresence object for the user
# with a last_connected_time slightly preceding the moment of flipping the setting
# and last_active_time set to None. The presence API defaults to user_profile.date_joined
# for backwards compatibility when dealing with a None value. Thus for this test to properly
# check that the presence event emitted will have "idle" status, we need to simulate
# the (more realistic) scenario where date_joined is further in the past and not super recent.
self.user_profile.date_joined = timezone_now() - datetime.timedelta(days=15)
self.user_profile.save()
for val in [True, False]:
events = self.verify_action(
lambda: do_change_user_setting(

64
zerver/tests/test_presence.py
View File

@@ -71,6 +71,13 @@ class UserPresenceModelTests(ZulipTestCase):
presence_dct = get_presence_dict_by_realm(user_profile.realm_id)
self.assert_length(presence_dct, 0)
# If the values are set to "never", ignore it just like for sufficiently old presence rows.
UserPresence.objects.filter(id=user_profile.id).update(
last_active_time=None, last_connected_time=None
)
presence_dct = get_presence_dict_by_realm(user_profile.realm_id)
self.assert_length(presence_dct, 0)
def test_pushable_always_false(self) -> None:
# This field was never used by clients of the legacy API, so we
# just want to have it always set to False for API format compatibility.
@@ -284,10 +291,16 @@ class UserPresenceTests(ZulipTestCase):
UserPresence.objects.all().delete()
self.assertEqual(filter_presence_idle_user_ids({user_profile.id}), [user_profile.id])
# Create a first presence for the user. It's the first one, so it'll initialize both last_active_time
# and last_connected time with the current time. Thus the user will be considered active and won't
# get filtered.
# Create a first presence for the user. It's the first one and has status idle,
# so it'll initialize just last_connected time with the current time and last_active_time with None.
# Thus the user will be considered idle.
self.client_post("/json/users/me/presence", {"status": "idle"})
self.assertEqual(filter_presence_idle_user_ids({user_profile.id}), [user_profile.id])
# Now create a first presence with active status and check that the user is not filtered.
# This initializes the presence with both last_connected_time and last_active_time set to
# current time.
self.client_post("/json/users/me/presence", {"status": "active"})
self.assertEqual(filter_presence_idle_user_ids({user_profile.id}), [])
# Make last_active_time be older than OFFLINE_THRESHOLD_SECS. That should
@@ -363,6 +376,48 @@ class UserPresenceTests(ZulipTestCase):
{hamlet.email},
)
def test_null_timestamps_handling(self) -> None:
"""
Checks that the API handles presences with null presence timestamps correctly.
The data model now supports them being null, but the API should still return
valid timestamps for backwards compatibility - using date_joined as the default
to fall back on. Also it should correctly filter out users with null presence
just like it would have filtered them out if they had very old presence.
"""
self.login("hamlet")
othello = self.example_user("othello")
# Set a predictable value for date_joined
othello.date_joined = timezone_now() - timedelta(days=1)
othello.save()
UserPresence.objects.filter(user_profile=othello).update(
last_active_time=None, last_connected_time=None
)
result = self.client_get(f"/json/users/{othello.id}/presence")
result_dict = self.assert_json_success(result)
# Ensure date_joined was used as the fallback.
self.assertEqual(
result_dict["presence"]["website"]["timestamp"],
datetime_to_timestamp(othello.date_joined),
)
# Othello has null presence values, so should not appear in the /realm/presence response
# just like a user with over two weeks old presence.
result = self.client_get("/json/realm/presence")
result_dict = self.assert_json_success(result)
self.assertEqual(result_dict["presences"], {})
# If othello's presence is fresh however, it should appear in the response.
now = timezone_now()
UserPresence.objects.filter(user_profile=othello).update(
last_active_time=now, last_connected_time=now
)
result = self.client_get("/json/realm/presence")
result_dict = self.assert_json_success(result)
self.assertEqual(set(result_dict["presences"].keys()), {othello.email})
class SingleUserPresenceTests(ZulipTestCase):
def test_email_access(self) -> None:
@@ -678,6 +733,7 @@ class FormatLegacyPresenceDictTest(ZulipTestCase):
presence = UserPresence(
user_profile=hamlet, realm=hamlet.realm, last_active_time=now, last_connected_time=now
)
assert presence.last_active_time is not None and presence.last_connected_time is not None
self.assertEqual(
format_legacy_presence_dict(presence.last_active_time, presence.last_connected_time),
dict(
@@ -694,6 +750,7 @@ class FormatLegacyPresenceDictTest(ZulipTestCase):
last_active_time=recently,
last_connected_time=now,
)
assert presence.last_active_time is not None and presence.last_connected_time is not None
self.assertEqual(
format_legacy_presence_dict(presence.last_active_time, presence.last_connected_time),
dict(
@@ -710,6 +767,7 @@ class FormatLegacyPresenceDictTest(ZulipTestCase):
last_active_time=a_while_ago,
last_connected_time=now,
)
assert presence.last_active_time is not None and presence.last_connected_time is not None
self.assertEqual(
format_legacy_presence_dict(presence.last_active_time, presence.last_connected_time),
dict(