From fd508ab03f024153ce7d9e1c6526d5c13c8e7991 Mon Sep 17 00:00:00 2001 From: Joe Bennett Date: Thu, 11 Jun 2020 00:01:31 +1000 Subject: [PATCH] #37180 deprecated extracting default database or collection from MongoDB connection URI --- UPGRADE-5.2.md | 5 ++ UPGRADE-6.0.md | 5 ++ src/Symfony/Component/Lock/CHANGELOG.md | 11 ++- .../Component/Lock/Store/MongoDbStore.php | 28 ++++---- .../Component/Lock/Store/StoreFactory.php | 2 + .../Lock/Tests/Store/MongoDbStoreTest.php | 67 +++++++++++++++++-- .../Lock/Tests/Store/StoreFactoryTest.php | 22 +++++- src/Symfony/Component/Lock/composer.json | 1 + 8 files changed, 119 insertions(+), 22 deletions(-) diff --git a/UPGRADE-5.2.md b/UPGRADE-5.2.md index 9828e616b3e7..dbe7cfd21515 100644 --- a/UPGRADE-5.2.md +++ b/UPGRADE-5.2.md @@ -1,6 +1,11 @@ UPGRADE FROM 5.1 to 5.2 ======================= +Lock +---- + + * Deprecated passing of `database` or `collection` to `MongoDbStore` via connection URI, use `$options` instead. + Mime ---- diff --git a/UPGRADE-6.0.md b/UPGRADE-6.0.md index 3c75072a71ff..0f0b7432c495 100644 --- a/UPGRADE-6.0.md +++ b/UPGRADE-6.0.md @@ -73,6 +73,11 @@ Inflector * The component has been removed, use `EnglishInflector` from the String component instead. +Lock +---- + +* `MongoDbStore` can no longer be constructed by passing `database` or `collection` via connection URI, use `$options` instead. + Mailer ------ diff --git a/src/Symfony/Component/Lock/CHANGELOG.md b/src/Symfony/Component/Lock/CHANGELOG.md index 66a3acbb76fb..4307ffd884fa 100644 --- a/src/Symfony/Component/Lock/CHANGELOG.md +++ b/src/Symfony/Component/Lock/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +5.2.0 +----- + + * deprecated passing of `database` or `collection` to `MongoDbStore` via connection URI, use `$options` instead. + 5.1.0 ----- @@ -19,10 +24,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`, + * `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/MongoDbStore.php b/src/Symfony/Component/Lock/Store/MongoDbStore.php index 296c68be1072..c2e2c89079d9 100644 --- a/src/Symfony/Component/Lock/Store/MongoDbStore.php +++ b/src/Symfony/Component/Lock/Store/MongoDbStore.php @@ -68,18 +68,12 @@ class MongoDbStore implements BlockingStoreInterface * * Options: * gcProbablity: Should a TTL Index be created expressed as a probability from 0.0 to 1.0 [default: 0.001] - * database: The name of the database [required when $mongo is a Client] - * collection: The name of the collection [required when $mongo is a Client] + * database: The name of the database [required when $mongo is not a Collection] + * collection: The name of the collection [required when $mongo is not a Collection] * uriOptions: Array of uri options. [used when $mongo is a URI] * driverOptions: Array of driver options. [used when $mongo is a URI] * - * When using a URI string: - * the database is determined from the "database" option, otherwise the uri's path is used. - * the collection is determined from the "collection" option, otherwise the uri's "collection" querystring parameter is used. - * - * For example: mongodb://myuser:mypass@myhost/mydatabase?collection=mycollection - * - * @see https://docs.mongodb.com/php-library/current/reference/method/MongoDBClient__construct/ + * For uriOptions and driverOptions @see https://docs.mongodb.com/php-library/current/reference/method/MongoDBClient__construct/ * * If gcProbablity is set to a value greater than 0.0 there is a chance * this store will attempt to create a TTL index on self::save(). @@ -89,6 +83,7 @@ class MongoDbStore implements BlockingStoreInterface * * writeConcern, readConcern and readPreference are not specified by * MongoDbStore meaning the collection's settings will take effect. + * * @see https://docs.mongodb.com/manual/applications/replication/ */ public function __construct($mongo, array $options = [], float $initialTtl = 300.0) @@ -123,12 +118,21 @@ public function __construct($mongo, array $options = [], float $initialTtl = 300 parse_str($parsedUrl['query'], $query); } $this->options['collection'] = $this->options['collection'] ?? $query['collection'] ?? null; - $this->options['database'] = $this->options['database'] ?? ltrim($parsedUrl['path'] ?? '', '/') ?: null; + $authSource = $this->options['uriOptions']['authSource'] ?? $query['authSource'] ?? null; + $pathDb = ltrim($parsedUrl['path'] ?? '', '/') ?: null; + $databaseDerivedFromPath = null === $this->options['database'] && null !== $pathDb && $authSource !== $pathDb; + $this->options['database'] = $this->options['database'] ?? $pathDb; if (null === $this->options['database']) { - throw new InvalidArgumentException(sprintf('"%s()" requires the "database" in the URI path or option when constructing with a URI.', __METHOD__)); + throw new InvalidArgumentException(sprintf('"%s()" requires the "database" to be passed via $options or the URI path.', __METHOD__)); } if (null === $this->options['collection']) { - throw new InvalidArgumentException(sprintf('"%s()" requires the "collection" in the URI querystring or option when constructing with a URI.', __METHOD__)); + throw new InvalidArgumentException(sprintf('"%s()" requires the "collection" to be passed via $options or the URI querystring.', __METHOD__)); + } + if ($databaseDerivedFromPath) { + trigger_deprecation('symfony/lock', '5.2', 'Constructing a "%s" by passing the "database" via a connection URI is deprecated. Either contruct with a "%s" or pass it via $options instead.', __CLASS__, Collection::class); + } + if (isset($query['collection'])) { + trigger_deprecation('symfony/lock', '5.2', 'Constructing a "%s" by passing the "collection" via a connection URI is deprecated. Either contruct with a "%s" or pass it via $options instead.', __CLASS__, Collection::class); } $this->uri = $mongo; diff --git a/src/Symfony/Component/Lock/Store/StoreFactory.php b/src/Symfony/Component/Lock/Store/StoreFactory.php index 69b53c792eae..8f9196429c8f 100644 --- a/src/Symfony/Component/Lock/Store/StoreFactory.php +++ b/src/Symfony/Component/Lock/Store/StoreFactory.php @@ -81,6 +81,8 @@ public static function createStore($connection) return new $storeClass($connection); case 0 === strpos($connection, 'mongodb'): + trigger_deprecation('symfony/lock', '5.2', 'Using "%s" to construct a "%s" with a connection URI string is deprecated. Use a "%s" instead.', __CLASS__, MongoDbStore::class, Collection::class); + return new MongoDbStore($connection); case 0 === strpos($connection, 'mssql://'): diff --git a/src/Symfony/Component/Lock/Tests/Store/MongoDbStoreTest.php b/src/Symfony/Component/Lock/Tests/Store/MongoDbStoreTest.php index 5e4985e352a7..fb5945c4af84 100644 --- a/src/Symfony/Component/Lock/Tests/Store/MongoDbStoreTest.php +++ b/src/Symfony/Component/Lock/Tests/Store/MongoDbStoreTest.php @@ -13,6 +13,7 @@ use MongoDB\Client; use MongoDB\Driver\Exception\ConnectionTimeoutException; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Lock\Exception\InvalidArgumentException; use Symfony\Component\Lock\Exception\NotSupportedException; use Symfony\Component\Lock\Key; @@ -27,6 +28,7 @@ */ class MongoDbStoreTest extends AbstractStoreTest { + use ExpectDeprecationTrait; use ExpiringStoreTestTrait; public static function setupBeforeClass(): void @@ -97,10 +99,10 @@ public function testNonBlocking() */ public function testConstructionMethods($mongo, array $options) { - $key = new Key(uniqid(__METHOD__, true)); - $store = new MongoDbStore($mongo, $options); + $key = new Key(uniqid(__METHOD__, true)); + $store->save($key); $this->assertTrue($store->exists($key)); @@ -116,9 +118,57 @@ public function provideConstructorArgs() $collection = $client->selectCollection('test', 'lock'); yield [$collection, []]; - yield ['mongodb://localhost/test?collection=lock', []]; - yield ['mongodb://localhost/test', ['collection' => 'lock']]; yield ['mongodb://localhost/', ['database' => 'test', 'collection' => 'lock']]; + yield ['mongodb://localhost/test', ['database' => 'test', 'collection' => 'lock']]; + } + + /** + * @dataProvider provideDeprecatedDatabaseConstructorArgs + * @group legacy + */ + public function testDeprecatedDatabaseConstructionMethods($mongo, array $options) + { + $this->expectDeprecation('Since symfony/lock 5.2: Constructing a "Symfony\Component\Lock\Store\MongoDbStore" by passing the "database" via a connection URI is deprecated. Either contruct with a "MongoDB\Collection" or pass it via $options instead.'); + + $store = new MongoDbStore($mongo, $options); + + $key = new Key(uniqid(__METHOD__, true)); + + $store->save($key); + $this->assertTrue($store->exists($key)); + + $store->delete($key); + $this->assertFalse($store->exists($key)); + } + + public function provideDeprecatedDatabaseConstructorArgs() + { + yield ['mongodb://localhost/test', ['collection' => 'lock']]; + } + + /** + * @dataProvider provideDeprecatedCollectionConstructorArgs + * @group legacy + */ + public function testDeprecatedCollectionConstructionMethods($mongo, array $options) + { + $this->expectDeprecation('Since symfony/lock 5.2: Constructing a "Symfony\Component\Lock\Store\MongoDbStore" by passing the "collection" via a connection URI is deprecated. Either contruct with a "MongoDB\Collection" or pass it via $options instead.'); + + $store = new MongoDbStore($mongo, $options); + + $key = new Key(uniqid(__METHOD__, true)); + + $store->save($key); + $this->assertTrue($store->exists($key)); + + $store->delete($key); + $this->assertFalse($store->exists($key)); + } + + public function provideDeprecatedCollectionConstructorArgs() + { + yield ['mongodb://localhost/?collection=lock', ['database' => 'test', 'collection' => 'lock']]; + yield ['mongodb://localhost/?collection=lock', ['database' => 'test']]; } /** @@ -134,11 +184,16 @@ public function testInvalidConstructionMethods($mongo, array $options) public function provideInvalidConstructorArgs() { $client = self::getMongoClient(); - yield [$client, ['collection' => 'lock']]; + yield [$client, []]; yield [$client, ['database' => 'test']]; + yield [$client, ['collection' => 'lock']]; + yield ['mongodb://localhost/?collection=lock', ['collection' => 'lock']]; yield ['mongodb://localhost/?collection=lock', []]; - yield ['mongodb://localhost/test', []]; + yield ['mongodb://localhost/', ['collection' => 'lock']]; + yield ['mongodb://localhost/', ['database' => 'test']]; yield ['mongodb://localhost/', []]; + yield ['mongodb://localhost/test', ['database' => 'test']]; + yield ['mongodb://localhost/test', []]; } } diff --git a/src/Symfony/Component/Lock/Tests/Store/StoreFactoryTest.php b/src/Symfony/Component/Lock/Tests/Store/StoreFactoryTest.php index 7ef02cb4a209..0f4ba41e9ffb 100644 --- a/src/Symfony/Component/Lock/Tests/Store/StoreFactoryTest.php +++ b/src/Symfony/Component/Lock/Tests/Store/StoreFactoryTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Lock\Tests\Store; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Cache\Adapter\AbstractAdapter; use Symfony\Component\Cache\Traits\RedisProxy; use Symfony\Component\Lock\Store\FlockStore; @@ -28,6 +29,8 @@ */ class StoreFactoryTest extends TestCase { + use ExpectDeprecationTrait; + /** * @dataProvider validConnections */ @@ -52,7 +55,6 @@ public function validConnections() } if (class_exists(\MongoDB\Collection::class)) { yield [$this->createMock(\MongoDB\Collection::class), MongoDbStore::class]; - yield ['mongodb://localhost/test?collection=lock', MongoDbStore::class]; } if (class_exists(\Zookeeper::class)) { yield [$this->createMock(\Zookeeper::class), ZookeeperStore::class]; @@ -88,4 +90,22 @@ public function validConnections() yield ['flock', FlockStore::class]; yield ['flock://'.sys_get_temp_dir(), FlockStore::class]; } + + /** + * @dataProvider deprecatedConnections + * @group legacy + */ + public function testDeprecatedCreateStore($connection, string $expectedStoreClass) + { + $this->expectDeprecation('Since symfony/lock 5.2: Using "Symfony\Component\Lock\Store\StoreFactory" to construct a "Symfony\Component\Lock\Store\MongoDbStore" with a connection URI string is deprecated. Use a "Symfony\Component\Lock\Store\Collection" instead.'); + + $store = StoreFactory::createStore($connection); + + $this->assertInstanceOf($expectedStoreClass, $store); + } + + public function deprecatedConnections() + { + yield ['mongodb://localhost/test?collection=lock', MongoDbStore::class]; + } } diff --git a/src/Symfony/Component/Lock/composer.json b/src/Symfony/Component/Lock/composer.json index 6e85ff516010..3723b9be62ae 100644 --- a/src/Symfony/Component/Lock/composer.json +++ b/src/Symfony/Component/Lock/composer.json @@ -18,6 +18,7 @@ "require": { "php": ">=7.2.5", "psr/log": "~1.0", + "symfony/deprecation-contracts": "^2.1", "symfony/polyfill-php80": "^1.15" }, "require-dev": {