From 845adfb5a3f37fb69523e31787a22c1e3be75a70 Mon Sep 17 00:00:00 2001 From: Joe Bennett Date: Wed, 10 Jun 2020 23:04: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 | 27 ++++---- .../Component/Lock/Store/StoreFactory.php | 2 + .../Lock/Tests/Store/MongoDbStoreTest.php | 63 +++++++++++++++++-- src/Symfony/Component/Lock/composer.json | 1 + 7 files changed, 93 insertions(+), 21 deletions(-) diff --git a/UPGRADE-5.2.md b/UPGRADE-5.2.md index 9828e616b3e76..dbe7cfd215153 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 3c75072a71ffd..0f0b7432c495e 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 66a3acbb76fbb..4307ffd884fa0 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 296c68be1072a..7975a1abda87a 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) @@ -122,13 +117,21 @@ public function __construct($mongo, array $options = [], float $initialTtl = 300 if (isset($parsedUrl['query'])) { parse_str($parsedUrl['query'], $query); } + 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->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; + if (null === $this->options['database'] && null !== $pathDb) { + 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); + } + $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__)); } $this->uri = $mongo; diff --git a/src/Symfony/Component/Lock/Store/StoreFactory.php b/src/Symfony/Component/Lock/Store/StoreFactory.php index 69b53c792eaef..8f9196429c8f7 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 5e4985e352a7f..17373b54a9e13 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/authDb', ['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']]; + yield ['mongodb://localhost/?collection=lock', ['database' => 'test', 'collection' => 'lock']]; } /** @@ -134,11 +184,12 @@ 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', []]; yield ['mongodb://localhost/test', []]; + yield ['mongodb://localhost/?collection=lock', []]; yield ['mongodb://localhost/', []]; } } diff --git a/src/Symfony/Component/Lock/composer.json b/src/Symfony/Component/Lock/composer.json index 6e85ff5160106..3723b9be62aeb 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": {