From c01c516323d91bb23f16186ffe0be09b3c412624 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 20 Apr 2023 12:55:06 +0200 Subject: [PATCH 01/15] feat(BackgroundJobs): Allow preventing parallel runs for a job class Signed-off-by: Marcel Klehr --- lib/private/BackgroundJob/JobList.php | 18 ++++++++++++++++++ lib/public/BackgroundJob/IJobList.php | 9 +++++++++ lib/public/BackgroundJob/Job.php | 25 +++++++++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index 67b736b8dd96c..6761aa282d155 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -35,6 +35,7 @@ use OCP\AutoloadNotAllowedException; use OCP\BackgroundJob\IJob; use OCP\BackgroundJob\IJobList; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IConfig; use OCP\IDBConnection; @@ -382,4 +383,21 @@ public function resetBackgroundJob(IJob $job): void { ->where($query->expr()->eq('id', $query->createNamedParameter($job->getId()), IQueryBuilder::PARAM_INT)); $query->executeStatement(); } + + public function hasReservedJob(?string $className): bool { + $query = $this->connection->getQueryBuilder(); + $query->select('*') + ->from('jobs') + ->where($query->expr()->neq('reserved_at', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT))); + + if ($className !== null) { + $query->andWhere($query->expr()->eq('class', $query->createNamedParameter($className))); + } + + try { + return $query->executeQuery()->rowCount() > 0; + } catch (Exception $e) { + return false; + } + } } diff --git a/lib/public/BackgroundJob/IJobList.php b/lib/public/BackgroundJob/IJobList.php index e8d0380e604e6..71faefb882562 100644 --- a/lib/public/BackgroundJob/IJobList.php +++ b/lib/public/BackgroundJob/IJobList.php @@ -145,4 +145,13 @@ public function setExecutionTime(IJob $job, int $timeTaken): void; * @since 23.0.0 */ public function resetBackgroundJob(IJob $job): void; + + /** + * Checks whether a job of the passed class is reserved to run + * + * @param string|null $className + * @return bool + * @since 27.0.0 + */ + public function hasReservedJob(?string $className): bool; } diff --git a/lib/public/BackgroundJob/Job.php b/lib/public/BackgroundJob/Job.php index d60fb5905c9d1..89ea41381c96f 100644 --- a/lib/public/BackgroundJob/Job.php +++ b/lib/public/BackgroundJob/Job.php @@ -43,6 +43,7 @@ abstract class Job implements IJob { protected int $lastRun = 0; protected $argument; protected ITimeFactory $time; + protected bool $allowParallelRuns = true; /** * @since 15.0.0 @@ -72,6 +73,11 @@ public function start(IJobList $jobList): void { $jobList->setLastRun($this); $logger = \OCP\Server::get(LoggerInterface::class); + if (!$this->getAllowParallelRuns() && $jobList->hasReservedJob(get_class($this))) { + $logger->debug('Skipping ' . get_class($this) . ' job with ID ' . $this->getId() . ' because another job with the same class is already running', ['app' => 'cron']); + return; + } + try { $jobStartTime = $this->time->getTime(); $logger->debug('Run ' . get_class($this) . ' job with ID ' . $this->getId(), ['app' => 'cron']); @@ -132,6 +138,25 @@ public function getArgument() { return $this->argument; } + /** + * Set this to false to prevent two Jobs from this class from running in parallel + * + * @param bool $allow + * @return void + * @since 27.0.0 + */ + public function setAllowParallelRuns(bool $allow) { + $this->allowParallelRuns = $allow; + } + + /** + * @return bool + * @since 27.0.0 + */ + public function getAllowParallelRuns(): bool { + return $this->allowParallelRuns; + } + /** * The actual function that is called to run the job * From ef27bd6e550530ccf37705f89d026ba2fb517f1c Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 20 Apr 2023 12:56:48 +0200 Subject: [PATCH 02/15] fix(SpeechToText): Prevent parallel runs of TranscriptionJob Signed-off-by: Marcel Klehr --- lib/private/SpeechToText/TranscriptionJob.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/private/SpeechToText/TranscriptionJob.php b/lib/private/SpeechToText/TranscriptionJob.php index d5cc9ed7c38c4..8921d52ecd15d 100644 --- a/lib/private/SpeechToText/TranscriptionJob.php +++ b/lib/private/SpeechToText/TranscriptionJob.php @@ -49,6 +49,7 @@ public function __construct( private LoggerInterface $logger, ) { parent::__construct($timeFactory); + $this->setAllowParallelRuns(false); } From 1296f3612e0a4c81edb922b5cd000842807ca899 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 20 Apr 2023 13:18:28 +0200 Subject: [PATCH 03/15] test(BackgroundJobs): Add tests for allowParallelRuns and hasReservedJobs Signed-off-by: Marcel Klehr --- lib/private/BackgroundJob/JobList.php | 2 +- tests/lib/BackgroundJob/DummyJobList.php | 13 ++++ tests/lib/BackgroundJob/JobListTest.php | 13 ++++ tests/lib/BackgroundJob/JobTest.php | 76 +++++++++++++++++++++++- tests/lib/BackgroundJob/TestJob.php | 7 ++- 5 files changed, 106 insertions(+), 5 deletions(-) diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index 6761aa282d155..707a8c799045f 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -384,7 +384,7 @@ public function resetBackgroundJob(IJob $job): void { $query->executeStatement(); } - public function hasReservedJob(?string $className): bool { + public function hasReservedJob(?string $className = null): bool { $query = $this->connection->getQueryBuilder(); $query->select('*') ->from('jobs') diff --git a/tests/lib/BackgroundJob/DummyJobList.php b/tests/lib/BackgroundJob/DummyJobList.php index be48259789aa9..700393828649c 100644 --- a/tests/lib/BackgroundJob/DummyJobList.php +++ b/tests/lib/BackgroundJob/DummyJobList.php @@ -21,6 +21,11 @@ class DummyJobList extends \OC\BackgroundJob\JobList { */ private array $jobs = []; + /** + * @var bool[] + */ + private array $reserved = []; + private int $last = 0; public function __construct() { @@ -135,6 +140,14 @@ public function setLastRun(IJob $job): void { $job->setLastRun(time()); } + public function hasReservedJob(?string $className = null): bool { + return $this->reserved[$className]; + } + + public function setHasReservedJob(?string $className, bool $hasReserved): void { + $this->reserved[$className] = $hasReserved; + } + public function setExecutionTime(IJob $job, $timeTaken): void { } diff --git a/tests/lib/BackgroundJob/JobListTest.php b/tests/lib/BackgroundJob/JobListTest.php index ea02e1cd8b97b..d42c2521761d5 100644 --- a/tests/lib/BackgroundJob/JobListTest.php +++ b/tests/lib/BackgroundJob/JobListTest.php @@ -244,4 +244,17 @@ public function testSetLastRun() { $this->assertGreaterThanOrEqual($timeStart, $addedJob->getLastRun()); $this->assertLessThanOrEqual($timeEnd, $addedJob->getLastRun()); } + + public function testHasReservedJobs() { + $job = new TestJob(); + $this->instance->add($job); + + $this->assertFalse($this->instance->hasReservedJob()); + $this->assertFalse($this->instance->hasReservedJob(TestJob::class)); + + $job->start($this->instance); + + $this->assertTrue($this->instance->hasReservedJob()); + $this->assertTrue($this->instance->hasReservedJob(TestJob::class)); + } } diff --git a/tests/lib/BackgroundJob/JobTest.php b/tests/lib/BackgroundJob/JobTest.php index b4048aa1c2257..26e6b4ed85b7c 100644 --- a/tests/lib/BackgroundJob/JobTest.php +++ b/tests/lib/BackgroundJob/JobTest.php @@ -8,20 +8,23 @@ namespace Test\BackgroundJob; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\ILogger; class JobTest extends \Test\TestCase { private $run = false; + private ITimeFactory $timeFactory; protected function setUp(): void { parent::setUp(); $this->run = false; + $this->timeFactory = \OC::$server->get(ITimeFactory::class); } public function testRemoveAfterException() { $jobList = new DummyJobList(); $e = new \Exception(); - $job = new TestJob($this, function () use ($e) { + $job = new TestJob($this->timeFactory, $this, function () use ($e) { throw $e; }); $jobList->add($job); @@ -41,7 +44,7 @@ public function testRemoveAfterException() { public function testRemoveAfterError() { $jobList = new DummyJobList(); - $job = new TestJob($this, function () { + $job = new TestJob($this->timeFactory, $this, function () { $test = null; $test->someMethod(); }); @@ -60,6 +63,75 @@ public function testRemoveAfterError() { $this->assertCount(1, $jobList->getAll()); } + public function testRemoveAfterError() { + $jobList = new DummyJobList(); + $job = new TestJob($this->timeFactory, $this, function () { + $test = null; + $test->someMethod(); + }); + $jobList->add($job); + + $logger = $this->getMockBuilder(ILogger::class) + ->disableOriginalConstructor() + ->getMock(); + $logger->expects($this->once()) + ->method('logException') + ->with($this->isInstanceOf(\Throwable::class)); + + $this->assertCount(1, $jobList->getAll()); + $job->execute($jobList, $logger); + $this->assertTrue($this->run); + $this->assertCount(1, $jobList->getAll()); + } + + public function testDisallowParallelRunsWithNoOtherJobs() { + $jobList = new DummyJobList(); + $job = new TestJob($this->timeFactory, $this); + $job->setAllowParallelRuns(false); + $jobList->add($job); + + $jobList->setHasReservedJob(null, false); + $jobList->setHasReservedJob(TestJob::class, false); + $job->start($jobList); + $this->assertTrue($this->run); + } + + public function testAllowParallelRunsWithNoOtherJobs() { + $jobList = new DummyJobList(); + $job = new TestJob($this->timeFactory, $this); + $job->setAllowParallelRuns(true); + $jobList->add($job); + + $jobList->setHasReservedJob(null, false); + $jobList->setHasReservedJob(TestJob::class, false); + $job->start($jobList); + $this->assertTrue($this->run); + } + + public function testAllowParallelRunsWithOtherJobs() { + $jobList = new DummyJobList(); + $job = new TestJob($this->timeFactory, $this); + $job->setAllowParallelRuns(true); + $jobList->add($job); + + $jobList->setHasReservedJob(null, true); + $jobList->setHasReservedJob(TestJob::class, true); + $job->start($jobList); + $this->assertTrue($this->run); + } + + public function testDisallowParallelRunsWithOtherJobs() { + $jobList = new DummyJobList(); + $job = new TestJob($this->timeFactory, $this); + $job->setAllowParallelRuns(false); + $jobList->add($job); + + $jobList->setHasReservedJob(null, true); + $jobList->setHasReservedJob(TestJob::class, true); + $job->start($jobList); + $this->assertFalse($this->run); + } + public function markRun() { $this->run = true; } diff --git a/tests/lib/BackgroundJob/TestJob.php b/tests/lib/BackgroundJob/TestJob.php index e15c7e86c998e..cc7a4651c4b8f 100644 --- a/tests/lib/BackgroundJob/TestJob.php +++ b/tests/lib/BackgroundJob/TestJob.php @@ -8,7 +8,9 @@ namespace Test\BackgroundJob; -class TestJob extends \OC\BackgroundJob\Job { +use OCP\AppFramework\Utility\ITimeFactory; + +class TestJob extends \OCP\BackgroundJob\Job { private $testCase; /** @@ -20,7 +22,8 @@ class TestJob extends \OC\BackgroundJob\Job { * @param JobTest $testCase * @param callable $callback */ - public function __construct($testCase = null, $callback = null) { + public function __construct(ITimeFactory $time = null, $testCase = null, $callback = null) { + parent::__construct($time ?? \OC::$server->get(ITimeFactory::class)); $this->testCase = $testCase; $this->callback = $callback; } From 56cb5d1d13d5402a2281722bd82454e8c39e3107 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 20 Apr 2023 13:27:36 +0200 Subject: [PATCH 04/15] fix(tests): Remove duplicated method Signed-off-by: Marcel Klehr --- tests/lib/BackgroundJob/JobTest.php | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/tests/lib/BackgroundJob/JobTest.php b/tests/lib/BackgroundJob/JobTest.php index 26e6b4ed85b7c..37ced6e475732 100644 --- a/tests/lib/BackgroundJob/JobTest.php +++ b/tests/lib/BackgroundJob/JobTest.php @@ -63,27 +63,6 @@ public function testRemoveAfterError() { $this->assertCount(1, $jobList->getAll()); } - public function testRemoveAfterError() { - $jobList = new DummyJobList(); - $job = new TestJob($this->timeFactory, $this, function () { - $test = null; - $test->someMethod(); - }); - $jobList->add($job); - - $logger = $this->getMockBuilder(ILogger::class) - ->disableOriginalConstructor() - ->getMock(); - $logger->expects($this->once()) - ->method('logException') - ->with($this->isInstanceOf(\Throwable::class)); - - $this->assertCount(1, $jobList->getAll()); - $job->execute($jobList, $logger); - $this->assertTrue($this->run); - $this->assertCount(1, $jobList->getAll()); - } - public function testDisallowParallelRunsWithNoOtherJobs() { $jobList = new DummyJobList(); $job = new TestJob($this->timeFactory, $this); From 6f9a3218d02ca857e0ade4658fc371193e91dd52 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 20 Apr 2023 13:34:16 +0200 Subject: [PATCH 05/15] Update lib/private/BackgroundJob/JobList.php Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com> Signed-off-by: Marcel Klehr --- lib/private/BackgroundJob/JobList.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index 707a8c799045f..d00d219541ea2 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -388,7 +388,8 @@ public function hasReservedJob(?string $className = null): bool { $query = $this->connection->getQueryBuilder(); $query->select('*') ->from('jobs') - ->where($query->expr()->neq('reserved_at', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT))); + ->where($query->expr()->neq('reserved_at', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT))) + ->setMaxResults(1); if ($className !== null) { $query->andWhere($query->expr()->eq('class', $query->createNamedParameter($className))); From 0f3211c4e46492eb90b24af231cd738f989eed3d Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 20 Apr 2023 13:55:28 +0200 Subject: [PATCH 06/15] Add IParallelAwareJob interface Signed-off-by: Marcel Klehr --- .../BackgroundJob/IParallelAwareJob.php | 20 +++++++++++++++++++ lib/public/BackgroundJob/Job.php | 4 ++-- 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 lib/public/BackgroundJob/IParallelAwareJob.php diff --git a/lib/public/BackgroundJob/IParallelAwareJob.php b/lib/public/BackgroundJob/IParallelAwareJob.php new file mode 100644 index 0000000000000..9374901e09f9d --- /dev/null +++ b/lib/public/BackgroundJob/IParallelAwareJob.php @@ -0,0 +1,20 @@ +allowParallelRuns = $allow; } From 281c8047b6a1d6581201c126180a5570eca5d612 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 20 Apr 2023 14:21:57 +0200 Subject: [PATCH 07/15] JobListTest: clearJobsList before testHasReservedJobs Signed-off-by: Marcel Klehr --- tests/lib/BackgroundJob/JobListTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/lib/BackgroundJob/JobListTest.php b/tests/lib/BackgroundJob/JobListTest.php index d42c2521761d5..b370a233075ee 100644 --- a/tests/lib/BackgroundJob/JobListTest.php +++ b/tests/lib/BackgroundJob/JobListTest.php @@ -246,6 +246,7 @@ public function testSetLastRun() { } public function testHasReservedJobs() { + $this->clearJobsList(); $job = new TestJob(); $this->instance->add($job); From 94038e0673d38a0ddfcbda2d6ece8d0d145b0956 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 20 Apr 2023 15:23:13 +0200 Subject: [PATCH 08/15] IParallelAwareJob: Add missing boilerplate Signed-off-by: Marcel Klehr --- .../BackgroundJob/IParallelAwareJob.php | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lib/public/BackgroundJob/IParallelAwareJob.php b/lib/public/BackgroundJob/IParallelAwareJob.php index 9374901e09f9d..30c69e16b5f4e 100644 --- a/lib/public/BackgroundJob/IParallelAwareJob.php +++ b/lib/public/BackgroundJob/IParallelAwareJob.php @@ -1,7 +1,34 @@ + * + * @author Marcel Klehr + * + * @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; +/** + * @since 27.0.0 + */ interface IParallelAwareJob { /** * Set this to false to prevent two Jobs from the same class from running in parallel From 5608b5077873d1d6b83b2a0c7c45b6a6b243a61c Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 20 Apr 2023 15:29:47 +0200 Subject: [PATCH 09/15] Fix BackgroundJob tests Signed-off-by: Marcel Klehr --- lib/private/BackgroundJob/JobList.php | 3 ++- lib/public/BackgroundJob/Job.php | 6 ++++-- tests/lib/BackgroundJob/JobListTest.php | 13 ++++++++++--- tests/lib/BackgroundJob/JobTest.php | 18 ++++++++++-------- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index d00d219541ea2..638919d4c3927 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -396,7 +396,8 @@ public function hasReservedJob(?string $className = null): bool { } try { - return $query->executeQuery()->rowCount() > 0; + $result = $query->executeQuery(); + return count($result->fetchAll()) > 0; } catch (Exception $e) { return false; } diff --git a/lib/public/BackgroundJob/Job.php b/lib/public/BackgroundJob/Job.php index c1e129f12212e..455fb3d42e78d 100644 --- a/lib/public/BackgroundJob/Job.php +++ b/lib/public/BackgroundJob/Job.php @@ -44,6 +44,7 @@ abstract class Job implements IJob, IParallelAwareJob { protected $argument; protected ITimeFactory $time; protected bool $allowParallelRuns = true; + private ?ILogger $logger = null; /** * @since 15.0.0 @@ -62,6 +63,7 @@ public function __construct(ITimeFactory $time) { * @since 15.0.0 */ public function execute(IJobList $jobList, ILogger $logger = null) { + $this->logger = $logger; $this->start($jobList); } @@ -71,7 +73,7 @@ public function execute(IJobList $jobList, ILogger $logger = null) { */ public function start(IJobList $jobList): void { $jobList->setLastRun($this); - $logger = \OCP\Server::get(LoggerInterface::class); + $logger = $this->logger ?? \OCP\Server::get(LoggerInterface::class); if (!$this->getAllowParallelRuns() && $jobList->hasReservedJob(get_class($this))) { $logger->debug('Skipping ' . get_class($this) . ' job with ID ' . $this->getId() . ' because another job with the same class is already running', ['app' => 'cron']); @@ -86,7 +88,7 @@ public function start(IJobList $jobList): void { $logger->debug('Finished ' . get_class($this) . ' job with ID ' . $this->getId() . ' in ' . $timeTaken . ' seconds', ['app' => 'cron']); $jobList->setExecutionTime($this, $timeTaken); - } catch (\Exception $e) { + } catch (\Throwable $e) { if ($logger) { $logger->error('Error while running background job (class: ' . get_class($this) . ', arguments: ' . print_r($this->argument, true) . ')', [ 'app' => 'core', diff --git a/tests/lib/BackgroundJob/JobListTest.php b/tests/lib/BackgroundJob/JobListTest.php index b370a233075ee..78565c11780a0 100644 --- a/tests/lib/BackgroundJob/JobListTest.php +++ b/tests/lib/BackgroundJob/JobListTest.php @@ -32,6 +32,7 @@ class JobListTest extends TestCase { /** @var \OCP\AppFramework\Utility\ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ protected $timeFactory; + private bool $ran = false; protected function setUp(): void { parent::setUp(); @@ -247,7 +248,10 @@ public function testSetLastRun() { public function testHasReservedJobs() { $this->clearJobsList(); - $job = new TestJob(); + $job = new TestJob($this->timeFactory, $this, function () { + $this->assertTrue($this->instance->hasReservedJob()); + $this->assertTrue($this->instance->hasReservedJob(TestJob::class)); + }); $this->instance->add($job); $this->assertFalse($this->instance->hasReservedJob()); @@ -255,7 +259,10 @@ public function testHasReservedJobs() { $job->start($this->instance); - $this->assertTrue($this->instance->hasReservedJob()); - $this->assertTrue($this->instance->hasReservedJob(TestJob::class)); + $this->assertTrue($this->ran); + } + + public function markRun() { + $this->ran = true; } } diff --git a/tests/lib/BackgroundJob/JobTest.php b/tests/lib/BackgroundJob/JobTest.php index 37ced6e475732..b44324711b865 100644 --- a/tests/lib/BackgroundJob/JobTest.php +++ b/tests/lib/BackgroundJob/JobTest.php @@ -33,8 +33,7 @@ public function testRemoveAfterException() { ->disableOriginalConstructor() ->getMock(); $logger->expects($this->once()) - ->method('logException') - ->with($e); + ->method('error'); $this->assertCount(1, $jobList->getAll()); $job->execute($jobList, $logger); @@ -54,8 +53,7 @@ public function testRemoveAfterError() { ->disableOriginalConstructor() ->getMock(); $logger->expects($this->once()) - ->method('logException') - ->with($this->isInstanceOf(\Throwable::class)); + ->method('error'); $this->assertCount(1, $jobList->getAll()); $job->execute($jobList, $logger); @@ -65,7 +63,8 @@ public function testRemoveAfterError() { public function testDisallowParallelRunsWithNoOtherJobs() { $jobList = new DummyJobList(); - $job = new TestJob($this->timeFactory, $this); + $job = new TestJob($this->timeFactory, $this, function() { + }); $job->setAllowParallelRuns(false); $jobList->add($job); @@ -77,7 +76,8 @@ public function testDisallowParallelRunsWithNoOtherJobs() { public function testAllowParallelRunsWithNoOtherJobs() { $jobList = new DummyJobList(); - $job = new TestJob($this->timeFactory, $this); + $job = new TestJob($this->timeFactory, $this, function() { + }); $job->setAllowParallelRuns(true); $jobList->add($job); @@ -89,7 +89,8 @@ public function testAllowParallelRunsWithNoOtherJobs() { public function testAllowParallelRunsWithOtherJobs() { $jobList = new DummyJobList(); - $job = new TestJob($this->timeFactory, $this); + $job = new TestJob($this->timeFactory, $this, function() { + }); $job->setAllowParallelRuns(true); $jobList->add($job); @@ -101,7 +102,8 @@ public function testAllowParallelRunsWithOtherJobs() { public function testDisallowParallelRunsWithOtherJobs() { $jobList = new DummyJobList(); - $job = new TestJob($this->timeFactory, $this); + $job = new TestJob($this->timeFactory, $this, function() { + }); $job->setAllowParallelRuns(false); $jobList->add($job); From a0c722ad693e4f5ba580b22f720aa00d31504cad Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 20 Apr 2023 16:39:38 +0200 Subject: [PATCH 10/15] Run cs:Fix Signed-off-by: Marcel Klehr --- tests/lib/BackgroundJob/JobTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/lib/BackgroundJob/JobTest.php b/tests/lib/BackgroundJob/JobTest.php index b44324711b865..ca9d68f0a2b4c 100644 --- a/tests/lib/BackgroundJob/JobTest.php +++ b/tests/lib/BackgroundJob/JobTest.php @@ -63,7 +63,7 @@ public function testRemoveAfterError() { public function testDisallowParallelRunsWithNoOtherJobs() { $jobList = new DummyJobList(); - $job = new TestJob($this->timeFactory, $this, function() { + $job = new TestJob($this->timeFactory, $this, function () { }); $job->setAllowParallelRuns(false); $jobList->add($job); @@ -76,7 +76,7 @@ public function testDisallowParallelRunsWithNoOtherJobs() { public function testAllowParallelRunsWithNoOtherJobs() { $jobList = new DummyJobList(); - $job = new TestJob($this->timeFactory, $this, function() { + $job = new TestJob($this->timeFactory, $this, function () { }); $job->setAllowParallelRuns(true); $jobList->add($job); @@ -89,7 +89,7 @@ public function testAllowParallelRunsWithNoOtherJobs() { public function testAllowParallelRunsWithOtherJobs() { $jobList = new DummyJobList(); - $job = new TestJob($this->timeFactory, $this, function() { + $job = new TestJob($this->timeFactory, $this, function () { }); $job->setAllowParallelRuns(true); $jobList->add($job); @@ -102,7 +102,7 @@ public function testAllowParallelRunsWithOtherJobs() { public function testDisallowParallelRunsWithOtherJobs() { $jobList = new DummyJobList(); - $job = new TestJob($this->timeFactory, $this, function() { + $job = new TestJob($this->timeFactory, $this, function () { }); $job->setAllowParallelRuns(false); $jobList->add($job); From 5a7f023bfc35fa9bc3b24d724541c270db917009 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 21 Apr 2023 10:34:24 +0200 Subject: [PATCH 11/15] JobList#hasReservedJobs: Close cursor so Joas doesn't die Signed-off-by: Marcel Klehr --- lib/private/BackgroundJob/JobList.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index 638919d4c3927..3846aed3d61ba 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -397,7 +397,9 @@ public function hasReservedJob(?string $className = null): bool { try { $result = $query->executeQuery(); - return count($result->fetchAll()) > 0; + $hasReservedJobs = $result->fetch() !== false; + $result->closeCursor(); + return $hasReservedJobs; } catch (Exception $e) { return false; } From d1e4a7a75069f4a24b2334fceeaa1da855944d59 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Sun, 23 Apr 2023 13:36:47 +0200 Subject: [PATCH 12/15] Update autoloaders Signed-off-by: Marcel Klehr --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 3e1b9559d5ead..f1614900fa02c 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -117,6 +117,7 @@ 'OCP\\AutoloadNotAllowedException' => $baseDir . '/lib/public/AutoloadNotAllowedException.php', 'OCP\\BackgroundJob\\IJob' => $baseDir . '/lib/public/BackgroundJob/IJob.php', 'OCP\\BackgroundJob\\IJobList' => $baseDir . '/lib/public/BackgroundJob/IJobList.php', + 'OCP\\BackgroundJob\\IParallelAwareJob' => $baseDir . '/lib/public/BackgroundJob/IParallelAwareJob.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', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 1a0d68fd20424..4df24813e7936 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -150,6 +150,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\AutoloadNotAllowedException' => __DIR__ . '/../../..' . '/lib/public/AutoloadNotAllowedException.php', 'OCP\\BackgroundJob\\IJob' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob/IJob.php', 'OCP\\BackgroundJob\\IJobList' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob/IJobList.php', + 'OCP\\BackgroundJob\\IParallelAwareJob' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob/IParallelAwareJob.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', From 6148c5e0a126d267cfd9aa74be29539a15b11490 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Mon, 24 Apr 2023 12:35:52 +0200 Subject: [PATCH 13/15] Update tests/lib/BackgroundJob/DummyJobList.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com> Signed-off-by: Marcel Klehr --- tests/lib/BackgroundJob/DummyJobList.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/lib/BackgroundJob/DummyJobList.php b/tests/lib/BackgroundJob/DummyJobList.php index 700393828649c..42b69cfbe418c 100644 --- a/tests/lib/BackgroundJob/DummyJobList.php +++ b/tests/lib/BackgroundJob/DummyJobList.php @@ -141,11 +141,11 @@ public function setLastRun(IJob $job): void { } public function hasReservedJob(?string $className = null): bool { - return $this->reserved[$className]; + return $this->reserved[$className ?? '']; } public function setHasReservedJob(?string $className, bool $hasReserved): void { - $this->reserved[$className] = $hasReserved; + $this->reserved[$className ?? ''] = $hasReserved; } public function setExecutionTime(IJob $job, $timeTaken): void { From 524d053eb2de5f412a65863368520719628fb002 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Mon, 24 Apr 2023 17:16:15 +0200 Subject: [PATCH 14/15] JobList: add debug log when hasReservedJob query fails Signed-off-by: Marcel Klehr --- lib/private/BackgroundJob/JobList.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php index 3846aed3d61ba..fbeee1f56e94c 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -39,16 +39,19 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IConfig; use OCP\IDBConnection; +use Psr\Log\LoggerInterface; class JobList implements IJobList { protected IDBConnection $connection; protected IConfig $config; protected ITimeFactory $timeFactory; + protected LoggerInterface $logger; - public function __construct(IDBConnection $connection, IConfig $config, ITimeFactory $timeFactory) { + public function __construct(IDBConnection $connection, IConfig $config, ITimeFactory $timeFactory, LoggerInterface $logger) { $this->connection = $connection; $this->config = $config; $this->timeFactory = $timeFactory; + $this->logger = $logger; } /** @@ -401,6 +404,7 @@ public function hasReservedJob(?string $className = null): bool { $result->closeCursor(); return $hasReservedJobs; } catch (Exception $e) { + $this->logger->debug('Querying reserved jobs failed', ['exception' => $e]); return false; } } From 06d6cf4ebcc49d97f89b46e127599b97f8fa1800 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Mon, 24 Apr 2023 19:07:13 +0200 Subject: [PATCH 15/15] JobListTest: Fix JobList constructor call Signed-off-by: Marcel Klehr --- tests/lib/BackgroundJob/JobListTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/lib/BackgroundJob/JobListTest.php b/tests/lib/BackgroundJob/JobListTest.php index 78565c11780a0..c15e556d5f7cb 100644 --- a/tests/lib/BackgroundJob/JobListTest.php +++ b/tests/lib/BackgroundJob/JobListTest.php @@ -12,6 +12,7 @@ use OCP\BackgroundJob\IJob; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IConfig; +use Psr\Log\LoggerInterface; use Test\TestCase; /** @@ -44,7 +45,8 @@ protected function setUp(): void { $this->instance = new \OC\BackgroundJob\JobList( $this->connection, $this->config, - $this->timeFactory + $this->timeFactory, + \OC::$server->get(LoggerInterface::class), ); }