Skip to content

Commit

Permalink
bug #37269 [Lock][Messenger] Fix precedence of DSN options for 5.1 (j…
Browse files Browse the repository at this point in the history
…derusse)

This PR was squashed before being merged into the 5.1 branch.

Discussion
----------

[Lock][Messenger] Fix precedence of DSN options for 5.1

| Q             | A
| ------------- | ---
| Branch?       | 5.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #37218 (comment)
| License       | MIT
| Doc PR        | N/A

This PR fix précédence of DSN options over constructor options in all component on branch 5.1

Commits
-------

9670e9f [Lock][Messenger] Fix precedence of DSN options for 5.1
  • Loading branch information
nicolas-grekas committed Jun 18, 2020
2 parents 38c0971 + 9670e9f commit ab24fb9
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 11 deletions.
4 changes: 2 additions & 2 deletions src/Symfony/Component/Lock/Store/MongoDbStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ public function __construct($mongo, array $options = [], float $initialTtl = 300
if (isset($parsedUrl['query'])) {
parse_str($parsedUrl['query'], $query);
}
$this->options['collection'] = $this->options['collection'] ?? $query['collection'] ?? null;
$this->options['database'] = $this->options['database'] ?? ltrim($parsedUrl['path'] ?? '', '/') ?: null;
$this->options['collection'] = $query['collection'] ?? $this->options['collection'] ?? null;
$this->options['database'] = ltrim($parsedUrl['path'] ?? '', '/') ?: $this->options['database'] ?? null;
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__));
}
Expand Down
13 changes: 13 additions & 0 deletions src/Symfony/Component/Lock/Tests/Store/MongoDbStoreTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,19 @@ public function provideConstructorArgs()
yield ['mongodb://localhost/', ['database' => 'test', 'collection' => 'lock']];
}

public function testDsnPrecedence()
{
$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']);
}

/**
* @dataProvider provideInvalidConstructorArgs
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ public function testFromDsn()
);
}

public function testDsnPrecedence()
{
$httpClient = $this->getMockBuilder(HttpClientInterface::class)->getMock();
$this->assertEquals(
new Connection(['queue_name' => 'queue_dsn'], new SqsClient(['region' => 'us-east-2', 'accessKeyId' => 'key_dsn', 'accessKeySecret' => 'secret_dsn'], null, $httpClient)),
Connection::fromDsn('sqs://key_dsn:secret_dsn@default/queue_dsn?region=us-east-2', ['region' => 'eu-west-3', 'queue_name' => 'queue_options', 'access_key' => 'key_option', 'secret_key' => 'secret_option'], $httpClient)
);
}

public function testFromDsnWithRegion()
{
$httpClient = $this->getMockBuilder(HttpClientInterface::class)->getMock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,19 +94,19 @@ public static function fromDsn(string $dsn, array $options = [], HttpClientInter
if (isset($parsedUrl['query'])) {
parse_str($parsedUrl['query'], $query);
}

$options = $query + $options + self::DEFAULT_OPTIONS;
$configuration = [
'buffer_size' => $options['buffer_size'] ?? (int) ($query['buffer_size'] ?? self::DEFAULT_OPTIONS['buffer_size']),
'wait_time' => $options['wait_time'] ?? (int) ($query['wait_time'] ?? self::DEFAULT_OPTIONS['wait_time']),
'poll_timeout' => $options['poll_timeout'] ?? ($query['poll_timeout'] ?? self::DEFAULT_OPTIONS['poll_timeout']),
'visibility_timeout' => $options['visibility_timeout'] ?? ($query['visibility_timeout'] ?? self::DEFAULT_OPTIONS['visibility_timeout']),
'auto_setup' => $options['auto_setup'] ?? (bool) ($query['auto_setup'] ?? self::DEFAULT_OPTIONS['auto_setup']),
'buffer_size' => (int) $options['buffer_size'],
'wait_time' => (int) $options['wait_time'],
'poll_timeout' => $options['poll_timeout'],
'visibility_timeout' => $options['visibility_timeout'],
'auto_setup' => (bool) $options['auto_setup'],
];

$clientConfiguration = [
'region' => $options['region'] ?? ($query['region'] ?? self::DEFAULT_OPTIONS['region']),
'accessKeyId' => $options['access_key'] ?? (urldecode($parsedUrl['user'] ?? '') ?: self::DEFAULT_OPTIONS['access_key']),
'accessKeySecret' => $options['secret_key'] ?? (urldecode($parsedUrl['pass'] ?? '') ?: self::DEFAULT_OPTIONS['secret_key']),
'region' => $options['region'],
'accessKeyId' => urldecode($parsedUrl['user'] ?? '') ?: $options['access_key'] ?? self::DEFAULT_OPTIONS['access_key'],
'accessKeySecret' => urldecode($parsedUrl['pass'] ?? '') ?: $options['secret_key'] ?? self::DEFAULT_OPTIONS['secret_key'],
];
unset($query['region']);

Expand Down

0 comments on commit ab24fb9

Please sign in to comment.