diff options
28 files changed, 160 insertions, 229 deletions
diff --git a/apps/federatedfilesharing/lib/BackgroundJob/RetryJob.php b/apps/federatedfilesharing/lib/BackgroundJob/RetryJob.php index cf0691de3ac..f785f4fc528 100644 --- a/apps/federatedfilesharing/lib/BackgroundJob/RetryJob.php +++ b/apps/federatedfilesharing/lib/BackgroundJob/RetryJob.php @@ -41,19 +41,14 @@ use OCP\ILogger; * @package OCA\FederatedFileSharing\BackgroundJob */ class RetryJob extends Job { - - /** @var bool */ - private $retainJob = true; - - /** @var Notifications */ - private $notifications; + private bool $retainJob = true; + private Notifications $notifications; /** @var int max number of attempts to send the request */ - private $maxTry = 20; + private int $maxTry = 20; /** @var int how much time should be between two tries (10 minutes) */ - private $interval = 600; - + private int $interval = 600; public function __construct(Notifications $notifications, ITimeFactory $time) { @@ -62,14 +57,11 @@ class RetryJob extends Job { } /** - * run the job, then remove it from the jobList - * - * @param IJobList $jobList - * @param ILogger|null $logger + * Run the job, then remove it from the jobList */ - public function execute(IJobList $jobList, ILogger $logger = null) { + public function start(IJobList $jobList): void { if ($this->shouldRun($this->argument)) { - parent::execute($jobList, $logger); + parent::start($jobList); $jobList->remove($this, $this->argument); if ($this->retainJob) { $this->reAddJob($jobList, $this->argument); @@ -93,12 +85,9 @@ class RetryJob extends Job { } /** - * re-add background job with new arguments - * - * @param IJobList $jobList - * @param array $argument + * Re-add background job with new arguments */ - protected function reAddJob(IJobList $jobList, array $argument) { + protected function reAddJob(IJobList $jobList, array $argument): void { $jobList->add(RetryJob::class, [ 'remote' => $argument['remote'], @@ -113,12 +102,9 @@ class RetryJob extends Job { } /** - * test if it is time for the next run - * - * @param array $argument - * @return bool + * Test if it is time for the next run */ - protected function shouldRun(array $argument) { + protected function shouldRun(array $argument): bool { $lastRun = (int)$argument['lastRun']; return (($this->time->getTime() - $lastRun) > $this->interval); } diff --git a/apps/federation/lib/BackgroundJob/GetSharedSecret.php b/apps/federation/lib/BackgroundJob/GetSharedSecret.php index 75faa7ce1d9..3ca3e0a906b 100644 --- a/apps/federation/lib/BackgroundJob/GetSharedSecret.php +++ b/apps/federation/lib/BackgroundJob/GetSharedSecret.php @@ -39,7 +39,6 @@ use OCP\BackgroundJob\Job; use OCP\Http\Client\IClient; use OCP\Http\Client\IClientService; use OCP\Http\Client\IResponse; -use OCP\ILogger; use OCP\IURLGenerator; use OCP\OCS\IDiscoveryService; use Psr\Log\LoggerInterface; @@ -60,7 +59,6 @@ class GetSharedSecret extends Job { private LoggerInterface $logger; protected bool $retainJob = false; private string $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/shared-secret'; - /** 30 day = 2592000sec */ private int $maxLifespan = 2592000; @@ -83,16 +81,13 @@ class GetSharedSecret extends Job { } /** - * run the job, then remove it from the joblist - * - * @param IJobList $jobList - * @param ILogger|null $logger + * Run the job, then remove it from the joblist */ - public function execute(IJobList $jobList, ILogger $logger = null) { + public function start(IJobList $jobList): void { $target = $this->argument['url']; // only execute if target is still in the list of trusted domains if ($this->trustedServers->isTrustedServer($target)) { - $this->parentExecute($jobList, $logger); + $this->parentStart($jobList); } $jobList->remove($this, $this->argument); @@ -102,14 +97,8 @@ class GetSharedSecret extends Job { } } - /** - * Call execute() method of parent - * - * @param IJobList $jobList - * @param ILogger $logger - */ - protected function parentExecute($jobList, $logger = null) { - parent::execute($jobList, $logger); + protected function parentStart(IJobList $jobList): void { + parent::start($jobList); } protected function run($argument) { @@ -162,12 +151,10 @@ class GetSharedSecret extends Job { $status = -1; // There is no status code if we could not connect $this->logger->info('Could not connect to ' . $target, [ 'exception' => $e, - 'app' => 'federation', ]); } catch (\Throwable $e) { $status = Http::STATUS_INTERNAL_SERVER_ERROR; $this->logger->error($e->getMessage(), [ - 'app' => 'federation', 'exception' => $e, ]); } @@ -190,8 +177,8 @@ class GetSharedSecret extends Job { ); } else { $this->logger->error( - 'remote server "' . $target . '"" does not return a valid shared secret. Received data: ' . $body, - ['app' => 'federation'] + 'remote server "' . $target . '"" does not return a valid shared secret. Received data: ' . $body, + ['app' => 'federation'] ); $this->trustedServers->setServerStatus($target, TrustedServers::STATUS_FAILURE); } @@ -199,13 +186,13 @@ class GetSharedSecret extends Job { } /** - * re-add background job + * Re-add background job * * @param array $argument */ protected function reAddJob(array $argument): void { $url = $argument['url']; - $created = isset($argument['created']) ? (int)$argument['created'] : $this->time->getTime(); + $created = $argument['created'] ?? $this->time->getTime(); $token = $argument['token']; $this->jobList->add( GetSharedSecret::class, diff --git a/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php b/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php index 5344736b7f9..c0316b8247f 100644 --- a/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php +++ b/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php @@ -76,8 +76,7 @@ class GetSharedSecretTest extends TestCase { /** @var \PHPUnit\Framework\MockObject\MockObject|ITimeFactory */ private $timeFactory; - /** @var GetSharedSecret */ - private $getSharedSecret; + private GetSharedSecret $getSharedSecret; protected function setUp(): void { parent::setUp(); @@ -113,9 +112,9 @@ class GetSharedSecretTest extends TestCase { * @param bool $isTrustedServer * @param bool $retainBackgroundJob */ - public function testExecute($isTrustedServer, $retainBackgroundJob) { + public function testExecute(bool $isTrustedServer, bool $retainBackgroundJob): void { /** @var GetSharedSecret |\PHPUnit\Framework\MockObject\MockObject $getSharedSecret */ - $getSharedSecret = $this->getMockBuilder('OCA\Federation\BackgroundJob\GetSharedSecret') + $getSharedSecret = $this->getMockBuilder(GetSharedSecret::class) ->setConstructorArgs( [ $this->httpClientService, @@ -126,15 +125,15 @@ class GetSharedSecretTest extends TestCase { $this->discoverService, $this->timeFactory ] - )->setMethods(['parentExecute'])->getMock(); + )->setMethods(['parentStart'])->getMock(); $this->invokePrivate($getSharedSecret, 'argument', [['url' => 'url', 'token' => 'token']]); $this->trustedServers->expects($this->once())->method('isTrustedServer') ->with('url')->willReturn($isTrustedServer); if ($isTrustedServer) { - $getSharedSecret->expects($this->once())->method('parentExecute'); + $getSharedSecret->expects($this->once())->method('parentStart'); } else { - $getSharedSecret->expects($this->never())->method('parentExecute'); + $getSharedSecret->expects($this->never())->method('parentStart'); } $this->invokePrivate($getSharedSecret, 'retainJob', [$retainBackgroundJob]); $this->jobList->expects($this->once())->method('remove'); diff --git a/apps/files/lib/BackgroundJob/ScanFiles.php b/apps/files/lib/BackgroundJob/ScanFiles.php index 07794a28774..8dc9dcf37ff 100644 --- a/apps/files/lib/BackgroundJob/ScanFiles.php +++ b/apps/files/lib/BackgroundJob/ScanFiles.php @@ -25,6 +25,8 @@ namespace OCA\Files\BackgroundJob; use OC\Files\Utils\Scanner; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\TimedJob; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; @@ -37,13 +39,11 @@ use Psr\Log\LoggerInterface; * * @package OCA\Files\BackgroundJob */ -class ScanFiles extends \OC\BackgroundJob\TimedJob { - /** @var IConfig */ - private $config; - /** @var IEventDispatcher */ - private $dispatcher; +class ScanFiles extends TimedJob { + private IConfig $config; + private IEventDispatcher $dispatcher; private LoggerInterface $logger; - private $connection; + private IDBConnection $connection; /** Amount of users that should get scanned per execution */ public const USERS_PER_SESSION = 500; @@ -52,8 +52,10 @@ class ScanFiles extends \OC\BackgroundJob\TimedJob { IConfig $config, IEventDispatcher $dispatcher, LoggerInterface $logger, - IDBConnection $connection + IDBConnection $connection, + ITimeFactory $time ) { + parent::__construct($time); // Run once per 10 minutes $this->setInterval(60 * 10); @@ -63,10 +65,7 @@ class ScanFiles extends \OC\BackgroundJob\TimedJob { $this->connection = $connection; } - /** - * @param string $user - */ - protected function runScanner(string $user) { + protected function runScanner(string $user): void { try { $scanner = new Scanner( $user, @@ -95,7 +94,7 @@ class ScanFiles extends \OC\BackgroundJob\TimedJob { ->andWhere($query->expr()->gt('parent', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT))) ->setMaxResults(1); - return $query->execute()->fetchOne(); + return $query->executeQuery()->fetchOne(); } /** diff --git a/apps/files/tests/BackgroundJob/ScanFilesTest.php b/apps/files/tests/BackgroundJob/ScanFilesTest.php index 5b2d52b48d3..cf0120da09d 100644 --- a/apps/files/tests/BackgroundJob/ScanFilesTest.php +++ b/apps/files/tests/BackgroundJob/ScanFilesTest.php @@ -26,6 +26,7 @@ namespace OCA\Files\Tests\BackgroundJob; use OC\Files\Mount\MountPoint; use OC\Files\Storage\Temporary; use OCA\Files\BackgroundJob\ScanFiles; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IUser; @@ -64,6 +65,7 @@ class ScanFilesTest extends TestCase { $dispatcher, $logger, $connection, + $this->createMock(ITimeFactory::class) ]) ->setMethods(['runScanner']) ->getMock(); diff --git a/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php b/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php index 9e35273544b..c76033e0c79 100644 --- a/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php +++ b/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php @@ -30,46 +30,30 @@ use OCA\Files_Trashbin\AppInfo\Application; use OCA\Files_Trashbin\Expiration; use OCA\Files_Trashbin\Helper; use OCA\Files_Trashbin\Trashbin; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\TimedJob; use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; -class ExpireTrash extends \OC\BackgroundJob\TimedJob { +class ExpireTrash extends TimedJob { + private IConfig $config; + private Expiration $expiration; + private IUserManager $userManager; - /** @var IConfig */ - private $config; - - /** - * @var Expiration - */ - private $expiration; - - /** - * @var IUserManager - */ - private $userManager; - - public function __construct(IConfig $config = null, - IUserManager $userManager = null, - Expiration $expiration = null) { + public function __construct( + IConfig $config, + IUserManager $userManager, + Expiration $expiration, + ITimeFactory $time + ) { + parent::__construct($time); // Run once per 30 minutes $this->setInterval(60 * 30); - if ($config === null || $expiration === null || $userManager === null) { - $this->fixDIForJobs(); - } else { - $this->config = $config; - $this->userManager = $userManager; - $this->expiration = $expiration; - } - } - - protected function fixDIForJobs() { - /** @var Application $application */ - $application = \OC::$server->query(Application::class); - $this->config = $application->getContainer()->get(IConfig::class); - $this->userManager = \OC::$server->getUserManager(); - $this->expiration = $application->getContainer()->query('Expiration'); + $this->config = $config; + $this->userManager = $userManager; + $this->expiration = $expiration; } /** @@ -101,10 +85,8 @@ class ExpireTrash extends \OC\BackgroundJob\TimedJob { /** * Act on behalf on trash item owner - * @param string $user - * @return boolean */ - protected function setupFS($user) { + protected function setupFS(string $user): bool { \OC_Util::tearDownFS(); \OC_Util::setupFS($user); diff --git a/apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php b/apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php index c49501608f7..d8cce61ca52 100644 --- a/apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php +++ b/apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php @@ -27,12 +27,13 @@ namespace OCA\Files_Trashbin\Tests\BackgroundJob; use OCA\Files_Trashbin\BackgroundJob\ExpireTrash; use OCA\Files_Trashbin\Expiration; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJobList; use OCP\IConfig; -use OCP\ILogger; use OCP\IUserManager; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; +use Psr\Log\LoggerInterface; class ExpireTrashTest extends TestCase { /** @var IConfig|MockObject */ @@ -47,9 +48,12 @@ class ExpireTrashTest extends TestCase { /** @var IJobList|MockObject */ private $jobList; - /** @var ILogger|MockObject */ + /** @var LoggerInterface|MockObject */ private $logger; + /** @var ITimeFactory|MockObject */ + private $time; + protected function setUp(): void { parent::setUp(); @@ -58,6 +62,7 @@ class ExpireTrashTest extends TestCase { $this->expiration = $this->createMock(Expiration::class); $this->jobList = $this->createMock(IJobList::class); $this->logger = $this->createMock(ILogger::class); + $this->time = $this->createMock(ITimeFactory::class); $this->jobList->expects($this->once()) ->method('setLastRun'); @@ -77,7 +82,7 @@ class ExpireTrashTest extends TestCase { $this->expiration->expects($this->never()) ->method('getMaxAgeAsTimestamp'); - $job = new ExpireTrash($this->config, $this->userManager, $this->expiration); - $job->execute($this->jobList, $this->logger); + $job = new ExpireTrash($this->config, $this->userManager, $this->expiration, $this->time); + $job->start($this->jobList); } } diff --git a/apps/files_versions/lib/BackgroundJob/ExpireVersions.php b/apps/files_versions/lib/BackgroundJob/ExpireVersions.php index a8a311f0a05..7e714a059d0 100644 --- a/apps/files_versions/lib/BackgroundJob/ExpireVersions.php +++ b/apps/files_versions/lib/BackgroundJob/ExpireVersions.php @@ -27,27 +27,21 @@ namespace OCA\Files_Versions\BackgroundJob; use OCA\Files_Versions\Expiration; use OCA\Files_Versions\Storage; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\TimedJob; use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; -class ExpireVersions extends \OC\BackgroundJob\TimedJob { +class ExpireVersions extends TimedJob { public const ITEMS_PER_SESSION = 1000; - /** @var IConfig */ - private $config; + private IConfig $config; + private Expiration $expiration; + private IUserManager $userManager; - /** - * @var Expiration - */ - private $expiration; - - /** - * @var IUserManager - */ - private $userManager; - - public function __construct(IConfig $config, IUserManager $userManager, Expiration $expiration) { + public function __construct(IConfig $config, IUserManager $userManager, Expiration $expiration, ITimeFactory $time) { + parent::__construct($time); // Run once per 30 minutes $this->setInterval(60 * 30); @@ -78,10 +72,8 @@ class ExpireVersions extends \OC\BackgroundJob\TimedJob { /** * Act on behalf on trash item owner - * @param string $user - * @return boolean */ - protected function setupFS($user) { + protected function setupFS(string $user): bool { \OC_Util::tearDownFS(); \OC_Util::setupFS($user); diff --git a/apps/files_versions/tests/BackgroundJob/ExpireVersionsTest.php b/apps/files_versions/tests/BackgroundJob/ExpireVersionsTest.php index a1acffb8543..ca584ae9f53 100644 --- a/apps/files_versions/tests/BackgroundJob/ExpireVersionsTest.php +++ b/apps/files_versions/tests/BackgroundJob/ExpireVersionsTest.php @@ -25,6 +25,7 @@ namespace OCA\Files_Versions\Tests\BackgroundJob; use OCA\Files_Versions\BackgroundJob\ExpireVersions; use OCA\Files_Versions\Expiration; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJobList; use OCP\IConfig; use OCP\ILogger; @@ -46,9 +47,6 @@ class ExpireVersionsTest extends TestCase { /** @var IJobList|MockObject */ private $jobList; - /** @var ILogger|MockObject */ - private $logger; - protected function setUp(): void { parent::setUp(); @@ -56,7 +54,6 @@ class ExpireVersionsTest extends TestCase { $this->userManager = $this->createMock(IUserManager::class); $this->expiration = $this->createMock(Expiration::class); $this->jobList = $this->createMock(IJobList::class); - $this->logger = $this->createMock(ILogger::class); $this->jobList->expects($this->once()) ->method('setLastRun'); @@ -71,7 +68,7 @@ class ExpireVersionsTest extends TestCase { $this->expiration->expects($this->never()) ->method('getMaxAgeAsTimestamp'); - $job = new ExpireVersions($this->config, $this->userManager, $this->expiration); - $job->execute($this->jobList, $this->logger); + $job = new ExpireVersions($this->config, $this->userManager, $this->expiration, $this->createMock(ITimeFactory::class)); + $job->start($this->jobList); } } diff --git a/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php b/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php index f905f9173b6..39d8118abe7 100644 --- a/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php +++ b/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php @@ -44,22 +44,14 @@ use OCP\IUser; use OCP\IUserManager; class RetryJob extends Job { - /** @var IClientService */ - private $clientService; - /** @var string */ - private $lookupServer; - /** @var IConfig */ - private $config; - /** @var IUserManager */ - private $userManager; - /** @var IAccountManager */ - private $accountManager; - /** @var Signer */ - private $signer; - /** @var int */ - protected $retries = 0; - /** @var bool */ - protected $retainJob = false; + private IClientService $clientService; + private string $lookupServer; + private IConfig $config; + private IUserManager $userManager; + private IAccountManager $accountManager; + private Signer $signer; + protected int $retries = 0; + protected bool $retainJob = false; /** * @param ITimeFactory $time @@ -90,19 +82,16 @@ class RetryJob extends Job { } /** - * run the job, then remove it from the jobList - * - * @param IJobList $jobList - * @param ILogger|null $logger + * Run the job, then remove it from the jobList */ - public function execute(IJobList $jobList, ILogger $logger = null): void { + public function start(IJobList $jobList): void { if (!isset($this->argument['userId'])) { // Old background job without user id, just drop it. $jobList->remove($this, $this->argument); return; } - $this->retries = (int) $this->config->getUserValue($this->argument['userId'], 'lookup_server_connector', 'update_retries', 0); + $this->retries = (int) $this->config->getUserValue($this->argument['userId'], 'lookup_server_connector', 'update_retries', '0'); if ($this->shouldRemoveBackgroundJob()) { $jobList->remove($this, $this->argument); @@ -110,7 +99,7 @@ class RetryJob extends Job { } if ($this->shouldRun()) { - parent::execute($jobList, $logger); + parent::start($jobList); if (!$this->retainJob) { $jobList->remove($this, $this->argument); } @@ -124,8 +113,6 @@ class RetryJob extends Job { * - no valid lookup server URL given * - lookup server was disabled by the admin * - max retries are reached (set to 5) - * - * @return bool */ protected function shouldRemoveBackgroundJob(): bool { return $this->config->getSystemValueBool('has_internet_connection', true) === false || diff --git a/apps/user_ldap/lib/Jobs/Sync.php b/apps/user_ldap/lib/Jobs/Sync.php index 3d0dd88dfd2..d9171f4aab7 100644 --- a/apps/user_ldap/lib/Jobs/Sync.php +++ b/apps/user_ldap/lib/Jobs/Sync.php @@ -24,7 +24,6 @@ */ namespace OCA\User_LDAP\Jobs; -use OC\BackgroundJob\TimedJob; use OC\ServerNotAvailableException; use OCA\User_LDAP\AccessFactory; use OCA\User_LDAP\Configuration; @@ -33,6 +32,8 @@ use OCA\User_LDAP\Helper; use OCA\User_LDAP\LDAP; use OCA\User_LDAP\Mapping\UserMapping; use OCA\User_LDAP\User\Manager; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\TimedJob; use OCP\IAvatarManager; use OCP\IConfig; use OCP\IDBConnection; @@ -68,7 +69,8 @@ class Sync extends TimedJob { /** @var AccessFactory */ protected $accessFactory; - public function __construct(Manager $userManager) { + public function __construct(Manager $userManager, ITimeFactory $time) { + parent::__construct($time); $this->userManager = $userManager; $this->setInterval( \OC::$server->getConfig()->getAppValue( @@ -298,8 +300,6 @@ class Sync extends TimedJob { /** * "fixes" DI - * - * @param array $argument */ public function setArgument($argument) { if (isset($argument['config'])) { diff --git a/apps/user_ldap/tests/Jobs/SyncTest.php b/apps/user_ldap/tests/Jobs/SyncTest.php index 5592a2531bc..8d23efb4da8 100644 --- a/apps/user_ldap/tests/Jobs/SyncTest.php +++ b/apps/user_ldap/tests/Jobs/SyncTest.php @@ -34,6 +34,7 @@ use OCA\User_LDAP\Jobs\Sync; use OCA\User_LDAP\LDAP; use OCA\User_LDAP\Mapping\UserMapping; use OCA\User_LDAP\User\Manager; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IAvatarManager; use OCP\IConfig; use OCP\IDBConnection; @@ -98,7 +99,7 @@ class SyncTest extends TestCase { 'accessFactory' => $this->accessFactory, ]; - $this->sync = new Sync($this->userManager); + $this->sync = new Sync($this->userManager, $this->createMock(ITimeFactory::class)); } public function intervalDataProvider() { diff --git a/core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php b/core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php index ceff579b1c7..7ab9fb47d54 100644 --- a/core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php +++ b/core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php @@ -26,7 +26,8 @@ declare(strict_types=1); */ namespace OC\Core\BackgroundJobs; -use OC\BackgroundJob\QueuedJob; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\QueuedJob; use OCP\IConfig; use Psr\Log\LoggerInterface; @@ -34,7 +35,8 @@ class BackgroundCleanupUpdaterBackupsJob extends QueuedJob { protected IConfig $config; protected LoggerInterface $log; - public function __construct(IConfig $config, LoggerInterface $log) { + public function __construct(IConfig $config, LoggerInterface $log, ITimeFactory $time) { + parent::__construct($time); $this->config = $config; $this->log = $log; } diff --git a/core/BackgroundJobs/CheckForUserCertificates.php b/core/BackgroundJobs/CheckForUserCertificates.php index 492fadde854..11b851fff94 100644 --- a/core/BackgroundJobs/CheckForUserCertificates.php +++ b/core/BackgroundJobs/CheckForUserCertificates.php @@ -26,7 +26,8 @@ declare(strict_types=1); */ namespace OC\Core\BackgroundJobs; -use OC\BackgroundJob\QueuedJob; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\QueuedJob; use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; @@ -39,7 +40,8 @@ class CheckForUserCertificates extends QueuedJob { private IUserManager $userManager; private IRootFolder $rootFolder; - public function __construct(IConfig $config, IUserManager $userManager, IRootFolder $rootFolder) { + public function __construct(IConfig $config, IUserManager $userManager, IRootFolder $rootFolder, ITimeFactory $time) { + parent::__construct($time); $this->config = $config; $this->userManager = $userManager; $this->rootFolder = $rootFolder; diff --git a/core/BackgroundJobs/LookupServerSendCheckBackgroundJob.php b/core/BackgroundJobs/LookupServerSendCheckBackgroundJob.php index 8945be11a1c..76e0550830c 100644 --- a/core/BackgroundJobs/LookupServerSendCheckBackgroundJob.php +++ b/core/BackgroundJobs/LookupServerSendCheckBackgroundJob.php @@ -25,7 +25,8 @@ declare(strict_types=1); */ namespace OC\Core\BackgroundJobs; -use OC\BackgroundJob\QueuedJob; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\QueuedJob; use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; @@ -34,7 +35,8 @@ class LookupServerSendCheckBackgroundJob extends QueuedJob { protected IConfig $config; private IUserManager $userManager; - public function __construct(IConfig $config, IUserManager $userManager) { + public function __construct(IConfig $config, IUserManager $userManager, ITimeFactory $time) { + parent::__construct($time); $this->config = $config; $this->userManager = $userManager; } diff --git a/lib/private/BackgroundJob/Job.php b/lib/private/BackgroundJob/Job.php index 399ff05134e..ffcaaf8c36d 100644 --- a/lib/private/BackgroundJob/Job.php +++ b/lib/private/BackgroundJob/Job.php @@ -66,6 +66,10 @@ abstract class Job implements IJob { } } + public function start(IJobList $jobList): void { + $this->execute($jobList); + } + abstract protected function run($argument); public function setId(int $id) { diff --git a/lib/private/BackgroundJob/Legacy/QueuedJob.php b/lib/private/BackgroundJob/Legacy/QueuedJob.php index fd001738d4f..680c12046c9 100644 --- a/lib/private/BackgroundJob/Legacy/QueuedJob.php +++ b/lib/private/BackgroundJob/Legacy/QueuedJob.php @@ -26,7 +26,7 @@ namespace OC\BackgroundJob\Legacy; /** * @deprecated internal class, use \OCP\BackgroundJob\QueuedJob */ -class QueuedJob extends \OC\BackgroundJob\QueuedJob { +class QueuedJob extends \OCP\BackgroundJob\QueuedJob { public function run($argument) { $class = $argument['klass']; $method = $argument['method']; diff --git a/lib/private/BackgroundJob/Legacy/RegularJob.php b/lib/private/BackgroundJob/Legacy/RegularJob.php index 0f337a35eb0..c01d58efa2f 100644 --- a/lib/private/BackgroundJob/Legacy/RegularJob.php +++ b/lib/private/BackgroundJob/Legacy/RegularJob.php @@ -27,7 +27,7 @@ use OCP\AutoloadNotAllowedException; /** * @deprecated internal class, use \OCP\BackgroundJob\QueuedJob */ -class RegularJob extends \OC\BackgroundJob\Job { +class RegularJob extends \OCP\BackgroundJob\Job { public function run($argument) { try { if (is_callable($argument)) { diff --git a/lib/private/Command/CallableJob.php b/lib/private/Command/CallableJob.php index 8bb3c76c9af..7f515660955 100644 --- a/lib/private/Command/CallableJob.php +++ b/lib/private/Command/CallableJob.php @@ -21,7 +21,7 @@ */ namespace OC\Command; -use OC\BackgroundJob\QueuedJob; +use OCP\BackgroundJob\QueuedJob; class CallableJob extends QueuedJob { protected function run($serializedCallable) { diff --git a/lib/private/Command/ClosureJob.php b/lib/private/Command/ClosureJob.php index 5639852e4db..7ca45cd5fc0 100644 --- a/lib/private/Command/ClosureJob.php +++ b/lib/private/Command/ClosureJob.php @@ -22,7 +22,7 @@ */ namespace OC\Command; -use OC\BackgroundJob\QueuedJob; +use OCP\BackgroundJob\QueuedJob; use Laravel\SerializableClosure\SerializableClosure as LaravelClosure; use Opis\Closure\SerializableClosure as OpisClosure; diff --git a/lib/private/Command/CommandJob.php b/lib/private/Command/CommandJob.php index 5b267162c81..477fd2a8683 100644 --- a/lib/private/Command/CommandJob.php +++ b/lib/private/Command/CommandJob.php @@ -22,7 +22,7 @@ */ namespace OC\Command; -use OC\BackgroundJob\QueuedJob; +use OCP\BackgroundJob\QueuedJob; use OCP\Command\ICommand; /** diff --git a/lib/private/Log/Rotate.php b/lib/private/Log/Rotate.php index 58b2932b417..20bc3327f92 100644 --- a/lib/private/Log/Rotate.php +++ b/lib/private/Log/Rotate.php @@ -32,7 +32,7 @@ use OCP\Log\RotationTrait; * For more professional log management set the 'logfile' config to a different * location and manage that with your own tools. */ -class Rotate extends \OC\BackgroundJob\Job { +class Rotate extends \OCP\BackgroundJob\Job { use RotationTrait; public function run($dummy) { diff --git a/lib/private/Migration/BackgroundRepair.php b/lib/private/Migration/BackgroundRepair.php index 03a3d3f4a7c..f3ae8f4bdcf 100644 --- a/lib/private/Migration/BackgroundRepair.php +++ b/lib/private/Migration/BackgroundRepair.php @@ -26,13 +26,12 @@ */ namespace OC\Migration; -use OC\BackgroundJob\JobList; -use OC\BackgroundJob\TimedJob; use OC\NeedsUpdateException; use OC\Repair; use OC_App; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJobList; -use OCP\ILogger; +use OCP\BackgroundJob\TimedJob; use Psr\Log\LoggerInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -42,33 +41,16 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface; * @package OC\Migration */ class BackgroundRepair extends TimedJob { + private IJobList $jobList; + private LoggerInterface $logger; + private EventDispatcherInterface $dispatcher; - /** @var IJobList */ - private $jobList; - - /** @var ILogger */ - private $logger; - - /** @var EventDispatcherInterface */ - private $dispatcher; - - public function __construct(EventDispatcherInterface $dispatcher) { + public function __construct(EventDispatcherInterface $dispatcher, ITimeFactory $time, LoggerInterface $logger, IJobList $jobList) { + parent::__construct($time); $this->dispatcher = $dispatcher; - } - - /** - * run the job, then remove it from the job list - * - * @param JobList $jobList - * @param ILogger|null $logger - */ - public function execute($jobList, ILogger $logger = null) { - // add an interval of 15 mins - $this->setInterval(15 * 60); - - $this->jobList = $jobList; $this->logger = $logger; - parent::execute($jobList, $logger); + $this->jobList = $jobList; + $this->setInterval(15 * 60); } /** @@ -97,8 +79,9 @@ class BackgroundRepair extends TimedJob { try { $repair->addStep($step); } catch (\Exception $ex) { - $this->logger->logException($ex, [ - 'app' => 'migration' + $this->logger->error($ex->getMessage(), [ + 'app' => 'migration', + 'exception' => $ex, ]); // remove the job - we can never execute it diff --git a/lib/private/Security/VerificationToken/CleanUpJob.php b/lib/private/Security/VerificationToken/CleanUpJob.php index be9d24ece45..4510dcffe0a 100644 --- a/lib/private/Security/VerificationToken/CleanUpJob.php +++ b/lib/private/Security/VerificationToken/CleanUpJob.php @@ -28,27 +28,20 @@ namespace OC\Security\VerificationToken; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IConfig; -use OCP\ILogger; use OCP\IUserManager; +use OCP\BackgroundJob\IJobList; +use OCP\BackgroundJob\Job; use OCP\Security\VerificationToken\InvalidTokenException; use OCP\Security\VerificationToken\IVerificationToken; -class CleanUpJob extends \OCP\BackgroundJob\Job { - - /** @var int */ - protected $runNotBefore; - /** @var string */ - protected $userId; - /** @var string */ - protected $subject; - /** @var string */ - protected $pwdPrefix; - /** @var IConfig */ - private $config; - /** @var IVerificationToken */ - private $verificationToken; - /** @var IUserManager */ - private $userManager; +class CleanUpJob extends Job { + protected ?int $runNotBefore = null; + protected ?string $userId = null; + protected ?string $subject = null; + protected ?string $pwdPrefix = null; + private IConfig $config; + private IVerificationToken $verificationToken; + private IUserManager $userManager; public function __construct(ITimeFactory $time, IConfig $config, IVerificationToken $verificationToken, IUserManager $userManager) { parent::__construct($time); @@ -81,10 +74,10 @@ class CleanUpJob extends \OCP\BackgroundJob\Job { } } - public function execute($jobList, ILogger $logger = null) { + public function start(IJobList $jobList): void { if ($this->time->getTime() >= $this->runNotBefore) { $jobList->remove($this, $this->argument); - parent::execute($jobList, $logger); + parent::start($jobList); } } } diff --git a/lib/public/BackgroundJob/Job.php b/lib/public/BackgroundJob/Job.php index 5831891705a..d60fb5905c9 100644 --- a/lib/public/BackgroundJob/Job.php +++ b/lib/public/BackgroundJob/Job.php @@ -28,6 +28,7 @@ namespace OCP\BackgroundJob; use OCP\AppFramework\Utility\ITimeFactory; use OCP\ILogger; +use Psr\Log\LoggerInterface; /** * Base class for background jobs diff --git a/lib/public/BackgroundJob/QueuedJob.php b/lib/public/BackgroundJob/QueuedJob.php index 71f09fccdea..b9c4d693b8f 100644 --- a/lib/public/BackgroundJob/QueuedJob.php +++ b/lib/public/BackgroundJob/QueuedJob.php @@ -51,7 +51,7 @@ abstract class QueuedJob extends Job { /** * Run the job, then remove it from the joblist * - * @since 15.0.0 + * @since 25.0.0 */ final public function start(IJobList $jobList): void { $jobList->remove($this, $this->argument); diff --git a/tests/lib/BackgroundJob/DummyJobList.php b/tests/lib/BackgroundJob/DummyJobList.php index be9c06257b7..4d14ed9e7db 100644 --- a/tests/lib/BackgroundJob/DummyJobList.php +++ b/tests/lib/BackgroundJob/DummyJobList.php @@ -33,7 +33,7 @@ class DummyJobList extends \OC\BackgroundJob\JobList { public function add($job, $argument = null): void { if (is_string($job)) { /** @var IJob $job */ - $job = new $job; + $job = \OCP\Server::get($job); } $job->setArgument($argument); if (!$this->has($job, null)) { diff --git a/tests/lib/Migration/BackgroundRepairTest.php b/tests/lib/Migration/BackgroundRepairTest.php index 4e95915e624..7da9b18de30 100644 --- a/tests/lib/Migration/BackgroundRepairTest.php +++ b/tests/lib/Migration/BackgroundRepairTest.php @@ -23,12 +23,13 @@ namespace Test\Migration; use OC\Migration\BackgroundRepair; use OC\NeedsUpdateException; -use OCP\ILogger; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\Migration\IOutput; use OCP\Migration\IRepairStep; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\EventDispatcher\GenericEvent; use Test\TestCase; +use Psr\Log\LoggerInterface; class TestRepairStep implements IRepairStep { @@ -62,31 +63,37 @@ class BackgroundRepairTest extends TestCase { /** @var BackgroundRepair|\PHPUnit\Framework\MockObject\MockObject */ private $job; - /** @var ILogger|\PHPUnit\Framework\MockObject\MockObject */ + /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */ private $logger; /** @var EventDispatcherInterface|\PHPUnit\Framework\MockObject\MockObject $dispatcher */ private $dispatcher; + /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject $dispatcher */ + private $time; + protected function setUp(): void { parent::setUp(); $this->jobList = $this->getMockBuilder('OC\BackgroundJob\JobList') ->disableOriginalConstructor() ->getMock(); - $this->logger = $this->getMockBuilder(ILogger::class) + $this->logger = $this->getMockBuilder(LoggerInterface::class) ->disableOriginalConstructor() ->getMock(); $this->dispatcher = $this->createMock(EventDispatcherInterface::class); + $this->time = $this->createMock(ITimeFactory::class); + $this->time->method('getTime') + ->willReturn(999999); $this->job = $this->getMockBuilder(BackgroundRepair::class) - ->setConstructorArgs([$this->dispatcher]) + ->setConstructorArgs([$this->dispatcher, $this->time, $this->logger, $this->jobList]) ->setMethods(['loadApp']) ->getMock(); } public function testNoArguments() { $this->jobList->expects($this->once())->method('remove'); - $this->job->execute($this->jobList); + $this->job->start($this->jobList); } public function testAppUpgrading() { @@ -96,20 +103,20 @@ class BackgroundRepairTest extends TestCase { 'app' => 'test', 'step' => 'j' ]); - $this->job->execute($this->jobList); + $this->job->start($this->jobList); } public function testUnknownStep() { $this->dispatcher->expects($this->never())->method('dispatch'); $this->jobList->expects($this->once())->method('remove'); - $this->logger->expects($this->once())->method('logException'); + $this->logger->expects($this->once())->method('error'); $this->job->setArgument([ 'app' => 'test', 'step' => 'j' ]); - $this->job->execute($this->jobList, $this->logger); + $this->job->start($this->jobList); } public function testWorkingStep() { @@ -122,6 +129,6 @@ class BackgroundRepairTest extends TestCase { 'app' => 'test', 'step' => '\Test\Migration\TestRepairStep' ]); - $this->job->execute($this->jobList, $this->logger); + $this->job->start($this->jobList); } } |