diff --git a/module/Common/src/Middleware/CloseDbConnectionMiddleware.php b/module/Common/src/Middleware/CloseDbConnectionMiddleware.php index c365da51..a407a31a 100644 --- a/module/Common/src/Middleware/CloseDbConnectionMiddleware.php +++ b/module/Common/src/Middleware/CloseDbConnectionMiddleware.php @@ -3,11 +3,13 @@ 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 { @@ -19,16 +21,23 @@ class CloseDbConnectionMiddleware implements MiddlewareInterface $this->em = $em; } - /** - * Process an incoming server request and return a response, optionally delegating - * response creation to a handler. - */ public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - $handledRequest = $handler->handle($request); - $this->em->getConnection()->close(); - $this->em->clear(); + 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)(); + } - return $handledRequest; + throw $e; + } finally { + $this->em->getConnection()->close(); + $this->em->clear(); + } } } diff --git a/module/Common/test/Middleware/CloseDbConnectionMiddlewareTest.php b/module/Common/test/Middleware/CloseDbConnectionMiddlewareTest.php index cd95b26d..34d141ad 100644 --- a/module/Common/test/Middleware/CloseDbConnectionMiddlewareTest.php +++ b/module/Common/test/Middleware/CloseDbConnectionMiddlewareTest.php @@ -8,7 +8,9 @@ use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\TestCase; 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; @@ -20,35 +22,80 @@ class CloseDbConnectionMiddlewareTest extends TestCase private $handler; /** @var ObjectProphecy */ private $em; + /** @var ObjectProphecy */ + private $conn; public function setUp(): void { $this->handler = $this->prophesize(RequestHandlerInterface::class); $this->em = $this->prophesize(EntityManagerInterface::class); + $this->conn = $this->prophesize(Connection::class); + $this->conn->close()->will(function () { + }); + $this->em->getConnection()->willReturn($this->conn->reveal()); + $this->em->clear()->will(function () { + }); + $this->em->isOpen()->willReturn(true); $this->middleware = new CloseDbConnectionMiddleware($this->em->reveal()); } /** @test */ - public function connectionIsClosedWhenMiddlewareIsProcessed() + public function connectionIsClosedWhenMiddlewareIsProcessed(): void { $req = new ServerRequest(); $resp = new Response(); - - $conn = $this->prophesize(Connection::class); - $closeConn = $conn->close()->will(function () { - }); - $getConn = $this->em->getConnection()->willReturn($conn->reveal()); - $clear = $this->em->clear()->will(function () { - }); $handle = $this->handler->handle($req)->willReturn($resp); $result = $this->middleware->process($req, $this->handler->reveal()); $this->assertSame($result, $resp); - $getConn->shouldHaveBeenCalledOnce(); - $closeConn->shouldHaveBeenCalledOnce(); - $clear->shouldHaveBeenCalledOnce(); + $this->em->getConnection()->shouldHaveBeenCalledOnce(); + $this->conn->close()->shouldHaveBeenCalledOnce(); + $this->em->clear()->shouldHaveBeenCalledOnce(); $handle->shouldHaveBeenCalledOnce(); } + + /** @test */ + public function connectionIsClosedEvenIfExceptionIsThrownOnInnerMiddlewares(): void + { + $req = new ServerRequest(); + $expectedError = new RuntimeException(); + $this->handler->handle($req)->willThrow($expectedError) + ->shouldBeCalledOnce(); + + $this->em->getConnection()->shouldBeCalledOnce(); + $this->conn->close()->shouldBeCalledOnce(); + $this->em->clear()->shouldBeCalledOnce(); + $this->expectExceptionObject($expectedError); + + $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]]; + } } diff --git a/phpstan.neon b/phpstan.neon index 7756bc14..c735bfec 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -3,3 +3,6 @@ parameters: - '#League\\Plates\\callback#' - '#is not subtype of Throwable#' - '#ObjectManager::flush()#' + - + message: '#Access to an undefined property#' + path: %currentWorkingDirectory%/module/Common/src/Middleware/CloseDbConnectionMiddleware.php