Codeception / module-doctrine

Next gen Doctrine module for Codeception
MIT License
2 stars 1 forks source link

"The EntityManager is closed" with Symfony and Doctrine2 modules #19

Open benr77 opened 5 years ago

benr77 commented 5 years ago

What are you trying to achieve?

Use the Symfony and Doctrine2 modules together, to allow "cleanup" by reversing transactions between tests, in functional tests against an SQLite database.

What do you get instead?

Many tests are simply failing with

[Doctrine\ORM\ORMException] The EntityManager is closed.

If I do not enable the Doctrine2 module, the tests all succeed. But without it enabled, the transactions are not implemented and reversed between tests.

functional.suite.yml is as follows

class_name: FunctionalTester
modules:
  enabled:
    - \Helper\Functional
    - Asserts
    - Cli
    - Filesystem
    - REST
    - Symfony:
        environment: test
        var_path: 'var'
    - Doctrine2:
        depends: Symfony
        cleanup: true

Details

freiondrej commented 5 years ago

Hello @benr77,

the problem can be caused by a \Throwable thrown during a transaction. When you look into the code of method UnitOfWork#commit(), it does an automatic rollback after a \Throwable is thrown and it closes EntityManager. When trying to work with EntityManager after this, an exception is thrown because EntityManager is already closed. I hope this will be of any help to you :)

Amunak commented 5 years ago

@freiondrej, did you find a workaround? I have the same issue, and besides writing some dirty patch for Codeception I found no solution.

Also would be great to hear from the devs (@DavertMik?) whether they plan to fix this...

Amunak commented 5 years ago

Well apparently this fixes the issue by reopening the entity manager:

--- /dev/null
+++ src/Codeception/Module/Symfony.php
@@ -231,9 +231,15 @@
         if ($this->kernel === null) {
             $this->fail('Symfony2 platform module is not loaded');
         }
-        if (!isset($this->permanentServices[$this->config['em_service']])) {
+        if (!isset($this->permanentServices[$this->config['em_service']]) || !$this->permanentServices[$this->config['em_service']]->isOpen()) {
             // try to persist configured EM
             $this->persistService($this->config['em_service'], true);
+
+            // based on https://github.com/symfony/symfony/issues/5339
+            if (!$this->permanentServices[$this->config['em_service']]->isOpen()) {
+                $this->grabService('doctrine')->resetManager();
+                $this->persistService($this->config['em_service'], true);
+            }

             if ($this->_getContainer()->has('doctrine')) {
                 $this->persistService('doctrine', true);

see also symfony/symfony#5339

freiondrej commented 5 years ago

@Amunak Hi, is it no option for you to make the transaction not fail? That fixed it for me - I was actually looking for the wrong error as the "entity manager is closed" error attracted my attention while it should have been the one causing the rollback.

Amunak commented 5 years ago

It's not an option since I actually need to test some cases where the rollback (and following EM closing) happens, as it's a regular outcome of some ("botched") requests.

And even if your goal was to not have the entity manager closed at any time during the test, one test should never influence the others, which is what happens here - every test after the one that has closed EM fails. That is undesirable.

freiondrej commented 5 years ago

@Amunak understood. Unfortunately I cannot have any advice for this situation :(

Amunak commented 5 years ago

@freiondrej the patch I posted above works :)

Dukecz commented 5 years ago

@Amunak patch posted is not enough in order to allow persisting entitites in following cests in some cases.

Failed transaction will set $_isRollbackOnly variable in doctrine connection https://github.com/doctrine/dbal/blob/v2.8.0/lib/Doctrine/DBAL/Connection.php#L202 to true and reseting the entity manager won't change that. So new connection for next cest still keeps the variable and deny any persists into transaction.

So after cest all open nested transactions (created inside cest) and the transaction made by codeception for cest itself must be rolled back:

--- src/Codeception/Module/Doctrine2.php
+++ src/Codeception/Module/Doctrine2.php
@@ -157,7 +157,9 @@

         if ($this->config['cleanup'] && $this->em->getConnection()->isTransactionActive()) {
             try {
-                $this->em->getConnection()->rollback();
+                while ($this->em->getConnection()->getTransactionNestingLevel() > 0) {
+                    $this->em->getConnection()->rollback();
+                }
                 $this->debugSection('Database', 'Transaction cancelled; all changes reverted.');
             } catch (\PDOException $e) {
             }
redthor commented 4 years ago

FWIW I created the following in src/Company/AppBundle/tests/_support/Helper/Functional.php:

    /**
     * After an exception is thrown the entity manager will be closed.
     * An example is a unique constraint exception.
     * This will create a new entity manager so tests can continue.
     *
     * @see https://github.com/Codeception/Codeception/pull/5447
     *
     * @throws \Codeception\Exception\ModuleException
     */
    public function reopenEntityManager()
    {
        /** @var Doctrine2 $doctrineModule */
        $doctrineModule = $this->getModule('Doctrine2');

        // If the em is fine then there is nothing to do
        if ($doctrineModule->em->isOpen()) {
            return;
        }

        /** @var Symfony $sfModule */
        $sfModule = $this->getModule('Symfony');
        // Container can change between tests
        $this->container = $sfModule->_getContainer();

        // Get a new EM
        $em = $this->container->get('doctrine')->resetManager('app');
        // Set it in the Symfony module container
        $this->container->set('doctrine.orm.app_entity_manager', $em);
        // Ensure the symfony module has the new EM
        $sfModule->persistService('doctrine.orm.app_entity_manager', true);

        // And ensure that the doctrine module has the new EM
        $doctrineModule->em = $em;
    }

In some tests I expect an exception to close the EntityManager. In those cases I:

$I->reopenEntityManager();

I'm not sure whether https://github.com/Codeception/Codeception/pull/5447 works for me because we have multiple entity managers...

hrkyoung commented 3 years ago

Another FWIW, this also works for us:

$doctrineModule = $this->getModule('Doctrine2');
$em = EntityManager::create($doctrineModule->_getEntityManager()->getConnection(), $doctrineModule->_getEntityManager()->getConfiguration());
$doctrineModule->em = $em;

The limitation is that whatever test is causing the exception (therefore causing the entity manager to close) needs to be isolated in its own test method or appear as the very last step in a test method. Otherwise, the above will throw up an error because the exception also causes the transaction to fail and be rolled back. No SQL commands will be entertained after failure (with postgres at least).

To make it easier, I added the above code to an extension that runs during BEFORE_TEST event and wrapped the code in $doctrineModule->em->isOpen()=== false statement so it will only run when the entity manager is closed.

akireev1984 commented 2 years ago

@Amunak patch posted is not enough in order to allow persisting entitites in following cests in some cases.

Failed transaction will set $_isRollbackOnly variable in doctrine connection https://github.com/doctrine/dbal/blob/v2.8.0/lib/Doctrine/DBAL/Connection.php#L202 to true and reseting the entity manager won't change that. So new connection for next cest still keeps the variable and deny any persists into transaction.

So after cest all open nested transactions (created inside cest) and the transaction made by codeception for cest itself must be rolled back:

--- src/Codeception/Module/Doctrine2.php
+++ src/Codeception/Module/Doctrine2.php
@@ -157,7 +157,9 @@

         if ($this->config['cleanup'] && $this->em->getConnection()->isTransactionActive()) {
             try {
-                $this->em->getConnection()->rollback();
+                while ($this->em->getConnection()->getTransactionNestingLevel() > 0) {
+                    $this->em->getConnection()->rollback();
+                }
                 $this->debugSection('Database', 'Transaction cancelled; all changes reverted.');
             } catch (\PDOException $e) {
             }

This fix is not working anymore.