From 21352e1fdbbf225cdf3b0167afa768d2edfcb98d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Barto=C5=A1?= <bartos.developer152@gmail.com>
Date: Mon, 11 Dec 2023 01:50:57 +0100
Subject: [PATCH] ManagedScheduler: acquired job locks are scoped just to their
 id

Changing run frequency or job name will not make process loose the lock.
---
 CHANGELOG.md                             |  3 +++
 docs/README.md                           | 13 ++++++++++++-
 src/ManagedScheduler.php                 |  4 +---
 tests/Unit/Command/RunCommandTest.php    |  2 +-
 tests/Unit/Command/RunJobCommandTest.php |  2 +-
 tests/Unit/SimpleSchedulerTest.php       |  2 +-
 6 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ef61653..ebb29ed 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -38,3 +38,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 	- `runJobs()` returns `Generator<int, JobSummary, void, RunSummary>` instead of `RunSummary`
 - `ProcessJobExecutor`
 	- constructor requires `JobManager` as first parameter
+- `ManagedScheduler`
+	- acquired job locks are scoped just to their id - changing run frequency or job name will not make process loose
+	  the lock
diff --git a/docs/README.md b/docs/README.md
index 6352261..f67e025 100644
--- a/docs/README.md
+++ b/docs/README.md
@@ -237,7 +237,7 @@ $scheduler = new SimpleScheduler($errorHandler);
 
 ## Locks and job overlapping
 
-Crontab jobs are time-based and simply run at specified intervals. If they take too long, they may overlap and run
+Jobs are time-based and simply run at specified intervals. If they take too long, they may overlap and run
 simultaneously. This may cause issues if the jobs access the same resources, such as files or databases, leading to
 conflicts or data corruption.
 
@@ -274,6 +274,17 @@ new CallbackJob(function (JobLock $lock): void {
 });
 ```
 
+To make sure locks are correctly used during deployments, specify constant id for every added job, lock identifiers rely
+on that fact. Otherwise, your job id will change when new jobs are added before it and acquired lock will be ignored.
+
+```php
+$scheduler->addJob(
+	/* ... */,
+	/* ... */,
+	'job-id',
+);
+```
+
 ## Parallelization and process isolation
 
 It is important for crontab scheduler tasks to be executed asynchronously and in separate processes because this
diff --git a/src/ManagedScheduler.php b/src/ManagedScheduler.php
index 604efb6..4d0c818 100644
--- a/src/ManagedScheduler.php
+++ b/src/ManagedScheduler.php
@@ -152,9 +152,7 @@ private function runInternal($id, Job $job, CronExpression $expression, int $sec
 			$this->clock->now(),
 		);
 
-		$lock = $this->lockFactory->createLock(
-			"Orisai.Scheduler.Job/{$info->getExpression()}-{$info->getName()}-$id",
-		);
+		$lock = $this->lockFactory->createLock("Orisai.Scheduler.Job/$id");
 
 		if (!$lock->acquire()) {
 			return [
diff --git a/tests/Unit/Command/RunCommandTest.php b/tests/Unit/Command/RunCommandTest.php
index 5d4d432..8c52844 100644
--- a/tests/Unit/Command/RunCommandTest.php
+++ b/tests/Unit/Command/RunCommandTest.php
@@ -222,7 +222,7 @@ public function testSkip(): void
 			new CronExpression('* * * * *'),
 		);
 
-		$lock = $lockFactory->createLock('Orisai.Scheduler.Job/* * * * *-job1-0');
+		$lock = $lockFactory->createLock('Orisai.Scheduler.Job/0');
 		$lock->acquire();
 
 		$command = new RunCommand($scheduler);
diff --git a/tests/Unit/Command/RunJobCommandTest.php b/tests/Unit/Command/RunJobCommandTest.php
index 3c60c7e..89ec786 100644
--- a/tests/Unit/Command/RunJobCommandTest.php
+++ b/tests/Unit/Command/RunJobCommandTest.php
@@ -163,7 +163,7 @@ public function testSkip(): void
 			new CronExpression('* * * * *'),
 		);
 
-		$lock = $lockFactory->createLock('Orisai.Scheduler.Job/* * * * *-job1-0');
+		$lock = $lockFactory->createLock('Orisai.Scheduler.Job/0');
 		$lock->acquire();
 
 		$command = new RunJobCommand($scheduler);
diff --git a/tests/Unit/SimpleSchedulerTest.php b/tests/Unit/SimpleSchedulerTest.php
index 95dfc17..6c626a6 100644
--- a/tests/Unit/SimpleSchedulerTest.php
+++ b/tests/Unit/SimpleSchedulerTest.php
@@ -512,7 +512,7 @@ static function () use (&$i2): void {
 			new CronExpression('* * * * *'),
 		);
 
-		$lock = $lockFactory->createLock('Orisai.Scheduler.Job/* * * * *-job1-0');
+		$lock = $lockFactory->createLock('Orisai.Scheduler.Job/0');
 		$lock->acquire();
 
 		// Lock is active, job is not executed (but the other one is)