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

[BUG] Circular dependency issue when using EntityMananger in constructor of subscribers #428

Open
vv12131415 opened this issue Jan 28, 2020 · 2 comments

Comments

@vv12131415
Copy link
Contributor

Package version, Laravel version
Package version - latest (1.5.4)
Laravel - latest (6.x)

Expected behaviour

I can resolve depdendency as ctor injection

<?php

declare(strict_types=1);

namespace Foo;

use Doctrine\Common\EventSubscriber;
use Doctrine\ORM\EntityManagerInterface;

class Bar implements EventSubscriber
{
    /** @var EntityManagerInterface */
    private $em;
    
    public function __construct(EntityManagerInterface $em)
    {
        $this->em = $em;
    }
}

Actual behaviour

<?php

declare(strict_types=1);

namespace Foo;

use Doctrine\Common\EventSubscriber;
use Doctrine\ORM\EntityManagerInterface;

class Bar implements EventSubscriber
{
    /** @var EntityManagerInterface|null */
    private $em;
    
    public function getEntityManager()
    {
        if (!isset(this->em)) {
            $this->em = app(EntityManagerInterface::class); // with service locator, but it's wrong
        }
        
        return $this->em;
    }
}

Steps to reproduce the behaviour

just inject EntityManager/SomeRepository into EventSubscriber

@eigan
Copy link
Member

eigan commented Jan 28, 2020

This is not so easy to fix. Which EntityManager should be sent to the repository/constructor? I guess the one which is being created, but not so sure how we can provide that when the constructor is requesting a repository.

Relevant code is EntityManagerFactory::registerSubscribers.

We could to:

$resolvedSubscriber = $this->container->make($subscriber, [
    'entityManager' => $manager,
    'em' => $manager
]);

But this will not solve the issue with repositories.


Your subscribe-methods will receive an EventArgs object which should provide the EntityManager though. Isnt this sufficient?

public function onFlush(OnFlushEventArgs $args): void 
{
   $entityManager = $args->getEntityManager();
   $repo = $entityManager->getRepository(MyEntity::class);
 }

@vv12131415
Copy link
Contributor Author

Yes we can use your last example.
We can also do something like this

https://github.com/EnMarche/en-marche.fr/blob/2d43a966357ff2bddbca36ae9599025ff95862dc/src/EventListener/ManageReferentTeamMembersListener.php#L39

but best result for me is ctor injection, like here

https://github.com/EnMarche/en-marche.fr/blob/33111a0151098cfde26bb676b4432774b1a3e0df/src/EventSubscriber/ChangeIdeaAuthorCategoryListener.php#L11

the last one is available since lazy services in symfony

https://github.com/EnMarche/en-marche.fr/blob/8e0a12e8f774278fcf82033edebae6e87d4d17e8/app/config/services/services.xml#L182-L185


I know that I'm showing examples from symfony project.
I'm trying to push lazy services to Laravel (laravel/ideas#1436), but don't have results

@eigan eigan changed the title [BUG] EventSubscriber has memory leaks [BUG] Circular dependency issue when using EntityMananger in constructor of subscribers Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants