[Cache] serialize tags separately from values in AbstractTagAwareAdapter

symfony · Oct 10, 2019 · a60d466 · a60d466
1 parent 6e7f325
commit a60d466
Show file tree

Hide file tree

Showing 6 changed files with 124 additions and 13 deletions.
diff --git a/src/Symfony/Component/Cache/Adapter/AbstractTagAwareAdapter.php b/src/Symfony/Component/Cache/Adapter/AbstractTagAwareAdapter.php
@@ -14,6 +14,7 @@
 use Psr\Log\LoggerAwareInterface;
 use Symfony\Component\Cache\CacheItem;
 use Symfony\Component\Cache\Exception\InvalidArgumentException;
+use Symfony\Component\Cache\Marshaller\MarshallerInterface;
 use Symfony\Component\Cache\ResettableInterface;
 use Symfony\Component\Cache\Traits\AbstractAdapterTrait;
 use Symfony\Component\Cache\Traits\ContractsTrait;
@@ -37,14 +38,17 @@ abstract class AbstractTagAwareAdapter implements TagAwareAdapterInterface, TagA
 
     private const TAGS_PREFIX = "\0tags\0";
 
-    protected function __construct(string $namespace = '', int $defaultLifetime = 0)
+    private $marshaller;
+
+    protected function __construct(string $namespace = '', int $defaultLifetime = 0, MarshallerInterface $marshaller = null)
     {
+        $this->marshaller = $marshaller;
         $this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace).':';
         if (null !== $this->maxIdLength && \strlen($namespace) > $this->maxIdLength - 24) {
             throw new InvalidArgumentException(sprintf('Namespace must be %d chars max, %d given ("%s")', $this->maxIdLength - 24, \strlen($namespace), $namespace));
         }
         $this->createCacheItem = \Closure::bind(
-            static function ($key, $value, $isHit) use ($defaultLifetime) {
+            static function ($key, $value, $isHit) use ($defaultLifetime, $marshaller) {
                 $item = new CacheItem();
                 $item->key = $key;
                 $item->defaultLifetime = $defaultLifetime;
@@ -53,6 +57,10 @@ static function ($key, $value, $isHit) use ($defaultLifetime) {
                 if (!\is_array($value) || !\array_key_exists('value', $value)) {
                     return $item;
                 }
+                if ($marshaller && \is_string($value['tags'] ?? null)) {
+                    $value['value'] = $marshaller->unmarshall($value['value']);
+                    $value['tags'] = '' === $value['tags'] ? [] : $marshaller->unmarshall($value['tags']);
+                }
                 $item->isHit = $isHit;
                 // Extract value, tags and meta data from the cache value
                 $item->value = $value['value'];
@@ -72,7 +80,7 @@ static function ($key, $value, $isHit) use ($defaultLifetime) {
         $getId = \Closure::fromCallable([$this, 'getId']);
         $tagPrefix = self::TAGS_PREFIX;
         $this->mergeByLifetime = \Closure::bind(
-            static function ($deferred, &$expiredIds) use ($getId, $tagPrefix) {
+            static function ($deferred, &$expiredIds) use ($getId, $tagPrefix, $marshaller) {
                 $byLifetime = [];
                 $now = microtime(true);
                 $expiredIds = [];
@@ -93,6 +101,20 @@ static function ($deferred, &$expiredIds) use ($getId, $tagPrefix) {
                         $value = ['value' => $item->value, 'tags' => []];
                     }
 
+                    $newTags = $value['tags'];
+
+                    if ($marshaller) {
+                        $value = $marshaller->marshall($value, $failed);
+
+                        if ([] === $newTags) {
+                            $value['tags'] = '';
+                        }
+
+                        if ($failed) {
+                            $value['fail'] = $item->value;
+                        }
+                    }
+
                     if ($metadata) {
                         // For compactness, expiry and creation duration are packed, using magic numbers as separators
                         $value['meta'] = pack('VN', (int) (0.1 + $metadata[self::METADATA_EXPIRY] - self::METADATA_EXPIRY_OFFSET), $metadata[self::METADATA_CTIME]);
@@ -101,10 +123,10 @@ static function ($deferred, &$expiredIds) use ($getId, $tagPrefix) {
                     // Extract tag changes, these should be removed from values in doSave()
                     $value['tag-operations'] = ['add' => [], 'remove' => []];
                     $oldTags = $item->metadata[CacheItem::METADATA_TAGS] ?? [];
-                    foreach (array_diff($value['tags'], $oldTags) as $addedTag) {
+                    foreach (array_diff($newTags, $oldTags) as $addedTag) {
                         $value['tag-operations']['add'][] = $getId($tagPrefix.$addedTag);
                     }
-                    foreach (array_diff($oldTags, $value['tags']) as $removedTag) {
+                    foreach (array_diff($oldTags, $newTags) as $removedTag) {
                         $value['tag-operations']['remove'][] = $getId($tagPrefix.$removedTag);
                     }
 
@@ -149,6 +171,17 @@ abstract protected function doDelete(array $ids, array $tagData = []): bool;
      */
     abstract protected function doInvalidate(array $tagIds): bool;
 
+    protected function doFetchTags(array $ids): iterable
+    {
+        foreach ($this->doFetch($ids) as $id => $value) {
+            if ($this->marshaller && \is_string($value['tags'] ?? null)) {
+                $value['tags'] = '' === $value['tags'] ? [] : $this->marshaller->unmarshall($value['tags']);
+            }
+
+            yield $id => $value['tags'] ?? [];
+        }
+    }
+
     /**
      * {@inheritdoc}
      *
@@ -233,8 +266,8 @@ public function deleteItems(array $keys)
         }
 
         try {
-            foreach ($this->doFetch($ids) as $id => $value) {
-                foreach ($value['tags'] ?? [] as $tag) {
+            foreach ($this->doFetchTags($ids) as $id => $tags) {
+                foreach ($tags as $tag) {
                     $tagData[$this->getId(self::TAGS_PREFIX.$tag)][] = $id;
                 }
             }

diff --git a/src/Symfony/Component/Cache/Adapter/FilesystemTagAwareAdapter.php b/src/Symfony/Component/Cache/Adapter/FilesystemTagAwareAdapter.php
@@ -13,6 +13,7 @@
 
 use Symfony\Component\Cache\Marshaller\DefaultMarshaller;
 use Symfony\Component\Cache\Marshaller\MarshallerInterface;
+use Symfony\Component\Cache\Marshaller\TagAwareMarshaller;
 use Symfony\Component\Cache\PruneableInterface;
 use Symfony\Component\Cache\Traits\FilesystemTrait;
 
@@ -37,8 +38,8 @@ class FilesystemTagAwareAdapter extends AbstractTagAwareAdapter implements Prune
 
     public function __construct(string $namespace = '', int $defaultLifetime = 0, string $directory = null, MarshallerInterface $marshaller = null)
     {
-        $this->marshaller = $marshaller ?? new DefaultMarshaller();
-        parent::__construct('', $defaultLifetime);
+        $this->marshaller = new TagAwareMarshaller(new DefaultMarshaller());
+        parent::__construct('', $defaultLifetime, $marshaller ?? new DefaultMarshaller());
         $this->init($namespace, $directory);
     }
 

diff --git a/src/Symfony/Component/Cache/Adapter/RedisTagAwareAdapter.php b/src/Symfony/Component/Cache/Adapter/RedisTagAwareAdapter.php
@@ -15,7 +15,9 @@
 use Predis\Connection\Aggregate\PredisCluster;
 use Predis\Response\Status;
 use Symfony\Component\Cache\Exception\InvalidArgumentException;
+use Symfony\Component\Cache\Marshaller\DefaultMarshaller;
 use Symfony\Component\Cache\Marshaller\MarshallerInterface;
+use Symfony\Component\Cache\Marshaller\TagAwareMarshaller;
 use Symfony\Component\Cache\Traits\RedisTrait;
 
 /**
@@ -67,7 +69,9 @@ public function __construct($redisClient, string $namespace = '', int $defaultLi
             throw new InvalidArgumentException(sprintf('Unsupported Predis cluster connection: only "%s" is, "%s" given.', PredisCluster::class, \get_class($redisClient->getConnection())));
         }
 
-        $this->init($redisClient, $namespace, $defaultLifetime, $marshaller);
+        parent::__construct($namespace, $defaultLifetime, $marshaller ?? new DefaultMarshaller());
+
+        $this->init($redisClient, $namespace, $defaultLifetime, new TagAwareMarshaller(new DefaultMarshaller()), false);
     }
 
     /**

diff --git a/src/Symfony/Component/Cache/Marshaller/TagAwareMarshaller.php b/src/Symfony/Component/Cache/Marshaller/TagAwareMarshaller.php
@@ -0,0 +1,69 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Cache\Marshaller;
+
+/**
+ * A marshaller optimized for data structures generated by AbstractTagAwareAdapter.
+ *
+ * @author Nicolas Grekas <p@tchwork.com>
+ *
+ * @internal
+ */
+class TagAwareMarshaller implements MarshallerInterface
+{
+    private $marshaller = true;
+
+    public function __construct(MarshallerInterface $marshaller)
+    {
+        $this->marshaller = $marshaller;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function marshall(array $values, ?array &$failed): array
+    {
+        $failed = $notSerialized = $serialized = [];
+
+        foreach ($values as $id => $value) {
+            if (\is_array($value) && \is_string($value['tags'] ?? null) && \is_string($value['value'] ?? null) && \count($value) === 2 + (\is_string($value['meta'] ?? null) && 8 === \strlen($value['meta']))) {
+                $serialized[$id] = "\x9D".($value['meta'] ?? "\0\0\0\0\0\0\0\0").pack('N', \strlen($value['tags'])).$value['tags'].$value['value'];
+                $serialized[$id][9] = "\x5F";
+            } else {
+                $notSerialized[$id] = $value;
+            }
+        }
+
+        return $notSerialized ? $serialized + $this->marshaller->marshall($notSerialized, $failed) : $serialized;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function unmarshall(string $value)
+    {
+        if (13 >= \strlen($value) || "\x9D" !== $value[0] || "\0" !== $value[5] || "\x5F" !== $value[9]) {
+            return $this->marshaller->unmarshall($value);
+        }
+
+        $meta = substr($value, 1, 12);
+        $meta[8] = "\0";
+        $tagLen = unpack('Nlen', $meta, 8)['len'];
+        $meta = substr($meta, 0, 8);
+
+        return [
+            'value' => substr($value, 13 + $tagLen),
+            'tags' => substr($value, 13, $tagLen),
+            'meta' => "\0\0\0\0\0\0\0\0" === $meta ? null : $meta,
+        ];
+    }
+}
diff --git a/src/Symfony/Component/Cache/Traits/AbstractAdapterTrait.php b/src/Symfony/Component/Cache/Traits/AbstractAdapterTrait.php
@@ -51,11 +51,13 @@ public function getItem($key)
             foreach ($this->doFetch([$id]) as $value) {
                 $isHit = true;
             }
+
+            return $f($key, $value, $isHit);
         } catch (\Exception $e) {
             CacheItem::log($this->logger, 'Failed to fetch key "{key}": '.$e->getMessage(), ['key' => $key, 'exception' => $e]);
         }
 
-        return $f($key, $value, $isHit);
+        return $f($key, null, false);
     }
 
     /**

diff --git a/src/Symfony/Component/Cache/Traits/RedisTrait.php b/src/Symfony/Component/Cache/Traits/RedisTrait.php
@@ -48,9 +48,11 @@ trait RedisTrait
     /**
      * @param \Redis|\RedisArray|\RedisCluster|\Predis\ClientInterface $redisClient
      */
-    private function init($redisClient, string $namespace, int $defaultLifetime, ?MarshallerInterface $marshaller)
+    private function init($redisClient, string $namespace, int $defaultLifetime, ?MarshallerInterface $marshaller, bool $callConstructor = true)
     {
-        parent::__construct($namespace, $defaultLifetime);
+        if ($callConstructor) {
+            parent::__construct($namespace, $defaultLifetime);
+        }
 
         if (preg_match('#[^-+_.A-Za-z0-9]#', $namespace, $match)) {
             throw new InvalidArgumentException(sprintf('RedisAdapter namespace contains "%s" but only characters in [-+_.A-Za-z0-9] are allowed.', $match[0]));