From 2db24cf5823169f7ece2a1563fe9a73380a1b70b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Mon, 21 Oct 2019 08:58:12 +0200 Subject: [PATCH] Add missing lock connection string in FrameworkExtension --- UPGRADE-4.4.md | 2 + .../FrameworkExtension.php | 41 +++---------------- .../FrameworkBundle/Resources/config/lock.xml | 10 ++++- src/Symfony/Component/Lock/CHANGELOG.md | 5 ++- .../Component/Lock/Store/StoreFactory.php | 20 ++++++++- .../Component/Lock/Store/ZookeeperStore.php | 19 +++++++++ .../Lock/Tests/Store/StoreFactoryTest.php | 22 ++++++++++ .../Lock/Tests/Store/ZookeeperStoreTest.php | 17 +++++++- 8 files changed, 94 insertions(+), 42 deletions(-) diff --git a/UPGRADE-4.4.md b/UPGRADE-4.4.md index d6f6cbe529..eb3cf27fc6 100644 --- a/UPGRADE-4.4.md +++ b/UPGRADE-4.4.md @@ -162,6 +162,8 @@ Lock * Deprecated `Symfony\Component\Lock\StoreInterface` in favor of `Symfony\Component\Lock\BlockingStoreInterface` and `Symfony\Component\Lock\PersistingStoreInterface`. * `Factory` is deprecated, use `LockFactory` instead + * Deprecated services `lock.store.flock`, `lock.store.semaphore`, `lock.store.memcached.abstract` and `lock.store.redis.abstract`, + use `StoreFactory::createStore` instead. Messenger --------- diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index f0767ab997..375852b4a8 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -28,7 +28,6 @@ use Symfony\Bundle\FrameworkBundle\Routing\RouteLoaderInterface; use Symfony\Bundle\FullStack; use Symfony\Component\Asset\PackageInterface; use Symfony\Component\BrowserKit\AbstractBrowser; -use Symfony\Component\Cache\Adapter\AbstractAdapter; use Symfony\Component\Cache\Adapter\AdapterInterface; use Symfony\Component\Cache\Adapter\ArrayAdapter; use Symfony\Component\Cache\Adapter\ChainAdapter; @@ -75,7 +74,6 @@ use Symfony\Component\Lock\Lock; use Symfony\Component\Lock\LockFactory; use Symfony\Component\Lock\LockInterface; use Symfony\Component\Lock\PersistingStoreInterface; -use Symfony\Component\Lock\Store\FlockStore; use Symfony\Component\Lock\Store\StoreFactory; use Symfony\Component\Lock\StoreInterface; use Symfony\Component\Mailer\Bridge\Amazon\Transport\SesTransportFactory; @@ -1621,42 +1619,13 @@ class FrameworkExtension extends Extension $storeDefinitions = []; foreach ($resourceStores as $storeDsn) { $storeDsn = $container->resolveEnvPlaceholders($storeDsn, null, $usedEnvs); - switch (true) { - case 'flock' === $storeDsn: - $storeDefinition = new Reference('lock.store.flock'); - break; - case 0 === strpos($storeDsn, 'flock://'): - $flockPath = substr($storeDsn, 8); + $storeDefinition = new Definition(PersistingStoreInterface::class); + $storeDefinition->setFactory([StoreFactory::class, 'createStore']); + $storeDefinition->setArguments([$storeDsn]); - $storeDefinitionId = '.lock.flock.store.'.$container->hash($storeDsn); - $container->register($storeDefinitionId, FlockStore::class)->addArgument($flockPath); + $container->setDefinition($storeDefinitionId = '.lock.'.$resourceName.'.store.'.$container->hash($storeDsn), $storeDefinition); - $storeDefinition = new Reference($storeDefinitionId); - break; - case 'semaphore' === $storeDsn: - $storeDefinition = new Reference('lock.store.semaphore'); - break; - case $usedEnvs || preg_match('#^[a-z]++://#', $storeDsn): - if (!$container->hasDefinition($connectionDefinitionId = '.lock_connection.'.$container->hash($storeDsn))) { - $connectionDefinition = new Definition(\stdClass::class); - $connectionDefinition->setPublic(false); - $connectionDefinition->setFactory([AbstractAdapter::class, 'createConnection']); - $connectionDefinition->setArguments([$storeDsn, ['lazy' => true]]); - $container->setDefinition($connectionDefinitionId, $connectionDefinition); - } - - $storeDefinition = new Definition(PersistingStoreInterface::class); - $storeDefinition->setPublic(false); - $storeDefinition->setFactory([StoreFactory::class, 'createStore']); - $storeDefinition->setArguments([new Reference($connectionDefinitionId)]); - - $container->setDefinition($storeDefinitionId = '.lock.'.$resourceName.'.store.'.$container->hash($storeDsn), $storeDefinition); - - $storeDefinition = new Reference($storeDefinitionId); - break; - default: - throw new InvalidArgumentException(sprintf('Lock store DSN "%s" is not valid in resource "%s"', $storeDsn, $resourceName)); - } + $storeDefinition = new Reference($storeDefinitionId); $storeDefinitions[] = $storeDefinition; } diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/lock.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/lock.xml index cdefecd176..a82003c004 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/lock.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/lock.xml @@ -7,16 +7,22 @@ - + + The "%service_id%" service is deprecated since Symfony 4.4 and will be removed in 5.0. + - + + The "%service_id%" service is deprecated since Symfony 4.4 and will be removed in 5.0. + + The "%service_id%" service is deprecated since Symfony 4.4 and will be removed in 5.0. + The "%service_id%" service is deprecated since Symfony 4.4 and will be removed in 5.0. diff --git a/src/Symfony/Component/Lock/CHANGELOG.md b/src/Symfony/Component/Lock/CHANGELOG.md index 0189982baa..b0d0659803 100644 --- a/src/Symfony/Component/Lock/CHANGELOG.md +++ b/src/Symfony/Component/Lock/CHANGELOG.md @@ -7,7 +7,10 @@ CHANGELOG * added InvalidTtlException * deprecated `StoreInterface` in favor of `BlockingStoreInterface` and `PersistingStoreInterface` * `Factory` is deprecated, use `LockFactory` instead - + * `StoreFactory::createStore` allows PDO and Zookeeper DSN. + * deprecated services `lock.store.flock`, `lock.store.semaphore`, `lock.store.memcached.abstract` and `lock.store.redis.abstract`, + use `StoreFactory::createStore` instead. + 4.2.0 ----- diff --git a/src/Symfony/Component/Lock/Store/StoreFactory.php b/src/Symfony/Component/Lock/Store/StoreFactory.php index db354afd8a..255a72a736 100644 --- a/src/Symfony/Component/Lock/Store/StoreFactory.php +++ b/src/Symfony/Component/Lock/Store/StoreFactory.php @@ -58,8 +58,24 @@ class StoreFactory return new FlockStore(substr($connection, 8)); case 'semaphore' === $connection: return new SemaphoreStore(); - case class_exists(AbstractAdapter::class) && preg_match('#^[a-z]++://#', $connection): - return static::createStore(AbstractAdapter::createConnection($connection)); + case 0 === strpos($connection, 'redis://') && class_exists(AbstractAdapter::class): + case 0 === strpos($connection, 'rediss://') && class_exists(AbstractAdapter::class): + return new RedisStore(AbstractAdapter::createConnection($connection, ['lazy' => true])); + case 0 === strpos($connection, 'memcached://') && class_exists(AbstractAdapter::class): + return new MemcachedStore(AbstractAdapter::createConnection($connection, ['lazy' => true])); + case 0 === strpos($connection, 'sqlite:'): + case 0 === strpos($connection, 'mysql:'): + case 0 === strpos($connection, 'pgsql:'): + case 0 === strpos($connection, 'oci:'): + case 0 === strpos($connection, 'sqlsrv:'): + case 0 === strpos($connection, 'sqlite3://'): + case 0 === strpos($connection, 'mysql2://'): + case 0 === strpos($connection, 'postgres://'): + case 0 === strpos($connection, 'postgresql://'): + case 0 === strpos($connection, 'mssql://'): + return new PdoStore($connection); + case 0 === strpos($connection, 'zookeeper://'): + return new ZookeeperStore(ZookeeperStore::createConnection($connection)); default: throw new InvalidArgumentException(sprintf('Unsupported Connection: %s.', $connection)); } diff --git a/src/Symfony/Component/Lock/Store/ZookeeperStore.php b/src/Symfony/Component/Lock/Store/ZookeeperStore.php index 112e1e8d75..cc46db1ff5 100644 --- a/src/Symfony/Component/Lock/Store/ZookeeperStore.php +++ b/src/Symfony/Component/Lock/Store/ZookeeperStore.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Lock\Store; +use Symfony\Component\Lock\Exception\InvalidArgumentException; use Symfony\Component\Lock\Exception\LockAcquiringException; use Symfony\Component\Lock\Exception\LockConflictedException; use Symfony\Component\Lock\Exception\LockReleasingException; @@ -34,6 +35,24 @@ class ZookeeperStore implements StoreInterface $this->zookeeper = $zookeeper; } + public static function createConnection(string $dsn): \Zookeeper + { + if (0 !== strpos($dsn, 'zookeeper:')) { + throw new InvalidArgumentException(sprintf('Unsupported DSN: %s.', $dsn)); + } + + if (false === $params = parse_url($dsn)) { + throw new InvalidArgumentException(sprintf('Invalid Zookeeper DSN: %s.', $dsn)); + } + + $host = $params['host'] ?? ''; + if (isset($params['port'])) { + $host .= ':'.$params['port']; + } + + return new \Zookeeper($host); + } + /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Lock/Tests/Store/StoreFactoryTest.php b/src/Symfony/Component/Lock/Tests/Store/StoreFactoryTest.php index 3291b14f4e..4e5f387988 100644 --- a/src/Symfony/Component/Lock/Tests/Store/StoreFactoryTest.php +++ b/src/Symfony/Component/Lock/Tests/Store/StoreFactoryTest.php @@ -16,6 +16,7 @@ use Symfony\Component\Cache\Adapter\AbstractAdapter; use Symfony\Component\Cache\Traits\RedisProxy; use Symfony\Component\Lock\Store\FlockStore; use Symfony\Component\Lock\Store\MemcachedStore; +use Symfony\Component\Lock\Store\PdoStore; use Symfony\Component\Lock\Store\RedisStore; use Symfony\Component\Lock\Store\SemaphoreStore; use Symfony\Component\Lock\Store\StoreFactory; @@ -50,6 +51,7 @@ class StoreFactoryTest extends TestCase } if (class_exists(\Zookeeper::class)) { yield [$this->createMock(\Zookeeper::class), ZookeeperStore::class]; + yield ['zookeeper://localhost:2181', ZookeeperStore::class]; } if (\extension_loaded('sysvsem')) { yield ['semaphore', SemaphoreStore::class]; @@ -57,6 +59,26 @@ class StoreFactoryTest extends TestCase if (class_exists(\Memcached::class) && class_exists(AbstractAdapter::class)) { yield ['memcached://server.com', MemcachedStore::class]; } + if (class_exists(\Redis::class) && class_exists(AbstractAdapter::class)) { + yield ['redis://localhost', RedisStore::class]; + } + if (class_exists(\PDO::class)) { + yield ['sqlite:/tmp/sqlite.db', PdoStore::class]; + yield ['sqlite::memory:', PdoStore::class]; + yield ['mysql:host=localhost;dbname=test;', PdoStore::class]; + yield ['pgsql:host=localhost;dbname=test;', PdoStore::class]; + yield ['oci:host=localhost;dbname=test;', PdoStore::class]; + yield ['sqlsrv:server=localhost;Database=test', PdoStore::class]; + yield ['mysql://server.com/test', PdoStore::class]; + yield ['mysql2://server.com/test', PdoStore::class]; + yield ['pgsql://server.com/test', PdoStore::class]; + yield ['postgres://server.com/test', PdoStore::class]; + yield ['postgresql://server.com/test', PdoStore::class]; + yield ['sqlite:///tmp/test', PdoStore::class]; + yield ['sqlite3:///tmp/test', PdoStore::class]; + yield ['oci:///server.com/test', PdoStore::class]; + yield ['mssql:///server.com/test', PdoStore::class]; + } yield ['flock', FlockStore::class]; yield ['flock://'.sys_get_temp_dir(), FlockStore::class]; diff --git a/src/Symfony/Component/Lock/Tests/Store/ZookeeperStoreTest.php b/src/Symfony/Component/Lock/Tests/Store/ZookeeperStoreTest.php index 981b62c6a0..b3cd685a23 100644 --- a/src/Symfony/Component/Lock/Tests/Store/ZookeeperStoreTest.php +++ b/src/Symfony/Component/Lock/Tests/Store/ZookeeperStoreTest.php @@ -30,11 +30,26 @@ class ZookeeperStoreTest extends AbstractStoreTest { $zookeeper_server = getenv('ZOOKEEPER_HOST').':2181'; - $zookeeper = new \Zookeeper(implode(',', [$zookeeper_server])); + $zookeeper = new \Zookeeper($zookeeper_server); return StoreFactory::createStore($zookeeper); } + /** + * @dataProvider provideValidConnectionString + */ + public function testCreateConnection(string $connectionString) + { + $this->assertInstanceOf(\Zookeeper::class, ZookeeperStore::createConnection($connectionString)); + } + + public function provideValidConnectionString(): iterable + { + yield 'single host' => ['zookeeper://localhost:2181']; + yield 'single multiple host' => ['zookeeper://localhost:2181,localhost:2181']; + yield 'with extra attributes' => ['zookeeper://localhost:2181/path?option=value']; + } + public function testSaveSucceedsWhenPathContainsMoreThanOneNode() { $store = $this->getStore();