diff --git a/composer.json b/composer.json index 8f15d83e..ba72716d 100644 --- a/composer.json +++ b/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", diff --git a/config/autoload/events.global.php b/config/autoload/events.global.php new file mode 100644 index 00000000..e2d3561e --- /dev/null +++ b/config/autoload/events.global.php @@ -0,0 +1,26 @@ + [ + '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], + ], + +]; diff --git a/config/config.php b/config/config.php index fcee89fb..4ad8e8bb 100644 --- a/config/config.php +++ b/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, diff --git a/module/Common/src/Entity/AbstractEntity.php b/module/Common/src/Entity/AbstractEntity.php index b798a087..9358d2c5 100644 --- a/module/Common/src/Entity/AbstractEntity.php +++ b/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; diff --git a/module/Common/src/EventDispatcher/ListenerProviderFactory.php b/module/Common/src/EventDispatcher/ListenerProviderFactory.php new file mode 100644 index 00000000..a69f019a --- /dev/null +++ b/module/Common/src/EventDispatcher/ListenerProviderFactory.php @@ -0,0 +1,28 @@ +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; + } +} diff --git a/module/Common/src/EventDispatcher/SwooleEventDispatcher.php b/module/Common/src/EventDispatcher/SwooleEventDispatcher.php new file mode 100644 index 00000000..9fada852 --- /dev/null +++ b/module/Common/src/EventDispatcher/SwooleEventDispatcher.php @@ -0,0 +1,43 @@ +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); + } +} diff --git a/module/Common/test/EventDispatcher/ListenerProviderFactoryTest.php b/module/Common/test/EventDispatcher/ListenerProviderFactoryTest.php new file mode 100644 index 00000000..9834c4cd --- /dev/null +++ b/module/Common/test/EventDispatcher/ListenerProviderFactoryTest.php @@ -0,0 +1,99 @@ +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); + } +} diff --git a/module/Common/test/EventDispatcher/SwooleEventDispatcherTest.php b/module/Common/test/EventDispatcher/SwooleEventDispatcherTest.php new file mode 100644 index 00000000..50bc8a26 --- /dev/null +++ b/module/Common/test/EventDispatcher/SwooleEventDispatcherTest.php @@ -0,0 +1,41 @@ +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]; + } +} diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 960b74ac..28ea8a56 100644 --- a/module/Core/config/dependencies.config.php +++ b/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'], diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php new file mode 100644 index 00000000..fb9be0c4 --- /dev/null +++ b/module/Core/config/event_dispatcher.config.php @@ -0,0 +1,25 @@ + [ + EventDispatcher\ShortUrlVisited::class => [ + EventDispatcher\LocateShortUrlVisit::class, + ], + ], + + 'dependencies' => [ + EventDispatcher\LocateShortUrlVisit::class => ConfigAbstractFactory::class, + ], + + ConfigAbstractFactory::class => [ + EventDispatcher\LocateShortUrlVisit::class => [IpLocationResolverInterface::class, 'em', 'Logger_Shlink'], + ], + +]; diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index 8910ae1d..58401088 100644 --- a/module/Core/src/Entity/Visit.php +++ b/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; diff --git a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php new file mode 100644 index 00000000..defa0bf8 --- /dev/null +++ b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php @@ -0,0 +1,52 @@ +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); + } +} diff --git a/module/Core/src/EventDispatcher/ShortUrlVisited.php b/module/Core/src/EventDispatcher/ShortUrlVisited.php new file mode 100644 index 00000000..7874bbc6 --- /dev/null +++ b/module/Core/src/EventDispatcher/ShortUrlVisited.php @@ -0,0 +1,20 @@ +visitId = $visitId; + } + + public function visitId(): string + { + return $this->visitId; + } +} diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index 6b5b9873..e5723850 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/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())); } /** diff --git a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php new file mode 100644 index 00000000..7dfaacf0 --- /dev/null +++ b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php @@ -0,0 +1,111 @@ +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(); + } +} diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index c76bfc1f..7caef547 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/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);