Skip to content

Commit

Permalink
[Routing] added the possibility to define default values and requirem…
Browse files Browse the repository at this point in the history
…ents for placeholders in prefix
  • Loading branch information
fabpot committed Oct 23, 2011
1 parent 1bd6e4d commit 2e1344e
Show file tree
Hide file tree
Showing 12 changed files with 108 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-2.1.md
Expand Up @@ -79,6 +79,7 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c

### Routing

* added the possibility to define default values and requirements for placeholders in prefix
* added RouterInterface::getRouteCollection

### Translation
Expand Down
23 changes: 22 additions & 1 deletion src/Symfony/Component/Routing/Loader/XmlFileLoader.php
Expand Up @@ -76,8 +76,29 @@ protected function parseNode(RouteCollection $collection, \DOMElement $node, $pa
$resource = (string) $node->getAttribute('resource');
$type = (string) $node->getAttribute('type');
$prefix = (string) $node->getAttribute('prefix');

$defaults = array();
$requirements = array();

foreach ($node->childNodes as $n) {
if (!$n instanceof \DOMElement) {
continue;
}

switch ($n->tagName) {
case 'default':
$defaults[(string) $n->getAttribute('key')] = trim((string) $n->nodeValue);
break;
case 'requirement':
$requirements[(string) $n->getAttribute('key')] = trim((string) $n->nodeValue);
break;
default:
throw new \InvalidArgumentException(sprintf('Unable to parse tag "%s"', $n->tagName));
}
}

$this->setCurrentDir(dirname($path));
$collection->addCollection($this->import($resource, ('' !== $type ? $type : null), false, $file), $prefix);
$collection->addCollection($this->import($resource, ('' !== $type ? $type : null), false, $file), $prefix, $defaults, $requirements);
break;
default:
throw new \InvalidArgumentException(sprintf('Unable to parse tag "%s"', $node->tagName));
Expand Down
5 changes: 4 additions & 1 deletion src/Symfony/Component/Routing/Loader/YamlFileLoader.php
Expand Up @@ -67,8 +67,11 @@ public function load($file, $type = null)
if (isset($config['resource'])) {
$type = isset($config['type']) ? $config['type'] : null;
$prefix = isset($config['prefix']) ? $config['prefix'] : null;
$defaults = isset($config['defaults']) ? $config['defaults'] : array();
$requirements = isset($config['requirements']) ? $config['requirements'] : array();

$this->setCurrentDir(dirname($path));
$collection->addCollection($this->import($config['resource'], $type, false, $file), $prefix);
$collection->addCollection($this->import($config['resource'], $type, false, $file), $prefix, $defaults, $requirements);
} else {
$this->parseRoute($collection, $name, $config, $path);
}
Expand Down
Expand Up @@ -26,6 +26,11 @@
</xsd:complexType>

<xsd:complexType name="import">
<xsd:sequence>
<xsd:element name="default" type="element" minOccurs="0" maxOccurs="unbounded" />
<xsd:element name="requirement" type="element" minOccurs="0" maxOccurs="unbounded" />
</xsd:sequence>

<xsd:attribute name="resource" type="xsd:string" />
<xsd:attribute name="type" type="xsd:string" />
<xsd:attribute name="prefix" type="xsd:string" />
Expand Down
30 changes: 30 additions & 0 deletions src/Symfony/Component/Routing/Route.php
Expand Up @@ -162,6 +162,21 @@ public function getDefaults()
public function setDefaults(array $defaults)
{
$this->defaults = array();

return $this->addDefaults($defaults);
}

/**
* Adds defaults.
*
* This method implements a fluent interface.
*
* @param array $defaults The defaults
*
* @return Route The current Route instance
*/
public function addDefaults(array $defaults)
{
foreach ($defaults as $name => $default) {
$this->defaults[(string) $name] = $default;
}
Expand Down Expand Up @@ -232,6 +247,21 @@ public function getRequirements()
public function setRequirements(array $requirements)
{
$this->requirements = array();

return $this->addRequirements($requirements);
}

/**
* Adds requirements.
*
* This method implements a fluent interface.
*
* @param array $requirements The requirements
*
* @return Route The current Route instance
*/
public function addRequirements(array $requirements)
{
foreach ($requirements as $key => $regex) {
$this->requirements[$key] = $this->sanitizeRequirement($key, $regex);
}
Expand Down
20 changes: 13 additions & 7 deletions src/Symfony/Component/Routing/RouteCollection.php
Expand Up @@ -165,15 +165,17 @@ public function remove($name)
/**
* Adds a route collection to the current set of routes (at the end of the current set).
*
* @param RouteCollection $collection A RouteCollection instance
* @param string $prefix An optional prefix to add before each pattern of the route collection
* @param RouteCollection $collection A RouteCollection instance
* @param string $prefix An optional prefix to add before each pattern of the route collection
* @param array $defaults An array of default values
* @param array $requirements An array of requirements
*
* @api
*/
public function addCollection(RouteCollection $collection, $prefix = '')
public function addCollection(RouteCollection $collection, $prefix = '', $defaults = array(), $requirements = array())
{
$collection->setParent($this);
$collection->addPrefix($prefix);
$collection->addPrefix($prefix, $defaults, $requirements);

// remove all routes with the same name in all existing collections
foreach (array_keys($collection->all()) as $name) {
Expand All @@ -186,11 +188,13 @@ public function addCollection(RouteCollection $collection, $prefix = '')
/**
* Adds a prefix to all routes in the current set.
*
* @param string $prefix An optional prefix to add before each pattern of the route collection
* @param string $prefix An optional prefix to add before each pattern of the route collection
* @param array $defaults An array of default values
* @param array $requirements An array of requirements
*
* @api
*/
public function addPrefix($prefix)
public function addPrefix($prefix, $defaults = array(), $requirements = array())
{
// a prefix must not end with a slash
$prefix = rtrim($prefix, '/');
Expand All @@ -208,9 +212,11 @@ public function addPrefix($prefix)

foreach ($this->routes as $name => $route) {
if ($route instanceof RouteCollection) {
$route->addPrefix($prefix);
$route->addPrefix($prefix, $defaults, $requirements);
} else {
$route->setPattern($prefix.$route->getPattern());
$route->addDefaults($defaults);
$route->addRequirements($requirements);

This comment has been minimized.

Copy link
@stof

stof Oct 23, 2011

Member

This implementation let me think that requirements and defaults set when importing the file win over requirements specified directly for the route. Am I right ? and if yes, is it the intended behavior ?

This comment has been minimized.

Copy link
@fabpot

fabpot Oct 23, 2011

Author Member

Right, and that should not be a problem as a placeholder in a prefix should probably not be used in the embedded routes. And this has the nice side-effect of being able to override the _scheme for all the routes for instance.

This comment has been minimized.

Copy link
@stof

stof Oct 23, 2011

Member

Indeed, it gives the power to the end user when importing the routing of a third-party bundle so it is a good way. (it also give the end-user the power to break the imported routing when adding requirements ^^)

}
}
}
Expand Down
Expand Up @@ -4,5 +4,8 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/routing http://symfony.com/schema/routing/routing-1.0.xsd">

<import resource="validpattern.xml" />
<import resource="validpattern.xml" prefix="/{foo}">
<default key="foo">foo</default>
<requirement key="foo">\d+</requirement>
</import>
</routes>
@@ -1,2 +1,5 @@
blog_show:
resource: validpattern.yml
resource: validpattern.yml
prefix: /{foo}
defaults: { 'foo': 'foo' }
requirements: { 'foo': '\d+' }
Expand Up @@ -51,6 +51,8 @@ public function testLoadWithImport()

$this->assertEquals(1, count($routes), 'One route is loaded');
$this->assertContainsOnly('Symfony\Component\Routing\Route', $routes);
$this->assertEquals('foo', $routes['blog_show']->getDefault('foo'));
$this->assertEquals('\d+', $routes['blog_show']->getRequirement('foo'));
}

/**
Expand Down Expand Up @@ -89,4 +91,3 @@ protected function validate(\DOMDocument $dom)
return true;
}
}

Expand Up @@ -88,6 +88,8 @@ public function testLoadWithResource()

$this->assertEquals(1, count($routes), 'One route is loaded');
$this->assertContainsOnly('Symfony\Component\Routing\Route', $routes);
$this->assertEquals('foo', $routes['blog_show']->getDefault('foo'));
$this->assertEquals('\d+', $routes['blog_show']->getRequirement('foo'));
}

/**
Expand Down
16 changes: 11 additions & 5 deletions tests/Symfony/Tests/Component/Routing/RouteCollectionTest.php
Expand Up @@ -103,8 +103,10 @@ public function testAddCollection()
$collection->add('foo', $foo = new Route('/foo'));
$collection1 = new RouteCollection();
$collection1->add('foo', $foo1 = new Route('/foo1'));
$collection->addCollection($collection1, '/foo');
$this->assertEquals('/foo/foo1', $collection->get('foo')->getPattern(), '->addCollection() can add a prefix to all merged routes');
$collection->addCollection($collection1, '/{foo}', array('foo' => 'foo'), array('foo' => '\d+'));
$this->assertEquals('/{foo}/foo1', $collection->get('foo')->getPattern(), '->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');

$collection = new RouteCollection();
$collection->addResource($foo = new FileResource(__DIR__.'/Fixtures/foo.xml'));
Expand All @@ -119,9 +121,13 @@ public function testAddPrefix()
$collection = new RouteCollection();
$collection->add('foo', $foo = new Route('/foo'));
$collection->add('bar', $bar = new Route('/bar'));
$collection->addPrefix('/admin');
$this->assertEquals('/admin/foo', $collection->get('foo')->getPattern(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals('/admin/bar', $collection->get('bar')->getPattern(), '->addPrefix() adds a prefix to all routes');
$collection->addPrefix('/{admin}', array('admin' => 'admin'), array('admin' => '\d+'));
$this->assertEquals('/{admin}/foo', $collection->get('foo')->getPattern(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals('/{admin}/bar', $collection->get('bar')->getPattern(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals(array('admin' => 'admin'), $collection->get('foo')->getDefaults(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals(array('admin' => 'admin'), $collection->get('bar')->getDefaults(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals(array('admin' => '\d+'), $collection->get('foo')->getRequirements(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals(array('admin' => '\d+'), $collection->get('bar')->getRequirements(), '->addPrefix() adds a prefix to all routes');
}

public function testResource()
Expand Down
10 changes: 10 additions & 0 deletions tests/Symfony/Tests/Component/Routing/RouteTest.php
Expand Up @@ -68,6 +68,11 @@ public function testDefaults()

$route->setDefault('_controller', $closure = function () { return 'Hello'; });
$this->assertEquals($closure, $route->getDefault('_controller'), '->setDefault() sets a default value');

$route->setDefaults(array('foo' => 'foo'));
$route->addDefaults(array('bar' => 'bar'));
$this->assertEquals($route, $route->addDefaults(array()), '->addDefaults() implements a fluent interface');
$this->assertEquals(array('foo' => 'foo', 'bar' => 'bar'), $route->getDefaults(), '->addDefaults() keep previous defaults');
}

public function testRequirements()
Expand All @@ -80,6 +85,11 @@ public function testRequirements()
$route->setRequirements(array('foo' => '^\d+$'));
$this->assertEquals('\d+', $route->getRequirement('foo'), '->getRequirement() removes ^ and $ from the pattern');
$this->assertEquals($route, $route->setRequirements(array()), '->setRequirements() implements a fluent interface');

$route->setRequirements(array('foo' => '\d+'));
$route->addRequirements(array('bar' => '\d+'));
$this->assertEquals($route, $route->addRequirements(array()), '->addRequirements() implements a fluent interface');
$this->assertEquals(array('foo' => '\d+', 'bar' => '\d+'), $route->getRequirements(), '->addRequirement() keep previous requirements');
}

public function testRequirement()
Expand Down

1 comment on commit 2e1344e

@arghav
Copy link
Contributor

@arghav arghav commented on 2e1344e Jan 6, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this support default values for special routing parameters like _locale in prefix?

Please sign in to comment.