Skip to content

Commit 8204069

Browse files
miaulalalamiaulalala
committed
feat(user_status): add occ user-status:repair for statuses left behind
The preceding fixes stop new damage, but nothing repairs what is already in the database: reverts for call, availability and out-of-office are driven by automations that never fire again for a user who is already stuck. Add a command that repairs the three shapes, with --dry-run to see the scope first: - statuses whose is_backup is NULL, which every query comparing the column against false skips - live rows on an automated status with no backup to revert into - backup rows that can no longer be matched Orphaned rows are deleted rather than rewritten, matching what revertUserStatus() now does, and the next heartbeat recreates a normal status. AI-Assisted-By: Claude Opus 5 Signed-off-by: Anna Larch <anna@nextcloud.com>
1 parent a9a5eb3 commit 8204069

8 files changed

Lines changed: 477 additions & 46 deletions

File tree

‎apps/user_status/appinfo/info.xml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929
<background-jobs>
3030
<job>OCA\UserStatus\BackgroundJob\ClearOldStatusesBackgroundJob</job>
3131
</background-jobs>
32+
<commands>
33+
<command>OCA\UserStatus\Command\Repair</command>
34+
</commands>
3235
<contactsmenu>
3336
<provider>OCA\UserStatus\ContactsMenu\StatusProvider</provider>
3437
</contactsmenu>

‎apps/user_status/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
'OCA\\UserStatus\\AppInfo\\Application' => $baseDir . '/../lib/AppInfo/Application.php',
1111
'OCA\\UserStatus\\BackgroundJob\\ClearOldStatusesBackgroundJob' => $baseDir . '/../lib/BackgroundJob/ClearOldStatusesBackgroundJob.php',
1212
'OCA\\UserStatus\\Capabilities' => $baseDir . '/../lib/Capabilities.php',
13+
'OCA\\UserStatus\\Command\\Repair' => $baseDir . '/../lib/Command/Repair.php',
1314
'OCA\\UserStatus\\Connector\\UserStatus' => $baseDir . '/../lib/Connector/UserStatus.php',
1415
'OCA\\UserStatus\\Connector\\UserStatusProvider' => $baseDir . '/../lib/Connector/UserStatusProvider.php',
1516
'OCA\\UserStatus\\ContactsMenu\\StatusProvider' => $baseDir . '/../lib/ContactsMenu/StatusProvider.php',

‎apps/user_status/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class ComposerStaticInitUserStatus
2525
'OCA\\UserStatus\\AppInfo\\Application' => __DIR__ . '/..' . '/../lib/AppInfo/Application.php',
2626
'OCA\\UserStatus\\BackgroundJob\\ClearOldStatusesBackgroundJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/ClearOldStatusesBackgroundJob.php',
2727
'OCA\\UserStatus\\Capabilities' => __DIR__ . '/..' . '/../lib/Capabilities.php',
28+
'OCA\\UserStatus\\Command\\Repair' => __DIR__ . '/..' . '/../lib/Command/Repair.php',
2829
'OCA\\UserStatus\\Connector\\UserStatus' => __DIR__ . '/..' . '/../lib/Connector/UserStatus.php',
2930
'OCA\\UserStatus\\Connector\\UserStatusProvider' => __DIR__ . '/..' . '/../lib/Connector/UserStatusProvider.php',
3031
'OCA\\UserStatus\\ContactsMenu\\StatusProvider' => __DIR__ . '/..' . '/../lib/ContactsMenu/StatusProvider.php',

‎apps/user_status/lib/Command/Repair.php‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
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\Command;
11+
12+
use OCA\UserStatus\Db\UserStatusMapper;
13+
use OCA\UserStatus\Service\StatusService;
14+
use Symfony\Component\Console\Command\Command;
15+
use Symfony\Component\Console\Input\InputInterface;
16+
use Symfony\Component\Console\Input\InputOption;
17+
use Symfony\Component\Console\Output\OutputInterface;
18+
19+
class Repair extends Command {
20+
21+
public function __construct(
22+
private UserStatusMapper $mapper,
23+
) {
24+
parent::__construct();
25+
}
26+
27+
#[\Override]
28+
protected function configure(): void {
29+
$this
30+
->setName('user-status:repair')
31+
->setDescription('Repair user statuses left behind by an interrupted automated status')
32+
->addOption('dry-run', null, InputOption::VALUE_NONE, 'Only report what would be repaired');
33+
}
34+
35+
#[\Override]
36+
public function execute(InputInterface $input, OutputInterface $output): int {
37+
$dryRun = (bool)$input->getOption('dry-run');
38+
if ($dryRun) {
39+
$output->writeln('<comment>Dry run, no changes will be written.</comment>');
40+
$output->writeln('');
41+
}
42+
43+
$this->repairMissingBackupFlags($output, $dryRun);
44+
$this->repairOrphanedStatuses($output, $dryRun);
45+
$this->repairStrandedBackups($output, $dryRun);
46+
47+
return self::SUCCESS;
48+
}
49+
50+
/**
51+
* Rows written before is_backup had a default are invisible to every query
52+
* comparing it against false, so other users see them as offline and the
53+
* cleanup job skips them.
54+
*/
55+
private function repairMissingBackupFlags(OutputInterface $output, bool $dryRun): void {
56+
$ids = $this->mapper->findStatusesWithoutBackupFlagIds();
57+
if ($ids === []) {
58+
$output->writeln('No statuses with a missing backup flag.');
59+
return;
60+
}
61+
62+
$count = count($ids);
63+
if ($dryRun) {
64+
$output->writeln("Would set the backup flag on <info>$count</info> status(es).");
65+
$this->listIds($output, $ids);
66+
return;
67+
}
68+
69+
$fixed = $this->mapper->normalizeBackupFlagByIds($ids);
70+
$output->writeln("Set the backup flag on <info>$fixed</info> status(es).");
71+
}
72+
73+
/**
74+
* A live status on an automated message id with no backup row can never be
75+
* reverted by the automation that set it, and the heartbeat refuses to
76+
* overwrite it, so the user is stuck. Removing the row lets the next
77+
* heartbeat recreate a normal status.
78+
*/
79+
private function repairOrphanedStatuses(OutputInterface $output, bool $dryRun): void {
80+
$ids = $this->mapper->findOrphanedAutomatedStatusIds(StatusService::AUTOMATED_MESSAGE_IDS);
81+
if ($ids === []) {
82+
$output->writeln('No users stuck on an automated status.');
83+
return;
84+
}
85+
86+
if ($dryRun) {
87+
$output->writeln('Would clear <info>' . count($ids) . '</info> status(es) stuck on an automated status.');
88+
$this->listIds($output, $ids);
89+
return;
90+
}
91+
92+
$deleted = $this->mapper->deleteByIds($ids);
93+
$output->writeln("Cleared <info>$deleted</info> status(es) stuck on an automated status.");
94+
}
95+
96+
/**
97+
* A backup that can no longer be matched blocks every future automated
98+
* status change for that user, because createBackupStatus() keeps hitting
99+
* the unique constraint on user_id.
100+
*/
101+
private function repairStrandedBackups(OutputInterface $output, bool $dryRun): void {
102+
$ids = $this->mapper->findStrandedBackupIds(StatusService::AUTOMATED_MESSAGE_IDS);
103+
if ($ids === []) {
104+
$output->writeln('No stranded backup statuses.');
105+
return;
106+
}
107+
108+
if ($dryRun) {
109+
$output->writeln('Would remove <info>' . count($ids) . '</info> stranded backup status(es).');
110+
$this->listIds($output, $ids);
111+
return;
112+
}
113+
114+
$deleted = $this->mapper->deleteByIds($ids);
115+
$output->writeln("Removed <info>$deleted</info> stranded backup status(es).");
116+
}
117+
118+
/**
119+
* The ids are what an administrator needs to look the rows up themselves,
120+
* but there can be a lot of them, so only spell them out when asked.
121+
*
122+
* @param list<int> $ids
123+
*/
124+
private function listIds(OutputInterface $output, array $ids): void {
125+
if ($output->getVerbosity() >= OutputInterface::VERBOSITY_VERBOSE) {
126+
$output->writeln(' ids: ' . implode(', ', $ids));
127+
}
128+
}
129+
}

‎apps/user_status/lib/Db/UserStatusMapper.php‎

Lines changed: 119 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@
2020
*/
2121
class UserStatusMapper extends QBMapper {
2222

23+
/**
24+
* Oracle rejects an IN list with more than 1000 expressions, so anything
25+
* built from an unbounded set of ids has to be split into chunks.
26+
*/
27+
private const MAX_IN_CHUNK = 1000;
28+
2329
/**
2430
* @param IDBConnection $db
2531
*/
@@ -176,58 +182,132 @@ public function deleteCurrentStatusToRestoreBackup(string $userId, string $messa
176182
* @return int Number of deleted backup rows
177183
*/
178184
public function deleteStrandedBackups(array $automatedMessageIds): int {
185+
return $this->deleteByIds($this->findStrandedBackupIds($automatedMessageIds));
186+
}
187+
188+
/**
189+
* Ids of backup rows that can never be restored. See deleteStrandedBackups().
190+
*
191+
* A backup is reachable exactly when the live row it belongs to still carries
192+
* one of the automated message ids, because that is what revertUserStatus()
193+
* matches on. The live row is the one whose user id is the backup's user id
194+
* without the underscore prefix, so the two are matched with a self join.
195+
*
196+
* @param list<string> $automatedMessageIds
197+
* @return list<int>
198+
*/
199+
public function findStrandedBackupIds(array $automatedMessageIds): array {
179200
$qb = $this->db->getQueryBuilder();
180-
$qb->select('id', 'user_id')
181-
->from($this->tableName)
182-
->where($qb->expr()->eq('is_backup', $qb->createNamedParameter(true, IQueryBuilder::PARAM_BOOL)));
201+
$qb->select('b.id')
202+
->from($this->tableName, 'b')
203+
->where($qb->expr()->eq('b.is_backup', $qb->createNamedParameter(true, IQueryBuilder::PARAM_BOOL)));
204+
205+
if ($automatedMessageIds === []) {
206+
// No automated status can own a backup, so none of them is reachable.
207+
return $this->fetchIds($qb);
208+
}
209+
210+
// Not filtering the live side on is_backup is deliberate: a row whose
211+
// is_backup is NULL is still treated as a live row, so unexpected data
212+
// errs towards keeping the backup.
213+
$qb->leftJoin('b', $this->tableName, 'l', $qb->expr()->andX(
214+
$qb->expr()->eq('l.user_id', $qb->func()->substring('b.user_id', $qb->createNamedParameter(2, IQueryBuilder::PARAM_INT))),
215+
$qb->expr()->in('l.message_id', $qb->createNamedParameter($automatedMessageIds, IQueryBuilder::PARAM_STR_ARRAY)),
216+
))
217+
->andWhere($qb->expr()->isNull('l.id'));
218+
219+
return $this->fetchIds($qb);
220+
}
221+
222+
/**
223+
* Ids of live rows that sit on an automated status with no backup row to
224+
* revert into. Those can never be reverted by the automation that set them,
225+
* so the user is stuck on that status until it is cleared.
226+
*
227+
* @param list<string> $automatedMessageIds
228+
* @return list<int>
229+
*/
230+
public function findOrphanedAutomatedStatusIds(array $automatedMessageIds): array {
231+
if ($automatedMessageIds === []) {
232+
return [];
233+
}
234+
235+
$qb = $this->db->getQueryBuilder();
236+
// The backup of a live row carries the same user id with an underscore
237+
// prefix, so the two are matched with a self join on the concatenation.
238+
$qb->select('l.id')
239+
->from($this->tableName, 'l')
240+
->leftJoin('l', $this->tableName, 'b', $qb->expr()->eq(
241+
'b.user_id',
242+
$qb->func()->concat($qb->createNamedParameter('_'), 'l.user_id'),
243+
))
244+
->where($qb->expr()->in('l.message_id', $qb->createNamedParameter($automatedMessageIds, IQueryBuilder::PARAM_STR_ARRAY)))
245+
->andWhere($qb->expr()->isNull('b.id'))
246+
// Skip backup rows on the live side. Testing the prefix rather than
247+
// is_backup keeps this correct for rows where is_backup is NULL, and
248+
// a substring comparison avoids having to escape the underscore for
249+
// a LIKE pattern.
250+
->andWhere($qb->expr()->neq(
251+
$qb->func()->substring('l.user_id', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT), $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT)),
252+
$qb->createNamedParameter('_'),
253+
));
254+
255+
return $this->fetchIds($qb);
256+
}
183257

258+
/**
259+
* @return list<int>
260+
*/
261+
private function fetchIds(IQueryBuilder $qb): array {
184262
$result = $qb->executeQuery();
185-
/** @var array<string, int> $backups live user id => backup row id */
186-
$backups = [];
263+
$ids = [];
187264
while ($row = $result->fetch()) {
188-
// Strip the underscore prefix that was added when creating the backup
189-
$backups[substr((string)$row['user_id'], 1)] = (int)$row['id'];
265+
$ids[] = (int)$row['id'];
190266
}
191267
$result->closeCursor();
192268

193-
if ($backups === []) {
194-
return 0;
195-
}
269+
return $ids;
270+
}
196271

197-
$reachable = [];
198-
if ($automatedMessageIds !== []) {
199-
foreach (array_chunk(array_keys($backups), 1000) as $chunk) {
200-
$qb = $this->db->getQueryBuilder();
201-
// Matching on the exact user id is enough to exclude backup rows,
202-
// since those are always prefixed and user ids cannot start with
203-
// an underscore. Not filtering on is_backup also means a row with
204-
// a NULL is_backup errs towards keeping the backup.
205-
$qb->select('user_id')
206-
->from($this->tableName)
207-
->where($qb->expr()->in('user_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_STR_ARRAY)))
208-
->andWhere($qb->expr()->in('message_id', $qb->createNamedParameter($automatedMessageIds, IQueryBuilder::PARAM_STR_ARRAY)));
209-
210-
$liveResult = $qb->executeQuery();
211-
while ($row = $liveResult->fetch()) {
212-
$reachable[(string)$row['user_id']] = true;
213-
}
214-
$liveResult->closeCursor();
215-
}
216-
}
272+
/**
273+
* Ids of rows where is_backup is NULL. Those predate the column default and
274+
* are invisible to every query that compares is_backup against false.
275+
*
276+
* @return list<int>
277+
*/
278+
public function findStatusesWithoutBackupFlagIds(): array {
279+
$qb = $this->db->getQueryBuilder();
280+
$qb->select('id')
281+
->from($this->tableName)
282+
->where($qb->expr()->isNull('is_backup'));
217283

218-
$stranded = [];
219-
foreach ($backups as $userId => $id) {
220-
if (!isset($reachable[$userId])) {
221-
$stranded[] = $id;
222-
}
223-
}
284+
return $this->fetchIds($qb);
285+
}
224286

225-
if ($stranded === []) {
226-
return 0;
287+
/**
288+
* @param list<int> $ids
289+
* @return int Number of rows that were given an explicit is_backup value
290+
*/
291+
public function normalizeBackupFlagByIds(array $ids): int {
292+
$updated = 0;
293+
foreach (array_chunk($ids, self::MAX_IN_CHUNK) as $chunk) {
294+
$qb = $this->db->getQueryBuilder();
295+
$qb->update($this->tableName)
296+
->set('is_backup', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))
297+
->where($qb->expr()->in('id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)));
298+
$updated += $qb->executeStatement();
227299
}
228300

301+
return $updated;
302+
}
303+
304+
/**
305+
* @param list<int> $ids
306+
* @return int Number of deleted rows
307+
*/
308+
public function deleteByIds(array $ids): int {
229309
$deleted = 0;
230-
foreach (array_chunk($stranded, 1000) as $chunk) {
310+
foreach (array_chunk($ids, self::MAX_IN_CHUNK) as $chunk) {
231311
$qb = $this->db->getQueryBuilder();
232312
$qb->delete($this->tableName)
233313
->where($qb->expr()->in('id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)));
@@ -237,13 +317,6 @@ public function deleteStrandedBackups(array $automatedMessageIds): int {
237317
return $deleted;
238318
}
239319

240-
public function deleteByIds(array $ids): void {
241-
$qb = $this->db->getQueryBuilder();
242-
$qb->delete($this->tableName)
243-
->where($qb->expr()->in('id', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY)));
244-
$qb->executeStatement();
245-
}
246-
247320
/**
248321
* @param string $userId
249322
* @return bool

0 commit comments

Comments
 (0)