Fix #18213: Do not load fixtures with circular dependencies twice instead of throwing an exception

framework/CHANGELOG.md

@@ -4,6 +4,7 @@ Yii Framework 2 Change Log
 2.0.38 under development
 ------------------------
 
+- Enh #18213: Do not load fixtures with circular dependencies twice instead of throwing an exception (JesseHines0)
 - Bug #18066: Fixed `yii\db\Query::create()` wasn't using all info from `withQuery()` (maximkou)
 - Bug #18269: Fix integer safe attribute to work properly in `yii\base\Model` (Ladone)
 - Enh #18236: Allow `yii\filters\RateLimiter` to accept a closure function for the `$user` property in order to assign values on runtime (nadar)

framework/test/FixtureTrait.php

@@ -165,12 +165,11 @@ trait FixtureTrait
 
     /**
      * Creates the specified fixture instances.
-     * All dependent fixtures will also be created.
+     * All dependent fixtures will also be created. Duplicate fixtures and circular dependencies will only be created once.
      * @param array $fixtures the fixtures to be created. You may provide fixture names or fixture configurations.
      * If this parameter is not provided, the fixtures specified in [[globalFixtures()]] and [[fixtures()]] will be created.
      * @return Fixture[] the created fixture instances
-     * @throws InvalidConfigException if fixtures are not properly configured or if a circular dependency among
-     * the fixtures is detected.
+     * @throws InvalidConfigException if fixtures are not properly configured
      */
     protected function createFixtures(array $fixtures)
     {
@@ -210,9 +209,8 @@ trait FixtureTrait
                         // need to use the configuration provided in test case
                         $stack[] = isset($config[$dep]) ? $config[$dep] : ['class' => $dep];
                     }
-                } elseif ($instances[$name] === false) {
-                    throw new InvalidConfigException("A circular dependency is detected for fixture '$class'.");
                 }
+                // if the fixture is already loaded (ie. a circular dependency or if two fixtures depend on the same fixture) just skip it.
             }
         }
 

tests/framework/test/FixtureTest.php

@@ -11,9 +11,6 @@ use yii\test\Fixture;
 use yii\test\FixtureTrait;
 use yiiunit\TestCase;
 
-/**
- * @group fixture
- */
 class Fixture1 extends Fixture
 {
     public $depends = ['yiiunit\framework\test\Fixture2'];
@@ -56,6 +53,35 @@ class Fixture3 extends Fixture
     }
 }
 
+class Fixture4 extends Fixture
+{
+    public $depends = ['yiiunit\framework\test\Fixture5'];
+    public function load()
+    {
+        MyTestCase::$load .= '4';
+    }
+
+    public function unload()
+    {
+        MyTestCase::$unload .= '4';
+    }
+}
+
+class Fixture5 extends Fixture
+{
+    public $depends = ['yiiunit\framework\test\Fixture4'];
+    public function load()
+    {
+        MyTestCase::$load .= '5';
+    }
+
+    public function unload()
+    {
+        MyTestCase::$unload .= '5';
+    }
+}
+
+
 class MyTestCase
 {
     use FixtureTrait;
@@ -104,16 +130,30 @@ class MyTestCase
                 'fixture1' => Fixture1::className(),
                 'fixture3' => Fixture3::className(),
             ];
-            case 7:
-            default: return [
+            case 7: return [
                 'fixture1' => Fixture1::className(),
                 'fixture2' => Fixture2::className(),
                 'fixture3' => Fixture3::className(),
             ];
+            case 8: return [
+                'fixture4' => Fixture4::className(),
+            ];
+            case 9: return [
+                'fixture5' => Fixture5::className(),
+                'fixture4' => Fixture4::className(),
+            ];
+            case 10: return [
+                'fixture3a' => Fixture3::className(), // duplicate fixtures may occur two fixtures depend on the same fixture.
+                'fixture3b' => Fixture3::className(),
+            ];
+            default: return [];
         }
     }
 }
 
+/**
+ * @group fixture
+ */
 class FixtureTest extends TestCase
 {
     public function testDependencies()
@@ -145,14 +185,16 @@ class FixtureTest extends TestCase
     protected function getDependencyTests()
     {
         return [
-            0 => ['fixture1' => false, 'fixture2' => false, 'fixture3' => false],
-            1 => ['fixture1' => true, 'fixture2' => false, 'fixture3' => false],
-            2 => ['fixture1' => false, 'fixture2' => true, 'fixture3' => false],
-            3 => ['fixture1' => false, 'fixture2' => false, 'fixture3' => true],
-            4 => ['fixture1' => true, 'fixture2' => true, 'fixture3' => false],
-            5 => ['fixture1' => false, 'fixture2' => true, 'fixture3' => true],
-            6 => ['fixture1' => true, 'fixture2' => false, 'fixture3' => true],
-            7 => ['fixture1' => true, 'fixture2' => true, 'fixture3' => true],
+            0 => ['fixture1' => false, 'fixture2' => false, 'fixture3' => false, 'fixture4' => false, 'fixture5' => false],
+            1 => ['fixture1' => true, 'fixture2' => false, 'fixture3' => false, 'fixture4' => false, 'fixture5' => false],
+            2 => ['fixture1' => false, 'fixture2' => true, 'fixture3' => false, 'fixture4' => false, 'fixture5' => false],
+            3 => ['fixture1' => false, 'fixture2' => false, 'fixture3' => true, 'fixture4' => false, 'fixture5' => false],
+            4 => ['fixture1' => true, 'fixture2' => true, 'fixture3' => false, 'fixture4' => false, 'fixture5' => false],
+            5 => ['fixture1' => false, 'fixture2' => true, 'fixture3' => true, 'fixture4' => false, 'fixture5' => false],
+            6 => ['fixture1' => true, 'fixture2' => false, 'fixture3' => true, 'fixture4' => false, 'fixture5' => false],
+            7 => ['fixture1' => true, 'fixture2' => true, 'fixture3' => true, 'fixture4' => false, 'fixture5' => false],
+            8 => ['fixture1' => false, 'fixture2' => false, 'fixture3' => false, 'fixture4' => true, 'fixture5' => false],
+            9 => ['fixture1' => false, 'fixture2' => false, 'fixture3' => false, 'fixture4' => true, 'fixture5' => true],
         ];
     }
 
@@ -167,6 +209,9 @@ class FixtureTest extends TestCase
             5 => ['32', '23'],
             6 => ['321', '123'],
             7 => ['321', '123'],
+            8 => ['54', '45'],
+            9 => ['45', '54'],
+            10 => ['3', '3'],
         ];
     }
 }