Skip to content

Commit

Permalink
merged branch Tobion/deprecation-trigger (PR symfony#7266)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fabpot committed Mar 6, 2013
2 parents 5e3756f + c0687cd commit d0022e3
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 92 deletions.
24 changes: 22 additions & 2 deletions src/Symfony/Component/Routing/RouteCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ public function __clone()
*/
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;
}

Expand All @@ -77,6 +80,9 @@ public function getParent()
*/
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();
Expand Down Expand Up @@ -157,7 +163,10 @@ public function get($name)
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]);
Expand All @@ -184,6 +193,8 @@ public function addCollection(RouteCollection $collection)
// 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));
Expand Down Expand Up @@ -232,7 +243,13 @@ public function addPrefix($prefix, array $defaults = array(), array $requirement
$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());
Expand All @@ -251,6 +268,9 @@ public function addPrefix($prefix, array $defaults = array(), array $requirement
*/
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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ private function getRouteCollection()
$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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,17 @@ public function getRouteCollections()
$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();
Expand All @@ -137,23 +140,25 @@ public function getRouteCollections()
$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'));

// 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

Expand Down Expand Up @@ -215,10 +220,12 @@ public function getRouteCollections()
$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 */

Expand Down
20 changes: 6 additions & 14 deletions src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,29 +130,21 @@ public function testMatch()

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'));
}

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'));
Expand Down
Loading

0 comments on commit d0022e3

Please sign in to comment.