From 7f51f1619850545609e016dfc6473a7d2b6d007e Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Mon, 28 Sep 2026 12:04:18 +0200 Subject: [PATCH 1/3] fix(transformer) #204 Fix CachedTransformer, SlugifyTransformer and TypeSetterTransformer edge cases: accept a non-string input in `cached` (key built after `key_transformers`), reject an invalid `transliterator` in `slugify` with an `InvalidOptionsException`, remove the unreachable error branch of `type_setter`. Update documentation, add tests. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + .../transformers/cached_transformer.md | 9 +- .../transformers/slugify_transformer.md | 9 +- .../transformers/type_setter_transformer.md | 4 +- src/Transformer/CachedTransformer.php | 2 +- src/Transformer/String/SlugifyTransformer.php | 10 +- src/Transformer/TypeSetterTransformer.php | 9 +- tests/Transformer/CachedTransformerTest.php | 109 ++++++++++++++++++ .../String/SlugifyTransformerTest.php | 72 ++++++++++++ .../Transformer/TypeSetterTransformerTest.php | 86 ++++++++++++++ 10 files changed, 296 insertions(+), 15 deletions(-) create mode 100644 tests/Transformer/CachedTransformerTest.php create mode 100644 tests/Transformer/String/SlugifyTransformerTest.php create mode 100644 tests/Transformer/TypeSetterTransformerTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index e918fb94..79d8df13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Latest * [#192](https://github.com/cleverage/process-bundle/issues/192) Fix CommandRunnerTask: only pass the `options` option to `Process::setOptions()`, support string `commandline` through `Process::fromShellCommandline()`, validate option types. Update documentation, add tests. * [#194](https://github.com/cleverage/process-bundle/issues/194) Fix ProcessLauncherTask: the `process_options` normalizer returned an array despite its scalar return type, so the task always failed with a `TypeError`. Update documentation, add tests. * [#189](https://github.com/cleverage/process-bundle/issues/189) Fix EventDispatcherTask: dispatch the event under the `event_name` option (regression since v4.0). `event_name` is now optional: when `null` (default), the event is dispatched under its class name. Update documentation, add tests. +* [#204](https://github.com/cleverage/process-bundle/issues/204) Fix CachedTransformer, SlugifyTransformer and TypeSetterTransformer edge cases: accept a non-string input in `cached` (key built after `key_transformers`), reject an invalid `transliterator` in `slugify` with an `InvalidOptionsException`, remove the unreachable error branch of `type_setter`. Update documentation, add tests. ## Deprecated * [#189](https://github.com/cleverage/process-bundle/issues/189) EventDispatcherTask: when `event_name` is set, listening to `CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent` is deprecated (the event is still dispatched under its class name, with an `E_USER_DEPRECATED` error, if it has listeners). Listen to the configured `event_name` instead: the BC layer will be removed in v6.0. diff --git a/docs/reference/transformers/cached_transformer.md b/docs/reference/transformers/cached_transformer.md index ad14de6a..19dffa16 100644 --- a/docs/reference/transformers/cached_transformer.md +++ b/docs/reference/transformers/cached_transformer.md @@ -14,8 +14,8 @@ Transformer reference Accepted inputs --------------- -`string`: the value is used both to build the cache key (after `key_transformers`) and as input of `transformers`. -Any other type raises a `TypeError`. +`any`: the value is used both to build the cache key (after `key_transformers`) and as input of `transformers`. +The key value must be a `string` once `key_transformers` are applied, otherwise the cache is bypassed (see Notes). Possible outputs ---------------- @@ -56,5 +56,6 @@ Notes application). Items are saved with `saveDeferred()`, a warning is logged if the save fails. * A string `ttl` is converted to a date when the options are resolved (i.e. once, when the transformer is configured), not each time an item is saved: all items share the same absolute expiration date. -* If the key value is not a string after `key_transformers`, or if the cache pool raises a PSR-6 - `InvalidArgumentException` (logged as a warning), the transformers are applied without cache. +* If the key value is not a string after `key_transformers` (e.g. an `int` input without a `cast` key transformer), + or if the cache pool raises a PSR-6 `InvalidArgumentException` (logged as a warning), the transformers are applied + without cache. diff --git a/docs/reference/transformers/slugify_transformer.md b/docs/reference/transformers/slugify_transformer.md index 8476aa6a..e3441c68 100644 --- a/docs/reference/transformers/slugify_transformer.md +++ b/docs/reference/transformers/slugify_transformer.md @@ -29,7 +29,7 @@ Options | Code | Type | Required | Default | Description | |------------------|----------|:--------:|------------------------------------------|-----------------------------------------------------------------------------------------| -| `transliterator` | `string` | | `'NFD; [:Nonspacing Mark:] Remove; NFC'` | Transliterator identifier, passed to `\Transliterator::create()` | +| `transliterator` | `string` | | `'NFD; [:Nonspacing Mark:] Remove; NFC'` | Transliterator identifier, passed to `\Transliterator::create()` (see Notes) | | `replace` | `string` | | `'/[^a-z0-9]+/'` | Regular expression of the characters to replace (applied on the lowercased string) | | `separator` | `string` | | `'_'` | Replacement string, also trimmed from both ends of the result | @@ -51,3 +51,10 @@ slugify: transliterator: 'Any-Latin; Latin-ASCII' separator: '-' ``` + +Notes +----- + +An invalid `transliterator` identifier (rejected by `\Transliterator::create()`) raises an +`InvalidOptionsException` when the options are resolved, i.e. when the transformer is configured, not on the first +transformed value. diff --git a/docs/reference/transformers/type_setter_transformer.md b/docs/reference/transformers/type_setter_transformer.md index b83fa713..0ef17802 100644 --- a/docs/reference/transformers/type_setter_transformer.md +++ b/docs/reference/transformers/type_setter_transformer.md @@ -39,4 +39,6 @@ type_setter: Notes ----- -A `TransformerException` is thrown if `settype()` returns `false`. +An unsupported `type` raises an `InvalidOptionsException` when the options are resolved. Conversion errors follow +`settype()` semantics (e.g. converting an object to `int` emits a warning, converting an array to `string` gives +`'Array'` with a warning). diff --git a/src/Transformer/CachedTransformer.php b/src/Transformer/CachedTransformer.php index d4bd385c..70e7bf46 100644 --- a/src/Transformer/CachedTransformer.php +++ b/src/Transformer/CachedTransformer.php @@ -104,7 +104,7 @@ public function getCode(): string return 'cached'; } - protected function generateCacheKey(string $cacheKeyRoot, string $value, array $options): bool|string + protected function generateCacheKey(string $cacheKeyRoot, mixed $value, array $options): bool|string { $value = $this->applyTransformers($options['key_transformers'], $value); diff --git a/src/Transformer/String/SlugifyTransformer.php b/src/Transformer/String/SlugifyTransformer.php index 7d23bf92..22dcfd64 100644 --- a/src/Transformer/String/SlugifyTransformer.php +++ b/src/Transformer/String/SlugifyTransformer.php @@ -14,6 +14,7 @@ namespace CleverAge\ProcessBundle\Transformer\String; use CleverAge\ProcessBundle\Transformer\ConfigurableTransformerInterface; +use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; use Symfony\Component\OptionsResolver\Options; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -58,7 +59,14 @@ public function configureOptions(OptionsResolver $resolver): void $resolver->setNormalizer( 'transliterator', - static fn (Options $options, $value): ?\Transliterator => \Transliterator::create($value) + static function (Options $options, $value): \Transliterator { + $transliterator = \Transliterator::create($value); + if (null === $transliterator) { + throw new InvalidOptionsException(\sprintf('Invalid "transliterator" option: %s', intl_get_error_message())); + } + + return $transliterator; + } ); } } diff --git a/src/Transformer/TypeSetterTransformer.php b/src/Transformer/TypeSetterTransformer.php index 64804d8a..606c62ff 100644 --- a/src/Transformer/TypeSetterTransformer.php +++ b/src/Transformer/TypeSetterTransformer.php @@ -13,7 +13,6 @@ namespace CleverAge\ProcessBundle\Transformer; -use CleverAge\ProcessBundle\Exception\TransformerException; use Symfony\Component\OptionsResolver\OptionsResolver; class TypeSetterTransformer implements ConfigurableTransformerInterface @@ -30,13 +29,9 @@ public function configureOptions(OptionsResolver $resolver): void public function transform(mixed $value, array $options = []): mixed { - $return = settype($value, $options['type']); + settype($value, $options['type']); - if ($return) { - return $value; - } - - throw new TransformerException("Failed to change value type in {$options['type']}"); + return $value; } public function getCode(): string diff --git a/tests/Transformer/CachedTransformerTest.php b/tests/Transformer/CachedTransformerTest.php new file mode 100644 index 00000000..85fc5731 --- /dev/null +++ b/tests/Transformer/CachedTransformerTest.php @@ -0,0 +1,109 @@ +addTransformer(new CastTransformer()); + $this->cache = new ArrayAdapter(); + $this->transformer = new CachedTransformer($registry, $this->cache, new NullLogger()); + } + + public function testTransformStringValueIsCached(): void + { + $options = $this->resolveOptions(['cache_key' => 'prefix']); + + $this->assertSame('foo bar', $this->transformer->transform('foo bar', $options)); + $this->cache->commit(); + + $item = $this->cache->getItem('prefix|foo%20bar'); + $this->assertTrue($item->isHit()); + $this->assertSame('foo bar', $item->get()); + } + + public function testTransformReturnsCachedValueOnHit(): void + { + $this->cache->save($this->cache->getItem('prefix|foo')->set('from cache')); + $options = $this->resolveOptions(['cache_key' => 'prefix']); + + $this->assertSame('from cache', $this->transformer->transform('foo', $options)); + } + + public function testTransformNonStringValueWithKeyTransformers(): void + { + $options = $this->resolveOptions([ + 'cache_key' => 'prefix', + 'key_transformers' => ['cast' => ['type' => 'string']], + 'transformers' => ['cast' => ['type' => 'float']], + ]); + + $this->assertSame(42.0, $this->transformer->transform(42, $options)); + $this->cache->commit(); + + $item = $this->cache->getItem('prefix|42'); + $this->assertTrue($item->isHit()); + $this->assertSame(42.0, $item->get()); + } + + public function testTransformNonStringKeyValueBypassesCache(): void + { + $options = $this->resolveOptions([ + 'cache_key' => 'prefix', + 'transformers' => ['cast' => ['type' => 'string']], + ]); + + $this->assertSame('42', $this->transformer->transform(42, $options)); + $this->cache->commit(); + + $this->assertSame([], $this->cache->getValues()); + } + + public function testGetCodeReturnsCorrectCode(): void + { + $this->assertSame('cached', $this->transformer->getCode()); + } + + /** + * @param array $options + * + * @return array + */ + private function resolveOptions(array $options): array + { + $resolver = new OptionsResolver(); + $this->transformer->configureOptions($resolver); + + return $resolver->resolve($options); + } +} diff --git a/tests/Transformer/String/SlugifyTransformerTest.php b/tests/Transformer/String/SlugifyTransformerTest.php new file mode 100644 index 00000000..99e6efed --- /dev/null +++ b/tests/Transformer/String/SlugifyTransformerTest.php @@ -0,0 +1,72 @@ +assertSame('helene_dupont', $transformer->transform(' Hélène Dupont! ', $this->resolveOptions($transformer))); + } + + public function testTransformWithCustomTransliteratorAndSeparator(): void + { + $transformer = new SlugifyTransformer(); + $options = $this->resolveOptions($transformer, [ + 'transliterator' => 'Any-Latin; Latin-ASCII', + 'separator' => '-', + ]); + + $this->assertSame('privet-mir', $transformer->transform('Привет мир', $options)); + } + + public function testConfigureOptionsRejectsInvalidTransliterator(): void + { + $transformer = new SlugifyTransformer(); + + $this->expectException(InvalidOptionsException::class); + $this->expectExceptionMessage('Invalid "transliterator" option'); + + $this->resolveOptions($transformer, ['transliterator' => 'Not-A-Real-Transliterator']); + } + + public function testGetCodeReturnsCorrectCode(): void + { + $this->assertSame('slugify', new SlugifyTransformer()->getCode()); + } + + /** + * @param array $options + * + * @return array + */ + private function resolveOptions(SlugifyTransformer $transformer, array $options = []): array + { + $resolver = new OptionsResolver(); + $transformer->configureOptions($resolver); + + return $resolver->resolve($options); + } +} diff --git a/tests/Transformer/TypeSetterTransformerTest.php b/tests/Transformer/TypeSetterTransformerTest.php new file mode 100644 index 00000000..3cf7040e --- /dev/null +++ b/tests/Transformer/TypeSetterTransformerTest.php @@ -0,0 +1,86 @@ + + */ + public static function typesProvider(): iterable + { + yield 'string to int' => ['123', 'int', 123]; + yield 'string to integer' => ['12abc', 'integer', 12]; + yield 'string to float' => ['1.5', 'float', 1.5]; + yield 'string to double' => ['2', 'double', 2.0]; + yield 'int to string' => [123, 'string', '123']; + yield 'string to bool' => ['0', 'bool', false]; + yield 'int to boolean' => [1, 'boolean', true]; + yield 'scalar to array' => ['foo', 'array', ['foo']]; + yield 'value to null' => ['foo', 'null', null]; + } + + #[DataProvider('typesProvider')] + public function testTransform(mixed $value, string $type, mixed $expected): void + { + $transformer = new TypeSetterTransformer(); + + $this->assertSame($expected, $transformer->transform($value, $this->resolveOptions($transformer, $type))); + } + + public function testTransformToObject(): void + { + $transformer = new TypeSetterTransformer(); + + $result = $transformer->transform(['foo' => 'bar'], $this->resolveOptions($transformer, 'object')); + + $this->assertInstanceOf(\stdClass::class, $result); + $this->assertSame('bar', $result->foo); + } + + public function testConfigureOptionsRejectsInvalidType(): void + { + $transformer = new TypeSetterTransformer(); + + $this->expectException(InvalidOptionsException::class); + + $this->resolveOptions($transformer, 'resource'); + } + + public function testGetCodeReturnsCorrectCode(): void + { + $this->assertSame('type_setter', new TypeSetterTransformer()->getCode()); + } + + /** + * @return array + */ + private function resolveOptions(TypeSetterTransformer $transformer, string $type): array + { + $resolver = new OptionsResolver(); + $transformer->configureOptions($resolver); + + return $resolver->resolve(['type' => $type]); + } +} From 7ad76ed7b51a0e9f9d985dd7dd5ad3a9edc941aa Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Mon, 28 Sep 2026 13:59:08 +0200 Subject: [PATCH 2/3] test #204 Declare the classes used by the new tests (UsesClass), fixing risky tests when coverage is enabled Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/Transformer/CachedTransformerTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Transformer/CachedTransformerTest.php b/tests/Transformer/CachedTransformerTest.php index 85fc5731..da79df88 100644 --- a/tests/Transformer/CachedTransformerTest.php +++ b/tests/Transformer/CachedTransformerTest.php @@ -22,6 +22,8 @@ use Symfony\Component\OptionsResolver\OptionsResolver; #[\PHPUnit\Framework\Attributes\CoversClass(CachedTransformer::class)] +#[\PHPUnit\Framework\Attributes\UsesClass(TransformerRegistry::class)] +#[\PHPUnit\Framework\Attributes\UsesClass(CastTransformer::class)] #[\PHPUnit\Framework\Attributes\CoversMethod(CachedTransformer::class, 'transform')] #[\PHPUnit\Framework\Attributes\CoversMethod(CachedTransformer::class, 'generateCacheKey')] #[\PHPUnit\Framework\Attributes\CoversMethod(CachedTransformer::class, 'configureOptions')] From 3ff82d5d5d4dfa6ddc05dfecc8c61002668933d9 Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Mon, 28 Sep 2026 14:07:22 +0200 Subject: [PATCH 3/3] test #204 Wrap new expressions in parentheses: new Foo()->bar() requires PHP 8.4, the bundle supports PHP >= 8.2 Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/Transformer/String/SlugifyTransformerTest.php | 2 +- tests/Transformer/TypeSetterTransformerTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Transformer/String/SlugifyTransformerTest.php b/tests/Transformer/String/SlugifyTransformerTest.php index 99e6efed..be620304 100644 --- a/tests/Transformer/String/SlugifyTransformerTest.php +++ b/tests/Transformer/String/SlugifyTransformerTest.php @@ -54,7 +54,7 @@ public function testConfigureOptionsRejectsInvalidTransliterator(): void public function testGetCodeReturnsCorrectCode(): void { - $this->assertSame('slugify', new SlugifyTransformer()->getCode()); + $this->assertSame('slugify', (new SlugifyTransformer())->getCode()); } /** diff --git a/tests/Transformer/TypeSetterTransformerTest.php b/tests/Transformer/TypeSetterTransformerTest.php index 3cf7040e..c1290576 100644 --- a/tests/Transformer/TypeSetterTransformerTest.php +++ b/tests/Transformer/TypeSetterTransformerTest.php @@ -70,7 +70,7 @@ public function testConfigureOptionsRejectsInvalidType(): void public function testGetCodeReturnsCorrectCode(): void { - $this->assertSame('type_setter', new TypeSetterTransformer()->getCode()); + $this->assertSame('type_setter', (new TypeSetterTransformer())->getCode()); } /**