From 218d13a2bc1b3c6f945bdea57d9663aef15c7bc3 Mon Sep 17 00:00:00 2001 From: Alexander Strizhak Date: Tue, 27 May 2025 22:58:39 +0300 Subject: [PATCH 1/5] $initialWaitTtl = 0 (sec) by default; 60 - is too big value --- composer.json | 2 +- src/RoadRunnerStore.php | 10 +++++----- tests/RoadRunnerStoreTest.php | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/composer.json b/composer.json index cb34729..4a824c0 100644 --- a/composer.json +++ b/composer.json @@ -34,7 +34,7 @@ }, "require-dev": { "phpunit/phpunit": "^10.0", - "vimeo/psalm": "^5.9" + "vimeo/psalm": "^5.9 || ^6.0" }, "autoload": { "psr-4": { diff --git a/src/RoadRunnerStore.php b/src/RoadRunnerStore.php index cda3a8c..42781f9 100644 --- a/src/RoadRunnerStore.php +++ b/src/RoadRunnerStore.php @@ -25,15 +25,15 @@ public function __construct( private readonly RR\LockInterface $lock, private readonly TokenGeneratorInterface $tokens = new RandomTokenGenerator(), private readonly float $initialTtl = 300.0, - private readonly float $initialWaitTtl = 60, + private readonly float $initialWaitTtl = 0, ) { \assert($this->initialTtl >= 0); \assert($this->initialWaitTtl >= 0); } - public function withTtl(float $ttl): self + public function withTtl(float $ttl, float $waitTtl = 0): self { - return new self($this->lock, $this->tokens, $ttl, $this->initialWaitTtl); + return new self($this->lock, $this->tokens, $ttl, $waitTtl); } public function save(Key $key): void @@ -46,7 +46,7 @@ public function save(Key $key): void /** @var non-empty-string $resource */ $resource = (string)$key; - $status = $this->lock->lock($resource, $lockId, $this->initialTtl); + $status = $this->lock->lock($resource, $lockId, $this->initialTtl, $this->initialWaitTtl); if (false === $status) { throw new LockConflictedException('RoadRunner. Failed to make lock'); @@ -65,7 +65,7 @@ public function saveRead(Key $key): void /** @var non-empty-string $resource */ $resource = (string)$key; - $status = $this->lock->lockRead($resource, $lockId, $this->initialTtl); + $status = $this->lock->lockRead($resource, $lockId, $this->initialTtl, $this->initialWaitTtl); if (false === $status) { throw new LockConflictedException('RoadRunner. Failed to make read lock'); diff --git a/tests/RoadRunnerStoreTest.php b/tests/RoadRunnerStoreTest.php index 1d74891..c19b79f 100644 --- a/tests/RoadRunnerStoreTest.php +++ b/tests/RoadRunnerStoreTest.php @@ -166,7 +166,7 @@ public function testWaitAndSaveSuccess(): void { $this->rrLock->expects($this->once()) ->method('lock') - ->with('resource-name', 'random-id', 300, 60) + ->with('resource-name', 'random-id', 300, 0) ->willReturn('lock-id'); $store = new RoadRunnerStore($this->rrLock, $this->tokens); @@ -184,7 +184,7 @@ public function testWaitAndSaveFail(): void $this->rrLock->expects($this->once()) ->method('lock') - ->with('resource-name', 'random-id', 300, 60) + ->with('resource-name', 'random-id', 300, 0) ->willReturn(false); $store = new RoadRunnerStore($this->rrLock, $this->tokens); From b7e3e9004ac0a1b55ef2da5572f0a2f66033d468 Mon Sep 17 00:00:00 2001 From: Alexander Strizhak Date: Tue, 27 May 2025 23:02:12 +0300 Subject: [PATCH 2/5] psalm fix --- src/RandomTokenGenerator.php | 1 + src/RoadRunnerStore.php | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/src/RandomTokenGenerator.php b/src/RandomTokenGenerator.php index 5e8efb2..b5d58f9 100644 --- a/src/RandomTokenGenerator.php +++ b/src/RandomTokenGenerator.php @@ -14,6 +14,7 @@ public function __construct( ) { } + #[\Override] public function generate(): string { return \bin2hex(\random_bytes($this->length)); diff --git a/src/RoadRunnerStore.php b/src/RoadRunnerStore.php index 42781f9..c99974d 100644 --- a/src/RoadRunnerStore.php +++ b/src/RoadRunnerStore.php @@ -36,6 +36,7 @@ public function withTtl(float $ttl, float $waitTtl = 0): self return new self($this->lock, $this->tokens, $ttl, $waitTtl); } + #[\Override] public function save(Key $key): void { \assert(false === $key->hasState(__CLASS__)); @@ -58,6 +59,7 @@ public function save(Key $key): void } } + #[\Override] public function saveRead(Key $key): void { \assert(false === $key->hasState(__CLASS__)); @@ -74,6 +76,7 @@ public function saveRead(Key $key): void $key->setState(__CLASS__, $lockId); } + #[\Override] public function exists(Key $key): bool { \assert($key->hasState(__CLASS__)); @@ -86,6 +89,7 @@ public function exists(Key $key): bool return $this->lock->exists($resource, $lockId); } + #[\Override] public function putOffExpiration(Key $key, float $ttl): void { \assert($key->hasState(__CLASS__)); @@ -101,6 +105,7 @@ public function putOffExpiration(Key $key, float $ttl): void } } + #[\Override] public function delete(Key $key): void { \assert($key->hasState(__CLASS__)); @@ -111,6 +116,7 @@ public function delete(Key $key): void $this->lock->release($resource, $lockId); } + #[\Override] public function waitAndSave(Key $key): void { $lockId = $this->getUniqueToken($key); From d97f92920bc41f88f9bb7102ddf71645c5fd6ce7 Mon Sep 17 00:00:00 2001 From: Alexander Strizhak Date: Thu, 29 May 2025 23:55:48 +0300 Subject: [PATCH 3/5] use symfony/polyfill --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 4a824c0..e1f8170 100644 --- a/composer.json +++ b/composer.json @@ -30,7 +30,8 @@ "require": { "php": ">=8.1", "roadrunner-php/lock": "^1.0", - "symfony/lock": "^6.0 || ^7.0" + "symfony/lock": "^6.0 || ^7.0", + "symfony/polyfill": "^1.32" }, "require-dev": { "phpunit/phpunit": "^10.0", From c5b187cc27f4b0d6407e13ca684f8887b84b755b Mon Sep 17 00:00:00 2001 From: Alexander Strizhak Date: Fri, 30 May 2025 00:07:37 +0300 Subject: [PATCH 4/5] tests for withTtl --- src/RoadRunnerStore.php | 6 ++++- tests/RoadRunnerStoreTest.php | 41 +++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/RoadRunnerStore.php b/src/RoadRunnerStore.php index c99974d..602eb29 100644 --- a/src/RoadRunnerStore.php +++ b/src/RoadRunnerStore.php @@ -31,8 +31,12 @@ public function __construct( \assert($this->initialWaitTtl >= 0); } - public function withTtl(float $ttl, float $waitTtl = 0): self + /** + * Clone current instance with another values of ttl. + */ + public function withTtl(float $ttl, ?float $waitTtl = null): self { + $waitTtl ??= $this->initialWaitTtl; return new self($this->lock, $this->tokens, $ttl, $waitTtl); } diff --git a/tests/RoadRunnerStoreTest.php b/tests/RoadRunnerStoreTest.php index c19b79f..12acf1e 100644 --- a/tests/RoadRunnerStoreTest.php +++ b/tests/RoadRunnerStoreTest.php @@ -190,4 +190,45 @@ public function testWaitAndSaveFail(): void $store = new RoadRunnerStore($this->rrLock, $this->tokens); $store->waitAndSave(new Key('resource-name')); } + + /** + * @dataProvider dataWithTtl + */ + public function testWithTtl(float $ttl, ?float $waitTtl, float $ttlExp, float $waitTtlExp): void + { + $this->rrLock->expects($this->once()) + ->method('lock') + ->with('resource-name', 'random-id', $ttlExp, $waitTtlExp) + ->willReturn('lock-id'); + + $s = new RoadRunnerStore($this->rrLock, $this->tokens); + $s->withTtl($ttl, $waitTtl)->save(new Key('resource-name')); + } + + /** + * @return iterable + */ + public static function dataWithTtl(): iterable + { + yield [ + 100, + null, + 100, + 0, + ]; + + yield [ + 0.1, + 0.1, + 0.1, + 0.1, + ]; + + yield [ + 0, + 0, + 0, + 0, + ]; + } } From a0a8b5994a890b5e6dcae8c6749aa254027ababa Mon Sep 17 00:00:00 2001 From: Aleksei Gagarin Date: Fri, 30 May 2025 11:46:57 +0400 Subject: [PATCH 5/5] Apply suggestions from code review --- composer.json | 2 +- src/RoadRunnerStore.php | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index e1f8170..db89d67 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,7 @@ "php": ">=8.1", "roadrunner-php/lock": "^1.0", "symfony/lock": "^6.0 || ^7.0", - "symfony/polyfill": "^1.32" + "symfony/polyfill-php83": "^1.32" }, "require-dev": { "phpunit/phpunit": "^10.0", diff --git a/src/RoadRunnerStore.php b/src/RoadRunnerStore.php index 602eb29..3c064fc 100644 --- a/src/RoadRunnerStore.php +++ b/src/RoadRunnerStore.php @@ -33,6 +33,8 @@ public function __construct( /** * Clone current instance with another values of ttl. + * @param float $ttl The time-to-live of the lock, in seconds. Defaults to 0 (forever). + * @param float $waitTtl How long to wait to acquire lock until returning false, in seconds. */ public function withTtl(float $ttl, ?float $waitTtl = null): self {