Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
refactor: convert sanitize account properties repair step to backgrou…
…nd job

Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
susnux committed Feb 24, 2025
commit d3dbe3ab2c14760d4801c1167dc78d55943b9ebe
3 changes: 2 additions & 1 deletion lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1818,7 +1818,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',
Expand Down
3 changes: 2 additions & 1 deletion lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1851,7 +1851,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',
Expand Down
4 changes: 2 additions & 2 deletions lib/private/Repair.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -191,6 +191,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),
];
}

Expand All @@ -205,7 +206,6 @@ public static function getExpensiveRepairSteps() {
new OldGroupMembershipShares(\OC::$server->getDatabaseConnection(), \OC::$server->getGroupManager()),
new RemoveBrokenProperties(\OC::$server->getDatabaseConnection()),
new RepairMimeTypes(\OC::$server->getConfig(), \OC::$server->getDatabaseConnection()),
\OC::$server->get(ValidateAccountProperties::class),
\OC::$server->get(DeleteSchedulingObjects::class),
];
}
Expand Down
30 changes: 30 additions & 0 deletions lib/private/Repair/NC29/SanitizeAccountProperties.php
Original file line number Diff line number Diff line change
@@ -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.');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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');
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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]);
}
}
43 changes: 43 additions & 0 deletions tests/lib/Repair/NC29/SanitizeAccountPropertiesTest.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
Loading