From ba954958572321c985db4b9c5c097f7c259eb568 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 9 Oct 2018 12:36:43 +0200 Subject: [PATCH 1/4] Add backgroundjobs to OCP This adds abstract classes to base background jobs on. Right now almost all uses of this use the private namespace. For most usages it will be enough to just extend the the abstract classes QueuedJob or TimedJob. It should be a straight forward drop in replacement. The private jobs can then be killed off after a few releases. So we have a nice public API. Signed-off-by: Roeland Jago Douma --- lib/public/BackgroundJob/Job.php | 141 +++++++++++++++++++++++++ lib/public/BackgroundJob/QueuedJob.php | 48 +++++++++ lib/public/BackgroundJob/TimedJob.php | 62 +++++++++++ 3 files changed, 251 insertions(+) create mode 100644 lib/public/BackgroundJob/Job.php create mode 100644 lib/public/BackgroundJob/QueuedJob.php create mode 100644 lib/public/BackgroundJob/TimedJob.php diff --git a/lib/public/BackgroundJob/Job.php b/lib/public/BackgroundJob/Job.php new file mode 100644 index 00000000000..25f57ce6094 --- /dev/null +++ b/lib/public/BackgroundJob/Job.php @@ -0,0 +1,141 @@ + + * + * @author Roeland Jago Douma + * + * @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 OCP\BackgroundJob; + +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\ILogger; + +/** + * Base class for background jobs + * + * This is here if you want to do advanced stuff in your background jobs. + * For the most common use cases have a look at QueuedJob and TimedJob + * + * @since 15.0.0 + */ +abstract class Job implements IJob { + + /** @var int $id */ + protected $id; + + /** @var int $lastRun */ + protected $lastRun; + + /** @var mixed $argument */ + protected $argument; + + /** @var ITimeFactory */ + protected $time; + + /** + * @since 15.0.0 + */ + public function __construct(ITimeFactory $time) { + $this->time = $time; + } + + /** + * The function to prepare the execution of the job. + + * + * @param IJobList $jobList + * @param ILogger|null $logger + * + * @since 15.0.0 + */ + public function execute($jobList, ILogger $logger = null) { + $jobList->setLastRun($this); + if ($logger === null) { + $logger = \OC::$server->getLogger(); + } + + try { + $jobStartTime = $this->time->getTime(); + $logger->debug('Run ' . get_class($this) . ' job with ID ' . $this->getId(), ['app' => 'cron']); + $this->run($this->argument); + $timeTaken = $this->time->getTime() - $jobStartTime; + + $logger->debug('Finished ' . get_class($this) . ' job with ID ' . $this->getId() . ' in ' . $timeTaken . ' seconds', ['app' => 'cron']); + $jobList->setExecutionTime($this, $timeTaken); + } catch (\Exception $e) { + if ($logger) { + $logger->logException($e, [ + 'app' => 'core', + 'message' => 'Error while running background job (class: ' . get_class($this) . ', arguments: ' . print_r($this->argument, true) . ')' + ]); + } + } + } + + /** + * @since 15.0.0 + */ + final public function setId($id) { + $this->id = $id; + } + + /** + * @since 15.0.0 + */ + final public function setLastRun($lastRun) { + $this->lastRun = $lastRun; + } + + /** + * @since 15.0.0 + */ + public function setArgument($argument) { + $this->argument = $argument; + } + + /** + * @since 15.0.0 + */ + final public function getId(): int { + return $this->id; + } + + /** + * @since 15.0.0 + */ + final public function getLastRun(): int { + return $this->lastRun; + } + + /** + * @since 15.0.0 + */ + public function getArgument() { + return $this->argument; + } + + /** + * The actual function that is called to run the job + * + * @param $argument + * @return mixed + */ + abstract protected function run($argument); +} diff --git a/lib/public/BackgroundJob/QueuedJob.php b/lib/public/BackgroundJob/QueuedJob.php new file mode 100644 index 00000000000..4964ed9ad33 --- /dev/null +++ b/lib/public/BackgroundJob/QueuedJob.php @@ -0,0 +1,48 @@ + + * + * @author Roeland Jago Douma + * + * @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 OCP\BackgroundJob; + +use OCP\ILogger; + +/** + * Simple base class for a one time background job + * + * @since 15.0.0 + */ +abstract class QueuedJob extends Job { + + /** + * run the job, then remove it from the joblist + * + * @param IJobList $jobList + * @param ILogger|null $logger + * + * @since 15.0.0 + */ + final public function execute($jobList, ILogger $logger = null) { + $jobList->remove($this, $this->argument); + parent::execute($jobList, $logger); + } +} diff --git a/lib/public/BackgroundJob/TimedJob.php b/lib/public/BackgroundJob/TimedJob.php new file mode 100644 index 00000000000..d58ea05d31c --- /dev/null +++ b/lib/public/BackgroundJob/TimedJob.php @@ -0,0 +1,62 @@ + + * + * @author Roeland Jago Douma + * + * @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 OCP\BackgroundJob; + +use OC\BackgroundJob\JobList; +use OCP\ILogger; + +/** + * Simple base class to extend to run periodic background jobs. + * Call setInterval with your desired interval in seconds from the constructor. + * + * @since 15.0.0 + */ +abstract class TimedJob extends Job { + /** @var int */ + protected $interval = 0; + + /** + * set the interval for the job + * + * @since 15.0.0 + */ + public function setInterval(int $interval) { + $this->interval = $interval; + } + + /** + * run the job if the last run is is more than the interval ago + * + * @param JobList $jobList + * @param ILogger|null $logger + * + * @since 15.0.0 + */ + final public function execute($jobList, ILogger $logger = null) { + if (($this->time->getTime() - $this->lastRun) > $this->interval) { + parent::execute($jobList, $logger); + } + } +} From 2dbbc37852a10c59153c6af66db662afc9171e6a Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 9 Oct 2018 12:51:08 +0200 Subject: [PATCH 2/4] Bump autolaoders Signed-off-by: Roeland Jago Douma --- lib/composer/composer/autoload_classmap.php | 3 +++ lib/composer/composer/autoload_static.php | 3 +++ 2 files changed, 6 insertions(+) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 0223cafd1d4..8028ea34fcd 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -84,6 +84,9 @@ return array( 'OCP\\BackgroundJob' => $baseDir . '/lib/public/BackgroundJob.php', 'OCP\\BackgroundJob\\IJob' => $baseDir . '/lib/public/BackgroundJob/IJob.php', 'OCP\\BackgroundJob\\IJobList' => $baseDir . '/lib/public/BackgroundJob/IJobList.php', + 'OCP\\BackgroundJob\\Job' => $baseDir . '/lib/public/BackgroundJob/Job.php', + 'OCP\\BackgroundJob\\QueuedJob' => $baseDir . '/lib/public/BackgroundJob/QueuedJob.php', + 'OCP\\BackgroundJob\\TimedJob' => $baseDir . '/lib/public/BackgroundJob/TimedJob.php', 'OCP\\Calendar\\BackendTemporarilyUnavailableException' => $baseDir . '/lib/public/Calendar/BackendTemporarilyUnavailableException.php', 'OCP\\Calendar\\ICalendar' => $baseDir . '/lib/public/Calendar/ICalendar.php', 'OCP\\Calendar\\IManager' => $baseDir . '/lib/public/Calendar/IManager.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 4b853936887..b57bb6c914d 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -114,6 +114,9 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OCP\\BackgroundJob' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob.php', 'OCP\\BackgroundJob\\IJob' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob/IJob.php', 'OCP\\BackgroundJob\\IJobList' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob/IJobList.php', + 'OCP\\BackgroundJob\\Job' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob/Job.php', + 'OCP\\BackgroundJob\\QueuedJob' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob/QueuedJob.php', + 'OCP\\BackgroundJob\\TimedJob' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob/TimedJob.php', 'OCP\\Calendar\\BackendTemporarilyUnavailableException' => __DIR__ . '/../../..' . '/lib/public/Calendar/BackendTemporarilyUnavailableException.php', 'OCP\\Calendar\\ICalendar' => __DIR__ . '/../../..' . '/lib/public/Calendar/ICalendar.php', 'OCP\\Calendar\\IManager' => __DIR__ . '/../../..' . '/lib/public/Calendar/IManager.php', From 7370fb37bb3dfe684a8abeb3867ade082a29ff1a Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 9 Oct 2018 15:10:38 +0200 Subject: [PATCH 3/4] fixup! Add backgroundjobs to OCP Signed-off-by: Roeland Jago Douma --- lib/public/BackgroundJob/Job.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/public/BackgroundJob/Job.php b/lib/public/BackgroundJob/Job.php index 25f57ce6094..0f872355a94 100644 --- a/lib/public/BackgroundJob/Job.php +++ b/lib/public/BackgroundJob/Job.php @@ -136,6 +136,8 @@ abstract class Job implements IJob { * * @param $argument * @return mixed + * + * @since 15.0.0 */ abstract protected function run($argument); } From 8493f49211290e3aea48cc9055a615b69725ac84 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 9 Oct 2018 15:25:56 +0200 Subject: [PATCH 4/4] fixup! fixup! Add backgroundjobs to OCP Signed-off-by: Roeland Jago Douma --- tests/lib/BackgroundJob/QueuedJobTest.php | 56 ++++++----- tests/lib/BackgroundJob/TimedJobTest.php | 115 +++++++++++++++------- 2 files changed, 111 insertions(+), 60 deletions(-) diff --git a/tests/lib/BackgroundJob/QueuedJobTest.php b/tests/lib/BackgroundJob/QueuedJobTest.php index a3e9cc296a0..73b245f93a5 100644 --- a/tests/lib/BackgroundJob/QueuedJobTest.php +++ b/tests/lib/BackgroundJob/QueuedJobTest.php @@ -8,18 +8,24 @@ namespace Test\BackgroundJob; -class TestQueuedJob extends \OC\BackgroundJob\QueuedJob { - private $testCase; +use OCP\AppFramework\Utility\ITimeFactory; + +class TestQueuedJob extends \OC\BackgroundJob\QueuedJob { + public $ran = false; - /** - * @param QueuedJobTest $testCase - */ - public function __construct($testCase) { - $this->testCase = $testCase; - } public function run($argument) { - $this->testCase->markRun(); + $this->ran = true; + } +} + + +class TestQueuedJobNew extends \OCP\BackgroundJob\QueuedJob { + public $ran = false; + + + public function run($argument) { + $this->ran = true; } } @@ -28,29 +34,29 @@ class QueuedJobTest extends \Test\TestCase { * @var DummyJobList $jobList */ private $jobList; - /** - * @var \OC\BackgroundJob\TimedJob $job - */ - private $job; - - private $jobRun = false; - - public function markRun() { - $this->jobRun = true; - } protected function setup() { parent::setUp(); $this->jobList = new DummyJobList(); - $this->job = new TestQueuedJob($this); - $this->jobList->add($this->job); - $this->jobRun = false; } public function testJobShouldBeRemoved() { - $this->assertTrue($this->jobList->has($this->job, null)); - $this->job->execute($this->jobList); - $this->assertTrue($this->jobRun); + $job = new TestQueuedJob(); + $this->jobList->add($job); + + $this->assertTrue($this->jobList->has($job, null)); + $job->execute($this->jobList); + $this->assertTrue($job->ran); + } + + public function testJobShouldBeRemovedNew() { + $job = new TestQueuedJobNew(\OC::$server->query(ITimeFactory::class)); + $job->setId(42); + $this->jobList->add($job); + + $this->assertTrue($this->jobList->has($job, null)); + $job->execute($this->jobList); + $this->assertTrue($job->ran); } } diff --git a/tests/lib/BackgroundJob/TimedJobTest.php b/tests/lib/BackgroundJob/TimedJobTest.php index 5652a5c9901..e684ad8c1e6 100644 --- a/tests/lib/BackgroundJob/TimedJobTest.php +++ b/tests/lib/BackgroundJob/TimedJobTest.php @@ -8,65 +8,110 @@ namespace Test\BackgroundJob; -class TestTimedJob extends \OC\BackgroundJob\TimedJob { - private $testCase; +use OCP\AppFramework\Utility\ITimeFactory; - /** - * @param TimedJobTest $testCase - */ - public function __construct($testCase) { +class TestTimedJob extends \OC\BackgroundJob\TimedJob { + /** @var bool */ + public $ran = false; + + public function __construct() { $this->setInterval(10); - $this->testCase = $testCase; } public function run($argument) { - $this->testCase->markRun(); + $this->ran = true; + } +} + +class TestTimedJobNew extends \OCP\BackgroundJob\TimedJob { + /** @var bool */ + public $ran = false; + + public function __construct(ITimeFactory $timeFactory) { + parent::__construct($timeFactory); + $this->setInterval(10); + } + + public function run($argument) { + $this->ran = true; } } class TimedJobTest extends \Test\TestCase { - /** - * @var DummyJobList $jobList - */ + /** @var DummyJobList $jobList */ private $jobList; - /** - * @var \OC\BackgroundJob\TimedJob $job - */ - private $job; - private $jobRun = false; - - public function markRun() { - $this->jobRun = true; - } + /** @var ITimeFactory */ + private $time; protected function setup() { parent::setUp(); $this->jobList = new DummyJobList(); - $this->job = new TestTimedJob($this); - $this->jobList->add($this->job); - $this->jobRun = false; + $this->time = \OC::$server->query(ITimeFactory::class); } public function testShouldRunAfterInterval() { - $this->job->setLastRun(time() - 12); - $this->job->execute($this->jobList); - $this->assertTrue($this->jobRun); + $job = new TestTimedJob(); + $this->jobList->add($job); + + $job->setLastRun(time() - 12); + $job->execute($this->jobList); + $this->assertTrue($job->ran); } public function testShouldNotRunWithinInterval() { - $this->job->setLastRun(time() - 5); - $this->job->execute($this->jobList); - $this->assertFalse($this->jobRun); + $job = new TestTimedJob(); + $this->jobList->add($job); + + $job->setLastRun(time() - 5); + $job->execute($this->jobList); + $this->assertFalse($job->ran); } public function testShouldNotTwice() { - $this->job->setLastRun(time() - 15); - $this->job->execute($this->jobList); - $this->assertTrue($this->jobRun); - $this->jobRun = false; - $this->job->execute($this->jobList); - $this->assertFalse($this->jobRun); + $job = new TestTimedJob(); + $this->jobList->add($job); + + $job->setLastRun(time() - 15); + $job->execute($this->jobList); + $this->assertTrue($job->ran); + $job->ran = false; + $job->execute($this->jobList); + $this->assertFalse($job->ran); + } + + + public function testShouldRunAfterIntervalNew() { + $job = new TestTimedJobNew($this->time); + $job->setId(42); + $this->jobList->add($job); + + $job->setLastRun(time() - 12); + $job->execute($this->jobList); + $this->assertTrue($job->ran); + } + + public function testShouldNotRunWithinIntervalNew() { + $job = new TestTimedJobNew($this->time); + $job->setId(42); + $this->jobList->add($job); + + $job->setLastRun(time() - 5); + $job->execute($this->jobList); + $this->assertFalse($job->ran); + } + + public function testShouldNotTwiceNew() { + $job = new TestTimedJobNew($this->time); + $job->setId(42); + $this->jobList->add($job); + + $job->setLastRun(time() - 15); + $job->execute($this->jobList); + $this->assertTrue($job->ran); + $job->ran = false; + $job->execute($this->jobList); + $this->assertFalse($job->ran); } }