doctrine / data-fixtures

Doctrine2 ORM Data Fixtures Extensions
http://www.doctrine-project.org
MIT License
2.77k stars 224 forks source link

ORMExecutor wraps all fixtures in a single transaction #126

Open gnat42 opened 10 years ago

gnat42 commented 10 years ago

Hello,

So I have a situation where the current executor can't load my fixtures. I have three fixture files. The third one loaded has an entity that has a custom id generator. The id generator is based of an incrementing value in objects in the first fixture.

The current executor is

public function execute(array $fixtures, $append = false)
{
        $executor = $this;
        $this->em->transactional(function(EntityManager $em) use ($executor, $fixtures, $append) {
            if ($append === false) {
                $executor->purge();
            }
            foreach ($fixtures as $fixture) {
                $executor->load($em, $fixture);
            }
        });
}

I've used a couple of different machines with this, once got a segfault other times got other php fatal errors - don't have all of them at the moment but can dig up specific issues if necessary. In any case I'm wondering why the entire fixture loading is in one transaction. When I change that execute to

public function execute(array $fixtures, $append = false)
{
    if ($append === false) {
        $this->purge();
    }
    foreach ($fixtures as $fixture) {
        $this->em->beginTransaction();
        $this->load($this->em, $fixture);
        $this->em->commit();
    }
}

My fixtures load.

So I'm not sure why the entire process needs to be one transaction as opposed to a transaction per fixture file. Thoughts?

stof commented 10 years ago

the goal is to have atomicity of the whole loading: it either succeeds or fails cleanly (i.e. leaving the DB in its previous state, not in an intermediary one)

gnat42 commented 10 years ago

I can understand that in a non-fixtures case - however with fixtures which is by definition just test data of some sort it would be nice to be able to perhaps expose this as a choice. I may want to inspect why fixture # 3 for example failed and need to see the state of the DB or some such. Otherwise I'm not sure how or where the bug is that causes my third fixture to fail to load with the custom id generator... The one thing is that my id generator uses a transaction itself but I presume transactions can be nested?

gnat42 commented 10 years ago

something like:

diff --git a/lib/Doctrine/Common/DataFixtures/Executor/AbstractExecutor.php b/lib/Doctrine/Common/DataFixtures/Executor/AbstractExecutor.php
index 94b3ff2..71f1435 100644
--- a/lib/Doctrine/Common/DataFixtures/Executor/AbstractExecutor.php
+++ b/lib/Doctrine/Common/DataFixtures/Executor/AbstractExecutor.php
@@ -141,6 +141,7 @@ abstract class AbstractExecutor
      *
      * @param array $fixtures Array of fixtures to execute.
      * @param boolean $append Whether to append the data fixtures or purge the database before loading.
+     * @param boolean $single_transaction Whether to use a single transaction when loading fixtures
      */
-    abstract public function execute(array $fixtures, $append = false);
+    abstract public function execute(array $fixtures, $append = false, $single_transaction = true);
 }
diff --git a/lib/Doctrine/Common/DataFixtures/Executor/ORMExecutor.php b/lib/Doctrine/Common/DataFixtures/Executor/ORMExecutor.php
index cc228eb..a2ad028 100644
--- a/lib/Doctrine/Common/DataFixtures/Executor/ORMExecutor.php
+++ b/lib/Doctrine/Common/DataFixtures/Executor/ORMExecutor.php
@@ -72,16 +72,27 @@ class ORMExecutor extends AbstractExecutor
     }

     /** @inheritDoc */
-    public function execute(array $fixtures, $append = false)
+    public function execute(array $fixtures, $append = false, $single_transaction = true)
     {
-        $executor = $this;
-        $this->em->transactional(function(EntityManager $em) use ($executor, $fixtures, $append) {
-            if ($append === false) {
-                $executor->purge();
+        if($single_transaction) {
+            $executor = $this;
+            $this->em->transactional(function(EntityManager $em) use ($executor, $fixtures, $append) {
+                if ($append === false) {
+                    $executor->purge();
+                }
+                foreach ($fixtures as $fixture) {
+                    $executor->load($em, $fixture);
+                }
+            });
+        } else {
+            if($append === false) {
+                $this->purge();
             }
             foreach ($fixtures as $fixture) {
-                $executor->load($em, $fixture);
+                $this->em->beginTransaction();
+                $this->load($this->em, $fixture);
+                $this->em->commit();
             }
-        });
+        }
     }
 }
gnat42 commented 10 years ago

I've created a pull request - wasn't sure how to link it with this request so its pull request #132

bassrock commented 10 years ago

What if we also add the ability to let the user manage the transactions normally with a persist/flush.

I use the doctrine fixtures to load data using my utility classes and I use Resque to process tasks in the background, but they fail running because the entities are not in the database yet.