Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doctrine Listener not loaded lazily doctrine-bridge #31683

Closed
zghosts opened this issue May 29, 2019 · 6 comments
Closed

Doctrine Listener not loaded lazily doctrine-bridge #31683

zghosts opened this issue May 29, 2019 · 6 comments

Comments

@zghosts
Copy link
Contributor

zghosts commented May 29, 2019

Symfony version(s) affected: 3.4.28

Description
Since symfony 3.4.28 a "Circular reference detected for service" exception is thrown on cache warmup.

How to reproduce
This seems to be tied to doctrine eventlisteners which get Repository services injected, which should be lazy loaded, but arn't.

comment beforehand: I know listeners should only listen to one event, but I have my reasons for doing this.

my configuration is something similar to:

class EventListener {
    private $repository;

    public function __construct(SomeRepositoryService $repository)
    {
         $this->repository = $repository;
    }

    public function postPersist(LifecycleEventArgs $event)
    {
        //Do something
    }
    public function preRemove(LifecycleEventArgs $event)
    {
        //Do something
    }
}

ServiceDefinition:

EventListener:
    tags:
        - { name: doctrine.event_listener, connection: default, event: postPersist, priority: 10, lazy: true }
        - { name: doctrine.event_listener, connection: default, event: preRemove, priority: 10, lazy: true }

Possible Solution
I've trace this back to recent changes in the doctrine-bridge, specifically line 72-75 which seems to always initialize all listeners even if they should be lazy loaded.

Restoring old behaviour by removing the foreach loop fixes the issue.

@zghosts zghosts changed the title Doctrine Listener not loaded lazily doctrine-brige Doctrine Listener not loaded lazily doctrine-bridge May 29, 2019
@Koc
Copy link
Contributor

Koc commented May 29, 2019

Do you call ContainerAwareEventManager::getListeners() without arguments anywhere in your code? This method not executed anywhere in Symfony IFIK, so listeners shouldn't be initialized all the time.

I'd suggest to use Service Locator for cases when Doctrine's repository is dependency of the Doctrine's event listener.

@Koc
Copy link
Contributor

Koc commented May 29, 2019

relates to #31636, #31335

@zghosts
Copy link
Contributor Author

zghosts commented May 29, 2019

You are correct that it's not being called anywhere in Symfony, however I'm using the gedmo/doctrine-extensions and this does indeed call ContainerAwareEventManager::getListeners() without arguments
see example in the sortable repository

So this is either a BC break or the doctrine extensions are doing it wrong.

@Koc
Copy link
Contributor

Koc commented May 29, 2019

I can suggest make call to getConfiguration lazy + instead of getting and iterating all listeners dispatch special event. Something like

<?php

    public function __construct(EntityManagerInterface $em, ClassMetadata $class)
    {
        parent::__construct($em, $class);

        $this->meta = $this->getClassMetadata();
    }

    private function getConfiguration()
    {
        if (null === $this->config) {
            $event = new InhectConfigEvent();
            $this->_em->getEventManager()->dispatch('gedmo.inject_sortableConfiguration', $event);

            if (null === $this->config = $event->getConfiguration()) {
                throw new \Gedmo\Exception\InvalidMappingException('This repository can be attached only to ORM sortable listener');
            }

        }

        return $this->config;
    }

@zghosts
Copy link
Contributor Author

zghosts commented Jun 14, 2019

@Koc your suggestion requires changing the base repositories in gedmo/doctrine-extensions, which for a minor release I should not have to do.

As it stands, as of 3.4.28, it's now very much impossible to inject any service into a doctrine eventlistener or subscriber which depends on a repository being injected or any service having a repository injected, where that repository extends any of the doctrine-extensions base repositories (in my case sortable, blameable and translatable).

I could inject the eventmanager and fetch the repositories from there, this would work, however that defeats the whole idea of having the repositories injected (which is easier for testing).

I comes down to the fact that this change breaks having repositories extending any gedmo/doctrine-extensions repository injectable as a service.

@nicolas-grekas
Copy link
Member

This should be fixed. Please report back if not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants