From bdfba77c363b60414f9a8e80cf13b5eed7bda49c Mon Sep 17 00:00:00 2001 From: ebln <34722048+ebln@users.noreply.github.com> Date: Thu, 14 Oct 2021 22:26:53 +0200 Subject: [PATCH 01/13] Remove too-risky rule for CS-Fixer --- .php-cs-fixer.dist.php | 1 + 1 file changed, 1 insertion(+) diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 7c721c9..6ac490c 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -29,6 +29,7 @@ 'phpdoc_summary' => false, 'cast_spaces' => ['space' => 'none'], 'binary_operator_spaces' => ['default' => null, 'operators' => ['=' => 'align_single_space_minimal', '=>' => 'align_single_space_minimal']], + 'php_unit_strict' => false, ] ) ->setFinder($finder); From 637a115a1dd34eacbbca476bdf63d6969ce75159 Mon Sep 17 00:00:00 2001 From: ebln <34722048+ebln@users.noreply.github.com> Date: Thu, 14 Oct 2021 22:27:50 +0200 Subject: [PATCH 02/13] Fix makefile and add coverage & stylefix --- makefile | 14 ++++++++---- tests/CacheItemTest.php | 33 ++++++++++++++++++++++++++++ tests/helper/FalsePsr16Mock.php | 38 +++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 tests/CacheItemTest.php create mode 100644 tests/helper/FalsePsr16Mock.php diff --git a/makefile b/makefile index 83f78e8..ed5b19c 100644 --- a/makefile +++ b/makefile @@ -5,7 +5,7 @@ DOCKER_COMPOSE=docker-compose -f $(DOCKER_COMPOSE_YML) MAKE=make -s .DEFAULT_GOAL := help -.PHONY: help build rm enter test test-all +.PHONY: help build rm enter test quality style-fix coverage help: ## Show this help. @grep -E '^[a-zA-Z_-]+:.*?##\s*.*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?##\\s*"}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' @@ -26,7 +26,13 @@ enter:##Log into the main container $(DOCKER_COMPOSE) run ${DEFAULT_CONTAINER} /bin/bash test:##Run unit tests - $(DOCKER_COMPOSE) run ${DEFAULT_CONTAINER} composer phpunit + $(DOCKER_COMPOSE) run ${DEFAULT_CONTAINER} composer test-unit -test-all:##Run the complete code quality suite - $(DOCKER_COMPOSE) run ${DEFAULT_CONTAINER} composer test +quality:##Run the complete code quality suite + $(DOCKER_COMPOSE) run ${DEFAULT_CONTAINER} composer quality + +style-fix:##Apply code style + $(DOCKER_COMPOSE) run ${DEFAULT_CONTAINER} composer style-fix + +coverage:##Generate coverage report + $(DOCKER_COMPOSE) run ${DEFAULT_CONTAINER} composer coverage diff --git a/tests/CacheItemTest.php b/tests/CacheItemTest.php new file mode 100644 index 0000000..8fa5e92 --- /dev/null +++ b/tests/CacheItemTest.php @@ -0,0 +1,33 @@ +expiresAt($dateTime); + + static::assertEquals($dateTime, $item->getExpiry()); + static::assertNotSame($dateTime, $item->getExpiry()); + } + + public function testExpiresAtWithDateTimeImmutable(): void + { + $item = new CacheItem('foo', null, false, null); + $dateTime = new \DateTimeImmutable(); + $item->expiresAt($dateTime); + static::assertSame($dateTime, $item->getExpiry()); + } +} diff --git a/tests/helper/FalsePsr16Mock.php b/tests/helper/FalsePsr16Mock.php new file mode 100644 index 0000000..16305cf --- /dev/null +++ b/tests/helper/FalsePsr16Mock.php @@ -0,0 +1,38 @@ + Date: Thu, 14 Oct 2021 23:10:15 +0200 Subject: [PATCH 03/13] Use NowFactory in CacheItem & add tests --- CHANGELOG.md | 5 ++- src/CacheItemPool.php | 6 +-- src/Model/CacheItem.php | 9 +++-- tests/CacheItemTest.php | 76 ++++++++++++++++++++++++++++++++--- tests/Psr6IntegrationTest.php | 1 + 5 files changed, 84 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab2dd53..b9efb7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,12 @@ Intended to follow [«Keep a Changelog»](https://keepachangelog.com/en/) ## [Unreleased] (meant as staging area) +- Using `NowFactory` also for `CacheItem` + ### TODO -- Refactor `NowFactory` to PSR-20 [BREAKING] +- Refactor `NowFactory` to PSR-20 [BREAKING] - Revisit `\Brnc\CachePsr16Adapter\CacheItemPool::getTimeToLive` -- Also `NowFactory` also for `CacheItem` - Add tests to ensure PSR-6 commits make use of PSR-16's bulk setters - Increase test coverage (using PSR-16 returning false & exceptions) diff --git a/src/CacheItemPool.php b/src/CacheItemPool.php index 03cac7a..f21642d 100644 --- a/src/CacheItemPool.php +++ b/src/CacheItemPool.php @@ -54,14 +54,14 @@ public function getItems(array $keys = []): array $this->validateKey($key); if (isset($this->defered[$key])) { - $item = new CacheItem($key, $this->defered[$key]->getValue(), true, $this->defered[$key]->getExpiresAt()); + $item = new CacheItem($key, $this->defered[$key]->getValue(), true, $this->defered[$key]->getExpiresAt(), $this->nowFactory); } else { /** @var ?SerializedItem $rawItem */ $rawItem = $this->cache->get($key); if ($rawItem instanceof SerializedItem) { - $item = new CacheItem($key, $rawItem->getValue(), true, $rawItem->getExpiresAt()); + $item = new CacheItem($key, $rawItem->getValue(), true, $rawItem->getExpiresAt(), $this->nowFactory); } else { - $item = new CacheItem($key, null, false, null); + $item = new CacheItem($key, null, false, null, $this->nowFactory); } } diff --git a/src/Model/CacheItem.php b/src/Model/CacheItem.php index b3b210e..81ddb83 100644 --- a/src/Model/CacheItem.php +++ b/src/Model/CacheItem.php @@ -5,6 +5,7 @@ namespace Brnc\CachePsr16Adapter\Model; use Brnc\CachePsr16Adapter\Exception\InvalidArgumentException; +use Brnc\CachePsr16Adapter\NowFactory; use DateInterval; use DateTimeImmutable; use Psr\Cache\CacheItemInterface; @@ -16,16 +17,18 @@ class CacheItem implements CacheItemInterface private string $key; private bool $hit; private ?DateTimeImmutable $expiry; + private NowFactory $clock; /** * @psalm-param mixed $value */ - public function __construct(string $key, $value, bool $hit, ?DateTimeImmutable $expiry) + public function __construct(string $key, $value, bool $hit, ?DateTimeImmutable $expiry, NowFactory $clock) { $this->key = $key; $this->value = $value; $this->hit = $hit; $this->expiry = $expiry; + $this->clock = $clock; } public function getKey(): string @@ -90,9 +93,9 @@ public function expiresAfter($time) if (null === $time) { $this->setExpiry(null); } elseif ($time instanceof DateInterval) { - $this->setExpiry((new DateTimeImmutable())->add($time)); + $this->setExpiry($this->clock->now()->add($time)); } elseif (is_int($time)) { - $this->setExpiry((new DateTimeImmutable())->add(new DateInterval('PT' . $time . 'S'))); + $this->setExpiry($this->clock->now()->add(new DateInterval('PT' . $time . 'S'))); } else { throw new InvalidArgumentException(); } diff --git a/tests/CacheItemTest.php b/tests/CacheItemTest.php index 8fa5e92..8346f45 100644 --- a/tests/CacheItemTest.php +++ b/tests/CacheItemTest.php @@ -4,19 +4,24 @@ namespace Brnc\tests\CachePsr16Adapter; +use Brnc\CachePsr16Adapter\Exception\InvalidArgumentException; use Brnc\CachePsr16Adapter\Model\CacheItem; +use Brnc\CachePsr16Adapter\NowFactory; +use DateInterval; +use DateTime; +use DateTimeImmutable; use PHPUnit\Framework\TestCase; /** * @internal - * @coversNothing + * @covers \Brnc\CachePsr16Adapter\Model\CacheItem */ final class CacheItemTest extends TestCase { public function testExpiresAtWithDateTime(): void { - $item = new CacheItem('foo', null, false, null); - $dateTime = new \DateTime(); + $item = new CacheItem('foo', null, false, null, $this->getClock()); + $dateTime = new DateTime(); $item->expiresAt($dateTime); static::assertEquals($dateTime, $item->getExpiry()); @@ -25,9 +30,70 @@ public function testExpiresAtWithDateTime(): void public function testExpiresAtWithDateTimeImmutable(): void { - $item = new CacheItem('foo', null, false, null); - $dateTime = new \DateTimeImmutable(); + $item = new CacheItem('foo', null, false, null, $this->getClock()); + $dateTime = new DateTimeImmutable(); $item->expiresAt($dateTime); static::assertSame($dateTime, $item->getExpiry()); } + + public function testExpiresAtWithBogus(): void + { + $this->expectException(InvalidArgumentException::class); + $implementing = class_implements(InvalidArgumentException::class); + $implementing = $implementing ?: []; + static::assertArrayHasKey(\Psr\Cache\InvalidArgumentException::class, $implementing); + $item = new CacheItem('foo', null, false, null, $this->getClock()); + $item->expiresAt((object)['foo']); + } + + public function testExpiresAtWithNull(): void + { + $item = new CacheItem('foo', null, false, null, $this->getClock()); + $item->expiresAt(null); + static::assertNull($item->getExpiry()); + } + + public function testExpiresAfterWithNull(): void + { + $item = new CacheItem('foo', null, false, null, $this->getClock()); + $item->expiresAfter(null); + static::assertNull($item->getExpiry()); + } + + public function testExpiresAfterWithBogus(): void + { + $this->expectException(InvalidArgumentException::class); + $item = new CacheItem('foo', null, false, null, $this->getClock()); + $item->expiresAfter('foobar'); + } + + public function testExpiresAfterWithInt(): void + { + $item = new CacheItem('foo', null, false, null, $this->getFixedClock()); + $item->expiresAfter(60); + static::assertEquals($this->getFixedClock()->now()->add(new DateInterval('PT60S')), $item->getExpiry()); + } + + public function testExpiresAfterWithDateInterval(): void + { + $item = new CacheItem('foo', null, false, null, $this->getFixedClock()); + $interval = new DateInterval('PT60S'); + $item->expiresAfter($interval); + static::assertEquals($this->getFixedClock()->now()->add(new DateInterval('PT60S')), $item->getExpiry()); + } + + private function getClock(): NowFactory + { + return new NowFactory(); + } + + private function getFixedClock(): NowFactory + { + $stub = $this->createStub(NowFactory::class); + $stub->method('now') + ->willReturn(new DateTimeImmutable('2037-12-31')) + ; + + return $stub; + } } diff --git a/tests/Psr6IntegrationTest.php b/tests/Psr6IntegrationTest.php index f779f9f..d1465a8 100644 --- a/tests/Psr6IntegrationTest.php +++ b/tests/Psr6IntegrationTest.php @@ -13,6 +13,7 @@ * @covers \Brnc\CachePsr16Adapter\CacheItemPool * @covers \Brnc\CachePsr16Adapter\Model\CacheItem * @covers \Brnc\CachePsr16Adapter\Model\SerializedItem + * @covers \Brnc\CachePsr16Adapter\NowFactory * @covers \Brnc\tests\CachePsr16Adapter\helper\Psr16ArraySingleton * @codeCoverageIgnore */ From c2dfc82d6594f26cbab36f994369db5071b7b800 Mon Sep 17 00:00:00 2001 From: ebln <34722048+ebln@users.noreply.github.com> Date: Thu, 14 Oct 2021 23:33:48 +0200 Subject: [PATCH 04/13] Add todo --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9efb7a..76e67fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Intended to follow [«Keep a Changelog»](https://keepachangelog.com/en/) - Refactor `NowFactory` to PSR-20 [BREAKING] - Revisit `\Brnc\CachePsr16Adapter\CacheItemPool::getTimeToLive` +- Change `getItems` to use `getMultiple` - Add tests to ensure PSR-6 commits make use of PSR-16's bulk setters - Increase test coverage (using PSR-16 returning false & exceptions) From 4a2b1524e4ba92bc43089c009a3ae266aafeb664 Mon Sep 17 00:00:00 2001 From: ebln <34722048+ebln@users.noreply.github.com> Date: Fri, 15 Oct 2021 00:06:53 +0200 Subject: [PATCH 05/13] Fix makefile --- makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/makefile b/makefile index ed5b19c..acc8a05 100644 --- a/makefile +++ b/makefile @@ -5,7 +5,7 @@ DOCKER_COMPOSE=docker-compose -f $(DOCKER_COMPOSE_YML) MAKE=make -s .DEFAULT_GOAL := help -.PHONY: help build rm enter test quality style-fix coverage +.PHONY: help build rm down stop enter test quality style-fix coverage help: ## Show this help. @grep -E '^[a-zA-Z_-]+:.*?##\s*.*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?##\\s*"}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' From 0d18bf147473b5eb49564832767b36427c1acbba Mon Sep 17 00:00:00 2001 From: ebln <34722048+ebln@users.noreply.github.com> Date: Fri, 15 Oct 2021 00:07:12 +0200 Subject: [PATCH 06/13] Add BrokenPsr16Tests --- tests/BrokenPsr16Test.php | 65 +++++++++++++++++++++++++++++++ tests/helper/BrokenPsr16Mock.php | 66 ++++++++++++++++++++++++++++++++ tests/helper/FalsePsr16Mock.php | 38 ------------------ 3 files changed, 131 insertions(+), 38 deletions(-) create mode 100644 tests/BrokenPsr16Test.php create mode 100644 tests/helper/BrokenPsr16Mock.php delete mode 100644 tests/helper/FalsePsr16Mock.php diff --git a/tests/BrokenPsr16Test.php b/tests/BrokenPsr16Test.php new file mode 100644 index 0000000..bbf7942 --- /dev/null +++ b/tests/BrokenPsr16Test.php @@ -0,0 +1,65 @@ +createCache(); + static::assertFalse($cache->hasItem('foo')); + } + + public function testClear(): void + { + $cache = $this->createCache(); + static::assertFalse($cache->clear()); + } + + public function testCommit(): void + { + $cache = $this->createCache(); + $cache->saveDeferred($this->createItem()); + static::assertFalse($cache->commit()); + } + + public function testSave(): void + { + $cache = $this->createCache(); + static::assertFalse($cache->save($this->createItem())); + } + + public function testDeleteItem(): void + { + $cache = $this->createCache(); + static::assertFalse($cache->deleteItem('foo')); + } + + public function testDeleteItems(): void + { + $cache = $this->createCache(); + static::assertFalse($cache->deleteItems(['foo'])); + } + + private function createItem(): CacheItem + { + return new CacheItem('foo_' . bin2hex(random_bytes(2)), bin2hex(random_bytes(4)), true, null, new NowFactory()); + } + + private function createCache(): CacheItemPool + { + return new CacheItemPool(new BrokenPsr16Mock()); + } +} diff --git a/tests/helper/BrokenPsr16Mock.php b/tests/helper/BrokenPsr16Mock.php new file mode 100644 index 0000000..ba634c3 --- /dev/null +++ b/tests/helper/BrokenPsr16Mock.php @@ -0,0 +1,66 @@ + Date: Fri, 15 Oct 2021 00:14:19 +0200 Subject: [PATCH 07/13] Fix coverage --- tests/helper/Psr16Test.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/helper/Psr16Test.php b/tests/helper/Psr16Test.php index f2d945b..192d2eb 100644 --- a/tests/helper/Psr16Test.php +++ b/tests/helper/Psr16Test.php @@ -8,7 +8,8 @@ /** * @internal - * @covers \Brnc\tests\CachePsr16Adapter\helper\Psr16Array@covers \Brnc\tests\CachePsr16Adapter\helper\Psr16ArraySingleton + * @covers \Brnc\tests\CachePsr16Adapter\helper\Psr16Array + * @covers \Brnc\tests\CachePsr16Adapter\helper\Psr16ArraySingleton * @codeCoverageIgnore */ final class Psr16Test extends SimpleCacheTest From 49f23919918b9316924b8dfbae891cde36772838 Mon Sep 17 00:00:00 2001 From: ebln <34722048+ebln@users.noreply.github.com> Date: Fri, 15 Oct 2021 14:13:45 +0200 Subject: [PATCH 08/13] Increase Psr16Array coverage --- tests/helper/Psr16Array.php | 2 ++ tests/helper/Psr16Test.php | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/tests/helper/Psr16Array.php b/tests/helper/Psr16Array.php index 67cac3d..a84b2b5 100644 --- a/tests/helper/Psr16Array.php +++ b/tests/helper/Psr16Array.php @@ -85,7 +85,9 @@ public function deleteMultiple($keys) } foreach ($keys as $key) { if (!$this->delete($key)) { + // @codeCoverageIgnoreStart return false; + // @codeCoverageIgnoreEnd } } diff --git a/tests/helper/Psr16Test.php b/tests/helper/Psr16Test.php index 192d2eb..8f46395 100644 --- a/tests/helper/Psr16Test.php +++ b/tests/helper/Psr16Test.php @@ -18,4 +18,11 @@ public function createSimpleCache(): Psr16Array { return new Psr16Array(); } + + /* just for coverage, TTL cannot be read back and Psr16Array has no injected clock */ + public function testOverlyLargeTtl(): void + { + $cache = $this->createSimpleCache(); + static::assertTrue($cache->set('foo', 'bar', PHP_INT_MAX - (int)(time() / 2))); + } } From b44edc8522ae196e954672a0a945c76bc4d3f450 Mon Sep 17 00:00:00 2001 From: ebln <34722048+ebln@users.noreply.github.com> Date: Fri, 15 Oct 2021 14:14:45 +0200 Subject: [PATCH 09/13] Increase CacheItemPool coverage --- CHANGELOG.md | 9 +++++-- tests/CacheItemPoolTest.php | 51 +++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 tests/CacheItemPoolTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 76e67fa..135e8a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,15 +7,20 @@ Intended to follow [«Keep a Changelog»](https://keepachangelog.com/en/) ## [Unreleased] (meant as staging area) +### Changed + - Using `NowFactory` also for `CacheItem` +### Added + +- Increase test coverage (using PSR-16 exceptions) + ### TODO - Refactor `NowFactory` to PSR-20 [BREAKING] - Revisit `\Brnc\CachePsr16Adapter\CacheItemPool::getTimeToLive` -- Change `getItems` to use `getMultiple` +- Change `getItems` to use `getMultiple` - Add tests to ensure PSR-6 commits make use of PSR-16's bulk setters -- Increase test coverage (using PSR-16 returning false & exceptions) ---- diff --git a/tests/CacheItemPoolTest.php b/tests/CacheItemPoolTest.php new file mode 100644 index 0000000..300800e --- /dev/null +++ b/tests/CacheItemPoolTest.php @@ -0,0 +1,51 @@ +createStub(CacheItemInterface::class); + $this->expectException(InvalidArgumentException::class); + // ensure that exception is implementing the right interface for the PSR-6 + // phpstan is quite strict: expects class-string yet PSR-6 doesn't implement throwable yet + $implementing = class_implements(InvalidArgumentException::class); + $implementing = $implementing ?: []; + static::assertArrayHasKey(\Psr\Cache\InvalidArgumentException::class, $implementing); + $cache->save($item); + } + + public function testGetItemsWithFatalError(): void + { + $pool = new CacheItemPool(new BrokenPsr16Mock()); + static::assertSame([], $pool->getItems(['foo', 'bar'])); + } + + public function testFailingCommit(): void + { + $psr16 = $this->createStub(CacheInterface::class); + $psr16->method('setMultiple')->willReturn(false); + $pool = new CacheItemPool($psr16); + $item = new CacheItem('foo', null, false, null, new NowFactory()); + $pool->saveDeferred($item); + static::assertFalse($pool->commit()); + } +} From 5166ba70649a4c4326210164f3f3c726f92619e0 Mon Sep 17 00:00:00 2001 From: ebln <34722048+ebln@users.noreply.github.com> Date: Fri, 15 Oct 2021 15:10:17 +0200 Subject: [PATCH 10/13] Add note for a getItems quirk --- src/CacheItemPool.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/CacheItemPool.php b/src/CacheItemPool.php index f21642d..bbd2fac 100644 --- a/src/CacheItemPool.php +++ b/src/CacheItemPool.php @@ -54,6 +54,7 @@ public function getItems(array $keys = []): array $this->validateKey($key); if (isset($this->defered[$key])) { + // NOTE In constrast to hasItem() this apparently always returns the defered item, even expired ones $item = new CacheItem($key, $this->defered[$key]->getValue(), true, $this->defered[$key]->getExpiresAt(), $this->nowFactory); } else { /** @var ?SerializedItem $rawItem */ From fc824b3ea51ff3ab28a7d54d3c347e5ac6e6e69c Mon Sep 17 00:00:00 2001 From: ebln <34722048+ebln@users.noreply.github.com> Date: Fri, 15 Oct 2021 15:10:31 +0200 Subject: [PATCH 11/13] Add transactional tests --- CHANGELOG.md | 2 +- tests/TransactionTest.php | 74 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 tests/TransactionTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 135e8a4..c836dab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,13 +14,13 @@ Intended to follow [«Keep a Changelog»](https://keepachangelog.com/en/) ### 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` -- Add tests to ensure PSR-6 commits make use of PSR-16's bulk setters ---- diff --git a/tests/TransactionTest.php b/tests/TransactionTest.php new file mode 100644 index 0000000..24626d1 --- /dev/null +++ b/tests/TransactionTest.php @@ -0,0 +1,74 @@ +createMock(CacheInterface::class); + $psr16->expects(static::once()) + ->method('deleteMultiple') + ->with(['foo', 'bar'])->willReturn(true); + + $pool = new CacheItemPool($psr16); + static::assertTrue($pool->deleteItems(['foo', 'bar'])); + } + + /** + * Ensures underlying PSR-16's `setMultiple()` is only called once per TTL + */ + public function testCommit(): void + { + $item1 = new CacheItem('foo', 'item 1', false, new DateTimeImmutable('2037-12-12'), $this->getClock()); + $item2 = new CacheItem('bar', 'item 2', false, null, $this->getClock()); + $item3 = new CacheItem('baz', 'item 3', false, new DateTimeImmutable('2037-12-12'), $this->getClock()); + $chunk1 = [ + 'foo' => new SerializedItem($item1->getExpiry(), $item1->get()), + 'baz' => new SerializedItem($item3->getExpiry(), $item3->get()), + ]; + $chunk2 = ['bar' => new SerializedItem($item2->getExpiry(), $item2->get())]; + + $psr16 = $this->createMock(CacheInterface::class); + $psr16->expects(static::exactly(2)) + ->method('setMultiple') + ->withConsecutive([$chunk1, 187574400], [$chunk2, null])->willReturn(true); + + $pool = new CacheItemPool($psr16, $this->getFixedClock()); + $pool->saveDeferred($item1); + $pool->saveDeferred($item2); + $pool->saveDeferred($item3); + + static::assertTrue($pool->commit()); + } + + private function getClock(): NowFactory + { + return new NowFactory(); + } + + private function getFixedClock(): NowFactory + { + $stub = $this->createStub(NowFactory::class); + $stub->method('now')->willReturn(new DateTimeImmutable('2031-12-31')); + + return $stub; + } +} From 467d546196f9098f59f397cba85420c176f27cc3 Mon Sep 17 00:00:00 2001 From: ebln <34722048+ebln@users.noreply.github.com> Date: Fri, 15 Oct 2021 15:44:19 +0200 Subject: [PATCH 12/13] Refactor getItems: deduplicate & separate concerns --- src/CacheItemPool.php | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/CacheItemPool.php b/src/CacheItemPool.php index bbd2fac..8623ee0 100644 --- a/src/CacheItemPool.php +++ b/src/CacheItemPool.php @@ -50,22 +50,24 @@ public function getItems(array $keys = []): array $items = []; try { + $downstreamKeys = []; foreach ($keys as $key) { $this->validateKey($key); - if (isset($this->defered[$key])) { - // NOTE In constrast to hasItem() this apparently always returns the defered item, even expired ones - $item = new CacheItem($key, $this->defered[$key]->getValue(), true, $this->defered[$key]->getExpiresAt(), $this->nowFactory); + // "Revive" defered item. // NOTE In constrast to hasItem() this apparently always returns the defered item, even expired ones + $items[$key] = $this->unserializeItem($key, $this->defered[$key]); } else { - /** @var ?SerializedItem $rawItem */ - $rawItem = $this->cache->get($key); - if ($rawItem instanceof SerializedItem) { - $item = new CacheItem($key, $rawItem->getValue(), true, $rawItem->getExpiresAt(), $this->nowFactory); - } else { - $item = new CacheItem($key, null, false, null, $this->nowFactory); - } + $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); } - $items[$key] = $item; } } catch (InvalidArgumentException $k) { @@ -185,6 +187,11 @@ public function commit(): bool return true; } + private function unserializeItem(string $key, SerializedItem $serialized): CacheItem + { + return new CacheItem($key, $serialized->getValue(), true, $serialized->getExpiresAt(), $this->nowFactory); + } + /** * @psalm-assert CacheItem $item */ From 5ace34775b26b0f0dcd17ffba435da62d2c6fa88 Mon Sep 17 00:00:00 2001 From: ebln <34722048+ebln@users.noreply.github.com> Date: Fri, 15 Oct 2021 15:54:16 +0200 Subject: [PATCH 13/13] Refactor getItems: Using at most one call to getMultiple() --- CHANGELOG.md | 18 +++++++------- src/CacheItemPool.php | 22 ++++++++++------- tests/TransactionTest.php | 50 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c836dab..0d6a880 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,18 @@ 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 @@ -16,14 +26,6 @@ Intended to follow [«Keep a Changelog»](https://keepachangelog.com/en/) - 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 index 8623ee0..a42a69c 100644 --- 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 $keys * * @return array @@ -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 index 24626d1..99967bf 100644 --- 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 */