Connection created twice if listeners depend on repositories

[ossinkine]

After updating my project from Symfony 4.4 to 5.4 tests began fail. I've looked deep into the issue and discovered that after this feature was introduced the subscribers (Doctrine listeneres) initialize after addEventListener call, and if the subscriber depends on Doctrine services (such a repository) the connection creation called twice. Here is a stack trace:

-> StaticConnectionFactory.php:27, DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticConnectionFactory->createConnection()
   App_KernelTestDebugContainer.php:1355, ContainerLXdkVH1\App_KernelTestDebugContainer->getDoctrine_Dbal_DefaultConnectionService()
   App_KernelTestDebugContainer.php:1383, ContainerLXdkVH1\App_KernelTestDebugContainer->getDoctrine_Orm_DefaultEntityManagerService()
   App_KernelTestDebugContainer.php:1368, ContainerLXdkVH1\App_KernelTestDebugContainer->ContainerLXdkVH1\{closure:/var/www/html/var/cache/test/ContainerLXdkVH1/App_KernelTestDebugContainer.php:1367-1373}()
   EntityManager_9a5be93.php:36, Closure->__invoke()
   EntityManager_9a5be93.php:36, ContainerLXdkVH1\EntityManager_9a5be93->getMetadataFactory()
   AbstractManagerRegistry.php:182, Doctrine\Bundle\DoctrineBundle\Registry->getManagerForClass()
   ServiceEntityRepository.php:37, App\Repository\UserRepository->__construct()
   UserRepository.php:31, App\Repository\UserRepository->__construct()
   App_KernelTestDebugContainer.php:2674, ContainerLXdkVH1\App_KernelTestDebugContainer->getUserRepositoryService()
   getUserListenerService.php:23, ContainerLXdkVH1\getUserListenerService::do()
   App_KernelTestDebugContainer.php:1005, ContainerLXdkVH1\App_KernelTestDebugContainer->load()
   Container.php:422, ContainerLXdkVH1\App_KernelTestDebugContainer->getService()
   ServiceLocator.php:42, Symfony\Component\DependencyInjection\Argument\ServiceLocator->get()
   ContainerAwareEventManager.php:203, Symfony\Bridge\Doctrine\ContainerAwareEventManager->initializeSubscribers()
   ContainerAwareEventManager.php:121, Symfony\Bridge\Doctrine\ContainerAwareEventManager->addEventListener()
-> StaticConnectionFactory.php:44, DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticConnectionFactory->createConnection()
   App_KernelTestDebugContainer.php:1355, ContainerLXdkVH1\App_KernelTestDebugContainer->getDoctrine_Dbal_DefaultConnectionService()
   App_KernelTestDebugContainer.php:1383, ContainerLXdkVH1\App_KernelTestDebugContainer->getDoctrine_Orm_DefaultEntityManagerService()
   App_KernelTestDebugContainer.php:1368, ContainerLXdkVH1\App_KernelTestDebugContainer->ContainerLXdkVH1\{closure:/var/www/html/var/cache/test/ContainerLXdkVH1/App_KernelTestDebugContainer.php:1367-1373}()
   EntityManager_9a5be93.php:239, Closure->__invoke()
   EntityManager_9a5be93.php:239, ContainerLXdkVH1\EntityManager_9a5be93->getConfiguration()
   ProxyCacheWarmer.php:54, Symfony\Bridge\Doctrine\CacheWarmer\ProxyCacheWarmer->warmUp()
   CacheWarmerAggregate.php:99, Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerAggregate->warmUp()
   Kernel.php:587, App\Kernel->initializeContainer()
   Kernel.php:789, App\Kernel->preBoot()
   Kernel.php:128, App\Kernel->boot()
   KernelTestCase.php:83, Symfony\Bundle\FrameworkBundle\Test\KernelTestCase::bootKernel()
   KernelTestCase.php:103, Symfony\Bundle\FrameworkBundle\Test\KernelTestCase::getContainer()
   WebTestCase.php:65, App\Tests\Controller\DefaultControllerTest->getEntityManager()
   DefaultControllerTest.php:23, App\Tests\Controller\DefaultControllerTest->setUp()
   TestCase.php:1145, App\Tests\Controller\DefaultControllerTest->runBare()
   TestResult.php:726, PHPUnit\Framework\TestResult->run()
   TestCase.php:904, App\Tests\Controller\DefaultControllerTest->run()
   TestSuite.php:678, PHPUnit\Framework\DataProviderTestSuite->run()
   TestSuite.php:678, PHPUnit\Framework\TestSuite->run()
   TestSuite.php:678, PHPUnit\Framework\TestSuite->run()
   TestRunner.php:670, PHPUnit\TextUI\TestRunner->run()
   Command.php:143, Symfony\Bridge\PhpUnit\Legacy\CommandForV9->run()
   Command.php:96, PHPUnit\TextUI\Command::main()
   phpunit:22, include()
   simple-phpunit.php:436, require()
   phpunit:13, {main}()

I'm not sure this a right place to fix it but looks like event listener might be attached another way.

[dmaicher]

Hm I don't see how this can be related to this bundle. Do you have the same issue without using this bundle?

[ossinkine]

Yes, without this bundle works good, because default connection factory used. In the stacktrace you can see that second initialization starts in StaticConnectionFactory.

I've removed repository as dependency from listener as workaroud but maybe there is a prettier solution.

[dmaicher]

@ossinkine ah ok. Now I get it. I misread the stacktrace.

I will have a look later today if there is something we can change here

[dmaicher]

So I had a deeper look and the only solution I currently see is to create a custom EventManager that decorates the existing event manager and is lazy in registering the PostConnectEventListener until the event is actually dispatched.

But this seems rather complex and also would break things for people that expect the event manager to be an instance of Symfony\Bridge\Doctrine\ContainerAwareEventManager :confused:

So to solve this "circular reference"-like issue the best seems indeed to remove that dependency on your end.

Let me know if you have another idea.

[dmaicher]

Or actually we could maybe register the PostConnectEventListener using DI config on the Symfony\Bridge\Doctrine\ContainerAwareEventManager directly :thinking: Will have another look

[dmaicher]

@ossinkine could you try this by any chance?

https://github.com/dmaicher/doctrine-test-bundle/compare/fix-issue-191

[ossinkine]

Your fix is looking good and working for me 👍

[dmaicher]

@ossinkine unfortunately there were side-effects to the fix and I decided to revert it

So for now I don't have a solution for this edge-case with the circular dependency via the ContainerAwareEventManager

[ossinkine]

@dmaicher sad to hear this. Maybe reopen the issue to see the bug still exists?

[raziel057]

Hello, I just notice an issue related to this one while testing to modify a loggable entity: https://github.com/KnpLabs/DoctrineBehaviors/blob/master/docs/loggable.md

My Unit Test update an entity and a log should be inserted in database (confirmed if I disable the DAMA\DoctrineTestBundle extension, or if I apply the patch https://github.com/dmaicher/doctrine-test-bundle/pull/194/commits/c3ea20eb450e0d5c6a6846c7ae1702ef3052a5fc)

When I enable the extension, my entity is modified successfully and I can trace the persist and flush of the Log but it seems not related to the same connection as I'm able to assert the entity changes but not the creation of the log. When I disable the extension or apply the patch https://github.com/dmaicher/doctrine-test-bundle/pull/194/commits/c3ea20eb450e0d5c6a6846c7ae1702ef3052a5fc I can see the log created successfully.

[dmaicher]

@raziel057 yes indeed :confused: This can be an issue if registering the PostConnectEventListener in turn causes other listeners to be instantiated that may have a dependency on a dbal connection themselves.

Now since we dropped support for DBAL 2 maybe there is a way to make patch https://github.com/dmaicher/doctrine-test-bundle/commit/c3ea20eb450e0d5c6a6846c7ae1702ef3052a5fc work without side-effects... :thinking: need to find some time to dig into it

[dmaicher]

I think I found a way to resolve this issue :blush: I will create a PR within the next days

[raziel057]

@dmaicher It would be really nice. Thanks for this.

[dmaicher]

@raziel057 would you be able to test branch fix_issue_191 ? Needs some polishing but I believe this will fix the issue.

[raziel057]

@dmaicher I confirm that it works fine when I apply the branch fix_issue_191

[raziel057]

Thank you very much for this great bundle!