From 6bc59af2dbea24c5822975d1558458d4f990645d Mon Sep 17 00:00:00 2001 From: George Steel Date: Sat, 29 Jun 2024 17:07:56 +0100 Subject: [PATCH] Refactor `Callback` Filter - Removes runtime mutation of the filter - Drops inheritance Signed-off-by: George Steel --- docs/book/v3/standard-filters.md | 82 ++++----- psalm-baseline.xml | 43 +---- src/Callback.php | 105 +++-------- test/CallbackTest.php | 170 ++++++++++++++---- test/FilterPluginManagerCompatibilityTest.php | 10 ++ test/TestAsset/CallbackClass.php | 28 --- 6 files changed, 205 insertions(+), 233 deletions(-) delete mode 100644 test/TestAsset/CallbackClass.php diff --git a/docs/book/v3/standard-filters.md b/docs/book/v3/standard-filters.md index 1f97f7f9..a82b92e4 100644 --- a/docs/book/v3/standard-filters.md +++ b/docs/book/v3/standard-filters.md @@ -304,22 +304,18 @@ $result = $filter->filter(2); ## Callback -This filter allows you to use own methods in conjunction with `Laminas\Filter`. You -don't have to create a new filter when you already have a method which does the -job. +This filter wraps any callable so that it can be invoked using the filter contract. ### Supported Options The following options are supported for `Laminas\Filter\Callback`: - `callback`: This sets the callback which should be used. -- `callback_params`: This property sets the options which are used when the - callback is processed. +- `callback_params`: Optional. When provided this should be an array where each element of the array is an additional argument to your callback. ### Basic Usage -The usage of this filter is quite simple. In this example, we want to create a -filter which reverses a string: +The following example uses PHP's built-in function `strrev` as the callback: ```php $filter = new Laminas\Filter\Callback('strrev'); @@ -328,60 +324,46 @@ print $filter->filter('Hello!'); // returns "!olleH" ``` -As you can see it's really simple to use a callback to define custom filters. It -is also possible to use a method, which is defined within a class, by giving an -array as the callback: +As previously mentioned, any type of callable can be used as a callback, for example a closure: ```php -class MyClass -{ - public static function reverse($param); -} +$filter = new Laminas\Filter\Callback([ + 'callback' => function (mixed $input): mixed { + if (is_string($input)) { + return $input . ' there!'; + } + + return $input; + }, +]); -// The filter definition -$filter = new Laminas\Filter\Callback(array('MyClass', 'reverse')); -print $filter->filter('Hello!'); +$filter->filter('Hey'); // Returns 'Hey there!' ``` -As of PHP 5.5 you can use ::class resolution for given callback class: +### Additional Callback Parameters + +If your callback requires additional arguments, these can be passed as a list, or an associative array to the `callback_params` option. The first argument will be the value to be filtered. ```php -class MyClass -{ - public function __invoke($param); +class MyClass { + public function __invoke(mixed $input, string $a, string $b): string + { + if (is_string($input)) { + return implode(', ', [$input, $a, $b]); + } + + return 'Foo'; + } } -// The filter definition -$filter = new Laminas\Filter\Callback(MyClass::class); -print $filter->filter('Hello!'); -``` - -To get the actual set callback use `getCallback()` and to set another callback -use `setCallback()`. - -> ### Possible Exceptions -> -> You should note that defining a callback method which can not be called will -> raise an exception. - -### Default Parameters within a Callback - -It is also possible to define default parameters, which are given to the called -method as an array when the filter is executed. This array will be concatenated -with the value which will be filtered. - -```php $filter = new Laminas\Filter\Callback([ - 'callback' => 'MyMethod', - 'options' => ['key' => 'param1', 'key2' => 'param2'] + 'callback' => new MyClass(), + 'callback_params' => [ + 'a' => 'baz', + 'b' => 'bat', + ], ]); -$filter->filter(['value' => 'Hello']); -``` - -Calling the above method definition manually would look like this: - -```php -$value = MyMethod('Hello', 'param1', 'param2'); +$filter->filter('bing'); // returns 'bing,baz,bat' ``` ## Compress and Decompress diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 28847e81..ff5f12ea 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -23,34 +23,6 @@ - - - options['callback'], $params)]]> - - - - - - - - - - options['callback']]]> - options['callback_params']]]> - - - - - - - - - - - - - - @@ -777,10 +749,12 @@ - - - - + + + + + + @@ -1063,11 +1037,6 @@ - - - - - diff --git a/src/Callback.php b/src/Callback.php index 417a43ee..04362881 100644 --- a/src/Callback.php +++ b/src/Callback.php @@ -4,110 +4,45 @@ namespace Laminas\Filter; -use Traversable; +use Closure; use function array_unshift; -use function call_user_func_array; -use function class_exists; use function is_callable; -use function is_string; /** * @psalm-type Options = array{ - * callback?: callable, - * callback_params?: array, - * ... + * callback: callable(mixed): mixed, + * callback_params?: array, * } - * @extends AbstractFilter + * @implements FilterInterface */ -final class Callback extends AbstractFilter +final class Callback implements FilterInterface { - /** @var array */ - protected $options = [ - 'callback' => null, - 'callback_params' => [], - ]; + /** @var Closure(mixed): mixed */ + private readonly Closure $callback; + private readonly array $arguments; /** - * @param callable|array|string|Traversable $callbackOrOptions - * @param array $callbackParams + * @param callable(mixed): mixed|Options $options */ - public function __construct($callbackOrOptions = [], $callbackParams = []) + public function __construct(array|callable $options) { - if (is_callable($callbackOrOptions) || is_string($callbackOrOptions)) { - $this->setCallback($callbackOrOptions); - $this->setCallbackParams($callbackParams); - } else { - $this->setOptions($callbackOrOptions); - } + $callback = is_callable($options) ? $options : $options['callback']; + $arguments = ! is_callable($options) ? $options['callback_params'] ?? [] : []; + $this->callback = $callback(...); + $this->arguments = $arguments; } - /** - * Sets a new callback for this filter - * - * @param callable $callback - * @throws Exception\InvalidArgumentException - * @return self - */ - public function setCallback($callback) - { - if (is_string($callback) && class_exists($callback)) { - $callback = new $callback(); - } - - if (! is_callable($callback)) { - throw new Exception\InvalidArgumentException( - 'Invalid parameter for callback: must be callable' - ); - } - - $this->options['callback'] = $callback; - return $this; - } - - /** - * Returns the set callback - * - * @return callable - */ - public function getCallback() - { - return $this->options['callback']; - } - - /** - * Sets parameters for the callback - * - * @param array $params - * @return self - */ - public function setCallbackParams($params) + public function filter(mixed $value): mixed { - $this->options['callback_params'] = (array) $params; - return $this; - } + $params = $this->arguments; + array_unshift($params, $value); - /** - * Get parameters for the callback - * - * @return array - */ - public function getCallbackParams() - { - return $this->options['callback_params']; + return ($this->callback)(...$params); } - /** - * Calls the filter per callback - * - * @param mixed $value Options for the set callable - * @return mixed Result from the filter which was called - */ - public function filter(mixed $value): mixed + public function __invoke(mixed $value): mixed { - $params = (array) $this->options['callback_params']; - array_unshift($params, $value); - - return call_user_func_array($this->options['callback'], $params); + return $this->filter($value); } } diff --git a/test/CallbackTest.php b/test/CallbackTest.php index 4779de68..034a34b1 100644 --- a/test/CallbackTest.php +++ b/test/CallbackTest.php @@ -4,66 +4,170 @@ namespace LaminasTest\Filter; +use Generator; use Laminas\Filter\Callback as CallbackFilter; -use LaminasTest\Filter\TestAsset\CallbackClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use function PHPUnit\Framework\assertSame; + class CallbackTest extends TestCase { - public function testObjectCallback(): void + /** @return Generator */ + public static function callbackProvider(): Generator { - $filter = new CallbackFilter([new CallbackClass(), 'objectCallback']); - self::assertSame('objectCallback-test', $filter('test')); + yield 'Invokable Class' => [ + new class () + { + public function __invoke(mixed $input): string + { + assertSame('INPUT', $input); + return $input; + } + }, + ]; + + yield 'Closure' => [ + function (mixed $input): string { + assertSame('INPUT', $input); + return $input; + }, + ]; + + yield 'Static method' => [ + [self::class, 'staticMethodTest'], + ]; + + $instance = new class + { + public function doStuff(mixed $input): string + { + assertSame('INPUT', $input); + return $input; + } + }; + + yield 'Instance method' => [ + [$instance, 'doStuff'], + ]; } - public function testConstructorWithOptions(): void + /** @param callable(mixed): mixed $callback */ + #[DataProvider('callbackProvider')] + public function testBasicBehaviour(callable $callback): void { - $filter = new CallbackFilter([ - 'callback' => [new CallbackClass(), 'objectCallbackWithParams'], - 'callback_params' => 0, - ]); + $filter = new CallbackFilter($callback); + self::assertSame('INPUT', $filter->filter('INPUT')); + } - self::assertSame('objectCallbackWithParams-test-0', $filter('test')); + public function testCallbackWithPHPBuiltIn(): void + { + $filter = new CallbackFilter('strrev'); + self::assertSame('!olleH', $filter('Hello!')); } - public function testStaticCallback(): void + /** @see self::callbackProvider() */ + public static function staticMethodTest(mixed $input): string { - $filter = new CallbackFilter( - [CallbackClass::class, 'staticCallback'] - ); - self::assertSame('staticCallback-test', $filter('test')); + assertSame('INPUT', $input); + return $input; } - public function testStringClassCallback(): void + /** @return Generator */ + public static function callbackWithArgumentsProvider(): Generator { - $filter = new CallbackFilter(CallbackClass::class); - self::assertSame('stringClassCallback-test', $filter('test')); + yield 'Invokable Class' => [ + new class () + { + public function __invoke(mixed $input, int $a, int $b): int + { + assertSame('INPUT', $input); + + return $a + $b; + } + }, + ]; + + yield 'Closure' => [ + function (mixed $input, int $a, int $b): int { + assertSame('INPUT', $input); + + return $a + $b; + }, + ]; + + yield 'Static method' => [ + [self::class, 'staticMethodWithArgumentsTest'], + ]; + + $instance = new class + { + public function doStuff(mixed $input, int $a, int $b): int + { + assertSame('INPUT', $input); + + return $a + $b; + } + }; + + yield 'Instance method' => [ + [$instance, 'doStuff'], + ]; } - public function testSettingDefaultOptions(): void + /** @see self::callbackWithArgumentsProvider() */ + public static function staticMethodWithArgumentsTest(mixed $input, int $a, int $b): int { - $filter = new CallbackFilter([new CallbackClass(), 'objectCallback'], 'param'); - self::assertSame(['param'], $filter->getCallbackParams()); - self::assertSame('objectCallback-test', $filter('test')); + assertSame('INPUT', $input); + + return $a + $b; } - public function testSettingDefaultOptionsAfterwards(): void + /** @param callable(mixed): mixed $callback */ + #[DataProvider('callbackWithArgumentsProvider')] + public function testCallbackWithArguments(callable $callback): void { - $filter = new CallbackFilter([new CallbackClass(), 'objectCallback']); - $filter->setCallbackParams('param'); - self::assertSame(['param'], $filter->getCallbackParams()); - self::assertSame('objectCallback-test', $filter('test')); + $filter = new CallbackFilter([ + 'callback' => $callback, + 'callback_params' => [ + 'a' => 2, + 'b' => 2, + ], + ]); + self::assertSame(4, $filter('INPUT')); } - public function testCallbackWithStringParameter(): void + public function testAssocArrayTreatedAsNamedArguments(): void { - $filter = new CallbackFilter('strrev'); - self::assertSame('!olleH', $filter('Hello!')); + $filter = new CallbackFilter([ + 'callback' => function (mixed $input, string $foo, string $bar): string { + assertSame('INPUT', $input); + + return $foo . $bar; + }, + 'callback_params' => [ + 'bar' => 'Baz', + 'foo' => 'Bing', + ], + ]); + + self::assertSame('BingBaz', $filter->filter('INPUT')); } - public function testCallbackWithArrayParameters(): void + public function testListArgumentsAreOrdered(): void { - $filter = new CallbackFilter('strrev'); - self::assertSame('!olleH', $filter('Hello!')); + $filter = new CallbackFilter([ + 'callback' => function (mixed $input, string $foo, string $bar): string { + assertSame('INPUT', $input); + + return $foo . $bar; + }, + 'callback_params' => [ + 'Baz', + 'Bing', + ], + ]); + + self::assertSame('BazBing', $filter->filter('INPUT')); } } diff --git a/test/FilterPluginManagerCompatibilityTest.php b/test/FilterPluginManagerCompatibilityTest.php index 9df0e2fa..a78580e9 100644 --- a/test/FilterPluginManagerCompatibilityTest.php +++ b/test/FilterPluginManagerCompatibilityTest.php @@ -5,6 +5,7 @@ namespace LaminasTest\Filter; use Generator; +use Laminas\Filter\Callback; use Laminas\Filter\FilterPluginManager; use Laminas\ServiceManager\Exception\InvalidServiceException; use PHPUnit\Framework\TestCase; @@ -14,11 +15,16 @@ use function assert; use function class_exists; +use function in_array; use function is_string; use function strpos; class FilterPluginManagerCompatibilityTest extends TestCase { + private const FILTERS_WITH_REQUIRED_OPTIONS = [ + Callback::class, + ]; + protected static function getPluginManager(): FilterPluginManager { return CreatePluginManager::withDefaults(); @@ -41,6 +47,10 @@ public static function aliasProvider(): Generator continue; } + if (in_array($target, self::FILTERS_WITH_REQUIRED_OPTIONS, true)) { + continue; + } + assert(class_exists($target)); yield $alias => [$alias, $target]; diff --git a/test/TestAsset/CallbackClass.php b/test/TestAsset/CallbackClass.php deleted file mode 100644 index 145225ca..00000000 --- a/test/TestAsset/CallbackClass.php +++ /dev/null @@ -1,28 +0,0 @@ -