From 06ddb7aff62213213bbdc46cb2e6d6fdfd8402c9 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Mon, 24 Feb 2025 14:52:24 +0100 Subject: [PATCH] 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); + } +}