diogo
/
symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

merged branch Tobion/deprecation-trigger (PR #7266) 

This PR was merged into the 2.2 branch.

Commits
-------

c0687cd remove() should not use deprecated getParent() so it does not trigger deprecation internally
708c0d3 adjust routing tests to not use prefix in addCollection
6180c5b add test for uniqueness of resources
c0de07b added tests for addDefaults, addRequirements, addOptions
0a1cfcd adjust RouteCollectionTest for the addCollection change and refactor the tests to only skip the part that really needs the config component
ea694e4 added tests for remove() that wasnt covered yet and special route name
9e2bcb5 refactor interator test that was still assuming a tree
ceb9ab4 adjust tests to no use addPrefix with options
2b8bf6b adjusted tests to not use RouteCollection::getPrefix
acff735 [Routing] trigger deprecation warning for deprecated features that will be removed in 2.3

Discussion
----------

[2.2][Routing] Trigger deprecation and refactor tests to not use deprecated methods

| Q             | A
| ------------- | ---
| Bug fix?      | [yes]
| New feature?  | [no]
| BC breaks?    | [no]
| Deprecations? | [no]
| Tests pass?   | [yes]
| License       | MIT

@fabpot please don't squash because it also added new tests
This commit is contained in:
Fabien Potencier 2013-03-06 17:23:04 +01:00
parent 5e3756fae0 c0687cd5fa
commit d0022e31d7
5 changed files with 128 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
src/Symfony/Component/Routing/RouteCollection.php
View File

@ -65,6 +65,9 @@ class RouteCollection implements \IteratorAggregate, \Countable
*/
public function getParent()
{
trigger_error('getParent() is deprecated since version 2.2 and will be removed in 2.3. There is no substitution ' .
'because RouteCollection is not tree structure anymore.', E_USER_DEPRECATED);
return $this->parent;
}
@ -77,6 +80,9 @@ class RouteCollection implements \IteratorAggregate, \Countable
*/
public function getRoot()
{
trigger_error('getRoot() is deprecated since version 2.2 and will be removed in 2.3. There is no substitution ' .
'because RouteCollection is not tree structure anymore.', E_USER_DEPRECATED);
$parent = $this;
while ($parent->getParent()) {
$parent = $parent->getParent();
@ -157,7 +163,10 @@ class RouteCollection implements \IteratorAggregate, \Countable
public function remove($name)
{
// just for BC
$root = $this->getRoot();
$root = $this;
while ($root->parent) {
$root = $root->parent;
}
foreach ((array) $name as $n) {
unset($root->routes[$n]);
@ -184,6 +193,8 @@ class RouteCollection implements \IteratorAggregate, \Countable
// this is to keep BC
$numargs = func_num_args();
if ($numargs > 1) {
trigger_error('addCollection() should only be used with a single parameter. The params $prefix, $defaults, $requirements and $options ' .
'are deprecated since version 2.2 and will be removed in 2.3. Use addPrefix() and addOptions() instead.', E_USER_DEPRECATED);
$collection->addPrefix($this->prefix . func_get_arg(1));
if ($numargs > 2) {
$collection->addDefaults(func_get_arg(2));
@ -232,7 +243,13 @@ class RouteCollection implements \IteratorAggregate, \Countable
$this->prefix = '/' . $prefix . $this->prefix;
// this is to keep BC
$options = func_num_args() > 3 ? func_get_arg(3) : array();
if (func_num_args() > 3) {
trigger_error('The fourth parameter ($options) of addPrefix() is deprecated since version 2.2 and will be removed in 2.3. ' .
'Use addOptions() instead.', E_USER_DEPRECATED);
$options = func_get_arg(3);
} else {
$options = array();
}
foreach ($this->routes as $route) {
$route->setPath('/' . $prefix . $route->getPath());
@ -251,6 +268,9 @@ class RouteCollection implements \IteratorAggregate, \Countable
*/
public function getPrefix()
{
trigger_error('getPrefix() is deprecated since version 2.2 and will be removed in 2.3. The method suggests that ' .
'all routes in the collection would have this prefix, which is not necessarily true.', E_USER_DEPRECATED);
return $this->prefix;
}

3
src/Symfony/Component/Routing/Tests/Matcher/Dumper/ApacheMatcherDumperTest.php
View File

@ -150,7 +150,8 @@ class ApacheMatcherDumperTest extends \PHPUnit_Framework_TestCase
$route3 = new Route('/route3', array(), array(), array(), 'b.example.com');
$collection2->add('route3', $route3);
$collection1->addCollection($collection2, '/c2');
$collection2->addPrefix('/c2');
$collection1->addCollection($collection2);
$route4 = new Route('/route4', array(), array(), array(), 'a.example.com');
$collection1->add('route4', $route4);

31
src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php
View File

@ -118,14 +118,17 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
$collection1->add('overridden', new Route('/overridden1'));
$collection1->add('foo1', new Route('/{foo}'));
$collection1->add('bar1', new Route('/{bar}'));
$collection1->addPrefix('/b\'b');
$collection2 = new RouteCollection();
$collection2->addCollection($collection1, '/b\'b');
$collection2->addCollection($collection1);
$collection2->add('overridden', new Route('/{var}', array(), array('var' => '.*')));
$collection1 = new RouteCollection();
$collection1->add('foo2', new Route('/{foo1}'));
$collection1->add('bar2', new Route('/{bar1}'));
$collection2->addCollection($collection1, '/b\'b');
$collection->addCollection($collection2, '/a');
$collection1->addPrefix('/b\'b');
$collection2->addCollection($collection1);
$collection2->addPrefix('/a');
$collection->addCollection($collection2);
// overridden through addCollection() and multiple sub-collections with no own prefix
$collection1 = new RouteCollection();
@ -137,15 +140,16 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
$collection3->add('hey', new Route('/hey/'));
$collection2->addCollection($collection3);
$collection1->addCollection($collection2);
$collection->addCollection($collection1, '/multi');
$collection1->addPrefix('/multi');
$collection->addCollection($collection1);
// "dynamic" prefix
$collection1 = new RouteCollection();
$collection1->add('foo3', new Route('/{foo}'));
$collection1->add('bar3', new Route('/{bar}'));
$collection2 = new RouteCollection();
$collection2->addCollection($collection1, '/b');
$collection->addCollection($collection2, '/{_locale}');
$collection1->addPrefix('/b');
$collection1->addPrefix('{_locale}');
$collection->addCollection($collection1);
// route between collections
$collection->add('ababa', new Route('/ababa'));
@ -153,7 +157,8 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
// collection with static prefix but only one route
$collection1 = new RouteCollection();
$collection1->add('foo4', new Route('/{foo}'));
$collection->addCollection($collection1, '/aba');
$collection1->addPrefix('/aba');
$collection->addCollection($collection1);
// prefix and host
@ -215,10 +220,12 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
$collection2->add('b', new Route('/{var}'));
$collection3 = new RouteCollection();
$collection3->add('c', new Route('/{var}'));
$collection2->addCollection($collection3, '/c');
$collection1->addCollection($collection2, '/b');
$collection->addCollection($collection1, '/a');
$collection3->addPrefix('/c');
$collection2->addCollection($collection3);
$collection2->addPrefix('/b');
$collection1->addCollection($collection2);
$collection1->addPrefix('/a');
$collection->addCollection($collection1);
/* test case 2 */

20
src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php
View File

@ -130,14 +130,10 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
public function testMatchWithPrefixes()
{
$collection1 = new RouteCollection();
$collection1->add('foo', new Route('/{foo}'));
$collection2 = new RouteCollection();
$collection2->addCollection($collection1, '/b');
$collection = new RouteCollection();
$collection->addCollection($collection2, '/a');
$collection->add('foo', new Route('/{foo}'));
$collection->addPrefix('/b');
$collection->addPrefix('/a');
$matcher = new UrlMatcher($collection, new RequestContext());
$this->assertEquals(array('_route' => 'foo', 'foo' => 'foo'), $matcher->match('/a/b/foo'));
@ -145,14 +141,10 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
public function testMatchWithDynamicPrefix()
{
$collection1 = new RouteCollection();
$collection1->add('foo', new Route('/{foo}'));
$collection2 = new RouteCollection();
$collection2->addCollection($collection1, '/b');
$collection = new RouteCollection();
$collection->addCollection($collection2, '/{_locale}');
$collection->add('foo', new Route('/{foo}'));
$collection->addPrefix('/b');
$collection->addPrefix('/{_locale}');
$matcher = new UrlMatcher($collection, new RequestContext());
$this->assertEquals(array('_locale' => 'fr', '_route' => 'foo', 'foo' => 'foo'), $matcher->match('/fr/b/foo'));

143
src/Symfony/Component/Routing/Tests/RouteCollectionTest.php
View File

@ -54,16 +54,19 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase
$this->assertEquals('/foo2', $collection->get('foo')->getPath());
}
public function testIteratorWithOverriddenRoutes()
public function testIterator()
{
$collection = new RouteCollection();
$collection->add('foo', new Route('/foo'));
$collection1 = new RouteCollection();
$collection1->add('foo', new Route('/foo1'));
$collection1->add('bar', $bar = new Route('/bar'));
$collection1->add('foo', $foo = new Route('/foo-new'));
$collection->addCollection($collection1);
$collection->add('last', $last = new Route('/last'));
$this->assertEquals('/foo1', $this->getFirstNamedRoute($collection, 'foo')->getPath());
$this->assertInstanceOf('\ArrayIterator', $collection->getIterator());
$this->assertSame(array('bar' => $bar, 'foo' => $foo, 'last' => $last), $collection->getIterator()->getArrayCopy());
}
public function testCount()
@ -72,52 +75,38 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase
$collection->add('foo', new Route('/foo'));
$collection1 = new RouteCollection();
$collection1->add('foo1', new Route('/foo1'));
$collection1->add('bar', new Route('/bar'));
$collection->addCollection($collection1);
$this->assertCount(2, $collection);
}
protected function getFirstNamedRoute(RouteCollection $routeCollection, $name)
public function testAddCollection()
{
foreach ($routeCollection as $key => $route) {
if ($route instanceof RouteCollection) {
return $this->getFirstNamedRoute($route, $name);
}
$collection = new RouteCollection();
$collection->add('foo', new Route('/foo'));
if ($name === $key) {
return $route;
}
}
$collection1 = new RouteCollection();
$collection1->add('bar', $bar = new Route('/bar'));
$collection1->add('foo', $foo = new Route('/foo-new'));
$collection2 = new RouteCollection();
$collection2->add('grandchild', $grandchild = new Route('/grandchild'));
$collection1->addCollection($collection2);
$collection->addCollection($collection1);
$collection->add('last', $last = new Route('/last'));
$this->assertSame(array('bar' => $bar, 'foo' => $foo, 'grandchild' => $grandchild, 'last' => $last), $collection->all(),
'->addCollection() imports routes of another collection, overrides if necessary and adds them at the end');
}
public function testAddCollection()
public function testAddCollectionWithResources()
{
if (!class_exists('Symfony\Component\Config\Resource\FileResource')) {
$this->markTestSkipped('The "Config" component is not available');
}
$collection = new RouteCollection();
$collection->add('foo', $foo = new Route('/foo'));
$collection1 = new RouteCollection();
$collection1->add('foo', $foo1 = new Route('/foo1'));
$collection1->add('bar', $bar1 = new Route('/bar1'));
$collection->addCollection($collection1);
$this->assertEquals(array('foo' => $foo1, 'bar' => $bar1), $collection->all(), '->addCollection() adds routes from another collection');
$collection = new RouteCollection();
$collection->add('foo', $foo = new Route('/foo'));
$collection1 = new RouteCollection();
$collection1->add('foo', $foo1 = new Route('/foo1'));
$collection->addCollection($collection1, '/{foo}', array('foo' => 'foo'), array('foo' => '\d+'), array('foo' => 'bar'));
$this->assertEquals('/{foo}/foo1', $collection->get('foo')->getPath(), '->addCollection() can add a prefix to all merged routes');
$this->assertEquals(array('foo' => 'foo'), $collection->get('foo')->getDefaults(), '->addCollection() can add a prefix to all merged routes');
$this->assertEquals(array('foo' => '\d+'), $collection->get('foo')->getRequirements(), '->addCollection() can add a prefix to all merged routes');
$this->assertEquals(
array('foo' => 'bar', 'compiler_class' => 'Symfony\\Component\\Routing\\RouteCompiler'),
$collection->get('foo')->getOptions(), '->addCollection() can add an option to all merged routes'
);
$collection = new RouteCollection();
$collection->addResource($foo = new FileResource(__DIR__.'/Fixtures/foo.xml'));
$collection1 = new RouteCollection();
@ -126,6 +115,33 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase
$this->assertEquals(array($foo, $foo1), $collection->getResources(), '->addCollection() merges resources');
}
public function testAddDefaultsAndRequirementsAndOptions()
{
$collection = new RouteCollection();
$collection->add('foo', new Route('/{placeholder}'));
$collection1 = new RouteCollection();
$collection1->add('bar', new Route('/{placeholder}',
array('_controller' => 'fixed', 'placeholder' => 'default'), array('placeholder' => '.+'), array('option' => 'value'))
);
$collection->addCollection($collection1);
$collection->addDefaults(array('placeholder' => 'new-default'));
$this->assertEquals(array('placeholder' => 'new-default'), $collection->get('foo')->getDefaults(), '->addDefaults() adds defaults to all routes');
$this->assertEquals(array('_controller' => 'fixed', 'placeholder' => 'new-default'), $collection->get('bar')->getDefaults(),
'->addDefaults() adds defaults to all routes and overwrites existing ones');
$collection->addRequirements(array('placeholder' => '\d+'));
$this->assertEquals(array('placeholder' => '\d+'), $collection->get('foo')->getRequirements(), '->addRequirements() adds requirements to all routes');
$this->assertEquals(array('placeholder' => '\d+'), $collection->get('bar')->getRequirements(),
'->addRequirements() adds requirements to all routes and overwrites existing ones');
$collection->addOptions(array('option' => 'new-value'));
$this->assertEquals(
array('option' => 'new-value', 'compiler_class' => 'Symfony\\Component\\Routing\\RouteCompiler'),
$collection->get('bar')->getOptions(), '->addOptions() adds options to all routes and overwrites existing ones'
);
}
public function testAddPrefix()
{
$collection = new RouteCollection();
@ -133,30 +149,20 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase
$collection2 = new RouteCollection();
$collection2->add('bar', $bar = new Route('/bar'));
$collection->addCollection($collection2);
$this->assertSame('', $collection->getPrefix(), '->getPrefix() is initialized with ""');
$collection->addPrefix(' / ');
$this->assertSame('', $collection->getPrefix(), '->addPrefix() trims the prefix and a single slash has no effect');
$collection->addPrefix('/{admin}', array('admin' => 'admin'), array('admin' => '\d+'), array('foo' => 'bar'));
$this->assertSame('/foo', $collection->get('foo')->getPattern(), '->addPrefix() trims the prefix and a single slash has no effect');
$collection->addPrefix('/{admin}', array('admin' => 'admin'), array('admin' => '\d+'));
$this->assertEquals('/{admin}/foo', $collection->get('foo')->getPath(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals('/{admin}/bar', $collection->get('bar')->getPath(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals(array('admin' => 'admin'), $collection->get('foo')->getDefaults(), '->addPrefix() adds defaults to all routes');
$this->assertEquals(array('admin' => 'admin'), $collection->get('bar')->getDefaults(), '->addPrefix() adds defaults to all routes');
$this->assertEquals(array('admin' => '\d+'), $collection->get('foo')->getRequirements(), '->addPrefix() adds requirements to all routes');
$this->assertEquals(array('admin' => '\d+'), $collection->get('bar')->getRequirements(), '->addPrefix() adds requirements to all routes');
$this->assertEquals(
array('foo' => 'bar', 'compiler_class' => 'Symfony\\Component\\Routing\\RouteCompiler'),
$collection->get('foo')->getOptions(), '->addPrefix() adds an option to all routes'
);
$this->assertEquals(
array('foo' => 'bar', 'compiler_class' => 'Symfony\\Component\\Routing\\RouteCompiler'),
$collection->get('bar')->getOptions(), '->addPrefix() adds an option to all routes'
);
$collection->addPrefix('0');
$this->assertEquals('/0/{admin}', $collection->getPrefix(), '->addPrefix() ensures a prefix must start with a slash and must not end with a slash');
$this->assertEquals('/0/{admin}/foo', $collection->get('foo')->getPattern(), '->addPrefix() ensures a prefix must start with a slash and must not end with a slash');
$collection->addPrefix('/ /');
$this->assertSame('/ /0/{admin}', $collection->getPrefix(), '->addPrefix() can handle spaces if desired');
$this->assertSame('/ /0/{admin}/foo', $collection->get('foo')->getPath(), 'the route path is in synch with the collection prefix');
$this->assertSame('/ /0/{admin}/bar', $collection->get('bar')->getPath(), 'the route path in a sub-collection is in synch with the collection prefix');
$this->assertSame('/ /0/{admin}/foo', $collection->get('foo')->getPath(), '->addPrefix() can handle spaces if desired');
$this->assertSame('/ /0/{admin}/bar', $collection->get('bar')->getPath(), 'the route pattern of an added collection is in synch with the added prefix');
}
public function testAddPrefixOverridesDefaultsAndRequirements()
@ -170,19 +176,6 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase
$this->assertEquals('https', $collection->get('bar')->getRequirement('_scheme'), '->addPrefix() overrides existing requirements');
}
public function testAddCollectionOverridesDefaultsAndRequirements()
{
$imported = new RouteCollection();
$imported->add('foo', $foo = new Route('/foo'));
$imported->add('bar', $bar = new Route('/bar', array(), array('_scheme' => 'http')));
$collection = new RouteCollection();
$collection->addCollection($imported, null, array(), array('_scheme' => 'https'));
$this->assertEquals('https', $collection->get('foo')->getRequirement('_scheme'), '->addCollection() overrides existing requirements');
$this->assertEquals('https', $collection->get('bar')->getRequirement('_scheme'), '->addCollection() overrides existing requirements');
}
public function testResource()
{
if (!class_exists('Symfony\Component\Config\Resource\FileResource')) {
@ -191,7 +184,11 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase
$collection = new RouteCollection();
$collection->addResource($foo = new FileResource(__DIR__.'/Fixtures/foo.xml'));
$this->assertEquals(array($foo), $collection->getResources(), '->addResources() adds a resource');
$collection->addResource($bar = new FileResource(__DIR__.'/Fixtures/bar.xml'));
$collection->addResource(new FileResource(__DIR__.'/Fixtures/foo.xml'));
$this->assertEquals(array($foo, $bar), $collection->getResources(),
'->addResource() adds a resource and getResources() only returns unique ones by comparing the string representation');
}
public function testUniqueRouteWithGivenName()
@ -217,13 +214,31 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase
$collection2 = new RouteCollection();
$collection2->add('b', $b = new Route('/b'));
$collection1->addCollection($collection2);
$collection1->add('$péß^a|', $c = new Route('/special'));
$this->assertSame($b, $collection1->get('b'), '->get() returns correct route in child collection');
$this->assertSame($c, $collection1->get('$péß^a|'), '->get() can handle special characters');
$this->assertNull($collection2->get('a'), '->get() does not return the route defined in parent collection');
$this->assertNull($collection1->get('non-existent'), '->get() returns null when route does not exist');
$this->assertNull($collection1->get(0), '->get() does not disclose internal child RouteCollection');
}
public function testRemove()
{
$collection = new RouteCollection();
$collection->add('foo', $foo = new Route('/foo'));
$collection1 = new RouteCollection();
$collection1->add('bar', $bar = new Route('/bar'));
$collection->addCollection($collection1);
$collection->add('last', $last = new Route('/last'));
$collection->remove('foo');
$this->assertSame(array('bar' => $bar, 'last' => $last), $collection->all(), '->remove() can remove a single route');
$collection->remove(array('bar', 'last'));
$this->assertSame(array(), $collection->all(), '->remove() accepts an array and can remove multiple routes at once');
}
public function testSetHost()
{
$collection = new RouteCollection();