Fix DBAL and ORM configuration inheritance to not overide default_con…

…nection and default_entity_manager This another take at fixing 1337 but also at dbal section that has the same issue with `default_connection`
doctrine · Apr 14, 2023 · 34af8c5 · 34af8c5
1 parent 0464322
commit 34af8c5
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 23 deletions.
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -72,16 +72,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 considered?
+                        return !empty(array_filter($v, function ($key) use ($excludedKeys) {
+                            return !isset($excludedKeys[$key]);
+                        }, ARRAY_FILTER_USE_KEY));
                     })
-                    ->then(static function ($v) {
-                        // Key that should not be rewritten to the connection config
-                        $excludedKeys = ['default_connection' => true, 'types' => true, 'type' => true];
+                    ->then(static function ($v) use ($excludedKeys) {
                         $connection   = [];
                         foreach ($v as $key => $value) {
                             if (isset($excludedKeys[$key])) {
@@ -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,47 @@ 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 ($v === null) {
+                                return false;
+                            }
+
+                            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 considered?
+                            return !empty(array_filter($v, function ($key) use ($excludedKeys) {
+                                return !isset($excludedKeys[$key]);
+                            }, ARRAY_FILTER_USE_KEY));
                         })
-                        ->then(static function ($v) {
+                        ->then(static function ($v) use ($excludedKeys) {
                             $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,
-                            ];
+
                             $entityManager = [];
                             foreach ($v as $key => $value) {
                                 if (isset($excludedKeys[$key])) {
@@ -414,8 +441,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
@@ -91,6 +91,19 @@ public function load(array $configs, ContainerBuilder $container)
         $config        = $this->processConfiguration($configuration, $configs);
 
         if (! empty($config['dbal'])) {
+            // 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 (empty($config['dbal']['connections'])) {
+                $config = $this->processConfiguration($configuration, array_merge([[
+                    'dbal' => [
+                        'connections' => [
+                            ($config['dbal']['default_connection'] ?? 'default') => [],
+                        ],
+                    ],
+                ]], $configs));
+            }
+
             $this->dbalLoad($config['dbal'], $container);
 
             $this->loadMessengerServices($container);
@@ -104,6 +117,18 @@ public function load(array $configs, ContainerBuilder $container)
             throw new LogicException('Configuring the ORM layer requires to configure the DBAL layer as well.');
         }
 
+        // if no EM defined, prepend an empty one for the default EM name in
+        // order to make Symfony Config resolve the default values
+        if (empty($config['orm']['entity_managers'])) {
+            $config = $this->processConfiguration($configuration, array_merge([[
+                'orm' => [
+                    'entity_managers' => [
+                        ($config['orm']['default_entity_manager'] ?? 'default') => [],
+                    ],
+                ],
+            ]], $configs));
+        }
+
         $this->ormLoad($config['orm'], $container);
     }
 

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)) {
@@ -695,6 +709,27 @@ 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();
+
+        $config        = BundleConfigurationBuilder::createBuilder()
+            ->addBaseConnection()
+            ->build();
+        $config['orm'] = ['default_entity_manager' => 'app', 'entity_managers' => ['app' => ['mappings' => ['YamlBundle' => ['alias' => 'yml']]]]];
+        $extension->load([
+            $config,
+            ['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)) {