From ca53a2e8e4eb8b7522ba4d3ea4784f3d2eb7b87f Mon Sep 17 00:00:00 2001 From: Doeke Norg Date: Tue, 10 Sep 2024 12:02:14 +0200 Subject: [PATCH 1/3] Add tags to cache - Update ArrayCacheProvider to implement tags - Add CacheItem to type and validate parameters - Add tests --- phpstan.dist.neon | 1 + src/Cache/ArrayCacheProvider.php | 81 ++++++++++---- src/Cache/CacheItem.php | 144 +++++++++++++++++++++++++ src/Cache/CacheProvider.php | 13 ++- src/Clock/Clock.php | 4 +- src/Data/CachedDataSource.php | 24 ++++- src/Field/FilterableField.php | 2 +- tests/Cache/ArrayCacheProviderTest.php | 92 ++++++++++++---- tests/Cache/CacheItemTest.php | 35 ++++++ tests/Data/CachedDataSourceTest.php | 19 ++++ 10 files changed, 366 insertions(+), 49 deletions(-) create mode 100644 src/Cache/CacheItem.php create mode 100644 tests/Cache/CacheItemTest.php diff --git a/phpstan.dist.neon b/phpstan.dist.neon index 278e025..aaa5d63 100644 --- a/phpstan.dist.neon +++ b/phpstan.dist.neon @@ -2,6 +2,7 @@ includes: - vendor/szepeviktor/phpstan-wordpress/extension.neon parameters: + editorUrl: 'phpstorm://open?file=%%file%%&line=%%line%%' level: 5 paths: - src diff --git a/src/Cache/ArrayCacheProvider.php b/src/Cache/ArrayCacheProvider.php index acf0248..d5be3be 100644 --- a/src/Cache/ArrayCacheProvider.php +++ b/src/Cache/ArrayCacheProvider.php @@ -4,6 +4,8 @@ use DataKit\DataViews\Clock\Clock; use DataKit\DataViews\Clock\SystemClock; +use DateInterval; +use Exception; /** * Cache provider backed by an array. @@ -18,10 +20,19 @@ final class ArrayCacheProvider implements CacheProvider { * * @since $ver$ * - * @var array + * @var CacheItem[] */ private array $items; + /** + * Contains the reference to the tags with their tagged cache keys. + * + * @since $ver$ + * + * @var array + */ + private array $tags = []; + /** * The clock instance. * @@ -36,8 +47,8 @@ final class ArrayCacheProvider implements CacheProvider { * * @since $ver$ * - * @param Clock|null $clock The clock instance. - * @param array $items The pre-filled cache items. + * @param Clock|null $clock The clock instance. + * @param CacheItem[] $items The pre-filled cache items. */ public function __construct( ?Clock $clock = null, array $items = [] ) { $this->clock = $clock ?? new SystemClock(); @@ -49,12 +60,18 @@ public function __construct( ?Clock $clock = null, array $items = [] ) { * * @since $ver$ */ - public function set( string $key, $value, ?int $ttl = null ): void { - $time = $ttl - ? ( $this->clock->now()->getTimestamp() + $ttl ) - : null; + public function set( string $key, $value, ?int $ttl = null, array $tags = [] ): void { + try { + $time = (int) $ttl > 0 + ? ( $this->clock->now()->add( new DateInterval( 'PT' . $ttl . 'S' ) ) ) + : null; + } catch ( Exception $e ) { + throw new \InvalidArgumentException( $e->getMessage(), $e->getCode(), $e ); + } + + $this->items[ $key ] = new CacheItem( $key, $value, $time, $tags ); - $this->items[ $key ] = compact( 'value', 'time' ); + $this->add_tags( $key, $tags ); } /** @@ -63,15 +80,15 @@ public function set( string $key, $value, ?int $ttl = null ): void { * @since $ver$ */ public function get( string $key, $fallback = null ) { - $item = $this->items[ $key ] ?? []; + $item = $this->items[ $key ] ?? null; - if ( $this->is_expired( $item ) ) { + if ( ! $item || $item->is_expired( $this->clock ) ) { unset( $this->items[ $key ] ); return $fallback; } - return $item['value'] ?? $fallback; + return $item->value(); } /** @@ -82,7 +99,7 @@ public function get( string $key, $fallback = null ) { public function has( string $key ): bool { if ( isset( $this->items[ $key ] ) - && ! $this->is_expired( $this->items[ $key ] ) + && ! $this->items[ $key ]->is_expired( $this->clock ) ) { return true; } @@ -103,6 +120,23 @@ public function delete( string $key ): bool { return true; } + /** + * @inheritDoc + * + * @since $ver$ + */ + public function delete_by_tags( array $tags ): bool { + foreach ( $tags as $tag ) { + foreach ( $this->tags[ $tag ] ?? [] as $key ) { + $this->delete( $key ); + } + + unset( $this->tags[ $tag ] ); + } + + return true; + } + /** * @inheritDoc * @@ -110,23 +144,28 @@ public function delete( string $key ): bool { */ public function clear(): bool { $this->items = []; + $this->tags = []; return true; } /** - * Returns whether the provided cache item is expired. + * Records a key for all provided tags. * * @since $ver$ * - * @param array $item The cache item. - * - * @return bool Whether the cache is expired. + * @param string $key The key to tag. + * @param array $tags The tags. */ - private function is_expired( array $item ): bool { - return ( - ( $item['time'] ?? null ) - && $this->clock->now()->getTimestamp() > $item['time'] - ); + private function add_tags( string $key, array $tags ): void { + foreach ( $tags as $tag ) { + if ( ! is_string( $tag ) ) { + throw new \InvalidArgumentException( 'A tag must be a string.' ); + } + + $this->tags[ $tag ] ??= []; + + $this->tags[ $tag ] = array_unique( array_merge( $this->tags[ $tag ], [ $key ] ) ); + } } } diff --git a/src/Cache/CacheItem.php b/src/Cache/CacheItem.php new file mode 100644 index 0000000..4b65bfa --- /dev/null +++ b/src/Cache/CacheItem.php @@ -0,0 +1,144 @@ +set_key( $key ); + $this->value = $value; + $this->expires_at = $expires_at; + + $this->add_tags( ...$tags ); + } + + /** + * Sets the key, and ensures it is valid. + * + * @since $ver$ + * + * @param string $key The cache key. + */ + private function set_key( string $key ): void { + if ( strlen( $key ) > 64 ) { + throw new \InvalidArgumentException( 'Cache keys may not exceed a length of 64 characters.' ); + } + + if ( preg_replace( '/[^a-z0-9_.]+/i', '', $key ) !== $key ) { + throw new \InvalidArgumentException( 'Cache keys may only contain a-z, A-Z, 0-9, underscores (_) and periods (.).' ); + } + + $this->key = $key; + } + + /** + * Returns the key for the cache item. + * + * @since $ver$ + * + * @return string The cache key. + */ + public function key(): string { + return $this->key; + } + + /** + * Returns the value on the CacheItem. + * + * @since $ver$ + * + * @return mixed The cached value. + */ + public function value() { + return $this->value; + } + + /** + * Returns the tags for the cache item. + * + * @since $ver$ + * + * @return string[] The tags. + */ + public function tags(): array { + return $this->tags; + } + + /** + * Returns whether the cache item is expired. + * + * @param Clock $clock The clock to test expiration. + * + * @return bool Whether the cache item is expired. + */ + public function is_expired( Clock $clock ): bool { + if ( null === $this->expires_at ) { + return false; + } + + return $clock->now() > $this->expires_at; + } + + /** + * Ensures all tags are strings. + * + * @since $ver$ + * + * @param string ...$tags The tags. + */ + private function add_tags( string ...$tags ): void { + $this->tags = $tags; + } +} diff --git a/src/Cache/CacheProvider.php b/src/Cache/CacheProvider.php index ab5565f..84d68c8 100644 --- a/src/Cache/CacheProvider.php +++ b/src/Cache/CacheProvider.php @@ -17,7 +17,7 @@ interface CacheProvider { * @param mixed $value The value to cache. * @param int|null $ttl The time to live in seconds. */ - public function set( string $key, $value, ?int $ttl = null ): void; + public function set( string $key, $value, ?int $ttl = null, array $tags = [] ): void; /** * Retrieves the value from the cache, or the default if no value is stored. @@ -56,6 +56,17 @@ public function has( string $key ): bool; */ public function delete( string $key ): bool; + /** + * .Deletes any entries tagged with one of the provided tags. + * + * @since $ver$ + * + * @param string[] $tags The tags to clear. + * + * @return bool Whether the deletion of the cache items was successful. + */ + public function delete_by_tags( array $tags ): bool; + /** * Clears the entire cache. * diff --git a/src/Clock/Clock.php b/src/Clock/Clock.php index b7d7fc9..20360c2 100644 --- a/src/Clock/Clock.php +++ b/src/Clock/Clock.php @@ -2,7 +2,7 @@ namespace DataKit\DataViews\Clock; -use DateTimeInterface; +use DateTimeImmutable; /** * Object that represents a clock. @@ -15,5 +15,5 @@ interface Clock { * * @since $ver$ */ - public function now(): DateTimeInterface; + public function now(): DateTimeImmutable; } diff --git a/src/Data/CachedDataSource.php b/src/Data/CachedDataSource.php index 9efb3ba..4147668 100644 --- a/src/Data/CachedDataSource.php +++ b/src/Data/CachedDataSource.php @@ -38,6 +38,13 @@ final class CachedDataSource extends BaseDataSource implements MutableDataSource */ private CacheProvider $cache; + /** + * Used to mark cache entries that are tags. + * + * @since $ver$ + */ + private const CACHE_TAG_PREFIX = 'DATAKIT_DATASOURCE_TAG_'; + /** * Creates a cached data source decorator. * @@ -60,6 +67,17 @@ public function id(): string { return $this->inner->id(); } + /** + * Returns the tags used for this data source. + * + * @since $ver$ + * + * @return string[] The tags. + */ + private function get_tag_keys(): array { + return [ self::CACHE_TAG_PREFIX . $this->get_cache_key() ]; + } + /** * Returns a calculated key based on a set of arguments. * @@ -119,8 +137,7 @@ private function fetch( string $cache_key, callable $retrieve_result ) { $result = $retrieve_result(); - // Todo: record this key on a group cache, so we can invalidate all cached keys in one swoop. - $this->cache->set( $cache_key, $result ); + $this->cache->set( $cache_key, $result, null, $this->get_tag_keys() ); return $result; } @@ -254,7 +271,6 @@ public function delete_data_by_id( string ...$ids ): void { * @return bool Whether the cache was cleared. */ public function clear_cache(): bool { - // Todo: only clear the cache for the keys for this data source. - return $this->cache->clear(); + return $this->cache->delete_by_tags( $this->get_tag_keys() ); } } diff --git a/src/Field/FilterableField.php b/src/Field/FilterableField.php index 19c9554..7ee1f63 100644 --- a/src/Field/FilterableField.php +++ b/src/Field/FilterableField.php @@ -35,7 +35,7 @@ abstract class FilterableField extends Field { * * @param Operator ...$operators The operators. * - * @return self A new instance with the filters applied. + * @return static A new instance with the filters applied. */ public function filterable_by( Operator ...$operators ) { $clone = clone $this; diff --git a/tests/Cache/ArrayCacheProviderTest.php b/tests/Cache/ArrayCacheProviderTest.php index bd26493..c16985d 100644 --- a/tests/Cache/ArrayCacheProviderTest.php +++ b/tests/Cache/ArrayCacheProviderTest.php @@ -7,7 +7,7 @@ use PHPUnit\Framework\TestCase; /** - * Unit tests for {@see ArrayCacheProvider} + * Unit tests for {@see ArrayCacheProvider}. * * @since $ver$ */ @@ -17,26 +17,26 @@ final class ArrayCacheProviderTest extends TestCase { * * @since $ver$ */ - public function test_cache() : void { + public function test_cache(): void { $cache = new ArrayCacheProvider(); - self::assertFalse( $cache->has( 'some-key' ) ); - $cache->set( 'some-key', 'A value' ); - $cache->set( 'other-key', 'Other value' ); + self::assertFalse( $cache->has( 'some_key' ) ); + $cache->set( 'some_key', 'A value' ); + $cache->set( 'other_key', 'Other value' ); - self::assertTrue( $cache->has( 'some-key' ) ); - self::assertTrue( $cache->has( 'other-key' ) ); + self::assertTrue( $cache->has( 'some_key' ) ); + self::assertTrue( $cache->has( 'other_key' ) ); - self::assertSame( 'A value', $cache->get( 'some-key', 'different default' ) ); + self::assertSame( 'A value', $cache->get( 'some_key', 'different default' ) ); self::assertSame( 'different default', $cache->get( 'invalid', 'different default' ) ); self::assertNull( $cache->get( 'invalid' ) ); - self::assertTrue( $cache->delete( 'some-key' ) ); - self::assertFalse( $cache->has( 'some-key' ) ); - self::assertNull( $cache->get( 'some-key' ) ); + self::assertTrue( $cache->delete( 'some_key' ) ); + self::assertFalse( $cache->has( 'some_key' ) ); + self::assertNull( $cache->get( 'some_key' ) ); self::assertTrue( $cache->clear() ); - self::assertFalse( $cache->has( 'other-key' ) ); + self::assertFalse( $cache->has( 'other_key' ) ); } /** @@ -44,21 +44,73 @@ public function test_cache() : void { * * @return void */ - public function test_ttl() : void { + public function test_ttl(): void { $clock = new FrozenClock( '2024-06-27 12:34:56' ); $cache = new ArrayCacheProvider( $clock ); - $cache->set( 'some-key', $value = 'This value is stored for 5 seconds.', 5 ); - $cache->set( 'another-key', $value, 5 ); + $cache->set( 'some_key', $value = 'This value is stored for 5 seconds.', 5 ); + $cache->set( 'another_key', $value, 5 ); - self::assertTrue( $cache->has( 'some-key' ) ); - self::assertSame( $value, $cache->get( 'another-key' ) ); + self::assertTrue( $cache->has( 'some_key' ) ); + self::assertSame( $value, $cache->get( 'another_key' ) ); $clock->travel_to( '2024-06-27 13:00:00' ); - self::assertFalse( $cache->has( 'some-key' ) ); - self::assertNull( $cache->get( 'another-key' ) ); + self::assertFalse( $cache->has( 'some_key' ) ); + self::assertNull( $cache->get( 'another_key' ) ); $clock->travel_to( '2024-06-27 12:34:54' ); // back in time. - self::assertFalse( $cache->has( 'some-key' ) ); + self::assertFalse( $cache->has( 'some_key' ) ); + } + + public static function dataprovider_for_test_tags(): array { + return [ + 'none' => [ + [], + [ 'key_1_1', 'key_1_2', 'key_2_1', 'key_2_2' ], + ], + 'tag_1' => [ + [ 'tag_1' ], + [ 'key_2_1', 'key_2_2' ], + ], + 'tag_2' => [ + [ 'tag_2' ], + [ 'key_1_1', 'key_1_2' ], + ], + 'both tags' => [ + [ 'tag_1', 'tag_2' ], + [], + ], + ]; + } + + /** + * Test case for cache items with tags. + * + * @since $ver$ + * + * @param array $tags_to_delete The tags to delete during the test. + * @param array $expected_keys The keys expected to remain after the deletion. + * + * @dataProvider dataprovider_for_test_tags The data provider. + */ + public function test_tags( array $tags_to_delete, array $expected_keys ): void { + $cache = new ArrayCacheProvider(); + $keys = [ 'key_1_1', 'key_1_2', 'key_2_1', 'key_2_2' ]; + + $cache->set( 'key_1_1', 'value 1.1', null, [ 'tag_1' ] ); + $cache->set( 'key_1_2', 'value 1.2', null, [ 'tag_1' ] ); + + $cache->set( 'key_2_1', 'value 2.1', null, [ 'tag_2' ] ); + $cache->set( 'key_2_2', 'value 2.2', null, [ 'tag_2' ] ); + + foreach ( $keys as $key ) { + self::assertTrue( $cache->has( $key ) ); + } + + $cache->delete_by_tags( $tags_to_delete ); + + foreach ( $keys as $key ) { + self::assertEquals( in_array( $key, $expected_keys, true ), $cache->has( $key ) ); + } } } diff --git a/tests/Cache/CacheItemTest.php b/tests/Cache/CacheItemTest.php new file mode 100644 index 0000000..6c7792e --- /dev/null +++ b/tests/Cache/CacheItemTest.php @@ -0,0 +1,35 @@ +key() ); + self::assertSame( 'value-1', $item->value() ); + self::assertSame( [ 'tag_1', 'tag_2' ], $item->tags() ); + + self::assertFalse( $item->is_expired( $clock ) ); + $clock->travel_to( '2024-09-10 10:36:00' ); + self::assertTrue( $item->is_expired( $clock ) ); + } +} diff --git a/tests/Data/CachedDataSourceTest.php b/tests/Data/CachedDataSourceTest.php index 933e7a3..d873658 100644 --- a/tests/Data/CachedDataSourceTest.php +++ b/tests/Data/CachedDataSourceTest.php @@ -2,6 +2,7 @@ namespace DataKit\DataViews\Tests\Data; +use Closure; use DataKit\DataViews\Cache\ArrayCacheProvider; use DataKit\DataViews\Data\ArrayDataSource; use DataKit\DataViews\Data\CachedDataSource; @@ -91,8 +92,26 @@ public function test_caching(): void { // Get fields is always piped through. self::assertCount( 6, $this->trace->get_calls() ); + $ds_2 = new CachedDataSource( + new ArrayDataSource( 'second-source', [ 'three' => [ 'name' => 'Person 3' ] ] ), + $cache + ); + + // Helper method to inspect the private $items variable. + $get_items_count = Closure::fromCallable( function () { + return count( $this->items ); // @phpstan-ignore property.notFound + } )->bindTo( $cache, $cache ); + + // Trigger caching for second data source. + self::assertSame( [ 'three' ], $ds_2->get_data_ids() ); + self::assertSame( 2, $get_items_count() ); + // Clear cache for this data source only. $ds->clear_cache(); + + self::assertSame( 1, $get_items_count() ); $ds->get_data_ids(); + + self::assertSame( 2, $get_items_count() ); // Cache cleared. self::assertCount( 7, $this->trace->get_calls() ); } From ab033179004596d8d33e2d4a4f05d705e0baeca6 Mon Sep 17 00:00:00 2001 From: Doeke Norg Date: Tue, 10 Sep 2024 17:26:20 +0200 Subject: [PATCH 2/3] Add SQLite cache provider --- .gitignore | 3 + composer.json | 3 + src/Cache/ArrayCacheProvider.php | 65 ++----- src/Cache/BaseCacheProvider.php | 72 +++++++ src/Cache/CacheItem.php | 9 +- src/Cache/SqliteCacheProvider.php | 224 ++++++++++++++++++++++ src/Data/CachedDataSource.php | 9 +- tests/Cache/AbstractCacheProviderTest.php | 115 +++++++++++ tests/Cache/ArrayCacheProviderTest.php | 105 +--------- tests/Cache/CacheItemTest.php | 4 +- tests/Cache/SqliteCacheProviderTest.php | 28 +++ 11 files changed, 473 insertions(+), 164 deletions(-) create mode 100644 src/Cache/BaseCacheProvider.php create mode 100644 src/Cache/SqliteCacheProvider.php create mode 100644 tests/Cache/AbstractCacheProviderTest.php create mode 100644 tests/Cache/SqliteCacheProviderTest.php diff --git a/.gitignore b/.gitignore index 13cf476..df86f0d 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,6 @@ phpcs.xml .idea/**/.name .idea/**/codeStyles .idea/**/php.xml + +## Tests +tests/assets/test-sqlite.db diff --git a/composer.json b/composer.json index 715b6f7..62947d5 100644 --- a/composer.json +++ b/composer.json @@ -40,6 +40,9 @@ "phpcompatibility/phpcompatibility-wp": "^2.1", "overtrue/phplint": "^3.4" }, + "suggest": { + "ext-pdo": "PDO is required to use the SQLite cache provider" + }, "scripts": { "suite": [ "composer test", diff --git a/src/Cache/ArrayCacheProvider.php b/src/Cache/ArrayCacheProvider.php index d5be3be..a9f387d 100644 --- a/src/Cache/ArrayCacheProvider.php +++ b/src/Cache/ArrayCacheProvider.php @@ -14,7 +14,7 @@ * * @since $ver$ */ -final class ArrayCacheProvider implements CacheProvider { +final class ArrayCacheProvider extends BaseCacheProvider { /** * The cached items. * @@ -22,7 +22,7 @@ final class ArrayCacheProvider implements CacheProvider { * * @var CacheItem[] */ - private array $items; + private array $items = []; /** * Contains the reference to the tags with their tagged cache keys. @@ -33,26 +33,17 @@ final class ArrayCacheProvider implements CacheProvider { */ private array $tags = []; - /** - * The clock instance. - * - * @since $ver$ - * - * @var Clock - */ - private Clock $clock; - /** * Creates an Array cache provider. * * @since $ver$ * - * @param Clock|null $clock The clock instance. - * @param CacheItem[] $items The pre-filled cache items. + * @param Clock|null $clock The clock instance. */ - public function __construct( ?Clock $clock = null, array $items = [] ) { + public function __construct( ?Clock $clock = null ) { + parent::__construct( $clock ); + $this->clock = $clock ?? new SystemClock(); - $this->items = $items; } /** @@ -74,41 +65,6 @@ public function set( string $key, $value, ?int $ttl = null, array $tags = [] ): $this->add_tags( $key, $tags ); } - /** - * @inheritDoc - * - * @since $ver$ - */ - public function get( string $key, $fallback = null ) { - $item = $this->items[ $key ] ?? null; - - if ( ! $item || $item->is_expired( $this->clock ) ) { - unset( $this->items[ $key ] ); - - return $fallback; - } - - return $item->value(); - } - - /** - * @inheritDoc - * - * @since $ver$ - */ - public function has( string $key ): bool { - if ( - isset( $this->items[ $key ] ) - && ! $this->items[ $key ]->is_expired( $this->clock ) - ) { - return true; - } - - unset( $this->items[ $key ] ); - - return false; - } - /** * @inheritDoc * @@ -168,4 +124,13 @@ private function add_tags( string $key, array $tags ): void { $this->tags[ $tag ] = array_unique( array_merge( $this->tags[ $tag ], [ $key ] ) ); } } + + /** + * @inheritDoc + * + * @since $ver$ + */ + protected function doGet( string $key ): ?CacheItem { + return $this->items[ $key ] ?? null; + } } diff --git a/src/Cache/BaseCacheProvider.php b/src/Cache/BaseCacheProvider.php new file mode 100644 index 0000000..38ab1fd --- /dev/null +++ b/src/Cache/BaseCacheProvider.php @@ -0,0 +1,72 @@ +clock = $clock ?? new SystemClock(); + } + + /** + * Returns the {@see CacheItem} if found by key. + * + * @param string $key The key. + * + * @return CacheItem|null The cache item. + */ + abstract protected function doGet( string $key ): ?CacheItem; + + /** + * @inheritDoc + * @since $ver$ + */ + public function get( string $key, $fallback = null ) { + $item = $this->doGet( $key ); + if ( ! $item || $item->is_expired( $this->clock->now() ) ) { + $this->delete( $key ); + + return $fallback; + } + + return $item->value(); + } + + /** + * @inheritDoc + * @since $ver$ + */ + public function has( string $key ): bool { + $item = $this->doGet( $key ); + + if ( ! $item || $item->is_expired( $this->clock->now() ) ) { + $this->delete( $key ); + + return false; + } + + return true; + } +} diff --git a/src/Cache/CacheItem.php b/src/Cache/CacheItem.php index 4b65bfa..6f07cfb 100644 --- a/src/Cache/CacheItem.php +++ b/src/Cache/CacheItem.php @@ -2,8 +2,7 @@ namespace DataKit\DataViews\Cache; -use DataKit\DataViews\Clock\Clock; -use DateTime; +use DateTimeImmutable; use DateTimeInterface; /** @@ -119,16 +118,16 @@ public function tags(): array { /** * Returns whether the cache item is expired. * - * @param Clock $clock The clock to test expiration. + * @param DateTimeImmutable $now The date time to test against. * * @return bool Whether the cache item is expired. */ - public function is_expired( Clock $clock ): bool { + public function is_expired( DateTimeImmutable $now ): bool { if ( null === $this->expires_at ) { return false; } - return $clock->now() > $this->expires_at; + return $now > $this->expires_at; } /** diff --git a/src/Cache/SqliteCacheProvider.php b/src/Cache/SqliteCacheProvider.php new file mode 100644 index 0000000..56a817b --- /dev/null +++ b/src/Cache/SqliteCacheProvider.php @@ -0,0 +1,224 @@ +file_path = $file_path; + } + + /** + * Creates the tables on the database. + * + * @since $ver$ + */ + private function create_tables(): void { + $this->db->exec( + 'CREATE TABLE IF NOT EXISTS `items` ( + item_key VARCHAR(64) NOT NULL PRIMARY KEY, + item_value TEXT, + expires_at VARCHAR(100) + );' + ); + + $this->db->exec( + 'CREATE TABLE if NOT EXISTS `tags` ( + item_tag VARCHAR( 64 ) NOT null, + item_key VARCHAR( 64 ) NOT null, + PRIMARY KEY( item_tag, item_key ) + );' + ); + } + + /** + * Lazily creates the connection to the database. + * + * @since $ver$ + */ + private function init(): void { + if ( ! isset( $this->db ) ) { + $this->db = new PDO( 'sqlite:' . $this->file_path ); + $this->create_tables(); + } + } + + /** + * @inheritDoc + * @since $ver$ + */ + public function set( string $key, $value, ?int $ttl = null, array $tags = [] ): void { + $this->init(); + + try { + $time = (int) $ttl > 0 + ? ( $this->clock->now()->add( new DateInterval( 'PT' . $ttl . 'S' ) ) ) + : null; + } catch ( Exception $e ) { + throw new \InvalidArgumentException( $e->getMessage(), $e->getCode(), $e ); + } + + $this->db->prepare( + 'INSERT INTO items (item_key, item_value, expires_at) VALUES (:key, :value, :expires_at) + ON CONFLICT (item_key) DO UPDATE SET item_value = :value, expires_at = :expires_at' + )->execute( + [ + 'key' => $key, + 'value' => json_encode( $value, JSON_THROW_ON_ERROR ), + 'expires_at' => $time ? $time->format( 'c' ) : null, + ] + ); + + $this->add_tags( $key, $tags ); + } + + /** + * @inheritDoc + * + * @since $ver$ + */ + protected function doGet( string $key ): ?CacheItem { + $this->init(); + + $statement = $this->db->prepare( 'SELECT * FROM items WHERE item_key = :key' ); + $statement->execute( [ 'key' => $key ] ); + $result = $statement->fetch( PDO::FETCH_ASSOC ); + if ( ! $result ) { + return null; + } + + return new CacheItem( + $key, + json_decode( $result['item_value'], true ), + $result['expires_at'] ? new DateTimeImmutable( $result['expires_at'] ) : null + ); + } + + /** + * @inheritDoc + * @since $ver$ + */ + public function delete( string $key ): bool { + $this->init(); + + return $this->db + ->prepare( 'DELETE FROM items WHERE item_key = :key' ) + ->execute( [ 'key' => $key ] ); + } + + /** + * @inheritDoc + * @since $ver$ + */ + public function delete_by_tags( array $tags ): bool { + $this->init(); + + if ( ! $tags ) { + return true; + } + + $tags_string = implode( + ',', + array_map( + fn( string $tag ): string => $this->db->quote( $tag ), + $tags + ) + ); + + // Remove items. + $this->db->exec( + "DELETE FROM items WHERE item_key IN ( + SELECT item_key FROM tags WHERE item_tag IN ($tags_string) + );" + ); + + // Remove tags. + $this->db->exec( "DELETE FROM tags WHERE item_tag IN ($tags_string)" ); + + return true; + } + + /** + * @inheritDoc + * @since $ver$ + */ + public function clear(): bool { + if ( file_exists( $this->file_path ) ) { + @unlink( $this->file_path ); //@phpcs:ignore WordPress.WP.AlternativeFunctions.unlink_unlink, WordPress.PHP.NoSilencedErrors.Discouraged + unset( $this->db ); + } + + $this->init(); + + return true; + } + + /** + * Records a key for all provided tags. + * + * @since $ver$ + * + * @param string $key The key to tag. + * @param array $tags The tags. + */ + private function add_tags( string $key, array $tags ): void { + if ( ! $tags ) { + return; + } + + $query = 'INSERT INTO tags (item_key, item_tag) VALUES '; + $values = []; + $parts = []; + + foreach ( $tags as $tag ) { + $values[] = $key; + $values[] = $tag; + + $parts[] = '(?,?)'; + } + + $this->db->prepare( $query . implode( ',', $parts ) ) + ->execute( $values ); + } +} diff --git a/src/Data/CachedDataSource.php b/src/Data/CachedDataSource.php index 4147668..4e5b3bd 100644 --- a/src/Data/CachedDataSource.php +++ b/src/Data/CachedDataSource.php @@ -38,13 +38,6 @@ final class CachedDataSource extends BaseDataSource implements MutableDataSource */ private CacheProvider $cache; - /** - * Used to mark cache entries that are tags. - * - * @since $ver$ - */ - private const CACHE_TAG_PREFIX = 'DATAKIT_DATASOURCE_TAG_'; - /** * Creates a cached data source decorator. * @@ -75,7 +68,7 @@ public function id(): string { * @return string[] The tags. */ private function get_tag_keys(): array { - return [ self::CACHE_TAG_PREFIX . $this->get_cache_key() ]; + return [ 'DATASOURCE_' . $this->get_cache_key() ]; } /** diff --git a/tests/Cache/AbstractCacheProviderTest.php b/tests/Cache/AbstractCacheProviderTest.php new file mode 100644 index 0000000..adb7868 --- /dev/null +++ b/tests/Cache/AbstractCacheProviderTest.php @@ -0,0 +1,115 @@ +create_provider( null ); + + self::assertFalse( $cache->has( 'some_key' ) ); + $cache->set( 'some_key', 'A value' ); + $cache->set( 'other_key', 'Other value' ); + + self::assertTrue( $cache->has( 'some_key' ) ); + self::assertTrue( $cache->has( 'other_key' ) ); + + self::assertSame( 'A value', $cache->get( 'some_key', 'different default' ) ); + self::assertSame( 'different default', $cache->get( 'invalid', 'different default' ) ); + self::assertNull( $cache->get( 'invalid' ) ); + + self::assertTrue( $cache->delete( 'some_key' ) ); + self::assertFalse( $cache->has( 'some_key' ) ); + self::assertNull( $cache->get( 'some_key' ) ); + + self::assertTrue( $cache->clear() ); + self::assertFalse( $cache->has( 'other_key' ) ); + } + + /** + * Test case for a TTL. + * + * @return void + */ + public function test_ttl(): void { + $clock = new FrozenClock( '2024-06-27 12:34:56' ); + $cache = $this->create_provider( $clock ); + + $cache->set( 'some_key', $value = 'This value is stored for 5 seconds.', 5 ); + $cache->set( 'another_key', $value, 5 ); + + self::assertTrue( $cache->has( 'some_key' ) ); + self::assertSame( $value, $cache->get( 'another_key' ) ); + + $clock->travel_to( '2024-06-27 13:00:00' ); + self::assertFalse( $cache->has( 'some_key' ) ); + self::assertNull( $cache->get( 'another_key' ) ); + + $clock->travel_to( '2024-06-27 12:34:54' ); // back in time. + self::assertFalse( $cache->has( 'some_key' ) ); + } + + public static function dataprovider_for_test_tags(): array { + return [ + 'none' => [ + [], + [ 'key_1_1', 'key_1_2', 'key_2_1', 'key_2_2' ], + ], + 'tag_1' => [ + [ 'tag_1' ], + [ 'key_2_1', 'key_2_2' ], + ], + 'tag_2' => [ + [ 'tag_2' ], + [ 'key_1_1', 'key_1_2' ], + ], + 'both tags' => [ + [ 'tag_1', 'tag_2' ], + [], + ], + ]; + } + + /** + * Test case for cache items with tags. + * + * @since $ver$ + * + * @param array $tags_to_delete The tags to delete during the test. + * @param array $expected_keys The keys expected to remain after the deletion. + * + * @dataProvider dataprovider_for_test_tags The data provider. + */ + public function test_tags( array $tags_to_delete, array $expected_keys ): void { + $cache = $this->create_provider( null ); + $keys = [ 'key_1_1', 'key_1_2', 'key_2_1', 'key_2_2' ]; + + $cache->set( 'key_1_1', 'value 1.1', null, [ 'tag_1' ] ); + $cache->set( 'key_1_2', 'value 1.2', null, [ 'tag_1' ] ); + + $cache->set( 'key_2_1', 'value 2.1', null, [ 'tag_2' ] ); + $cache->set( 'key_2_2', 'value 2.2', null, [ 'tag_2' ] ); + + foreach ( $keys as $key ) { + self::assertTrue( $cache->has( $key ) ); + } + + $cache->delete_by_tags( $tags_to_delete ); + + foreach ( $keys as $key ) { + self::assertEquals( in_array( $key, $expected_keys, true ), $cache->has( $key ) ); + } + } +} diff --git a/tests/Cache/ArrayCacheProviderTest.php b/tests/Cache/ArrayCacheProviderTest.php index c16985d..62562b2 100644 --- a/tests/Cache/ArrayCacheProviderTest.php +++ b/tests/Cache/ArrayCacheProviderTest.php @@ -3,114 +3,21 @@ namespace DataKit\DataViews\Tests\Cache; use DataKit\DataViews\Cache\ArrayCacheProvider; -use DataKit\DataViews\Tests\Clock\FrozenClock; -use PHPUnit\Framework\TestCase; +use DataKit\DataViews\Cache\CacheProvider; +use DataKit\DataViews\Clock\Clock; /** * Unit tests for {@see ArrayCacheProvider}. * * @since $ver$ */ -final class ArrayCacheProviderTest extends TestCase { +final class ArrayCacheProviderTest extends AbstractCacheProviderTest { /** - * Test case entire array cache. + * @inheritDoc * * @since $ver$ */ - public function test_cache(): void { - $cache = new ArrayCacheProvider(); - - self::assertFalse( $cache->has( 'some_key' ) ); - $cache->set( 'some_key', 'A value' ); - $cache->set( 'other_key', 'Other value' ); - - self::assertTrue( $cache->has( 'some_key' ) ); - self::assertTrue( $cache->has( 'other_key' ) ); - - self::assertSame( 'A value', $cache->get( 'some_key', 'different default' ) ); - self::assertSame( 'different default', $cache->get( 'invalid', 'different default' ) ); - self::assertNull( $cache->get( 'invalid' ) ); - - self::assertTrue( $cache->delete( 'some_key' ) ); - self::assertFalse( $cache->has( 'some_key' ) ); - self::assertNull( $cache->get( 'some_key' ) ); - - self::assertTrue( $cache->clear() ); - self::assertFalse( $cache->has( 'other_key' ) ); - } - - /** - * Test case for a TTL. - * - * @return void - */ - public function test_ttl(): void { - $clock = new FrozenClock( '2024-06-27 12:34:56' ); - $cache = new ArrayCacheProvider( $clock ); - - $cache->set( 'some_key', $value = 'This value is stored for 5 seconds.', 5 ); - $cache->set( 'another_key', $value, 5 ); - - self::assertTrue( $cache->has( 'some_key' ) ); - self::assertSame( $value, $cache->get( 'another_key' ) ); - - $clock->travel_to( '2024-06-27 13:00:00' ); - self::assertFalse( $cache->has( 'some_key' ) ); - self::assertNull( $cache->get( 'another_key' ) ); - - $clock->travel_to( '2024-06-27 12:34:54' ); // back in time. - self::assertFalse( $cache->has( 'some_key' ) ); - } - - public static function dataprovider_for_test_tags(): array { - return [ - 'none' => [ - [], - [ 'key_1_1', 'key_1_2', 'key_2_1', 'key_2_2' ], - ], - 'tag_1' => [ - [ 'tag_1' ], - [ 'key_2_1', 'key_2_2' ], - ], - 'tag_2' => [ - [ 'tag_2' ], - [ 'key_1_1', 'key_1_2' ], - ], - 'both tags' => [ - [ 'tag_1', 'tag_2' ], - [], - ], - ]; - } - - /** - * Test case for cache items with tags. - * - * @since $ver$ - * - * @param array $tags_to_delete The tags to delete during the test. - * @param array $expected_keys The keys expected to remain after the deletion. - * - * @dataProvider dataprovider_for_test_tags The data provider. - */ - public function test_tags( array $tags_to_delete, array $expected_keys ): void { - $cache = new ArrayCacheProvider(); - $keys = [ 'key_1_1', 'key_1_2', 'key_2_1', 'key_2_2' ]; - - $cache->set( 'key_1_1', 'value 1.1', null, [ 'tag_1' ] ); - $cache->set( 'key_1_2', 'value 1.2', null, [ 'tag_1' ] ); - - $cache->set( 'key_2_1', 'value 2.1', null, [ 'tag_2' ] ); - $cache->set( 'key_2_2', 'value 2.2', null, [ 'tag_2' ] ); - - foreach ( $keys as $key ) { - self::assertTrue( $cache->has( $key ) ); - } - - $cache->delete_by_tags( $tags_to_delete ); - - foreach ( $keys as $key ) { - self::assertEquals( in_array( $key, $expected_keys, true ), $cache->has( $key ) ); - } + protected function create_provider( ?Clock $clock ): CacheProvider { + return new ArrayCacheProvider( $clock ); } } diff --git a/tests/Cache/CacheItemTest.php b/tests/Cache/CacheItemTest.php index 6c7792e..3aad938 100644 --- a/tests/Cache/CacheItemTest.php +++ b/tests/Cache/CacheItemTest.php @@ -28,8 +28,8 @@ public function test_cache_item(): void { self::assertSame( 'value-1', $item->value() ); self::assertSame( [ 'tag_1', 'tag_2' ], $item->tags() ); - self::assertFalse( $item->is_expired( $clock ) ); + self::assertFalse( $item->is_expired( $clock->now() ) ); $clock->travel_to( '2024-09-10 10:36:00' ); - self::assertTrue( $item->is_expired( $clock ) ); + self::assertTrue( $item->is_expired( $clock->now() ) ); } } diff --git a/tests/Cache/SqliteCacheProviderTest.php b/tests/Cache/SqliteCacheProviderTest.php new file mode 100644 index 0000000..b062b85 --- /dev/null +++ b/tests/Cache/SqliteCacheProviderTest.php @@ -0,0 +1,28 @@ + Date: Wed, 11 Sep 2024 12:58:34 +0200 Subject: [PATCH 3/3] Change cache key build up to avoid needless repeats --- src/Data/CachedDataSource.php | 12 ++++++++---- tests/Data/CachedDataSourceTest.php | 12 +++++++++++- tests/Data/TraceableDataSource.php | 19 +++++++++---------- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/Data/CachedDataSource.php b/src/Data/CachedDataSource.php index 4e5b3bd..3ac008a 100644 --- a/src/Data/CachedDataSource.php +++ b/src/Data/CachedDataSource.php @@ -84,7 +84,7 @@ private function get_cache_key( ...$arguments ): string { $arguments[] = $this->inner->id(); try { - return md5( json_encode( $arguments, JSON_THROW_ON_ERROR ) ); + return md5( json_encode( array_values( array_filter( $arguments ) ), JSON_THROW_ON_ERROR ) ); } catch ( JsonException $e ) { throw new InvalidArgumentException( 'The cache key could not be generated based on the provide arguments', @@ -105,8 +105,7 @@ private function get_cache_key( ...$arguments ): string { */ private function get_filter_aware_cache_key( ...$arguments ): string { $arguments[] = $this->filters ? $this->filters->to_array() : null; - $arguments[] = $this->sort ? $this->sort->to_array() : null; - $arguments[] = $this->search; + $arguments[] = (string) $this->search; return $this->get_cache_key( ...$arguments ); } @@ -141,7 +140,12 @@ private function fetch( string $cache_key, callable $retrieve_result ) { * @since $ver$ */ public function get_data_ids( int $limit = 20, int $offset = 0 ): array { - $key = $this->get_filter_aware_cache_key( __FUNCTION__, $limit, $offset ); + $key = $this->get_filter_aware_cache_key( + __FUNCTION__, + $this->sort ? $this->sort->to_array() : null, + $limit, + $offset + ); return $this->fetch( $key, diff --git a/tests/Data/CachedDataSourceTest.php b/tests/Data/CachedDataSourceTest.php index d873658..cb2b1ac 100644 --- a/tests/Data/CachedDataSourceTest.php +++ b/tests/Data/CachedDataSourceTest.php @@ -172,7 +172,17 @@ public function testCount(): void { self::assertCount( 2, $ds ); self::assertSame( 2, $ds->count() ); // second call - self::assertCount( 1, $this->trace->get_calls() ); + $asc_sort = $ds->sort_by( Sort::asc( 'name' ) ); + self::assertSame( 2, $asc_sort->count() ); + + $desc_sort = $ds->sort_by( Sort::desc( 'name' ) ); + self::assertSame( 2, $desc_sort->count() ); + + // Count should be called only once on the trace, as the sorting does not influence the cache key. + self::assertSame( + [ 'count', 'sort_by', 'sort_by' ], + array_column( $this->trace->get_calls(), 0 ) + ); } /** diff --git a/tests/Data/TraceableDataSource.php b/tests/Data/TraceableDataSource.php index 654324b..614a484 100644 --- a/tests/Data/TraceableDataSource.php +++ b/tests/Data/TraceableDataSource.php @@ -16,7 +16,6 @@ * @since $ver$ */ final class TraceableDataSource implements MutableDataSource { - /** * The decorated data source. * @@ -59,7 +58,7 @@ public function id() : string { * @since $ver$ */ public function get_data_by_id( string $id ) : array { - $this->calls[] = [ __METHOD__, ...func_get_args() ]; + $this->calls[] = [ __FUNCTION__, ...func_get_args() ]; return $this->inner->get_data_by_id( $id ); } @@ -69,7 +68,7 @@ public function get_data_by_id( string $id ) : array { * @since $ver$ */ public function get_fields() : array { - $this->calls[] = [ __METHOD__ ]; + $this->calls[] = [ __FUNCTION__ ]; return $this->inner->get_fields(); } @@ -79,7 +78,7 @@ public function get_fields() : array { * @since $ver$ */ public function get_data_ids( int $limit = 20, int $offset = 0 ) : array { - $this->calls[] = [ __METHOD__, ...func_get_args() ]; + $this->calls[] = [ __FUNCTION__, ...func_get_args() ]; return $this->inner->get_data_ids( $limit, $offset ); } @@ -89,7 +88,7 @@ public function get_data_ids( int $limit = 20, int $offset = 0 ) : array { * @since $ver$ */ public function filter_by( ?Filters $filters ) : self { - $this->calls[] = [ __METHOD__, ...func_get_args() ]; + $this->calls[] = [ __FUNCTION__, ...func_get_args() ]; $this->inner = $this->inner->filter_by( $filters ); @@ -101,7 +100,7 @@ public function filter_by( ?Filters $filters ) : self { * @since $ver$ */ public function sort_by( ?Sort $sort ) : self { - $this->calls[] = [ __METHOD__, ...func_get_args() ]; + $this->calls[] = [ __FUNCTION__, ...func_get_args() ]; $this->inner = $this->inner->sort_by( $sort ); @@ -113,7 +112,7 @@ public function sort_by( ?Sort $sort ) : self { * @since $ver$ */ public function search_by( ?Search $search ) : self { - $this->calls[] = [ __METHOD__, ...func_get_args() ]; + $this->calls[] = [ __FUNCTION__, ...func_get_args() ]; $this->inner = $this->inner->search_by( $search ); return $this; @@ -143,7 +142,7 @@ public function reset() : void { * @since $ver$ */ public function count() : int { - $this->calls[] = [ __METHOD__ ]; + $this->calls[] = [ __FUNCTION__ ]; return $this->inner->count(); } @@ -156,7 +155,7 @@ public function can_delete() : bool { if ( ! $this->inner instanceof MutableDataSource ) { return false; } - $this->calls[] = [ __METHOD__ ]; + $this->calls[] = [ __FUNCTION__ ]; return $this->inner->can_delete(); } @@ -170,7 +169,7 @@ public function delete_data_by_id( string ...$ids ) : void { return; } - $this->calls[] = [ __METHOD__ ]; + $this->calls[] = [ __FUNCTION__ ]; $this->inner->delete_data_by_id( ...$ids ); }