Skip to content

Commit d22f78a

Browse files
MatTheCatMatTheCat
committed
Deprecate RouteCollection overriding its routes' properties
1 parent 3c2566c commit d22f78a

File tree

4 files changed

+189
-41
lines changed

4 files changed

+189
-41
lines changed

‎UPGRADE-7.4.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
UPGRADE FROM 7.3 to 7.4
2+
=======================
3+
4+
Symfony 7.4 is a minor release. According to the Symfony release process, there should be no significant
5+
backward compatibility breaks. Minor backward compatibility breaks are prefixed in this document with
6+
`[BC BREAK]`, make sure your code is compatible with these entries before upgrading.
7+
Read more about this in the [Symfony documentation](https://symfony.com/doc/7.4/setup/upgrade_minor.html).
8+
9+
If you're upgrading from a version below 7.3, follow the [7.3 upgrade guide](UPGRADE-7.3.md) first.
10+
11+
Routing
12+
-------
13+
14+
* Deprecate `RouteCollection` overriding its routes' properties

‎src/Symfony/Component/Routing/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
7.4
5+
---
6+
7+
* Deprecate `RouteCollection` overriding its routes' properties
8+
49
7.3
510
---
611

‎src/Symfony/Component/Routing/RouteCollection.php

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,15 @@ public function addNamePrefix(string $prefix): void
242242
public function setHost(?string $pattern, array $defaults = [], array $requirements = []): void
243243
{
244244
foreach ($this->routes as $route) {
245+
if ('' !== $route->getHost() && $route->getHost() !== $pattern) {
246+
trigger_deprecation('symfony/routing', '7.3', 'Overriding a route\'s host with its group\'s is deprecated, you should remove it from the route.');
247+
}
248+
245249
$route->setHost($pattern);
246-
$route->addDefaults($defaults);
247-
$route->addRequirements($requirements);
248250
}
251+
252+
$this->addDefaults($defaults);
253+
$this->addRequirements($requirements);
249254
}
250255

251256
/**
@@ -256,6 +261,10 @@ public function setHost(?string $pattern, array $defaults = [], array $requireme
256261
public function setCondition(?string $condition): void
257262
{
258263
foreach ($this->routes as $route) {
264+
if ('' !== $route->getCondition() && $route->getCondition() !== $condition) {
265+
trigger_deprecation('symfony/routing', '7.3', 'Overriding a route\'s condition with its group\'s is deprecated, you should remove it from the route.');
266+
}
267+
259268
$route->setCondition($condition);
260269
}
261270
}
@@ -267,10 +276,18 @@ public function setCondition(?string $condition): void
267276
*/
268277
public function addDefaults(array $defaults): void
269278
{
270-
if ($defaults) {
271-
foreach ($this->routes as $route) {
272-
$route->addDefaults($defaults);
279+
if (!$defaults) {
280+
return;
281+
}
282+
283+
foreach ($this->routes as $route) {
284+
foreach ($defaults as $name => $value) {
285+
if ($route->hasDefault($name) && $route->getDefault($name) !== $value) {
286+
trigger_deprecation('symfony/routing', '7.3', 'Overriding a route\'s default with its group\'s is deprecated, you should remove it from the route.');
287+
}
273288
}
289+
290+
$route->addDefaults($defaults);
274291
}
275292
}
276293

@@ -281,10 +298,18 @@ public function addDefaults(array $defaults): void
281298
*/
282299
public function addRequirements(array $requirements): void
283300
{
284-
if ($requirements) {
285-
foreach ($this->routes as $route) {
286-
$route->addRequirements($requirements);
301+
if (!$requirements) {
302+
return;
303+
}
304+
305+
foreach ($this->routes as $route) {
306+
foreach ($requirements as $key => $regex) {
307+
if ($route->hasRequirement($key) && $route->getRequirement($key) !== $regex) {
308+
trigger_deprecation('symfony/routing', '7.3', 'Overriding a route\'s requirement with its group\'s is deprecated, you should remove it from the route.');
309+
}
287310
}
311+
312+
$route->addRequirements($requirements);
288313
}
289314
}
290315

@@ -295,10 +320,18 @@ public function addRequirements(array $requirements): void
295320
*/
296321
public function addOptions(array $options): void
297322
{
298-
if ($options) {
299-
foreach ($this->routes as $route) {
300-
$route->addOptions($options);
323+
if (!$options) {
324+
return;
325+
}
326+
327+
foreach ($this->routes as $route) {
328+
foreach ($options as $name => $value) {
329+
if ($route->hasOption($name) && $route->getOption($name) !== $value) {
330+
trigger_deprecation('symfony/routing', '7.3', 'Overriding a route\'s option with its group\'s is deprecated, you should remove it from the route.');
331+
}
301332
}
333+
334+
$route->addOptions($options);
302335
}
303336
}
304337

@@ -310,6 +343,10 @@ public function addOptions(array $options): void
310343
public function setSchemes(string|array $schemes): void
311344
{
312345
foreach ($this->routes as $route) {
346+
if ($route->getSchemes() !== [] && $route->getSchemes() != (array) $schemes) {
347+
trigger_deprecation('symfony/routing', '7.3', 'Overriding a route\'s schemes with its group\'s is deprecated, you should remove it from the route.');
348+
}
349+
313350
$route->setSchemes($schemes);
314351
}
315352
}
@@ -322,6 +359,10 @@ public function setSchemes(string|array $schemes): void
322359
public function setMethods(string|array $methods): void
323360
{
324361
foreach ($this->routes as $route) {
362+
if ($route->getMethods() !== [] && $route->getMethods() != (array) $methods) {
363+
trigger_deprecation('symfony/routing', '7.3', 'Overriding a route\'s methods with its group\'s is deprecated, you should remove it from the route.');
364+
}
365+
325366
$route->setMethods($methods);
326367
}
327368
}

‎src/Symfony/Component/Routing/Tests/RouteCollectionTest.php

Lines changed: 118 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,15 @@
1212
namespace Symfony\Component\Routing\Tests;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Bridge\PhpUnit\ExpectUserDeprecationMessageTrait;
1516
use Symfony\Component\Config\Resource\FileResource;
1617
use Symfony\Component\Routing\Route;
1718
use Symfony\Component\Routing\RouteCollection;
1819

1920
class RouteCollectionTest extends TestCase
2021
{
22+
use ExpectUserDeprecationMessageTrait;
23+
2124
public function testRoute()
2225
{
2326
$collection = new RouteCollection();
@@ -115,27 +118,65 @@ public function testAddCollectionWithResources()
115118
public function testAddDefaultsAndRequirementsAndOptions()
116119
{
117120
$collection = new RouteCollection();
118-
$collection->add('foo', new Route('/{placeholder}'));
119121
$collection1 = new RouteCollection();
120-
$collection1->add('bar', new Route('/{placeholder}',
121-
['_controller' => 'fixed', 'placeholder' => 'default'], ['placeholder' => '.+'], ['option' => 'value'])
122-
);
122+
$collection1->add('foo', new Route('/{placeholder}'));
123123
$collection->addCollection($collection1);
124124

125125
$collection->addDefaults(['placeholder' => 'new-default']);
126126
$this->assertEquals(['placeholder' => 'new-default'], $collection->get('foo')->getDefaults(), '->addDefaults() adds defaults to all routes');
127-
$this->assertEquals(['_controller' => 'fixed', 'placeholder' => 'new-default'], $collection->get('bar')->getDefaults(),
128-
'->addDefaults() adds defaults to all routes and overwrites existing ones');
129127

130128
$collection->addRequirements(['placeholder' => '\d+']);
131129
$this->assertEquals(['placeholder' => '\d+'], $collection->get('foo')->getRequirements(), '->addRequirements() adds requirements to all routes');
132-
$this->assertEquals(['placeholder' => '\d+'], $collection->get('bar')->getRequirements(),
133-
'->addRequirements() adds requirements to all routes and overwrites existing ones');
134130

135131
$collection->addOptions(['option' => 'new-value']);
136132
$this->assertEquals(
137133
['option' => 'new-value', 'compiler_class' => 'Symfony\\Component\\Routing\\RouteCompiler'],
138-
$collection->get('bar')->getOptions(), '->addOptions() adds options to all routes and overwrites existing ones'
134+
$collection->get('foo')->getOptions(), '->addOptions() adds options to all routes'
135+
);
136+
}
137+
138+
/**
139+
* @group legacy
140+
*/
141+
public function testCollectionOverridingDefaultsIsDeprecated()
142+
{
143+
$collection = new RouteCollection();
144+
$collection->add('foo', new Route('/{placeholder}', ['placeholder' => 'default']));
145+
146+
$this->expectUserDeprecationMessage('Since symfony/routing 7.3: Overriding a route\'s default with its group\'s is deprecated, you should remove it from the route.');
147+
$collection->addDefaults(['placeholder' => 'new-default']);
148+
149+
$this->assertEquals(['placeholder' => 'new-default'], $collection->get('foo')->getDefaults());
150+
}
151+
152+
/**
153+
* @group legacy
154+
*/
155+
public function testCollectionOverridingRequirementsIsDeprecated()
156+
{
157+
$collection = new RouteCollection();
158+
$collection->add('foo', new Route('/{placeholder}', requirements: ['placeholder' => '.+']));
159+
160+
$this->expectUserDeprecationMessage('Since symfony/routing 7.3: Overriding a route\'s requirement with its group\'s is deprecated, you should remove it from the route.');
161+
$collection->addRequirements(['placeholder' => '\d+']);
162+
163+
$this->assertEquals(['placeholder' => '\d+'], $collection->get('foo')->getRequirements());
164+
}
165+
166+
/**
167+
* @group legacy
168+
*/
169+
public function testCollectionOverridingOptionsIsDeprecated()
170+
{
171+
$collection = new RouteCollection();
172+
$collection->add('foo', new Route('/{placeholder}', options: ['option' => 'value']));
173+
174+
$this->expectUserDeprecationMessage('Since symfony/routing 7.3: Overriding a route\'s option with its group\'s is deprecated, you should remove it from the route.');
175+
$collection->addOptions(['option' => 'new-value']);
176+
177+
$this->assertEquals(
178+
['option' => 'new-value', 'compiler_class' => 'Symfony\\Component\\Routing\\RouteCompiler'],
179+
$collection->get('foo')->getOptions()
139180
);
140181
}
141182

@@ -240,29 +281,52 @@ public function testRemove()
240281
public function testSetHost()
241282
{
242283
$collection = new RouteCollection();
243-
$routea = new Route('/a');
244-
$routeb = new Route('/b', [], [], [], '{locale}.example.net');
245-
$collection->add('a', $routea);
246-
$collection->add('b', $routeb);
284+
$route = new Route('/a');
285+
$collection->add('a', $route);
247286

248287
$collection->setHost('{locale}.example.com');
249288

250-
$this->assertEquals('{locale}.example.com', $routea->getHost());
251-
$this->assertEquals('{locale}.example.com', $routeb->getHost());
289+
$this->assertEquals('{locale}.example.com', $route->getHost());
290+
}
291+
292+
/**
293+
* @group legacy
294+
*/
295+
public function testCollectionOverridingHostIsDeprecated()
296+
{
297+
$collection = new RouteCollection();
298+
$route = new Route('/a', [], [], [], '{locale}.example.net');
299+
$collection->add('a', $route);
300+
301+
$this->expectUserDeprecationMessage('Since symfony/routing 7.3: Overriding a route\'s host with its group\'s is deprecated, you should remove it from the route.');
302+
$collection->setHost('{locale}.example.com');
303+
304+
$this->assertEquals('{locale}.example.com', $route->getHost());
252305
}
253306

254307
public function testSetCondition()
255308
{
256309
$collection = new RouteCollection();
257310
$routea = new Route('/a');
258-
$routeb = new Route('/b', [], [], [], '{locale}.example.net', [], [], 'context.getMethod() == "GET"');
259311
$collection->add('a', $routea);
260-
$collection->add('b', $routeb);
261312

262313
$collection->setCondition('context.getMethod() == "POST"');
263314

264315
$this->assertEquals('context.getMethod() == "POST"', $routea->getCondition());
265-
$this->assertEquals('context.getMethod() == "POST"', $routeb->getCondition());
316+
}
317+
318+
/**
319+
* @group legacy
320+
*/
321+
public function testCollectionOverridingConditionsIsDeprecated()
322+
{
323+
$collection = new RouteCollection();
324+
$collection->add('foo', new Route('/{placeholder}', condition: 'condition'));
325+
326+
$this->expectUserDeprecationMessage('Since symfony/routing 7.3: Overriding a route\'s condition with its group\'s is deprecated, you should remove it from the route.');
327+
$collection->setCondition(null);
328+
329+
$this->assertEquals('', $collection->get('foo')->getCondition());
266330
}
267331

268332
public function testClone()
@@ -283,29 +347,53 @@ public function testClone()
283347
public function testSetSchemes()
284348
{
285349
$collection = new RouteCollection();
286-
$routea = new Route('/a', [], [], [], '', 'http');
287-
$routeb = new Route('/b');
288-
$collection->add('a', $routea);
289-
$collection->add('b', $routeb);
350+
$route = new Route('/a');
351+
$collection->add('a', $route);
352+
353+
$collection->setSchemes(['http', 'https']);
354+
355+
$this->assertEquals(['http', 'https'], $route->getSchemes());
356+
}
290357

358+
/**
359+
* @group legacy
360+
*/
361+
public function testCollectionOverridingSchemesIsDeprecated()
362+
{
363+
$collection = new RouteCollection();
364+
$route = new Route('/a', [], [], [], '', 'http');
365+
$collection->add('a', $route);
366+
367+
$this->expectUserDeprecationMessage('Since symfony/routing 7.3: Overriding a route\'s schemes with its group\'s is deprecated, you should remove it from the route.');
291368
$collection->setSchemes(['http', 'https']);
292369

293-
$this->assertEquals(['http', 'https'], $routea->getSchemes());
294-
$this->assertEquals(['http', 'https'], $routeb->getSchemes());
370+
$this->assertEquals(['http', 'https'], $route->getSchemes());
295371
}
296372

297373
public function testSetMethods()
298374
{
299375
$collection = new RouteCollection();
300-
$routea = new Route('/a', [], [], [], '', [], ['GET', 'POST']);
301-
$routeb = new Route('/b');
302-
$collection->add('a', $routea);
303-
$collection->add('b', $routeb);
376+
$route = new Route('/a');
377+
$collection->add('a', $route);
378+
379+
$collection->setMethods('PUT');
380+
381+
$this->assertEquals(['PUT'], $route->getMethods());
382+
}
383+
384+
/**
385+
* @group legacy
386+
*/
387+
public function testCollectionOverridingMethodsIsDeprecated()
388+
{
389+
$collection = new RouteCollection();
390+
$route = new Route('/a', [], [], [], '', [], ['GET', 'POST']);
391+
$collection->add('a', $route);
304392

393+
$this->expectUserDeprecationMessage('Since symfony/routing 7.3: Overriding a route\'s methods with its group\'s is deprecated, you should remove it from the route.');
305394
$collection->setMethods('PUT');
306395

307-
$this->assertEquals(['PUT'], $routea->getMethods());
308-
$this->assertEquals(['PUT'], $routeb->getMethods());
396+
$this->assertEquals(['PUT'], $route->getMethods());
309397
}
310398

311399
public function testAddNamePrefix()

0 commit comments

Comments
 (0)