Browse Source

Added support to publish orphan visits in mercure

Alejandro Celaya 3 years ago
parent
commit
7d6d8e3a68

+ 57 - 0
docs/async-api/async-api.json

@@ -58,6 +58,23 @@
                     }
                 }
             }
+        },
+        "http://shlink.io/new-orphan-visit": {
+            "subscribe": {
+                "summary": "Receive information about any new orphan visit.",
+                "operationId": "newOrphanVisit",
+                "message": {
+                    "payload": {
+                        "type": "object",
+                        "additionalProperties": false,
+                        "properties": {
+                            "visit": {
+                                "$ref": "#/components/schemas/OrphanVisit"
+                            }
+                        }
+                    }
+                }
+            }
         }
     },
     "components": {
@@ -179,6 +196,46 @@
                     }
                 }
             },
+            "OrphanVisit": {
+                "allOf": [
+                    {"$ref": "#/components/schemas/Visit"},
+                    {
+                        "type": "object",
+                        "properties": {
+                            "visitedUrl": {
+                                "type": "string",
+                                "nullable": true,
+                                "description": "The originally visited URL that triggered the tracking of this visit"
+                            },
+                            "type": {
+                                "type": "string",
+                                "enum": [
+                                    "invalid_short_url",
+                                    "base_url",
+                                    "regular_404"
+                                ],
+                                "description": "Tells the type of orphan visit"
+                            }
+                        }
+                    }
+                ],
+                "example": {
+                    "referer": "https://t.co",
+                    "date": "2015-08-20T05:05:03+04:00",
+                    "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36",
+                    "visitLocation": {
+                        "cityName": "Cupertino",
+                        "countryCode": "US",
+                        "countryName": "United States",
+                        "latitude": 37.3042,
+                        "longitude": -122.0946,
+                        "regionName": "California",
+                        "timezone": "America/Los_Angeles"
+                    },
+                    "visitedUrl": "https://doma.in",
+                    "type": "base_url"
+                }
+            },
             "VisitLocation": {
                 "type": "object",
                 "properties": {

+ 4 - 1
module/Core/config/dependencies.config.php

@@ -133,7 +133,10 @@ return [
         ShortUrl\Helper\ShortUrlTitleResolutionHelper::class => [Util\UrlValidator::class],
         ShortUrl\Transformer\ShortUrlDataTransformer::class => [ShortUrl\Helper\ShortUrlStringifier::class],
 
-        Mercure\MercureUpdatesGenerator::class => [ShortUrl\Transformer\ShortUrlDataTransformer::class],
+        Mercure\MercureUpdatesGenerator::class => [
+            ShortUrl\Transformer\ShortUrlDataTransformer::class,
+            Visit\Transformer\OrphanVisitDataTransformer::class,
+        ],
 
         Importer\ImportedLinksProcessor::class => [
             'em',

+ 1 - 3
module/Core/src/EventDispatcher/LocateVisit.php

@@ -58,9 +58,7 @@ class LocateVisit
             $this->locateVisit($visitId, $shortUrlVisited->originalIpAddress(), $visit);
         }
 
-        if (! $visit->isOrphan()) {
-            $this->eventDispatcher->dispatch(new VisitLocated($visitId));
-        }
+        $this->eventDispatcher->dispatch(new VisitLocated($visitId));
     }
 
     private function downloadOrUpdateGeoLiteDb(string $visitId): bool

+ 19 - 2
module/Core/src/EventDispatcher/NotifyVisitToMercure.php

@@ -10,8 +10,11 @@ use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated;
 use Shlinkio\Shlink\Core\Mercure\MercureUpdatesGeneratorInterface;
 use Symfony\Component\Mercure\PublisherInterface;
+use Symfony\Component\Mercure\Update;
 use Throwable;
 
+use function Functional\each;
+
 class NotifyVisitToMercure
 {
     private PublisherInterface $publisher;
@@ -45,12 +48,26 @@ class NotifyVisitToMercure
         }
 
         try {
-            ($this->publisher)($this->updatesGenerator->newShortUrlVisitUpdate($visit));
-            ($this->publisher)($this->updatesGenerator->newVisitUpdate($visit));
+            each($this->determineUpdatesForVisit($visit), fn (Update $update) => ($this->publisher)($update));
         } catch (Throwable $e) {
             $this->logger->debug('Error while trying to notify mercure hub with new visit. {e}', [
                 'e' => $e,
             ]);
         }
     }
+
+    /**
+     * @return Update[]
+     */
+    private function determineUpdatesForVisit(Visit $visit): array
+    {
+        if ($visit->isOrphan()) {
+            return [$this->updatesGenerator->newOrphanVisitUpdate($visit)];
+        }
+
+        return [
+            $this->updatesGenerator->newShortUrlVisitUpdate($visit),
+            $this->updatesGenerator->newVisitUpdate($visit),
+        ];
+    }
 }

+ 18 - 6
module/Core/src/Mercure/MercureUpdatesGenerator.php

@@ -16,29 +16,41 @@ use const JSON_THROW_ON_ERROR;
 final class MercureUpdatesGenerator implements MercureUpdatesGeneratorInterface
 {
     private const NEW_VISIT_TOPIC = 'https://shlink.io/new-visit';
+    private const NEW_ORPHAN_VISIT_TOPIC = 'https://shlink.io/new-orphan-visit';
 
-    private DataTransformerInterface $transformer;
+    private DataTransformerInterface $shortUrlTransformer;
+    private DataTransformerInterface $orphanVisitTransformer;
 
-    public function __construct(DataTransformerInterface $transformer)
-    {
-        $this->transformer = $transformer;
+    public function __construct(
+        DataTransformerInterface $shortUrlTransformer,
+        DataTransformerInterface $orphanVisitTransformer
+    ) {
+        $this->shortUrlTransformer = $shortUrlTransformer;
+        $this->orphanVisitTransformer = $orphanVisitTransformer;
     }
 
     public function newVisitUpdate(Visit $visit): Update
     {
         return new Update(self::NEW_VISIT_TOPIC, $this->serialize([
-            'shortUrl' => $this->transformer->transform($visit->getShortUrl()),
+            'shortUrl' => $this->shortUrlTransformer->transform($visit->getShortUrl()),
             'visit' => $visit,
         ]));
     }
 
+    public function newOrphanVisitUpdate(Visit $visit): Update
+    {
+        return new Update(self::NEW_ORPHAN_VISIT_TOPIC, $this->serialize([
+            'visit' => $this->orphanVisitTransformer->transform($visit),
+        ]));
+    }
+
     public function newShortUrlVisitUpdate(Visit $visit): Update
     {
         $shortUrl = $visit->getShortUrl();
         $topic = sprintf('%s/%s', self::NEW_VISIT_TOPIC, $shortUrl->getShortCode());
 
         return new Update($topic, $this->serialize([
-            'shortUrl' => $this->transformer->transform($shortUrl),
+            'shortUrl' => $this->shortUrlTransformer->transform($shortUrl),
             'visit' => $visit,
         ]));
     }

+ 2 - 0
module/Core/src/Mercure/MercureUpdatesGeneratorInterface.php

@@ -11,5 +11,7 @@ interface MercureUpdatesGeneratorInterface
 {
     public function newVisitUpdate(Visit $visit): Update;
 
+    public function newOrphanVisitUpdate(Visit $visit): Update;
+
     public function newShortUrlVisitUpdate(Visit $visit): Update;
 }

+ 6 - 11
module/Core/test/EventDispatcher/LocateVisitTest.php

@@ -136,11 +136,8 @@ class LocateVisitTest extends TestCase
      * @test
      * @dataProvider provideIpAddresses
      */
-    public function locatableVisitsResolveToLocation(
-        Visit $visit,
-        ?string $originalIpAddress,
-        int $expectedDispatchCalls
-    ): void {
+    public function locatableVisitsResolveToLocation(Visit $visit, ?string $originalIpAddress): void
+    {
         $ipAddr = $originalIpAddress ?? $visit->getRemoteAddr();
         $location = new Location('', '', '', '', 0.0, 0.0, '');
         $event = new UrlVisited('123', $originalIpAddress);
@@ -159,7 +156,7 @@ class LocateVisitTest extends TestCase
         $flush->shouldHaveBeenCalledOnce();
         $resolveIp->shouldHaveBeenCalledOnce();
         $this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled();
-        $dispatch->shouldHaveBeenCalledTimes($expectedDispatchCalls);
+        $dispatch->shouldHaveBeenCalledOnce();
     }
 
     public function provideIpAddresses(): iterable
@@ -167,16 +164,14 @@ class LocateVisitTest extends TestCase
         yield 'no original IP address' => [
             Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')),
             null,
-            1,
         ];
         yield 'original IP address' => [
             Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')),
             '1.2.3.4',
-            1,
         ];
-        yield 'base url' => [Visit::forBasePath(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4', 0];
-        yield 'invalid short url' => [Visit::forInvalidShortUrl(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4', 0];
-        yield 'regular not found' => [Visit::forRegularNotFound(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4', 0];
+        yield 'base url' => [Visit::forBasePath(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4'];
+        yield 'invalid short url' => [Visit::forInvalidShortUrl(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4'];
+        yield 'regular not found' => [Visit::forRegularNotFound(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4'];
     }
 
     /** @test */

+ 44 - 3
module/Core/test/EventDispatcher/NotifyVisitToMercureTest.php

@@ -57,10 +57,9 @@ class NotifyVisitToMercureTest extends TestCase
         $logDebug = $this->logger->debug(Argument::cetera());
         $buildNewShortUrlVisitUpdate = $this->updatesGenerator->newShortUrlVisitUpdate(
             Argument::type(Visit::class),
-        )->willReturn(new Update('', ''));
-        $buildNewVisitUpdate = $this->updatesGenerator->newVisitUpdate(Argument::type(Visit::class))->willReturn(
-            new Update('', ''),
         );
+        $buildNewOrphanVisitUpdate = $this->updatesGenerator->newOrphanVisitUpdate(Argument::type(Visit::class));
+        $buildNewVisitUpdate = $this->updatesGenerator->newVisitUpdate(Argument::type(Visit::class));
         $publish = $this->publisher->__invoke(Argument::type(Update::class));
 
         ($this->listener)(new VisitLocated($visitId));
@@ -70,6 +69,7 @@ class NotifyVisitToMercureTest extends TestCase
         $logDebug->shouldNotHaveBeenCalled();
         $buildNewShortUrlVisitUpdate->shouldNotHaveBeenCalled();
         $buildNewVisitUpdate->shouldNotHaveBeenCalled();
+        $buildNewOrphanVisitUpdate->shouldNotHaveBeenCalled();
         $publish->shouldNotHaveBeenCalled();
     }
 
@@ -84,6 +84,7 @@ class NotifyVisitToMercureTest extends TestCase
         $logWarning = $this->logger->warning(Argument::cetera());
         $logDebug = $this->logger->debug(Argument::cetera());
         $buildNewShortUrlVisitUpdate = $this->updatesGenerator->newShortUrlVisitUpdate($visit)->willReturn($update);
+        $buildNewOrphanVisitUpdate = $this->updatesGenerator->newOrphanVisitUpdate($visit)->willReturn($update);
         $buildNewVisitUpdate = $this->updatesGenerator->newVisitUpdate($visit)->willReturn($update);
         $publish = $this->publisher->__invoke($update);
 
@@ -94,6 +95,7 @@ class NotifyVisitToMercureTest extends TestCase
         $logDebug->shouldNotHaveBeenCalled();
         $buildNewShortUrlVisitUpdate->shouldHaveBeenCalledOnce();
         $buildNewVisitUpdate->shouldHaveBeenCalledOnce();
+        $buildNewOrphanVisitUpdate->shouldNotHaveBeenCalled();
         $publish->shouldHaveBeenCalledTimes(2);
     }
 
@@ -111,6 +113,7 @@ class NotifyVisitToMercureTest extends TestCase
             'e' => $e,
         ]);
         $buildNewShortUrlVisitUpdate = $this->updatesGenerator->newShortUrlVisitUpdate($visit)->willReturn($update);
+        $buildNewOrphanVisitUpdate = $this->updatesGenerator->newOrphanVisitUpdate($visit)->willReturn($update);
         $buildNewVisitUpdate = $this->updatesGenerator->newVisitUpdate($visit)->willReturn($update);
         $publish = $this->publisher->__invoke($update)->willThrow($e);
 
@@ -120,7 +123,45 @@ class NotifyVisitToMercureTest extends TestCase
         $logWarning->shouldNotHaveBeenCalled();
         $logDebug->shouldHaveBeenCalledOnce();
         $buildNewShortUrlVisitUpdate->shouldHaveBeenCalledOnce();
+        $buildNewVisitUpdate->shouldHaveBeenCalledOnce();
+        $buildNewOrphanVisitUpdate->shouldNotHaveBeenCalled();
+        $publish->shouldHaveBeenCalledOnce();
+    }
+
+    /**
+     * @test
+     * @dataProvider provideOrphanVisits
+     */
+    public function notificationsAreSentForOrphanVisits(Visit $visit): void
+    {
+        $visitId = '123';
+        $update = new Update('', '');
+
+        $findVisit = $this->em->find(Visit::class, $visitId)->willReturn($visit);
+        $logWarning = $this->logger->warning(Argument::cetera());
+        $logDebug = $this->logger->debug(Argument::cetera());
+        $buildNewShortUrlVisitUpdate = $this->updatesGenerator->newShortUrlVisitUpdate($visit)->willReturn($update);
+        $buildNewOrphanVisitUpdate = $this->updatesGenerator->newOrphanVisitUpdate($visit)->willReturn($update);
+        $buildNewVisitUpdate = $this->updatesGenerator->newVisitUpdate($visit)->willReturn($update);
+        $publish = $this->publisher->__invoke($update);
+
+        ($this->listener)(new VisitLocated($visitId));
+
+        $findVisit->shouldHaveBeenCalledOnce();
+        $logWarning->shouldNotHaveBeenCalled();
+        $logDebug->shouldNotHaveBeenCalled();
+        $buildNewShortUrlVisitUpdate->shouldNotHaveBeenCalled();
         $buildNewVisitUpdate->shouldNotHaveBeenCalled();
+        $buildNewOrphanVisitUpdate->shouldHaveBeenCalledOnce();
         $publish->shouldHaveBeenCalledOnce();
     }
+
+    public function provideOrphanVisits(): iterable
+    {
+        $visitor = Visitor::emptyInstance();
+
+        yield Visit::TYPE_REGULAR_404 => [Visit::forRegularNotFound($visitor)];
+        yield Visit::TYPE_INVALID_SHORT_URL => [Visit::forInvalidShortUrl($visitor)];
+        yield Visit::TYPE_BASE_URL => [Visit::forBasePath($visitor)];
+    }
 }

+ 35 - 1
module/Core/test/Mercure/MercureUpdatesGeneratorTest.php

@@ -12,6 +12,7 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier;
 use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer;
+use Shlinkio\Shlink\Core\Visit\Transformer\OrphanVisitDataTransformer;
 
 use function Shlinkio\Shlink\Common\json_decode;
 
@@ -21,7 +22,10 @@ class MercureUpdatesGeneratorTest extends TestCase
 
     public function setUp(): void
     {
-        $this->generator = new MercureUpdatesGenerator(new ShortUrlDataTransformer(new ShortUrlStringifier([])));
+        $this->generator = new MercureUpdatesGenerator(
+            new ShortUrlDataTransformer(new ShortUrlStringifier([])),
+            new OrphanVisitDataTransformer(),
+        );
     }
 
     /**
@@ -70,4 +74,34 @@ class MercureUpdatesGeneratorTest extends TestCase
         yield 'newVisitUpdate' => ['newVisitUpdate', 'https://shlink.io/new-visit', 'the cool title'];
         yield 'newShortUrlVisitUpdate' => ['newShortUrlVisitUpdate', 'https://shlink.io/new-visit/foo', null];
     }
+
+    /**
+     * @test
+     * @dataProvider provideOrphanVisits
+     */
+    public function orphanVisitIsProperlySerializedIntoUpdate(Visit $orphanVisit): void
+    {
+        $update = $this->generator->newOrphanVisitUpdate($orphanVisit);
+
+        self::assertEquals(['https://shlink.io/new-orphan-visit'], $update->getTopics());
+        self::assertEquals([
+            'visit' => [
+                'referer' => '',
+                'userAgent' => '',
+                'visitLocation' => null,
+                'date' => $orphanVisit->getDate()->toAtomString(),
+                'visitedUrl' => $orphanVisit->visitedUrl(),
+                'type' => $orphanVisit->type(),
+            ],
+        ], json_decode($update->getData()));
+    }
+
+    public function provideOrphanVisits(): iterable
+    {
+        $visitor = Visitor::emptyInstance();
+
+        yield Visit::TYPE_REGULAR_404 => [Visit::forRegularNotFound($visitor)];
+        yield Visit::TYPE_INVALID_SHORT_URL => [Visit::forInvalidShortUrl($visitor)];
+        yield Visit::TYPE_BASE_URL => [Visit::forBasePath($visitor)];
+    }
 }