feature #38593 [Lock][Semaphore] Add Factory::createFromKey and deprecate lock.store services (jderusse)

This PR was merged into the 5.x branch.

Discussion
----------

[Lock][Semaphore] Add Factory::createFromKey and deprecate lock.store services

I| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Tickets       | /
| License       | MIT
| Doc PR        | todo

The `lock` service and aliases have been deprecated in #38576. This PR also deprecate the `lock.store` service and aliases.

People should always inject the factory into their services, except for one scenario: When they want building the `Lock` manually because they want to keep the key.

This scenario require declaring `lock.store` service and `PersisingStoreInterface` aliases.
This could be avoided if people had a method to create a lock from a given key.

This PR adds a `LockFactory::createFromKey()` method and deprecate all `lock.store` services

I also updated the Semaphore component for consistency, and helping people to only inject the SemahoreFactory.

nb: use cases for serializing the keys:
- Netflix allows only 5 users of the same family at the same time.
- An holiday apartment rental avoid users putting the same apartment in the basket
- ...

Commits
-------

91fa3e311d Deprecate lock.store aliases

UPGRADE-5.2.md

@@ -15,7 +15,7 @@ FrameworkBundle
    keep cache namespaces separated by environment of the app. The `%kernel.container_class%` (which includes the environment)
    used to be added by default to the seed, which is not the case anymore. This allows sharing caches between
    apps or different environments.
- * Deprecated the `lock.RESOURCE_NAME` service and the `lock` and `LockInterface` aliases, use `lock.RESOURCE_NAME.factory`, `lock.factory` or `LockFactory` instead.
+ * Deprecated the `lock.RESOURCE_NAME` and `lock.RESOURCE_NAME.store` services and the `lock`, `LockInterface`, `lock.store` and `PersistingStoreInterface` aliases, use `lock.RESOURCE_NAME.factory`, `lock.factory` or `LockFactory` instead.
 
 Form
 ----

UPGRADE-6.0.md

@@ -58,7 +58,7 @@ FrameworkBundle
  * Removed `session.attribute_bag` service and `session.flash_bag` service.
  * The `form.factory`, `form.type.file`, `translator`, `security.csrf.token_manager`, `serializer`,
    `cache_clearer`, `filesystem` and `validator` services are now private.
- * Removed the `lock.RESOURCE_NAME` service and the `lock` and `LockInterface` aliases, use `lock.RESOURCE_NAME.factory`, `lock.factory` or `LockFactory` instead.
+ * Removed the `lock.RESOURCE_NAME` and `lock.RESOURCE_NAME.store` services and the `lock`, `LockInterface`, `lock.store` and `PersistingStoreInterface` aliases, use `lock.RESOURCE_NAME.factory`, `lock.factory` or `LockFactory` instead.
 
 HttpFoundation
 --------------

src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md

@@ -14,7 +14,7 @@ CHANGELOG
  * added `assertCheckboxChecked()` and `assertCheckboxNotChecked()` in `WebTestCase`
  * added `assertFormValue()` and `assertNoFormValue()` in `WebTestCase`
  * Added "--as-tree=3" option to `translation:update` command to dump messages as a tree-like structure. The given value defines the level where to switch to inline YAML
- * Deprecated the `lock.RESOURCE_NAME` service and the `lock` and `LockInterface` aliases, use `lock.RESOURCE_NAME.factory`, `lock.factory` or `LockFactory` instead.
+ * Deprecated the `lock.RESOURCE_NAME` and `lock.RESOURCE_NAME.store` services and the `lock`, `LockInterface`, `lock.store` and `PersistingStoreInterface` aliases, use `lock.RESOURCE_NAME.factory`, `lock.factory` or `LockFactory` instead.
 
 5.1.0
 -----

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

@@ -1674,14 +1674,15 @@ class FrameworkExtension extends Extension
             if (\count($storeDefinitions) > 1) {
                 $combinedDefinition = new ChildDefinition('lock.store.combined.abstract');
                 $combinedDefinition->replaceArgument(0, $storeDefinitions);
-                $container->setDefinition('lock.'.$resourceName.'.store', $combinedDefinition);
+                $container->setDefinition('lock.'.$resourceName.'.store', $combinedDefinition)->setDeprecated('symfony/framework-bundle', '5.2', 'The "%service_id%" service is deprecated, use "lock.'.$resourceName.'.factory" instead.');
+                $container->setDefinition($storeDefinitionId = '.lock.'.$resourceName.'.store.'.$container->hash($resourceStores), $combinedDefinition);
             } else {
-                $container->setAlias('lock.'.$resourceName.'.store', new Alias((string) $storeDefinitions[0], false));
+                $container->setAlias('lock.'.$resourceName.'.store', (new Alias($storeDefinitionId, false))->setDeprecated('symfony/framework-bundle', '5.2', 'The "%alias_id%" alias is deprecated, use "lock.'.$resourceName.'.factory" instead.'));
             }
 
             // Generate factories for each resource
             $factoryDefinition = new ChildDefinition('lock.factory.abstract');
-            $factoryDefinition->replaceArgument(0, new Reference('lock.'.$resourceName.'.store'));
+            $factoryDefinition->replaceArgument(0, new Reference($storeDefinitionId));
             $container->setDefinition('lock.'.$resourceName.'.factory', $factoryDefinition);
 
             // Generate services for lock instances
@@ -1693,14 +1694,14 @@ class FrameworkExtension extends Extension
 
             // provide alias for default resource
             if ('default' === $resourceName) {
-                $container->setAlias('lock.store', new Alias('lock.'.$resourceName.'.store', false));
+                $container->setAlias('lock.store', (new Alias($storeDefinitionId, false))->setDeprecated('symfony/framework-bundle', '5.2', 'The "%alias_id%" alias is deprecated, use "lock.factory" instead.'));
                 $container->setAlias('lock.factory', new Alias('lock.'.$resourceName.'.factory', false));
                 $container->setAlias('lock', (new Alias('lock.'.$resourceName, false))->setDeprecated('symfony/framework-bundle', '5.2', 'The "%alias_id%" alias is deprecated, use "lock.factory" instead.'));
-                $container->setAlias(PersistingStoreInterface::class, new Alias('lock.store', false));
+                $container->setAlias(PersistingStoreInterface::class, (new Alias($storeDefinitionId, false))->setDeprecated('symfony/framework-bundle', '5.2', 'The "%alias_id%" alias is deprecated, use "'.LockFactory::class.'" instead.'));
                 $container->setAlias(LockFactory::class, new Alias('lock.factory', false));
                 $container->setAlias(LockInterface::class, (new Alias('lock.'.$resourceName, false))->setDeprecated('symfony/framework-bundle', '5.2', 'The "%alias_id%" alias is deprecated, use "'.LockFactory::class.'" instead.'));
             } else {
-                $container->registerAliasForArgument('lock.'.$resourceName.'.store', PersistingStoreInterface::class, $resourceName.'.lock.store');
+                $container->registerAliasForArgument($storeDefinitionId, PersistingStoreInterface::class, $resourceName.'.lock.store')->setDeprecated('symfony/framework-bundle', '5.2', 'The "%alias_id%" alias is deprecated, use "'.LockFactory::class.' '.$resourceName.'LockFactory" instead.');
                 $container->registerAliasForArgument('lock.'.$resourceName.'.factory', LockFactory::class, $resourceName.'.lock.factory');
                 $container->registerAliasForArgument('lock.'.$resourceName, LockInterface::class, $resourceName.'.lock')->setDeprecated('symfony/framework-bundle', '5.2', 'The "%alias_id%" alias is deprecated, use "'.LockFactory::class.' $'.$resourceName.'LockFactory" instead.');
             }

src/Symfony/Component/Lock/CHANGELOG.md

@@ -11,6 +11,7 @@ CHANGELOG
  * deprecated `RetryTillSaveStore`, logic has been moved in `Lock` and is not needed anymore.
  * added `InMemoryStore`
  * added `PostgreSqlStore`
+ * added the `LockFactory::CreateLockFromKey()` method.
 
 5.1.0
 -----

src/Symfony/Component/Lock/LockFactory.php

@@ -43,7 +43,19 @@ class LockFactory implements LoggerAwareInterface
      */
     public function createLock(string $resource, ?float $ttl = 300.0, bool $autoRelease = true): LockInterface
     {
-        $lock = new Lock(new Key($resource), $this->store, $ttl, $autoRelease);
+        return $this->createLockFromKey(new Key($resource), $ttl, $autoRelease);
+    }
+
+    /**
+     * Creates a lock from the given key.
+     *
+     * @param Key        $key         The key containing the lock's state
+     * @param float|null $ttl         Maximum expected lock duration in seconds
+     * @param bool       $autoRelease Whether to automatically release the lock or not when the lock instance is destroyed
+     */
+    public function createLockFromKey(Key $key, ?float $ttl = 300.0, bool $autoRelease = true): LockInterface
+    {
+        $lock = new Lock($key, $this->store, $ttl, $autoRelease);
         $lock->setLogger($this->logger);
 
         return $lock;

src/Symfony/Component/Lock/Tests/LockFactoryTest.php

@@ -13,8 +13,8 @@ namespace Symfony\Component\Lock\Tests;
 
 use PHPUnit\Framework\TestCase;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\Lock\Key;
 use Symfony\Component\Lock\LockFactory;
-use Symfony\Component\Lock\LockInterface;
 use Symfony\Component\Lock\PersistingStoreInterface;
 
 /**
@@ -25,12 +25,61 @@ class LockFactoryTest extends TestCase
     public function testCreateLock()
     {
         $store = $this->getMockBuilder(PersistingStoreInterface::class)->getMock();
+        $store->expects($this->any())->method('exists')->willReturn(false);
+
+        $keys = [];
+        $store
+            ->expects($this->exactly(2))
+            ->method('save')
+            ->with($this->callback(function ($key) use (&$keys) {
+                $keys[] = $key;
+
+                return true;
+            }))
+            ->willReturn(true);
+
         $logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
         $factory = new LockFactory($store);
         $factory->setLogger($logger);
 
-        $lock = $factory->createLock('foo');
+        $lock1 = $factory->createLock('foo');
+        $lock2 = $factory->createLock('foo');
 
-        $this->assertInstanceOf(LockInterface::class, $lock);
+        // assert lock1 and lock2 don't share the same state
+        $lock1->acquire();
+        $lock2->acquire();
+
+        $this->assertNotSame($keys[0], $keys[1]);
+    }
+
+    public function testCreateLockFromKey()
+    {
+        $store = $this->getMockBuilder(PersistingStoreInterface::class)->getMock();
+        $store->expects($this->any())->method('exists')->willReturn(false);
+
+        $keys = [];
+        $store
+            ->expects($this->exactly(2))
+            ->method('save')
+            ->with($this->callback(function ($key) use (&$keys) {
+                $keys[] = $key;
+
+                return true;
+            }))
+            ->willReturn(true);
+
+        $logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
+        $factory = new LockFactory($store);
+        $factory->setLogger($logger);
+
+        $key = new Key('foo');
+        $lock1 = $factory->createLockFromKey($key);
+        $lock2 = $factory->createLockFromKey($key);
+
+        // assert lock1 and lock2 share the same state
+        $lock1->acquire();
+        $lock2->acquire();
+
+        $this->assertSame($keys[0], $keys[1]);
     }
 }

src/Symfony/Component/Semaphore/SemaphoreFactory.php

@@ -42,7 +42,16 @@ class SemaphoreFactory implements LoggerAwareInterface
      */
     public function createSemaphore(string $resource, int $limit, int $weight = 1, ?float $ttlInSecond = 300.0, bool $autoRelease = true): SemaphoreInterface
     {
-        $semaphore = new Semaphore(new Key($resource, $limit, $weight), $this->store, $ttlInSecond, $autoRelease);
+        return $this->createSemaphoreFromKey(new Key($resource, $limit, $weight), $ttlInSecond, $autoRelease);
+    }
+
+    /**
+     * @param float|null $ttlInSecond Maximum expected semaphore duration in seconds
+     * @param bool       $autoRelease Whether to automatically release the semaphore or not when the semaphore instance is destroyed
+     */
+    public function createSemaphoreFromKey(Key $key, ?float $ttlInSecond = 300.0, bool $autoRelease = true): SemaphoreInterface
+    {
+        $semaphore = new Semaphore($key, $this->store, $ttlInSecond, $autoRelease);
         $semaphore->setLogger($this->logger);
 
         return $semaphore;

src/Symfony/Component/Semaphore/Tests/SemaphoreFactoryTest.php

@@ -13,9 +13,9 @@ namespace Symfony\Component\Semaphore\Tests;
 
 use PHPUnit\Framework\TestCase;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\Semaphore\Key;
 use Symfony\Component\Semaphore\PersistingStoreInterface;
 use Symfony\Component\Semaphore\SemaphoreFactory;
-use Symfony\Component\Semaphore\SemaphoreInterface;
 
 /**
  * @author Jérémy Derussé <jeremy@derusse.com>
@@ -26,12 +26,59 @@ class SemaphoreFactoryTest extends TestCase
     public function testCreateSemaphore()
     {
         $store = $this->getMockBuilder(PersistingStoreInterface::class)->getMock();
+
+        $keys = [];
+        $store
+            ->expects($this->exactly(2))
+            ->method('save')
+            ->with($this->callback(function ($key) use (&$keys) {
+                $keys[] = $key;
+
+                return true;
+            }))
+            ->willReturn(true);
+
         $logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
         $factory = new SemaphoreFactory($store);
         $factory->setLogger($logger);
 
-        $semaphore = $factory->createSemaphore('foo', 4);
+        $semaphore1 = $factory->createSemaphore('foo', 4);
+        $semaphore2 = $factory->createSemaphore('foo', 4);
 
-        $this->assertInstanceOf(SemaphoreInterface::class, $semaphore);
+        // assert lock1 and lock2 don't share the same state
+        $semaphore1->acquire();
+        $semaphore2->acquire();
+
+        $this->assertNotSame($keys[0], $keys[1]);
+    }
+
+    public function testCreateSemaphoreFromKey()
+    {
+        $store = $this->getMockBuilder(PersistingStoreInterface::class)->getMock();
+
+        $keys = [];
+        $store
+            ->expects($this->exactly(2))
+            ->method('save')
+            ->with($this->callback(function ($key) use (&$keys) {
+                $keys[] = $key;
+
+                return true;
+            }))
+            ->willReturn(true);
+
+        $logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
+        $factory = new SemaphoreFactory($store);
+        $factory->setLogger($logger);
+
+        $key = new Key('foo', 4);
+        $semaphore1 = $factory->createSemaphoreFromKey($key);
+        $semaphore2 = $factory->createSemaphoreFromKey($key);
+
+        // assert lock1 and lock2 share the same state
+        $semaphore1->acquire();
+        $semaphore2->acquire();
+
+        $this->assertSame($keys[0], $keys[1]);
     }
 }