Skip to content

Commit

Permalink
#37180 don't pass collection query parameter to driver
Browse files Browse the repository at this point in the history
  • Loading branch information
Joe Bennett committed Jun 18, 2020
1 parent d033677 commit 523bf2a
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 32 deletions.
69 changes: 45 additions & 24 deletions src/Symfony/Component/Lock/Store/MongoDbStore.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ class MongoDbStore implements BlockingStoreInterface
* 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.
* The database is determined from the uri's path, otherwise the "database" option is used. To specify an alternate authentication database; "authSource" uriOption or querystring parameter must be used.
* The collection is determined from the uri's "collection" querystring parameter, otherwise the "collection" option is used.
*
* For example: mongodb://myuser:mypass@myhost/mydatabase?collection=mycollection
*
Expand Down Expand Up @@ -104,34 +104,20 @@ public function __construct($mongo, array $options = [], float $initialTtl = 300
if ($mongo instanceof Collection) {
$this->collection = $mongo;
} elseif ($mongo instanceof Client) {
if (null === $this->options['database']) {
throw new InvalidArgumentException(sprintf('"%s()" requires the "database" option when constructing with a "%s".', __METHOD__, Client::class));
}
if (null === $this->options['collection']) {
throw new InvalidArgumentException(sprintf('"%s()" requires the "collection" option when constructing with a "%s".', __METHOD__, Client::class));
}

$this->client = $mongo;
} elseif (\is_string($mongo)) {
if (false === $parsedUrl = parse_url($mongo)) {
throw new InvalidArgumentException(sprintf('The given MongoDB Connection URI "%s" is invalid.', $mongo));
}
$query = [];
if (isset($parsedUrl['query'])) {
parse_str($parsedUrl['query'], $query);
}
$this->options['collection'] = $query['collection'] ?? $this->options['collection'] ?? null;
$this->options['database'] = ltrim($parsedUrl['path'] ?? '', '/') ?: $this->options['database'] ?? null;
$this->uri = $this->skimUri($mongo);
} else {
throw new InvalidArgumentException(sprintf('"%s()" requires "%s" or "%s" or URI as first argument, "%s" given.', __METHOD__, Collection::class, Client::class, get_debug_type($mongo)));
}

if (!($mongo instanceof Collection)) {
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" 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 when constructing with a URI.', __METHOD__));
throw new InvalidArgumentException(sprintf('"%s()" requires the "collection" in the URI querystring or option.', __METHOD__));
}

$this->uri = $mongo;
} else {
throw new InvalidArgumentException(sprintf('"%s()" requires "%s" or "%s" or URI as first argument, "%s" given.', __METHOD__, Collection::class, Client::class, get_debug_type($mongo)));
}

if ($this->options['gcProbablity'] < 0.0 || $this->options['gcProbablity'] > 1.0) {
Expand All @@ -143,6 +129,41 @@ public function __construct($mongo, array $options = [], float $initialTtl = 300
}
}

/**
* Extract default database and collection from given connection uri and remove collection querystring.
*/
private function skimUri(string $uri): string
{
if (false === $parsedUrl = parse_url($uri)) {
throw new InvalidArgumentException(sprintf('The given MongoDB Connection URI "%s" is invalid.', $mongo));
}

$query = [];
if (isset($parsedUrl['query'])) {
parse_str($parsedUrl['query'], $query);
}

if (isset($query['collection'])) {
$this->options['collection'] = $query['collection'];
$queryStringPos = strrpos($uri, $parsedUrl['query']);
unset($query['collection']);
$prefix = substr($uri, 0, $queryStringPos);
$newQuery = http_build_query($query, '', '&', PHP_QUERY_RFC3986);
if (empty($newQuery)) {
$prefix = rtrim($prefix, '?');
}
$suffix = substr($uri, $queryStringPos + \strlen($parsedUrl['query']));
$uri = $prefix.$newQuery.$suffix;
}

$pathDb = ltrim($parsedUrl['path'] ?? '', '/') ?: null;
if (null !== $pathDb) {
$this->options['database'] = $pathDb;
}

return $uri;
}

/**
* Creates a TTL index to automatically remove expired locks.
*
Expand Down
50 changes: 42 additions & 8 deletions src/Symfony/Component/Lock/Tests/Store/MongoDbStoreTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,22 @@ public function provideConstructorArgs()
yield ['mongodb://localhost/', ['database' => 'test', 'collection' => 'lock']];
}

public function testDsnPrecedence()
public function testUriPrecedence()
{
$client = self::getMongoClient();

$store = new MongoDbStore('mongodb://localhost/test_dsn?collection=lock_dns', ['collection' => 'lock_option', 'database' => 'test_option']);
$r = new \ReflectionObject($store);
$p = $r->getProperty('options');
$p->setAccessible(true);
$options = $p->getValue($store);
$this->assertSame('lock_dns', $options['collection']);
$this->assertSame('test_dsn', $options['database']);
$store = new MongoDbStore('mongodb://localhost/test_uri?collection=lock_uri', [
'database' => 'test_option',
'collection' => 'lock_option',
]);
$storeReflection = new \ReflectionObject($store);

$optionsProperty = $storeReflection->getProperty('options');
$optionsProperty->setAccessible(true);
$options = $optionsProperty->getValue($store);

$this->assertSame('test_uri', $options['database']);
$this->assertSame('lock_uri', $options['collection']);
}

/**
Expand All @@ -154,4 +159,33 @@ public function provideInvalidConstructorArgs()
yield ['mongodb://localhost/test', []];
yield ['mongodb://localhost/', []];
}

/**
* @dataProvider provideUriCollectionStripArgs
*/
public function testUriCollectionStrip(string $uri, array $options, string $driverUri)
{
$client = self::getMongoClient();

$store = new MongoDbStore($uri, $options);
$storeReflection = new \ReflectionObject($store);

$uriProperty = $storeReflection->getProperty('uri');
$uriProperty->setAccessible(true);
$uri = $uriProperty->getValue($store);
$this->assertSame($driverUri, $uri);
}

public function provideUriCollectionStripArgs()
{
yield ['mongodb://localhost/?collection=lock', ['database' => 'test'], 'mongodb://localhost/'];
yield ['mongodb://localhost/', ['database' => 'test', 'collection' => 'lock'], 'mongodb://localhost/'];
yield ['mongodb://localhost/test?collection=lock', [], 'mongodb://localhost/test'];
yield ['mongodb://localhost/test', ['collection' => 'lock'], 'mongodb://localhost/test'];

yield ['mongodb://localhost/?collection=lock&replicaSet=repl', ['database' => 'test'], 'mongodb://localhost/?replicaSet=repl'];
yield ['mongodb://localhost/?replicaSet=repl', ['database' => 'test', 'collection' => 'lock'], 'mongodb://localhost/?replicaSet=repl'];
yield ['mongodb://localhost/test?collection=lock&replicaSet=repl', [], 'mongodb://localhost/test?replicaSet=repl'];
yield ['mongodb://localhost/test?replicaSet=repl', ['collection' => 'lock'], 'mongodb://localhost/test?replicaSet=repl'];
}
}

0 comments on commit 523bf2a

Please sign in to comment.