Merge pull request #1054 from doctrine/circular-refs-resolution

Fix circular ref in dependency resolution
doctrine · Feb 7, 2021 · d761978 · d761978
2 parents 780b67e + e6bf877
commit d761978
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 1 deletion.
diff --git a/lib/Doctrine/Migrations/DependencyFactory.php b/lib/Doctrine/Migrations/DependencyFactory.php
@@ -59,6 +59,9 @@
  */
 class DependencyFactory
 {
+    /** @psalm-var array<string, bool> */
+    private $inResolution = [];
+
     /** @var Configuration */
     private $configuration;
 
@@ -476,8 +479,10 @@ public function getRollup(): Rollup
      */
     private function getDependency(string $id, callable $callback)
     {
-        if (array_key_exists($id, $this->factories) && ! array_key_exists($id, $this->dependencies)) {
+        if (! isset($this->inResolution[$id]) && array_key_exists($id, $this->factories) && ! array_key_exists($id, $this->dependencies)) {
+            $this->inResolution[$id] = true;
             $this->dependencies[$id] = call_user_func($this->factories[$id], $this);
+            unset($this->inResolution);
         }
 
         if (! array_key_exists($id, $this->dependencies)) {

diff --git a/tests/Doctrine/Migrations/Tests/DependencyFactoryTest.php b/tests/Doctrine/Migrations/Tests/DependencyFactoryTest.php
@@ -16,6 +16,9 @@
 use Doctrine\Migrations\Finder\GlobFinder;
 use Doctrine\Migrations\Finder\RecursiveRegexFinder;
 use Doctrine\Migrations\Metadata\Storage\TableMetadataStorageConfiguration;
+use Doctrine\Migrations\Tests\MigrationRepository\Migrations\A\A;
+use Doctrine\Migrations\Tests\Stub\CustomClassNameMigrationFactory;
+use Doctrine\Migrations\Version\MigrationFactory;
 use Doctrine\ORM\EntityManager;
 use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
@@ -156,6 +159,23 @@ public function testServiceDefinition(): void
         self::assertSame($anotherLogger, $di->getLogger());
     }
 
+    public function testServiceDecoratesDefaultImplementation(): void
+    {
+        $configuration = new Configuration();
+        $configuration->addMigrationClass(A::class);
+
+        $di = DependencyFactory::fromConnection(new ExistingConfiguration($configuration), new ExistingConnection($this->connection));
+
+        $di->setDefinition(MigrationFactory::class, static function (DependencyFactory $innerDi): CustomClassNameMigrationFactory {
+            $serviceToDecorate = $innerDi->getMigrationFactory();
+
+            return new CustomClassNameMigrationFactory($serviceToDecorate, A::class);
+        });
+
+        $factory = $di->getMigrationFactory();
+        self::assertInstanceOf(A::class, $factory->createVersion('SomeVersion'));
+    }
+
     public function testServiceHasPriorityOverDefinition(): void
     {
         $logger        = $this->createMock(LoggerInterface::class);

diff --git a/tests/Doctrine/Migrations/Tests/Stub/CustomClassNameMigrationFactory.php b/tests/Doctrine/Migrations/Tests/Stub/CustomClassNameMigrationFactory.php
@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Migrations\Tests\Stub;
+
+use Doctrine\Migrations\AbstractMigration;
+use Doctrine\Migrations\Version\MigrationFactory;
+
+class CustomClassNameMigrationFactory implements MigrationFactory
+{
+    /** @var MigrationFactory */
+    private $parentMigrationFactory;
+
+    /** @psalm-var class-string<AbstractMigration> */
+    private $migrationClassName;
+
+    /**
+     * @param class-string<AbstractMigration> $migrationClassName
+     */
+    public function __construct(MigrationFactory $parentMigrationFactory, string $migrationClassName)
+    {
+        $this->parentMigrationFactory = $parentMigrationFactory;
+        $this->migrationClassName     = $migrationClassName;
+    }
+
+    public function createVersion(string $migrationClassName): AbstractMigration
+    {
+        return $this->parentMigrationFactory->createVersion($this->migrationClassName);
+    }
+}