feature #28718 [Cache] add CacheInterface::delete() + improve CacheTrait (nicolas-grekas)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Cache] add CacheInterface::delete() + improve CacheTrait

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

I've been hesitating a lot on this topic, but I think we should add a `delete()` method to `CacheInterface`.
Deleting is a very common invalidation strategy and invalidating by passing `$beta=INF` to `get()` has the drawback of requiring a fetch+unserialize+save-with-past-expiration. That's complexity that a delete removes.

This PR fixes an issue found meanwhile on `get()`: passing an `ItemInterface` to its callback makes it harder than required to implement on top of PSR-6. Let's pass a `CacheItemInterface`.

Lastly, the early expiration logic can be moved from the component to the trait shipped on contracts.

Here we are for all these steps.

Commits
-------

c6cf690b2f [Cache] add CacheInterface::delete() + improve CacheTrait

Author: fabpot

Cache/CacheInterface.php

@@ -11,31 +11,46 @@
 
 namespace Symfony\Contracts\Cache;
 
+use Psr\Cache\CacheItemInterface;
 use Psr\Cache\InvalidArgumentException;
 
 /**
- * Gets/Stores items from/to a cache.
- *
- * On cache misses, a callback is called that should return the missing value.
- * This callback is given an ItemInterface object corresponding to the needed key,
- * that could be used e.g. for expiration control.
+ * Covers most simple to advanced caching needs.
  *
  * @author Nicolas Grekas <[email protected]>
  */
 interface CacheInterface
 {
     /**
-     * @param string                        $key      The key of the item to retrieve from the cache
-     * @param callable(ItemInterface):mixed $callback Should return the computed value for the given key/item
-     * @param float|null                    $beta     A float that, as it grows, controls the likeliness of triggering
-     *                                                early expiration. 0 disables it, INF forces immediate expiration.
-     *                                                The default (or providing null) is implementation dependent but should
-     *                                                typically be 1.0, which should provide optimal stampede protection.
-     *                                                See https://en.wikipedia.org/wiki/Cache_stampede#Probabilistic_early_expiration
+     * Fetches a value from the pool or computes it if not found.
+     *
+     * On cache misses, a callback is called that should return the missing value.
+     * This callback is given a PSR-6 CacheItemInterface instance corresponding to the
+     * requested key, that could be used e.g. for expiration control. It could also
+     * be an ItemInterface instance when its additional features are needed.
+     *
+     * @param string                             $key      The key of the item to retrieve from the cache
+     * @param callable(CacheItemInterface):mixed $callback Should return the computed value for the given key/item
+     * @param float|null                         $beta     A float that, as it grows, controls the likeliness of triggering
+     *                                                     early expiration. 0 disables it, INF forces immediate expiration.
+     *                                                     The default (or providing null) is implementation dependent but should
+     *                                                     typically be 1.0, which should provide optimal stampede protection.
+     *                                                     See https://en.wikipedia.org/wiki/Cache_stampede#Probabilistic_early_expiration
      *
      * @return mixed The value corresponding to the provided key
      *
      * @throws InvalidArgumentException When $key is not valid or when $beta is negative
      */
     public function get(string $key, callable $callback, float $beta = null);
+
+    /**
+     * Removes an item from the pool.
+     *
+     * @param string $key The key to delete
+     *
+     * @throws InvalidArgumentException When $key is not valid
+     *
+     * @return bool True if the item was successfully removed, false if there was any error
+     */
+    public function delete(string $key): bool;
 }

Cache/CacheTrait.php

@@ -0,0 +1,67 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Contracts\Cache;
+
+use Psr\Cache\CacheItemPoolInterface;
+use Psr\Cache\InvalidArgumentException;
+
+/**
+ * An implementation of CacheInterface for PSR-6 CacheItemPoolInterface classes.
+ *
+ * @author Nicolas Grekas <[email protected]>
+ */
+trait CacheTrait
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function get(string $key, callable $callback, float $beta = null)
+    {
+        return $this->doGet($this, $key, $callback, $beta);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function delete(string $key): bool
+    {
+        return $this->deleteItem($key);
+    }
+
+    private function doGet(CacheItemPoolInterface $pool, string $key, callable $callback, ?float $beta)
+    {
+        if (0 > $beta = $beta ?? 1.0) {
+            throw new class(sprintf('Argument "$beta" provided to "%s::get()" must be a positive number, %f given.', \get_class($this), $beta)) extends \InvalidArgumentException implements InvalidArgumentException {
+            };
+        }
+
+        $item = $pool->getItem($key);
+        $recompute = !$item->isHit() || INF === $beta;
+
+        if (!$recompute && $item instanceof ItemInterface) {
+            $metadata = $item->getMetadata();
+            $expiry = $metadata[ItemInterface::METADATA_EXPIRY] ?? false;
+            $ctime = $metadata[ItemInterface::METADATA_CTIME] ?? false;
+
+            if ($recompute = $ctime && $expiry && $expiry <= microtime(true) - $ctime / 1000 * $beta * log(random_int(1, PHP_INT_MAX) / PHP_INT_MAX)) {
+                // force applying defaultLifetime to expiry
+                $item->expiresAt(null);
+            }
+        }
+
+        if ($recompute) {
+            $pool->save($item->set($callback($item)));
+        }
+
+        return $item->get();
+    }
+}

Cache/GetForCacheItemPoolTrait.php

@@ -16,6 +16,8 @@
 use Psr\Cache\InvalidArgumentException;
 
 /**
+ * Augments PSR-6's CacheItemInterface with support for tags and metadata.
+ *
  * @author Nicolas Grekas <[email protected]>
  */
 interface ItemInterface extends CacheItemInterface

Cache/ItemInterface.php

@@ -14,12 +14,19 @@
 use Psr\Cache\InvalidArgumentException;
 
 /**
- * Interface for invalidating cached items using tags.
+ * Allows invalidating cached items using tags.
  *
  * @author Nicolas Grekas <[email protected]>
  */
 interface TagAwareCacheInterface extends CacheInterface
 {
+    /**
+     * {@inheritdoc}
+     *
+     * @param callable(ItemInterface):mixed $callback Should return the computed value for the given key/item
+     */
+    public function get(string $key, callable $callback, float $beta = null);
+
     /**
      * Invalidates cached items using tags.
      *

Cache/TagAwareCacheInterface.php

@@ -13,24 +13,27 @@
 
 use PHPUnit\Framework\TestCase;
 use Psr\Cache\CacheItemInterface;
-use Symfony\Contracts\Cache\GetForCacheItemPoolTrait;
+use Psr\Cache\CacheItemPoolInterface;
+use Symfony\Contracts\Cache\CacheTrait;
 
 /**
  * @author Tobias Nyholm <[email protected]>
  */
-class GetForCacheItemPoolTraitTest extends TestCase
+class CacheTraitTest extends TestCase
 {
     public function testSave()
     {
         $item = $this->getMockBuilder(CacheItemInterface::class)->getMock();
+        $item->method('set')
+            ->willReturn($item);
         $item->method('isHit')
             ->willReturn(false);
 
         $item->expects($this->once())
             ->method('set')
             ->with('computed data');
 
-        $cache = $this->getMockBuilder(ClassUsingTrait::class)
+        $cache = $this->getMockBuilder(TestPool::class)
             ->setMethods(array('getItem', 'save'))
             ->getMock();
         $cache->expects($this->once())
@@ -56,7 +59,7 @@ public function testNoCallbackCallOnHit()
         $item->expects($this->never())
             ->method('set');
 
-        $cache = $this->getMockBuilder(ClassUsingTrait::class)
+        $cache = $this->getMockBuilder(TestPool::class)
             ->setMethods(array('getItem', 'save'))
             ->getMock();
 
@@ -77,6 +80,8 @@ public function testNoCallbackCallOnHit()
     public function testRecomputeOnBetaInf()
     {
         $item = $this->getMockBuilder(CacheItemInterface::class)->getMock();
+        $item->method('set')
+            ->willReturn($item);
         $item->method('isHit')
             // We want to recompute even if it is a hit
             ->willReturn(true);
@@ -85,7 +90,7 @@ public function testRecomputeOnBetaInf()
             ->method('set')
             ->with('computed data');
 
-        $cache = $this->getMockBuilder(ClassUsingTrait::class)
+        $cache = $this->getMockBuilder(TestPool::class)
             ->setMethods(array('getItem', 'save'))
             ->getMock();
 
@@ -105,7 +110,7 @@ public function testRecomputeOnBetaInf()
 
     public function testExceptionOnNegativeBeta()
     {
-        $cache = $this->getMockBuilder(ClassUsingTrait::class)
+        $cache = $this->getMockBuilder(TestPool::class)
             ->setMethods(array('getItem', 'save'))
             ->getMock();
 
@@ -118,15 +123,43 @@ public function testExceptionOnNegativeBeta()
     }
 }
 
-class ClassUsingTrait
+class TestPool implements CacheItemPoolInterface
 {
-    use GetForCacheItemPoolTrait;
+    use CacheTrait;
+
+    public function hasItem($key)
+    {
+    }
+
+    public function deleteItem($key)
+    {
+    }
+
+    public function deleteItems(array $keys = array())
+    {
+    }
 
     public function getItem($key)
     {
     }
 
+    public function getItems(array $key = array())
+    {
+    }
+
+    public function saveDeferred(CacheItemInterface $item)
+    {
+    }
+
     public function save(CacheItemInterface $item)
     {
     }
+
+    public function commit()
+    {
+    }
+
+    public function clear()
+    {
+    }
 }