#204 - Fix CachedTransformer, SlugifyTransformer and TypeSetterTransformer edge cases

CHANGELOG.md

@@ -14,6 +14,7 @@ Latest
 * [#200](https://github.com/cleverage/process-bundle/issues/200) Fix GenericTransformer: a contextual option declared with `required: false` and no default can now be used; its placeholders are replaced by `null` when omitted. Update documentation, add tests.
 * [#202](https://github.com/cleverage/process-bundle/issues/202) Fix XmlReaderTask: throw an explicit `\UnexpectedValueException` on an empty file or invalid XML (with the libxml error messages) instead of a `ValueError` or a silent empty `\DOMDocument`. Update documentation, add tests.
 * [#203](https://github.com/cleverage/process-bundle/issues/203) Fix `ProcessState::addErrorContextValue()`: accept any value type, so that iterable tasks on an empty iterable are skipped and PropertySetterTask reports the original exception instead of a `TypeError`. 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.

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.

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.

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).

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);
 

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;
+            }
         );
     }
 }

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

tests/Transformer/CachedTransformerTest.php

@@ -0,0 +1,111 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/ProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\ProcessBundle\Tests\Transformer;
+
+use CleverAge\ProcessBundle\Registry\TransformerRegistry;
+use CleverAge\ProcessBundle\Transformer\CachedTransformer;
+use CleverAge\ProcessBundle\Transformer\CastTransformer;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
+use Symfony\Component\Cache\Adapter\ArrayAdapter;
+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')]
+#[\PHPUnit\Framework\Attributes\CoversMethod(CachedTransformer::class, 'getCode')]
+class CachedTransformerTest extends TestCase
+{
+    private ArrayAdapter $cache;
+
+    private CachedTransformer $transformer;
+
+    protected function setUp(): void
+    {
+        $registry = new TransformerRegistry();
+        $registry->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<string, mixed> $options
+     *
+     * @return array<string, mixed>
+     */
+    private function resolveOptions(array $options): array
+    {
+        $resolver = new OptionsResolver();
+        $this->transformer->configureOptions($resolver);
+
+        return $resolver->resolve($options);
+    }
+}

tests/Transformer/String/SlugifyTransformerTest.php

@@ -0,0 +1,72 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/ProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\ProcessBundle\Tests\Transformer\String;
+
+use CleverAge\ProcessBundle\Transformer\String\SlugifyTransformer;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException;
+use Symfony\Component\OptionsResolver\OptionsResolver;
+
+#[\PHPUnit\Framework\Attributes\CoversClass(SlugifyTransformer::class)]
+#[\PHPUnit\Framework\Attributes\CoversMethod(SlugifyTransformer::class, 'transform')]
+#[\PHPUnit\Framework\Attributes\CoversMethod(SlugifyTransformer::class, 'configureOptions')]
+#[\PHPUnit\Framework\Attributes\CoversMethod(SlugifyTransformer::class, 'getCode')]
+class SlugifyTransformerTest extends TestCase
+{
+    public function testTransformWithDefaultOptions(): void
+    {
+        $transformer = new SlugifyTransformer();
+
+        $this->assertSame('helene_dupont', $transformer->transform(' Hélène  <b>Dupont</b>! ', $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<string, mixed> $options
+     *
+     * @return array<string, mixed>
+     */
+    private function resolveOptions(SlugifyTransformer $transformer, array $options = []): array
+    {
+        $resolver = new OptionsResolver();
+        $transformer->configureOptions($resolver);
+
+        return $resolver->resolve($options);
+    }
+}

tests/Transformer/TypeSetterTransformerTest.php

@@ -0,0 +1,86 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/ProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\ProcessBundle\Tests\Transformer;
+
+use CleverAge\ProcessBundle\Transformer\TypeSetterTransformer;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException;
+use Symfony\Component\OptionsResolver\OptionsResolver;
+
+#[\PHPUnit\Framework\Attributes\CoversClass(TypeSetterTransformer::class)]
+#[\PHPUnit\Framework\Attributes\CoversMethod(TypeSetterTransformer::class, 'transform')]
+#[\PHPUnit\Framework\Attributes\CoversMethod(TypeSetterTransformer::class, 'configureOptions')]
+#[\PHPUnit\Framework\Attributes\CoversMethod(TypeSetterTransformer::class, 'getCode')]
+class TypeSetterTransformerTest extends TestCase
+{
+    /**
+     * @return iterable<string, array{mixed, string, mixed}>
+     */
+    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<string, mixed>
+     */
+    private function resolveOptions(TypeSetterTransformer $transformer, string $type): array
+    {
+        $resolver = new OptionsResolver();
+        $transformer->configureOptions($resolver);
+
+        return $resolver->resolve(['type' => $type]);
+    }
+}