8000 [Routing][2.0] fixed several bugs and applied improvements by Tobion · Pull Request #3810 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Routing][2.0] fixed several bugs and applied improvements #3810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

[Routing][2.0] fixed several bugs and applied improvements #3810

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f695b3d
performance improvement in PhpMatcherDumper
Tobion Apr 6, 2012
eaa1f48
fix bugs and improvements in RouteCollection
Tobion Apr 1, 2012
2d00c29
fixed bugs and added tests for Routing
Tobion Apr 4, 2012
8da6d60
fixed CS and minor tweaks
Tobion Apr 5, 2012
File filter

Filter by extension

Filter by extension
Conversations
8000 Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next
fixed CS and minor tweaks 
6bd80a7f0 for 2.0
  • Loading branch information
@Tobion
Tobion committed Apr 6, 2012
commit 8da6d607e38ea39c19f0e0c99b00d010fb1ef52a
17 changes: 7 additions & 10 deletions src/Symfony/Component/Routing/RouteCollection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public function addCollection(RouteCollection $collection, $prefix = '')
$this->remove(array_keys($collection->all()));

$collection->setParent($this);
// the sub-collection must have the prefix of the parent (current instance) prepended because it it does not
// the sub-collection must have the prefix of the parent (current instance) prepended because it does not
// necessarily already have it applied (depending on the order RouteCollections are added to each other)
$collection->addPrefix($this->getPrefix() . $prefix);
$this->routes[] = $collection;
Expand Down Expand Up @@ -297,10 +297,8 @@ private function removeRecursively($name)
}

foreach ($this->routes as $routes) {
if ($routes instanceof RouteCollection) {
if ($routes->removeRecursively($name)) {
return true;
}
if ($routes instanceof RouteCollection && $routes->removeRecursively($name)) {
return true;
}
}

Expand All @@ -310,7 +308,7 @@ private function removeRecursively($name)
/**
* Checks whether the given RouteCollection is already set in any child of the current instance.
*
* @param RouteCollection $collection A RouteCollection instance
* @param RouteCollection $collection A RouteCollection instance
*
* @return Boolean
*/
Expand All @@ -319,10 +317,9 @@ private function existsSubCollection(RouteCollection $collection)
foreach ($this->routes as $routes) {
if ($routes === $collection) {
return true;
} elseif ($routes instanceof RouteCollection) {
if ($routes->existsSubCollection($collection)) {
return true;
}
}
if ($routes instanceof RouteCollection && $routes->existsSubCollection($collection)) {
return true;
}
}

Expand Down
21 changes: 12 additions & 9 deletions tests/Symfony/Tests/Component/Routing/RouteCollectionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,24 @@ public function testResource()
public function testUniqueRouteWithGivenName()
{
$collection1 = new RouteCollection();
$collection1->add('foo', $old = new Route('/old'));
$collection1->add('foo', new Route('/old'));
$collection2 = new RouteCollection();
$collection3 = new RouteCollection();
$collection3->add('foo', $new = new Route('/new'));

$collection1->addCollection($collection2);
$collection2->addCollection($collection3);

$collection1->add('stay', new Route('/stay'));

$iterator = $collection1->getIterator();

$this->assertSame($new, $collection1->get('foo'), '->get() returns new route that overrode previous one');
$p = new \ReflectionProperty('Symfony\Component\Routing\RouteCollection', 'routes');
$p->setAccessible(true);
// size of 1 because collection1 contains collection2 but not $old anymore
$this->assertCount(1, $p->getValue($collection1), '->addCollection() removes previous routes when adding new routes with the same name');
// size of 2 because collection1 contains collection2 and /stay but not /old anymore
$this->assertCount(2, $iterator, '->addCollection() removes previous routes when adding new routes with the same name');
$this->assertInstanceOf('Symfony\Component\Routing\RouteCollection', $iterator->current(), '->getIterator returns both Routes and RouteCollections');
$iterator->next();
$this->assertInstanceOf('Symfony\Component\Routing\Route', $iterator->current(), '->getIterator returns both Routes and RouteCollections');
}

public function testGet()
Expand Down Expand Up @@ -215,10 +220,8 @@ public function testDefinitionOrderDoesNotMatter()
$collection2->addCollection($collection3, '/c');
$rootCollection_B->addCollection($collection1, '/a');

// test it
// test it now

$p = new \ReflectionProperty('Symfony\Component\Routing\RouteCollection', 'routes');
$p->setAccessible(true);
$this->assertEquals($p->getValue($rootCollection_A), $p->getValue($rootCollection_B));
$this->assertEquals($rootCollection_A, $rootCollection_B);
}
}
0