From 51728cb699dfa200576368e75987fd2d0db13261 Mon Sep 17 00:00:00 2001 From: Johan Rosenson Date: Sat, 4 Feb 2023 22:27:18 +0700 Subject: [PATCH 1/3] use the lock functionality of the cache store (if possible) --- .../Console/Scheduling/CacheEventMutex.php | 24 +++++++++ .../Scheduling/CacheEventMutexTest.php | 51 +++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/src/Illuminate/Console/Scheduling/CacheEventMutex.php b/src/Illuminate/Console/Scheduling/CacheEventMutex.php index 1f6b15eacbea..c06022be0e87 100644 --- a/src/Illuminate/Console/Scheduling/CacheEventMutex.php +++ b/src/Illuminate/Console/Scheduling/CacheEventMutex.php @@ -3,6 +3,7 @@ namespace Illuminate\Console\Scheduling; use Illuminate\Contracts\Cache\Factory as Cache; +use Illuminate\Contracts\Cache\LockProvider; class CacheEventMutex implements EventMutex, CacheAware { @@ -39,6 +40,13 @@ public function __construct(Cache $cache) */ public function create(Event $event) { + if ($this->cache->store($this->store)->getStore() instanceof LockProvider) { + $lock = $this->cache->store($this->store)->getStore() + ->lock($event->mutexName(), $event->expiresAt * 60); + + return $lock->acquire(); + } + return $this->cache->store($this->store)->add( $event->mutexName(), true, $event->expiresAt * 60 ); @@ -52,6 +60,13 @@ public function create(Event $event) */ public function exists(Event $event) { + if ($this->cache->store($this->store)->getStore() instanceof LockProvider) { + $lock = $this->cache->store($this->store)->getStore() + ->lock($event->mutexName(), $event->expiresAt * 60); + + return ! $lock->get(fn () : bool => true); + } + return $this->cache->store($this->store)->has($event->mutexName()); } @@ -63,6 +78,15 @@ public function exists(Event $event) */ public function forget(Event $event) { + if ($this->cache->store($this->store)->getStore() instanceof LockProvider) { + $lock = $this->cache->store($this->store)->getStore() + ->lock($event->mutexName(), $event->expiresAt * 60); + + $lock->forceRelease(); + + return; + } + $this->cache->store($this->store)->forget($event->mutexName()); } diff --git a/tests/Console/Scheduling/CacheEventMutexTest.php b/tests/Console/Scheduling/CacheEventMutexTest.php index 1e770abd3031..acc7eb31b9c0 100644 --- a/tests/Console/Scheduling/CacheEventMutexTest.php +++ b/tests/Console/Scheduling/CacheEventMutexTest.php @@ -2,6 +2,7 @@ namespace Illuminate\Tests\Console\Scheduling; +use Illuminate\Cache\ArrayStore; use Illuminate\Console\Scheduling\CacheEventMutex; use Illuminate\Console\Scheduling\Event; use Illuminate\Contracts\Cache\Factory; @@ -44,6 +45,7 @@ protected function setUp(): void public function testPreventOverlap() { + $this->cacheRepository->shouldReceive('getStore')->andReturn(new \stdClass); $this->cacheRepository->shouldReceive('add')->once(); $this->cacheMutex->create($this->event); @@ -51,6 +53,7 @@ public function testPreventOverlap() public function testCustomConnection() { + $this->cacheRepository->shouldReceive('getStore')->andReturn(new \stdClass); $this->cacheFactory->shouldReceive('store')->with('test')->andReturn($this->cacheRepository); $this->cacheRepository->shouldReceive('add')->once(); $this->cacheMutex->useStore('test'); @@ -60,6 +63,7 @@ public function testCustomConnection() public function testPreventOverlapFails() { + $this->cacheRepository->shouldReceive('getStore')->andReturn(new \stdClass); $this->cacheRepository->shouldReceive('add')->once()->andReturn(false); $this->assertFalse($this->cacheMutex->create($this->event)); @@ -67,6 +71,7 @@ public function testPreventOverlapFails() public function testOverlapsForNonRunningTask() { + $this->cacheRepository->shouldReceive('getStore')->andReturn(new \stdClass); $this->cacheRepository->shouldReceive('has')->once()->andReturn(false); $this->assertFalse($this->cacheMutex->exists($this->event)); @@ -74,6 +79,7 @@ public function testOverlapsForNonRunningTask() public function testOverlapsForRunningTask() { + $this->cacheRepository->shouldReceive('getStore')->andReturn(new \stdClass); $this->cacheRepository->shouldReceive('has')->once()->andReturn(true); $this->assertTrue($this->cacheMutex->exists($this->event)); @@ -81,8 +87,53 @@ public function testOverlapsForRunningTask() public function testResetOverlap() { + $this->cacheRepository->shouldReceive('getStore')->andReturn(new \stdClass); $this->cacheRepository->shouldReceive('forget')->once(); $this->cacheMutex->forget($this->event); } + + public function testPreventOverlapWithLockProvider() + { + $this->cacheRepository->shouldReceive('getStore')->andReturn(new ArrayStore); + + $this->assertTrue($this->cacheMutex->create($this->event)); + } + + public function testPreventOverlapFailsWithLockProvider() + { + $this->cacheRepository->shouldReceive('getStore')->andReturn(new ArrayStore); + + // first create the lock, so we can test that the next call fails. + $this->cacheMutex->create($this->event); + + $this->assertFalse($this->cacheMutex->create($this->event)); + } + + public function testOverlapsForNonRunningTaskWithLockProvider() + { + $this->cacheRepository->shouldReceive('getStore')->andReturn(new ArrayStore); + + $this->assertFalse($this->cacheMutex->exists($this->event)); + } + + public function testOverlapsForRunningTaskWithLockProvider() + { + $this->cacheRepository->shouldReceive('getStore')->andReturn(new ArrayStore); + + $this->cacheMutex->create($this->event); + + $this->assertTrue($this->cacheMutex->exists($this->event)); + } + + public function testResetOverlapWithLockProvider() + { + $this->cacheRepository->shouldReceive('getStore')->andReturn(new ArrayStore); + + $this->cacheMutex->create($this->event); + + $this->cacheMutex->forget($this->event); + + $this->assertFalse($this->cacheMutex->exists($this->event)); + } } From a5070dc587b905e9f44ea98295011826147a6f3c Mon Sep 17 00:00:00 2001 From: Johan Rosenson Date: Sat, 4 Feb 2023 22:49:36 +0700 Subject: [PATCH 2/3] fix style --- src/Illuminate/Console/Scheduling/CacheEventMutex.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Console/Scheduling/CacheEventMutex.php b/src/Illuminate/Console/Scheduling/CacheEventMutex.php index c06022be0e87..cf128e08bd97 100644 --- a/src/Illuminate/Console/Scheduling/CacheEventMutex.php +++ b/src/Illuminate/Console/Scheduling/CacheEventMutex.php @@ -64,7 +64,7 @@ public function exists(Event $event) $lock = $this->cache->store($this->store)->getStore() ->lock($event->mutexName(), $event->expiresAt * 60); - return ! $lock->get(fn () : bool => true); + return ! $lock->get(fn () => true); } return $this->cache->store($this->store)->has($event->mutexName()); From 259cb354332905e177a44b31df806d7154f13efd Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Tue, 7 Feb 2023 09:30:30 -0600 Subject: [PATCH 3/3] formatting --- .../Console/Scheduling/CacheEventMutex.php | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/Illuminate/Console/Scheduling/CacheEventMutex.php b/src/Illuminate/Console/Scheduling/CacheEventMutex.php index cf128e08bd97..76036835310c 100644 --- a/src/Illuminate/Console/Scheduling/CacheEventMutex.php +++ b/src/Illuminate/Console/Scheduling/CacheEventMutex.php @@ -41,10 +41,9 @@ public function __construct(Cache $cache) public function create(Event $event) { if ($this->cache->store($this->store)->getStore() instanceof LockProvider) { - $lock = $this->cache->store($this->store)->getStore() - ->lock($event->mutexName(), $event->expiresAt * 60); - - return $lock->acquire(); + return $this->cache->store($this->store)->getStore() + ->lock($event->mutexName(), $event->expiresAt * 60) + ->acquire(); } return $this->cache->store($this->store)->add( @@ -61,10 +60,9 @@ public function create(Event $event) public function exists(Event $event) { if ($this->cache->store($this->store)->getStore() instanceof LockProvider) { - $lock = $this->cache->store($this->store)->getStore() - ->lock($event->mutexName(), $event->expiresAt * 60); - - return ! $lock->get(fn () => true); + return ! $this->cache->store($this->store)->getStore() + ->lock($event->mutexName(), $event->expiresAt * 60) + ->get(fn () => true); } return $this->cache->store($this->store)->has($event->mutexName()); @@ -79,10 +77,9 @@ public function exists(Event $event) public function forget(Event $event) { if ($this->cache->store($this->store)->getStore() instanceof LockProvider) { - $lock = $this->cache->store($this->store)->getStore() - ->lock($event->mutexName(), $event->expiresAt * 60); - - $lock->forceRelease(); + $this->cache->store($this->store)->getStore() + ->lock($event->mutexName(), $event->expiresAt * 60) + ->forceRelease(); return; }