Skip to content

Commit 523bf2a

Browse files
author
Joe Bennett
committed
#37180 don't pass collection query parameter to driver
1 parent d033677 commit 523bf2a

File tree

2 files changed

+87
-32
lines changed

2 files changed

+87
-32
lines changed

src/Symfony/Component/Lock/Store/MongoDbStore.php

+45-24
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ class MongoDbStore implements BlockingStoreInterface
7272
* driverOptions: Array of driver options. [used when $mongo is a URI]
7373
*
7474
* When using a URI string:
75-
* the database is determined from the "database" option, otherwise the uri's path is used.
76-
* the collection is determined from the "collection" option, otherwise the uri's "collection" querystring parameter is used.
75+
* 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.
76+
* The collection is determined from the uri's "collection" querystring parameter, otherwise the "collection" option is used.
7777
*
7878
* For example: mongodb://myuser:mypass@myhost/mydatabase?collection=mycollection
7979
*
@@ -104,34 +104,20 @@ public function __construct($mongo, array $options = [], float $initialTtl = 300
104104
if ($mongo instanceof Collection) {
105105
$this->collection = $mongo;
106106
} elseif ($mongo instanceof Client) {
107-
if (null === $this->options['database']) {
108-
throw new InvalidArgumentException(sprintf('"%s()" requires the "database" option when constructing with a "%s".', __METHOD__, Client::class));
109-
}
110-
if (null === $this->options['collection']) {
111-
throw new InvalidArgumentException(sprintf('"%s()" requires the "collection" option when constructing with a "%s".', __METHOD__, Client::class));
112-
}
113-
114107
$this->client = $mongo;
115108
} elseif (\is_string($mongo)) {
116-
if (false === $parsedUrl = parse_url($mongo)) {
117-
throw new InvalidArgumentException(sprintf('The given MongoDB Connection URI "%s" is invalid.', $mongo));
118-
}
119-
$query = [];
120-
if (isset($parsedUrl['query'])) {
121-
parse_str($parsedUrl['query'], $query);
122-
}
123-
$this->options['collection'] = $query['collection'] ?? $this->options['collection'] ?? null;
124-
$this->options['database'] = ltrim($parsedUrl['path'] ?? '', '/') ?: $this->options['database'] ?? null;
109+
$this->uri = $this->skimUri($mongo);
110+
} else {
111+
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)));
112+
}
113+
114+
if (!($mongo instanceof Collection)) {
125115
if (null === $this->options['database']) {
126-
throw new InvalidArgumentException(sprintf('"%s()" requires the "database" in the URI path or option when constructing with a URI.', __METHOD__));
116+
throw new InvalidArgumentException(sprintf('"%s()" requires the "database" in the URI path or option.', __METHOD__));
127117
}
128118
if (null === $this->options['collection']) {
129-
throw new InvalidArgumentException(sprintf('"%s()" requires the "collection" in the URI querystring or option when constructing with a URI.', __METHOD__));
119+
throw new InvalidArgumentException(sprintf('"%s()" requires the "collection" in the URI querystring or option.', __METHOD__));
130120
}
131-
132-
$this->uri = $mongo;
133-
} else {
134-
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)));
135121
}
136122

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

132+
/**
133+
* Extract default database and collection from given connection uri and remove collection querystring.
134+
*/
135+
private function skimUri(string $uri): string
136+
{
137+
if (false === $parsedUrl = parse_url($uri)) {
138+
throw new InvalidArgumentException(sprintf('The given MongoDB Connection URI "%s" is invalid.', $mongo));
139+
}
140+
141+
$query = [];
142+
if (isset($parsedUrl['query'])) {
143+
parse_str($parsedUrl['query'], $query);
144+
}
145+
146+
if (isset($query['collection'])) {
147+
$this->options['collection'] = $query['collection'];
148+
$queryStringPos = strrpos($uri, $parsedUrl['query']);
149+
unset($query['collection']);
150+
$prefix = substr($uri, 0, $queryStringPos);
151+
$newQuery = http_build_query($query, '', '&', PHP_QUERY_RFC3986);
152+
if (empty($newQuery)) {
153+
$prefix = rtrim($prefix, '?');
154+
}
155+
$suffix = substr($uri, $queryStringPos + \strlen($parsedUrl['query']));
156+
$uri = $prefix.$newQuery.$suffix;
157+
}
158+
159+
$pathDb = ltrim($parsedUrl['path'] ?? '', '/') ?: null;
160+
if (null !== $pathDb) {
161+
$this->options['database'] = $pathDb;
162+
}
163+
164+
return $uri;
165+
}
166+
146167
/**
147168
* Creates a TTL index to automatically remove expired locks.
148169
*

src/Symfony/Component/Lock/Tests/Store/MongoDbStoreTest.php

+42-8
Original file line numberDiff line numberDiff line change
@@ -121,17 +121,22 @@ public function provideConstructorArgs()
121121
yield ['mongodb://localhost/', ['database' => 'test', 'collection' => 'lock']];
122122
}
123123

124-
public function testDsnPrecedence()
124+
public function testUriPrecedence()
125125
{
126126
$client = self::getMongoClient();
127127

128-
$store = new MongoDbStore('mongodb://localhost/test_dsn?collection=lock_dns', ['collection' => 'lock_option', 'database' => 'test_option']);
129-
$r = new \ReflectionObject($store);
130-
$p = $r->getProperty('options');
131-
$p->setAccessible(true);
132-
$options = $p->getValue($store);
133-
$this->assertSame('lock_dns', $options['collection']);
134-
$this->assertSame('test_dsn', $options['database']);
128+
$store = new MongoDbStore('mongodb://localhost/test_uri?collection=lock_uri', [
129+
'database' => 'test_option',
130+
'collection' => 'lock_option',
131+
]);
132+
$storeReflection = new \ReflectionObject($store);
133+
134+
$optionsProperty = $storeReflection->getProperty('options');
135+
$optionsProperty->setAccessible(true);
136+
$options = $optionsProperty->getValue($store);
137+
138+
$this->assertSame('test_uri', $options['database']);
139+
$this->assertSame('lock_uri', $options['collection']);
135140
}
136141

137142
/**
@@ -154,4 +159,33 @@ public function provideInvalidConstructorArgs()
154159
yield ['mongodb://localhost/test', []];
155160
yield ['mongodb://localhost/', []];
156161
}
162+
163+
/**
164+
* @dataProvider provideUriCollectionStripArgs
165+
*/
166+
public function testUriCollectionStrip(string $uri, array $options, string $driverUri)
167+
{
168+
$client = self::getMongoClient();
169+
170+
$store = new MongoDbStore($uri, $options);
171+
$storeReflection = new \ReflectionObject($store);
172+
173+
$uriProperty = $storeReflection->getProperty('uri');
174+
$uriProperty->setAccessible(true);
175+
$uri = $uriProperty->getValue($store);
176+
$this->assertSame($driverUri, $uri);
177+
}
178+
179+
public function provideUriCollectionStripArgs()
180+
{
181+
yield ['mongodb://localhost/?collection=lock', ['database' => 'test'], 'mongodb://localhost/'];
182+
yield ['mongodb://localhost/', ['database' => 'test', 'collection' => 'lock'], 'mongodb://localhost/'];
183+
yield ['mongodb://localhost/test?collection=lock', [], 'mongodb://localhost/test'];
184+
yield ['mongodb://localhost/test', ['collection' => 'lock'], 'mongodb://localhost/test'];
185+
186+
yield ['mongodb://localhost/?collection=lock&replicaSet=repl', ['database' => 'test'], 'mongodb://localhost/?replicaSet=repl'];
187+
yield ['mongodb://localhost/?replicaSet=repl', ['database' => 'test', 'collection' => 'lock'], 'mongodb://localhost/?replicaSet=repl'];
188+
yield ['mongodb://localhost/test?collection=lock&replicaSet=repl', [], 'mongodb://localhost/test?replicaSet=repl'];
189+
yield ['mongodb://localhost/test?replicaSet=repl', ['collection' => 'lock'], 'mongodb://localhost/test?replicaSet=repl'];
190+
}
157191
}

0 commit comments

Comments
 (0)