Created EntityManagerDecorator to handle the automatic reopening, and removed this behavior from ClosDbConnectionMiddleware

config/autoload/entity-manager.global.php

@@ -1,7 +1,7 @@
 <?php
 declare(strict_types=1);
 
-use function Shlinkio\Shlink\Common\env;
+namespace Shlinkio\Shlink\Common;
 
 return [
 

module/Common/config/dependencies.config.php

@@ -4,7 +4,6 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Common;
 
 use Doctrine\Common\Cache\Cache;
-use Doctrine\ORM\EntityManager;
 use GeoIp2\Database\Reader;
 use GuzzleHttp\Client as GuzzleClient;
 use Monolog\Logger;
@@ -20,7 +19,6 @@ return [
 
     'dependencies' => [
         'factories' => [
-            EntityManager::class => Factory\EntityManagerFactory::class,
             GuzzleClient::class => InvokableFactory::class,
             Cache::class => Factory\CacheFactory::class,
             Filesystem::class => InvokableFactory::class,
@@ -45,7 +43,6 @@ return [
             Service\PreviewGenerator::class => ConfigAbstractFactory::class,
         ],
         'aliases' => [
-            'em' => EntityManager::class,
             'httpClient' => GuzzleClient::class,
             'translator' => Translator::class,
 

module/Common/config/doctrine.config.php

@@ -0,0 +1,32 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Common;
+
+use Doctrine\ORM\EntityManager;
+
+return [
+
+    'entity_manager' => [
+        'orm' => [
+            'types' => [
+                Type\ChronosDateTimeType::CHRONOS_DATETIME => Type\ChronosDateTimeType::class,
+            ],
+        ],
+    ],
+
+    'dependencies' => [
+        'factories' => [
+            EntityManager::class => Doctrine\EntityManagerFactory::class,
+        ],
+        'aliases' => [
+            'em' => EntityManager::class,
+        ],
+        'delegators' => [
+            EntityManager::class => [
+                Doctrine\ReopeningEntityManagerDelegator::class,
+            ],
+        ],
+    ],
+
+];

module/Common/src/Doctrine/EntityManagerFactory.php

@@ -1,7 +1,7 @@
 <?php
 declare(strict_types=1);
 
-namespace Shlinkio\Shlink\Common\Factory;
+namespace Shlinkio\Shlink\Common\Doctrine;
 
 use Doctrine\Common\Cache\ArrayCache;
 use Doctrine\Common\Cache\Cache;
@@ -11,36 +11,42 @@ use Doctrine\DBAL\Types\Type;
 use Doctrine\ORM\EntityManager;
 use Doctrine\ORM\ORMException;
 use Doctrine\ORM\Tools\Setup;
-use Interop\Container\ContainerInterface;
+use Psr\Container\ContainerInterface;
-use Shlinkio\Shlink\Common\Type\ChronosDateTimeType;
-use Zend\ServiceManager\Exception\ServiceNotCreatedException;
-use Zend\ServiceManager\Exception\ServiceNotFoundException;
-use Zend\ServiceManager\Factory\FactoryInterface;
 
-class EntityManagerFactory implements FactoryInterface
+class EntityManagerFactory
 {
     /**
-     * @throws ServiceNotFoundException if unable to resolve the service.
-     * @throws ServiceNotCreatedException if an exception is raised when creating a service.
      * @throws ORMException
      * @throws DBALException
      */
-    public function __invoke(ContainerInterface $container, $requestedName, ?array $options = null)
+    public function __invoke(ContainerInterface $container): EntityManager
     {
         $globalConfig = $container->get('config');
-        $isDevMode = isset($globalConfig['debug']) ? ((bool) $globalConfig['debug']) : false;
+        $isDevMode = (bool) ($globalConfig['debug'] ?? false);
         $cache = $container->has(Cache::class) ? $container->get(Cache::class) : new ArrayCache();
         $emConfig = $globalConfig['entity_manager'] ?? [];
         $connectionConfig = $emConfig['connection'] ?? [];
         $ormConfig = $emConfig['orm'] ?? [];
 
-        if (! Type::hasType(ChronosDateTimeType::CHRONOS_DATETIME)) {
+        $this->registerTypes($ormConfig);
-            Type::addType(ChronosDateTimeType::CHRONOS_DATETIME, ChronosDateTimeType::class);
-        }
 
         $config = Setup::createConfiguration($isDevMode, $ormConfig['proxies_dir'] ?? null, $cache);
         $config->setMetadataDriverImpl(new PHPDriver($ormConfig['entities_mappings'] ?? []));
 
         return EntityManager::create($connectionConfig, $config);
     }
+
+    /**
+     * @throws DBALException
+     */
+    private function registerTypes(array $ormConfig): void
+    {
+        $types = $ormConfig['types'] ?? [];
+
+        foreach ($types as $name => $className) {
+            if (! Type::hasType($name)) {
+                Type::addType($name, $className);
+            }
+        }
+    }
 }

module/Common/src/Doctrine/ReopeningEntityManager.php

@@ -0,0 +1,48 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Common\Doctrine;
+
+use Doctrine\ORM\Decorator\EntityManagerDecorator;
+use Doctrine\ORM\EntityManager;
+
+class ReopeningEntityManager extends EntityManagerDecorator
+{
+    protected function getWrappedEntityManager(): EntityManager
+    {
+        if (! $this->wrapped->isOpen()) {
+            $this->wrapped= EntityManager::create(
+                $this->wrapped->getConnection(),
+                $this->wrapped->getConfiguration(),
+                $this->wrapped->getEventManager()
+            );
+        }
+
+        return $this->wrapped;
+    }
+
+    public function flush($entity = null): void
+    {
+        $this->getWrappedEntityManager()->flush($entity);
+    }
+
+    public function persist($object): void
+    {
+        $this->getWrappedEntityManager()->persist($object);
+    }
+
+    public function remove($object): void
+    {
+        $this->getWrappedEntityManager()->remove($object);
+    }
+
+    public function refresh($object): void
+    {
+        $this->getWrappedEntityManager()->refresh($object);
+    }
+
+    public function merge($object)
+    {
+        return $this->getWrappedEntityManager()->merge($object);
+    }
+}

module/Common/src/Doctrine/ReopeningEntityManagerDelegator.php

@@ -0,0 +1,17 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Common\Doctrine;
+
+use Doctrine\ORM\EntityManagerInterface;
+use Psr\Container\ContainerInterface;
+
+class ReopeningEntityManagerDelegator
+{
+    public function __invoke(ContainerInterface $container, string $name, callable $callback): ReopeningEntityManager
+    {
+        /** @var EntityManagerInterface $em */
+        $em = $callback();
+        return new ReopeningEntityManager($em);
+    }
+}

module/Common/src/Middleware/CloseDbConnectionMiddleware.php

@@ -3,13 +3,11 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Common\Middleware;
 
-use Doctrine\ORM\EntityManager;
 use Doctrine\ORM\EntityManagerInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
-use Throwable;
 
 class CloseDbConnectionMiddleware implements MiddlewareInterface
 {
@@ -25,16 +23,6 @@ class CloseDbConnectionMiddleware implements MiddlewareInterface
     {
         try {
             return $handler->handle($request);
-        } catch (Throwable $e) {
-            // FIXME Mega ugly hack to avoid a closed EntityManager to make shlink fail forever on swoole contexts
-            //       Should be fixed with request-shared EntityManagers, which is not supported by the ServiceManager
-            if (! $this->em->isOpen()) {
-                (function () {
-                    $this->closed = false;
-                })->bindTo($this->em, EntityManager::class)();
-            }
-
-            throw $e;
         } finally {
             $this->em->getConnection()->close();
             $this->em->clear();

module/Common/test/Doctrine/EntityManagerFactoryTest.php

@@ -1,11 +1,12 @@
 <?php
 declare(strict_types=1);
 
-namespace ShlinkioTest\Shlink\Common\Factory;
+namespace ShlinkioTest\Shlink\Common\Doctrine;
 
 use Doctrine\ORM\EntityManager;
 use PHPUnit\Framework\TestCase;
-use Shlinkio\Shlink\Common\Factory\EntityManagerFactory;
+use Shlinkio\Shlink\Common\Doctrine\EntityManagerFactory;
+use Shlinkio\Shlink\Common\Type\ChronosDateTimeType;
 use Zend\ServiceManager\ServiceManager;
 
 class EntityManagerFactoryTest extends TestCase
@@ -19,12 +20,17 @@ class EntityManagerFactoryTest extends TestCase
     }
 
     /** @test */
-    public function serviceIsCreated()
+    public function serviceIsCreated(): void
     {
         $sm = new ServiceManager(['services' => [
             'config' => [
                 'debug' => true,
                 'entity_manager' => [
+                    'orm' => [
+                        'types' => [
+                            ChronosDateTimeType::CHRONOS_DATETIME => ChronosDateTimeType::class,
+                        ],
+                    ],
                     'connection' => [
                         'driver' => 'pdo_sqlite',
                     ],
@@ -32,7 +38,7 @@ class EntityManagerFactoryTest extends TestCase
             ],
         ]]);
 
-        $em = $this->factory->__invoke($sm, EntityManager::class);
+        $em = ($this->factory)($sm, EntityManager::class);
         $this->assertInstanceOf(EntityManager::class, $em);
     }
 }

module/Common/test/Middleware/CloseDbConnectionMiddlewareTest.php

@@ -10,7 +10,6 @@ use Prophecy\Prophecy\ObjectProphecy;
 use Psr\Http\Server\RequestHandlerInterface;
 use RuntimeException;
 use Shlinkio\Shlink\Common\Middleware\CloseDbConnectionMiddleware;
-use Throwable;
 use Zend\Diactoros\Response;
 use Zend\Diactoros\ServerRequest;
 
@@ -35,7 +34,6 @@ class CloseDbConnectionMiddlewareTest extends TestCase
         $this->em->getConnection()->willReturn($this->conn->reveal());
         $this->em->clear()->will(function () {
         });
-        $this->em->isOpen()->willReturn(true);
 
         $this->middleware = new CloseDbConnectionMiddleware($this->em->reveal());
     }
@@ -71,31 +69,4 @@ class CloseDbConnectionMiddlewareTest extends TestCase
 
         $this->middleware->process($req, $this->handler->reveal());
     }
-
-    /**
-     * @test
-     * @dataProvider provideClosed
-     */
-    public function entityManagerIsReopenedAfterAnExceptionWhichClosedIt(bool $closed): void
-    {
-        $req = new ServerRequest();
-        $expectedError = new RuntimeException();
-        $this->handler->handle($req)->willThrow($expectedError)
-                                    ->shouldBeCalledOnce();
-        $this->em->closed = $closed;
-        $this->em->isOpen()->willReturn(false);
-
-        try {
-            $this->middleware->process($req, $this->handler->reveal());
-            $this->fail('Expected exception to be thrown but it did not.');
-        } catch (Throwable $e) {
-            $this->assertSame($expectedError, $e);
-            $this->assertFalse($this->em->closed);
-        }
-    }
-
-    public function provideClosed(): iterable
-    {
-        return [[true, false]];
-    }
 }