[Cache] Add (pdo|chain) cache (adapter|simple) prune method #23603

robfrawley · 2017-07-20T17:24:11Z

Q	A
Branch?	3.4
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR	symfony/symfony-docs#8209

This is additional work toward making more cache implementations use the PruneableInterface contract (an extension of #23451). Specifically, this pull request focuses on (Pdo|Chain)(Cache|Adapter), as suggested by @nicolas-grekas in #23451 (comment).

robfrawley · 2017-07-21T06:49:29Z

@nicolas-grekas Is passing PruneableInterface mocks to ChainCache|ChainAdaper and ensuring prune() is called on the mocks a solid enough way to test the Chain implementations?

nicolas-grekas · 2017-07-21T06:52:36Z

I think it is yes

robfrawley · 2017-07-21T15:51:06Z

Aside from the tests, which take up the bulk of the additions, this amends ChainAdapter, PdoAdapter, ChainCache, and PdoCache so they implement PruneableInterface. All seems to work as expected. :-)

robfrawley · 2017-07-21T15:51:20Z

Test failures seem unrelated, FYI.

nicolas-grekas · 2017-07-22T14:03:33Z

rebase unlocked

robfrawley · 2017-07-22T19:00:39Z

Wonderful; thanks. Rebased.

nicolas-grekas · 2017-07-22T19:13:25Z

src/Symfony/Component/Cache/Traits/ChainTrait.php

+     *
+     * @return bool
+     */
+    private function doPrune(array $caches)


I think this doesn't deserve a trait :)

nicolas-grekas · 2017-07-22T19:18:23Z

src/Symfony/Component/Cache/Traits/PdoTrait.php

+    public function prune()
+    {
+        $pdoConn = $this->getConnection();
+        $selects = $pdoConn->prepare("SELECT $this->idCol FROM $this->table WHERE $this->lifetimeCol + $this->timeCol <= :time");


This implementation is no-go, it doesn't scale at all. Can't we just do one single DELETE instead?

Of course, you are right; I'd pulled the from another method, but that one had a need to also return values. :-) Only three lines now.

robfrawley · 2017-07-22T21:03:33Z

src/Symfony/Component/Cache/Tests/Adapter/AdapterTestCase.php

@@ -96,6 +98,18 @@ public function testPrune()
            $cache->save($item);
        };

+        $doSet('foo', 'foo-val', new \DateInterval('PT05S'));


This may seem to duplicate tests, but the prior tests didn't check pruning of more than one item at a time; so this test ensures four items can be all pruned at once.

robfrawley · 2017-07-22T21:04:40Z

Update pushed that addresses your review comments.

nicolas-grekas · 2017-07-22T21:26:25Z

src/Symfony/Component/Cache/Traits/PdoTrait.php

@@ -140,6 +140,17 @@ public function createTable()
    /**
     * {@inheritdoc}
     */
+    public function prune()
+    {
+        $deletes = $this->getConnection()->prepare("DELETE FROM $this->table WHERE $this->lifetimeCol + $this->timeCol <= :time");


What about adding a LIKE clause for the namespace?

Since the namespace property is a private property of AbstractAdapter (well, actually the trait AbstractTrait but that is used by AbstractAdapter) how am I to determine the namespace? I think it's a fine idea, just not sure how to accomplish it.

we have the namespace in the init method, we can borrow it from there

robfrawley · 2017-07-23T16:33:10Z

Pushed a commit that re-declares namespace for the PdoTrait context and grabs a copy from the init method; this is later utilized in the prune call. Let me know if that is the implementation you had in mind.

nicolas-grekas

looks good.
just one thing:
PHP Strict Standards: Static function Symfony\Component\Cache\Tests\Traits\PdoPruneableTrait::fail() should not be abstract in /home/travis/build/symfony/symfony/src/Symfony/Component/Cache/Tests/Traits/PdoPruneableTrait.php on line 44

nicolas-grekas · 2017-07-23T18:42:32Z

src/Symfony/Component/Cache/Traits/PdoTrait.php

+    {
+        $deleteSql = "DELETE FROM $this->table WHERE $this->lifetimeCol + $this->timeCol <= :time";
+
+        if ($this->namespace) {


'' !== $this->namespace

nicolas-grekas · 2017-07-23T18:42:37Z

src/Symfony/Component/Cache/Traits/PdoTrait.php

+        $delete = $this->getConnection()->prepare($deleteSql);
+        $delete->bindValue(':time', time(), \PDO::PARAM_INT);
+
+        if ($this->namespace) {


robfrawley · 2017-07-23T19:01:26Z

@nicolas-grekas Comments addressed and changes pushed. I've squashed everything, as well.

I'm sure you noticed I didn't add PruneableInterface to the apcu caches, as, after considering it and playing around a bit, I came to the same conclusion you mentioned in #23451 (comment):

not sure about [apcu] esp. since this will be unusable from the command because of different shared memory segments used by the webserver/FPM

If you decide it does add value, though, let me know and I'll be happy to include it here. Otherwise, thanks for the quick reviews again!

robfrawley · 2017-07-24T21:44:05Z

Pushed two very small changes in latest commit:

Amend PruneableInterface documentation description to clarify its purpose and fix a typo to read "Interface extends psr-6 and psr-16 caches to allow for pruning (deletion) of all expired cache items.".
Remove unused and forgotten sprintf from PdoPruneableTrait::isPruned() method (was a holdover from when I was passing some replacements into the string in an earlier iteration).

Trivial stuff, but just mentioning so as not to slip in changes without people knowing.

nicolas-grekas · 2017-08-07T07:19:59Z

@robfrawley the title of this PR still says "WIP", what's the real status? (btw, I personally prefer not putting statuses in PR title, we have labels for that :) )

robfrawley · 2017-08-10T04:57:08Z

@nicolas-grekas Sorry about the delay, but now I've updated the title to remove the "WIP" --- as far as I'm concerned this is ready for merge. (Note taken about the use of titles versus labels ;) ).

robfrawley · 2017-08-14T03:43:19Z

Are there any additional changed you need me to implement? Also, when you have a moment, can you review the documentation PR (symfony/symfony-docs#8209) and advise if it adequately describes the changes from this PR as well as #23451.

nicolas-grekas

with minor comment

nicolas-grekas · 2017-08-22T12:30:44Z

src/Symfony/Component/Cache/Tests/Traits/PdoPruneableTrait.php

+
+        $getPdoConn = $o->getMethod('getConnection');
+        $getPdoConn->setAccessible(true);
+        $refPropVal = function ($name) use ($cache, $o) {


tests can know the default value so I would hard code them instead to reduce the boilerplate and logic in test cases

robfrawley · 2017-08-30T20:10:36Z

Test failure unrelated.

nicolas-grekas · 2017-08-31T14:19:44Z

Thank you @robfrawley.

@nicolas-grekas

…ethod (robfrawley) This PR was merged into the 3.4 branch. Discussion ---------- [Cache] Add (pdo|chain) cache (adapter|simple) prune method | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | symfony/symfony-docs#8209 This is additional work toward making more cache implementations use the `PruneableInterface` contract. Specifically, this pull request focuses on `(Pdo|Chain)(Cache|Adapter)`, as suggested by @nicolas-grekas in #23451 (comment). Commits ------- b20a237 add (pdo|chain) cache (adapter|simple) prune method

…dapter|simple) prune method and prune command (robfrawley, javiereguiluz) This PR was merged into the 3.4 branch. Discussion ---------- [Cache] Document (filesystem|phpfiles|pdo|chain) cache (adapter|simple) prune method and prune command This PR documents the changes introduced in symfony/symfony#23451 and symfony/symfony#23603: - The addition of [`PruneableInterface`](https://github.com/symfony/cache/blob/master/PruneableInterface.php) - The variouse, concrete [`PruneableInterface::prune()`](https://github.com/symfony/cache/blob/master/PruneableInterface.php#L22) implementations in [`ChainAdapter`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Adapter/ChainAdapter.php), [`FilesystemAdapter`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Adapter/FilesystemAdapter.php), [`PdoAdapter`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Adapter/PdoAdapter.php), [`PhpFilesAdapter`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Adapter/PhpFilesAdapter.php),[ `ChainCache`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Simple/ChainCache.php), [`FilesystemCache`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Simple/FilesystemCache.php), [`PdoCache`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Simple/PdoCache.php), [`PhpFilesCache`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Simple/PhpFilesCache.php), and [`TagAwareAdapter`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Adapter/TagAwareAdapter.php). - The addition of [`CachePoolPruneCommand`](https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Command/CachePoolPruneCommand.php), invokable via `cache:pool:prune`. This command iterates over all services tagged `cache.pool` and calls the [`PruneableInterface::prune()`](https://github.com/symfony/cache/blob/master/PruneableInterface.php#L22) method on those that implement [`PruneableInterface`](https://github.com/symfony/cache/blob/master/PruneableInterface.php) Additionally, some language changes and cleanup has been implemented for the various cache docs. Of note are the following changes: - Additional `note` blocks for all caches that implement [`PruneableInterface`](https://github.com/symfony/cache/blob/master/PruneableInterface.php) - Addition of `prune()` description and usage example on the chain adapter - Addition of a `tip` about achieving better performance with the filesystem adapter by utilizing `tmpfs` or another ram disk solution - Fix for an incorrect `use` statement in the PHP array cache adapter code example - Addition of documentation page for PHP files adapter - Addition of a "pruning cache items" explanation and example on the main cache pools page Commits ------- 54a1e62 Typo 1322ffd Typos and wrapped long lines 5bea1e6 Fixed a typo eb70a36 Wrapped long lines ea29252 document cache pruning functionality - note the addition of PruneableInterface - note the variouse, concrete PruneableInterface::prune() implementations in ChainAdapter, FilesystemAdapter, PdoAdapter, PhpFilesAdapter, ChainCache, FilesystemCache, PdoCache, PhpFilesCache, and TagAwareAdapter. - note the addition of CachePoolPruneCommand, invokable via cache:pool:prune. This command iterates over all services tagged cache.pool and calls the PruneableInterface::prune() method on those that implement PruneableInterface - rewording of some adapter text for clarity - additional note blocks for all caches that implement PruneableInterface - addition of prune() description and usage example on the chain adapter - addition of a tip about achieving better performance with the filesystem adapter by utilizing tmpfs or another ram disk solution - fix for an incorrect use statement in the PHP array cache adapter code example - addition of documentation page for PHP files adapter - addition of a "pruning cache items" explanation and example on the main cache pools page

carsonbot added Status: Needs Review Cache Feature labels Jul 20, 2017

robfrawley force-pushed the feature-pdo-chain-cache-prune branch 2 times, most recently from 109d18f to f332e68 Compare July 20, 2017 17:30

robfrawley changed the title ~~[WIP] [Cache] Add pdo cache (adapter|simple) prune method~~ [WIP] [Cache] Add (pdo|chain) cache (adapter|simple) prune method Jul 21, 2017

robfrawley force-pushed the feature-pdo-chain-cache-prune branch 2 times, most recently from fb85ed6 to 2e328b0 Compare July 21, 2017 15:43

robfrawley force-pushed the feature-pdo-chain-cache-prune branch 2 times, most recently from 5d68adb to 7385048 Compare July 21, 2017 17:32

nicolas-grekas added this to the 3.4 milestone Jul 22, 2017

robfrawley force-pushed the feature-pdo-chain-cache-prune branch from 7385048 to 78d31e1 Compare July 22, 2017 18:59

nicolas-grekas reviewed Jul 22, 2017

View reviewed changes

robfrawley force-pushed the feature-pdo-chain-cache-prune branch from 17fce58 to 87ac9a6 Compare July 22, 2017 21:01

robfrawley commented Jul 22, 2017

View reviewed changes

nicolas-grekas reviewed Jul 22, 2017

View reviewed changes

robfrawley force-pushed the feature-pdo-chain-cache-prune branch from 11bc2ad to 5d0e016 Compare July 23, 2017 16:39

nicolas-grekas reviewed Jul 23, 2017

View reviewed changes

robfrawley force-pushed the feature-pdo-chain-cache-prune branch from 5d0e016 to d89caf3 Compare July 23, 2017 18:55

robfrawley changed the title ~~[WIP] [Cache] Add (pdo|chain) cache (adapter|simple) prune method~~ [Cache] Add (pdo|chain) cache (adapter|simple) prune method Aug 10, 2017

nicolas-grekas approved these changes Aug 22, 2017

View reviewed changes

add (pdo|chain) cache (adapter|simple) prune method

b20a237

robfrawley force-pushed the feature-pdo-chain-cache-prune branch from c3ef400 to b20a237 Compare August 30, 2017 19:10

nicolas-grekas merged commit b20a237 into symfony:3.4 Aug 31, 2017

Toflar mentioned this pull request Sep 3, 2017

Symfony taggable cache FriendsOfSymfony/FOSHttpCache#373

Closed

This was referenced Oct 18, 2017

Release v3.4.0-BETA1 #24609

Merged

Release v4.0.0-BETA1 #24612

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cache] Add (pdo|chain) cache (adapter|simple) prune method #23603

[Cache] Add (pdo|chain) cache (adapter|simple) prune method #23603

robfrawley commented Jul 20, 2017 •

edited

Loading

robfrawley commented Jul 21, 2017

nicolas-grekas commented Jul 21, 2017

robfrawley commented Jul 21, 2017 •

edited

Loading

robfrawley commented Jul 21, 2017

nicolas-grekas commented Jul 22, 2017

robfrawley commented Jul 22, 2017

nicolas-grekas Jul 22, 2017

nicolas-grekas Jul 22, 2017

robfrawley Jul 22, 2017

robfrawley Jul 22, 2017

robfrawley commented Jul 22, 2017

nicolas-grekas Jul 22, 2017

robfrawley Jul 23, 2017

nicolas-grekas Jul 23, 2017

robfrawley commented Jul 23, 2017 •

edited

Loading

nicolas-grekas left a comment

nicolas-grekas Jul 23, 2017

nicolas-grekas Jul 23, 2017

robfrawley commented Jul 23, 2017 •

edited

Loading

robfrawley commented Jul 24, 2017 •

edited

Loading

nicolas-grekas commented Aug 7, 2017

robfrawley commented Aug 10, 2017

robfrawley commented Aug 14, 2017

nicolas-grekas left a comment

nicolas-grekas Aug 22, 2017

robfrawley Aug 30, 2017

robfrawley commented Aug 30, 2017

nicolas-grekas commented Aug 31, 2017

[Cache] Add (pdo|chain) cache (adapter|simple) prune method #23603

[Cache] Add (pdo|chain) cache (adapter|simple) prune method #23603

Conversation

robfrawley commented Jul 20, 2017 • edited Loading

robfrawley commented Jul 21, 2017

nicolas-grekas commented Jul 21, 2017

robfrawley commented Jul 21, 2017 • edited Loading

robfrawley commented Jul 21, 2017

nicolas-grekas commented Jul 22, 2017

robfrawley commented Jul 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robfrawley commented Jul 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robfrawley commented Jul 23, 2017 • edited Loading

nicolas-grekas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robfrawley commented Jul 23, 2017 • edited Loading

robfrawley commented Jul 24, 2017 • edited Loading

nicolas-grekas commented Aug 7, 2017

robfrawley commented Aug 10, 2017

robfrawley commented Aug 14, 2017

nicolas-grekas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robfrawley commented Aug 30, 2017

nicolas-grekas commented Aug 31, 2017

robfrawley commented Jul 20, 2017 •

edited

Loading

robfrawley commented Jul 21, 2017 •

edited

Loading

robfrawley commented Jul 23, 2017 •

edited

Loading

robfrawley commented Jul 23, 2017 •

edited

Loading

robfrawley commented Jul 24, 2017 •

edited

Loading