From 08d33a9f571712c6ee0cfa40bcc23097e67fbea2 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Sun, 23 Feb 2025 19:08:10 +0100 Subject: [PATCH 1/3] fix: validate account properties as a repair step Replace `ValidatePhoneNumber` from Nextcloud 21 with a new repair step, `ValidateAccountProperties` which validates and sanitizes all account properties. Signed-off-by: Ferdinand Thiessen --- lib/composer/composer/autoload_classmap.php | 2 +- lib/composer/composer/autoload_static.php | 2 +- lib/private/Repair.php | 4 +- .../Repair/NC21/ValidatePhoneNumber.php | 70 --------------- .../Repair/NC29/ValidateAccountProperties.php | 58 ++++++++++++ .../NC29/ValidateAccountPropertiesTest.php | 89 +++++++++++++++++++ 6 files changed, 151 insertions(+), 74 deletions(-) delete mode 100644 lib/private/Repair/NC21/ValidatePhoneNumber.php create mode 100644 lib/private/Repair/NC29/ValidateAccountProperties.php create mode 100644 tests/lib/Repair/NC29/ValidateAccountPropertiesTest.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 5c36d30554b67..5630c19352279 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1882,10 +1882,10 @@ 'OC\\Repair\\NC20\\EncryptionMigration' => $baseDir . '/lib/private/Repair/NC20/EncryptionMigration.php', 'OC\\Repair\\NC20\\ShippedDashboardEnable' => $baseDir . '/lib/private/Repair/NC20/ShippedDashboardEnable.php', 'OC\\Repair\\NC21\\AddCheckForUserCertificatesJob' => $baseDir . '/lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php', - 'OC\\Repair\\NC21\\ValidatePhoneNumber' => $baseDir . '/lib/private/Repair/NC21/ValidatePhoneNumber.php', 'OC\\Repair\\NC22\\LookupServerSendCheck' => $baseDir . '/lib/private/Repair/NC22/LookupServerSendCheck.php', 'OC\\Repair\\NC24\\AddTokenCleanupJob' => $baseDir . '/lib/private/Repair/NC24/AddTokenCleanupJob.php', 'OC\\Repair\\NC25\\AddMissingSecretJob' => $baseDir . '/lib/private/Repair/NC25/AddMissingSecretJob.php', + 'OC\\Repair\\NC29\\ValidateAccountProperties' => $baseDir . '/lib/private/Repair/NC29/ValidateAccountProperties.php', 'OC\\Repair\\NC30\\RemoveLegacyDatadirFile' => $baseDir . '/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php', 'OC\\Repair\\OldGroupMembershipShares' => $baseDir . '/lib/private/Repair/OldGroupMembershipShares.php', 'OC\\Repair\\Owncloud\\CleanPreviews' => $baseDir . '/lib/private/Repair/Owncloud/CleanPreviews.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index d366d58472f94..37fd0e03b8cbf 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1931,10 +1931,10 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Repair\\NC20\\EncryptionMigration' => __DIR__ . '/../../..' . '/lib/private/Repair/NC20/EncryptionMigration.php', 'OC\\Repair\\NC20\\ShippedDashboardEnable' => __DIR__ . '/../../..' . '/lib/private/Repair/NC20/ShippedDashboardEnable.php', 'OC\\Repair\\NC21\\AddCheckForUserCertificatesJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php', - 'OC\\Repair\\NC21\\ValidatePhoneNumber' => __DIR__ . '/../../..' . '/lib/private/Repair/NC21/ValidatePhoneNumber.php', 'OC\\Repair\\NC22\\LookupServerSendCheck' => __DIR__ . '/../../..' . '/lib/private/Repair/NC22/LookupServerSendCheck.php', 'OC\\Repair\\NC24\\AddTokenCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC24/AddTokenCleanupJob.php', 'OC\\Repair\\NC25\\AddMissingSecretJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC25/AddMissingSecretJob.php', + 'OC\\Repair\\NC29\\ValidateAccountProperties' => __DIR__ . '/../../..' . '/lib/private/Repair/NC29/ValidateAccountProperties.php', 'OC\\Repair\\NC30\\RemoveLegacyDatadirFile' => __DIR__ . '/../../..' . '/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php', 'OC\\Repair\\OldGroupMembershipShares' => __DIR__ . '/../../..' . '/lib/private/Repair/OldGroupMembershipShares.php', 'OC\\Repair\\Owncloud\\CleanPreviews' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/CleanPreviews.php', diff --git a/lib/private/Repair.php b/lib/private/Repair.php index cc2add4a8cf59..1192ea50ae079 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -37,10 +37,10 @@ use OC\Repair\NC20\EncryptionMigration; use OC\Repair\NC20\ShippedDashboardEnable; use OC\Repair\NC21\AddCheckForUserCertificatesJob; -use OC\Repair\NC21\ValidatePhoneNumber; use OC\Repair\NC22\LookupServerSendCheck; use OC\Repair\NC24\AddTokenCleanupJob; use OC\Repair\NC25\AddMissingSecretJob; +use OC\Repair\NC29\ValidateAccountProperties; use OC\Repair\NC30\RemoveLegacyDatadirFile; use OC\Repair\OldGroupMembershipShares; use OC\Repair\Owncloud\CleanPreviews; @@ -212,7 +212,7 @@ public static function getExpensiveRepairSteps() { \OCP\Server::get(IAppConfig::class), \OCP\Server::get(IDBConnection::class) ), - \OC::$server->get(ValidatePhoneNumber::class), + \OC::$server->get(ValidateAccountProperties::class), \OC::$server->get(DeleteSchedulingObjects::class), ]; } diff --git a/lib/private/Repair/NC21/ValidatePhoneNumber.php b/lib/private/Repair/NC21/ValidatePhoneNumber.php deleted file mode 100644 index 3a6ace37bd2b3..0000000000000 --- a/lib/private/Repair/NC21/ValidatePhoneNumber.php +++ /dev/null @@ -1,70 +0,0 @@ -config = $config; - $this->userManager = $userManager; - $this->accountManager = $accountManager; - } - - public function getName(): string { - return 'Validate the phone number and store it in a known format for search'; - } - - public function run(IOutput $output): void { - if ($this->config->getSystemValueString('default_phone_region', '') === '') { - $output->warning('Can not validate phone numbers without `default_phone_region` being set in the config file'); - return; - } - - $numUpdated = 0; - $numRemoved = 0; - - $this->userManager->callForSeenUsers(function (IUser $user) use (&$numUpdated, &$numRemoved) { - $account = $this->accountManager->getAccount($user); - $property = $account->getProperty(IAccountManager::PROPERTY_PHONE); - - if ($property->getValue() !== '') { - $this->accountManager->updateAccount($account); - $updatedAccount = $this->accountManager->getAccount($user); - $updatedProperty = $updatedAccount->getProperty(IAccountManager::PROPERTY_PHONE); - - if ($property->getValue() !== $updatedProperty->getValue()) { - if ($updatedProperty->getValue() === '') { - $numRemoved++; - } else { - $numUpdated++; - } - } - } - }); - - if ($numRemoved > 0 || $numUpdated > 0) { - $output->info('Updated ' . $numUpdated . ' entries and cleaned ' . $numRemoved . ' invalid phone numbers'); - } - } -} diff --git a/lib/private/Repair/NC29/ValidateAccountProperties.php b/lib/private/Repair/NC29/ValidateAccountProperties.php new file mode 100644 index 0000000000000..266266c8a1c93 --- /dev/null +++ b/lib/private/Repair/NC29/ValidateAccountProperties.php @@ -0,0 +1,58 @@ +userManager->callForSeenUsers(function (IUser $user) use (&$numRemoved) { + $account = $this->accountManager->getAccount($user); + while (true) { + try { + $this->accountManager->updateAccount($account); + break; + } catch (InvalidArgumentException $e) { + if (in_array($e->getMessage(), IAccountManager::ALLOWED_PROPERTIES)) { + $numRemoved++; + $property = $account->getProperty($e->getMessage()); + $account->setProperty($property->getName(), '', $property->getScope(), IAccountManager::NOT_VERIFIED); + } else { + $this->logger->error('Error while sanitizing account property', ['exception' => $e, 'user' => $user->getUID()]); + break; + } + } + } + }); + + if ($numRemoved > 0) { + $output->info('Cleaned ' . $numRemoved . ' invalid account property entries'); + } + } +} diff --git a/tests/lib/Repair/NC29/ValidateAccountPropertiesTest.php b/tests/lib/Repair/NC29/ValidateAccountPropertiesTest.php new file mode 100644 index 0000000000000..cc43a63ca19c3 --- /dev/null +++ b/tests/lib/Repair/NC29/ValidateAccountPropertiesTest.php @@ -0,0 +1,89 @@ +userManager = $this->createMock(IUserManager::class); + $this->accountManager = $this->createMock(IAccountManager::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->repairStep = new ValidateAccountProperties($this->userManager, $this->accountManager, $this->logger); + } + + public function testGetName(): void { + self::assertStringContainsString('Validate account properties', $this->repairStep->getName()); + } + + public function testRun(): void { + $users = [ + $this->createMock(IUser::class), + $this->createMock(IUser::class), + ]; + $this->userManager + ->expects(self::once()) + ->method('callForSeenUsers') + ->willReturnCallback(fn ($fn) => array_map($fn, $users)); + + $property = $this->createMock(IAccountProperty::class); + $property->expects(self::once())->method('getName')->willReturn(IAccountManager::PROPERTY_PHONE); + $property->expects(self::once())->method('getScope')->willReturn(IAccountManager::SCOPE_LOCAL); + + $account1 = $this->createMock(IAccount::class); + $account1->expects(self::once()) + ->method('getProperty') + ->with(IAccountManager::PROPERTY_PHONE) + ->willReturn($property); + $account1->expects(self::once()) + ->method('setProperty') + ->with(IAccountManager::PROPERTY_PHONE, '', IAccountManager::SCOPE_LOCAL, IAccountManager::NOT_VERIFIED); + $account2 = $this->createMock(IAccount::class); + $account2->expects(self::never()) + ->method('getProperty'); + $this->accountManager + ->expects(self::exactly(2)) + ->method('getAccount') + ->willReturnMap([ + [$users[0], $account1], + [$users[1], $account2], + ]); + $valid = false; + $this->accountManager->expects(self::exactly(3)) + ->method('updateAccount') + ->willReturnCallback(function (IAccount $account) use (&$account1, &$valid) { + if (!$valid && $account === $account1) { + $valid = true; + throw new InvalidArgumentException(IAccountManager::PROPERTY_PHONE); + } + }); + + $output = $this->createMock(IOutput::class); + $output->expects(self::once())->method('info')->with('Cleaned 1 invalid account property entries'); + + $this->repairStep->run($output); + } +} From 575222b5afd20a487ef07e7d08e1519fc07724ec Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Sun, 23 Feb 2025 19:24:04 +0100 Subject: [PATCH 2/3] fix: Optimize repair step performance Signed-off-by: Ferdinand Thiessen --- .../Repair/NC29/ValidateAccountProperties.php | 24 ++++++++++++++--- .../NC29/ValidateAccountPropertiesTest.php | 27 ++++++++++++++++++- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/lib/private/Repair/NC29/ValidateAccountProperties.php b/lib/private/Repair/NC29/ValidateAccountProperties.php index 266266c8a1c93..640cd678ba07a 100644 --- a/lib/private/Repair/NC29/ValidateAccountProperties.php +++ b/lib/private/Repair/NC29/ValidateAccountProperties.php @@ -18,6 +18,13 @@ class ValidateAccountProperties implements IRepairStep { + private const PROPERTIES_TO_CHECK = [ + IAccountManager::PROPERTY_PHONE, + IAccountManager::PROPERTY_WEBSITE, + IAccountManager::PROPERTY_TWITTER, + IAccountManager::PROPERTY_FEDIVERSE, + ]; + public function __construct( private IUserManager $userManager, private IAccountManager $accountManager, @@ -34,10 +41,20 @@ public function run(IOutput $output): void { $this->userManager->callForSeenUsers(function (IUser $user) use (&$numRemoved) { $account = $this->accountManager->getAccount($user); - while (true) { + $properties = array_keys($account->jsonSerialize()); + + // Check if there are some properties we can sanitize - reduces number of db queries + if (empty(array_intersect($properties, self::PROPERTIES_TO_CHECK))) { + return; + } + + // Limit the loop to the properties we check to ensure there are no infinite loops + // we add one additional loop (+ 1) as we need 1 loop for checking + 1 for update. + $iteration = count(self::PROPERTIES_TO_CHECK) + 1; + while ($iteration-- > 0) { try { $this->accountManager->updateAccount($account); - break; + return; } catch (InvalidArgumentException $e) { if (in_array($e->getMessage(), IAccountManager::ALLOWED_PROPERTIES)) { $numRemoved++; @@ -45,10 +62,11 @@ public function run(IOutput $output): void { $account->setProperty($property->getName(), '', $property->getScope(), IAccountManager::NOT_VERIFIED); } else { $this->logger->error('Error while sanitizing account property', ['exception' => $e, 'user' => $user->getUID()]); - break; + return; } } } + $this->logger->error('Iteration limit exceeded while cleaning account properties', ['user' => $user->getUID()]); }); if ($numRemoved > 0) { diff --git a/tests/lib/Repair/NC29/ValidateAccountPropertiesTest.php b/tests/lib/Repair/NC29/ValidateAccountPropertiesTest.php index cc43a63ca19c3..e113f1809980e 100644 --- a/tests/lib/Repair/NC29/ValidateAccountPropertiesTest.php +++ b/tests/lib/Repair/NC29/ValidateAccountPropertiesTest.php @@ -43,6 +43,7 @@ public function testRun(): void { $users = [ $this->createMock(IUser::class), $this->createMock(IUser::class), + $this->createMock(IUser::class), ]; $this->userManager ->expects(self::once()) @@ -61,15 +62,39 @@ public function testRun(): void { $account1->expects(self::once()) ->method('setProperty') ->with(IAccountManager::PROPERTY_PHONE, '', IAccountManager::SCOPE_LOCAL, IAccountManager::NOT_VERIFIED); + $account1->expects(self::once()) + ->method('jsonSerialize') + ->willReturn([ + IAccountManager::PROPERTY_DISPLAYNAME => [], + IAccountManager::PROPERTY_PHONE => [], + ]); + $account2 = $this->createMock(IAccount::class); $account2->expects(self::never()) ->method('getProperty'); + $account2->expects(self::once()) + ->method('jsonSerialize') + ->willReturn([ + IAccountManager::PROPERTY_DISPLAYNAME => [], + IAccountManager::PROPERTY_PHONE => [], + ]); + + $account3 = $this->createMock(IAccount::class); + $account3->expects(self::never()) + ->method('getProperty'); + $account3->expects(self::once()) + ->method('jsonSerialize') + ->willReturn([ + IAccountManager::PROPERTY_DISPLAYNAME => [], + ]); + $this->accountManager - ->expects(self::exactly(2)) + ->expects(self::exactly(3)) ->method('getAccount') ->willReturnMap([ [$users[0], $account1], [$users[1], $account2], + [$users[2], $account3], ]); $valid = false; $this->accountManager->expects(self::exactly(3)) From e3af27b28018f36dc828dd9f60b06292386f6804 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Mon, 24 Feb 2025 14:52:24 +0100 Subject: [PATCH 3/3] refactor: convert sanitize account properties repair step to background job Signed-off-by: Ferdinand Thiessen --- lib/composer/composer/autoload_classmap.php | 3 +- lib/composer/composer/autoload_static.php | 3 +- lib/private/Repair.php | 6 +-- .../Repair/NC29/SanitizeAccountProperties.php | 30 +++++++++++++ ...s.php => SanitizeAccountPropertiesJob.php} | 17 ++++---- ...p => SanitizeAccountPropertiesJobTest.php} | 22 +++++----- .../NC29/SanitizeAccountPropertiesTest.php | 43 +++++++++++++++++++ 7 files changed, 100 insertions(+), 24 deletions(-) create mode 100644 lib/private/Repair/NC29/SanitizeAccountProperties.php rename lib/private/Repair/NC29/{ValidateAccountProperties.php => SanitizeAccountPropertiesJob.php} (84%) rename tests/lib/Repair/NC29/{ValidateAccountPropertiesTest.php => SanitizeAccountPropertiesJobTest.php} (84%) create mode 100644 tests/lib/Repair/NC29/SanitizeAccountPropertiesTest.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 5630c19352279..b6095032c4c2d 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1885,7 +1885,8 @@ 'OC\\Repair\\NC22\\LookupServerSendCheck' => $baseDir . '/lib/private/Repair/NC22/LookupServerSendCheck.php', 'OC\\Repair\\NC24\\AddTokenCleanupJob' => $baseDir . '/lib/private/Repair/NC24/AddTokenCleanupJob.php', 'OC\\Repair\\NC25\\AddMissingSecretJob' => $baseDir . '/lib/private/Repair/NC25/AddMissingSecretJob.php', - 'OC\\Repair\\NC29\\ValidateAccountProperties' => $baseDir . '/lib/private/Repair/NC29/ValidateAccountProperties.php', + 'OC\\Repair\\NC29\\SanitizeAccountProperties' => $baseDir . '/lib/private/Repair/NC29/SanitizeAccountProperties.php', + 'OC\\Repair\\NC29\\SanitizeAccountPropertiesJob' => $baseDir . '/lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php', 'OC\\Repair\\NC30\\RemoveLegacyDatadirFile' => $baseDir . '/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php', 'OC\\Repair\\OldGroupMembershipShares' => $baseDir . '/lib/private/Repair/OldGroupMembershipShares.php', 'OC\\Repair\\Owncloud\\CleanPreviews' => $baseDir . '/lib/private/Repair/Owncloud/CleanPreviews.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 37fd0e03b8cbf..66df44dfb0644 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1934,7 +1934,8 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Repair\\NC22\\LookupServerSendCheck' => __DIR__ . '/../../..' . '/lib/private/Repair/NC22/LookupServerSendCheck.php', 'OC\\Repair\\NC24\\AddTokenCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC24/AddTokenCleanupJob.php', 'OC\\Repair\\NC25\\AddMissingSecretJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC25/AddMissingSecretJob.php', - 'OC\\Repair\\NC29\\ValidateAccountProperties' => __DIR__ . '/../../..' . '/lib/private/Repair/NC29/ValidateAccountProperties.php', + 'OC\\Repair\\NC29\\SanitizeAccountProperties' => __DIR__ . '/../../..' . '/lib/private/Repair/NC29/SanitizeAccountProperties.php', + 'OC\\Repair\\NC29\\SanitizeAccountPropertiesJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php', 'OC\\Repair\\NC30\\RemoveLegacyDatadirFile' => __DIR__ . '/../../..' . '/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php', 'OC\\Repair\\OldGroupMembershipShares' => __DIR__ . '/../../..' . '/lib/private/Repair/OldGroupMembershipShares.php', 'OC\\Repair\\Owncloud\\CleanPreviews' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/CleanPreviews.php', diff --git a/lib/private/Repair.php b/lib/private/Repair.php index 1192ea50ae079..c5069bff48e5c 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -40,7 +40,7 @@ use OC\Repair\NC22\LookupServerSendCheck; use OC\Repair\NC24\AddTokenCleanupJob; use OC\Repair\NC25\AddMissingSecretJob; -use OC\Repair\NC29\ValidateAccountProperties; +use OC\Repair\NC29\SanitizeAccountProperties; use OC\Repair\NC30\RemoveLegacyDatadirFile; use OC\Repair\OldGroupMembershipShares; use OC\Repair\Owncloud\CleanPreviews; @@ -194,6 +194,7 @@ public static function getRepairSteps(): array { \OCP\Server::get(RepairLogoDimension::class), \OCP\Server::get(RemoveLegacyDatadirFile::class), \OCP\Server::get(AddCleanupDeletedUsersBackgroundJob::class), + \OCP\Server::get(SanitizeAccountProperties::class), ]; } @@ -212,8 +213,7 @@ public static function getExpensiveRepairSteps() { \OCP\Server::get(IAppConfig::class), \OCP\Server::get(IDBConnection::class) ), - \OC::$server->get(ValidateAccountProperties::class), - \OC::$server->get(DeleteSchedulingObjects::class), + \OCP\Server::get(DeleteSchedulingObjects::class), ]; } diff --git a/lib/private/Repair/NC29/SanitizeAccountProperties.php b/lib/private/Repair/NC29/SanitizeAccountProperties.php new file mode 100644 index 0000000000000..412570ba71dec --- /dev/null +++ b/lib/private/Repair/NC29/SanitizeAccountProperties.php @@ -0,0 +1,30 @@ +jobList->add(SanitizeAccountPropertiesJob::class, null); + $output->info('Queued background to validate account properties.'); + } +} diff --git a/lib/private/Repair/NC29/ValidateAccountProperties.php b/lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php similarity index 84% rename from lib/private/Repair/NC29/ValidateAccountProperties.php rename to lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php index 640cd678ba07a..55ec445e9daec 100644 --- a/lib/private/Repair/NC29/ValidateAccountProperties.php +++ b/lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php @@ -10,13 +10,13 @@ use InvalidArgumentException; use OCP\Accounts\IAccountManager; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\QueuedJob; use OCP\IUser; use OCP\IUserManager; -use OCP\Migration\IOutput; -use OCP\Migration\IRepairStep; use Psr\Log\LoggerInterface; -class ValidateAccountProperties implements IRepairStep { +class SanitizeAccountPropertiesJob extends QueuedJob { private const PROPERTIES_TO_CHECK = [ IAccountManager::PROPERTY_PHONE, @@ -26,17 +26,16 @@ class ValidateAccountProperties implements IRepairStep { ]; public function __construct( + ITimeFactory $timeFactory, private IUserManager $userManager, private IAccountManager $accountManager, private LoggerInterface $logger, ) { + parent::__construct($timeFactory); + $this->setAllowParallelRuns(false); } - public function getName(): string { - return 'Validate account properties and store phone numbers in a known format for search'; - } - - public function run(IOutput $output): void { + protected function run(mixed $argument): void { $numRemoved = 0; $this->userManager->callForSeenUsers(function (IUser $user) use (&$numRemoved) { @@ -70,7 +69,7 @@ public function run(IOutput $output): void { }); if ($numRemoved > 0) { - $output->info('Cleaned ' . $numRemoved . ' invalid account property entries'); + $this->logger->info('Cleaned ' . $numRemoved . ' invalid account property entries'); } } } diff --git a/tests/lib/Repair/NC29/ValidateAccountPropertiesTest.php b/tests/lib/Repair/NC29/SanitizeAccountPropertiesJobTest.php similarity index 84% rename from tests/lib/Repair/NC29/ValidateAccountPropertiesTest.php rename to tests/lib/Repair/NC29/SanitizeAccountPropertiesJobTest.php index e113f1809980e..e0f4eb3cbc1aa 100644 --- a/tests/lib/Repair/NC29/ValidateAccountPropertiesTest.php +++ b/tests/lib/Repair/NC29/SanitizeAccountPropertiesJobTest.php @@ -12,31 +12,36 @@ use OCP\Accounts\IAccount; use OCP\Accounts\IAccountManager; use OCP\Accounts\IAccountProperty; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IUser; use OCP\IUserManager; -use OCP\Migration\IOutput; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Test\TestCase; -class ValidateAccountPropertiesTest extends TestCase { +class SanitizeAccountPropertiesJobTest extends TestCase { private IUserManager&MockObject $userManager; private IAccountManager&MockObject $accountManager; private LoggerInterface&MockObject $logger; - private ValidateAccountProperties $repairStep; + private SanitizeAccountPropertiesJob $job; protected function setUp(): void { $this->userManager = $this->createMock(IUserManager::class); $this->accountManager = $this->createMock(IAccountManager::class); $this->logger = $this->createMock(LoggerInterface::class); - $this->repairStep = new ValidateAccountProperties($this->userManager, $this->accountManager, $this->logger); + $this->job = new SanitizeAccountPropertiesJob( + $this->createMock(ITimeFactory::class), + $this->userManager, + $this->accountManager, + $this->logger, + ); } - public function testGetName(): void { - self::assertStringContainsString('Validate account properties', $this->repairStep->getName()); + public function testParallel() { + self::assertFalse($this->job->getAllowParallelRuns()); } public function testRun(): void { @@ -106,9 +111,6 @@ public function testRun(): void { } }); - $output = $this->createMock(IOutput::class); - $output->expects(self::once())->method('info')->with('Cleaned 1 invalid account property entries'); - - $this->repairStep->run($output); + self::invokePrivate($this->job, 'run', [null]); } } diff --git a/tests/lib/Repair/NC29/SanitizeAccountPropertiesTest.php b/tests/lib/Repair/NC29/SanitizeAccountPropertiesTest.php new file mode 100644 index 0000000000000..d0d33eb2817ca --- /dev/null +++ b/tests/lib/Repair/NC29/SanitizeAccountPropertiesTest.php @@ -0,0 +1,43 @@ +jobList = $this->createMock(IJobList::class); + + $this->repairStep = new SanitizeAccountProperties($this->jobList); + } + + public function testGetName(): void { + self::assertStringContainsString('Validate account properties', $this->repairStep->getName()); + } + + public function testRun(): void { + $this->jobList->expects(self::once()) + ->method('add') + ->with(SanitizeAccountPropertiesJob::class, null); + + $output = $this->createMock(IOutput::class); + $output->expects(self::once()) + ->method('info') + ->with(self::matchesRegularExpression('/queued background/i')); + + $this->repairStep->run($output); + } +}