Skip to content

Commit e3af27b

Browse files
committed
refactor: convert sanitize account properties repair step to background job
Signed-off-by: Ferdinand Thiessen <[email protected]>
1 parent 575222b commit e3af27b

File tree

7 files changed

+100
-24
lines changed

7 files changed

+100
-24
lines changed

lib/composer/composer/autoload_classmap.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1885,7 +1885,8 @@
18851885
'OC\\Repair\\NC22\\LookupServerSendCheck' => $baseDir . '/lib/private/Repair/NC22/LookupServerSendCheck.php',
18861886
'OC\\Repair\\NC24\\AddTokenCleanupJob' => $baseDir . '/lib/private/Repair/NC24/AddTokenCleanupJob.php',
18871887
'OC\\Repair\\NC25\\AddMissingSecretJob' => $baseDir . '/lib/private/Repair/NC25/AddMissingSecretJob.php',
1888-
'OC\\Repair\\NC29\\ValidateAccountProperties' => $baseDir . '/lib/private/Repair/NC29/ValidateAccountProperties.php',
1888+
'OC\\Repair\\NC29\\SanitizeAccountProperties' => $baseDir . '/lib/private/Repair/NC29/SanitizeAccountProperties.php',
1889+
'OC\\Repair\\NC29\\SanitizeAccountPropertiesJob' => $baseDir . '/lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php',
18891890
'OC\\Repair\\NC30\\RemoveLegacyDatadirFile' => $baseDir . '/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php',
18901891
'OC\\Repair\\OldGroupMembershipShares' => $baseDir . '/lib/private/Repair/OldGroupMembershipShares.php',
18911892
'OC\\Repair\\Owncloud\\CleanPreviews' => $baseDir . '/lib/private/Repair/Owncloud/CleanPreviews.php',

lib/composer/composer/autoload_static.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1934,7 +1934,8 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
19341934
'OC\\Repair\\NC22\\LookupServerSendCheck' => __DIR__ . '/../../..' . '/lib/private/Repair/NC22/LookupServerSendCheck.php',
19351935
'OC\\Repair\\NC24\\AddTokenCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC24/AddTokenCleanupJob.php',
19361936
'OC\\Repair\\NC25\\AddMissingSecretJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC25/AddMissingSecretJob.php',
1937-
'OC\\Repair\\NC29\\ValidateAccountProperties' => __DIR__ . '/../../..' . '/lib/private/Repair/NC29/ValidateAccountProperties.php',
1937+
'OC\\Repair\\NC29\\SanitizeAccountProperties' => __DIR__ . '/../../..' . '/lib/private/Repair/NC29/SanitizeAccountProperties.php',
1938+
'OC\\Repair\\NC29\\SanitizeAccountPropertiesJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php',
19381939
'OC\\Repair\\NC30\\RemoveLegacyDatadirFile' => __DIR__ . '/../../..' . '/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php',
19391940
'OC\\Repair\\OldGroupMembershipShares' => __DIR__ . '/../../..' . '/lib/private/Repair/OldGroupMembershipShares.php',
19401941
'OC\\Repair\\Owncloud\\CleanPreviews' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/CleanPreviews.php',

lib/private/Repair.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
use OC\Repair\NC22\LookupServerSendCheck;
4141
use OC\Repair\NC24\AddTokenCleanupJob;
4242
use OC\Repair\NC25\AddMissingSecretJob;
43-
use OC\Repair\NC29\ValidateAccountProperties;
43+
use OC\Repair\NC29\SanitizeAccountProperties;
4444
use OC\Repair\NC30\RemoveLegacyDatadirFile;
4545
use OC\Repair\OldGroupMembershipShares;
4646
use OC\Repair\Owncloud\CleanPreviews;
@@ -194,6 +194,7 @@ public static function getRepairSteps(): array {
194194
\OCP\Server::get(RepairLogoDimension::class),
195195
\OCP\Server::get(RemoveLegacyDatadirFile::class),
196196
\OCP\Server::get(AddCleanupDeletedUsersBackgroundJob::class),
197+
\OCP\Server::get(SanitizeAccountProperties::class),
197198
];
198199
}
199200

@@ -212,8 +213,7 @@ public static function getExpensiveRepairSteps() {
212213
\OCP\Server::get(IAppConfig::class),
213214
\OCP\Server::get(IDBConnection::class)
214215
),
215-
\OC::$server->get(ValidateAccountProperties::class),
216-
\OC::$server->get(DeleteSchedulingObjects::class),
216+
\OCP\Server::get(DeleteSchedulingObjects::class),
217217
];
218218
}
219219

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OC\Repair\NC29;
10+
11+
use OCP\BackgroundJob\IJobList;
12+
use OCP\Migration\IOutput;
13+
use OCP\Migration\IRepairStep;
14+
15+
class SanitizeAccountProperties implements IRepairStep {
16+
17+
public function __construct(
18+
private IJobList $jobList,
19+
) {
20+
}
21+
22+
public function getName(): string {
23+
return 'Validate account properties and store phone numbers in a known format for search';
24+
}
25+
26+
public function run(IOutput $output): void {
27+
$this->jobList->add(SanitizeAccountPropertiesJob::class, null);
28+
$output->info('Queued background to validate account properties.');
29+
}
30+
}

lib/private/Repair/NC29/ValidateAccountProperties.php renamed to lib/private/Repair/NC29/SanitizeAccountPropertiesJob.php

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,13 @@
1010

1111
use InvalidArgumentException;
1212
use OCP\Accounts\IAccountManager;
13+
use OCP\AppFramework\Utility\ITimeFactory;
14+
use OCP\BackgroundJob\QueuedJob;
1315
use OCP\IUser;
1416
use OCP\IUserManager;
15-
use OCP\Migration\IOutput;
16-
use OCP\Migration\IRepairStep;
1717
use Psr\Log\LoggerInterface;
1818

19-
class ValidateAccountProperties implements IRepairStep {
19+
class SanitizeAccountPropertiesJob extends QueuedJob {
2020

2121
private const PROPERTIES_TO_CHECK = [
2222
IAccountManager::PROPERTY_PHONE,
@@ -26,17 +26,16 @@ class ValidateAccountProperties implements IRepairStep {
2626
];
2727

2828
public function __construct(
29+
ITimeFactory $timeFactory,
2930
private IUserManager $userManager,
3031
private IAccountManager $accountManager,
3132
private LoggerInterface $logger,
3233
) {
34+
parent::__construct($timeFactory);
35+
$this->setAllowParallelRuns(false);
3336
}
3437

35-
public function getName(): string {
36-
return 'Validate account properties and store phone numbers in a known format for search';
37-
}
38-
39-
public function run(IOutput $output): void {
38+
protected function run(mixed $argument): void {
4039
$numRemoved = 0;
4140

4241
$this->userManager->callForSeenUsers(function (IUser $user) use (&$numRemoved) {
@@ -70,7 +69,7 @@ public function run(IOutput $output): void {
7069
});
7170

7271
if ($numRemoved > 0) {
73-
$output->info('Cleaned ' . $numRemoved . ' invalid account property entries');
72+
$this->logger->info('Cleaned ' . $numRemoved . ' invalid account property entries');
7473
}
7574
}
7675
}

tests/lib/Repair/NC29/ValidateAccountPropertiesTest.php renamed to tests/lib/Repair/NC29/SanitizeAccountPropertiesJobTest.php

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,31 +12,36 @@
1212
use OCP\Accounts\IAccount;
1313
use OCP\Accounts\IAccountManager;
1414
use OCP\Accounts\IAccountProperty;
15+
use OCP\AppFramework\Utility\ITimeFactory;
1516
use OCP\IUser;
1617
use OCP\IUserManager;
17-
use OCP\Migration\IOutput;
1818
use PHPUnit\Framework\MockObject\MockObject;
1919
use Psr\Log\LoggerInterface;
2020
use Test\TestCase;
2121

22-
class ValidateAccountPropertiesTest extends TestCase {
22+
class SanitizeAccountPropertiesJobTest extends TestCase {
2323

2424
private IUserManager&MockObject $userManager;
2525
private IAccountManager&MockObject $accountManager;
2626
private LoggerInterface&MockObject $logger;
2727

28-
private ValidateAccountProperties $repairStep;
28+
private SanitizeAccountPropertiesJob $job;
2929

3030
protected function setUp(): void {
3131
$this->userManager = $this->createMock(IUserManager::class);
3232
$this->accountManager = $this->createMock(IAccountManager::class);
3333
$this->logger = $this->createMock(LoggerInterface::class);
3434

35-
$this->repairStep = new ValidateAccountProperties($this->userManager, $this->accountManager, $this->logger);
35+
$this->job = new SanitizeAccountPropertiesJob(
36+
$this->createMock(ITimeFactory::class),
37+
$this->userManager,
38+
$this->accountManager,
39+
$this->logger,
40+
);
3641
}
3742

38-
public function testGetName(): void {
39-
self::assertStringContainsString('Validate account properties', $this->repairStep->getName());
43+
public function testParallel() {
44+
self::assertFalse($this->job->getAllowParallelRuns());
4045
}
4146

4247
public function testRun(): void {
@@ -106,9 +111,6 @@ public function testRun(): void {
106111
}
107112
});
108113

109-
$output = $this->createMock(IOutput::class);
110-
$output->expects(self::once())->method('info')->with('Cleaned 1 invalid account property entries');
111-
112-
$this->repairStep->run($output);
114+
self::invokePrivate($this->job, 'run', [null]);
113115
}
114116
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OC\Repair\NC29;
10+
11+
use OCP\BackgroundJob\IJobList;
12+
use OCP\Migration\IOutput;
13+
use PHPUnit\Framework\MockObject\MockObject;
14+
use Test\TestCase;
15+
16+
class SanitizeAccountPropertiesTest extends TestCase {
17+
18+
private IJobList&MockObject $jobList;
19+
private SanitizeAccountProperties $repairStep;
20+
21+
protected function setUp(): void {
22+
$this->jobList = $this->createMock(IJobList::class);
23+
24+
$this->repairStep = new SanitizeAccountProperties($this->jobList);
25+
}
26+
27+
public function testGetName(): void {
28+
self::assertStringContainsString('Validate account properties', $this->repairStep->getName());
29+
}
30+
31+
public function testRun(): void {
32+
$this->jobList->expects(self::once())
33+
->method('add')
34+
->with(SanitizeAccountPropertiesJob::class, null);
35+
36+
$output = $this->createMock(IOutput::class);
37+
$output->expects(self::once())
38+
->method('info')
39+
->with(self::matchesRegularExpression('/queued background/i'));
40+
41+
$this->repairStep->run($output);
42+
}
43+
}

0 commit comments

Comments
 (0)