Fix issue 191 (#214)

dmaicher · Jul 1, 2022 · 279a554 · 279a554
1 parent 1cf583f
commit 279a554
Show file tree

Hide file tree

Showing 12 changed files with 130 additions and 56 deletions.
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,51 @@
+<?php
+
+namespace DAMA\DoctrineTestBundle\DependencyInjection;
+
+use DAMA\DoctrineTestBundle\Doctrine\DBAL\PostConnectEventListener;
+use Doctrine\DBAL\Events;
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+
+final class RegisterDoctrineEventListenersPass implements CompilerPassInterface
+{
+    public function process(ContainerBuilder $container): void
+    {
+        /** @var bool|array $enableStaticConnectionsConfig */
+        $enableStaticConnectionsConfig = $container->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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,30 +24,27 @@ 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
     {
         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
@@ -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
@@ -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);
+    }
 }