Fix configuration inheritance to not override default connection/EM v…

…alues (#1648)
doctrine · Apr 21, 2023 · 1550341 · 1550341
1 parent 0464322
commit 1550341
Show file tree

Hide file tree

Showing 3 changed files with 134 additions and 26 deletions.
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -14,6 +14,7 @@
 use Symfony\Component\Config\Definition\ConfigurationInterface;
 use Symfony\Component\DependencyInjection\Exception\LogicException;
 
+use function array_diff_key;
 use function array_intersect_key;
 use function array_key_exists;
 use function array_keys;
@@ -72,17 +73,27 @@ public function getConfigTreeBuilder(): TreeBuilder
      */
     private function addDbalSection(ArrayNodeDefinition $node): void
     {
+        // Key that should not be rewritten to the connection config
+        $excludedKeys = ['default_connection' => true, 'types' => true, 'type' => true];
+
         $node
             ->children()
             ->arrayNode('dbal')
                 ->beforeNormalization()
-                    ->ifTrue(static function ($v) {
-                        return is_array($v) && ! array_key_exists('connections', $v) && ! array_key_exists('connection', $v);
+                    ->ifTrue(static function ($v) use ($excludedKeys) {
+                        if (! is_array($v)) {
+                            return false;
+                        }
+
+                        if (array_key_exists('connections', $v) || array_key_exists('connection', $v)) {
+                            return false;
+                        }
+
+                        // Is there actually anything to use once excluded keys are considered?
+                        return (bool) array_diff_key($v, $excludedKeys);
                     })
-                    ->then(static function ($v) {
-                        // Key that should not be rewritten to the connection config
-                        $excludedKeys = ['default_connection' => true, 'types' => true, 'type' => true];
-                        $connection   = [];
+                    ->then(static function ($v) use ($excludedKeys) {
+                        $connection = [];
                         foreach ($v as $key => $value) {
                             if (isset($excludedKeys[$key])) {
                                 continue;
@@ -92,8 +103,7 @@ private function addDbalSection(ArrayNodeDefinition $node): void
                             unset($v[$key]);
                         }
 
-                        $v['default_connection'] = isset($v['default_connection']) ? (string) $v['default_connection'] : 'default';
-                        $v['connections']        = [$v['default_connection'] => $connection];
+                        $v['connections'] = [($v['default_connection'] ?? 'default') => $connection];
 
                         return $v;
                     })
@@ -380,30 +390,39 @@ private function configureDbalDriverNode(ArrayNodeDefinition $node): void
      */
     private function addOrmSection(ArrayNodeDefinition $node): void
     {
+        // Key that should not be rewritten to the entity-manager config
+        $excludedKeys = [
+            'default_entity_manager' => true,
+            'auto_generate_proxy_classes' => true,
+            'enable_lazy_ghost_objects' => true,
+            'proxy_dir' => true,
+            'proxy_namespace' => true,
+            'resolve_target_entities' => true,
+            'resolve_target_entity' => true,
+            'controller_resolver' => true,
+        ];
+
         $node
             ->children()
                 ->arrayNode('orm')
                     ->beforeNormalization()
-                        ->ifTrue(static function ($v) {
+                        ->ifTrue(static function ($v) use ($excludedKeys) {
                             if (! empty($v) && ! class_exists(EntityManager::class)) {
                                 throw new LogicException('The doctrine/orm package is required when the doctrine.orm config is set.');
                             }
 
-                            return $v === null || (is_array($v) && ! array_key_exists('entity_managers', $v) && ! array_key_exists('entity_manager', $v));
+                            if (! is_array($v)) {
+                                return false;
+                            }
+
+                            if (array_key_exists('entity_managers', $v) || array_key_exists('entity_manager', $v)) {
+                                return false;
+                            }
+
+                            // Is there actually anything to use once excluded keys are considered?
+                            return (bool) array_diff_key($v, $excludedKeys);
                         })
-                        ->then(static function ($v) {
-                            $v = (array) $v;
-                            // Key that should not be rewritten to the entity-manager config
-                            $excludedKeys  = [
-                                'default_entity_manager' => true,
-                                'auto_generate_proxy_classes' => true,
-                                'enable_lazy_ghost_objects' => true,
-                                'proxy_dir' => true,
-                                'proxy_namespace' => true,
-                                'resolve_target_entities' => true,
-                                'resolve_target_entity' => true,
-                                'controller_resolver' => true,
-                            ];
+                        ->then(static function ($v) use ($excludedKeys) {
                             $entityManager = [];
                             foreach ($v as $key => $value) {
                                 if (isset($excludedKeys[$key])) {
@@ -414,8 +433,7 @@ private function addOrmSection(ArrayNodeDefinition $node): void
                                 unset($v[$key]);
                             }
 
-                            $v['default_entity_manager'] = isset($v['default_entity_manager']) ? (string) $v['default_entity_manager'] : 'default';
-                            $v['entity_managers']        = [$v['default_entity_manager'] => $entityManager];
+                            $v['entity_managers'] = [($v['default_entity_manager'] ?? 'default') => $entityManager];
 
                             return $v;
                         })

diff --git a/DependencyInjection/DoctrineExtension.php b/DependencyInjection/DoctrineExtension.php
@@ -45,6 +45,7 @@
 use Symfony\Bridge\Doctrine\Validator\DoctrineLoader;
 use Symfony\Component\Cache\Adapter\ArrayAdapter;
 use Symfony\Component\Cache\Adapter\PhpArrayAdapter;
+use Symfony\Component\Config\Definition\ConfigurationInterface;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\Alias;
 use Symfony\Component\DependencyInjection\ChildDefinition;
@@ -62,6 +63,7 @@
 
 use function array_intersect_key;
 use function array_keys;
+use function array_merge;
 use function class_exists;
 use function interface_exists;
 use function is_dir;
@@ -88,7 +90,7 @@ class DoctrineExtension extends AbstractDoctrineExtension
     public function load(array $configs, ContainerBuilder $container)
     {
         $configuration = $this->getConfiguration($configs, $container);
-        $config        = $this->processConfiguration($configuration, $configs);
+        $config        = $this->processConfigurationPrependingDefaults($configuration, $configs);
 
         if (! empty($config['dbal'])) {
             $this->dbalLoad($config['dbal'], $container);
@@ -107,6 +109,40 @@ public function load(array $configs, ContainerBuilder $container)
         $this->ormLoad($config['orm'], $container);
     }
 
+    /**
+     * Process user configuration and adds a default DBAL connection and/or a
+     * default EM if required, then process again the configuration to get
+     * default values for each.
+     *
+     * @param array<array<mixed>> $configs
+     *
+     * @return array<mixed>
+     */
+    private function processConfigurationPrependingDefaults(ConfigurationInterface $configuration, array $configs): array
+    {
+        $config      = $this->processConfiguration($configuration, $configs);
+        $configToAdd = [];
+
+        // if no DB connection defined, prepend an empty one for the default
+        // connection name in order to make Symfony Config resolve the default
+        // values
+        if (isset($config['dbal']) && empty($config['dbal']['connections'])) {
+            $configToAdd['dbal'] = ['connections' => [($config['dbal']['default_connection'] ?? 'default') => []]];
+        }
+
+        // if no EM defined, prepend an empty one for the default EM name in
+        // order to make Symfony Config resolve the default values
+        if (isset($config['orm']) && empty($config['orm']['entity_managers'])) {
+            $configToAdd['orm'] = ['entity_managers' => [($config['orm']['default_entity_manager'] ?? 'default') => []]];
+        }
+
+        if (! $configToAdd) {
+            return $config;
+        }
+
+        return $this->processConfiguration($configuration, array_merge([$configToAdd], $configs));
+    }
+
     /**
      * Loads the DBAL configuration.
      *

diff --git a/Tests/DependencyInjection/DoctrineExtensionTest.php b/Tests/DependencyInjection/DoctrineExtensionTest.php
@@ -217,6 +217,20 @@ public function testDbalOverrideDefaultConnection(): void
         $this->assertEquals('foo', $container->getParameter('doctrine.default_connection'), '->load() overrides existing configuration options');
     }
 
+    public function testDbalOverrideDefaultConnectionWithAdditionalConfiguration(): void
+    {
+        $container = $this->getContainer();
+        $extension = new DoctrineExtension();
+
+        $container->registerExtension($extension);
+
+        $extension->load([['dbal' => ['default_connection' => 'foo']], ['dbal' => ['types' => ['foo' => 'App\\Doctrine\\FooType']]]], $container);
+
+        // doctrine.dbal.default_connection
+        $this->assertEquals('%doctrine.default_connection%', $container->getDefinition('doctrine')->getArgument(3), '->load() overrides existing configuration options');
+        $this->assertEquals('foo', $container->getParameter('doctrine.default_connection'), '->load() overrides existing configuration options');
+    }
+
     public function testOrmRequiresDbal(): void
     {
         if (! interface_exists(EntityManagerInterface::class)) {
@@ -586,6 +600,29 @@ public function testSingleEntityManagerWithDefaultConfiguration(): void
         ]);
     }
 
+    /**
+     * @testWith [[]]
+     *           [null]
+     */
+    public function testSingleEntityManagerWithEmptyConfiguration(?array $ormConfiguration): void
+    {
+        if (! interface_exists(EntityManagerInterface::class)) {
+            self::markTestSkipped('This test requires ORM');
+        }
+
+        $container = $this->getContainer();
+        $extension = new DoctrineExtension();
+
+        $extension->load([
+            [
+                'dbal' => [],
+                'orm' => $ormConfiguration,
+            ],
+        ], $container);
+
+        $this->assertEquals('default', $container->getParameter('doctrine.default_entity_manager'));
+    }
+
     public function testSingleEntityManagerWithDefaultSecondLevelCacheConfiguration(): void
     {
         if (! interface_exists(EntityManagerInterface::class)) {
@@ -695,6 +732,23 @@ public function testOverwriteEntityAliases(): void
         );
     }
 
+    public function testOverrideDefaultEntityManagerWithAdditionalConfiguration(): void
+    {
+        if (! interface_exists(EntityManagerInterface::class)) {
+            self::markTestSkipped('This test requires ORM');
+        }
+
+        $container = $this->getContainer();
+        $extension = new DoctrineExtension();
+
+        $extension->load([
+            ['dbal' => [], 'orm' => ['default_entity_manager' => 'app', 'entity_managers' => ['app' => ['mappings' => ['YamlBundle' => ['alias' => 'yml']]]]]],
+            ['orm' => ['metadata_cache_driver' => ['type' => 'pool', 'pool' => 'doctrine.system_cache_pool']]],
+        ], $container);
+
+        $this->assertEquals('app', $container->getParameter('doctrine.default_entity_manager'));
+    }
+
     public function testYamlBundleMappingDetection(): void
     {
         if (! interface_exists(EntityManagerInterface::class)) {