Skip to content

Commit 2cf50b7

Browse files
committed
merged branch Tobion/requirementscheck (PR symfony#5187)
Commits ------- 3363832 extended phpdoc of ConfigurableRequirementsInterface 5f64503 [Routing] added test for disabled requirements check 4225869 [Routing] allow disabling the requirements check on URL generation Discussion ---------- [Routing] allow disabling the requirements check on URL generation This adds the possibility to disable the requirements check (`strict_requirements = null`) on URL generation. To sum up, here the possibilities: - `strict_requirements = true`: throw exception for mismatching requirements (most useful in development environment). - `strict_requirements = false`: don't throw exception but return null as URL for mismatching requirements and log it (useful when you cannot control all params because they come from third party libs or something but don't want to have a 404 in production environment. it logs the mismatch so you can review it). - `strict_requirements = null`: Return the URL with the given parameters without checking the requirements at all. When generating an URL you should either trust your params or you validated them beforehand because otherwise it would break your link anyway (just as with strict_requirements=false). So in production environment you should know that params allways pass the requirements. Thus you could disable the check on each URL generation for performance reasons. If you have 300 links on a page and each URL at least one param you safe 300 unneeded `preg_match` calls. I tested the performance in one of my projects. The rendering time of a single template that contains ~300 links with several params was reduced from avg. 46ms to avg. 42ms. That are 8.7% reduction in the twig layer where the links are created on each request. So this option combines the improved usability of strict_requirements=false with an additional increased performance. --------------------------------------------------------------------------- by fabpot at 2012-08-30T13:40:38Z Can you put the comment about all the possibilities you have mentioned here in the phpdoc for future reference? Thanks. --------------------------------------------------------------------------- by Tobion at 2012-08-30T13:49:25Z In `ConfigurableRequirementsInterface` or which phpdoc would you like to have it? Because `ConfigurableRequirementsInterface` already has it explained. But I can extend its description if thats what you mean. --------------------------------------------------------------------------- by fabpot at 2012-08-30T13:50:40Z The comment in the PR is more explicit and more detailed than the one in the interface. So, yes, basically, it would be great if you can move all the information in the interface phpdoc. --------------------------------------------------------------------------- by Tobion at 2012-08-30T14:35:59Z Done.
2 parents 569e29d + 3363832 commit 2cf50b7

File tree

3 files changed

+35
-8
lines changed

3 files changed

+35
-8
lines changed

src/Symfony/Component/Routing/Generator/ConfigurableRequirementsInterface.php

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,44 @@
1212
namespace Symfony\Component\Routing\Generator;
1313

1414
/**
15-
* ConfigurableRequirementsInterface must be implemented by URL generators in order
16-
* to be able to configure whether an exception should be generated when the
17-
* parameters do not match the requirements.
15+
* ConfigurableRequirementsInterface must be implemented by URL generators that
16+
* can be configured whether an exception should be generated when the parameters
17+
* do not match the requirements. It is also possible to disable the requirements
18+
* check for URL generation completely.
19+
*
20+
* The possible configurations and use-cases:
21+
* - setStrictRequirements(true): Throw an exception for mismatching requirements. This
22+
* is mostly useful in development environment.
23+
* - setStrictRequirements(false): Don't throw an exception but return null as URL for
24+
* mismatching requirements and log the problem. Useful when you cannot control all
25+
* params because they come from third party libs but don't want to have a 404 in
26+
* production environment. It should log the mismatch so one can review it.
27+
* - setStrictRequirements(null): Return the URL with the given parameters without
28+
* checking the requirements at all. When generating an URL you should either trust
29+
* your params or you validated them beforehand because otherwise it would break your
30+
* link anyway. So in production environment you should know that params always pass
31+
* the requirements. Thus this option allows to disable the check on URL generation for
32+
* performance reasons (saving a preg_match for each requirement every time a URL is
33+
* generated).
1834
*
1935
* @author Fabien Potencier <fabien@symfony.com>
36+
* @author Tobias Schultze <http://tobion.de>
2037
*/
2138
interface ConfigurableRequirementsInterface
2239
{
2340
/**
2441
* Enables or disables the exception on incorrect parameters.
42+
* Passing null will deactivate the requirements check completely.
2543
*
26-
* @param Boolean $enabled
44+
* @param Boolean|null $enabled
2745
*/
2846
public function setStrictRequirements($enabled);
2947

3048
/**
31-
* Gets the strict check of incorrect parameters.
49+
* Returns whether to throw an exception on incorrect parameters.
50+
* Null means the requirements check is deactivated completely.
3251
*
33-
* @return Boolean
52+
* @return Boolean|null
3453
*/
3554
public function isStrictRequirements();
3655
}

src/Symfony/Component/Routing/Generator/UrlGenerator.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public function getContext()
100100
*/
101101
public function setStrictRequirements($enabled)
102102
{
103-
$this->strictRequirements = (Boolean) $enabled;
103+
$this->strictRequirements = null === $enabled ? null : (Boolean) $enabled;
104104
}
105105

106106
/**
@@ -146,7 +146,7 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa
146146
if ('variable' === $token[0]) {
147147
if (!$optional || !array_key_exists($token[3], $defaults) || (string) $mergedParams[$token[3]] !== (string) $defaults[$token[3]]) {
148148
// check requirement
149-
if (!preg_match('#^'.$token[2].'$#', $mergedParams[$token[3]])) {
149+
if (null !== $this->strictRequirements && !preg_match('#^'.$token[2].'$#', $mergedParams[$token[3]])) {
150150
$message = sprintf('Parameter "%s" for route "%s" must match "%s" ("%s" given).', $token[3], $name, $token[2], $mergedParams[$token[3]]);
151151
if ($this->strictRequirements) {
152152
throw new InvalidParameterException($message);

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,14 @@ public function testGenerateForRouteWithInvalidOptionalParameterNonStrictWithLog
207207
$this->assertNull($generator->generate('test', array('foo' => 'bar'), true));
208208
}
209209

210+
public function testGenerateForRouteWithInvalidParameterButDisabledRequirementsCheck()
211+
{
212+
$routes = $this->getRoutes('test', new Route('/testing/{foo}', array('foo' => '1'), array('foo' => 'd+')));
213+
$generator = $this->getGenerator($routes);
214+
$generator->setStrictRequirements(null);
215+
$this->assertSame('/app.php/testing/bar', $generator->generate('test', array('foo' => 'bar')));
216+
}
217+
210218
/**
211219
* @expectedException Symfony\Component\Routing\Exception\InvalidParameterException
212220
*/

0 commit comments

Comments
 (0)