Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/geolocation middleware #2266

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion config/autoload/middleware-pipeline.global.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Shlinkio\Shlink\Common\Middleware\AccessLogMiddleware;
use Shlinkio\Shlink\Common\Middleware\ContentLengthMiddleware;
use Shlinkio\Shlink\Common\Middleware\RequestIdMiddleware;
use Shlinkio\Shlink\Core\Geolocation\Middleware\IpGeolocationMiddleware;

return [

Expand Down Expand Up @@ -67,8 +68,11 @@
],
'not-found' => [
'middleware' => [
// This middleware is in front of tracking actions explicitly. Putting here for orphan visits tracking
// These two middlewares are in front of other tracking actions.
// Putting them here for orphan visits tracking
IpAddress::class,
IpGeolocationMiddleware::class,

Core\ErrorHandler\NotFoundTypeResolverMiddleware::class,
Core\ShortUrl\Middleware\ExtraPathRedirectMiddleware::class,
Core\ErrorHandler\NotFoundTrackerMiddleware::class,
Expand Down
3 changes: 3 additions & 0 deletions config/autoload/routes.config.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use RKA\Middleware\IpAddress;
use Shlinkio\Shlink\Core\Action as CoreAction;
use Shlinkio\Shlink\Core\Config\EnvVars;
use Shlinkio\Shlink\Core\Geolocation\Middleware\IpGeolocationMiddleware;
use Shlinkio\Shlink\Core\ShortUrl\Middleware\TrimTrailingSlashMiddleware;
use Shlinkio\Shlink\Rest\Action;
use Shlinkio\Shlink\Rest\ConfigProvider;
Expand Down Expand Up @@ -88,6 +89,7 @@
'path' => '/{shortCode}/track',
'middleware' => [
IpAddress::class,
IpGeolocationMiddleware::class,
CoreAction\PixelAction::class,
],
'allowed_methods' => [RequestMethodInterface::METHOD_GET],
Expand All @@ -105,6 +107,7 @@
'path' => sprintf('/{shortCode}%s', $shortUrlRouteSuffix),
'middleware' => [
IpAddress::class,
IpGeolocationMiddleware::class,
TrimTrailingSlashMiddleware::class,
CoreAction\RedirectAction::class,
],
Expand Down
2 changes: 1 addition & 1 deletion module/CLI/src/GeoLite/GeolocationDbUpdater.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public function checkDbUpdate(
callable|null $beforeDownload = null,
callable|null $handleProgress = null,
): GeolocationResult {
if ($this->trackingOptions->disableTracking || $this->trackingOptions->disableIpTracking) {
if (! $this->trackingOptions->isGeolocationRelevant()) {
return GeolocationResult::CHECK_SKIPPED;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ protected function setUp(): void
public function outputIsProperlyGenerated(): void
{
$shortUrl = ShortUrl::createFake();
$visit = Visit::forValidShortUrl($shortUrl, new Visitor('bar', 'foo', '', ''))->locate(
$visit = Visit::forValidShortUrl($shortUrl, Visitor::fromParams('bar', 'foo', ''))->locate(
VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')),
);
$domain = 's.test';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public function providingInvalidDatesPrintsWarning(): void
#[Test]
public function outputIsProperlyGenerated(): void
{
$visit = Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('bar', 'foo', '', ''))->locate(
$visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::fromParams('bar', 'foo', ''))->locate(
VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')),
);
$shortCode = 'abc123';
Expand Down
2 changes: 1 addition & 1 deletion module/CLI/test/Command/Tag/GetTagVisitsCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ protected function setUp(): void
public function outputIsProperlyGenerated(): void
{
$shortUrl = ShortUrl::createFake();
$visit = Visit::forValidShortUrl($shortUrl, new Visitor('bar', 'foo', '', ''))->locate(
$visit = Visit::forValidShortUrl($shortUrl, Visitor::fromParams('bar', 'foo', ''))->locate(
VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')),
);
$tag = 'abc123';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ protected function setUp(): void
public function outputIsProperlyGenerated(): void
{
$shortUrl = ShortUrl::createFake();
$visit = Visit::forValidShortUrl($shortUrl, new Visitor('bar', 'foo', '', ''))->locate(
$visit = Visit::forValidShortUrl($shortUrl, Visitor::fromParams('bar', 'foo', ''))->locate(
VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')),
);
$this->visitsHelper->expects($this->once())->method('nonOrphanVisits')->withAnyParameters()->willReturn(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ protected function setUp(): void
#[TestWith([['--type' => OrphanVisitType::BASE_URL->value], true])]
public function outputIsProperlyGenerated(array $args, bool $includesType): void
{
$visit = Visit::forBasePath(new Visitor('bar', 'foo', '', ''))->locate(
$visit = Visit::forBasePath(Visitor::fromParams('bar', 'foo', ''))->locate(
VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')),
);
$this->visitsHelper->expects($this->once())->method('orphanVisits')->with($this->callback(
Expand Down
6 changes: 3 additions & 3 deletions module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ public function expectedSetOfVisitsIsProcessedBasedOnArgs(
bool $expectWarningPrint,
array $args,
): void {
$visit = Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', ''));
$location = VisitLocation::fromGeolocation(Location::emptyInstance());
$visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::fromParams('', '', '1.2.3.4'));
$location = VisitLocation::fromGeolocation(Location::empty());
$mockMethodBehavior = $this->invokeHelperMethods($visit, $location);

$this->lock->method('acquire')->with($this->isFalse())->willReturn(true);
Expand Down Expand Up @@ -134,7 +134,7 @@ public static function provideIgnoredAddresses(): iterable
#[Test]
public function errorWhileLocatingIpIsDisplayed(): void
{
$visit = Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', ''));
$visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::fromParams(remoteAddress: '1.2.3.4'));
$location = VisitLocation::fromGeolocation(Location::emptyInstance());

$this->lock->method('acquire')->with($this->isFalse())->willReturn(true);
Expand Down
10 changes: 10 additions & 0 deletions module/Core/config/dependencies.config.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Shlinkio\Shlink\Core\Config\Options\NotFoundRedirectOptions;
use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier;
use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface;
use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater;
use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
use Symfony\Component\Lock;

Expand Down Expand Up @@ -102,6 +103,8 @@

EventDispatcher\PublishingUpdatesGenerator::class => ConfigAbstractFactory::class,

Geolocation\Middleware\IpGeolocationMiddleware::class => ConfigAbstractFactory::class,

Importer\ImportedLinksProcessor::class => ConfigAbstractFactory::class,

Crawling\CrawlingHelper::class => ConfigAbstractFactory::class,
Expand Down Expand Up @@ -237,6 +240,13 @@

EventDispatcher\PublishingUpdatesGenerator::class => [ShortUrl\Transformer\ShortUrlDataTransformer::class],

Geolocation\Middleware\IpGeolocationMiddleware::class => [
IpLocationResolverInterface::class,
DbUpdater::class,
'Logger_Shlink',
Config\Options\TrackingOptions::class,
],

Importer\ImportedLinksProcessor::class => [
'em',
ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class,
Expand Down
19 changes: 3 additions & 16 deletions module/Core/config/event_dispatcher.config.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,18 @@
use Shlinkio\Shlink\Core\Visit\Geolocation\VisitLocator;
use Shlinkio\Shlink\Core\Visit\Geolocation\VisitToLocationHelper;
use Shlinkio\Shlink\EventDispatcher\Listener\EnabledListenerCheckerInterface;
use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater;
use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2Options;
use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;

use function Shlinkio\Shlink\Config\runningInRoadRunner;

return (static function (): array {
$regularEvents = [
EventDispatcher\Event\UrlVisited::class => [
EventDispatcher\LocateVisit::class,
],
EventDispatcher\Event\GeoLiteDbCreated::class => [
EventDispatcher\LocateUnlocatedVisits::class,
],
];
$asyncEvents = [
EventDispatcher\Event\VisitLocated::class => [
EventDispatcher\Event\UrlVisited::class => [
EventDispatcher\Mercure\NotifyVisitToMercure::class,
EventDispatcher\RabbitMq\NotifyVisitToRabbitMq::class,
EventDispatcher\RedisPubSub\NotifyVisitToRedis::class,
Expand All @@ -46,9 +41,9 @@

// Send visits to matomo asynchronously if the runtime allows it
if (runningInRoadRunner()) {
$asyncEvents[EventDispatcher\Event\VisitLocated::class][] = EventDispatcher\Matomo\SendVisitToMatomo::class;
$asyncEvents[EventDispatcher\Event\UrlVisited::class][] = EventDispatcher\Matomo\SendVisitToMatomo::class;
} else {
$regularEvents[EventDispatcher\Event\VisitLocated::class] = [EventDispatcher\Matomo\SendVisitToMatomo::class];
$regularEvents[EventDispatcher\Event\UrlVisited::class] = [EventDispatcher\Matomo\SendVisitToMatomo::class];
}

return [
Expand All @@ -60,7 +55,6 @@

'dependencies' => [
'factories' => [
EventDispatcher\LocateVisit::class => ConfigAbstractFactory::class,
EventDispatcher\Matomo\SendVisitToMatomo::class => ConfigAbstractFactory::class,
EventDispatcher\LocateUnlocatedVisits::class => ConfigAbstractFactory::class,
EventDispatcher\Mercure\NotifyVisitToMercure::class => ConfigAbstractFactory::class,
Expand Down Expand Up @@ -104,13 +98,6 @@
],

ConfigAbstractFactory::class => [
EventDispatcher\LocateVisit::class => [
IpLocationResolverInterface::class,
'em',
'Logger_Shlink',
DbUpdater::class,
EventDispatcherInterface::class,
],
EventDispatcher\LocateUnlocatedVisits::class => [VisitLocator::class, VisitToLocationHelper::class],
EventDispatcher\Mercure\NotifyVisitToMercure::class => [
MercureHubPublishingHelper::class,
Expand Down
11 changes: 11 additions & 0 deletions module/Core/functions/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory;
use Shlinkio\Shlink\Common\Util\DateRange;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode;
use Shlinkio\Shlink\IpGeolocation\Model\Location;

use function array_keys;
use function array_map;
Expand Down Expand Up @@ -289,3 +290,13 @@ function ipAddressFromRequest(ServerRequestInterface $request): string|null
{
return $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR);
}

function geolocationFromRequest(ServerRequestInterface $request): Location|null
{
$geolocation = $request->getAttribute(Location::class);
if ($geolocation !== null && ! $geolocation instanceof Location) {
// TODO Throw exception
}

return $geolocation;
}
8 changes: 2 additions & 6 deletions module/Core/src/Action/AbstractTrackingAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface;
use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface;
use Shlinkio\Shlink\IpGeolocation\Model\Location;

abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMethodInterface
{
Expand All @@ -31,12 +30,9 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface

try {
$shortUrl = $this->urlResolver->resolveEnabledShortUrl($identifier);
$visit = $this->requestTracker->trackIfApplicable($shortUrl, $request);
$this->requestTracker->trackIfApplicable($shortUrl, $request);

return $this->createSuccessResp(
$shortUrl,
$request->withAttribute(Location::class, $visit?->getVisitLocation()),
);
return $this->createSuccessResp($shortUrl, $request);
} catch (ShortUrlNotFoundException) {
return $this->createErrorResp($request, $handler);
}
Expand Down
8 changes: 8 additions & 0 deletions module/Core/src/Config/Options/TrackingOptions.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,12 @@ public function queryHasDisableTrackParam(array $query): bool
{
return $this->disableTrackParam !== null && array_key_exists($this->disableTrackParam, $query);
}

/**
* If IP address tracking is disabled, or tracking is disabled all together, then geolocation is not relevant
*/
public function isGeolocationRelevant(): bool
{
return ! $this->disableTracking && ! $this->disableIpTracking;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use Psr\Log\LoggerInterface;
use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface;
use Shlinkio\Shlink\Common\UpdatePublishing\Update;
use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated;
use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited;
use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface;
use Shlinkio\Shlink\Core\Visit\Entity\Visit;
use Throwable;
Expand All @@ -25,7 +25,7 @@ public function __construct(
) {
}

public function __invoke(VisitLocated $visitLocated): void
public function __invoke(UrlVisited $visitLocated): void
{
if (! $this->isEnabled()) {
return;
Expand Down
27 changes: 0 additions & 27 deletions module/Core/src/EventDispatcher/Event/AbstractVisitEvent.php

This file was deleted.

4 changes: 2 additions & 2 deletions module/Core/src/EventDispatcher/Event/ShortUrlCreated.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
use JsonSerializable;
use Shlinkio\Shlink\EventDispatcher\Util\JsonUnserializable;

final class ShortUrlCreated implements JsonSerializable, JsonUnserializable
final readonly class ShortUrlCreated implements JsonSerializable, JsonUnserializable
{
public function __construct(public readonly string $shortUrlId)
public function __construct(public string $shortUrlId)
{
}

Expand Down
20 changes: 19 additions & 1 deletion module/Core/src/EventDispatcher/Event/UrlVisited.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,24 @@

namespace Shlinkio\Shlink\Core\EventDispatcher\Event;

final class UrlVisited extends AbstractVisitEvent
use JsonSerializable;
use Shlinkio\Shlink\EventDispatcher\Util\JsonUnserializable;

final readonly class UrlVisited implements JsonSerializable, JsonUnserializable
{
final public function __construct(
public string $visitId,
public string|null $originalIpAddress = null,
) {
}

public function jsonSerialize(): array

Check warning on line 18 in module/Core/src/EventDispatcher/Event/UrlVisited.php

View check run for this annotation

Codecov / codecov/patch

module/Core/src/EventDispatcher/Event/UrlVisited.php#L18

Added line #L18 was not covered by tests
{
return ['visitId' => $this->visitId, 'originalIpAddress' => $this->originalIpAddress];

Check warning on line 20 in module/Core/src/EventDispatcher/Event/UrlVisited.php

View check run for this annotation

Codecov / codecov/patch

module/Core/src/EventDispatcher/Event/UrlVisited.php#L20

Added line #L20 was not covered by tests
}

public static function fromPayload(array $payload): self

Check warning on line 23 in module/Core/src/EventDispatcher/Event/UrlVisited.php

View check run for this annotation

Codecov / codecov/patch

module/Core/src/EventDispatcher/Event/UrlVisited.php#L23

Added line #L23 was not covered by tests
{
return new self($payload['visitId'] ?? '', $payload['originalIpAddress'] ?? null);

Check warning on line 25 in module/Core/src/EventDispatcher/Event/UrlVisited.php

View check run for this annotation

Codecov / codecov/patch

module/Core/src/EventDispatcher/Event/UrlVisited.php#L25

Added line #L25 was not covered by tests
}
}
9 changes: 0 additions & 9 deletions module/Core/src/EventDispatcher/Event/VisitLocated.php

This file was deleted.

Loading
Loading