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); diff --git a/CHANGELOG.md b/CHANGELOG.md index ab2dd53..0d6a880 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,14 +9,23 @@ Intended to follow [«Keep a Changelog»](https://keepachangelog.com/en/) ### 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) ---- +## [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 + ## [1.0.0] - 2021-10-10 ### Added diff --git a/makefile b/makefile index 83f78e8..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 test-all +.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}' @@ -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/src/CacheItemPool.php b/src/CacheItemPool.php index 03cac7a..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 @@ -50,22 +52,29 @@ public function getItems(array $keys = []): array $items = []; try { + $downstreamKeys = []; foreach ($keys as $key) { $this->validateKey($key); - if (isset($this->defered[$key])) { - $item = new CacheItem($key, $this->defered[$key]->getValue(), true, $this->defered[$key]->getExpiresAt()); + // "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); + $downstreamKeys[] = $key; + } + } + + if ($downstreamKeys) { + /** + * @var string $key + * @var ?SerializedItem $rawItem + */ + foreach ($this->cache->getMultiple($downstreamKeys) as $key => $rawItem) { if ($rawItem instanceof SerializedItem) { - $item = new CacheItem($key, $rawItem->getValue(), true, $rawItem->getExpiresAt()); + $items[$key] = $this->unserializeItem($key, $rawItem); } else { - $item = new CacheItem($key, null, false, null); + $items[$key] = new CacheItem($key, null, false, null, $this->nowFactory); } } - - $items[$key] = $item; } } catch (InvalidArgumentException $k) { throw $k; @@ -184,6 +193,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 */ 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/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/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()); + } +} diff --git a/tests/CacheItemTest.php b/tests/CacheItemTest.php new file mode 100644 index 0000000..8346f45 --- /dev/null +++ b/tests/CacheItemTest.php @@ -0,0 +1,99 @@ +getClock()); + $dateTime = new DateTime(); + $item->expiresAt($dateTime); + + static::assertEquals($dateTime, $item->getExpiry()); + static::assertNotSame($dateTime, $item->getExpiry()); + } + + public function testExpiresAtWithDateTimeImmutable(): void + { + $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 */ diff --git a/tests/TransactionTest.php b/tests/TransactionTest.php new file mode 100644 index 0000000..99967bf --- /dev/null +++ b/tests/TransactionTest.php @@ -0,0 +1,124 @@ +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 + */ + public function testDeleteItems(): void + { + $psr16 = $this->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; + } +} 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 @@ +delete($key)) { + // @codeCoverageIgnoreStart return false; + // @codeCoverageIgnoreEnd } } diff --git a/tests/helper/Psr16Test.php b/tests/helper/Psr16Test.php index f2d945b..8f46395 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 @@ -17,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))); + } }