Skip to content

fix(user_status): stabilise the user_status#59535

Open
miaulalala wants to merge 4 commits intomasterfrom
fix/noid/user-status-phase1-stabilization
Open

fix(user_status): stabilise the user_status#59535
miaulalala wants to merge 4 commits intomasterfrom
fix/noid/user-status-phase1-stabilization

Conversation

@miaulalala
Copy link
Copy Markdown
Contributor

@miaulalala miaulalala commented Apr 9, 2026

The user_status backup mechanism (saves/restores status during calls, calendar events, OOO) had several bugs causing unreliable status restoration.

Bugs fixed

1. Background job actively destroys backups (most critical)

  • clearOlderThanClearAt() deleted backup rows with expired clear_at
  • clearStatusesOlderThan() overwrote backup statuses to OFFLINE
  • Both ran every 60s, corrupting saved statuses before they could be restored
  • Fix: Added is_backup=false filter to both queries

2. Backup rows leaked into user-facing queries

  • findAll() returned backup rows (prefixed _userId) to callers
  • findAllRecent() filtered via slow LIKE '_%' instead of is_backup column
  • Fix: Added is_backup=false to findAll(), replaced LIKE with is_backup check in findAllRecent()

3. User deletion orphaned backups

  • UserDeletedListener only removed active status, leaving backup rows forever
  • Fix: Also call removeBackupUserStatus()

4. Silent failures with no logging

  • Backup conflicts, aborted status changes, failed reverts — all returned null/false with zero logging
  • Fix: Added debug logging to 4 silent failure paths in StatusService

Tests added

  • 4 new mapper tests (backups excluded from queries, backups survive cleanup)
  • 1 updated listener test (backup removal on user deletion)

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@miaulalala miaulalala added this to the Nextcloud 34 milestone Apr 9, 2026
@miaulalala miaulalala self-assigned this Apr 9, 2026
@miaulalala miaulalala requested a review from a team as a code owner April 9, 2026 13:11
@miaulalala miaulalala added bug 3. to review Waiting for reviews labels Apr 9, 2026
@miaulalala miaulalala requested review from ArtificialOwl and icewind1991 and removed request for a team April 9, 2026 13:11
@miaulalala miaulalala changed the title fix(user_status): stabilise the user_status to some degree fix(user_status): stabilise the user_status Apr 9, 2026
@miaulalala miaulalala force-pushed the fix/noid/user-status-phase1-stabilization branch from 8eec494 to 361fc17 Compare April 9, 2026 13:13
@miaulalala
Copy link
Copy Markdown
Contributor Author

/backport to stable33

@miaulalala
Copy link
Copy Markdown
Contributor Author

/backport to stable32

…m list methods

The background cleanup job operated on all rows including backups:
- clearOlderThanClearAt() deleted backup rows with expired clear_at,
  destroying saved statuses before they could be restored
- clearStatusesOlderThan() overwrote backup statuses to OFFLINE,
  corrupting saved DND/away states

Additionally, findAll() leaked backup rows into user-facing lists,
and findAllRecent() filtered backups via slow LIKE pattern instead
of the is_backup column.

Add is_backup=false filter to all four methods.

AI-Assisted-By: Claude Opus 4.6
Signed-off-by: Anna Larch <anna@nextcloud.com>
UserDeletedListener only called removeUserStatus(), leaving backup
status rows (prefixed _userId) orphaned in the database forever.

AI-Assisted-By: Claude Opus 4.6
Signed-off-by: Anna Larch <anna@nextcloud.com>
Multiple code paths in StatusService silently swallowed failures,
making it impossible to debug status reliability issues. Add debug
logging for: backup creation conflicts, aborted automated status
changes, failed revert operations, and concurrent insert conflicts.

AI-Assisted-By: Claude Opus 4.6
Signed-off-by: Anna Larch <anna@nextcloud.com>
… queries

- testFindAllExcludesBackups: verify findAll() doesn't return backup rows
- testFindAllRecentExcludesBackups: verify findAllRecent() excludes backups
- testClearOlderThanClearAtPreservesBackups: verify backup rows survive
  clear_at cleanup
- testClearStatusesOlderThanPreservesBackups: verify backup statuses aren't
  overwritten to OFFLINE by age-based cleanup
- testHandleWithCorrectEvent: verify removeBackupUserStatus() is also called
  when a user is deleted

AI-Assisted-By: Claude Opus 4.6
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala miaulalala force-pushed the fix/noid/user-status-phase1-stabilization branch from 361fc17 to 7db0863 Compare April 9, 2026 13:58
->where($qb->expr()->isNotNull('clear_at'))
->andWhere($qb->expr()->lte('clear_at', $qb->createNamedParameter($timestamp, IQueryBuilder::PARAM_INT)));
->andWhere($qb->expr()->lte('clear_at', $qb->createNamedParameter($timestamp, IQueryBuilder::PARAM_INT)))
->andWhere($qb->expr()->eq('is_backup', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL)));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also backups that are in the past should be cleared?

$qb->expr()->eq('status', $qb->createNamedParameter(IUserStatus::ONLINE))
));
))
->andWhere($qb->expr()->eq('is_backup', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL)));
Copy link
Copy Markdown
Member

@nickvergessen nickvergessen Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also backups that are in the past should be cleared?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants