Merge pull request #1212 from acelaya-forks/feature/wrong-transactionality

Removed transactionality when dispatching async events

CHANGELOG.md

@@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).
 
+## [Unreleased]
+### Added
+* *Nothing*
+
+### Changed
+* *Nothing*
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* [#1210](https://github.com/shlinkio/shlink/issues/1210) Fixed real time updates not being notified.
+
+
 ## [2.9.1] - 2021-10-11
 ### Added
 * *Nothing*

composer.json

@@ -18,7 +18,7 @@
         "akrabat/ip-address-middleware": "^2.0",
         "cakephp/chronos": "^2.2",
         "cocur/slugify": "^4.0",
-        "doctrine/migrations": "^3.2",
+        "doctrine/migrations": "^3.2 <3.3",
         "doctrine/orm": "^2.9",
         "endroid/qr-code": "^4.2",
         "geoip2/geoip2": "^2.11",

module/CLI/src/Command/Db/CreateDatabaseCommand.php

@@ -71,7 +71,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand
         $databases = $schemaManager->listDatabases();
         $shlinkDatabase = $this->regularConn->getDatabase();
 
-        if (! contains($databases, $shlinkDatabase)) {
+        if ($shlinkDatabase !== null && ! contains($databases, $shlinkDatabase)) {
             $schemaManager->createDatabase($shlinkDatabase);
         }
     }

module/Core/src/Visit/VisitsTracker.php

@@ -69,11 +69,9 @@ class VisitsTracker implements VisitsTrackerInterface
         }
 
         $visit = $createVisit($visitor->normalizeForTrackingOptions($this->options));
-        $this->em->transactional(function () use ($visit, $visitor): void {
-            $this->em->persist($visit);
-            $this->em->flush();
+        $this->em->persist($visit);
+        $this->em->flush();
 
-            $this->eventDispatcher->dispatch(new UrlVisited($visit->getId(), $visitor->getRemoteAddress()));
-        });
+        $this->eventDispatcher->dispatch(new UrlVisited($visit->getId(), $visitor->getRemoteAddress()));
     }
 }

module/Core/test/Visit/VisitsTrackerTest.php

@@ -29,10 +29,6 @@ class VisitsTrackerTest extends TestCase
     public function setUp(): void
     {
         $this->em = $this->prophesize(EntityManager::class);
-        $this->em->transactional(Argument::any())->will(function (array $args) {
-            [$callback] = $args;
-            return $callback();
-        });
 
         $this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class);
         $this->options = new TrackingOptions();
@@ -52,7 +48,6 @@ class VisitsTrackerTest extends TestCase
         $this->visitsTracker->{$method}(...$args);
 
         $persist->shouldHaveBeenCalledOnce();
-        $this->em->transactional(Argument::cetera())->shouldHaveBeenCalledOnce();
         $this->em->flush()->shouldHaveBeenCalledOnce();
         $this->eventDispatcher->dispatch(Argument::type(UrlVisited::class))->shouldHaveBeenCalled();
     }
@@ -68,7 +63,6 @@ class VisitsTrackerTest extends TestCase
         $this->visitsTracker->{$method}(...$args);
 
         $this->eventDispatcher->dispatch(Argument::cetera())->shouldNotHaveBeenCalled();
-        $this->em->transactional(Argument::cetera())->shouldNotHaveBeenCalled();
         $this->em->persist(Argument::cetera())->shouldNotHaveBeenCalled();
         $this->em->flush()->shouldNotHaveBeenCalled();
     }
@@ -92,7 +86,6 @@ class VisitsTrackerTest extends TestCase
         $this->visitsTracker->{$method}(Visitor::emptyInstance());
 
         $this->eventDispatcher->dispatch(Argument::cetera())->shouldNotHaveBeenCalled();
-        $this->em->transactional(Argument::cetera())->shouldNotHaveBeenCalled();
         $this->em->persist(Argument::cetera())->shouldNotHaveBeenCalled();
         $this->em->flush()->shouldNotHaveBeenCalled();
     }

module/Rest/src/Action/HealthAction.php

@@ -32,7 +32,9 @@ class HealthAction extends AbstractRestAction
     public function handle(ServerRequestInterface $request): ResponseInterface
     {
         try {
-            $connected = $this->em->getConnection()->ping();
+            $connection = $this->em->getConnection();
+            $connection->executeQuery($connection->getDatabasePlatform()->getDummySelectSQL());
+            $connected = true;
         } catch (Throwable) {
             $connected = false;
         }

module/Rest/test/Action/HealthActionTest.php

@@ -5,11 +5,13 @@ declare(strict_types=1);
 namespace ShlinkioTest\Shlink\Rest\Action;
 
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\ORM\EntityManagerInterface;
 use Exception;
 use Laminas\Diactoros\Response\JsonResponse;
 use Laminas\Diactoros\ServerRequest;
 use PHPUnit\Framework\TestCase;
+use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Options\AppOptions;
@@ -25,6 +27,12 @@ class HealthActionTest extends TestCase
     public function setUp(): void
     {
         $this->conn = $this->prophesize(Connection::class);
+        $this->conn->executeQuery(Argument::cetera())->will(function (): void {
+        });
+        $dbPlatform = $this->prophesize(AbstractPlatform::class);
+        $dbPlatform->getDummySelectSQL()->willReturn('');
+        $this->conn->getDatabasePlatform()->willReturn($dbPlatform->reveal());
+
         $em = $this->prophesize(EntityManagerInterface::class);
         $em->getConnection()->willReturn($this->conn->reveal());
 
@@ -32,10 +40,8 @@ class HealthActionTest extends TestCase
     }
 
     /** @test */
-    public function passResponseIsReturnedWhenConnectionSucceeds(): void
+    public function passResponseIsReturnedWhenDummyQuerySucceeds(): void
     {
-        $ping = $this->conn->ping()->willReturn(true);
-
         /** @var JsonResponse $resp */
         $resp = $this->action->handle(new ServerRequest());
         $payload = $resp->getPayload();
@@ -48,13 +54,13 @@ class HealthActionTest extends TestCase
             'project' => 'https://github.com/shlinkio/shlink',
         ], $payload['links']);
         self::assertEquals('application/health+json', $resp->getHeaderLine('Content-type'));
-        $ping->shouldHaveBeenCalledOnce();
+        $this->conn->executeQuery(Argument::cetera())->shouldHaveBeenCalledOnce();
     }
 
     /** @test */
-    public function failResponseIsReturnedWhenConnectionFails(): void
+    public function failResponseIsReturnedWhenDummyQueryThrowsException(): void
     {
-        $ping = $this->conn->ping()->willReturn(false);
+        $executeQuery = $this->conn->executeQuery(Argument::cetera())->willThrow(Exception::class);
 
         /** @var JsonResponse $resp */
         $resp = $this->action->handle(new ServerRequest());
@@ -68,26 +74,6 @@ class HealthActionTest extends TestCase
             'project' => 'https://github.com/shlinkio/shlink',
         ], $payload['links']);
         self::assertEquals('application/health+json', $resp->getHeaderLine('Content-type'));
-        $ping->shouldHaveBeenCalledOnce();
-    }
-
-    /** @test */
-    public function failResponseIsReturnedWhenConnectionThrowsException(): void
-    {
-        $ping = $this->conn->ping()->willThrow(Exception::class);
-
-        /** @var JsonResponse $resp */
-        $resp = $this->action->handle(new ServerRequest());
-        $payload = $resp->getPayload();
-
-        self::assertEquals(503, $resp->getStatusCode());
-        self::assertEquals('fail', $payload['status']);
-        self::assertEquals('1.2.3', $payload['version']);
-        self::assertEquals([
-            'about' => 'https://shlink.io',
-            'project' => 'https://github.com/shlinkio/shlink',
-        ], $payload['links']);
-        self::assertEquals('application/health+json', $resp->getHeaderLine('Content-type'));
-        $ping->shouldHaveBeenCalledOnce();
+        $executeQuery->shouldHaveBeenCalledOnce();
     }
 }