Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
Refactor behaviour triggered by a flag into separate classes.#7782
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
Closed
mpdude wants to merge1 commit intosymfony:masterfromwebfactory:issue-7230/picks/configcache-behaviour-flag
Closed
Refactor behaviour triggered by a flag into separate classes.#7782
mpdude wants to merge1 commit intosymfony:masterfromwebfactory:issue-7230/picks/configcache-behaviour-flag
Uh oh!
There was an error while loading.Please reload this page.
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
Just noticed we can probably solve validation with a decorator instead of inheritance. |
1 task
This PR has been superseeded by#15738 |
fabpot added a commit that referenced this pull requestSep 25, 2015
…pdude)This PR was squashed before being merged into the 2.8 branch (closes#15738).Discussion----------Implement service-based Resource (cache) validation| Q | A| ------------- | ---| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | yes| Tests pass? | yes| Fixed tickets |#7230,#15692,#7782| License | MIT| Doc PR |symfony/symfony-docs#5136### OverviewCurrently, any metadata passed to `ConfigCache` (namely implementations of `ResourceInterface`) is serialized to disk. When the `ConfigCache` is validated, the metadata is unserialized and queried through `ResourceInterface::isFresh()` to determine whether the cache is fresh. That way, `ResourceInterface` implementations cannot interact with services, for example a database connection.This PR introduces the new concept of `ResourceCheckers`. Services implementing `ResourceCheckerInterface` can be tagged as `config_cache.resource_checker` with an optional priority.Clients that wish to use `ConfigCache` can then obtain an instance from the `config_cache_factory` service (which implements `ConfigCacheFactoryInterface`). The factory will take care of injecting resource checkers into the `ConfigCache` instance so that they can be used for cache validation.Checking cache metadata is easy for `ResourceCheckers`:* First, the `ResourceCheckerInterface::supports()` implementation is passed the metadata object in question. If the checker cannot handle the type of resource passed, `supports()` should return `false`.* Otherwise, the `ResourceCheckerInterface::isFresh()` method will be called and given the resource as well as the timestamp at which the cache was initialized. If that method returns `false`, the cache is considered stale. If it returns `true`, the resource is considered unchanged and will *not* be passed to any additional checkers.### BC and migration pathThis PR does not (intend to) break BC but it comes with deprecations. The main reason is that `ResourceInterface` contains an `isFresh()` method that does not make sense in the general case of resources.Thus, `ResourceInterface::isFresh()` is marked as deprecated and should be removed in Symfony 3.0. Resource implementations that can (or wish to) be validated in that simple manner can implement the `SelfCheckingResourceInterface` sub-interface that still contains (and will keep) the `isFresh()` method. The change should be as simple as changing the `extends` list.Apart from that, `ResourceInterface` will be kept as the base interface for resource implementations. It is used in several `@api` interfaces and thus cannot easily be substituted.For the Symfony 2.x series, a `BCResourceInterfaceChecker` will be kept that performs validation through `ResourceInterface::isFresh()` but will trigger a deprecation warning. The remedy is to either implement a custom ResourceChecker with a priority higher than -1000; or to switch to the aforementioned `SelfCheckingResourceInterface` which is used at a priority of -990 (without deprecation warning).The `ConfigCache` and `ConfigCacheFactory` classes can be used as previously but do not feature checker-based cache validation.### Outlook and closing remarks:This PR supersedes#7230,#15692 and works at least in parts towards the goal of#7176.The `ResourceCheckerInterface`, `...ConfigCache` and `...ConfigCacheFactory` no longer need to be aware of the `debug` flag. The different validation rules applied previously are now just a matter of `ResourceChecker` configuration (i. e. "no checkers" in `prod`).It might be possible to remove the `debug` flag from Symfony's `Router` and/or `Translator` classes in the future as well because it was only passed on to the `ConfigCache` there.Commits-------20d3722 Implement service-based Resource (cache) validation
Sign up for freeto 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.
The ConfigCache class has two modes of behavior, toggled by the "debug"flag.
Refactoring that into two different classes may seem pointless or overkill at the moment, but gets more interesting once resource checking/validation gets more intricate. Peek atthis if you're curious :-).
It's a pick off#7230.
NB. With the changes from#7781, you probably don't want to use
ConfigCache
anymore but instead have the factory create the right implementation and use it directly.