Merge branch 'fix-dep'

Closes #216. * fix-dep: Fix DependentFixtureInterface
doctrine · Nov 30, 2017 · 493961a · 493961a
2 parents d966d2a + eff2fc6
commit 493961a
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 11 deletions.
diff --git a/DependencyInjection/CompilerPass/FixturesCompilerPass.php b/DependencyInjection/CompilerPass/FixturesCompilerPass.php
@@ -30,8 +30,11 @@ public function process(ContainerBuilder $container)
         $definition = $container->getDefinition('doctrine.fixtures.loader');
         $taggedServices = $container->findTaggedServiceIds(self::FIXTURE_TAG);
 
+        $fixtures = [];
         foreach ($taggedServices as $serviceId => $tags) {
-            $definition->addMethodCall('addFixture', [new Reference($serviceId)]);
+            $fixtures[] = new Reference($serviceId);
         }
+
+        $definition->addMethodCall('addFixtures', [$fixtures]);
     }
 }
diff --git a/Loader/SymfonyFixturesLoader.php b/Loader/SymfonyFixturesLoader.php
@@ -19,8 +19,36 @@
  */
 final class SymfonyFixturesLoader extends ContainerAwareLoader
 {
+    private $loadedFixtures = [];
+
+    /**
+     * @internal
+     */
+    public function addFixtures(array $fixtures)
+    {
+        // Store all loaded fixtures so that we can resolve the dependencies correctly.
+        foreach ($fixtures as $fixture) {
+            $this->loadedFixtures[get_class($fixture)] = $fixture;
+        }
+
+        // Now load all the fixtures
+        foreach ($this->loadedFixtures as $fixture) {
+            $this->addFixture($fixture);
+        }
+    }
+
     public function addFixture(FixtureInterface $fixture)
     {
+        $class = get_class($fixture);
+        if (isset($this->loadedFixtures[$class]) && $this->loadedFixtures[$class] !== $fixture) {
+            throw new \LogicException(sprintf(
+                'The "%s" fixture class is already loaded and instantiated. It is not possible to add a new instance of this fixture. Upgrade to "doctrine/data-fixtures" version 1.3 or higher to support this.',
+                $class
+            ));
+        }
+
+        $this->loadedFixtures[$class] = $fixture;
+
         // see https://github.com/doctrine/data-fixtures/pull/274
         // this is to give a clear error if you do not have this version
         if (!method_exists(Loader::class, 'createFixture')) {
@@ -35,15 +63,18 @@ public function addFixture(FixtureInterface $fixture)
      */
     protected function createFixture($class)
     {
-        try {
-            /*
-             * We don't actually need to create the fixture. We just
-             * return the one that already exists.
-             */
-            return $this->getFixture($class);
-        } catch (\InvalidArgumentException $e) {
-            throw new \LogicException(sprintf('The "%s" fixture class is trying to be loaded, but is not available. Make sure this class is defined as a service and tagged with "%s".', $class, FixturesCompilerPass::FIXTURE_TAG));
+        /*
+         * We don't actually need to create the fixture. We just
+         * return the one that already exists.
+         */
+
+        if (!isset($this->loadedFixtures[$class])) {
+            throw new \LogicException(sprintf(
+                'The "%s" fixture class is trying to be loaded, but is not available. Make sure this class is defined as a service and tagged with "%s".', $class, FixturesCompilerPass::FIXTURE_TAG
+            ));
         }
+
+        return $this->loadedFixtures[$class];
     }
 
     /**

diff --git a/Tests/IntegrationTest.php b/Tests/IntegrationTest.php
@@ -4,7 +4,6 @@
 
 use Doctrine\Bundle\FixturesBundle\DependencyInjection\CompilerPass\FixturesCompilerPass;
 use Doctrine\Bundle\FixturesBundle\DoctrineFixturesBundle;
-use Doctrine\Bundle\FixturesBundle\EmptyFixture;
 use Doctrine\Bundle\FixturesBundle\Tests\Fixtures\FooBundle\DataFixtures\DependentOnRequiredConstructorArgsFixtures;
 use Doctrine\Bundle\FixturesBundle\Tests\Fixtures\FooBundle\DataFixtures\OtherFixtures;
 use Doctrine\Bundle\FixturesBundle\Tests\Fixtures\FooBundle\DataFixtures\RequiredConstructorArgsFixtures;
@@ -54,6 +53,39 @@ public function testFixturesLoader()
         $this->assertInstanceOf(WithDependenciesFixtures::class, $actualFixtures[1]);
     }
 
+    public function testFixturesLoaderWhenFixtureHasDepdencenyThatIsNotYetLoaded()
+    {
+        // See https://github.com/doctrine/DoctrineFixturesBundle/issues/215
+
+        $kernel = new IntegrationTestKernel('dev', true);
+        $kernel->addServices(function(ContainerBuilder $c) {
+            $c->autowire(WithDependenciesFixtures::class)
+                ->addTag(FixturesCompilerPass::FIXTURE_TAG);
+
+            $c->autowire(OtherFixtures::class)
+                ->addTag(FixturesCompilerPass::FIXTURE_TAG);
+
+            $c->setAlias('test.doctrine.fixtures.loader', new Alias('doctrine.fixtures.loader', true));
+        });
+        $kernel->boot();
+        $container = $kernel->getContainer();
+
+        /** @var ContainerAwareLoader $loader */
+        $loader = $container->get('test.doctrine.fixtures.loader');
+
+        $actualFixtures = $loader->getFixtures();
+        $this->assertCount(2, $actualFixtures);
+        $actualFixtureClasses = array_map(function($fixture) {
+            return get_class($fixture);
+        }, $actualFixtures);
+
+        $this->assertSame([
+            OtherFixtures::class,
+            WithDependenciesFixtures::class,
+        ], $actualFixtureClasses);
+        $this->assertInstanceOf(WithDependenciesFixtures::class, $actualFixtures[1]);
+    }
+
     /**
      * @expectedException \LogicException
      * @expectedExceptionMessage The getDependencies() method returned a class (Doctrine\Bundle\FixturesBundle\Tests\Fixtures\FooBundle\DataFixtures\RequiredConstructorArgsFixtures) that has required constructor arguments. Upgrade to "doctrine/data-fixtures" version 1.3 or higher to support this.
@@ -169,4 +201,4 @@ public function getLogDir()
     {
         return sys_get_temp_dir();
     }
-}
+}