[Routing] allow disabling the requirements check on URL generation #5187
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can you put the comment about all the possibilities you have mentioned here in the phpdoc for future reference? Thanks. |
|
In |
|
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. |
|
Done. |
|
merged, thanks |
fabpot
added a commit
that referenced
this pull request
Aug 30, 2012
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.
fabpot
added a commit
that referenced
this pull request
Sep 5, 2012
|
I've just reverted this pull request as it introduces some regressions? It was a very bad idea to merge this so close to the release of 2.1 and I'm the one to blame. Sorry about that. @Tobion As apparently it is not possible to reopen a pull request, can you re-submit it again, so that it is not lost? Thanks and sorry again for the trouble. |
|
What regressions does it introduce? I cannot reopen as i already deleted the branch locally and remote. |
|
@Tobion as the commits are still in the symfony history, you can recreate the branches (note that you need to cherry-pick the commits anyway as the existing commits are already merged) |
This was referenced Sep 6, 2012
fabpot
added a commit
that referenced
this pull request
Oct 3, 2012
This PR was merged into the master branch. Commits ------- faee47c added note in changelog about disabling requirements check 5fbed36 extended phpdoc of ConfigurableRequirementsInterface 1964d43 [Routing] added test for disabled requirements check 98fb915 [Routing] allow disabling the requirements check on URL generation Discussion ---------- [Routing] allow disabling the requirements check on URL generation Reopened version of #5187 Requires #5445 first --------------------------------------------------------------------------- by fabpot at 2012-10-03T19:03:25Z Can you rebase on master and add a note in the CHANGELOG? Also, I think the doc should be updated accordingly. Thanks. --------------------------------------------------------------------------- by Tobion at 2012-10-03T19:04:07Z @fabpot ping. The config for this feature is already present in master in symfony-standard. See [config_prod.yml](https://github.com/symfony/symfony-standard/blob/master/app/config/config_prod.yml) --------------------------------------------------------------------------- by Tobion at 2012-10-03T19:06:53Z Hehe, I already rebased while you wrote that. You mean I should add a tip to the routing component documentation? Sure, I have alread planned to adjust the symfony doc for the recent routing PRs. --------------------------------------------------------------------------- by fabpot at 2012-10-03T19:12:36Z Just adding a note about the new possibility in the CHANGELOG. --------------------------------------------------------------------------- by Tobion at 2012-10-03T19:53:19Z @fabpot done
sarramegnag
pushed a commit
to sarramegnag/symfony-vue
that referenced
this pull request
Sep 16, 2018
sarramegnag
pushed a commit
to sarramegnag/symfony-vue
that referenced
this pull request
Sep 16, 2018
Commits ------- ef43472 added upgrade note for strict_requirements = null 33eaa39 configure strict_requirements: null in production Discussion ---------- configure strict_requirements: null in production possible since symfony/symfony#5187
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 unneededpreg_matchcalls. 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.