Refactor getItems: Using at most one call to getMultiple()

ebln · Oct 15, 2021 · 5ace347 · 5ace347
1 parent 467d546
commit 5ace347
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,23 +7,25 @@ Intended to follow [«Keep a Changelog»](https://keepachangelog.com/en/)
 
 ## [Unreleased] (meant as staging area)
 
+### TODO
+
+- Refactor `NowFactory` to PSR-20 [BREAKING]
+- Revisit `\Brnc\CachePsr16Adapter\CacheItemPool::getTimeToLive`
+
+----
+
+## [1.1.0] - 2021-10-15
+
 ### Changed
 
+- Change `getItems` to use `getMultiple`
 - Using `NowFactory` also for `CacheItem`
 
 ### Added
 
 - Increase test coverage (using PSR-16 exceptions)
 - Add tests to ensure PSR-6 commits make use of PSR-16's bulk setters
 
-### TODO
-
-- Refactor `NowFactory` to PSR-20 [BREAKING]
-- Revisit `\Brnc\CachePsr16Adapter\CacheItemPool::getTimeToLive`
-- Change `getItems` to use `getMultiple`
-
-----
-
 ## [1.0.0] - 2021-10-10
 
 ### Added

diff --git a/src/CacheItemPool.php b/src/CacheItemPool.php
@@ -41,6 +41,8 @@ public function getItem($key): CacheItem
     }
 
     /**
+     * NOTE: Returned key order is not preserved from the argument!
+     *
      * @psalm-param array<mixed> $keys
      *
      * @return array<string, CacheItem>
@@ -60,15 +62,19 @@ public function getItems(array $keys = []): array
                     $downstreamKeys[] = $key;
                 }
             }
-            foreach ($downstreamKeys as $key) {
-                /** @var ?SerializedItem $rawItem */
-                $rawItem = $this->cache->get($key);
-                if ($rawItem instanceof SerializedItem) {
-                    $item = $this->unserializeItem($key, $rawItem);
-                } else {
-                    $item = new CacheItem($key, null, false, null, $this->nowFactory);
+
+            if ($downstreamKeys) {
+                /**
+                 * @var string          $key
+                 * @var ?SerializedItem $rawItem
+                 */
+                foreach ($this->cache->getMultiple($downstreamKeys) as $key => $rawItem) {
+                    if ($rawItem instanceof SerializedItem) {
+                        $items[$key] = $this->unserializeItem($key, $rawItem);
+                    } else {
+                        $items[$key] = new CacheItem($key, null, false, null, $this->nowFactory);
+                    }
                 }
-                $items[$key] = $item;
             }
         } catch (InvalidArgumentException $k) {
             throw $k;

diff --git a/tests/TransactionTest.php b/tests/TransactionTest.php
@@ -18,6 +18,56 @@
  */
 final class TransactionTest extends TestCase
 {
+    /**
+     * Ensures underlying PSR-16's `getMultiple()` is called not more than once per `getItems()`-call
+     */
+    public function testGetItems(): void
+    {
+        $psr16 = $this->createMock(CacheInterface::class);
+        $psr16->expects(static::once())
+            ->method('getMultiple')
+            ->with(['foo', 'bar', 'far'])->willReturn(
+                [
+                    'foo' => null,
+                    'bar' => ['bogus'],
+                    'far' => new SerializedItem(new DateTimeImmutable('2045-12-12'), ['hello', 'world']),
+                ]
+            );
+
+        $deferedItem = new CacheItem('baz', 'item 1', true, null, $this->getClock());
+        $pool        = new CacheItemPool($psr16);
+        $pool->saveDeferred($deferedItem);
+
+        static::assertEquals(
+            [
+                'baz' => $deferedItem,
+                'foo' => new CacheItem('foo', null, false, null, $this->getClock()),
+                'bar' => new CacheItem('bar', null, false, null, $this->getClock()),
+                'far' => new CacheItem('far', ['hello', 'world'], true, new DateTimeImmutable('2045-12-12'), $this->getClock()),
+            ],
+            $pool->getItems(['foo', 'bar', 'baz', 'far'])
+        );
+    }
+
+    /**
+     * Ensures underlying PSR-16's `getMultiple()` is never called
+     */
+    public function testGetItemsAllDefered(): void
+    {
+        $psr16 = $this->createMock(CacheInterface::class);
+        $psr16->expects(static::never())->method('getMultiple');
+
+        $item1 = new CacheItem('foo', 'item 1', true, null, $this->getClock());
+        $item2 = new CacheItem('bar', 'item 2', true, null, $this->getClock());
+        $pool  = new CacheItemPool($psr16);
+        $pool->saveDeferred($item1);
+        $pool->saveDeferred($item2);
+
+        $gotItems = $pool->getItems(['foo', 'bar']);
+
+        static::assertEquals(['foo' => $item1, 'bar' => $item2], $gotItems);
+    }
+
     /**
      * Ensures underlying PSR-16's `deleteMultiple()` is only called once per `deleteItems()`-call
      */