merged branch vicb/lenient_generator (PR #5114)
Commits
-------
03bbaaf [Routing] Add an interface for configuring strict_parameters
Discussion
----------
[RFC][Routing] Add an interface for configuring strict_parameters
This is a proposal to fix #4697 (related to #4592).
The main point left to discuss was the name of the interface, which is now `LenientInterface`. We could change the name to anything else is someone has a better idea.
@stof @Tobion what do you think ?
---------------------------------------------------------------------------
by stof at 2012-07-30T16:34:20Z
@vicb I already said I had no idea to name it, and it has not changed. :)
So let's wait for other people to see if they have a better idea
---------------------------------------------------------------------------
by breerly at 2012-07-30T16:38:38Z
Maybe `PermissibleInterface` or `PermissiveInterface`.
---------------------------------------------------------------------------
by Partugal at 2012-07-30T17:00:09Z
`StrictUrlGeneratorInterface`, `StrictParametersInterface` or `StrictInterface`
---------------------------------------------------------------------------
by pborreli at 2012-07-30T17:04:46Z
👍 for `PermissiveInterface`
---------------------------------------------------------------------------
by stof at 2012-07-30T17:07:59Z
yes, because the Router currently can only use this interface to set it to ``not-strict``. It assumes that the url generator is already strict by default (which is probably a bad assumption btw as the base class for the generated generator can be changed)
---------------------------------------------------------------------------
by pborreli at 2012-07-30T17:09:33Z
@stof thx, got it
---------------------------------------------------------------------------
by Partugal at 2012-07-30T17:10:03Z
this interface realize setting Strict by setStrictParameters, and get by getStrictParameters, and imho named it by `Strictable` is more logic
---------------------------------------------------------------------------
by pborreli at 2012-07-30T17:11:07Z
@Partugal let's try to find an english term :)
---------------------------------------------------------------------------
by Partugal at 2012-07-30T17:11:31Z
)
---------------------------------------------------------------------------
by breerly at 2012-07-30T17:15:23Z
@Partugal I like using "able" in interface names because it describes a behavior instead of a noun. This type of naming makes following the Interface Segregation Principle easy to follow. Good work.
---------------------------------------------------------------------------
by vicb at 2012-07-30T18:24:26Z
As explained by @stof I did not consider `StrictInterface` because as of now the interface is used to disabled the strict bevahior (which is enabled by default).
I am not satisfied with `PermissiveInterface` / `LenientInterface` because implementing this interface does not mean that the generator will be permissive but only that the behavior is configurable - yes I did consider `Configurable` but the term is a too vague.
---------------------------------------------------------------------------
by breerly at 2012-07-30T18:35:45Z
I see. Perhaps ```StrictConfigurableInterface``` would do the trick.
---------------------------------------------------------------------------
by Tobion at 2012-07-30T21:02:21Z
I think renaming strict_parameters to `strict_requirements` is the way to go because it determines how requirements are handled when generating a URL. Also we should allow another option:
strict_requirements = true: throw exception for mismatching requirements
strict_requirements = null: return null as URL for mismatching requirements and log it.
strict_requirements = false: return the URL with the given parameters without checking the requirements and don't log it.
(Maybe use constants for these).
The Interface I would then call `ConfigurableRequirementsInterface` or `RequirementsHandlingInterface`.
---------------------------------------------------------------------------
by vicb at 2012-07-31T07:23:24Z
Thanks all for the feeback, this is what is now implemented:
- A `ConfigurableRequirementsInterface` that should be implemented by generators that can be configure not to throw an exception when the parameters do not match the requirements,
- The interface has two methods: `setStrictRequirements()` and `isStrictRequirements()`,
- `setStrictRequirements()` always gets called to configure the generator (whatever the option value is)
Note: The Router option name has not changed (i.e. `strict_parameters`)
Does that fit everyone ?
---------------------------------------------------------------------------
by vicb at 2012-07-31T07:39:22Z
So the option name is now consistent (`strict_requirements`) with the interface. We should sync the change [in the standard edition](https://github.com/symfony/symfony-standard/blob/master/app/config/config.yml#L11) if we agree to merge this.
---------------------------------------------------------------------------
by fabpot at 2012-07-31T07:51:47Z
@vicb you forgot to rename the property in `UrlGenerator` as @stof mentioned above.
---------------------------------------------------------------------------
by vicb at 2012-07-31T07:59:57Z
@fabpot fixed. If the code is ok, I'll squash the commits and open a PR on symfony-standard
This commit is contained in:
Fabien Potencier
commit
4b521985f1
@ -169,7 +169,7 @@ class Configuration implements ConfigurationInterface
|
||||
->scalarNode('type')->end()
|
||||
->scalarNode('http_port')->defaultValue(80)->end()
|
||||
->scalarNode('https_port')->defaultValue(443)->end()
|
||||
->scalarNode('strict_parameters')
|
||||
->scalarNode('strict_requirements')
|
||||
->info(
|
||||
'set to false to disable exceptions when a route is '.
|
||||
'generated with invalid parameters (and return null instead)'
|
||||
|
||||
@ -246,7 +246,7 @@ class FrameworkExtension extends Extension
|
||||
$router = $container->findDefinition('router.default');
|
||||
|
||||
$argument = $router->getArgument(2);
|
||||
$argument['strict_parameters'] = $config['strict_parameters'];
|
||||
$argument['strict_requirements'] = $config['strict_requirements'];
|
||||
if (isset($config['type'])) {
|
||||
$argument['resource_type'] = $config['type'];
|
||||
}
|
||||
|
||||
@ -22,5 +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
|
||||
* added strict_requirements option to disable exceptions (and generate empty
|
||||
URLs instead) when generating a route with an invalid parameter value
|
||||
|
||||
@ -0,0 +1,36 @@
|
||||
<?php
|
||||
|
||||
/*
|
||||
* This file is part of the Symfony package.
|
||||
*
|
||||
* (c) Fabien Potencier <fabien@symfony.com>
|
||||
*
|
||||
* For the full copyright and license information, please view the LICENSE
|
||||
* file that was distributed with this source code.
|
||||
*/
|
||||
|
||||
namespace Symfony\Component\Routing\Generator;
|
||||
|
||||
/**
|
||||
* ConfigurableRequirementsInterface must be implemented by URL generators in order
|
||||
* to be able to configure whether an exception should be generated when the
|
||||
* parameters do not match the requirements.
|
||||
*
|
||||
* @author Fabien Potencier <fabien@symfony.com>
|
||||
*/
|
||||
interface ConfigurableRequirementsInterface
|
||||
{
|
||||
/**
|
||||
* Enables or disables the exception on incorrect parameters.
|
||||
*
|
||||
* @param Boolean $enabled
|
||||
*/
|
||||
public function setStrictRequirements($enabled);
|
||||
|
||||
/**
|
||||
* Gets the strict check of incorrect parameters.
|
||||
*
|
||||
* @return Boolean
|
||||
*/
|
||||
public function isStrictRequirements();
|
||||
}
|
||||
@ -26,10 +26,10 @@ use Symfony\Component\HttpKernel\Log\LoggerInterface;
|
||||
*
|
||||
* @api
|
||||
*/
|
||||
class UrlGenerator implements UrlGeneratorInterface
|
||||
class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInterface
|
||||
{
|
||||
protected $context;
|
||||
protected $strictParameters = true;
|
||||
protected $strictRequirements = true;
|
||||
protected $logger;
|
||||
|
||||
/**
|
||||
@ -95,23 +95,19 @@ class UrlGenerator implements UrlGeneratorInterface
|
||||
}
|
||||
|
||||
/**
|
||||
* Enables or disables the exception on incorrect parameters.
|
||||
*
|
||||
* @param Boolean $enabled
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function setStrictParameters($enabled)
|
||||
public function setStrictRequirements($enabled)
|
||||
{
|
||||
$this->strictParameters = $enabled;
|
||||
$this->strictRequirements = (Boolean) $enabled;
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the strict check of incorrect parameters.
|
||||
*
|
||||
* @return Boolean
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function getStrictParameters()
|
||||
public function isStrictRequirements()
|
||||
{
|
||||
return $this->strictParameters;
|
||||
return $this->strictRequirements;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -155,7 +151,7 @@ class UrlGenerator implements UrlGeneratorInterface
|
||||
// check requirement
|
||||
if ($tparams[$token[3]] && !preg_match('#^'.$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) {
|
||||
if ($this->strictRequirements) {
|
||||
throw new InvalidParameterException($message);
|
||||
}
|
||||
|
||||
|
||||
@ -14,6 +14,7 @@ namespace Symfony\Component\Routing;
|
||||
use Symfony\Component\Config\Loader\LoaderInterface;
|
||||
use Symfony\Component\Config\ConfigCache;
|
||||
use Symfony\Component\HttpKernel\Log\LoggerInterface;
|
||||
use Symfony\Component\Routing\Generator\ConfigurableRequirementsInterface;
|
||||
|
||||
/**
|
||||
* The Router class is an example of the integration of all pieces of the
|
||||
@ -77,7 +78,7 @@ class Router implements RouterInterface
|
||||
'matcher_dumper_class' => 'Symfony\\Component\\Routing\\Matcher\\Dumper\\PhpMatcherDumper',
|
||||
'matcher_cache_class' => 'ProjectUrlMatcher',
|
||||
'resource_type' => null,
|
||||
'strict_parameters' => true,
|
||||
'strict_requirements' => true,
|
||||
);
|
||||
|
||||
// check option names and live merge, if errors are encountered Exception will be thrown
|
||||
@ -244,8 +245,8 @@ class Router implements RouterInterface
|
||||
$this->generator = new $class($this->context, $this->logger);
|
||||
}
|
||||
|
||||
if (false === $this->options['strict_parameters']) {
|
||||
$this->generator->setStrictParameters(false);
|
||||
if ($this->generator instanceof ConfigurableRequirementsInterface) {
|
||||
$this->generator->setStrictRequirements($this->options['strict_requirements']);
|
||||
}
|
||||
|
||||
return $this->generator;
|
||||
|
||||
@ -169,7 +169,7 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
|
||||
{
|
||||
$routes = $this->getRoutes('test', new Route('/testing/{foo}', array('foo' => '1'), array('foo' => 'd+')));
|
||||
$generator = $this->getGenerator($routes);
|
||||
$generator->setStrictParameters(false);
|
||||
$generator->setStrictRequirements(false);
|
||||
$this->assertNull($generator->generate('test', array('foo' => 'bar'), true));
|
||||
}
|
||||
|
||||
@ -184,7 +184,7 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
|
||||
$logger->expects($this->once())
|
||||
->method('err');
|
||||
$generator = $this->getGenerator($routes, array(), $logger);
|
||||
$generator->setStrictParameters(false);
|
||||
$generator->setStrictRequirements(false);
|
||||
$this->assertNull($generator->generate('test', array('foo' => 'bar'), true));
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user