Skip to content

Commit a07f841

Browse files
OskarStarkfabpot
OskarStark
authored and
fabpot
committed
[Notifier] Change Dsn api
1 parent f1ab964 commit a07f841

File tree

6 files changed

+171
-84
lines changed

6 files changed

+171
-84
lines changed

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ CHANGELOG
55
-----
66

77
* The component is not marked as `@experimental` anymore
8+
* [BC BREAK] Change signature of `Dsn::__construct()` method from:
9+
`public function __construct(string $scheme, string $host, ?string $user = null, ?string $password = null, ?int $port = null, array $options = [], ?string $path = null)`
10+
to:
11+
`public function __construct(string $dsn)`
12+
* [BC BREAK] Remove `Dsn::fromString()` method
813
* [BC BREAK] Changed the return type of `AbstractTransportFactory::getEndpoint()` from `?string` to `string`
914
* Added `DSN::getRequiredOption` method which throws a new `MissingRequiredOptionException`.
1015

‎Tests/Transport/DsnTest.php‎

Lines changed: 137 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -19,79 +19,136 @@
1919
final class DsnTest extends TestCase
2020
{
2121
/**
22-
* @dataProvider fromStringProvider
22+
* @dataProvider constructProvider
2323
*/
24-
public function testFromString(string $string, Dsn $expectedDsn)
24+
public function testConstruct(string $dsnString, string $scheme, string $host, ?string $user = null, ?string $password = null, ?int $port = null, array $options = [], ?string $path = null)
2525
{
26-
$actualDsn = Dsn::fromString($string);
26+
$dsn = new Dsn($dsnString);
27+
$this->assertSame($dsnString, $dsn->getOriginalDsn());
2728

28-
$this->assertSame($expectedDsn->getScheme(), $actualDsn->getScheme());
29-
$this->assertSame($expectedDsn->getHost(), $actualDsn->getHost());
30-
$this->assertSame($expectedDsn->getPort(), $actualDsn->getPort());
31-
$this->assertSame($expectedDsn->getUser(), $actualDsn->getUser());
32-
$this->assertSame($expectedDsn->getPassword(), $actualDsn->getPassword());
33-
$this->assertSame($expectedDsn->getPath(), $actualDsn->getPath());
34-
$this->assertSame($expectedDsn->getOption('from'), $actualDsn->getOption('from'));
35-
36-
$this->assertSame($string, $actualDsn->getOriginalDsn());
29+
$this->assertSame($scheme, $dsn->getScheme());
30+
$this->assertSame($host, $dsn->getHost());
31+
$this->assertSame($user, $dsn->getUser());
32+
$this->assertSame($password, $dsn->getPassword());
33+
$this->assertSame($port, $dsn->getPort());
34+
$this->assertSame($path, $dsn->getPath());
35+
$this->assertSame($options, $dsn->getOptions());
3736
}
3837

39-
public function fromStringProvider(): iterable
38+
public function constructProvider(): iterable
4039
{
4140
yield 'simple dsn' => [
4241
'scheme://localhost',
43-
new Dsn('scheme', 'localhost', null, null, null, [], null),
42+
'scheme',
43+
'localhost',
4444
];
4545

4646
yield 'simple dsn including @ sign, but no user/password/token' => [
4747
'scheme://@localhost',
48-
new Dsn('scheme', 'localhost', null, null),
48+
'scheme',
49+
'localhost',
4950
];
5051

5152
yield 'simple dsn including : sign and @ sign, but no user/password/token' => [
5253
'scheme://:@localhost',
53-
new Dsn('scheme', 'localhost', null, null),
54+
'scheme',
55+
'localhost',
5456
];
5557

5658
yield 'simple dsn including user, : sign and @ sign, but no password' => [
5759
'scheme://user1:@localhost',
58-
new Dsn('scheme', 'localhost', 'user1', null),
60+
'scheme',
61+
'localhost',
62+
'user1',
5963
];
6064

6165
yield 'simple dsn including : sign, password, and @ sign, but no user' => [
6266
'scheme://:pass@localhost',
63-
new Dsn('scheme', 'localhost', null, 'pass'),
67+
'scheme',
68+
'localhost',
69+
null,
70+
'pass',
6471
];
6572

6673
yield 'dsn with user and pass' => [
6774
'scheme://u$er:pa$s@localhost',
68-
new Dsn('scheme', 'localhost', 'u$er', 'pa$s', null, [], null),
75+
'scheme',
76+
'localhost',
77+
'u$er',
78+
'pa$s',
6979
];
7080

7181
yield 'dsn with user and pass and custom port' => [
7282
'scheme://u$er:pa$s@localhost:8000',
73-
new Dsn('scheme', 'localhost', 'u$er', 'pa$s', '8000', [], null),
83+
'scheme',
84+
'localhost',
85+
'u$er',
86+
'pa$s',
87+
8000,
7488
];
7589

7690
yield 'dsn with user and pass, custom port and custom path' => [
7791
'scheme://u$er:pa$s@localhost:8000/channel',
78-
new Dsn('scheme', 'localhost', 'u$er', 'pa$s', '8000', [], '/channel'),
92+
'scheme',
93+
'localhost',
94+
'u$er',
95+
'pa$s',
96+
8000,
97+
[],
98+
'/channel',
7999
];
80100

81-
yield 'dsn with user and pass, custom port, custom path and custom options' => [
101+
yield 'dsn with user and pass, custom port, custom path and custom option' => [
82102
'scheme://u$er:pa$s@localhost:8000/channel?from=FROM',
83-
new Dsn('scheme', 'localhost', 'u$er', 'pa$s', '8000', ['from' => 'FROM'], '/channel'),
103+
'scheme',
104+
'localhost',
105+
'u$er',
106+
'pa$s',
107+
8000,
108+
[
109+
'from' => 'FROM',
110+
],
111+
'/channel',
112+
];
113+
114+
yield 'dsn with user and pass, custom port, custom path and custom options' => [
115+
'scheme://u$er:pa$s@localhost:8000/channel?from=FROM&to=TO',
116+
'scheme',
117+
'localhost',
118+
'u$er',
119+
'pa$s',
120+
8000,
121+
[
122+
'from' => 'FROM',
123+
'to' => 'TO',
124+
],
125+
'/channel',
126+
];
127+
128+
yield 'dsn with user and pass, custom port, custom path and custom options and custom options keep the same order' => [
129+
'scheme://u$er:pa$s@localhost:8000/channel?to=TO&from=FROM',
130+
'scheme',
131+
'localhost',
132+
'u$er',
133+
'pa$s',
134+
8000,
135+
[
136+
'to' => 'TO',
137+
'from' => 'FROM',
138+
],
139+
'/channel',
84140
];
85141
}
86142

87143
/**
88144
* @dataProvider invalidDsnProvider
89145
*/
90-
public function testInvalidDsn(string $dsn, string $exceptionMessage)
146+
public function testInvalidDsn(string $dsnString, string $exceptionMessage)
91147
{
92148
$this->expectException(InvalidArgumentException::class);
93149
$this->expectExceptionMessage($exceptionMessage);
94-
Dsn::fromString($dsn);
150+
151+
new Dsn($dsnString);
95152
}
96153

97154
public function invalidDsnProvider(): iterable
@@ -112,38 +169,75 @@ public function invalidDsnProvider(): iterable
112169
];
113170
}
114171

115-
public function testGetOption()
172+
/**
173+
* @dataProvider getOptionProvider
174+
*/
175+
public function testGetOption($expected, string $dsnString, string $option, ?string $default = null)
116176
{
117-
$options = ['with_value' => 'some value', 'nullable' => null];
118-
$dsn = new Dsn('scheme', 'localhost', 'u$er', 'pa$s', '8000', $options, '/channel');
177+
$dsn = new Dsn($dsnString);
119178

120-
$this->assertSame('some value', $dsn->getOption('with_value'));
121-
$this->assertSame('default', $dsn->getOption('nullable', 'default'));
122-
$this->assertSame('default', $dsn->getOption('not_existent_property', 'default'));
179+
$this->assertSame($expected, $dsn->getOption($option, $default));
123180
}
124181

125-
public function testGetRequiredOptionGetsOptionIfSet()
182+
public function getOptionProvider(): iterable
126183
{
127-
$options = ['with_value' => 'some value'];
128-
$dsn = new Dsn('scheme', 'localhost', 'u$er', 'pa$s', '8000', $options, '/channel');
184+
yield [
185+
'foo',
186+
'scheme://localhost?with_value=foo',
187+
'with_value',
188+
];
189+
190+
yield [
191+
'',
192+
'scheme://localhost?empty=',
193+
'empty',
194+
];
129195

130-
$this->assertSame('some value', $dsn->getRequiredOption('with_value'));
196+
yield [
197+
'0',
198+
'scheme://localhost?zero=0',
199+
'zero',
200+
];
201+
202+
yield [
203+
'default-value',
204+
'scheme://localhost?option=value',
205+
'non_existent_property',
206+
'default-value',
207+
];
208+
}
209+
210+
/**
211+
* @dataProvider getRequiredOptionProvider
212+
*/
213+
public function testGetRequiredOption(string $expectedValue, string $options, string $option)
214+
{
215+
$dsn = new Dsn(sprintf('scheme://localhost?%s', $options));
216+
217+
$this->assertSame($expectedValue, $dsn->getRequiredOption($option));
131218
}
132219

133-
public function testGetRequiredOptionGetsOptionIfValueIsZero()
220+
public function getRequiredOptionProvider(): iterable
134221
{
135-
$options = ['timeout' => 0];
136-
$dsn = new Dsn('scheme', 'localhost', 'u$er', 'pa$s', '8000', $options, '/channel');
222+
yield [
223+
'value',
224+
'with_value=value',
225+
'with_value',
226+
];
137227

138-
$this->assertSame(0, $dsn->getRequiredOption('timeout'));
228+
yield [
229+
'0',
230+
'timeout=0',
231+
'timeout',
232+
];
139233
}
140234

141235
/**
142236
* @dataProvider getRequiredOptionThrowsMissingRequiredOptionExceptionProvider
143237
*/
144-
public function testGetRequiredOptionThrowsMissingRequiredOptionException(string $expectedExceptionMessage, array $options, string $option)
238+
public function testGetRequiredOptionThrowsMissingRequiredOptionException(string $expectedExceptionMessage, string $options, string $option)
145239
{
146-
$dsn = new Dsn('scheme', 'localhost', 'u$er', 'pa$s', '8000', $options, '/channel');
240+
$dsn = new Dsn(sprintf('scheme://localhost?%s', $options));
147241

148242
$this->expectException(MissingRequiredOptionException::class);
149243
$this->expectExceptionMessage($expectedExceptionMessage);
@@ -155,20 +249,14 @@ public function getRequiredOptionThrowsMissingRequiredOptionExceptionProvider():
155249
{
156250
yield [
157251
'The option "foo_bar" is required but missing.',
158-
['with_value' => 'some value'],
252+
'with_value=value',
159253
'foo_bar',
160254
];
161255

162256
yield [
163257
'The option "with_empty_string" is required but missing.',
164-
['with_empty_string' => ''],
258+
'with_empty_string=',
165259
'with_empty_string',
166260
];
167-
168-
yield [
169-
'The option "with_null" is required but missing.',
170-
['with_null' => null],
171-
'with_null',
172-
];
173261
}
174262
}

‎Tests/Transport/NullTransportFactoryTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@ public function testCreateThrowsUnsupportedSchemeException()
4141
{
4242
$this->expectException(UnsupportedSchemeException::class);
4343

44-
$this->nullTransportFactory->create(new Dsn('foo', ''));
44+
$this->nullTransportFactory->create(new Dsn('foo://localhost'));
4545
}
4646

4747
public function testCreate()
4848
{
4949
$this->assertInstanceOf(
5050
NullTransport::class,
51-
$this->nullTransportFactory->create(new Dsn('null', ''))
51+
$this->nullTransportFactory->create(new Dsn('null://null'))
5252
);
5353
}
5454
}

‎Tests/TransportFactoryTestCase.php‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Notifier\Exception\IncompleteDsnException;
16-
use Symfony\Component\Notifier\Exception\UnsupportedSchemeException;
1716
use Symfony\Component\Notifier\Exception\MissingRequiredOptionException;
17+
use Symfony\Component\Notifier\Exception\UnsupportedSchemeException;
1818
use Symfony\Component\Notifier\Transport\Dsn;
1919
use Symfony\Component\Notifier\Transport\TransportFactoryInterface;
2020

@@ -68,7 +68,7 @@ public function testSupports(bool $expected, string $dsn)
6868
{
6969
$factory = $this->createFactory();
7070

71-
$this->assertSame($expected, $factory->supports(Dsn::fromString($dsn)));
71+
$this->assertSame($expected, $factory->supports(new Dsn($dsn)));
7272
}
7373

7474
/**
@@ -77,7 +77,7 @@ public function testSupports(bool $expected, string $dsn)
7777
public function testCreate(string $expected, string $dsn)
7878
{
7979
$factory = $this->createFactory();
80-
$transport = $factory->create(Dsn::fromString($dsn));
80+
$transport = $factory->create(new Dsn($dsn));
8181

8282
$this->assertSame($expected, (string) $transport);
8383
}
@@ -89,7 +89,7 @@ public function testUnsupportedSchemeException(string $dsn, string $message = nu
8989
{
9090
$factory = $this->createFactory();
9191

92-
$dsn = Dsn::fromString($dsn);
92+
$dsn = new Dsn($dsn);
9393

9494
$this->expectException(UnsupportedSchemeException::class);
9595
if (null !== $message) {
@@ -106,7 +106,7 @@ public function testIncompleteDsnException(string $dsn, string $message = null)
106106
{
107107
$factory = $this->createFactory();
108108

109-
$dsn = Dsn::fromString($dsn);
109+
$dsn = new Dsn($dsn);
110110

111111
$this->expectException(IncompleteDsnException::class);
112112
if (null !== $message) {
@@ -119,11 +119,11 @@ public function testIncompleteDsnException(string $dsn, string $message = null)
119119
/**
120120
* @dataProvider missingRequiredOptionProvider
121121
*/
122-
public function testMissingRequiredOptionException(string $dsn, string $message = null): void
122+
public function testMissingRequiredOptionException(string $dsn, string $message = null)
123123
{
124124
$factory = $this->createFactory();
125125

126-
$dsn = Dsn::fromString($dsn);
126+
$dsn = new Dsn($dsn);
127127

128128
$this->expectException(MissingRequiredOptionException::class);
129129
if (null !== $message) {

‎Transport.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public function fromString(string $dsn): TransportInterface
112112
return new RoundRobinTransport($this->createFromDsns($dsns));
113113
}
114114

115-
return $this->fromDsnObject(Dsn::fromString($dsn));
115+
return $this->fromDsnObject(new Dsn($dsn));
116116
}
117117

118118
public function fromDsnObject(Dsn $dsn): TransportInterface
@@ -133,7 +133,7 @@ private function createFromDsns(array $dsns): array
133133
{
134134
$transports = [];
135135
foreach ($dsns as $dsn) {
136-
$transports[] = $this->fromDsnObject(Dsn::fromString($dsn));
136+
$transports[] = $this->fromDsnObject(new Dsn($dsn));
137137
}
138138

139139
return $transports;

0 commit comments

Comments
 (0)