[Semaphore] Fixed some bugs

src/Symfony/Component/Semaphore/README.md

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

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

@@ -78,9 +78,17 @@ class RedisStore implements PersistingStoreInterface
 
         $script = file_get_contents(__DIR__.'/Resources/redis_put_off_expiration.lua');
 
-        if ($this->evaluate($script, sprintf('{%s}', $key), [time() + $ttlInSecond, $this->getUniqueToken($key)])) {
+        $ret = $this->evaluate($script, sprintf('{%s}', $key), [time() + $ttlInSecond, $this->getUniqueToken($key)]);
+
+        // Occurs when redis has been reset
+        if (false === $ret) {
             throw new SemaphoreExpiredException($key, 'the script returns false');
         }
+
+        // Occurs when redis has added an item in the set
+        if (0 < $ret) {
+            throw new SemaphoreExpiredException($key, 'the script returns a positive number');
+        }
     }
 
     /**

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

@@ -9,10 +9,12 @@ if added == 1 then
 end
 
 -- Extend the TTL
-local curentTtl = redis.call("TTL", weightKey)
+local maxExpiration = redis.call("ZREVRANGE", timeKey, 0, 0, "WITHSCORES")[2]
-if curentTtl < now + ttlInSecond then
+if nil == maxExpiration then
-    redis.call("EXPIRE", weightKey, curentTtl + 10)
+    return 1
-    redis.call("EXPIRE", timeKey, curentTtl + 10)
 end
 
+redis.call("EXPIREAT", weightKey, maxExpiration + 10)
+redis.call("EXPIREAT", timeKey, maxExpiration + 10)
+
 return added

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

@@ -34,10 +34,8 @@ redis.call("ZADD", timeKey, now + ttlInSecond, identifier)
 redis.call("ZADD", weightKey, weight, identifier)
 
 -- Extend the TTL
-local curentTtl = redis.call("TTL", weightKey)
+local maxExpiration = redis.call("ZREVRANGE", timeKey, 0, 0, "WITHSCORES")[2]
-if curentTtl < now + ttlInSecond then
+redis.call("EXPIREAT", weightKey, maxExpiration + 10)
-    redis.call("EXPIRE", weightKey, curentTtl + 10)
+redis.call("EXPIREAT", timeKey, maxExpiration + 10)
-    redis.call("EXPIRE", timeKey, curentTtl + 10)
-end
 
 return true

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

@@ -13,6 +13,7 @@ namespace Symfony\Component\Semaphore\Tests\Store;
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Semaphore\Exception\SemaphoreAcquiringException;
+use Symfony\Component\Semaphore\Exception\SemaphoreExpiredException;
 use Symfony\Component\Semaphore\Key;
 use Symfony\Component\Semaphore\PersistingStoreInterface;
 
@@ -28,7 +29,7 @@ abstract class AbstractStoreTest extends TestCase
     {
         $store = $this->getStore();
 
-        $key = new Key('key', 1);
+        $key = new Key(__METHOD__, 1);
 
         $this->assertFalse($store->exists($key));
         $store->save($key, 10);
@@ -41,8 +42,8 @@ abstract class AbstractStoreTest extends TestCase
     {
         $store = $this->getStore();
 
-        $key1 = new Key('key1', 1);
+        $key1 = new Key(__METHOD__.'1', 1);
-        $key2 = new Key('key2', 1);
+        $key2 = new Key(__METHOD__.'2', 1);
 
         $store->save($key1, 10);
         $this->assertTrue($store->exists($key1));
@@ -65,7 +66,7 @@ abstract class AbstractStoreTest extends TestCase
     {
         $store = $this->getStore();
 
-        $resource = 'resource';
+        $resource = __METHOD__;
         $key1 = new Key($resource, 1);
         $key2 = new Key($resource, 1);
 
@@ -100,7 +101,7 @@ abstract class AbstractStoreTest extends TestCase
     {
         $store = $this->getStore();
 
-        $resource = 'resource';
+        $resource = __METHOD__;
         $key1 = new Key($resource, 2);
         $key2 = new Key($resource, 2);
         $key3 = new Key($resource, 2);
@@ -144,7 +145,7 @@ abstract class AbstractStoreTest extends TestCase
     {
         $store = $this->getStore();
 
-        $resource = 'resource';
+        $resource = __METHOD__;
         $key1 = new Key($resource, 4, 2);
         $key2 = new Key($resource, 4, 2);
         $key3 = new Key($resource, 4, 2);
@@ -184,17 +185,40 @@ abstract class AbstractStoreTest extends TestCase
         $store->delete($key3);
     }
 
+    public function testPutOffExpiration()
+    {
+        $store = $this->getStore();
+        $key = new Key(__METHOD__, 4, 2);
+        $store->save($key, 20);
+
+        $store->putOffExpiration($key, 20);
+
+        // just asserts it doesn't throw an exception
+        $this->addToAssertionCount(1);
+    }
+
+    public function testPutOffExpirationWhenSaveHasNotBeenCalled()
+    {
+        // This test simulate the key has expired since it does not exist
+        $store = $this->getStore();
+        $key1 = new Key(__METHOD__, 4, 2);
+
+        $this->expectException(SemaphoreExpiredException::class);
+        $this->expectExceptionMessage('The semaphore "Symfony\Component\Semaphore\Tests\Store\AbstractStoreTest::testPutOffExpirationWhenSaveHasNotBeenCalled" has expired: the script returns a positive number.');
+
+        $store->putOffExpiration($key1, 20);
+    }
+
     public function testSaveTwice()
     {
         $store = $this->getStore();
 
-        $resource = 'resource';
+        $key = new Key(__METHOD__, 1);
-        $key = new Key($resource, 1);
 
         $store->save($key, 10);
         $store->save($key, 10);
 
-        // just asserts it don't throw an exception
+        // just asserts it doesn't throw an exception
         $this->addToAssertionCount(1);
 
         $store->delete($key);

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

@@ -18,6 +18,11 @@ namespace Symfony\Component\Semaphore\Tests\Store;
  */
 class RedisStoreTest extends AbstractRedisStoreTest
 {
+    protected function setUp(): void
+    {
+        $this->getRedisConnection()->flushDB();
+    }
+
     public static function setUpBeforeClass(): void
     {
         try {