8000 [Routing] fixed route overriden mechanism when using embedded collect… · Faianca/symfony@5c8a2fb · GitHub
[go: up one dir, main page]
Skip to content

Commit 5c8a2fb

Browse files
fabpotfabpot
committed
[Routing] fixed route overriden mechanism when using embedded collections (closes symfony#2139)
1 parent 245d57a commit 5c8a2fb

File tree

7 files changed

+225
-72
lines changed

7 files changed

+225
-72
lines changed

‎src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ private function compileRoutes(RouteCollection $routes, $supportsRedirections, $
9191
if ($optimizable) {
9292
for ($j = $i; $j < $keysCount; $j++) {
9393
if ($keys[$j] === null) {
94-
continue;
94+
continue;
9595
}
9696

9797
$testRoute = $routeIterator->offsetGet($keys[$j]);

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
/**
1717
* A RouteCollection represents a set of Route instances.
1818
*
19+
* When adding a route, it overrides existing routes with the
20+
* same name defined in theinstance or its children and parents.
21+
*
1922
* @author Fabien Potencier <fabien@symfony.com>
2023
*
2124
* @api
@@ -25,6 +28,7 @@ class RouteCollection implements \IteratorAggregate
2528
private $routes;
2629
private $resources;
2730
private $prefix;
31+
private $parent;
2832

2933
/**
3034
* Constructor.
@@ -38,6 +42,31 @@ public function __construct()
3842
$this->prefix = '';
3943
}
4044

45+
/**
46+
* Gets the parent RouteCollection.
47+
*
48+
* @return RouteCollection The parent RouteCollection
49+
*/
50+
public function getParent()
51+
{
52+
return $this->parent;
53+
}
54+
55+
/**
56+
* Sets the parent RouteCollection.
57+
*
58+
* @param RouteCollection $parent The parent RouteCollection
59+
*/
60+
public function setParent(RouteCollection $parent)
61+
{
62+
$this->parent = $parent;
63+
}
64+
65+
/**
66+
* Gets the current RouteCollection as an Iterator.
67+
*
68+
* @return \ArrayIterator An \ArrayIterator interface
69+
*/
4170
public function getIterator()
4271
{
4372
return new \ArrayIterator($this->routes);
@@ -59,6 +88,15 @@ public function add($name, Route $route)
5988
throw new \InvalidArgumentException(sprintf('Name "%s" contains non valid characters for a route name.', $name));
6089
}
6190

91+
$parent = $this;
92+
while ($parent->getParent()) {
93+
$parent = $parent->getParent();
94+
}
95+
96+
if ($parent) {
97+
$parent->remove($name);
98+
}
99+
62100
$this->routes[$name] = $route;
63101
}
64102

@@ -106,6 +144,24 @@ public function get($name)
106144
}
107145
}
108146

147+
/**
148+
* Removes a route by name.
149+
*
150+
* @param string $name The route name
151+
*/
152+
public function remove($name)
153+
{
154+
if (isset($this->routes[$name])) {
155+
unset($this->routes[$name]);
156+
}
157+
158+
foreach ($this->routes as $routes) {
159+
if ($routes instanceof RouteCollection) {
160+
$routes->remove($name);
161+
}
162+
}
163+
}
164+
109165
/**
110166
* Adds a route collection to the current set of routes (at the end of the current set).
111167
*
@@ -116,8 +172,14 @@ public function get($name)
116172
*/
117173
public function addCollection(RouteCollection $collection, $prefix = '')
118174
{
175+
$collection->setParent($this);
119176
$collection->addPrefix($prefix);
120177

178+
// remove all routes with the same name in all existing collections
179+
foreach (array_keys($collection->all()) as $name) {
180+
$this->remove($name);
181+
}
182+
121183
$this->routes[] = $collection;
122184
}
123185

‎tests/Symfony/Tests/Component/Routing/Fixtures/dumper/url_matcher1.php

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -108,54 +108,59 @@ public function match($pathinfo)
108108

109109
if (0 === strpos($pathinfo, '/a')) {
110110
if (0 === strpos($pathinfo, '/a/b\'b')) {
111-
// foo
111+
// foo1
112112
if (preg_match('#^/a/b\'b/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
113-
$matches['_route'] = 'foo';
113+
$matches['_route'] = 'foo1';
114114
return $matches;
115115
}
116116

117-
// bar
117+
// bar1
118118
if (preg_match('#^/a/b\'b/(?P<bar>[^/]+?)$#xs', $pathinfo, $matches)) {
119-
$matches['_route'] = 'bar';
119+
$matches['_route'] = 'bar1';
120120
return $matches;
121121
}
122122

123-
// foo1
123+
// foo2
124124
if (preg_match('#^/a/b\'b/(?P<foo1>[^/]+?)$#xs', $pathinfo, $matches)) {
125-
$matches['_route'] = 'foo1';
125+
$matches['_route'] = 'foo2';
126126
return $matches;
127127
}
128128

129-
// bar1
129+
// bar2
130130
if (preg_match('#^/a/b\'b/(?P<bar1>[^/]+?)$#xs', $pathinfo, $matches)) {
131-
$matches['_route'] = 'bar1';
131+
$matches['_route'] = 'bar2';
132132
return $matches;
133133
}
134134

135135
}
136136

137+
// overriden
138+
if ($pathinfo === '/a/overriden2') {
139+
return array('_route' => 'overriden');
140+
}
141+
137142
// ababa
138143
if ($pathinfo === '/ababa') {
139144
return array('_route' => 'ababa');
140145
}
141146

142-
// foo
147+
// foo4
143148
if (preg_match('#^/aba/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
144-
$matches['_route'] = 'foo';
149+
$matches['_route'] = 'foo4';
145150
return $matches;
146151
}
147152

148153
}
149154

150-
// foo
155+
// foo3
151156
if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
152-
$matches['_route'] = 'foo';
157+
$matches['_route'] = 'foo3';
153158
return $matches;
154159
}
155160

156-
// bar
161+
// bar3
157162
if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<bar>[^/]+?)$#xs', $pathinfo, $matches)) {
158-
$matches['_route'] = 'bar';
163+
$matches['_route'] = 'bar3';
159164
return $matches;
160165
}
161166

‎tests/Symfony/Tests/Component/Routing/Fixtures/dumper/url_matcher2.php

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -120,54 +120,59 @@ public function match($pathinfo)
120120

121121
if (0 === strpos($pathinfo, '/a')) {
122122
if (0 === strpos($pathinfo, '/a/b\'b')) {
123-
// foo
123+
// foo1
124124
if (preg_match('#^/a/b\'b/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
125-
$matches['_route'] = 'foo';
125+
$matches['_route'] = 'foo1';
126126
return $matches;
127127
}
128128

129-
// bar
129+
// bar1
130130
if (preg_match('#^/a/b\'b/(?P<bar>[^/]+?)$#xs', $pathinfo, $matches)) {
131-
$matches['_route'] = 'bar';
131+
$matches['_route'] = 'bar1';
132132
return $matches;
133133
}
134134

135-
// foo1
135+
// foo2
136136
if (preg_match('#^/a/b\'b/(?P<foo1>[^/]+?)$#xs', $pathinfo, $matches)) {
137-
$matches['_route'] = 'foo1';
137+
$matches['_route'] = 'foo2';
138138
return $matches;
139139
}
140140

141-
// bar1
141+
// bar2
142142
if (preg_match('#^/a/b\'b/(?P<bar1>[^/]+?)$#xs', $pathinfo, $matches)) {
143-
$matches['_route'] = 'bar1';
143+
$matches['_route'] = 'bar2';
144144
return $matches;
145145
}
146146

147147
}
148148

149+
// overriden
150+
if ($pathinfo === '/a/overriden2') {
151+
return array('_route' => 'overriden');
152+
}
153+
149154
// ababa
150155
if ($pathinfo === '/ababa') {
151156
return array('_route' => 'ababa');
152157
}
153158

154-
// foo
159+
// foo4
155160
if (preg_match('#^/aba/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
156-
$matches['_route'] = 'foo';
161+
$matches['_route'] = 'foo4';
157162
return $matches;
158163
}
159164

160165
}
161166

162-
// foo
167+
// foo3
163168
if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
164-
$matches['_route'] = 'foo';
169+
$matches['_route'] = 'foo3';
165170
return $matches;
166171
}
167172

168-
// bar
173+
// bar3
169174
if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<bar>[^/]+?)$#xs', $pathinfo, $matches)) {
170-
$matches['_route'] = 'bar';
175+
$matches['_route'] = 'bar3';
171176
return $matches;
172177
}
173178

‎tests/Symfony/Tests/Component/Routing/Matcher/Dumper/PhpMatcherDumperTest.php

Lines changed: 54 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,53 @@
1919
class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
2020
{
2121
public function testDump()
22+
{
23+
$dumper = new PhpMatcherDumper($this->getRouteCollection(), new RequestContext());
24+
25+
$this->assertStringEqualsFile(__DIR__.'/../../Fixtures/dumper/url_matcher1.php', $dumper->dump(), '->dump() dumps basic routes to the correct PHP file.');
26+
27+
$collection = $this->getRouteCollection();
28+
29+
// force HTTPS redirection
30+
$collection->add('secure', new Route(
31+
'/secure',
32+
array(),
33+
array('_scheme' => 'https')
34+
));
35+
36+
// force HTTP redirection
37+
$collection->add('nonsecure', new Route(
38+
'/nonsecure',
39+
array(),
40+
array('_scheme' => 'http')
41+
));
42+
43+
$dumper = new PhpMatcherDumper($collection, new RequestContext());
44+
45+
$this->assertStringEqualsFile(__DIR__.'/../../Fixtures/dumper/url_matcher2.php', $dumper->dump(array('base_class' => 'Symfony\Tests\Component\Routing\Fixtures\RedirectableUrlMatcher')), '->dump() dumps basic routes to the correct PHP file.');
46+
}
47+
48+
/**
49+
* @expectedException \LogicException
50+
*/
51+
public function testDumpWhenSchemeIsUsedWithoutAProperDumper()
52+
{
53+
$collection = new RouteCollection();
54+
$collection->add('secure', new Route(
55+
'/secure',
56+
array(),
57+
array('_scheme' => 'https')
58+
));
59+
$dumper = new PhpMatcherDumper($collection, new RequestContext());
60+
$dumper->dump();
61+
}
62+
63+
protected function getRouteCollection()
2264
{
2365
$collection = new RouteCollection();
2466

67+
$collection->add('overriden', new Route('/overriden'));
68+
2569
// defaults and requirements
2670
$collection->add('foo', new Route(
2771
'/foo/{bar}',
@@ -82,20 +126,22 @@ public function testDump()
82126

83127
// prefixes
84128
$collection1 = new RouteCollection();
85-
$collection1->add('foo', new Route('/{foo}'));
86-
$collection1->add('bar', new Route('/{bar}'));
129+
$collection1->add('overriden', new Route('/overriden1'));
130+
$collection1->add('foo1', new Route('/{foo}'));
131+
$collection1->add('bar1', new Route('/{bar}'));
87132
$collection2 = new RouteCollection();
88133
$collection2->addCollection($collection1, '/b\'b');
134+
$collection2->add('overriden', new Route('/overriden2'));
89135
$collection1 = new RouteCollection();
90-
$collection1->add('foo1', new Route('/{foo1}'));
91-
$collection1->add('bar1', new Route('/{bar1}'));
136+
$collection1->add('foo2', new Route('/{foo1}'));
137+
$collection1->add('bar2', new Route('/{bar1}'));
92138
$collection2->addCollection($collection1, '/b\'b');
93139
$collection->addCollection($collection2, '/a');
94140

95141
// "dynamic" prefix
96142
$collection1 = new RouteCollection();
97-
$collection1->add('foo', new Route('/{foo}'));
98-
$collection1->add('bar', new Route('/{bar}'));
143+
$collection1->add('foo3', new Route('/{foo}'));
144+
$collection1->add('bar3', new Route('/{bar}'));
99145
$collection2 = new RouteCollection();
100146
$collection2->addCollection($collection1, '/b');
101147
$collection->addCollection($collection2, '/{_locale}');
@@ -104,42 +150,9 @@ public function testDump()
104150

105151
// some more prefixes
106152
$collection1 = new RouteCollection();
107-
$collection1->add('foo', new Route('/{foo}'));
153+
$collection1->add('foo4', new Route('/{foo}'));
108154
$collection->addCollection($collection1, '/aba');
109155

110-
$dumper = new PhpMatcherDumper($collection, new RequestContext());
111-
112-
$this->assertStringEqualsFile(__DIR__.'/../../Fixtures/dumper/url_matcher1.php', $dumper->dump(), '->dump() dumps basic routes to the correct PHP file.');
113-
114-
// force HTTPS redirection
115-
$collection->add('secure', new Route(
116-
'/secure',
117-
array(),
118-
array('_scheme' => 'https')
119-
));
120-
121-
// force HTTP redirection
122-
$collection->add('nonsecure', new Route(
123-
'/nonsecure',
124-
array(),
125-
array('_scheme' => 'http')
126-
));
127-
128-
$this->assertStringEqualsFile(__DIR__.'/../../Fixtures/dumper/url_matcher2.php', $dumper->dump(array('base_class' => 'Symfony\Tests\Component\Routing\Fixtures\RedirectableUrlMatcher')), '->dump() dumps basic routes to the correct PHP file.');
129-
}
130-
131-
/**
132-
* @expectedException \LogicException
133-
*/
134-
public function testDumpWhenSchemeIsUsedWithoutAProperDumper()
135-
{
136-
$collection = new RouteCollection();
137-
$collection->add('secure', new Route(
138-
'/secure',
139-
array(),
140-
array('_scheme' => 'https')
141-
));
142-
$dumper = new PhpMatcherDumper($collection, new RequestContext());
143-
$dumper->dump();
156+
return $collection;
144157
}
145158
}

0 commit comments

Comments
 (0)
0