diff --git a/CHANGELOG.md b/CHANGELOG.md index f469925..fc5571d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ Latest ------ +### Fixes +* [#22](https://github.com/cleverage/cache-process-bundle/issues/22) Fix GetTask and SetTask: throw an explicit `\UnexpectedValueException` on a non-array input (a `\TypeError` was triggered by `array_merge()`). Update documentation, add tests. +* [#23](https://github.com/cleverage/cache-process-bundle/issues/23) Fix GetTask and SetTask: validate the option values given by the input with the options resolver (they were used as is). Update documentation, add tests. + v2.1 ------ diff --git a/docs/index.md b/docs/index.md index eee5bc4..0977c82 100644 --- a/docs/index.md +++ b/docs/index.md @@ -41,8 +41,8 @@ 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) -and `$this->registry->getAdapter($code)`. +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)`. ```php + * + * @throws ExceptionInterface */ protected function getMergedOptions(ProcessState $state): array { /** @var array $options */ $options = $this->getOptions($state); - /** @var array $input */ $input = $state->getInput() ?: []; + if (!\is_array($input)) { + throw new \UnexpectedValueException(\sprintf('%s expects an array or null input, %s given', (new \ReflectionClass($this))->getShortName(), get_debug_type($input))); + } - return array_merge($options, $input); + $resolver = new OptionsResolver(); + $this->configureOptions($resolver); + + return $resolver->resolve(array_merge($options, array_intersect_key($input, array_flip($resolver->getDefinedOptions())))); } } diff --git a/tests/Task/GetTaskTest.php b/tests/Task/GetTaskTest.php new file mode 100644 index 0000000..5942993 --- /dev/null +++ b/tests/Task/GetTaskTest.php @@ -0,0 +1,113 @@ +adapter = new Adapter(new ArrayAdapter(), 'memory'); + $this->adapter->save($this->adapter->getItem('key1')->set('value1')); + $this->adapter->save($this->adapter->getItem('key2')->set('value2')); + } + + public function testGetValue(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => 'key1']); + + self::assertSame('value1', $this->execute($task, $state, null)); + } + + public function testGetMissingValue(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => 'missing']); + + self::assertNull($this->execute($task, $state, null)); + } + + public function testInputOverridesOptions(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => '']); + + self::assertSame('value1', $this->execute($task, $state, ['key' => 'key1', 'sku' => 'ignored'])); + self::assertSame('value2', $this->execute($task, $state, ['key' => 'key2'])); + } + + public function testNonArrayInputIsRejected(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => '']); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('GetTask expects an array or null input, string given'); + $this->execute($task, $state, 'key1'); + } + + public function testInputValuesAreValidated(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => '']); + + $this->expectException(InvalidOptionsException::class); + $this->expectExceptionMessage('The option "key" with value 1 is expected to be of type "string", but is of type "int".'); + $this->execute($task, $state, ['key' => 1]); + } + + /** + * @param array $options + * + * @return array{GetTask, ProcessState} + */ + private function createTask(array $options): array + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('get', GetTask::class, $options)); + + $registry = new AdapterRegistry(); + $registry->addAdapter($this->adapter); + $task = new GetTask($registry); + $task->initialize($state); + + return [$task, $state]; + } + + private function execute(GetTask $task, ProcessState $state, mixed $input): mixed + { + $state->reset(false); + $state->setInput($input); + $task->execute($state); + + return $state->getOutput(); + } +} diff --git a/tests/Task/SetTaskTest.php b/tests/Task/SetTaskTest.php new file mode 100644 index 0000000..51f9654 --- /dev/null +++ b/tests/Task/SetTaskTest.php @@ -0,0 +1,107 @@ +adapter = new Adapter(new ArrayAdapter(), 'memory'); + } + + public function testSetValue(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => 'key1', 'value' => ['column1' => 'value1']]); + + $this->execute($task, $state, null); + + self::assertSame(['column1' => 'value1'], $this->adapter->getItem('key1')->get()); + } + + public function testInputOverridesOptions(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => '', 'value' => null]); + + $this->execute($task, $state, ['key' => 'key1', 'value' => 'value1', 'sku' => 'ignored']); + $this->execute($task, $state, ['key' => 'key2', 'value' => 'value2']); + + self::assertSame('value1', $this->adapter->getItem('key1')->get()); + self::assertSame('value2', $this->adapter->getItem('key2')->get()); + } + + public function testNonArrayInputIsRejected(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => 'key1', 'value' => null]); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('SetTask expects an array or null input, string given'); + $this->execute($task, $state, 'value1'); + } + + public function testInputValuesAreValidated(): void + { + [$task, $state] = $this->createTask(['adapter' => 'memory', 'key' => '', 'value' => null]); + + $this->expectException(InvalidOptionsException::class); + $this->expectExceptionMessage('The option "adapter" with value 1 is expected to be of type "string", but is of type "int".'); + $this->execute($task, $state, ['adapter' => 1, 'key' => 'key1', 'value' => 'value1']); + } + + /** + * @param array $options + * + * @return array{SetTask, ProcessState} + */ + private function createTask(array $options): array + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('set', SetTask::class, $options)); + + $registry = new AdapterRegistry(); + $registry->addAdapter($this->adapter); + $task = new SetTask($registry); + $task->initialize($state); + + return [$task, $state]; + } + + private function execute(SetTask $task, ProcessState $state, mixed $input): void + { + $state->reset(false); + $state->setInput($input); + $task->execute($state); + } +}