Added event dispatcher to track when a short URL is visited

composer.json

@@ -29,6 +29,7 @@
         "lstrojny/functional-php": "^1.8",
         "mikehaertl/phpwkhtmltopdf": "^2.2",
         "monolog/monolog": "^1.21",
+        "phly/phly-event-dispatcher": "^1.0",
         "shlinkio/shlink-installer": "^1.1",
         "symfony/console": "^4.2",
         "symfony/filesystem": "^4.2",

config/autoload/events.global.php

@@ -0,0 +1,26 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink;
+
+use Phly\EventDispatcher as Phly;
+use Psr\EventDispatcher as Psr;
+use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory;
+
+return [
+
+    'dependencies' => [
+        'factories' => [
+            Common\EventDispatcher\SwooleEventDispatcher::class => ConfigAbstractFactory::class,
+            Psr\ListenerProviderInterface::class => Common\EventDispatcher\ListenerProviderFactory::class,
+        ],
+        'aliases' => [
+            Psr\EventDispatcherInterface::class => Common\EventDispatcher\SwooleEventDispatcher::class,
+        ],
+    ],
+
+    ConfigAbstractFactory::class => [
+        Common\EventDispatcher\SwooleEventDispatcher::class => [Phly\EventDispatcher::class],
+    ],
+
+];

config/config.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink;
 
 use Acelaya\ExpressiveErrorHandler;
+use Phly\EventDispatcher;
 use Zend\ConfigAggregator;
 use Zend\Expressive;
 
@@ -16,6 +17,7 @@ return (new ConfigAggregator\ConfigAggregator([
     Expressive\Plates\ConfigProvider::class,
     Expressive\Swoole\ConfigProvider::class,
     ExpressiveErrorHandler\ConfigProvider::class,
+    EventDispatcher\ConfigProvider::class,
     Common\ConfigProvider::class,
     Core\ConfigProvider::class,
     CLI\ConfigProvider::class,

module/Common/src/Entity/AbstractEntity.php

@@ -20,6 +20,9 @@ abstract class AbstractEntity
         return $this->id;
     }
 
+    /**
+     * @internal
+     */
     public function setId(string $id): self
     {
         $this->id = $id;

module/Common/src/EventDispatcher/ListenerProviderFactory.php

@@ -0,0 +1,28 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Common\EventDispatcher;
+
+use Interop\Container\ContainerInterface;
+use Phly\EventDispatcher\ListenerProvider\AttachableListenerProvider;
+use Zend\ServiceManager\Factory\FactoryInterface;
+
+use function Phly\EventDispatcher\lazyListener;
+
+class ListenerProviderFactory implements FactoryInterface
+{
+    public function __invoke(ContainerInterface $container, $requestedName, array $options = null)
+    {
+        $config = $container->has('config') ? $container->get('config') : [];
+        $events = $config['events'] ?? [];
+        $provider = new AttachableListenerProvider();
+
+        foreach ($events as $eventName => $listeners) {
+            foreach ($listeners as $listenerName) {
+                $provider->listen($eventName, lazyListener($container, $listenerName));
+            }
+        }
+
+        return $provider;
+    }
+}

module/Common/src/EventDispatcher/SwooleEventDispatcher.php

@@ -0,0 +1,43 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Common\EventDispatcher;
+
+use Psr\EventDispatcher\EventDispatcherInterface;
+
+use const PHP_SAPI;
+
+use function extension_loaded;
+
+class SwooleEventDispatcher implements EventDispatcherInterface
+{
+    /** @var bool */
+    private $isSwoole;
+    /** @var EventDispatcherInterface */
+    private $innerDispatcher;
+
+    public function __construct(EventDispatcherInterface $innerDispatcher, ?bool $isSwoole = null)
+    {
+        $this->innerDispatcher = $innerDispatcher;
+        $this->isSwoole = $isSwoole ?? PHP_SAPI === 'cli' && extension_loaded('swoole');
+    }
+
+    /**
+     * Provide all relevant listeners with an event to process.
+     *
+     * @param object $event
+     *   The object to process.
+     *
+     * @return object
+     *   The Event that was passed, now modified by listeners.
+     */
+    public function dispatch(object $event)
+    {
+        // Do not really dispatch the event if the app is not being run with swoole
+        if (! $this->isSwoole) {
+            return $event;
+        }
+
+        return $this->innerDispatcher->dispatch($event);
+    }
+}

module/Common/test/EventDispatcher/ListenerProviderFactoryTest.php

@@ -0,0 +1,99 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Common\EventDispatcher;
+
+use Interop\Container\ContainerInterface;
+use Phly\EventDispatcher\ListenerProvider\AttachableListenerProvider;
+use PHPUnit\Framework\TestCase;
+use ReflectionObject;
+use Shlinkio\Shlink\Common\EventDispatcher\ListenerProviderFactory;
+
+use function Phly\EventDispatcher\lazyListener;
+
+class ListenerProviderFactoryTest extends TestCase
+{
+    /** @var ListenerProviderFactory */
+    private $factory;
+
+    public function setUp(): void
+    {
+        $this->factory = new ListenerProviderFactory();
+    }
+
+    /**
+     * @test
+     * @dataProvider provideContainersWithoutEvents
+     */
+    public function noListenersAreAttachedWhenNoConfigOrEventsAreRegistered(ContainerInterface $container): void
+    {
+        $provider = ($this->factory)($container, '');
+        $listeners = $this->getListenersFromProvider($provider);
+
+        $this->assertInstanceOf(AttachableListenerProvider::class, $provider);
+        $this->assertEmpty($listeners);
+    }
+
+    public function provideContainersWithoutEvents(): iterable
+    {
+        yield 'no config' => [(function () {
+            $container = $this->prophesize(ContainerInterface::class);
+            $container->has('config')->willReturn(false);
+
+            return $container->reveal();
+        })()];
+        yield 'no events' => [(function () {
+            $container = $this->prophesize(ContainerInterface::class);
+            $container->has('config')->willReturn(true);
+            $container->get('config')->willReturn([]);
+
+            return $container->reveal();
+        })()];
+    }
+
+    /** @test */
+    public function configuredEventsAreProperlyAttached(): void
+    {
+        $containerMock = $this->prophesize(ContainerInterface::class);
+        $containerMock->has('config')->willReturn(true);
+        $containerMock->get('config')->willReturn([
+            'events' => [
+                'foo' => [
+                    'bar',
+                    'baz',
+                ],
+                'something' => [
+                    'some_listener',
+                    'another_listener',
+                    'foobar',
+                ],
+            ],
+        ]);
+        $container = $containerMock->reveal();
+
+        $provider = ($this->factory)($container, '');
+        $listeners = $this->getListenersFromProvider($provider);
+
+        $this->assertInstanceOf(AttachableListenerProvider::class, $provider);
+        $this->assertEquals([
+            'foo' => [
+                lazyListener($container, 'bar'),
+                lazyListener($container, 'baz'),
+            ],
+            'something' => [
+                lazyListener($container, 'some_listener'),
+                lazyListener($container, 'another_listener'),
+                lazyListener($container, 'foobar'),
+            ],
+        ], $listeners);
+    }
+
+    private function getListenersFromProvider($provider): array
+    {
+        $ref = new ReflectionObject($provider);
+        $prop = $ref->getProperty('listeners');
+        $prop->setAccessible(true);
+
+        return $prop->getValue($provider);
+    }
+}

module/Common/test/EventDispatcher/SwooleEventDispatcherTest.php

@@ -0,0 +1,41 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Common\EventDispatcher;
+
+use PHPUnit\Framework\TestCase;
+use Prophecy\Prophecy\ObjectProphecy;
+use Psr\EventDispatcher\EventDispatcherInterface;
+use Shlinkio\Shlink\Common\EventDispatcher\SwooleEventDispatcher;
+use stdClass;
+
+class SwooleEventDispatcherTest extends TestCase
+{
+    /** @var ObjectProphecy */
+    private $innerDispatcher;
+
+    public function setUp(): void
+    {
+        $this->innerDispatcher = $this->prophesize(EventDispatcherInterface::class);
+    }
+
+    /**
+     * @test
+     * @dataProvider provideIsSwoole
+     */
+    public function callsInnerDispatcherOnlyWhenInSwooleContext(bool $isSwoole, int $expectedCalls): void
+    {
+        $dispatcher = new SwooleEventDispatcher($this->innerDispatcher->reveal(), $isSwoole);
+        $event = new stdClass();
+
+        $dispatcher->dispatch($event);
+
+        $this->innerDispatcher->dispatch($event)->shouldHaveBeenCalledTimes($expectedCalls);
+    }
+
+    public function provideIsSwoole(): iterable
+    {
+        yield 'with swoole' => [true, 1];
+        yield 'without swoole' => [false, 0];
+    }
+}

module/Core/config/dependencies.config.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core;
 
 use Doctrine\Common\Cache\Cache;
+use Psr\EventDispatcher\EventDispatcherInterface;
 use Shlinkio\Shlink\Common\Service\PreviewGenerator;
 use Shlinkio\Shlink\Core\Response\NotFoundHandler;
 use Zend\Expressive\Router\RouterInterface;
@@ -46,7 +47,7 @@ return [
         Options\UrlShortenerOptions::class => ['config.url_shortener'],
 
         Service\UrlShortener::class => ['httpClient', 'em', Options\UrlShortenerOptions::class],
-        Service\VisitsTracker::class => ['em'],
+        Service\VisitsTracker::class => ['em', EventDispatcherInterface::class],
         Service\ShortUrlService::class => ['em'],
         Service\VisitService::class => ['em'],
         Service\Tag\TagService::class => ['em'],

module/Core/config/event_dispatcher.config.php

@@ -0,0 +1,25 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core;
+
+use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface;
+use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory;
+
+return [
+
+    'events' => [
+        EventDispatcher\ShortUrlVisited::class => [
+            EventDispatcher\LocateShortUrlVisit::class,
+        ],
+    ],
+
+    'dependencies' => [
+        EventDispatcher\LocateShortUrlVisit::class => ConfigAbstractFactory::class,
+    ],
+
+    ConfigAbstractFactory::class => [
+        EventDispatcher\LocateShortUrlVisit::class => [IpLocationResolverInterface::class, 'em', 'Logger_Shlink'],
+    ],
+
+];

module/Core/src/Entity/Visit.php

@@ -65,6 +65,11 @@ class Visit extends AbstractEntity implements JsonSerializable
         return $this->visitLocation ?? new UnknownVisitLocation();
     }
 
+    public function isLocatable(): bool
+    {
+        return $this->hasRemoteAddr() && $this->remoteAddr !== IpAddress::LOCALHOST;
+    }
+
     public function locate(VisitLocation $visitLocation): self
     {
         $this->visitLocation = $visitLocation;

module/Core/src/EventDispatcher/LocateShortUrlVisit.php

@@ -0,0 +1,52 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\EventDispatcher;
+
+use Doctrine\ORM\EntityManagerInterface;
+use Psr\Log\LoggerInterface;
+use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface;
+use Shlinkio\Shlink\Common\IpGeolocation\Model\Location;
+use Shlinkio\Shlink\Core\Entity\Visit;
+use Shlinkio\Shlink\Core\Entity\VisitLocation;
+
+use function sprintf;
+
+class LocateShortUrlVisit
+{
+    /** @var IpLocationResolverInterface */
+    private $ipLocationResolver;
+    /** @var EntityManagerInterface */
+    private $em;
+    /** @var LoggerInterface */
+    private $logger;
+
+    public function __construct(
+        IpLocationResolverInterface $ipLocationResolver,
+        EntityManagerInterface $em,
+        LoggerInterface $logger
+    ) {
+        $this->ipLocationResolver = $ipLocationResolver;
+        $this->em = $em;
+        $this->logger = $logger;
+    }
+
+    public function __invoke(ShortUrlVisited $shortUrlVisited): void
+    {
+        $visitId = $shortUrlVisited->visitId();
+
+        /** @var Visit|null $visit */
+        $visit = $this->em->find(Visit::class, $visitId);
+        if ($visit === null) {
+            $this->logger->warning(sprintf('Tried to locate visit with id "%s", but it does not exist.', $visitId));
+            return;
+        }
+
+        $location = $visit->isLocatable()
+            ? $this->ipLocationResolver->resolveIpLocation($visit->getRemoteAddr())
+            : Location::emptyInstance();
+
+        $visit->locate(new VisitLocation($location));
+        $this->em->flush($visit);
+    }
+}

module/Core/src/EventDispatcher/ShortUrlVisited.php

@@ -0,0 +1,20 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\EventDispatcher;
+
+final class ShortUrlVisited
+{
+    /** @var string */
+    private $visitId;
+
+    public function __construct(string $visitId)
+    {
+        $this->visitId = $visitId;
+    }
+
+    public function visitId(): string
+    {
+        return $this->visitId;
+    }
+}

module/Core/src/Service/VisitsTracker.php

@@ -4,8 +4,10 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\Service;
 
 use Doctrine\ORM;
+use Psr\EventDispatcher\EventDispatcherInterface;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
+use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited;
 use Shlinkio\Shlink\Core\Exception\InvalidArgumentException;
 use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Model\VisitsParams;
@@ -19,10 +21,13 @@ class VisitsTracker implements VisitsTrackerInterface
 {
     /** @var ORM\EntityManagerInterface */
     private $em;
+    /** @var EventDispatcherInterface */
+    private $eventDispatcher;
 
-    public function __construct(ORM\EntityManagerInterface $em)
+    public function __construct(ORM\EntityManagerInterface $em, EventDispatcherInterface $eventDispatcher)
     {
         $this->em = $em;
+        $this->eventDispatcher = $eventDispatcher;
     }
 
     /**
@@ -41,6 +46,8 @@ class VisitsTracker implements VisitsTrackerInterface
         $em = $this->em;
         $em->persist($visit);
         $em->flush($visit);
+
+        $this->eventDispatcher->dispatch(new ShortUrlVisited($visit->getId()));
     }
 
     /**

module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php

@@ -0,0 +1,111 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Core\EventDispatcher;
+
+use Doctrine\ORM\EntityManagerInterface;
+use PHPUnit\Framework\TestCase;
+use Prophecy\Argument;
+use Prophecy\Prophecy\ObjectProphecy;
+use Psr\Log\LoggerInterface;
+use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface;
+use Shlinkio\Shlink\Common\IpGeolocation\Model\Location;
+use Shlinkio\Shlink\Common\Util\IpAddress;
+use Shlinkio\Shlink\Core\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\Entity\Visit;
+use Shlinkio\Shlink\Core\Entity\VisitLocation;
+use Shlinkio\Shlink\Core\EventDispatcher\LocateShortUrlVisit;
+use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited;
+use Shlinkio\Shlink\Core\Model\Visitor;
+
+class LocateShortUrlVisitTest extends TestCase
+{
+    /** @var LocateShortUrlVisit */
+    private $locateVisit;
+    /** @var ObjectProphecy */
+    private $ipLocationResolver;
+    /** @var ObjectProphecy */
+    private $em;
+    /** @var ObjectProphecy */
+    private $logger;
+
+    public function setUp(): void
+    {
+        $this->ipLocationResolver = $this->prophesize(IpLocationResolverInterface::class);
+        $this->em = $this->prophesize(EntityManagerInterface::class);
+        $this->logger = $this->prophesize(LoggerInterface::class);
+
+        $this->locateVisit = new LocateShortUrlVisit(
+            $this->ipLocationResolver->reveal(),
+            $this->em->reveal(),
+            $this->logger->reveal()
+        );
+    }
+
+    /** @test */
+    public function invalidVisitLogsWarning(): void
+    {
+        $event = new ShortUrlVisited('123');
+        $findVisit = $this->em->find(Visit::class, '123')->willReturn(null);
+        $logWarning = $this->logger->warning('Tried to locate visit with id "123", but it does not exist.');
+
+        ($this->locateVisit)($event);
+
+        $findVisit->shouldHaveBeenCalledOnce();
+        $this->em->flush(Argument::cetera())->shouldNotHaveBeenCalled();
+        $this->ipLocationResolver->resolveIpLocation(Argument::cetera())->shouldNotHaveBeenCalled();
+        $logWarning->shouldHaveBeenCalled();
+    }
+
+    /**
+     * @test
+     * @dataProvider provideNonLocatableVisits
+     */
+    public function nonLocatableVisitsResolveToEmptyLocations(Visit $visit): void
+    {
+        $event = new ShortUrlVisited('123');
+        $findVisit = $this->em->find(Visit::class, '123')->willReturn($visit);
+        $flush = $this->em->flush($visit)->will(function () {
+        });
+        $resolveIp = $this->ipLocationResolver->resolveIpLocation(Argument::any());
+
+        ($this->locateVisit)($event);
+
+        $this->assertEquals($visit->getVisitLocation(), new VisitLocation(Location::emptyInstance()));
+        $findVisit->shouldHaveBeenCalledOnce();
+        $flush->shouldHaveBeenCalledOnce();
+        $resolveIp->shouldNotHaveBeenCalled();
+        $this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled();
+    }
+
+    public function provideNonLocatableVisits(): iterable
+    {
+        $shortUrl = new ShortUrl('');
+
+        yield 'null IP' => [new Visit($shortUrl, new Visitor('', '', null))];
+        yield 'empty IP' => [new Visit($shortUrl, new Visitor('', '', ''))];
+        yield 'localhost' => [new Visit($shortUrl, new Visitor('', '', IpAddress::LOCALHOST))];
+    }
+
+    /** @test */
+    public function locatableVisitsResolveToLocation(): void
+    {
+        $ipAddr = '1.2.3.0';
+        $visit = new Visit(new ShortUrl(''), new Visitor('', '', $ipAddr));
+        $location = new Location('', '', '', '', 0.0, 0.0, '');
+        $event = new ShortUrlVisited('123');
+
+        $findVisit = $this->em->find(Visit::class, '123')->willReturn($visit);
+        $flush = $this->em->flush($visit)->will(function () {
+        });
+        $resolveIp = $this->ipLocationResolver->resolveIpLocation($ipAddr)->willReturn($location);
+
+        ($this->locateVisit)($event);
+
+        $this->assertEquals($visit->getVisitLocation(), new VisitLocation($location));
+        $findVisit->shouldHaveBeenCalledOnce();
+        $flush->shouldHaveBeenCalledOnce();
+        $resolveIp->shouldHaveBeenCalledOnce();
+        $this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled();
+    }
+}

module/Core/test/Service/VisitsTrackerTest.php

@@ -9,9 +9,11 @@ use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
+use Psr\EventDispatcher\EventDispatcherInterface;
 use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
+use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited;
 use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Model\VisitsParams;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
@@ -24,15 +26,19 @@ class VisitsTrackerTest extends TestCase
     private $visitsTracker;
     /** @var ObjectProphecy */
     private $em;
+    /** @var EventDispatcherInterface */
+    private $eventDispatcher;
 
     public function setUp(): void
     {
         $this->em = $this->prophesize(EntityManager::class);
-        $this->visitsTracker  = new VisitsTracker($this->em->reveal());
+        $this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class);
+
+        $this->visitsTracker  = new VisitsTracker($this->em->reveal(), $this->eventDispatcher->reveal());
     }
 
     /** @test */
-    public function trackPersistsVisit()
+    public function trackPersistsVisit(): void
     {
         $shortCode = '123ABC';
         $repo = $this->prophesize(EntityRepository::class);
@@ -40,13 +46,18 @@ class VisitsTrackerTest extends TestCase
 
         $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce();
         $this->em->persist(Argument::any())->shouldBeCalledOnce();
-        $this->em->flush(Argument::type(Visit::class))->shouldBeCalledOnce();
+        $this->em->flush(Argument::that(function (Visit $visit) {
+            $visit->setId('1');
+            return $visit;
+        }))->shouldBeCalledOnce();
 
         $this->visitsTracker->track($shortCode, Visitor::emptyInstance());
+
+        $this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled();
     }
 
     /** @test */
-    public function trackedIpAddressGetsObfuscated()
+    public function trackedIpAddressGetsObfuscated(): void
     {
         $shortCode = '123ABC';
         $repo = $this->prophesize(EntityRepository::class);
@@ -58,13 +69,18 @@ class VisitsTrackerTest extends TestCase
             $visit = $args[0];
             Assert::assertEquals('4.3.2.0', $visit->getRemoteAddr());
         })->shouldBeCalledOnce();
-        $this->em->flush(Argument::type(Visit::class))->shouldBeCalledOnce();
+        $this->em->flush(Argument::that(function (Visit $visit) {
+            $visit->setId('1');
+            return $visit;
+        }))->shouldBeCalledOnce();
 
         $this->visitsTracker->track($shortCode, new Visitor('', '', '4.3.2.1'));
+
+        $this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled();
     }
 
     /** @test */
-    public function infoReturnsVisistForCertainShortCode()
+    public function infoReturnsVisitsForCertainShortCode(): void
     {
         $shortCode = '123ABC';
         $repo = $this->prophesize(EntityRepository::class);