laminas-filter icon indicating copy to clipboard operation
laminas-filter copied to clipboard

Mark filters as attributes

Open delolmo opened this issue 2 years ago • 7 comments

Feature Request

Q A
New Feature yes
RFC no
BC Break no

Summary

Adding the #[Attribute] tag to filters would allow several new use cases for the library.

For example, object validation (i.e., a DTO):

use Laminas\Filter;

final class CreateNewBookDto extends Dto
{
    #[Filter\StringTrim]
    public $author;

    #[Filter\StringTrim]
    #[Filter\StringToUpper]
    public $title;

    public function __construct($author, $title)
    {
         $this->author = $author;
         $this->title = $title;
    }
}

In this case, filters could be read using reflection to create an automated pipeline to filter object properties.

Other cases could include route filtering of HTTP requests, console input filtering, etc.

delolmo avatar Dec 22 '22 15:12 delolmo

Sounds like an OK idea, TBH (same for validators).

Note that some of these require external dependencies, and cannot really be safely used as attributes.

Ocramius avatar Dec 22 '22 15:12 Ocramius

Just as a proof of concept, perhaps this could be an interesting implementation:

namespace Laminas\Filter;

use ReflectionAttribute;
use ReflectionClass;

use function class_exists;
use function is_object;

final class AnnotatedObjectFilter implements FilterInterface
{
    public function filter(mixed $object): mixed
    {
        if (! is_object($object)) {
            return $object;
        }

        $className = $object::class;

        if (! class_exists($className)) {
            return $object;
        }

        $reflectionClass = new ReflectionClass($className);

        $properties = $reflectionClass->getProperties();

        foreach ($properties as $property) {
            $attributes = $property->getAttributes(
                name: FilterInterface::class,
                flags: ReflectionAttribute::IS_INSTANCEOF,
            );

            foreach ($attributes as $attribute) {
                $filter = $attribute->newInstance();
                $value  = $filter->filter($property->getValue($object));
                $property->setValue($object, $value);
            }
        }

        return $object;
    }
}

delolmo avatar Dec 23 '22 10:12 delolmo

@delolmo Your implementation ignores external dependencies of filters. The filter plugin manager must be used to retrieve a filter, as the manager resolves the dependencies and also the alias names of filters:

https://github.com/laminas/laminas-filter/blob/b986ac119af4610cae00f68fe10bac440b994e0e/src/FilterPluginManager.php#L20-L30

More on this topic can be found in the documentation of laminas-servicemanager: Plugin Managers

froschdesign avatar Dec 23 '22 10:12 froschdesign

What about something like this then? I am sure there are better ways to do it, just trying to contribute something here. Let me know if there is something else I need to take into account.

use Laminas\ServiceManager\ServiceManager;
use ReflectionAttribute;
use ReflectionClass;

use function is_object;

final class AnnotatedObject implements Filter
{
    /** @var FilterPluginManager|null */
    protected $plugins;

    /**
     * Get plugin manager instance
     *
     * @return FilterPluginManager
     */
    public function getPluginManager()
    {
        $plugins = $this->plugins;
        if (! $plugins instanceof FilterPluginManager) {
            $plugins = new FilterPluginManager(new ServiceManager());
            $this->setPluginManager($plugins);
        }

        return $plugins;
    }

    public function filter(mixed $value): mixed
    {
        if (! is_object($value)) {
            return $value;
        }

        $reflectionClass = new ReflectionClass($value);

        $properties = $reflectionClass->getProperties();

        foreach ($properties as $property) {
            $attributes = $property->getAttributes(
                name: Filter::class,
                flags: ReflectionAttribute::IS_INSTANCEOF,
            );

            foreach ($attributes as $attribute) {
            	$filter = $this->plugin(
            		name: $attribute->getName(), 
            		options: $attribute->getArguments()
        		);
                /** @var mixed $origValue */
                $origValue = $property->getValue($value);
                /** @var mixed $newValue */
                $newValue = $filter->filter($origValue);
                $property->setValue($value, $newValue);
            }
        }

        return $value;
    }

    /**
     * Retrieve a filter plugin by name
     *
     * @param string $name
     * @return FilterInterface|callable(mixed): mixed
     */
    public function plugin($name, array $options = [])
    {
        $plugins = $this->getPluginManager();
        return $plugins->get($name, $options);
    }
    
    /**
     * Set plugin manager instance
     *
     * @return self
     */
    public function setPluginManager(FilterPluginManager $plugins)
    {
        $this->plugins = $plugins;
        return $this;
    }
}

delolmo avatar Dec 23 '22 12:12 delolmo

Maybe we find a way that it is somehow related to laminas-form as well, we do have Filters and Validators there as well and afair there are attributes.

But Overall I like the idea.

boesing avatar Dec 23 '22 15:12 boesing

I was revisiting this topic and I think the idea still is as an interesting one. Would you like to revisit it?

I have been thinking about what you said about laminas-form, but laminas-filter and laminas-validator should work in a standalone manner, shouldn't they? Perhaps we can move towards implementing the feature in filters and validators and then try to use this new feature in the laminas-form library.

Should I open a PR with my AnnotatedObject implementation?

delolmo avatar Dec 02 '23 15:12 delolmo

I think a PR would be very welcome. However, you might want to have a look into https://github.com/laminas/laminas-filter/pull/118 first and maybe wait some time until we decided on how we want to proceed with this library (esp. the option passing via __construct).

@froschdesign I am not sure if we need the filter plugin manager. Attributes are not annotations which can be invalid. Not every filter can be an attribute either, so those filters which can be used as attributes are not allowed to have external dependencies anyways, as these cannot be resolved via the attribute notation.

I am not 100% sure if we should actually have this in-place as (at least I) do not see a real use-case for this (besides the very limited functionality provided by @delolmo). One of my concerns are, that one has to have a mixed property and just by adding attributes, no static analyzer will understand what value is in those properties.

If we provide plugins for analyzers, that might work with analyzers but there is no way of actually verifying that the object has already applied filters or not and thus static analyzers will have a tough time.

That is why I do not use filters outside of forms (in which I can just pass array). I'd rather have something like:

final class CreateNewBookDto extends Dto
{
    /** 
      * @param non-empty-string $author
      * @param uppercase-string $title
      */
    public function __construct(public readonly string $author, public readonly string $title)
    {
    }
}

With having StringTrim being applied prior instantiating the class. This provides proper types without having mixed somewhere.

But thats most probably a personal thing and there might be projects and developers who are not that strict when it comes to their types. I would never pass any code review in our company with that but thats just because I have other expectations.

boesing avatar Dec 04 '23 18:12 boesing