Merge pull request #37835 from nextcloud/feat/background-allow-parallel-runs

feat(BackgroundJobs): Allow preventing parallel runs for a job class

4 files changed, 101 insertions, 9 deletions

diff --git a/tests/lib/BackgroundJob/DummyJobList.php b/tests/lib/BackgroundJob/DummyJobList.php
index be48259789a..42b69cfbe41 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 @@ class DummyJobList extends \OC\BackgroundJob\JobList {
 		$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 ea02e1cd8b9..c15e556d5f7 100644
--- a/tests/lib/BackgroundJob/JobListTest.php
+++ b/tests/lib/BackgroundJob/JobListTest.php
@@ -12,6 +12,7 @@ use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJob;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IConfig;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 /**
@@ -32,6 +33,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();
@@ -43,7 +45,8 @@ class JobListTest extends TestCase {
 		$this->instance = new \OC\BackgroundJob\JobList(
 			$this->connection,
 			$this->config,
-			$this->timeFactory
+			$this->timeFactory,
+			\OC::$server->get(LoggerInterface::class),
 		);
 	}
 
@@ -244,4 +247,24 @@ class JobListTest extends TestCase {
 		$this->assertGreaterThanOrEqual($timeStart, $addedJob->getLastRun());
 		$this->assertLessThanOrEqual($timeEnd, $addedJob->getLastRun());
 	}
+
+	public function testHasReservedJobs() {
+		$this->clearJobsList();
+		$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());
+		$this->assertFalse($this->instance->hasReservedJob(TestJob::class));
+
+		$job->start($this->instance);
+
+		$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 b4048aa1c22..ca9d68f0a2b 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);
@@ -30,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);
@@ -41,7 +43,7 @@ class JobTest extends \Test\TestCase {
 
 	public function testRemoveAfterError() {
 		$jobList = new DummyJobList();
-		$job = new TestJob($this, function () {
+		$job = new TestJob($this->timeFactory, $this, function () {
 			$test = null;
 			$test->someMethod();
 		});
@@ -51,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);
@@ -60,6 +61,58 @@ class JobTest extends \Test\TestCase {
 		$this->assertCount(1, $jobList->getAll());
 	}
 
+	public function testDisallowParallelRunsWithNoOtherJobs() {
+		$jobList = new DummyJobList();
+		$job = new TestJob($this->timeFactory, $this, function () {
+		});
+		$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, function () {
+		});
+		$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, function () {
+		});
+		$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, function () {
+		});
+		$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 e15c7e86c99..cc7a4651c4b 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;
 	}