-
Notifications
You must be signed in to change notification settings - Fork 76
Created Unit tests for getRoutesByName with defined idPrefix. #189
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -119,8 +119,9 @@ protected function getCandidates($url) | |
*/ | ||
public function getRouteByName($name, $parameters = array()) | ||
{ | ||
|
||
// $name is the route document path | ||
if ($this->idPrefix && 0 === strpos($name, $this->idPrefix)) { | ||
if ('' === $this->idPrefix || 0 === strpos($name, $this->idPrefix)) { | ||
$route = $this->getObjectManager()->find($this->className, $name); | ||
} | ||
|
||
|
@@ -140,10 +141,10 @@ public function getRouteByName($name, $parameters = array()) | |
*/ | ||
public function getRoutesByNames($names, $parameters = array()) | ||
{ | ||
if ($this->idPrefix) { | ||
$routes = array(); | ||
$routes = array(); | ||
if ('' === $this->idPrefix) { | ||
foreach ($names as $name) { | ||
if (0 === strpos($name, $this->idPrefix)) { | ||
if ('' === $this->idPrefix || 0 === strpos($name, $this->idPrefix)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry, but you still need to undo the changes here, the existing code was correct. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can remove the idPrefix check here .. since its redundant |
||
$routes[] = $name; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
|
@@ -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'); | ||
} | ||
|
||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2 spaces |
||
*/ | ||
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()); | ||
} | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Empty line to much |
||
/** | ||
* @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'); | ||
} | ||
|
@@ -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()); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was correct as lukas did it. what you do is only go into the loop if there is no prefix, effectively never filtering