diff --git a/src/Progressable.php b/src/Progressable.php index 0997237..cc9f57f 100644 --- a/src/Progressable.php +++ b/src/Progressable.php @@ -90,6 +90,41 @@ trait Progressable { */ protected mixed $onComplete = null; + /** + * Indicates if the process is currently locked. + */ + protected bool $isLocked = false; + + /** + * Execute a callback within a cache lock. + */ + protected function withLock(callable $callback): mixed + { + if ($this->customSaveData !== null || $this->customGetData !== null) { + return $callback(); + } + + if ($this->isLocked) { + return $callback(); + } + + $key = $this->getStorageKeyName().'_lock'; + + try { + return Cache::lock($key, 5)->block(5, function () use ($callback) { + $this->isLocked = true; + + try { + return $callback(); + } finally { + $this->isLocked = false; + } + }); + } catch (\Exception $e) { + throw $e; + } + } + /** * Set the callback function for saving cache data. * @@ -244,17 +279,19 @@ public function isOverallComplete(): bool { * @throws UniqueNameNotSetException */ public function removeLocalFromOverall(): static { - $progressData = $this->getOverallProgressData(); - $localKey = $this->getLocalKey(); + return $this->withLock(function () { + $progressData = $this->getOverallProgressData(); + $localKey = $this->getLocalKey(); - if (isset($progressData[$localKey])) { - unset($progressData[$localKey]); - $this->saveOverallProgressData($progressData); - } + if (isset($progressData[$localKey])) { + unset($progressData[$localKey]); + $this->saveOverallProgressData($progressData); + } - $this->progress = 0; + $this->progress = 0; - return $this; + return $this; + }); } /** @@ -287,23 +324,25 @@ public function setLocalProgress(float $progress): static { * Update the progress data in storage. */ protected function updateLocalProgressData(float $progress): static { - $progressData = $this->getOverallProgressData(); + return $this->withLock(function () use ($progress) { + $progressData = $this->getOverallProgressData(); - $localData = [ - 'progress' => $progress, - ]; + $localData = [ + 'progress' => $progress, + ]; - if ($this->statusMessage !== null) { - $localData['message'] = $this->statusMessage; - } + if ($this->statusMessage !== null) { + $localData['message'] = $this->statusMessage; + } - if (! empty($this->metadata)) { - $localData['metadata'] = $this->metadata; - } + if (! empty($this->metadata)) { + $localData['metadata'] = $this->metadata; + } - $progressData[$this->getLocalKey()] = $localData; + $progressData[$this->getLocalKey()] = $localData; - return $this->saveOverallProgressData($progressData); + return $this->saveOverallProgressData($progressData); + }); } /** @@ -319,21 +358,23 @@ public function getLocalKey(): string { * @param string $name The new local key name */ public function setLocalKey(string $name): static { - $currentKey = $this->getLocalKey(); - $overallProgressData = $this->getOverallProgressData(); - - if (isset($overallProgressData[$currentKey])) { - // Rename the local key preserving the data - $overallProgressData[$name] = $overallProgressData[$currentKey]; - unset($overallProgressData[$currentKey]); - $this->saveOverallProgressData($overallProgressData); - } + return $this->withLock(function () use ($name) { + $currentKey = $this->getLocalKey(); + $overallProgressData = $this->getOverallProgressData(); - $this->localKey = $name; + if (isset($overallProgressData[$currentKey])) { + // Rename the local key preserving the data + $overallProgressData[$name] = $overallProgressData[$currentKey]; + unset($overallProgressData[$currentKey]); + $this->saveOverallProgressData($overallProgressData); + } - $this->makeSureLocalIsPartOfTheCalc(); + $this->localKey = $name; - return $this; + $this->makeSureLocalIsPartOfTheCalc(); + + return $this; + }); } /** diff --git a/tests/LockingTest.php b/tests/LockingTest.php new file mode 100644 index 0000000..56e87cc --- /dev/null +++ b/tests/LockingTest.php @@ -0,0 +1,77 @@ +overallUniqueName = 'test_locking'; + // Reset trait state + $this->isLocked = false; + $this->customSaveData = null; + $this->customGetData = null; + $this->progress = 0; + $this->metadata = []; + $this->statusMessage = null; + } + + public function test_update_local_progress_uses_lock() + { + $lockMock = Mockery::mock(\Illuminate\Contracts\Cache\Lock::class); + $lockMock->shouldReceive('block') + ->once() + ->with(5, Mockery::type('callable')) + ->andReturnUsing(function ($seconds, $callback) { + return $callback(); + }); + + Cache::shouldReceive('lock') + ->once() + ->with('progressable_test_locking_lock', 5) + ->andReturn($lockMock); + + Cache::shouldReceive('get') + ->once() + ->andReturn([]); + + Cache::shouldReceive('put') + ->once(); + + $this->setLocalProgress(50); + } + + public function test_nested_locks_are_avoided() + { + $lockMock = Mockery::mock(\Illuminate\Contracts\Cache\Lock::class); + $lockMock->shouldReceive('block') + ->once() + ->with(5, Mockery::type('callable')) + ->andReturnUsing(function ($seconds, $callback) { + return $callback(); + }); + + // The key for lock will be based on overallUniqueName + Cache::shouldReceive('lock') + ->once() + ->with('progressable_test_locking_lock', 5) + ->andReturn($lockMock); + + Cache::shouldReceive('get') + ->atLeast()->once() + ->andReturn([]); + + Cache::shouldReceive('put') + ->atLeast()->once(); + + $this->setLocalKey('new_key'); + } +}