[FrameworkBundle] Add command to delete an item from a cache pool #26223

pierredup · 2018-02-19T11:06:31Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	N/A
License	MIT
Doc PR	TBD

Currently there is no way to clear a specific item from a cache pool (except programatically), the entire pool needs to be cleared.
Especially during development, when implementing caching, it is useful to delete a specific key to test functionality. Clearing the entire pool, means that everything will need to be cached again, adding unnecessary execution time.

I propose adding a new command, cache:pool:delete to delete a specific item from a cache pool

nicolas-grekas · 2018-02-19T12:15:58Z

src/Symfony/Bundle/FrameworkBundle/Command/CachePoolDeleteCommand.php

+        $io = new SymfonyStyle($input, $output);
+        $pool = $input->getArgument('pool');
+        $key = $input->getArgument('key');
+        $cachePool = $this->getApplication()->getKernel()->getContainer()->get($pool);


This means the pool has to be public.
What about going through the Psr6CacheClearer instead?

linaori · 2018-02-20T07:33:43Z

src/Symfony/Bundle/FrameworkBundle/Command/CachePoolDeleteCommand.php

+        }
+
+        if (!$cachePool->deleteItem($key)) {
+            return $io->error(sprintf('Cache item "%s" could not be deleted', $key));


The warning and error functions do not return a status code, however, I would imagine that at least the error should return a status code other than 0.

I'm aware that they don't return a status code, but for me this is just cleaner than

$io->warning(sprintf('Cache item "%s" does not exist in cache pool "%s"', $key, $pool)); return;

For the error I think it might be better to rather throw an exception, which should then have a proper error return code set

go for an exception then :)

Destroy666x · 2018-02-20T10:35:46Z

src/Symfony/Bundle/FrameworkBundle/Command/CachePoolDeleteCommand.php

+ *
+ * @author Pierre du Plessis <pdples@gmail.com>
+ */
+final class CachePoolDeleteCommand extends Command


Since this can only delete an item as specified in descriptions (key is required), wouldn't CachePooltemDeleteCommand be more accurate? Also, there's a tyop in the comment above.

This matches the name of the command, and is good enough IMHO as nothing else than an item can be deleted.

This matches the name of the command

I was thinking about renaming both.

as nothing else than an item can be deleted.

Not everyone may know that though and can have different expectations. But as long as it's documented well, it's ok, I guess.

nicolas-grekas · 2018-02-21T11:00:13Z

src/Symfony/Bundle/FrameworkBundle/Command/CachePoolDeleteCommand.php

+        $cachePool = $this->poolClearer->getPool($pool);
+
+        if (!$cachePool->hasItem($key)) {
+            return $io->warning(sprintf('Cache item "%s" does not exist in cache pool "%s"', $key, $pool));


I'd suggest to turn this into a notice, and to explicitly return instead ($io->warning doesn't return anything AFAIK)

nicolas-grekas · 2018-02-21T11:01:39Z

src/Symfony/Bundle/FrameworkBundle/Command/CachePoolDeleteCommand.php

+        }
+
+        if (!$cachePool->deleteItem($key)) {
+            return $io->error(sprintf('Cache item "%s" could not be deleted', $key));


go for an exception then :)

nicolas-grekas · 2018-02-21T11:02:42Z

src/Symfony/Bundle/FrameworkBundle/Command/CachePoolDeleteCommand.php

+            return $io->error(sprintf('Cache item "%s" could not be deleted', $key));
+        }
+
+        $io->success(sprintf('Cache item "%s" was successfully deleted.', $key));


I'd suggest to log only in verbose mode (same above)

hum, maybe not actually :)

nicolas-grekas · 2018-02-21T13:18:29Z

src/Symfony/Bundle/FrameworkBundle/Command/CachePooltemDeleteCommand.php

+ *
+ * @author Pierre du Plessis <pdples@gmail.com>
+ */
+final class CachePooltemDeleteCommand extends Command


missing I before tem, nice typo :)

but this doesn't match the name of the command
cache:pool:delete LGTM, so the previous name was OK IMHO

nicolas-grekas · 2018-02-22T12:08:16Z

src/Symfony/Bundle/FrameworkBundle/Command/CachePoolDeleteCommand.php

+        $cachePool = $this->poolClearer->getPool($pool);
+
+        if (!$cachePool->hasItem($key)) {
+            $io->warning(sprintf('Cache item "%s" does not exist in cache pool "%s".', $key, $pool));


should be a notice

Right, forgot about this. Thanks

nicolas-grekas

with one minor comment

chalasr · 2018-02-22T12:45:05Z

src/Symfony/Bundle/FrameworkBundle/Command/CachePoolDeleteCommand.php

+    {
+        $this
+            ->setDefinition(array(
+                new InputArgument('pool', InputArgument::REQUIRED, 'The cache pool to delete an item from'),


The cache pool from which to delete an item?

nicolas-grekas · 2018-03-02T12:13:25Z

Thank you @pierredup.

… cache pool (pierredup) This PR was squashed before being merged into the 4.1-dev branch (closes #26223). Discussion ---------- [FrameworkBundle] Add command to delete an item from a cache pool | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | N/A | License | MIT | Doc PR | TBD Currently there is no way to clear a specific item from a cache pool (except programatically), the entire pool needs to be cleared. Especially during development, when implementing caching, it is useful to delete a specific key to test functionality. Clearing the entire pool, means that everything will need to be cached again, adding unnecessary execution time. I propose adding a new command, `cache:pool:delete` to delete a specific item from a cache pool Commits ------- fd43e81 [FrameworkBundle] Add command to delete an item from a cache pool

xabbuh · 2018-03-09T13:41:42Z

src/Symfony/Bundle/FrameworkBundle/Command/CachePoolDeleteCommand.php

+
+    private $poolClearer;
+
+    public function __construct(Psr6CacheClearer $poolClearer)


Do we really want to inject the cache clearer to then use it to fetch a pool? Shouldn't we rather inject the pools directly?

carsonbot added Status: Needs Review FrameworkBundle Feature labels Feb 19, 2018

pierredup force-pushed the cache-commands branch from 06148c8 to 26a049e Compare February 19, 2018 11:12

nicolas-grekas added this to the 4.1 milestone Feb 19, 2018

nicolas-grekas reviewed Feb 19, 2018

View reviewed changes

pierredup force-pushed the cache-commands branch from 26a049e to 56dd080 Compare February 19, 2018 12:29

linaori reviewed Feb 20, 2018

View reviewed changes

Destroy666x reviewed Feb 20, 2018

View reviewed changes

nicolas-grekas requested changes Feb 21, 2018

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Feb 21, 2018

pierredup force-pushed the cache-commands branch 3 times, most recently from 3451432 to dccd939 Compare February 21, 2018 12:59

nicolas-grekas reviewed Feb 21, 2018

View reviewed changes

Add command to delete an item from a cache pool

89355ba

pierredup force-pushed the cache-commands branch from dccd939 to 89355ba Compare February 21, 2018 14:13

nicolas-grekas reviewed Feb 22, 2018

View reviewed changes

nicolas-grekas approved these changes Feb 22, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Feb 22, 2018

Change warning to notice

e556003

chalasr approved these changes Feb 22, 2018

View reviewed changes

chalasr reviewed Feb 22, 2018

View reviewed changes

pierredup added 2 commits February 22, 2018 15:40

Change pool description

6cfae6f

Fix typo

0e00b76

nicolas-grekas closed this Mar 2, 2018

xabbuh reviewed Mar 9, 2018

View reviewed changes

javiereguiluz mentioned this pull request Mar 20, 2018

[FrameworkBundle] Add command to delete an item from a cache pool symfony/symfony-docs#9454

Closed

fabpot mentioned this pull request May 7, 2018

Release v4.1.0-BETA1 #27181

Merged

pierredup deleted the cache-commands branch May 25, 2018 09:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FrameworkBundle] Add command to delete an item from a cache pool #26223

[FrameworkBundle] Add command to delete an item from a cache pool #26223

pierredup commented Feb 19, 2018

nicolas-grekas Feb 19, 2018

linaori Feb 20, 2018

pierredup Feb 20, 2018 •

edited

Loading

pierredup Feb 20, 2018 •

edited

Loading

nicolas-grekas Feb 21, 2018

Destroy666x Feb 20, 2018 •

edited

Loading

nicolas-grekas Feb 22, 2018

Destroy666x Feb 22, 2018 •

edited

Loading

nicolas-grekas Feb 21, 2018 •

edited

Loading

nicolas-grekas Feb 21, 2018

nicolas-grekas Feb 21, 2018

nicolas-grekas Feb 21, 2018

nicolas-grekas Feb 21, 2018

nicolas-grekas Feb 21, 2018

nicolas-grekas Feb 22, 2018

pierredup Feb 22, 2018

nicolas-grekas left a comment

chalasr Feb 22, 2018

nicolas-grekas commented Mar 2, 2018

xabbuh Mar 9, 2018


		private $poolClearer;

		public function __construct(Psr6CacheClearer $poolClearer)

[FrameworkBundle] Add command to delete an item from a cache pool #26223

[FrameworkBundle] Add command to delete an item from a cache pool #26223

Conversation

pierredup commented Feb 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pierredup Feb 20, 2018 • edited Loading

Choose a reason for hiding this comment

pierredup Feb 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Destroy666x Feb 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Destroy666x Feb 22, 2018 • edited Loading

Choose a reason for hiding this comment

nicolas-grekas Feb 21, 2018 • edited Loading

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

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

nicolas-grekas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolas-grekas commented Mar 2, 2018

Choose a reason for hiding this comment

pierredup Feb 20, 2018 •

edited

Loading

pierredup Feb 20, 2018 •

edited

Loading

Destroy666x Feb 20, 2018 •

edited

Loading

Destroy666x Feb 22, 2018 •

edited

Loading

nicolas-grekas Feb 21, 2018 •

edited

Loading