diff options
-rw-r--r-- | lib/private/BackgroundJob/JobList.php | 3 | ||||
-rw-r--r-- | lib/public/BackgroundJob/Job.php | 6 | ||||
-rw-r--r-- | tests/lib/BackgroundJob/JobListTest.php | 13 | ||||
-rw-r--r-- | 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 d00d219541e..638919d4c39 100644 --- a/lib/private/BackgroundJob/JobList.php +++ b/lib/private/BackgroundJob/JobList.php @@ -396,7 +396,8 @@ class JobList implements IJobList { } 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 c1e129f1221..455fb3d42e7 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 @@ abstract class Job implements IJob, IParallelAwareJob { * @since 15.0.0 */ public function execute(IJobList $jobList, ILogger $logger = null) { + $this->logger = $logger; $this->start($jobList); } @@ -71,7 +73,7 @@ abstract class Job implements IJob, IParallelAwareJob { */ 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 @@ abstract class Job implements IJob, IParallelAwareJob { $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 b370a233075..78565c11780 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 @@ class JobListTest extends TestCase { 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 @@ class JobListTest extends TestCase { $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 37ced6e4757..b44324711b8 100644 --- a/tests/lib/BackgroundJob/JobTest.php +++ b/tests/lib/BackgroundJob/JobTest.php @@ -33,8 +33,7 @@ class JobTest extends \Test\TestCase { ->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 @@ class JobTest extends \Test\TestCase { ->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 @@ class JobTest extends \Test\TestCase { 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 @@ class JobTest extends \Test\TestCase { 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 @@ class JobTest extends \Test\TestCase { 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 @@ class JobTest extends \Test\TestCase { 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); |