From 0b7779b6ff7dcb603d1088f3ea8fbf42ce5a98c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 23 May 2022 11:21:22 +0200 Subject: [PATCH 01/10] Clean up JobList class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/BackgroundJob/JobList.php | 78 +++++++++------------------ 1 file changed, 26 insertions(+), 52 deletions(-) diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index 7ab86df84556a..fa47f9b6daa1a 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -41,20 +41,10 @@ class JobList implements IJobList { - /** @var IDBConnection */ - protected $connection; + protected IDBConnection $connection; + protected IConfig $config; + protected ITimeFactory $timeFactory; - /**@var IConfig */ - protected $config; - - /**@var ITimeFactory */ - protected $timeFactory; - - /** - * @param IDBConnection $connection - * @param IConfig $config - * @param ITimeFactory $timeFactory - */ public function __construct(IDBConnection $connection, IConfig $config, ITimeFactory $timeFactory) { $this->connection = $connection; $this->config = $config; @@ -65,7 +55,7 @@ public function __construct(IDBConnection $connection, IConfig $config, ITimeFac * @param IJob|string $job * @param mixed $argument */ - public function add($job, $argument = null) { + public function add($job, $argument = null): void { if ($job instanceof IJob) { $class = get_class($job); } else { @@ -101,7 +91,7 @@ public function add($job, $argument = null) { * @param IJob|string $job * @param mixed $argument */ - public function remove($job, $argument = null) { + public function remove($job, $argument = null): void { if ($job instanceof IJob) { $class = get_class($job); } else { @@ -133,14 +123,11 @@ public function remove($job, $argument = null) { } } - /** - * @param int $id - */ - protected function removeById($id) { + protected function removeById(int $id): void { $query = $this->connection->getQueryBuilder(); $query->delete('jobs') ->where($query->expr()->eq('id', $query->createNamedParameter($id, IQueryBuilder::PARAM_INT))); - $query->execute(); + $query->executeStatement(); } /** @@ -148,9 +135,8 @@ protected function removeById($id) { * * @param IJob|string $job * @param mixed $argument - * @return bool */ - public function has($job, $argument) { + public function has($job, $argument): bool { if ($job instanceof IJob) { $class = get_class($job); } else { @@ -165,7 +151,7 @@ public function has($job, $argument) { ->andWhere($query->expr()->eq('argument_hash', $query->createNamedParameter(md5($argument)))) ->setMaxResults(1); - $result = $query->execute(); + $result = $query->executeQuery(); $row = $result->fetch(); $result->closeCursor(); @@ -183,7 +169,7 @@ public function getAll() { $query = $this->connection->getQueryBuilder(); $query->select('*') ->from('jobs'); - $result = $query->execute(); + $result = $query->executeQuery(); $jobs = []; while ($row = $result->fetch()) { @@ -199,9 +185,6 @@ public function getAll() { /** * get the next job in the list - * - * @param bool $onlyTimeSensitive - * @return IJob|null */ public function getNext(bool $onlyTimeSensitive = false): ?IJob { $query = $this->connection->getQueryBuilder(); @@ -224,7 +207,7 @@ public function getNext(bool $onlyTimeSensitive = false): ?IJob { ->andWhere($update->expr()->eq('reserved_at', $update->createParameter('reserved_at'))) ->andWhere($update->expr()->eq('last_checked', $update->createParameter('last_checked'))); - $result = $query->execute(); + $result = $query->executeQuery(); $row = $result->fetch(); $result->closeCursor(); @@ -232,7 +215,7 @@ public function getNext(bool $onlyTimeSensitive = false): ?IJob { $update->setParameter('jobid', $row['id']); $update->setParameter('reserved_at', $row['reserved_at']); $update->setParameter('last_checked', $row['last_checked']); - $count = $update->execute(); + $count = $update->executeStatement(); if ($count === 0) { // Background job already executed elsewhere, try again. @@ -247,7 +230,7 @@ public function getNext(bool $onlyTimeSensitive = false): ?IJob { ->set('reserved_at', $reset->expr()->literal(0, IQueryBuilder::PARAM_INT)) ->set('last_checked', $reset->createNamedParameter($this->timeFactory->getTime() + 12 * 3600, IQueryBuilder::PARAM_INT)) ->where($reset->expr()->eq('id', $reset->createNamedParameter($row['id'], IQueryBuilder::PARAM_INT))); - $reset->execute(); + $reset->executeStatement(); // Background job from disabled app, try again. return $this->getNext($onlyTimeSensitive); @@ -261,9 +244,8 @@ public function getNext(bool $onlyTimeSensitive = false): ?IJob { /** * @param int $id - * @return IJob|null */ - public function getById($id) { + public function getById($id): ?IJob { $row = $this->getDetailsById($id); if ($row) { @@ -292,15 +274,14 @@ public function getDetailsById(int $id): ?array { /** * get the job object from a row in the db * - * @param array $row - * @return IJob|null + * @param array{class:class-string, id:mixed, last_run:mixed, argument:string} $row */ - private function buildJob($row) { + private function buildJob(array $row): ?IJob { try { try { // Try to load the job as a service /** @var IJob $job */ - $job = \OC::$server->query($row['class']); + $job = \OC::$server->get($row['class']); } catch (QueryException $e) { if (class_exists($row['class'])) { $class = $row['class']; @@ -327,33 +308,27 @@ private function buildJob($row) { /** * set the job that was last ran - * - * @param IJob $job */ - public function setLastJob(IJob $job) { + public function setLastJob(IJob $job): void { $this->unlockJob($job); - $this->config->setAppValue('backgroundjob', 'lastjob', $job->getId()); + $this->config->setAppValue('backgroundjob', 'lastjob', (string)$job->getId()); } /** * Remove the reservation for a job - * - * @param IJob $job */ - public function unlockJob(IJob $job) { + public function unlockJob(IJob $job): void { $query = $this->connection->getQueryBuilder(); $query->update('jobs') ->set('reserved_at', $query->expr()->literal(0, IQueryBuilder::PARAM_INT)) ->where($query->expr()->eq('id', $query->createNamedParameter($job->getId(), IQueryBuilder::PARAM_INT))); - $query->execute(); + $query->executeStatement(); } /** * set the lastRun of $job to now - * - * @param IJob $job */ - public function setLastRun(IJob $job) { + public function setLastRun(IJob $job): void { $query = $this->connection->getQueryBuilder(); $query->update('jobs') ->set('last_run', $query->createNamedParameter(time(), IQueryBuilder::PARAM_INT)) @@ -364,19 +339,18 @@ public function setLastRun(IJob $job) { $query->set('time_sensitive', $query->createNamedParameter(IJob::TIME_INSENSITIVE)); } - $query->execute(); + $query->executeStatement(); } /** - * @param IJob $job - * @param $timeTaken + * @param int $timeTaken */ - public function setExecutionTime(IJob $job, $timeTaken) { + public function setExecutionTime(IJob $job, $timeTaken): void { $query = $this->connection->getQueryBuilder(); $query->update('jobs') ->set('execution_duration', $query->createNamedParameter($timeTaken, IQueryBuilder::PARAM_INT)) ->where($query->expr()->eq('id', $query->createNamedParameter($job->getId(), IQueryBuilder::PARAM_INT))); - $query->execute(); + $query->executeStatement(); } /** From 3d0117990749bebe5394ff664d62494ae9a6fa1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 23 May 2022 12:30:26 +0200 Subject: [PATCH 02/10] Add command to list jobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- core/Command/Background/ListCommand.php | 95 +++++++++++++++++++++ core/register_command.php | 1 + lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/BackgroundJob/JobList.php | 18 +++- lib/public/BackgroundJob/IJobList.php | 18 ++-- 6 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 core/Command/Background/ListCommand.php diff --git a/core/Command/Background/ListCommand.php b/core/Command/Background/ListCommand.php new file mode 100644 index 0000000000000..a0bf611606f2f --- /dev/null +++ b/core/Command/Background/ListCommand.php @@ -0,0 +1,95 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Core\Command\Background; + +use OC\Core\Command\Base; +use OCP\BackgroundJob\IJob; +use OCP\BackgroundJob\IJobList; +use OCP\ILogger; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; + +class ListCommand extends Base { + /** @var IJobList */ + protected $jobList; + /** @var ILogger */ + protected $logger; + + public function __construct(IJobList $jobList, + ILogger $logger) { + parent::__construct(); + $this->jobList = $jobList; + $this->logger = $logger; + } + + protected function configure(): void { + $this + ->setName('background-job:list') + ->setDescription('List background jobs') + ->addOption( + 'class', + 'c', + InputOption::VALUE_OPTIONAL, + 'Job class to search for', + null + )->addOption( + 'limit', + 'l', + InputOption::VALUE_OPTIONAL, + 'Number of jobs to retrieve', + '10' + )->addOption( + 'offset', + 'o', + InputOption::VALUE_OPTIONAL, + 'Offset for retrieving jobs', + '0' + ) + ; + parent::configure(); + } + + protected function execute(InputInterface $input, OutputInterface $output): int { + $jobs = $this->jobList->getJobs($input->getOption('class'), (int)$input->getOption('limit'), (int)$input->getOption('offset')); + $this->writeArrayInOutputFormat($input, $output, $this->formatJobs($jobs)); + return 0; + } + + protected function formatJobs(array $jobs): array { + return array_map( + fn($job) => [ + 'id' => $job->getId(), + 'class' => get_class($job), + 'last_run' => $job->getLastRun(), + 'argument' => json_encode($job->getArgument()), + ], + $jobs + ); + } +} diff --git a/core/register_command.php b/core/register_command.php index de04fedf30d92..25c368d915c86 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -90,6 +90,7 @@ $application->add(new OC\Core\Command\Background\WebCron(\OC::$server->getConfig())); $application->add(new OC\Core\Command\Background\Ajax(\OC::$server->getConfig())); $application->add(new OC\Core\Command\Background\Job(\OC::$server->getJobList(), \OC::$server->getLogger())); + $application->add(new OC\Core\Command\Background\ListCommand(\OC::$server->getJobList(), \OC::$server->getLogger())); $application->add(\OC::$server->query(\OC\Core\Command\Broadcast\Test::class)); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 49957c1f3acde..f1436219db035 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -859,6 +859,7 @@ 'OC\\Core\\Command\\Background\\Base' => $baseDir . '/core/Command/Background/Base.php', 'OC\\Core\\Command\\Background\\Cron' => $baseDir . '/core/Command/Background/Cron.php', 'OC\\Core\\Command\\Background\\Job' => $baseDir . '/core/Command/Background/Job.php', + 'OC\\Core\\Command\\Background\\ListCommand' => $baseDir . '/core/Command/Background/ListCommand.php', 'OC\\Core\\Command\\Background\\WebCron' => $baseDir . '/core/Command/Background/WebCron.php', 'OC\\Core\\Command\\Base' => $baseDir . '/core/Command/Base.php', 'OC\\Core\\Command\\Broadcast\\Test' => $baseDir . '/core/Command/Broadcast/Test.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index bb9cb0de31f3a..f8733fbc73b8c 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -892,6 +892,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Command\\Background\\Base' => __DIR__ . '/../../..' . '/core/Command/Background/Base.php', 'OC\\Core\\Command\\Background\\Cron' => __DIR__ . '/../../..' . '/core/Command/Background/Cron.php', 'OC\\Core\\Command\\Background\\Job' => __DIR__ . '/../../..' . '/core/Command/Background/Job.php', + 'OC\\Core\\Command\\Background\\ListCommand' => __DIR__ . '/../../..' . '/core/Command/Background/ListCommand.php', 'OC\\Core\\Command\\Background\\WebCron' => __DIR__ . '/../../..' . '/core/Command/Background/WebCron.php', 'OC\\Core\\Command\\Base' => __DIR__ . '/../../..' . '/core/Command/Base.php', 'OC\\Core\\Command\\Broadcast\\Test' => __DIR__ . '/../../..' . '/core/Command/Broadcast/Test.php', diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index fa47f9b6daa1a..8ba993646a951 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -166,9 +166,25 @@ public function has($job, $argument): bool { * memory problems when creating too many instances. */ public function getAll() { + return $this->getJobs(null, null, 0); + } + + public function getJobs($job, ?int $limit, int $offset): array { $query = $this->connection->getQueryBuilder(); $query->select('*') - ->from('jobs'); + ->from('jobs') + ->setMaxResults($limit) + ->setFirstResult($offset); + + if ($job !== null) { + if ($job instanceof IJob) { + $class = get_class($job); + } else { + $class = $job; + } + $query->where($query->expr()->eq('class', $query->createNamedParameter($class))); + } + $result = $query->executeQuery(); $jobs = []; diff --git a/lib/public/BackgroundJob/IJobList.php b/lib/public/BackgroundJob/IJobList.php index eab37a03f36ac..7bc7420f35b93 100644 --- a/lib/public/BackgroundJob/IJobList.php +++ b/lib/public/BackgroundJob/IJobList.php @@ -82,6 +82,15 @@ public function has($job, $argument); */ public function getAll(); + /** + * Get jobs matching the search + * + * @param \OCP\BackgroundJob\IJob|class-string|null $job + * @return \OCP\BackgroundJob\IJob[] + * @since 25.0.0 + */ + public function getJobs($job, ?int $limit, int $offset): array; + /** * get the next job in the list * @@ -99,8 +108,6 @@ public function getNext(bool $onlyTimeSensitive = false): ?IJob; public function getById($id); /** - * @param int $id - * @return array|null * @since 23.0.0 */ public function getDetailsById(int $id): ?array; @@ -108,7 +115,6 @@ public function getDetailsById(int $id): ?array; /** * set the job that was last ran to the current time * - * @param \OCP\BackgroundJob\IJob $job * @since 7.0.0 */ public function setLastJob(IJob $job); @@ -116,7 +122,6 @@ public function setLastJob(IJob $job); /** * Remove the reservation for a job * - * @param IJob $job * @since 9.1.0 */ public function unlockJob(IJob $job); @@ -124,7 +129,6 @@ public function unlockJob(IJob $job); /** * set the lastRun of $job to now * - * @param IJob $job * @since 7.0.0 */ public function setLastRun(IJob $job); @@ -132,8 +136,7 @@ public function setLastRun(IJob $job); /** * set the run duration of $job * - * @param IJob $job - * @param $timeTaken + * @param int $timeTaken * @since 12.0.0 */ public function setExecutionTime(IJob $job, $timeTaken); @@ -141,7 +144,6 @@ public function setExecutionTime(IJob $job, $timeTaken); /** * Reset the $job so it executes on the next trigger * - * @param IJob $job * @since 23.0.0 */ public function resetBackgroundJob(IJob $job): void; From 868d748dbfa51461b5ee7cc827be6f314ae0959d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 27 Jun 2022 11:53:10 +0200 Subject: [PATCH 03/10] Code cleaning of Background/ListCommand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- core/Command/Background/ListCommand.php | 19 +++++-------------- lib/private/BackgroundJob/JobList.php | 9 ++++++--- lib/public/BackgroundJob/IJobList.php | 18 +++++++++--------- 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/core/Command/Background/ListCommand.php b/core/Command/Background/ListCommand.php index a0bf611606f2f..0bbf01d76db06 100644 --- a/core/Command/Background/ListCommand.php +++ b/core/Command/Background/ListCommand.php @@ -2,9 +2,9 @@ declare(strict_types=1); /** - * @copyright Copyright (c) 2021, Joas Schilling + * @copyright Copyright (c) 2022, Côme Chilliet * - * @author Joas Schilling + * @author Côme Chilliet * * @license GNU AGPL version 3 or any later version * @@ -26,26 +26,17 @@ namespace OC\Core\Command\Background; use OC\Core\Command\Base; -use OCP\BackgroundJob\IJob; use OCP\BackgroundJob\IJobList; -use OCP\ILogger; -use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; class ListCommand extends Base { - /** @var IJobList */ - protected $jobList; - /** @var ILogger */ - protected $logger; + protected IJobList $jobList; - public function __construct(IJobList $jobList, - ILogger $logger) { + public function __construct(IJobList $jobList) { parent::__construct(); $this->jobList = $jobList; - $this->logger = $logger; } protected function configure(): void { @@ -83,7 +74,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int protected function formatJobs(array $jobs): array { return array_map( - fn($job) => [ + fn ($job) => [ 'id' => $job->getId(), 'class' => get_class($job), 'last_run' => $job->getLastRun(), diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index 8ba993646a951..82e8e86c5c693 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -40,7 +40,6 @@ use OCP\IDBConnection; class JobList implements IJobList { - protected IDBConnection $connection; protected IConfig $config; protected ITimeFactory $timeFactory; @@ -163,12 +162,16 @@ public function has($job, $argument): bool { * * @return IJob[] * @deprecated 9.0.0 - This method is dangerous since it can cause load and - * memory problems when creating too many instances. + * memory problems when creating too many instances. Use getJobs instead. */ public function getAll() { return $this->getJobs(null, null, 0); } + /** + * @param IJob|class-string|null $job + * @return IJob[] + */ public function getJobs($job, ?int $limit, int $offset): array { $query = $this->connection->getQueryBuilder(); $query->select('*') @@ -297,7 +300,7 @@ private function buildJob(array $row): ?IJob { try { // Try to load the job as a service /** @var IJob $job */ - $job = \OC::$server->get($row['class']); + $job = \OCP\Server::get($row['class']); } catch (QueryException $e) { if (class_exists($row['class'])) { $class = $row['class']; diff --git a/lib/public/BackgroundJob/IJobList.php b/lib/public/BackgroundJob/IJobList.php index 7bc7420f35b93..e2fd5b87bcbd3 100644 --- a/lib/public/BackgroundJob/IJobList.php +++ b/lib/public/BackgroundJob/IJobList.php @@ -47,7 +47,7 @@ interface IJobList { /** * Add a job to the list * - * @param \OCP\BackgroundJob\IJob|string $job + * @param IJob|string $job * @param mixed $argument The argument to be passed to $job->run() when the job is exectured * @since 7.0.0 */ @@ -56,7 +56,7 @@ public function add($job, $argument = null); /** * Remove a job from the list * - * @param \OCP\BackgroundJob\IJob|string $job + * @param IJob|string $job * @param mixed $argument * @since 7.0.0 */ @@ -65,7 +65,7 @@ public function remove($job, $argument = null); /** * check if a job is in the list * - * @param \OCP\BackgroundJob\IJob|string $job + * @param IJob|string $job * @param mixed $argument * @return bool * @since 7.0.0 @@ -75,18 +75,18 @@ public function has($job, $argument); /** * get all jobs in the list * - * @return \OCP\BackgroundJob\IJob[] + * @return IJob[] * @since 7.0.0 * @deprecated 9.0.0 - This method is dangerous since it can cause load and - * memory problems when creating too many instances. + * memory problems when creating too many instances. Use getJobs instead. */ public function getAll(); /** * Get jobs matching the search * - * @param \OCP\BackgroundJob\IJob|class-string|null $job - * @return \OCP\BackgroundJob\IJob[] + * @param IJob|class-string|null $job + * @return IJob[] * @since 25.0.0 */ public function getJobs($job, ?int $limit, int $offset): array; @@ -95,14 +95,14 @@ public function getJobs($job, ?int $limit, int $offset): array; * get the next job in the list * * @param bool $onlyTimeSensitive - * @return \OCP\BackgroundJob\IJob|null + * @return IJob|null * @since 7.0.0 - In 24.0.0 parameter $onlyTimeSensitive got added */ public function getNext(bool $onlyTimeSensitive = false): ?IJob; /** * @param int $id - * @return \OCP\BackgroundJob\IJob|null + * @return IJob|null * @since 7.0.0 */ public function getById($id); From 50a7f612d9a3e1bc25b989f8efa3a34780e86726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 27 Jun 2022 12:06:23 +0200 Subject: [PATCH 04/10] Fix DummyJobList class used by tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/lib/BackgroundJob/DummyJobList.php | 44 +++++++++++++++--------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/tests/lib/BackgroundJob/DummyJobList.php b/tests/lib/BackgroundJob/DummyJobList.php index 0751409f62cb1..d8ec0479c7896 100644 --- a/tests/lib/BackgroundJob/DummyJobList.php +++ b/tests/lib/BackgroundJob/DummyJobList.php @@ -19,20 +19,20 @@ class DummyJobList extends \OC\BackgroundJob\JobList { /** * @var IJob[] */ - private $jobs = []; + private array $jobs = []; - private $last = 0; + private int $last = 0; public function __construct() { } /** - * @param IJob|string $job + * @param IJob|class-string $job * @param mixed $argument */ - public function add($job, $argument = null) { + public function add($job, $argument = null): void { if (is_string($job)) { - /** @var \OC\BackgroundJob\Job $job */ + /** @var IJob $job */ $job = new $job; } $job->setArgument($argument); @@ -45,7 +45,7 @@ public function add($job, $argument = null) { * @param IJob|string $job * @param mixed $argument */ - public function remove($job, $argument = null) { + public function remove($job, $argument = null): void { $index = array_search($job, $this->jobs); if ($index !== false) { unset($this->jobs[$index]); @@ -59,7 +59,7 @@ public function remove($job, $argument = null) { * @param mixed $argument * @return bool */ - public function has($job, $argument) { + public function has($job, $argument): bool { return array_search($job, $this->jobs) !== false; } @@ -72,6 +72,22 @@ public function getAll() { return $this->jobs; } + public function getJobs($job, ?int $limit, int $offset): array { + if ($job instanceof IJob) { + $jobClass = get_class($job); + } else { + $jobClass = $job; + } + return array_slice( + array_filter( + $this->jobs, + fn ($job) => ($jobClass === null) || (get_class($job) == $jobClass) + ), + $offset, + $limit + ); + } + /** * get the next job in the list * @@ -96,7 +112,7 @@ public function getNext(bool $onlyTimeSensitive = false): ?IJob { * * @param \OC\BackgroundJob\Job $job */ - public function setLastJob(IJob $job) { + public function setLastJob(IJob $job): void { $i = array_search($job, $this->jobs); if ($i !== false) { $this->last = $i; @@ -107,9 +123,8 @@ public function setLastJob(IJob $job) { /** * @param int $id - * @return IJob */ - public function getById($id) { + public function getById($id): IJob { foreach ($this->jobs as $job) { if ($job->getId() === $id) { return $job; @@ -122,16 +137,11 @@ public function getDetailsById(int $id): ?array { return null; } - /** - * set the lastRun of $job to now - * - * @param IJob $job - */ - public function setLastRun(IJob $job) { + public function setLastRun(IJob $job): void { $job->setLastRun(time()); } - public function setExecutionTime(IJob $job, $timeTaken) { + public function setExecutionTime(IJob $job, $timeTaken): void { } public function resetBackgroundJob(IJob $job): void { From 3cf8c63409a19741200bcab781a9921963fd5e07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 27 Jun 2022 12:08:21 +0200 Subject: [PATCH 05/10] Fix ListCommand constructor call in register_command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- core/register_command.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/register_command.php b/core/register_command.php index 25c368d915c86..d80465e09066e 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -90,7 +90,7 @@ $application->add(new OC\Core\Command\Background\WebCron(\OC::$server->getConfig())); $application->add(new OC\Core\Command\Background\Ajax(\OC::$server->getConfig())); $application->add(new OC\Core\Command\Background\Job(\OC::$server->getJobList(), \OC::$server->getLogger())); - $application->add(new OC\Core\Command\Background\ListCommand(\OC::$server->getJobList(), \OC::$server->getLogger())); + $application->add(new OC\Core\Command\Background\ListCommand(\OC::$server->getJobList())); $application->add(\OC::$server->query(\OC\Core\Command\Broadcast\Test::class)); From 0cc39d185c27505bd6c1f8809bbbea3f3f6b19a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 27 Jun 2022 15:20:16 +0200 Subject: [PATCH 06/10] Use symfony console table to render the job list properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- core/Command/Background/ListCommand.php | 2 +- core/Command/Base.php | 25 +++++++++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/core/Command/Background/ListCommand.php b/core/Command/Background/ListCommand.php index 0bbf01d76db06..d29ac38a7bf0d 100644 --- a/core/Command/Background/ListCommand.php +++ b/core/Command/Background/ListCommand.php @@ -68,7 +68,7 @@ protected function configure(): void { protected function execute(InputInterface $input, OutputInterface $output): int { $jobs = $this->jobList->getJobs($input->getOption('class'), (int)$input->getOption('limit'), (int)$input->getOption('offset')); - $this->writeArrayInOutputFormat($input, $output, $this->formatJobs($jobs)); + $this->writeTableInOutputFormat($input, $output, $this->formatJobs($jobs)); return 0; } diff --git a/core/Command/Base.php b/core/Command/Base.php index d228e36244723..abf9f95773a59 100644 --- a/core/Command/Base.php +++ b/core/Command/Base.php @@ -26,9 +26,10 @@ namespace OC\Core\Command; use OC\Core\Command\User\ListCommand; -use Stecman\Component\Symfony\Console\BashCompletion\Completion\CompletionAwareInterface; use Stecman\Component\Symfony\Console\BashCompletion\CompletionContext; +use Stecman\Component\Symfony\Console\BashCompletion\Completion\CompletionAwareInterface; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Helper\Table; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; @@ -54,7 +55,7 @@ protected function configure() { ; } - protected function writeArrayInOutputFormat(InputInterface $input, OutputInterface $output, array $items, string $prefix = ' - ') { + protected function writeArrayInOutputFormat(InputInterface $input, OutputInterface $output, array $items, string $prefix = ' - '): void { switch ($input->getOption('output')) { case self::OUTPUT_FORMAT_JSON: $output->writeln(json_encode($items)); @@ -84,6 +85,26 @@ protected function writeArrayInOutputFormat(InputInterface $input, OutputInterfa } } + protected function writeTableInOutputFormat(InputInterface $input, OutputInterface $output, array $items): void { + switch ($input->getOption('output')) { + case self::OUTPUT_FORMAT_JSON: + $output->writeln(json_encode($items)); + break; + case self::OUTPUT_FORMAT_JSON_PRETTY: + $output->writeln(json_encode($items, JSON_PRETTY_PRINT)); + break; + default: + $table = new Table($output); + $table->setRows($items); + if (!empty($items) && is_string(array_key_first(reset($items)))) { + $table->setHeaders(array_keys(reset($items))); + } + $table->render(); + break; + } + } + + /** * @param mixed $item */ From 3920fc1ef267c4189f29481c58c9d7866e53f7a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 27 Jun 2022 15:28:01 +0200 Subject: [PATCH 07/10] Format the datetime for last run in ATOM format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- core/Command/Background/ListCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/Command/Background/ListCommand.php b/core/Command/Background/ListCommand.php index d29ac38a7bf0d..bdd45f3a25f04 100644 --- a/core/Command/Background/ListCommand.php +++ b/core/Command/Background/ListCommand.php @@ -77,7 +77,7 @@ protected function formatJobs(array $jobs): array { fn ($job) => [ 'id' => $job->getId(), 'class' => get_class($job), - 'last_run' => $job->getLastRun(), + 'last_run' => date(DATE_ATOM, $job->getLastRun()), 'argument' => json_encode($job->getArgument()), ], $jobs From cc89da26c6b18db8680b75b58b965052c9c9a60f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 27 Jun 2022 16:01:55 +0200 Subject: [PATCH 08/10] Fix background-job:execute command for QueuedJob instances MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- core/Command/Background/Job.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/Command/Background/Job.php b/core/Command/Background/Job.php index 742bdd4329e07..823498cf8ca6c 100644 --- a/core/Command/Background/Job.php +++ b/core/Command/Background/Job.php @@ -85,10 +85,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $job = $this->jobList->getById($jobId); + if ($job === null) { + $output->writeln('Something went wrong when trying to retrieve Job with ID ' . $jobId . ' from database'); + return 1; + } $job->execute($this->jobList, $this->logger); $job = $this->jobList->getById($jobId); - if ($lastRun !== $job->getLastRun()) { + if (($job === null) || ($lastRun !== $job->getLastRun())) { $output->writeln('Job executed!'); $output->writeln(''); From 2e921f8b3df2f53bd780b95303b74ff2c7cc444f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 11 Jul 2022 11:41:47 +0200 Subject: [PATCH 09/10] Fix typing in IJobList MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/BackgroundJob/JobList.php | 12 ++++------ lib/public/BackgroundJob/IJobList.php | 34 +++++++++++++-------------- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index 82e8e86c5c693..20176e451251d 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -51,7 +51,7 @@ public function __construct(IDBConnection $connection, IConfig $config, ITimeFac } /** - * @param IJob|string $job + * @param IJob|class-string $job * @param mixed $argument */ public function add($job, $argument = null): void { @@ -132,7 +132,7 @@ protected function removeById(int $id): void { /** * check if a job is in the list * - * @param IJob|string $job + * @param IJob|class-string $job * @param mixed $argument */ public function has($job, $argument): bool { @@ -164,7 +164,7 @@ public function has($job, $argument): bool { * @deprecated 9.0.0 - This method is dangerous since it can cause load and * memory problems when creating too many instances. Use getJobs instead. */ - public function getAll() { + public function getAll(): array { return $this->getJobs(null, null, 0); } @@ -261,10 +261,7 @@ public function getNext(bool $onlyTimeSensitive = false): ?IJob { } } - /** - * @param int $id - */ - public function getById($id): ?IJob { + public function getById(int $id): ?IJob { $row = $this->getDetailsById($id); if ($row) { @@ -375,7 +372,6 @@ public function setExecutionTime(IJob $job, $timeTaken): void { /** * Reset the $job so it executes on the next trigger * - * @param IJob $job * @since 23.0.0 */ public function resetBackgroundJob(IJob $job): void { diff --git a/lib/public/BackgroundJob/IJobList.php b/lib/public/BackgroundJob/IJobList.php index e2fd5b87bcbd3..8f6449b070b4d 100644 --- a/lib/public/BackgroundJob/IJobList.php +++ b/lib/public/BackgroundJob/IJobList.php @@ -7,6 +7,7 @@ * @author Noveen Sachdeva * @author Robin Appelman * @author Robin McCorkell + * @author Côme Chilliet * * @license AGPL-3.0 * @@ -41,36 +42,38 @@ * be specified in the constructor of the job by calling * $this->setInterval($interval) with $interval in seconds. * + * This interface should be used directly and not implemented by an application. + * The implementation is provided by the server. + * * @since 7.0.0 */ interface IJobList { /** * Add a job to the list * - * @param IJob|string $job + * @param IJob|class-string $job * @param mixed $argument The argument to be passed to $job->run() when the job is exectured * @since 7.0.0 */ - public function add($job, $argument = null); + public function add($job, $argument = null): void; /** * Remove a job from the list * - * @param IJob|string $job + * @param IJob|class-string $job * @param mixed $argument * @since 7.0.0 */ - public function remove($job, $argument = null); + public function remove($job, $argument = null): void; /** * check if a job is in the list * - * @param IJob|string $job + * @param IJob|class-string $job * @param mixed $argument - * @return bool * @since 7.0.0 */ - public function has($job, $argument); + public function has($job, $argument): bool; /** * get all jobs in the list @@ -80,7 +83,7 @@ public function has($job, $argument); * @deprecated 9.0.0 - This method is dangerous since it can cause load and * memory problems when creating too many instances. Use getJobs instead. */ - public function getAll(); + public function getAll(): array; /** * Get jobs matching the search @@ -94,18 +97,14 @@ public function getJobs($job, ?int $limit, int $offset): array; /** * get the next job in the list * - * @param bool $onlyTimeSensitive - * @return IJob|null * @since 7.0.0 - In 24.0.0 parameter $onlyTimeSensitive got added */ public function getNext(bool $onlyTimeSensitive = false): ?IJob; /** - * @param int $id - * @return IJob|null * @since 7.0.0 */ - public function getById($id); + public function getById(int $id): ?IJob; /** * @since 23.0.0 @@ -117,29 +116,28 @@ public function getDetailsById(int $id): ?array; * * @since 7.0.0 */ - public function setLastJob(IJob $job); + public function setLastJob(IJob $job): void; /** * Remove the reservation for a job * * @since 9.1.0 */ - public function unlockJob(IJob $job); + public function unlockJob(IJob $job): void; /** * set the lastRun of $job to now * * @since 7.0.0 */ - public function setLastRun(IJob $job); + public function setLastRun(IJob $job): void; /** * set the run duration of $job * - * @param int $timeTaken * @since 12.0.0 */ - public function setExecutionTime(IJob $job, $timeTaken); + public function setExecutionTime(IJob $job, int $timeTaken): void; /** * Reset the $job so it executes on the next trigger From 0386f4270befeff97bd128e17bfae676a74aacf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 12 Jul 2022 14:27:02 +0200 Subject: [PATCH 10/10] Fix DummyJobList typing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- tests/lib/BackgroundJob/DummyJobList.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/lib/BackgroundJob/DummyJobList.php b/tests/lib/BackgroundJob/DummyJobList.php index d8ec0479c7896..be9c06257b79a 100644 --- a/tests/lib/BackgroundJob/DummyJobList.php +++ b/tests/lib/BackgroundJob/DummyJobList.php @@ -68,7 +68,7 @@ public function has($job, $argument): bool { * * @return IJob[] */ - public function getAll() { + public function getAll(): array { return $this->jobs; } @@ -90,9 +90,6 @@ public function getJobs($job, ?int $limit, int $offset): array { /** * get the next job in the list - * - * @param bool $onlyTimeSensitive - * @return IJob|null */ public function getNext(bool $onlyTimeSensitive = false): ?IJob { if (count($this->jobs) > 0) { @@ -121,10 +118,7 @@ public function setLastJob(IJob $job): void { } } - /** - * @param int $id - */ - public function getById($id): IJob { + public function getById(int $id): IJob { foreach ($this->jobs as $job) { if ($job->getId() === $id) { return $job;