Skip to content

Commit

Permalink
Merge pull request FriendsOfSymfony#947 from xabbuh/remove-deprecated…
Browse files Browse the repository at this point in the history
…-routing-features

don't use deprecated routing features
  • Loading branch information
lsmith77 committed Jan 27, 2015
2 parents 7aa99c5 + 6b772f0 commit 95fbb40
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 43 deletions.
22 changes: 16 additions & 6 deletions Routing/Loader/Reader/RestActionReader.php
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ public function read(RestRouteCollection $collection, \ReflectionMethod $method,
$requirements = array('_method' => strtoupper($httpMethod));
$options = array();
$host = '';
$schemes = array();
$condition = null;

$annotations = $this->readRouteAnnotation($method);
Expand All @@ -195,16 +194,19 @@ public function read(RestRouteCollection $collection, \ReflectionMethod $method,

$pattern = implode('/', $urlParts);
$defaults = array('_controller' => $method->getName());
$requirements = array('_method' => strtoupper($httpMethod));
$requirements = array();
$options = array();
$methods = explode('|', $httpMethod);
$condition = null;

$annoRequirements = $annotation->getRequirements();

if (!isset($annoRequirements['_method'])) {
$annoRequirements['_method'] = $requirements['_method'];
if (isset($annoRequirements['_method'])) {
$methods = explode('|', strtoupper($annoRequirements['_method']));
}

unset($annoRequirements['_method']);

$pattern = $annotation->getPath() !== null ? $this->routePrefix.$annotation->getPath() : $pattern;
$requirements = array_merge($requirements, $annoRequirements);
$options = array_merge($options, $annotation->getOptions());
Expand All @@ -225,7 +227,7 @@ public function read(RestRouteCollection $collection, \ReflectionMethod $method,
}
// add route to collection
$route = new Route(
$pattern, $defaults, $requirements, $options, $host, $schemes, null, $condition
$pattern, $defaults, $requirements, $options, $host, $schemes, $methods, $condition
);
$this->addRoute($collection, $routeName, $route, $isCollection, $isInflectable, $annotation);
}
Expand All @@ -238,9 +240,17 @@ public function read(RestRouteCollection $collection, \ReflectionMethod $method,
$requirements['_format'] = implode('|', array_keys($this->formats));
}
}

if (isset($requirements['_method'])) {
$methods = explode('|', $requirements['_method']);
unset($requirements['_method']);
} else {
$methods = array();
}

// add route to collection
$route = new Route(
$pattern, $defaults, $requirements, $options, $host, $schemes, null, $condition
$pattern, $defaults, $requirements, $options, $host, array(), $methods, $condition
);
$this->addRoute($collection, $routeName, $route, $isCollection, $isInflectable);
}
Expand Down
2 changes: 1 addition & 1 deletion Routing/Loader/RestYamlCollectionLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public function load($file, $type = null)
{
$path = $this->locator->locate($file);

$config = Yaml::parse($path);
$config = Yaml::parse(file_get_contents($path));

$collection = new RouteCollection();
$collection->addResource(new FileResource($path));
Expand Down
2 changes: 1 addition & 1 deletion Tests/Routing/Loader/LoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ abstract class LoaderTest extends \PHPUnit_Framework_TestCase
*/
protected function loadEtalonRoutesInfo($etalonName)
{
return Yaml::parse(__DIR__.'/../../Fixtures/Etalon/'.$etalonName);
return Yaml::parse(file_get_contents(__DIR__.'/../../Fixtures/Etalon/'.$etalonName));
}

private function getAnnotationReader()
Expand Down
58 changes: 34 additions & 24 deletions Tests/Routing/Loader/RestRouteLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ public function testUsersFixture()

foreach ($etalonRoutes as $name => $params) {
$route = $collection->get($name);
$methods = $route->getMethods();

$this->assertNotNull($route, sprintf('route for %s does not exist', $name));
$this->assertEquals($params['pattern'], $route->getPattern(), 'Pattern does not match for route: '.$name);
$this->assertEquals($params['method'], $route->getRequirement('_method'), 'Method does not match for route: '.$name);
$this->assertEquals($params['pattern'], $route->getPath(), 'Pattern does not match for route: '.$name);
$this->assertEquals($params['method'], $methods[0], 'Method does not match for route: '.$name);
$this->assertContains($params['controller'], $route->getDefault('_controller'), 'Controller does not match for route: '.$name);
}
}
Expand All @@ -55,10 +56,11 @@ public function testResourceFixture()

foreach ($etalonRoutes as $name => $params) {
$route = $collection->get($name);
$methods = $route->getMethods();

$this->assertNotNull($route, sprintf('route for %s does not exist', $name));
$this->assertEquals($params['pattern'], $route->getPattern(), 'Pattern does not match for route: '.$name);
$this->assertEquals($params['method'], $route->getRequirement('_method'), 'Method does not match for route: '.$name);
$this->assertEquals($params['pattern'], $route->getPath(), 'Pattern does not match for route: '.$name);
$this->assertEquals($params['method'], $methods[0], 'Method does not match for route: '.$name);
$this->assertContains($params['controller'], $route->getDefault('_controller'), 'Controller does not match for route: '.$name);
}
}
Expand Down Expand Up @@ -99,8 +101,12 @@ public function testAnnotatedUsersFixture()
foreach ($etalonRoutes as $name => $params) {
$route = $collection->get($name);

if (!array_key_exists('_method', $route->getRequirements())) {
unset($params['requirements']['_method']);
}

$this->assertNotNull($route, "no route found for '$name'");
$this->assertEquals($params['pattern'], $route->getPattern(), 'pattern failed to match for '.$name);
$this->assertEquals($params['pattern'], $route->getPath(), 'pattern failed to match for '.$name);
$this->assertEquals($params['requirements'], $route->getRequirements(), 'requirements failed to match for '.$name);
$this->assertContains($params['controller'], $route->getDefault('_controller'), 'controller failed to match for '.$name);
if (isset($params['condition'])) {
Expand Down Expand Up @@ -136,8 +142,12 @@ public function testAnnotatedConditionalUsersFixture()
foreach ($etalonRoutes as $name => $params) {
$route = $collection->get($name);

if (!array_key_exists('_method', $route->getRequirements())) {
unset($params['requirements']['_method']);
}

$this->assertNotNull($route, "no route found for '$name'");
$this->assertEquals($params['pattern'], $route->getPattern(), 'pattern failed to match for '.$name);
$this->assertEquals($params['pattern'], $route->getPath(), 'pattern failed to match for '.$name);
$this->assertEquals($params['requirements'], $route->getRequirements(), 'requirements failed to match for '.$name);
$this->assertContains($params['controller'], $route->getDefault('_controller'), 'controller failed to match for '.$name);
if (isset($params['condition'])) {
Expand Down Expand Up @@ -176,12 +186,12 @@ public function testPrefixIsResetForEachController()
// get the pattern for the prefixed controller, and verify it is prefixed
$collection = $loader->load('FOS\RestBundle\Tests\Fixtures\Controller\AnnotatedPrefixedController', 'rest');
$prefixedRoute = $collection->get('get_something');
$this->assertEquals('/aprefix/', substr($prefixedRoute->getPattern(), 0, 9));
$this->assertEquals('/aprefix/', substr($prefixedRoute->getPath(), 0, 9));

// get the pattern for the non-prefixed controller, and verify it's not prefixed
$collection2 = $loader->load('FOS\RestBundle\Tests\Fixtures\Controller\UsersController', 'rest');
$nonPrefixedRoute = $collection2->get('get_users');
$this->assertNotEquals('/aprefix/', substr($nonPrefixedRoute->getPattern(), 0, 9));
$this->assertNotEquals('/aprefix/', substr($nonPrefixedRoute->getPath(), 0, 9));
}

/**
Expand All @@ -191,30 +201,30 @@ public function testPrefixIsResetForEachController()
*/
public function testConventionalActions()
{
$expectedMethod = 'GET';
$expectedMethod = array('GET');
$collection = $this->loadFromControllerFixture('UsersController');
$subcollection = $this->loadFromControllerFixture('UserTopicsController');
$subsubcollection = $this->loadFromControllerFixture('UserTopicCommentsController');

// resource actions
$this->assertEquals($expectedMethod, $collection->get('new_users')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $collection->get('edit_user')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $collection->get('remove_user')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $collection->get('new_users')->getMethods());
$this->assertEquals($expectedMethod, $collection->get('edit_user')->getMethods());
$this->assertEquals($expectedMethod, $collection->get('remove_user')->getMethods());

// subresource actions
$this->assertEquals($expectedMethod, $collection->get('new_user_comments')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $collection->get('edit_user_comment')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $collection->get('remove_user_comment')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $collection->get('new_user_comments')->getMethods());
$this->assertEquals($expectedMethod, $collection->get('edit_user_comment')->getMethods());
$this->assertEquals($expectedMethod, $collection->get('remove_user_comment')->getMethods());

// resource collection actions
$this->assertEquals($expectedMethod, $subcollection->get('new_topics')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $subcollection->get('edit_topic')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $subcollection->get('remove_topic')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $subcollection->get('new_topics')->getMethods());
$this->assertEquals($expectedMethod, $subcollection->get('edit_topic')->getMethods());
$this->assertEquals($expectedMethod, $subcollection->get('remove_topic')->getMethods());

// resource collection's resource collection actions
$this->assertEquals($expectedMethod, $subsubcollection->get('new_comments')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $subsubcollection->get('edit_comment')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $subsubcollection->get('remove_comment')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $subsubcollection->get('new_comments')->getMethods());
$this->assertEquals($expectedMethod, $subsubcollection->get('edit_comment')->getMethods());
$this->assertEquals($expectedMethod, $subsubcollection->get('remove_comment')->getMethods());
}

/**
Expand Down Expand Up @@ -247,12 +257,12 @@ public function testCustomActionRoutesInDeveloperOrder()
*/
public function testMediaFixture()
{
$expectedMethod = 'GET';
$expectedMethod = array('GET');
$collection = $this->loadFromControllerFixture('MediaController');

$this->assertCount(2, $collection->all());
$this->assertEquals($expectedMethod, $collection->get('get_media')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $collection->get('cget_media')->getRequirement('_method'));
$this->assertEquals($expectedMethod, $collection->get('get_media')->getMethods());
$this->assertEquals($expectedMethod, $collection->get('cget_media')->getMethods());
}

/**
Expand Down
10 changes: 6 additions & 4 deletions Tests/Routing/Loader/RestXmlCollectionLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ public function testUsersFixture()

foreach ($etalonRoutes as $name => $params) {
$route = $collection->get($name);
$methods = $route->getMethods();

$this->assertNotNull($route, $name);
$this->assertEquals($params['pattern'], $route->getPattern(), $name);
$this->assertEquals($params['method'], $route->getRequirement('_method'), $name);
$this->assertEquals($params['pattern'], $route->getPath(), $name);
$this->assertEquals($params['method'], $methods[0], $name);
$this->assertContains($params['controller'], $route->getDefault('_controller'), $name);
}
}
Expand All @@ -52,10 +53,11 @@ public function testPrefixedUsersFixture()

foreach ($etalonRoutes as $name => $params) {
$route = $collection->get($name);
$methods = $route->getMethods();

$this->assertNotNull($route, $name);
$this->assertEquals($params['pattern'], $route->getPattern(), $name);
$this->assertEquals($params['method'], $route->getRequirement('_method'), $name);
$this->assertEquals($params['pattern'], $route->getPath(), $name);
$this->assertEquals($params['method'], $methods[0], $name);
$this->assertContains($params['controller'], $route->getDefault('_controller'), $name);
}
}
Expand Down
17 changes: 10 additions & 7 deletions Tests/Routing/Loader/RestYamlCollectionLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ public function testUsersFixture()

foreach ($etalonRoutes as $name => $params) {
$route = $collection->get($name);
$methods = $route->getMethods();

$this->assertNotNull($route, $name);
$this->assertEquals($params['pattern'], $route->getPattern(), $name);
$this->assertEquals($params['method'], $route->getRequirement('_method'), $name);
$this->assertEquals($params['pattern'], $route->getPath(), $name);
$this->assertEquals($params['method'], $methods[0], $name);
$this->assertContains($params['controller'], $route->getDefault('_controller'), $name);
}
}
Expand All @@ -53,10 +54,11 @@ public function testPrefixedUsersFixture()

foreach ($etalonRoutes as $name => $params) {
$route = $collection->get($name);
$methods = $route->getMethods();

$this->assertNotNull($route, $name);
$this->assertEquals($params['pattern'], $route->getPattern(), $name);
$this->assertEquals($params['method'], $route->getRequirement('_method'), $name);
$this->assertEquals($params['pattern'], $route->getPath(), $name);
$this->assertEquals($params['method'], $methods[0], $name);
$this->assertContains($params['controller'], $route->getDefault('_controller'), $name);
}
}
Expand All @@ -71,10 +73,11 @@ public function testNamedPrefixedReportsFixture()

foreach ($etalonRoutes as $name => $params) {
$route = $collection->get($name);
$methods = $route->getMethods();

$this->assertNotNull($route, $name);
$this->assertEquals($params['pattern'], $route->getPattern(), $name);
$this->assertEquals($params['method'], $route->getRequirement('_method'), $name);
$this->assertEquals($params['pattern'], $route->getPath(), $name);
$this->assertEquals($params['method'], $methods[0], $name);
$this->assertContains($params['controller'], $route->getDefault('_controller'), $name);
}
}
Expand All @@ -87,7 +90,7 @@ public function testNamedPrefixedReportsFixtureHasNoDuplicates()
$names = array();
$collection = $this->loadFromYamlCollectionFixture('named_prefixed_reports_collection.yml');
foreach ($collection as $route) {
$names[] = $route->getPattern();
$names[] = $route->getPath();
}
$this->assertEquals(count($names), count(array_unique($names)));
}
Expand Down

0 comments on commit 95fbb40

Please sign in to comment.