sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

Filter objects should provide a way to be built with a proper state #6268

Closed phansys closed 2 years ago

phansys commented 4 years ago

Feature Request

Since there is a specific state required for the filter objects (the classes implementing Sonata\AdminBundle\Filter\FilterInterface) to work, we should provide a way to ensure their state is ready to use, at least in our own implementation. I think we have 2 choices:

  1. Add a __construct() method at Sonata\AdminBundle\Filter\Filter and set the required state there;
  2. Transform Sonata\AdminBundle\Filter\Filter::initialize() method into a singleton and set the required state there.

This feature request is created to avoid situations like the exposed in this comment.

TO DO:

VincentLanglet commented 4 years ago

Should we add the 4.0 milestone ?

phansys commented 4 years ago

I think we could provide some of the required changes from 3.x.

github-actions[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

VincentLanglet commented 3 years ago

The FilterFactory is doing:

public function create(string $name, string $type, array $options = []): FilterInterface
    {
        if (!$this->container->has($type)) {
            throw new \RuntimeException(sprintf('No attached service to type named `%s`', $type));
        }

        $filter = $this->container->get($type);

        if (!$filter instanceof FilterInterface) {
            throw new \RuntimeException(sprintf('The service `%s` must implement `FilterInterface`', $type));
        }

        $filter->initialize($name, $options);

        return $filter;
    }

How do you want to use the construct instead ?

And how would work a singleton ?

VincentLanglet commented 2 years ago

Friendly ping @phansys

Can you detail the two choices you gave ? Currently the initialize method is used here: https://github.com/sonata-project/SonataAdminBundle/blob/4.x/src/Filter/FilterFactory.php#L33-L48

To me constructor cannot be used since we're loading the filter as a service in the container (because it can depends on some others services). And I dunno what you had in mind by initialize as a singleton.

VincentLanglet commented 2 years ago

Closing due to lack of interest