From 279a554556bed387a6aaae0c13cc982d4874773b Mon Sep 17 00:00:00 2001 From: David Maicher Date: Fri, 1 Jul 2022 14:06:31 +0200 Subject: [PATCH] Fix issue 191 (#214) --- .../DAMADoctrineTestBundle.php | 4 ++ .../DAMADoctrineTestExtension.php | 26 +++++----- .../DoctrineTestCompilerPass.php | 32 ++++-------- .../RegisterDoctrineEventListenersPass.php | 51 +++++++++++++++++++ .../DBAL/PostConnectEventListener.php | 5 ++ .../Doctrine/DBAL/StaticConnectionFactory.php | 3 -- .../DAMADoctrineTestExtensionTest.php | 10 ++-- .../DoctrineTestCompilerPassTest.php | 6 +++ .../Doctrine/DBAL/MockDriver.php | 3 +- .../DBAL/StaticConnectionFactoryTest.php | 13 ++--- tests/Functional/FunctionalTestTrait.php | 11 ++-- tests/Functional/PhpunitTest.php | 22 ++++++++ 12 files changed, 130 insertions(+), 56 deletions(-) create mode 100644 src/DAMA/DoctrineTestBundle/DependencyInjection/RegisterDoctrineEventListenersPass.php diff --git a/src/DAMA/DoctrineTestBundle/DAMADoctrineTestBundle.php b/src/DAMA/DoctrineTestBundle/DAMADoctrineTestBundle.php index 9a4a450..9a989f0 100644 --- a/src/DAMA/DoctrineTestBundle/DAMADoctrineTestBundle.php +++ b/src/DAMA/DoctrineTestBundle/DAMADoctrineTestBundle.php @@ -3,6 +3,7 @@ namespace DAMA\DoctrineTestBundle; use DAMA\DoctrineTestBundle\DependencyInjection\DoctrineTestCompilerPass; +use DAMA\DoctrineTestBundle\DependencyInjection\RegisterDoctrineEventListenersPass; use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; @@ -17,5 +18,8 @@ public function build(ContainerBuilder $container): void parent::build($container); // lower priority than CacheCompatibilityPass from DoctrineBundle $container->addCompilerPass(new DoctrineTestCompilerPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, -1); + + // after RegisterEventListenersAndSubscribersPass from DoctrineBundle + $container->addCompilerPass(new RegisterDoctrineEventListenersPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 1); } } diff --git a/src/DAMA/DoctrineTestBundle/DependencyInjection/DAMADoctrineTestExtension.php b/src/DAMA/DoctrineTestBundle/DependencyInjection/DAMADoctrineTestExtension.php index 79112e5..19b4f94 100644 --- a/src/DAMA/DoctrineTestBundle/DependencyInjection/DAMADoctrineTestExtension.php +++ b/src/DAMA/DoctrineTestBundle/DependencyInjection/DAMADoctrineTestExtension.php @@ -7,22 +7,22 @@ class DAMADoctrineTestExtension extends Extension { - /** - * @var array - */ - private $processedConfig; - public function load(array $configs, ContainerBuilder $container): void { $configuration = new Configuration(); - $this->processedConfig = $this->processConfiguration($configuration, $configs); - } + $config = $this->processConfiguration($configuration, $configs); - /** - * @internal - */ - public function getProcessedConfig(): array - { - return $this->processedConfig; + $container->setParameter( + 'dama.'.Configuration::STATIC_META_CACHE, + (bool) $config[Configuration::STATIC_META_CACHE] + ); + $container->setParameter( + 'dama.'.Configuration::STATIC_QUERY_CACHE, + (bool) $config[Configuration::STATIC_QUERY_CACHE] + ); + $container->setParameter( + 'dama.'.Configuration::ENABLE_STATIC_CONNECTION, + $config[Configuration::ENABLE_STATIC_CONNECTION] + ); } } diff --git a/src/DAMA/DoctrineTestBundle/DependencyInjection/DoctrineTestCompilerPass.php b/src/DAMA/DoctrineTestBundle/DependencyInjection/DoctrineTestCompilerPass.php index e5ffe48..e9c4260 100644 --- a/src/DAMA/DoctrineTestBundle/DependencyInjection/DoctrineTestCompilerPass.php +++ b/src/DAMA/DoctrineTestBundle/DependencyInjection/DoctrineTestCompilerPass.php @@ -18,14 +18,10 @@ class DoctrineTestCompilerPass implements CompilerPassInterface { public function process(ContainerBuilder $container): void { - /** @var DAMADoctrineTestExtension $extension */ - $extension = $container->getExtension('dama_doctrine_test'); - $config = $extension->getProcessedConfig(); + /** @var string[] $transactionalBehaviorEnabledConnections */ + $transactionalBehaviorEnabledConnections = $container->getParameter('dama.doctrine.enabled_connections'); - /** @var bool|array $enableStaticConnectionsConfig */ - $enableStaticConnectionsConfig = $config[Configuration::ENABLE_STATIC_CONNECTION]; - - if ($enableStaticConnectionsConfig !== false) { + if (count($transactionalBehaviorEnabledConnections) > 0) { $factoryDef = new Definition(StaticConnectionFactory::class); $factoryDef ->setDecoratedService('doctrine.dbal.connection_factory') @@ -38,23 +34,18 @@ public function process(ContainerBuilder $container): void $cacheNames = []; - if ($config[Configuration::STATIC_META_CACHE]) { + if ($container->getParameter('dama.'.Configuration::STATIC_META_CACHE)) { $cacheNames[] = 'doctrine.orm.%s_metadata_cache'; } - if ($config[Configuration::STATIC_QUERY_CACHE]) { + if ($container->getParameter('dama.'.Configuration::STATIC_QUERY_CACHE)) { $cacheNames[] = 'doctrine.orm.%s_query_cache'; } $connectionNames = array_keys($container->getParameter('doctrine.connections')); - if (is_array($enableStaticConnectionsConfig)) { - $this->validateConnectionNames(array_keys($enableStaticConnectionsConfig), $connectionNames); - } foreach ($connectionNames as $name) { - if ($enableStaticConnectionsConfig === true - || isset($enableStaticConnectionsConfig[$name]) && $enableStaticConnectionsConfig[$name] === true - ) { + if (in_array($name, $transactionalBehaviorEnabledConnections, true)) { $this->addConnectionOptions($container, $name); } @@ -74,15 +65,10 @@ public function process(ContainerBuilder $container): void $this->registerStaticCache($container, $definition, $cacheServiceId); } } - } - private function validateConnectionNames(array $configNames, array $existingNames): void - { - $unknown = array_diff($configNames, $existingNames); - - if (count($unknown)) { - throw new \InvalidArgumentException(sprintf('Unknown doctrine dbal connection name(s): %s.', implode(', ', $unknown))); - } + $container->getParameterBag()->remove('dama.'.Configuration::STATIC_META_CACHE); + $container->getParameterBag()->remove('dama.'.Configuration::STATIC_QUERY_CACHE); + $container->getParameterBag()->remove('dama.doctrine.enabled_connections'); } private function addConnectionOptions(ContainerBuilder $container, string $name): void diff --git a/src/DAMA/DoctrineTestBundle/DependencyInjection/RegisterDoctrineEventListenersPass.php b/src/DAMA/DoctrineTestBundle/DependencyInjection/RegisterDoctrineEventListenersPass.php new file mode 100644 index 0000000..5fa8bee --- /dev/null +++ b/src/DAMA/DoctrineTestBundle/DependencyInjection/RegisterDoctrineEventListenersPass.php @@ -0,0 +1,51 @@ +getParameter('dama.'.Configuration::ENABLE_STATIC_CONNECTION); + + $connectionNames = array_keys($container->getParameter('doctrine.connections')); + if (is_array($enableStaticConnectionsConfig)) { + $this->validateConnectionNames(array_keys($enableStaticConnectionsConfig), $connectionNames); + } + + $enabledConnections = []; + + foreach ($connectionNames as $name) { + if ($enableStaticConnectionsConfig === true + || isset($enableStaticConnectionsConfig[$name]) && $enableStaticConnectionsConfig[$name] === true + ) { + $enabledConnections[] = $name; + $postConnectListenerDef = new Definition(PostConnectEventListener::class); + $postConnectListenerDef->addTag( + 'doctrine.event_listener', + ['event' => Events::postConnect, 'connection' => $name, 'priority' => 1000] + ); + $container->setDefinition('dama.doctrine.dbal.post_connect_event_listener.'.$name, $postConnectListenerDef); + } + } + + $container->setParameter('dama.doctrine.enabled_connections', $enabledConnections); + $container->getParameterBag()->remove('dama.'.Configuration::ENABLE_STATIC_CONNECTION); + } + + private function validateConnectionNames(array $configNames, array $existingNames): void + { + $unknown = array_diff($configNames, $existingNames); + + if (count($unknown)) { + throw new \InvalidArgumentException(sprintf('Unknown doctrine dbal connection name(s): %s.', implode(', ', $unknown))); + } + } +} diff --git a/src/DAMA/DoctrineTestBundle/Doctrine/DBAL/PostConnectEventListener.php b/src/DAMA/DoctrineTestBundle/Doctrine/DBAL/PostConnectEventListener.php index a6cdaa5..0b267f8 100644 --- a/src/DAMA/DoctrineTestBundle/Doctrine/DBAL/PostConnectEventListener.php +++ b/src/DAMA/DoctrineTestBundle/Doctrine/DBAL/PostConnectEventListener.php @@ -8,6 +8,11 @@ class PostConnectEventListener { public function postConnect(ConnectionEventArgs $args): void { + // can be disabled at runtime + if (!StaticDriver::isKeepStaticConnections()) { + return; + } + // The underlying connection already has a transaction started. // We start a transaction on the connection as well // so the internal state ($_transactionNestingLevel) is in sync with the underlying connection. diff --git a/src/DAMA/DoctrineTestBundle/Doctrine/DBAL/StaticConnectionFactory.php b/src/DAMA/DoctrineTestBundle/Doctrine/DBAL/StaticConnectionFactory.php index 697e39e..96f40d0 100644 --- a/src/DAMA/DoctrineTestBundle/Doctrine/DBAL/StaticConnectionFactory.php +++ b/src/DAMA/DoctrineTestBundle/Doctrine/DBAL/StaticConnectionFactory.php @@ -6,7 +6,6 @@ use Doctrine\Common\EventManager; use Doctrine\DBAL\Configuration; use Doctrine\DBAL\Connection; -use Doctrine\DBAL\Events; class StaticConnectionFactory extends ConnectionFactory { @@ -29,8 +28,6 @@ public function createConnection(array $params, Configuration $config = null, Ev return $connection; } - $connection->getEventManager()->addEventListener(Events::postConnect, new PostConnectEventListener()); - // Make sure we use savepoints to be able to easily roll-back nested transactions if ($connection->getDriver()->getDatabasePlatform()->supportsSavepoints()) { $connection->setNestTransactionsWithSavepoints(true); diff --git a/tests/DAMA/DoctrineTestBundle/DependencyInjection/DAMADoctrineTestExtensionTest.php b/tests/DAMA/DoctrineTestBundle/DependencyInjection/DAMADoctrineTestExtensionTest.php index 6be10ed..83e9330 100644 --- a/tests/DAMA/DoctrineTestBundle/DependencyInjection/DAMADoctrineTestExtensionTest.php +++ b/tests/DAMA/DoctrineTestBundle/DependencyInjection/DAMADoctrineTestExtensionTest.php @@ -3,7 +3,6 @@ namespace Tests\DAMA\DoctrineTestBundle\DependencyInjection; use DAMA\DoctrineTestBundle\DependencyInjection\DAMADoctrineTestExtension; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -12,14 +11,15 @@ class DAMADoctrineTestExtensionTest extends TestCase /** * @dataProvider loadDataProvider */ - public function testLoad(array $configs, array $expectedProcessedConfig): void + public function testLoad(array $configs, array $expectedParameters): void { $extension = new DAMADoctrineTestExtension(); - /** @var ContainerBuilder|MockObject $containerBuilder */ - $containerBuilder = $this->createMock(ContainerBuilder::class); + $containerBuilder = new ContainerBuilder(); $extension->load($configs, $containerBuilder); - $this->assertEquals($extension->getProcessedConfig(), $expectedProcessedConfig); + foreach ($expectedParameters as $name => $value) { + $this->assertSame($value, $containerBuilder->getParameter('dama.'.$name)); + } } public function loadDataProvider(): array diff --git a/tests/DAMA/DoctrineTestBundle/DependencyInjection/DoctrineTestCompilerPassTest.php b/tests/DAMA/DoctrineTestBundle/DependencyInjection/DoctrineTestCompilerPassTest.php index 90e83e5..10a9005 100644 --- a/tests/DAMA/DoctrineTestBundle/DependencyInjection/DoctrineTestCompilerPassTest.php +++ b/tests/DAMA/DoctrineTestBundle/DependencyInjection/DoctrineTestCompilerPassTest.php @@ -4,6 +4,7 @@ use DAMA\DoctrineTestBundle\DependencyInjection\DAMADoctrineTestExtension; use DAMA\DoctrineTestBundle\DependencyInjection\DoctrineTestCompilerPass; +use DAMA\DoctrineTestBundle\DependencyInjection\RegisterDoctrineEventListenersPass; use DAMA\DoctrineTestBundle\Doctrine\Cache\Psr6StaticArrayCache; use DAMA\DoctrineTestBundle\Doctrine\Cache\StaticArrayCache; use Doctrine\Bundle\DoctrineBundle\ConnectionFactory; @@ -59,8 +60,13 @@ public function testProcess(array $config, callable $assertCallback, callable $e $expectationCallback($this, $containerBuilder); } + (new RegisterDoctrineEventListenersPass())->process($containerBuilder); (new DoctrineTestCompilerPass())->process($containerBuilder); + foreach (array_keys($containerBuilder->getParameterBag()->all()) as $parameterName) { + $this->assertStringStartsNotWith('dama.', $parameterName); + } + $assertCallback($containerBuilder); } diff --git a/tests/DAMA/DoctrineTestBundle/Doctrine/DBAL/MockDriver.php b/tests/DAMA/DoctrineTestBundle/Doctrine/DBAL/MockDriver.php index 9af5186..ed65124 100644 --- a/tests/DAMA/DoctrineTestBundle/Doctrine/DBAL/MockDriver.php +++ b/tests/DAMA/DoctrineTestBundle/Doctrine/DBAL/MockDriver.php @@ -6,6 +6,7 @@ use Doctrine\DBAL\Driver; use Doctrine\DBAL\Driver\API\ExceptionConverter; use Doctrine\DBAL\Platforms\AbstractPlatform; +use Doctrine\DBAL\Platforms\MySQL80Platform; use Doctrine\DBAL\Schema\AbstractSchemaManager; use PHPUnit\Framework\MockObject\Generator; @@ -35,7 +36,7 @@ public function connect(array $params): \Doctrine\DBAL\Driver\Connection */ public function getDatabasePlatform(): AbstractPlatform { - return $this->getMock(AbstractPlatform::class); + return new MySQL80Platform(); } /** diff --git a/tests/DAMA/DoctrineTestBundle/Doctrine/DBAL/StaticConnectionFactoryTest.php b/tests/DAMA/DoctrineTestBundle/Doctrine/DBAL/StaticConnectionFactoryTest.php index 1d99187..a27580b 100644 --- a/tests/DAMA/DoctrineTestBundle/Doctrine/DBAL/StaticConnectionFactoryTest.php +++ b/tests/DAMA/DoctrineTestBundle/Doctrine/DBAL/StaticConnectionFactoryTest.php @@ -12,7 +12,7 @@ class StaticConnectionFactoryTest extends TestCase /** * @dataProvider createConnectionDataProvider */ - public function testCreateConnection(bool $keepStaticConnections, array $params, int $expectedNestingLevel): void + public function testCreateConnection(bool $keepStaticConnections, array $params, bool $expectedNestingWithSavepoints): void { $factory = new StaticConnectionFactory(new ConnectionFactory([])); @@ -24,10 +24,7 @@ public function testCreateConnection(bool $keepStaticConnections, array $params, $this->assertInstanceOf(MockDriver::class, $connection->getDriver()); - $this->assertSame(0, $connection->getTransactionNestingLevel()); - - $connection->connect(); - $this->assertSame($expectedNestingLevel, $connection->getTransactionNestingLevel()); + $this->assertSame($expectedNestingWithSavepoints, $connection->getNestTransactionsWithSavepoints()); } public function createConnectionDataProvider(): \Generator @@ -35,19 +32,19 @@ public function createConnectionDataProvider(): \Generator yield 'disabled by static property' => [ false, ['dama.keep_static' => true], - 0, + false, ]; yield 'disabled by param' => [ true, ['dama.keep_static' => false], - 0, + false, ]; yield 'enabled' => [ true, ['dama.keep_static' => true, 'dama.connection_name' => 'a'], - 1, + true, ]; } } diff --git a/tests/Functional/FunctionalTestTrait.php b/tests/Functional/FunctionalTestTrait.php index a56ee0c..a4ca6da 100644 --- a/tests/Functional/FunctionalTestTrait.php +++ b/tests/Functional/FunctionalTestTrait.php @@ -24,9 +24,7 @@ trait FunctionalTestTrait */ public function setUp(): void { - $this->kernel = new AppKernel('test', true); - $this->kernel->boot(); - $this->connection = $this->kernel->getContainer()->get('doctrine.dbal.default_connection'); + $this->init(); } /** @@ -95,4 +93,11 @@ public function rollbackSavepoint(string $name): void { $this->connection->rollbackSavepoint($name); } + + private function init(): void + { + $this->kernel = new AppKernel('test', true); + $this->kernel->boot(); + $this->connection = $this->kernel->getContainer()->get('doctrine.dbal.default_connection'); + } } diff --git a/tests/Functional/PhpunitTest.php b/tests/Functional/PhpunitTest.php index efeb3d0..4d86bbf 100644 --- a/tests/Functional/PhpunitTest.php +++ b/tests/Functional/PhpunitTest.php @@ -2,6 +2,7 @@ namespace Tests\Functional; +use DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticDriver; use Doctrine\DBAL\Exception\TableNotFoundException; use PHPUnit\Framework\TestCase; @@ -90,4 +91,25 @@ public function testWillThrowSpecificException(): void $this->expectException(TableNotFoundException::class); $this->connection->insert('does_not_exist', ['foo' => 'bar']); } + + public function testTransactionalBehaviorCanBeDisabledDuringRuntime(): void + { + StaticDriver::setKeepStaticConnections(false); + + $this->kernel->shutdown(); + $this->init(); + + $this->insertRow(); + $this->assertRowCount(1); + + StaticDriver::setKeepStaticConnections(true); + } + + /** + * @depends testTransactionalBehaviorCanBeDisabledDuringRuntime + */ + public function testChangesFromPreviousTestAreVisibleWhenDisabledDuringRuntime(): void + { + $this->assertRowCount(1); + } }