From 7c6f6edf0fd06005a6eb1ae046fb010dddfad09d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Ostroluck=C3=BD?= Date: Sun, 5 Jan 2020 04:25:28 +0100 Subject: [PATCH] Clear open entity managers on kernel reset In some scenarios involving long running processes and proxies, resetting the entity manager results in a creation of a new entity manager, while some services still hold a reference to the old one, which keeps its previous state. This abandons resetting the entity manager in favor of a call to clear() on the existing entity manager if it is open. Fixes #1112 --- Registry.php | 5 +- .../ImportMappingDoctrineCommandTest.php | 72 +++---------------- .../TestCustomClassRepoRepository.php | 5 ++ .../Fixtures/TestKernel.php | 68 ++++++++++++++++++ Tests/RegistryTest.php | 35 ++++++++- composer.json | 1 + 6 files changed, 122 insertions(+), 64 deletions(-) create mode 100644 Tests/DependencyInjection/Fixtures/TestKernel.php diff --git a/Registry.php b/Registry.php index ec4690f08..c21d44294 100644 --- a/Registry.php +++ b/Registry.php @@ -3,6 +3,7 @@ namespace Doctrine\Bundle\DoctrineBundle; use Doctrine\ORM\EntityManager; +use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\ORMException; use ProxyManager\Proxy\LazyLoadingInterface; use Psr\Container\ContainerInterface; @@ -182,7 +183,9 @@ private function resetOrClearManager(string $managerName, string $serviceId) : v $manager = $this->container->get($serviceId); - if (! $manager instanceof LazyLoadingInterface) { + assert($manager instanceof EntityManagerInterface); + + if (! $manager instanceof LazyLoadingInterface || $manager->isOpen()) { $manager->clear(); return; diff --git a/Tests/Command/ImportMappingDoctrineCommandTest.php b/Tests/Command/ImportMappingDoctrineCommandTest.php index e0e6c3b77..07bd08a6e 100644 --- a/Tests/Command/ImportMappingDoctrineCommandTest.php +++ b/Tests/Command/ImportMappingDoctrineCommandTest.php @@ -2,24 +2,19 @@ namespace Doctrine\Bundle\DoctrineBundle\Tests\Command; -use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; +use Doctrine\Bundle\DoctrineBundle\Tests\DependencyInjection\Fixtures\TestKernel; use PHPUnit\Framework\TestCase; -use Psr\Log\NullLogger; use Symfony\Bundle\FrameworkBundle\Console\Application; -use Symfony\Bundle\FrameworkBundle\FrameworkBundle; -use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\Console\Tester\CommandTester; -use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\Filesystem\Filesystem; use Symfony\Component\HttpKernel\Bundle\Bundle; -use Symfony\Component\HttpKernel\Kernel; /** * @group legacy */ class ImportMappingDoctrineCommandTest extends TestCase { - /** @var ImportMappingTestingKernel|null */ + /** @var TestKernel|null */ private $kernel; /** @var CommandTester|null */ @@ -27,7 +22,14 @@ class ImportMappingDoctrineCommandTest extends TestCase protected function setup() { - $this->kernel = new ImportMappingTestingKernel(); + $this->kernel = new class() extends TestKernel { + public function registerBundles() : iterable + { + yield from parent::registerBundles(); + yield new ImportMappingTestFooBundle(); + } + }; + $this->kernel->boot(); $connection = $this->kernel->getContainer() @@ -108,60 +110,6 @@ public function testExecuteAnnotationsWithNamespace() } } -class ImportMappingTestingKernel extends Kernel -{ - /** @var string|null */ - private $projectDir; - - public function __construct() - { - parent::__construct('test', true); - } - - public function registerBundles() : iterable - { - return [ - new FrameworkBundle(), - new DoctrineBundle(), - new ImportMappingTestFooBundle(), - ]; - } - - public function registerContainerConfiguration(LoaderInterface $loader) - { - $loader->load(function (ContainerBuilder $container) { - // @todo Setting the kernel.name parameter can be removed once the dependency on DoctrineCacheBundle has been dropped - $container->setParameter('kernel.name', 'foo'); - $container->loadFromExtension('framework', ['secret' => 'F00']); - - $container->loadFromExtension('doctrine', [ - 'dbal' => [ - 'driver' => 'pdo_sqlite', - 'path' => $this->getCacheDir() . '/testing.db', - ], - 'orm' => [], - ]); - - // Register a NullLogger to avoid getting the stderr default logger of FrameworkBundle - $container->register('logger', NullLogger::class); - }); - } - - public function getProjectDir() : string - { - if ($this->projectDir === null) { - $this->projectDir = sys_get_temp_dir() . '/sf_kernel_' . md5(mt_rand()); - } - - return $this->projectDir; - } - - public function getRootDir() : string - { - return $this->getProjectDir(); - } -} - class ImportMappingTestFooBundle extends Bundle { public function getPath() : string diff --git a/Tests/DependencyInjection/Fixtures/Bundles/RepositoryServiceBundle/Repository/TestCustomClassRepoRepository.php b/Tests/DependencyInjection/Fixtures/Bundles/RepositoryServiceBundle/Repository/TestCustomClassRepoRepository.php index c00dfa3d2..d93d696dc 100644 --- a/Tests/DependencyInjection/Fixtures/Bundles/RepositoryServiceBundle/Repository/TestCustomClassRepoRepository.php +++ b/Tests/DependencyInjection/Fixtures/Bundles/RepositoryServiceBundle/Repository/TestCustomClassRepoRepository.php @@ -2,8 +2,13 @@ namespace Fixtures\Bundles\RepositoryServiceBundle\Repository; +use Doctrine\ORM\EntityManager; use Doctrine\ORM\EntityRepository; class TestCustomClassRepoRepository extends EntityRepository { + public function getEntityManager() : EntityManager + { + return parent::getEntityManager(); + } } diff --git a/Tests/DependencyInjection/Fixtures/TestKernel.php b/Tests/DependencyInjection/Fixtures/TestKernel.php new file mode 100644 index 000000000..b4ec20014 --- /dev/null +++ b/Tests/DependencyInjection/Fixtures/TestKernel.php @@ -0,0 +1,68 @@ +load(static function (ContainerBuilder $container) { + // @todo Setting the kernel.name parameter can be removed once the dependency on DoctrineCacheBundle has been dropped + $container->setParameter('kernel.name', 'foo'); + $container->loadFromExtension('framework', ['secret' => 'F00']); + + $container->loadFromExtension('doctrine', [ + 'dbal' => ['driver' => 'pdo_sqlite'], + 'orm' => [ + 'mappings' => [ + 'RepositoryServiceBundle' => [ + 'type' => 'annotation', + 'dir' => __DIR__ . '/Bundles/RepositoryServiceBundle/Entity', + 'prefix' => 'Fixtures\Bundles\RepositoryServiceBundle\Entity', + ], + ], + ], + ]); + + // Register a NullLogger to avoid getting the stderr default logger of FrameworkBundle + $container->register('logger', NullLogger::class); + }); + } + + public function getProjectDir() : string + { + if ($this->projectDir === null) { + $this->projectDir = sys_get_temp_dir() . '/sf_kernel_' . md5(mt_rand()); + } + + return $this->projectDir; + } + + public function getRootDir() : string + { + return $this->getProjectDir(); + } +} diff --git a/Tests/RegistryTest.php b/Tests/RegistryTest.php index 2468fcca9..94bb35a94 100644 --- a/Tests/RegistryTest.php +++ b/Tests/RegistryTest.php @@ -4,8 +4,12 @@ use Closure; use Doctrine\Bundle\DoctrineBundle\Registry; +use Doctrine\Bundle\DoctrineBundle\Tests\DependencyInjection\Fixtures\TestKernel; use Doctrine\ORM\EntityManagerInterface; +use Fixtures\Bundles\RepositoryServiceBundle\Entity\TestCustomClassRepoEntity; +use Fixtures\Bundles\RepositoryServiceBundle\Repository\TestCustomClassRepoRepository; use ProxyManager\Proxy\LazyLoadingInterface; +use ProxyManager\Proxy\ProxyInterface; use stdClass; class RegistryTest extends TestCase @@ -132,7 +136,7 @@ public function testReset() $noProxyManager->expects($this->once()) ->method('clear'); - $proxyManager = $this->getMockBuilder(LazyLoadingInterface::class)->getMock(); + $proxyManager = $this->getMockBuilder([LazyLoadingInterface::class, EntityManagerInterface::class])->getMock(); $proxyManager->expects($this->once()) ->method('setProxyInitializer') ->with($this->isInstanceOf(Closure::class)); @@ -157,4 +161,33 @@ public function testReset() $registry = new Registry($container, [], $entityManagers, 'default', 'default'); $registry->reset(); } + + public function testIdentityMapsStayConsistentAfterReset() + { + $kernel = new TestKernel(); + $kernel->boot(); + + $container = $kernel->getContainer(); + $registry = $container->get('doctrine'); + $entityManager = $container->get('doctrine.orm.default_entity_manager'); + $repository = $entityManager->getRepository(TestCustomClassRepoEntity::class); + + $this->assertInstanceOf(ProxyInterface::class, $entityManager); + assert($entityManager instanceof EntityManagerInterface); + assert($registry instanceof Registry); + assert($repository instanceof TestCustomClassRepoRepository); + + $entity = new TestCustomClassRepoEntity(); + $repository->getEntityManager()->persist($entity); + + $this->assertTrue($entityManager->getUnitOfWork()->isEntityScheduled($entity)); + $this->assertTrue($repository->getEntityManager()->getUnitOfWork()->isEntityScheduled($entity)); + + $registry->reset(); + + $this->assertFalse($entityManager->getUnitOfWork()->isEntityScheduled($entity)); + $this->assertFalse($repository->getEntityManager()->getUnitOfWork()->isEntityScheduled($entity)); + + $entityManager->flush(); + } } diff --git a/composer.json b/composer.json index e73d86bd0..370df8d80 100644 --- a/composer.json +++ b/composer.json @@ -46,6 +46,7 @@ "phpunit/phpunit": "^7.5", "symfony/phpunit-bridge": "^4.2", "symfony/property-info": "^3.4.30|^4.3.3", + "symfony/proxy-manager-bridge": "^3.4|^4|^5", "symfony/twig-bridge": "^3.4|^4.1", "symfony/validator": "^3.4.30|^4.3.3", "symfony/web-profiler-bundle": "^3.4.30|^4.3.3",