Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Cache] Add [Taggable]CacheInterface, the easiest way to use a cache#26929
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
e959679 to71fe02aCompare3cffe56 to521b376CompareKoc commentedApr 15, 2018
Looks interesting and useful, but not sure that creating another interface and not depends on PSR6 is good idea. |
nicolas-grekas commentedApr 15, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The FIG provides the most common denominator. On the other side, it should not slowdown innovation. |
Koc commentedApr 15, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Is it possible to wrap any PSR6-compatible adapter to use it with this new interfaces? Maybe through ProxyAdapter. If yes - 👍 |
nicolas-grekas commentedApr 15, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Correct, through ProxyAdapter! |
| * | ||
| * @return mixed The value corresponding to the provided key | ||
| */ | ||
| publicfunctionget(string$key,callable$callback); |
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.
what's the point of defining those methods when they are the same as the inherited ones?
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.
They are not the same actually: the second argument to the closure is a CacheItemInterface in one case, and a CacheItem for taggable caches, in order to ship the tag() method when needed.
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.
Please add that to the phpdoc description. The difference is not obvious.
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.
comments updated
| * | ||
| * @return iterable The values corresponding to the provided keys | ||
| */ | ||
| publicfunctiongetMultiple(iterable$keys,callable$callback):iterable; |
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.
How about making tagging also support iterable?
| if (!is_array($tags)) { |
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.
good idea, done
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.
$tags A tag or array of tags
iterable of tags
8e0bb54 tofe065d7Compare
weaverryan left a comment
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.
I love this! The seeming downside is that there are 2 new interfaces. But, all the underlying adapters still use the PSR interfaces, and the CacheItemInterface is used. That's a really nice way to do something more innovative, but sticking to the standard. The names are also WAY better for autowiring. Overall, we get the simplicity of PSR16, with an easy opt-in for the more power of PSR6.
Yes!
| ----- | ||
| * added`CacheInterface` and`TaggableCacheInterface` | ||
| * throw`LogicException` when`CacheItem::tag()` is called on an item coming from a non tag-aware pool |
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.
Nice
weaverryan commentedApr 16, 2018
Test failures are "legit" though (at least, it looks like some tests try to tag with a non-taggable adapter). |
fe065d7 to5a89638Comparenicolas-grekas commentedApr 16, 2018
Failures fixed (the fabbot one is a false positive.) |
5a89638 toc33e47cComparenicolas-grekas commentedApr 17, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Abstracting this common fetch-or-compute logic also creates a nice extension point to implementstampede protection. |
4f005c4 to82df0a6Compare6557df0 tob672e2cCompare61530c8 to22b6659Comparenicolas-grekas commentedApr 23, 2018
ping@Nyholm FYI |
7836159 to0bf5a3aCompare…to use a cache (nicolas-grekas)This PR was merged into the 4.2-dev branch.Discussion----------[Cache] Add [Taggable]CacheInterface, the easiest way to use a cache| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#25903| License | MIT| Doc PR | -This feature is a no-brainer, yet it provides a wonderful DX when using a cache:by type-hinting the new `CacheInterface` or `TaggableCacheInterface`, you get access to:```phppublic function get(string $key, callable $callback);````$callback` is called when `$key` is not found in the cache pool.It is given one arguments: a `CacheItemInterface $item` (a `CacheItem` for a `TaggableCacheInterface`), and should return the corresponding value.```php$value = $cache->get($key, function (CacheItemInterface $item) { $item->expiresAfter(3600); return $this->computeValue();});```or for tags, on a `TaggableCacheInterface $cache`:```php$value = $cache->get($key, function (CacheItem $item) { $item->tag('foo_tag'); return $this->computeValue();});```Plain simple, I love it, why didn't we have the idea earlier, isn't it ?! :)Commits-------589ff69 [Cache] Add [Taggable]CacheInterface, the easiest way to use a cache
…lculations (nicolas-grekas)This PR was merged into the 4.2-dev branch.Discussion----------[Cache] Use sub-second accuracy for internal expiry calculations| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | not really| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Embeds#26929,#27009 and#27028, let's focus on the 4th commit for now.This is my last significant PR in the Cache series :)By using integer expiries internally, our current implementations are sensitive to abrupt transitions when time() goes to next second: `$s = time(); sleep(1); echo time() - $s;` *can* display 2 from time to time.This means that we do expire items earlier than required by the expiration settings on items.This also means that there is no way to have a sub-second expiry. For remote backends, that's fine, but for ArrayAdapter, that's a limitation we can remove.This PR replaces calls to `time()` by `microtime(true)`, providing more accurate timing measurements internally.Commits-------08554ea [Cache] Use sub-second accuracy for internal expiry calculations
… out of the Symfony components (nicolas-grekas)This PR was merged into the 4.2-dev branch.Discussion----------Add symfony/contracts: a set of abstractions extracted out of the Symfony components| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | -| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -A set of abstractions extracted out of the Symfony components.This is a topic I've been thinking about for a long time. I feel like the time has come for Symfony to publish some abstractions so that people could build on them in a decoupled way.I've identified interfaces in some components that would greatly benefit from being moved out from the components where they are for now. E.g.#26929 is something that has a broader scope than the Cache component itself.By putting them in a new `symfony/abstractions` package, we would allow more innovation in the Symfony community, at the abstraction level.In order to start small, I propose only one interface that gathers a concept that is shared amongst many components already: `ResetInterface`. It would provide a standard `reset()` method, whose purpose is to set an object back to its initial state, allowing it to be reused many times with no side effects/leaks related to its history. By this definition, it could also be autoconfigured (as done here, see update in FrameworkExtension). See wording in the docblock in the attached source code.Ideally, I'd like this package to provide not only interfaces, by also generic traits, and reference test suites when possible. We could work on adding more abstractions during the 4.2 cycle. WDYT?## Here is the attached README:A set of abstractions extracted out of the Symfony components.Can be used to build on semantics that the Symfony components proved useful - andthat already have battle tested implementations.Design Principles----------------- * contracts are split by domain, each into their own sub-namespaces; * contracts are small and consistent sets of PHP interfaces, traits, normative docblocks and reference test suites when applicable, etc.; * all contracts must have a proven implementation to enter this repository; * they must be backward compatible with existing Symfony components.FAQ---### How to use this package?The abstractions in this package are useful to achieve loose coupling andinteroperability. By using the provided interfaces as type hints, you are ableto reuse any implementations that match their contracts. It could be a Symfonycomponent, or another one provided by the PHP community at large.Depending on their semantics, some interfaces can be combined with autowiring toseamlessly inject a service in your classes.Others might be useful as labeling interfaces, to hint about a specific behaviorthat could be enabled when using autoconfiguration or manual service tagging (orany other means provided by your framework.)### How is this different from PHP-FIG's PSRs?When applicable, the provided contracts are built on top of PHP-FIG's PSR. Weencourage relying on them and won't duplicate the effort. Still, the FIG hasdifferent goals and different processes. Here, we don't need to seek universalstandards. Instead, we're providing abstractions that are compatible with theimplementations provided by Symfony. This should actually also contributepositively to the PHP-FIG (from which Symfony is a member), by hinting the groupat some abstractions the PHP world might like to take inspiration from.### Why isn't this package split into several packages?Putting all interfaces in one package eases discoverability and dependencymanagement. Instead of dealing with a myriad of small packages and thecorresponding matrix of versions, you just need to deal with one package and oneversion. Also when using IDE autocompletion or just reading the source code, itmakes it easier to figure out which contracts are provided.There are two downsides to this approach: you may have unused files in your`vendor/` directory, and in the future, it will be impossible to use twodifferent sub-namespaces in different major versions of the package. For the"unused files" downside, it has no practical consequences: their file sizes arevery small, and there is no performance overhead at all since they are neverloaded. For major versions, this package follows the Symfony BC + deprecationpolicies, with an additional restriction to never remove deprecated interfaces.Resources--------- * [Documentation](https://symfony.com/doc/current/components/contracts.html) * [Contributing](https://symfony.com/doc/current/contributing/index.html) * [Report issues](https://github.com/symfony/symfony/issues) and [send Pull Requests](https://github.com/symfony/symfony/pulls) in the [main Symfony repository](https://github.com/symfony/symfony)Commits-------8982036 Added symfony/contracts: a set of abstractions extracted out of the components
Uh oh!
There was an error while loading.Please reload this page.
This feature is a no-brainer, yet it provides a wonderful DX when using a cache:
by type-hinting the new
CacheInterfaceorTaggableCacheInterface, you get access to:$callbackis called when$keyis not found in the cache pool.It is given one arguments: a
CacheItemInterface $item(aCacheItemfor aTaggableCacheInterface), and should return the corresponding value.or for tags, on a
TaggableCacheInterface $cache:Plain simple, I love it, why didn't we have the idea earlier, isn't it ?! :)