Skip to content

Commit b3313cd

Browse files
committed
fix(user_status): make setUserStatus() idempotent for concurrent duplicate requests
When two automated events of the same type fire simultaneously (e.g. a calendar webhook is delivered twice, or two Talk clients both trigger a call status at the same time), both requests race through setUserStatus(). The first request succeeds: it creates a backup of the user's previous status and sets the new automated status. The second request finds the backup slot already occupied (unique constraint on user_id), so backupCurrentStatus() returns false. Previously this caused setUserStatus() to return null, which the caller interpreted as a failure even though the desired status was already correctly set by the first request. Fix: after backupCurrentStatus() returns false, re-read the current active status row. If it already carries the same messageId that we were trying to set, the operation is effectively complete — return the existing status instead of null. If it carries a different messageId (a higher-priority automated status is active), we still return null to abort as before. This also covers the meeting-with-call scenario: if a calendar BUSY status is active and a Talk call starts, the call correctly overrides it via the priority shortcut path. If the calendar fires a duplicate webhook while the call is already active, the re-read finds MESSAGE_CALL ≠ MESSAGE_CALENDAR_BUSY and correctly aborts rather than overriding the ongoing call. AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Anna Larch <anna@nextcloud.com>
1 parent 354c641 commit b3313cd

2 files changed

Lines changed: 45 additions & 2 deletions

File tree

apps/user_status/lib/Service/StatusService.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,18 @@ public function setUserStatus(string $userId,
284284

285285
if ($createBackup) {
286286
if ($this->backupCurrentStatus($userId) === false) {
287-
return null; // Already a status set automatically => abort.
287+
// A backup already exists, meaning another automated status is active.
288+
// If the active status already carries the same messageId, this is a
289+
// duplicate/concurrent request for the same event — treat it as success.
290+
try {
291+
$currentStatus = $this->mapper->findByUserId($userId);
292+
if ($currentStatus->getMessageId() === $messageId) {
293+
return $currentStatus;
294+
}
295+
} catch (DoesNotExistException) {
296+
// No active status row exists, fall through to abort
297+
}
298+
return null; // A different automated status is active => abort.
288299
}
289300

290301
// If we just created the backup

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,7 @@ public function testSetUserStatus(string $messageId, string $oldMessageId, bool
802802
$previous->setUserId('john');
803803
$previous->setIsBackup(false);
804804

805-
$this->mapper->expects($this->once())
805+
$this->mapper->expects($this->exactly($expectedUpdateShortcut ? 1 : 2))
806806
->method('findByUserId')
807807
->with('john')
808808
->willReturn($previous);
@@ -825,4 +825,36 @@ public function testSetUserStatus(string $messageId, string $oldMessageId, bool
825825

826826
$this->service->setUserStatus('john', IUserStatus::DND, $messageId, true);
827827
}
828+
829+
public function testSetUserStatusIdempotentOnConcurrentDuplicateRequest(): void {
830+
$existing = new UserStatus();
831+
$existing->setId(1);
832+
$existing->setStatus(IUserStatus::DND);
833+
$existing->setStatusTimestamp(1337);
834+
$existing->setIsUserDefined(true);
835+
$existing->setMessageId(IUserStatus::MESSAGE_CALL);
836+
$existing->setUserId('john');
837+
$existing->setIsBackup(false);
838+
839+
$this->mapper->expects($this->exactly(2))
840+
->method('findByUserId')
841+
->with('john')
842+
->willReturn($existing);
843+
844+
/** @var MockObject&Exception $exception */
845+
$exception = $this->createMock(Exception::class);
846+
$exception->method('getReason')->willReturn(Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION);
847+
$this->mapper->expects($this->once())
848+
->method('createBackupStatus')
849+
->willThrowException($exception);
850+
851+
$this->predefinedStatusService->expects($this->once())
852+
->method('isValidId')
853+
->with(IUserStatus::MESSAGE_CALL)
854+
->willReturn(true);
855+
856+
// Active status already has MESSAGE_CALL — concurrent duplicate request should succeed
857+
$result = $this->service->setUserStatus('john', IUserStatus::DND, IUserStatus::MESSAGE_CALL, true);
858+
$this->assertSame($existing, $result);
859+
}
828860
}

0 commit comments

Comments
 (0)