From 91fa3e311d029de2bdd3fff276bee2eaccc05563 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Thu, 15 Oct 2020 21:27:13 +0200 Subject: [PATCH] Deprecate lock.store aliases --- UPGRADE-5.2.md | 2 +- UPGRADE-6.0.md | 2 +- .../Bundle/FrameworkBundle/CHANGELOG.md | 2 +- .../FrameworkExtension.php | 13 +++-- src/Symfony/Component/Lock/CHANGELOG.md | 1 + src/Symfony/Component/Lock/LockFactory.php | 14 ++++- .../Component/Lock/Tests/LockFactoryTest.php | 55 ++++++++++++++++++- .../Component/Semaphore/SemaphoreFactory.php | 11 +++- .../Semaphore/Tests/SemaphoreFactoryTest.php | 53 +++++++++++++++++- 9 files changed, 136 insertions(+), 17 deletions(-) diff --git a/UPGRADE-5.2.md b/UPGRADE-5.2.md index ff29d10c8cfbe..36859edc762cf 100644 --- a/UPGRADE-5.2.md +++ b/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 ---- diff --git a/UPGRADE-6.0.md b/UPGRADE-6.0.md index 1bedeaad74a51..2ee58d9ce8f78 100644 --- a/UPGRADE-6.0.md +++ b/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 -------------- diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index 0d12982902503..dde9bada0961f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/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 ----- diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 3a7ab77a563bb..c9f9d405cc65e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -1674,14 +1674,15 @@ private function registerLockConfiguration(array $config, ContainerBuilder $cont 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 @@ private function registerLockConfiguration(array $config, ContainerBuilder $cont // 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.'); } diff --git a/src/Symfony/Component/Lock/CHANGELOG.md b/src/Symfony/Component/Lock/CHANGELOG.md index 360c1ebbe914d..4c85f76587973 100644 --- a/src/Symfony/Component/Lock/CHANGELOG.md +++ b/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 ----- diff --git a/src/Symfony/Component/Lock/LockFactory.php b/src/Symfony/Component/Lock/LockFactory.php index 82bcd01605f53..11e3bd2ec8fb3 100644 --- a/src/Symfony/Component/Lock/LockFactory.php +++ b/src/Symfony/Component/Lock/LockFactory.php @@ -43,7 +43,19 @@ public function __construct(PersistingStoreInterface $store) */ 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; diff --git a/src/Symfony/Component/Lock/Tests/LockFactoryTest.php b/src/Symfony/Component/Lock/Tests/LockFactoryTest.php index 376bd692a5d00..7026d02b24440 100644 --- a/src/Symfony/Component/Lock/Tests/LockFactoryTest.php +++ b/src/Symfony/Component/Lock/Tests/LockFactoryTest.php @@ -13,8 +13,8 @@ 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'); + + // 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->assertInstanceOf(LockInterface::class, $lock); + $this->assertSame($keys[0], $keys[1]); } } diff --git a/src/Symfony/Component/Semaphore/SemaphoreFactory.php b/src/Symfony/Component/Semaphore/SemaphoreFactory.php index bf4743e8f767b..9194817f096e3 100644 --- a/src/Symfony/Component/Semaphore/SemaphoreFactory.php +++ b/src/Symfony/Component/Semaphore/SemaphoreFactory.php @@ -42,7 +42,16 @@ public function __construct(PersistingStoreInterface $store) */ 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; diff --git a/src/Symfony/Component/Semaphore/Tests/SemaphoreFactoryTest.php b/src/Symfony/Component/Semaphore/Tests/SemaphoreFactoryTest.php index cf79154632691..5156f64671d03 100644 --- a/src/Symfony/Component/Semaphore/Tests/SemaphoreFactoryTest.php +++ b/src/Symfony/Component/Semaphore/Tests/SemaphoreFactoryTest.php @@ -13,9 +13,9 @@ 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é @@ -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); + + // 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->assertInstanceOf(SemaphoreInterface::class, $semaphore); + $this->assertSame($keys[0], $keys[1]); } }