Skip to content

Commit

Permalink
Merge pull request #186 from symfony-cmf/optimize_get_route_by_name
Browse files Browse the repository at this point in the history
filter routes with the id prefix before doing lookups
  • Loading branch information
lsmith77 committed Oct 8, 2013
2 parents 3082ce5 + b9b6e1a commit fcabe73
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 3 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
Changelog
=========

1.1.0-RC8
---------

* **2013-10-08**: The idPrefix is now used as a filter in getRouteByName() and getRoutesByNames()
in the PHPCR RouteProvider. This means its no longer possible to get routes that are not children
of a path that begins with idPrefix

1.1.0-RC5
---------

Expand Down
14 changes: 12 additions & 2 deletions Doctrine/Phpcr/RouteProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,11 @@ protected function getCandidates($url)
public function getRouteByName($name, $parameters = array())
{
// $name is the route document path
$route = $this->getObjectManager()->find($this->className, $name);
if ('' === $this->idPrefix || 0 === strpos($name, $this->idPrefix)) {
$route = $this->getObjectManager()->find($this->className, $name);
}

if (!$route) {
if (empty($route)) {
throw new RouteNotFoundException(sprintf('No route found for path "%s"', $name));
}

Expand All @@ -138,6 +140,14 @@ public function getRouteByName($name, $parameters = array())
*/
public function getRoutesByNames($names, $parameters = array())
{
if ('' !== $this->idPrefix) {
foreach ($names as $key => $name) {
if (0 !== strpos($name, $this->idPrefix)) {
unset($names[$key]);
}
}
}

$collection = $this->getObjectManager()->findMany($this->className, $names);
foreach ($collection as $key => $document) {
if (!$document instanceof SymfonyRoute) {
Expand Down
111 changes: 110 additions & 1 deletion Tests/Unit/Doctrine/Phpcr/RouteProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public function testGetRouteByName()
$routeProvider = new RouteProvider($this->managerRegistry);
$routeProvider->setManagerName('default');

$routeProvider->setPrefix('/cms/routes/');
$foundRoute = $routeProvider->getRouteByName('/cms/routes/test-route');

$this->assertInstanceOf('Symfony\Component\Routing\Route', $foundRoute);
Expand All @@ -92,7 +93,7 @@ public function testGetRouteByNameNotFound()

$routeProvider = new RouteProvider($this->managerRegistry);
$routeProvider->setManagerName('default');

$routeProvider->setPrefix('/cms/routes/');
$routeProvider->getRouteByName('/cms/routes/test-route');
}

Expand All @@ -116,6 +117,112 @@ public function testGetRouteByNameNoRoute()

$routeProvider = new RouteProvider($this->managerRegistry);
$routeProvider->setManagerName('default');
$routeProvider->setPrefix('/cms/routes/');
$routeProvider->getRouteByName('/cms/routes/test-route');
}

/**
* @expectedException \Symfony\Component\Routing\Exception\RouteNotFoundException
*/
public function testGetRouteByNameInvalidRoute()
{
$this->objectManager
->expects($this->never())
->method('find')
;

$this->managerRegistry
->expects($this->any())
->method('getManager')
->will($this->returnValue($this->objectManager))
;

$routeProvider = new RouteProvider($this->managerRegistry);
$routeProvider->setManagerName('default');

$routeProvider->setPrefix('/cms/routes');

$routeProvider->getRouteByName('invalid_route');
}

public function testGetRouteByNameIdPrefixEmptyString()
{

$this->route
->expects($this->any())
->method('getPath')
->will($this->returnValue('/cms/routes/test-route'))
;

$this->objectManager
->expects($this->any())
->method('find')
->with(null, '/cms/routes/test-route')
->will($this->returnValue($this->route))
;

$this->managerRegistry
->expects($this->any())
->method('getManager')
->will($this->returnValue($this->objectManager))
;

$routeProvider = new RouteProvider($this->managerRegistry);
$routeProvider->setManagerName('default');

$routeProvider->setPrefix('');
$foundRoute = $routeProvider->getRouteByName('/cms/routes/test-route');

$this->assertInstanceOf('Symfony\Component\Routing\Route', $foundRoute);
$this->assertEquals('/cms/routes/test-route', $foundRoute->getPath());
}


/**
* @expectedException \Symfony\Component\Routing\Exception\RouteNotFoundException
*/
public function testGetRouteByNameNotFoundIdPrefixEmptyString()
{
$this->objectManager
->expects($this->any())
->method('find')
->with(null, '/cms/routes/test-route')
->will($this->returnValue(null))
;

$this->managerRegistry
->expects($this->any())
->method('getManager')
->will($this->returnValue($this->objectManager))
;

$routeProvider = new RouteProvider($this->managerRegistry);
$routeProvider->setManagerName('default');
$routeProvider->setPrefix('');
$routeProvider->getRouteByName('/cms/routes/test-route');
}

/**
* @expectedException \Symfony\Component\Routing\Exception\RouteNotFoundException
*/
public function testGetRouteByNameNoRoutePrefixEmptyString()
{
$this->objectManager
->expects($this->any())
->method('find')
->with(null, '/cms/routes/test-route')
->will($this->returnValue($this))
;

$this->managerRegistry
->expects($this->any())
->method('getManager')
->will($this->returnValue($this->objectManager))
;

$routeProvider = new RouteProvider($this->managerRegistry);
$routeProvider->setManagerName('default');
$routeProvider->setPrefix('');

$routeProvider->getRouteByName('/cms/routes/test-route');
}
Expand Down Expand Up @@ -173,6 +280,8 @@ function ($name) use ($objectManagers) {
$routeProvider = new RouteProvider($this->managerRegistry);

$routeProvider->setManagerName('default');
$routeProvider->setPrefix('/cms/routes/');

$foundRoute = $routeProvider->getRouteByName('/cms/routes/test-route');
$this->assertInstanceOf('Symfony\Component\Routing\Route', $foundRoute);
$this->assertEquals('/cms/routes/test-route', $foundRoute->getPath());
Expand Down

0 comments on commit fcabe73

Please sign in to comment.