Skip to content

Commit

Permalink
bug #32392 [Messenger] Doctrine Transport: Support setting auto_setup…
Browse files Browse the repository at this point in the history 
… from DSN (bendavies)

This PR was squashed before being merged into the 4.3 branch (closes #32392).

Discussion
----------

[Messenger] Doctrine Transport: Support setting auto_setup from DSN

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets |    <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        |  <!-- required for new features -->

It was not possible to set `auto_setup` via the dsn, as the result would always be a string, resulting in setup always being performed because a bool is required.
The same was true if `auto_setup` was provided in `options` as a string.

I've fixed ensuring that the final configuration contains a bool for `auto_setup`.

Additionally, constructing the `configuration` was overly complex and hard to grok, so I've hopefully simplified it as part of this PR.

As an aside the three transports all do configuration construction in different ways with varying styles. It would be nice to neaten them up.

Commits
-------

213dfd1 [Messenger] Doctrine Transport: Support setting auto_setup from DSN
  • Loading branch information
@fabpot
fabpot committed Jul 8, 2019
2 parents 4ef1f2f + 213dfd1 commit 94db7d4
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 53 deletions.
114 changes: 70 additions & 44 deletions src/Symfony/Component/Messenger/Tests/Transport/Doctrine/ConnectionTest.php
View file
Expand Up @@ -157,62 +157,88 @@ private function getSchemaSynchronizerMock()
/**
* @dataProvider buildConfigurationProvider
*/
public function testBuildConfiguration($dsn, $options, $expectedManager, $expectedTableName, $expectedRedeliverTimeout, $expectedQueue)
public function testBuildConfiguration($dsn, $options, $expectedConnection, $expectedTableName, $expectedRedeliverTimeout, $expectedQueue, $expectedAutoSetup)
{
$config = Connection::buildConfiguration($dsn, $options);
$this->assertEquals($expectedManager, $config['connection']);
$this->assertEquals($expectedConnection, $config['connection']);
$this->assertEquals($expectedTableName, $config['table_name']);
$this->assertEquals($expectedRedeliverTimeout, $config['redeliver_timeout']);
$this->assertEquals($expectedQueue, $config['queue_name']);
$this->assertEquals($expectedAutoSetup, $config['auto_setup']);
}

public function buildConfigurationProvider()
{
return [
[
'dsn' => 'doctrine://default',
'options' => [],
'expectedManager' => 'default',
'expectedTableName' => 'messenger_messages',
'expectedRedeliverTimeout' => 3600,
'expectedQueue' => 'default',
],
// test options from options array
[
'dsn' => 'doctrine://default',
'options' => [
'table_name' => 'name_from_options',
'redeliver_timeout' => 1800,
'queue_name' => 'important',
],
'expectedManager' => 'default',
'expectedTableName' => 'name_from_options',
'expectedRedeliverTimeout' => 1800,
'expectedQueue' => 'important',
],
// tests options from dsn
[
'dsn' => 'doctrine://default?table_name=name_from_dsn&redeliver_timeout=1200&queue_name=normal',
'options' => [],
'expectedManager' => 'default',
'expectedTableName' => 'name_from_dsn',
'expectedRedeliverTimeout' => 1200,
'expectedQueue' => 'normal',
yield 'no options' => [
'dsn' => 'doctrine://default',
'options' => [],
'expectedConnection' => 'default',
'expectedTableName' => 'messenger_messages',
'expectedRedeliverTimeout' => 3600,
'expectedQueue' => 'default',
'expectedAutoSetup' => true,
];

yield 'test options array' => [
'dsn' => 'doctrine://default',
'options' => [
'table_name' => 'name_from_options',
'redeliver_timeout' => 1800,
'queue_name' => 'important',
'auto_setup' => false,
],
// test options from options array wins over options from dsn
[
'dsn' => 'doctrine://default?table_name=name_from_dsn&redeliver_timeout=1200&queue_name=normal',
'options' => [
'table_name' => 'name_from_options',
'redeliver_timeout' => 1800,
'queue_name' => 'important',
],
'expectedManager' => 'default',
'expectedTableName' => 'name_from_options',
'expectedRedeliverTimeout' => 1800,
'expectedQueue' => 'important',
'expectedConnection' => 'default',
'expectedTableName' => 'name_from_options',
'expectedRedeliverTimeout' => 1800,
'expectedQueue' => 'important',
'expectedAutoSetup' => false,
];

yield 'options from dsn' => [
'dsn' => 'doctrine://default?table_name=name_from_dsn&redeliver_timeout=1200&queue_name=normal&auto_setup=false',
'options' => [],
'expectedConnection' => 'default',
'expectedTableName' => 'name_from_dsn',
'expectedRedeliverTimeout' => 1200,
'expectedQueue' => 'normal',
'expectedAutoSetup' => false,
];

yield 'options from options array wins over options from dsn' => [
'dsn' => 'doctrine://default?table_name=name_from_dsn&redeliver_timeout=1200&queue_name=normal&auto_setup=true',
'options' => [
'table_name' => 'name_from_options',
'redeliver_timeout' => 1800,
'queue_name' => 'important',
'auto_setup' => false,
],
'expectedConnection' => 'default',
'expectedTableName' => 'name_from_options',
'expectedRedeliverTimeout' => 1800,
'expectedQueue' => 'important',
'expectedAutoSetup' => false,
];

yield 'options from dsn with falsey boolean' => [
'dsn' => 'doctrine://default?auto_setup=0',
'options' => [],
'expectedConnection' => 'default',
'expectedTableName' => 'messenger_messages',
'expectedRedeliverTimeout' => 3600,
'expectedQueue' => 'default',
'expectedAutoSetup' => false,
];

yield 'options from dsn with thruthy boolean' => [
'dsn' => 'doctrine://default?auto_setup=1',
'options' => [],
'expectedConnection' => 'default',
'expectedTableName' => 'messenger_messages',
'expectedRedeliverTimeout' => 3600,
'expectedQueue' => 'default',
'expectedAutoSetup' => true,
];

}

/**
Expand Down
15 changes: 6 additions & 9 deletions src/Symfony/Component/Messenger/Transport/Doctrine/Connection.php
View file
Expand Up @@ -76,22 +76,19 @@ public static function buildConfiguration($dsn, array $options = [])
parse_str($components['query'], $query);
}

$configuration = [
'connection' => $components['host'],
'table_name' => $options['table_name'] ?? ($query['table_name'] ?? self::DEFAULT_OPTIONS['table_name']),
'queue_name' => $options['queue_name'] ?? ($query['queue_name'] ?? self::DEFAULT_OPTIONS['queue_name']),
'redeliver_timeout' => $options['redeliver_timeout'] ?? ($query['redeliver_timeout'] ?? self::DEFAULT_OPTIONS['redeliver_timeout']),
'auto_setup' => $options['auto_setup'] ?? ($query['auto_setup'] ?? self::DEFAULT_OPTIONS['auto_setup']),
];
$configuration = ['connection' => $components['host']];
$configuration += $options + $query + self::DEFAULT_OPTIONS;

$configuration['auto_setup'] = filter_var($configuration['auto_setup'], FILTER_VALIDATE_BOOLEAN);

// check for extra keys in options
$optionsExtraKeys = array_diff(array_keys($options), array_keys($configuration));
$optionsExtraKeys = array_diff(array_keys($options), array_keys(self::DEFAULT_OPTIONS));
if (0 < \count($optionsExtraKeys)) {
throw new InvalidArgumentException(sprintf('Unknown option found : [%s]. Allowed options are [%s]', implode(', ', $optionsExtraKeys), implode(', ', self::DEFAULT_OPTIONS)));
}

// check for extra keys in options
$queryExtraKeys = array_diff(array_keys($query), array_keys($configuration));
$queryExtraKeys = array_diff(array_keys($query), array_keys(self::DEFAULT_OPTIONS));
if (0 < \count($queryExtraKeys)) {
throw new InvalidArgumentException(sprintf('Unknown option found in DSN: [%s]. Allowed options are [%s]', implode(', ', $queryExtraKeys), implode(', ', self::DEFAULT_OPTIONS)));
}
Expand Down

0 comments on commit 94db7d4

Please sign in to comment.