Skip to content

Commit fc9c32f

Browse files
committed
fix(user_status): refresh a live status before it can be invalidated
UserLiveStatusListener only refreshed status_timestamp once it was already older than INVALIDATE_STATUS_THRESHOLD, which is the same 15 minutes at which ClearOldStatusesBackgroundJob sweeps a status to offline. With a five minute client interval that leaves a window, up to one interval wide, in which a user who never stopped working is shown as offline until their next heartbeat arrives. The refresh now happens at REFRESH_STATUS_THRESHOLD, far enough below the invalidation threshold to leave room for a full heartbeat interval. Until now the sheer number of heartbeats hid the problem, because one always landed within seconds of the cleanup job. Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
1 parent a9b90bc commit fc9c32f

5 files changed

Lines changed: 118 additions & 3 deletions

File tree

apps/user_status/lib/Listener/UserLiveStatusListener.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,7 @@ public function handle(Event $event): void {
7676

7777
$needsUpdate = false;
7878

79-
// If the current status is older than 5 minutes,
80-
// treat it as outdated and update
81-
if ($userStatus->getStatusTimestamp() < ($this->timeFactory->getTime() - StatusService::INVALIDATE_STATUS_THRESHOLD)) {
79+
if ($userStatus->getStatusTimestamp() < ($this->timeFactory->getTime() - StatusService::REFRESH_STATUS_THRESHOLD)) {
8280
$needsUpdate = true;
8381
}
8482

apps/user_status/lib/Service/StatusService.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,13 @@ class StatusService {
6262
/** @var int */
6363
public const INVALIDATE_STATUS_THRESHOLD = 15 /* minutes */ * 60 /* seconds */;
6464

65+
/**
66+
* Has to stay at least one client heartbeat interval below INVALIDATE_STATUS_THRESHOLD.
67+
*
68+
* @var int
69+
*/
70+
public const REFRESH_STATUS_THRESHOLD = 7 /* minutes */ * 60 /* seconds */;
71+
6572
/** @var int */
6673
public const MAXIMUM_MESSAGE_LENGTH = 80;
6774

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\UserStatus\Tests\Integration\Listener;
11+
12+
use OCA\DAV\CalDAV\Status\StatusService as CalendarStatusService;
13+
use OCA\UserStatus\Db\UserStatusMapper;
14+
use OCA\UserStatus\Listener\UserLiveStatusListener;
15+
use OCA\UserStatus\Service\StatusService;
16+
use OCP\AppFramework\Utility\ITimeFactory;
17+
use OCP\IDBConnection;
18+
use OCP\IUser;
19+
use OCP\Server;
20+
use OCP\User\Events\UserLiveStatusEvent;
21+
use OCP\UserStatus\IUserStatus;
22+
use Psr\Log\LoggerInterface;
23+
use Test\TestCase;
24+
use function time;
25+
26+
#[\PHPUnit\Framework\Attributes\Group(name: 'DB')]
27+
class UserLiveStatusListenerIntegrationTest extends TestCase {
28+
29+
private const USER_ID = 'test123';
30+
31+
/** HEARTBEAT_INTERVAL in apps/user_status/src/services/heartbeatScheduler.ts */
32+
private const CLIENT_HEARTBEAT_INTERVAL = 5 * 60;
33+
34+
/** ClearOldStatusesBackgroundJob::setInterval() */
35+
private const CLEANUP_JOB_INTERVAL = 60;
36+
37+
private UserStatusMapper $mapper;
38+
private StatusService $service;
39+
private UserLiveStatusListener $listener;
40+
41+
protected function setUp(): void {
42+
parent::setUp();
43+
44+
$this->mapper = Server::get(UserStatusMapper::class);
45+
$this->service = Server::get(StatusService::class);
46+
47+
$db = Server::get(IDBConnection::class);
48+
$qb = $db->getQueryBuilder();
49+
$qb->delete('user_status')->executeStatement();
50+
51+
$this->listener = new UserLiveStatusListener(
52+
$this->mapper,
53+
$this->service,
54+
Server::get(ITimeFactory::class),
55+
$this->createMock(CalendarStatusService::class),
56+
$this->createMock(LoggerInterface::class),
57+
);
58+
}
59+
60+
public function testActiveUserSurvivesTheInvalidationSweep(): void {
61+
$this->service->setStatus(self::USER_ID, IUserStatus::ONLINE, time(), false);
62+
63+
for ($minute = 1; $minute <= 30; $minute++) {
64+
$this->passTime(self::CLEANUP_JOB_INTERVAL);
65+
$this->runCleanupJob();
66+
67+
self::assertSame(
68+
IUserStatus::ONLINE,
69+
$this->mapper->findByUserId(self::USER_ID)->getStatus(),
70+
"User went offline after $minute minutes while still sending heartbeats",
71+
);
72+
73+
if ($minute % (self::CLIENT_HEARTBEAT_INTERVAL / self::CLEANUP_JOB_INTERVAL) === 0) {
74+
$this->heartbeat();
75+
}
76+
}
77+
}
78+
79+
/** Equivalent to letting time pass, without faking the clock of every collaborator. */
80+
private function passTime(int $seconds): void {
81+
$status = $this->mapper->findByUserId(self::USER_ID);
82+
$status->setStatusTimestamp($status->getStatusTimestamp() - $seconds);
83+
$this->mapper->update($status);
84+
}
85+
86+
private function heartbeat(): void {
87+
$user = $this->createMock(IUser::class);
88+
$user->method('getUID')->willReturn(self::USER_ID);
89+
90+
$this->listener->handle(new UserLiveStatusEvent($user, IUserStatus::ONLINE, time()));
91+
}
92+
93+
private function runCleanupJob(): void {
94+
$now = time();
95+
$this->mapper->clearStatusesOlderThan($now - StatusService::INVALIDATE_STATUS_THRESHOLD, $now);
96+
}
97+
}

apps/user_status/tests/Unit/Listener/UserLiveStatusListenerTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ public static function handleEventWithCorrectEventDataProvider(): array {
136136
['john.doe', 'online', 5000, false, 'away', 5000, true, false],
137137
['john.doe', 'away', 5000, true, 'online', 5000, true, false],
138138
['john.doe', 'online', 5000, true, 'away', 5000, true, false],
139+
// a status older than REFRESH_STATUS_THRESHOLD is refreshed, a younger one is not
140+
['john.doe', 'online', 4500, false, 'online', 5000, true, true],
141+
['john.doe', 'online', 4700, false, 'online', 5000, true, false],
139142
];
140143
}
141144
}

apps/user_status/tests/Unit/Service/StatusServiceTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,16 @@ public function testRemoveUserStatusDoesNotExist(): void {
629629
$this->assertFalse($actual);
630630
}
631631

632+
public function testRefreshThresholdLeavesRoomForAHeartbeat(): void {
633+
// HEARTBEAT_INTERVAL in apps/user_status/src/services/heartbeatScheduler.ts
634+
$clientHeartbeatInterval = 5 * 60;
635+
636+
self::assertGreaterThan(
637+
$clientHeartbeatInterval,
638+
StatusService::INVALIDATE_STATUS_THRESHOLD - StatusService::REFRESH_STATUS_THRESHOLD,
639+
);
640+
}
641+
632642
public function testCleanStatusAutomaticOnline(): void {
633643
$status = new UserStatus();
634644
$status->setStatus(IUserStatus::ONLINE);

0 commit comments

Comments
 (0)