[Lock] MongoDbStore deprecated extracting database/collection from MongoDB connection URI

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
 ----
 

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
 ------
 

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
 -----
 

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;

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://'):

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', []];
     }
 }

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];
+    }
 }

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": {