Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apps/user_status/appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
<background-jobs>
<job>OCA\UserStatus\BackgroundJob\ClearOldStatusesBackgroundJob</job>
</background-jobs>
<commands>
<command>OCA\UserStatus\Command\Repair</command>
</commands>
<contactsmenu>
<provider>OCA\UserStatus\ContactsMenu\StatusProvider</provider>
</contactsmenu>
Expand Down
1 change: 1 addition & 0 deletions apps/user_status/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
'OCA\\UserStatus\\AppInfo\\Application' => $baseDir . '/../lib/AppInfo/Application.php',
'OCA\\UserStatus\\BackgroundJob\\ClearOldStatusesBackgroundJob' => $baseDir . '/../lib/BackgroundJob/ClearOldStatusesBackgroundJob.php',
'OCA\\UserStatus\\Capabilities' => $baseDir . '/../lib/Capabilities.php',
'OCA\\UserStatus\\Command\\Repair' => $baseDir . '/../lib/Command/Repair.php',
'OCA\\UserStatus\\Connector\\UserStatus' => $baseDir . '/../lib/Connector/UserStatus.php',
'OCA\\UserStatus\\Connector\\UserStatusProvider' => $baseDir . '/../lib/Connector/UserStatusProvider.php',
'OCA\\UserStatus\\ContactsMenu\\StatusProvider' => $baseDir . '/../lib/ContactsMenu/StatusProvider.php',
Expand Down
1 change: 1 addition & 0 deletions apps/user_status/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class ComposerStaticInitUserStatus
'OCA\\UserStatus\\AppInfo\\Application' => __DIR__ . '/..' . '/../lib/AppInfo/Application.php',
'OCA\\UserStatus\\BackgroundJob\\ClearOldStatusesBackgroundJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/ClearOldStatusesBackgroundJob.php',
'OCA\\UserStatus\\Capabilities' => __DIR__ . '/..' . '/../lib/Capabilities.php',
'OCA\\UserStatus\\Command\\Repair' => __DIR__ . '/..' . '/../lib/Command/Repair.php',
'OCA\\UserStatus\\Connector\\UserStatus' => __DIR__ . '/..' . '/../lib/Connector/UserStatus.php',
'OCA\\UserStatus\\Connector\\UserStatusProvider' => __DIR__ . '/..' . '/../lib/Connector/UserStatusProvider.php',
'OCA\\UserStatus\\ContactsMenu\\StatusProvider' => __DIR__ . '/..' . '/../lib/ContactsMenu/StatusProvider.php',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,6 @@ protected function run($argument) {

$this->mapper->clearOlderThanClearAt($now);
$this->mapper->clearStatusesOlderThan($now - StatusService::INVALIDATE_STATUS_THRESHOLD, $now);
$this->mapper->deleteStrandedBackups(StatusService::AUTOMATED_MESSAGE_IDS);
}
}
129 changes: 129 additions & 0 deletions apps/user_status/lib/Command/Repair.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\UserStatus\Command;

use OCA\UserStatus\Db\UserStatusMapper;
use OCA\UserStatus\Service\StatusService;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;

class Repair extends Command {
Comment thread
miaulalala marked this conversation as resolved.

public function __construct(
private UserStatusMapper $mapper,
) {
parent::__construct();
}

#[\Override]
protected function configure(): void {
$this
->setName('user-status:repair')
->setDescription('Repair user statuses left behind by an interrupted automated status')
->addOption('dry-run', null, InputOption::VALUE_NONE, 'Only report what would be repaired');
}

#[\Override]
public function execute(InputInterface $input, OutputInterface $output): int {
$dryRun = (bool)$input->getOption('dry-run');
if ($dryRun) {
$output->writeln('<comment>Dry run, no changes will be written.</comment>');
$output->writeln('');
}

$this->repairMissingBackupFlags($output, $dryRun);
$this->repairOrphanedStatuses($output, $dryRun);
$this->repairStrandedBackups($output, $dryRun);

return self::SUCCESS;
}

/**
* Rows written before is_backup had a default are invisible to every query
* comparing it against false, so other users see them as offline and the
* cleanup job skips them.
*/
private function repairMissingBackupFlags(OutputInterface $output, bool $dryRun): void {
$ids = $this->mapper->findStatusesWithoutBackupFlagIds();
if ($ids === []) {
$output->writeln('No statuses with a missing backup flag.');
return;
}

$count = count($ids);
if ($dryRun) {
$output->writeln("Would set the backup flag on <info>$count</info> status(es).");
$this->listIds($output, $ids);
return;
}

$fixed = $this->mapper->normalizeBackupFlagByIds($ids);
$output->writeln("Set the backup flag on <info>$fixed</info> status(es).");
}

/**
* A live status on an automated message id with no backup row can never be
* reverted by the automation that set it, and the heartbeat refuses to
* overwrite it, so the user is stuck. Removing the row lets the next
* heartbeat recreate a normal status.
*/
private function repairOrphanedStatuses(OutputInterface $output, bool $dryRun): void {
$ids = $this->mapper->findOrphanedAutomatedStatusIds(StatusService::AUTOMATED_MESSAGE_IDS);
if ($ids === []) {
$output->writeln('No users stuck on an automated status.');
return;
}

if ($dryRun) {
$output->writeln('Would clear <info>' . count($ids) . '</info> status(es) stuck on an automated status.');
$this->listIds($output, $ids);
return;
}

$deleted = $this->mapper->deleteByIds($ids);
$output->writeln("Cleared <info>$deleted</info> status(es) stuck on an automated status.");
}

/**
* A backup that can no longer be matched blocks every future automated
* status change for that user, because createBackupStatus() keeps hitting
* the unique constraint on user_id.
*/
private function repairStrandedBackups(OutputInterface $output, bool $dryRun): void {
$ids = $this->mapper->findStrandedBackupIds(StatusService::AUTOMATED_MESSAGE_IDS);
if ($ids === []) {
$output->writeln('No stranded backup statuses.');
return;
}

if ($dryRun) {
$output->writeln('Would remove <info>' . count($ids) . '</info> stranded backup status(es).');
$this->listIds($output, $ids);
return;
}

$deleted = $this->mapper->deleteByIds($ids);
$output->writeln("Removed <info>$deleted</info> stranded backup status(es).");
}

/**
* The ids are what an administrator needs to look the rows up themselves,
* but there can be a lot of them, so only spell them out when asked.
*
* @param list<int> $ids
*/
private function listIds(OutputInterface $output, array $ids): void {
if ($output->getVerbosity() >= OutputInterface::VERBOSITY_VERBOSE) {
$output->writeln(' ids: ' . implode(', ', $ids));
}
}
}
155 changes: 151 additions & 4 deletions apps/user_status/lib/Db/UserStatusMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@
*/
class UserStatusMapper extends QBMapper {

/**
* Oracle rejects an IN list with more than 1000 expressions, so anything
* built from an unbounded set of ids has to be split into chunks.
*/
private const MAX_IN_CHUNK = 1000;

/**
* @param IDBConnection $db
*/
Expand Down Expand Up @@ -163,11 +169,152 @@ public function deleteCurrentStatusToRestoreBackup(string $userId, string $messa
return $qb->executeStatement() > 0;
}

public function deleteByIds(array $ids): void {
/**
* Deletes backup rows that can never be restored, because the matching live
* status is gone or is no longer on one of the automated statuses that would
* revert into it.
*
* Such a row is not just clutter: while it exists, createBackupStatus() keeps
* hitting the unique constraint on user_id, which makes setUserStatus()
* silently abort every automated status change for that user.
*
* @param list<string> $automatedMessageIds Message ids that own a backup
* @return int Number of deleted backup rows
*/
public function deleteStrandedBackups(array $automatedMessageIds): int {
return $this->deleteByIds($this->findStrandedBackupIds($automatedMessageIds));
}

/**
* Ids of backup rows that can never be restored. See deleteStrandedBackups().
*
* A backup is reachable exactly when the live row it belongs to still carries
* one of the automated message ids, because that is what revertUserStatus()
* matches on. The live row is the one whose user id is the backup's user id
* without the underscore prefix, so the two are matched with a self join.
*
* @param list<string> $automatedMessageIds
* @return list<int>
*/
public function findStrandedBackupIds(array $automatedMessageIds): array {
$qb = $this->db->getQueryBuilder();
$qb->delete($this->tableName)
->where($qb->expr()->in('id', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY)));
$qb->executeStatement();
$qb->select('b.id')
->from($this->tableName, 'b')
->where($qb->expr()->eq('b.is_backup', $qb->createNamedParameter(true, IQueryBuilder::PARAM_BOOL)));

if ($automatedMessageIds === []) {
// No automated status can own a backup, so none of them is reachable.
return $this->fetchIds($qb);
}

// Not filtering the live side on is_backup is deliberate: a row whose
// is_backup is NULL is still treated as a live row, so unexpected data
// errs towards keeping the backup.
$qb->leftJoin('b', $this->tableName, 'l', $qb->expr()->andX(
$qb->expr()->eq('l.user_id', $qb->func()->substring('b.user_id', $qb->createNamedParameter(2, IQueryBuilder::PARAM_INT))),
$qb->expr()->in('l.message_id', $qb->createNamedParameter($automatedMessageIds, IQueryBuilder::PARAM_STR_ARRAY)),
))
->andWhere($qb->expr()->isNull('l.id'));

return $this->fetchIds($qb);
}
Comment on lines +199 to +220

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All in all this reads really convoluted to me, I tried to understand what it does but I’m still a bit lost.
It gets statuses where is_backup is true, and in this case user_id also start with a an underscore, which we remove and then search for statuses matching those user ids and the id given as parameter?
It feels expensive to compute a list of all backed up users only to try a few ids.

Is it not feasible in one request with a join? Or would that be a bad idea for some reason?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed, changed to self join. Added some tests to make sure it also works on Oracle, let's see what CI says.


/**
* Ids of live rows that sit on an automated status with no backup row to
* revert into. Those can never be reverted by the automation that set them,
* so the user is stuck on that status until it is cleared.
*
* @param list<string> $automatedMessageIds
* @return list<int>
*/
public function findOrphanedAutomatedStatusIds(array $automatedMessageIds): array {
if ($automatedMessageIds === []) {
return [];
}

$qb = $this->db->getQueryBuilder();
// The backup of a live row carries the same user id with an underscore
// prefix, so the two are matched with a self join on the concatenation.
$qb->select('l.id')
->from($this->tableName, 'l')
->leftJoin('l', $this->tableName, 'b', $qb->expr()->eq(
'b.user_id',
$qb->func()->concat($qb->createNamedParameter('_'), 'l.user_id'),
))
->where($qb->expr()->in('l.message_id', $qb->createNamedParameter($automatedMessageIds, IQueryBuilder::PARAM_STR_ARRAY)))
->andWhere($qb->expr()->isNull('b.id'))
// Skip backup rows on the live side. Testing the prefix rather than
// is_backup keeps this correct for rows where is_backup is NULL, and
// a substring comparison avoids having to escape the underscore for
// a LIKE pattern.
->andWhere($qb->expr()->neq(
$qb->func()->substring('l.user_id', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT), $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT)),
$qb->createNamedParameter('_'),
));

return $this->fetchIds($qb);
}

/**
* @return list<int>
*/
private function fetchIds(IQueryBuilder $qb): array {
$result = $qb->executeQuery();
$ids = [];
while ($row = $result->fetch()) {
$ids[] = (int)$row['id'];
}
$result->closeCursor();

return $ids;
}

/**
* Ids of rows where is_backup is NULL. Those predate the column default and
* are invisible to every query that compares is_backup against false.
*
* @return list<int>
*/
public function findStatusesWithoutBackupFlagIds(): array {
$qb = $this->db->getQueryBuilder();
$qb->select('id')
->from($this->tableName)
->where($qb->expr()->isNull('is_backup'));

return $this->fetchIds($qb);
}

/**
* @param list<int> $ids
* @return int Number of rows that were given an explicit is_backup value
*/
public function normalizeBackupFlagByIds(array $ids): int {
$updated = 0;
foreach (array_chunk($ids, self::MAX_IN_CHUNK) as $chunk) {
$qb = $this->db->getQueryBuilder();
$qb->update($this->tableName)
->set('is_backup', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))
->where($qb->expr()->in('id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)));
$updated += $qb->executeStatement();
}

return $updated;
}

/**
* @param list<int> $ids
* @return int Number of deleted rows
*/
public function deleteByIds(array $ids): int {
$deleted = 0;
foreach (array_chunk($ids, self::MAX_IN_CHUNK) as $chunk) {
$qb = $this->db->getQueryBuilder();
$qb->delete($this->tableName)
->where($qb->expr()->in('id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)));
$deleted += $qb->executeStatement();
}

return $deleted;
}

/**
Expand Down
37 changes: 30 additions & 7 deletions apps/user_status/lib/Service/StatusService.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,20 @@ class StatusService {
IUserStatus::INVISIBLE,
];

/**
* Message ids that are only ever set by an automation (calendar, call,
* availability, out-of-office). A status carrying one of these owns the
* backup of whatever the user had set before, and is expected to be
* reverted once the automation stops applying.
*/
public const AUTOMATED_MESSAGE_IDS = [
IUserStatus::MESSAGE_CALENDAR_BUSY,
IUserStatus::MESSAGE_CALENDAR_BUSY_TENTATIVE,
IUserStatus::MESSAGE_CALL,
IUserStatus::MESSAGE_AVAILABILITY,
IUserStatus::MESSAGE_OUT_OF_OFFICE,
];

/** @var int */
public const INVALIDATE_STATUS_THRESHOLD = 15 /* minutes */ * 60 /* seconds */;

Expand Down Expand Up @@ -530,7 +544,13 @@ public function revertUserStatus(string $userId, string $messageId, bool $revert
/** @var UserStatus $userStatus */
$backupUserStatus = $this->mapper->findByUserId($userId, true);
} catch (DoesNotExistException $ex) {
// No user status to revert, do nothing
// There is no backup to restore. The automated status still has to
// go, otherwise the user is stuck on it forever: UserLiveStatusListener
// refuses to overwrite an automated status, so no heartbeat can ever
// bring them back online.
if ($this->mapper->deleteCurrentStatusToRestoreBackup($userId, $messageId)) {
$this->logger->debug('Cleared automated status "' . $messageId . '" for user ' . $userId . ': there was no backup to restore', ['app' => 'user_status']);
}
return null;
}

Expand All @@ -540,14 +560,17 @@ public function revertUserStatus(string $userId, string $messageId, bool $revert
return null;
}

if ($revertedManually) {
if ($backupUserStatus->getStatus() === IUserStatus::OFFLINE) {
// When the user reverts the status manually they are online
$backupUserStatus->setStatus(IUserStatus::ONLINE);
}
$backupUserStatus->setStatusTimestamp($this->timeFactory->getTime());
if ($revertedManually && $backupUserStatus->getStatus() === IUserStatus::OFFLINE) {
// When the user reverts the status manually they are online
$backupUserStatus->setStatus(IUserStatus::ONLINE);
}

// The restored status becomes the current one now. Keeping the timestamp
// from before the automation would make it instantly stale for anything
// longer than INVALIDATE_STATUS_THRESHOLD, so the next read would clean
// the user straight to offline.
$backupUserStatus->setStatusTimestamp($this->timeFactory->getTime());

$backupUserStatus->setIsBackup(false);
// Remove the underscore prefix added when creating the backup
$backupUserStatus->setUserId(substr($backupUserStatus->getUserId(), 1));
Expand Down
Loading
Loading