From 4aab5d95813f9d813d15339903c9737768a7b82c Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Thu, 3 Apr 2025 11:50:09 +0200 Subject: [PATCH 01/17] Ad getter for lock key --- src/Lock.php | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Lock.php b/src/Lock.php index 4c2b6f2..cd73a7d 100644 --- a/src/Lock.php +++ b/src/Lock.php @@ -182,9 +182,9 @@ public static function setDelayPerUnavailableRetry(int $delayPerRetry): void /** * Unique key for the resource * - * @var string|null + * @var string */ - protected ?string $key = null; + protected string $key; /** * Timeout time of the lock @@ -414,6 +414,15 @@ public function break(): bool return true; } + /** + * Get the unique key for the resource + * @return string + */ + public function getKey(): string + { + return $this->key; + } + /** * Generate the lock object * @@ -671,4 +680,4 @@ public function isLockedByOtherExclusively(): bool } return false; } -} \ No newline at end of file +} From 9810e16efb5dd2880d82e2ccac2ae5bd4a58ae3b Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Thu, 3 Apr 2025 11:54:01 +0200 Subject: [PATCH 02/17] Add test for getKey --- test/LockTest.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/LockTest.php b/test/LockTest.php index 2dd2eab..0ead660 100644 --- a/test/LockTest.php +++ b/test/LockTest.php @@ -49,6 +49,12 @@ public function testSetIdentifier(): void $this->assertEquals("identifier", $lock->getIdentifier()); } + public function testGetkey(): void + { + $lock = new Lock("key"); + $this->assertEquals("key", $lock->getKey()); + } + /** * @throws InvalidResponseStatusCodeException * @throws TooManySaveRetriesException @@ -512,4 +518,4 @@ public function removeLock(): bool { return parent::removeLock(); } -} \ No newline at end of file +} From f82ab1e9c6b644caca54b8c166761fd457c87016 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Thu, 3 Apr 2025 12:06:03 +0200 Subject: [PATCH 03/17] Remove support for EOL php and etcd versions, update gh actions --- .github/workflows/tests.yaml | 8 ++++---- composer.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 871ae71..a05fcbd 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -12,14 +12,14 @@ jobs: strategy: matrix: - php-version: [ '8.1', '8.2', '8.3', '8.4' ] - etcd-version: [ '3.3.27', '3.4.18', '3.5.2' ] + php-version: [ '8.3', '8.4' ] + etcd-version: [ '3.4.36', '3.5.21' ] name: Run tests on PHP v${{ matrix.php-version }} with etcd v${{ matrix.etcd-version }} steps: - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Setup PHP uses: shivammathur/setup-php@v2 @@ -47,7 +47,7 @@ jobs: run: echo "::set-output name=dir::$(composer config cache-files-dir)" - name: Restore composer from cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ${{ steps.composer-cache.outputs.dir }} key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} diff --git a/composer.json b/composer.json index 42a7fbf..cfd6f4d 100644 --- a/composer.json +++ b/composer.json @@ -12,7 +12,7 @@ "require": { "aternos/etcd": "^1.5.0", "ext-json": "*", - "php": ">=8.1" + "php": ">=8.3" }, "require-dev": { "phpunit/phpunit": "^10.5" From 97504b900bd0218a44a165dede9b76a537047818 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Thu, 3 Apr 2025 12:21:11 +0200 Subject: [PATCH 04/17] Remove EtcdLock and apply breaking changes to lock interface --- phpunit.xml | 5 +- src/EtcdLock.php | 33 ------------- src/Lock.php | 2 +- src/LockInterface.php | 17 +++++-- test/LockTest.php | 105 ++++++++++++++++++++++++------------------ 5 files changed, 75 insertions(+), 87 deletions(-) delete mode 100644 src/EtcdLock.php diff --git a/phpunit.xml b/phpunit.xml index e68aeb1..cc1b233 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,5 +1,6 @@ - + test/ @@ -11,4 +12,4 @@ src - \ No newline at end of file + diff --git a/src/EtcdLock.php b/src/EtcdLock.php deleted file mode 100644 index 17b735f..0000000 --- a/src/EtcdLock.php +++ /dev/null @@ -1,33 +0,0 @@ -lock($exclusive, $time, $wait, $identifier) - * It mainly exists for backwards compatibility reasons. - * - * @param string $key Can be anything, should describe the resource in a unique way - * @param bool $exclusive Should the lock be exclusive (true) or shared (false) - * @param int $time Time until the lock should be released automatically - * @param int $wait Time to wait for an existing lock to get released - * @param string|null $identifier An identifier (if different from Lock::$defaultIdentifier, see Lock::setDefaultIdentifier()) - * @throws InvalidResponseStatusCodeException - * @throws TooManySaveRetriesException - */ - public function __construct(string $key, bool $exclusive = false, int $time = 120, int $wait = 300, ?string $identifier = null) - { - parent::__construct($key); - $this->lock($exclusive, $time, $wait, $identifier); - } -} \ No newline at end of file diff --git a/src/Lock.php b/src/Lock.php index cd73a7d..c125541 100644 --- a/src/Lock.php +++ b/src/Lock.php @@ -14,7 +14,7 @@ * * @package Aternos\Lock */ -class Lock +class Lock implements LockInterface { /** * see Lock::setClient() diff --git a/src/LockInterface.php b/src/LockInterface.php index 9beaba2..a39094a 100644 --- a/src/LockInterface.php +++ b/src/LockInterface.php @@ -13,12 +13,19 @@ interface LockInterface * LockInterface constructor. * * @param string $key - * @param bool $exclusive - * @param int $time - * @param int $wait * @param string|null $identifier */ - public function __construct(string $key, bool $exclusive = false, int $time = 60, int $wait = 300, ?string $identifier = null); + public function __construct(string $key, ?string $identifier = null); + + /** + * Try to acquire lock + * + * @param bool $exclusive true for exclusive lock, false for shared lock + * @param int $time duration in seconds for which the lock should be held + * @param int $wait duration in seconds to wait for existing locks to be released + * @return bool true if lock was acquired, false otherwise + */ + public function lock(bool $exclusive = false, int $time = 120, int $wait = 300): bool; /** * Check if is locked and returns time until lock runs out or false @@ -44,4 +51,4 @@ public function refresh(int $time = 60, int $remainingThreshold = 30): bool; * @return bool */ public function break(): bool; -} \ No newline at end of file +} diff --git a/test/LockTest.php b/test/LockTest.php index 0ead660..e125218 100644 --- a/test/LockTest.php +++ b/test/LockTest.php @@ -3,10 +3,10 @@ namespace Aternos\Lock\Test; use Aternos\Etcd\Exception\Status\InvalidResponseStatusCodeException; -use Aternos\Lock\EtcdLock; use Aternos\Lock\Lock; use Aternos\Lock\TooManySaveRetriesException; use PHPUnit\Framework\TestCase; +use ReflectionProperty; class LockTest extends TestCase { @@ -55,21 +55,6 @@ public function testGetkey(): void $this->assertEquals("key", $lock->getKey()); } - /** - * @throws InvalidResponseStatusCodeException - * @throws TooManySaveRetriesException - */ - public function testCreateEtcdLock(): void - { - $key = $this->getRandomString(); - $identifier = $this->getRandomString(); - - $lock = new EtcdLock($key, false, 10, 0, $identifier); - $this->assertTrue($lock->isLocked() >= 8); - - $lock->break(); - } - /** * @throws TooManySaveRetriesException * @throws InvalidResponseStatusCodeException @@ -79,7 +64,8 @@ public function testBreakLock(): void $key = $this->getRandomString(); $identifier = $this->getRandomString(); - $lock = new EtcdLock($key, false, 10, 0, $identifier); + $lock = new Lock($key, $identifier); + $lock->lock(false, 10, 0); $this->assertTrue($lock->isLocked() > 0); $lock->break(); $this->assertFalse($lock->isLocked()); @@ -94,7 +80,8 @@ public function testAutoReleaseLock(): void $key = $this->getRandomString(); $identifier = $this->getRandomString(); - $lock = new EtcdLock($key, false, 3, 0, $identifier); + $lock = new Lock($key, $identifier); + $lock->lock(false, 3, 0); $this->assertTrue($lock->isLocked() > 0); sleep(3); $this->assertFalse($lock->isLocked()); @@ -111,7 +98,8 @@ public function testRefreshLock(): void $key = $this->getRandomString(); $identifier = $this->getRandomString(); - $lock = new EtcdLock($key, false, 3, 0, $identifier); + $lock = new Lock($key, $identifier); + $lock->lock(false, 3, 0); $this->assertTrue($lock->isLocked() > 0); sleep(1); $lock->refresh(5); @@ -131,7 +119,8 @@ public function testRefreshLockThreshold(): void $key = $this->getRandomString(); $identifier = $this->getRandomString(); - $lock = new EtcdLock($key, false, 10, 0, $identifier); + $lock = new Lock($key, $identifier); + $lock->lock(false, 10, 0); $this->assertTrue($lock->isLocked() > 0); sleep(3); $lock->refresh(10, 5); @@ -155,9 +144,13 @@ public function testMultipleSharedLocks(): void $identifierB = $this->getRandomString(); $identifierC = $this->getRandomString(); - $lockA = new EtcdLock($key, false, 3, 0, $identifierA); - $lockB = new EtcdLock($key, false, 3, 0, $identifierB); - $lockC = new EtcdLock($key, false, 3, 0, $identifierC); + $lockA = new Lock($key, $identifierA); + $lockB = new Lock($key, $identifierB); + $lockC = new Lock($key, $identifierC); + + $lockA->lock(false, 3, 0); + $lockB->lock(false, 3, 0); + $lockC->lock(false, 3, 0); $this->assertTrue($lockA->isLocked() > 0); $this->assertTrue($lockB->isLocked() > 0); @@ -188,7 +181,8 @@ public function testExclusiveLock(): void $key = $this->getRandomString(); $identifier = $this->getRandomString(); - $lock = new EtcdLock($key, true, 10, 0, $identifier); + $lock = new Lock($key, $identifier); + $lock->lock(true, 10, 0); $this->assertTrue($lock->isLocked() > 0); $lock->break(); $this->assertFalse($lock->isLocked()); @@ -206,9 +200,12 @@ public function testWaitForExclusiveLockAfterExclusiveLock(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new EtcdLock($key, true, 3, 0, $identifierA); + $lockA = new Lock($key, $identifierA); + $lockA->lock(true, 3, 0); $this->assertTrue($lockA->isLocked() > 0); - $lockB = new EtcdLock($key, true, 3, 5, $identifierB); + + $lockB = new Lock($key, $identifierB); + $lockB->lock(true, 3, 5); $this->assertTrue($lockB->isLocked() > 0); $lockA->break(); @@ -225,9 +222,11 @@ public function testRejectSharedLockWhileExclusiveLock(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new EtcdLock($key, true, 3, 0, $identifierA); + $lockA = new Lock($key, $identifierA); + $lockA->lock(true, 3, 0); $this->assertTrue($lockA->isLocked() > 0); - $lockB = new EtcdLock($key, false, 3, 0, $identifierB); + $lockB = new Lock($key, $identifierB); + $lockB->lock(false, 3, 0); $this->assertFalse($lockB->isLocked()); $lockA->break(); @@ -244,9 +243,11 @@ public function testRejectExclusiveLockWhileSharedLock(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new EtcdLock($key, false, 3, 0, $identifierA); + $lockA = new Lock($key, $identifierA); + $lockA->lock(false, 3, 0); $this->assertTrue($lockA->isLocked() > 0); - $lockB = new EtcdLock($key, true, 3, 0, $identifierB); + $lockB = new Lock($key, $identifierB); + $lockB->lock(true, 3, 0); $this->assertFalse($lockB->isLocked()); $lockA->break(); @@ -265,16 +266,21 @@ public function testWaitForExclusiveLockAfterMultipleSharedLocks(): void $identifierC = $this->getRandomString(); $identifierD = $this->getRandomString(); - $lockA = new EtcdLock($key, false, 3, 0, $identifierA); - $lockB = new EtcdLock($key, false, 5, 0, $identifierB); - $lockC = new EtcdLock($key, false, 8, 0, $identifierC); + $lockA = new Lock($key, $identifierA); + $lockB = new Lock($key, $identifierB); + $lockC = new Lock($key, $identifierC); + + $lockA->lock(false, 3, 0); + $lockB->lock(false, 5, 0); + $lockC->lock(false, 8, 0); $this->assertTrue($lockA->isLocked() > 0); $this->assertTrue($lockB->isLocked() > 0); $this->assertTrue($lockC->isLocked() > 0); $time = time(); - $lockD = new EtcdLock($key, true, 3, 10, $identifierD); + $lockD = new Lock($key, $identifierD); + $lockD->lock(true, 3, 10); $this->assertTrue(time() - $time >= 7); $lockA->break(); @@ -293,8 +299,10 @@ public function testLockWriteConflict(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new PublicLock($key, false, 5, 0, $identifierA); - $lockB = new PublicLock($key, false, 5, 0, $identifierB); + $lockA = new PublicLock($key, $identifierA); + $lockA->lock(false, 5, 0); + $lockB = new PublicLock($key, $identifierB); + $lockB->lock(false, 5, 0); $this->assertTrue($lockA->isLocked() > 0); $this->assertTrue($lockB->isLocked() > 0); @@ -326,8 +334,10 @@ public function testLockDeleteConflict(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new PublicLock($key, false, 5, 0, $identifierA); - $lockB = new PublicLock($key, false, 5, 0, $identifierB); + $lockA = new PublicLock($key, $identifierA); + $lockA->lock(false, 5, 0); + $lockB = new PublicLock($key, $identifierB); + $lockB->lock(false, 5, 0); $this->assertTrue($lockA->isLocked() > 0); $this->assertTrue($lockB->isLocked() > 0); @@ -354,9 +364,8 @@ public function testLockFunctionUsesUniqueDefaultIdentifierIfNoIdentifierParamet { # Default identifier is not set explicitly and its default value is null. # (We have to set it to null manually because other tests might have set the default identifier before.) - $reflection = new \ReflectionProperty(Lock::class, 'defaultIdentifier'); - $reflection->setAccessible(true); - $reflection->setValue(null); + $reflection = new ReflectionProperty(Lock::class, 'defaultIdentifier'); + $reflection->setValue(null, null); $this->assertNull($reflection->getValue()); $lock = new Lock($this->getRandomString()); @@ -441,10 +450,12 @@ public function testCanLockWithSameKeyTwiceWhenIsNotExclusive(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new PublicLock($key, false, 5, 0, $identifierA); + $lockA = new PublicLock($key, $identifierA); + $lockA->lock(false, 5, 0); $this->assertFalse($lockA->isLockedByOther()); - $lockB = new PublicLock($key, false, 5, 0, $identifierB); + $lockB = new PublicLock($key, $identifierB); + $lockB->lock(false, 5, 0); # is not locked by other because it was not updated $this->assertFalse($lockA->isLockedByOther()); # is locked by other because it was updated @@ -474,11 +485,13 @@ public function testCannotLockWithSameKeyTwiceWhenIsExclusive(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new PublicLock($key, true, 999, 0, $identifierA); + $lockA = new PublicLock($key, $identifierA); + $lockA->lock(true, 999, 0); $this->assertNotFalse($lockA->isLocked()); $this->assertFalse($lockA->isLockedByOtherExclusively()); - $lockB = new PublicLock($key, true, 999, 0, $identifierB); + $lockB = new PublicLock($key, $identifierB); + $lockB->lock(true, 999, 0); # is not locked because lockA already got the lock $this->assertFalse($lockB->isLocked()); # is locked by other because it was updated @@ -507,7 +520,7 @@ public function testCannotLockWithSameKeyTwiceWhenIsExclusive(): void * * Make some functions public for testing purposes */ -class PublicLock extends EtcdLock +class PublicLock extends Lock { public function addOrUpdateLock(): bool { From df68454fe67646ad808c0c8609bfe3204355f436 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Thu, 3 Apr 2025 12:26:33 +0200 Subject: [PATCH 05/17] Remove identifier parameter from Lock->lock() --- src/Lock.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Lock.php b/src/Lock.php index c125541..371a39a 100644 --- a/src/Lock.php +++ b/src/Lock.php @@ -268,20 +268,15 @@ public function __construct(string $key, ?string $identifier = null) * @param bool $exclusive * @param int $time * @param int $wait - * @param string|null $identifier * @return bool * @throws InvalidResponseStatusCodeException * @throws TooManySaveRetriesException */ - public function lock(bool $exclusive = false, int $time = 120, int $wait = 300, ?string $identifier = null): bool + public function lock(bool $exclusive = false, int $time = 120, int $wait = 300): bool { $this->exclusive = $exclusive; $this->time = $time; - if ($identifier !== null) { - $this->identifier = $identifier; - } - $this->retries = 0; do { From 8ee0dcd4e241dcb037ba71d4f5aa9fe284e79c41 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Thu, 3 Apr 2025 17:42:52 +0200 Subject: [PATCH 06/17] Make setBreakOnDestruct, setIdentifier and update fluent methods --- src/Lock.php | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/Lock.php b/src/Lock.php index 371a39a..b962e68 100644 --- a/src/Lock.php +++ b/src/Lock.php @@ -331,15 +331,16 @@ public function isLocked(): bool|int * Set the identifier for this lock, falls back to the default identifier if null * * @param string|null $identifier - * @return void + * @return $this */ - public function setIdentifier(?string $identifier): void + public function setIdentifier(?string $identifier): static { if ($identifier === null) { $this->identifier = static::$defaultIdentifier; } else { $this->identifier = $identifier; } + return $this; } /** @@ -356,10 +357,12 @@ public function getIdentifier(): ?string * Dis/enable automatic lock break on object destruct * * @param bool $breakOnDestruct + * @return $this */ - public function setBreakOnDestruct(bool $breakOnDestruct): void + public function setBreakOnDestruct(bool $breakOnDestruct): static { $this->breakOnDestruct = $breakOnDestruct; + return $this; } /** @@ -586,8 +589,9 @@ protected function canLock(): bool * Update the locks array from etcd * * @throws InvalidResponseStatusCodeException + * @return $this */ - public function update(): void + public function update(): static { $etcdLockString = false; for ($i = 1; $i <= static::$maxUnavailableRetries; $i++) { @@ -604,15 +608,16 @@ public function update(): void } } - $this->updateFromString($etcdLockString); + return $this->updateFromString($etcdLockString); } /** * Update the locks array from a JSON string * * @param string|bool $lockString + * @return $this */ - protected function updateFromString(string|bool $lockString): void + protected function updateFromString(string|bool $lockString): static { $this->previousLockString = $lockString; @@ -621,6 +626,8 @@ protected function updateFromString(string|bool $lockString): void } else { $this->locks = []; } + + return $this; } /** From a20afacfb661d85799cb23aa4b1a6ce700a28823 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Thu, 3 Apr 2025 18:09:32 +0200 Subject: [PATCH 07/17] WIP: Replace parameters for `lock`, `waitForOtherLocks` and `refresh` with properties --- src/Lock.php | 192 +++++++++++++++++++++++++++++++++--------- src/LockInterface.php | 9 +- 2 files changed, 156 insertions(+), 45 deletions(-) diff --git a/src/Lock.php b/src/Lock.php index b962e68..c5b5d01 100644 --- a/src/Lock.php +++ b/src/Lock.php @@ -187,13 +187,16 @@ public static function setDelayPerUnavailableRetry(int $delayPerRetry): void protected string $key; /** - * Timeout time of the lock - * - * The lock will be released if this timeout is reached - * - * @var int|null + * Timeout time of the lock in seconds. The lock will be released if this timeout is reached. + * @var int + */ + protected int $time = 120; + + /** + * Time in seconds to wait for existing locks to be released. + * @var int */ - protected ?int $time = null; + protected int $waitTime = 300; /** * Is this an exclusive lock (true) or shared (false) @@ -202,6 +205,20 @@ public static function setDelayPerUnavailableRetry(int $delayPerRetry): void */ protected bool $exclusive = false; + /** + * Duration in seconds the timeout should be set to when refreshing the lock. + * If null the initial timeout will be used. + * @var int|null + */ + protected ?int $refreshTime = null; + + /** + * Maximum duration in seconds the existing lock may be valid for to be refreshed. If the lock is valid for longer + * than this time, the lock will not be refreshed. + * @var int + */ + protected int $refreshThreshold = 30; + /** * Full name of the key in etcd (prefix + key) * @@ -265,25 +282,19 @@ public function __construct(string $key, ?string $identifier = null) /** * Try to acquire lock * - * @param bool $exclusive - * @param int $time - * @param int $wait - * @return bool + * @return bool true if the lock was acquired, false if it was not * @throws InvalidResponseStatusCodeException * @throws TooManySaveRetriesException */ - public function lock(bool $exclusive = false, int $time = 120, int $wait = 300): bool + public function lock(): bool { - $this->exclusive = $exclusive; - $this->time = $time; - $this->retries = 0; do { - $this->waitForOtherLocks($wait, $exclusive); + $this->waitForOtherLocks(); $retry = false; if ($this->canLock()) { - $retry = !$this->addOrUpdateLock(); + $retry = !$this->addOrUpdateLock($this->time); } } while ($retry); @@ -291,18 +302,21 @@ public function lock(bool $exclusive = false, int $time = 120, int $wait = 300): } /** - * @param int $wait - * @param bool $exclusive + * Wait for other locks to be released. This method will only wait if the timeout of the existing lock ends within + * the specified wait time. If the timeout of the existing lock ends after the wait time, this method will return + * immediately, even though the lock holder might voluntarily break the lock before the timeout. + * + * @param int|null $waitTime maximum time in seconds to wait for other locks * @return bool * @throws InvalidResponseStatusCodeException */ - public function waitForOtherLocks(int $wait = 300, bool $exclusive = false): bool + public function waitForOtherLocks(?int $waitTime = null): bool { + $waitTime ??= $this->waitTime; $startTime = time(); - $this->exclusive = $exclusive; $this->update(); - while (!$this->canLock() && $startTime + $wait > time()) { + while (!$this->canLock() && $startTime + $waitTime > time()) { sleep(static::$waitRetryInterval); $this->update(); } @@ -327,6 +341,15 @@ public function isLocked(): bool|int return false; } + /** + * Get the unique key for the resource + * @return string + */ + public function getKey(): string + { + return $this->key; + } + /** * Set the identifier for this lock, falls back to the default identifier if null * @@ -365,23 +388,124 @@ public function setBreakOnDestruct(bool $breakOnDestruct): static return $this; } + /** + * Set the timeout time of the lock. The lock will be released if this timeout is reached. + * @param int $time time in seconds + * @return $this + */ + public function setTime(int $time): static + { + $this->time = $time; + return $this; + } + + /** + * Get the timeout time of the lock in seconds. The lock will be released if this timeout is reached. + * @return int + */ + public function getTime(): int + { + return $this->time; + } + + /** + * Is this lock exclusive (true) or shared (false) + * @return bool + */ + public function isExclusive(): bool + { + return $this->exclusive; + } + + /** + * Make this lock exclusive (true) or shared (false) + * @param bool $exclusive + * @return $this + */ + public function setExclusive(bool $exclusive): static + { + $this->exclusive = $exclusive; + return $this; + } + + /** + * Get the wait time in seconds to wait for existing locks to be released. + * @return int + */ + public function getWaitTime(): int + { + return $this->waitTime; + } + + /** + * Set the wait time in seconds to wait for existing locks to be released. + * @param int $waitTime + * @return $this + */ + public function setWaitTime(int $waitTime): static + { + $this->waitTime = $waitTime; + return $this; + } + + /** + * Duration in seconds the timeout should be set to when refreshing the lock. + * If null the initial timeout will be used. + * @return int|null + */ + public function getRefreshTime(): ?int + { + return $this->refreshTime; + } + + /** + * Duration in seconds the timeout should be set to when refreshing the lock. + * If null the initial timeout will be used. + * @param int|null $refreshTime + * @return $this + */ + public function setRefreshTime(?int $refreshTime): static + { + $this->refreshTime = $refreshTime; + return $this; + } + + /** + * Maximum duration in seconds the existing lock may be valid for to be refreshed. If the lock is valid for longer + * than this time, the lock will not be refreshed. + * @return int + */ + public function getRefreshThreshold(): int + { + return $this->refreshThreshold; + } + + /** + * Maximum duration in seconds the existing lock may be valid for to be refreshed. If the lock is valid for longer + * than this time, the lock will not be refreshed. + * @param int $refreshThreshold + * @return $this + */ + public function setRefreshThreshold(int $refreshThreshold): static + { + $this->refreshThreshold = $refreshThreshold; + return $this; + } + /** * Refresh the lock * - * @param int $time Time until the lock should be released automatically - * @param int $remainingThreshold The lock will only be refreshed if the remaining time is below this threshold (0 to disable) * @return boolean * @throws InvalidResponseStatusCodeException * @throws TooManySaveRetriesException */ - public function refresh(int $time = 60, int $remainingThreshold = 30): bool + public function refresh(): bool { - if ($remainingThreshold > 0 && $this->isLocked() > $remainingThreshold) { + if ($this->refreshThreshold > 0 && $this->isLocked() > $this->refreshThreshold) { return true; } $this->update(); - $this->time = $time; $this->retries = 0; do { @@ -389,7 +513,7 @@ public function refresh(int $time = 60, int $remainingThreshold = 30): bool return false; } - $retry = !$this->addOrUpdateLock(); + $retry = !$this->addOrUpdateLock($this->refreshTime ?? $this->time); } while ($retry); return true; } @@ -412,15 +536,6 @@ public function break(): bool return true; } - /** - * Get the unique key for the resource - * @return string - */ - public function getKey(): string - { - return $this->key; - } - /** * Generate the lock object * @@ -456,15 +571,16 @@ protected function removeLock(): bool * * A 'false' return value can/should be retried, see Lock::saveLocks() * + * @param int $time * @return bool * @throws InvalidResponseStatusCodeException * @throws TooManySaveRetriesException */ - protected function addOrUpdateLock(): bool + protected function addOrUpdateLock(int $time): bool { foreach ($this->locks as $lock) { if ($lock->isBy($this->identifier)) { - $lock->setRemaining($this->time); + $lock->setRemaining($time); return $this->saveLocks(); } } diff --git a/src/LockInterface.php b/src/LockInterface.php index a39094a..a0de1bf 100644 --- a/src/LockInterface.php +++ b/src/LockInterface.php @@ -20,12 +20,9 @@ public function __construct(string $key, ?string $identifier = null); /** * Try to acquire lock * - * @param bool $exclusive true for exclusive lock, false for shared lock - * @param int $time duration in seconds for which the lock should be held - * @param int $wait duration in seconds to wait for existing locks to be released * @return bool true if lock was acquired, false otherwise */ - public function lock(bool $exclusive = false, int $time = 120, int $wait = 300): bool; + public function lock(): bool; /** * Check if is locked and returns time until lock runs out or false @@ -37,11 +34,9 @@ public function isLocked(): bool|int; /** * Refresh the lock * - * @param int $time - * @param int $remainingThreshold * @return bool */ - public function refresh(int $time = 60, int $remainingThreshold = 30): bool; + public function refresh(): bool; /** * Break the lock From b1ce9d9e6e9fbb921ae017e20480289e98002998 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Fri, 4 Apr 2025 11:52:02 +0200 Subject: [PATCH 08/17] Make Lock->$identifier non-nullable --- src/Lock.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Lock.php b/src/Lock.php index c5b5d01..07e96f8 100644 --- a/src/Lock.php +++ b/src/Lock.php @@ -175,9 +175,9 @@ public static function setDelayPerUnavailableRetry(int $delayPerRetry): void * * Probably the same as Lock::$defaultIdentifier if not overwritten in Lock::__construct() * - * @var string|null + * @var string */ - protected ?string $identifier = null; + protected string $identifier; /** * Unique key for the resource From 73ea9f8c649f091df3f2e27bce6c0796d76bf9e4 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Fri, 4 Apr 2025 11:57:23 +0200 Subject: [PATCH 09/17] Allow initializing most properties from the constructor --- src/Lock.php | 75 ++++++++++++++-------------------------------------- 1 file changed, 20 insertions(+), 55 deletions(-) diff --git a/src/Lock.php b/src/Lock.php index 07e96f8..f21c1c1 100644 --- a/src/Lock.php +++ b/src/Lock.php @@ -179,46 +179,6 @@ public static function setDelayPerUnavailableRetry(int $delayPerRetry): void */ protected string $identifier; - /** - * Unique key for the resource - * - * @var string - */ - protected string $key; - - /** - * Timeout time of the lock in seconds. The lock will be released if this timeout is reached. - * @var int - */ - protected int $time = 120; - - /** - * Time in seconds to wait for existing locks to be released. - * @var int - */ - protected int $waitTime = 300; - - /** - * Is this an exclusive lock (true) or shared (false) - * - * @var bool - */ - protected bool $exclusive = false; - - /** - * Duration in seconds the timeout should be set to when refreshing the lock. - * If null the initial timeout will be used. - * @var int|null - */ - protected ?int $refreshTime = null; - - /** - * Maximum duration in seconds the existing lock may be valid for to be refreshed. If the lock is valid for longer - * than this time, the lock will not be refreshed. - * @var int - */ - protected int $refreshThreshold = 30; - /** * Full name of the key in etcd (prefix + key) * @@ -250,33 +210,38 @@ public static function setDelayPerUnavailableRetry(int $delayPerRetry): void */ protected int $retries = 0; - /** - * Automatically try to break the lock on destruct if possible - * - * @var bool - */ - protected bool $breakOnDestruct = true; - /** * Create a lock * * @param string $key Can be anything, should describe the resource in a unique way * @param string|null $identifier An identifier for this lock, falls back to the default identifier if null - */ - public function __construct(string $key, ?string $identifier = null) + * @param int $time Timeout time of the lock in seconds. The lock will be released if this timeout is reached. + * @param bool $exclusive Is this lock exclusive (true) or shared (false) + * @param int $waitTime Time in seconds to wait for existing locks to be released. + * @param int|null $refreshTime Duration in seconds the timeout should be set to when refreshing the lock. + * If null the initial timeout will be used. + * @param int $refreshThreshold Maximum duration in seconds the existing lock may be valid for to be refreshed. + * If the lock is valid for longer than this time, the lock will not be refreshed. + * @param bool $breakOnDestruct Automatically try to break the lock on destruct if possible + */ + public function __construct( + protected string $key, + ?string $identifier = null, + protected bool $exclusive = false, + protected int $time = 120, + protected int $waitTime = 300, + protected ?int $refreshTime = null, + protected int $refreshThreshold = 30, + protected bool $breakOnDestruct = true, + ) { - $this->key = $key; $this->etcdKey = static::$prefix . $this->key; if (static::$defaultIdentifier === null) { static::setDefaultIdentifier(); } - if ($identifier === null) { - $this->identifier = static::$defaultIdentifier; - } else { - $this->identifier = $identifier; - } + $this->identifier = $identifier ?? static::$defaultIdentifier; } /** From 48c2fab8d2f84a05db0cf164a0123849fa4c03e9 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Fri, 4 Apr 2025 12:21:43 +0200 Subject: [PATCH 10/17] Update tests to use class properties --- test/LockTest.php | 120 +++++++++++++++++++++++----------------------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/test/LockTest.php b/test/LockTest.php index e125218..6ba2512 100644 --- a/test/LockTest.php +++ b/test/LockTest.php @@ -24,7 +24,7 @@ public function testCreateLock(): void $key = $this->getRandomString(); $identifier = $this->getRandomString(); - $this->assertTrue(($lock = new Lock($key))->lock(false, 10, 0, $identifier)); + $this->assertTrue(($lock = new Lock($key, $identifier, false, 10, 0))->lock()); $this->assertTrue($lock->isLocked() >= 8); $lock->break(); @@ -64,8 +64,8 @@ public function testBreakLock(): void $key = $this->getRandomString(); $identifier = $this->getRandomString(); - $lock = new Lock($key, $identifier); - $lock->lock(false, 10, 0); + $lock = new Lock($key, $identifier, false, 10, 0); + $lock->lock(); $this->assertTrue($lock->isLocked() > 0); $lock->break(); $this->assertFalse($lock->isLocked()); @@ -80,8 +80,8 @@ public function testAutoReleaseLock(): void $key = $this->getRandomString(); $identifier = $this->getRandomString(); - $lock = new Lock($key, $identifier); - $lock->lock(false, 3, 0); + $lock = new Lock($key, $identifier, false, 3, 0); + $lock->lock(); $this->assertTrue($lock->isLocked() > 0); sleep(3); $this->assertFalse($lock->isLocked()); @@ -98,11 +98,11 @@ public function testRefreshLock(): void $key = $this->getRandomString(); $identifier = $this->getRandomString(); - $lock = new Lock($key, $identifier); - $lock->lock(false, 3, 0); + $lock = new Lock($key, $identifier, false, 3, 0, 5); + $lock->lock(); $this->assertTrue($lock->isLocked() > 0); sleep(1); - $lock->refresh(5); + $lock->refresh(); $this->assertTrue($lock->isLocked() > 3); sleep(2); $this->assertTrue($lock->isLocked() > 0); @@ -119,15 +119,15 @@ public function testRefreshLockThreshold(): void $key = $this->getRandomString(); $identifier = $this->getRandomString(); - $lock = new Lock($key, $identifier); - $lock->lock(false, 10, 0); + $lock = new Lock($key, $identifier, false, 10, 0, refreshThreshold: 5); + $lock->lock(); $this->assertTrue($lock->isLocked() > 0); sleep(3); - $lock->refresh(10, 5); + $lock->refresh(); $this->assertTrue($lock->isLocked() > 3); $this->assertTrue($lock->isLocked() < 8); sleep(3); - $lock->refresh(10, 5); + $lock->refresh(); $this->assertTrue($lock->isLocked() > 8); $lock->break(); $this->assertFalse($lock->isLocked()); @@ -144,20 +144,20 @@ public function testMultipleSharedLocks(): void $identifierB = $this->getRandomString(); $identifierC = $this->getRandomString(); - $lockA = new Lock($key, $identifierA); - $lockB = new Lock($key, $identifierB); - $lockC = new Lock($key, $identifierC); + $lockA = new Lock($key, $identifierA, false, 3, 0, refreshTime: 5); + $lockB = new Lock($key, $identifierB, false, 3, 0); + $lockC = new Lock($key, $identifierC, false, 3, 0); - $lockA->lock(false, 3, 0); - $lockB->lock(false, 3, 0); - $lockC->lock(false, 3, 0); + $lockA->lock(); + $lockB->lock(); + $lockC->lock(); $this->assertTrue($lockA->isLocked() > 0); $this->assertTrue($lockB->isLocked() > 0); $this->assertTrue($lockC->isLocked() > 0); sleep(1); - $lockA->refresh(5); + $lockA->refresh(); $this->assertTrue($lockA->isLocked() > 3); sleep(2); @@ -181,8 +181,8 @@ public function testExclusiveLock(): void $key = $this->getRandomString(); $identifier = $this->getRandomString(); - $lock = new Lock($key, $identifier); - $lock->lock(true, 10, 0); + $lock = new Lock($key, $identifier, true, 10, 0); + $lock->lock(); $this->assertTrue($lock->isLocked() > 0); $lock->break(); $this->assertFalse($lock->isLocked()); @@ -200,12 +200,12 @@ public function testWaitForExclusiveLockAfterExclusiveLock(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new Lock($key, $identifierA); - $lockA->lock(true, 3, 0); + $lockA = new Lock($key, $identifierA, true, 3, 0); + $lockA->lock(); $this->assertTrue($lockA->isLocked() > 0); - $lockB = new Lock($key, $identifierB); - $lockB->lock(true, 3, 5); + $lockB = new Lock($key, $identifierB, true, 3, 5); + $lockB->lock(); $this->assertTrue($lockB->isLocked() > 0); $lockA->break(); @@ -222,11 +222,11 @@ public function testRejectSharedLockWhileExclusiveLock(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new Lock($key, $identifierA); - $lockA->lock(true, 3, 0); + $lockA = new Lock($key, $identifierA, true, 3, 0); + $lockA->lock(); $this->assertTrue($lockA->isLocked() > 0); - $lockB = new Lock($key, $identifierB); - $lockB->lock(false, 3, 0); + $lockB = new Lock($key, $identifierB, false, 3, 0); + $lockB->lock(); $this->assertFalse($lockB->isLocked()); $lockA->break(); @@ -243,11 +243,11 @@ public function testRejectExclusiveLockWhileSharedLock(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new Lock($key, $identifierA); - $lockA->lock(false, 3, 0); + $lockA = new Lock($key, $identifierA, false, 3, 0); + $lockA->lock(); $this->assertTrue($lockA->isLocked() > 0); - $lockB = new Lock($key, $identifierB); - $lockB->lock(true, 3, 0); + $lockB = new Lock($key, $identifierB, true, 3, 0); + $lockB->lock(); $this->assertFalse($lockB->isLocked()); $lockA->break(); @@ -266,21 +266,21 @@ public function testWaitForExclusiveLockAfterMultipleSharedLocks(): void $identifierC = $this->getRandomString(); $identifierD = $this->getRandomString(); - $lockA = new Lock($key, $identifierA); - $lockB = new Lock($key, $identifierB); - $lockC = new Lock($key, $identifierC); + $lockA = new Lock($key, $identifierA, false, 3, 0); + $lockB = new Lock($key, $identifierB, false, 5, 0); + $lockC = new Lock($key, $identifierC, false, 8, 0); - $lockA->lock(false, 3, 0); - $lockB->lock(false, 5, 0); - $lockC->lock(false, 8, 0); + $lockA->lock(); + $lockB->lock(); + $lockC->lock(); $this->assertTrue($lockA->isLocked() > 0); $this->assertTrue($lockB->isLocked() > 0); $this->assertTrue($lockC->isLocked() > 0); $time = time(); - $lockD = new Lock($key, $identifierD); - $lockD->lock(true, 3, 10); + $lockD = new Lock($key, $identifierD, true, 3, 10); + $lockD->lock(); $this->assertTrue(time() - $time >= 7); $lockA->break(); @@ -299,15 +299,15 @@ public function testLockWriteConflict(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new PublicLock($key, $identifierA); - $lockA->lock(false, 5, 0); - $lockB = new PublicLock($key, $identifierB); - $lockB->lock(false, 5, 0); + $lockA = new PublicLock($key, $identifierA, false, 5, 0); + $lockA->lock(); + $lockB = new PublicLock($key, $identifierB, false, 5, 0); + $lockB->lock(); $this->assertTrue($lockA->isLocked() > 0); $this->assertTrue($lockB->isLocked() > 0); - $lockA->addOrUpdateLock(); + $lockA->addOrUpdateLock($lockA->getTime()); $this->assertTrue($lockA->isLocked() > 0); $this->assertTrue($lockB->isLocked() > 0); @@ -334,10 +334,10 @@ public function testLockDeleteConflict(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new PublicLock($key, $identifierA); - $lockA->lock(false, 5, 0); - $lockB = new PublicLock($key, $identifierB); - $lockB->lock(false, 5, 0); + $lockA = new PublicLock($key, $identifierA, false, 5, 0); + $lockA->lock(); + $lockB = new PublicLock($key, $identifierB, false, 5, 0); + $lockB->lock(); $this->assertTrue($lockA->isLocked() > 0); $this->assertTrue($lockB->isLocked() > 0); @@ -450,12 +450,12 @@ public function testCanLockWithSameKeyTwiceWhenIsNotExclusive(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new PublicLock($key, $identifierA); - $lockA->lock(false, 5, 0); + $lockA = new PublicLock($key, $identifierA, false, 5, 0); + $lockA->lock(); $this->assertFalse($lockA->isLockedByOther()); - $lockB = new PublicLock($key, $identifierB); - $lockB->lock(false, 5, 0); + $lockB = new PublicLock($key, $identifierB, false, 5, 0); + $lockB->lock(); # is not locked by other because it was not updated $this->assertFalse($lockA->isLockedByOther()); # is locked by other because it was updated @@ -485,13 +485,13 @@ public function testCannotLockWithSameKeyTwiceWhenIsExclusive(): void $identifierA = $this->getRandomString(); $identifierB = $this->getRandomString(); - $lockA = new PublicLock($key, $identifierA); - $lockA->lock(true, 999, 0); + $lockA = new PublicLock($key, $identifierA, true, 999, 0); + $lockA->lock(); $this->assertNotFalse($lockA->isLocked()); $this->assertFalse($lockA->isLockedByOtherExclusively()); - $lockB = new PublicLock($key, $identifierB); - $lockB->lock(true, 999, 0); + $lockB = new PublicLock($key, $identifierB, true, 999, 0); + $lockB->lock(); # is not locked because lockA already got the lock $this->assertFalse($lockB->isLocked()); # is locked by other because it was updated @@ -522,9 +522,9 @@ public function testCannotLockWithSameKeyTwiceWhenIsExclusive(): void */ class PublicLock extends Lock { - public function addOrUpdateLock(): bool + public function addOrUpdateLock(int $time): bool { - return parent::addOrUpdateLock(); + return parent::addOrUpdateLock($time); } public function removeLock(): bool From 9b9e2fea68699a579e559b27ed34c2f72e9b4f49 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Fri, 4 Apr 2025 12:35:39 +0200 Subject: [PATCH 11/17] Remove return value of break and removeLock --- src/Lock.php | 6 ++---- src/LockInterface.php | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Lock.php b/src/Lock.php index f21c1c1..5197527 100644 --- a/src/Lock.php +++ b/src/Lock.php @@ -488,17 +488,15 @@ public function refresh(): bool * * Should be only used if you have the lock * - * @return boolean + * @return void * @throws InvalidResponseStatusCodeException * @throws TooManySaveRetriesException */ - public function break(): bool + public function break(): void { $this->update(); $this->retries = 0; $this->removeLock(); - - return true; } /** diff --git a/src/LockInterface.php b/src/LockInterface.php index a0de1bf..ed52194 100644 --- a/src/LockInterface.php +++ b/src/LockInterface.php @@ -43,7 +43,7 @@ public function refresh(): bool; * * Should be only used if you have the lock * - * @return bool + * @return void */ - public function break(): bool; + public function break(): void; } From 89a6148902634078b6f322cf41bbfa4ac62d9457 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Fri, 4 Apr 2025 12:36:50 +0200 Subject: [PATCH 12/17] Do not perform any requests if lock is not acquired when calling break() --- src/Lock.php | 11 +++++++---- test/LockTest.php | 8 ++++++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/Lock.php b/src/Lock.php index 5197527..d990586 100644 --- a/src/Lock.php +++ b/src/Lock.php @@ -494,6 +494,10 @@ public function refresh(): bool */ public function break(): void { + if (!$this->isLocked()) { + return; + } + $this->update(); $this->retries = 0; $this->removeLock(); @@ -512,11 +516,11 @@ protected function generateLock(): LockEntry /** * Remove a lock from the locking array and save the locks * - * @return bool + * @return void * @throws InvalidResponseStatusCodeException * @throws TooManySaveRetriesException */ - protected function removeLock(): bool + protected function removeLock(): void { do { foreach ($this->locks as $i => $lock) { @@ -526,7 +530,6 @@ protected function removeLock(): bool } $success = $this->saveLocks(); } while ($success === false); - return true; } /** @@ -717,7 +720,7 @@ protected function updateFromString(string|bool $lockString): static */ public function __destruct() { - if ($this->breakOnDestruct && $this->isLocked()) { + if ($this->breakOnDestruct) { $this->break(); } } diff --git a/test/LockTest.php b/test/LockTest.php index 6ba2512..38a4379 100644 --- a/test/LockTest.php +++ b/test/LockTest.php @@ -69,6 +69,10 @@ public function testBreakLock(): void $this->assertTrue($lock->isLocked() > 0); $lock->break(); $this->assertFalse($lock->isLocked()); + + // Check that calling break again does not throw an error + $lock->break(); + $this->assertFalse($lock->isLocked()); } /** @@ -527,8 +531,8 @@ public function addOrUpdateLock(int $time): bool return parent::addOrUpdateLock($time); } - public function removeLock(): bool + public function removeLock(): void { - return parent::removeLock(); + parent::removeLock(); } } From ab1d9347bcb0aae36ddd27def07d942332da8ab5 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Fri, 4 Apr 2025 13:33:41 +0200 Subject: [PATCH 13/17] Improve tests for break only executing once --- test/LockTest.php | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/test/LockTest.php b/test/LockTest.php index 38a4379..2c2bd4b 100644 --- a/test/LockTest.php +++ b/test/LockTest.php @@ -69,10 +69,28 @@ public function testBreakLock(): void $this->assertTrue($lock->isLocked() > 0); $lock->break(); $this->assertFalse($lock->isLocked()); + } - // Check that calling break again does not throw an error - $lock->break(); - $this->assertFalse($lock->isLocked()); + public function testBreakLockTwice(): void + { + # Check that break() and isLocked() are called when breakOnDestruct is set to true + $breakingLock = $this->getMockBuilder(Lock::class) + ->setConstructorArgs([$this->getRandomString()]) + ->onlyMethods(['removeLock', 'isLocked']) + ->getMock(); + $breakingLock->setBreakOnDestruct(true); + $breakingLock + ->expects($this->once()) + ->method('removeLock'); + $breakingLock + ->expects($this->exactly(3)) + ->method('isLocked') + ->willReturn(true, true, false); + + $breakingLock->lock(); + + $breakingLock->break(); + $breakingLock->break(); } /** @@ -407,13 +425,9 @@ public function testBreaksOnDestructWhenBreakOnDestructPropertyIsTrue(): void # Check that break() and isLocked() are called when breakOnDestruct is set to true $breakingLock = $this->getMockBuilder(Lock::class) ->setConstructorArgs([$this->getRandomString()]) - ->onlyMethods(['isLocked', 'break']) + ->onlyMethods(['break']) ->getMock(); $breakingLock->setBreakOnDestruct(true); - $breakingLock - ->expects($this->once()) - ->method('isLocked') - ->willReturn(true); $breakingLock ->expects($this->once()) ->method('break'); From 00424a24fdf0f6280d3408df4c589c6ce32a3fc9 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Fri, 4 Apr 2025 13:35:34 +0200 Subject: [PATCH 14/17] Make Lock->getClient static --- src/Lock.php | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Lock.php b/src/Lock.php index d990586..f6314c8 100644 --- a/src/Lock.php +++ b/src/Lock.php @@ -170,6 +170,20 @@ public static function setDelayPerUnavailableRetry(int $delayPerRetry): void static::$delayPerUnavailableRetry = $delayPerRetry; } + /** + * Get an Aternos\Etcd\Client instance + * + * @return ClientInterface + */ + protected static function getClient(): ClientInterface + { + if (static::$client === null) { + static::$client = new Client(); + } + + return static::$client; + } + /** * Identifier of the current lock * @@ -633,20 +647,6 @@ protected function saveLocks(): bool } } - /** - * Get an Aternos\Etcd\Client instance - * - * @return ClientInterface - */ - protected function getClient(): ClientInterface - { - if (static::$client === null) { - static::$client = new Client(); - } - - return static::$client; - } - /** * Check if it is possible to lock * From 1f913c345f8e4ce375badaf2c6586add43c8b4b7 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Fri, 4 Apr 2025 13:49:29 +0200 Subject: [PATCH 15/17] Split isLocked into isLocked and getRemainingLockDuration --- src/Lock.php | 22 ++++++++++---- src/LockInterface.php | 14 +++++++-- test/LockTest.php | 68 +++++++++++++++++++++---------------------- 3 files changed, 61 insertions(+), 43 deletions(-) diff --git a/src/Lock.php b/src/Lock.php index f6314c8..3aa9458 100644 --- a/src/Lock.php +++ b/src/Lock.php @@ -306,18 +306,28 @@ public function waitForOtherLocks(?int $waitTime = null): bool /** * Check if is locked and returns time until lock runs out or false * - * @return bool|int + * @return bool */ - public function isLocked(): bool|int + public function isLocked(): bool + { + return $this->getRemainingLockDuration() > 0; + } + + /** + * Get the time until the lock runs out. This method will return -1 if the lock is not valid or other negative values + * if the lock has already run out. + * + * @return int + */ + public function getRemainingLockDuration(): int { foreach ($this->locks as $lock) { if ($lock->isBy($this->identifier)) { - $remaining = $lock->getRemainingTime(); - return ($remaining > 0) ? $remaining : false; + return $lock->getRemainingTime(); } } - return false; + return -1; } /** @@ -480,7 +490,7 @@ public function setRefreshThreshold(int $refreshThreshold): static */ public function refresh(): bool { - if ($this->refreshThreshold > 0 && $this->isLocked() > $this->refreshThreshold) { + if ($this->refreshThreshold > 0 && $this->getRemainingLockDuration() > $this->refreshThreshold) { return true; } diff --git a/src/LockInterface.php b/src/LockInterface.php index ed52194..39818cd 100644 --- a/src/LockInterface.php +++ b/src/LockInterface.php @@ -25,11 +25,19 @@ public function __construct(string $key, ?string $identifier = null); public function lock(): bool; /** - * Check if is locked and returns time until lock runs out or false + * Check if is locked * - * @return bool|int + * @return bool + */ + public function isLocked(): bool; + + /** + * * Get the time until the lock runs out. This method will return -1 if the lock is not valid or other negative values + * * if the lock has already run out. + * + * @return int */ - public function isLocked(): bool|int; + public function getRemainingLockDuration(): int; /** * Refresh the lock diff --git a/test/LockTest.php b/test/LockTest.php index 2c2bd4b..48ebf63 100644 --- a/test/LockTest.php +++ b/test/LockTest.php @@ -25,7 +25,7 @@ public function testCreateLock(): void $identifier = $this->getRandomString(); $this->assertTrue(($lock = new Lock($key, $identifier, false, 10, 0))->lock()); - $this->assertTrue($lock->isLocked() >= 8); + $this->assertGreaterThanOrEqual(8, $lock->getRemainingLockDuration()); $lock->break(); } @@ -66,7 +66,7 @@ public function testBreakLock(): void $lock = new Lock($key, $identifier, false, 10, 0); $lock->lock(); - $this->assertTrue($lock->isLocked() > 0); + $this->assertTrue($lock->isLocked()); $lock->break(); $this->assertFalse($lock->isLocked()); } @@ -104,7 +104,7 @@ public function testAutoReleaseLock(): void $lock = new Lock($key, $identifier, false, 3, 0); $lock->lock(); - $this->assertTrue($lock->isLocked() > 0); + $this->assertTrue($lock->isLocked()); sleep(3); $this->assertFalse($lock->isLocked()); @@ -122,12 +122,12 @@ public function testRefreshLock(): void $lock = new Lock($key, $identifier, false, 3, 0, 5); $lock->lock(); - $this->assertTrue($lock->isLocked() > 0); + $this->assertTrue($lock->isLocked()); sleep(1); $lock->refresh(); - $this->assertTrue($lock->isLocked() > 3); + $this->assertGreaterThan(3, $lock->getRemainingLockDuration()); sleep(2); - $this->assertTrue($lock->isLocked() > 0); + $this->assertTrue($lock->isLocked()); $lock->break(); $this->assertFalse($lock->isLocked()); } @@ -143,14 +143,14 @@ public function testRefreshLockThreshold(): void $lock = new Lock($key, $identifier, false, 10, 0, refreshThreshold: 5); $lock->lock(); - $this->assertTrue($lock->isLocked() > 0); + $this->assertTrue($lock->isLocked()); sleep(3); $lock->refresh(); - $this->assertTrue($lock->isLocked() > 3); - $this->assertTrue($lock->isLocked() < 8); + $this->assertGreaterThan(3, $lock->getRemainingLockDuration()); + $this->assertLessThan(8, $lock->getRemainingLockDuration()); sleep(3); $lock->refresh(); - $this->assertTrue($lock->isLocked() > 8); + $this->assertGreaterThan(8, $lock->getRemainingLockDuration()); $lock->break(); $this->assertFalse($lock->isLocked()); } @@ -174,16 +174,16 @@ public function testMultipleSharedLocks(): void $lockB->lock(); $lockC->lock(); - $this->assertTrue($lockA->isLocked() > 0); - $this->assertTrue($lockB->isLocked() > 0); - $this->assertTrue($lockC->isLocked() > 0); + $this->assertTrue($lockA->isLocked()); + $this->assertTrue($lockB->isLocked()); + $this->assertTrue($lockC->isLocked()); sleep(1); $lockA->refresh(); - $this->assertTrue($lockA->isLocked() > 3); + $this->assertGreaterThan(3, $lockA->getRemainingLockDuration()); sleep(2); - $this->assertTrue($lockA->isLocked() > 0); + $this->assertTrue($lockA->isLocked()); $this->assertFalse($lockB->isLocked()); $this->assertFalse($lockC->isLocked()); @@ -205,7 +205,7 @@ public function testExclusiveLock(): void $lock = new Lock($key, $identifier, true, 10, 0); $lock->lock(); - $this->assertTrue($lock->isLocked() > 0); + $this->assertTrue($lock->isLocked()); $lock->break(); $this->assertFalse($lock->isLocked()); @@ -224,11 +224,11 @@ public function testWaitForExclusiveLockAfterExclusiveLock(): void $lockA = new Lock($key, $identifierA, true, 3, 0); $lockA->lock(); - $this->assertTrue($lockA->isLocked() > 0); + $this->assertTrue($lockA->isLocked()); $lockB = new Lock($key, $identifierB, true, 3, 5); $lockB->lock(); - $this->assertTrue($lockB->isLocked() > 0); + $this->assertTrue($lockB->isLocked()); $lockA->break(); $lockB->break(); @@ -246,7 +246,7 @@ public function testRejectSharedLockWhileExclusiveLock(): void $lockA = new Lock($key, $identifierA, true, 3, 0); $lockA->lock(); - $this->assertTrue($lockA->isLocked() > 0); + $this->assertTrue($lockA->isLocked()); $lockB = new Lock($key, $identifierB, false, 3, 0); $lockB->lock(); $this->assertFalse($lockB->isLocked()); @@ -267,7 +267,7 @@ public function testRejectExclusiveLockWhileSharedLock(): void $lockA = new Lock($key, $identifierA, false, 3, 0); $lockA->lock(); - $this->assertTrue($lockA->isLocked() > 0); + $this->assertTrue($lockA->isLocked()); $lockB = new Lock($key, $identifierB, true, 3, 0); $lockB->lock(); $this->assertFalse($lockB->isLocked()); @@ -296,14 +296,14 @@ public function testWaitForExclusiveLockAfterMultipleSharedLocks(): void $lockB->lock(); $lockC->lock(); - $this->assertTrue($lockA->isLocked() > 0); - $this->assertTrue($lockB->isLocked() > 0); - $this->assertTrue($lockC->isLocked() > 0); + $this->assertTrue($lockA->isLocked()); + $this->assertTrue($lockB->isLocked()); + $this->assertTrue($lockC->isLocked()); $time = time(); $lockD = new Lock($key, $identifierD, true, 3, 10); $lockD->lock(); - $this->assertTrue(time() - $time >= 7); + $this->assertGreaterThan(7, time() - $time); $lockA->break(); $lockB->break(); @@ -326,21 +326,21 @@ public function testLockWriteConflict(): void $lockB = new PublicLock($key, $identifierB, false, 5, 0); $lockB->lock(); - $this->assertTrue($lockA->isLocked() > 0); - $this->assertTrue($lockB->isLocked() > 0); + $this->assertTrue($lockA->isLocked()); + $this->assertTrue($lockB->isLocked()); $lockA->addOrUpdateLock($lockA->getTime()); - $this->assertTrue($lockA->isLocked() > 0); - $this->assertTrue($lockB->isLocked() > 0); + $this->assertTrue($lockA->isLocked()); + $this->assertTrue($lockB->isLocked()); $lockB->update(); - $this->assertTrue($lockB->isLocked() > 0); + $this->assertTrue($lockB->isLocked()); $lockA->update(); - $this->assertTrue($lockA->isLocked() > 0); + $this->assertTrue($lockA->isLocked()); $lockA->break(); $lockB->break(); @@ -361,18 +361,18 @@ public function testLockDeleteConflict(): void $lockB = new PublicLock($key, $identifierB, false, 5, 0); $lockB->lock(); - $this->assertTrue($lockA->isLocked() > 0); - $this->assertTrue($lockB->isLocked() > 0); + $this->assertTrue($lockA->isLocked()); + $this->assertTrue($lockB->isLocked()); $lockA->removeLock(); $this->assertFalse($lockA->isLocked()); $lockA->update(); $this->assertFalse($lockA->isLocked()); - $this->assertTrue($lockB->isLocked() > 0); + $this->assertTrue($lockB->isLocked()); $lockB->update(); - $this->assertTrue($lockB->isLocked() > 0); + $this->assertTrue($lockB->isLocked()); $lockA->break(); $lockB->break(); From fae4cf06f9cc845acb90c94946abe8f51f16d331 Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Fri, 4 Apr 2025 14:25:23 +0200 Subject: [PATCH 16/17] Remove unnecessary boolean cast --- src/Lock.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lock.php b/src/Lock.php index 3aa9458..af10155 100644 --- a/src/Lock.php +++ b/src/Lock.php @@ -277,7 +277,7 @@ public function lock(): bool } } while ($retry); - return !!$this->isLocked(); + return $this->isLocked(); } /** From 34f29ebcb00928f81aa2d8b7c1a7a87acfbaf18c Mon Sep 17 00:00:00 2001 From: Julian Vennen Date: Fri, 4 Apr 2025 14:25:36 +0200 Subject: [PATCH 17/17] Remove duplicate stars from comment --- src/LockInterface.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/LockInterface.php b/src/LockInterface.php index 39818cd..6cff504 100644 --- a/src/LockInterface.php +++ b/src/LockInterface.php @@ -32,8 +32,8 @@ public function lock(): bool; public function isLocked(): bool; /** - * * Get the time until the lock runs out. This method will return -1 if the lock is not valid or other negative values - * * if the lock has already run out. + * Get the time until the lock runs out. This method will return -1 if the lock is not valid or other negative values + * if the lock has already run out. * * @return int */