doctrine / DoctrineMongoDBBundle

Integrates Doctrine MongoDB ODM with Symfony
http://symfony.com/doc/current/bundles/DoctrineMongoDBBundle/index.html
MIT License
379 stars 229 forks source link

Dependency Injection and ODM Filter #547

Open t-veron opened 5 years ago

t-veron commented 5 years ago

Hello,

Did you plan to add the possibility to inject services in filters?

In fact, actually we can only inject static parameters.

This bundle MongodbOdmFiltersBundle add this functionnality.

malarzm commented 5 years ago

This makes sense, however given latest trends I think we should either allow pointing to a service or somehow discover the filters and autoconfigure them. The first option seems to be easier and leaves the door opened for the 2nd one so I think it might be better. So instead of injecting services manually like in the bundle you've linked we could leverage autowiring and start with

filters:
    Some\Filter:
        class: Some\Filter
        service: true

aiming to have simpler construct in the future for enabled filters.

filters:
    Some\Filter: ~

This however would require reworking a bit filters to accept instance of a filter instead of class name.

Having said that, injecting services as parameters might be easier for first step, a PR will be appreciated!

t-veron commented 5 years ago

I'm agree with you.

However I analysed the code of doctrine/mongodb-odm and we can't use directly dependancy injection because they create new objects from constructor call's.

  public function enable(string $name) : BsonFilter
    {
        if (! $this->has($name)) {
            throw new InvalidArgumentException("Filter '" . $name . "' does not exist.");
        }
        if (! $this->isEnabled($name)) {
            $filterClass      = $this->config->getFilterClassName($name);
            $filterParameters = $this->config->getFilterParameters($name);
            **$filter           = new $filterClass($this->dm);**
            foreach ($filterParameters as $param => $value) {
                $filter->setParameter($param, $value);
            }
            $this->enabledFilters[$name] = $filter;
        }
        return $this->enabledFilters[$name];
    }
malarzm commented 5 years ago

Yeah, I realized that by the time I was finishing my comment :)

Having said that, injecting services as parameters might be easier for first step, a PR will be appreciated!

alcaeus commented 5 years ago

I've created https://github.com/doctrine/mongodb-odm/issues/1986 to track the refactoring of the filter logic to allow better configuration. Once this has been implemented in ODM we can then talk about using auto configuration and tags on services to tie filters to document managers 👍