diff --git a/apps/user_status/appinfo/info.xml b/apps/user_status/appinfo/info.xml
index 7702671a50248..4a9d95be21a7e 100644
--- a/apps/user_status/appinfo/info.xml
+++ b/apps/user_status/appinfo/info.xml
@@ -29,6 +29,9 @@
OCA\UserStatus\BackgroundJob\ClearOldStatusesBackgroundJob
+
+ OCA\UserStatus\Command\Repair
+
OCA\UserStatus\ContactsMenu\StatusProvider
diff --git a/apps/user_status/composer/composer/autoload_classmap.php b/apps/user_status/composer/composer/autoload_classmap.php
index b57df813bc9c4..5641b449531f2 100644
--- a/apps/user_status/composer/composer/autoload_classmap.php
+++ b/apps/user_status/composer/composer/autoload_classmap.php
@@ -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',
@@ -37,5 +38,6 @@
'OCA\\UserStatus\\ResponseDefinitions' => $baseDir . '/../lib/ResponseDefinitions.php',
'OCA\\UserStatus\\Service\\JSDataService' => $baseDir . '/../lib/Service/JSDataService.php',
'OCA\\UserStatus\\Service\\PredefinedStatusService' => $baseDir . '/../lib/Service/PredefinedStatusService.php',
+ 'OCA\\UserStatus\\Service\\StatusRepairService' => $baseDir . '/../lib/Service/StatusRepairService.php',
'OCA\\UserStatus\\Service\\StatusService' => $baseDir . '/../lib/Service/StatusService.php',
);
diff --git a/apps/user_status/composer/composer/autoload_static.php b/apps/user_status/composer/composer/autoload_static.php
index 17f45ab9bbf58..fceb7d2c0269a 100644
--- a/apps/user_status/composer/composer/autoload_static.php
+++ b/apps/user_status/composer/composer/autoload_static.php
@@ -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',
@@ -52,6 +53,7 @@ class ComposerStaticInitUserStatus
'OCA\\UserStatus\\ResponseDefinitions' => __DIR__ . '/..' . '/../lib/ResponseDefinitions.php',
'OCA\\UserStatus\\Service\\JSDataService' => __DIR__ . '/..' . '/../lib/Service/JSDataService.php',
'OCA\\UserStatus\\Service\\PredefinedStatusService' => __DIR__ . '/..' . '/../lib/Service/PredefinedStatusService.php',
+ 'OCA\\UserStatus\\Service\\StatusRepairService' => __DIR__ . '/..' . '/../lib/Service/StatusRepairService.php',
'OCA\\UserStatus\\Service\\StatusService' => __DIR__ . '/..' . '/../lib/Service/StatusService.php',
);
diff --git a/apps/user_status/lib/BackgroundJob/ClearOldStatusesBackgroundJob.php b/apps/user_status/lib/BackgroundJob/ClearOldStatusesBackgroundJob.php
index 2bce800c069ae..e9a34035613cc 100644
--- a/apps/user_status/lib/BackgroundJob/ClearOldStatusesBackgroundJob.php
+++ b/apps/user_status/lib/BackgroundJob/ClearOldStatusesBackgroundJob.php
@@ -10,6 +10,7 @@
namespace OCA\UserStatus\BackgroundJob;
use OCA\UserStatus\Db\UserStatusMapper;
+use OCA\UserStatus\Service\StatusRepairService;
use OCA\UserStatus\Service\StatusService;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\TimedJob;
@@ -26,10 +27,12 @@ class ClearOldStatusesBackgroundJob extends TimedJob {
*
* @param ITimeFactory $time
* @param UserStatusMapper $mapper
+ * @param StatusRepairService $repairService
*/
public function __construct(
ITimeFactory $time,
private UserStatusMapper $mapper,
+ private StatusRepairService $repairService,
) {
parent::__construct($time);
@@ -45,5 +48,6 @@ protected function run($argument) {
$this->mapper->clearOlderThanClearAt($now);
$this->mapper->clearStatusesOlderThan($now - StatusService::INVALIDATE_STATUS_THRESHOLD, $now);
+ $this->repairService->deleteStrandedBackups();
}
}
diff --git a/apps/user_status/lib/Command/Repair.php b/apps/user_status/lib/Command/Repair.php
new file mode 100644
index 0000000000000..3b10e83b1cf0a
--- /dev/null
+++ b/apps/user_status/lib/Command/Repair.php
@@ -0,0 +1,110 @@
+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('Dry run, no changes will be written.');
+ $output->writeln('');
+ }
+
+ $this->repairMissingBackupFlags($output, $dryRun);
+ $this->repairOrphanedStatuses($output, $dryRun);
+ $this->repairStrandedBackups($output, $dryRun);
+
+ return self::SUCCESS;
+ }
+
+ /** The flag comes from the user id, so a pre-default backup stays a backup. */
+ private function repairMissingBackupFlags(OutputInterface $output, bool $dryRun): void {
+ $ids = $this->repairService->findStatusesWithoutBackupFlagIds();
+ if ($ids === []) {
+ $output->writeln('No statuses with a missing backup flag.');
+ return;
+ }
+
+ $count = count($ids);
+ if ($dryRun) {
+ $output->writeln("Would give $count status(es) an explicit backup flag.");
+ $this->listIds($output, $ids);
+ return;
+ }
+
+ $fixed = $this->repairService->normalizeBackupFlagByIds($ids);
+ $output->writeln("Gave $fixed status(es) an explicit backup flag.");
+ }
+
+ private function repairOrphanedStatuses(OutputInterface $output, bool $dryRun): void {
+ $ids = $this->repairService->findOrphanedAutomatedStatusIds();
+ if ($ids === []) {
+ $output->writeln('No users stuck on an automated status.');
+ return;
+ }
+
+ if ($dryRun) {
+ $output->writeln('Would clear ' . count($ids) . ' status(es) stuck on an automated status.');
+ $this->listIds($output, $ids);
+ return;
+ }
+
+ $deleted = $this->repairService->deleteByIds($ids);
+ $output->writeln("Cleared $deleted status(es) stuck on an automated status.");
+ }
+
+ private function repairStrandedBackups(OutputInterface $output, bool $dryRun): void {
+ $ids = $this->repairService->findStrandedBackupIds();
+ if ($ids === []) {
+ $output->writeln('No stranded backup statuses.');
+ return;
+ }
+
+ if ($dryRun) {
+ $output->writeln('Would remove ' . count($ids) . ' stranded backup status(es).');
+ $this->listIds($output, $ids);
+ return;
+ }
+
+ $deleted = $this->repairService->deleteByIds($ids);
+ $output->writeln("Removed $deleted stranded backup status(es).");
+ }
+
+ /**
+ * @param list $ids
+ */
+ private function listIds(OutputInterface $output, array $ids): void {
+ if ($output->getVerbosity() >= OutputInterface::VERBOSITY_VERBOSE) {
+ $output->writeln(' ids: ' . implode(', ', $ids));
+ }
+ }
+}
diff --git a/apps/user_status/lib/Db/UserStatusMapper.php b/apps/user_status/lib/Db/UserStatusMapper.php
index ea3b76c8e564d..71d2631bffe37 100644
--- a/apps/user_status/lib/Db/UserStatusMapper.php
+++ b/apps/user_status/lib/Db/UserStatusMapper.php
@@ -163,11 +163,135 @@ public function deleteCurrentStatusToRestoreBackup(string $userId, string $messa
return $qb->executeStatement() > 0;
}
- public function deleteByIds(array $ids): void {
+ /**
+ * @param list $automatedMessageIds
+ * @return int Number of deleted backup rows
+ */
+ public function deleteStrandedBackups(array $automatedMessageIds): int {
+ return $this->deleteByIds($this->findStrandedBackupIds($automatedMessageIds));
+ }
+
+ /**
+ * @param list $automatedMessageIds
+ * @return list
+ */
+ public function findStrandedBackupIds(array $automatedMessageIds): array {
+ if ($automatedMessageIds === []) {
+ return [];
+ }
+
$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)));
+
+ // A NULL is_backup counts as live: odd data keeps 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);
+ }
+
+ /**
+ * @param list $automatedMessageIds
+ * @return list
+ */
+ public function findOrphanedAutomatedStatusIds(array $automatedMessageIds): array {
+ if ($automatedMessageIds === []) {
+ return [];
+ }
+
+ $qb = $this->db->getQueryBuilder();
+ $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'))
+ ->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
+ */
+ private function fetchIds(IQueryBuilder $qb): array {
+ $result = $qb->executeQuery();
+ $ids = [];
+ while ($row = $result->fetch()) {
+ $ids[] = (int)$row['id'];
+ }
+ $result->closeCursor();
+
+ return $ids;
+ }
+
+ /**
+ * @return list
+ */
+ public function findStatusesWithoutBackupFlagIds(): array {
+ $qb = $this->db->getQueryBuilder();
+ $qb->select('id')
+ ->from($this->tableName)
+ ->where($qb->expr()->isNull('is_backup'));
+
+ return $this->fetchIds($qb);
+ }
+
+ /**
+ * Takes is_backup from the user id prefix: false for everything would make a
+ * pre-default backup an unrestorable live row called "_alice".
+ *
+ * @param list $ids
+ * @return int Number of rows given an explicit is_backup value
+ */
+ public function normalizeBackupFlagByIds(array $ids): int {
+ $updated = 0;
+ foreach (array_chunk($ids, IQueryBuilder::MAX_IN_PARAMETERS) as $chunk) {
+ foreach ([true, false] as $isBackup) {
+ $qb = $this->db->getQueryBuilder();
+ $firstCharacter = $qb->func()->substring(
+ 'user_id',
+ $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT),
+ $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT),
+ );
+ $underscore = $qb->createNamedParameter('_');
+ $qb->update($this->tableName)
+ ->set('is_backup', $qb->createNamedParameter($isBackup, IQueryBuilder::PARAM_BOOL))
+ ->where($qb->expr()->in('id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)))
+ ->andWhere($isBackup
+ ? $qb->expr()->eq($firstCharacter, $underscore)
+ : $qb->expr()->neq($firstCharacter, $underscore));
+ $updated += $qb->executeStatement();
+ }
+ }
+
+ return $updated;
+ }
+
+ /**
+ * @param list $ids
+ * @return int Number of deleted rows
+ */
+ public function deleteByIds(array $ids): int {
+ $deleted = 0;
+ foreach (array_chunk($ids, IQueryBuilder::MAX_IN_PARAMETERS) 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;
}
/**
@@ -187,13 +311,20 @@ public function createBackupStatus(string $userId): bool {
return $qb->executeStatement() > 0;
}
- public function restoreBackupStatuses(array $ids): void {
- $qb = $this->db->getQueryBuilder();
- $qb->update($this->tableName)
- ->set('is_backup', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))
- ->set('user_id', $qb->func()->substring('user_id', $qb->createNamedParameter(2, IQueryBuilder::PARAM_INT)))
- ->where($qb->expr()->in('id', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY)));
+ /**
+ * @param list $ids
+ * @param int $statusTimestamp The backed up one would already be stale.
+ */
+ public function restoreBackupStatuses(array $ids, int $statusTimestamp): void {
+ foreach (array_chunk($ids, IQueryBuilder::MAX_IN_PARAMETERS) as $chunk) {
+ $qb = $this->db->getQueryBuilder();
+ $qb->update($this->tableName)
+ ->set('is_backup', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))
+ ->set('status_timestamp', $qb->createNamedParameter($statusTimestamp, IQueryBuilder::PARAM_INT))
+ ->set('user_id', $qb->func()->substring('user_id', $qb->createNamedParameter(2, IQueryBuilder::PARAM_INT)))
+ ->where($qb->expr()->in('id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)));
- $qb->executeStatement();
+ $qb->executeStatement();
+ }
}
}
diff --git a/apps/user_status/lib/Service/StatusRepairService.php b/apps/user_status/lib/Service/StatusRepairService.php
new file mode 100644
index 0000000000000..51f7e8a11150c
--- /dev/null
+++ b/apps/user_status/lib/Service/StatusRepairService.php
@@ -0,0 +1,67 @@
+
+ */
+ public function findStrandedBackupIds(): array {
+ return $this->mapper->findStrandedBackupIds(StatusService::AUTOMATED_MESSAGE_IDS);
+ }
+
+ /**
+ * @return int Number of deleted backup rows
+ */
+ public function deleteStrandedBackups(): int {
+ return $this->mapper->deleteStrandedBackups(StatusService::AUTOMATED_MESSAGE_IDS);
+ }
+
+ /**
+ * @return list
+ */
+ public function findOrphanedAutomatedStatusIds(): array {
+ return $this->mapper->findOrphanedAutomatedStatusIds(StatusService::AUTOMATED_MESSAGE_IDS);
+ }
+
+ /**
+ * @return list
+ */
+ public function findStatusesWithoutBackupFlagIds(): array {
+ return $this->mapper->findStatusesWithoutBackupFlagIds();
+ }
+
+ /**
+ * @param list $ids
+ * @return int Number of rows given an explicit is_backup value
+ */
+ public function normalizeBackupFlagByIds(array $ids): int {
+ return $this->mapper->normalizeBackupFlagByIds($ids);
+ }
+
+ /**
+ * @param list $ids
+ * @return int Number of deleted rows
+ */
+ public function deleteByIds(array $ids): int {
+ return $this->mapper->deleteByIds($ids);
+ }
+}
diff --git a/apps/user_status/lib/Service/StatusService.php b/apps/user_status/lib/Service/StatusService.php
index 6c1805963c881..bbe28a478538e 100644
--- a/apps/user_status/lib/Service/StatusService.php
+++ b/apps/user_status/lib/Service/StatusService.php
@@ -17,9 +17,11 @@
use OCA\UserStatus\Exception\InvalidStatusTypeException;
use OCA\UserStatus\Exception\StatusMessageTooLongException;
use OCP\AppFramework\Db\DoesNotExistException;
+use OCP\AppFramework\Db\TTransactional;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\DB\Exception;
use OCP\IConfig;
+use OCP\IDBConnection;
use OCP\IEmojiHelper;
use OCP\IUserManager;
use OCP\UserStatus\IUserStatus;
@@ -32,6 +34,8 @@
* @package OCA\UserStatus\Service
*/
class StatusService {
+ use TTransactional;
+
private bool $shareeEnumeration;
private bool $shareeEnumerationInGroupOnly;
private bool $shareeEnumerationPhone;
@@ -59,6 +63,17 @@ class StatusService {
IUserStatus::INVISIBLE,
];
+ /**
+ * Message ids only ever set by an automation, expected to be reverted.
+ */
+ 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 */;
@@ -73,6 +88,7 @@ public function __construct(
private IConfig $config,
private IUserManager $userManager,
private LoggerInterface $logger,
+ private IDBConnection $connection,
) {
$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
$this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
@@ -525,67 +541,83 @@ public function backupCurrentStatus(string $userId): bool {
}
}
+ /**
+ * Looking up the backup, dropping the automated status and promoting the backup
+ * back to the live row is one unit: in between, the user has no live row and the
+ * backup looks stranded to the cleanup job, which would delete it.
+ */
public function revertUserStatus(string $userId, string $messageId, bool $revertedManually = false): ?UserStatus {
- try {
- /** @var UserStatus $userStatus */
- $backupUserStatus = $this->mapper->findByUserId($userId, true);
- } catch (DoesNotExistException $ex) {
- // No user status to revert, do nothing
- return null;
- }
+ return $this->atomic(function () use ($userId, $messageId, $revertedManually): ?UserStatus {
+ try {
+ /** @var UserStatus $userStatus */
+ $backupUserStatus = $this->mapper->findByUserId($userId, true);
+ } catch (DoesNotExistException $ex) {
+ // No backup, but the status must go or the user stays stuck on it.
+ 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;
+ }
- $deleted = $this->mapper->deleteCurrentStatusToRestoreBackup($userId, $messageId);
- if (!$deleted) {
- $this->logger->debug('Status revert skipped for user ' . $userId . ': current status does not match messageId "' . $messageId . '" (user may have changed status manually)', ['app' => 'user_status']);
- return null;
- }
+ $deleted = $this->mapper->deleteCurrentStatusToRestoreBackup($userId, $messageId);
+ if (!$deleted) {
+ $this->logger->debug('Status revert skipped for user ' . $userId . ': current status does not match messageId "' . $messageId . '" (user may have changed status manually)', ['app' => 'user_status']);
+ return null;
+ }
- if ($revertedManually) {
- if ($backupUserStatus->getStatus() === IUserStatus::OFFLINE) {
+ if ($revertedManually && $backupUserStatus->getStatus() === IUserStatus::OFFLINE) {
// When the user reverts the status manually they are online
$backupUserStatus->setStatus(IUserStatus::ONLINE);
}
+
+ // Stale after a long meeting otherwise, which reads as offline.
$backupUserStatus->setStatusTimestamp($this->timeFactory->getTime());
- }
- $backupUserStatus->setIsBackup(false);
- // Remove the underscore prefix added when creating the backup
- $backupUserStatus->setUserId(substr($backupUserStatus->getUserId(), 1));
- $this->mapper->update($backupUserStatus);
+ $backupUserStatus->setIsBackup(false);
+ // Remove the underscore prefix added when creating the backup
+ $backupUserStatus->setUserId(substr($backupUserStatus->getUserId(), 1));
+ $this->mapper->update($backupUserStatus);
- return $backupUserStatus;
+ return $backupUserStatus;
+ }, $this->connection);
}
+ /**
+ * Same unit as revertUserStatus(), for the bulk path: the ids are read, the
+ * automated statuses deleted and the backups restored in one transaction, so
+ * the cleanup job never sees a backup without its live row.
+ */
public function revertMultipleUserStatus(array $userIds, string $messageId): void {
- // Get all user statuses and the backups
- $findById = $userIds;
- foreach ($userIds as $userId) {
- $findById[] = '_' . $userId;
- }
- $userStatuses = $this->mapper->findByUserIds($findById);
-
- $backups = $restoreIds = $statuesToDelete = [];
- foreach ($userStatuses as $userStatus) {
- if (!$userStatus->getIsBackup()
- && $userStatus->getMessageId() === $messageId) {
- $statuesToDelete[$userStatus->getUserId()] = $userStatus->getId();
- } elseif ($userStatus->getIsBackup()) {
- $backups[$userStatus->getUserId()] = $userStatus->getId();
+ $this->atomic(function () use ($userIds, $messageId): void {
+ // Get all user statuses and the backups
+ $findById = $userIds;
+ foreach ($userIds as $userId) {
+ $findById[] = '_' . $userId;
+ }
+ $userStatuses = $this->mapper->findByUserIds($findById);
+
+ $backups = $restoreIds = $statuesToDelete = [];
+ foreach ($userStatuses as $userStatus) {
+ if (!$userStatus->getIsBackup()
+ && $userStatus->getMessageId() === $messageId) {
+ $statuesToDelete[$userStatus->getUserId()] = $userStatus->getId();
+ } elseif ($userStatus->getIsBackup()) {
+ $backups[$userStatus->getUserId()] = $userStatus->getId();
+ }
}
- }
- // For users with both (normal and backup) delete the status when matching
- foreach ($statuesToDelete as $userId => $statusId) {
- $backupUserId = '_' . $userId;
- if (isset($backups[$backupUserId])) {
- $restoreIds[] = $backups[$backupUserId];
+ // For users with both (normal and backup) delete the status when matching
+ foreach ($statuesToDelete as $userId => $statusId) {
+ $backupUserId = '_' . $userId;
+ if (isset($backups[$backupUserId])) {
+ $restoreIds[] = $backups[$backupUserId];
+ }
}
- }
- $this->mapper->deleteByIds(array_values($statuesToDelete));
+ $this->mapper->deleteByIds(array_values($statuesToDelete));
- // For users that matched restore the previous status
- $this->mapper->restoreBackupStatuses($restoreIds);
+ $this->mapper->restoreBackupStatuses($restoreIds, $this->timeFactory->getTime());
+ }, $this->connection);
}
protected function insertWithoutThrowingUniqueConstrain(UserStatus $userStatus): UserStatus {
diff --git a/apps/user_status/tests/Integration/Service/StatusServiceIntegrationTest.php b/apps/user_status/tests/Integration/Service/StatusServiceIntegrationTest.php
index a2acfe4458b7d..126813f47e3cf 100644
--- a/apps/user_status/tests/Integration/Service/StatusServiceIntegrationTest.php
+++ b/apps/user_status/tests/Integration/Service/StatusServiceIntegrationTest.php
@@ -9,6 +9,9 @@
namespace OCA\UserStatus\Tests\Integration\Service;
+use OCA\UserStatus\Db\UserStatus;
+use OCA\UserStatus\Db\UserStatusMapper;
+use OCA\UserStatus\Service\StatusRepairService;
use OCA\UserStatus\Service\StatusService;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\IDBConnection;
@@ -22,17 +25,39 @@
class StatusServiceIntegrationTest extends TestCase {
private StatusService $service;
+ private StatusRepairService $repairService;
+ private UserStatusMapper $mapper;
+ private IDBConnection $db;
protected function setUp(): void {
parent::setUp();
$this->service = Server::get(StatusService::class);
+ $this->repairService = Server::get(StatusRepairService::class);
+ $this->mapper = Server::get(UserStatusMapper::class);
- $db = Server::get(IDBConnection::class);
- $qb = $db->getQueryBuilder();
+ $this->db = Server::get(IDBConnection::class);
+ $qb = $this->db->getQueryBuilder();
$qb->delete('user_status')->executeStatement();
}
+ /** Reads a row without processStatus() rewriting a stale status first. */
+ private function readRaw(string $userId): ?UserStatus {
+ try {
+ return $this->mapper->findByUserId($userId);
+ } catch (DoesNotExistException) {
+ return null;
+ }
+ }
+
+ /** Simulates elapsed time by ageing the stored timestamps backwards. */
+ private function age(string $userId, int $seconds): void {
+ $this->db->executeStatement(
+ 'UPDATE `*PREFIX*user_status` SET `status_timestamp` = `status_timestamp` - ? WHERE `user_id` IN (?, ?)',
+ [$seconds, $userId, '_' . $userId],
+ );
+ }
+
public function testNoStatusYet(): void {
$this->expectException(DoesNotExistException::class);
@@ -191,4 +216,329 @@ public function testOtherAutomationsDoNotOverwriteEachOther(): void {
$this->service->findByUserId('test123')->getMessageId(),
);
}
+
+ /* An automated status with no backup must still be cleared. */
+
+ public function testRevertWithoutBackupClearsAutomatedStatus(): void {
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ false,
+ );
+ self::assertSame(
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ $this->readRaw('test123')?->getMessageId(),
+ );
+
+ $reverted = $this->service->revertUserStatus('test123', IUserStatus::MESSAGE_CALENDAR_BUSY);
+
+ self::assertNull($reverted, 'Nothing can be restored without a backup');
+ self::assertNull(
+ $this->readRaw('test123'),
+ 'The unreachable automated status must be cleared, not left behind',
+ );
+ }
+
+ public function testRevertWithoutBackupKeepsStatusTheUserChangedThemselves(): void {
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ false,
+ );
+ // The user replaces the automated message with their own.
+ $this->service->setCustomMessage('test123', '🍕', 'Lunch', null);
+
+ $reverted = $this->service->revertUserStatus('test123', IUserStatus::MESSAGE_CALENDAR_BUSY);
+
+ self::assertNull($reverted);
+ $status = $this->readRaw('test123');
+ self::assertNotNull($status, 'A status the user set themselves must not be deleted');
+ self::assertSame('Lunch', $status->getCustomMessage());
+ self::assertNull($status->getMessageId());
+ }
+
+ public function testRevertWithoutBackupKeepsOtherAutomatedStatus(): void {
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALL,
+ false,
+ );
+
+ $reverted = $this->service->revertUserStatus('test123', IUserStatus::MESSAGE_CALENDAR_BUSY);
+
+ self::assertNull($reverted);
+ self::assertSame(
+ IUserStatus::MESSAGE_CALL,
+ $this->readRaw('test123')?->getMessageId(),
+ 'An unrelated automated status must be left alone',
+ );
+ }
+
+ public function testFreshUserAutomatedStatusIsClearedOnRevert(): void {
+ $applied = $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ true,
+ );
+
+ self::assertNotNull($applied, 'A user without a previous status should still get the meeting status');
+ self::assertNull($this->readRaw('_test123'), 'There was no status to back up');
+
+ $this->service->revertUserStatus('test123', IUserStatus::MESSAGE_CALENDAR_BUSY);
+
+ self::assertNull(
+ $this->readRaw('test123'),
+ 'The meeting status must be cleared when the meeting ends',
+ );
+ }
+
+ public function testRevertAfterLongMeetingRefreshesTimestamp(): void {
+ $this->service->setStatus('test123', IUserStatus::ONLINE, null, false);
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ true,
+ );
+
+ $this->age('test123', StatusService::INVALIDATE_STATUS_THRESHOLD * 2);
+
+ $before = time();
+ $reverted = $this->service->revertUserStatus('test123', IUserStatus::MESSAGE_CALENDAR_BUSY);
+
+ self::assertNotNull($reverted);
+ self::assertGreaterThanOrEqual(
+ $before,
+ $this->readRaw('test123')?->getStatusTimestamp(),
+ 'A restored status must not carry the stale timestamp from before the meeting',
+ );
+ }
+
+ public function testRevertAfterLongMeetingDoesNotFallBackToOffline(): void {
+ $this->service->setStatus('test123', IUserStatus::ONLINE, null, false);
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ true,
+ );
+ $this->age('test123', StatusService::INVALIDATE_STATUS_THRESHOLD * 2);
+
+ $this->service->revertUserStatus('test123', IUserStatus::MESSAGE_CALENDAR_BUSY);
+
+ // findByUserId() runs processStatus(), which cleans stale statuses.
+ self::assertSame(
+ IUserStatus::ONLINE,
+ $this->service->findByUserId('test123')->getStatus(),
+ 'The user was online before the meeting and must not be flipped to offline by reading the status',
+ );
+ }
+
+ public function testRevertMultipleAfterLongMeetingRefreshesTimestamp(): void {
+ $this->service->setStatus('test123', IUserStatus::ONLINE, null, false);
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ true,
+ );
+
+ $this->age('test123', StatusService::INVALIDATE_STATUS_THRESHOLD * 2);
+
+ $before = time();
+ $this->service->revertMultipleUserStatus(['test123'], IUserStatus::MESSAGE_CALENDAR_BUSY);
+
+ self::assertGreaterThanOrEqual(
+ $before,
+ $this->readRaw('test123')?->getStatusTimestamp(),
+ 'A bulk-restored status must not carry the stale timestamp from before the meeting',
+ );
+ }
+
+ public function testRevertMultipleAfterLongMeetingDoesNotFallBackToOffline(): void {
+ $this->service->setStatus('test123', IUserStatus::ONLINE, null, false);
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ true,
+ );
+ $this->age('test123', StatusService::INVALIDATE_STATUS_THRESHOLD * 2);
+
+ $this->service->revertMultipleUserStatus(['test123'], IUserStatus::MESSAGE_CALENDAR_BUSY);
+
+ // findByUserId() runs processStatus(), which cleans stale statuses.
+ self::assertSame(
+ IUserStatus::ONLINE,
+ $this->service->findByUserId('test123')->getStatus(),
+ 'The user was online before the meeting and must not be flipped to offline after bulk status update followed by reading the status',
+ );
+ }
+
+ public function testRevertMultipleWithoutBackupClearsAutomatedStatus(): void {
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ true,
+ );
+
+ $this->service->revertMultipleUserStatus(['test123'], IUserStatus::MESSAGE_CALENDAR_BUSY);
+
+ self::assertNull(
+ $this->readRaw('test123'),
+ 'A bulk revert must clear an automated status that has no backup, or the user stays stuck on it',
+ );
+ }
+
+ public function testRevertMultipleKeepsStatusTheUserChangedThemselves(): void {
+ $this->service->setStatus('test123', IUserStatus::ONLINE, null, false);
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ true,
+ );
+ // The user replaces the automated message with their own.
+ $this->service->setCustomMessage('test123', '🍕', 'Lunch', null);
+
+ $this->service->revertMultipleUserStatus(['test123'], IUserStatus::MESSAGE_CALENDAR_BUSY);
+
+ self::assertSame(
+ 'Lunch',
+ $this->readRaw('test123')?->getCustomMessage(),
+ 'A status the user set themselves must survive the bulk revert',
+ );
+ }
+
+ /* A backup whose live row moved off the automated status is stranded. */
+
+ public function testStrandedBackupIsCleanedUp(): void {
+ $this->service->setStatus('test123', IUserStatus::ONLINE, null, false);
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ true,
+ );
+ $this->service->clearMessage('test123');
+ self::assertNotNull($this->readRaw('_test123'), 'Precondition: the backup is stranded');
+
+ $deleted = $this->repairService->deleteStrandedBackups();
+
+ self::assertSame(1, $deleted);
+ self::assertNull($this->readRaw('_test123'), 'The stranded backup must be removed');
+ self::assertNotNull($this->readRaw('test123'), 'The live status must be untouched');
+ }
+
+ public function testBackupOfAnOngoingMeetingSurvivesCleanup(): void {
+ $this->service->setStatus('test123', IUserStatus::ONLINE, null, false);
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ true,
+ );
+
+ $deleted = $this->repairService->deleteStrandedBackups();
+
+ self::assertSame(0, $deleted);
+ self::assertNotNull(
+ $this->readRaw('_test123'),
+ 'The backup for a meeting that is still running must survive',
+ );
+ }
+
+ public function testLongOutOfOfficeBackupSurvivesCleanup(): void {
+ $this->service->setStatus('test123', IUserStatus::ONLINE, null, false);
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::DND,
+ IUserStatus::MESSAGE_OUT_OF_OFFICE,
+ true,
+ );
+ $this->age('test123', StatusService::INVALIDATE_STATUS_THRESHOLD * 100);
+
+ $deleted = $this->repairService->deleteStrandedBackups();
+
+ self::assertSame(0, $deleted);
+ self::assertNotNull(
+ $this->readRaw('_test123'),
+ 'A long running out-of-office backup must not be treated as stranded',
+ );
+ }
+
+ public function testAutomatedStatusWorksAgainAfterStrandedBackupCleanup(): void {
+ $this->service->setStatus('test123', IUserStatus::ONLINE, null, false);
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ true,
+ );
+ $this->service->clearMessage('test123');
+
+ self::assertNull(
+ $this->service->setUserStatus('test123', IUserStatus::BUSY, IUserStatus::MESSAGE_CALL, true),
+ 'Precondition: the stranded backup blocks automated statuses',
+ );
+
+ $this->repairService->deleteStrandedBackups();
+
+ self::assertNotNull(
+ $this->service->setUserStatus('test123', IUserStatus::BUSY, IUserStatus::MESSAGE_CALL, true),
+ 'Automated statuses must work again once the stranded backup is gone',
+ );
+ }
+
+ public function testCleanupLeavesUsersWithoutBackupsAlone(): void {
+ $this->service->setStatus('test123', IUserStatus::ONLINE, null, false);
+ $this->service->setCustomMessage('test123', '🍕', 'Lunch', null);
+
+ $deleted = $this->repairService->deleteStrandedBackups();
+
+ self::assertSame(0, $deleted);
+ self::assertSame('Lunch', $this->readRaw('test123')?->getCustomMessage());
+ }
+
+ /** The prefix concatenation is SQL, so it needs a real database. */
+ public function testFindsOrphanedAutomatedStatusOnARealDatabase(): void {
+ $this->service->setUserStatus(
+ 'test123',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ true,
+ );
+ self::assertNull($this->readRaw('_test123'), 'Precondition: there is no backup');
+
+ $this->service->setStatus('test456', IUserStatus::ONLINE, null, false);
+ $this->service->setUserStatus(
+ 'test456',
+ IUserStatus::BUSY,
+ IUserStatus::MESSAGE_CALENDAR_BUSY,
+ true,
+ );
+
+ $orphaned = $this->repairService->findOrphanedAutomatedStatusIds();
+
+ self::assertSame([$this->readRaw('test123')?->getId()], $orphaned);
+ }
+
+ public function testFindsStatusesWithoutBackupFlagOnARealDatabase(): void {
+ $this->service->setStatus('test123', IUserStatus::ONLINE, null, false);
+ $this->db->executeStatement(
+ 'UPDATE `*PREFIX*user_status` SET `is_backup` = NULL WHERE `user_id` = ?',
+ ['test123'],
+ );
+
+ $ids = $this->mapper->findStatusesWithoutBackupFlagIds();
+
+ self::assertSame([$this->readRaw('test123')?->getId()], $ids);
+ self::assertSame(1, $this->mapper->normalizeBackupFlagByIds($ids));
+ self::assertSame([], $this->mapper->findStatusesWithoutBackupFlagIds());
+ }
}
diff --git a/apps/user_status/tests/Unit/BackgroundJob/ClearOldStatusesBackgroundJobTest.php b/apps/user_status/tests/Unit/BackgroundJob/ClearOldStatusesBackgroundJobTest.php
index 6d19d19e2702a..d668a53b7ab1b 100644
--- a/apps/user_status/tests/Unit/BackgroundJob/ClearOldStatusesBackgroundJobTest.php
+++ b/apps/user_status/tests/Unit/BackgroundJob/ClearOldStatusesBackgroundJobTest.php
@@ -11,6 +11,7 @@
use OCA\UserStatus\BackgroundJob\ClearOldStatusesBackgroundJob;
use OCA\UserStatus\Db\UserStatusMapper;
+use OCA\UserStatus\Service\StatusRepairService;
use OCP\AppFramework\Utility\ITimeFactory;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;
@@ -18,6 +19,7 @@
class ClearOldStatusesBackgroundJobTest extends TestCase {
private ITimeFactory&MockObject $time;
private UserStatusMapper&MockObject $mapper;
+ private StatusRepairService&MockObject $repairService;
private ClearOldStatusesBackgroundJob $job;
protected function setUp(): void {
@@ -25,8 +27,9 @@ protected function setUp(): void {
$this->time = $this->createMock(ITimeFactory::class);
$this->mapper = $this->createMock(UserStatusMapper::class);
+ $this->repairService = $this->createMock(StatusRepairService::class);
- $this->job = new ClearOldStatusesBackgroundJob($this->time, $this->mapper);
+ $this->job = new ClearOldStatusesBackgroundJob($this->time, $this->mapper, $this->repairService);
}
public function testRun(): void {
@@ -36,6 +39,8 @@ public function testRun(): void {
$this->mapper->expects($this->once())
->method('clearStatusesOlderThan')
->with(437, 1337);
+ $this->repairService->expects($this->once())
+ ->method('deleteStrandedBackups');
$this->time->method('getTime')
->willReturn(1337);
diff --git a/apps/user_status/tests/Unit/Command/RepairTest.php b/apps/user_status/tests/Unit/Command/RepairTest.php
new file mode 100644
index 0000000000000..86e6d39988a46
--- /dev/null
+++ b/apps/user_status/tests/Unit/Command/RepairTest.php
@@ -0,0 +1,82 @@
+repairService = $this->createMock(StatusRepairService::class);
+ $this->tester = new CommandTester(new Repair($this->repairService));
+ }
+
+ public function testRepairsEverything(): void {
+ $this->repairService->expects($this->once())
+ ->method('findStatusesWithoutBackupFlagIds')
+ ->willReturn([1, 2]);
+ $this->repairService->expects($this->once())
+ ->method('normalizeBackupFlagByIds')
+ ->with([1, 2])
+ ->willReturn(2);
+
+ $this->repairService->expects($this->once())
+ ->method('findOrphanedAutomatedStatusIds')
+ ->willReturn([7, 8, 9]);
+ $this->repairService->expects($this->once())
+ ->method('findStrandedBackupIds')
+ ->willReturn([11, 12, 13, 14]);
+ $this->repairService->expects($this->exactly(2))
+ ->method('deleteByIds')
+ ->willReturnCallback(static fn (array $ids): int => count($ids));
+
+ self::assertSame(Command::SUCCESS, $this->tester->execute([]));
+
+ $display = $this->tester->getDisplay();
+ self::assertStringContainsString('2', $display);
+ self::assertStringContainsString('3', $display);
+ self::assertStringContainsString('4', $display);
+ }
+
+ public function testDryRunChangesNothing(): void {
+ $this->repairService->method('findStatusesWithoutBackupFlagIds')->willReturn([1, 2]);
+ $this->repairService->method('findOrphanedAutomatedStatusIds')->willReturn([7, 8, 9]);
+ $this->repairService->method('findStrandedBackupIds')->willReturn([11, 12, 13, 14]);
+
+ $this->repairService->expects($this->never())->method('normalizeBackupFlagByIds');
+ $this->repairService->expects($this->never())->method('deleteByIds');
+ $this->repairService->expects($this->never())->method('deleteStrandedBackups');
+
+ self::assertSame(Command::SUCCESS, $this->tester->execute(['--dry-run' => true]));
+
+ self::assertStringContainsString('dry run', strtolower($this->tester->getDisplay()));
+ }
+
+ public function testNothingToRepair(): void {
+ $this->repairService->method('findStatusesWithoutBackupFlagIds')->willReturn([]);
+ $this->repairService->method('findOrphanedAutomatedStatusIds')->willReturn([]);
+ $this->repairService->method('findStrandedBackupIds')->willReturn([]);
+
+ // Nothing to normalise and nothing to delete.
+ $this->repairService->expects($this->never())->method('normalizeBackupFlagByIds');
+ $this->repairService->expects($this->never())->method('deleteByIds');
+
+ self::assertSame(Command::SUCCESS, $this->tester->execute([]));
+ }
+}
diff --git a/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php b/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php
index a697571bd5709..c3c93b449c369 100644
--- a/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php
+++ b/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php
@@ -388,12 +388,13 @@ public function testRestoreBackupStatuses(): void {
$userStatus3->setClearAt(50000);
$this->mapper->insert($userStatus3);
- $this->mapper->restoreBackupStatuses([$userStatus1->getId(), $userStatus2->getId()]);
+ $this->mapper->restoreBackupStatuses([$userStatus1->getId(), $userStatus2->getId()], 123456);
$user1Status = $this->mapper->findByUserId('user1', false);
$this->assertEquals('user1', $user1Status->getUserId());
$this->assertEquals(false, $user1Status->getIsBackup());
$this->assertEquals('Releasing', $user1Status->getCustomMessage());
+ $this->assertSame(123456, $user1Status->getStatusTimestamp(), 'A restored status becomes current, so it carries the restore timestamp');
$user2Status = $this->mapper->findByUserId('user2', false);
$this->assertEquals('user2', $user2Status->getUserId());
@@ -405,4 +406,232 @@ public function testRestoreBackupStatuses(): void {
$this->assertEquals(true, $user3Status->getIsBackup());
$this->assertEquals('Vacationing', $user3Status->getCustomMessage());
}
+
+ /**
+ * @param string[] $liveMessageIds keyed by user id; null means no live row
+ */
+ private function insertBackupWithLiveStatus(string $userId, ?string $liveMessageId): void {
+ $backup = new UserStatus();
+ $backup->setUserId('_' . $userId);
+ $backup->setStatus('online');
+ $backup->setStatusTimestamp(5000);
+ $backup->setIsUserDefined(false);
+ $backup->setIsBackup(true);
+ $this->mapper->insert($backup);
+
+ if ($liveMessageId === null) {
+ return;
+ }
+
+ $live = new UserStatus();
+ $live->setUserId($userId);
+ $live->setStatus('busy');
+ $live->setStatusTimestamp(6000);
+ $live->setIsUserDefined(true);
+ $live->setIsBackup(false);
+ $live->setMessageId($liveMessageId === '' ? null : $liveMessageId);
+ $this->mapper->insert($live);
+ }
+
+ public function testDeleteStrandedBackupsWithNoBackups(): void {
+ $this->insertSampleStatuses();
+
+ $this->assertSame(0, $this->mapper->deleteStrandedBackups(['meeting', 'call']));
+ $this->assertCount(3, $this->mapper->findAll());
+ }
+
+ public function testDeleteStrandedBackupsKeepsBackupsOfAutomatedStatuses(): void {
+ $this->insertBackupWithLiveStatus('user1', 'meeting');
+ $this->insertBackupWithLiveStatus('user2', 'call');
+ $this->insertBackupWithLiveStatus('user3', 'availability');
+ $this->insertBackupWithLiveStatus('user4', 'out-of-office');
+
+ $deleted = $this->mapper->deleteStrandedBackups(['meeting', 'call', 'availability', 'out-of-office']);
+
+ $this->assertSame(0, $deleted);
+ foreach (['user1', 'user2', 'user3', 'user4'] as $userId) {
+ $this->assertEquals('_' . $userId, $this->mapper->findByUserId($userId, true)->getUserId());
+ }
+ }
+
+ public function testDeleteStrandedBackupsRemovesBackupWithoutLiveStatus(): void {
+ $this->insertBackupWithLiveStatus('user1', null);
+
+ $deleted = $this->mapper->deleteStrandedBackups(['meeting', 'call']);
+
+ $this->assertSame(1, $deleted);
+ $this->expectException(DoesNotExistException::class);
+ $this->mapper->findByUserId('user1', true);
+ }
+
+ public function testDeleteStrandedBackupsRemovesBackupWhenLiveStatusHasNoMessageId(): void {
+ $this->insertBackupWithLiveStatus('user1', '');
+
+ $deleted = $this->mapper->deleteStrandedBackups(['meeting', 'call']);
+
+ $this->assertSame(1, $deleted);
+ $this->assertEquals('user1', $this->mapper->findByUserId('user1')->getUserId());
+ }
+
+ public function testDeleteStrandedBackupsRemovesBackupWhenLiveStatusIsUserDefinedMessage(): void {
+ $this->insertBackupWithLiveStatus('user1', 'vacationing');
+
+ $deleted = $this->mapper->deleteStrandedBackups(['meeting', 'call']);
+
+ $this->assertSame(1, $deleted);
+ $this->assertEquals('vacationing', $this->mapper->findByUserId('user1')->getMessageId());
+ }
+
+ public function testDeleteStrandedBackupsOnlyRemovesTheStrandedOnes(): void {
+ $this->insertBackupWithLiveStatus('keepme', 'meeting');
+ $this->insertBackupWithLiveStatus('stranded1', 'vacationing');
+ $this->insertBackupWithLiveStatus('stranded2', null);
+ $this->insertBackupWithLiveStatus('keepme2', 'call');
+
+ $deleted = $this->mapper->deleteStrandedBackups(['meeting', 'call']);
+
+ $this->assertSame(2, $deleted);
+ $this->assertEquals('_keepme', $this->mapper->findByUserId('keepme', true)->getUserId());
+ $this->assertEquals('_keepme2', $this->mapper->findByUserId('keepme2', true)->getUserId());
+ foreach (['stranded1', 'stranded2'] as $userId) {
+ try {
+ $this->mapper->findByUserId($userId, true);
+ $this->fail("Backup for $userId should have been deleted");
+ } catch (DoesNotExistException) {
+ }
+ }
+ }
+
+ public function testDeleteStrandedBackupsDoesNotConfuseUsersWithSimilarNames(): void {
+ // 'user1x' is a real user, judged on its own live row, not user1's.
+ $this->insertBackupWithLiveStatus('user1', 'meeting');
+ $this->insertBackupWithLiveStatus('user1x', 'vacationing');
+
+ $deleted = $this->mapper->deleteStrandedBackups(['meeting', 'call']);
+
+ $this->assertSame(1, $deleted);
+ $this->assertEquals('_user1', $this->mapper->findByUserId('user1', true)->getUserId());
+ $this->expectException(DoesNotExistException::class);
+ $this->mapper->findByUserId('user1x', true);
+ }
+
+ public function testFindStrandedBackupIds(): void {
+ $this->insertBackupWithLiveStatus('keepme', 'meeting');
+ $this->insertBackupWithLiveStatus('stranded', 'vacationing');
+
+ $ids = $this->mapper->findStrandedBackupIds(['meeting', 'call']);
+
+ $this->assertCount(1, $ids);
+ $this->assertSame(
+ $this->mapper->findByUserId('stranded', true)->getId(),
+ $ids[0],
+ );
+ }
+
+ public function testFindStrandedBackupIdsDoesNotDelete(): void {
+ $this->insertBackupWithLiveStatus('stranded', 'vacationing');
+
+ $this->mapper->findStrandedBackupIds(['meeting']);
+
+ $this->assertEquals('_stranded', $this->mapper->findByUserId('stranded', true)->getUserId());
+ }
+
+ public function testFindOrphanedAutomatedStatusIds(): void {
+ $orphan = new UserStatus();
+ $orphan->setUserId('orphan');
+ $orphan->setStatus('busy');
+ $orphan->setStatusTimestamp(5000);
+ $orphan->setIsUserDefined(true);
+ $orphan->setIsBackup(false);
+ $orphan->setMessageId('meeting');
+ $this->mapper->insert($orphan);
+
+ $this->insertBackupWithLiveStatus('inmeeting', 'meeting');
+
+ $own = new UserStatus();
+ $own->setUserId('ownstatus');
+ $own->setStatus('dnd');
+ $own->setStatusTimestamp(5000);
+ $own->setIsUserDefined(true);
+ $own->setIsBackup(false);
+ $own->setMessageId('vacationing');
+ $this->mapper->insert($own);
+
+ $ids = $this->mapper->findOrphanedAutomatedStatusIds(['meeting', 'call']);
+
+ $this->assertCount(1, $ids);
+ $this->assertSame($this->mapper->findByUserId('orphan')->getId(), $ids[0]);
+ }
+
+ public function testFindOrphanedAutomatedStatusIdsIgnoresBackupRows(): void {
+ // A backup carrying an automated id is not an orphaned live status.
+ $backup = new UserStatus();
+ $backup->setUserId('_someone');
+ $backup->setStatus('busy');
+ $backup->setStatusTimestamp(5000);
+ $backup->setIsUserDefined(true);
+ $backup->setIsBackup(true);
+ $backup->setMessageId('meeting');
+ $this->mapper->insert($backup);
+
+ $this->assertSame([], $this->mapper->findOrphanedAutomatedStatusIds(['meeting', 'call']));
+ }
+
+ public function testFindOrphanedAutomatedStatusIdsWithEmptyAutomatedList(): void {
+ $this->insertBackupWithLiveStatus('user1', 'meeting');
+
+ $this->assertSame([], $this->mapper->findOrphanedAutomatedStatusIds([]));
+ }
+
+ public function testNormalizeBackupFlag(): void {
+ $this->insertSampleStatuses();
+ self::$realDatabase->executeStatement(
+ 'UPDATE `*PREFIX*user_status` SET `is_backup` = NULL WHERE `user_id` = ?',
+ ['user1'],
+ );
+
+ $ids = $this->mapper->findStatusesWithoutBackupFlagIds();
+ $this->assertCount(1, $ids);
+ $this->assertSame(1, $this->mapper->normalizeBackupFlagByIds($ids));
+ $this->assertSame([], $this->mapper->findStatusesWithoutBackupFlagIds());
+ $this->assertCount(3, $this->mapper->findAll());
+ }
+
+ public function testNormalizeBackupFlagWithNothingToDo(): void {
+ $this->insertSampleStatuses();
+
+ $this->assertSame([], $this->mapper->findStatusesWithoutBackupFlagIds());
+ $this->assertSame(0, $this->mapper->normalizeBackupFlagByIds([]));
+ }
+
+ public function testDeleteStrandedBackupsWithEmptyAutomatedListDoesNothing(): void {
+ $this->insertBackupWithLiveStatus('user1', 'meeting');
+ $this->insertBackupWithLiveStatus('user2', 'call');
+
+ $this->assertSame(0, $this->mapper->deleteStrandedBackups([]));
+ $this->assertNotNull($this->mapper->findByUserId('user1', true));
+ $this->assertNotNull($this->mapper->findByUserId('user2', true));
+ }
+
+ public function testNormalizeBackupFlagKeepsBackupRowsAsBackups(): void {
+ self::$realDatabase->executeStatement(
+ 'INSERT INTO `*PREFIX*user_status` (`user_id`, `status`, `status_timestamp`, `is_user_defined`, `is_backup`) VALUES (?, ?, ?, ?, ?)',
+ ['user1', 'online', 5000, 0, null],
+ );
+ self::$realDatabase->executeStatement(
+ 'INSERT INTO `*PREFIX*user_status` (`user_id`, `status`, `status_timestamp`, `is_user_defined`, `is_backup`) VALUES (?, ?, ?, ?, ?)',
+ ['_user1', 'away', 4000, 0, null],
+ );
+
+ $ids = $this->mapper->findStatusesWithoutBackupFlagIds();
+ $this->assertCount(2, $ids);
+ $this->assertSame(2, $this->mapper->normalizeBackupFlagByIds($ids));
+
+ $this->assertSame([], $this->mapper->findStatusesWithoutBackupFlagIds());
+ $this->assertFalse($this->mapper->findByUserId('user1', false)->getIsBackup());
+ $this->assertTrue(
+ $this->mapper->findByUserId('user1', true)->getIsBackup(),
+ 'A backup row that predates the column default must stay a backup, or no revert can ever restore it',
+ );
+ }
}
diff --git a/apps/user_status/tests/Unit/Service/StatusRepairServiceTest.php b/apps/user_status/tests/Unit/Service/StatusRepairServiceTest.php
new file mode 100644
index 0000000000000..b614edd54e6ea
--- /dev/null
+++ b/apps/user_status/tests/Unit/Service/StatusRepairServiceTest.php
@@ -0,0 +1,52 @@
+mapper = $this->createMock(UserStatusMapper::class);
+ $this->service = new StatusRepairService($this->mapper);
+ }
+
+ /** The reason this class exists. */
+ public function testStrandedIsAlwaysDecidedAgainstTheAutomatedMessageIds(): void {
+ $this->mapper->expects($this->once())
+ ->method('findStrandedBackupIds')
+ ->with(StatusService::AUTOMATED_MESSAGE_IDS)
+ ->willReturn([1, 2]);
+ $this->mapper->expects($this->once())
+ ->method('deleteStrandedBackups')
+ ->with(StatusService::AUTOMATED_MESSAGE_IDS)
+ ->willReturn(2);
+ $this->mapper->expects($this->once())
+ ->method('findOrphanedAutomatedStatusIds')
+ ->with(StatusService::AUTOMATED_MESSAGE_IDS)
+ ->willReturn([3]);
+
+ self::assertSame([1, 2], $this->service->findStrandedBackupIds());
+ self::assertSame(2, $this->service->deleteStrandedBackups());
+ self::assertSame([3], $this->service->findOrphanedAutomatedStatusIds());
+ }
+
+ public function testAutomatedMessageIdsAreNeverEmpty(): void {
+ self::assertNotEmpty(StatusService::AUTOMATED_MESSAGE_IDS);
+ }
+}
diff --git a/apps/user_status/tests/Unit/Service/StatusServiceTest.php b/apps/user_status/tests/Unit/Service/StatusServiceTest.php
index 69128033631e0..0e33083a1049a 100644
--- a/apps/user_status/tests/Unit/Service/StatusServiceTest.php
+++ b/apps/user_status/tests/Unit/Service/StatusServiceTest.php
@@ -22,6 +22,7 @@
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\DB\Exception;
use OCP\IConfig;
+use OCP\IDBConnection;
use OCP\IEmojiHelper;
use OCP\IUserManager;
use OCP\UserStatus\IUserStatus;
@@ -37,6 +38,7 @@ class StatusServiceTest extends TestCase {
private IConfig&MockObject $config;
private IUserManager&MockObject $userManager;
private LoggerInterface&MockObject $logger;
+ private IDBConnection&MockObject $connection;
private StatusService $service;
@@ -50,6 +52,7 @@ protected function setUp(): void {
$this->userManager = $this->createMock(IUserManager::class);
$this->config = $this->createMock(IConfig::class);
$this->logger = $this->createMock(LoggerInterface::class);
+ $this->connection = $this->createMock(IDBConnection::class);
$this->config->method('getAppValue')
->willReturnMap([
@@ -64,6 +67,7 @@ protected function setUp(): void {
$this->config,
$this->userManager,
$this->logger,
+ $this->connection,
);
}
@@ -121,6 +125,7 @@ public function testFindAllRecentStatusChangesNoEnumeration(): void {
$this->config,
$this->userManager,
$this->logger,
+ $this->connection,
);
$this->assertEquals([], $this->service->findAllRecentStatusChanges(20, 50));
@@ -141,6 +146,7 @@ public function testFindAllRecentStatusChangesNoEnumeration(): void {
$this->config,
$this->userManager,
$this->logger,
+ $this->connection,
);
$this->assertEquals([], $this->service->findAllRecentStatusChanges(20, 50));
@@ -700,6 +706,106 @@ public function testBackup(): void {
$this->assertTrue($this->service->backupCurrentStatus('john'));
}
+ public function testBackupNothingToBackUp(): void {
+ // A user without a status row has nothing to move into a backup. That
+ // is not a conflict, so the automated status may still be applied.
+ $this->mapper->expects($this->once())
+ ->method('createBackupStatus')
+ ->with('john')
+ ->willReturn(false);
+
+ $this->assertTrue($this->service->backupCurrentStatus('john'));
+ }
+
+ public function testRevertUserStatusWithoutBackupClearsAutomatedStatus(): void {
+ $this->mapper->expects($this->once())
+ ->method('findByUserId')
+ ->with('john', true)
+ ->willThrowException(new DoesNotExistException(''));
+
+ // There is nothing to restore, but the unreachable automated status
+ // must still be removed so the user is not stuck on it.
+ $this->mapper->expects($this->once())
+ ->method('deleteCurrentStatusToRestoreBackup')
+ ->with('john', 'meeting')
+ ->willReturn(true);
+
+ $this->assertNull($this->service->revertUserStatus('john', 'meeting'));
+ }
+
+ public function testRevertUserStatusWithoutBackupAndNoMatchingStatus(): void {
+ $this->mapper->expects($this->once())
+ ->method('findByUserId')
+ ->with('john', true)
+ ->willThrowException(new DoesNotExistException(''));
+
+ // Nothing matched, so nothing was removed. Must not blow up.
+ $this->mapper->expects($this->once())
+ ->method('deleteCurrentStatusToRestoreBackup')
+ ->with('john', 'meeting')
+ ->willReturn(false);
+
+ $this->mapper->expects($this->never())->method('update');
+
+ $this->assertNull($this->service->revertUserStatus('john', 'meeting'));
+ }
+
+ public function testRevertUserStatusRefreshesTimestamp(): void {
+ $backup = new UserStatus();
+ $backup->setId(2);
+ $backup->setUserId('_john');
+ $backup->setStatus(IUserStatus::ONLINE);
+ $backup->setStatusTimestamp(1000);
+ $backup->setIsUserDefined(false);
+ $backup->setIsBackup(true);
+
+ $this->mapper->expects($this->once())
+ ->method('findByUserId')
+ ->with('john', true)
+ ->willReturn($backup);
+ $this->mapper->expects($this->once())
+ ->method('deleteCurrentStatusToRestoreBackup')
+ ->with('john', 'meeting')
+ ->willReturn(true);
+ $this->timeFactory->method('getTime')->willReturn(9999);
+
+ $this->mapper->expects($this->once())
+ ->method('update')
+ ->willReturnArgument(0);
+
+ $reverted = $this->service->revertUserStatus('john', 'meeting');
+
+ self::assertNotNull($reverted);
+ self::assertSame('john', $reverted->getUserId());
+ self::assertFalse($reverted->getIsBackup());
+ self::assertSame(
+ 9999,
+ $reverted->getStatusTimestamp(),
+ 'A restored status must not keep the timestamp from before the automated status',
+ );
+ }
+
+ public function testRevertUserStatusManuallyStillPromotesOfflineToOnline(): void {
+ $backup = new UserStatus();
+ $backup->setId(2);
+ $backup->setUserId('_john');
+ $backup->setStatus(IUserStatus::OFFLINE);
+ $backup->setStatusTimestamp(1000);
+ $backup->setIsUserDefined(false);
+ $backup->setIsBackup(true);
+
+ $this->mapper->method('findByUserId')->with('john', true)->willReturn($backup);
+ $this->mapper->method('deleteCurrentStatusToRestoreBackup')->willReturn(true);
+ $this->timeFactory->method('getTime')->willReturn(9999);
+ $this->mapper->expects($this->once())->method('update')->willReturnArgument(0);
+
+ $reverted = $this->service->revertUserStatus('john', 'meeting', true);
+
+ self::assertNotNull($reverted);
+ self::assertSame(IUserStatus::ONLINE, $reverted->getStatus());
+ self::assertSame(9999, $reverted->getStatusTimestamp());
+ }
+
public function testRevertMultipleUserStatus(): void {
$john = new UserStatus();
$john->setId(1);
@@ -761,13 +867,161 @@ public function testRevertMultipleUserStatus(): void {
->method('deleteByIds')
->with([1, 3, 5]);
+ $this->timeFactory->method('getTime')
+ ->willReturn(1337);
+
$this->mapper->expects($this->once())
->method('restoreBackupStatuses')
- ->with([2]);
+ ->with([2], 1337);
$this->service->revertMultipleUserStatus(['john', 'nobackup', 'backuponly', 'nobackupanddnd'], 'call');
}
+ /**
+ * The delete and the restore must not be observable separately: in between, the
+ * backup has no live row and the cleanup job would treat it as stranded.
+ */
+ public function testRevertUserStatusRunsInOneTransaction(): void {
+ $backup = new UserStatus();
+ $backup->setId(2);
+ $backup->setUserId('_john');
+ $backup->setStatus(IUserStatus::ONLINE);
+ $backup->setStatusTimestamp(1000);
+ $backup->setIsBackup(true);
+
+ $calls = [];
+ $this->connection->expects($this->once())
+ ->method('beginTransaction')
+ ->willReturnCallback(function () use (&$calls): void {
+ $calls[] = 'begin';
+ });
+ $this->connection->expects($this->once())
+ ->method('commit')
+ ->willReturnCallback(function () use (&$calls): void {
+ $calls[] = 'commit';
+ });
+ $this->connection->expects($this->never())->method('rollBack');
+
+ $this->mapper->method('findByUserId')
+ ->with('john', true)
+ ->willReturnCallback(function () use (&$calls, $backup): UserStatus {
+ $calls[] = 'find';
+ return $backup;
+ });
+ $this->mapper->method('deleteCurrentStatusToRestoreBackup')
+ ->willReturnCallback(function () use (&$calls): bool {
+ $calls[] = 'delete';
+ return true;
+ });
+ $this->mapper->method('update')
+ ->willReturnCallback(function (UserStatus $status) use (&$calls): UserStatus {
+ $calls[] = 'restore';
+ return $status;
+ });
+ $this->timeFactory->method('getTime')->willReturn(9999);
+
+ $this->service->revertUserStatus('john', 'meeting');
+
+ self::assertSame(['begin', 'find', 'delete', 'restore', 'commit'], $calls);
+ }
+
+ public function testRevertUserStatusRollsBackWhenTheRestoreFails(): void {
+ $backup = new UserStatus();
+ $backup->setId(2);
+ $backup->setUserId('_john');
+ $backup->setStatus(IUserStatus::ONLINE);
+ $backup->setIsBackup(true);
+
+ $this->mapper->method('findByUserId')->with('john', true)->willReturn($backup);
+ $this->mapper->method('deleteCurrentStatusToRestoreBackup')->willReturn(true);
+ $this->timeFactory->method('getTime')->willReturn(9999);
+ $this->mapper->method('update')->willThrowException(new \RuntimeException('nope'));
+
+ $this->connection->expects($this->once())->method('beginTransaction');
+ $this->connection->expects($this->never())->method('commit');
+ // Without the rollback the automated status would stay deleted while the
+ // backup keeps its backup flag, which strands the backup for good.
+ $this->connection->expects($this->once())->method('rollBack');
+
+ $this->expectException(\RuntimeException::class);
+
+ $this->service->revertUserStatus('john', 'meeting');
+ }
+
+ public function testRevertMultipleUserStatusRunsInOneTransaction(): void {
+ $live = new UserStatus();
+ $live->setId(1);
+ $live->setUserId('john');
+ $live->setMessageId('call');
+ $live->setIsBackup(false);
+
+ $backup = new UserStatus();
+ $backup->setId(2);
+ $backup->setUserId('_john');
+ $backup->setMessageId('hello');
+ $backup->setIsBackup(true);
+
+ $calls = [];
+ $this->connection->expects($this->once())
+ ->method('beginTransaction')
+ ->willReturnCallback(function () use (&$calls): void {
+ $calls[] = 'begin';
+ });
+ $this->connection->expects($this->once())
+ ->method('commit')
+ ->willReturnCallback(function () use (&$calls): void {
+ $calls[] = 'commit';
+ });
+ $this->connection->expects($this->never())->method('rollBack');
+
+ $this->mapper->method('findByUserIds')
+ ->willReturnCallback(function () use (&$calls, $live, $backup): array {
+ $calls[] = 'find';
+ return [$live, $backup];
+ });
+ $this->mapper->method('deleteByIds')
+ ->willReturnCallback(function (array $ids) use (&$calls): int {
+ $calls[] = 'delete';
+ return count($ids);
+ });
+ $this->mapper->method('restoreBackupStatuses')
+ ->willReturnCallback(function () use (&$calls): void {
+ $calls[] = 'restore';
+ });
+ $this->timeFactory->method('getTime')->willReturn(1337);
+
+ $this->service->revertMultipleUserStatus(['john'], 'call');
+
+ self::assertSame(['begin', 'find', 'delete', 'restore', 'commit'], $calls);
+ }
+
+ public function testRevertMultipleUserStatusRollsBackWhenTheRestoreFails(): void {
+ $live = new UserStatus();
+ $live->setId(1);
+ $live->setUserId('john');
+ $live->setMessageId('call');
+ $live->setIsBackup(false);
+
+ $backup = new UserStatus();
+ $backup->setId(2);
+ $backup->setUserId('_john');
+ $backup->setMessageId('hello');
+ $backup->setIsBackup(true);
+
+ $this->mapper->method('findByUserIds')->willReturn([$live, $backup]);
+ $this->mapper->method('deleteByIds')->willReturn(1);
+ $this->timeFactory->method('getTime')->willReturn(1337);
+ $this->mapper->method('restoreBackupStatuses')->willThrowException(new \RuntimeException('nope'));
+
+ $this->connection->expects($this->once())->method('beginTransaction');
+ $this->connection->expects($this->never())->method('commit');
+ $this->connection->expects($this->once())->method('rollBack');
+
+ $this->expectException(\RuntimeException::class);
+
+ $this->service->revertMultipleUserStatus(['john'], 'call');
+ }
+
public static function dataSetUserStatus(): array {
return [
[IUserStatus::MESSAGE_CALENDAR_BUSY, '', false],