diff --git a/CHANGELOG.md b/CHANGELOG.md index 27b10deb..18d4b0fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Latest * [#367](https://github.com/cleverage/process-bundle/issues/367) `ProcessLauncherTask` (`process_launcher`) with `json_buffering`: when the sub-process writes nothing into its JSON buffer (no `end_point`, or an output that is not an array), the task outputs nothing again, as before 5.5, instead of the path of an empty file, and the empty buffer is removed. The buffer of a failed sub-process, and those of the still running sub-processes stopped because of it, are removed too (one file was left in the temporary directory for each launch). Update documentation, add tests. * [#368](https://github.com/cleverage/process-bundle/issues/368) `YamlWriterTask` (`yaml_writer`): the parent directory is created for a local path only (no scheme, or `file://`), not for a stream wrapper (`compress.zlib://`, `s3://`...), which failed, or `php://stdout`, which created a `php:` directory. Update documentation, add tests. * [#369](https://github.com/cleverage/process-bundle/issues/369) `StopTask` (`stop`): a process that has already failed (e.g. after a task initialization failure, going on being deprecated) stays failed instead of being flagged as stopped: the `Process failed` log is written at the `failed_level` and `ProcessEvent::isProcessStopped()` is `false`, as before #337. Update documentation, add tests. +* [#370](https://github.com/cleverage/process-bundle/issues/370) `RulesTransformer` (`rules`): the result of a `condition` is cast to `bool`, so a condition returning an `int` (`preg_match()`), a string, `null` or any other non-boolean value (a single variable) no longer throws a `TypeError`: the rule matches if the result is truthy. Update documentation, add tests. v5.5 ----- diff --git a/docs/reference/transformers/rules_transformer.md b/docs/reference/transformers/rules_transformer.md index 04589890..b4c8f46f 100644 --- a/docs/reference/transformers/rules_transformer.md +++ b/docs/reference/transformers/rules_transformer.md @@ -39,7 +39,7 @@ Each rule of `rules_set` has the following options: | Code | Type | Required | Default | Description | |----------------|----------------|:--------:|---------|---------------------------------------------------------------------------------------------------------------------------------------| -| `condition` | `string\|ParsedExpression\|null` | | `null` | Expression (or an already parsed `Symfony\Component\ExpressionLanguage\ParsedExpression`); the rule matches if it is truthy | +| `condition` | `string\|ParsedExpression\|null` | | `null` | Expression (or an already parsed `Symfony\Component\ExpressionLanguage\ParsedExpression`); the rule matches if its result is truthy (cast to `bool`: `1` of `preg_match()` matches, `0`, `''`, `null` do not) | | `default` | `bool` | | `false` | Mark the rule as the default one (always matches). It cannot have a `condition`, no conditional rule may follow it, only one allowed | | `set_null` | `bool` | | `false` | If `true`, return `null` (takes precedence on `constant` and `transformers`) | | `constant` | `any` | | `null` | If not `null`, return this value (takes precedence on `transformers`) | @@ -88,4 +88,6 @@ Notes ----- Conditions are parsed with the bundle's `cleverage_process.expression_language` service, which also exposes the PHP -`preg_match` function. +`preg_match` function. The result of a condition does not need to be a boolean: it is cast to `bool`, so a +`preg_match('/^foo/', value)` (`1` or `0`), a `matches` expression or a single variable (`value`) can be used as is. +Beware that PHP truthiness applies: the strings `"0"` and `""`, `0`, `[]` and `null` do not match. diff --git a/src/Transformer/RulesTransformer.php b/src/Transformer/RulesTransformer.php index 6c59aa23..583338f9 100644 --- a/src/Transformer/RulesTransformer.php +++ b/src/Transformer/RulesTransformer.php @@ -145,7 +145,7 @@ protected function matchRule(mixed $value, array $rule, bool $useValueAsVariable 'value' => $value, ]; - return $this->language->evaluate($rule['condition'], $expressionValues); + return (bool) $this->language->evaluate($rule['condition'], $expressionValues); } /* @noinspection PhpStrictTypeCheckingInspection */ diff --git a/tests/Transformer/RulesTransformerTest.php b/tests/Transformer/RulesTransformerTest.php index 6a72ae86..3aedea35 100644 --- a/tests/Transformer/RulesTransformerTest.php +++ b/tests/Transformer/RulesTransformerTest.php @@ -17,9 +17,12 @@ use CleverAge\ProcessBundle\Transformer\Array\ArrayLastTransformer; use CleverAge\ProcessBundle\Transformer\RulesTransformer; use CleverAge\ProcessBundle\Transformer\TransformerTrait; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ServiceLocator; +use Symfony\Component\ExpressionLanguage\ExpressionFunction; use Symfony\Component\ExpressionLanguage\ExpressionLanguage; +use Symfony\Component\ExpressionLanguage\ParsedExpression; use Symfony\Component\OptionsResolver\OptionsResolver; #[\PHPUnit\Framework\Attributes\CoversClass(RulesTransformer::class)] @@ -42,6 +45,68 @@ public function testTransform(): void $this->assertSame('is not foo', $transformer->transform('bar', $options)); } + /** + * @return iterable + */ + public static function nonBooleanConditionProvider(): iterable + { + yield 'preg_match int match' => ['preg_match("/^foo/", value)', 'foobar', 'matched']; + yield 'preg_match int no match' => ['preg_match("/^foo/", value)', 'barfoo', 'default']; + yield 'matches operator' => ['value matches "/^foo/"', 'foobar', 'matched']; + yield 'matches operator no match' => ['value matches "/^foo/"', 'barfoo', 'default']; + yield 'truthy string variable' => ['value', 'foo', 'matched']; + yield 'falsy string variable' => ['value', '0', 'default']; + yield 'null variable' => ['value', null, 'default']; + yield 'truthy array variable' => ['value', ['a'], 'matched']; + yield 'empty array variable' => ['value', [], 'default']; + } + + #[DataProvider('nonBooleanConditionProvider')] + public function testNonBooleanCondition(string $condition, mixed $input, string $expected): void + { + $transformer = $this->createTransformer(); + $options = $this->resolveOptions($transformer, [ + 'rules_set' => [ + ['condition' => $condition, 'constant' => 'matched'], + ['default' => true, 'constant' => 'default'], + ], + ]); + + $this->assertSame($expected, $transformer->transform($input, $options)); + } + + public function testNonBooleanConditionWithValueAsVariables(): void + { + $transformer = $this->createTransformer(); + $options = $this->resolveOptions($transformer, [ + 'use_value_as_variables' => true, + 'expression_variables' => ['name'], + 'rules_set' => [ + ['condition' => 'name', 'constant' => 'matched'], + ['default' => true, 'constant' => 'default'], + ], + ]); + + $this->assertSame('matched', $transformer->transform(['name' => 'foo'], $options)); + $this->assertSame('default', $transformer->transform(['name' => null], $options)); + } + + public function testParsedExpressionCondition(): void + { + $transformer = $this->createTransformer(); + $parsed = (new ExpressionLanguage())->parse('value', ['value']); + $this->assertInstanceOf(ParsedExpression::class, $parsed); + $options = $this->resolveOptions($transformer, [ + 'rules_set' => [ + ['condition' => $parsed, 'constant' => 'matched'], + ['default' => true, 'constant' => 'default'], + ], + ]); + + $this->assertSame('matched', $transformer->transform('foo', $options)); + $this->assertSame('default', $transformer->transform(null, $options)); + } + public function testRulesSetCannotHaveMoreThanOneDefaultRule(): void { $transformer = $this->createTransformer(); @@ -77,7 +142,10 @@ private function createTransformer(): RulesTransformer 'array_last' => static fn (): ArrayLastTransformer => new ArrayLastTransformer(), ])); - return new RulesTransformer($registry, new ExpressionLanguage()); + $language = new ExpressionLanguage(); + $language->addFunction(ExpressionFunction::fromPhp('preg_match')); + + return new RulesTransformer($registry, $language); } /**