Skip to content
Browse files

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

:+1: 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
  • Loading branch information...
2 parents 5b9b995 + 03bbaaf commit 4b521985f121be25360365e2dc7a36645daa26b1 @fabpot fabpot committed
View
2 src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
@@ -169,7 +169,7 @@ private function addRouterSection(ArrayNodeDefinition $rootNode)
->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)'
View
2 src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -246,7 +246,7 @@ private function registerRouterConfiguration(array $config, ContainerBuilder $co
$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'];
}
View
2 src/Symfony/Component/Routing/CHANGELOG.md
@@ -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
View
36 src/Symfony/Component/Routing/Generator/ConfigurableRequirementsInterface.php
@@ -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();
+}
View
22 src/Symfony/Component/Routing/Generator/UrlGenerator.php
@@ -26,10 +26,10 @@
*
* @api
*/
-class UrlGenerator implements UrlGeneratorInterface
+class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInterface
{
protected $context;
- protected $strictParameters = true;
+ protected $strictRequirements = true;
protected $logger;
/**
@@ -95,23 +95,19 @@ public function getContext()
}
/**
- * 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 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa
// 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);
}
View
7 src/Symfony/Component/Routing/Router.php
@@ -14,6 +14,7 @@
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 @@ public function setOptions(array $options)
'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 @@ public function getGenerator()
$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;
View
4 src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
@@ -169,7 +169,7 @@ public function testGenerateForRouteWithInvalidOptionalParameterNonStrict()
{
$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 @@ public function testGenerateForRouteWithInvalidOptionalParameterNonStrictWithLog
$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));
}

0 comments on commit 4b52198

Please sign in to comment.
Something went wrong with that request. Please try again.