Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[RFC] [Config] Use a Factory for ConfigCache instances#7781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Rebased on master. By using such an exchangeable ConfigCache factory it becomes possible to implement different resource (freshness) checking strategies, especially service-based ones. The goal is to be able to validate Translators and Routers generated by database-based loaders. It might also help with symfony/AsseticBundle#168. I think this is the smallest set of changes from the long-standing#7230 necessary to implement the rest of it as a bundle outside of Symfony itself (in case you don't want/like it). Tests will follow once I've got some feedback on this. The relevant code sections in the Router (with ConfigCache enabled) seem to be not covered by tests at all at the moment. |
There is a lot of changes in this class since November 2013. Seehttps://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php |
👍 ping @symfony/deciders |
@mpdude Can you create a doc issue (a PR would be even better) on how to use the new classes; I think it can be a quick cookbook entry. |
| @@ -308,7 +332,7 @@ public function getGenerator() | |||
| /** | |||
| * @return GeneratorDumperInterface | |||
| */ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can you add an@internal tag here as making it public is only because of limitations in PHP 5.3. This should be converted back toprotected in 3.0.
In#11373, the cache handling was moved to the |
@mpdude is going to update this PR to make it work for 2.7 really soon now. |
What is the use-case? In general looks ok but without a different implementation of the factory, it doesn't seem that useful alone. |
I just created#14178 for this change on the 2.7 branch. (Or can I somehow change the base branch for a PR in GitHub?) |
@mpdude No, you cannot, closing this one. |
… a factory. (mpdude)This PR was squashed before being merged into the 2.7 branch (closes#14178).Discussion----------[Config] Delegate creation of ConfigCache instances to a factory.| Q | A| ------------- | ---| Bug fix? | no| New feature? | yes (refactoring, new flex point)| BC breaks? | no| Deprecations? | yes| Tests pass? | we'll see :-)| Fixed tickets | n/a| License | MIT| Doc PR |symfony/symfony-docs#5136In the Routing/Router and Translation/Translator, delegate creation of ConfigCache instances to a factory. The factory can be setter-injected but will default to a BC implementation.The ```ConfigCacheFactoryInterface``` is designed in a way that captures the common ```$cache = new ...; if (!$cache->isFresh()) { ... do sth }``` pattern. But more importantly, this design allows factory implementations to take additional measures to avoid race conditions before actually filling the cache.By using an exchangeable ConfigCache factory it becomes possible to implement different resource (freshness) checking strategies, especially service-based ones.The goal is to be able to validate Translators and Routers generated by database-based loaders. It might also help with symfony/AsseticBundle#168. This PR only contains the minimum changes needed, so the rest could be implemented in a bundle outside the core (at least for the beginning).Component/HttpKernel/Kernel::initializeContainer still uses the ConfigCache implementation directly as there is no sensible way of getting/injecting a factory service (chicken-egg).This is a pick off#7230. It replaces#7781 which was against the master branch. Also see#7781 for additional comments/explanations.## Todo* [ ] Allow `symfony/config` `~3.0.0` in `composer.json` for the HttpKernel and Translator component as well as TwigBundle once this PR has been merged into the master branch (fail deps=high tests for the time being).Commits-------6fbe9b1 [Config] Delegate creation of ConfigCache instances to a factory.
This PR has been replaced by#14178.