feature #13361 [Routing] apply deprecation triggers and fix tests (Tobion)

This PR was merged into the 2.7 branch.

Discussion
----------

[Routing] apply deprecation triggers and fix tests

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | [#12641, #12640, #12719]
| License       | MIT
| Doc PR        |

Correctly add deprecation for _scheme and _method requirements and fix code to not trigger deprecations itself. And fix test so that they either explicitly test deprecated functionality or alternatively refactor tests to use new style.

It also fixes the deprecation triggers for "pattern" which was not correctly done for YAML/XML loading.

Commits
-------

9af0ff2 [FrameworkBundle] fix routing container param resolving to not access deprecated requirements while maintaining BC
bd91867 [FrameworkBundle] remove superfluous test that is already covered in routing
bc1c5c8 [Routing] apply deprecation triggers and fix tests

src/Symfony/Bundle/FrameworkBundle/Routing/Router.php

@@ -77,8 +77,10 @@ class Router extends BaseRouter implements WarmableInterface
      * Replaces placeholders with service container parameter values in:
      * - the route defaults,
      * - the route requirements,
-     * - the route pattern.
-     * - the route host.
+     * - the route path,
+     * - the route host,
+     * - the route schemes,
+     * - the route methods.
      *
      * @param RouteCollection $collection
      */
@@ -90,11 +92,27 @@ class Router extends BaseRouter implements WarmableInterface
             }
 
             foreach ($route->getRequirements() as $name => $value) {
+                if ('_scheme' === $name || '_method' === $name) {
+                    continue; // ignore deprecated requirements to not trigger deprecation warnings
+                }
+
                 $route->setRequirement($name, $this->resolve($value));
             }
 
             $route->setPath($this->resolve($route->getPath()));
             $route->setHost($this->resolve($route->getHost()));
+
+            $schemes = array();
+            foreach ($route->getSchemes() as $scheme) {
+                $schemes = array_merge($schemes, explode('|', $this->resolve($scheme)));
+            }
+            $route->setSchemes($schemes);
+
+            $methods = array();
+            foreach ($route->getMethods() as $method) {
+                $methods = array_merge($methods, explode('|', $this->resolve($method)));
+            }
+            $route->setMethods($methods);
         }
     }
 

src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RedirectableUrlMatcherTest.php

@@ -38,26 +38,6 @@ class RedirectableUrlMatcherTest extends \PHPUnit_Framework_TestCase
         );
     }
 
-    public function testSchemeRedirectBC()
-    {
-        $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')
-        );
-    }
-
     public function testSchemeRedirect()
     {
         $coll = new RouteCollection();

src/Symfony/Component/Routing/Loader/XmlFileLoader.php

@@ -122,6 +122,8 @@ class XmlFileLoader extends FileLoader
                 throw new \InvalidArgumentException(sprintf('The <route> element in file "%s" cannot define both a "path" and a "pattern" attribute. Use only "path".', $path));
             }
 
+            trigger_error('The "pattern" is deprecated since version 2.2 and will be removed in 3.0. Use the "path" option in the route definition instead.', E_USER_DEPRECATED);
+
             $node->setAttribute('path', $node->getAttribute('pattern'));
             $node->removeAttribute('pattern');
         }

src/Symfony/Component/Routing/Loader/YamlFileLoader.php

@@ -81,6 +81,8 @@ class YamlFileLoader extends FileLoader
                     throw new \InvalidArgumentException(sprintf('The file "%s" cannot define both a "path" and a "pattern" attribute. Use only "path".', $path));
                 }
 
+                trigger_error('The "pattern" is deprecated since version 2.2 and will be removed in 3.0. Use the "path" option in the route definition instead.', E_USER_DEPRECATED);
+
                 $config['path'] = $config['pattern'];
                 unset($config['pattern']);
             }

src/Symfony/Component/Routing/Matcher/Dumper/ApacheMatcherDumper.php

@@ -90,10 +90,7 @@ class ApacheMatcherDumper extends MatcherDumper
 
             $rules[] = $this->dumpRoute($name, $route, $options, $hostRegexUnique);
 
-            if ($req = $route->getRequirement('_method')) {
-                $methods = explode('|', strtoupper($req));
-                $methodVars = array_merge($methodVars, $methods);
-            }
+            $methodVars = array_merge($methodVars, $route->getMethods());
         }
         if (0 < count($methodVars)) {
             $rule = array('# 405 Method Not Allowed');
@@ -200,13 +197,11 @@ class ApacheMatcherDumper extends MatcherDumper
      */
     private function getRouteMethods(Route $route)
     {
-        $methods = array();
-        if ($req = $route->getRequirement('_method')) {
-            $methods = explode('|', strtoupper($req));
-            // GET and HEAD are equivalent
-            if (in_array('GET', $methods) && !in_array('HEAD', $methods)) {
-                $methods[] = 'HEAD';
-            }
+        $methods = $route->getMethods();
+
+        // GET and HEAD are equivalent
+        if (in_array('GET', $methods) && !in_array('HEAD', $methods)) {
+            $methods[] = 'HEAD';
         }
 
         return $methods;

src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php

@@ -215,14 +215,11 @@ EOF;
         $hasTrailingSlash = false;
         $matches = false;
         $hostMatches = false;
-        $methods = array();
+        $methods = $route->getMethods();
 
-        if ($req = $route->getRequirement('_method')) {
-            $methods = explode('|', strtoupper($req));
-            // GET and HEAD are equivalent
-            if (in_array('GET', $methods) && !in_array('HEAD', $methods)) {
-                $methods[] = 'HEAD';
-            }
+        // GET and HEAD are equivalent
+        if (in_array('GET', $methods) && !in_array('HEAD', $methods)) {
+            $methods[] = 'HEAD';
         }
 
         $supportsTrailingSlash = $supportsRedirections && (!$methods || in_array('HEAD', $methods));

src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php

@@ -78,16 +78,16 @@ class TraceableUrlMatcher extends UrlMatcher
             }
 
             // check HTTP method requirement
-            if ($req = $route->getRequirement('_method')) {
+            if ($requiredMethods = $route->getMethods()) {
                 // HEAD and GET are equivalent as per RFC
                 if ('HEAD' === $method = $this->context->getMethod()) {
                     $method = 'GET';
                 }
 
-                if (!in_array($method, $req = explode('|', strtoupper($req)))) {
-                    $this->allow = array_merge($this->allow, $req);
+                if (!in_array($method, $requiredMethods)) {
+                    $this->allow = array_merge($this->allow, $requiredMethods);
 
-                    $this->addTrace(sprintf('Method "%s" does not match the requirement ("%s")', $this->context->getMethod(), implode(', ', $req)), self::ROUTE_ALMOST_MATCHES, $name, $route);
+                    $this->addTrace(sprintf('Method "%s" does not match any of the required methods (%s)', $this->context->getMethod(), implode(', ', $requiredMethods)), self::ROUTE_ALMOST_MATCHES, $name, $route);
 
                     continue;
                 }
@@ -107,7 +107,7 @@ class TraceableUrlMatcher extends UrlMatcher
                 $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);
+                    $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;
                 }

src/Symfony/Component/Routing/Matcher/UrlMatcher.php

@@ -98,7 +98,7 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
         }
 
         throw 0 < count($this->allow)
-            ? new MethodNotAllowedException(array_unique(array_map('strtoupper', $this->allow)))
+            ? new MethodNotAllowedException(array_unique($this->allow))
             : new ResourceNotFoundException(sprintf('No routes found for "%s".', $pathinfo));
     }
 
@@ -152,14 +152,14 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
             }
 
             // check HTTP method requirement
-            if ($req = $route->getRequirement('_method')) {
+            if ($requiredMethods = $route->getMethods()) {
                 // HEAD and GET are equivalent as per RFC
                 if ('HEAD' === $method = $this->context->getMethod()) {
                     $method = 'GET';
                 }
 
-                if (!in_array($method, $req = explode('|', strtoupper($req)))) {
-                    $this->allow = array_merge($this->allow, $req);
+                if (!in_array($method, $requiredMethods)) {
+                    $this->allow = array_merge($this->allow, $requiredMethods);
 
                     continue;
                 }

src/Symfony/Component/Routing/Route.php

@@ -151,7 +151,7 @@ class Route implements \Serializable
      */
     public function getPattern()
     {
-        trigger_error('The '.__METHOD__.' method is deprecated since version 2.2 and will be removed in 3.0. Use the getPath() method instead and use the "path" option instead of the "pattern" option in the route definition.', E_USER_DEPRECATED);
+        trigger_error('The '.__METHOD__.' method is deprecated since version 2.2 and will be removed in 3.0. Use the getPath() method instead.', E_USER_DEPRECATED);
 
         return $this->path;
     }
@@ -169,7 +169,7 @@ class Route implements \Serializable
      */
     public function setPattern($pattern)
     {
-        trigger_error('The '.__METHOD__.' method is deprecated since version 2.2 and will be removed in 3.0. Use the setPath() method instead and use the "path" option instead of the "pattern" option in the route definition.', E_USER_DEPRECATED);
+        trigger_error('The '.__METHOD__.' method is deprecated since version 2.2 and will be removed in 3.0. Use the setPath() method instead.', E_USER_DEPRECATED);
 
         return $this->setPath($pattern);
     }
@@ -548,6 +548,12 @@ class Route implements \Serializable
      */
     public function getRequirement($key)
     {
+        if ('_scheme' === $key) {
+            trigger_error('The "_scheme" requirement is deprecated since version 2.2 and will be removed in 3.0. Use getSchemes() instead.', E_USER_DEPRECATED);
+        } elseif ('_method' === $key) {
+            trigger_error('The "_method" requirement is deprecated since version 2.2 and will be removed in 3.0. Use getMethods() instead.', E_USER_DEPRECATED);
+        }
+
         return isset($this->requirements[$key]) ? $this->requirements[$key] : null;
     }
 
@@ -649,8 +655,12 @@ class Route implements \Serializable
 
         // this is to keep BC and will be removed in a future version
         if ('_scheme' === $key) {
+            trigger_error('The "_scheme" requirement is deprecated since version 2.2 and will be removed in 3.0. Use the setSchemes() method instead or the "schemes" option in the route definition.', E_USER_DEPRECATED);
+
             $this->setSchemes(explode('|', $regex));
         } elseif ('_method' === $key) {
+            trigger_error('The "_method" requirement is deprecated since version 2.2 and will be removed in 3.0. Use the setMethods() method instead or the "methods" option in the route definition.', E_USER_DEPRECATED);
+
             $this->setMethods(explode('|', $regex));
         }
 

src/Symfony/Component/Routing/Tests/Annotation/RouteTest.php

@@ -36,14 +36,14 @@ class RouteTest extends \PHPUnit_Framework_TestCase
     {
         return array(
             array('value', '/Blog', 'getPath'),
-            array('requirements', array('_method' => 'GET'), 'getRequirements'),
+            array('requirements', array('locale' => 'en'), 'getRequirements'),
             array('options', array('compiler_class' => 'RouteCompiler'), 'getOptions'),
             array('name', 'blog_index', 'getName'),
             array('defaults', array('_controller' => 'MyBlogBundle:Blog:index'), 'getDefaults'),
             array('schemes', array('https'), 'getSchemes'),
             array('methods', array('GET', 'POST'), 'getMethods'),
-            array('host', array('{locale}.example.com'), 'getHost'),
-            array('condition', array('context.getMethod() == "GET"'), 'getCondition'),
+            array('host', '{locale}.example.com', 'getHost'),
+            array('condition', 'context.getMethod() == "GET"', 'getCondition'),
         );
     }
 

src/Symfony/Component/Routing/Tests/Fixtures/legacy_validpattern.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+
+<routes xmlns="http://symfony.com/schema/routing"
+    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">
+
+    <route id="blog_show_legacy" pattern="/blog/{slug}" host="{locale}.example.com">
+        <default key="_controller">MyBundle:Blog:show</default>
+        <default key="slug" xsi:nil="true" />
+        <requirement key="_method">GET|POST|put|OpTiOnS</requirement>
+        <requirement key="_scheme">hTTps</requirement>
+        <requirement key="locale">\w+</requirement>
+        <option key="compiler_class">RouteCompiler</option>
+        <condition>context.getMethod() == "GET"</condition>
+    </route>
+</routes>

src/Symfony/Component/Routing/Tests/Fixtures/legacy_validpattern.yml

@@ -0,0 +1,8 @@
+blog_show_legacy:
+    pattern:      /blog/{slug}
+    defaults:     { _controller: "MyBundle:Blog:show" }
+    host:         "{locale}.example.com"
+    requirements: { '_method': 'GET|POST|put|OpTiOnS', _scheme: https, 'locale': '\w+' }
+    condition:    'context.getMethod() == "GET"'
+    options:
+        compiler_class: RouteCompiler

src/Symfony/Component/Routing/Tests/Fixtures/nonvalid.xml

@@ -6,6 +6,5 @@
 
     <route id="blog_show" path="/blog/{slug}">
         <default key="_controller">MyBundle:Blog:show</default>
-        <requirement key="_method">GET</requirement>
     <!-- </route> -->
 </routes>

src/Symfony/Component/Routing/Tests/Fixtures/nonvalidroute.xml

@@ -6,7 +6,6 @@
 
     <route id="blog_show" path="/blog/{slug}">
         <default key="_controller">MyBundle:Blog:show</default>
-        <requirement key="_method">GET</requirement>
         <option key="compiler_class">RouteCompiler</option>
         <foo key="bar">baz</foo>
     </route>

src/Symfony/Component/Routing/Tests/Fixtures/validpattern.php

@@ -13,15 +13,5 @@ $collection->add('blog_show', new Route(
     array('GET', 'POST', 'put', 'OpTiOnS'),
     'context.getMethod() == "GET"'
 ));
-$collection->add('blog_show_legacy', new Route(
-    '/blog/{slug}',
-    array('_controller' => 'MyBlogBundle:Blog:show'),
-    array('_method' => 'GET|POST|put|OpTiOnS', '_scheme' => 'https', 'locale' => '\w+'),
-    array('compiler_class' => 'RouteCompiler'),
-    '{locale}.example.com',
-    array(),
-    array(),
-    'context.getMethod() == "GET"'
-));
 
 return $collection;

src/Symfony/Component/Routing/Tests/Fixtures/validpattern.xml

@@ -11,15 +11,5 @@
         <condition>context.getMethod() == "GET"</condition>
     </route>
 
-    <route id="blog_show_legacy" pattern="/blog/{slug}" host="{locale}.example.com">
-        <default key="_controller">MyBundle:Blog:show</default>
-        <default key="slug" xsi:nil="true" />
-        <requirement key="_method">GET|POST|put|OpTiOnS</requirement>
-        <requirement key="_scheme">hTTps</requirement>
-        <requirement key="locale">\w+</requirement>
-        <option key="compiler_class">RouteCompiler</option>
-        <condition>context.getMethod() == "GET"</condition>
-    </route>
-
     <route id="blog_show_inherited" path="/blog/{slug}" />
 </routes>

src/Symfony/Component/Routing/Tests/Fixtures/validpattern.yml

@@ -9,14 +9,5 @@ blog_show:
     options:
         compiler_class: RouteCompiler
 
-blog_show_legacy:
-    pattern:      /blog/{slug}
-    defaults:     { _controller: "MyBundle:Blog:show" }
-    host:         "{locale}.example.com"
-    requirements: { '_method': 'GET|POST|put|OpTiOnS', _scheme: https, 'locale': '\w+' }
-    condition:    'context.getMethod() == "GET"'
-    options:
-        compiler_class: RouteCompiler
-
 blog_show_inherited:
     path:      /blog/{slug}

src/Symfony/Component/Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php

@@ -118,7 +118,6 @@ class PhpGeneratorDumperTest extends \PHPUnit_Framework_TestCase
     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;
@@ -126,25 +125,17 @@ class PhpGeneratorDumperTest extends \PHPUnit_Framework_TestCase
         $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');
     }
 }

src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php

@@ -244,12 +244,6 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
 
     public function testSchemeRequirementDoesNothingIfSameCurrentScheme()
     {
-        $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'))); // 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'));
 
@@ -259,12 +253,6 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
 
     public function testSchemeRequirementForcesAbsoluteUrl()
     {
-        $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'))); // 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'));
 
@@ -461,24 +449,6 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
         $this->assertNull($generator->generate('test', array('foo' => 'baz'), false));
     }
 
-    public function testGenerateNetworkPathBC()
-    {
-        $routes = $this->getRoutes('test', new Route('/{name}', array(), array('_scheme' => 'http'), array(), '{locale}.example.com'));
-
-        $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 testGenerateNetworkPath()
     {
         $routes = $this->getRoutes('test', new Route('/{name}', array(), array(), array(), '{locale}.example.com', array('http')));
@@ -503,7 +473,6 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
         $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('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'));
 
@@ -524,9 +493,6 @@ 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('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)
         );

src/Symfony/Component/Routing/Tests/Loader/PhpFileLoaderTest.php

@@ -33,7 +33,7 @@ class PhpFileLoaderTest extends \PHPUnit_Framework_TestCase
         $routeCollection = $loader->load('validpattern.php');
         $routes = $routeCollection->all();
 
-        $this->assertCount(2, $routes, 'Two routes are loaded');
+        $this->assertCount(1, $routes, 'One route is loaded');
         $this->assertContainsOnly('Symfony\Component\Routing\Route', $routes);
 
         foreach ($routes as $route) {
@@ -52,7 +52,7 @@ class PhpFileLoaderTest extends \PHPUnit_Framework_TestCase
         $routeCollection = $loader->load('validresource.php');
         $routes = $routeCollection->all();
 
-        $this->assertCount(2, $routes, 'Two routes are loaded');
+        $this->assertCount(1, $routes, 'One route is loaded');
         $this->assertContainsOnly('Symfony\Component\Routing\Route', $routes);
 
         foreach ($routes as $route) {

src/Symfony/Component/Routing/Tests/Loader/XmlFileLoaderTest.php

@@ -32,23 +32,36 @@ class XmlFileLoaderTest extends \PHPUnit_Framework_TestCase
     {
         $loader = new XmlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures')));
         $routeCollection = $loader->load('validpattern.xml');
-        $routes = $routeCollection->all();
+        $route = $routeCollection->get('blog_show');
 
-        $this->assertCount(3, $routes, 'Three routes are loaded');
-        $this->assertContainsOnly('Symfony\Component\Routing\Route', $routes);
+        $this->assertInstanceOf('Symfony\Component\Routing\Route', $route);
+        $this->assertSame('/blog/{slug}', $route->getPath());
+        $this->assertSame('{locale}.example.com', $route->getHost());
+        $this->assertSame('MyBundle:Blog:show', $route->getDefault('_controller'));
+        $this->assertSame('\w+', $route->getRequirement('locale'));
+        $this->assertSame('RouteCompiler', $route->getOption('compiler_class'));
+        $this->assertEquals(array('GET', 'POST', 'PUT', 'OPTIONS'), $route->getMethods());
+        $this->assertEquals(array('https'), $route->getSchemes());
+        $this->assertEquals('context.getMethod() == "GET"', $route->getCondition());
+    }
 
-        $identicalRoutes = array_slice($routes, 0, 2);
+    public function testLegacyRouteDefinitionLoading()
+    {
+        $this->iniSet('error_reporting', -1 & ~E_USER_DEPRECATED);
 
-        foreach ($identicalRoutes as $route) {
-            $this->assertSame('/blog/{slug}', $route->getPath());
-            $this->assertSame('{locale}.example.com', $route->getHost());
-            $this->assertSame('MyBundle:Blog:show', $route->getDefault('_controller'));
-            $this->assertSame('\w+', $route->getRequirement('locale'));
-            $this->assertSame('RouteCompiler', $route->getOption('compiler_class'));
-            $this->assertEquals(array('GET', 'POST', 'PUT', 'OPTIONS'), $route->getMethods());
-            $this->assertEquals(array('https'), $route->getSchemes());
-            $this->assertEquals('context.getMethod() == "GET"', $route->getCondition());
-        }
+        $loader = new XmlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures')));
+        $routeCollection = $loader->load('legacy_validpattern.xml');
+        $route = $routeCollection->get('blog_show_legacy');
+
+        $this->assertInstanceOf('Symfony\Component\Routing\Route', $route);
+        $this->assertSame('/blog/{slug}', $route->getPath());
+        $this->assertSame('{locale}.example.com', $route->getHost());
+        $this->assertSame('MyBundle:Blog:show', $route->getDefault('_controller'));
+        $this->assertSame('\w+', $route->getRequirement('locale'));
+        $this->assertSame('RouteCompiler', $route->getOption('compiler_class'));
+        $this->assertEquals(array('GET', 'POST', 'PUT', 'OPTIONS'), $route->getMethods());
+        $this->assertEquals(array('https'), $route->getSchemes());
+        $this->assertEquals('context.getMethod() == "GET"', $route->getCondition());
     }
 
     public function testLoadWithNamespacePrefix()
@@ -74,7 +87,7 @@ class XmlFileLoaderTest extends \PHPUnit_Framework_TestCase
         $routeCollection = $loader->load('validresource.xml');
         $routes = $routeCollection->all();
 
-        $this->assertCount(3, $routes, 'Three routes are loaded');
+        $this->assertCount(2, $routes, 'Two routes are loaded');
         $this->assertContainsOnly('Symfony\Component\Routing\Route', $routes);
 
         foreach ($routes as $route) {

src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php

@@ -66,23 +66,36 @@ class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase
     {
         $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures')));
         $routeCollection = $loader->load('validpattern.yml');
-        $routes = $routeCollection->all();
+        $route = $routeCollection->get('blog_show');
 
-        $this->assertCount(3, $routes, 'Three routes are loaded');
-        $this->assertContainsOnly('Symfony\Component\Routing\Route', $routes);
+        $this->assertInstanceOf('Symfony\Component\Routing\Route', $route);
+        $this->assertSame('/blog/{slug}', $route->getPath());
+        $this->assertSame('{locale}.example.com', $route->getHost());
+        $this->assertSame('MyBundle:Blog:show', $route->getDefault('_controller'));
+        $this->assertSame('\w+', $route->getRequirement('locale'));
+        $this->assertSame('RouteCompiler', $route->getOption('compiler_class'));
+        $this->assertEquals(array('GET', 'POST', 'PUT', 'OPTIONS'), $route->getMethods());
+        $this->assertEquals(array('https'), $route->getSchemes());
+        $this->assertEquals('context.getMethod() == "GET"', $route->getCondition());
+    }
 
-        $identicalRoutes = array_slice($routes, 0, 2);
+    public function testLegacyRouteDefinitionLoading()
+    {
+        $this->iniSet('error_reporting', -1 & ~E_USER_DEPRECATED);
 
-        foreach ($identicalRoutes as $route) {
-            $this->assertSame('/blog/{slug}', $route->getPath());
-            $this->assertSame('{locale}.example.com', $route->getHost());
-            $this->assertSame('MyBundle:Blog:show', $route->getDefault('_controller'));
-            $this->assertSame('\w+', $route->getRequirement('locale'));
-            $this->assertSame('RouteCompiler', $route->getOption('compiler_class'));
-            $this->assertEquals(array('GET', 'POST', 'PUT', 'OPTIONS'), $route->getMethods());
-            $this->assertEquals(array('https'), $route->getSchemes());
-            $this->assertEquals('context.getMethod() == "GET"', $route->getCondition());
-        }
+        $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures')));
+        $routeCollection = $loader->load('legacy_validpattern.yml');
+        $route = $routeCollection->get('blog_show_legacy');
+
+        $this->assertInstanceOf('Symfony\Component\Routing\Route', $route);
+        $this->assertSame('/blog/{slug}', $route->getPath());
+        $this->assertSame('{locale}.example.com', $route->getHost());
+        $this->assertSame('MyBundle:Blog:show', $route->getDefault('_controller'));
+        $this->assertSame('\w+', $route->getRequirement('locale'));
+        $this->assertSame('RouteCompiler', $route->getOption('compiler_class'));
+        $this->assertEquals(array('GET', 'POST', 'PUT', 'OPTIONS'), $route->getMethods());
+        $this->assertEquals(array('https'), $route->getSchemes());
+        $this->assertEquals('context.getMethod() == "GET"', $route->getCondition());
     }
 
     public function testLoadWithResource()
@@ -91,7 +104,7 @@ class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase
         $routeCollection = $loader->load('validresource.yml');
         $routes = $routeCollection->all();
 
-        $this->assertCount(3, $routes, 'Three routes are loaded');
+        $this->assertCount(2, $routes, 'Two routes are loaded');
         $this->assertContainsOnly('Symfony\Component\Routing\Route', $routes);
 
         foreach ($routes as $route) {

src/Symfony/Component/Routing/Tests/Matcher/Dumper/LegacyApacheMatcherDumperTest.php

@@ -84,13 +84,21 @@ class LegacyApacheMatcherDumperTest extends \PHPUnit_Framework_TestCase
         $collection->add('bar', new Route(
             '/bar/{foo}',
             array(),
-            array('_method' => 'GET|head')
+            array(),
+            array(),
+            '',
+            array(),
+            array('GET', 'head')
         ));
         // method requirement (again)
         $collection->add('baragain', new Route(
             '/baragain/{foo}',
             array(),
-            array('_method' => 'get|post')
+            array(),
+            array(),
+            '',
+            array(),
+            array('get', 'post')
         ));
         // simple
         $collection->add('baz', new Route(
@@ -112,13 +120,21 @@ class LegacyApacheMatcherDumperTest extends \PHPUnit_Framework_TestCase
         $collection->add('baz5', new Route(
             '/test/{foo}/',
             array(),
-            array('_method' => 'get')
+            array(),
+            array(),
+            '',
+            array(),
+            array('GET')
         ));
         // trailing slash and unsafe method
         $collection->add('baz5unsafe', new Route(
             '/testunsafe/{foo}/',
             array(),
-            array('_method' => 'post')
+            array(),
+            array(),
+            '',
+            array(),
+            array('post')
         ));
         // complex
         $collection->add('baz6', new Route(

src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php

@@ -26,7 +26,10 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
         $collection->add('secure', new Route(
             '/secure',
             array(),
-            array('_scheme' => 'https')
+            array(),
+            array(),
+            '',
+            array('https')
         ));
         $dumper = new PhpMatcherDumper($collection);
         $dumper->dump();
@@ -61,13 +64,21 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
         $collection->add('bar', new Route(
             '/bar/{foo}',
             array(),
-            array('_method' => 'GET|head')
+            array(),
+            array(),
+            '',
+            array(),
+            array('GET', 'head')
         ));
         // GET method requirement automatically adds HEAD as valid
         $collection->add('barhead', new Route(
             '/barhead/{foo}',
             array(),
-            array('_method' => 'GET')
+            array(),
+            array(),
+            '',
+            array(),
+            array('GET')
         ));
         // simple
         $collection->add('baz', new Route(
@@ -89,13 +100,21 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
         $collection->add('baz5', new Route(
             '/test/{foo}/',
             array(),
-            array('_method' => 'post')
+            array(),
+            array(),
+            '',
+            array(),
+            array('post')
         ));
         // complex name
         $collection->add('baz.baz6', new Route(
             '/test/{foo}/',
             array(),
-            array('_method' => 'put')
+            array(),
+            array(),
+            '',
+            array(),
+            array('put')
         ));
         // defaults without variable
         $collection->add('foofoo', new Route(
@@ -235,14 +254,20 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
         $redirectCollection->add('secure', new Route(
             '/secure',
             array(),
-            array('_scheme' => 'https')
+            array(),
+            array(),
+            '',
+            array('https')
         ));
 
         // force HTTP redirection
         $redirectCollection->add('nonsecure', new Route(
             '/nonsecure',
             array(),
-            array('_scheme' => 'http')
+            array(),
+            array(),
+            '',
+            array('http')
         ));
 
         /* test case 3 */

src/Symfony/Component/Routing/Tests/Matcher/RedirectableUrlMatcherTest.php

@@ -41,21 +41,6 @@ class RedirectableUrlMatcherTest extends \PHPUnit_Framework_TestCase
         $matcher->match('/foo');
     }
 
-    public function testSchemeRedirectBC()
-    {
-        $coll = new RouteCollection();
-        $coll->add('foo', new Route('/foo', array(), array('_scheme' => 'https')));
-
-        $matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
-        $matcher
-            ->expects($this->once())
-            ->method('redirect')
-            ->with('/foo', 'foo', 'https')
-            ->will($this->returnValue(array('_route' => 'foo')))
-        ;
-        $matcher->match('/foo');
-    }
-
     public function testSchemeRedirectRedirectsToFirstScheme()
     {
         $coll = new RouteCollection();

src/Symfony/Component/Routing/Tests/Matcher/TraceableUrlMatcherTest.php

@@ -21,9 +21,9 @@ class TraceableUrlMatcherTest extends \PHPUnit_Framework_TestCase
     public function test()
     {
         $coll = new RouteCollection();
-        $coll->add('foo', new Route('/foo', array(), array('_method' => 'POST')));
+        $coll->add('foo', new Route('/foo', array(), array(), array(), '', array(), array('POST')));
         $coll->add('bar', new Route('/bar/{id}', array(), array('id' => '\d+')));
-        $coll->add('bar1', new Route('/bar/{name}', array(), array('id' => '\w+', '_method' => 'POST')));
+        $coll->add('bar1', new Route('/bar/{name}', array(), array('id' => '\w+'), array(), '', array(), array('POST')));
         $coll->add('bar2', new Route('/foo', array(), array(), array(), 'baz'));
         $coll->add('bar3', new Route('/foo1', array(), array(), array(), 'baz'));
         $coll->add('bar4', new Route('/foo2', array(), array(), array(), 'baz', array(), array(), 'context.getMethod() == "GET"'));
@@ -33,29 +33,29 @@ class TraceableUrlMatcherTest extends \PHPUnit_Framework_TestCase
 
         $matcher = new TraceableUrlMatcher($coll, $context);
         $traces = $matcher->getTraces('/babar');
-        $this->assertEquals(array(0, 0, 0, 0, 0, 0), $this->getLevels($traces));
+        $this->assertSame(array(0, 0, 0, 0, 0, 0), $this->getLevels($traces));
 
         $traces = $matcher->getTraces('/foo');
-        $this->assertEquals(array(1, 0, 0, 2), $this->getLevels($traces));
+        $this->assertSame(array(1, 0, 0, 2), $this->getLevels($traces));
 
         $traces = $matcher->getTraces('/bar/12');
-        $this->assertEquals(array(0, 2), $this->getLevels($traces));
+        $this->assertSame(array(0, 2), $this->getLevels($traces));
 
         $traces = $matcher->getTraces('/bar/dd');
-        $this->assertEquals(array(0, 1, 1, 0, 0, 0), $this->getLevels($traces));
+        $this->assertSame(array(0, 1, 1, 0, 0, 0), $this->getLevels($traces));
 
         $traces = $matcher->getTraces('/foo1');
-        $this->assertEquals(array(0, 0, 0, 0, 2), $this->getLevels($traces));
+        $this->assertSame(array(0, 0, 0, 0, 2), $this->getLevels($traces));
 
         $context->setMethod('POST');
         $traces = $matcher->getTraces('/foo');
-        $this->assertEquals(array(2), $this->getLevels($traces));
+        $this->assertSame(array(2), $this->getLevels($traces));
 
         $traces = $matcher->getTraces('/bar/dd');
-        $this->assertEquals(array(0, 1, 2), $this->getLevels($traces));
+        $this->assertSame(array(0, 1, 2), $this->getLevels($traces));
 
         $traces = $matcher->getTraces('/foo2');
-        $this->assertEquals(array(0, 0, 0, 0, 0, 1), $this->getLevels($traces));
+        $this->assertSame(array(0, 0, 0, 0, 0, 1), $this->getLevels($traces));
     }
 
     public function testMatchRouteOnMultipleHosts()
@@ -83,7 +83,7 @@ class TraceableUrlMatcherTest extends \PHPUnit_Framework_TestCase
         $matcher = new TraceableUrlMatcher($routes, $context);
 
         $traces = $matcher->getTraces('/mypath/');
-        $this->assertEquals(
+        $this->assertSame(
             array(TraceableUrlMatcher::ROUTE_ALMOST_MATCHES, TraceableUrlMatcher::ROUTE_ALMOST_MATCHES),
             $this->getLevels($traces)
         );

src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php

@@ -26,13 +26,13 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
         $coll->add('foo', new Route('/foo'));
 
         $matcher = new UrlMatcher($coll, new RequestContext());
-        $matcher->match('/foo');
+        $this->assertInternalType('array', $matcher->match('/foo'));
     }
 
     public function testMethodNotAllowed()
     {
         $coll = new RouteCollection();
-        $coll->add('foo', new Route('/foo', array(), array('_method' => 'post')));
+        $coll->add('foo', new Route('/foo', array(), array(), array(), '', array(), array('post')));
 
         $matcher = new UrlMatcher($coll, new RequestContext());
 
@@ -47,17 +47,17 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
     public function testHeadAllowedWhenRequirementContainsGet()
     {
         $coll = new RouteCollection();
-        $coll->add('foo', new Route('/foo', array(), array('_method' => 'get')));
+        $coll->add('foo', new Route('/foo', array(), array(), array(), '', array(), array('get')));
 
         $matcher = new UrlMatcher($coll, new RequestContext('', 'head'));
-        $matcher->match('/foo');
+        $this->assertInternalType('array', $matcher->match('/foo'));
     }
 
     public function testMethodNotAllowedAggregatesAllowedMethods()
     {
         $coll = new RouteCollection();
-        $coll->add('foo1', new Route('/foo', array(), array('_method' => 'post')));
-        $coll->add('foo2', new Route('/foo', array(), array('_method' => 'put|delete')));
+        $coll->add('foo1', new Route('/foo', array(), array(), array(), '', array(), array('post')));
+        $coll->add('foo2', new Route('/foo', array(), array(), array(), '', array(), array('put', 'delete')));
 
         $matcher = new UrlMatcher($coll, new RequestContext());
 
@@ -90,7 +90,7 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
 
         // test that route "method" is ignored if no method is given in the context
         $collection = new RouteCollection();
-        $collection->add('foo', new Route('/foo', array(), array('_method' => 'GET|head')));
+        $collection->add('foo', new Route('/foo', array(), array(), array(), '', array(), array('get', 'head')));
         $matcher = new UrlMatcher($collection, new RequestContext());
         $this->assertInternalType('array', $matcher->match('/foo'));
 
@@ -312,16 +312,6 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
         $matcher->match('/do.t.html');
     }
 
-    /**
-     * @expectedException \Symfony\Component\Routing\Exception\ResourceNotFoundException
-     */
-    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
      */

src/Symfony/Component/Routing/Tests/RouteCollectionTest.php

@@ -164,12 +164,12 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase
     public function testAddPrefixOverridesDefaultsAndRequirements()
     {
         $collection = new RouteCollection();
-        $collection->add('foo', $foo = new Route('/foo'));
-        $collection->add('bar', $bar = new Route('/bar', array(), array('_scheme' => 'http')));
-        $collection->addPrefix('/admin', array(), array('_scheme' => 'https'));
+        $collection->add('foo', $foo = new Route('/foo.{_format}'));
+        $collection->add('bar', $bar = new Route('/bar.{_format}', array(), array('_format' => 'json')));
+        $collection->addPrefix('/admin', array(), array('_format' => 'html'));
 
-        $this->assertEquals('https', $collection->get('foo')->getRequirement('_scheme'), '->addPrefix() overrides existing requirements');
-        $this->assertEquals('https', $collection->get('bar')->getRequirement('_scheme'), '->addPrefix() overrides existing requirements');
+        $this->assertEquals('html', $collection->get('foo')->getRequirement('_format'), '->addPrefix() overrides existing requirements');
+        $this->assertEquals('html', $collection->get('bar')->getRequirement('_format'), '->addPrefix() overrides existing requirements');
     }
 
     public function testResource()

src/Symfony/Component/Routing/Tests/RouteTest.php

@@ -164,8 +164,10 @@ class RouteTest extends \PHPUnit_Framework_TestCase
         $this->assertTrue($route->hasScheme('httpS'));
     }
 
-    public function testSchemeIsBC()
+    public function testLegacySchemeRequirement()
     {
+        $this->iniSet('error_reporting', -1 & ~E_USER_DEPRECATED);
+
         $route = new Route('/');
         $route->setRequirement('_scheme', 'http|https');
         $this->assertEquals('http|https', $route->getRequirement('_scheme'));
@@ -189,8 +191,10 @@ class RouteTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(array('GET', 'POST'), $route->getMethods(), '->setMethods() accepts an array of methods and uppercases them');
     }
 
-    public function testMethodIsBC()
+    public function testLegacyMethodRequirement()
     {
+        $this->iniSet('error_reporting', -1 & ~E_USER_DEPRECATED);
+
         $route = new Route('/');
         $route->setRequirement('_method', 'GET|POST');
         $this->assertEquals('GET|POST', $route->getRequirement('_method'));