refactor: convert sanitize account properties repair step to backgrou…

…nd job Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
nextcloud · Feb 24, 2025 · e3af27b · e3af27b
1 parent 575222b
commit e3af27b
Show file tree

Hide file tree

Showing 7 changed files with 100 additions and 24 deletions.
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OC\Repair\NC29;
+
+use OCP\BackgroundJob\IJobList;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+
+class SanitizeAccountProperties implements IRepairStep {
+
+	public function __construct(
+		private IJobList $jobList,
+	) {
+	}
+
+	public function getName(): string {
+		return 'Validate account properties and store phone numbers in a known format for search';
+	}
+
+	public function run(IOutput $output): void {
+		$this->jobList->add(SanitizeAccountPropertiesJob::class, null);
+		$output->info('Queued background to validate account properties.');
+	}
+}
diff --git a/...Repair/NC29/ValidateAccountProperties.php → ...air/NC29/SanitizeAccountPropertiesJob.php b/...Repair/NC29/ValidateAccountProperties.php → ...air/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/...ir/NC29/ValidateAccountPropertiesTest.php → ...NC29/SanitizeAccountPropertiesJobTest.php b/...ir/NC29/ValidateAccountPropertiesTest.php → ...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
@@ -0,0 +1,43 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OC\Repair\NC29;
+
+use OCP\BackgroundJob\IJobList;
+use OCP\Migration\IOutput;
+use PHPUnit\Framework\MockObject\MockObject;
+use Test\TestCase;
+
+class SanitizeAccountPropertiesTest extends TestCase {
+
+	private IJobList&MockObject $jobList;
+	private SanitizeAccountProperties $repairStep;
+
+	protected function setUp(): void {
+		$this->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);
+	}
+}