Remove usage of deprecated _scheme in Routing Component

Instead correctly use the array of schemes from the Route.
Also adjusted the dumpers to dump the correct data.

I extended the tests to not only test the deprecated behavior, but also
the new schemes-requirement.
This commit is contained in:
Daniel Tschinder 2013-11-23 00:07:07 +01:00 committed by Fabien Potencier
parent 6063b499e7
commit 8cd8ec001a
14 changed files with 219 additions and 34 deletions

View File

@ -38,7 +38,7 @@ class RedirectableUrlMatcherTest extends \PHPUnit_Framework_TestCase
);
}
public function testSchemeRedirect()
public function testSchemeRedirectBC()
{
$coll = new RouteCollection();
$coll->add('foo', new Route('/foo', array(), array('_scheme' => 'https')));
@ -57,4 +57,24 @@ class RedirectableUrlMatcherTest extends \PHPUnit_Framework_TestCase
$matcher->match('/foo')
);
}
public function testSchemeRedirect()
{
$coll = new RouteCollection();
$coll->add('foo', new Route('/foo', array(), array(), array(), '', array('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')
);
}
}

View File

@ -92,6 +92,7 @@ EOF;
$properties[] = $route->getRequirements();
$properties[] = $compiledRoute->getTokens();
$properties[] = $compiledRoute->getHostTokens();
$properties[] = $route->getSchemes();
$routes .= sprintf(" '%s' => %s,\n", $name, str_replace("\n", '', var_export($properties, true)));
}
@ -114,9 +115,9 @@ EOF;
throw new RouteNotFoundException(sprintf('Unable to generate a URL for the named route "%s" as such route does not exist.', \$name));
}
list(\$variables, \$defaults, \$requirements, \$tokens, \$hostTokens) = self::\$declaredRoutes[\$name];
list(\$variables, \$defaults, \$requirements, \$tokens, \$hostTokens, \$requiredSchemes) = self::\$declaredRoutes[\$name];
return \$this->doGenerate(\$variables, \$defaults, \$requirements, \$tokens, \$parameters, \$name, \$referenceType, \$hostTokens);
return \$this->doGenerate(\$variables, \$defaults, \$requirements, \$tokens, \$parameters, \$name, \$referenceType, \$hostTokens, \$requiredSchemes);
}
EOF;
}

View File

@ -137,7 +137,7 @@ class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInt
// the Route has a cache of its own and is not recompiled as long as it does not get modified
$compiledRoute = $route->compile();
return $this->doGenerate($compiledRoute->getVariables(), $route->getDefaults(), $route->getRequirements(), $compiledRoute->getTokens(), $parameters, $name, $referenceType, $compiledRoute->getHostTokens());
return $this->doGenerate($compiledRoute->getVariables(), $route->getDefaults(), $route->getRequirements(), $compiledRoute->getTokens(), $parameters, $name, $referenceType, $compiledRoute->getHostTokens(), $route->getSchemes());
}
/**
@ -145,7 +145,7 @@ class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInt
* @throws InvalidParameterException When a parameter value for a placeholder is not correct because
* it does not match the requirement
*/
protected function doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens)
protected function doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens, array $requiredSchemes = array())
{
$variables = array_flip($variables);
$mergedParams = array_replace($defaults, $this->context->getParameters(), $parameters);
@ -204,7 +204,24 @@ class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInt
$schemeAuthority = '';
if ($host = $this->context->getHost()) {
$scheme = $this->context->getScheme();
if (isset($requirements['_scheme']) && ($req = strtolower($requirements['_scheme'])) && $scheme !== $req) {
if ($requiredSchemes) {
$schemeMatched = false;
foreach ($requiredSchemes as $requiredScheme) {
if ($scheme === $requiredScheme) {
$schemeMatched = true;
break;
}
}
if (!$schemeMatched) {
$referenceType = self::ABSOLUTE_URL;
$scheme = current($requiredSchemes);
}
} elseif (isset($requirements['_scheme']) && ($req = strtolower($requirements['_scheme'])) && $scheme !== $req) {
// We do this for BC; to be removed if _scheme is not supported anymore
$referenceType = self::ABSOLUTE_URL;
$scheme = $req;
}

View File

@ -288,14 +288,15 @@ EOF;
EOF;
}
if ($scheme = $route->getRequirement('_scheme')) {
if ($schemes = $route->getSchemes()) {
if (!$supportsRedirections) {
throw new \LogicException('The "_scheme" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
throw new \LogicException('The "schemes" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
}
$schemes = str_replace("\n", '', var_export(array_flip($schemes), true));
$code .= <<<EOF
if (\$this->context->getScheme() !== '$scheme') {
return \$this->redirect(\$pathinfo, '$name', '$scheme');
\$requiredSchemes = $schemes;
if (!isset(\$requiredSchemes[\$this->context->getScheme()])) {
return \$this->redirect(\$pathinfo, '$name', key(\$requiredSchemes));
}
@ -313,8 +314,11 @@ EOF;
}
$vars[] = "array('_route' => '$name')";
$code .= sprintf(" return \$this->mergeDefaults(array_replace(%s), %s);\n"
, implode(', ', $vars), str_replace("\n", '', var_export($route->getDefaults(), true)));
$code .= sprintf(
" return \$this->mergeDefaults(array_replace(%s), %s);\n",
implode(', ', $vars),
str_replace("\n", '', var_export($route->getDefaults(), true))
);
} elseif ($route->getDefaults()) {
$code .= sprintf(" return %s;\n", str_replace("\n", '', var_export(array_replace($route->getDefaults(), array('_route' => $name)), true)));

View File

@ -56,9 +56,10 @@ abstract class RedirectableUrlMatcher extends UrlMatcher implements Redirectable
}
// check HTTP scheme requirement
$scheme = $route->getRequirement('_scheme');
if ($scheme && $this->context->getScheme() !== $scheme) {
return array(self::ROUTE_MATCH, $this->redirect($pathinfo, $name, $scheme));
$scheme = $this->context->getScheme();
$schemes = $route->getSchemes();
if ($schemes && !$route->hasScheme($scheme)) {
return array(self::ROUTE_MATCH, $this->redirect($pathinfo, $name, current($schemes)));
}
return array(self::REQUIREMENT_MATCH, null);

View File

@ -103,9 +103,11 @@ class TraceableUrlMatcher extends UrlMatcher
}
// check HTTP scheme requirement
if ($scheme = $route->getRequirement('_scheme')) {
if ($this->context->getScheme() !== $scheme) {
$this->addTrace(sprintf('Scheme "%s" does not match the requirement ("%s"); the user will be redirected', $this->context->getScheme(), $scheme), self::ROUTE_ALMOST_MATCHES, $name, $route);
if ($requiredSchemes = $route->getSchemes()) {
$scheme = $this->context->getScheme();
if (!$route->hasScheme($scheme)) {
$this->addTrace(sprintf('Scheme "%s" does not match any of the required schemes ("%s"); the user will be redirected to first required scheme', $scheme, implode(', ', $requiredSchemes)), self::ROUTE_ALMOST_MATCHES, $name, $route);
return true;
}

View File

@ -205,8 +205,8 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
}
// check HTTP scheme requirement
$scheme = $route->getRequirement('_scheme');
$status = $scheme && $scheme !== $this->context->getScheme() ? self::REQUIREMENT_MISMATCH : self::REQUIREMENT_MATCH;
$scheme = $this->context->getScheme();
$status = $route->getSchemes() && !$route->hasScheme($scheme) ? self::REQUIREMENT_MISMATCH : self::REQUIREMENT_MATCH;
return array($status, null);
}

View File

@ -247,6 +247,25 @@ class Route implements \Serializable
return $this;
}
/**
* Checks if a scheme requirement has been set.
*
* @param string $scheme
*
* @return Boolean true if the scheme requirement exists, otherwise false
*/
public function hasScheme($scheme)
{
$scheme = strtolower($scheme);
foreach ($this->schemes as $requiredScheme) {
if ($scheme === $requiredScheme) {
return true;
}
}
return false;
}
/**
* Returns the uppercased HTTP methods this route is restricted to.
* So an empty array means that any method is allowed.

View File

@ -321,8 +321,9 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
// secure
if ($pathinfo === '/secure') {
if ($this->context->getScheme() !== 'https') {
return $this->redirect($pathinfo, 'secure', 'https');
$requiredSchemes = array ( 'https' => 0,);
if (!isset($requiredSchemes[$this->context->getScheme()])) {
return $this->redirect($pathinfo, 'secure', key($requiredSchemes));
}
return array('_route' => 'secure');
@ -330,8 +331,9 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
// nonsecure
if ($pathinfo === '/nonsecure') {
if ($this->context->getScheme() !== 'http') {
return $this->redirect($pathinfo, 'nonsecure', 'http');
$requiredSchemes = array ( 'http' => 0,);
if (!isset($requiredSchemes[$this->context->getScheme()])) {
return $this->redirect($pathinfo, 'nonsecure', key($requiredSchemes));
}
return array('_route' => 'nonsecure');

View File

@ -114,4 +114,37 @@ class PhpGeneratorDumperTest extends \PHPUnit_Framework_TestCase
$this->assertEquals($url, '/testing');
}
public function testDumpWithSchemeRequirement()
{
$this->routeCollection->add('Test1', new Route('/testing', array(), array(), array(), '', array('ftp', 'https')));
$this->routeCollection->add('Test2', new Route('/testing_bc', array(), array('_scheme' => 'https'))); // BC
file_put_contents($this->testTmpFilepath, $this->generatorDumper->dump(array('class' => 'SchemeUrlGenerator')));
include ($this->testTmpFilepath);
$projectUrlGenerator = new \SchemeUrlGenerator(new RequestContext('/app.php'));
$absoluteUrl = $projectUrlGenerator->generate('Test1', array(), true);
$absoluteUrlBC = $projectUrlGenerator->generate('Test2', array(), true);
$relativeUrl = $projectUrlGenerator->generate('Test1', array(), false);
$relativeUrlBC = $projectUrlGenerator->generate('Test2', array(), false);
$this->assertEquals($absoluteUrl, 'ftp://localhost/app.php/testing');
$this->assertEquals($absoluteUrlBC, 'https://localhost/app.php/testing_bc');
$this->assertEquals($relativeUrl, 'ftp://localhost/app.php/testing');
$this->assertEquals($relativeUrlBC, 'https://localhost/app.php/testing_bc');
$projectUrlGenerator = new \SchemeUrlGenerator(new RequestContext('/app.php', 'GET', 'localhost', 'https'));
$absoluteUrl = $projectUrlGenerator->generate('Test1', array(), true);
$absoluteUrlBC = $projectUrlGenerator->generate('Test2', array(), true);
$relativeUrl = $projectUrlGenerator->generate('Test1', array(), false);
$relativeUrlBC = $projectUrlGenerator->generate('Test2', array(), false);
$this->assertEquals($absoluteUrl, 'https://localhost/app.php/testing');
$this->assertEquals($absoluteUrlBC, 'https://localhost/app.php/testing_bc');
$this->assertEquals($relativeUrl, '/app.php/testing');
$this->assertEquals($relativeUrlBC, '/app.php/testing_bc');
}
}

View File

@ -244,20 +244,38 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
public function testSchemeRequirementDoesNothingIfSameCurrentScheme()
{
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http')));
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); // BC
$this->assertEquals('/app.php/', $this->getGenerator($routes)->generate('test'));
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https')));
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); // BC
$this->assertEquals('/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
$routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('http')));
$this->assertEquals('/app.php/', $this->getGenerator($routes)->generate('test'));
$routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('https')));
$this->assertEquals('/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
}
public function testSchemeRequirementForcesAbsoluteUrl()
{
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https')));
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); // BC
$this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test'));
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http')));
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); // BC
$this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
$routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('https')));
$this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test'));
$routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('http')));
$this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
}
public function testSchemeRequirementCreatesUrlForFirstRequiredScheme()
{
$routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('Ftp', 'https')));
$this->assertEquals('ftp://localhost/app.php/', $this->getGenerator($routes)->generate('test'));
}
public function testPathWithTwoStartingSlashes()
@ -443,7 +461,7 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
$this->assertNull($generator->generate('test', array('foo' => 'baz'), false));
}
public function testGenerateNetworkPath()
public function testGenerateNetworkPathBC()
{
$routes = $this->getRoutes('test', new Route('/{name}', array(), array('_scheme' => 'http'), array(), '{locale}.example.com'));
@ -461,13 +479,32 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
);
}
public function testGenerateNetworkPath()
{
$routes = $this->getRoutes('test', new Route('/{name}', array(), array(), array(), '{locale}.example.com', array('http')));
$this->assertSame('//fr.example.com/app.php/Fabien', $this->getGenerator($routes)->generate('test',
array('name' =>'Fabien', 'locale' => 'fr'), UrlGeneratorInterface::NETWORK_PATH), 'network path with different host'
);
$this->assertSame('//fr.example.com/app.php/Fabien?query=string', $this->getGenerator($routes, array('host' => 'fr.example.com'))->generate('test',
array('name' =>'Fabien', 'locale' => 'fr', 'query' => 'string'), UrlGeneratorInterface::NETWORK_PATH), 'network path although host same as context'
);
$this->assertSame('http://fr.example.com/app.php/Fabien', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test',
array('name' =>'Fabien', 'locale' => 'fr'), UrlGeneratorInterface::NETWORK_PATH), 'absolute URL because scheme requirement does not match context'
);
$this->assertSame('http://fr.example.com/app.php/Fabien', $this->getGenerator($routes)->generate('test',
array('name' =>'Fabien', 'locale' => 'fr'), UrlGeneratorInterface::ABSOLUTE_URL), 'absolute URL with same scheme because it is requested'
);
}
public function testGenerateRelativePath()
{
$routes = new RouteCollection();
$routes->add('article', new Route('/{author}/{article}/'));
$routes->add('comments', new Route('/{author}/{article}/comments'));
$routes->add('host', new Route('/{article}', array(), array(), array(), '{author}.example.com'));
$routes->add('scheme', new Route('/{author}', array(), array('_scheme' => 'https')));
$routes->add('schemeBC', new Route('/{author}', array(), array('_scheme' => 'https'))); // BC
$routes->add('scheme', new Route('/{author}/blog', array(), array(), array(), '', array('https')));
$routes->add('unrelated', new Route('/about'));
$generator = $this->getGenerator($routes, array('host' => 'example.com', 'pathInfo' => '/fabien/symfony-is-great/'));
@ -487,9 +524,12 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
$this->assertSame('//bernhard.example.com/app.php/forms-are-great', $generator->generate('host',
array('author' =>'bernhard', 'article' => 'forms-are-great'), UrlGeneratorInterface::RELATIVE_PATH)
);
$this->assertSame('https://example.com/app.php/bernhard', $generator->generate('scheme',
$this->assertSame('https://example.com/app.php/bernhard', $generator->generate('schemeBC',
array('author' =>'bernhard'), UrlGeneratorInterface::RELATIVE_PATH)
);
$this->assertSame('https://example.com/app.php/bernhard/blog', $generator->generate('scheme',
array('author' =>'bernhard'), UrlGeneratorInterface::RELATIVE_PATH)
);
$this->assertSame('../../about', $generator->generate('unrelated',
array(), UrlGeneratorInterface::RELATIVE_PATH)
);

View File

@ -41,7 +41,7 @@ class RedirectableUrlMatcherTest extends \PHPUnit_Framework_TestCase
$matcher->match('/foo');
}
public function testSchemeRedirect()
public function testSchemeRedirectBC()
{
$coll = new RouteCollection();
$coll->add('foo', new Route('/foo', array(), array('_scheme' => 'https')));
@ -55,4 +55,32 @@ class RedirectableUrlMatcherTest extends \PHPUnit_Framework_TestCase
;
$matcher->match('/foo');
}
public function testSchemeRedirectRedirectsToFirstScheme()
{
$coll = new RouteCollection();
$coll->add('foo', new Route('/foo', array(), array(), array(), '', array('FTP', 'HTTPS')));
$matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
$matcher
->expects($this->once())
->method('redirect')
->with('/foo', 'foo', 'ftp')
->will($this->returnValue(array('_route' => 'foo')))
;
$matcher->match('/foo');
}
public function testNoSchemaRedirectIfOnOfMultipleSchemesMatches()
{
$coll = new RouteCollection();
$coll->add('foo', new Route('/foo', array(), array(), array(), '', array('https', 'http')));
$matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
$matcher
->expects($this->never())
->method('redirect')
;
$matcher->match('/foo');
}
}

View File

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

View File

@ -153,10 +153,15 @@ class RouteTest extends \PHPUnit_Framework_TestCase
{
$route = new Route('/');
$this->assertEquals(array(), $route->getSchemes(), 'schemes is initialized with array()');
$this->assertFalse($route->hasScheme('http'));
$route->setSchemes('hTTp');
$this->assertEquals(array('http'), $route->getSchemes(), '->setSchemes() accepts a single scheme string and lowercases it');
$this->assertTrue($route->hasScheme('htTp'));
$this->assertFalse($route->hasScheme('httpS'));
$route->setSchemes(array('HttpS', 'hTTp'));
$this->assertEquals(array('https', 'http'), $route->getSchemes(), '->setSchemes() accepts an array of schemes and lowercases them');
$this->assertTrue($route->hasScheme('htTp'));
$this->assertTrue($route->hasScheme('httpS'));
}
public function testSchemeIsBC()
@ -165,6 +170,9 @@ class RouteTest extends \PHPUnit_Framework_TestCase
$route->setRequirement('_scheme', 'http|https');
$this->assertEquals('http|https', $route->getRequirement('_scheme'));
$this->assertEquals(array('http', 'https'), $route->getSchemes());
$this->assertTrue($route->hasScheme('https'));
$this->assertTrue($route->hasScheme('http'));
$this->assertFalse($route->hasScheme('ftp'));
$route->setSchemes(array('hTTp'));
$this->assertEquals('http', $route->getRequirement('_scheme'));
$route->setSchemes(array());