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

merged branch vicb/routing-ok (PR #3313) 

Commits
-------

9d6eb82 [Routing] Fix a bug in the TraceableUrlMatcher
9fc8d28 [FrameworkBundle] Fix a bug in the RedirectableUrlMatcher
4fcf9ef [Routing] Small optimization in the UrlMatcher
abc2141 [Routing] Added a missing property declaration
d86e1eb [Routing] Remove a weird dependency

Discussion
----------

[Routing] Remove a dependency on a derived class, fixes, optim

Subset of #3296 which should be acceptable.

Travis is happy.

The side effect of removing the dependency is that the `UrlMatcher` does not throw an exception any more when the scheme does not match the required scheme. I think it is better because:

* it removes a dependency on a derived class,
* it was an undocumented "feature",
* other thrown excs are component specific while this one was raw SPL.

---------------------------------------------------------------------------

by vicb at 2012-02-09T14:43:02Z

let me know what should go in 2.0 as well.
This commit is contained in:
Fabien Potencier 2012-02-15 00:01:15 +01:00
parent b8322b306a 9d6eb8216e
commit 803fba887a
7 changed files with 131 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG-2.1.md
View File

@ -279,6 +279,7 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
### Routing
* the UrlMatcher does not throw a \LogicException any more when the required scheme is not the current one
* added a TraceableUrlMatcher
* added the possibility to define default values and requirements for placeholders in prefix, including imported routes
* added RouterInterface::getRouteCollection

5
src/Symfony/Bundle/FrameworkBundle/Routing/RedirectableUrlMatcher.php
View File

@ -11,13 +11,12 @@
namespace Symfony\Bundle\FrameworkBundle\Routing;
use Symfony\Component\Routing\Matcher\UrlMatcher;
use Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface;
use Symfony\Component\Routing\Matcher\RedirectableUrlMatcher as BaseMatcher;
/**
* @author Fabien Potencier <fabien@symfony.com>
*/
class RedirectableUrlMatcher extends UrlMatcher implements RedirectableUrlMatcherInterface
class RedirectableUrlMatcher extends BaseMatcher
{
/**
* Redirects the user to another URL.

61
src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RedirectableUrlMatcherTest.php Normal file
View File

@ -0,0 +1,61 @@
<?php
/*
* This file is part of the Symfony framework.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* This source file is subject to the MIT license that is bundled
* with this source code in the file LICENSE.
*/
namespace Symfony\Bundle\FrameworkBundle\Tests\Routing;
use Symfony\Bundle\FrameworkBundle\Routing\Router;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;
use Symfony\Bundle\FrameworkBundle\Routing\RedirectableUrlMatcher;
use Symfony\Component\Routing\RequestContext;
class RedirectableUrlMatcherTest extends \PHPUnit_Framework_TestCase
{
public function testRedirectWhenNoSlash()
{
$coll = new RouteCollection();
$coll->add('foo', new Route('/foo/'));
$matcher = new RedirectableUrlMatcher($coll, $context = new RequestContext());
$this->assertEquals(array(
'_controller' => 'Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction',
'path' => '/foo/',
'permanent' => true,
'scheme' => null,
'httpPort' => $context->getHttpPort(),
'httpsPort' => $context->getHttpsPort(),
'_route' => null,
),
$matcher->match('/foo')
);
}
public function testSchemeRedirect()
{
$coll = new RouteCollection();
$coll->add('foo', new Route('/foo', array(), array('_scheme' => 'https')));
$matcher = new RedirectableUrlMatcher($coll, $context = new RequestContext());
$this->assertEquals(array(
'_controller' => 'Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction',
'path' => '/foo',
'permanent' => true,
'scheme' => 'https',
'httpPort' => $context->getHttpPort(),
'httpsPort' => $context->getHttpsPort(),
'_route' => 'foo',
),
$matcher->match('/foo')
);
}
}

33
src/Symfony/Component/Routing/Matcher/RedirectableUrlMatcher.php
View File

@ -12,6 +12,7 @@
namespace Symfony\Component\Routing\Matcher;
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
use Symfony\Component\Routing\Route;
/**
* @author Fabien Potencier <fabien@symfony.com>
@ -20,8 +21,6 @@ use Symfony\Component\Routing\Exception\ResourceNotFoundException;
*/
abstract class RedirectableUrlMatcher extends UrlMatcher implements RedirectableUrlMatcherInterface
{
private $trailingSlashTest = false;
/**
* @see UrlMatcher::match()
*
@ -36,18 +35,28 @@ abstract class RedirectableUrlMatcher extends UrlMatcher implements Redirectable
throw $e;
}
// try with a / at the end
$this->trailingSlashTest = true;
return $this->match($pathinfo.'/');
}
if ($this->trailingSlashTest) {
$this->trailingSlashTest = false;
return $this->redirect($pathinfo, null);
try {
parent::match($pathinfo.'/');
return $this->redirect($pathinfo.'/', null);
} catch (ResourceNotFoundException $e2) {
throw $e;
}
}
return $parameters;
}
/**
* {@inheritDoc}
*/
protected function handleRouteRequirements($pathinfo, $name, Route $route)
{
// check HTTP scheme requirement
$scheme = $route->getRequirement('_scheme');
if ($scheme && $this->context->getScheme() !== $scheme) {
return array(self::ROUTE_MATCH, $this->redirect($pathinfo, $name, $scheme));
}
return array(self::REQUIREMENT_MATCH, null);
}
}

2
src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php
View File

@ -73,7 +73,7 @@ class TraceableUrlMatcher extends UrlMatcher
if (in_array($n, $cr->getVariables()) && !preg_match($cr->getRegex(), $pathinfo)) {
$this->addTrace(sprintf('Requirement for "%s" does not match (%s)', $n, $regex), self::ROUTE_ALMOST_MATCHES, $name, $route);
continue;
continue 2;
}
}

55
src/Symfony/Component/Routing/Matcher/UrlMatcher.php
View File

@ -15,7 +15,7 @@ use Symfony\Component\Routing\Exception\MethodNotAllowedException;
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
use Symfony\Component\Routing\RouteCollection;
use Symfony\Component\Routing\RequestContext;
use Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface;
use Symfony\Component\Routing\Route;
/**
* UrlMatcher matches URL based on a set of routes.
@ -26,7 +26,12 @@ use Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface;
*/
class UrlMatcher implements UrlMatcherInterface
{
const REQUIREMENT_MATCH = 0;
const REQUIREMENT_MISMATCH = 1;
const ROUTE_MATCH = 2;
protected $context;
protected $allow;
private $routes;
@ -75,7 +80,7 @@ class UrlMatcher implements UrlMatcherInterface
{
$this->allow = array();
if ($ret = $this->matchCollection($pathinfo, $this->routes)) {
if ($ret = $this->matchCollection(urldecode($pathinfo), $this->routes)) {
return $ret;
}
@ -84,10 +89,19 @@ class UrlMatcher implements UrlMatcherInterface
: new ResourceNotFoundException();
}
/**
* Tries to match a URL with a set of routes.
*
* @param string $pathinfo The path info to be parsed
* @param RouteCollection $routes The set of routes
*
* @return array An array of parameters
*
* @throws ResourceNotFoundException If the resource could not be found
* @throws MethodNotAllowedException If the resource was found but the request method is not allowed
*/
protected function matchCollection($pathinfo, RouteCollection $routes)
{
$pathinfo = urldecode($pathinfo);
foreach ($routes as $name => $route) {
if ($route instanceof RouteCollection) {
if (false === strpos($route->getPrefix(), '{') && $route->getPrefix() !== substr($pathinfo, 0, strlen($route->getPrefix()))) {
@ -126,21 +140,38 @@ class UrlMatcher implements UrlMatcherInterface
}
}
// check HTTP scheme requirement
if ($scheme = $route->getRequirement('_scheme')) {
if (!$this instanceof RedirectableUrlMatcherInterface) {
throw new \LogicException('The "_scheme" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
}
$status = $this->handleRouteRequirements($pathinfo, $name, $route);
if ($this->context->getScheme() !== $scheme) {
return $this->redirect($pathinfo, $name, $scheme);
}
if (self::ROUTE_MATCH === $status[0]) {
return $status[1];
}
if (self::REQUIREMENT_MISMATCH === $status[0]) {
continue;
}
return array_merge($this->mergeDefaults($matches, $route->getDefaults()), array('_route' => $name));
}
}
/**
* Handles specific route requirements.
*
* @param string $pathinfo The path
* @param string $name The route name
* @param string $route The route
*
* @return array The first element represents the status, the second contains additional information
*/
protected function handleRouteRequirements($pathinfo, $name, Route $route)
{
// check HTTP scheme requirement
$scheme = $route->getRequirement('_scheme');
$status = $scheme && $scheme !== $this->context->getScheme() ? self::REQUIREMENT_MISMATCH : self::REQUIREMENT_MATCH;
return array($status, null);
}
protected function mergeDefaults($params, $defaults)
{
$parameters = $defaults;

5
tests/Symfony/Tests/Component/Routing/Matcher/UrlMatcherTest.php
View File

@ -207,14 +207,13 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
}
/**
* @expectedException \LogicException
* @expectedException Symfony\Component\Routing\Exception\ResourceNotFoundException
*/
public function testSchemeRequirement()
{
$coll = new RouteCollection();
$coll->add('foo', new Route('/foo', array(), array('_scheme' => 'https')));
$matcher = new UrlMatcher($coll, new RequestContext());
$matcher->match('/foo');
}
}
}