From 406de16a0dabd63b2d01d632979bfe0e6429e298 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Wed, 31 Jul 2019 20:08:46 +0200
Subject: [PATCH] Ensured database connection is closed even if an error is
 thrown during dispatch process

---
 .../CloseDbConnectionMiddleware.php           | 15 +++----
 .../CloseDbConnectionMiddlewareTest.php       | 40 ++++++++++++++-----
 2 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/module/Common/src/Middleware/CloseDbConnectionMiddleware.php b/module/Common/src/Middleware/CloseDbConnectionMiddleware.php
index c365da51..1294848d 100644
--- a/module/Common/src/Middleware/CloseDbConnectionMiddleware.php
+++ b/module/Common/src/Middleware/CloseDbConnectionMiddleware.php
@@ -19,16 +19,13 @@ 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();
-
-        return $handledRequest;
+        try {
+            return $handler->handle($request);
+        } 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..2bf93a85 100644
--- a/module/Common/test/Middleware/CloseDbConnectionMiddlewareTest.php
+++ b/module/Common/test/Middleware/CloseDbConnectionMiddlewareTest.php
@@ -8,6 +8,7 @@ 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 Zend\Diactoros\Response;
 use Zend\Diactoros\ServerRequest;
@@ -20,35 +21,52 @@ 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->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());
+    }
 }