Merge pull request #32552 from nextcloud/enh/improve-job-handling-commands

Improve job handling through occ
pull/27929/head
Côme Chilliet 4 years ago committed by GitHub
commit c20d34b2ca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 6
      core/Command/Background/Job.php
  2. 86
      core/Command/Background/ListCommand.php
  3. 25
      core/Command/Base.php
  4. 1
      core/register_command.php
  5. 1
      lib/composer/composer/autoload_classmap.php
  6. 1
      lib/composer/composer/autoload_static.php
  7. 113
      lib/private/BackgroundJob/JobList.php
  8. 54
      lib/public/BackgroundJob/IJobList.php
  9. 52
      tests/lib/BackgroundJob/DummyJobList.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('');

@ -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
);
}
}

@ -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
*/

@ -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));

@ -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',

@ -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',

@ -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 {

@ -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;

@ -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 {

Loading…
Cancel
Save