merged branch Seldaek/route-gen (PR #4534)

Commits ------- 31843cf [FrameworkBundle] Add info to config d5ab4c1 [Routing] Update changelog bbef65e [Routing] Add strict_parameters option to disable exceptions when a route generation fails due to an invalid parameter Discussion ---------- [Routing] Add strict_parameters option to disable exceptions on invalid parameters --------------------------------------------------------------------------- by travisbot at 2012-06-09T15:07:26Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1577025) (merged bbef65e6 into 37678e17). --------------------------------------------------------------------------- by stof at 2012-06-09T15:43:48Z Seems good, but you forgot to update the Changelog of the component. Anyway, let's wait for @vicb's review as he knows the Routing component better than me. --------------------------------------------------------------------------- by Seldaek at 2012-06-09T16:35:56Z I updated the changelog --------------------------------------------------------------------------- by travisbot at 2012-06-09T16:38:31Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1577716) (merged d5ab4c1d into 37678e17). --------------------------------------------------------------------------- by travisbot at 2012-06-11T10:10:37Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1590901) (merged a54e59e4 into 37678e17). --------------------------------------------------------------------------- by travisbot at 2012-06-11T13:50:21Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1591926) (merged 31843cf0 into 0995b1f2).
2012-06-12 19:58:14 +02:00 · 2012-06-12 19:58:14 +02:00 · 28f6c5889b
commit 28f6c5889b
parent ef41e308cc 31843cf0bf
9 changed files with 103 additions and 24 deletions
--- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
+++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
@ -155,6 +155,13 @@ class Configuration implements ConfigurationInterface
                        ->scalarNode('type')->end()
                        ->scalarNode('http_port')->defaultValue(80)->end()
                        ->scalarNode('https_port')->defaultValue(443)->end()
+                        ->scalarNode('strict_parameters')
+                            ->info(
+                                'set to false to disable exceptions when a route is '.
+                                'generated with invalid parameters (and return null instead)'
+                            )
+                            ->defaultTrue()
+                        ->end()
                    ->end()
                ->end()
            ->end()
--- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
+++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@ -255,11 +255,12 @@ class FrameworkExtension extends Extension
        $container->setParameter('router.resource', $config['resource']);
        $router = $container->findDefinition('router.default');

+        $argument = $router->getArgument(2);
+        $argument['strict_parameters'] = $config['strict_parameters'];
        if (isset($config['type'])) {
-            $argument = $router->getArgument(2);
            $argument['resource_type'] = $config['type'];
-            $router->replaceArgument(2, $argument);
        }
+        $router->replaceArgument(2, $argument);

        $container->setParameter('request_listener.http_port', $config['http_port']);
        $container->setParameter('request_listener.https_port', $config['https_port']);
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml
@ -50,6 +50,7 @@
        </service>

        <service id="router.default" class="%router.class%" public="false">
+            <tag name="monolog.logger" channel="router" />
            <argument type="service" id="service_container" />
            <argument>%router.resource%</argument>
            <argument type="collection">
@ -65,6 +66,7 @@
                <argument key="matcher_cache_class">%router.options.matcher.cache_class%</argument>
            </argument>
            <argument type="service" id="router.request_context" on-invalid="ignore" />
+            <argument type="service" id="logger" on-invalid="ignore" />
        </service>

        <service id="router" alias="router.default" />
--- a/src/Symfony/Component/Routing/CHANGELOG.md
+++ b/src/Symfony/Component/Routing/CHANGELOG.md
@ -22,3 +22,5 @@ CHANGELOG
   been used anyway without creating inconsistencies
 * [BC BREAK] RouteCollection::remove also removes a route from parent
   collections (not only from its children)
+ * added strict_parameters option to disable exceptions (and generate empty
+   URLs instead) when generating a route with an invalid parameter value
--- a/src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php
+++ b/src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php
@ -49,6 +49,7 @@ class PhpGeneratorDumper extends GeneratorDumper

 use Symfony\Component\Routing\RequestContext;
 use Symfony\Component\Routing\Exception\RouteNotFoundException;
+use Symfony\Component\HttpKernel\Log\LoggerInterface;

 /**
 * {$options['class']}
@ -63,9 +64,10 @@ class {$options['class']} extends {$options['base_class']}
    /**
     * Constructor.
     */
-    public function __construct(RequestContext \$context)
+    public function __construct(RequestContext \$context, LoggerInterface \$logger = null)
    {
        \$this->context = \$context;
+        \$this->logger = \$logger;
    }

 {$this->generateGenerateMethod()}
--- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php
+++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php
@ -17,6 +17,7 @@ use Symfony\Component\Routing\RequestContext;
 use Symfony\Component\Routing\Exception\InvalidParameterException;
 use Symfony\Component\Routing\Exception\RouteNotFoundException;
 use Symfony\Component\Routing\Exception\MissingMandatoryParametersException;
+use Symfony\Component\HttpKernel\Log\LoggerInterface;

 /**
 * UrlGenerator generates a URL based on a set of routes.
@ -28,6 +29,8 @@ use Symfony\Component\Routing\Exception\MissingMandatoryParametersException;
 class UrlGenerator implements UrlGeneratorInterface
 {
    protected $context;
+    protected $strictParameters = true;
+    protected $logger;
    protected $decodedChars = array(
        // %2F is not valid in a URL, so we don't encode it (which is fine as the requirements explicitly allowed it)
        '%2F' => '/',
@ -40,13 +43,15 @@ class UrlGenerator implements UrlGeneratorInterface
     *
     * @param RouteCollection $routes  A RouteCollection instance
     * @param RequestContext  $context The context
+     * @param LoggerInterface $logger  A logger instance
     *
     * @api
     */
-    public function __construct(RouteCollection $routes, RequestContext $context)
+    public function __construct(RouteCollection $routes, RequestContext $context, LoggerInterface $logger = null)
    {
        $this->routes = $routes;
        $this->context = $context;
+        $this->logger = $logger;
    }

    /**
@ -66,7 +71,27 @@ class UrlGenerator implements UrlGeneratorInterface
    }

    /**
-     * {@inheritdoc}
+     * Enables or disables the exception on incorrect parameters.
+     *
+     * @param Boolean $enabled
+     */
+    public function setStrictParameters($enabled)
+    {
+        $this->strictParameters = $enabled;
+    }
+
+    /**
+     * Gets the strict check of incorrect parameters.
+     *
+     * @return Boolean
+     */
+    public function getStrictParameters()
+    {
+        return $this->strictParameters;
+    }
+
+    /**
+     * {@inheritDoc}
     */
    public function generate($name, $parameters = array(), $absolute = false)
    {
@ -105,7 +130,16 @@ class UrlGenerator implements UrlGeneratorInterface
                    if (!$isEmpty = in_array($tparams[$token[3]], array(null, '', false), true)) {
                        // check requirement
                        if ($tparams[$token[3]] && !preg_match('#^'.$token[2].'$#', $tparams[$token[3]])) {
-                            throw new InvalidParameterException(sprintf('Parameter "%s" for route "%s" must match "%s" ("%s" given).', $token[3], $name, $token[2], $tparams[$token[3]]));
+                            $message = sprintf('Parameter "%s" for route "%s" must match "%s" ("%s" given).', $token[3], $name, $token[2], $tparams[$token[3]]);
+                            if ($this->strictParameters) {
+                                throw new InvalidParameterException($message);
+                            }
+
+                            if ($this->logger) {
+                                $this->logger->err($message);
+                            }
+
+                            return null;
                        }
                    }

--- a/src/Symfony/Component/Routing/Router.php
+++ b/src/Symfony/Component/Routing/Router.php
@ -13,6 +13,7 @@ namespace Symfony\Component\Routing;

 use Symfony\Component\Config\Loader\LoaderInterface;
 use Symfony\Component\Config\ConfigCache;
+use Symfony\Component\HttpKernel\Log\LoggerInterface;

 /**
 * The Router class is an example of the integration of all pieces of the
@ -29,6 +30,7 @@ class Router implements RouterInterface
    protected $collection;
    protected $resource;
    protected $options;
+    protected $logger;

    /**
     * Constructor.
@ -37,11 +39,13 @@ class Router implements RouterInterface
     * @param mixed           $resource The main resource to load
     * @param array           $options  An array of options
     * @param RequestContext  $context  The context
+     * @param LoggerInterface $logger   A logger instance
     */
-    public function __construct(LoaderInterface $loader, $resource, array $options = array(), RequestContext $context = null)
+    public function __construct(LoaderInterface $loader, $resource, array $options = array(), RequestContext $context = null, LoggerInterface $logger = null)
    {
        $this->loader = $loader;
        $this->resource = $resource;
+        $this->logger = $logger;
        $this->context = null === $context ? new RequestContext() : $context;
        $this->setOptions($options);
    }
@ -73,6 +77,7 @@ class Router implements RouterInterface
            'matcher_dumper_class'   => 'Symfony\\Component\\Routing\\Matcher\\Dumper\\PhpMatcherDumper',
            'matcher_cache_class'    => 'ProjectUrlMatcher',
            'resource_type'          => null,
+            'strict_parameters'      => true,
        );

        // check option names and live merge, if errors are encountered Exception will be thrown
@ -219,24 +224,30 @@ class Router implements RouterInterface
        }

        if (null === $this->options['cache_dir'] || null === $this->options['generator_cache_class']) {
-            return $this->generator = new $this->options['generator_class']($this->getRouteCollection(), $this->context);
+            $this->generator = new $this->options['generator_class']($this->getRouteCollection(), $this->context, $this->logger);
+        } else {
+            $class = $this->options['generator_cache_class'];
+            $cache = new ConfigCache($this->options['cache_dir'].'/'.$class.'.php', $this->options['debug']);
+            if (!$cache->isFresh($class)) {
+                $dumper = new $this->options['generator_dumper_class']($this->getRouteCollection());
+
+                $options = array(
+                    'class'      => $class,
+                    'base_class' => $this->options['generator_base_class'],
+                );
+
+                $cache->write($dumper->dump($options), $this->getRouteCollection()->getResources());
+            }
+
+            require_once $cache;
+
+            $this->generator = new $class($this->context, $this->logger);
        }

-        $class = $this->options['generator_cache_class'];
-        $cache = new ConfigCache($this->options['cache_dir'].'/'.$class.'.php', $this->options['debug']);
-        if (!$cache->isFresh($class)) {
-            $dumper = new $this->options['generator_dumper_class']($this->getRouteCollection());
-
-            $options = array(
-                'class'      => $class,
-                'base_class' => $this->options['generator_base_class'],
-            );
-
-            $cache->write($dumper->dump($options), $this->getRouteCollection()->getResources());
+        if (false === $this->options['strict_parameters']) {
+            $this->generator->setStrictParameters(false);
        }

-        require_once $cache;
-
-        return $this->generator = new $class($this->context);
+        return $this->generator;
    }
 }
--- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
+++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
@ -165,6 +165,25 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
        $this->getGenerator($routes)->generate('test', array('foo' => 'bar'), true);
    }

+    public function testGenerateForRouteWithInvalidOptionalParameterNonStrict()
+    {
+        $routes = $this->getRoutes('test', new Route('/testing/{foo}', array('foo' => '1'), array('foo' => 'd+')));
+        $generator = $this->getGenerator($routes);
+        $generator->setStrictParameters(false);
+        $this->assertNull($generator->generate('test', array('foo' => 'bar'), true));
+    }
+
+    public function testGenerateForRouteWithInvalidOptionalParameterNonStrictWithLogger()
+    {
+        $routes = $this->getRoutes('test', new Route('/testing/{foo}', array('foo' => '1'), array('foo' => 'd+')));
+        $logger = $this->getMock('Symfony\Component\HttpKernel\Log\LoggerInterface');
+        $logger->expects($this->once())
+            ->method('err');
+        $generator = $this->getGenerator($routes, array(), $logger);
+        $generator->setStrictParameters(false);
+        $this->assertNull($generator->generate('test', array('foo' => 'bar'), true));
+    }
+
    /**
     * @expectedException Symfony\Component\Routing\Exception\InvalidParameterException
     */
@ -206,14 +225,14 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
        $this->assertEquals('/app.php/foo', $this->getGenerator($routes)->generate('test', array('default' => 'foo')));
    }

-    protected function getGenerator(RouteCollection $routes, array $parameters = array())
+    protected function getGenerator(RouteCollection $routes, array $parameters = array(), $logger = null)
    {
        $context = new RequestContext('/app.php');
        foreach ($parameters as $key => $value) {
            $method = 'set'.$key;
            $context->$method($value);
        }
-        $generator = new UrlGenerator($routes, $context);
+        $generator = new UrlGenerator($routes, $context, $logger);

        return $generator;
    }
--- a/src/Symfony/Component/Routing/composer.json
+++ b/src/Symfony/Component/Routing/composer.json
@ -21,6 +21,7 @@
    "require-dev": {
        "symfony/config": "2.1.*",
        "symfony/yaml": "2.1.*",
+        "symfony/http-kernel": "2.1.*",
        "doctrine/common": ">=2.2,<2.4-dev"
    },
    "suggest": {