From 1213aa30134d54a125810b28efeca325ad28d3d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Sun, 29 Oct 2023 21:56:38 +0100 Subject: [PATCH] [HttpFoundation][Lock] Makes MongoDB adapters usable with `ext-mongodb` only --- CHANGELOG.md | 5 + Store/MongoDbStore.php | 130 ++++++++++++++---------- Tests/Store/MongoDbStoreFactoryTest.php | 25 +++-- Tests/Store/MongoDbStoreTest.php | 89 ++++++++++------ Tests/Store/stubs/mongodb.php | 44 ++++++++ 5 files changed, 201 insertions(+), 92 deletions(-) create mode 100644 Tests/Store/stubs/mongodb.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 59b02ce..f386464 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +6.4 +--- + + * Make `MongoDbStore` instantiable with the mongodb extension directly + 6.3 --- diff --git a/Store/MongoDbStore.php b/Store/MongoDbStore.php index ada8438..cab51c6 100644 --- a/Store/MongoDbStore.php +++ b/Store/MongoDbStore.php @@ -14,8 +14,12 @@ use MongoDB\BSON\UTCDateTime; use MongoDB\Client; use MongoDB\Collection; +use MongoDB\Database; +use MongoDB\Driver\BulkWrite; +use MongoDB\Driver\Command; use MongoDB\Driver\Exception\WriteException; -use MongoDB\Driver\ReadPreference; +use MongoDB\Driver\Manager; +use MongoDB\Driver\Query; use MongoDB\Exception\DriverRuntimeException; use MongoDB\Exception\InvalidArgumentException as MongoInvalidArgumentException; use MongoDB\Exception\UnsupportedException; @@ -44,21 +48,22 @@ * @see https://docs.mongodb.com/manual/reference/limits/#Index-Key-Limit * * @author Joe Bennett + * @author Jérôme Tamarelle */ class MongoDbStore implements PersistingStoreInterface { use ExpiringStoreTrait; - private Collection $collection; - private Client $client; + private Manager $manager; + private string $namespace; private string $uri; private array $options; private float $initialTtl; /** - * @param Collection|Client|string $mongo An instance of a Collection or Client or URI @see https://docs.mongodb.com/manual/reference/connection-string/ - * @param array $options See below - * @param float $initialTtl The expiration delay of locks in seconds + * @param Collection|Client|Manager|string $mongo An instance of a Collection or Client or URI @see https://docs.mongodb.com/manual/reference/connection-string/ + * @param array $options See below + * @param float $initialTtl The expiration delay of locks in seconds * * @throws InvalidArgumentException If required options are not provided * @throws InvalidTtlException When the initial ttl is not valid @@ -88,7 +93,7 @@ class MongoDbStore implements PersistingStoreInterface * readPreference is primary for all queries. * @see https://docs.mongodb.com/manual/applications/replication/ */ - public function __construct(Collection|Client|string $mongo, array $options = [], float $initialTtl = 300.0) + public function __construct(Collection|Database|Client|Manager|string $mongo, array $options = [], float $initialTtl = 300.0) { if (isset($options['gcProbablity'])) { trigger_deprecation('symfony/lock', '6.3', 'The "gcProbablity" option (notice the typo in its name) is deprecated in "%s"; use the "gcProbability" option instead.', __CLASS__); @@ -108,21 +113,27 @@ public function __construct(Collection|Client|string $mongo, array $options = [] $this->initialTtl = $initialTtl; if ($mongo instanceof Collection) { - $this->collection = $mongo; + $this->options['database'] ??= $mongo->getDatabaseName(); + $this->options['collection'] ??= $mongo->getCollectionName(); + $this->manager = $mongo->getManager(); + } elseif ($mongo instanceof Database) { + $this->options['database'] ??= $mongo->getDatabaseName(); + $this->manager = $mongo->getManager(); } elseif ($mongo instanceof Client) { - $this->client = $mongo; + $this->manager = $mongo->getManager(); + } elseif ($mongo instanceof Manager) { + $this->manager = $mongo; } else { $this->uri = $this->skimUri($mongo); } - if (!($mongo instanceof Collection)) { - if (null === $this->options['database']) { - throw new InvalidArgumentException(sprintf('"%s()" requires the "database" in the URI path or option.', __METHOD__)); - } - if (null === $this->options['collection']) { - throw new InvalidArgumentException(sprintf('"%s()" requires the "collection" in the URI querystring or option.', __METHOD__)); - } + if (null === $this->options['database']) { + throw new InvalidArgumentException(sprintf('"%s()" requires the "database" in the URI path or option.', __METHOD__)); + } + if (null === $this->options['collection']) { + throw new InvalidArgumentException(sprintf('"%s()" requires the "collection" in the URI querystring or option.', __METHOD__)); } + $this->namespace = $this->options['database'].'.'.$this->options['collection']; if ($this->options['gcProbability'] < 0.0 || $this->options['gcProbability'] > 1.0) { throw new InvalidArgumentException(sprintf('"%s()" gcProbability must be a float from 0.0 to 1.0, "%f" given.', __METHOD__, $this->options['gcProbability'])); @@ -142,6 +153,10 @@ public function __construct(Collection|Client|string $mongo, array $options = [] */ private function skimUri(string $uri): string { + if (!str_starts_with($uri, 'mongodb://') && !str_starts_with($uri, 'mongodb+srv://')) { + throw new InvalidArgumentException(sprintf('The given MongoDB Connection URI "%s" is invalid. Expecting "mongodb://" or "mongodb+srv://".', $uri)); + } + if (false === $parsedUrl = parse_url($uri)) { throw new InvalidArgumentException(sprintf('The given MongoDB Connection URI "%s" is invalid.', $uri)); } @@ -195,14 +210,19 @@ private function skimUri(string $uri): string */ public function createTtlIndex(int $expireAfterSeconds = 0) { - $this->getCollection()->createIndex( - [ // key - 'expires_at' => 1, + $server = $this->getManager()->selectServer(); + $server->executeCommand($this->options['database'], new Command([ + 'createIndexes' => $this->options['collection'], + 'indexes' => [ + [ + 'key' => [ + 'expires_at' => 1, + ], + 'name' => 'expires_at_1', + 'expireAfterSeconds' => $expireAfterSeconds, + ], ], - [ // options - 'expireAfterSeconds' => $expireAfterSeconds, - ] - ); + ])); } /** @@ -257,23 +277,35 @@ public function putOffExpiration(Key $key, float $ttl) */ public function delete(Key $key) { - $this->getCollection()->deleteOne([ // filter - '_id' => (string) $key, - 'token' => $this->getUniqueToken($key), - ]); + $write = new BulkWrite(); + $write->delete( + [ + '_id' => (string) $key, + 'token' => $this->getUniqueToken($key), + ], + ['limit' => 1] + ); + + $this->getManager()->executeBulkWrite($this->namespace, $write); } public function exists(Key $key): bool { - return null !== $this->getCollection()->findOne([ // filter - '_id' => (string) $key, - 'token' => $this->getUniqueToken($key), - 'expires_at' => [ - '$gt' => $this->createMongoDateTime(microtime(true)), + $cursor = $this->manager->executeQuery($this->namespace, new Query( + [ + '_id' => (string) $key, + 'token' => $this->getUniqueToken($key), + 'expires_at' => [ + '$gt' => $this->createMongoDateTime(microtime(true)), + ], ], - ], [ - 'readPreference' => new ReadPreference(\defined(ReadPreference::PRIMARY) ? ReadPreference::PRIMARY : ReadPreference::RP_PRIMARY), - ]); + [ + 'limit' => 1, + 'projection' => ['_id' => 1], + ] + )); + + return [] !== $cursor->toArray(); } /** @@ -286,8 +318,9 @@ private function upsert(Key $key, float $ttl): void $now = microtime(true); $token = $this->getUniqueToken($key); - $this->getCollection()->updateOne( - [ // filter + $write = new BulkWrite(); + $write->update( + [ '_id' => (string) $key, '$or' => [ [ @@ -300,17 +333,19 @@ private function upsert(Key $key, float $ttl): void ], ], ], - [ // update + [ '$set' => [ '_id' => (string) $key, 'token' => $token, 'expires_at' => $this->createMongoDateTime($now + $ttl), ], ], - [ // options + [ 'upsert' => true, ] ); + + $this->getManager()->executeBulkWrite($this->namespace, $write); } private function isDuplicateKeyException(WriteException $e): bool @@ -326,20 +361,9 @@ private function isDuplicateKeyException(WriteException $e): bool return 11000 === $code; } - private function getCollection(): Collection + private function getManager(): Manager { - if (isset($this->collection)) { - return $this->collection; - } - - $this->client ??= new Client($this->uri, $this->options['uriOptions'], $this->options['driverOptions']); - - $this->collection = $this->client->selectCollection( - $this->options['database'], - $this->options['collection'] - ); - - return $this->collection; + return $this->manager ??= new Manager($this->uri, $this->options['uriOptions'], $this->options['driverOptions']); } /** @@ -351,7 +375,7 @@ private function createMongoDateTime(float $seconds): UTCDateTime } /** - * Retrieves an unique token for the given key namespaced to this store. + * Retrieves a unique token for the given key namespaced to this store. * * @param Key $key lock state container */ diff --git a/Tests/Store/MongoDbStoreFactoryTest.php b/Tests/Store/MongoDbStoreFactoryTest.php index 7782f97..aa13197 100644 --- a/Tests/Store/MongoDbStoreFactoryTest.php +++ b/Tests/Store/MongoDbStoreFactoryTest.php @@ -12,12 +12,13 @@ namespace Symfony\Component\Lock\Tests\Store; use MongoDB\Collection; -use MongoDB\Client; -use PHPUnit\Framework\SkippedTestSuiteError; +use MongoDB\Driver\Manager; use PHPUnit\Framework\TestCase; use Symfony\Component\Lock\Store\MongoDbStore; use Symfony\Component\Lock\Store\StoreFactory; +require_once __DIR__.'/stubs/mongodb.php'; + /** * @author Alexandre Daubois * @@ -25,16 +26,20 @@ */ class MongoDbStoreFactoryTest extends TestCase { - public static function setupBeforeClass(): void - { - if (!class_exists(Client::class)) { - throw new SkippedTestSuiteError('The mongodb/mongodb package is required.'); - } - } - public function testCreateMongoDbCollectionStore() { - $store = StoreFactory::createStore($this->createMock(Collection::class)); + $collection = $this->createMock(Collection::class); + $collection->expects($this->once()) + ->method('getManager') + ->willReturn(new Manager()); + $collection->expects($this->once()) + ->method('getCollectionName') + ->willReturn('lock'); + $collection->expects($this->once()) + ->method('getDatabaseName') + ->willReturn('test'); + + $store = StoreFactory::createStore($collection); $this->assertInstanceOf(MongoDbStore::class, $store); } diff --git a/Tests/Store/MongoDbStoreTest.php b/Tests/Store/MongoDbStoreTest.php index 0f60511..92732e0 100644 --- a/Tests/Store/MongoDbStoreTest.php +++ b/Tests/Store/MongoDbStoreTest.php @@ -12,12 +12,18 @@ namespace Symfony\Component\Lock\Tests\Store; use MongoDB\Client; +use MongoDB\Collection; +use MongoDB\Database; +use MongoDB\Driver\Command; use MongoDB\Driver\Exception\ConnectionTimeoutException; +use MongoDB\Driver\Manager; use Symfony\Component\Lock\Exception\InvalidArgumentException; use Symfony\Component\Lock\Key; use Symfony\Component\Lock\PersistingStoreInterface; use Symfony\Component\Lock\Store\MongoDbStore; +require_once __DIR__.'/stubs/mongodb.php'; + /** * @author Joe Bennett * @@ -31,21 +37,18 @@ class MongoDbStoreTest extends AbstractStoreTestCase public static function setupBeforeClass(): void { - if (!class_exists(Client::class)) { - throw new SkippedTestSuiteError('The mongodb/mongodb package is required.'); - } - - $client = self::getMongoClient(); + $manager = self::getMongoManager(); try { - $client->listDatabases(); + $server = $manager->selectServer(); + $server->executeCommand('admin', new Command(['ping' => 1])); } catch (ConnectionTimeoutException $e) { self::markTestSkipped('MongoDB server not found.'); } } - private static function getMongoClient(): Client + private static function getMongoManager(): Manager { - return new Client('mongodb://'.getenv('MONGODB_HOST')); + return new Manager('mongodb://'.getenv('MONGODB_HOST')); } protected function getClockDelay(): int @@ -55,7 +58,7 @@ protected function getClockDelay(): int public function getStore(): PersistingStoreInterface { - return new MongoDbStore(self::getMongoClient(), [ + return new MongoDbStore(self::getMongoManager(), [ 'database' => 'test', 'collection' => 'lock', ]); @@ -66,14 +69,12 @@ public function testCreateIndex() $store = $this->getStore(); $store->createTtlIndex(); - $client = self::getMongoClient(); - $collection = $client->selectCollection( - 'test', - 'lock' - ); + $manager = self::getMongoManager(); + $result = $manager->executeReadCommand('test', new Command(['listIndexes' => 'lock'])); + $indexes = []; - foreach ($collection->listIndexes() as $index) { - $indexes[] = $index->getName(); + foreach ($result as $index) { + $indexes[] = $index->name; } $this->assertContains('expires_at_1', $indexes); } @@ -96,21 +97,53 @@ public function testConstructionMethods($mongo, array $options) public static function provideConstructorArgs() { - $client = self::getMongoClient(); - yield [$client, ['database' => 'test', 'collection' => 'lock']]; - - $collection = $client->selectCollection('test', 'lock'); - yield [$collection, []]; - + yield [self::getMongoManager(), ['database' => 'test', 'collection' => 'lock']]; yield ['mongodb://localhost/test?collection=lock', []]; yield ['mongodb://localhost/test', ['collection' => 'lock']]; yield ['mongodb://localhost/', ['database' => 'test', 'collection' => 'lock']]; } - public function testUriPrecedence() + public function testConstructWithClient() + { + $client = $this->createMock(Client::class); + $client->expects($this->once()) + ->method('getManager') + ->willReturn(self::getMongoManager()); + + $this->testConstructionMethods($client, ['database' => 'test', 'collection' => 'lock']); + } + + public function testConstructWithDatabase() { - $client = self::getMongoClient(); + $database = $this->createMock(Database::class); + $database->expects($this->once()) + ->method('getManager') + ->willReturn(self::getMongoManager()); + $database->expects($this->once()) + ->method('getDatabaseName') + ->willReturn('test'); + + $this->testConstructionMethods($database, ['collection' => 'lock']); + } + public function testConstructWithCollection() + { + $collection = $this->createMock(Collection::class); + $collection->expects($this->once()) + ->method('getManager') + ->willReturn(self::getMongoManager()); + $collection->expects($this->once()) + ->method('getDatabaseName') + ->willReturn('test'); + $collection->expects($this->once()) + ->method('getCollectionName') + ->willReturn('lock'); + + $this->testConstructionMethods($collection, []); + } + + public function testUriPrecedence() + { $store = new MongoDbStore('mongodb://localhost/test_uri?collection=lock_uri', [ 'database' => 'test_option', 'collection' => 'lock_option', @@ -136,9 +169,9 @@ public function testInvalidConstructionMethods($mongo, array $options) public static function provideInvalidConstructorArgs() { - $client = self::getMongoClient(); - yield [$client, ['collection' => 'lock']]; - yield [$client, ['database' => 'test']]; + $manager = self::getMongoManager(); + yield [$manager, ['collection' => 'lock']]; + yield [$manager, ['database' => 'test']]; yield ['mongodb://localhost/?collection=lock', []]; yield ['mongodb://localhost/test', []]; @@ -150,8 +183,6 @@ public static function provideInvalidConstructorArgs() */ public function testUriCollectionStrip(string $uri, array $options, string $driverUri) { - $client = self::getMongoClient(); - $store = new MongoDbStore($uri, $options); $storeReflection = new \ReflectionObject($store); diff --git a/Tests/Store/stubs/mongodb.php b/Tests/Store/stubs/mongodb.php new file mode 100644 index 0000000..7997a9f --- /dev/null +++ b/Tests/Store/stubs/mongodb.php @@ -0,0 +1,44 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace MongoDB; + +use MongoDB\Driver\Manager; + +/* + * Stubs for the mongodb/mongodb library version ~1.16 + */ +if (!class_exists(Client::class)) { + abstract class Client + { + abstract public function getManager(): Manager; + } +} + +if (!class_exists(Database::class)) { + abstract class Database + { + abstract public function getManager(): Manager; + + abstract public function getDatabaseName(): string; + } +} + +if (!class_exists(Collection::class)) { + abstract class Collection + { + abstract public function getManager(): Manager; + + abstract public function getCollectionName(): string; + + abstract public function getDatabaseName(): string; + } +}