From 2868d1599da544529a404aebab0afe74a743333c Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Thu, 1 Oct 2026 09:16:31 +0200 Subject: [PATCH] feat(task) #30 #31 #32 Options given by the input only, key validation, GetTask on_miss and SetTask expires_after options Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 3 + docs/cookbooks/cache_warmup.md | 24 ++-- docs/cookbooks/share_data_between_branches.md | 4 +- docs/index.md | 5 +- docs/reference/adapter.md | 8 +- docs/reference/tasks/get_task.md | 73 +++++++++--- docs/reference/tasks/set_task.md | 40 ++++--- src/Task/AbstractCacheTask.php | 26 ++++- src/Task/GetTask.php | 43 ++++++- src/Task/SetTask.php | 18 ++- tests/Task/GetTaskTest.php | 87 +++++++++++++- tests/Task/SetTaskTest.php | 110 +++++++++++++++++- 12 files changed, 376 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b66207..2e38bf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ Latest ------ ### Changes +* [#30](https://github.com/cleverage/cache-process-bundle/issues/30) GetTask and SetTask: `adapter`, `key` and `value` are no longer required at configuration level (no placeholders needed), only once merged with the input; the key is validated by the task, so an invalid key always throws (Symfony adapters only validate keys with `assert()`). New `AbstractCacheTask::getRequiredOptions()`. Update documentation, add tests. +* [#31](https://github.com/cleverage/cache-process-bundle/issues/31) GetTask: add an `on_miss` option (`output_null` by default, `skip` to send the input to the error outputs, `fail`) to handle cache misses. Update documentation, add tests. +* [#32](https://github.com/cleverage/cache-process-bundle/issues/32) SetTask: add an `expires_after` option, to set the lifetime of the items. Update documentation, add tests. * [#20](https://github.com/cleverage/cache-process-bundle/issues/20) Add missing tests: GetTask and SetTask (options validation at initialization, context, missing adapter, stored `null`, overwriting), custom tasks extending AbstractCacheTask, Adapter, bundle and DI extension. * [#25](https://github.com/cleverage/cache-process-bundle/issues/25) Give the ids of both services in the error on duplicate adapter codes: the adapters are registered by a compiler pass of the bundle, `AdapterRegistry::addAdapter()` gets an optional `$serviceId` argument. Update documentation, add tests. diff --git a/docs/cookbooks/cache_warmup.md b/docs/cookbooks/cache_warmup.md index 851c36c..bfe1539 100644 --- a/docs/cookbooks/cache_warmup.md +++ b/docs/cookbooks/cache_warmup.md @@ -63,9 +63,7 @@ clever_age_process: service: '@CleverAge\CacheProcessBundle\Task\SetTask' error_strategy: skip # A sku which is not a valid cache key is logged and skipped options: - adapter: 'catalog' - key: '' # Overridden by the input - value: ~ # Overridden by the input + adapter: 'catalog' # The key and the value are given by the input count_rows: service: '@CleverAge\ProcessBundle\Task\Reporting\StatCounterTask' @@ -79,10 +77,7 @@ clever_age_process: options: adapter: 'catalog' key: '{{ sku }}' - outputs: [skip_missing] - - skip_missing: - service: '@CleverAge\ProcessBundle\Task\SkipEmptyTask' + on_miss: skip outputs: [log] log: @@ -101,18 +96,17 @@ How it works: builds a `key` / `value` array with the [mapping](https://github.com/cleverage/process-bundle/blob/main/docs/reference/transformers/mapping_transformer.md) transformer (`code: '.'` maps the whole line). -- [SetTask](../reference/tasks/set_task.md) merges this array over its options: `key` and `value` placeholders are - replaced by the values of the current line, which is stored in the `catalog` adapter. With `error_strategy: skip`, +- [SetTask](../reference/tasks/set_task.md) merges this array over its options: the `key` and `value` of the current + line complete the configured `adapter`, and the line is stored in the `catalog` adapter. With `error_strategy: skip`, a `sku` containing a PSR-6 reserved character (`{}()/\@:`) is logged and the next line is processed. - [StatCounterTask](https://github.com/cleverage/process-bundle/blob/main/docs/reference/tasks/stat_counter_task.md) logs the number of stored lines at the end of the process. - In the second process, [GetTask](../reference/tasks/get_task.md) reads the key given by the `sku` context value (see [contextual values](https://github.com/cleverage/process-bundle/blob/main/docs/01-quick_start.md#contextual-values)). Since the pool is persistent, the lines stored by the first process are available until they expire. -- A missing key outputs `null`: - [SkipEmptyTask](https://github.com/cleverage/process-bundle/blob/main/docs/reference/tasks/skip_empty_task.md) stops - the branch, so the [LoggerTask](https://github.com/cleverage/process-bundle/blob/main/docs/reference/tasks/logger_task.md) - only logs found lines. +- With `on_miss: skip`, a missing key stops the branch, so the + [LoggerTask](https://github.com/cleverage/process-bundle/blob/main/docs/reference/tasks/logger_task.md) only logs + found lines. -Note that the items expire after the `default_lifetime` of the pool: schedule the warm up process more often than -this lifetime if the other processes must always find the data. +Note that the items expire after the `default_lifetime` of the pool (or the `expires_after` option of SetTask): +schedule the warm up process more often than this lifetime if the other processes must always find the data. diff --git a/docs/cookbooks/share_data_between_branches.md b/docs/cookbooks/share_data_between_branches.md index 97ecb38..8aebdce 100644 --- a/docs/cookbooks/share_data_between_branches.md +++ b/docs/cookbooks/share_data_between_branches.md @@ -74,9 +74,7 @@ clever_age_process: set: service: '@CleverAge\CacheProcessBundle\Task\SetTask' options: - adapter: 'memory' - key: '' # Overridden by the input - value: ~ # Overridden by the input + adapter: 'memory' # The key and the value are given by the input get: service: '@CleverAge\CacheProcessBundle\Task\GetTask' diff --git a/docs/index.md b/docs/index.md index 0977c82..4769796 100644 --- a/docs/index.md +++ b/docs/index.md @@ -41,8 +41,9 @@ services: `CleverAge\CacheProcessBundle\Task\AbstractCacheTask` can be extended to implement other cache operations. It extends [AbstractConfigurableTask](https://github.com/cleverage/process-bundle/blob/main/docs/03-custom_tasks.md), requires the `cleverage_cache_process.registry.adapter` service (`AdapterRegistry`) as constructor argument, defines the -required `adapter` and `key` string options, and provides `getMergedOptions()` (options merged with the array input, -resolved again so that the input values are validated) and `$this->registry->getAdapter($code)`. +`adapter` and `key` string options (required once merged with the input, see `getRequiredOptions()`), and provides +`getMergedOptions()` (options merged with the array input, resolved again so that the input values are validated, and +key validated) and `$this->registry->getAdapter($code)`. ```php is missing`) when the task is executed. -* The cache tasks do not handle any expiration: the lifetime of the items is the default lifetime of the decorated - pool (`default_lifetime` of a FrameworkBundle pool, `$defaultLifetime` constructor argument of Symfony adapters). +* The lifetime of the items is the default lifetime of the decorated pool (`default_lifetime` of a FrameworkBundle + pool, `$defaultLifetime` constructor argument of Symfony adapters), unless the `expires_after` option of + [SetTask](tasks/set_task.md) is set. * Cache keys must follow the PSR-6 rules: no empty key, and none of the reserved characters `{}()/\@:`. The keys are validated by the decorated pool, and Symfony adapters only validate them with `assert()`: an invalid key throws a `Psr\Cache\InvalidArgumentException` when assertions are enabled (`zend.assertions=1`, usual in development), but is - silently accepted when they are not (`zend.assertions=-1`, production `php.ini`). + silently accepted when they are not (`zend.assertions=-1`, production `php.ini`). The cache tasks validate the key + themselves, so an invalid key always throws when using them. * Only the PSR-6 methods are forwarded by the base class: tag-aware features of the decorated pool are not exposed. diff --git a/docs/reference/tasks/get_task.md b/docs/reference/tasks/get_task.md index 282446e..b5cb459 100644 --- a/docs/reference/tasks/get_task.md +++ b/docs/reference/tasks/get_task.md @@ -21,15 +21,27 @@ Any other non-empty input (e.g. a `string`) throws an `\UnexpectedValueException Possible outputs ---------------- -`mixed`: the value of the cache item, or `null` if the key is missing from the cache. +`mixed`: the value of the cache item. If the key is missing from the cache, depends on `on_miss`: `null` (default), no +output (the input is sent to the error outputs), or an exception. Options ------- -| Code | Type | Required | Default | Description | -|-----------|----------|:--------:|---------|-------------------------------------------------------------------------------------------| -| `adapter` | `string` | **X** | | Code of the [adapter](../adapter.md) to read from (see `AdapterInterface::getCode()`) | -| `key` | `string` | **X** | | Key of the cache item to read, must be a valid PSR-6 key (can be overridden by the input) | +| Code | Type | Required | Default | Description | +|-----------|----------|:--------:|---------------|---------------------------------------------------------------------------------------| +| `adapter` | `string` | **X** | | Code of the [adapter](../adapter.md) to read from (see `AdapterInterface::getCode()`) | +| `key` | `string` | **X** | | Key of the cache item to read, must be a valid PSR-6 key | +| `on_miss` | `string` | | `output_null` | Behaviour when the key is missing from the cache (see below) | + +Every option can be given by the configuration or by the input: `adapter` and `key` are required once merged with the +input, an option given by neither throws a `MissingOptionsException` on execution. + +`on_miss` values: + +* `output_null`: output `null`, as for an item stored with a `null` value. +* `skip`: skip the item and send the input to the error outputs (`error_outputs`), e.g. to compute the missing value + and store it. +* `fail`: throw an `\UnexpectedValueException` (`Cache item is missing from adapter `). Examples -------- @@ -55,10 +67,38 @@ get: options: adapter: 'catalog' key: '{{ sku }}' - outputs: [skip_missing] -skip_missing: - service: '@CleverAge\ProcessBundle\Task\SkipEmptyTask' + on_miss: skip + outputs: [debug] +``` + +* Compute and store the missing values ("cache-aside") + +```yaml +# Task configuration level +get: + service: '@CleverAge\CacheProcessBundle\Task\GetTask' + options: + adapter: 'catalog' + on_miss: skip # The input ({ key: ... }) is sent to the error outputs outputs: [debug] + error_outputs: [compute] +compute: + service: '@CleverAge\ProcessBundle\Task\TransformerTask' + options: + transformers: + mapping: + mapping: + key: + code: '[key]' + value: + code: '[key]' + transformers: + slugify: ~ # Any computation + outputs: [set] +set: + service: '@CleverAge\CacheProcessBundle\Task\SetTask' + options: + adapter: 'catalog' ``` * Read a key computed from the input @@ -77,18 +117,17 @@ build_key: get: service: '@CleverAge\CacheProcessBundle\Task\GetTask' options: - adapter: 'catalog' - key: '' # Overridden by the input + adapter: 'catalog' # The key is given by the input ``` Notes ----- -* `adapter` and `key` are required at configuration level, even when they are always given by the input: set them to - a placeholder value (e.g. `key: ''`). If the input does not override the placeholder key, the empty key is rejected - only when assertions are enabled (see [Adapter](../adapter.md#notes)): in production, the item stored under the - empty key is read. -* A missing key and an item stored with a `null` value both output `null`. Chain a - [SkipEmptyTask](https://github.com/cleverage/process-bundle/blob/main/docs/reference/tasks/skip_empty_task.md) to - stop the branch when nothing is found. +* The key is validated by the task (`CacheItem::validateKey()`): an empty key, or a key containing one of the PSR-6 + reserved characters `{}()/\@:`, throws a `Psr\Cache\InvalidArgumentException`, whatever the adapter and the + `zend.assertions` setting (see [Adapter](../adapter.md#notes)). +* A cache miss is detected with `CacheItemInterface::isHit()`: an item stored with a `null` value is a hit, and is + always output. +* With `on_miss: skip`, the next tasks of `outputs` are not executed for this input, but the tasks of `error_outputs` + are (whatever the `error_strategy`). * The input is replaced by the cached value: the rest of the input is not transmitted to the next tasks. diff --git a/docs/reference/tasks/set_task.md b/docs/reference/tasks/set_task.md index d0a30d8..23fe5f6 100644 --- a/docs/reference/tasks/set_task.md +++ b/docs/reference/tasks/set_task.md @@ -26,11 +26,15 @@ Possible outputs Options ------- -| Code | Type | Required | Default | Description | -|-----------|----------|:--------:|---------|--------------------------------------------------------------------------------------------| -| `adapter` | `string` | **X** | | Code of the [adapter](../adapter.md) to write to (see `AdapterInterface::getCode()`) | -| `key` | `string` | **X** | | Key of the cache item to store, must be a valid PSR-6 key (can be overridden by the input) | -| `value` | `mixed` | **X** | | Value to store, must be serializable by the adapter (can be overridden by the input) | +| Code | Type | Required | Default | Description | +|-----------------|---------------|:--------:|---------|----------------------------------------------------------------------------------------------| +| `adapter` | `string` | **X** | | Code of the [adapter](../adapter.md) to write to (see `AdapterInterface::getCode()`) | +| `key` | `string` | **X** | | Key of the cache item to store, must be a valid PSR-6 key | +| `value` | `mixed` | **X** | | Value to store (can be `null`), must be serializable by the adapter | +| `expires_after` | `int`, `null` | | `null` | Lifetime of the item in seconds (strictly positive), `null` for the default adapter lifetime | + +Every option can be given by the configuration or by the input: `adapter`, `key` and `value` are required once merged +with the input, an option given by neither throws a `MissingOptionsException` on execution. Examples -------- @@ -79,18 +83,26 @@ format: set: service: '@CleverAge\CacheProcessBundle\Task\SetTask' options: - adapter: 'memory' - key: '' # Overridden by the input - value: ~ # Overridden by the input + adapter: 'memory' # The key and the value are given by the input +``` + +* Store an item for one hour + +```yaml +# Task configuration level +set: + service: '@CleverAge\CacheProcessBundle\Task\SetTask' + options: + adapter: 'catalog' + expires_after: 3600 ``` Notes ----- -* `adapter`, `key` and `value` are required at configuration level, even when they are always given by the input: - set them to a placeholder value (e.g. `key: ''`, `value: ~`). If the input does not override the placeholder key, - the empty key is rejected only when assertions are enabled (see [Adapter](../adapter.md#notes)): in production, every - item is stored under the same empty key. -* No expiration is set on the item: its lifetime is the default lifetime of the adapter (see - [Adapter](../adapter.md#notes)). +* The key is validated by the task (`CacheItem::validateKey()`): an empty key, or a key containing one of the PSR-6 + reserved characters `{}()/\@:`, throws a `Psr\Cache\InvalidArgumentException`, whatever the adapter and the + `zend.assertions` setting (see [Adapter](../adapter.md#notes)). +* Without `expires_after`, the lifetime of the item is the default lifetime of the adapter (see + [Adapter](../adapter.md#notes)). Give `expires_after` in the input to set a lifetime per item. * The item is saved immediately (`save()`, not `saveDeferred()`), an existing item with the same key is overwritten. diff --git a/src/Task/AbstractCacheTask.php b/src/Task/AbstractCacheTask.php index 1544ac4..8ef3a2b 100644 --- a/src/Task/AbstractCacheTask.php +++ b/src/Task/AbstractCacheTask.php @@ -16,6 +16,8 @@ use CleverAge\CacheProcessBundle\Registry\AdapterRegistry; use CleverAge\ProcessBundle\Model\AbstractConfigurableTask; use CleverAge\ProcessBundle\Model\ProcessState; +use Psr\Cache\InvalidArgumentException; +use Symfony\Component\Cache\CacheItem; use Symfony\Component\OptionsResolver\Exception\AccessException; use Symfony\Component\OptionsResolver\Exception\ExceptionInterface; use Symfony\Component\OptionsResolver\Exception\UndefinedOptionsException; @@ -33,18 +35,30 @@ public function __construct(protected AdapterRegistry $registry) */ protected function configureOptions(OptionsResolver $resolver): void { - $resolver->setRequired(['adapter', 'key']); + // Can be given by the input only: required once merged with the input (see getRequiredOptions()) + $resolver->setDefined(['adapter', 'key']); $resolver->setAllowedTypes('adapter', ['string']); $resolver->setAllowedTypes('key', ['string']); } + /** + * Options required once merged with the input, they can be given by the configuration or by the input. + * + * @return list + */ + protected function getRequiredOptions(): array + { + return ['adapter', 'key']; + } + /** * Resolve the options merged with the input keys matching a defined option, the other input keys are ignored. * * @return array * * @throws ExceptionInterface + * @throws InvalidArgumentException */ protected function getMergedOptions(ProcessState $state): array { @@ -58,7 +72,15 @@ protected function getMergedOptions(ProcessState $state): array $resolver = new OptionsResolver(); $this->configureOptions($resolver); + $resolver->setRequired($this->getRequiredOptions()); + + $mergedOptions = $resolver->resolve(array_merge($options, array_intersect_key($input, array_flip($resolver->getDefinedOptions())))); + + // Symfony adapters only validate the keys with assert(): validate it in any environment + /** @var string $key */ + $key = $mergedOptions['key']; + CacheItem::validateKey($key); - return $resolver->resolve(array_merge($options, array_intersect_key($input, array_flip($resolver->getDefinedOptions())))); + return $mergedOptions; } } diff --git a/src/Task/GetTask.php b/src/Task/GetTask.php index 36ff51d..bbe4b64 100644 --- a/src/Task/GetTask.php +++ b/src/Task/GetTask.php @@ -14,15 +14,28 @@ namespace CleverAge\CacheProcessBundle\Task; use CleverAge\ProcessBundle\Model\ProcessState; +use Symfony\Component\OptionsResolver\Exception\AccessException; +use Symfony\Component\OptionsResolver\Exception\UndefinedOptionsException; +use Symfony\Component\OptionsResolver\OptionsResolver; /** * @phpstan-type Options array{ * adapter: string, - * key: string + * key: string, + * on_miss: string * } */ class GetTask extends AbstractCacheTask { + /** Output null on a cache miss, as for an item stored with a null value */ + public const ON_MISS_OUTPUT_NULL = 'output_null'; + + /** Skip the item and send the input to the error outputs on a cache miss */ + public const ON_MISS_SKIP = 'skip'; + + /** Throw an exception on a cache miss */ + public const ON_MISS_FAIL = 'fail'; + /** * @throws \Throwable */ @@ -32,7 +45,33 @@ public function execute(ProcessState $state): void $mergedOptions = $this->getMergedOptions($state); $cache = $this->registry->getAdapter($mergedOptions['adapter']); + $item = $cache->getItem($mergedOptions['key']); + + if (!$item->isHit()) { + if (self::ON_MISS_SKIP === $mergedOptions['on_miss']) { + $state->setErrorOutput($state->getInput()); + $state->setSkipped(true); + + return; + } + if (self::ON_MISS_FAIL === $mergedOptions['on_miss']) { + throw new \UnexpectedValueException("Cache item {$mergedOptions['key']} is missing from adapter {$mergedOptions['adapter']}"); + } + } + + $state->setOutput($item->get()); + } + + /** + * @throws UndefinedOptionsException + * @throws AccessException + */ + #[\Override] + protected function configureOptions(OptionsResolver $resolver): void + { + parent::configureOptions($resolver); - $state->setOutput($cache->getItem($mergedOptions['key'])->get()); + $resolver->setDefault('on_miss', self::ON_MISS_OUTPUT_NULL); + $resolver->setAllowedValues('on_miss', [self::ON_MISS_OUTPUT_NULL, self::ON_MISS_SKIP, self::ON_MISS_FAIL]); } } diff --git a/src/Task/SetTask.php b/src/Task/SetTask.php index 12e5501..3ed3822 100644 --- a/src/Task/SetTask.php +++ b/src/Task/SetTask.php @@ -22,7 +22,8 @@ * @phpstan-type Options array{ * adapter: string, * key: string, - * value: mixed + * value: mixed, + * expires_after: int|null * } */ class SetTask extends AbstractCacheTask @@ -38,6 +39,9 @@ public function execute(ProcessState $state): void $cache = $this->registry->getAdapter($mergedOptions['adapter']); $item = $cache->getItem($mergedOptions['key'])->set($mergedOptions['value']); + if (null !== $mergedOptions['expires_after']) { + $item->expiresAfter($mergedOptions['expires_after']); + } $cache->save($item); } @@ -51,6 +55,16 @@ protected function configureOptions(OptionsResolver $resolver): void { parent::configureOptions($resolver); - $resolver->setRequired(['value']); + $resolver->setDefined(['value']); + + $resolver->setDefault('expires_after', null); + $resolver->setAllowedTypes('expires_after', ['null', 'int']); + $resolver->setAllowedValues('expires_after', static fn (?int $value): bool => null === $value || $value > 0); + } + + #[\Override] + protected function getRequiredOptions(): array + { + return [...parent::getRequiredOptions(), 'value']; } } diff --git a/tests/Task/GetTaskTest.php b/tests/Task/GetTaskTest.php index fcc498c..fc04076 100644 --- a/tests/Task/GetTaskTest.php +++ b/tests/Task/GetTaskTest.php @@ -23,8 +23,10 @@ use CleverAge\ProcessBundle\Model\ProcessHistory; use CleverAge\ProcessBundle\Model\ProcessState; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; +use Psr\Cache\InvalidArgumentException; use Symfony\Component\Cache\Adapter\ArrayAdapter; use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; use Symfony\Component\OptionsResolver\Exception\MissingOptionsException; @@ -123,11 +125,92 @@ public function testMissingAdapter(): void $this->execute($task, $state, null); } - public function testRequiredOptionsAtInitialization(): void + public function testOptionsFromInputOnly(): void { + [$task, $state] = $this->createTask([]); + + self::assertSame('value1', $this->execute($task, $state, ['adapter' => 'memory', 'key' => 'key1'])); + } + + public function testRequiredOptionsOnExecution(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory']); + $this->expectException(MissingOptionsException::class); $this->expectExceptionMessage('The required option "key" is missing.'); - $this->createTask(['adapter' => 'memory']); + $this->execute($task, $state, ['sku' => 'ABC-001']); + } + + /** + * @return iterable + */ + public static function provideInvalidKeys(): iterable + { + yield 'empty' => ['']; + yield 'reserved character' => ['a/b']; + } + + /** + * Rejected by the task, also when assertions are disabled (Symfony adapters only validate the keys with assert()). + */ + #[DataProvider('provideInvalidKeys')] + public function testInvalidKey(string $key): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => $key]); + + $this->expectException(InvalidArgumentException::class); + $this->execute($task, $state, null); + } + + public function testOnMissSkip(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'on_miss' => 'skip']); + + $this->execute($task, $state, ['key' => 'missing', 'sku' => 'ABC-001']); + + self::assertTrue($state->isSkipped()); + self::assertTrue($state->hasErrorOutput()); + self::assertSame(['key' => 'missing', 'sku' => 'ABC-001'], $state->getErrorOutput()); + self::assertNull($state->getOutput()); + } + + public function testOnMissSkipWithHit(): void + { + $this->adapter->save($this->adapter->getItem('null')->set(null)); + [$task, $state] = $this->createTask(['adapter' => 'memory', 'on_miss' => 'skip']); + + self::assertSame('value1', $this->execute($task, $state, ['key' => 'key1'])); + self::assertFalse($state->isSkipped()); + self::assertFalse($state->hasErrorOutput()); + + // A stored null value is a hit + self::assertNull($this->execute($task, $state, ['key' => 'null'])); + self::assertFalse($state->isSkipped()); + self::assertFalse($state->hasErrorOutput()); + } + + public function testOnMissFail(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => 'missing', 'on_miss' => 'fail']); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('Cache item missing is missing from adapter memory'); + $this->execute($task, $state, null); + } + + public function testOnMissOutputNullByDefault(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => 'missing']); + + self::assertNull($this->execute($task, $state, null)); + self::assertFalse($state->isSkipped()); + self::assertFalse($state->hasErrorOutput()); + } + + public function testInvalidOnMissAtInitialization(): void + { + $this->expectException(InvalidOptionsException::class); + $this->createTask(['adapter' => 'memory', 'key' => 'key1', 'on_miss' => 'ignore']); } public function testUndefinedOptionAtInitialization(): void diff --git a/tests/Task/SetTaskTest.php b/tests/Task/SetTaskTest.php index c2f87f7..858f50e 100644 --- a/tests/Task/SetTaskTest.php +++ b/tests/Task/SetTaskTest.php @@ -23,9 +23,14 @@ use CleverAge\ProcessBundle\Model\ProcessHistory; use CleverAge\ProcessBundle\Model\ProcessState; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; +use Psr\Cache\CacheItemInterface; +use Psr\Cache\InvalidArgumentException; +use Symfony\Component\Cache\Adapter\AdapterInterface as SymfonyAdapterInterface; use Symfony\Component\Cache\Adapter\ArrayAdapter; +use Symfony\Component\Cache\CacheItem; use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; use Symfony\Component\OptionsResolver\Exception\MissingOptionsException; use Symfony\Component\OptionsResolver\Exception\UndefinedOptionsException; @@ -38,9 +43,28 @@ class SetTaskTest extends TestCase { private Adapter $adapter; + private ?CacheItemInterface $savedItem = null; + protected function setUp(): void { - $this->adapter = new Adapter(new ArrayAdapter(), 'memory'); + // Keep the last saved item, to check its expiry + $onSave = function (CacheItemInterface $item): void { + $this->savedItem = $item; + }; + $this->adapter = new class(new ArrayAdapter(), 'memory', $onSave) extends Adapter { + public function __construct(SymfonyAdapterInterface $adapter, string $code, private readonly \Closure $onSave) + { + parent::__construct($adapter, $code); + } + + #[\Override] + public function save(CacheItemInterface $item): bool + { + ($this->onSave)($item); + + return parent::save($item); + } + }; } public function testSetValue(): void @@ -128,11 +152,78 @@ public function testMissingAdapter(): void $this->execute($task, $state, null); } - public function testRequiredOptionsAtInitialization(): void + public function testOptionsFromInputOnly(): void + { + [$task, $state] = $this->createTask([]); + + $this->execute($task, $state, ['adapter' => 'memory', 'key' => 'key1', 'value' => 'value1']); + + self::assertSame('value1', $this->adapter->getItem('key1')->get()); + } + + public function testRequiredOptionsOnExecution(): void { + [$task, $state] = $this->createTask(['adapter' => 'memory']); + $this->expectException(MissingOptionsException::class); $this->expectExceptionMessage('The required option "value" is missing.'); - $this->createTask(['adapter' => 'memory', 'key' => 'key1']); + $this->execute($task, $state, ['key' => 'key1']); + } + + /** + * Rejected by the task, also when assertions are disabled (Symfony adapters only validate the keys with assert()). + */ + public function testInvalidKey(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => '']); + + $this->expectException(InvalidArgumentException::class); + $this->execute($task, $state, ['value' => 'value1']); + } + + public function testExpiresAfter(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => 'key1', 'value' => 'value1', 'expires_after' => 60]); + + $this->execute($task, $state, null); + + self::assertEqualsWithDelta(time() + 60, $this->getSavedExpiry(), 2); + } + + public function testExpiresAfterFromInput(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'value' => 'value1']); + + $this->execute($task, $state, ['key' => 'key1', 'expires_after' => 3600]); + + self::assertEqualsWithDelta(time() + 3600, $this->getSavedExpiry(), 2); + } + + public function testNoExpirationByDefault(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => 'key1', 'value' => 'value1']); + + $this->execute($task, $state, null); + + // Default lifetime of the adapter + self::assertNull($this->getSavedExpiry()); + } + + /** + * @return iterable + */ + public static function provideInvalidExpiresAfter(): iterable + { + yield 'zero' => [0]; + yield 'negative' => [-60]; + yield 'string' => ['60']; + } + + #[DataProvider('provideInvalidExpiresAfter')] + public function testInvalidExpiresAfterAtInitialization(mixed $expiresAfter): void + { + $this->expectException(InvalidOptionsException::class); + $this->createTask(['adapter' => 'memory', 'key' => 'key1', 'value' => 'value1', 'expires_after' => $expiresAfter]); } public function testUndefinedOptionAtInitialization(): void @@ -141,6 +232,19 @@ public function testUndefinedOptionAtInitialization(): void $this->createTask(['adapter' => 'memory', 'key' => 'key1', 'value' => 'value1', 'ttl' => 60]); } + /** + * Expiry timestamp of the last item saved by the task (the PSR-6 items do not expose it). + */ + private function getSavedExpiry(): float|int|null + { + self::assertInstanceOf(CacheItem::class, $this->savedItem); + + /** @var float|int|null $expiry */ + $expiry = (new \ReflectionProperty(CacheItem::class, 'expiry'))->getValue($this->savedItem); + + return $expiry; + } + /** * @param array $options * @param array $context