diff options
-rw-r--r-- | core/Command/Background/Job.php | 6 | ||||
-rw-r--r-- | core/Command/Background/ListCommand.php | 86 | ||||
-rw-r--r-- | core/Command/Base.php | 25 | ||||
-rw-r--r-- | core/register_command.php | 1 | ||||
-rw-r--r-- | lib/composer/composer/autoload_classmap.php | 1 | ||||
-rw-r--r-- | lib/composer/composer/autoload_static.php | 1 | ||||
-rw-r--r-- | lib/private/BackgroundJob/JobList.php | 113 | ||||
-rw-r--r-- | lib/public/BackgroundJob/IJobList.php | 54 | ||||
-rw-r--r-- | tests/lib/BackgroundJob/DummyJobList.php | 52 |
9 files changed, 223 insertions, 116 deletions
diff --git a/core/Command/Background/Job.php b/core/Command/Background/Job.php index 742bdd4329e..823498cf8ca 100644 --- a/core/Command/Background/Job.php +++ b/core/Command/Background/Job.php @@ -85,10 +85,14 @@ class Job extends Command { } $job = $this->jobList->getById($jobId); + if ($job === null) { + $output->writeln('<error>Something went wrong when trying to retrieve Job with ID ' . $jobId . ' from database</error>'); + 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('<info>Job executed!</info>'); $output->writeln(''); diff --git a/core/Command/Background/ListCommand.php b/core/Command/Background/ListCommand.php new file mode 100644 index 00000000000..bdd45f3a25f --- /dev/null +++ b/core/Command/Background/ListCommand.php @@ -0,0 +1,86 @@ +<?php + +declare(strict_types=1); +/** + * @copyright Copyright (c) 2022, Côme Chilliet <come.chilliet@nextcloud.com> + * + * @author Côme Chilliet <come.chilliet@nextcloud.com> + * + * @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 <http://www.gnu.org/licenses/>. + * + */ + +namespace OC\Core\Command\Background; + +use OC\Core\Command\Base; +use OCP\BackgroundJob\IJobList; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; + +class ListCommand extends Base { + protected IJobList $jobList; + + public function __construct(IJobList $jobList) { + parent::__construct(); + $this->jobList = $jobList; + } + + 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->writeTableInOutputFormat($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' => date(DATE_ATOM, $job->getLastRun()), + 'argument' => json_encode($job->getArgument()), + ], + $jobs + ); + } +} diff --git a/core/Command/Base.php b/core/Command/Base.php index d228e362447..abf9f95773a 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 @@ class Base extends Command implements CompletionAwareInterface { ; } - 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 @@ class Base extends Command implements CompletionAwareInterface { } } + 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 */ diff --git a/core/register_command.php b/core/register_command.php index de04fedf30d..d80465e0906 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -90,6 +90,7 @@ if (\OC::$server->getConfig()->getSystemValue('installed', false)) { $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())); $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 251c547116f..e3572aa833c 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -860,6 +860,7 @@ return array( '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 d6566462a8e..658f2cdfe2d 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -893,6 +893,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 7ab86df8455..20176e45125 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -40,21 +40,10 @@ use OCP\IConfig; use OCP\IDBConnection; class JobList implements IJobList { + protected IDBConnection $connection; + protected IConfig $config; + protected ITimeFactory $timeFactory; - /** @var IDBConnection */ - protected $connection; - - /**@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; @@ -62,10 +51,10 @@ class JobList implements IJobList { } /** - * @param IJob|string $job + * @param IJob|class-string<IJob> $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 +90,7 @@ class JobList implements IJobList { * @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,24 +122,20 @@ class JobList implements IJobList { } } - /** - * @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(); } /** * check if a job is in the list * - * @param IJob|string $job + * @param IJob|class-string<IJob> $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 +150,7 @@ class JobList implements IJobList { ->andWhere($query->expr()->eq('argument_hash', $query->createNamedParameter(md5($argument)))) ->setMaxResults(1); - $result = $query->execute(); + $result = $query->executeQuery(); $row = $result->fetch(); $result->closeCursor(); @@ -177,13 +162,33 @@ class JobList implements IJobList { * * @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(): array { + return $this->getJobs(null, null, 0); + } + + /** + * @param IJob|class-string<IJob>|null $job + * @return IJob[] */ - public function getAll() { + public function getJobs($job, ?int $limit, int $offset): array { $query = $this->connection->getQueryBuilder(); $query->select('*') - ->from('jobs'); - $result = $query->execute(); + ->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 = []; while ($row = $result->fetch()) { @@ -199,9 +204,6 @@ class JobList implements IJobList { /** * 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 +226,7 @@ class JobList implements IJobList { ->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 +234,7 @@ class JobList implements IJobList { $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 +249,7 @@ class JobList implements IJobList { ->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); @@ -259,11 +261,7 @@ class JobList implements IJobList { } } - /** - * @param int $id - * @return IJob|null - */ - public function getById($id) { + public function getById(int $id): ?IJob { $row = $this->getDetailsById($id); if ($row) { @@ -292,15 +290,14 @@ class JobList implements IJobList { /** * get the job object from a row in the db * - * @param array $row - * @return IJob|null + * @param array{class:class-string<IJob>, 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 = \OCP\Server::get($row['class']); } catch (QueryException $e) { if (class_exists($row['class'])) { $class = $row['class']; @@ -327,33 +324,27 @@ class JobList implements IJobList { /** * 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,25 +355,23 @@ class JobList implements IJobList { $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(); } /** * 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 eab37a03f36..8f6449b070b 100644 --- a/lib/public/BackgroundJob/IJobList.php +++ b/lib/public/BackgroundJob/IJobList.php @@ -7,6 +7,7 @@ * @author Noveen Sachdeva <noveen.sachdeva@research.iiit.ac.in> * @author Robin Appelman <robin@icewind.nl> * @author Robin McCorkell <robin@mccorkell.me.uk> + * @author Côme Chilliet <come.chilliet@nextcloud.com> * * @license AGPL-3.0 * @@ -41,66 +42,71 @@ namespace OCP\BackgroundJob; * 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 \OCP\BackgroundJob\IJob|string $job + * @param IJob|class-string<IJob> $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 \OCP\BackgroundJob\IJob|string $job + * @param IJob|class-string<IJob> $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 \OCP\BackgroundJob\IJob|string $job + * @param IJob|class-string<IJob> $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 * - * @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(): array; + + /** + * Get jobs matching the search + * + * @param IJob|class-string<IJob>|null $job + * @return IJob[] + * @since 25.0.0 */ - public function getAll(); + public function getJobs($job, ?int $limit, int $offset): array; /** * get the next job in the list * - * @param bool $onlyTimeSensitive - * @return \OCP\BackgroundJob\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 * @since 7.0.0 */ - public function getById($id); + public function getById(int $id): ?IJob; /** - * @param int $id - * @return array|null * @since 23.0.0 */ public function getDetailsById(int $id): ?array; @@ -108,40 +114,34 @@ interface IJobList { /** * 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); + public function setLastJob(IJob $job): void; /** * Remove the reservation for a job * - * @param IJob $job * @since 9.1.0 */ - public function unlockJob(IJob $job); + public function unlockJob(IJob $job): void; /** * set the lastRun of $job to now * - * @param IJob $job * @since 7.0.0 */ - public function setLastRun(IJob $job); + public function setLastRun(IJob $job): void; /** * set the run duration of $job * - * @param IJob $job - * @param $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 * - * @param IJob $job * @since 23.0.0 */ public function resetBackgroundJob(IJob $job): void; diff --git a/tests/lib/BackgroundJob/DummyJobList.php b/tests/lib/BackgroundJob/DummyJobList.php index 0751409f62c..be9c06257b7 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<IJob> $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 @@ class DummyJobList extends \OC\BackgroundJob\JobList { * @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 @@ class DummyJobList extends \OC\BackgroundJob\JobList { * @param mixed $argument * @return bool */ - public function has($job, $argument) { + public function has($job, $argument): bool { return array_search($job, $this->jobs) !== false; } @@ -68,15 +68,28 @@ class DummyJobList extends \OC\BackgroundJob\JobList { * * @return IJob[] */ - public function getAll() { + public function getAll(): array { 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 - * - * @param bool $onlyTimeSensitive - * @return IJob|null */ public function getNext(bool $onlyTimeSensitive = false): ?IJob { if (count($this->jobs) > 0) { @@ -96,7 +109,7 @@ class DummyJobList extends \OC\BackgroundJob\JobList { * * @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; @@ -105,11 +118,7 @@ class DummyJobList extends \OC\BackgroundJob\JobList { } } - /** - * @param int $id - * @return IJob - */ - public function getById($id) { + public function getById(int $id): IJob { foreach ($this->jobs as $job) { if ($job->getId() === $id) { return $job; @@ -122,16 +131,11 @@ class DummyJobList extends \OC\BackgroundJob\JobList { 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 { |