From 07bd4cec652890a7d4637b7aed6cd804a9fe82ef Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Mon, 4 Mar 2019 21:28:36 +0100 Subject: [PATCH] Fixing redis cachekey --- src/Core/Lock/AbstractLockDriver.php | 22 ++++++++++++++----- src/Core/Lock/CacheLockDriver.php | 8 +++++-- src/Core/Lock/DatabaseLockDriver.php | 8 ++++--- src/Core/Lock/ILockDriver.php | 4 ++-- .../src/Core/Lock/DatabaseLockDriverTest.php | 11 ++++++++++ tests/src/Core/Lock/LockTest.php | 18 ++++++++++++--- 6 files changed, 55 insertions(+), 16 deletions(-) diff --git a/src/Core/Lock/AbstractLockDriver.php b/src/Core/Lock/AbstractLockDriver.php index 033d6f356e..0aedeeb1b0 100644 --- a/src/Core/Lock/AbstractLockDriver.php +++ b/src/Core/Lock/AbstractLockDriver.php @@ -23,7 +23,8 @@ abstract class AbstractLockDriver extends BaseObject implements ILockDriver * @param string key The Name of the lock * @return bool Returns true if the lock is set */ - protected function hasAcquiredLock($key) { + protected function hasAcquiredLock($key) + { return isset($this->acquireLock[$key]) && $this->acquiredLocks[$key] === true; } @@ -32,7 +33,8 @@ abstract class AbstractLockDriver extends BaseObject implements ILockDriver * * @param string $key The Name of the lock */ - protected function markAcquire($key) { + protected function markAcquire($key) + { $this->acquiredLocks[$key] = true; } @@ -41,18 +43,26 @@ abstract class AbstractLockDriver extends BaseObject implements ILockDriver * * @param string $key The Name of the lock */ - protected function markRelease($key) { + protected function markRelease($key) + { unset($this->acquiredLocks[$key]); } /** * Releases all lock that were set by us * - * @return void + * @return boolean Was the unlock of all locks successful? */ - public function releaseAll() { + public function releaseAll() + { + $return = true; + foreach ($this->acquiredLocks as $acquiredLock => $hasLock) { - $this->releaseLock($acquiredLock); + if (!$this->releaseLock($acquiredLock)) { + $return = false; + } } + + return $return; } } diff --git a/src/Core/Lock/CacheLockDriver.php b/src/Core/Lock/CacheLockDriver.php index c1cd8ffb43..6f34a0d6a9 100644 --- a/src/Core/Lock/CacheLockDriver.php +++ b/src/Core/Lock/CacheLockDriver.php @@ -65,12 +65,16 @@ class CacheLockDriver extends AbstractLockDriver { $cachekey = self::getLockKey($key); + $return = false; + if ($override) { - $this->cache->delete($cachekey); + $return = $this->cache->delete($cachekey); } else { - $this->cache->compareDelete($cachekey, getmypid()); + $return = $this->cache->compareDelete($cachekey, getmypid()); } $this->markRelease($key); + + return $return; } /** diff --git a/src/Core/Lock/DatabaseLockDriver.php b/src/Core/Lock/DatabaseLockDriver.php index a137ef12ed..a8269bc92e 100644 --- a/src/Core/Lock/DatabaseLockDriver.php +++ b/src/Core/Lock/DatabaseLockDriver.php @@ -76,11 +76,11 @@ class DatabaseLockDriver extends AbstractLockDriver $where = ['name' => $key, 'pid' => $this->pid]; } - DBA::delete('locks', $where); + $return = DBA::delete('locks', $where); $this->markRelease($key); - return; + return $return; } /** @@ -88,9 +88,11 @@ class DatabaseLockDriver extends AbstractLockDriver */ public function releaseAll() { - DBA::delete('locks', ['pid' => $this->pid]); + $return = DBA::delete('locks', ['pid' => $this->pid]); $this->acquiredLocks = []; + + return $return; } /** diff --git a/src/Core/Lock/ILockDriver.php b/src/Core/Lock/ILockDriver.php index 7cbaa4fc69..7df5b3f87a 100644 --- a/src/Core/Lock/ILockDriver.php +++ b/src/Core/Lock/ILockDriver.php @@ -36,14 +36,14 @@ interface ILockDriver * @param string $key The Name of the lock * @param bool $override Overrides the lock to get released * - * @return void + * @return boolean Was the unlock successful? */ public function releaseLock($key, $override = false); /** * Releases all lock that were set by us * - * @return void + * @return boolean Was the unlock of all locks successful? */ public function releaseAll(); } diff --git a/tests/src/Core/Lock/DatabaseLockDriverTest.php b/tests/src/Core/Lock/DatabaseLockDriverTest.php index 3d64137699..297e76d50b 100644 --- a/tests/src/Core/Lock/DatabaseLockDriverTest.php +++ b/tests/src/Core/Lock/DatabaseLockDriverTest.php @@ -104,4 +104,15 @@ class DatabaseLockDriverTest extends LockTest parent::testReleaseAfterUnlock(); } + + public function testReleaseWitTTL() + { + $this->mockIsLocked('test', false, $this->startTime, 1); + $this->mockAcquireLock('test', 10, false, $this->pid, false, $this->startTime, 1); + $this->mockIsLocked('test', true, $this->startTime, 1); + $this->mockReleaseLock('test', $this->pid, 1); + $this->mockIsLocked('test', false, $this->startTime, 1); + + parent::testReleaseWitTTL(); + } } diff --git a/tests/src/Core/Lock/LockTest.php b/tests/src/Core/Lock/LockTest.php index 6dc170e514..28f51733f4 100644 --- a/tests/src/Core/Lock/LockTest.php +++ b/tests/src/Core/Lock/LockTest.php @@ -87,7 +87,7 @@ abstract class LockTest extends MockedTest $this->assertTrue($this->instance->isLocked('bar')); $this->assertTrue($this->instance->isLocked('nice')); - $this->instance->releaseAll(); + $this->assertTrue($this->instance->releaseAll()); $this->assertFalse($this->instance->isLocked('foo')); $this->assertFalse($this->instance->isLocked('bar')); @@ -105,18 +105,30 @@ abstract class LockTest extends MockedTest $this->assertTrue($this->instance->acquireLock('bar', 1)); $this->assertTrue($this->instance->acquireLock('nice', 1)); - $this->instance->releaseLock('foo'); + $this->assertTrue($this->instance->releaseLock('foo')); $this->assertFalse($this->instance->isLocked('foo')); $this->assertTrue($this->instance->isLocked('bar')); $this->assertTrue($this->instance->isLocked('nice')); - $this->instance->releaseAll(); + $this->assertTrue($this->instance->releaseAll()); $this->assertFalse($this->instance->isLocked('bar')); $this->assertFalse($this->instance->isLocked('nice')); } + /** + * @small + */ + public function testReleaseWitTTL() + { + $this->assertFalse($this->instance->isLocked('test')); + $this->assertTrue($this->instance->acquireLock('test', 1, 10)); + $this->assertTrue($this->instance->isLocked('test')); + $this->assertTrue($this->instance->releaseLock('test')); + $this->assertFalse($this->instance->isLocked('test')); + } + /** * @medium */