8000 bug #37977 [Semaphore] Fixed some bugs (lyrixx) · symfony/symfony@69f8eae · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 69f8eae

Browse files
fabpotfabpot
committed
bug #37977 [Semaphore] Fixed some bugs (lyrixx)
This PR was merged into the 5.2-dev branch. Discussion ---------- [Semaphore] Fixed some bugs | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | Commits ------- 3c943b9 [Semaphore] Fixed some bugs
2 parents 2d21c39 + 3c943b9 commit 69f8eae

File tree

6 files changed

+60
-19
lines changed

6 files changed

+60
-19
lines changed

‎src/Symfony/Component/Semaphore/README.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
Semaphore Component
22
===================
33

4+
The Semaphore Component manages
5+
[semaphores](https://en.wikipedia.org/wiki/Semaphore_(programming)), a mechanism
6+
to provide exclusive access to a shared resource.
7+
48
**This Component is experimental**.
59
[Experimental features](https://symfony.com/doc/current/contributing/code/experimental.html)
610
are not covered by Symfony's

‎src/Symfony/Component/Semaphore/Store/RedisStore.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,17 @@ public function putOffExpiration(Key $key, float $ttlInSecond)
7878

7979
$script = file_get_contents(__DIR__.'/Resources/redis_put_off_expiration.lua');
8080

81-
if ($this->evaluate($script, sprintf('{%s}', $key), [time() + $ttlInSecond, $this->getUniqueToken($key)])) {
81+
$ret = $this->evaluate($script, sprintf('{%s}', $key), [time() + $ttlInSecond, $this->getUniqueToken($key)]);
82+
83+
// Occurs when redis has been reset
84+
if (false === $ret) {
8285
throw new SemaphoreExpiredException($key, 'the script returns false');
8386
}
87+
88+
// Occurs when redis has added an item in the set
89+
if (0 < $ret) {
90+
throw new SemaphoreExpiredException($key, 'the script returns a positive number');
91+
}
8492
}
8593

8694
/**

‎src/Symfony/Component/Semaphore/Store/Resources/redis_put_off_expiration.lua

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@ if added == 1 then
99
end
1010

1111
-- Extend the TTL
12-
local curentTtl = redis.call("TTL", weightKey)
13-
if curentTtl < now + ttlInSecond then
14-
redis.call("EXPIRE", weightKey, curentTtl + 10)
15-
redis.call("EXPIRE", timeKey, curentTtl + 10)
12+
local maxExpiration = redis.call("ZREVRANGE", timeKey, 0, 0, "WITHSCORES")[2]
13+
if nil == maxExpiration then
14+
return 1
1615
end
1716

17+
redis.call("EXPIREAT", weightKey, maxExpiration + 10)
18+
redis.call("EXPIREAT", timeKey, maxExpiration + 10)
19+
1820
return added

‎src/Symfony/Component/Semaphore/Store/Resources/redis_save.lua

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,8 @@ redis.call("ZADD", timeKey, now + ttlInSecond, identifier)
3434
redis.call("ZADD", weightKey, weight, identifier)
3535

3636
-- Extend the TTL
37-
local curentTtl = redis.call("TTL", weightKey)
38-
if curentTtl < now + ttlInSecond then
39-
redis.call("EXPIRE", weightKey, curentTtl + 10)
40-
redis.call("EXPIRE", timeKey, curentTtl + 10)
41-
end
37+
local maxExpiration = redis.call("ZREVRANGE", timeKey, 0, 0, "WITHSCORES")[2]
38+
redis.call("EXPIREAT", weightKey, maxExpiration + 10)
39+
redis.call("EXPIREAT", timeKey, maxExpiration + 10)
4240

4341
return true

‎src/Symfony/Component/Semaphore/Tests/Store/AbstractStoreTest.php

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Semaphore\Exception\SemaphoreAcquiringException;
16+
use Symfony\Component\Semaphore\Exception\SemaphoreExpiredException;
1617
use Symfony\Component\Semaphore\Key;
1718
use Symfony\Component\Semaphore\PersistingStoreInterface;
1819

@@ -28,7 +29,7 @@ public function testSaveExistAndDelete()
2829
{
2930
$store = $this->getStore();
3031

31-
$key = new Key('key', 1);
32+
$key = new Key(__METHOD__, 1);
3233

3334
$this->assertFalse($store->exists($key));
3435
$store->save($key, 10);
@@ -41,8 +42,8 @@ public function testSaveWithDifferentResources()
4142
{
4243
$store = $this->getStore();
4344

44-
$key1 = new Key('key1', 1);
45-
$key2 = new Key('key2', 1);
45+
$key1 = new Key(__METHOD__.'1', 1);
46+
$key2 = new Key(__METHOD__.'2', 1);
4647

4748
$store->save($key1, 10);
4849
$this->assertTrue($store->exists($key1));
@@ -65,7 +66,7 @@ public function testSaveWithDifferentKeysOnSameResource()
6566
{
6667
$store = $this->getStore();
6768

68-
$resource = 'resource';
69+
$resource = __METHOD__;
6970
$key1 = new Key($resource, 1);
7071
$key2 = new Key($resource, 1);
7172

@@ -100,7 +101,7 @@ public function testSaveWithLimitAt2()
100101
{
101102
$store = $this->getStore();
102103

103-
$resource = 'resource';
104+
$resource = __METHOD__;
104105
$key1 = new Key($resource, 2);
105106
$key2 = new Key($resource, 2);
106107
$key3 = new Key($resource, 2);
@@ -144,7 +145,7 @@ public function testSaveWithWeightAndLimitAt3()
144145
{
145146
$store = $this->getStore();
146147

147-
$resource = 'resource';
148+
$resource = __METHOD__;
148149
$key1 = new Key($resource, 4, 2);
149150
$key2 = new Key($resource, 4, 2);
150151
$key3 = new Key($resource, 4, 2);
@@ -184,17 +185,40 @@ public function testSaveWithWeightAndLimitAt3()
184185
$store->delete($key3);
185186
}
186187

188+
public function testPutOffExpiration()
189+
{
190+
$store = $this->getStore();
191+
$key = new Key(__METHOD__, 4, 2);
192+
$store->save($key, 20);
193+
194+
$store->putOffExpiration($key, 20);
195+
196+
// just asserts it doesn't throw an exception
197+
$this->addToAssertionCount(1);
198+
}
199+
200+
public function testPutOffExpirationWhenSaveHasNotBeenCalled()
201+
{
202+
// This test simulate the key has expired since it does not exist
203+
$store = $this->getStore();
204+
$key1 = new Key(__METHOD__, 4, 2);
205+
206+
$this->expectException(SemaphoreExpiredException::class);
207+
$this->expectExceptionMessage('The semaphore "Symfony\Component\Semaphore\Tests\Store\AbstractStoreTest::testPutOffExpirationWhenSaveHasNotBeenCalled" has expired: the script returns a positive number.');
208+
209+
$store->putOffExpiration($key1, 20);
210+
}
211+
187212
public function testSaveTwice()
188213
{
189214
$store = $this->getStore();
190215

191-
$resource = 'resource';
192-
$key = new Key($resource, 1);
216+
$key = new Key(__METHOD__, 1);
193217

194218
$store->save($key, 10);
195219
$store->save($key, 10);
196220

197-
// just asserts it don't throw an exception
221+
// just asserts it doesn't throw an exception
198222
$this->addToAssertionCount(1);
199223

200224
$store->delete($key);

‎src/Symfony/Component/Semaphore/Tests/Store/RedisStoreTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@
1818
*/
1919
class RedisStoreTest extends AbstractRedisStoreTest
2020
{
21+
protected function setUp(): void
22+
{
23+
$this->getRedisConnection()->flushDB();
24+
}
25+
2126
public static function setUpBeforeClass(): void
2227
{
2328
try {

0 commit comments

Comments
 (0)
0